mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 11:49:28 +00:00
feat(model): let the project say what it ships, and rank that first
contributor-scan 2.10. Every one of libpng's twenty-five entry-point regions was a `main()` -- twenty-three under contrib/, plus example.c and pngtest.c -- and those regions carried the fifty interprocedural taint requests that dominated a 300s query phase. A library has no entry points, so entry-point-driven analysis had nothing to anchor to except its own example programs. The task said to derive this rather than hardcode `contrib/`, and a project already declares it. Makefile.am names installed targets in `lib_LTLIBRARIES` and `bin_PROGRAMS`, each target's `<target>_SOURCES` names its files, and `check_PROGRAMS`/`noinst_PROGRAMS` are automake's own word for "not installed" -- exactly the distinction wanted. Taking every `_SOURCES` would have called libpng's whole test suite shipped code, since contrib/libtests declares its sources the same way the library does. interprocedural regions 25 -> 1 taint query 300.1s -> 125.0s total query 311.9s -> 138.5s regions 121 -> 121, unchanged by design The surviving interprocedural region is contrib/tools/pngfix.c:main, which libpng genuinely installs. Two rules keep it honest. **Silence is not exclusion**: a project with no build file this can read gets None back and nothing changes, so vampi's numbers are untouched. **Not shipped is not unscanned**: those regions are still arbitrated function-locally, because a contributor editing a test must still get an answer -- what they lose is the expensive treatment and first claim on the budget. Entailed findings went 0 -> 3, and all three are false positives at contrib/gregbook/wpng.c:374, `strcpy(outname+len, ".png")`. It is safe: outname is char[256] and line 325 guards with `if ((len = strlen(inname)) > 250)`. They were not introduced so much as uncovered -- a different callDepth meant a different sink set, and the 26 fgets false positives were masking them. The structural cause is recorded as 2.12: c/injection and c/memory carry ZERO sanitizers, so no C flow can ever be discharged and every `argv -> strcpy` entails however carefully it is bounded. The discharge here is not a call but a guard that dominates the sink, which is precisely what dominance.sc already does for access control and is wired to that one family. Also adds Group 7, five tasks for stopping the same false positive twice. Two of today's three false-positive classes were one bug in three matchers -- `resolve` in `resolveUrl`, `user` in `users`, `gets` in `fgets` -- each fixed locally and each next one surviving because nobody carried the fix across. A model may group counter-examples by cause and draft an edit; only a split-respecting measurement may adopt one, because the predecessor's improve lever admitted shapes taught by the holdout pairs they then recovered and turned a -0.059 Youden into +0.059. Gates byte-identical. 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
67497c4d68
commit
6dc25d77e4
@@ -917,13 +917,15 @@ def _run_model_layer(
|
||||
from .model.endpoint import resolve_chat_endpoint, resolve_models
|
||||
from .model.regions import regions_for
|
||||
from .model.scan import ScanBudget, scan_repository
|
||||
from .model.shipped import declared_sources
|
||||
|
||||
backend = resolve_cpg_backend()
|
||||
if not backend.available():
|
||||
runtime.state["degradations"].append({"stage": "model", "reason": "cpg_unavailable"})
|
||||
return [], _model_skipped("cpg_unavailable")
|
||||
|
||||
regions = regions_for(entries, targets)
|
||||
# The project's own build declaration, when it makes one, decides what the budget is spent on first.
|
||||
regions = regions_for(entries, targets, shipped=declared_sources(root))
|
||||
if not regions:
|
||||
return [], _model_skipped("no_regions")
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ Two restraints are deliberate:
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Sequence
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import dataclass, replace
|
||||
|
||||
from .specs import config_specs, dominance_specs, taint_specs
|
||||
|
||||
@@ -59,9 +59,12 @@ class ScanRegion:
|
||||
families: tuple[str, ...]
|
||||
rank: float
|
||||
source: str # "entry_point" | "module_scope" | "file_fallback"
|
||||
# Does the project's own build declaration name this file? True when it declares nothing at all --
|
||||
# silence is not exclusion, and no repository is penalised for a build system we cannot read.
|
||||
shipped: bool = True
|
||||
|
||||
|
||||
def regions_for(entries: Sequence[object], targets: Sequence[object]) -> tuple[ScanRegion, ...]:
|
||||
def regions_for(entries: Sequence[object], targets: Sequence[object], *, shipped: frozenset[str] | None = None) -> tuple[ScanRegion, ...]:
|
||||
"""Regions for a repository, strongest first.
|
||||
|
||||
A file with an entry point yields regions for its handlers only — a fallback region beside them would
|
||||
@@ -142,7 +145,13 @@ def regions_for(entries: Sequence[object], targets: Sequence[object]) -> tuple[S
|
||||
regions.append(ScanRegion(path=path, function=None, language=language, families=families, rank=0.1, source="file_fallback"))
|
||||
|
||||
# A total order: rank descending, then path and function, so two runs of one repository agree.
|
||||
return tuple(sorted(regions, key=lambda r: (-r.rank, r.path, r.function or "")))
|
||||
if shipped is not None:
|
||||
regions = [replace(region, shipped=region.path in shipped) for region in regions]
|
||||
|
||||
# Shipped first, then rank. A project's own build declaration outranks any score computed here: libpng's
|
||||
# twenty-five entry points were all `main()` in example programs and test tools, and they took the
|
||||
# interprocedural analysis with them.
|
||||
return tuple(sorted(regions, key=lambda r: (not r.shipped, -r.rank, r.path, r.function or "")))
|
||||
|
||||
|
||||
def _language_of(target: object) -> str | None:
|
||||
|
||||
@@ -129,7 +129,7 @@ def scan_repository(
|
||||
|
||||
# Sort here rather than trusting the caller. The budget decides what goes unexamined, so the order it is
|
||||
# spent in belongs to whoever holds the budget.
|
||||
ordered_regions = sorted(regions, key=lambda r: (-r.rank, r.path, r.function or ""))
|
||||
ordered_regions = sorted(regions, key=lambda r: (not r.shipped, -r.rank, r.path, r.function or ""))
|
||||
|
||||
# Phase 1: collect the whole scan's questions, for at most `max_regions` regions. Nothing is asked yet.
|
||||
work: list[tuple[str, ScanRegion, ArbiterSpec]] = []
|
||||
@@ -271,8 +271,13 @@ def _is_entry_point(region: ScanRegion) -> bool:
|
||||
|
||||
A module body is excluded even when the mapper marked it an entry point: a module has no parameters, so
|
||||
there is nothing for the parameter-source rule to mean there.
|
||||
|
||||
So is a file the project does not declare it ships. Every one of libpng's entry points was a `main()` in
|
||||
an example program or a test tool, and they carried the whole interprocedural budget with them; a
|
||||
library has no entry points, so there was nothing else for it to anchor to. Those regions are still
|
||||
arbitrated, function-locally -- not shipped is not unscanned.
|
||||
"""
|
||||
return region.source == "entry_point" and bool(region.function) and region.function != MODULE_SCOPE
|
||||
return region.source == "entry_point" and bool(region.function) and region.function != MODULE_SCOPE and region.shipped
|
||||
|
||||
|
||||
def _spec_for(family: str, language: str) -> ArbiterSpec | None:
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
"""What the project itself says it ships (contributor-scan Req 4.2, 8.1).
|
||||
|
||||
libpng's first repository scan spent its expensive analysis on the wrong two thirds of the tree. Every one
|
||||
of its twenty-five entry-point regions was a ``main()``, twenty-three of them under ``contrib/`` -- the
|
||||
example programs and test tools shipped beside the library -- and those regions produced the fifty
|
||||
interprocedural taint requests that dominated a 243-second query phase. A library has no entry points, so
|
||||
entry-point-driven analysis had nothing to anchor to except its examples.
|
||||
|
||||
The fix is not a list of directory names invented here. A project already declares what it ships, in its own
|
||||
build files, and that declaration is evidence in the repository rather than a convention someone assumed:
|
||||
|
||||
Makefile.am ``lib_LTLIBRARIES``/``bin_PROGRAMS`` name what is INSTALLED, and each target's
|
||||
``<target>_SOURCES`` names its files. ``check_PROGRAMS`` and ``noinst_PROGRAMS`` are
|
||||
declared by automake as not installed -- that is the project saying "these are our
|
||||
tests and internal tools", which is exactly the distinction wanted here.
|
||||
CMakeLists.txt ``add_library(png ...)``, ``target_sources(png PRIVATE ...)``
|
||||
|
||||
Two rules keep this honest:
|
||||
|
||||
* **Silence is not exclusion.** A project that declares nothing gets ``None`` back and nothing changes. No
|
||||
repository is penalised for not using a build system this module can read.
|
||||
* **Not shipped is not unscanned.** A file outside the declaration is still analysed; it is ranked below
|
||||
everything shipped and does not receive the interprocedural treatment. A contributor editing a test must
|
||||
still get an answer about it.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
# `libpng16_la_SOURCES = ...`, `foo_SOURCES += ...`. The value continues while lines end in a backslash.
|
||||
_AUTOMAKE_SOURCES = re.compile(r"^\s*(?P<target>\w[\w@.]*)_SOURCES\s*\+?=(?P<value>.*)$")
|
||||
# Which targets automake INSTALLS. `check_` and `noinst_` are declared not-installed and are skipped.
|
||||
_AUTOMAKE_INSTALLED = re.compile(r"^\s*(?:bin|sbin|lib|pkglib|libexec)_(?:PROGRAMS|LTLIBRARIES|LIBRARIES)\s*\+?=(?P<value>.*)$")
|
||||
_AT_SUBSTITUTION = re.compile(r"@[^@]*@")
|
||||
# `add_library(png SHARED a.c b.c)` and `target_sources(png PRIVATE a.c)`.
|
||||
_CMAKE_SOURCES = re.compile(r"\b(?:add_library|target_sources)\s*\((?P<value>[^)]*)\)", re.IGNORECASE | re.DOTALL)
|
||||
_CMAKE_KEYWORDS = frozenset({"static", "shared", "module", "interface", "object", "public", "private", "excludefromall"})
|
||||
|
||||
_SOURCE_SUFFIXES = frozenset({".c", ".cc", ".cpp", ".cxx", ".h", ".hpp", ".hxx", ".py", ".js", ".ts", ".java"})
|
||||
_MANIFESTS = ("Makefile.am", "CMakeLists.txt")
|
||||
|
||||
|
||||
def declared_sources(root: Path) -> frozenset[str] | None:
|
||||
"""Repository-relative paths the build files declare, or ``None`` when the project declares nothing.
|
||||
|
||||
Paths are resolved against the manifest's own directory, because a nested ``Makefile.am`` names its
|
||||
sources relative to itself.
|
||||
"""
|
||||
found: set[str] = set()
|
||||
seen_manifest = False
|
||||
for name in _MANIFESTS:
|
||||
for manifest in sorted(root.rglob(name)):
|
||||
if not manifest.is_file():
|
||||
continue
|
||||
seen_manifest = True
|
||||
try:
|
||||
text = manifest.read_text(encoding="utf-8", errors="replace")
|
||||
except OSError:
|
||||
continue
|
||||
base = manifest.parent
|
||||
for token in _tokens(name, text):
|
||||
path = (base / token).resolve()
|
||||
try:
|
||||
found.add(str(path.relative_to(root.resolve())))
|
||||
except ValueError: # a declaration pointing outside the tree
|
||||
continue
|
||||
if not seen_manifest:
|
||||
return None
|
||||
return frozenset(found)
|
||||
|
||||
|
||||
def _tokens(manifest: str, text: str) -> list[str]:
|
||||
if manifest == "Makefile.am":
|
||||
return _automake_tokens(text)
|
||||
return _cmake_tokens(text)
|
||||
|
||||
|
||||
def _automake_tokens(text: str) -> list[str]:
|
||||
"""Sources of INSTALLED targets only.
|
||||
|
||||
Taking every ``_SOURCES`` would defeat the purpose: libpng's `check_PROGRAMS` are pngstest, pngvalid and
|
||||
the rest of contrib/libtests, and they declare their sources the same way the library does.
|
||||
"""
|
||||
lines = text.splitlines()
|
||||
installed: set[str] = set()
|
||||
for index in range(len(lines)):
|
||||
match = _AUTOMAKE_INSTALLED.match(lines[index])
|
||||
if match is not None:
|
||||
installed.update(_targets(_continued(lines, index)[0]))
|
||||
|
||||
out: list[str] = []
|
||||
index = 0
|
||||
while index < len(lines):
|
||||
match = _AUTOMAKE_SOURCES.match(lines[index])
|
||||
if match is None:
|
||||
index += 1
|
||||
continue
|
||||
value, index = _continued(lines, index)
|
||||
value = value.split("=", 1)[1] if "=" in value else value
|
||||
if _normalise(match.group("target")) in installed:
|
||||
out.extend(_files(value))
|
||||
index += 1
|
||||
return out
|
||||
|
||||
|
||||
def _continued(lines: list[str], index: int) -> tuple[str, int]:
|
||||
"""One logical line, following automake's backslash continuations, and where it ended."""
|
||||
value = lines[index]
|
||||
while value.rstrip().endswith("\\") and index + 1 < len(lines):
|
||||
index += 1
|
||||
value = value.rstrip().removesuffix("\\") + " " + lines[index]
|
||||
return value, index
|
||||
|
||||
|
||||
def _targets(line: str) -> set[str]:
|
||||
"""The target names on an installed-targets line, normalised the way a `_SOURCES` prefix appears."""
|
||||
value = line.split("=", 1)[1] if "=" in line else line
|
||||
out = set()
|
||||
for raw in value.replace("\\", " ").split():
|
||||
# `libpng16.la` declares `libpng16_la_SOURCES`; a plain program name is used as-is.
|
||||
out.add(_normalise(raw.replace(".", "_").removesuffix("_la") + "_la" if raw.endswith(".la") else raw))
|
||||
return out
|
||||
|
||||
|
||||
def _normalise(name: str) -> str:
|
||||
"""`libpng@PNGLIB_MAJOR@@PNGLIB_MINOR@_la` and `libpng16_la` must compare equal."""
|
||||
return _AT_SUBSTITUTION.sub("", name).replace(".", "_")
|
||||
|
||||
|
||||
def _cmake_tokens(text: str) -> list[str]:
|
||||
out: list[str] = []
|
||||
for match in _CMAKE_SOURCES.finditer(text):
|
||||
for token in _files(match.group("value")):
|
||||
if token.lower() not in _CMAKE_KEYWORDS:
|
||||
out.append(token)
|
||||
return out
|
||||
|
||||
|
||||
def _files(value: str) -> list[str]:
|
||||
out = []
|
||||
for raw in value.replace("\\", " ").split():
|
||||
token = raw.strip().strip("\"'")
|
||||
# `$(wildcard ...)` and `@PNGLIB_MAJOR@` expand at build time and cannot be resolved here.
|
||||
if not token or "$" in token or "@" in token or "*" in token:
|
||||
continue
|
||||
if Path(token).suffix.lower() in _SOURCE_SUFFIXES:
|
||||
out.append(token)
|
||||
return out
|
||||
|
||||
|
||||
__all__ = ["declared_sources"]
|
||||
Reference in New Issue
Block a user