fix(security): gate brain-emitted tool calls on the local chat path - #58
fix(security): gate brain-emitted tool calls on the local chat path#58AetherAI3 wants to merge 3 commits into
Conversation
Spec-only. Companion to AETHER-CLOUD's task-chain agent panel spec - this repo and AETHER-CLOUD share a backend (the CODEPRO multi-agent swarm protocol), so this covers the terminal-CLI side of the same investigation. Findings specific to this repo: the live "task chain popout" is src/ui/workflow_viewer.ts + chat.ts's viewerOpen toggle (not a sidebar - an inline scrollback popout). Two independent bugs found: (1) every arrow-key move re-prints the whole agent tree instead of redrawing in place, stacking duplicates in scrollback; (2) WorkflowViewerState silently drops phase_start/phase_done frames (no case in its switch) even though src/ui/task_chain.ts already models phases correctly - task_chain.ts is unused in production (test-only), a superseded prototype never absorbed. Also flags that selectAgent/renderAgentFeed are fully built but never wired to a key handler. Proposes a phased fix: everything except per-agent tokens/tool-calls is achievable with zero protocol changes (client- side timestamps, session telemetry, prompt text already in hand); the wire extension for tokens/tool-calls mirrors the additive fields proposed in the AETHER-CLOUD companion spec. No src/ changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-down Implements PR #42's spec (docs/specs/2026-07-10-workflow-viewer-agent-panel-design.md), Approach A: upgrade the existing in-REPL workflow popout (src/ui/workflow_viewer.ts + chat.ts) in place instead of adopting TuiLayout as a persistent sidebar. - Fix a redraw bug where every arrow-key move re-printed the whole tree instead of redrawing in place, stacking duplicate copies in terminal scrollback (viewerClearSequence/viewerLineCount replace a single \r\x1b[2K). - Absorb task_chain.ts's phase-tracking (phase_start/phase_done) into WorkflowViewerState, then delete task_chain.ts as dead code (zero production call sites) and migrate its meaningful test cases. - Wire selectAgent/renderAgentFeed to Enter/Escape (built but previously unreachable) for a per-agent raw-feed drill-down, and Left/Right to togglePhaseExpanded for phase collapse/expand. - Extend AgentDoneFrame with optional tokens/toolCalls/durationMs (Tier-2/3 metrics), decoded as undefined (never a fabricated 0) when the backend hasn't sent them — wired through both cloud paths that actually feed the live popout (stream.ts's SSE decoder + chat.ts) and the NDJSON local-brain decoder (brain_protocol.ts) + brain_cloud.ts's mapping, for consistency. - Live-refresh the popout on frame arrival while open (previously frozen until the next keypress), and clear it from the terminal when the workflow finishes mid-view (previously left stuck in scrollback). - Fix docs/CONTRACTS.md drift: PROTOCOL_VERSION was documented as 2 while the code has been at 3 for a while; the 7 workflow-swarm frame types were never in the message table at all; Invariant 2's tool list was missing web_search/web_fetch. Reviewed via a 4-dimension adversarial code-review pass (correctness x2, test-coverage, simplification) with an independent refute pass; 11 of 13 confirmed findings fixed in this same commit (including a CRITICAL: the wire extension had only reached brain_protocol.ts's NDJSON decoder, not stream.ts's SSE decoder the live chat popout actually runs on). Two LOW-severity findings are deliberately deferred: an agent_spawn/phase_start for a phaseN outside the declared workflow_start.phases set is client-side unenforceable (backend protocol violation), and chat.ts's raw-mode key-dispatch wiring has no test harness in this codebase yet to exercise directly (covered indirectly via the pure state/render helpers it calls). 622 tests pass (npm run clean && npm run build && node --test), tsc --strict clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AGENT-CMD-001. runLocalTurn dispatched every brain-emitted tool_call straight to
ToolExecutor.executeAsync, so a model-chosen run_shell command string reached
ToolExecutor.run -> spawnSync(command, { shell: true|ComSpec }) with no executable
allow-list and no approval — while the `code` command gated the identical sink
through decideGate. A guard only some callers apply is not a guard.
The gate construction moved out of commands/code.ts into core/tool_gate.ts and
both commands now build it from there, so the two cannot drift apart again. Policy
is unchanged: decideGate stays the single source of truth, and the shared module
only performs the prompt I/O it asks for.
interactive terminal -> y/N prompt showing the actual command
--yes / permissionMode -> explicit operator opt-out, no prompt
non-interactive terminal -> FAILS CLOSED, the call is refused
A refused call is never executed; the brain receives
{output: "[denied: <tool> not approved by user]", exitCode: 1} and the turn
continues honestly. Read-only tools (read_file, repo_search) are unaffected, so
ordinary local chat only prompts when the model actually asks to mutate or run
something. This does change local-chat UX, deliberately, to make command safety
consistent across the Agent.
test/tool_gate.test.ts covers the three required behaviours plus the regression
that caused this: read-only allowed silently, TTY prompts with the command shown,
declining refuses, --yes opts out, no TTY fails closed without attempting a
prompt, skip mode allows, long commands truncate, deniedResult shape, the
chat-path source invariant (gate built and consulted before executeAsync, with
deniedResult on refusal), and that neither command re-implements decideGate
inline.
632/632 tests pass.
Residual, recorded not fixed here: src/core/smoke.ts also dispatches brain tool
calls ungated. It runs a fixed internal SMOKE_PROMPT and has no command-surface
caller, so it is lower severity and out of scope for this focused repair.
|
Closing this as a security repair — current Evidence
That is the same policy this PR proposed — interactive prompt, Why the finding looked liveThe scanned commit
Not lostThe workflow-viewer agent panel ( |
Summary
runLocalTurndispatched every brain-emittedtool_callstraight toToolExecutor.executeAsync, so a model-chosenrun_shellcommand reachedToolExecutor.run -> spawnSync(command, { shell: true|ComSpec })with no executable allow-list and no approval — while thecodecommand gated the identical sink throughdecideGate. A guard only some callers apply is not a guard.Found by Predator as
AGENT-CMD-001. Least-privilege / guard-completeness finding, not a live exploit: it needs a user-initiated local session and a model-emitted command. The Agent's file tools were never affected —ToolExecutor.safe()canonicalizes then enforces within-root.The change
Gate construction moved out of
commands/code.tsintocore/tool_gate.ts; both commands build it from there so they cannot drift apart again. Policy is unchanged —decideGateremains the single source of truth and the shared module only performs the prompt I/O it asks for.A refused call is never executed; the brain receives
{output: "[denied: <tool> not approved by user]", exitCode: 1}and the turn continues.UX note, deliberate: local chat now prompts when the model asks to mutate or run something. Read-only tools (
read_file,repo_search) are unaffected, so ordinary chat is unchanged.Test plan
test/tool_gate.test.ts:--yesopts out without promptingskipmode allowsdeniedResultshapeexecuteAsync,deniedResulton refusaldecideGateinline (anti-drift)632/632 tests pass.
Residual, recorded not fixed here
src/core/smoke.tsalso dispatches brain tool calls ungated. It runs a fixed internalSMOKE_PROMPTand has no command-surface caller, so it is lower severity and deliberately out of scope for this focused repair.