Refresh Opus parity handoff
ober
8dd414c8876849d8321c0da2d70de598f918152c
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,70 +1,103 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-30 05:46 MDT +Date: 2026-05-30 05:52 MDT Workspace: `/Users/user/mine/jerboa-semgrep` Sibling upstream Semgrep checkout: `/Users/user/mine/semgrep` Packaged Semgrep oracle: `/Users/user/.local/bin/semgrep` -Base HEAD before this checkpoint: `1d6987f Cover JavaScript taint throw reachability` +Branch: `main` +Base HEAD before this handoff-only checkpoint: +`355e728 Respect taint metavariable unification` -The user wants this project carried forward until the pure Jerboa port reaches -Semgrep parity. Do not treat this handoff as completion. This checkpoint closes -one more upstream JavaScript taint mismatch by matching Semgrep's -`taint_unify_mvars` behavior after assignment propagation. +The user wants the pure Jerboa Semgrep port carried forward until it reaches +Semgrep parity. Do not treat this handoff as completion. This file is a +detailed continuation memo for Opus 4.8 and intentionally preserves the next +known failing case, the current baseline, the recent design decisions, and the +exact validation commands. -## Current Headline +## Immediate State + +The worktree was clean at `355e728` before this handoff edit. The latest code +checkpoint already committed the implementation work for +`taint_unify_mvars`; this handoff commit should modify only this file. + +Current headline: - Full Python `.sgrep` pattern sweep: 164 passed / 0 mismatched. - Full JavaScript `.sgrep` pattern sweep: 132 passed / 0 mismatched. - Promoted JavaScript pattern oracle: 91 passed / 0 mismatched. - Promoted Python pattern oracle: 116 passed / 0 mismatched. - Local oracle: 42 passed / 0 failed. -- Smoke suite: 302 tests, 302 passed, 0 failed. -- Broad same-basename upstream `tests/rules` sweep: 437 passed / 0 mismatched - / 0 Jerboa errors, with 3 packaged-Semgrep current errors. +- Smoke suite: 302 tests / 302 passed. +- Broad same-basename upstream `tests/rules` sweep: 437 passed / + 0 mismatched / 0 Jerboa errors, with 3 packaged-Semgrep current errors. - Upstream `tests/tainting_rules/js` sweep: 6 passed / 5 mismatched / - 0 Jerboa errors / 0 current errors. This improved from 5 passed / 6 - mismatched by closing `metavar_eq_simple`. + 0 Jerboa errors / 0 current errors. -Semgrep parity is not reached. Python and JavaScript pattern fixtures are clean, -but taint/dataflow, parser compatibility, rule validation, CLI behavior, output, -autofix, target selection, ignore handling, and broader upstream test corpora -remain open. +Semgrep parity is not reached. The cleanest active frontier is JavaScript taint +semantics under upstream `tests/tainting_rules/js`. -## Checkpoint Chain +## Repository Context -- `4e0eedf Cover JavaScript deep pattern fixtures`: closed the remaining - JavaScript `.sgrep` pattern fixture mismatches. Full JS pattern sweep became - 132 passed / 0 mismatched. -- `1d6987f Cover JavaScript taint throw reachability`: started the upstream JS - taint frontier, normalized `tests/tainting_rules` check-id prefixes, and fixed - unreachable sinks after `throw`. JS tainting-rule sweep became 5 passed / - 6 mismatched. -- This checkpoint: fixes propagated-source handling for - `options: taint_unify_mvars: true`. JS tainting-rule sweep is now 6 passed / - 5 mismatched. +The long-form port plan is [what.md](/Users/user/mine/jerboa-semgrep/what.md). +It is still useful for architecture and scope, but some status counts there are +older than this handoff. Prefer the numbers in this handoff for current +baselines. -## Files In This Checkpoint +Important local implementation files: -This checkpoint modifies: +- [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss) + is the main matcher, formula, and taint implementation. +- [lib/semgrep/scan.sls](/Users/user/mine/jerboa-semgrep/lib/semgrep/scan.sls) + is generated from `src/semgrep/scan.ss` and is tracked. +- [tests/smoke.ss](/Users/user/mine/jerboa-semgrep/tests/smoke.ss) is the + local smoke suite. +- [tests/oracle/upstream-sweep.sh](/Users/user/mine/jerboa-semgrep/tests/oracle/upstream-sweep.sh) + compares Jerboa against the current Semgrep oracle on upstream same-basename + rule/target pairs. +- [tests/oracle/normalize-findings.ss](/Users/user/mine/jerboa-semgrep/tests/oracle/normalize-findings.ss) + normalizes both outputs for parity comparisons. -```text -HANDOFF_OPUS_4_8.md -src/semgrep/scan.ss -lib/semgrep/scan.sls -src/.jerbuild-hashes -tests/smoke.ss -``` +Generated/tracked files: + +- `lib/**/*.sls` and `src/.jerbuild-hashes` are generated by `make build` or + `make test`. +- If `src/semgrep/scan.ss` changes, expect `lib/semgrep/scan.sls` and + `src/.jerbuild-hashes` to change too. Commit them together. + +Operational defaults: + +- Use `rg` for code discovery. +- Use `apply_patch` for manual edits. +- Do not revert unrelated user changes in a dirty worktree. +- Keep `SEMGREP_CURRENT=/Users/user/.local/bin/semgrep` in oracle commands. +- Use the sibling checkout under `/Users/user/mine/semgrep` as the upstream + fixture source. + +## Recent Checkpoint Chain + +`4e0eedf Cover JavaScript deep pattern fixtures` + +- Closed the remaining JavaScript `.sgrep` pattern fixture mismatches. +- Full JS pattern sweep became 132 passed / 0 mismatched. + +`1d6987f Cover JavaScript taint throw reachability` -`lib/semgrep/scan.sls` and `src/.jerbuild-hashes` are generated by -`make build`/`make test`, but they are tracked and must stay in the commit with -the matching `src/semgrep/scan.ss` changes. +- Started the upstream JS taint frontier. +- Normalized `tests/tainting_rules` check-id prefixes in the oracle normalizer. +- Fixed unreachable sinks after `throw`. +- JS tainting-rule sweep became 5 passed / 6 mismatched. -## What Changed +`355e728 Respect taint metavariable unification` -### Propagated Taint Unification +- Fixed propagated-source handling for + `options: taint_unify_mvars: true`. +- JS tainting-rule sweep became 6 passed / 5 mismatched. +- Added smoke coverage: + `scan JavaScript taint unifies propagated source metavariables`. -Semgrep's `taint_unify_mvars` keeps source metavariable bindings as constraints -after taint is assigned to another variable. In the upstream case: +## Latest Code Change Details + +The latest code checkpoint fixed `metavar_eq_simple`. The upstream shape was: ```javascript var source1 = get(A) @@ -78,43 +111,34 @@ sink(A, source3) // do not report sink(B, source3) // do not report ``` -Jerboa previously overreported the last three sinks. The root cause was not -that source bindings were lost; they were preserved. The problem was that token -reachability treated every source metavariable binding as tainted data. For -example, the state for `source2 = get(B)` contained both the carrier token -`source2` and the source constraint `$X = B`; the old token check let it reach -`sink(B, source3)` just because `B` appeared in the sink's first argument. - -The scanner now distinguishes those roles for `taint_unify_mvars`: - -- `taint-state-mvars-compatible?` checks sink metavariables against both the - current propagated source finding and the original source finding. -- `shared-finding-binding-names` identifies binding names shared by the source - and sink findings. -- `taint-unify-ignored-source-token-names` computes the shared binding names - that should be used only for unification. -- `source-token-in-sink/except?` and - `source-compatible-with-sink/except?` keep the carrier token behavior but - ignore shared unification-only binding names as tainted token carriers. +Jerboa had preserved the original source metavariable bindings, but it treated +every preserved source metavariable binding as if it were tainted data. For +example, the propagated state for `source2 = get(B)` contained both the carrier +token `source2` and the source constraint `$X = B`; the old token check let +that state reach `sink(B, source3)` just because `B` appeared in the sink's +first argument. -This keeps `source2` reaching `sink(B, source2)` while preventing `$X = B` from -making unrelated sinks tainted. +The scanner now separates carrier tokens from unification constraints for +`taint_unify_mvars`: -### Smoke Coverage +- `source-token-in-sink/except?` +- `source-compatible-with-sink/except?` +- `taint-state-mvars-compatible?` +- `shared-finding-binding-names` +- `taint-unify-ignored-source-token-names` -`tests/smoke.ss` adds: - -```text -scan JavaScript taint unifies propagated source metavariables -``` +Relevant locations in [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss): -It mirrors the upstream `metavar_eq_simple` shape and guards against treating -source metavariable constraints as carrier tokens. +- `source-token-in-sink/except?`: around line 30325. +- `taint-unify-mvars?`: around line 30463. +- `taint-state-mvars-compatible?`: around line 30471. +- `taint-unify-ignored-source-token-names`: around line 30493. +- Taint reachability uses those helpers around lines 31230 and 31279. -The prior smoke test from `1d6987f`, -`scan JavaScript taint ignores unreachable after throw`, remains in place. +If future taint work touches token reachability, rerun `metavar_eq_simple` +before trusting the result. -## Verification Run +## Verification Already Run All commands below were run from `/Users/user/mine/jerboa-semgrep`. @@ -124,13 +148,13 @@ Smoke: make test ``` -Result: +Result from the last code checkpoint: ```text 302 tests, 302 passed, 0 failed ``` -Focused upstream JS taint `metavar_eq_simple` case: +Focused upstream JS taint `metavar_eq_simple`: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ @@ -145,7 +169,7 @@ Result: upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared ``` -Focused regression guard for broader taint-rule behavior: +Focused regression guard: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ @@ -160,157 +184,412 @@ Result: upstream-sweep: 2 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 2 compared ``` -Full upstream JS tainting-rule subdirectory: +Local oracle: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep make oracle +``` + +Result: + +```text +oracle: 42 passed, 0 failed +``` + +Broad same-basename upstream `tests/rules` sweep: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ -LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules \ +LIST_MISMATCHES=1 MAX_DIFFS=40 tests/oracle/upstream-sweep.sh ``` Result: ```text -upstream-sweep: 6 passed, 5 mismatched, 0 jerboa errors, 0 current errors, 11 compared +upstream-sweep: 437 passed, 0 mismatched, 0 jerboa errors, 3 current errors, 440 compared ``` -Local oracle: +The 3 current-side errors are packaged-Semgrep schema failures for +`anywhere_global`, `anywhere_include`, and `anywhere_metavar`; they are not +Jerboa mismatches. + +Fresh handoff confirmation for `sanitized_by_side_effect`: ```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep make oracle +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(sanitized_by_side_effect)$' LIST_MISMATCHES=1 MAX_DIFFS=180 \ +tests/oracle/upstream-sweep.sh ``` Result: ```text -oracle: 42 passed, 0 failed +upstream-sweep: 0 passed, 1 mismatched, 0 jerboa errors, 0 current errors, 1 compared ``` -Broad same-basename upstream `tests/rules` sweep: +The missing Jerboa finding is Semgrep's line 31 finding in +`sanitized_by_side_effect.js`. + +Fresh handoff confirmation for full JS tainting rules: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules \ -LIST_MISMATCHES=1 MAX_DIFFS=40 tests/oracle/upstream-sweep.sh +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +LIST_MISMATCHES=1 MAX_DIFFS=80 tests/oracle/upstream-sweep.sh ``` Result: ```text -upstream-sweep: 437 passed, 0 mismatched, 0 jerboa errors, 3 current errors, 440 compared +upstream-sweep: 6 passed, 5 mismatched, 0 jerboa errors, 0 current errors, 11 compared ``` -The three current-side errors are packaged-Semgrep schema failures for -`anywhere_global`, `anywhere_include`, and `anywhere_metavar`, not Jerboa -mismatches. +## Remaining JS Taint Mismatches + +The 5 remaining mismatches under `/Users/user/mine/semgrep/tests/tainting_rules/js` +are: -## Remaining `tests/tainting_rules/js` Mismatches +1. `await` -The `throw` and `metavar_eq_simple` mismatches are fixed. The five remaining JS -tainting-rule mismatches are real semantic gaps: +Semgrep reports two string-concat SQL sinks under Express-style arrow +callbacks; Jerboa reports zero. Likely gap: `pattern-inside: +function ... ($REQ, $RES) {...}` compatibility with arrow callback parameters, +and/or taint through `await` plus string concatenation. -1. `await`: Semgrep reports two string-concat SQL sinks under Express-style - arrow callbacks; Jerboa reports zero. Likely gap: source matching with - `pattern-inside: function ... ($REQ, $RES) {...}` does not model arrow - callback parameters as compatible sources, and/or the string-concat sink is - not connected through `await`. +2. `eslint_obj_inj` -2. `eslint_obj_inj`: Semgrep reports `return o[c]` after `c` is conditionally - assigned from tainted parameter `x`; Jerboa misses that branch merge. Likely - gap: path-insensitive propagation from conditional assignments such as - `if (...) c = x else c = 1`. +Semgrep reports `return o[c]` after `c` is conditionally assigned from tainted +parameter `x`; Jerboa misses the branch-merged taint. Likely gap: +path-insensitive propagation from conditional assignments such as +`if (...) c = x else c = 1`. -3. `metavar_eq_conditional`: Semgrep keeps both possible taint bindings from - `cond ? get(A) : get(B)` and reports both `A` and `B` messages at both - sinks; Jerboa keeps only the locally matching binding at each sink. Likely - gap: conditional-expression sources need multiple alternative taint - states/metavariable environments. +3. `metavar_eq_conditional` -4. `sanitized_by_side_effect`: Semgrep still reports `jwt.decode(token, true)` - when `jwt.verify(token2, key)` sanitized a different token. Jerboa - over-sanitizes. Likely gap: by-side-effect sanitizer identity should require - compatible token/source binding, not just same function scope and ordering. +Semgrep keeps both possible taint bindings from `cond ? get(A) : get(B)` and +reports both `A` and `B` messages at both sinks. Jerboa keeps only the locally +matching binding at each sink. Likely gap: conditional-expression sources need +multiple alternative taint states/metavariable environments. -5. `simpl_nodejs_eval`: Jerboa misses the direct concat sink - `s.run('lol(' + req.query.userInput + ')', cb)` and has a one-column - overlong range on the template-literal sink. Likely gaps: direct string - concatenation taint into a focused sink expression, plus template literal - range trimming. +4. `sanitized_by_side_effect` -## Recommended Next Work +Semgrep reports `jwt.decode(token, true)` at line 31 because only `token2` was +verified in that function. Jerboa misses that finding by over-sanitizing +`token`. This is the best next target because the root cause is now fairly +well isolated. -The best next target is probably `await` or `sanitized_by_side_effect`. +5. `simpl_nodejs_eval` -For `await`, start with: +Jerboa misses the direct concat sink +`s.run('lol(' + req.query.userInput + ')', cb)` and has a one-column overlong +range on the template-literal sink. Likely gaps: direct string concatenation +taint into a focused sink expression, plus template literal range trimming. + +## Best Next Target: `sanitized_by_side_effect` + +Start with this command: ```sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ -CASE_REGEX='^(await)$' LIST_MISMATCHES=1 MAX_DIFFS=120 \ +CASE_REGEX='^(sanitized_by_side_effect)$' LIST_MISMATCHES=1 MAX_DIFFS=180 \ tests/oracle/upstream-sweep.sh ``` -Inspect how `pattern-inside: function ... ($REQ, $RES) {...}` is matched for -arrow callbacks like `(req, res) => { ... }`. If you touch source/sink token -logic, rerun `metavar_eq_simple` because this checkpoint depends on shared -metavariables not doubling as carrier tokens. +Current diff: -For `sanitized_by_side_effect`, start with: +```diff +- (finding "jwt-decode-without-verify" ".../sanitized_by_side_effect.js" 31 20 728 31 25 733 ...) + (finding "jwt-decode-without-verify" ".../sanitized_by_side_effect.js" 6 16 144 6 21 149 ...) +``` -```sh -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ -UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ -CASE_REGEX='^(sanitized_by_side_effect)$' LIST_MISMATCHES=1 MAX_DIFFS=120 \ -tests/oracle/upstream-sweep.sh +Upstream rule: + +```yaml +pattern-sanitizers: + - patterns: + - pattern-inside: | + $JWT = require('jsonwebtoken'); + ... + - pattern-inside: | + $JWT.verify($TOKEN, ...) + ... + - pattern: $TOKEN +pattern-sinks: + - patterns: + - pattern-inside: | + $JWT = require('jsonwebtoken'); + ... + - pattern: $JWT.decode($TOKEN, ...) + - pattern: $TOKEN +pattern-sources: + - pattern: $TOKEN +``` + +Relevant target shape: + +```javascript +const jwt = require('jsonwebtoken'); + +token = getToken(); +if (jwt.decode(token, true).param === true) { ... } // report + +function ok(token, key) { + jwt.verify(token, key); + if (jwt.decode(token, true).param === true) { ... } // ok +} + +const ok2 = (token, key) => { + jwt.verify(token, key); + if (jwt.decode(token, true).param === true) { ... } // ok +}; + +function bad_different_token(token, key) { + token2 = getToken(); + jwt.verify(token2, key); + if (jwt.decode(token, true).param === true) { ... } // report +} +``` + +Observed behavior: + +- Semgrep reports lines 6 and 31. +- Jerboa reports only line 6. +- Therefore Jerboa is incorrectly treating some prior `jwt.verify(...)` as a + sanitizer for the later `jwt.decode(token, ...)` at line 31. + +### Important Investigation Already Done + +Do not start from the assumption that by-side-effect sanitizer identity alone is +the issue. The stronger evidence points to the JavaScript trailing-ellipsis +`pattern-inside` fallback being too broad. + +A direct scan of the full taint rule only outputs line 6 in Jerboa. + +A temporary regular search rule equivalent to the sanitizer formula: + +```yaml +rules: + - id: sanitizer-only + languages: [javascript, typescript] + message: sanitizer $TOKEN + severity: WARNING + patterns: + - pattern-inside: | + $JWT = require('jsonwebtoken'); + ... + - pattern-inside: | + $JWT.verify($TOKEN, ...) + ... + - pattern: $TOKEN +``` + +matched `token` at line 31 even though the only verifier in +`bad_different_token` is `jwt.verify(token2, key)`. + +A narrower temporary rule: + +```yaml +rules: + - id: verify-inside-token + languages: [javascript, typescript] + message: inside token $TOKEN + severity: WARNING + patterns: + - pattern-inside: | + $JWT.verify($TOKEN, ...) + ... + - pattern: $TOKEN +``` + +matched line 31 `token` with `$JWT` effectively coming from a previous verify +in another function. That means this fallback is treating +`$JWT.verify($TOKEN, ...)\n...` as "any later node in the file" rather than +"inside the lexical region following this verify". + +Direct scan of the base pattern: + +```yaml +pattern: $JWT.verify($TOKEN, ...) +``` + +does find line 28 with `$TOKEN = token2`, and the binding merge should reject +that candidate when the sink token is `token`. The incorrect line 31 sanitizer +is likely coming from the earlier line 19 verify in `ok2`, because the +after-clause fallback checks ordering only. + +### Probable Patch Point + +Relevant code in [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss): + +- `find-matching-close-brace`: around line 2331. +- `javascript-deep-sequence-scope-end`: around line 6225. +- `trailing-ellipsis-base-entry`: around line 21220. +- `inside-after-clause-apply`: around line 21230. +- `patterns-clause-apply*` uses the trailing-ellipsis fallback around lines + 29256 and 29273. + +Current shape of `inside-after-clause-apply`: + +```scheme +(def (inside-after-clause-apply rule candidate findings source) + (let loop ([xs findings] [best #f] [best-start #f] [best-bindings #f]) + ... + (let* ([finding (car xs)] + [before? (< (finding-end-offset finding) + (finding-start-offset candidate))] + [merged + (and before? + (merge-binding-list + (finding-metavars candidate) + (finding-metavars finding)))] + [start (and merged (finding-start-offset finding))]) + ...))) +``` + +That helper needs a lexical/scope bound, not just `before?`. + +Suggested implementation direction: + +- Add a helper that returns the end of the smallest enclosing JavaScript block + for the base finding, or EOF if there is no enclosing block. +- Use it in `inside-after-clause-apply` so a candidate must be after the base + finding and before that lexical end. +- For line 19 in `ok2`, the lexical end should be the `};` around line 24, so + it must not sanitize line 31. +- For top-level `const jwt = require('jsonwebtoken'); ...`, there is no + enclosing block, so EOF remains correct. This preserves the first + `pattern-inside` clause in the upstream rule. + +A robust enough helper can scan from start of file to the base finding start, +track `{` positions outside comments and strings, then call +`find-matching-close-brace` on the top stack entry. Existing code already has +small brace scanners; do not add a full parser unless necessary. + +One possible shape: + +```scheme +(def (javascript-enclosing-block-end source offset) + ;; Scan to offset, tracking unmatched open braces outside strings/comments. + ;; If stack is non-empty, return (find-matching-close-brace source top). + ;; Otherwise return (string-length source). + ...) + +(def (inside-after-clause-apply rule candidate findings source) + ... + (let* ([before? (< (finding-end-offset finding) + (finding-start-offset candidate))] + [scope-end (and before? + (javascript-enclosing-block-end + source + (finding-start-offset finding)))] + [within? (and before? + (<= (finding-end-offset candidate) scope-end))] + [merged (and within? + (merge-binding-list ...))]) + ...)) ``` -The likely fix is to make by-side-effect sanitizer blocking check the same -token/source identity, so `jwt.verify(token2, key)` does not sanitize -`jwt.decode(token, true)`. +Use candidate end rather than candidate start for the containment check, since +Semgrep `pattern-inside` means the candidate match should be contained by the +inside region. + +Risk to watch: a naive nearest-brace helper may pick an object literal instead +of a block. That is probably still safer than file-wide leakage for this +fallback, but the broad `tests/rules` sweep should be rerun. + +### Smoke Test to Add With That Fix + +Add a focused smoke test near the other JavaScript taint sanitizer tests. A +minimal shape: + +```scheme +(test-case "scan JavaScript taint sanitizer after verify stays in function scope" + (let* ([taint-config + "rules:\n - id: demo.taint.jwt.verify\n mode: taint\n languages: [javascript]\n message: jwt token\n severity: WARNING\n pattern-sources:\n - pattern: $TOKEN\n pattern-sanitizers:\n - patterns:\n - pattern-inside: |\n $JWT.verify($TOKEN, ...)\n ...\n - pattern: $TOKEN\n pattern-sinks:\n - patterns:\n - pattern: $JWT.decode($TOKEN, ...)\n - pattern: $TOKEN\n"] + [findings + (scan-config-string + taint-config + "javascript" + "demo.js" + "function ok(token, key) {\n jwt.verify(token, key);\n jwt.decode(token, true);\n}\nfunction bad(token, key) {\n token2 = getToken();\n jwt.verify(token2, key);\n jwt.decode(token, true);\n}\n")]) + (check (length findings) => 1) + (check (finding-start-line (car findings)) => 8))) +``` + +If that minimal source pattern is too broad in practice, include the +`require('jsonwebtoken')` inside-clause from the upstream rule and assert only +the line 8 decode token survives. + +### Required Validation After This Patch -After any taint change, run at least: +Run these before committing a fix: ```sh make test -SEMGREP_CURRENT=/Users/user/.local/bin/semgrep make oracle +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(sanitized_by_side_effect)$' LIST_MISMATCHES=1 MAX_DIFFS=180 \ +tests/oracle/upstream-sweep.sh SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ LIST_MISMATCHES=1 MAX_DIFFS=160 tests/oracle/upstream-sweep.sh -``` - -For broader safety, rerun: - -```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep make oracle SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/rules \ LIST_MISMATCHES=1 MAX_DIFFS=40 tests/oracle/upstream-sweep.sh +git diff --check ``` -It is slow and quiet for long periods, but it caught an earlier overbroad -return-reachability implementation. +Expected improvement if the fix is right: -## Operational Notes +- Focused `sanitized_by_side_effect`: 1 passed / 0 mismatched. +- Full JS tainting rules: 7 passed / 4 mismatched. +- Smoke: one more test, all passing. +- Broad `tests/rules`: still 0 mismatches, with the same 3 current-side + packaged-Semgrep errors unless upstream/current behavior changes. -- Use `rg` first for source discovery. -- Use `apply_patch` for manual edits. -- Do not revert unrelated user changes in a dirty tree. -- `make build` can pass even when scanner logic is behaviorally wrong. Always - run a focused oracle for newly touched paths. -- Keep `SEMGREP_CURRENT=/Users/user/.local/bin/semgrep` set for comparisons - against packaged Semgrep. -- The sibling upstream checkout at `/Users/user/mine/semgrep` provides the - fixture and rule corpora. -- The upstream sweep harness compares same-basename rule/target pairs in one - directory at a time. Use `UPSTREAM_RULE_DIR=.../tests/tainting_rules/js` for - the active JS taint frontier. -- `tests/oracle/normalize-findings.ss` already strips known - `tests.tainting_rules.*` check-id prefixes. If you broaden it again, prefer - explicit known prefixes over arbitrary suffix trimming. - -Expected post-commit state: +## Alternative Next Targets + +If `sanitized_by_side_effect` unexpectedly fans out, the next most tractable +target is probably `await`: ```sh -git status --short --branch +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(await)$' LIST_MISMATCHES=1 MAX_DIFFS=120 \ +tests/oracle/upstream-sweep.sh ``` -should show a clean `main` branch. +Investigate whether a `pattern-inside: function ... ($REQ, $RES) {...}` source +matches arrow callbacks like `(req, res) => { ... }`, and then check whether +the string-concat sink is connected through `await`. + +If touching conditional-source logic, keep `metavar_eq_simple` and +`metavar_eq_conditional` separate in your head: + +- `metavar_eq_simple` is now fixed by not treating source unification bindings + as tainted carrier tokens. +- `metavar_eq_conditional` still needs multiple alternative source binding + states from a conditional expression. + +## Commit Hygiene + +For a code checkpoint: + +1. Confirm `git status --short --branch`. +2. Make source edits in `src/**/*.ss` with `apply_patch`. +3. Run `make test`; let generated `lib/**/*.sls` and `.jerbuild-hashes` update. +4. Run focused upstream oracle for the touched case. +5. Run full JS taint sweep and local oracle. +6. Run broad `tests/rules` sweep for changes to pattern-inside, formula merge, + or taint reachability. +7. Run `git diff --check`. +8. Update this handoff with new baseline and remaining mismatches. +9. Commit all tracked changes together. + +Do not mark the broader Semgrep parity goal complete until the project has real +Semgrep-compatible behavior across parser coverage, rule parsing, search mode, +taint/dataflow, target selection, ignore handling, CLI behavior, output, and +the upstream oracle corpus.