From eb94b984dfb8f9a2490c9d66673cdbaffc9db5ff Mon Sep 17 00:00:00 2001 From: tianyuan <2720711917@qq.com> Date: Tue, 25 Aug 2026 22:50:27 +0800 Subject: [PATCH] fix(core): refuse substitutions hidden in pattern words and heredoc bodies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two places where tree-sitter-bash yields a single leaf node, so the substitution walk finds nothing to collect while bash still runs what is inside. The pattern word of `${v%%…}`, `${v%…}`, `${v##…}`, `${v#…}`, `${v^^…}`, `${v^…}`, `${v,,…}`, `${v,…}` is one leaf, and so is each half of `${v/pat/rep}` and the operand of `${v:-…}`, `${v:=…}`, `${v:?…}`, `${v:+…}`. `echo ${x%%$(rm -rf build)}` therefore classified `read-only` and would have run unattended. Since the collection pass found nothing, an opener still present in the expansion text is exactly that hidden channel, so the leaf is refused on the text. A heredoc body is one leaf too — always for `<<-`, and for `<<` whenever nothing inside it parsed — and bash expands it before feeding it to stdin. Expansion there follows double-quote rules, so `$(…)`, backticks and `${v@P}` run while `<(…)` does not; the body is refused for the first three only. A quoted delimiter (`<<'EOF'`, `<<"EOF"`, `<<\EOF`) makes the body inert and is exempted. `${v@P}` is included in both because a prompt expansion runs any `$(…)` held in the variable's value, and in a pattern word or a body it is a leaf that the `@`/`P` child-adjacency check never sees. The regex is deliberately not anchored to a brace-free span: `${a[${b}]@P}` nests a brace, and a `[^{}]*` bridge stops at it. These are leaf fallbacks for sites the node walk cannot reach, so over-refusing costs at most a prompt. Mutation-verified: neutralising the three regexes fails 28, 10 and 2 tests respectively, and dropping the quoted-delimiter exemption fails 1. --- .../core/src/utils/shellAstParser.test.ts | 212 ++++++++++++++++++ packages/core/src/utils/shellAstParser.ts | 58 +++++ 2 files changed, 270 insertions(+) diff --git a/packages/core/src/utils/shellAstParser.test.ts b/packages/core/src/utils/shellAstParser.test.ts index 8f32755b341..0127ccbfbd6 100644 --- a/packages/core/src/utils/shellAstParser.test.ts +++ b/packages/core/src/utils/shellAstParser.test.ts @@ -1336,3 +1336,215 @@ describe('consistency: isShellCommandReadOnly (regex) vs isShellCommandReadOnlyA }); }); }); + +describe('substitution hidden in an expansion pattern word', () => { + // tree-sitter-bash parses the pattern word as a leaf, so the substitution + // never becomes a command_substitution node — but bash still runs it. + it.each(['%%', '%', '##', '#', '^^', '^', ',,', ','])( + 'refuses a command substitution hidden by the operator %s', + async (operator) => { + expect( + await classifyShellCommandSafety( + `echo \${HOME${operator}$(rm -rf build)}`, + ), + ).toBe('unknown'); + expect( + await classifyShellCommandSafety( + `echo "\${HOME${operator}$(rm -rf build)}"`, + ), + ).toBe('unknown'); + expect( + await classifyShellCommandSafety( + `echo \${HOME${operator}\`rm -rf build\`}`, + ), + ).toBe('unknown'); + }, + ); + + // bash runs `<(…)` and `>(…)` in a pattern word exactly as it runs `$(…)`, + // and tree-sitter emits no node for those either. + it.each(['%%', '%', '##', '#', '^^', '^', ',,', ','])( + 'refuses a process substitution hidden by the operator %s', + async (operator) => { + for (const opener of ['<(', '>(']) { + expect( + await classifyShellCommandSafety( + `echo \${HOME${operator}${opener}rm -rf build)}`, + ), + ).toBe('unknown'); + expect( + await classifyShellCommandSafety( + `echo "\${HOME${operator}${opener}rm -rf build)}"`, + ), + ).toBe('unknown'); + } + }, + ); + + // `${v@P}` runs any $(…) held in the variable's value, and in a pattern word + // it is a leaf, so the @/P child-adjacency check never sees it either. + it.each(['%%', '%', '##', '#', '^^', '^', ',,', ','])( + 'refuses a prompt expansion hidden by the operator %s', + async (operator) => { + expect( + await classifyShellCommandSafety(`echo \${x${operator}\${v@P}}`), + ).toBe('unknown'); + }, + ); + + // `${var/pat/rep}` has two halves and bash expands both, so each needs its + // own pin — the pattern half is where the other operators put their word, + // and the replacement half is the one an operator-shaped test never reaches. + // A `$(…)` here does become a real command_substitution node, so it is + // classified from the command inside it — `write`, which is stronger than + // the `unknown` the leaf-parsed spellings get. Both are refusals; they are + // pinned apart so that a spelling silently changing category is a failure. + it.each([ + ['pattern', 'echo ${x/$(rm -rf build)/rep}', 'write'], + ['pattern', 'echo ${x/`rm -rf build`/rep}', 'unknown'], + ['pattern', 'echo ${x/<(rm -rf build)/rep}', 'unknown'], + ['pattern', 'echo ${x/${v@P}/rep}', 'unknown'], + ['replacement', 'echo ${x/pat/$(rm -rf build)}', 'write'], + ['replacement', 'echo ${x/pat/`rm -rf build`}', 'unknown'], + ['replacement', 'echo ${x/pat/<(rm -rf build)}', 'unknown'], + ['replacement', 'echo ${x//pat/$(rm -rf build)}', 'write'], + ])( + 'refuses a substitution in the %s half of ${var/…/…}', + async (_half, command, expected) => { + expect(await classifyShellCommandSafety(command)).toBe(expected); + expect(await classifyShellCommandSafety(`"${command}"`)).toBe(expected); + }, + ); + + it('treats ${var/pat/${v@P}} as unknown', async () => { + expect(await classifyShellCommandSafety('echo ${x/pat/${v@P}}')).toBe( + 'unknown', + ); + }); + + // The default/assign/error/alternate operators take a *word* just as the + // trim and case operators do, and bash expands it the same way. + it.each([':-', '-', ':=', '=', ':?', '?', ':+', '+'])( + 'refuses a substitution behind the value operator %s', + async (operator) => { + // `$(…)` becomes a real node and is classified from the command inside + // it; the leaf-parsed spellings reach the regex instead. + expect( + await classifyShellCommandSafety(`echo \${x${operator}$(rm -rf b)}`), + ).toBe('write'); + for (const payload of ['`rm -rf b`', '<(rm -rf b)', '${v@P}']) { + expect( + await classifyShellCommandSafety(`echo \${x${operator}${payload}}`), + ).toBe('unknown'); + } + }, + ); + + it('refuses a substitution in a substring or subscript position', async () => { + expect(await classifyShellCommandSafety('echo ${x:1:$(rm -rf b)}')).toBe( + 'write', + ); + expect(await classifyShellCommandSafety('echo ${x[$(rm -rf b)]}')).toBe( + 'write', + ); + expect(await classifyShellCommandSafety('echo ${!x@P}')).toBe('unknown'); + }); + + it('does not flag expansions without a substitution', async () => { + expect(await classifyShellCommandSafety('echo ${HOME%%/*}')).toBe( + 'read-only', + ); + expect(await classifyShellCommandSafety('echo ${HOME}')).toBe('read-only'); + }); +}); + +describe('substitution hidden in a heredoc body', () => { + // The body is one leaf too, and bash expands it before feeding it to stdin. + // Expansion there follows double-quote rules, so `$(…)`, backticks and the + // `@P` operator run while `<(…)` does not. + it('treats an unquoted-delimiter body containing a substitution as unsafe', async () => { + expect( + await classifyShellCommandSafety('cat < { + // A `<<-` body is always one raw leaf, so the `$(` branch of the body + // regex is the only thing that catches this — the AST walk sees no + // command_substitution node to classify. + // Only the tab-indented `<<-` spelling is the always-leaf case the body + // regex has to catch; the others parse into a real command_substitution + // node and are classified from the command inside it. Pinned apart so a + // spelling silently changing category is a failure, not a pass. + expect( + await classifyShellCommandSafety('cat <<-EOF\n\t$(rm -rf build)\n\tEOF'), + ).toBe('unknown'); + expect( + await classifyShellCommandSafety('cat <<-EOF\n$(rm -rf build)\nEOF'), + ).toBe('write'); + expect( + await classifyShellCommandSafety('cat < { + // A `<<-` body is always one raw leaf, so the expansion never becomes a + // child node the walk above could see. + expect( + await classifyShellCommandSafety('cat <<-EOF\n\t${v@P}\n\tEOF'), + ).toBe('unknown'); + expect(await classifyShellCommandSafety('cat < { + expect( + await classifyShellCommandSafety('cat < { + expect( + await classifyShellCommandSafety("cat <<'EOF'\n`rm -rf build`\nEOF"), + ).toBe('read-only'); + expect( + await classifyShellCommandSafety('cat <<"EOF"\n`rm -rf build`\nEOF'), + ).toBe('read-only'); + // `<<\EOF` quotes the delimiter just as surely. + expect( + await classifyShellCommandSafety('cat <<\\EOF\n$(rm -rf build)\nEOF'), + ).toBe('read-only'); + }); + + it('refuses a substitution in the heredoc DELIMITER, which bash expands', async () => { + // The body and the opener-line segments are pinned above and below, but + // the delimiter itself was not — and bash expands it. Today the refusal + // rides on two mechanisms this block never asserts (the `root.hasError` + // bailout and the ERROR node landing in the unknown-floored default arm), + // so a tree-sitter upgrade that parses the `$(…)` into real nodes, or a + // refactor of either mechanism, would flip this to read-only unnoticed. + expect( + await classifyShellCommandSafety('cat <<$(rm -rf build)\nhello\nEOF'), + ).toBe('unknown'); + expect( + await classifyShellCommandSafety('cat <<`rm -rf build`\nhello\nEOF'), + ).toBe('unknown'); + }); + + it('does not flag a body without a substitution', async () => { + expect(await classifyShellCommandSafety('cat <(…)` wherever it runs `$(…)` — + * in a pattern word, that is. See `HEREDOC_SUBSTITUTION` for the one place it + * does not. + */ +const HIDDEN_SUBSTITUTION = /\$\(|`|<\(|>\(/; + function evaluateSubstitutions(node: SyntaxNode): ShellCommandSafety { const substitutions = collectDescendants( node, @@ -975,6 +1003,36 @@ function evaluateSubstitutions(node: SyntaxNode): ShellCommandSafety { return 'unknown'; } } + // tree-sitter-bash parses the pattern word of `${v%%…}`, `${v%…}`, + // `${v##…}` and `${v#…}` as a single leaf, so a substitution inside it + // yields no node of its own even though bash runs it while expanding. + // Nothing was collected above, so an opener still present in an + // expansion is exactly that hidden channel. + if ( + HIDDEN_SUBSTITUTION.test(expansion.text) || + PROMPT_EXPANSION.test(expansion.text) + ) { + return 'unknown'; + } + } + // A heredoc body is one leaf too (`<<-` bodies always, `<<` bodies when + // nothing inside them parsed), and bash expands it before feeding it to + // stdin — unless the delimiter is quoted, which makes the body inert. + // Expansion there follows double-quote rules: `$(…)`, backticks and `@P` + // run, `<(…)` does not. + for (const body of collectDescendants(node, new Set(['heredoc_body']))) { + if ( + !HEREDOC_SUBSTITUTION.test(body.text) && + !PROMPT_EXPANSION.test(body.text) + ) { + continue; + } + const delimiter = body.parent?.namedChildren.find( + (child) => child.type === 'heredoc_start', + ); + // `<<\EOF` quotes the delimiter as surely as `<<'EOF'` does. + if (delimiter && /['"\\]/.test(delimiter.text)) continue; + return 'unknown'; } return 'read-only'; }