From a024656da043c1005ea7590b3f41e1e68664793a Mon Sep 17 00:00:00 2001 From: Marius Ciepluch <11855163+norandom@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:16:36 +0200 Subject: [PATCH] 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 --- .../taint-sc-single-query-signature-skew.md | 24 +++++- .../verify-single-query-fix.out | 22 ++++++ src/openultrasast/cpg/backend.py | 78 +++++++++++++++++++ tests/test_cpg_backend.py | 37 +++++++++ 4 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 benchmarks/measurements/2026-10-12-banks-fixed-search-mvp/verify-single-query-fix.out diff --git a/.kiro/specs/engine-change-scoped-analysis/bugs/taint-sc-single-query-signature-skew.md b/.kiro/specs/engine-change-scoped-analysis/bugs/taint-sc-single-query-signature-skew.md index e3ae6f6..45c83ac 100644 --- a/.kiro/specs/engine-change-scoped-analysis/bugs/taint-sc-single-query-signature-skew.md +++ b/.kiro/specs/engine-change-scoped-analysis/bugs/taint-sc-single-query-signature-skew.md @@ -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 diff --git a/benchmarks/measurements/2026-10-12-banks-fixed-search-mvp/verify-single-query-fix.out b/benchmarks/measurements/2026-10-12-banks-fixed-search-mvp/verify-single-query-fix.out new file mode 100644 index 0000000..c6e6e17 --- /dev/null +++ b/benchmarks/measurements/2026-10-12-banks-fixed-search-mvp/verify-single-query-fix.out @@ -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 diff --git a/src/openultrasast/cpg/backend.py b/src/openultrasast/cpg/backend.py index 9fddbeb..95cb8a9 100644 --- a/src/openultrasast/cpg/backend.py +++ b/src/openultrasast/cpg/backend.py @@ -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): diff --git a/tests/test_cpg_backend.py b/tests/test_cpg_backend.py index 3778b32..482cbcf 100644 --- a/tests/test_cpg_backend.py +++ b/tests/test_cpg_backend.py @@ -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