mirror of
https://github.com/trailofbits/buttercup
synced 2026-06-21 14:11:39 +00:00
feat: filter harness files from patches to prevent modifying test infrastructure
Add harness file detection and filtering to ensure patches only modify actual source code, not fuzzing harnesses or test infrastructure. - Add is_harness_file_path() to detect harness/fuzzing files by path patterns - Add filter_harness_files_from_diff() to remove harness changes from patches - Update apply_patch_diff() to automatically filter out harness file changes - Set can_patch=False for code snippets in harness files - Reject patch attempts targeting harness files in SWE agent
This commit is contained in:
@@ -27,6 +27,88 @@ from buttercup.common.utils import copyanything, get_diffs
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Patterns that indicate a file is a harness/fuzzing file (should not be patched)
|
||||
HARNESS_PATH_PATTERNS = (
|
||||
"/fuzz/",
|
||||
"/fuzzer/",
|
||||
"/fuzzing/",
|
||||
"/fuzzers/",
|
||||
"Fuzz",
|
||||
"_fuzz",
|
||||
"_fuzzer",
|
||||
"harness",
|
||||
)
|
||||
|
||||
|
||||
def is_harness_file_path(file_path: str | Path) -> bool:
|
||||
"""Check if a file path appears to be a harness/fuzzing file.
|
||||
|
||||
Harness files should never be patched - they are test infrastructure,
|
||||
not the actual project code that contains vulnerabilities.
|
||||
|
||||
Args:
|
||||
file_path: The file path to check (can be absolute or relative)
|
||||
|
||||
Returns:
|
||||
True if the file appears to be a harness file, False otherwise.
|
||||
"""
|
||||
path_str = str(file_path)
|
||||
|
||||
# Check for common harness path patterns
|
||||
for pattern in HARNESS_PATH_PATTERNS:
|
||||
if pattern in path_str:
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
|
||||
def filter_harness_files_from_diff(diff_content: str) -> tuple[str, list[str]]:
|
||||
"""Filter out harness file changes from a unified diff.
|
||||
|
||||
Args:
|
||||
diff_content: The full content of a unified diff file.
|
||||
|
||||
Returns:
|
||||
A tuple of (filtered_diff, list_of_removed_harness_files).
|
||||
The filtered_diff has harness file hunks removed.
|
||||
list_of_removed_harness_files contains the paths of files that were filtered out.
|
||||
"""
|
||||
removed_files: list[str] = []
|
||||
filtered_hunks: list[str] = []
|
||||
current_hunk: list[str] = []
|
||||
current_file: str | None = None
|
||||
is_harness = False
|
||||
|
||||
for line in diff_content.splitlines(keepends=True):
|
||||
# Detect start of a new file's diff
|
||||
if line.startswith("diff --git "):
|
||||
# Save the previous hunk if it's not a harness file
|
||||
if current_hunk and not is_harness:
|
||||
filtered_hunks.extend(current_hunk)
|
||||
elif current_file and is_harness:
|
||||
removed_files.append(current_file)
|
||||
|
||||
# Start a new hunk
|
||||
current_hunk = [line]
|
||||
# Extract file path from "diff --git a/path b/path"
|
||||
match = re.search(r"diff --git a/(.*?) b/", line)
|
||||
if match:
|
||||
current_file = match.group(1)
|
||||
is_harness = is_harness_file_path(current_file)
|
||||
else:
|
||||
current_file = None
|
||||
is_harness = False
|
||||
else:
|
||||
current_hunk.append(line)
|
||||
|
||||
# Don't forget the last hunk
|
||||
if current_hunk and not is_harness:
|
||||
filtered_hunks.extend(current_hunk)
|
||||
elif current_file and is_harness:
|
||||
removed_files.append(current_file)
|
||||
|
||||
return "".join(filtered_hunks), removed_files
|
||||
|
||||
|
||||
@contextmanager
|
||||
def create_tmp_dir(
|
||||
@@ -374,6 +456,71 @@ class ChallengeTask:
|
||||
"""
|
||||
return len(self.get_external_harness_sources()) > 0
|
||||
|
||||
def copy_external_harnesses(self) -> bool:
|
||||
"""Copy external harness sources from OSS-Fuzz project to source directory.
|
||||
|
||||
For public OSS-Fuzz projects, harnesses live in projects/<project>/<dir>/
|
||||
in the fuzz-tooling repository. These need to be copied to the source
|
||||
directory so they're available when the source is mounted during build.
|
||||
|
||||
Returns:
|
||||
True if copying succeeded or no external harnesses detected,
|
||||
False if copying failed.
|
||||
"""
|
||||
external_sources = self.task_meta.metadata.get("external_harness_sources", [])
|
||||
logger.debug(f"[task {self.task_meta.task_id}] Metadata: {self.task_meta.metadata}")
|
||||
|
||||
if not external_sources:
|
||||
logger.debug(f"[task {self.task_meta.task_id}] No external harness sources in metadata")
|
||||
return True
|
||||
|
||||
logger.info(f"[task {self.task_meta.task_id}] Copying external harness sources: {external_sources}")
|
||||
|
||||
try:
|
||||
oss_fuzz_path = self.get_oss_fuzz_path()
|
||||
source_path = self.get_source_path()
|
||||
project_name = self.project_name
|
||||
|
||||
logger.debug(f"[task {self.task_meta.task_id}] OSS-Fuzz path: {oss_fuzz_path}")
|
||||
logger.debug(f"[task {self.task_meta.task_id}] Source path: {source_path}")
|
||||
logger.debug(f"[task {self.task_meta.task_id}] Project name: {project_name}")
|
||||
|
||||
for src_dir, dest_dir in external_sources:
|
||||
# Source: fuzz-tooling/<oss-fuzz>/projects/<project>/<src_dir>/
|
||||
harness_src = oss_fuzz_path / "projects" / project_name / src_dir
|
||||
|
||||
if not harness_src.exists():
|
||||
logger.warning(f"[task {self.task_meta.task_id}] External harness source not found: {harness_src}")
|
||||
continue
|
||||
|
||||
# Destination: src/<focus>/<dest_dir>/
|
||||
harness_dst = source_path / dest_dir
|
||||
|
||||
logger.info(f"[task {self.task_meta.task_id}] Copying {harness_src} -> {harness_dst}")
|
||||
|
||||
# Remove existing directory if present (will be replaced)
|
||||
if harness_dst.exists():
|
||||
logger.debug(f"[task {self.task_meta.task_id}] Removing existing {harness_dst}")
|
||||
shutil.rmtree(harness_dst)
|
||||
|
||||
# Copy harnesses
|
||||
shutil.copytree(harness_src, harness_dst)
|
||||
|
||||
# Verify copy
|
||||
if harness_dst.exists():
|
||||
files = list(harness_dst.iterdir())
|
||||
logger.info(
|
||||
f"[task {self.task_meta.task_id}] Successfully copied {len(files)} files to {harness_dst}"
|
||||
)
|
||||
else:
|
||||
logger.error(f"[task {self.task_meta.task_id}] Copy failed - destination doesn't exist!")
|
||||
return False
|
||||
|
||||
return True
|
||||
except Exception as e:
|
||||
logger.exception(f"[task {self.task_meta.task_id}] Failed to copy external harnesses: {e}")
|
||||
return False
|
||||
|
||||
@property
|
||||
def task_dir(self) -> Path:
|
||||
if self.local_task_dir is None:
|
||||
@@ -890,7 +1037,11 @@ class ChallengeTask:
|
||||
|
||||
@read_write_decorator
|
||||
def apply_patch_diff(self, diff_file: Path | None = None) -> bool:
|
||||
"""Apply the patch diff to the source code."""
|
||||
"""Apply the patch diff to the source code.
|
||||
|
||||
Note: Harness/fuzzing files are automatically filtered out from patches.
|
||||
These files should never be patched - only the actual source code should be modified.
|
||||
"""
|
||||
try:
|
||||
if diff_file is None:
|
||||
# Find all .patch and .diff files in the directory
|
||||
@@ -906,20 +1057,55 @@ class ChallengeTask:
|
||||
|
||||
logger.info(f"[task {self.task_dir}] Applying diff file: {diff_file}")
|
||||
|
||||
# Use git apply command to apply the patch
|
||||
subprocess.run(
|
||||
[
|
||||
"git",
|
||||
"-C",
|
||||
str(self.get_source_path()),
|
||||
"apply",
|
||||
str(diff_file),
|
||||
],
|
||||
text=True,
|
||||
check=True,
|
||||
timeout=10,
|
||||
capture_output=True,
|
||||
)
|
||||
# Read the diff content and filter out harness files
|
||||
diff_content = diff_file.read_text()
|
||||
filtered_diff, removed_files = filter_harness_files_from_diff(diff_content)
|
||||
|
||||
if removed_files:
|
||||
logger.warning(f"[task {self.task_dir}] Filtered out harness files from patch: {removed_files}")
|
||||
|
||||
if not filtered_diff.strip():
|
||||
logger.warning(
|
||||
f"[task {self.task_dir}] Patch only contained harness file changes, nothing to apply"
|
||||
)
|
||||
continue
|
||||
|
||||
# Write the filtered diff to a temporary file if we removed any harness files
|
||||
if removed_files:
|
||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".patch", delete=False) as f:
|
||||
f.write(filtered_diff)
|
||||
filtered_diff_path = Path(f.name)
|
||||
try:
|
||||
subprocess.run(
|
||||
[
|
||||
"git",
|
||||
"-C",
|
||||
str(self.get_source_path()),
|
||||
"apply",
|
||||
str(filtered_diff_path),
|
||||
],
|
||||
text=True,
|
||||
check=True,
|
||||
timeout=10,
|
||||
capture_output=True,
|
||||
)
|
||||
finally:
|
||||
filtered_diff_path.unlink(missing_ok=True)
|
||||
else:
|
||||
# No harness files to filter, apply original diff
|
||||
subprocess.run(
|
||||
[
|
||||
"git",
|
||||
"-C",
|
||||
str(self.get_source_path()),
|
||||
"apply",
|
||||
str(diff_file),
|
||||
],
|
||||
text=True,
|
||||
check=True,
|
||||
timeout=10,
|
||||
capture_output=True,
|
||||
)
|
||||
|
||||
logger.info(f"[task {self.task_dir}] Successfully applied patch {diff_file}")
|
||||
|
||||
|
||||
@@ -59,7 +59,13 @@ from buttercup.patcher.agents.tools import (
|
||||
grep,
|
||||
ls,
|
||||
)
|
||||
from buttercup.patcher.utils import TruncatePosition, find_file_in_source_dir, get_challenge, truncate_output
|
||||
from buttercup.patcher.utils import (
|
||||
TruncatePosition,
|
||||
find_file_in_source_dir,
|
||||
get_challenge,
|
||||
is_harness_file_path,
|
||||
truncate_output,
|
||||
)
|
||||
|
||||
# ruff: noqa: E501
|
||||
|
||||
@@ -509,7 +515,7 @@ def track_snippet(
|
||||
end_line=end_line,
|
||||
code="\n".join(get_lines_output),
|
||||
description=code_snippet_description,
|
||||
can_patch=find_file_in_source_dir(challenge, path) is not None,
|
||||
can_patch=find_file_in_source_dir(challenge, path) is not None and not is_harness_file_path(path),
|
||||
)
|
||||
code_snippets = [code_snippet]
|
||||
|
||||
|
||||
@@ -40,7 +40,7 @@ from buttercup.patcher.agents.common import (
|
||||
PatchStatus,
|
||||
PatchStrategy,
|
||||
)
|
||||
from buttercup.patcher.utils import PatchOutput, find_file_in_source_dir, pick_temperature
|
||||
from buttercup.patcher.utils import PatchOutput, find_file_in_source_dir, is_harness_file_path, pick_temperature
|
||||
|
||||
# ruff: noqa: E501
|
||||
|
||||
@@ -481,6 +481,15 @@ class SWEAgent(PatcherAgentBase):
|
||||
logger.warning("Invalid code snippet: %s (%d)", code_snippet.key, idx)
|
||||
return None
|
||||
|
||||
# Reject patches for harness/fuzzing files - these should never be patched
|
||||
if code_snippet.key.file_path and is_harness_file_path(code_snippet.key.file_path):
|
||||
logger.warning(
|
||||
"Rejecting patch for harness file: %s (%d) - harness files should not be patched",
|
||||
code_snippet.key.file_path,
|
||||
idx,
|
||||
)
|
||||
return None
|
||||
|
||||
code_snippet_key = self._get_code_snippet_key(code_snippet, orig_code_snippets)
|
||||
if not code_snippet_key:
|
||||
logger.warning("Could not find a valid code snippet key for %s (%d)", code_snippet.key, idx)
|
||||
|
||||
@@ -11,7 +11,13 @@ from langgraph.prebuilt import InjectedState
|
||||
|
||||
from buttercup.common.challenge_task import ChallengeTask, CommandResult
|
||||
from buttercup.patcher.agents.common import BaseCtxState, CodeSnippetKey, ContextCodeSnippet
|
||||
from buttercup.patcher.utils import find_file_in_source_dir, get_challenge, get_codequery, truncate_output
|
||||
from buttercup.patcher.utils import (
|
||||
find_file_in_source_dir,
|
||||
get_challenge,
|
||||
get_codequery,
|
||||
is_harness_file_path,
|
||||
truncate_output,
|
||||
)
|
||||
from buttercup.program_model.codequery import CodeQueryPersistent
|
||||
from buttercup.program_model.utils.common import Function, TypeDefinition
|
||||
|
||||
@@ -128,7 +134,8 @@ def _add_functions_code_snippets(
|
||||
end_line=body.end_line,
|
||||
code=body.body,
|
||||
description=f"Implementation of function {function.name}{suffix} in {function.file_path.as_posix()}",
|
||||
can_patch=find_file_in_source_dir(challenge, function.file_path) is not None,
|
||||
can_patch=find_file_in_source_dir(challenge, function.file_path) is not None
|
||||
and not is_harness_file_path(function.file_path),
|
||||
)
|
||||
for function in functions
|
||||
for body in function.bodies
|
||||
@@ -148,7 +155,8 @@ def _add_type_definitions_code_snippets(
|
||||
start_line=type_def.definition_line,
|
||||
end_line=type_def.definition_line + len(type_def.definition.splitlines()),
|
||||
description=f"Definition of type {type_def.name}",
|
||||
can_patch=find_file_in_source_dir(challenge, type_def.file_path) is not None,
|
||||
can_patch=find_file_in_source_dir(challenge, type_def.file_path) is not None
|
||||
and not is_harness_file_path(type_def.file_path),
|
||||
)
|
||||
for type_def in type_definitions
|
||||
]
|
||||
|
||||
@@ -18,9 +18,43 @@ from buttercup.program_model.codequery import CodeQueryPersistent
|
||||
|
||||
VALID_PATCH_EXTENSIONS = (".c", ".h", ".in", ".java")
|
||||
|
||||
# Patterns that indicate a file is a harness/fuzzing file (should not be patched)
|
||||
HARNESS_PATH_PATTERNS = (
|
||||
"/fuzz/",
|
||||
"/fuzzer/",
|
||||
"/fuzzing/",
|
||||
"/fuzzers/",
|
||||
"Fuzz",
|
||||
"_fuzz",
|
||||
"_fuzzer",
|
||||
"harness",
|
||||
)
|
||||
|
||||
CHAIN_CALL_TYPE = Callable[[Callable, Runnable, dict[str, Any], RunnableConfig | None, Any], Any]
|
||||
|
||||
|
||||
def is_harness_file_path(file_path: str | Path) -> bool:
|
||||
"""Check if a file path appears to be a harness/fuzzing file.
|
||||
|
||||
Harness files should never be patched - they are test infrastructure,
|
||||
not the actual project code that contains vulnerabilities.
|
||||
|
||||
Args:
|
||||
file_path: The file path to check (can be absolute or relative)
|
||||
|
||||
Returns:
|
||||
True if the file appears to be a harness file, False otherwise.
|
||||
"""
|
||||
path_str = str(file_path)
|
||||
|
||||
# Check for common harness path patterns
|
||||
for pattern in HARNESS_PATH_PATTERNS:
|
||||
if pattern in path_str:
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
|
||||
class PatchInputPoV(BaseModel):
|
||||
challenge_task_dir: Path
|
||||
sanitizer: str
|
||||
|
||||
Reference in New Issue
Block a user