Repository navigation
fix(agent): decide scoped rm in the shared bash fence - #1337
Merged
Merged
Conversation
On pi, a scoped `rm` of a project file (for example the audit skills' `rm -f .posthog-audit-checks.json`) ran, but the log said "Denying bash command (not in allowlist)" and analytics captured `bash denied`. evaluateToolCall asked wizardCanUseTool first, which logs and captures every allowlist deny, and only then applied the scoped-rm rule. The scoped-rm rule is now decided first. A scoped rm logs an allow line and skips wizardCanUseTool, but still blocks when the program disallows Bash. Every other call goes through wizardCanUseTool as before. Part of #1326 Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Test against a Context Mill branch:
Add Results will be posted here when complete. |
AGENTS.md keeps new code comments to one line. Part of #1326 Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
The scoped-rm rule lived in pi's gate as an exception beside the shared bash fence. wizardCanUseTool denied the command, logged it and captured `bash denied`, and then pi ran it anyway. The exception also skipped every other check in wizardCanUseTool, never reached orchestrator task runs, and let subagents delete files. The rule now lives in evaluateBashCommand, which takes a project root, and wizardCanUseTool passes workingDirectory through. evaluateToolCall makes one policy call, so an allowed rm logs an allow line and captures nothing. - Containment compares real paths, refuses `..`, and refuses quotes, escapes, globs, redirects, and any whitespace but a space. - createSecurityExtension requires workingDirectory, so task runs get the rule. Subagents get subagentFactory, the same fence and state with no rm, branded so the parent's gate doesn't type-check there. - The .env guard ignores case, checks pi paths after pi strips `@` and decodes file://, and denies a Grep glob that can select a .env file. minimatch matches those globs with dotfile semantics like ripgrep. - A denied rm states the rule, or says rm isn't available without a root. Part of #1326 Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
The Windows and POSIX containment tests exercise isScopedFileRemoval, which now lives in bash-fence.ts, so they move from the pi security tests to bash-fence-rm.test.ts. Also names the real-path target, reads a pi tool path once, and states the rm rule in the fence header without comparing it to another harness. No behavior change. Part of #1326 Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
gewenyu99
changed the title
gewenyu99
marked this pull request as ready for review
September 24, 2026 22:38
gewenyu99
changed the title
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
johncwaters
approved these changes
Oct 2, 2026
johncwaters
left a comment
johncwaters
left a comment
Contributor
There was a problem hiding this comment.
Approved by hand after checking the automated review.
Note
Automated review. Not written by a human.
| return isEnvFileName(name.toLowerCase()); | ||
| } | ||
|
|
||
| const ENV_FILE_SAMPLES = ['.env', '.env.local', 'app/.env', 'app/.env.local']; |
Contributor
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
globCanSelectEnvFile only tests the glob against these four sample names, so a glob that selects any other env file passes: .env.production, .env.prod*, *.production, .env.development.local, .envrc. Since a ripgrep --glob overrides .gitignore and the hidden-file skip, a Grep with glob .env.production still searches production secrets.
Fix: decide on the glob itself rather than a sample list (for example, deny when the glob's last segment could match a name starting with .env, any case), or at least widen the samples to the common suffixes plus .envrc, with tests for .env.production and *.production.
| /** True when a ripgrep `--glob` can select a `.env` file. Such a glob overrides `.gitignore`, and ripgrep's `*` matches dotfiles. */ | ||
| export function globCanSelectEnvFile(glob: string): boolean { | ||
| const options = { dot: true, nocase: true, matchBase: true }; | ||
| return ENV_FILE_SAMPLES.some((name) => minimatch(name, glob, options)); |
Contributor
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
Nit: minimatch reads a ripgrep exclude glob like !*.min.js as a negation that matches .env, so Grep calls that only narrow the search get denied. An exclude never adds files, so a leading ! can be allowed; worth a test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # src/agent/runner/harness/pi/security.ts
A leading-wildcard glob such as *.foo was tested only against .env, .envrc and stage names, so it could still select .env.foo. Check it against the project's real env files under the Grep path as well, keeping the name-list and literal-prefix rules. Co-Authored-By: Claude Sonnet 5.5 <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
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.
Problem
Dedupe rm tool fencing.
Changes
rmrule into the shared bash fence, so every pi path makes one policy call..envguard covers every path pi opens.%%{init: {"block": {"padding": 20}}}%% block-beta columns 11 callBand["pi callers old"]:11 callers["linear run · orchestrator task · subagent"]:7 space:4 piBand["src/agent/runner/harness/pi changed"]:11 gate["evaluateToolCall(piToolPath)"]:7 space:4 policyBand["src/agent shared policy old, fence changed"]:11 policy["wizardCanUseTool: disallowedTools, .env, Grep glob"]:7 space:4 fence["evaluateBashCommand: scoped rm decided first"]:7 space:1 deny["deny: block, log bash denied"]:3 allow["allow: YARA scan, then run"]:7 space:4 callers --> gate gate --> policy policy --> fence fence --> deny fence --> allow classDef old fill:#9ca3af1f,stroke:#9ca3af,stroke-width:1.5px classDef new fill:#3b82f626,stroke:#3b82f6,stroke-width:2px classDef oldBand fill:#9ca3af40,stroke:none classDef newBand fill:#3b82f640,stroke:none class callers,policy,allow,deny old class gate,fence new class callBand,policyBand oldBand class piBand newBandEvery pi call, from a linear run, an orchestrator task or a subagent, goes through one gate and one policy call. Blue is what changed: the gate passes the tool path, and the bash fence decides a scoped
rmbefore anything logs a deny. Deny is the only branch that logs and capturesbash denied.These guards are in effect on pi. The anthropic arm pre-allows Bash, Read, Write, Edit and Grep, and its SDK sandbox scopes writes to the project.
bash deniedstays insidewizardCanUseTool, since every deny it returns is now enforced.Test plan
Tests lock the rm bypasses, the pi
@andfile://paths, the Grep glob, and the subagent gate. A real pi audit ran the rm step and loggedAllowing bash command.pnpm typecheck,pnpm lint(0 errors) andpnpm test(200 files, 3393 tests) pass.LLM context
Written by Claude Code after a review of the first version of this PR.
Created with PostHog Desktop