fix(approval): allow quoted shell metacharacters in allowlist matching (port of oh-my-pi#7553) - #80830
Merged
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on 614def0 — Port from can1357/oh-my-pi#7553: allow quoted shell metachar
|
alt-glitch
added
type/bug
… allowlist matching command_allowlist glob rules (e.g. 'cargo *') rejected any command whose quoted arguments contained shell metacharacters — a cargo benchmark regex filter like '^layer3/write/(a|b)$' disqualified the whole command even though those characters are literal to the shell. _has_allowlist_shell_operator is now quote-aware: - metacharacters inside single/double quotes or behind a backslash are treated as literal arguments; - $ and backtick inside DOUBLE quotes still disqualify (expansion is active there); - quoted/escaped control characters still disqualify when the command carries a -c/-e/--command/--eval-style option that hands the payload to another interpreter (sh -c '...', git -c alias.x='!...' x); - unterminated quotes disqualify (shape can't be reasoned about). Compound commands (unquoted ; & | < > backtick $( newline) are rejected exactly as before. hermes_cli/approvals_suggest.derive_glob picks up the same semantics via its existing import.
teknium1
force-pushed
the
omp-port/allowlist-quoted-metachars
branch
from
August 17, 2026 02:58
12dd7d8 to
614def0
Compare
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
2 participants
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.
Summary
command_allowlistglob rules (e.g.cargo *) now match commands whose quoted arguments contain shell metacharacters — previouslycargo bench -- '^layer3/write/(a|b)$'was rejected as "compound" even though every metacharacter is literal to the shell.Port of can1357/oh-my-pi#7553 (their issue #7552). Our
_has_allowlist_shell_operatorused a flat regex ([;&|<>]|\$\() with no quote awareness, so any benchmark filter, grep pattern, or commit message containing|,;,(,$disqualified the whole command from the allowlist shortcut and forced an approval prompt the user had already opted out of.Security posture (unchanged or tightened)
; & | < >$( ` and newlines: rejected exactly as before.$/backtick inside double quotes: rejected (expansion is active there).-c/-e/--command/--eval-style option: rejected —sh -c '...'andgit -c alias.x='!...' xhand the quoted payload to another interpreter (this closes a hole the old regex also had for backslash-escaped payloads likegit -c alias.x=\!touch\ /tmp/pwn\;\ ok x— the old regex caught the;but the reinterpretation class is now handled explicitly).hermes approvals suggest'sderive_globinherits the same semantics via its existing import.Changes
tools/approval.py: quote-aware scanner replaces_ALLOWLIST_SHELL_OPERATOR_REtests/tools/test_allowlist_quoted_metachars.py: 24 new tests (literal-quote acceptance, compound rejection, interpreter-option payloads, unterminated quotes, end-to-end glob matching)Validation
cargo bench -- '^a(10|1000)$'vscargo *cargo build && rm -rf /tmp/xvscargo *git -c alias.x='!touch /tmp/pwn' xvsgit *Infographic