diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 388bddda5f..b6f8990b07 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -49,7 +49,10 @@ const { getAgentCheckpointer, hasDurableAgentInterruptCheckpoint, isHITLEnabled, + resolveToolApprovalPolicy, buildToolApprovalHooks, + collectAttachedCodeEnvironmentAgentIds, + createAttachedCodeEnvironmentPolicyHook, agentRunUsesCheckpointer, canAgentGraphPause, getPluginHookSource, @@ -3786,7 +3789,14 @@ class AgentClient extends BaseClient { * spending on provider work. */ /** @type {AppConfig['endpoints']['agents']} */ const agentsEConfig = appConfig.endpoints?.[EModelEndpoint.agents]; - const resolvedToolApprovalHooks = isHITLEnabled(agentsEConfig?.toolApproval) + const topLevelAgents = [this.options.agent, ...(this.agentConfigs?.values() ?? [])]; + const attachedCodeEnvironmentAgentIds = + collectAttachedCodeEnvironmentAgentIds(topLevelAgents); + const effectiveToolApprovalPolicy = resolveToolApprovalPolicy({ + endpoint: agentsEConfig?.toolApproval, + attachedCodeEnvironment: attachedCodeEnvironmentAgentIds.size > 0, + }); + const resolvedToolApprovalHooks = isHITLEnabled(effectiveToolApprovalPolicy) ? buildToolApprovalHooks({ userId: this.options.req?.user?.id, conversationId: this.conversationId, @@ -3794,19 +3804,28 @@ class AgentClient extends BaseClient { appConfig, }) : undefined; - const topLevelAgents = [this.options.agent, ...(this.agentConfigs?.values() ?? [])]; + const admissionToolApprovalHooks = [ + ...(resolvedToolApprovalHooks ?? []), + ...(attachedCodeEnvironmentAgentIds.size > 0 + ? [ + { + hook: createAttachedCodeEnvironmentPolicyHook(attachedCodeEnvironmentAgentIds), + }, + ] + : []), + ]; const askUserQuestionAdminDisabled = isAskUserQuestionAdminDisabled(appConfig); const runCanPause = canAgentGraphPause({ - policy: agentsEConfig?.toolApproval, + policy: effectiveToolApprovalPolicy, agents: topLevelAgents, hostGeneratedToolNames: this.options.subagentTasks == null ? undefined : [Constants.CHECK_BACKGROUND_TASK], - resolvedProgrammaticHooks: resolvedToolApprovalHooks, + resolvedProgrammaticHooks: admissionToolApprovalHooks, pluginHookSource: getPluginHookSource(), askUserQuestionAdminDisabled, }); const runUsesCheckpointer = agentRunUsesCheckpointer({ - policy: agentsEConfig?.toolApproval, + policy: effectiveToolApprovalPolicy, agents: topLevelAgents, askUserQuestionAdminDisabled, }); diff --git a/packages/api/src/agents/hitl/byom.spec.ts b/packages/api/src/agents/hitl/byom.spec.ts new file mode 100644 index 0000000000..03495616f1 --- /dev/null +++ b/packages/api/src/agents/hitl/byom.spec.ts @@ -0,0 +1,110 @@ +import { + assertAttachedCodeEnvironmentApprovalSupported, + collectAttachedCodeEnvironmentAgentIds, + createAttachedCodeEnvironmentPolicyHook, +} from './byom'; + +const signal = new AbortController().signal; + +describe('createAttachedCodeEnvironmentPolicyHook', () => { + test('asks before a shell action in an attached environment', async () => { + const hook = createAttachedCodeEnvironmentPolicyHook(new Set(['attached-agent'])); + + await expect( + hook({ toolName: 'bash_tool', executingAgentId: 'attached-agent' } as never, signal), + ).resolves.toEqual({ + decision: 'ask', + reason: 'bash_tool can modify your attached code environment', + }); + }); + + test('allows the baseline policy to auto-approve read-only coding actions', async () => { + const hook = createAttachedCodeEnvironmentPolicyHook(new Set(['attached-agent'])); + + await expect( + hook({ toolName: 'read_file', executingAgentId: 'attached-agent' } as never, signal), + ).resolves.toEqual({}); + }); + + test('does not apply the BYOM baseline to a managed-environment sibling agent', async () => { + const hook = createAttachedCodeEnvironmentPolicyHook(new Set(['attached-agent'])); + + await expect( + hook({ toolName: 'bash_tool', executingAgentId: 'managed-agent' } as never, signal), + ).resolves.toEqual({}); + }); + + test('fails closed when a risky call cannot be attributed to an agent', async () => { + const hook = createAttachedCodeEnvironmentPolicyHook(new Set(['attached-agent'])); + + await expect(hook({ toolName: 'write_file' } as never, signal)).resolves.toMatchObject({ + decision: 'ask', + }); + }); + + test.each(['create_file', 'edit_file'])( + 'asks before the canonical host file action %s', + async (toolName) => { + const hook = createAttachedCodeEnvironmentPolicyHook(new Set(['attached-agent'])); + + await expect( + hook({ toolName, executingAgentId: 'attached-agent' } as never, signal), + ).resolves.toMatchObject({ decision: 'ask' }); + }, + ); +}); + +describe('collectAttachedCodeEnvironmentAgentIds', () => { + test('finds attached agents across eager and graph subagents without including managed agents', () => { + const agents = [ + { + id: 'root', + codeExecutionContext: { environmentType: 'managed' }, + subagentAgentConfigs: [ + { id: 'attached-child', codeExecutionContext: { environmentType: 'attached' } }, + ], + subagentGraphConfigs: [ + { + memberConfigs: [ + { id: 'managed-member', codeExecutionContext: { environmentType: 'managed' } }, + { id: 'attached-member', codeExecutionContext: { environmentType: 'attached' } }, + ], + }, + ], + }, + ]; + + expect(collectAttachedCodeEnvironmentAgentIds(agents)).toEqual( + new Set(['attached-child', 'attached-member']), + ); + }); +}); + +describe('assertAttachedCodeEnvironmentApprovalSupported', () => { + test('rejects attached environments on callers without an approval/resume surface', () => { + expect(() => + assertAttachedCodeEnvironmentApprovalSupported({ + hasAttachedCodeEnvironment: true, + hitlCapable: false, + approvalExplicitlyDisabled: false, + }), + ).toThrow('Attached code environments require a tool-approval capable client'); + }); + + test('allows the admin emergency override and non-attached environments', () => { + expect(() => + assertAttachedCodeEnvironmentApprovalSupported({ + hasAttachedCodeEnvironment: true, + hitlCapable: false, + approvalExplicitlyDisabled: true, + }), + ).not.toThrow(); + expect(() => + assertAttachedCodeEnvironmentApprovalSupported({ + hasAttachedCodeEnvironment: false, + hitlCapable: false, + approvalExplicitlyDisabled: false, + }), + ).not.toThrow(); + }); +}); diff --git a/packages/api/src/agents/hitl/byom.ts b/packages/api/src/agents/hitl/byom.ts new file mode 100644 index 0000000000..42e17fa291 --- /dev/null +++ b/packages/api/src/agents/hitl/byom.ts @@ -0,0 +1,98 @@ +import { Constants } from '@librechat/agents'; +import type { HookCallback } from '@librechat/agents'; +import { CREATE_FILE_TOOL_NAME, EDIT_FILE_TOOL_NAME } from '~/agents/tools'; + +const BYOM_APPROVAL_TOOLS = new Set([ + Constants.BASH_TOOL, + Constants.EXECUTE_CODE, + Constants.PROGRAMMATIC_TOOL_CALLING, + Constants.BASH_PROGRAMMATIC_TOOL_CALLING, + Constants.WRITE_FILE, + Constants.EDIT_FILE, + Constants.COMPILE_CHECK, + CREATE_FILE_TOOL_NAME, + EDIT_FILE_TOOL_NAME, +]); + +type CodeEnvironmentPolicyAgent = { + id?: string; + codeExecutionContext?: { environmentType?: string }; + subagentAgentConfigs?: readonly (CodeEnvironmentPolicyAgent | null | undefined)[]; + lazySubagentConfigs?: readonly (CodeEnvironmentPolicyAgent | null | undefined)[]; + subagentGraphMemberMetadata?: readonly (CodeEnvironmentPolicyAgent | null | undefined)[]; + subagentGraphConfigs?: readonly { + memberConfigs?: readonly (CodeEnvironmentPolicyAgent | null | undefined)[]; + }[]; +}; + +export class AttachedCodeEnvironmentApprovalError extends Error { + readonly code = 'BYOM_TOOL_APPROVAL_UNSUPPORTED'; + + constructor() { + super('Attached code environments require a tool-approval capable client'); + this.name = 'AttachedCodeEnvironmentApprovalError'; + } +} + +/** Prevent an approval-gated BYOM tool from running on an ingress that cannot resume it. */ +export function assertAttachedCodeEnvironmentApprovalSupported({ + hasAttachedCodeEnvironment, + hitlCapable, + approvalExplicitlyDisabled, +}: { + hasAttachedCodeEnvironment: boolean; + hitlCapable: boolean; + approvalExplicitlyDisabled: boolean; +}): void { + if (hasAttachedCodeEnvironment && !hitlCapable && !approvalExplicitlyDisabled) { + throw new AttachedCodeEnvironmentApprovalError(); + } +} + +/** Collect the SDK agent identities whose execution route targets an attached VM. */ +export function collectAttachedCodeEnvironmentAgentIds( + roots: readonly (CodeEnvironmentPolicyAgent | null | undefined)[], +): Set { + const attachedAgentIds = new Set(); + const visited = new Set(); + const pending = [...roots]; + for (let index = 0; index < pending.length; index++) { + const agent = pending[index]; + if (agent == null || visited.has(agent)) { + continue; + } + visited.add(agent); + if (agent.id && agent.codeExecutionContext?.environmentType === 'attached') { + attachedAgentIds.add(agent.id); + } + pending.push(...(agent.subagentAgentConfigs ?? [])); + pending.push(...(agent.lazySubagentConfigs ?? [])); + pending.push(...(agent.subagentGraphMemberMetadata ?? [])); + for (const graph of agent.subagentGraphConfigs ?? []) { + pending.push(...(graph.memberConfigs ?? [])); + } + } + return attachedAgentIds; +} + +/** + * Safe default for user-operated code environments. Read-only file and search + * operations fall through to the run-wide policy; actions that can execute code + * or modify the workspace require approval for the agent that owns the BYOM route. + */ +export function createAttachedCodeEnvironmentPolicyHook( + attachedAgentIds: ReadonlySet, +): HookCallback<'PreToolUse'> { + return async (input) => { + if ( + !BYOM_APPROVAL_TOOLS.has(input.toolName) || + (input.executingAgentId != null && !attachedAgentIds.has(input.executingAgentId)) + ) { + return {}; + } + return { + decision: 'ask', + reason: `${input.toolName} can modify your attached code environment`, + }; + }; +} diff --git a/packages/api/src/agents/hitl/index.ts b/packages/api/src/agents/hitl/index.ts index 3688fa1ba1..b5e27c03b1 100644 --- a/packages/api/src/agents/hitl/index.ts +++ b/packages/api/src/agents/hitl/index.ts @@ -8,3 +8,4 @@ export * from './protection'; export * from './hooks'; export * from './hookLoader'; export * from './askUserQuestionTool'; +export * from './byom'; diff --git a/packages/api/src/agents/hitl/policy.spec.ts b/packages/api/src/agents/hitl/policy.spec.ts index 331e9464b8..436e44a0cd 100644 --- a/packages/api/src/agents/hitl/policy.spec.ts +++ b/packages/api/src/agents/hitl/policy.spec.ts @@ -72,9 +72,9 @@ describe('isToolDeniedByApprovalPolicy', () => { }); describe('resolveToolApprovalPolicy', () => { - test('returns the endpoint policy unchanged (single layer wired today)', () => { + test('returns the endpoint policy unchanged when BYOM is not active', () => { const endpoint: TToolApprovalPolicy = { enabled: true, mode: 'default', deny: ['rm'] }; - // Identity, not a copy — the resolver is a passthrough until more layers ship. + // Identity, not a copy — non-BYOM behavior remains unchanged. expect(resolveToolApprovalPolicy({ endpoint })).toBe(endpoint); }); @@ -83,6 +83,31 @@ describe('resolveToolApprovalPolicy', () => { expect(resolveToolApprovalPolicy({ endpoint: undefined })).toBeUndefined(); }); + test('enables the safe BYOM baseline without affecting unrelated tools', () => { + expect(resolveToolApprovalPolicy({ attachedCodeEnvironment: true })).toEqual({ + enabled: true, + mode: 'bypass', + }); + }); + + test('keeps the BYOM bypass baseline when the endpoint normally uses default mode', () => { + expect( + resolveToolApprovalPolicy({ + endpoint: { enabled: true, mode: 'default', deny: ['dangerous_tool'] }, + attachedCodeEnvironment: true, + }), + ).toEqual({ + enabled: true, + mode: 'bypass', + deny: ['dangerous_tool'], + }); + }); + + test('preserves the administrator emergency override for BYOM', () => { + const endpoint: TToolApprovalPolicy = { enabled: false }; + expect(resolveToolApprovalPolicy({ endpoint, attachedCodeEnvironment: true })).toBe(endpoint); + }); + test('ignores the reserved agent/skills layers for now (behaviour-preserving)', () => { const endpoint: TToolApprovalPolicy = { enabled: true, mode: 'bypass' }; const resolved = resolveToolApprovalPolicy({ diff --git a/packages/api/src/agents/hitl/policy.ts b/packages/api/src/agents/hitl/policy.ts index 7112cbd443..ef400e559e 100644 --- a/packages/api/src/agents/hitl/policy.ts +++ b/packages/api/src/agents/hitl/policy.ts @@ -16,10 +16,9 @@ const DEFAULT_REVIEW_DECISIONS: Agents.ToolApprovalDecisionType[] = ['approve', /** * Layered sources that combine into the effective tool-approval policy for a turn. * - * Only {@link ToolApprovalPolicyLayers.endpoint} is consumed today; `agent` and - * `skills` are reserved seams so future per-agent / per-skill plumbing lands in - * {@link resolveToolApprovalPolicy} rather than being threaded through the run - * call site. + * Endpoint policy remains the administrative baseline. Attached code environments + * also activate LibreChat's built-in BYOM baseline; `agent` and `skills` remain + * reserved seams for future persisted overrides. */ export interface ToolApprovalPolicyLayers { /** @@ -39,6 +38,12 @@ export interface ToolApprovalPolicyLayers { * skill can never silently auto-approve a tool. */ skills?: TToolApprovalPolicy[]; + /** + * At least one agent in this run executes in an attached, user-operated environment. + * Attached environments get LibreChat's safe approval baseline without requiring + * an administrator to opt the whole endpoint into prompts. + */ + attachedCodeEnvironment?: boolean; } /** @@ -51,13 +56,21 @@ export interface ToolApprovalPolicyLayers { * - `agent` overrides `mode`/`allow`/`deny`/`ask`/`reason`; * - `skills` may only tighten (add `ask`/`deny`), never loosen. * - * Today only `endpoint` is consumed, so the result is identical to reading - * `endpoints.agents.toolApproval` directly — `agent`/`skills` are accepted but - * not yet merged. Behaviour-preserving until those layers ship. + * The BYOM activation adds only `enabled: true, mode: 'bypass'`; an agent-scoped + * hook supplies the risky coding decisions. This avoids prompting managed sibling + * agents in the same graph. An explicit endpoint `enabled: false` remains the + * administrator emergency override. `agent`/`skills` are accepted but not yet merged. */ export function resolveToolApprovalPolicy( layers: ToolApprovalPolicyLayers, ): TToolApprovalPolicy | undefined { + if (layers.attachedCodeEnvironment === true && layers.endpoint?.enabled !== false) { + return { + ...layers.endpoint, + enabled: true, + mode: 'bypass', + }; + } return layers.endpoint; } diff --git a/packages/api/src/agents/run.ts b/packages/api/src/agents/run.ts index a76221b03f..ea1ef80064 100644 --- a/packages/api/src/agents/run.ts +++ b/packages/api/src/agents/run.ts @@ -54,6 +54,11 @@ import type { CodeExecutionContext } from '~/agents/execution'; import type { MCPToolAlias } from '~/tools/classification'; import type { SubagentUsageEvent } from '~/agents/usage'; import type * as t from '~/types'; +import { + assertAttachedCodeEnvironmentApprovalSupported, + collectAttachedCodeEnvironmentAgentIds, + createAttachedCodeEnvironmentPolicyHook, +} from '~/agents/hitl/byom'; import { CHECK_BACKGROUND_TASK_NAME, registerBackgroundTaskTool, @@ -91,6 +96,7 @@ import { getLLMConfig as getAnthropicLLMConfig } from '~/endpoints/anthropic/llm import { CREATE_FILE_TOOL_NAME, EDIT_FILE_TOOL_NAME } from '~/agents/tools'; import { buildAgentInitialToolSessions } from '~/agents/codeFilesSession'; import { getProviderConfig } from '~/endpoints/config/providers'; +import { buildToolApprovalHooks } from '~/agents/hitl/hooks'; import { resolveHeaders, createSafeUser } from '~/utils/env'; import { getAgentCheckpointer } from '~/agents/checkpointer'; import { getPluginHookSource } from '~/agents/hooks/source'; @@ -1776,6 +1782,12 @@ export async function createRun({ }; const agentsEndpointConfig = appConfig?.endpoints?.[EModelEndpoint.agents]; + const attachedCodeEnvironmentAgentIds = collectAttachedCodeEnvironmentAgentIds(agents); + assertAttachedCodeEnvironmentApprovalSupported({ + hasAttachedCodeEnvironment: attachedCodeEnvironmentAgentIds.size > 0, + hitlCapable, + approvalExplicitlyDisabled: agentsEndpointConfig?.toolApproval?.enabled === false, + }); // Assigned after the run-wide HITL registry is built. Lazy descriptors // capture this indirection now and report their aliases when they resolve. @@ -1874,12 +1886,11 @@ export async function createRun({ * and the resume route). When disabled, nothing attaches and the run is identical * to before this feature shipped. */ - // Resolve the effective policy through the single seam so per-agent / per-skill - // sources can layer in later without touching this call site (see - // `resolveToolApprovalPolicy`). Only the endpoint layer is wired today, so this - // is identical to reading `toolApproval` directly. + // Resolve the effective policy through the single seam so BYOM defaults and + // future persisted per-agent / per-skill sources do not leak into this call site. const toolApprovalPolicy = resolveToolApprovalPolicy({ endpoint: agentsEndpointConfig?.toolApproval, + attachedCodeEnvironment: attachedCodeEnvironmentAgentIds.size > 0, }); // Gate HITL to callers that actually implement the pause/resume lifecycle. The // OpenAI-compatible + Responses controllers also call createRun/processStream but never @@ -1916,10 +1927,28 @@ export async function createRun({ appConfig, }, mcpToolAliases, - resolvedToolApprovalHooks, + [ + ...(resolvedToolApprovalHooks ?? + buildToolApprovalHooks({ + userId: user?.id, + conversationId: requestBody?.conversationId, + tenantId: tenantId ?? user?.tenantId, + appConfig, + })), + ...(attachedCodeEnvironmentAgentIds.size > 0 + ? [ + { + hook: createAttachedCodeEnvironmentPolicyHook(attachedCodeEnvironmentAgentIds), + }, + ] + : []), + ], ) : undefined; registerResolvedMCPToolAliases = (resolvedAgent) => { + if (resolvedAgent.codeExecutionContext?.environmentType === 'attached') { + attachedCodeEnvironmentAgentIds.add(resolvedAgent.id); + } const discoveredAliases = collectRunMCPToolAliases([resolvedAgent]).filter( ({ name, aliasName }) => { const key = `${name}\u0000${aliasName}`;