Fix shell quote bypass in terminal permission system#48436
Merged
Conversation
90d876e to
da021d7
Compare
Normalize command strings by stripping shell quotes from the parsed AST
instead of using SimpleCommand::to_string(), which preserves raw lexical
quoting. This ensures security patterns match regardless of quoting style.
Previously, commands like rm -rf '/' or 'rm' -rf / would bypass both
hardcoded deny patterns and user-configured always_deny patterns because
the regex was matching against the quote-preserved string (e.g.,
"rm -rf '/'") rather than the semantic value ("rm -rf /").
The fix replaces the to_string() call in extract_commands_from_simple_command
with manual construction that iterates words and normalizes each one via
brush_parser::word::parse, which decomposes words into WordPieces (Text,
SingleQuotedText, EscapeSequence, etc.) from which we reconstruct the
unquoted semantic value. Parameter expansions, command substitutions, and
arithmetic expressions preserve their original source text so that
patterns like $HOME continue to match.
If word normalization fails, the failure propagates via Option<()> returns
through the extraction chain, causing extract_commands to return None —
the same as a shell parse failure. The caller then falls back to raw-input
matching with always_allow disabled.
da021d7 to
57cb5e9
Compare
rtfeldman
added a commit
that referenced
this pull request
Feb 5, 2026
## Problem
Zed's terminal permission system validates commands using regex patterns
against string representations of parsed commands. The
`SimpleCommand::to_string()` method in brush-parser preserves the raw
lexical form including shell quotes, causing pattern matching to fail on
obfuscated commands.
For example, all of these are semantically equivalent to `rm -rf /` in
POSIX shells, but produced different strings from `to_string()` that
bypassed the hardcoded deny pattern `rm\s+(-[rf]+\s+)*/\s*$`:
| Input | `extract_commands()` returned | Matched deny pattern? |
|---|---|---|
| `rm -rf /` | `"rm -rf /"` | ✅ Yes |
| `rm -rf '/'` | `"rm -rf '/'"` | ❌ No |
| `'rm' -rf /` | `"'rm' -rf /"` | ❌ No |
| `r'm' -rf /` | `"r'm' -rf /"` | ❌ No |
| `rm -r'f' /` | `"rm -r'f' /"` | ❌ No |
| `rm -rf \/` | `"rm -rf \\/"` | ❌ No |
Both hardcoded deny patterns (Phase 2, which operates on extracted
sub-commands) and user-configured `always_deny` patterns were affected.
## Fix
Replace the `simple_command.to_string()` call in
`extract_commands_from_simple_command` with manual construction that:
1. Iterates the command name (`word_or_name`) and word arguments from
the suffix
2. Normalizes each word by parsing it into `WordPiece`s via
`brush_parser::word::parse`
3. Reconstructs the semantic (unquoted) value from each piece:
- `Text` — used as-is
- `SingleQuotedText` — used as-is (brush-parser already strips the
wrapping quotes)
- `EscapeSequence` — leading `\` stripped to produce the unescaped
character
- `AnsiCQuotedText` — used as-is (brush-parser already evaluates
`$'...'`)
- `DoubleQuotedSequence` / `GettextDoubleQuotedSequence` — inner pieces
recursively normalized
- `TildePrefix` — preserved as `~` + suffix
- `ParameterExpansion` / `CommandSubstitution` /
`BackquotedCommandSubstitution` / `ArithmeticExpression` — original
source text preserved so patterns like `\$HOME` and `\${HOME}` continue
to match
4. Joins normalized words with spaces
Falls back to the raw `word.value` if word parsing fails.
## Cross-shell safety
The normalization operates on POSIX-parsed ASTs (brush-parser uses POSIX
grammar), so it applies POSIX quoting rules. For non-POSIX shells
(Nushell, PowerShell, Xonsh, Elvish, etc.) where quoting semantics
differ, any mismatch results in false positives (overly cautious
denials), never false negatives. This is the correct error direction for
a security feature. When brush-parser fails to parse a non-POSIX
command, `extract_commands` returns `None` and the existing fallback
(raw-string matching with `allow_enabled=false`) handles it.
(No release notes because granular tool permissions are still
feautre-flagged.)
Release Notes:
- N/A
naaiyy
added a commit
to Glass-HQ/Glass
that referenced
this pull request
Feb 16, 2026
Key changes: - Thermal state detection (zed-industries#45638) - GPUI detects system thermal state, throttles to ~60fps when overheating - Checkerboard shader for side-by-side diff (zed-industries#48417) - visual pattern for diff backgrounds - cosmic-text v0.17 (zed-industries#48504) - fixes font ligatures on Linux - Middle click tab close (zed-industries#44916) - on_aux_click/is_middle_click API additions - Soft wrap modes for wrap width (zed-industries#46422) - Tab switcher mode similar to vim/helix buffer picker (zed-industries#47079) - Multi_buffer optimization batch (zed-industries#48519) - TreeMap for diagnostics (zed-industries#48482) - performance improvement - Semantic token follow-up fixes (zed-industries#48485) - Claude Opus 4.6 and 1M context window model variants (zed-industries#48508) - Anthropic adaptive thinking types (zed-industries#48517) - Side-by-side diff: hunk gutter highlights restored, toolbar buttons for SplittableEditor - Shell quote bypass fix in terminal permission system (zed-industries#48436) - Project panel: Collapse All improvements (zed-industries#47328, zed-industries#48443) - Edit prediction: trailing newlines fix, cursor position in global coords - Properly discard tokens on language server stop (zed-industries#48490) - AgentTool::NAME const instead of hardcoded strings (zed-industries#48506) Conflict resolution: - collab/editor_tests.rs: deleted (collab removed) - vim (helix, motion, increment): deleted (vim removed) - GPUI (17 files): deleted from Glass (handled in Obsydian-HQ/gpui) - editor/items.rs: merged imports (added BufferId, kept Theme) - project_diff.rs: removed old native_button toggle (upstream uses toolbar buttons now) - lsp_store.rs: added SemanticTokenConfig, removed GlobalLogStore/LanguageServerKind - project_panel.rs: merged UI imports (added ContextMenuEntry, ScrollAxes) - Keymaps: took upstream JetBrains bindings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Problem
Zed's terminal permission system validates commands using regex patterns against string representations of parsed commands. The
SimpleCommand::to_string()method in brush-parser preserves the raw lexical form including shell quotes, causing pattern matching to fail on obfuscated commands.For example, all of these are semantically equivalent to
rm -rf /in POSIX shells, but produced different strings fromto_string()that bypassed the hardcoded deny patternrm\s+(-[rf]+\s+)*/\s*$:extract_commands()returnedrm -rf /"rm -rf /"rm -rf '/'"rm -rf '/'"'rm' -rf /"'rm' -rf /"r'm' -rf /"r'm' -rf /"rm -r'f' /"rm -r'f' /"rm -rf \/"rm -rf \\/"Both hardcoded deny patterns (Phase 2, which operates on extracted sub-commands) and user-configured
always_denypatterns were affected.Fix
Replace the
simple_command.to_string()call inextract_commands_from_simple_commandwith manual construction that:word_or_name) and word arguments from the suffixWordPieces viabrush_parser::word::parseText— used as-isSingleQuotedText— used as-is (brush-parser already strips the wrapping quotes)EscapeSequence— leading\stripped to produce the unescaped characterAnsiCQuotedText— used as-is (brush-parser already evaluates$'...')DoubleQuotedSequence/GettextDoubleQuotedSequence— inner pieces recursively normalizedTildePrefix— preserved as~+ suffixParameterExpansion/CommandSubstitution/BackquotedCommandSubstitution/ArithmeticExpression— original source text preserved so patterns like\$HOMEand\${HOME}continue to matchFalls back to the raw
word.valueif word parsing fails.Cross-shell safety
The normalization operates on POSIX-parsed ASTs (brush-parser uses POSIX grammar), so it applies POSIX quoting rules. For non-POSIX shells (Nushell, PowerShell, Xonsh, Elvish, etc.) where quoting semantics differ, any mismatch results in false positives (overly cautious denials), never false negatives. This is the correct error direction for a security feature. When brush-parser fails to parse a non-POSIX command,
extract_commandsreturnsNoneand the existing fallback (raw-string matching withallow_enabled=false) handles it.(No release notes because granular tool permissions are still feautre-flagged.)
Release Notes: