diff --git a/packages/core/src/utils/shellAstParser.test.ts b/packages/core/src/utils/shellAstParser.test.ts index 8f32755b341..7f2790c77b0 100644 --- a/packages/core/src/utils/shellAstParser.test.ts +++ b/packages/core/src/utils/shellAstParser.test.ts @@ -1336,3 +1336,69 @@ describe('consistency: isShellCommandReadOnly (regex) vs isShellCommandReadOnlyA }); }); }); + +describe('statements nested inside a heredoc redirect', () => { + // tree-sitter parses whatever follows the heredoc opener on the same line + // *inside* the redirect node, beside the body. The `redirected_statement` + // arm filtered every redirect child out before evaluation, so an entire + // write segment vanished from the analysis. + it.each([ + 'cat < { + expect(await isShellCommandReadOnlyAST(command)).toBe(false); + }); + + it.each([ + 'cat < { + // The first fix enumerated the shapes it had witnesses for; these are the + // ones that were still dropped. The skip-list is inverted now, so an + // unrecognised shape is evaluated rather than filtered away. + expect(await isShellCommandReadOnlyAST(command)).toBe(false); + }); + + it.each([ + 'cat <out.txt\nhello\nEOF', + 'cat <>out.txt\nhello\nEOF', + 'cat <out.txt\nhello\nEOF', + 'cat <&out.txt\nhello\nEOF', + ])( + 'sees a write redirect nested in the heredoc node in %j', + async (command) => { + // Same root cause from the other side: a redirect written after the + // opener is parsed *inside* the heredoc node, where the redirection walk + // never reached it — it only iterates the direct children of the + // `redirected_statement`. So the write was invisible. + expect(await isShellCommandReadOnlyAST(command)).toBe(false); + }, + ); + + it('still reads an ordinary heredoc as read-only', async () => { + // The inert leaves — the delimiters, the body — must not start + // classifying as statements, and a nested descriptor duplication is not a + // write: `2>&1` names a descriptor, not a file. + for (const command of [ + 'cat <&1\nhello\nEOF', + 'cat <&-\nhello\nEOF', + 'grep x < { return initPromise; } +/** + * Node types inside a redirect that carry no statement of their own: the + * heredoc's own delimiters and body, and the words and expansions naming a + * redirect's destination, which `evaluateRedirectionSafety` already owns. + * + * Deliberately a skip-list rather than an allow-list. tree-sitter-bash nests + * whatever follows `&&`, `||` or `;` on the opener line *inside* the redirect + * node, and that is an open set — `pipeline`, `negated_command`, + * `if_statement`, `for_statement`, `c_style_for_statement`, + * `select_statement`, `declaration_command` and more all appear there. + * Enumerating them has been wrong twice. Anything not listed here is handed to + * `evaluateStatementSafety`, whose default arm floors an unrecognised type at + * `unknown`, so a shape nobody anticipated prompts instead of vanishing. + */ +const INERT_REDIRECT_CHILD: ReadonlySet = new Set([ + 'command_substitution', + 'concatenation', + 'expansion', + 'file_descriptor', + 'heredoc_body', + 'heredoc_end', + 'heredoc_start', + 'number', + 'process_substitution', + 'raw_string', + 'simple_expansion', + 'string', + 'word', +]); + /** * Parse a shell command string into a tree-sitter Tree. * Initialises the parser lazily if needed. @@ -1100,6 +1130,26 @@ function evaluateStatementSafety(node: SyntaxNode): ShellCommandSafety { ...node.namedChildren .filter((child) => !child.type.endsWith('_redirect')) .map((child) => evaluateStatementSafety(child)), + // Whatever follows the heredoc opener on the same line is parsed + // *inside* the redirect node — `cat <out.txt` puts the + // write redirect there — so filtering the redirect out above dropped + // both from the analysis entirely. + ...node.namedChildren + .filter((child) => child.type.endsWith('_redirect')) + .flatMap((redirect) => [ + // A redirect nested in a redirect is a redirect, not a statement: + // it belongs to the redirection axis, which never reached inside + // the heredoc node because it only walks direct children. + evaluateRedirectionSafety(redirect), + ...redirect.namedChildren + .filter( + (child) => + !INERT_REDIRECT_CHILD.has(child.type) && + !child.type.endsWith('_redirect'), + ) + .map((child) => evaluateStatementSafety(child)), + ]), evaluateRedirectionSafety(node), ); if (/^variable_assignments?$/.test(node.type))