Add tool execution guardrails with confirmation popup (#438)

[itomek]

Summary

• Adds a blocking confirmation popup in the Agent UI before run_shell_command executes, so users can Allow, Deny, or Always Allow each shell command the agent wants to run
• CLI path (gaia chat) is unaffected — auto-approves as before
• Reuses the existing PermissionPrompt.tsx / GaiaNotification / notificationStore infrastructure already in the codebase

Architecture

Agent thread (sync)                  SSE consumer (async)          Frontend
────────────────────                 ────────────────────           ────────
_execute_tool("run_shell_command")
  → console.confirm_tool_execution()
    → emit {"type":"tool_confirm"}  ──→  yields SSE event    ──→  ChatView.onAgentEvent
    → threading.Event.wait(60s)                                   → checks localStorage
         ↑                                                        → shows PermissionPrompt
         │                           POST /api/chat/confirm  ←──  → user clicks Allow/Deny
         └── Event.set() ─────────── resolve_confirmation()
  → execute or return denial

Files Changed

File	Change
agents/base/console.py	confirm_tool_execution() on OutputHandler (default True)
agents/base/agent.py	TOOLS_REQUIRING_CONFIRMATION + guardrail in _execute_tool()
ui/sse_handler.py	Blocking confirm_tool_execution() + resolve_confirmation()
ui/server.py	app.state.active_sse_handlers registry
ui/_chat_helpers.py	Register/unregister handler; pass http_request
ui/routers/chat.py	POST /api/chat/confirm endpoint
ui/models.py	ToolConfirmRequest model
webui/src/types/index.ts	tool_confirm event type + fields
webui/src/services/api.ts	confirmToolExecution() + event routing
webui/src/components/ChatView.tsx	Handle tool_confirm, localStorage auto-approve
webui/src/stores/notificationStore.ts	HTTP fallback, Always Allow persistence

Files Reused (no changes)

• PermissionPrompt.tsx — full modal UI with countdown, Allow/Deny/Always Allow, keyboard shortcuts
• GaiaNotification type — already has tool, toolArgs, timeoutSeconds fields

Test plan

Manual (Agent UI):

1. gaia chat --ui → ask "run ls /tmp" → confirm popup appears with command shown
2. Click Allow → command executes, output visible in chat
3. Click Deny → agent responds gracefully without executing
4. Check Remember + Allow → reload page → same prompt skips popup
5. Do nothing for 60 s → popup auto-closes, command denied with warning

CLI regression:

• gaia chat → same shell command prompt → no popup, executes immediately

Fixes #438

[itomek]

@claude Please do a full code review of this PR following the GAIA review checklist. Focus on: security vulnerabilities, code quality, architecture consistency, testing coverage, and documentation. Report all 🔴 Critical and 🟡 Important findings with suggested fixes.

[github-actions]

Claude finished @itomek's task —— View job

---

Code Review Complete

• Read PR diff and changed files
• Security vulnerabilities analysis
• Code quality review
• Architecture consistency check
• Testing coverage evaluation
• Documentation review
• Compile findings report

---

Summary

This PR implements a tool execution guardrail system for the Agent UI, requiring user confirmation before run_shell_command executes. The architecture uses SSE events and threading.Event for blocking synchronization between the agent thread and frontend.

Overall Assessment: Well-designed implementation that follows GAIA patterns. The defense-in-depth approach (existing shell command whitelist + new confirmation popup) is excellent. A few issues need attention.

---

🔴 Critical Findings

1. Input Validation Gap on action Field

File: src/gaia/ui/models.py:362

action: str = Field(..., description="'allow' or 'deny'")

The action field accepts any string. A malicious or buggy client could send action: "allow\x00deny" or other unexpected values. The comparison allowed = request.action == "allow" at routers/chat.py:155 is case-sensitive and doesn't sanitize.

Fix: Use Literal type for strict validation:

from typing import Literal

class ToolConfirmRequest(BaseModel):
    session_id: str
    confirm_id: str
    action: Literal["allow", "deny"]
    remember: bool = False

2. Race Condition in Handler Registration

File: src/gaia/ui/_chat_helpers.py:238-241

if http_request is not None:
    _active = getattr(http_request.app.state, "active_sse_handlers", None)
    if _active is not None:
        _active[request.session_id] = sse_handler

If a user rapidly sends two messages for the same session (bypassing the 0.5s session lock timeout), the second request could overwrite the handler before the first completes, orphaning the first handler's threading.Event.

Mitigation: The existing session lock (asyncio.Lock in routers/chat.py:64) should prevent this in normal operation, but consider logging a warning if a handler already exists for the session.

---

🟡 Important Findings

3. No Authentication on /api/chat/confirm Endpoint

File: src/gaia/ui/routers/chat.py:140-162

The confirm endpoint only validates that a handler exists and the confirm_id matches. A local attacker who guesses/sniffs the confirm_id (UUID v4) could confirm/deny on behalf of the user.

Context: This is acceptable for local-only deployments, but if Agent UI is ever exposed to a network (e.g., via tunnel), this becomes a vulnerability.

Recommendation: Add a comment documenting this security assumption, and consider adding a session token check for tunnel scenarios.

4. Hard-coded 60s Timeout Without Documentation

File: src/gaia/ui/sse_handler.py:576

deadline = time.time() + 60

The timeout is hard-coded in multiple places (sse_handler.py, ChatView.tsx) without configuration. Users may want to adjust this.

Recommendation: Extract to a constant at module level (e.g., TOOL_CONFIRM_TIMEOUT_SECONDS = 60) and document in the user guide.

5. localStorage "Always Allow" Not Scoped to Session

File: src/gaia/apps/webui/src/stores/notificationStore.ts:107-116

The gaia_always_allow_tools localStorage key persists globally across all sessions. If a user clicks "Always Allow" for run_shell_command in a trusted session, it auto-approves in ALL future sessions, including potentially malicious ones where an attacker controls the prompt.

Recommendation: Consider scoping to session ID, or add a UI to clear remembered permissions, or document this behavior prominently.

6. Missing Unit Tests for New Functionality

Grep results: No tests found for TOOLS_REQUIRING_CONFIRMATION, confirm_tool_execution, or SSEOutputHandler.resolve_confirmation.

Recommendation: Add tests for:

• SSEOutputHandler.confirm_tool_execution() (timeout, cancellation, resolve)
• POST /api/chat/confirm endpoint (valid confirm, expired confirm, wrong session)
• Agent _execute_tool() guardrail integration

---

🟢 Minor Findings

7. Non-null Assertion on event.confirm_id

File: src/gaia/apps/webui/src/components/ChatView.tsx:614

api.confirmToolExecution(sessionId, event.confirm_id!, 'allow', false)

TypeScript non-null assertion (!) is used. If the backend ever sends a malformed event, this will throw.

Recommendation: Add defensive check:

if (!event.confirm_id) {
    console.error('[ChatView] tool_confirm event missing confirm_id');
    return;
}

8. Duplicate ALWAYS_ALLOW_KEY Definition

Files: ChatView.tsx:608, notificationStore.ts:108

The localStorage key gaia_always_allow_tools is defined in two places. Consider extracting to a shared constant.

9. Documentation Removed Warning Banner

File: docs/guides/agent-ui.mdx (lines 9-18 removed)

The "Tested Configuration" warning was removed. This is fine if the UI is now production-ready, but ensure this is intentional.

---

Architecture Consistency ✅

• Follows existing OutputHandler pattern correctly
• Reuses PermissionPrompt.tsx and notificationStore as designed
• SSE event types properly extended
• Handler lifecycle (register/unregister) is clean

Documentation 🟡

• PR description is excellent
• Missing: User-facing documentation about the confirmation popup feature
• Missing: Documentation about "Always Allow" behavior and how to reset it

---

Suggested Next Steps

1. Fix Critical Update installer and workflows/actions for CI/CD #1 - Use Literal["allow", "deny"] for action validation
2. Add tests - At minimum, unit tests for the SSEOutputHandler confirmation flow
3. Document the feature - Add a section to docs/guides/agent-ui.mdx explaining the confirmation popup
4. Consider localStorage scoping - Either scope "Always Allow" per-session or document the global behavior

---