fix(agent): tell the user a prefix approval leaves the sandbox

[Vasanthdev2004]

Approving a command prefix currently does more than the prompt says.

shellExecutionArgsForApproval (loop.go:1358, run after the permission decision) rewrites the call to require_escalated. That resolves to a nil sandbox engine, so the command runs unsandboxed on the host. The engine's escalation gate at engine.go:385 is then satisfied by PermissionGranted, which is the approval the user just gave for the sandboxed form.

The escalation is deliberate and I am not touching it. proposedCommandPrefix refuses to offer a prefix when any other segment of the command is not known-safe, precisely because approval covers the whole command, and there is a test documenting that. The problem is only that nobody tells the person approving. They pick "allow command prefix for session" and also get "run it outside the sandbox".

So this discloses instead of removing:

• PermissionRequest gains PrefixApprovalEscalates, computed by shellPrefixApprovalEscalates from the same guards the rewrite consults.
• The two prefix options append "(runs outside the sandbox)" when it is set.
• Both the TUI and the ACP path carry it. An editor user is deciding on the same terms as a terminal user, so a TUI-only disclosure would not be a fix.

The disclosure is conditional. When escalation will not happen (no sandbox, denied reads active, or the call already asked for escalation) the label stays as it was, because a prompt that warns on every prefix is one people learn to ignore.

On the tests

The main test asserts that the disclosure and the rewrite agree on every input, rather than checking each side against a fixed expectation. That is the part worth reviewing: the bug being fixed is exactly the two disagreeing, so if someone later changes the rewrite's guards and forgets the flag, this fails. Two independent expected-value tests would not catch it.

Mutation-verified in both directions:

• helper forced to always-false (the pre-fix behaviour) fails the coupling test on 5 cases
• label made unconditional fails both TUI assertions

One case is worth calling out because it contradicts the obvious reading: an unrecognized sandbox_permissions value is not an opt-out. The rewrite overwrites it, so the disclosure has to fire. I originally wrote that case asserting the wrong thing and it passed silently until the mutation run.

Verification

go build ./..., go vet, gofmt -l clean on the three packages. internal/acp and internal/sandbox fully green. Two failures in internal/agent and internal/tui (TestEagerToolSchemaTokenBudget, TestAltScreenTranscriptScrollKeepsFooterFixed) reproduce identically on a clean tree with my changes stashed, so they are pre-existing and local to Windows.

Kept off #808 deliberately. This is a cross-platform consent fix and #808 is already carrying a scope objection.

Summary by CodeRabbit

• New Features

  • Permission prompts now clearly indicate when approving a command prefix will run it outside the sandbox.
  • The disclosure appears only when sandbox restrictions will actually be bypassed and applies to relevant prefix-approval options.

• Bug Fixes

  • Improved consistency between permission prompts and the resulting command execution behavior.
  • Preserved permission option selections when displaying the new disclosure.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4099adb6-aef9-4a01-b18b-ab543dd949b4

📥 Commits

Reviewing files that changed from the base of the PR and between 7f39a63 and f5f24fe.

📒 Files selected for processing (7)

• internal/acp/permission.go
• internal/acp/permission_escalation_test.go
• internal/agent/loop.go
• internal/agent/prefix_escalation_disclosure_test.go
• internal/agent/types.go
• internal/tui/permission_prompt.go
• internal/tui/permission_prompt_escalation_test.go

---

Walkthrough

The agent now marks permission requests when prefix approval can run shell commands outside the sandbox. ACP and TUI labels conditionally show this disclosure. Tests cover escalation detection, label behavior, and option ID round-tripping.

Changes

Prefix escalation disclosure

Layer / File(s)	Summary
Compute and expose escalation state internal/agent/types.go, internal/agent/loop.go, internal/agent/prefix_escalation_disclosure_test.go	Permission requests include PrefixApprovalEscalates. The agent sets it for eligible sandboxed shell commands. Tests cover command, permission, and sandbox combinations.
Render conditional permission labels internal/acp/permission.go, internal/tui/permission_prompt.go	ACP and TUI append “(runs outside the sandbox)” to escalating prefix-approval options. Other options and non-escalating labels remain unchanged.
Validate disclosure and decision round trips internal/acp/permission_escalation_test.go, internal/tui/permission_prompt_escalation_test.go	Tests verify conditional disclosures, non-prefix behavior, escalation parity, and preservation of permission option IDs.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ShellTool
  participant AgentLoop
  participant PermissionRequest
  participant ACP
  participant TUI
  ShellTool->>AgentLoop: request shell permission
  AgentLoop->>PermissionRequest: compute PrefixApprovalEscalates
  PermissionRequest->>ACP: provide permission request
  PermissionRequest->>TUI: provide permission request
  ACP-->>PermissionRequest: render disclosed prefix options
  TUI-->>PermissionRequest: render disclosed prefix options

Loading

Possibly related PRs

• Gitlawb/zero#838: Both changes update permission handling in internal/agent/loop.go.
• Gitlawb/zero#883: Both changes update permission and sandbox escalation behavior in internal/agent/loop.go.

Suggested reviewers: gnanam1990, anandh8x

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: informing users that prefix approval can run commands outside the sandbox.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/prefix-approval-sandbox-disclosure

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Tests: go test ./...
• [pass] Build: go run ./cmd/zero-release build
• [pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: f5f24fe52570
Changed files (7): internal/acp/permission.go, internal/acp/permission_escalation_test.go, internal/agent/loop.go, internal/agent/prefix_escalation_disclosure_test.go, internal/agent/types.go, internal/tui/permission_prompt.go, internal/tui/permission_prompt_escalation_test.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.