Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions packages/core/src/utils/shellAstParser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <<EOF && rm -rf build\nhello\nEOF',
'cat <<EOF; rm -rf build\nhello\nEOF',
'cat <<EOF | rm -rf build\nhello\nEOF',
'cat <<EOF || rm -rf build\nhello\nEOF',
'cat <<EOF & rm -rf build\nhello\nEOF',
'cat <<EOF && mkdir -p build\nhello\nEOF',
'cat <<EOF && tee out.txt\nhello\nEOF',
'cat <<-EOF && rm -rf build\n\thello\n\tEOF',
])('does not lose the segment after the opener in %j', async (command) => {
expect(await isShellCommandReadOnlyAST(command)).toBe(false);
});

it.each([
'cat <<EOF && for ((i=0;i<1;i++)); do rm -rf build; done\nhello\nEOF',
'cat <<EOF && if true; then rm -rf build; fi\nhello\nEOF',
'cat <<EOF && while true; do rm -rf build; done\nhello\nEOF',
'cat <<EOF && { rm -rf build; }\nhello\nEOF',
'cat <<EOF && ! rm -rf build\nhello\nEOF',
'cat <<EOF && (rm -rf build)\nhello\nEOF',
'cat <<EOF && case x in x) rm -rf build;; esac\nhello\nEOF',
])('sees compound statements after the opener in %j', async (command) => {
// 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 <<EOF >out.txt\nhello\nEOF',
'cat <<EOF >>out.txt\nhello\nEOF',
'cat <<EOF 2>out.txt\nhello\nEOF',
'cat <<EOF >&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 <<EOF\nhello\nEOF',
'cat <<-EOF\n\thello\n\tEOF',
"cat <<'EOF'\n`rm -rf build`\nEOF",
'cat <<EOF 2>&1\nhello\nEOF',
'cat <<EOF 2>&-\nhello\nEOF',
'grep x <<EOF\nhello\nEOF',
]) {
expect(await isShellCommandReadOnlyAST(command)).toBe(true);
}
});
});
50 changes: 50 additions & 0 deletions packages/core/src/utils/shellAstParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,36 @@ export async function initParser(): Promise<void> {
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<string> = 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.
Expand Down Expand Up @@ -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 <<EOF && rm -rf build` puts the
// `rm` beside the heredoc body, and `cat <<EOF >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))
Expand Down
Loading