From d63be344518b2e3e06836c663a995fd9aa53c938 Mon Sep 17 00:00:00 2001 From: Marius Ciepluch <11855163+norandom@users.noreply.github.com> Date: Thu, 10 Sep 2026 08:28:42 +0200 Subject: [PATCH] fix(cpg): ask the graph before splitting it, not the frontend's log Sharding was triggered by `Failed to process` warnings. Those warnings are not evidence about the finished graph: a build reporting 2 drops -> 3 files, 139 methods, same size as a clean one The file it warned about was in the graph the whole time. Acting on the warning is not merely wasteful, it is destructive -- shards cannot see each other's flows, so splitting a two-file WordPress pair on a false alarm cost it CVE-2022-25148 outright. That is how this was found: the slices caught it, at a scale small enough to check by hand, before the whole-repository run could bury it. So the build now takes a census of the graph (`census.sc`) when the frontend warns, and splits only if the graph is actually short of the files handed to it. A warning is a symptom to investigate, never a verdict. Conservative in the only direction that matters: if the census cannot be taken, the caller does the careful thing and splits. wp-vuln 3 entailed incl. CVE at :225, NO shard (was: 2, sharded) wp-fixed 2, that one absent mwwpform 1 / 0 pmpro one-file 18 incl. CVE at :936 pmpro slice 18 incl. CVE at :936, sharded -- genuinely deficient here The census costs one JVM, and only on a build that warned; a clean build pays nothing. 849 tests pass; gates byte-identical. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG --- src/openultrasast/cpg/backend.py | 32 ++++++++++++++++++++ src/openultrasast/cpg/queries/census.sc | 18 ++++++++++++ tests/test_cpg_backend.py | 39 +++++++++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 src/openultrasast/cpg/queries/census.sc diff --git a/src/openultrasast/cpg/backend.py b/src/openultrasast/cpg/backend.py index 849a7fb..d1bfb69 100644 --- a/src/openultrasast/cpg/backend.py +++ b/src/openultrasast/cpg/backend.py @@ -78,6 +78,8 @@ _FRONTENDS = { # right there) and try again. Compacting the parser's output to a fifth of its size, running the parser # through a shim, and pinning `ForkJoinPool.common.parallelism=1` were each tried and each changed nothing. _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",)} FRONTEND_BUILD_ATTEMPTS = 4 # Request fields that describe the REPOSITORY rather than the region, and are therefore identical in every @@ -254,6 +256,15 @@ class JoernBackend: if not dropped: return (main,), () + # A warning is a symptom, not a verdict. `php2cpg` logs `Failed to process` for files that are + # nonetheless in the finished graph -- measured: a build reporting two drops produced a graph holding + # both of its files and all 139 methods, the same size as a clean one. Splitting on that evidence is + # actively harmful, because shards cannot see each other's flows: it cost a two-file WordPress pair + # its CVE. So ask the graph before doing anything drastic to it. + if self._graph_is_complete(main, root, language): + logger.info("the frontend warned about %d file(s) under %s but the graph holds them all", len(dropped), root) + return (main,), () + # Second pass over the same tree, this time telling the frontend to leave the difficult files alone, # so the rest of the repository is analysed instead of nothing being analysed. remainder = self._build_with_retries(root, main, scratch, language, exclude=dropped) @@ -271,6 +282,27 @@ 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? + + 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. + """ + extensions = _LANGUAGE_EXTENSIONS.get(language.lower()) + if not extensions: + return False + expected = sum(1 for path in root.rglob("*") if path.suffix.lower() in extensions and path.is_file()) + if not expected: + return False + payload = self.query(cpg_path, "census", {}) + if not isinstance(payload, Mapping): + return False + try: + files = int(str(payload.get("files", "0"))) + except ValueError: + return False + return files >= expected + 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/cpg/queries/census.sc b/src/openultrasast/cpg/queries/census.sc new file mode 100644 index 0000000..e3933fe --- /dev/null +++ b/src/openultrasast/cpg/queries/census.sc @@ -0,0 +1,18 @@ +// How much of the repository actually made it into the graph (contributor-scan 5.10). +// +// A frontend can log `Failed to process` for a file that is nonetheless present -- measured: a build +// reporting two drops produced a graph holding both of its files and all 139 methods, byte-identical in +// size to a clean one. So the warning is a symptom to investigate, never a verdict, and the only thing that +// settles whether a graph is deficient is the graph. +// +// This matters because acting on the warning alone is destructive: it makes the driver split a repository +// into shards that cannot see each other's flows, and a two-file WordPress pair lost its CVE that way. +// +// Parameters: cpgFile. +// Output: fenced JSON {files, methods}. +@main def exec(cpgFile: String) = { + importCpg(cpgFile) + println("---OUSAST-CPG-BEGIN---") + println(ujson.write(ujson.Obj("files" -> cpg.file.size.toString, "methods" -> cpg.method.size.toString))) + println("---OUSAST-CPG-END---") +} diff --git a/tests/test_cpg_backend.py b/tests/test_cpg_backend.py index 9408838..b6c504f 100644 --- a/tests/test_cpg_backend.py +++ b/tests/test_cpg_backend.py @@ -456,6 +456,11 @@ 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 "--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. + body = '---OUSAST-CPG-BEGIN---\n{"files": "1", "methods": "1"}\n---OUSAST-CPG-END---\n' + return subprocess.CompletedProcess(args=command, returncode=0, stdout=body, stderr="") Path(command[command.index("-o") + 1]).write_text("cpg") # The whole tree always drops big.php; any build that excludes it, or covers it alone, is clean. excluded = "--exclude" in command @@ -510,3 +515,37 @@ def test_a_shard_that_cannot_answer_does_not_lose_the_others(monkeypatch: pytest monkeypatch.setattr(JoernBackend, "query_batch", lambda self, path, query, requests: None) assert backend.query_batch_across([Path("a.bin")], "taint", {}) is None, "no shard answered is not an empty answer" + + +def test_a_warning_about_a_file_the_graph_actually_holds_does_not_split_it(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """`php2cpg` logs `Failed to process` for files that are nonetheless in the finished graph. Measured: a + build reporting two drops produced a graph holding both its files and all 139 methods, the same size as + a clean one. + + Acting on the warning alone is destructive rather than merely wasteful -- shards cannot see each other's + flows, and a two-file WordPress pair lost its CVE that way. So the graph is asked before it is split. + """ + import subprocess + + from openultrasast.cpg.backend import JoernBackend + + (tmp_path / "a.php").write_text("