Fix copilot permission denials in non-interactive mode

[wesm]

Summary

• Add --allow-all-tools with a deny-list of destructive operations (write, git push, git commit, git checkout, git reset, git rebase, git merge, git stash, git clean, rm) for review mode; agentic mode allows all tools unrestricted
• Add -s (silent) flag to suppress interactive stats in daemon context
• Add copilotSupportsAllowAllTools() feature detection with sync.Map caching, matching the existing pattern from Claude and Codex agents — old copilot binaries degrade gracefully to current behavior
• Respect AllowUnsafeAgents() global override, same as all other agents

Closes #555

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (4e92ffc)

Verdict: The proposed review-mode permissions for Copilot introduce a high-severity security risk by allowing arbitrary shell commands that can mutate the local environment.

High Severity

• Location: internal/agent/copilot.go (Lines
  31, 34, [50](/home/rob
  orev/repos/roborev/internal/agent/copilot.go#L50))
• Problem: Review mode passes --allow-all-tools and relies on an insufficient deny-list (e.g., blocking write, rm, and some git sub
  commands) to enforce read-only behavior. This still permits the generic shell(...) tool, allowing execution of arbitrary write-capable commands (like sed -i, cp, mv, git apply, python -c, etc.). Because review prompts can include attacker-controlled content from shared repositories
  or commit messages, this creates a prompt-injection-to-command-execution vulnerability that can mutate the local checkout or other files despite being in "review mode".
• Suggested Fix: Do not combine --allow-all-tools with a small deny-list for review mode. Either deny the generic
  shell tool entirely, use a strict allow-list of explicitly read-only tools, or fall back to the previous no-auto-approval behavior if a safe read-only policy cannot be enforced.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[wesm]

Accepted risk. roborev is supposed to be used with codebases you trust or if exposed to untrusted code, run from a sandbox where the reviewer agents can do no harm.

[roborev-ci]

roborev: Combined Review (1ebf63b)

Review complete: Found 1 High severity security issue regarding Copilot tool permissions and 1 Medium severity concurrency issue in the hook runner.

High Severity

• Location: internal/agent/copilot.go:53, internal/agent/copilot.go:57 (around copilotReviewDenyTools / buildArgs)
• Problem: Review mode enables --allow-all-tools and
  attempts to enforce read-only behavior using a narrow deny-list. This is a severe security risk for untrusted review input from shared repos. The generic shell tool remains available, meaning a malicious diff or commit message could instruct Copilot to use shell redirection, file mutation commands (sed -i, python -c , mv), or exfiltrate data over the network (curl, scp). Denying the built-in write tool and specific git verbs does not prevent arbitrary command execution.
• Fix: Do not enable --allow-all-tools in default review mode unless it can
  be constrained by a true read-only allow-list or a secure sandbox. As an immediate fallback, deny shell(*) entirely in review mode or require manual approval for all shell/tool execution.

Medium Severity

• Location: internal/daemon/hooks.go:123
• Problem: hr.broadcaster.Unsubscribe(hr.subID) is called after hr.wg.Wait(). If the hook runner's event processing loop exits immediately when stopCh is closed, the subscriber channel is no longer being read. Any events broadcast
  while waiting for in-flight hooks to finish could fill the channel buffer and potentially block the broadcaster.
• Fix: Move hr.broadcaster.Unsubscribe(hr.subID) to immediately follow close(hr.stopCh) and before hr.wg.Wait().

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (b3b6ba5)

Verdict: The PR successfully implements Copilot non-interactive permission handling but introduces a potential deadlock during HookRunner shutdown.

Medium

• Location: internal/daemon/hooks.go:125-126
• Problem: Stop() closes stopCh before removing the broadcaster subscription (Unsubscribe). This terminates the event loop early, creating a race condition where a concurrent publish can block on the now-unread subscriber
  channel, causing the broadcaster to deadlock and shutdown to hang despite the new Wait().
• Fix: Make the unsubscribe operation atomic with the shutdown of the event loop. Specifically, move hr.broadcaster.Unsubscribe(hr.subID) to execute before close(hr.stop Ch) or add an explicit loop-exit handshake.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[wesm]

False positive. Merging this