mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(cpg): build php through the frontend and retry while it drops files
The intermittent empty-graph builds that cost four manual retries to measure
WP Statistics at all.
First, a correction: the explanation this repo carried was wrong. It said
php-parse's stderr banners interleave into block-buffered stdout and corrupt
the JSON. Capturing the exact bytes php2cpg receives -- by putting a logging
wrapper on `php` -- shows its input arriving complete and valid: two
well-formed JSON documents, 5,176,594 bytes, byte-identical across runs, with
no banner anywhere inside them. The defect is in php2cpg's own reading of that
input, and nothing outside it repairs a given attempt.
Three fixes were tried against the old theory and each changed nothing, which
is how the theory died:
* an unbuffered PHP shim via --php-parser-bin 0 of 6 builds usable
* compacting the JSON, 5,176,594 -> 943,028 bytes 0 of 6
* -J-Djava.util.concurrent.ForkJoinPool.common.parallelism=1 0 of 6
What is true is that the failure is INTERMITTENT, and that `joern-parse` is
both worse at it and silent about it. Measured on one WordPress slice:
joern-parse 5 of 8 builds usable, and says nothing when they are not
php2cpg 11 of 12 builds usable, and names every file it dropped
So php builds through the frontend directly and retries while it reports
dropped files. Retrying is sound only because the failure is intermittent AND
reported -- retrying a silent failure would be superstition; retrying a
reported one is just using the report. It gives up after four attempts and
reports what is still missing rather than pretending a partial graph is whole.
wp-vuln through the driver before: 4+ manual retries to get one build
after: 6 of 6, 3 entailed findings each time
joern-parse remains the path for every other language, and the fallback when
no frontend is on PATH.
843 tests pass; gates byte-identical; mwwpform, pmpro and VAmPI unchanged.
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
d6908f18ba
commit
1ce7e9db9b
@@ -580,10 +580,24 @@
|
|||||||
One oversized file empties the whole graph, and `joern-parse` propagates none of the warnings -- thirteen
|
One oversized file empties the whole graph, and `joern-parse` propagates none of the warnings -- thirteen
|
||||||
lines of output, exit 0, "Successfully wrote graph". It is also a **race**, not a threshold: five builds
|
lines of output, exit 0, "Successfully wrote graph". It is also a **race**, not a threshold: five builds
|
||||||
of the same two-file input gave three graphs and two empty ones.
|
of the same two-file input gave three graphs and two empty ones.
|
||||||
- Two things follow. The engine now says so (`cpg_empty`, from a census carried on the batched query; see
|
- **Corrected 2026-09-09, and the first diagnosis above was WRONG.** The banner-interleaving story is not
|
||||||
`cpg/backend.py`), which is what turned this from a clean bill of health into a diagnosis. And the
|
what happens. Capturing the exact bytes php2cpg receives -- by putting a logging wrapper on `php` -- shows
|
||||||
mitigation is per-file, not per-repository: `--exclude` the file whose dump is oversized and the rest of
|
the parser's output arriving COMPLETE and VALID: two well-formed JSON documents, 5,176,594 bytes,
|
||||||
the tree builds, so the coverage disclosure should name the excluded files rather than the build failing.
|
byte-identical across runs. The defect is inside php2cpg's own reading of that input, and nothing outside
|
||||||
|
it repairs a given attempt. Tried and each changed nothing: an unbuffered PHP shim, compacting the JSON
|
||||||
|
to a fifth of its size (5,176,594 -> 943,028), and `ForkJoinPool.common.parallelism=1`.
|
||||||
|
- What IS true and useful: the failure is **intermittent**, and `joern-parse` is both worse at it and
|
||||||
|
silent about it. On one WordPress slice, measured:
|
||||||
|
|
||||||
|
joern-parse 5 of 8 builds usable, and says nothing when they are not
|
||||||
|
php2cpg 11 of 12 builds usable, and names every file it dropped
|
||||||
|
|
||||||
|
- So php now builds through the frontend directly and retries while it reports dropped files
|
||||||
|
(`_build_with_retries`, 4 attempts). Retrying is only sound because the failure is intermittent AND
|
||||||
|
reported -- retrying a silent failure would be superstition. Driver-level result on the slice that
|
||||||
|
previously needed four manual retries: **6 of 6 builds usable**, 3 entailed findings each time.
|
||||||
|
- Still open: a file that fails on every attempt is still reported rather than analysed, and `--exclude`
|
||||||
|
plus a second CPG for the excluded files is the remaining lever.
|
||||||
- Still open: wiring that exclusion loop, the 637-file build with it in place, and whether the REST handler
|
- 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
|
two calls away can reach the sink -- the slice found it through a same-function flow, not across the
|
||||||
object.
|
object.
|
||||||
|
|||||||
@@ -177,9 +177,24 @@ is still draining from a block-buffered stdout. The banner lands inside the JSON
|
|||||||
`joern-parse` propagates none of it: thirteen lines of output, exit status 0, "Successfully wrote graph".
|
`joern-parse` propagates none of it: thirteen lines of output, exit status 0, "Successfully wrote graph".
|
||||||
One oversized file empties the entire graph, so this was never the scale limit it looked like.
|
One oversized file empties the entire graph, so this was never the scale limit it looked like.
|
||||||
|
|
||||||
It is also a **race**, not a threshold. Five builds of the same two-file input: three produced a graph, two
|
It is also **intermittent**, not a threshold. Five builds of the same two-file input: three produced a graph,
|
||||||
came back empty. Before the batched query started carrying a census of the graph, two of every five scans of
|
two came back empty. Before the batched query started carrying a census of the graph, two of every five scans
|
||||||
that plugin would have reported a clean bill of health.
|
of that plugin would have reported a clean bill of health.
|
||||||
|
|
||||||
|
The first explanation offered here — stderr banners interleaving into buffered stdout — was wrong, and worth
|
||||||
|
recording as wrong. Capturing the exact bytes php2cpg receives shows the parser's output arriving complete
|
||||||
|
and valid, byte-identical across runs; the defect is inside php2cpg's own reading of it. An unbuffered shim,
|
||||||
|
compacting the JSON to a fifth of its size, and pinning ForkJoin parallelism each changed nothing.
|
||||||
|
|
||||||
|
What did work is not a repair but a measurement. `joern-parse` is worse at this than the frontend it calls,
|
||||||
|
and silent about it:
|
||||||
|
|
||||||
|
joern-parse 5 of 8 builds usable, and says nothing when they are not
|
||||||
|
php2cpg 11 of 12 builds usable, and names every file it dropped
|
||||||
|
|
||||||
|
So PHP builds through `php2cpg` directly and retries while it reports dropped files. Retrying is sound only
|
||||||
|
because the failure is intermittent *and* reported — retrying a silent failure would be superstition. The
|
||||||
|
slice that previously needed four manual retries now builds 6 times out of 6.
|
||||||
|
|
||||||
That is the argument for why a corpus of benchmarks is not enough on its own, made concretely. Every gate
|
That is the argument for why a corpus of benchmarks is not enough on its own, made concretely. Every gate
|
||||||
stayed byte-identical through all of it. A corpus tells you a verdict is right; it cannot tell you the
|
stayed byte-identical through all of it. A corpus tells you a verdict is right; it cannot tell you the
|
||||||
|
|||||||
@@ -63,6 +63,23 @@ _FRONTENDS = {
|
|||||||
"c_cpp": "c2cpg",
|
"c_cpp": "c2cpg",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Languages where the frontend is run DIRECTLY instead of through `joern-parse`, and how many times.
|
||||||
|
#
|
||||||
|
# `joern-parse` invokes the same frontend, but measured on one WordPress slice it succeeds far less often
|
||||||
|
# and, worse, tells you nothing when it does not: thirteen lines of output, exit status 0, "Successfully
|
||||||
|
# wrote graph", and a graph with no methods in it. The frontend logs a warning per file it drops.
|
||||||
|
#
|
||||||
|
# joern-parse 5 of 8 builds usable
|
||||||
|
# php2cpg 11 of 12 builds usable, and names the files it dropped
|
||||||
|
#
|
||||||
|
# php2cpg 4.0.623 fails intermittently when it reads its PHP parser's output: the parser's bytes are
|
||||||
|
# complete and valid -- verified by capturing them -- so the defect is inside the frontend's own JSON
|
||||||
|
# reading, and nothing outside it can repair a given attempt. What CAN be done is notice (the warning is
|
||||||
|
# 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"})
|
||||||
|
FRONTEND_BUILD_ATTEMPTS = 4
|
||||||
|
|
||||||
Runner = Callable[..., subprocess.CompletedProcess[str]]
|
Runner = Callable[..., subprocess.CompletedProcess[str]]
|
||||||
|
|
||||||
|
|
||||||
@@ -173,6 +190,20 @@ class JoernBackend:
|
|||||||
return None
|
return None
|
||||||
scratch = Path(tempfile.mkdtemp(prefix="ousast-cpg-"))
|
scratch = Path(tempfile.mkdtemp(prefix="ousast-cpg-"))
|
||||||
cpg_path = scratch / "cpg.bin"
|
cpg_path = scratch / "cpg.bin"
|
||||||
|
|
||||||
|
# A frontend whose failures are visible and retryable is worth more than one whose are not.
|
||||||
|
if language.lower() in _PREFER_FRONTEND:
|
||||||
|
retried = self._build_with_retries(root, cpg_path, scratch, language)
|
||||||
|
if retried is not None:
|
||||||
|
unparsed = retried
|
||||||
|
if unparsed:
|
||||||
|
logger.warning("the frontend could not parse %d file(s) under %s: %s", len(unparsed), root, ", ".join(unparsed[:5]))
|
||||||
|
return CpgResult(
|
||||||
|
cpg_path=cpg_path,
|
||||||
|
run=lambda query, params: self.query(cpg_path, query, params),
|
||||||
|
run_batch=lambda query, requests: self.query_batch(cpg_path, query, requests),
|
||||||
|
unparsed=unparsed,
|
||||||
|
)
|
||||||
command = [parse or "joern-parse", self._heap_flag(), str(root), "--output", str(cpg_path)]
|
command = [parse or "joern-parse", self._heap_flag(), str(root), "--output", str(cpg_path)]
|
||||||
completed = self._run(command, timeout=self.build_timeout, cwd=scratch)
|
completed = self._run(command, timeout=self.build_timeout, cwd=scratch)
|
||||||
unparsed = _unparsed_files(completed)
|
unparsed = _unparsed_files(completed)
|
||||||
@@ -192,6 +223,33 @@ class JoernBackend:
|
|||||||
unparsed=unparsed,
|
unparsed=unparsed,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def _build_with_retries(self, root: Path, cpg_path: Path, scratch: Path, language: str) -> tuple[str, ...] | None:
|
||||||
|
"""Build through the frontend, trying again while it reports files it could not parse.
|
||||||
|
|
||||||
|
Retrying is only sound because the failure is INTERMITTENT and the frontend says when it happened.
|
||||||
|
Retrying a silent failure would be superstition; retrying a reported one is just using the report.
|
||||||
|
The loop stops at the first clean build, and returns the last attempt's dropped files rather than
|
||||||
|
pretending a partial graph is a whole one.
|
||||||
|
|
||||||
|
``None`` means there was nothing to build with, so the caller falls back to ``joern-parse``.
|
||||||
|
"""
|
||||||
|
best: tuple[str, ...] | None = None
|
||||||
|
for attempt in range(FRONTEND_BUILD_ATTEMPTS):
|
||||||
|
unparsed = self._build_with_frontend(root, cpg_path, scratch, language)
|
||||||
|
if unparsed is None:
|
||||||
|
return best # the frontend is missing or failed outright; keep any earlier partial build
|
||||||
|
if not unparsed:
|
||||||
|
return ()
|
||||||
|
best = unparsed
|
||||||
|
logger.info(
|
||||||
|
"frontend dropped %d file(s) on attempt %d of %d for %s; retrying",
|
||||||
|
len(unparsed),
|
||||||
|
attempt + 1,
|
||||||
|
FRONTEND_BUILD_ATTEMPTS,
|
||||||
|
root,
|
||||||
|
)
|
||||||
|
return best
|
||||||
|
|
||||||
def _build_with_frontend(self, root: Path, cpg_path: Path, scratch: Path, language: str) -> tuple[str, ...] | None:
|
def _build_with_frontend(self, root: Path, cpg_path: Path, scratch: Path, language: str) -> tuple[str, ...] | None:
|
||||||
"""Retry the build through the language frontend alone.
|
"""Retry the build through the language frontend alone.
|
||||||
|
|
||||||
|
|||||||
@@ -265,7 +265,11 @@ def test_the_operator_can_raise_the_heap(tmp_path: Path, monkeypatch: pytest.Mon
|
|||||||
def test_a_failed_joern_parse_retries_through_the_language_frontend(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
def test_a_failed_joern_parse_retries_through_the_language_frontend(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
"""Measured on a 637-file WordPress plugin: `joern-parse` threw after applying overlays while `php2cpg`
|
"""Measured on a 637-file WordPress plugin: `joern-parse` threw after applying overlays while `php2cpg`
|
||||||
on the same tree produced a 4.3MB CPG with no errors. Joern's own message recommends the direct route for
|
on the same tree produced a 4.3MB CPG with no errors. Joern's own message recommends the direct route for
|
||||||
a large codebase, so this is the documented fallback rather than a workaround."""
|
a large codebase, so this is the documented fallback rather than a workaround.
|
||||||
|
|
||||||
|
Uses a language OUTSIDE `_PREFER_FRONTEND`, because php no longer waits for joern-parse to fail first --
|
||||||
|
see the test below.
|
||||||
|
"""
|
||||||
from openultrasast.cpg.backend import JoernBackend
|
from openultrasast.cpg.backend import JoernBackend
|
||||||
|
|
||||||
tried: list[str] = []
|
tried: list[str] = []
|
||||||
@@ -286,10 +290,10 @@ def test_a_failed_joern_parse_retries_through_the_language_frontend(tmp_path: Pa
|
|||||||
monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on")
|
monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on")
|
||||||
monkeypatch.setattr("shutil.which", lambda name: f"/opt/joern/{name}")
|
monkeypatch.setattr("shutil.which", lambda name: f"/opt/joern/{name}")
|
||||||
|
|
||||||
result = JoernBackend(runner=runner).build(tmp_path, language="php")
|
result = JoernBackend(runner=runner).build(tmp_path, language="python")
|
||||||
|
|
||||||
assert result is not None, "the frontend succeeded where joern-parse did not"
|
assert result is not None, "the frontend succeeded where joern-parse did not"
|
||||||
assert tried == ["joern-parse", "php2cpg"]
|
assert tried == ["joern-parse", "pysrc2cpg"]
|
||||||
|
|
||||||
|
|
||||||
def test_an_unknown_language_has_nothing_to_retry_with(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
def test_an_unknown_language_has_nothing_to_retry_with(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
@@ -380,3 +384,58 @@ def test_a_joern_log_line_inside_the_fence_does_not_destroy_the_payload() -> Non
|
|||||||
|
|
||||||
noisy = f'{BEGIN}\n[INFO ] Attempting to determine flows from empty list of sources.\n{{"0": [], "1": [1]}}\n{END}\n'
|
noisy = f'{BEGIN}\n[INFO ] Attempting to determine flows from empty list of sources.\n{{"0": [], "1": [1]}}\n{END}\n'
|
||||||
assert extract_payload(noisy) == {"0": [], "1": [1]}
|
assert extract_payload(noisy) == {"0": [], "1": [1]}
|
||||||
|
|
||||||
|
|
||||||
|
def test_php_builds_through_the_frontend_and_retries_while_files_are_dropped(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""php2cpg fails INTERMITTENTLY, and unlike joern-parse it says so.
|
||||||
|
|
||||||
|
Measured on one WordPress slice: joern-parse produced a usable graph 5 times in 8 and reported nothing
|
||||||
|
when it did not -- exit 0, "Successfully wrote graph", no methods in the graph. php2cpg on the same tree
|
||||||
|
managed 11 in 12 and named every file it dropped. So php goes straight to the frontend, and a build that
|
||||||
|
reports dropped files is simply attempted again.
|
||||||
|
"""
|
||||||
|
import subprocess
|
||||||
|
|
||||||
|
from openultrasast.cpg.backend import JoernBackend
|
||||||
|
|
||||||
|
tried: list[str] = []
|
||||||
|
warning = "WARN AstCreationPass Failed to process '/src/big.php'\n"
|
||||||
|
|
||||||
|
def runner(command, **kwargs): # type: ignore[no-untyped-def]
|
||||||
|
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.
|
||||||
|
stderr = warning if len(tried) <= 2 else ""
|
||||||
|
return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr=stderr)
|
||||||
|
|
||||||
|
monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on")
|
||||||
|
monkeypatch.setattr("shutil.which", lambda name: f"/opt/joern/{name}")
|
||||||
|
|
||||||
|
result = JoernBackend(runner=runner).build(tmp_path, language="php")
|
||||||
|
|
||||||
|
assert result is not None
|
||||||
|
assert tried == ["php2cpg", "php2cpg", "php2cpg"], "joern-parse is not consulted for php, and it retried"
|
||||||
|
assert result.unparsed == (), "the third attempt was clean, so nothing is reported as dropped"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_php_build_that_never_stops_dropping_files_reports_them(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""Retrying is not the same as pretending. When every attempt drops the same file, the caller is told."""
|
||||||
|
import subprocess
|
||||||
|
|
||||||
|
from openultrasast.cpg.backend import FRONTEND_BUILD_ATTEMPTS, JoernBackend
|
||||||
|
|
||||||
|
attempts = 0
|
||||||
|
|
||||||
|
def runner(command, **kwargs): # type: ignore[no-untyped-def]
|
||||||
|
nonlocal attempts
|
||||||
|
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")
|
||||||
|
|
||||||
|
monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on")
|
||||||
|
monkeypatch.setattr("shutil.which", lambda name: f"/opt/joern/{name}")
|
||||||
|
|
||||||
|
result = JoernBackend(runner=runner).build(tmp_path, language="php")
|
||||||
|
|
||||||
|
assert attempts == FRONTEND_BUILD_ATTEMPTS, "it gives up rather than retrying forever"
|
||||||
|
assert result is not None and result.unparsed == ("/src/stubborn.php",)
|
||||||
|
|||||||
Reference in New Issue
Block a user