fix(pi): allow rm of files inside the working directory#834
Merged
Conversation
… rm — claude-agent-sdk parity End-of-run remarks from pi runs (tracking: #793) showed agents burning turns on two frictions that came from the wizard's tool-use layer not matching what the anthropic harness ships natively: - read-before-write lived in a prompt commandment ("read every file immediately before writing, even if already read") while pi's built-in write/edit enforced nothing — over-constraining new-file creation (ENOENT reads) while leaving stale writes unprotected. - `rm` was blocked outright, so the skill's "delete the event plan" step was impossible; agents invented empty-the-file workarounds. Changes: - pi/file-state.ts + security.ts: the pi fence now enforces the exact claude-agent-sdk semantics, block messages verbatim — mutating an existing file requires a read since it last changed (mtime-tracked), brand-new files need no read, own writes count as knowing the content. Per-session, subagents included. - The read-before-write commandment is deleted; the tool layer owns the rule on both arms now. - agent-interface.ts: plain `rm [-f] <relative-file>` inside the project is allowed (no recursion/flags/globs/`..`/absolute/.env); recursive-force variants stay covered by warlock. - Tests: tracker semantics, fence integration, scoped-rm matrix; commandments snapshot documents the rule removal. Generated-By: PostHog Code Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
🧙 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
Results will be posted here when complete. |
… change The read-before-write commandment predates the pi harness by ~2.5 months (in commandments.ts since 2026-04-15; pi landed 2026-07-03), so it is an original-harness rule, not a pi-era workaround — it stays. This PR is now only the scoped `rm [-f] <relative-file>` bash-policy allowance plus its tests. Generated-By: PostHog Code Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
gewenyu99
changed the title
The rm allowance is pi-only pain (the anthropic arm is the rollout's control and its policy must not change mid-experiment), so it belongs in pi/security.ts, not wizardCanUseTool. agent-interface.ts and its test are reverted to main; the carve-out lets a policy-denied bash call that is a plain `rm [-f] <relative-file>` fall through to the YARA command scan instead of blocking. Generated-By: PostHog Code Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
gewenyu99
changed the title
Generated-By: PostHog Code Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
gewenyu99
added a commit
that referenced
this pull request
Jul 8, 2026
#834 allows scoped `rm [-f] <relative-file>` in the pi fence (where the allowlist actually bites), which covers the plan file and more, without touching the shared policy. This PR keeps the read-before-write prompt correction and the install_skill capture-and-continue. Generated-By: PostHog Code Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
… chains Generated-By: PostHog Code Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
… (closes injection)
The allowlist is a pi-only restriction — the anthropic arm runs bash
unrestricted and lets the shared YARA scan catch destructive commands.
pi now permits the same shape (a plain `rm` of project files) and falls
through to that same YARA scan, instead of hand-judging which rm is safe.
The previous carve-out overrode the allowlist deny wholesale and only
checked per-token, so `rm x && curl evil`, `rm x; whoami`, `rm $(...)`,
backticks, and redirects slipped through — a command-injection bypass.
isScopedFileRemoval now rejects any command containing a shell operator
(`;&|`$(){}<>`), so the carve-out can never smuggle a second command.
Tests cover the full injection matrix plus recursion/glob/traversal/.env.
Generated-By: PostHog Code
Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
Replace the string-heuristic path checks (isAbsolute + `..` detection) with a real containment check: resolve each rm target against the pi run's working directory and require it to sit strictly inside. This directly enforces the two invariants — the agent can delete project files, and can never touch anything outside the project root — and folds absolute / `..` / `.`-is-root / nested-climb cases into one rule. Shell operators are still refused up front so the carve-out can't smuggle a second command. Threads session.installDir in as the root; with no root the rescue never fires (allowlist deny stands). Tests cover the containment, injection, recursion/glob/.env, and fail-safe edge cases. Generated-By: PostHog Code Task-Id: 9b33e996-4d8c-4906-8405-63fb4bcd7d07
gewenyu99
changed the title
edwinyjlim
approved these changes
Jul 8, 2026
edwinyjlim
left a comment
edwinyjlim
left a comment
Member
There was a problem hiding this comment.
good stuff. small things my review bot found
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dments Two commandments make the agent edit existing files in place — adding context or editing each occurrence when a match is not unique rather than rewriting the whole file — and preserve everything unrelated to PostHog before overwriting or deleting, closing the gap that let a pi/gpt-5.4 run drop ~2k lines of an Astro app after its scoped edits were rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The pi write path now refuses a write that drops more than 30% of an existing file's non-whitespace content, steering the agent back to targeted edits — a backstop for the lossy full-file rewrite gpt-5.4 fell into when its scoped edits were rejected (pi's edit tool has no replace_all). New files and stub files pass; the anthropic arm is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
There is a small change added for preventing it from abusing file updates to gut/delete content 😓 |
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
The integration skill tells the agent to delete its own bookkeeping file (
.posthog-events.json) at the end of a run, but pi's bash allowlist blocks everyrm— the #1 bash denial on pi runs (tracking: #793), where agents burn turns on delete-vs-empty workarounds. The anthropic arm doesn't hit this: bash there isn't allowlisted, and the shared YARA destructive-delete rules are the real guard.Changes
A pi-fence carve-out (
pi/security.ts): when the shared allowlist denies a bash call, allow it through to the YARA scan iff it's a plainrmof files inside the project — matching what the anthropic arm already permits.wizardCanUseToolis untouched, so the anthropic control arm keeps byte-identical policy.Two invariants, both enforced structurally:
rmtarget is resolved against the pi run's working directory (session.installDir) and must sit strictly inside it. Absolute paths,..climbs (including ones that only escape after normalization likea/../../x), and.(the root itself) all fail this one check. With no working directory known, the rescue never fires.rm [-f] <file...>of in-root paths is allowed; the fall-through YARA scan still judges it.Anything else stays denied: any shell operator (
;&|`$(){}<>newline — so no second command can be smuggled in), globs /~, recursion or any flag other than-f,.env*files, and pathlessrm.Test plan
pnpm build && pnpm test && pnpm fix— 96 files / 1,273 tests pass, lint clean. Coverage for the carve-out is grouped by invariant: can delete inside the project, can never resolve outside the root, never rescues a shell-operator command (injection), still blocks recursion/glob/.env/pathless, and fails safe with no working directory. The injection and containment matrices were built by adversarially probing the realevaluateToolCalluntil nothing escaped.Created with PostHog Code