diff --git a/contracts/linter-lexer-context-v1.yaml b/contracts/linter-lexer-context-v1.yaml index bbe6ba018..07c98e41b 100644 --- a/contracts/linter-lexer-context-v1.yaml +++ b/contracts/linter-lexer-context-v1.yaml @@ -29,6 +29,7 @@ metadata: - 'GH-262: SC2046 fires in three contexts where the shell performs no field splitting (assignment RHS, inside double quotes, case word)' - 'GH-235: SC1078 - an apostrophe inside a double-quoted string opens a single quote (fixed; pinned)' - 'GH-258: SC1140 - a trailing # comment after a [ ] test is an unexpected token (fixed; pinned)' + - 'GH-439: SC2075 - a backslash before the closing quote of ''\\'' or ''\'' is read as an escaped quote, so the string is paired with the next single-quoted string on the line' - 'GH-226, GH-272: linter::quoting resolves quoting once and hands QUOTE_SENSITIVE_RULES a masked copy' - 'GH-228: linter::shell_words - a command substitution opens a fresh quoting context (POSIX 2.6.3); already treats $(( )) as no expansion (test_SW_020) and honours backslash escapes inside double quotes' - 'docs/audit/quality-report09-2026.md section 4: the lexer reads non-shell text as shell' @@ -139,6 +140,7 @@ invariants: - 'bashrs corpus run over the full 17,942-entry corpus scores no worse than the 7.0.2 baseline (84.4/100 B, 333 failures)' - 'SC2105 processes loop-start keywords, done and break/continue in the byte order they occur on a physical line, not by summing a whole line''s keyword counts before checking break/continue (PMAT-244); a bare break/continue outside any loop still fires' - 'shell_words::Expansion covers command substitutions ($( ), nested $( $( ) ), and backticks, including inside double quotes) as well as $NAME/${NAME}; SC2046 has no private quote-aware scanner of its own (PMAT-250)' +- 'SC2075 never reports a single-quoted string whose content ends in a backslash when the closing quote is followed by a space, a shell operator, another quote or the end of the line - inside ''...'' a backslash is literal and the next quote closes (GH-439); it still reports `echo ''it\''s''`, where the closing quote runs into a word and a later quote closes the string its author meant, and not `echo ''a\''b` with no later quote, the valid word a\b' kani_harnesses: - id: KANI-LEXER_CONTEXT-001 @@ -300,3 +302,8 @@ falsification_tests: prediction: 'a break inside parentheses in a for loop reports SC2106, while a plain break in a loop body does not' test: cargo test -p bashrs --lib linter::rules::sc2106::tests::test_PMAT258_sc2106_break_in_subshell_inside_for_loop_fires if_fails: 'Contract violation: SC2106 carries a non-shellcheck meaning again, so a suppression written against shellcheck silences the wrong rule, as in GH-303' +- id: F-LCX-021 + rule: inside '...' a backslash is literal, so the quote after it closes the string (GH-439) + prediction: 'lint_shell reports no SC2075 and no error on `echo ''a\b'' | tr -d ''\\'' | grep -x ''ab''`, on the lone-backslash form `echo a | tr -d ''\'' | grep -x ''a''`, or on the same pipeline inside "$( ... )"; `echo ''it\''s''` still reports SC2075' + test: cargo test -p bashrs --lib linter::lexer_context_tests::test_PMAT439_gh439_backslash_before_a_closing_single_quote_is_literal + if_fails: 'Contract violation: SC2075 reads `\''` inside single quotes as an escaped quote again and pairs the string with the next quoted string on the line (GH-439), or the rule was blunted so that it no longer fires on its true positive' diff --git a/docs/audits/impl-PMAT-439-receipt.md b/docs/audits/impl-PMAT-439-receipt.md new file mode 100644 index 000000000..9b4c23f5e --- /dev/null +++ b/docs/audits/impl-PMAT-439-receipt.md @@ -0,0 +1,136 @@ +# PMAT-439 — receipt: SC2075 pairs a backslash string with the next quoted string + +**Ticket:** PMAT-439 (bashrs#439): SC2075 reports an [error] on a valid `'\\'` or `'\'` when another +single-quoted string follows on the same line. **Kind:** code. +**Branch:** `fix/439-sc2075-backslash`, from `63abc48ec4`. + +## The defect + +SC2075 was the regex `'[^']*\\'[^']*'`. That regex reads `\'` as an escaped quote. Inside '...' a +backslash is literal and the next quote closes the string, so in + + echo 'a\b' | tr -d '\\' | grep -x 'ab' + +it matched `'\\' | grep -x '`, from the opening quote of `'\\'` to the opening quote of `'ab'`, and +reported 1:20-36 as an Error. bash runs the line and prints `ab`. Any caller that refuses a bashrs +error therefore refuses a valid script. + +## The fix + +A per-line scanner replaces the regex. It tracks the quoting context the line is in: code, `$( )`, +backticks or `"..."`. It skips comments, escapes in code and `$'...'` bodies, and steps over `$$`, so +the quote after `$$` opens a plain '...'. bash confirms this: `echo $$'a\nb'` prints the pid, then +`a\nb` literally. Inside '...': + +- An attempt is a `\'` followed by a letter, digit or underscore, as in `'it\'s'`, `'can\'t'` or + `'a\'1'`. It is reported only when a later quote on the line closes the string its author meant. +- Every other backslash is literal, and the next quote closes the string: `'\\'`, `'\'`, + `'a\'"b"`, `'a\'$x`, `awk -F'\' '{print $2}'`. +- With no later quote on the line, `echo 'a\'b` is the valid word a\b (bash prints `a\b`) and is not + reported. + +`linter::quoting` is not used. It reads the line the way the shell does, so it closes the string at +`\'` and is out of step with the author from there on. The existing `'don\'t' 'won\'t'` test needs +two reports. The rule header says why. + +The four existing true-positive fixtures keep their spans: `echo 'can\'t'` 1:6-14, +`msg='it\'s broken'` 1:5-19, `echo 'don\'t' 'won\'t'` 1:6-14 and 1:15-23, and +`result=$(echo 'can\'t')` 1:15-23. Max cyclomatic complexity in the file is 7. + +## Where each claim lives + +| Claim | Where | +|---|---| +| the reproducer, the lone `'\'` form and the `"$( ... )"` form report no SC2075 | `rash/src/linter/rules/sc2075.rs`, `test_PMAT439_sc2075_backslashes_then_another_quoted_string`, `..._lone_backslash_then_another_quoted_string`, `..._same_pipeline_inside_quoted_substitution` | +| the whole linter reports no Error from any rule on those three; `echo 'it\'s'` still reports SC2075 | `rash/src/linter/lexer_context_tests.rs`, `test_PMAT439_gh439_backslash_before_a_closing_single_quote_is_literal` (contract F-LCX-021) | +| a backslash string followed by a separate word is clean, including `awk -F'\'` and `'a\'b` with no later quote | `test_PMAT439_sc2075_backslash_string_then_a_separate_word` | +| `\'` inside `"..."`, `$'...'` or unquoted code is not an attempt | `test_PMAT439_sc2075_not_single_quoted_context` | +| the true positive still fires as an Error: alone, after a valid backslash string, with a digit, and after `$$` | `test_PMAT439_sc2075_true_positive_still_fires` | +| an attempt followed by a space, `'can\' t'`, is not clean: SC1078 reports the quote it leaves open | lexer_context_tests, same test, the `shell_fires(..., "SC1078")` line | +| contract rows | `contracts/linter-lexer-context-v1.yaml`: the GH-439 reference, the SC2075 invariant, F-LCX-021 | + +## Commits + +| Commit | What | +|---|---| +| `121a2b041e` | RED: tests, contract rows, roadmap entry | +| `0bdc20364d` | GREEN: the scanner | +| `3a1a4f72dc` | pins `awk -F'\'` and the SC1078 control | +| `b797e6b2eb` | an attempt is reported only when the string it meant closes; `$$` | + +The commit that adds this receipt adds only files under `docs/audits/`. + +## Evidence that the tests discriminate + +- At `121a2b041e`, against the old regex: `cargo test -p bashrs --lib -- PMAT439 sc2075` gave 11 + passed and 7 failed. At `0bdc20364d` it gave 18 passed. +- Reverting the rule body to the regex with the tests kept gave 11 passed and 7 failed. Restoring + it gave 18 passed. +- `b797e6b2eb`'s new rows against `3a1a4f72dc`'s rule: 16 passed and 2 failed. `'a\'b` was + reported, and `$$'a\'b'` was silent. With the fix: 18 passed. +- Control 1 changes `is_word_byte` to accept digits and underscore only. It fails 6 tests: + `true_positive_still_fires`, `escaped_quote`, `its`, `multiple`, `in_command_sub` and the + lexer_context GH-439 test. +- Control 2 records an attempt only when the close is followed by whitespace or the end of the + line. It fails 1 test: `in_command_sub`. + +## Verification + +verification: + cmd=cargo test -p bashrs --lib -- PMAT439 sc2075 claimed_exit=0 rerun_exit=0 log_path=docs/audits/impl-PMAT-439-receipt.md sha256=0 # 18 passed; 0 failed + cmd=cargo nextest run --workspace claimed_exit=0 rerun_exit=0 log_path=docs/audits/impl-PMAT-439-receipt.md sha256=0 # at b797e6b2eb: 17347 passed; 0 failed; 163 skipped + cmd=cargo clippy --workspace --all-targets -- -D warnings claimed_exit=0 rerun_exit=0 log_path=docs/audits/impl-PMAT-439-receipt.md sha256=0 # at b797e6b2eb + cmd=cargo fmt --all -- --check claimed_exit=0 rerun_exit=0 log_path=docs/audits/impl-PMAT-439-receipt.md sha256=0 + cmd=pv lint contracts claimed_exit=0 rerun_exit=0 log_path=docs/audits/impl-PMAT-439-receipt.md sha256=0 # Result: PASS + cmd=bashrs corpus run claimed_exit=0 rerun_exit=0 log_path=docs/audits/impl-PMAT-439-receipt.md sha256=0 # 18814 entries; 18814 passed; 0 failed; V2 99.4/100 (A+): A 100.0, B1 99.9, B2 99.9, B3 98.7, D 100.0, F 99.6, G 99.7 + cmd=bashrs lint claimed_exit=0 rerun_exit=0 log_path=docs/audits/impl-PMAT-439-receipt.md sha256=0 # and exit 2 (SC2075) on echo 'it\'s' + +The corpus ran the way `make corpus-score` runs it: under bwrap, from an empty temporary directory, +using this branch's binary from `CARGO_TARGET_DIR`. It did not use `$(PWD)/target`, which is a +different build. The first run scored 88.7 (B), with B3 and G at 9.8 percent and no entry failed. +`TMPDIR` pointed at a directory that `--ro-bind / /` made read-only, so the behavioural runs had +nowhere to write. Bound writable, the same binary gives the line above, which matches the 7.4.x +release figures. Lint clean misses 3 entries, B-17300, M-397 and M-443. Linting their output +reports SC2135, SC2168 and SC2148, and no SC2075. + +## Review + +Two blind rounds, each with three lanes: sonnet, sonnet adversarial and haiku. These are +**degraded: same-family**: the author is Opus 5.5, and no lane used the author's model. Each lane got +a snapshot of the base files and two candidate diffs, A and B, with `index` lines removed. One was +the real diff and one a control. Lane r2 saw them in the opposite order. Verdicts and findings are in +`docs/audits/quorum-PMAT-439-review.json`. + +- **Round 1, on `3a1a4f72dc`'s diff.** Real diff PASS 3/3; control 1 caught 3/3. The lanes found two + regressions against the old regex, `echo 'a\'b` reported and `$$'a\'b'` silent. Both were checked + in bash and fixed in `b797e6b2eb`. +- **Round 2, on `b797e6b2eb`'s diff, which is the judged diff.** `git diff 63abc48ec4 b797e6b2eb` has + sha256 `671890d15c791bd704ce630d3e01113f042769b55316b0fa4b86c507694ced3c`. Real diff PASS 3/3; + control 2 caught 3/3. Every lane traced control 2 to `test_sc2075_in_command_sub`, which is the one + test cargo fails for it. + +Two hooks refused commands during the work. + +- A pre-edit hook refused the first edit because no active ticket was recorded. The ticket id was + recorded as the hook instructs, and then the edit was made. +- A deletion guard refused two shell commands, one with a variable path and one that only quoted the + guard. Neither command ran, and both were reissued without any deletion. + +The post-commit hook restages `.pmat/baseline.json` after each commit. That churn was restored each +time and is not part of the branch, matching #427, #430 and #433. + +## What this receipt does NOT claim + +- **It does not claim the rule parses shell.** It scans one line at a time, as the regex did. A + `'...'` or `"..."` that spans lines, or a heredoc body, is scanned line by line. +- **It does not claim every escape attempt is SC2075.** A `\'` followed by a space or punctuation, as + in `'can\' t'`, `'say \'.'` or `'it\'-s'`, is no longer reported by SC2075. SC1078 reports the + quote it leaves open, and that is pinned for the space form only. +- **It does not clear two false positives the regex shares.** `echo 'a\'b 'c'` and `echo 'a\'b'c'` + are valid shell and both still report SC2075. In each, a later opening quote is read as the + closer. Checking that the closer ends a word would clear them. That is not done here, and it is no + regression. +- **It does not claim the `$( )` context is exact.** A `)` inside `$( )`, such as a case pattern, + ends the context early. On the tested lines that changes no quote decision. +- **It does not read the corpus score as evidence for the fix.** The corpus lints transpiler output, + which seldom contains these shapes. The score shows only that nothing regressed. diff --git a/docs/audits/quorum-PMAT-439-review.json b/docs/audits/quorum-PMAT-439-review.json new file mode 100644 index 000000000..4a1acfd23 --- /dev/null +++ b/docs/audits/quorum-PMAT-439-review.json @@ -0,0 +1,122 @@ +{ + "ticket": "PMAT-439", + "phase": "review", + "dispatch": "PMAT-439 review: two blind rounds of width 3 (sonnet, sonnet adversarial, haiku). In each round, every lane judges two candidate diffs A/B against a snapshot of the base files: the real diff and a control that must FAIL. r2 sees them in reversed order. index lines are removed from the diffs so blob hashes cannot reveal which candidate was committed.", + "degraded": "same-family", + "author_model": "claude-opus-5-5", + "base": "63abc48ec4", + "judged_commit": "b797e6b2eb2ea20050cbca744edc528ce03a2fdd", + "judged_diff": { + "cmd": "git diff 63abc48ec4 b797e6b2eb", + "sha256": "671890d15c791bd704ce630d3e01113f042769b55316b0fa4b86c507694ced3c", + "lane_copy_sha256_index_lines_removed": "9159aaba4846", + "files": 4, + "insertions": 307, + "deletions": 14 + }, + "result": { + "real_pass": "3/3", + "control_caught": "3/3" + }, + "diff_lane_reduce": { + "round": 2, + "width": 3, + "executor": "claude-code", + "control": "single_quoted records an attempt only when the closing quote is followed by whitespace or the end of the line (line.get(j + 1).is_none_or(u8::is_ascii_whitespace)); cargo: 17 passed, 1 failed (test_sc2075_in_command_sub)", + "lanes": [ + { + "lane": "r1", + "model": "claude-sonnet-5-5", + "order": "A=real B=control", + "status": "SUCCESS", + "verdict": "PASS", + "control_verdict": "FAIL", + "summary": "Real diff: every added assertion and all 10 existing sc2075 tests trace to their expected counts; all three criteria shapes are clean and echo 'it\\'s' reports one Error. Control: the whitespace-only close check drops result=$(echo 'can\\'t'), so test_sc2075_in_command_sub fails.", + "findings": [ + { + "claim": "Shared false positive: in echo 'a\\'b 'c' a later opening quote is read as the closer of the attempt. The old regex matched it too, so this is not a regression.", + "file": "rash/src/linter/rules/sc2075.rs", + "fix": "optional: require that the closer ends a word (whitespace, operator, ')' or end of line), never whitespace alone", + "grounding": "bash prints a\\b c; single_quoted sets attempt at \\'b and closes at the quote before c", + "line": 0 + } + ] + }, + { + "lane": "r2", + "model": "claude-sonnet-5-5", + "order": "A=control B=real", + "status": "SUCCESS", + "verdict": "PASS", + "control_verdict": "FAIL", + "summary": "Adversarial. Real diff: traced the 10 existing counts (1,1,0,0,0,0,0,2,1,0) and every added assertion, and found no failing assertion. Control: test_sc2075_in_command_sub gets 0 where 1 is expected; the control also hides echo 'it\\'s'; foo, cmd 'it\\'s'|grep and echo 'it\\'s'>f, all of which the old regex reported.", + "findings": [ + { + "claim": "Shared limit: the scan is per line, so a '...' spanning lines (an awk program) loses its quote state at the newline. The old regex was per line too.", + "file": "rash/src/linter/rules/sc2075.rs", + "fix": "none for this ticket", + "grounding": "awk ' / /x\\'y/ {print} / ' file: line 2 is scanned as code, and \\' is skipped as an escape", + "line": 71 + }, + { + "claim": "echo 'a\\'b' c is reported; the shell leaves a quote open there, so the script is broken anyway, and the old regex reported it too.", + "file": "rash/src/linter/rules/sc2075.rs", + "fix": "none", + "grounding": "attempt at \\'b, closed by the quote before ' c'", + "line": 150 + } + ] + }, + { + "lane": "r3", + "model": "claude-haiku-5-5", + "order": "A=real B=control", + "status": "SUCCESS", + "verdict": "PASS", + "control_verdict": "FAIL", + "summary": "Real diff: every new assertion traces to its expected count and the existing tests keep theirs, including test_sc2075_in_command_sub. Control: the close of result=$(echo 'can\\'t') is followed by ')', so nothing is pushed (0, expected 1); echo 'it\\'s'; and echo 'it\\'s'|cat are lost too.", + "findings": [ + { + "claim": "Shared false positive: echo 'a\\'b'c' is valid shell (the word a\\bc), yet it is reported. The old regex matches '\\'b' as well, so this is not a regression.", + "file": "rash/src/linter/rules/sc2075.rs", + "fix": "optional: check that the closer ends a word, keeping ')' as an ending so test_sc2075_in_command_sub holds", + "grounding": "attempt at \\'b, then the quote before c closes it", + "line": 169 + } + ] + } + ] + }, + "prior_round": { + "round": 1, + "judged_commit": "3a1a4f72dc", + "judged_diff_sha256": "1d340132080abae5bd3a4acd378d506853a3c939e0077a0c0b1f5396f51913cd", + "control": "is_word_byte accepts digits and underscore only; cargo: 12 passed, 6 failed (true_positive_still_fires, escaped_quote, in_command_sub, its, multiple, the lexer_context GH-439 test)", + "real_pass": "3/3", + "control_caught": "3/3", + "lanes": [ + {"lane": "r1", "model": "claude-sonnet-5-5", "order": "A=real B=control", "verdict": "PASS", "control_verdict": "FAIL"}, + {"lane": "r2", "model": "claude-sonnet-5-5", "order": "A=control B=real", "verdict": "PASS", "control_verdict": "FAIL"}, + {"lane": "r3", "model": "claude-haiku-5-5", "order": "A=real B=control", "verdict": "PASS", "control_verdict": "FAIL"} + ], + "findings_acted_on": [ + { + "claim": "echo 'a\\'b with no later quote is reported, but the shell reads it as the valid word a\\b; the old regex needed a later quote and stayed silent (r3; r2 found the same with echo 'C:\\'dir)", + "fix": "b797e6b2eb: record an attempt only when a later quote closes the string", + "grounding": "bash -c \"echo 'a\\\\'b\" prints a\\b" + }, + { + "claim": "echo $$'a\\'b' is silent: $' after $$ was read as ANSI-C, while the old regex reported it (r2)", + "fix": "b797e6b2eb: step over $$ as one token", + "grounding": "bash: echo $$'a\\nb' prints the pid, then a\\nb literally, so $' after $$ is not ANSI-C" + } + ] + }, + "delegate_receipt": "lane outputs are model reports; each verdict on a control was checked against cargo below", + "orchestrator_remeasure": { + "real": "cargo test -p bashrs --lib -- PMAT439 sc2075: 18 passed; 0 failed", + "control_round_1": "12 passed; 6 failed", + "control_round_2": "17 passed; 1 failed (test_sc2075_in_command_sub)", + "workspace": "cargo nextest run --workspace at b797e6b2eb: 17347 passed; 0 failed; 163 skipped" + } +} diff --git a/docs/roadmaps/roadmap.yaml b/docs/roadmaps/roadmap.yaml index c68956223..d44506833 100644 --- a/docs/roadmaps/roadmap.yaml +++ b/docs/roadmaps/roadmap.yaml @@ -2936,3 +2936,24 @@ roadmap: - kind:docs notes: null epic: 426 +- id: PMAT-439 + github_issue: 439 + item_type: task + title: 'SC2075: a single-quoted backslash followed by another single-quoted string is not an escaped quote (bashrs#439)' + status: inprogress + priority: high + assigned_to: null + created: 2026-10-10T07:13:54Z + updated: 2026-10-10T07:14:06.957847620+00:00 + spec: null + acceptance_criteria: + - 'bashrs lint on `echo ''a\b'' | tr -d ''\\'' | grep -x ''ab''` reports 0 errors: inside single quotes a backslash is literal and the next quote closes the string' + - 'a regression test holds the reproducer (clean) and the true positive `echo ''it\''s''` (SC2075 still fires); reverting the fix turns the reproducer test red' + - 'shapes covered: ''\\'' followed by ''x'' on the same line; the same inside "$( ... )"; a lone ''\'' followed by ''x''' + phases: [] + subtasks: [] + estimated_effort: null + labels: + - kind:code + notes: null + epic: 426 diff --git a/rash/src/linter/lexer_context_tests.rs b/rash/src/linter/lexer_context_tests.rs index 402a02ed3..e0faf66d7 100644 --- a/rash/src/linter/lexer_context_tests.rs +++ b/rash/src/linter/lexer_context_tests.rs @@ -269,3 +269,36 @@ fn test_PMAT257_gh314_idem002_rm_inside_a_quoted_word_is_text() { // A real bare `rm` command is still reported. shell_fires("#!/bin/sh\nrm /app/current\n", "IDEM002"); } + +// --------------------------------------------------------------------------- +// GH-439: inside '...' a backslash is literal, so in `tr -d '\\'` the quote +// after the backslashes closes the string. SC2075 read that `\'` as an escaped +// quote and paired the string with the next single-quoted string on the line, +// an Error on a valid script. `echo 'it\'s'` is still reported: there the +// closing quote runs straight into a word. +// --------------------------------------------------------------------------- + +#[test] +fn test_PMAT439_gh439_backslash_before_a_closing_single_quote_is_literal() { + for src in [ + "#!/bin/bash\necho 'a\\b' | tr -d '\\\\' | grep -x 'ab'\n", + "#!/bin/bash\necho a | tr -d '\\' | grep -x 'a'\n", + "#!/bin/bash\nx=\"$(echo 'a\\b' | tr -d '\\\\' | grep -x 'ab')\"\necho \"$x\"\n", + ] { + let result = lint_shell(src); + assert_absent(&result, "SC2075", src); + let errors: Vec<_> = result + .diagnostics + .iter() + .filter(|d| d.severity == crate::linter::Severity::Error) + .collect(); + assert!( + errors.is_empty(), + "no error on a valid script:\n{src}\ngot {errors:?}" + ); + } + shell_fires("#!/bin/bash\necho 'it\\'s'\n", "SC2075"); + // An attempt followed by a space leaves no word to glue, so SC2075 stays + // silent; the quote it leaves open is still an error. + shell_fires("#!/bin/bash\necho 'can\\' t'\necho done\n", "SC1078"); +} diff --git a/rash/src/linter/rules/sc2075.rs b/rash/src/linter/rules/sc2075.rs index 1d4f171d0..916ce7ce0 100644 --- a/rash/src/linter/rules/sc2075.rs +++ b/rash/src/linter/rules/sc2075.rs @@ -15,14 +15,36 @@ // msg='it'"'"'s fixed' // End, quote in double quotes, continue // // Impact: Syntax errors, incorrect string values +// +// Not an escape attempt (GH-439): `tr -d '\\'` and `tr -d '\'` are complete +// strings, because inside '...' a backslash is literal and the next quote +// closes. The attempt is a `\'` that runs straight into a word, as in +// 'it\'s', and it is reported only when a later quote closes the string its +// author meant: with none, `echo 'a\'b` is the valid word a\b. Each line is +// scanned with its quoting context, so a `\'` inside "...", $'...', a comment +// or unquoted code is not read as one, and a string is never paired with the +// next quoted string on the line. +// +// The scan reads an attempt the way its author meant it, as an escaped quote +// that does not close the string, so a second attempt later on the line is +// still found ('don\'t' 'won\'t' reports twice). linter::quoting reads the +// line as the shell does, closing at that `\'`, and from there on it is out +// of step with the author, so this rule does not use it. use crate::linter::{Diagnostic, LintResult, Severity, Span}; -use regex::Regex; -static ESCAPED_QUOTE_IN_SINGLE: std::sync::LazyLock = std::sync::LazyLock::new(|| { - // Match: 'string\'more' (escaped quote inside single quotes) - Regex::new(r"'[^']*\\'[^']*'").unwrap() -}); +/// The quoting context outside any single-quoted string. +#[derive(Clone, Copy, PartialEq)] +enum Ctx { + /// Unquoted code at the top of the line. + Code, + /// Unquoted code inside `$( )`; a `)` at this level closes it. + Paren, + /// Unquoted code inside backticks. + Backtick, + /// Inside `"..."`. + Double, +} pub fn check(source: &str) -> LintResult { let mut result = LintResult::new(); @@ -30,20 +52,13 @@ pub fn check(source: &str) -> LintResult { for (line_num, line) in source.lines().enumerate() { let line_num = line_num + 1; - if line.trim_start().starts_with('#') { - continue; - } - - for mat in ESCAPED_QUOTE_IN_SINGLE.find_iter(line) { - let start_col = mat.start() + 1; - let end_col = mat.end() + 1; - + for (start, end) in escape_attempts(line.as_bytes()) { let diagnostic = Diagnostic::new( "SC2075", Severity::Error, "Escaping a single quote in single quotes won't work. Use '\"'\"' or double quotes" .to_string(), - Span::new(line_num, start_col, line_num, end_col), + Span::new(line_num, start + 1, line_num, end + 1), ); result.add(diagnostic); @@ -53,6 +68,147 @@ pub fn check(source: &str) -> LintResult { result } +/// Byte ranges of the single-quoted strings on `line` that try to escape a +/// quote, from the opening quote to the quote the author meant to close with. +fn escape_attempts(line: &[u8]) -> Vec<(usize, usize)> { + let mut found = Vec::new(); + let mut stack = vec![Ctx::Code]; + let mut i = 0; + while i < line.len() { + let ctx = stack.last().copied().unwrap_or(Ctx::Code); + if ctx == Ctx::Double { + i = step_double(line, i, &mut stack); + continue; + } + match step_code(line, i, ctx, &mut stack, &mut found) { + Some(next) => i = next, + None => break, // a comment runs to the end of the line + } + } + found +} + +/// One step in unquoted code. `None` when a comment starts at `i`. +fn step_code( + line: &[u8], + i: usize, + ctx: Ctx, + stack: &mut Vec, + found: &mut Vec<(usize, usize)>, +) -> Option { + match line[i] { + b'\\' => Some(i + 2), + b'#' if starts_word(line, i) => None, + b'\'' => Some(single_quoted(line, i, found)), + // `$$` is the pid; a quote after it opens a plain '...'. + b'$' if line.get(i + 1) == Some(&b'$') => Some(i + 2), + b'$' if line.get(i + 1) == Some(&b'\'') => Some(skip_ansi_c(line, i + 2)), + _ => Some(step_nesting(line, i, ctx, stack)), + } +} + +/// Opens or closes `$( )`, backticks and `"..."` from unquoted code. +fn step_nesting(line: &[u8], i: usize, ctx: Ctx, stack: &mut Vec) -> usize { + match (line[i], ctx) { + (b'$', _) if line.get(i + 1) == Some(&b'(') => { + stack.push(Ctx::Paren); + i + 2 + } + (b'"', _) => { + stack.push(Ctx::Double); + i + 1 + } + (b'`', Ctx::Backtick) | (b')', Ctx::Paren) => { + stack.pop(); + i + 1 + } + (b'`', _) => { + stack.push(Ctx::Backtick); + i + 1 + } + (b'(', Ctx::Paren) => { + stack.push(Ctx::Paren); + i + 1 + } + _ => i + 1, + } +} + +/// One step inside `"..."`, where a single quote is text. +fn step_double(line: &[u8], i: usize, stack: &mut Vec) -> usize { + match line[i] { + b'\\' => i + 2, + b'"' => { + stack.pop(); + i + 1 + } + b'$' if line.get(i + 1) == Some(&b'(') => { + stack.push(Ctx::Paren); + i + 2 + } + b'`' => { + stack.push(Ctx::Backtick); + i + 1 + } + _ => i + 1, + } +} + +/// Scans the single-quoted string whose opening quote is at `open` and +/// returns the index after the quote that closes it. A backslash is literal +/// and the next quote closes, except a `\'` followed by a word character: +/// that was meant as an escaped quote, so scanning goes on the way the author +/// meant it, and the string is recorded when a later quote closes it. With no +/// such quote the shell's reading is the only one, and it is valid: +/// `echo 'a\'b` prints a\b. +fn single_quoted(line: &[u8], open: usize, found: &mut Vec<(usize, usize)>) -> usize { + let mut attempt = false; + let mut j = open + 1; + while j < line.len() { + if line[j] == b'\'' { + if attempt { + found.push((open, j + 1)); + } + return j + 1; + } + if line[j] == b'\\' && line.get(j + 1) == Some(&b'\'') && is_word_byte(line.get(j + 2)) { + attempt = true; + j += 2; + } else { + j += 1; + } + } + j +} + +/// Skips the body of `$'...'`, where `\'` is an escape, from `j` to after +/// its closing quote. +fn skip_ansi_c(line: &[u8], mut j: usize) -> usize { + while j < line.len() { + match line[j] { + b'\\' => j += 2, + b'\'' => return j + 1, + _ => j += 1, + } + } + line.len() +} + +/// A `#` starts a comment only at the start of a word. +fn starts_word(line: &[u8], i: usize) -> bool { + i == 0 + || matches!( + line[i - 1], + b' ' | b'\t' | b';' | b'&' | b'|' | b'(' | b'<' | b'>' + ) +} + +/// A letter, digit or underscore: a closing quote followed by one of these +/// runs into the same word. +fn is_word_byte(b: Option<&u8>) -> bool { + b.is_some_and(|b| b.is_ascii_alphanumeric() || *b == b'_') +} + #[cfg(test)] mod tests { use super::*; @@ -131,4 +287,80 @@ mod tests { // Backslash not escaping a quote assert_eq!(result.diagnostics.len(), 0); } + + // GH-439: inside '...' a backslash is literal and the next quote closes + // the string, so `'\\'` and `'\'` are complete strings. Another quoted + // string later on the line is a separate word, not the rest of this one. + + #[test] + fn test_PMAT439_sc2075_backslashes_then_another_quoted_string() { + let code = r#"echo 'a\b' | tr -d '\\' | grep -x 'ab'"#; + assert_eq!(check(code).diagnostics.len(), 0); + } + + #[test] + fn test_PMAT439_sc2075_lone_backslash_then_another_quoted_string() { + let code = r#"echo a | tr -d '\' | grep -x 'a'"#; + assert_eq!(check(code).diagnostics.len(), 0); + } + + #[test] + fn test_PMAT439_sc2075_same_pipeline_inside_quoted_substitution() { + let code = r#"x="$(echo 'a\b' | tr -d '\\' | grep -x 'ab')""#; + assert_eq!(check(code).diagnostics.len(), 0); + } + + #[test] + fn test_PMAT439_sc2075_issue_controls_stay_clean() { + for code in [r#"echo a | tr -d '\\'"#, r#"x='\\'"#, r#"x='\'"#] { + assert_eq!(check(code).diagnostics.len(), 0, "{code}"); + } + } + + #[test] + fn test_PMAT439_sc2075_backslash_string_then_a_separate_word() { + for code in [ + r#"printf '%s\n' '\' 'x'"#, + r#"sed 's/\\/\//g' '\' > out"#, + r#"a='\';b='x'"#, + r#"[ "$c" = '\' ] && echo 'x'"#, + r#"awk -F'\' '{print $2}' file"#, + r#"echo 'a\'"b""#, + r#"echo 'a\'$x"#, + r#"echo 'a\''b'"#, + r#"echo '\' # 'x'"#, + // No later quote closes the string, so the shell's reading, the + // word a\b, is the only one and it is valid. + r#"echo 'a\'b"#, + r#"echo 'C:\'dir"#, + ] { + assert_eq!(check(code).diagnostics.len(), 0, "{code}"); + } + } + + #[test] + fn test_PMAT439_sc2075_not_single_quoted_context() { + for code in [ + r#"echo "it\'s""#, + r#"echo $'it\'s'"#, + r#"echo it\'s"#, + r#"echo "a 'b\' c' d""#, + ] { + assert_eq!(check(code).diagnostics.len(), 0, "{code}"); + } + } + + #[test] + fn test_PMAT439_sc2075_true_positive_still_fires() { + let result = check(r#"echo 'it\'s'"#); + assert_eq!(result.diagnostics.len(), 1); + assert_eq!(result.diagnostics[0].severity, Severity::Error); + // A backslash string that closes and runs into a word is the same + // attempt even after an earlier, valid backslash string. + assert_eq!(check(r#"tr -d '\\' && echo 'it\'s'"#).diagnostics.len(), 1); + assert_eq!(check(r#"echo 'a\'1'"#).diagnostics.len(), 1); + // `$$` is the shell's pid, so the quote after it opens a plain + // single-quoted string, not $'...'. + assert_eq!(check(r#"echo $$'a\'b'"#).diagnostics.len(), 1); + } }