mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(cpg): single-query path drops params the script does not declare
taint.request_params emits guards/fixedOrigin/originAnchors/quotedSanitizers/
fieldParameterSources, which taint.sc reads from per-request JSON in its BATCH
branch but does NOT declare on `@main def exec` and does NOT consume in its
single-request branch (it defaults them). The single-query path passed them as
`--param guards=...` (disposable) or named args (session), so joern errored
`Unknown arguments` and every cpg.run("taint", ...) failed -- a silent zero of
the shape AGENTS.md warns about. scan_region->taint_verdict->_usable_flows and
the pair-path verdict()/verdicts() were dead; the push/scan batch route was
unaffected (it carries the extras as JSON the script parses itself).
Fix: parse the script's `@main def exec(...)` signature once (_declared_exec_params
+ memoised _declared_params) and, in query(), filter params to the declared set
before building either route. Faithful, not lossy -- the single-request branch
could never use those 5 fields. No-op for config/dominance (their request_params
emit only declared params; verified). Only taint sheds the 5 extras.
Verified end-to-end on banks (patched backend in the replay image): single-query
path now runs; baseline 0 findings, annotated search 2 path/model_entailed
findings (prompt->write_text). Regression test asserts no undeclared param
reaches the command line and the query parses, with a fake runner (no Joern).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
8f7fc4ae83
commit
a024656da0
+23
-1
@@ -1,6 +1,28 @@
|
||||
# BUG: `taint.sc` single-query path broken by exec-signature skew
|
||||
|
||||
Filed: 2026-10-12 · Repo HEAD at filing: `7ab5c2a` · Severity: **high (silent zero)** · Status: OPEN
|
||||
Filed: 2026-10-12 · Repo HEAD at filing: `7ab5c2a` · Severity: **high (silent zero)** · Status: **FIXED (2026-10-12)**
|
||||
|
||||
## Resolution (2026-10-12)
|
||||
|
||||
Fixed via **option 1** (filter params to the declared signature) in `src/openultrasast/cpg/backend.py`:
|
||||
|
||||
- New `_declared_exec_params(source)` parses the `@main def exec(...)` parameter names from a shipped script
|
||||
(brace-aware, tolerant of an unfindable signature → empty set → "do not filter").
|
||||
- New memoised `JoernBackend._declared_params(query)` reads+caches a script's declared names.
|
||||
- `JoernBackend.query()` now drops any param the script does not declare before building the `--param` list
|
||||
(disposable route) and the in-session named-arg call (session route). This is faithful, not lossy: the
|
||||
single-request branch of `taint.sc` (line 1542) calls `rowsFor(...)` with top-level params only and lets
|
||||
`fixedOrigin`/`originAnchors`/`quotedSanitizers`/`guards`/`fieldParameterSources` default — it never could
|
||||
consume them. The batch path keeps carrying them as JSON the script parses itself.
|
||||
- No-op for `config`/`dominance`: their own `request_params` emit only params their `.sc` declares (verified),
|
||||
so nothing is dropped. Only `taint` sheds the 5 extras.
|
||||
|
||||
**Verified end-to-end** (patched backend mounted into the replay image, single-query path on banks): the
|
||||
queries now RUN (no `cpg query taint failed`); baseline = 0 findings, fixed search (annotation) = 2 scored
|
||||
`path`/`model_entailed` findings (`from_prompt_path` → `prompt_file.write_text(prompt.raw)` L45; `_scan` →
|
||||
`self._index_path.write_text(...)` L155). Regression test:
|
||||
`tests/test_cpg_backend.py::test_a_single_query_passes_only_params_the_script_declares` (fake runner, no Joern
|
||||
— asserts the 5 undeclared params never reach the command line and the query parses).
|
||||
|
||||
## Summary
|
||||
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
PATCH PRESENT: True
|
||||
=== building banks CPG (python) ===
|
||||
CPG built.
|
||||
shipped path spec: sources=22 sinks=('FileResponse', 'open', 'os.remove', 'os.unlink', 'send_file', 'send_from_directory', 'shutil.copy', 'shutil.move', 'shutil.rmtree')
|
||||
ANNOTATED path spec sinks: ('FileResponse', 'open', 'os.remove', 'os.unlink', 'read_text', 'send_file', 'send_from_directory', 'shutil.copy', 'shutil.move', 'shutil.rmtree', 'write_text')
|
||||
|
||||
=== BASELINE (shipped search: no source annotation, no sink annotation) ===
|
||||
[baseline] from_prompt_path (parameter_sources=False): 0 finding(s)
|
||||
[baseline] set (parameter_sources=False): 0 finding(s)
|
||||
[baseline] _load (parameter_sources=False): 0 finding(s)
|
||||
[baseline] _scan (parameter_sources=False): 0 finding(s)
|
||||
|
||||
=== FIXED SEARCH (LLM annotation injected: param source + write_text/read_text sink) ===
|
||||
[fixed] from_prompt_path (parameter_sources=True): 1 finding(s)
|
||||
family=path rung=model_entailed site=src/banks/registries/directory.py:45:from_prompt_path witness=path -> prompt_file.write_text(prompt.raw) in src/banks/registries/directory.py (line 45,
|
||||
[fixed] set (parameter_sources=True): 0 finding(s)
|
||||
[fixed] _load (parameter_sources=True): 0 finding(s)
|
||||
[fixed] _scan (parameter_sources=True): 1 finding(s)
|
||||
family=path rung=model_entailed site=src/banks/registries/directory.py:155:_scan witness=self -> self._index_path.write_text(self._index.model_dump_json()) in src/banks/registries
|
||||
|
||||
=== RESULT: baseline path findings=0 fixed-search path findings=2 ===
|
||||
FIXED SEARCH CATCHES BANKS
|
||||
@@ -408,6 +408,8 @@ class JoernBackend:
|
||||
_cancel_deadline: float | None = field(default=None, init=False, repr=False)
|
||||
_session: EngineSession | None = field(default=None, init=False, repr=False)
|
||||
_defined: set[str] = field(default_factory=set, init=False, repr=False)
|
||||
# Declared `@main def exec(...)` parameter names per shipped script, parsed once. See `_declared_params`.
|
||||
_script_params: dict[str, frozenset[str]] = field(default_factory=dict, init=False, repr=False)
|
||||
|
||||
def available(self) -> bool:
|
||||
from .capability import has_cpg
|
||||
@@ -1263,6 +1265,23 @@ class JoernBackend:
|
||||
# A request the engine did not answer is absent, not empty: the caller must be able to tell them apart.
|
||||
return {str(rid): list(rows) for rid, rows in parsed.items() if rid != PARTIAL and isinstance(rows, list)}
|
||||
|
||||
def _declared_params(self, query: str) -> frozenset[str]:
|
||||
"""Declared ``exec`` parameter names for a shipped script, parsed once and memoised.
|
||||
|
||||
Empty means the signature could not be read or parsed; callers must then NOT filter, so an
|
||||
unexpected script shape degrades to today's behaviour rather than dropping every parameter.
|
||||
"""
|
||||
cached = self._script_params.get(query)
|
||||
if cached is not None:
|
||||
return cached
|
||||
try:
|
||||
source = (self.queries_dir / f"{query}.sc").read_text()
|
||||
except OSError:
|
||||
source = ""
|
||||
declared = _declared_exec_params(source)
|
||||
self._script_params[query] = declared
|
||||
return declared
|
||||
|
||||
def query(self, cpg_path: Path, query: str, params: Mapping[str, object]) -> object | None:
|
||||
"""Run a shipped CPGQL script against a built CPG and return its fenced JSON payload."""
|
||||
if not self._time_available():
|
||||
@@ -1271,6 +1290,14 @@ class JoernBackend:
|
||||
if not script.is_file():
|
||||
logger.warning("no such cpg query: %s", script)
|
||||
return None
|
||||
# A single query passes its params as `--param name=value` (disposable) or as named arguments to the
|
||||
# in-session `exec` (session). Either way a name the script does not declare is a hard error -- joern
|
||||
# prints `Unknown arguments: --guards ...` and the query fails, reading upstream as zero flows. The
|
||||
# batch path never hits this because the same extras travel inside the requests JSON the script parses
|
||||
# itself. So drop any param the script does not declare; the single-request branch defaults them anyway.
|
||||
declared = self._declared_params(query)
|
||||
if declared:
|
||||
params = {key: value for key, value in params.items() if key in declared}
|
||||
body = self._session_payload(cpg_path, query, {"cpgFile": str(cpg_path), **params})
|
||||
if body is None:
|
||||
command = [shutil.which("joern") or "joern", self._heap_flag(), "--script", str(script), "--param", f"cpgFile={cpg_path}"]
|
||||
@@ -1723,6 +1750,57 @@ def _engine_failure(completed: subprocess.CompletedProcess[str] | None) -> str:
|
||||
return head or text[:300]
|
||||
|
||||
|
||||
def _declared_exec_params(source: str) -> frozenset[str]:
|
||||
"""The parameter names a shipped ``@main def exec(...)`` script declares.
|
||||
|
||||
A single-query caller builds its params from a family spec (``request_params``), which carries fields
|
||||
only some scripts and some code paths consume. ``taint.sc`` reads ``fixedOrigin``/``originAnchors``/
|
||||
``quotedSanitizers``/``guards``/``fieldParameterSources`` from the per-request JSON object in its BATCH
|
||||
branch, but its single-request branch and its ``exec`` signature do not take them at all -- they default.
|
||||
Passed to ``exec`` as ``--param guards=...`` or as a named argument, joern reports ``Unknown arguments``
|
||||
and the whole query fails: a silent zero of exactly the shape AGENTS.md warns about. Knowing what a
|
||||
script declares lets the single-query path drop the rest, losing nothing the single-request branch could
|
||||
have used while the batch path keeps carrying them as JSON the script parses itself.
|
||||
|
||||
Returns an empty set when the signature cannot be found, which callers read as "do not filter".
|
||||
"""
|
||||
marker = "@main def exec("
|
||||
start = source.find(marker)
|
||||
if start < 0:
|
||||
return frozenset()
|
||||
index = start + len(marker)
|
||||
depth = 1
|
||||
begin = index
|
||||
while index < len(source) and depth > 0:
|
||||
char = source[index]
|
||||
if char in "([{":
|
||||
depth += 1
|
||||
elif char in ")]}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
break
|
||||
index += 1
|
||||
signature = source[begin:index]
|
||||
names: set[str] = set()
|
||||
depth = 0
|
||||
last = 0
|
||||
parts: list[str] = []
|
||||
for position, char in enumerate(signature):
|
||||
if char in "([{":
|
||||
depth += 1
|
||||
elif char in ")]}":
|
||||
depth -= 1
|
||||
elif char == "," and depth == 0:
|
||||
parts.append(signature[last:position])
|
||||
last = position + 1
|
||||
parts.append(signature[last:])
|
||||
for part in parts:
|
||||
head = part.split(":", 1)[0].strip()
|
||||
if head.isidentifier():
|
||||
names.add(head)
|
||||
return frozenset(names)
|
||||
|
||||
|
||||
def _render(value: object) -> str:
|
||||
"""CPGQL parameters are strings; a sequence becomes a comma-separated list the script splits."""
|
||||
if isinstance(value, str):
|
||||
|
||||
@@ -166,6 +166,43 @@ def test_a_batch_is_one_invocation_carrying_many_requests(monkeypatch: pytest.Mo
|
||||
assert rows["r1"] and rows["r2"] == []
|
||||
|
||||
|
||||
def test_a_single_query_passes_only_params_the_script_declares(tmp_path: Path) -> None:
|
||||
"""A taint spec carries fields only the BATCH path consumes (`guards`, `fixedOrigin`, ...): the script
|
||||
reads them from each request's JSON, and its single-request branch defaults them. Passed to `exec` as
|
||||
`--param guards=...`, joern reports `Unknown arguments` and the whole query fails -- a silent zero of the
|
||||
exact shape AGENTS.md warns about. The single-query path must drop any name the script does not declare.
|
||||
"""
|
||||
from openultrasast.cpg.backend import BEGIN, END, JoernBackend, _declared_exec_params
|
||||
from openultrasast.model.specs import taint_specs
|
||||
from openultrasast.model.taint import request_params
|
||||
|
||||
calls: list[list[str]] = []
|
||||
|
||||
class _Done:
|
||||
returncode = 0
|
||||
stderr = ""
|
||||
stdout = f"{BEGIN}\n[]\n{END}\n"
|
||||
|
||||
def runner(command, **kwargs): # type: ignore[no-untyped-def]
|
||||
calls.append(list(command))
|
||||
return _Done()
|
||||
|
||||
backend = JoernBackend(runner=runner)
|
||||
spec = taint_specs(language="python")["path"]
|
||||
params = request_params(spec, function="from_prompt_path", file="x.py", parameter_sources=True)
|
||||
# The spec DOES emit the undeclared fields; the guard is that they do not reach the command line.
|
||||
undeclared = set(params) - _declared_exec_params((backend.queries_dir / "taint.sc").read_text())
|
||||
assert {"guards", "fixedOrigin", "originAnchors", "quotedSanitizers", "fieldParameterSources"} <= undeclared
|
||||
|
||||
result = backend.query(tmp_path / "cpg.bin", "taint", params)
|
||||
|
||||
assert result == [], "the query must parse, not fail on unknown arguments"
|
||||
assert len(calls) == 1
|
||||
passed = {part.split("=", 1)[0] for part in calls[0] if "=" in part}
|
||||
assert not (passed & {f"{name}" for name in undeclared}), f"undeclared params reached joern: {passed & undeclared}"
|
||||
assert {"sources", "sinks", "function"} <= passed, "declared params must still be sent"
|
||||
|
||||
|
||||
def test_a_failed_batch_yields_no_rows_for_any_request(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Fail closed, as everywhere else: an engine that could not answer must not read as 'nothing here'."""
|
||||
from openultrasast.cpg.backend import JoernBackend
|
||||
|
||||
Reference in New Issue
Block a user