mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(cpg): exclude the files php2cpg miscompiles, and name them
Root cause of every failed whole-repository PHP scan, bisected from 637 files
to one file to one function to one construct:
<?php
function f($n) {
h("p", function($p) { global $g; return $p . $g; });
}
A `global` declaration inside a CLOSURE makes php2cpg 4.0.623 emit a node with
two AST parents, and Joern then refuses to apply its own dataflow overlay:
java.lang.AssertionError: Iterable was expected to have exactly one
element, but it has 2. Hint: trying to resolve astParent ... (which is
probably a malformed cpg)
Controlled, because four earlier hypotheses were refuted by measurement and I
was not going to add a fifth:
global inside a closure WITH use -> ASSERTS
global inside a closure, NO use -> ASSERTS (so `use` is irrelevant)
use, global OUTSIDE the closure -> ok (so scope is the trigger)
The property that matters: one such file costs the WHOLE GRAPH, not itself.
That is why a 2,500-request taint batch and a 13-request dominance batch failed
identically in ~25s, and why I twice called this a per-request cost problem. It
never was. Nothing was ever served; the graph is rejected at load.
Rare enough to exclude rather than surrender to -- 1 of 637 files in Paid
Memberships Pro, 1 of 357 in WP Statistics, 0 of 169 in MW WP Form, about 0.2%
-- so the construct is detected by reading the source, those files are excluded
from the build, and they are reported as `unparsed` so the coverage section
names them instead of implying they were clean.
Repository-scale PHP goes from blocked to 99.8% scanned with a stated gap.
854 tests pass; 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
128950a586
commit
4a1e06b75b
@@ -85,6 +85,30 @@ _LANGUAGE_EXTENSIONS: dict[str, tuple[str, ...]] = {"php": (".php",)}
|
||||
# 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"}
|
||||
|
||||
# A `global` declaration inside a CLOSURE makes php2cpg 4.0.623 emit a node with two AST parents, and Joern
|
||||
# then refuses to apply its own dataflow overlay to the graph:
|
||||
#
|
||||
# java.lang.AssertionError: Iterable was expected to have exactly one element, but it has 2.
|
||||
# Hint: trying to resolve astParent ... (which is probably a malformed cpg)
|
||||
#
|
||||
# Four lines reproduce it, and the whole graph is lost -- not the file, the GRAPH, so every query over the
|
||||
# repository fails at load:
|
||||
#
|
||||
# <?php
|
||||
# function f($n) {
|
||||
# h("p", function($p) { global $g; return $p . $g; });
|
||||
# }
|
||||
#
|
||||
# Controlled: `use` is irrelevant (it asserts with and without), and a `global` at function scope is fine.
|
||||
# Bisected from a 637-file plugin down to one file, one function, this construct.
|
||||
#
|
||||
# It is rare enough to exclude rather than to give up over: 1 of 637 files in Paid Memberships Pro, 1 of 357
|
||||
# in WP Statistics, 0 of 169 in MW WP Form -- about 0.2%. Excluding them costs those files and saves the
|
||||
# other 99.8%, which is the difference between a scan and no scan at all. They are reported as `unparsed`,
|
||||
# so the coverage section names them rather than implying they were clean.
|
||||
_CLOSURE = re.compile(r"\bfunction\s*\(")
|
||||
_GLOBAL_IN_BODY = re.compile(r"\bglobal\s+\$")
|
||||
FRONTEND_BUILD_ATTEMPTS = 4
|
||||
|
||||
# Request fields that describe the REPOSITORY rather than the region, and are therefore identical in every
|
||||
@@ -259,6 +283,41 @@ class JoernBackend:
|
||||
unparsed=unparsed,
|
||||
)
|
||||
|
||||
def _files_with_frontend_defect(self, root: Path, language: str) -> tuple[str, ...]:
|
||||
"""Files carrying a construct this frontend miscompiles, found by reading the source.
|
||||
|
||||
Excluding a file is a real loss and is only justified when the alternative is losing everything. It
|
||||
is here: one such file makes the whole graph unqueryable, so the choice is 99.8% of the repository or
|
||||
none of it. See `_CLOSURE` above for the construct, the reproducer, and the controls.
|
||||
"""
|
||||
if language.lower() != "php":
|
||||
return ()
|
||||
found: list[str] = []
|
||||
for path in sorted(root.rglob("*.php")):
|
||||
if not path.is_file():
|
||||
continue
|
||||
try:
|
||||
text = path.read_text(errors="ignore")
|
||||
except OSError:
|
||||
continue
|
||||
for match in _CLOSURE.finditer(text):
|
||||
opening = text.find("{", match.end())
|
||||
if opening < 0:
|
||||
continue
|
||||
depth, index = 0, opening
|
||||
while index < len(text):
|
||||
if text[index] == "{":
|
||||
depth += 1
|
||||
elif text[index] == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
break
|
||||
index += 1
|
||||
if _GLOBAL_IN_BODY.search(text[opening:index]):
|
||||
found.append(str(path))
|
||||
break
|
||||
return tuple(found)
|
||||
|
||||
def _interpreter_cannot_read(self, root: Path, language: str) -> str:
|
||||
"""``""`` when the language's interpreter can read the tree, else why not.
|
||||
|
||||
@@ -303,18 +362,28 @@ class JoernBackend:
|
||||
|
||||
Returns the CPGs to query and the files that defeated even a shard of their own.
|
||||
"""
|
||||
# Excluded from the start, because one of them costs the entire graph rather than itself.
|
||||
defective = self._files_with_frontend_defect(root, language)
|
||||
if defective:
|
||||
logger.warning(
|
||||
"excluding %d file(s) under %s that php2cpg miscompiles (a `global` inside a closure): %s",
|
||||
len(defective),
|
||||
root,
|
||||
", ".join(Path(name).name for name in defective[:5]),
|
||||
)
|
||||
|
||||
main = scratch / "cpg.bin"
|
||||
dropped = self._build_with_retries(root, main, scratch, language)
|
||||
dropped = self._build_with_retries(root, main, scratch, language, exclude=defective)
|
||||
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]:
|
||||
if census is not None and census[0] < census[1] - len(defective):
|
||||
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,), ()
|
||||
return (main,), defective
|
||||
|
||||
# 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
|
||||
|
||||
@@ -645,3 +645,42 @@ def test_a_silently_short_graph_is_reported_even_with_no_warnings(tmp_path: Path
|
||||
backend.build(tmp_path, language="php")
|
||||
|
||||
assert "1 of 3 source files" in backend.last_failure, "three files went in, one came out, and it said so"
|
||||
|
||||
|
||||
def test_a_file_php2cpg_miscompiles_is_excluded_and_named(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""A `global` inside a closure makes php2cpg emit a node with two AST parents, and Joern then refuses to
|
||||
apply its dataflow overlay -- to the WHOLE graph, not that file. Every query over the repository fails at
|
||||
load, so one such file costs everything.
|
||||
|
||||
Four lines reproduce it; `use` is irrelevant, and a `global` at function scope is fine. It appears in
|
||||
about 0.2% of files (1 of 637 in Paid Memberships Pro, 1 of 357 in WP Statistics, 0 of 169 in MW WP
|
||||
Form), so excluding it saves the other 99.8% -- and it is reported, not silently dropped.
|
||||
"""
|
||||
import subprocess
|
||||
|
||||
from openultrasast.cpg.backend import JoernBackend
|
||||
|
||||
(tmp_path / "ok.php").write_text("<?php\nfunction a() { global $g; return $g; }\n")
|
||||
(tmp_path / "bad.php").write_text('<?php\nfunction f($n) {\n h("p", function($p) { global $g; return $p . $g; });\n}\n')
|
||||
commands: list[list[str]] = []
|
||||
|
||||
def runner(command, **kwargs): # type: ignore[no-untyped-def]
|
||||
if "-r" in command:
|
||||
return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="")
|
||||
commands.append(list(command))
|
||||
if "--script" in command:
|
||||
body = '---OUSAST-CPG-BEGIN---\n{"files": "1", "methods": "2"}\n---OUSAST-CPG-END---\n'
|
||||
return subprocess.CompletedProcess(args=command, returncode=0, stdout=body, stderr="")
|
||||
Path(command[command.index("-o") + 1]).write_text("cpg")
|
||||
return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="")
|
||||
|
||||
monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on")
|
||||
monkeypatch.setattr("shutil.which", lambda name: f"/opt/bin/{name}")
|
||||
|
||||
result = JoernBackend(runner=runner).build(tmp_path, language="php")
|
||||
|
||||
assert result is not None
|
||||
assert [Path(name).name for name in result.unparsed] == ["bad.php"], "the miscompiled file is named"
|
||||
build = next(c for c in commands if "--script" not in c)
|
||||
assert "--exclude" in build and any("bad.php" in part for part in build), "and excluded from the build"
|
||||
assert not any("ok.php" in part for part in build if part != str(tmp_path)), "a global at function scope is fine"
|
||||
|
||||
Reference in New Issue
Block a user