Reject shell substitutions in terminal tool commands#51689
Merged
Conversation
cla-bot
Bot
added
the
cla-signed
zed-community-bot
Bot
added
the
staff
Fix two security bypasses in shell command validation: - CaseClause patterns (Vec<Word>) were not being validated, allowing command substitutions like $(whoami) in case patterns to slip through as Safe despite being executed by the shell at runtime. - ArithmeticForClause only validated the body, ignoring initializer, condition, and updater expressions that can contain command substitutions. Now treated as unconditionally Unsafe, mirroring standalone Arithmetic.
Clarify the return type of normalize_assignment_for_command_prefix by introducing a NormalizedAssignment enum with Included and Skipped variants, replacing the confusing double-Option.
When a command has both Unsafe and Unknown parts, return Unsafe since it is a more definitive and actionable signal for user-facing error messages.
Replace wildcard catch-all arms with explicit matches for Word and ProcessSubstitution variants so future enum additions cause compile errors rather than silently passing through.
'Unsupported' more accurately describes what the variant means: the command uses syntax we cannot analyze, not that the result is unknown for some other reason.
piper-of-dawn
pushed a commit
to piper-of-dawn/zed
that referenced
this pull request
Apr 25, 2026
…51689) 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)Pattern extraction (
agent/pattern_extraction)\s+while preserving display whitespaceTool permissions (
agent/tool_permissions)Terminal tool (
agent/tools/terminal_tool)${HOME},$1,$?,$$,$@,$(whoami), backticks,$((1+1)),<(ls),>(cat), env-prefix variants, multiline, nested)Closes SEC-264
Release Notes:
FOO=bar cmd arg1 arg2)$FOOare disallowed for security, since regexes wouldn't be able to match on them.