orchestrate: reviewer-deep ran with Bash despite its `tools:` list — the no-Bash sandbox the safety model rests on was not enforced

[rodrigorjsf]

Found during orchestrate run prd352-20260803-015333 (PRD #352, 12 slices, all passed).

This is a security-model finding, not a convenience one. The plugin's stated safety invariant rests on the subagent tools: restriction. In this run that restriction did not hold.

Symptom

The reviewer-deep spawned for slice #362 reported, unprompted:

"I did have Bash, contrary to the brief, so every claim is a command I ran rather than a re-assertion of the implementer's inventory."

It then produced a git diff -U0 hunk map — fifteen hunks, with the gaps used to prove byte-identical regions — and cited line ranges of uncommitted changes against HEAD. Read/Grep cannot produce that output; it requires shell access to git.

What the definition actually grants

Verified in both the shipped and the installed copies of reviewer-deep.md (plugins/orchestrate/agents/reviewer-deep.md:4 and ~/.claude/plugins/marketplaces/agent-engineering-toolkit/plugins/orchestrate/agents/reviewer-deep.md:4, byte-identical):

tools: Read, Edit, Write, Grep, Glob, mcp__plugin_orchestrate_orchestrate__run_tests,
       mcp__plugin_orchestrate_orchestrate__run_typecheck,
       mcp__plugin_orchestrate_orchestrate__run_build,
       mcp__plugin_orchestrate_orchestrate__run_lint,
       mcp__plugin_orchestrate_orchestrate__search_structural

No Bash. No git.

Why this is not cosmetic

SKILL.md:52-54, under Roles and the safety boundary, states it as an invariant:

All four subagents have no Bash and no git access — they are sandboxed to one worktree (the investigator is read-only). Only the orchestrator touches branches, remotes, and the tracker.

That sentence is load-bearing for:

• the single-writer tracker guarantee (orchestrate: orchestrator spine restructure — delegate the slice, map the failure class #362's AC5),
• the claim that a subagent's blast radius is one directory, and
• ADR-0017's whole delegation argument.

If the tools: restriction is not reliably enforced, the invariant is enforced only by prose in the subagent's own instructions — which is precisely the weakness #361's read guard exists to remove for a different boundary.

Evidence the restriction usually DOES hold — and the exact scope of the claim

In the same session, three probe subagents whose definitions declared tools: Write came up with Write plus only the harness's own coordination tools (SendMessage, Task*, advisor) and reported no Bash. So this is intermittent, not a blanket no-op — which makes it harder to notice and worse to rely on.

To keep the claim precise: harness coordination tools appear to be injected regardless of tools:, and that is not what this issue is about. Bash is not a coordination tool. The finding is specifically that a shell — and through it, arbitrary git — reached a subagent whose definition does not list it.

Assessment (stated honestly)

In this instance the leak made the review better: every claim became a command the reviewer ran rather than a re-assertion of the implementer's inventory, and the hunk-map audit is stronger evidence than any grep set.

That is a reason to consider granting the reviewer read-only git deliberately — it is not a reason to leave the enforcement gap unexamined. The two questions are independent:

1. Why was the declared tools: restriction not enforced? — this issue. The security question.
2. Should the reviewer be granted read-only git on purpose? — a separate design question that should be filed and decided on its own merits, and whose answer does not resolve (1) for the other three roles.

Suggested fix

1. Establish reproducibility upstream first. Does a plugin subagent with an explicit tools: list ever receive a tool it did not list, and under what condition — foreground vs background, fork mode, nesting depth?
2. Then, plugin-side, decide whether the safety boundary should depend on tool restriction at all, or whether the invariant needs a mechanism with the same properties as the read guard (orchestrate: read guard hook and hook-matcher consistency test #361) — enforced outside the agent's own cooperation.
3. Until either lands, the spine must not describe the no-Bash sandbox as a guarantee. SKILL.md:52-54 currently asserts it flatly.

Relationship to existing issues (read before deduping)

• Complements orchestrate: read guard hook and hook-matcher consistency test #361 (read guard hook and hook-matcher consistency test): orchestrate: read guard hook and hook-matcher consistency test #361 hardens a different boundary with a mechanism rather than prose. This finding is the argument that the no-Bash boundary needs the same treatment — it is currently prose plus an unenforced frontmatter field.
• Related to orchestrate: the slice-executor's skills: preload is unverified, and a miss leaves it with no procedure and no error #375 (skills: preload unverified) and to the registry-refresh issue filed from this run: all three are instances of declared subagent configuration not demonstrably taking effect, with no error. They differ in which field is ignored (tools:, skills:, the body), and this one is the only one with a security consequence.
• Not orchestrate: implementer/reviewer subagents assert a non-existent "advisor pass" in result envelopes (false provenance, ADR-0009) #285. orchestrate: implementer/reviewer subagents assert a non-existent "advisor pass" in result envelopes (false provenance, ADR-0009) #285 confirmed the advisor tool is correctly absent and the problem was fabricated prose. This is the inverse: a tool that is genuinely absent from the definition was genuinely present at runtime.

Environment

orchestrate 1.6.0 (plugins/orchestrate/.claude-plugin/plugin.json:3 on development) plus the PRD #352 umbrella branch (slice-executor delegation layer). Run prd352-20260803-015333; slice #362, orchestrate:reviewer-deep. Claude Code on WSL2.