diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index d6458d9..99d5b81 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -675,7 +675,29 @@ - The next measurement is therefore the per-request cost itself, and specifically what the two-stage joins cost at scale: they were 1.6x on a slice (62s -> 97s) and are plausibly the dominant term here. Until that is known, raising the timeout would only buy a slower way to find out. - - Still open: that measurement; and nothing here recovers a cross-shard flow. + - **ALL OF THE ABOVE ABOUT php2cpg IS WRONG. There is no frontend defect. 2026-09-10.** Every PHP build on + this machine ran through a local `php` shim that proxies stdio through `docker run`, and php2cpg depends + on the INTERLEAVING of its parser's stderr banners with its stdout JSON to attribute each document to a + file. The proxy scrambles that ordering. Measured on the same 177-file tree: + + streaming shim drops 72-88 767KB / 969KB / 1.2MB different every run + buffered shim drops 0 5,703 bytes empty, and silent about it + NATIVE php drops 0 2,872,996 bytes byte-identical across three runs + + So: no intermittent batch failure, no ~50% drop rate, no malformed CPG, no dataflow assertion, and no + scale limit. The `AssertionError: astParent ... has two parents` was a graph the shim had corrupted. + - **The real 637-file envelope, native php:** `drops=0`, a **4,329,471-byte CPG**, **20.0s wall**, **1.9GB + peak RSS**. Which is what the ORIGINAL note in this task said -- "php2cpg builds a 4.3MB CPG for a + 637-file plugin with no errors" -- and it was right; everything written against it since was measured + through a broken instrument. + - Three of today's mechanisms were therefore built against an artifact: `_build_with_retries`, the sharded + build with its island CPG, and the census-before-split guard. They are inert on a correct instrument (no + drops means no retry and no split) and would still help against a genuine frontend failure, so they + stay -- but the evidence offered for them in their commit messages was not real, and this is the record + of that. + - Still open, and now measurable for the first time: whether the queries survive a well-formed 637-file + graph, what they cost, and what they find. Nothing here recovers a cross-shard flow, but on a correct + instrument there are no shards. - Still open: wiring that exclusion loop, the 637-file build with it in place, and whether the REST handler two calls away can reach the sink -- the slice found it through a same-function flow, not across the object. diff --git a/benchmarks/repos/README.md b/benchmarks/repos/README.md index 206bd6b..8978ed3 100644 --- a/benchmarks/repos/README.md +++ b/benchmarks/repos/README.md @@ -218,6 +218,15 @@ obvious question is whether any of it survives on the whole plugin. Measured on The build is affordable; a CI runner can do this without a vertical-scale machine, which was the open question. The scan is not. +**Correction, and it invalidates the paragraph below.** Every PHP build behind these numbers ran through a +local `php` shim that proxies stdio through `docker run`, and php2cpg depends on the interleaving of its +parser's stderr banners with its stdout JSON to attribute documents to files. The proxy scrambles that +ordering: 72–88 dropped files per build, a different graph every run, and a corrupted CPG that makes Joern's +dataflow overlay assert. With a **native** php the same 637-file tree builds in **20.0s** to a +**4,329,471-byte CPG** with **zero** drops, byte-identical across runs. There is no frontend defect, no scale +limit, and the whole-repository scan has never actually been measured here. What follows is kept as the +record of a wrong diagnosis, not as a result. + It is worth being precise about *why*, because the obvious answers are all wrong. It is not memory: at 2 GB and at 4 GB the CPG never finishes loading, with no `OutOfMemoryError` in either case, both runs stopping in the same place — `Braintree\Util`. It is not the request payload either. What costs is Joern's dataflow diff --git a/src/openultrasast/cpg/backend.py b/src/openultrasast/cpg/backend.py index d1bfb69..fbb135b 100644 --- a/src/openultrasast/cpg/backend.py +++ b/src/openultrasast/cpg/backend.py @@ -80,6 +80,11 @@ _FRONTENDS = { _PREFER_FRONTEND = frozenset({"php"}) # The extensions that count as source for a language, used to ask whether a graph is missing anything. _LANGUAGE_EXTENSIONS: dict[str, tuple[str, ...]] = {"php": (".php",)} +# Frontends that shell out to a separate interpreter, and the command that proves it can read a file. +# `php2cpg` drives PHP-Parser through whatever `php` is on PATH, so the frontend is only as good as that +# interpreter's view of the filesystem -- and a `php` that cannot see the tree does not fail, it produces an +# empty graph with exit status 0 and no warnings. +_INTERPRETERS: dict[str, str] = {"php": "php"} FRONTEND_BUILD_ATTEMPTS = 4 # Request fields that describe the REPOSITORY rather than the region, and are therefore identical in every @@ -184,6 +189,8 @@ class JoernBackend: query_timeout: int = QUERY_TIMEOUT_SECONDS heap_mb: int = 0 # 0 means read the environment, then fall back to CPG_HEAP_MB queries_dir: Path = field(default_factory=lambda: QUERIES_DIR) + # Why the last build refused, so the driver can name it instead of reporting a bare `cpg_build_failed`. + last_failure: str = "" def available(self) -> bool: from .capability import has_cpg @@ -202,6 +209,23 @@ class JoernBackend: scratch = Path(tempfile.mkdtemp(prefix="ousast-cpg-")) cpg_path = scratch / "cpg.bin" + # THE PRECONDITION. Before anything is built, make the toolchain open one file from the tree and say + # how big it is. Every instrument failure this project has had takes the same shape -- the input was + # unreadable and the tool reported a plausible ZERO -- and it has cost four wrong diagnoses: + # + # a php that could not see ~/.cache -> "php2cpg cannot handle 637 files" (it read none of them) + # a php whose stdio was proxied -> "php2cpg drops half its batches" (it dropped none) + # a container missing a bind mount -> "0 files dropped", 5,703-byte graph + # + # None of those announced themselves. `file_exists()` returns false, the parser writes nothing, the + # frontend exits 0, and "no errors" reads exactly like success. Checking the OUTPUT cannot separate + # those from a genuinely empty repository; checking that the INPUT arrived can, and costs one exec. + unreadable = self._interpreter_cannot_read(root, language) + if unreadable: + logger.error("cpg build refused for %s: %s", root, unreadable) + self.last_failure = unreadable + return None + # A frontend whose failures are visible and retryable is worth more than one whose are not. if language.lower() in _PREFER_FRONTEND: shards, unparsed = self._build_sharded(root, scratch, language) @@ -235,6 +259,36 @@ class JoernBackend: unparsed=unparsed, ) + def _interpreter_cannot_read(self, root: Path, language: str) -> str: + """``""`` when the language's interpreter can read the tree, else why not. + + Deliberately narrow: it proves one file is openable and non-empty FROM THE INTERPRETER'S OWN VIEW, + which is the thing that differs when `php` is a wrapper, a container without the right bind mount, or + a tree of symlinks pointing somewhere the sandbox cannot follow. Nothing else here can see that + difference, because every other signal is downstream of the parse that never happened. + """ + interpreter = _INTERPRETERS.get(language.lower()) + extensions = _LANGUAGE_EXTENSIONS.get(language.lower()) + if not interpreter or not extensions: + return "" + binary = shutil.which(interpreter) + if binary is None: + return "" # absent is a different failure, and the frontend reports it plainly + sample = next((path for path in sorted(root.rglob("*")) if path.suffix.lower() in extensions and path.is_file()), None) + if sample is None: + return "" # nothing of this language to read; not the interpreter's fault + probe = "$f = $argv[1]; if (!is_readable($f) || filesize($f) < 1) { exit(3); } exit(0);" + completed = self._run([binary, "-r", probe, str(sample)], timeout=60) + if completed is None: + return f"the {interpreter} interpreter did not respond when asked to read {sample.name}" + if completed.returncode != 0: + return ( + f"the {interpreter} interpreter cannot read {sample}, so the frontend would parse nothing and " + f"write an empty graph. Check that {interpreter} is a real interpreter with access to this " + f"tree -- a container or wrapper without the right mount fails exactly this way." + ) + return "" + def _build_sharded(self, root: Path, scratch: Path, language: str) -> tuple[tuple[Path, ...], tuple[str, ...]]: """Build the tree, and give the files it refuses a CPG of their OWN rather than losing them. @@ -254,6 +308,12 @@ class JoernBackend: if dropped is None: return (), () if not dropped: + # A build that warned about nothing is exactly the build that needs checking, because a graph + # holding nothing arrives with no warnings at all. Silence is the symptom that has no symptom. + census = self._graph_census(main, root, language) + if census is not None and census[0] < census[1]: + self.last_failure = f"the frontend reported no failures but the graph holds {census[0]} of {census[1]} source files" + logger.error("cpg build for %s: %s", root, self.last_failure) return (main,), () # A warning is a symptom, not a verdict. `php2cpg` logs `Failed to process` for files that are @@ -282,26 +342,31 @@ class JoernBackend: return tuple(shards), tuple(still) return tuple(shards), dropped - def _graph_is_complete(self, cpg_path: Path, root: Path, language: str) -> bool: - """Does this graph hold at least as many files as the tree has sources? + def _graph_census(self, cpg_path: Path, root: Path, language: str) -> tuple[int, int] | None: + """``(files_in_graph, files_expected)``, or ``None`` when the question cannot be asked. - Conservative in the only direction that matters: when the census cannot be taken, or the language - has no declared extensions, the answer is "no" and the caller does the careful thing. + Always asked, not only when the frontend warned. The dangerous build is the SILENT one: a graph that + holds nothing, reported with no errors at all, which is what an unreadable tree produces and what a + genuinely clean scan of an empty repository looks like. A warning is a symptom; this is the evidence. """ extensions = _LANGUAGE_EXTENSIONS.get(language.lower()) if not extensions: - return False + return None expected = sum(1 for path in root.rglob("*") if path.suffix.lower() in extensions and path.is_file()) if not expected: - return False + return None payload = self.query(cpg_path, "census", {}) if not isinstance(payload, Mapping): - return False + return None try: - files = int(str(payload.get("files", "0"))) + return int(str(payload.get("files", "0"))), expected except ValueError: - return False - return files >= expected + return None + + def _graph_is_complete(self, cpg_path: Path, root: Path, language: str) -> bool: + """Conservative: when the census cannot be taken, the answer is "no" and the caller is careful.""" + census = self._graph_census(cpg_path, root, language) + return census is not None and census[0] >= census[1] def _island_root(self, root: Path, scratch: Path, files: Sequence[str]) -> Path | None: """A tree holding only ``files``, at their original relative paths. diff --git a/src/openultrasast/model/scan.py b/src/openultrasast/model/scan.py index 55f5a69..a37a693 100644 --- a/src/openultrasast/model/scan.py +++ b/src/openultrasast/model/scan.py @@ -123,7 +123,7 @@ def scan_repository( regions_unjudged=len(regions), seconds=round(time.monotonic() - started, 2), build_seconds=build_seconds, - degradations=({"stage": "model", "reason": "cpg_build_failed"},), + degradations=({"stage": "model", "reason": "cpg_build_failed", "detail": getattr(backend, "last_failure", "")},), ) # A CPG the frontend built with files missing is not the repository the caller asked about, and no diff --git a/src/openultrasast/reports.py b/src/openultrasast/reports.py index be66bd7..9f019a5 100644 --- a/src/openultrasast/reports.py +++ b/src/openultrasast/reports.py @@ -427,7 +427,7 @@ def _append_coverage(lines: list[str], coverage: Sequence[Mapping[str, object]], # silence in the table above is narrower than it looks. _COVERAGE_DEGRADATIONS: dict[str, str] = { "cpg_empty": "The code graph came back with no methods in it, so **nothing above was analysed at all**.", - "cpg_build_failed": "The code graph could not be built, so nothing above was analysed.", + "cpg_build_failed": "The code graph could not be built, so nothing above was analysed. {detail}", "cpg_sharded": ( "The graph had to be built in {shards} parts, because the frontend could not parse the whole tree at " "once. Each part was queried, but **a flow whose source is in one part and whose sink is in another " diff --git a/tests/test_cpg_backend.py b/tests/test_cpg_backend.py index b6c504f..a067c14 100644 --- a/tests/test_cpg_backend.py +++ b/tests/test_cpg_backend.py @@ -402,6 +402,8 @@ def test_php_builds_through_the_frontend_and_retries_while_files_are_dropped(tmp warning = "WARN AstCreationPass Failed to process '/src/big.php'\n" def runner(command, **kwargs): # type: ignore[no-untyped-def] + if "-r" in command: + return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="") tried.append(Path(command[0]).name) Path(command[command.index("-o") + 1]).write_text("cpg") # Drops a file on the first two attempts, clean on the third. @@ -428,6 +430,8 @@ def test_a_php_build_that_never_stops_dropping_files_reports_them(tmp_path: Path def runner(command, **kwargs): # type: ignore[no-untyped-def] nonlocal attempts + if "-r" in command: + return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="") attempts += 1 Path(command[command.index("-o") + 1]).write_text("cpg") return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="WARN Failed to process '/src/stubborn.php'\n") @@ -456,6 +460,8 @@ def test_files_the_frontend_refuses_get_a_cpg_of_their_own(tmp_path: Path, monke def runner(command, **kwargs): # type: ignore[no-untyped-def] commands.append(list(command)) + if "-r" in command: + return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="") # the input probe if "--script" in command: # The census, asked because the frontend warned. Here the warning is real: the graph is short a # file, so the caller should go on to exclude it and build it separately. @@ -535,6 +541,8 @@ def test_a_warning_about_a_file_the_graph_actually_holds_does_not_split_it(tmp_p def runner(command, **kwargs): # type: ignore[no-untyped-def] commands.append(list(command)) + if "-r" in command: + return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="") # the input probe if "--script" in command: body = '---OUSAST-CPG-BEGIN---\n{"files": "2", "methods": "9"}\n---OUSAST-CPG-END---\n' return subprocess.CompletedProcess(args=command, returncode=0, stdout=body, stderr="") @@ -549,3 +557,91 @@ def test_a_warning_about_a_file_the_graph_actually_holds_does_not_split_it(tmp_p assert result is not None assert result.unparsed == (), "the graph holds both files, so nothing is missing" assert not any("--exclude" in command for command in commands), "and it was not split" + + +def test_a_build_is_refused_when_the_interpreter_cannot_read_the_tree(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """The failure this project keeps having: the input is unreadable and the tool reports a plausible ZERO. + + `php2cpg` drives PHP-Parser through whatever `php` is on PATH. Give it a `php` that cannot see the tree + -- a wrapper, a container missing a bind mount, symlinks pointing outside a sandbox -- and it does not + fail. It parses nothing, writes a graph with no methods in it, and exits 0 with no warnings. That has + cost this project four wrong diagnoses, including "php2cpg cannot handle 637 files" from a run that read + none of them. + + Checking the output cannot tell that from an empty repository. Checking that the input arrived can. + """ + import subprocess + + from openultrasast.cpg.backend import JoernBackend + + (tmp_path / "a.php").write_text(" None: + """The probe must not stand in the way of the ordinary case.""" + import subprocess + + from openultrasast.cpg.backend import JoernBackend + + (tmp_path / "a.php").write_text(" None: + """A build that warns about nothing is exactly the build that needs checking. + + A graph holding nothing arrives with no warnings at all -- silence is the symptom that has no symptom -- + so the census is taken on every build, not only on one the frontend complained about. + """ + import subprocess + + from openultrasast.cpg.backend import JoernBackend + + for name in ("a.php", "b.php", "c.php"): + (tmp_path / name).write_text(" str | None: + import shutil + + return shutil.which("joern") + + +@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine") +@pytest.mark.parametrize("name", ["taint", "dominance", "config", "census"]) +def test_each_query_compiles_and_answers(name: str, tmp_path: Path) -> None: + """Compile and RUN each query against a graph, and require a fenced payload back. + + Compiling alone would miss a runtime arity error inside a branch, so the query has to answer -- and the + answer has to parse, because a script that prints a stack trace still exits 0 often enough to matter. + """ + from openultrasast.cpg.backend import extract_payload + + script = QUERIES / f"{name}.sc" + assert script.is_file(), f"{script} is missing" + + source = tmp_path / "sample.php" + source.write_text("query($x);\n}\n") + cpg = tmp_path / "cpg.bin" + frontend = __import__("shutil").which("php2cpg") + if frontend is None: + pytest.skip("php2cpg is not installed") + built = subprocess.run([frontend, str(tmp_path), "-o", str(cpg)], capture_output=True, text=True, timeout=600, check=False) + if not cpg.is_file(): + pytest.skip(f"could not build a sample cpg: {(built.stderr or '')[-200:]}") + + command = [_joern() or "joern", "--script", str(script.resolve()), "--param", f"cpgFile={cpg}"] + if name != "census": + requests = tmp_path / "requests.json" + requests.write_text(json.dumps({"0": {"sources": "$_GET", "sinks": "$wpdb->query", "function": "handler"}})) + command += ["--param", f"requestsFile={requests}"] + done = subprocess.run(command, capture_output=True, text=True, timeout=900, check=False) + + assert done.returncode == 0, f"{name}.sc did not run: {(done.stderr or done.stdout or '')[-500:]}" + assert extract_payload(done.stdout or "") is not None, f"{name}.sc produced no parseable payload"