mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 19:59:26 +00:00
feat(learning-harness): absence rows travel with their registration
Eight of the ten vendored access-control rows had no named handler at all, so every one of them would have scored `missing_context` forever rather than as a detector miss. After the re-harvest nine are named and the tenth states its reason. The handler-context mode now carries the class its framework registers, multi-line decorators, and commented-out guards -- `#@token_required` above a ThreatByte handler is the entire bug. Where the registration lives in another document (connexion's `operationId`, an express router), a recipe names `context = [...]`, the harvester writes that stanza per side, and the pair materializes both files at their repository-relative paths so the entry-point mapper sees the layout the project has. `harvest_recipe` fetches every blob before the first write and the driver takes `--report`, so a failed fetch leaves a reviewed excerpt exactly as it was. Two defects the re-harvest exposed: `.mjs` was not a known suffix, so the one pair vendored as an ES module had no file target at all; and `_python_route_access` matched auth words inside decorator prose, reading `@ns.doc(description='... without proper authorization checks')` as a guard. Recorded in the hygiene artifact: nine of the ten duplicate groups sit on both sides of the split, which the repository-level split cannot see. 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
7a33756a7e
commit
c2d9c4ad01
@@ -125,7 +125,7 @@ def score_case(case: PairCase, scan: Scan, *, taxonomy: FamilyTaxonomy, runs: in
|
||||
A pair the corpus already marked unscorable is never run: spending model calls on a row that cannot
|
||||
be scored buys nothing and would put its noise into a floor it does not belong in.
|
||||
"""
|
||||
from ..pairs import _materialize, _overlay_scan
|
||||
from ..pairs import _materialize_side, _overlay_scan
|
||||
|
||||
name = family or _family_of(case, taxonomy)
|
||||
if case.unscorable:
|
||||
@@ -138,8 +138,8 @@ def score_case(case: PairCase, scan: Scan, *, taxonomy: FamilyTaxonomy, runs: in
|
||||
unscorable_reason=case.unscorable,
|
||||
)
|
||||
with tempfile.TemporaryDirectory(prefix="ousast-round-") as scratch:
|
||||
vuln_root = _materialize(Path(scratch) / "vuln", case.vuln_file, case.relpath)
|
||||
fix_root = _materialize(Path(scratch) / "fixed", case.fixed_file, case.relpath)
|
||||
vuln_root = _materialize_side(Path(scratch) / "vuln", case, side="vuln")
|
||||
fix_root = _materialize_side(Path(scratch) / "fixed", case, side="fixed")
|
||||
ranges = _overlay_scan(vuln_root).ranges
|
||||
vuln_runs = [scan(vuln_root) for _ in range(max(runs, 1))]
|
||||
fix_runs = [scan(fix_root) for _ in range(max(runs, 1))]
|
||||
|
||||
@@ -105,6 +105,7 @@ def analyze_entry_points(root: Path, targets: list[FileTarget]) -> list[EntryPoi
|
||||
text = path.read_text(errors="ignore")
|
||||
records.extend(_language_entry_points(target, text))
|
||||
records.extend(_tag_entry_points(target))
|
||||
records.extend(_route_manifest_entry_points(root, targets))
|
||||
return sorted(records, key=lambda item: (item.path, item.line is None, item.line or 0, item.name))
|
||||
|
||||
|
||||
@@ -249,9 +250,8 @@ def _js_entry_points(target: FileTarget, text: str) -> list[EntryPointRecord]:
|
||||
route_tokens = (".get(", ".post(", ".put(", ".delete(", ".patch(", ".use(")
|
||||
lines = text.splitlines()
|
||||
bounds = _js_function_bounds(lines)
|
||||
for number, line in enumerate(lines, start=1):
|
||||
stripped = line.strip()
|
||||
if any(token in stripped for token in route_tokens) and any(prefix in stripped for prefix in ("app", "router", "server")):
|
||||
for number, stripped in _js_statements(lines):
|
||||
if any(token in stripped for token in route_tokens) and any(prefix in stripped.lower() for prefix in ("app", "router", "server")):
|
||||
handler, middleware = _js_registration(stripped, bounds)
|
||||
access, evidence = _js_route_access(middleware, stripped)
|
||||
start, end = bounds.get(handler or "", (number, number))
|
||||
@@ -288,6 +288,89 @@ def _js_entry_points(target: FileTarget, text: str) -> list[EntryPointRecord]:
|
||||
return records
|
||||
|
||||
|
||||
_OPERATION_ID = re.compile(r"""^\s*(?:-\s*)?["']?operationId["']?\s*:\s*["']?([\w.]+)["']?,?\s*$""")
|
||||
_MANIFEST_SUFFIXES = (".yml", ".yaml", ".json")
|
||||
_MANIFEST_MAX_BYTES = 2_000_000
|
||||
|
||||
|
||||
def _route_manifest_entry_points(root: Path, targets: Sequence[FileTarget]) -> list[EntryPointRecord]:
|
||||
"""Handlers a *document* registers: an OpenAPI `operationId` names the function connexion, FastAPI-from-spec or an
|
||||
API gateway will call, and the `security` block beside it is the guard.
|
||||
|
||||
Frameworks that keep the route table out of the handler file leave every handler looking like a plain function, so
|
||||
an absence bug on one is unreachable and unjudgeable. The document itself is not a source file — it never becomes a
|
||||
file target — so the record is attributed to the module the operation id points at when that module is present
|
||||
(learning-harness Req 10.2)."""
|
||||
modules = {target.path for target in targets}
|
||||
records: list[EntryPointRecord] = []
|
||||
for path in sorted(root.rglob("*")):
|
||||
if not path.is_file() or path.suffix.lower() not in _MANIFEST_SUFFIXES:
|
||||
continue
|
||||
try:
|
||||
if path.stat().st_size > _MANIFEST_MAX_BYTES:
|
||||
continue
|
||||
lines = path.read_text(errors="ignore").splitlines()
|
||||
except OSError:
|
||||
continue
|
||||
relative = path.relative_to(root).as_posix()
|
||||
for index, line in enumerate(lines):
|
||||
match = _OPERATION_ID.match(line)
|
||||
if match is None:
|
||||
continue
|
||||
operation = match.group(1)
|
||||
handler = operation.rsplit(".", 1)[-1]
|
||||
owner = _operation_module(operation, modules)
|
||||
access, evidence = _manifest_access(lines, index)
|
||||
records.append(
|
||||
EntryPointRecord(
|
||||
path=owner or relative,
|
||||
line=None if owner else index + 1,
|
||||
end_line=None if owner else index + 1,
|
||||
function_name=handler,
|
||||
name=operation,
|
||||
kind="route",
|
||||
access_level=access,
|
||||
trust_boundary="http_request",
|
||||
access_evidence=evidence,
|
||||
conditions=[],
|
||||
provenance=f"entrypoint:route:{access}",
|
||||
rationale=f"registered by operationId {operation} in {relative}",
|
||||
)
|
||||
)
|
||||
return records
|
||||
|
||||
|
||||
def _operation_module(operation: str, modules: set[str]) -> str | None:
|
||||
"""`api_views.books.get_by_title` -> `api_views/books.py` when that file is in the snapshot."""
|
||||
parts = operation.split(".")
|
||||
if len(parts) < 2:
|
||||
return None
|
||||
for depth in range(len(parts) - 1, 0, -1):
|
||||
candidate = "/".join(parts[:depth]) + ".py"
|
||||
if candidate in modules:
|
||||
return candidate
|
||||
return None
|
||||
|
||||
|
||||
def _manifest_access(lines: Sequence[str], hit: int) -> tuple[AccessLevel, list[str]]:
|
||||
"""The operation's own `security` block decides. `security: []` is an explicit opt-out and stays public."""
|
||||
indent = len(lines[hit]) - len(lines[hit].lstrip())
|
||||
start = hit
|
||||
while start > 0 and (not lines[start - 1].strip() or len(lines[start - 1]) - len(lines[start - 1].lstrip()) >= indent):
|
||||
start -= 1
|
||||
end = hit + 1
|
||||
while end < len(lines) and (not lines[end].strip() or len(lines[end]) - len(lines[end].lstrip()) >= indent):
|
||||
end += 1
|
||||
for index in range(start, end):
|
||||
stripped = lines[index].strip()
|
||||
if not stripped.startswith("security"):
|
||||
continue
|
||||
if stripped.replace(" ", "") in {"security:[]", '"security":[]', "security:[],", '"security":[],'}:
|
||||
return "public", [f"{stripped} declares the operation public"]
|
||||
return "authenticated", [f"security block at {lines[index].strip()}"]
|
||||
return "public", ["operation declares no security requirement"]
|
||||
|
||||
|
||||
_RESOURCE_BASES = ("resource", "methodview", "apiview", "viewset", "modelviewset", "httpendpoint")
|
||||
_HTTP_VERBS = ("get", "post", "put", "patch", "delete", "head", "options")
|
||||
|
||||
@@ -534,12 +617,21 @@ def _safe_unparse(node: ast.AST) -> str:
|
||||
return ""
|
||||
|
||||
|
||||
_STRING_LITERAL = re.compile(r"""(['"]).*?\1""", re.DOTALL)
|
||||
|
||||
|
||||
def _python_route_access(decorators: list[str]) -> tuple[AccessLevel, list[str]]:
|
||||
"""The guard is the decorator's *name*, never its prose.
|
||||
|
||||
`@ns.doc(description='... without proper authorization checks')` documents the bug; matching "authorization"
|
||||
inside it reads an unguarded endpoint as guarded and silences every obligation on it, which is exactly the
|
||||
absence the corpus exists to catch (learning-harness Req 10.2)."""
|
||||
role_tokens = ("role", "permission", "admin", "owner")
|
||||
auth_tokens = ("login_required", "authenticated", "auth", "jwt_required")
|
||||
auth_tokens = ("login_required", "authenticated", "auth", "jwt_required", "token_required", "require_token")
|
||||
access_decorators = [decorator for decorator in decorators if not _looks_like_route_decorator(decorator)]
|
||||
evidence = [decorator for decorator in access_decorators if any(token in decorator.lower() for token in role_tokens + auth_tokens)]
|
||||
lowered = "\n".join(evidence).lower()
|
||||
named = {decorator: _STRING_LITERAL.sub("", decorator).lower() for decorator in access_decorators}
|
||||
evidence = [decorator for decorator in access_decorators if any(token in named[decorator] for token in role_tokens + auth_tokens)]
|
||||
lowered = "\n".join(named[decorator] for decorator in evidence)
|
||||
if any(token in lowered for token in role_tokens):
|
||||
return "role-restricted", evidence
|
||||
if any(token in lowered for token in auth_tokens):
|
||||
@@ -593,6 +685,32 @@ def _js_function_bounds(lines: Sequence[str]) -> dict[str, tuple[int, int]]:
|
||||
return bounds
|
||||
|
||||
|
||||
def _js_statements(lines: Sequence[str]) -> list[tuple[int, str]]:
|
||||
"""One entry per statement, with the line its first token sits on.
|
||||
|
||||
An express route is often written over several lines, one middleware per line, and the guard is exactly what
|
||||
those middle lines say. Read line by line, `webRouter.post(` carries no handler and no middleware at all, so the
|
||||
route reads as public and the handler is never named (learning-harness Req 10.2)."""
|
||||
statements: list[tuple[int, str]] = []
|
||||
index = 0
|
||||
while index < len(lines):
|
||||
stripped = lines[index].strip()
|
||||
joined, span = stripped, 1
|
||||
while joined.count("(") > joined.count(")") and span < _MAX_STATEMENT_LINES and index + span < len(lines):
|
||||
joined = f"{joined} {lines[index + span].strip()}"
|
||||
span += 1
|
||||
if span > 1 and joined.count("(") <= joined.count(")"):
|
||||
statements.append((index + 1, joined))
|
||||
index += span
|
||||
continue
|
||||
statements.append((index + 1, stripped))
|
||||
index += 1
|
||||
return statements
|
||||
|
||||
|
||||
_MAX_STATEMENT_LINES = 12
|
||||
|
||||
|
||||
def _js_registration(line: str, bounds: Mapping[str, tuple[int, int]]) -> tuple[str | None, list[str]]:
|
||||
"""`router.get('/books', requireAuth, listBooks)` -> the handler identifier and the middleware identifiers before it."""
|
||||
body = line[line.find("(") + 1 : line.rfind(")")] if "(" in line and line.rfind(")") > line.find("(") else ""
|
||||
@@ -600,9 +718,9 @@ def _js_registration(line: str, bounds: Mapping[str, tuple[int, int]]) -> tuple[
|
||||
identifiers = [part for part in parts[1:] if _JS_IDENTIFIER.match(part)]
|
||||
if not identifiers:
|
||||
return None, [line]
|
||||
handler = next((name for name in reversed(identifiers) if name in bounds), identifiers[-1])
|
||||
handler = next((name for name in reversed(identifiers) if name in bounds or name.rsplit(".", 1)[-1] in bounds), identifiers[-1])
|
||||
middleware = [name for name in identifiers if name != handler]
|
||||
return handler, middleware
|
||||
return handler.rsplit(".", 1)[-1], middleware # `ExportsController.exportProject` is registered; `exportProject` is defined
|
||||
|
||||
|
||||
def _js_route_access(middleware: Sequence[str], line: str) -> tuple[AccessLevel, list[str]]:
|
||||
|
||||
@@ -132,6 +132,10 @@ class PairCase:
|
||||
split_declared: bool = True # False when the catalog row named no split, so the split helper may assign one
|
||||
fix_date: str = "" # ISO date of the fixing commit; the catalog generator fills it, the split helper orders by it
|
||||
recipe: tuple[tuple[str, object], ...] = () # pointer pairs carry their harvest recipe instead of excerpts
|
||||
# (relpath, vuln document, fixed document): files that register the handler from outside its own module — an
|
||||
# OpenAPI operation, a router table. Materialized beside the excerpt so the mapper can name the handler at all
|
||||
# (learning-harness Req 10.2).
|
||||
context_files: tuple[tuple[str, Path, Path], ...] = ()
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
@@ -264,6 +268,7 @@ def _load_catalog_file(path: Path) -> tuple[PairCase, ...]:
|
||||
vuln_file, fixed_file = _resolve(root, str(item["vuln"])), _resolve(root, str(item["fixed"]))
|
||||
else:
|
||||
vuln_file, fixed_file, recipe = _pointer_files(item, slice_name, name)
|
||||
context_files = _context_files(item, root, name)
|
||||
cases.append(
|
||||
PairCase(
|
||||
name=name,
|
||||
@@ -291,11 +296,30 @@ def _load_catalog_file(path: Path) -> tuple[PairCase, ...]:
|
||||
reviewer=reviewer,
|
||||
vendored=vendored,
|
||||
recipe=recipe,
|
||||
context_files=context_files,
|
||||
)
|
||||
)
|
||||
return tuple(cases)
|
||||
|
||||
|
||||
def _context_files(item: dict[str, object], root: Path, name: str) -> tuple[tuple[str, Path, Path], ...]:
|
||||
rows = item.get("context")
|
||||
if not rows:
|
||||
return ()
|
||||
if not isinstance(rows, list):
|
||||
raise CatalogError(f"pair {name}: context must be a list of tables")
|
||||
out: list[tuple[str, Path, Path]] = []
|
||||
for row in rows:
|
||||
if not isinstance(row, dict) or not {"relpath", "vuln", "fixed"} <= set(row):
|
||||
raise CatalogError(f"pair {name}: each context row needs relpath, vuln and fixed")
|
||||
vuln, fixed = _resolve(root, str(row["vuln"])), _resolve(root, str(row["fixed"]))
|
||||
for path in (vuln, fixed):
|
||||
if not path.is_file():
|
||||
raise CatalogError(f"pair {name}: context document {path} is missing; re-harvest the row")
|
||||
out.append((str(row["relpath"]), vuln, fixed))
|
||||
return tuple(out)
|
||||
|
||||
|
||||
def pair_cache_dir() -> Path:
|
||||
"""Cache root for pointer pairs (Req 10.2); outside the repository unless the operator points it elsewhere."""
|
||||
return Path(os.environ.get(PAIR_CACHE_ENV) or DEFAULT_PAIR_CACHE).expanduser()
|
||||
@@ -395,8 +419,8 @@ def _review_tier(item: dict[str, object], slice_name: str, name: str) -> tuple[s
|
||||
|
||||
def evaluate_pair(case: PairCase, *, hunter: HunterScan | None = None, ruleset: tuple[PatternRule, ...] | None = None) -> PairOutcome:
|
||||
with tempfile.TemporaryDirectory(prefix="ousast-pair-") as scratch:
|
||||
vuln_root = _materialize(Path(scratch) / "vuln", case.vuln_file, case.relpath)
|
||||
fix_root = _materialize(Path(scratch) / "fixed", case.fixed_file, case.relpath)
|
||||
vuln_root = _materialize_side(Path(scratch) / "vuln", case, side="vuln")
|
||||
fix_root = _materialize_side(Path(scratch) / "fixed", case, side="fixed")
|
||||
if hunter is not None:
|
||||
return _evaluate_hunter_pair(case, vuln_root, fix_root, hunter)
|
||||
if case.slice in OVERLAY_SLICES:
|
||||
@@ -609,7 +633,9 @@ def duplicate_groups(cases: Sequence[PairCase]) -> tuple[tuple[str, ...], ...]:
|
||||
digest = _body_digest(case.vuln_file) if case.vendored else None
|
||||
if digest is not None:
|
||||
by_digest.setdefault(digest, []).append(case.name)
|
||||
return tuple(tuple(sorted(names)) for _, names in sorted(by_digest.items()) if len(names) > 1)
|
||||
# Ordered by name, not by digest: the report is a committed artifact and a re-harvest that changes one excerpt
|
||||
# must not reshuffle rows it did not touch.
|
||||
return tuple(sorted(tuple(sorted(names)) for names in by_digest.values() if len(names) > 1))
|
||||
|
||||
|
||||
def split_by_repository(cases: Sequence[PairCase], *, holdout_fraction: float = 0.5) -> tuple[tuple[PairCase, ...], SplitReport]:
|
||||
@@ -783,6 +809,14 @@ def _materialize(dest: Path, source: Path, relpath: str) -> Path:
|
||||
return dest
|
||||
|
||||
|
||||
def _materialize_side(dest: Path, case: PairCase, *, side: str) -> Path:
|
||||
"""The excerpt plus the documents that register it, at their own repository-relative paths."""
|
||||
root = _materialize(dest, case.vuln_file if side == "vuln" else case.fixed_file, case.relpath)
|
||||
for relpath, vuln_doc, fixed_doc in case.context_files:
|
||||
_materialize(root, vuln_doc if side == "vuln" else fixed_doc, relpath)
|
||||
return root
|
||||
|
||||
|
||||
# --- scans -----------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -864,8 +898,8 @@ def _obligation_scan(root: Path, targets: Sequence[FileTarget]) -> list[StaticFi
|
||||
|
||||
def _inventory_only(case: PairCase, ruleset: tuple[PatternRule, ...] | None = None) -> PairOutcome:
|
||||
with tempfile.TemporaryDirectory(prefix="ousast-pair-inv-") as scratch:
|
||||
vuln_root = _materialize(Path(scratch) / "vuln", case.vuln_file, case.relpath)
|
||||
fix_root = _materialize(Path(scratch) / "fixed", case.fixed_file, case.relpath)
|
||||
vuln_root = _materialize_side(Path(scratch) / "vuln", case, side="vuln")
|
||||
fix_root = _materialize_side(Path(scratch) / "fixed", case, side="fixed")
|
||||
return _evaluate_inventory_pair(case, vuln_root, fix_root, ruleset=ruleset)
|
||||
|
||||
|
||||
|
||||
@@ -18,6 +18,8 @@ LANGUAGE_BY_EXTENSION = {
|
||||
".py": "python",
|
||||
".js": "javascript",
|
||||
".jsx": "javascript",
|
||||
".mjs": "javascript", # ES modules and CommonJS modules are JavaScript; without them the file has no target at all
|
||||
".cjs": "javascript",
|
||||
".ts": "typescript",
|
||||
".tsx": "typescript",
|
||||
".go": "go",
|
||||
|
||||
Reference in New Issue
Block a user