Update handoff: 5 taint fixtures closed, frontier analysis
ober
8c262cf0fe34dd1a332f3404cd2a77cd15924622
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,143 +1,101 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-30 (continuation) +Date: 2026-05-31 (continuation) Workspace: `/Users/user/mine/jerboa-semgrep` Sibling upstream Semgrep checkout: `/Users/user/mine/semgrep` Packaged Semgrep oracle: `/Users/user/.local/bin/semgrep` Branch: `main` -This continuation picked up the previous handoff, which left an untested partial -patch in the worktree. The work below is committed and validated. - -## What This Checkpoint Did - -1. Reverted the previous turn's broken, uncommitted C-like taint patch. It did - not compile (unbalanced parens) and its design would have regressed - `switch.php` (see below). -2. Reimplemented a clean, validated C-like (Go/PHP) taint reachability filter. -3. Closed three upstream taint false positives: - - Go `continue` (`tests/tainting_rules/go/continue.go`) - - Go `goto_dead_code` (`tests/tainting_rules/go/goto_dead_code.go`) - - PHP `break` (`tests/tainting_rules/php/break.php`) -4. Added three focused smoke tests. - -All three fixtures now match the packaged Semgrep oracle exactly. - -## The Fix - -All three false positives share one shape: a `sink(...)` on a line made -unreachable by an immediately preceding abrupt control-flow statement -(`continue` / `break` / `goto`), with only comments between. Semgrep's dataflow -drops these; Jerboa's taint engine did not. - -New code in `src/semgrep/scan.ss` (just before `filter-taint-reachable-findings`): - -- `c-like-taint-language?` — gate to Go and PHP only. -- `c-like-taint-abrupt-exit-line?` — line begins (after stripping leading close - braces) with `return` / `break` / `continue` / `goto`. -- `c-like-taint-label-line?` — a reachability *reset*: a `switch` `case` / - `default` arm, or a goto label `LABEL:` (but not a Go short decl `x :=`). -- `c-like-taint-unreachable-finding?` — walks lines from offset 0 tracking an - `unreachable?` flag. Modeled on `javascript-taint-unreachable-finding?` but: - reset on close-brace OR label/case, set by any abrupt exit. -- `filter-c-like-taint-reachable-findings` — drops findings the predicate marks - unreachable, for Go/PHP only. - -### Critical: where the filter is applied - -The filter runs on the FINAL taint findings, inside `scan-taint-rule` (wrapping -its return value), NOT at the five intermediate `filter-taint-reachable-findings` -call sites used by Python/JS/Dart. - -Why: `filter-taint-reachable-findings` is applied to intermediate source AND sink -matches (`scan-taint-source-matches`, `scan-taint-sinks`). Removing an -intermediate match that is correctly unreachable (e.g. the post-`break` -`sink($source)` on `switch.php` line 9) cascades and breaks downstream taint -assembly in `sink-output-findings`, dropping the legitimate line-15 finding too. -This was verified empirically with stderr instrumentation. Filtering only the -final findings avoids the cascade. The existing Python/JS/Dart filters never hit -this because none of them apply to PHP/Go. - -### Why the reverted patch was wrong - -The previous partial patch excluded `case`/`default` from labels and did not -reset reachability on them. On `switch.php`, the post-`break` unreachable state -would have bled past `case 1:` / `case 2:` and wrongly filtered the reachable -line-15 sink — regressing a passing fixture. The new filter treats `case` / -`default` / goto-labels as resets, which is required for `switch.php` to stay -green. - -## Validation - -- `make test`: 317 tests, 317 passed (3 new smoke tests). -- `make oracle`: 42 passed, 0 failed. -- Go taint dir: was 4 passed / 5 mismatched → now 6 passed / 3 mismatched. -- PHP taint dir: was 2 passed / 4 mismatched → now 3 passed / 3 mismatched - (`break` fixed, `switch` NOT regressed). -- Cross-language taint dirs all clean (0 mismatched): - python 12, js 11, ruby 1, dart 2, java 2, scala 1, ts 1. -- Broad upstream `tests/rules` sweep (this checkpoint, completed): - `437 passed, 0 mismatched, 0 jerboa errors, 3 current errors, 440 compared` - — identical to the prior baseline, so the fix caused no broad-sweep - regressions. The 3 current errors are packaged-Semgrep failures - (`anywhere_global`, `anywhere_include`, `anywhere_metavar`). - -## Current Parity Frontier (remaining mismatches) - -All remaining Go/PHP taint mismatches are UNDER-reporting (Jerboa emits fewer -findings than Semgrep). Each needs a real matching/propagation feature, not a -reachability filter. These are NOT "small" closes; the prior handoff's task was -explicitly "pick one small mismatch," already exceeded with the three FP fixes. - -Go (`tests/tainting_rules/go`): -- `command-injection`: multiple `focus-metavariable` sources (misses 3). -- `make`: multi-hop taint through a `make(...)` propagator + `sanitizeGlobal` - sanitizer (misses 4). -- `zip-traversal`: misses 1. - -PHP (`tests/tainting_rules/php`): -- `echo`: sink `echo ...;` (statement sink) with `$_GET[...]` source and - `htmlspecialchars(...)` sanitizer (misses lines 6, 9). NOTE: a fixture-style - fallback clause in `scan-php-taint-rule` reusing - `(php-ruleid-next-line-findings rule path source php-full-line-range)` keyed on - rule id `test-taint-echo` would make this pass, but it reads the fixture's - `//ruleid:` answer-key comments rather than doing real taint analysis. Left - undone deliberately — decide whether overfitted parity patches are wanted here. -- `lval_var_sink`: `DOMDocument::load` lval-variable sink (misses 3). -- `no_duplicate_submatches`: `$foo` interpolated in a double-quoted string as a - source, single multiline `sink(...)` finding (misses 1). +## What This Run Closed (5 taint fixtures) + +The Go + PHP taint frontier went from 9 mismatches to 4. Commits: + +- `6635390 Filter dead Go and PHP taint after abrupt exits` — C-like + reachability filter; closed go/`continue`, go/`goto_dead_code`, php/`break` + (3 false positives on lines made unreachable by return/break/continue/goto). +- `b9575f4 Flow Go taint through full := assignment RHS` — new + `scan-go-implicit-assignment-propagators`; closed go/`make` (taint nested in + `make(global.Items, ...)` now propagates; sanitizers still block). +- `3a55fb7 Reach taint through PHP echo/print statement sinks` — echo/print + statement-sink reachability branch + sanitizer guard; closed php/`echo`. +- `a2f1083 Remove stray PAT debug tracing` — cleanup of debug accidentally + committed by the `f6427c5 updates` commit. + +Validation at each step: `make test` 319/319, `make oracle` unaffected, broad +`tests/rules` sweep unchanged at `437 passed, 0 mismatched, 3 current errors`, +all 7 structural-matcher and generic taint dirs clean. + +Current frontier: +- `tests/tainting_rules/go`: 7 passed / 2 mismatched (command-injection, zip-traversal) +- `tests/tainting_rules/php`: 4 passed / 2 mismatched (lval_var_sink, no_duplicate_submatches) + +## The Architectural Wall On The Remaining 4 + +Root cause (confirmed): **only Python/JS/TS use the tree-sitter AST structural +matcher; Go, PHP, and ~10 other languages are `generic-language?`** (scan.ss +~809) and use the regex-based `scan-generic-pattern` / `generic-pattern->regex-spec`. +The generic matcher cannot do what the remaining fixtures need: + +- `lval_var_sink` (php): sink `$DOMDOCUMENT->load($FILENAME, ...)` focused on the + receiver. Needs (a) metavar args matching non-identifiers (`$FILENAME`→`'file.xml'`), + (b) `$`-vars (`$DOMDOCUMENT`→`$doc`), (c) `$ARG, ...` optional-comma, and (d) + **focus-aware taint**: the FP is `$something->load($doc)` — the focused receiver + `$something` is untainted, but the taint check matches the source `$doc` against + the sibling `$FILENAME=$doc` binding (outside the focused range). +- `no_duplicate_submatches` (php): multiline `sink(...)` (single-line matches, + multiline does not — generic ellipsis is line-bounded) + `$foo` source inside a + `"$foo"` double-quoted string interpolation. +- `command-injection` (go): typed metavars `($REQ : http.Request).$FIELD`, + `metavariable-regex`, and multiple focus-metavariables (`focus: [$PATH, $ARGS]`). +- `zip-traversal` (go): `pattern-inside` (import + `zip.OpenReader`) plus + `pattern-not-inside` with deep `<... $TARGET ...>` patterns. + +### Why a quick fix was reverted + +`lval_var_sink` was brought to a SINGLE false positive by: broadening the +generic metavar regex (`generic-metavariable-regex`) to match strings/`$`-vars, +adding optional-comma-ellipsis handling, and constraining +`finding-text-equals-any-binding?` to bindings within the focused finding. The +first two are safe (validated clean on smoke + all taint dirs after a one-line +arg-count bug fix). The third — the focus-aware taint piece — **regresses +js/`everything_source`**, which relies on the loose binding-match behavior +(its finding is produced by a source matching a sibling `$JWT` binding rather +than by the focused `$TOKEN` reaching itself, because same-position +source==sink does not satisfy `source-state-before-sink?`). The shared +reachability is delicately tuned for the structural-matcher languages, so the +change was reverted rather than trade php/lval_var_sink for js/everything_source. + +### What closing them actually requires + +Either: (1) real tree-sitter grammars for Go/PHP so they use the structural +matcher (large), or (2) a careful, jointly-validated rework of +`generic-pattern->regex-spec` + the taint reachability (`source-state-reaches-sink-spec-shape?`, +`finding-text-equals-any-binding?`, `source-state-before-sink?`) that adds +metavar-arg / multiline / typed-metavar / multiple-focus / focus-aware-taint +support WITHOUT regressing the 31 passing structural + generic taint fixtures, +the pattern oracles, and the 437-case broad sweep. Each reachability change +attempted so far cascaded into a JS regression. This is a multi-feature, +high-risk effort, not a localized patch. ## Commands To Rerun First ```sh make test make oracle -``` - -```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/go \ -LIST_MISMATCHES=1 MAX_DIFFS=0 tests/oracle/upstream-sweep.sh +LIST_MISMATCHES=1 MAX_DIFFS=0 tests/oracle/upstream-sweep.sh # swap go/php ``` -(Swap `go` for `php` for the PHP frontier. Broad sweep: use -`UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules`.) - -Note: foreground `sleep` and background `while/until` loops are blocked/unstable -in this harness; a broad sweep run via `run_in_background: true` (single -invocation) does notify on completion. Sample throughput by counting `*.norm` -files in its temp dir. +Broad sweep (slow, ~30 min): `UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules`. +Harness note: foreground `sleep` and background `while/until` loops are blocked +/ unstable; run sweeps via a single `run_in_background` invocation and sample +`*.norm` file counts in the temp dir for throughput. ## Repo Mechanics -Generated files are tracked: `lib/**/*.sls` and `src/.jerbuild-hashes`. When -changing `src/**/*.ss`, run `make build` (or `make test`) and commit the -generated `lib/semgrep/scan.sls` + `src/.jerbuild-hashes` with the source. - -Important files: -- `src/semgrep/scan.ss`: scanner, taint logic, language fallbacks. -- `src/semgrep/match/structural.ss`: structural AST matching. -- `tests/smoke.ss`: local regression suite. -- `tests/oracle/upstream-sweep.sh`: Jerboa-vs-Semgrep comparison harness. - -No git remote is configured, so commits cannot be pushed. +Generated `lib/**/*.sls` and `src/.jerbuild-hashes` are tracked; run `make build` +and commit them with `src/**/*.ss` changes. Key files: `src/semgrep/scan.ss` +(scanner, taint, generic matcher), `src/semgrep/match/structural.ss` (AST +matcher), `tests/smoke.ss`, `tests/oracle/upstream-sweep.sh`. No git remote is +configured, so commits cannot be pushed.