mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(learning-harness): a preprocessor directive is code, not a provenance header
Review of task 1.3 found the twin detector stripping every leading comment line, which in C and C++ swallows include and define directives, and in any language swallows an ordinary leading comment. Two files whose only difference was a buffer size on a define hashed equal and were marked unscorable, which removed a real pair from every denominator: the exact dishonesty this spec exists to end, running backwards. Measured before the fix: twelve file sides across six vendored pairs had directives inside the stripped prefix, including the Juliet stack-overflow pair whose whole point is the buffer size. The strip is now anchored to the grammar the harvester writes: a leading comment line counts as header only when it carries one of the provenance keys followed by a colon, and the scan stops at the first line that is code. Block and hash comment forms both work. Three regression tests pin the boundary in both directions, and a fourth asserts no vendored excerpt loses a directive. The five identical twins, the ten duplicate groups and the nineteen straddling repositories are unchanged, so the committed hygiene artifact still holds. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
co-authored by
Claude Opus 5
parent
6d98e47e42
commit
d05c516a10
@@ -502,6 +502,40 @@ class SplitReport:
|
||||
undated: int = 0 # assigned rows with no fix date, ordered by name instead
|
||||
|
||||
|
||||
# The keys the harvester writes into an excerpt's provenance header (benchmarks/pairs/harvest.py).
|
||||
# Only a leading comment line carrying one of these keys is metadata; anything else is code. A C
|
||||
# preprocessor directive and an ordinary leading comment are code, and stripping them would let two
|
||||
# different files hash equal and vanish from every denominator.
|
||||
_PROVENANCE_KEYS = frozenset(
|
||||
{"provenance", "repo", "commit", "parent", "commit_url", "cve", "license", "function", "relpath", "mechanism", "upstream_start"}
|
||||
)
|
||||
_COMMENT_MARKERS = ("#", "//", "*/", "/*", "*")
|
||||
|
||||
|
||||
def _is_header_line(line: str) -> bool:
|
||||
body = line.strip()
|
||||
if not body:
|
||||
return True
|
||||
for marker in _COMMENT_MARKERS:
|
||||
if body.startswith(marker):
|
||||
body = body[len(marker) :].strip()
|
||||
break
|
||||
else:
|
||||
return False
|
||||
if not body:
|
||||
return True # the framing line of a block comment
|
||||
key, separator, _rest = body.partition(":")
|
||||
return bool(separator) and key.strip().lower() in _PROVENANCE_KEYS
|
||||
|
||||
|
||||
def _header_lines(lines: Sequence[str]) -> int:
|
||||
"""How many leading lines are the provenance header. Stops at the first line that is code."""
|
||||
index = 0
|
||||
while index < len(lines) and _is_header_line(lines[index]):
|
||||
index += 1
|
||||
return index
|
||||
|
||||
|
||||
@lru_cache(maxsize=4096)
|
||||
def _body_digest(path: Path) -> str | None:
|
||||
"""Hash of an excerpt with its provenance header and trailing whitespace removed; None when unreadable or empty."""
|
||||
@@ -510,10 +544,7 @@ def _body_digest(path: Path) -> str | None:
|
||||
except OSError:
|
||||
return None
|
||||
lines = text.splitlines()
|
||||
index = 0
|
||||
while index < len(lines) and (not lines[index].strip() or lines[index].lstrip().startswith(("#", "//", "/*", "*"))):
|
||||
index += 1
|
||||
body = "\n".join(line.rstrip() for line in lines[index:]).strip()
|
||||
body = "\n".join(line.rstrip() for line in lines[_header_lines(lines) :]).strip()
|
||||
return hashlib.sha256(body.encode()).hexdigest() if body else None
|
||||
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
from openultrasast.pairs import DEFAULT_CATALOG, PairCase, load_pair_catalog, select_vendored
|
||||
@@ -147,3 +148,68 @@ def test_pair_signals_can_be_filtered_to_one_split() -> None:
|
||||
outcomes = [outcome("train-one", "train"), outcome("holdout-one", "holdout")]
|
||||
assert {signal["pair"] for signal in build_pair_signals(outcomes)} == {"train-one", "holdout-one"}
|
||||
assert {signal["pair"] for signal in build_pair_signals(outcomes, split="train")} == {"train-one"}
|
||||
|
||||
|
||||
HEADER_C = "/* Provenance: acme/lib bad ({side}).\n * repo: acme/lib\n * commit: abc\n * license: MIT\n */\n"
|
||||
HEADER_PY = "# Provenance: acme/lib f ({side}).\n# repo: acme/lib\n# license: MIT\n"
|
||||
|
||||
|
||||
def _twin(tmp_path: Path, suffix: str, vuln: str, fixed: str) -> object:
|
||||
from openultrasast.pairs import _load_catalog_file
|
||||
|
||||
(tmp_path / f"v{suffix}").write_text(vuln)
|
||||
(tmp_path / f"f{suffix}").write_text(fixed)
|
||||
(tmp_path / "c.toml").write_text(
|
||||
f'[[pair]]\nname = "t"\nslice = "sast"\nvuln = "v{suffix}"\nfixed = "f{suffix}"\nrelpath = "app{suffix}"\n\n'
|
||||
'[[pair.expected]]\ncwe = "CWE-121"\nclass = "x"\npath = "app.c"\nsink = "strcpy"\nmechanism = "other"\n'
|
||||
)
|
||||
(case,) = _load_catalog_file(tmp_path / "c.toml")
|
||||
return case
|
||||
|
||||
|
||||
def test_a_preprocessor_directive_is_code_not_a_provenance_header(tmp_path: Path) -> None:
|
||||
"""The whole point of the CWE-121 pairs is the buffer size, and it lives on a `#define`."""
|
||||
body = "#include <string.h>\n#define BUFSIZE {size}\n\nvoid bad(void) {{\n char buf[BUFSIZE];\n strcpy(buf, src);\n}}\n"
|
||||
case = _twin(
|
||||
tmp_path,
|
||||
".c",
|
||||
HEADER_C.format(side="vuln") + body.format(size="10"),
|
||||
HEADER_C.format(side="fixed") + body.format(size="4096"),
|
||||
)
|
||||
assert case.unscorable is None # type: ignore[attr-defined]
|
||||
|
||||
|
||||
def test_a_leading_code_comment_is_not_a_provenance_header(tmp_path: Path) -> None:
|
||||
same = "def f(value):\n return value\n"
|
||||
case = _twin(
|
||||
tmp_path,
|
||||
".py",
|
||||
HEADER_PY.format(side="vuln") + "# this function is fine\n" + same,
|
||||
HEADER_PY.format(side="fixed") + "# this function is not fine\n" + same,
|
||||
)
|
||||
assert case.unscorable is None # a comment that is not provenance metadata is part of the file
|
||||
|
||||
|
||||
def test_the_provenance_header_itself_is_still_stripped(tmp_path: Path) -> None:
|
||||
same = "def f(value):\n return value\n"
|
||||
case = _twin(tmp_path, ".py", HEADER_PY.format(side="vuln") + same, HEADER_PY.format(side="fixed") + same)
|
||||
assert case.unscorable == "identical_twin" # only the header differs, so the code is the same code
|
||||
block = _twin(
|
||||
tmp_path, ".c", HEADER_C.format(side="vuln") + "void bad(void) {}\n", HEADER_C.format(side="fixed") + "void bad(void) {}\n"
|
||||
)
|
||||
assert block.unscorable == "identical_twin" # the C block comment form too
|
||||
|
||||
|
||||
def test_no_vendored_excerpt_loses_a_preprocessor_directive_to_the_header_strip() -> None:
|
||||
from openultrasast.pairs import _header_lines
|
||||
|
||||
eaten: list[str] = []
|
||||
for case in select_vendored(load_pair_catalog(DEFAULT_CATALOG)):
|
||||
for path in (case.vuln_file, case.fixed_file):
|
||||
if not path.is_file():
|
||||
continue
|
||||
lines = path.read_text(errors="ignore").splitlines()
|
||||
stripped = lines[: _header_lines(lines)]
|
||||
if any(re.match(r"\s*#\s*(include|define|if|ifdef|ifndef|elif|else|endif|pragma|undef|import)\b", line) for line in stripped):
|
||||
eaten.append(f"{case.name}:{path.name}")
|
||||
assert eaten == []
|
||||
|
||||
Reference in New Issue
Block a user