🪬 fix: Enforce Tool Approval Policy on Headless Agent Runs

packages/api/src/agents/__tests__/run-summarization.test.ts

@@ -1,4 +1,6 @@
+import { z } from 'zod';
 import { encryptV3, logger } from '@librechat/data-schemas';
+import { DynamicStructuredTool } from '@langchain/core/tools';
 import { HumanMessage, AIMessage } from '@langchain/core/messages';
 import { CallbackManager } from '@langchain/core/callbacks/manager';
 import {
@@ -14,6 +16,7 @@ import type { BaseMessage } from '@langchain/core/messages';
 import type { OpenAI } from 'openai';
 import type { ModelBoundChatModelCallback } from '~/middleware/modelBoundContent';
 import type { OpenAIConfiguration, AzureOptions } from '~/types';
+import { clearToolApprovalHooks, registerToolApprovalHook } from '~/agents/hitl/hooks';
 import { createRun, isAskUserQuestionAdminDisabled } from '~/agents/run';
 import { initializeOpenAI } from '~/endpoints/openai/initialize';
 import { getOpenAIConfig } from '~/endpoints/openai/config';
@@ -93,7 +96,15 @@ jest.mock('~/agents/checkpointer', () => ({
 
 import { ChatOpenAI } from '@librechat/agents/llm/openai';
 import { ChatOpenRouter } from '@librechat/agents/llm/openrouter';
-import { Run, Providers, buildChildInputs, InMemorySubagentTaskStore } from '@librechat/agents';
+import {
+  Run,
+  Providers,
+  HookRegistry,
+  ToolNode,
+  buildChildInputs,
+  InMemorySubagentTaskStore,
+  executeHooks,
+} from '@librechat/agents';
 
 /** Minimal RunAgent factory */
 function makeAgent(
@@ -4407,10 +4418,8 @@ describe('createRun deferred-tool replay (HITL resume)', () => {
 // ---------------------------------------------------------------------------
 // Suite: HITL wiring gated to resumable callers (Codex J3)
 //
-// The tool-approval wiring (humanInTheLoop switch + PreToolUse hook) must engage ONLY for
-// callers that implement the pause/resume lifecycle. AgentClient passes hitlCapable: true;
-// the OpenAI-compatible + Responses controllers don't, so an approval-gated tool can't
-// pause on a route with no approval surface or resume endpoint.
+// Tool-approval policy must apply to every caller, while only an interactive
+// caller may pause for an 'ask' decision. API-key endpoints have no resume surface.
 // ---------------------------------------------------------------------------
 describe('HITL wiring is gated on hitlCapable', () => {
   const hitlAppConfig = {
@@ -4441,64 +4450,187 @@ describe('HITL wiring is gated on hitlCapable', () => {
     expect(config.hooks).toBeDefined();
   });
 
-  it('does NOT attach HITL for a non-resumable caller even when approval is enabled', async () => {
-    const config = await runAndGetConfig({ hitlCapable: false });
+  it.each([
+    [
+      { enabled: true, mode: 'bypass', deny: ['delete_*'], allow: ['delete_file'] },
+      'delete_file',
+      'deny',
+    ],
+    [{ enabled: true, mode: 'dontAsk', allow: ['read_*'] }, 'read_file', 'allow'],
+    [{ enabled: true, mode: 'dontAsk', allow: ['read_*'] }, 'write_file', 'deny'],
+    [{ enabled: true, mode: 'default' }, 'write_file', 'ask'],
+  ] as const)('evaluates headless tool policy for %s on %s', async (policy, toolName, decision) => {
+    const appConfig = {
+      ...hitlAppConfig,
+      endpoints: { [EModelEndpoint.agents]: { toolApproval: policy } },
+    } as unknown as AppConfig;
+    const config = await runAndGetConfig({ hitlCapable: false, appConfig });
     expect(config).not.toHaveProperty('humanInTheLoop');
-    expect(config.graphConfig).toBeDefined();
-    // No checkpointer either — the run is identical to the no-HITL path.
     expect(
       (config.graphConfig as { compileOptions?: { checkpointer?: unknown } }).compileOptions
         ?.checkpointer,
     ).toBeUndefined();
+    const result = await executeHooks({
+      registry: config.hooks as HookRegistry,
+      input: {
+        hook_event_name: 'PreToolUse',
+        runId: 'headless-test',
+        toolName,
+        toolInput: {},
+        toolUseId: 'call-1',
+      },
+      matchQuery: toolName,
+    });
+    expect(result.decision).toBe(decision);
+  });
+
+  it.each([
+    { policy: { enabled: true, mode: 'bypass', deny: ['delete_record'] }, name: 'delete_record' },
+    { policy: { enabled: true, mode: 'default' }, name: 'read_record' },
+  ] as const)(
+    'blocks a real SDK direct tool before execution for $name',
+    async ({ policy, name }) => {
+      const config = await runAndGetConfig({
+        appConfig: {
+          ...hitlAppConfig,
+          endpoints: { [EModelEndpoint.agents]: { toolApproval: policy } },
+        } as unknown as AppConfig,
+      });
+      const body = jest.fn(async () => 'executed');
+      const tool = new DynamicStructuredTool({
+        name,
+        description: 'A test-only tool',
+        schema: z.object({}),
+        func: body,
+      });
+      const node = new ToolNode({
+        tools: [tool],
+        agentId: 'agent_1',
+        hookRegistry: config.hooks as HookRegistry,
+      });
+      const result = await node.invoke(
+        {
+          messages: [
+            new AIMessage({ content: '', tool_calls: [{ id: 'call-1', name, args: {} }] }),
+          ],
+        },
+        { configurable: { run_id: 'headless-test', thread_id: 'thread-1' } },
+      );
+      expect(body).not.toHaveBeenCalled();
+      expect(JSON.stringify(result)).toContain('Blocked:');
+    },
+  );
+
+  it('registers trusted per-run hooks on headless calls without prompting', async () => {
+    const factory = jest.fn(() => async () => ({ decision: 'deny' as const }));
+    const unregister = registerToolApprovalHook(factory, { matcher: '^write_file$' });
+    try {
+      const config = await runAndGetConfig({
+        user: { id: 'user-1' },
+        appConfig: {
+          ...hitlAppConfig,
+          endpoints: {
+            [EModelEndpoint.agents]: { toolApproval: { enabled: true, mode: 'bypass' } },
+          },
+        } as AppConfig,
+      });
+      expect(factory).toHaveBeenCalledWith(expect.objectContaining({ userId: 'user-1' }));
+      expect(config).not.toHaveProperty('humanInTheLoop');
+      const result = await executeHooks({
+        registry: config.hooks as HookRegistry,
+        input: {
+          hook_event_name: 'PreToolUse',
+          runId: 'headless-test',
+          toolName: 'write_file',
+          toolInput: {},
+          toolUseId: 'call-1',
+        },
+        matchQuery: 'write_file',
+      });
+      expect(result.decision).toBe('deny');
+    } finally {
+      unregister();
+      clearToolApprovalHooks();
+    }
+  });
+
+  it('keeps approval fully off when the endpoint disables it', async () => {
+    const config = await runAndGetConfig({
+      appConfig: {
+        ...hitlAppConfig,
+        endpoints: { [EModelEndpoint.agents]: { toolApproval: { enabled: false, deny: ['*'] } } },
+      } as AppConfig,
+    });
+    expect((config.hooks as HookRegistry).hasHookFor('PreToolUse')).toBe(false);
+    expect(config).not.toHaveProperty('humanInTheLoop');
   });
 
   it('defaults to non-HITL when hitlCapable is omitted', async () => {
     const config = await runAndGetConfig({});
     expect(config).not.toHaveProperty('humanInTheLoop');
   });
 
-  it('heals aliases discovered when a lazy subagent resolves', async () => {
-    const alias = { name: 'delete_mcp_acme', aliasName: 'acme_delete_mcp_acme' };
-    const resolvedChild = makeAgent({ id: 'lazy-child', mcpToolAliases: [alias] });
-    const lazyChild = {
-      ...makeAgent({ id: 'lazy-child' }),
-      configId: 'lazy-child:v1',
-      resolve: jest.fn().mockResolvedValue(resolvedChild),
-    };
-    const parent = makeAgent({
-      subagents: { enabled: true, allowSelf: false },
-      lazySubagentConfigs: [lazyChild],
-    });
-    const appConfig = {
-      ...hitlAppConfig,
-      endpoints: {
-        [EModelEndpoint.agents]: {
-          toolApproval: { enabled: true, mode: 'bypass', deny: [alias.aliasName] },
+  it.each([true, false])(
+    'heals aliases discovered in lazy subagents (HITL=%s)',
+    async (hitlCapable) => {
+      const alias = { name: 'delete_mcp_acme', aliasName: 'acme_delete_mcp_acme' };
+      const resolvedChild = makeAgent({ id: 'lazy-child', mcpToolAliases: [alias] });
+      const lazyChild = {
+        ...makeAgent({ id: 'lazy-child' }),
+        configId: 'lazy-child:v1',
+        resolve: jest.fn().mockResolvedValue(resolvedChild),
+      };
+      const parent = makeAgent({
+        subagents: { enabled: true, allowSelf: false },
+        lazySubagentConfigs: [lazyChild],
+      });
+      const appConfig = {
+        ...hitlAppConfig,
+        endpoints: {
+          [EModelEndpoint.agents]: {
+            toolApproval: { enabled: true, mode: 'bypass', deny: [alias.aliasName] },
+          },
         },
-      },
-    } as unknown as AppConfig;
+      } as unknown as AppConfig;
 
-    await createRun({
-      agents: [parent] as never,
-      signal: new AbortController().signal,
-      appConfig,
-      streaming: true,
-      streamUsage: true,
-      hitlCapable: true,
-    });
-    const config = (Run.create as jest.Mock).mock.calls[0][0] as Record<string, unknown>;
-    const hooks = config.hooks as { getMatchers: (event: string) => unknown[] };
-    const lazyConfig = (
-      (config.graphConfig as { agents: Array<Record<string, unknown>> }).agents[0]
-        .subagentConfigs as Array<Record<string, unknown>>
-    ).find((entry) => entry.configId === lazyChild.configId);
-
-    expect(hooks.getMatchers('PreToolUse')).toHaveLength(1);
-    await (lazyConfig?.resolveAgentInputs as (context: never) => Promise<unknown>)({
-      signal: new AbortController().signal,
-    } as never);
-    expect(hooks.getMatchers('PreToolUse')).toHaveLength(1);
-  });
+      await createRun({
+        agents: [parent] as never,
+        signal: new AbortController().signal,
+        appConfig,
+        streaming: true,
+        streamUsage: true,
+        hitlCapable,
+      });
+      const config = (Run.create as jest.Mock).mock.calls[0][0] as Record<string, unknown>;
+      const hooks = config.hooks as { getMatchers: (event: string) => unknown[] };
+      const lazyConfig = (
+        (config.graphConfig as { agents: Array<Record<string, unknown>> }).agents[0]
+          .subagentConfigs as Array<Record<string, unknown>>
+      ).find((entry) => entry.configId === lazyChild.configId);
+
+      const decisionForAlias = async () =>
+        (
+          await executeHooks({
+            registry: config.hooks as HookRegistry,
+            input: {
+              hook_event_name: 'PreToolUse',
+              runId: 'alias-test',
+              toolName: alias.name,
+              toolInput: {},
+              toolUseId: 'call-1',
+            },
+            matchQuery: alias.name,
+          })
+        ).decision;
+      expect(hooks.getMatchers('PreToolUse')).toHaveLength(1);
+      expect(await decisionForAlias()).toBe('allow');
+      await (lazyConfig?.resolveAgentInputs as (context: never) => Promise<unknown>)({
+        signal: new AbortController().signal,
+      } as never);
+      expect(hooks.getMatchers('PreToolUse')).toHaveLength(1);
+      expect(await decisionForAlias()).toBe('deny');
+    },
+  );
 });
 
 // ---------------------------------------------------------------------------

packages/api/src/agents/hitl/hooks.ts

@@ -69,10 +69,10 @@ const registeredHooks: RegisteredHook[] = [];
  * Register a programmatic tool-approval hook (process-wide). Call once at startup. Returns an
  * unregister function that removes exactly this registration.
  *
- * Inert unless tool approval is enabled AND the caller is HITL-capable — hooks only run inside
- * the `PreToolUse` fold of an HITL run (see {@link buildToolApprovalHooks} /
- * `buildHITLRunWiring`). They compose with, and can only tighten, the static
- * `endpoints.agents.toolApproval` policy.
+ * Inert unless tool approval is enabled. Hooks run in the `PreToolUse` fold of
+ * both interactive and headless runs (see {@link buildToolApprovalHooks} /
+ * `buildHITLRunWiring`). Headless `ask` decisions are denied without pausing.
+ * They compose with, and can only tighten, the static policy.
  *
  * @param factory Builds the per-run hook from its context; return `undefined` to opt out.
  * @param options.matcher Optional regex string matched against the tool name — omit to run for

packages/api/src/agents/hitl/runtime.ts

@@ -25,10 +25,10 @@ export function buildToolApprovalExecutionConfig(
 /**
  * The HITL fragment spread onto a `RunConfig` when tool approval is enabled.
  *
- * Kept as one object so the run seam attaches the opt-in switch and the policy
- * hook together — they're meaningless apart. The checkpointer is resolved
- * separately (it's an async, process-wide singleton) and merged into
- * `graphConfig.compileOptions` at the call site.
+ * The policy hooks apply to every caller. Only callers that support pause/resume
+ * also attach the `humanInTheLoop` switch and the durable checkpointer.
+ * The checkpointer is resolved separately (it's an async, process-wide singleton)
+ * and merged into `graphConfig.compileOptions` at the call site.
  */
 export interface HITLRunWiring {
   humanInTheLoop: { enabled: true };
@@ -41,9 +41,9 @@ export interface HITLRunWiring {
 }
 
 /**
- * Assemble the run-level HITL wiring for a tool-approval policy, or `undefined`
- * when HITL is disabled (the default) — in which case the run attaches nothing
- * and behaves exactly as it did before this feature.
+ * Assemble tool-approval policy hooks and optional interactive HITL wiring, or
+ * `undefined` when the policy is disabled. Non-resumable callers attach only
+ * `hooks`; the SDK denies `ask` rather than pausing without a resume surface.
  *
  * The returned `hooks` registry carries the static-config `PreToolUse` policy hook built
  * from {@link mapToolApprovalPolicy} (an enabled policy with no allow/deny/ask lists falls