Trim JavaScript template metavariable ranges
ober
3213521c9456f75195194039f3ae4bc4a9e07b73
--- a/HANDOFF_OPUS_4_8.md +++ b/HANDOFF_OPUS_4_8.md @@ -1,24 +1,26 @@ # Opus 4.8 Handoff: jerboa-semgrep Semgrep Parity -Date: 2026-05-30 07:02 MDT +Date: 2026-05-30 08:10 MDT Workspace: `/Users/user/mine/jerboa-semgrep` Sibling upstream Semgrep checkout: `/Users/user/mine/semgrep` Packaged Semgrep oracle: `/Users/user/.local/bin/semgrep` Branch: `main` Base HEAD before this checkpoint: -`5abcf3d Scope JavaScript trailing inside matches` +`acca729 Cover JavaScript SQL concat taint` The user wants the pure Jerboa Semgrep port carried forward until it reaches Semgrep parity. Do not treat this handoff as completion. This checkpoint closes -the upstream JavaScript taint `await` mismatch by matching Semgrep's -associative string-concat sink behavior for `"$SQLSTR" + $EXPR`, and it also -fixes the direct concat finding in `simpl_nodejs_eval`. +the remaining upstream JavaScript taint `simpl_nodejs_eval` mismatch by +matching Semgrep's JavaScript template-string metavariable ranges: include the +opening backtick but exclude the closing backtick. ## Immediate State -The worktree was clean at `5abcf3d` before this checkpoint. The implementation -change is in `src/semgrep/scan.ss`; `lib/semgrep/scan.sls` and -`src/.jerbuild-hashes` were regenerated by `make test`. +The worktree was clean at `acca729` before this checkpoint. The implementation +change is in `src/semgrep/scan.ss` and +`src/semgrep/match/structural.ss`; `lib/semgrep/scan.sls`, +`lib/semgrep/match/structural.sls`, and `src/.jerbuild-hashes` were +regenerated by `make test`. Current headline: @@ -27,10 +29,10 @@ Current headline: - Promoted JavaScript pattern oracle: 91 passed / 0 mismatched. - Promoted Python pattern oracle: 116 passed / 0 mismatched. - Local oracle: 42 passed / 0 failed. -- Smoke suite: 304 tests / 304 passed. +- Smoke suite: 305 tests / 305 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: 8 passed / 3 mismatched / +- Upstream `tests/tainting_rules/js` sweep: 9 passed / 2 mismatched / 0 Jerboa errors / 0 current errors. Semgrep parity is not reached. The cleanest active frontier is JavaScript taint @@ -47,6 +49,8 @@ Important local implementation files: - [src/semgrep/scan.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/scan.ss) is the main matcher, formula, and taint implementation. +- [src/semgrep/match/structural.ss](/Users/user/mine/jerboa-semgrep/src/semgrep/match/structural.ss) + owns structural AST matching and metavariable bindings. - [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 @@ -61,7 +65,7 @@ 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 +- If `src/**/*.ss` changes, expect the matching `lib/**/*.sls` files and `src/.jerbuild-hashes` to change too. Commit them together. Operational defaults: @@ -111,7 +115,7 @@ Operational defaults: - Closes upstream JS tainting-rule case `sanitized_by_side_effect`. - JS tainting-rule sweep is now 7 passed / 4 mismatched. -This checkpoint: +`acca729 Cover JavaScript SQL concat taint` - Adds a JavaScript fallback for quoted-string concat patterns shaped like `"$SQLSTR" + $EXPR`, preserving `$SQLSTR` and `$EXPR` bindings. @@ -125,13 +129,29 @@ This checkpoint: `scan JavaScript taint Express request reaches SQL string concat`. - Closes upstream JS tainting-rule case `await` and the missing direct concat finding in `simpl_nodejs_eval`. -- JS tainting-rule sweep is now 8 passed / 3 mismatched. +- JS tainting-rule sweep became 8 passed / 3 mismatched. + +This checkpoint: + +- Adds `template-finding-end-byte` in `src/semgrep/scan.ss` so structural + findings on JavaScript `template_string` nodes end before the closing + backtick. +- Adjusts `bind-metavariable` in `src/semgrep/match/structural.ss` so + unquoted metavariables bound to `template_string` nodes use the same trimmed + end range and binding text. +- Keeps the existing quoted-template content binding behavior intact. +- Adds smoke coverage: + `scan JavaScript template metavariable ranges omit closing backtick`. +- Closes upstream JS tainting-rule case `simpl_nodejs_eval`. +- JS tainting-rule sweep is now 9 passed / 2 mismatched. This checkpoint modifies: ```text HANDOFF_OPUS_4_8.md +src/semgrep/match/structural.ss src/semgrep/scan.ss +lib/semgrep/match/structural.sls lib/semgrep/scan.sls src/.jerbuild-hashes tests/smoke.ss @@ -139,6 +159,39 @@ tests/smoke.ss ## Latest Code Change Details +### JavaScript Template String Metavariable Ranges + +Semgrep reports a JavaScript template-string metavariable range from the +opening backtick through the template content, excluding the closing backtick. +For example, with: + +```yaml +patterns: + - pattern-inside: foo($X) + - pattern: $X +``` + +and: + +```javascript +foo(`lol(${req.query.userInput})`); +``` + +Semgrep reports columns 5 to 33, not 5 to 34. Jerboa previously let structural +`$X` bind to the entire `template_string` node, so both `patterns` intersections +and `focus-metavariable` included the closing backtick. + +The scanner now trims JavaScript `template_string` structural findings to +`node-end-byte - 1`, while the structural matcher trims unquoted template-string +metavariable bindings the same way. This fixed `simpl_nodejs_eval`, whose final +mismatch was the sink on: + +```javascript +s.run(`lol(${req.query.userInput})`, cb); +``` + +Semgrep expects the focused sink to stop before the closing backtick. + ### JavaScript SQL String Concat Taint The upstream `await` taint rule uses this sink: @@ -177,9 +230,9 @@ if the source occurrence is contained inside a sink whose text includes `+`, that source can reach the sink. This avoids the broader token-source change that overreported typestate cases. -This closes `await` and also adds the missing direct concat report for -`simpl_nodejs_eval`; that case still has a one-column template-literal range -mismatch. +This closed `await` and also added the missing direct concat report for +`simpl_nodejs_eval`; the remaining template-literal range mismatch was closed +by the later template-string range checkpoint. ### Scoped Trailing-Ellipsis `pattern-inside` @@ -290,7 +343,7 @@ make test Result: ```text -304 tests, 304 passed, 0 failed +305 tests, 305 passed, 0 failed ``` Focused upstream JS taint `metavar_eq_simple`: @@ -383,6 +436,21 @@ Result: upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared ``` +Focused upstream JS taint `simpl_nodejs_eval`: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(simpl_nodejs_eval)$' LIST_MISMATCHES=1 MAX_DIFFS=160 \ +tests/oracle/upstream-sweep.sh +``` + +Result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + Full JS tainting-rule subdirectory: ```sh @@ -394,12 +462,12 @@ LIST_MISMATCHES=1 MAX_DIFFS=80 tests/oracle/upstream-sweep.sh Result: ```text -upstream-sweep: 8 passed, 3 mismatched, 0 jerboa errors, 0 current errors, 11 compared +upstream-sweep: 9 passed, 2 mismatched, 0 jerboa errors, 0 current errors, 11 compared ``` ## Remaining JS Taint Mismatches -The 3 remaining mismatches under `/Users/user/mine/semgrep/tests/tainting_rules/js` +The 2 remaining mismatches under `/Users/user/mine/semgrep/tests/tainting_rules/js` are: 1. `eslint_obj_inj` @@ -416,13 +484,6 @@ 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. `simpl_nodejs_eval` - -The direct concat sink is now fixed. The remaining mismatch is a one-column -overlong range on the template-literal sink at line 26. Likely gap: template -literal range trimming should exclude the closing backtick for this focused -sink shape. - ## Completed Target: `sanitized_by_side_effect` Verification command: @@ -688,21 +749,91 @@ Actual improvement: - Broad `tests/rules`: still 0 mismatches, with the same 3 current-side packaged-Semgrep errors unless upstream/current behavior changes. -## Recommended Next Target +## Completed Target: `simpl_nodejs_eval` + +Verification command: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(simpl_nodejs_eval)$' LIST_MISMATCHES=1 MAX_DIFFS=160 \ +tests/oracle/upstream-sweep.sh +``` + +Result: + +```text +upstream-sweep: 1 passed, 0 mismatched, 0 jerboa errors, 0 current errors, 1 compared +``` + +The direct concat report at line 11 was fixed in `acca729`. This checkpoint +fixed the remaining template-literal range at line 26 by trimming structural +template-string metavariable ranges before the closing backtick. + +### Smoke Test Added + +Added a structural range smoke test near the other JavaScript template tests: + +```scheme +(test-case "scan JavaScript template metavariable ranges omit closing backtick" + ...) +``` + +It covers both: + +- `patterns` with `pattern-inside: foo($X)` plus `pattern: $X`. +- `focus-metavariable: $X` on `pattern: foo($X)`. + +Both are expected to report end column 33 for: + +```javascript +foo(`lol(${req.query.userInput})`); +``` + +### Validation Run For This Patch -The next most tractable target is probably the remaining `simpl_nodejs_eval` -template-literal range mismatch: +These were run before committing the fix: ```sh +make test SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ CASE_REGEX='^(simpl_nodejs_eval)$' LIST_MISMATCHES=1 MAX_DIFFS=160 \ 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 +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 +``` + +Actual improvement: + +- Focused `simpl_nodejs_eval`: 1 passed / 0 mismatched. +- Full JS tainting rules: 9 passed / 2 mismatched. +- Smoke: one more test, all passing. +- Broad `tests/rules`: still 0 mismatches and 0 Jerboa errors, with the same + 3 current-side packaged-Semgrep errors. + +## Recommended Next Target + +The next target is probably `eslint_obj_inj`, because the missing report is a +single taint-flow gap: + +```sh +SEMGREP_CURRENT=/Users/user/.local/bin/semgrep \ +UPSTREAM_RULE_DIR=/Users/user/mine/semgrep/tests/tainting_rules/js \ +CASE_REGEX='^(eslint_obj_inj)$' LIST_MISMATCHES=1 MAX_DIFFS=160 \ +tests/oracle/upstream-sweep.sh ``` -The direct concat report at line 11 is now present, so focus only on the -template-literal range at line 26. Semgrep ends at column 39 / offset 641; -Jerboa ends at column 40 / offset 642, including one extra character. +Semgrep reports line 21 `return o[c]` after `c` is conditionally assigned from +tainted parameter `x`; Jerboa currently reports only the simpler flows. The +likely fix is to model branch-merged assignments for JavaScript taint, not to +globally loosen source/sink containment. If touching conditional-source logic, keep `metavar_eq_simple` and `metavar_eq_conditional` separate in your head: --- a/lib/semgrep/match/structural.sls +++ b/lib/semgrep/match/structural.sls @@ -585,10 +585,29 @@ start-line start-col end-line end-col)) bindings)]))) (def (bind-metavariable name target bindings) - (bind-metavariable-value name (node-text target) (node-start-byte target) - (node-end-byte target) (+ (node-start-row target) 1) - (+ (node-start-column target) 1) (+ (node-end-row target) 1) - (+ (node-end-column target) 1) bindings)) + (let* ([text (node-text target)] + [trim-template? (and (string=? + (node-type target) + "template_string") + (> (string-length text) 0) + (> (node-end-byte target) + (node-start-byte target)))] + [binding-text (if trim-template? + (string-slice + text + 0 + (- (string-length text) 1)) + text)] + [end-byte (if trim-template? + (- (node-end-byte target) 1) + (node-end-byte target))] + [end-col (if trim-template? + (node-end-column target) + (+ (node-end-column target) 1))]) + (bind-metavariable-value name binding-text (node-start-byte target) end-byte + (+ (node-start-row target) 1) + (+ (node-start-column target) 1) (+ (node-end-row target) 1) + end-col bindings))) (def (sequence-text target start-index end-index) (let loop ([i start-index] [acc ""]) (if (= i end-index) --- a/lib/semgrep/scan.sls +++ b/lib/semgrep/scan.sls @@ -413,6 +413,17 @@ content-end)) binding (loop (cdr xs))))))))) + (def (template-finding-end-byte node extra) + (let ([content-binding (template-content-binding + node + extra)]) + (cond + [content-binding + (metavariable-binding-end-byte content-binding)] + [(and (string=? (node-type node) "template_string") + (> (node-end-byte node) (node-start-byte node))) + (- (node-end-byte node) 1)] + [else (node-end-byte node)]))) (def (semicolon-trimmable-node? node) (or (string=? (node-type node) "variable_declaration") (string=? (node-type node) "lexical_declaration") @@ -429,11 +440,7 @@ source raw-start)] [start (source-index->semgrep-offset source start-index)] - [template-binding (template-content-binding node extra)] - [raw-end-byte (if template-binding - (metavariable-binding-end-byte - template-binding) - (node-end-byte node))] + [raw-end-byte (template-finding-end-byte node extra)] [raw-end-index (tree-byte-offset->source-index source raw-end-byte)] --- a/src/.jerbuild-hashes +++ b/src/.jerbuild-hashes @@ -3,11 +3,11 @@ ("src/semgrep/output/json.ss" . "293881CFA2ADB7BC") ("src/semgrep/lang.ss" . "6982E07679D20836") ("src/semgrep/parse/parse-target.ss" . "E74854DDDACF6BA") - ("src/semgrep/scan.ss" . "16C6606401DEC6D9") - ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") + ("src/semgrep/scan.ss" . "23B1556A1042F11C") ("src/semgrep/output/text.ss" . "BE476CB84B807FBA") - ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/fix.ss" . "2E5B65B1FEF3B2B1") ("src/semgrep/schema/lang.ss" . "CAE2CA859C9A9FD0") - ("src/semgrep/match/structural.ss" . "F7B63A9A6FA028B") + ("src/semgrep/rule.ss" . "E12C108153C181FA") + ("src/semgrep/match/structural.ss" . "6FE77014EE9FDCE4") ("src/semgrep/main.ss" . "A4EC9E7F2A09D25E") ("src/semgrep/cli.ss" . "EBDC4B1DAD3F13CC")) --- a/src/semgrep/match/structural.ss +++ b/src/semgrep/match/structural.ss @@ -616,16 +616,33 @@ bindings)]))) (def (bind-metavariable name target bindings) - (bind-metavariable-value - name - (node-text target) - (node-start-byte target) - (node-end-byte target) - (+ (node-start-row target) 1) - (+ (node-start-column target) 1) - (+ (node-end-row target) 1) - (+ (node-end-column target) 1) - bindings)) + (let* ([text (node-text target)] + [trim-template? + (and (string=? (node-type target) "template_string") + (> (string-length text) 0) + (> (node-end-byte target) (node-start-byte target)))] + [binding-text + (if trim-template? + (string-slice text 0 (- (string-length text) 1)) + text)] + [end-byte + (if trim-template? + (- (node-end-byte target) 1) + (node-end-byte target))] + [end-col + (if trim-template? + (node-end-column target) + (+ (node-end-column target) 1))]) + (bind-metavariable-value + name + binding-text + (node-start-byte target) + end-byte + (+ (node-start-row target) 1) + (+ (node-start-column target) 1) + (+ (node-end-row target) 1) + end-col + bindings))) (def (sequence-text target start-index end-index) (let loop ([i start-index] [acc ""]) --- a/src/semgrep/scan.ss +++ b/src/semgrep/scan.ss @@ -465,6 +465,15 @@ binding (loop (cdr xs))))))))) +(def (template-finding-end-byte node extra) + (let ([content-binding (template-content-binding node extra)]) + (cond + [content-binding (metavariable-binding-end-byte content-binding)] + [(and (string=? (node-type node) "template_string") + (> (node-end-byte node) (node-start-byte node))) + (- (node-end-byte node) 1)] + [else (node-end-byte node)]))) + (def (semicolon-trimmable-node? node) (or (string=? (node-type node) "variable_declaration") (string=? (node-type node) "lexical_declaration") @@ -481,10 +490,7 @@ (let* ([raw-start (node-start-byte node)] [start-index (tree-byte-offset->source-index source raw-start)] [start (source-index->semgrep-offset source start-index)] - [template-binding (template-content-binding node extra)] - [raw-end-byte (if template-binding - (metavariable-binding-end-byte template-binding) - (node-end-byte node))] + [raw-end-byte (template-finding-end-byte node extra)] [raw-end-index (tree-byte-offset->source-index source raw-end-byte)] [end-index (trim-node-finding-end source node start-index raw-end-index)] [end (source-index->semgrep-offset source end-index)]) --- a/tests/smoke.ss +++ b/tests/smoke.ss @@ -1489,6 +1489,30 @@ (check (finding-start-col (car call-findings)) => 7) (check (finding-end-col (car call-findings)) => 65))) +(test-case "scan JavaScript template metavariable ranges omit closing backtick" + (let* ([inside-config + "rules:\n - id: demo.js.template.inside.metavar\n languages: [javascript]\n message: template inside\n severity: WARNING\n patterns:\n - pattern-inside: foo($X)\n - pattern: $X\n"] + [inside-findings + (scan-config-string + inside-config + "javascript" + "demo.js" + "foo(`lol(${req.query.userInput})`);\n")] + [focus-config + "rules:\n - id: demo.js.template.focus.metavar\n languages: [javascript]\n message: template focus\n severity: WARNING\n patterns:\n - pattern: foo($X)\n - focus-metavariable: $X\n"] + [focus-findings + (scan-config-string + focus-config + "javascript" + "demo.js" + "foo(`lol(${req.query.userInput})`);\n")]) + (check (length inside-findings) => 1) + (check (finding-start-col (car inside-findings)) => 5) + (check (finding-end-col (car inside-findings)) => 33) + (check (length focus-findings) => 1) + (check (finding-start-col (car focus-findings)) => 5) + (check (finding-end-col (car focus-findings)) => 33))) + (test-case "scan TypeScript nested sequence pattern-inside" (let* ([ts-config "rules:\n - id: demo.ts.sequence.inside\n languages: [typescript]\n message: nested baz\n severity: WARNING\n patterns:\n - pattern: baz()\n - pattern-inside: |\n foo();\n ...\n $X = baz();\n - pattern-not-inside: |\n foo();\n ...\n bar();\n ...\n $X = baz();\n"]