From efec77550af13515f3bc275bcda0818a6511a31c Mon Sep 17 00:00:00 2001 From: Pranit Date: Thu, 22 Oct 2026 18:21:26 -0400 Subject: [PATCH] Read command substitutions in assignment values as one word word_end, which finds where a leading NAME=value assignment ends before the command classifier looks for the command, now reads $(...), ${...}, backquotes, and $'...' the way the shell does, so a space inside a substitution no longer ends the assignment early. The command_classification fuzz target now generates substitutions and leaves them out of its lexer comparison, because the lexer reads $ and backquotes literally. --- .../shell_command/command_classification.zig | 122 ++++++++++++++---- src/core/tooling/command_policy.zig | 21 +++ 2 files changed, 119 insertions(+), 24 deletions(-) diff --git a/src/core/shell_command/command_classification.zig b/src/core/shell_command/command_classification.zig index 28466143a..079373905 100644 --- a/src/core/shell_command/command_classification.zig +++ b/src/core/shell_command/command_classification.zig @@ -207,39 +207,85 @@ pub fn next_token(input: []const u8, start: usize) ?Token { return .{ .text = input[token_start..token_end], .start = token_start, .end = token_end }; } -/// Where the shell word that starts at `start` ends, reading quotes and -/// backslash escapes as the shell does, or null when a quote is never closed -/// or the input ends in a backslash. `next_token` splits only on whitespace, -/// so for `CI='x ls' rm` it would stop after `CI='x`. +/// Where the shell word that starts at `start` ends, reading quotes, +/// backslash escapes, and `$(...)`, `${...}`, and backquoted substitutions +/// as the shell does, or null when one is never closed or the input ends in +/// a backslash. `next_token` splits only on whitespace, so for `CI='x ls' rm` +/// or `X=$(date +%s) rm` it would stop inside the value. pub fn word_end(input: []const u8, start: usize) ?usize { - var in_single = false; - var in_double = false; + return shellTextEnd(input, start, .word, 0); +} + +/// What ends the text `shellTextEnd` reads. +const ShellText = enum { word, double_quote, command, parameter, backquote }; + +/// How deeply quotes and substitutions may nest before `word_end` gives up. +const max_shell_nesting = 32; + +/// Reads shell text from `start` until what ends `text`: whitespace for a +/// word, or the character that closes a quote or substitution, which it +/// reads past. Returns null when the text is never closed. +fn shellTextEnd(input: []const u8, start: usize, text: ShellText, depth: usize) ?usize { + if (depth > max_shell_nesting) return null; + // Parentheses inside `$(...)`, as in `$((1 + 2))` or a subshell. + var parens: usize = 0; var i = start; - while (i < input.len) : (i += 1) { + while (i < input.len) { const ch = input[i]; - if (in_single) { - if (ch == '\'') in_single = false; - continue; + switch (text) { + .word => if (is_whitespace(ch)) return i, + .double_quote => if (ch == '"') return i + 1, + .parameter => if (ch == '}') return i + 1, + .backquote => if (ch == '`') return i + 1, + .command => if (ch == ')') { + if (parens == 0) return i + 1; + parens -= 1; + } else if (ch == '(') { + parens += 1; + }, } if (ch == '\\') { if (i + 1 >= input.len) return null; + i += 2; + continue; + } + // A backquoted command is read as literal text up to its end. + if (text == .backquote) { i += 1; continue; } - if (in_double) { - if (ch == '"') in_double = false; - continue; - } - if (ch == '\'') { - in_single = true; + const next: u8 = if (i + 1 < input.len) input[i + 1] else 0; + if (ch == '\'' and text != .double_quote) { + i = (std.mem.findScalarPos(u8, input, i + 1, '\'') orelse return null) + 1; + } else if (ch == '$' and next == '\'' and text != .double_quote) { + i = ansiCQuoteEnd(input, i + 2) orelse return null; + } else if (ch == '$' and next == '(') { + i = shellTextEnd(input, i + 2, .command, depth + 1) orelse return null; + } else if (ch == '$' and next == '{') { + i = shellTextEnd(input, i + 2, .parameter, depth + 1) orelse return null; + } else if (ch == '`') { + i = shellTextEnd(input, i + 1, .backquote, depth + 1) orelse return null; } else if (ch == '"') { - in_double = true; - } else if (is_whitespace(ch)) { - break; + i = shellTextEnd(input, i + 1, .double_quote, depth + 1) orelse return null; + } else { + i += 1; } } - if (in_single or in_double) return null; - return i; + return if (text == .word) i else null; +} + +/// Where a `$'...'` quote whose text starts at `start` ends. A backslash +/// escapes the next character, so `\'` does not end it. +fn ansiCQuoteEnd(input: []const u8, start: usize) ?usize { + var i = start; + while (i < input.len) : (i += 1) { + if (input[i] == '\\') { + i += 1; + } else if (input[i] == '\'') { + return i + 1; + } + } + return null; } fn is_whitespace(ch: u8) bool { @@ -472,7 +518,14 @@ test "terminal segment ignores quoted pipe and rejects unbalanced quote" { } test "assignment values with quotes or escapes stay one word" { - for ([_][]const u8{ "CI='x ls' rm -rf .", "CI=\"x ls\" rm -rf .", "CI=x\\ ls rm -rf ." }) |command| { + for ([_][]const u8{ + "CI='x ls' rm -rf .", + "CI=\"x ls\" rm -rf .", + "CI=x\\ ls rm -rf .", + "CI=$(echo x ls) rm -rf .", + "CI=${X:-x ls} rm -rf .", + "CI=`echo x ls` rm -rf .", + }) |command| { try std.testing.expectEqualStrings("rm", base_command_token(command).?); try std.testing.expectEqualStrings("rm -rf .", analysis_command_tail(command)); } @@ -492,15 +545,33 @@ test "word end reads quotes and escapes as the shell does" { try std.testing.expect(word_end("CI=x\\", 0) == null); } +test "word end reads substitutions as the shell does" { + try std.testing.expectEqual(@as(?usize, 13), word_end("X=$(date +%s) rm", 0)); + try std.testing.expectEqual(@as(?usize, 11), word_end("X=${A:-a b} rm", 0)); + try std.testing.expectEqual(@as(?usize, 12), word_end("X=`date +%s` rm", 0)); + try std.testing.expectEqual(@as(?usize, 17), word_end("X=\"$(echo \"a b\")\" rm", 0)); + try std.testing.expectEqual(@as(?usize, 12), word_end("X=$((1 + 2)) rm", 0)); + try std.testing.expectEqual(@as(?usize, 13), word_end("X=$(echo ')') rm", 0)); + try std.testing.expectEqual(@as(?usize, 15), word_end("X=$(echo `a b`) rm", 0)); + // `\'` doesn't end an ANSI-C quote. + try std.testing.expectEqual(@as(?usize, 10), word_end("X=$'a\\' b' rm", 0)); + // Inside double quotes, single quotes and `$'` are literal. + try std.testing.expectEqual(@as(?usize, 6), word_end("X=\"'a\" b'", 0)); + for ([_][]const u8{ "X=$(echo a", "X=`a b", "X=${a b", "X=\"$(\"", "X=$'a\\' b" }) |command| { + try std.testing.expect(word_end(command, 0) == null); + } +} + /// Pieces of generated commands: the wrappers and environment assignments -/// the classifier peels, and the quotes and escapes that move a word boundary -/// that a split on whitespace cannot see. +/// the classifier peels, and the quotes, escapes, and substitutions that move +/// a word boundary that a split on whitespace cannot see. const fuzz_fragments = [_][]const u8{ " ", "\t", "\n", "ls", "rm", "grep", "git", "find", "-rf", "x", "[", "CI=", "CI=1", "TERM=", "FOO=", "env", "nice", "-n", "5", "-5", "time", "stdbuf", "-oL", "timeout", "10s", "sudo", "bash", "nohup", "'", "\"", "\\", "\\ ", "|", "&&", "&", ";", ">", "=", "-", "\u{e9}", + "$(", ")", "${", "}", "`", "$'", }; fn writeFuzzCommand(smith: *std.testing.Smith, out: *std.Io.Writer) std.Io.Writer.Error!void { @@ -542,6 +613,9 @@ fn fuzzClassification(_: void, smith: *std.testing.Smith) !void { // The base command is the first word the shell runs after the // environment assignments. Callers look for shell operators before they // ask for it, so only a simple command, with no operators, is compared. + // The lexer reads `$` and backquotes literally, so a command with a + // substitution is left to the `word_end` tests. + if (std.mem.findAny(u8, command, "$`") != null) return; var argv = lex.tokenize_argv(std.testing.allocator, command) catch return; defer argv.deinit(std.testing.allocator); for (argv.tokens) |token| { diff --git a/src/core/tooling/command_policy.zig b/src/core/tooling/command_policy.zig index 9c4745105..f9477697f 100644 --- a/src/core/tooling/command_policy.zig +++ b/src/core/tooling/command_policy.zig @@ -743,6 +743,11 @@ test "command risk note sees past quoted and escaped assignment values" { try std.testing.expect(command_risk_note_for("CI='x ls rm -rf /tmp/x") == null); } +test "command risk note leaves commands with substitutions unresolved" { + // A substitution can expand to any command, so the note doesn't guess. + try std.testing.expect(command_risk_note_for("X=$(date +%s) rm -rf /tmp/x") == null); +} + test "command risk note detects quoted su command payload" { try std.testing.expectEqualStrings("note: command may remove files forcefully", command_risk_note_for("su -c 'rm -rf /tmp'").?); try std.testing.expectEqualStrings("note: command may discard version-control state", command_risk_note_for("su -c \"git reset --hard\"").?); @@ -786,6 +791,22 @@ test "semantic annotation explains grep exit one" { try std.testing.expectEqualStrings("note: no matches found", semantic_exit_annotation("grep foo bar", 1, "").?); } +test "semantic annotation sees past substitutions in assignment values" { + // Each value holds a space, so a split on whitespace ended the word + // inside the substitution and found no command. + for ([_][]const u8{ + "CI=$(echo a b) grep foo .", + "CI=\"$(echo a b)\" grep foo .", + "CI=${X:-a b} grep foo .", + "CI=`echo a b` grep foo .", + "env CI=$(echo a b) grep foo .", + }) |command| { + try std.testing.expectEqualStrings("note: no matches found", semantic_exit_annotation(command, 1, "").?); + } + // A substitution that never closes names no command. + try std.testing.expect(semantic_exit_annotation("CI=$(echo a b grep foo .", 1, "") == null); +} + test "semantic annotation leaves grep real errors alone" { try std.testing.expect(semantic_exit_annotation("grep foo bar", 2, "") == null); }