Skip to content

Harden execpolicy deny matching against & chains and shell wrappers - #5164

Merged
Hmbown merged 2 commits into
agent/v094-release-train-20260802from
copilot/fix-execpolicy-bypass
Aug 3, 2026
Merged

Harden execpolicy deny matching against & chains and shell wrappers#5164
Hmbown merged 2 commits into
agent/v094-release-train-20260802from
copilot/fix-execpolicy-bypass

Conversation

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Execpolicy deny rules could be bypassed when a denied command appeared after a single &, or when the command was wrapped in shell syntax the segmenter/token matcher did not normalize. In AskForApproval::Never / UnlessTrusted modes, that weakened the last enforcement gate for destructive commands.

  • Deny matching now sees single-& chains

    • command_segments() now splits on single & in addition to &&, ||, |, and ;.
    • This prevents a benign leading command from shielding a denied suffix in background/foreground chains.
  • Deny matching now normalizes simple shell wrappers

    • denied_prefix_matches() now sanitizes tokens before comparison so the anchored positional match still lands on the underlying command.
    • Covered wrappers:
      • (cmd ...)
      • { cmd ...; }
      • $(cmd ...)
  • Regression coverage for the reported bypasses

    • Added a focused test that exercises the deny path for:
      • ls & rm -rf /
      • (rm -rf /)
      • { rm -rf /; }
      • $(rm -rf /)

Example:

let engine = ExecPolicyEngine::new(vec![], vec!["rm -rf /".to_string()]);

for command in [
    "ls & rm -rf /",
    "(rm -rf /)",
    "{ rm -rf /; }",
    "$(rm -rf /)",
] {
    let decision = engine.check(ctx(command, AskForApproval::Never)).unwrap();
    assert!(!decision.allow);
}

Copilot AI changed the title [WIP] Fix execpolicy deny rules bypass issue Harden execpolicy deny matching against & chains and shell wrappers Aug 3, 2026
Copilot AI requested a review from Hmbown August 3, 2026 03:51
@Hmbown
Hmbown changed the base branch from main to agent/v094-release-train-20260802 August 3, 2026 11:57
@Hmbown
Hmbown marked this pull request as ready for review August 3, 2026 12:48
Copilot AI review requested due to automatic review settings August 3, 2026 12:48
@Hmbown
Hmbown merged commit 353e413 into agent/v094-release-train-20260802 Aug 3, 2026
7 checks passed
@Hmbown
Hmbown deleted the copilot/fix-execpolicy-bypass branch August 3, 2026 12:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens ExecPolicyEngine deny-prefix enforcement to prevent bypasses via shell chaining and simple wrapper syntax, closing a gap that could allow destructive commands through in AskForApproval::Never / UnlessTrusted modes.

Changes:

  • Split deny matching across additional chain separators by treating single & as a segment boundary.
  • Normalize simple shell wrapper tokens in denied_prefix_matches() so anchored positional matching still lands on the underlying command.
  • Add regression tests covering the reported bypass cases (& chains and wrapper forms).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 653 to 657
command
.replace("&&", "\n")
.replace("||", "\n")
.replace(['|', ';'], "\n")
.replace(['&', '|', ';'], "\n")
.lines()
Hmbown pushed a commit that referenced this pull request Aug 4, 2026
…#5164)

* Initial plan

* WIP: harden execpolicy deny matching

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Hmbown pushed a commit that referenced this pull request Aug 4, 2026
…#5164)

* Initial plan

* WIP: harden execpolicy deny matching

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Hmbown added a commit that referenced this pull request Aug 4, 2026
Deny rules were matched against the command text as written, so any shell
construct that changes the text without changing what executes slipped past
them. The hardening in #5164 (issue #5161) closed a couple of spellings by
adding string handling per metacharacter; the rest of that class stayed open.
Under `AskForApproval::Never` a missed deny rule runs with no prompt at all,
so this is the one gate where a near miss is a full bypass.

Adding another pattern per metacharacter is a race the matcher loses by
construction, so add `shell_expand` instead: it word-splits a command the way
a POSIX shell does and returns every command line that would actually be
executed -- chained and grouped segments, command- and process-substitution
bodies, parameter-expansion bodies, and the payloads handed to `eval` and to a
shell's `-c`, plus the invocation left after wrapper words such as `sudo` and
`timeout`. Quoting is resolved rather than pattern-matched, so an operand loses
its quotes and, in the other direction, single-quoted text is correctly not
treated as code. Both live deny surfaces now match against that set: the
permission engine's denied-prefix scan and typed Deny rules, and the TOML
`execpolicy.toml` deny patterns.

Only the deny path widens. Allow and trusted-prefix matching still run against
the command as written, so a broader expansion cannot become a broader
auto-approval. The naive segment split is unioned in rather than replaced, so
no rule that used to fire can stop firing. Expansion is depth- and
count-bounded.

Tests are table-driven over every spelling now covered, with negative cases
that must stay allowed: substitution of a benign command, denied text inside
single quotes, a denied word appearing as an operand, and escaped operators.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.9.4: execpolicy deny rules evadable via single-& chains and subshell wrapping

3 participants