diff --git a/apps/sim/app/api/copilot/tools/execute/route.ts b/apps/sim/app/api/copilot/tools/execute/route.ts index 5575a25a746..703708a1a09 100644 --- a/apps/sim/app/api/copilot/tools/execute/route.ts +++ b/apps/sim/app/api/copilot/tools/execute/route.ts @@ -134,7 +134,7 @@ export const POST = withRouteHandler((request: NextRequest) => * where the gate lives. Inert while copilot tool permissions are disabled, which keeps * enabling the flag from silently leaving background lanes ungated. */ - if (toolRequiresApprovalLane(toolName)) { + if (toolRequiresApprovalLane(toolName, params)) { logger.warn('Refusing an approval-gated tool on the in-band lane', { toolName, toolCallId, diff --git a/apps/sim/lib/mothership/agent-cli/approval.ts b/apps/sim/lib/mothership/agent-cli/approval.ts new file mode 100644 index 00000000000..53885df7bed --- /dev/null +++ b/apps/sim/lib/mothership/agent-cli/approval.ts @@ -0,0 +1,71 @@ +import { resolveEmbeddedCliCommand, type V2OperationName } from 'sim/embed' +import { AgentCliRequest } from '@/lib/mothership/generated/agent-cli' +import { + CallIntegrationTool, + CancelWorkflowRun, + DeleteWorkspaceMcpServer, + DeployAsApi, + DeployAsChat, + DeployAsMcp, + PromoteToLive, + RunWorkflow, + TOOL_CATALOG, + type ToolCatalogEntry, +} from '@/lib/mothership/generated/tool-catalog-v1' + +/** + * The catalog tool whose capability each v2 operation is. The worker retired these + * tools in favour of `sim_cli`, so a command that calls one of these operations answers + * to that tool's approval flag: one policy, whichever name the capability arrives under. + * The deploy tools gate undeploy as well, so both directions map to them; rollback runs + * the same use case as activating a version. + */ +const CATALOG_TOOL_BY_OPERATION: Partial> = { + activateWorkflowVersion: PromoteToLive, + rollbackWorkflow: PromoteToLive, + deployWorkflow: DeployAsApi, + undeployWorkflow: DeployAsApi, + replaceWorkflowChatDeployment: DeployAsChat, + deleteWorkflowChatDeployment: DeployAsChat, + deployWorkflowMcpTool: DeployAsMcp, + undeployWorkflowMcpTool: DeployAsMcp, + deleteWorkflowMcpServer: DeleteWorkspaceMcpServer, + executeWorkflow: RunWorkflow, + resumeWorkflow: RunWorkflow, + cancelWorkflowRun: CancelWorkflowRun, + executeTool: CallIntegrationTool, +} + +/** + * Whether a `sim_cli` call must be held for the user's decision. Its argv is resolved by + * the CLI's own parser, so the answer is about the command that would run, and a `--yes` + * in that argv answers only the CLI's prompt, never this one. Argv the parser cannot + * follow is held; a malformed request is not, because `executeSimCli` refuses it. + */ +export function cliRequestRequiresApproval(request: unknown): boolean { + const parsed = AgentCliRequest.safeParse(request) + if (!parsed.success) return false + const { invocation } = parsed.data + if (invocation.kind === 'service') return TOOL_CATALOG[invocation.name]?.requiresApproval === true + if (invocation.kind !== 'cli') return false + const command = resolveEmbeddedCliCommand(invocation.argv) + if (command === undefined) return true + return ( + command?.operations.some( + (operation) => CATALOG_TOOL_BY_OPERATION[operation]?.requiresApproval === true + ) === true + ) +} + +/** + * The tool name (`cli_workflows_deploy`) of the command a `sim_cli` call would run, as the + * CLI's parser resolves it. The row's own name is the worker's reading of the same argv; + * an always-allow saved under that name covers the call only when the two agree, so a + * row named for one command, or for no command at all, never carries another's consent. + */ +export function cliCommandToolName(request: unknown): string | undefined { + const parsed = AgentCliRequest.safeParse(request) + if (!parsed.success || parsed.data.invocation.kind !== 'cli') return undefined + const command = resolveEmbeddedCliCommand(parsed.data.invocation.argv) + return command ? `cli_${command.path.join('_').replace(/-/g, '_')}` : undefined +} diff --git a/apps/sim/lib/mothership/request/handlers/handlers.test.ts b/apps/sim/lib/mothership/request/handlers/handlers.test.ts index 3d1ab77e7d2..1aadd9f8aa1 100644 --- a/apps/sim/lib/mothership/request/handlers/handlers.test.ts +++ b/apps/sim/lib/mothership/request/handlers/handlers.test.ts @@ -9,7 +9,7 @@ import { mothershipClientToolWaiterMockFns, } from '@sim/testing/mocks/mothership-client-tool-waiter.mock' import { sleep } from '@sim/utils/helpers' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AsyncToolCallOwnershipError } from '@/lib/mothership/async-runs/errors' import { restoreStreamingContext } from '@/lib/mothership/request/context/restore' import { TraceCollector } from '@/lib/mothership/request/trace' @@ -62,6 +62,7 @@ import { MothershipStreamV1ResourceOp, MothershipStreamV1RunKind, MothershipStreamV1TextChannel, + type MothershipStreamV1ToolCallDescriptor, MothershipStreamV1ToolExecutor, MothershipStreamV1ToolMode, MothershipStreamV1ToolOutcome, @@ -431,6 +432,83 @@ describe('sse-handlers tool lifecycle', () => { expect(event.payload.status).toBe('awaiting_approval') }) + describe('sim_cli approval', () => { + const activate = ['workflows', 'activate', 'create', 'wf-1', '1', '--yes'] + + function cliFrame( + toolCallId: string, + toolName: string, + argv: string[] + ): StreamEvent & { payload: MothershipStreamV1ToolCallDescriptor } { + return { + type: MothershipStreamV1EventType.tool, + payload: { + toolCallId, + toolName, + execName: 'sim_cli', + arguments: { args: argv, request: { invocation: { kind: 'cli', argv } } }, + executor: MothershipStreamV1ToolExecutor.sim, + mode: MothershipStreamV1ToolMode.async, + phase: MothershipStreamV1ToolPhase.call, + }, + } + } + + beforeEach(async () => { + const router = await vi.importActual( + '@/lib/mothership/tool-executor/router' + ) + toolRequiresApproval.mockImplementation(router.toolRequiresApproval) + context.runId = 'run-1' + context.toolPermissions = { + enabled: true, + autoAllowed: new Set(), + autoAllowPermitted: true, + } + }) + + afterEach(() => { + toolRequiresApproval.mockReset() + toolRequiresApproval.mockReturnValue(false) + }) + + it('holds a command that performs a gated capability, though its argv says --yes', async () => { + const event = cliFrame('cli-activate', 'cli_workflows_activate_create', activate) + + await prePersistClientExecutableToolCall(event, context, {}) + + expect(event.payload.status).toBe('awaiting_approval') + expect(upsertAsyncToolCall).toHaveBeenCalledWith( + expect.objectContaining({ + toolName: 'cli_workflows_activate_create', + executableName: 'sim_cli', + permissionRequested: true, + }) + ) + }) + + it('lets always-allow cover only the command it was granted for', async () => { + context.toolPermissions.autoAllowed.add('cli_workflows_list') + context.toolPermissions.autoAllowed.add('sim_cli') + const held = cliFrame('cli-held', 'cli_workflows_activate_create', activate) + await prePersistClientExecutableToolCall(held, context, {}) + expect(held.payload.status).toBe('awaiting_approval') + + // A row named for an allowed command, or for none, does not carry its consent + // to the command the CLI would actually run. + for (const name of ['cli_workflows_list', 'sim_cli']) { + const mislabeled = cliFrame(`cli-${name}`, name, activate) + await prePersistClientExecutableToolCall(mislabeled, context, {}) + expect(mislabeled.payload.status).toBe('awaiting_approval') + } + + context.toolPermissions.autoAllowed.add('cli_workflows_activate_create') + const allowed = cliFrame('cli-allowed', 'cli_workflows_activate_create', activate) + await prePersistClientExecutableToolCall(allowed, context, {}) + expect(allowed.payload.status).toBeUndefined() + }) + }) + it('clears a Go-stamped approval frame when the gate is off', async () => { // Go stamps integration calls regardless of Sim's feature flag. Forwarding // that stamp with nothing gating behind it would draw a card whose buttons diff --git a/apps/sim/lib/mothership/request/handlers/tool.ts b/apps/sim/lib/mothership/request/handlers/tool.ts index 3ec10080dae..33183671aef 100644 --- a/apps/sim/lib/mothership/request/handlers/tool.ts +++ b/apps/sim/lib/mothership/request/handlers/tool.ts @@ -330,7 +330,8 @@ export async function prePersistClientExecutableToolCall( context, options ?? {}, frameRequestsApproval, - data.arguments + data.arguments, + data.execName ) if (gated) { data.status = TOOL_AWAITING_APPROVAL_STATUS @@ -1029,7 +1030,14 @@ async function dispatchToolExecution( isAssistantIntegrationTool(getToolMetadata(toolCall.execName ?? toolName)) const gated = assistantAction ? frameRequestsApproval - : toolCallNeedsApproval(toolName, context, options, frameRequestsApproval, args) + : toolCallNeedsApproval( + toolName, + context, + options, + frameRequestsApproval, + args, + toolCall.execName + ) if (gated || clientExecutable) await adoptRecordedToolDeadline(toolCall) if (gated) { registerPendingToolPromise( diff --git a/apps/sim/lib/mothership/request/tools/permission.ts b/apps/sim/lib/mothership/request/tools/permission.ts index 44e4b231f65..2c179366551 100644 --- a/apps/sim/lib/mothership/request/tools/permission.ts +++ b/apps/sim/lib/mothership/request/tools/permission.ts @@ -1,6 +1,7 @@ import { createLogger } from '@sim/logger' import { TERMINAL_TOOL_NAME, type TerminalOperation } from '@sim/terminal-protocol' import { getErrorMessage } from '@sim/utils/errors' +import { cliCommandToolName } from '@/lib/mothership/agent-cli/approval' import type { AsyncCompletionSignal } from '@/lib/mothership/async-runs/lifecycle' import { PERMISSION_WAIT_TIMEOUT_MS } from '@/lib/mothership/constants' import { @@ -91,13 +92,19 @@ export function toolCallNeedsApproval( */ frameRequestsApproval = false, /** The call's arguments, for a tool whose gate depends on what it is doing. */ - args?: Record + args?: Record, + /** + * The tool that executes the call, when the frame names it apart from its display + * name: a `sim_cli` call is shown as the command it runs (`cli_workflows_deploy`), + * and always-allow is saved under that name. + */ + executableName = toolName ): boolean { if (!context.toolPermissions.enabled) return false if (options.interactive === false) return false if (!frameRequestsApproval) { - if (!toolRequiresApproval(toolName)) return false + if (!toolRequiresApproval(executableName, args)) return false if (toolName === TERMINAL_TOOL_NAME && !terminalOperationNeedsApproval(args)) return false // A go-routed tool executes inside mothership and never reaches Sim's // dispatch, so there is nothing here to hold. Stamping the frame anyway @@ -117,8 +124,9 @@ export function toolCallNeedsApproval( * leave every entry saved before the policy changed still silencing prompts. */ if (!context.toolPermissions.autoAllowPermitted) return true + if (!context.toolPermissions.autoAllowed.has(toolApprovalKey(toolName))) return true - return !context.toolPermissions.autoAllowed.has(toolApprovalKey(toolName)) + return executableName === 'sim_cli' && cliCommandToolName(args?.request) !== toolName } function skipOutput(toolName: string) { diff --git a/apps/sim/lib/mothership/tool-executor/router.test.ts b/apps/sim/lib/mothership/tool-executor/router.test.ts index da36cddacab..6d1afbae683 100644 --- a/apps/sim/lib/mothership/tool-executor/router.test.ts +++ b/apps/sim/lib/mothership/tool-executor/router.test.ts @@ -132,4 +132,15 @@ describe('toolRequiresApprovalLane', () => { setEnvFlags({ isCopilotToolPermissionsEnabled: true }) expect(toolRequiresApprovalLane('not_a_real_tool')).toBe(false) }) + + it.each([ + [['workflows', 'activate', 'create', 'wf-1', '1', '--yes'], true], + [['workflows', 'rollback', 'wf-1', '--to-version', '1', '--yes'], true], + [['workflows', 'list'], false], + ])('answers sim_cli %j by the command it runs: %s', (argv, gated) => { + setEnvFlags({ isCopilotToolPermissionsEnabled: true }) + expect( + toolRequiresApprovalLane('sim_cli', { request: { invocation: { kind: 'cli', argv } } }) + ).toBe(gated) + }) }) diff --git a/apps/sim/lib/mothership/tool-executor/router.ts b/apps/sim/lib/mothership/tool-executor/router.ts index b90e4f3ee75..e62c41028a7 100644 --- a/apps/sim/lib/mothership/tool-executor/router.ts +++ b/apps/sim/lib/mothership/tool-executor/router.ts @@ -1,5 +1,6 @@ import { BROWSER_TOOL_OBSERVES_ONLY, isBrowserToolName } from '@sim/browser-protocol' import { isCopilotToolPermissionsEnabled } from '@/lib/core/config/env-flags' +import { cliRequestRequiresApproval } from '@/lib/mothership/agent-cli/approval' import { TOOL_CATALOG, type ToolCatalogEntry } from '@/lib/mothership/generated/tool-catalog-v1' export function isToolInCatalog(toolId: string): boolean { @@ -30,9 +31,11 @@ export function isKnownTool(toolId: string): boolean { * only observing it. One that acts does so as the user, in a browser signed in * to their accounts, so it is gated like a terminal command. Reading the split * from `BROWSER_TOOL_OBSERVES_ONLY` gates a browser tool added later by default. + * `sim_cli` has no entry: each call answers for the command its arguments would run. */ -export function toolRequiresApproval(toolId: string): boolean { +export function toolRequiresApproval(toolId: string, args?: Record): boolean { if (isBrowserToolName(toolId)) return !BROWSER_TOOL_OBSERVES_ONLY[toolId] + if (toolId === 'sim_cli') return cliRequestRequiresApproval(args?.request) return getToolEntry(toolId)?.requiresApproval === true } @@ -46,8 +49,8 @@ export function toolRequiresApproval(toolId: string): boolean { * must never hang on a prompt with no row behind it. * * Lives here rather than beside the dispatch gate so that asking the question - * costs only the catalog. The gate module reaches the permission persistence - * layer, which opens a pub/sub channel when it loads. + * costs only the catalog and the CLI's parser. The gate module reaches the + * permission persistence layer, which opens a pub/sub channel when it loads. * * Deliberately blind to the stored auto-allow list. Consulting it here would add * a database read to every in-band call to reach the same place by a longer @@ -56,6 +59,6 @@ export function toolRequiresApproval(toolId: string): boolean { * leaves the one implementation of "has the user allowed this" on the lane that * already owns it. */ -export function toolRequiresApprovalLane(toolId: string): boolean { - return isCopilotToolPermissionsEnabled && toolRequiresApproval(toolId) +export function toolRequiresApprovalLane(toolId: string, args?: Record): boolean { + return isCopilotToolPermissionsEnabled && toolRequiresApproval(toolId, args) } diff --git a/packages/sim-cli/src/embed.test.ts b/packages/sim-cli/src/embed.test.ts index 9f6f72bbe80..f47df6ce2d5 100644 --- a/packages/sim-cli/src/embed.test.ts +++ b/packages/sim-cli/src/embed.test.ts @@ -1,6 +1,6 @@ import { sleep } from '@sim/utils/helpers' import { describe, expect, it, vi } from 'vitest' -import { runEmbeddedCli } from './embed' +import { resolveEmbeddedCliCommand, runEmbeddedCli } from './embed' const IDENTITY = { endpoint: 'https://sim.internal.test', @@ -189,3 +189,44 @@ describe('embedded request defaults', () => { ) }) }) + +describe('resolveEmbeddedCliCommand', () => { + it('names the command argv would run and what it may call, however it is spelled', () => { + expect( + resolveEmbeddedCliCommand([ + '-w', + 'ws-1', + 'workflows', + 'activate', + 'create', + 'wf-1', + '1', + '--yes', + ]) + ).toEqual({ + path: ['workflows', 'activate', 'create'], + operations: ['activateWorkflowVersion'], + }) + expect(resolveEmbeddedCliCommand(['workflows', '-P', 'default', 'undeploy', 'wf-1'])).toEqual({ + path: ['workflows', 'undeploy'], + operations: ['undeployWorkflow'], + }) + expect(resolveEmbeddedCliCommand(['kb', 'list'])).toEqual({ + path: ['knowledge', 'list'], + operations: ['listKnowledgeBases'], + }) + }) + + it.each([ + [['--help']], + [['workflows', 'no-such-command']], + [['workflows', 'activate', 'create']], + [['workflows', 'activate', 'create', 'wf-1', '1', '--dry-run']], + [['workflows', 'run', 'wf-1', '--help']], + ])('reports no command for %j, and running it calls nothing', async (argv) => { + expect(resolveEmbeddedCliCommand(argv)).toBeNull() + const transport = vi.fn(async () => jsonResponse({ data: {} })) + await runEmbeddedCli(argv, { ...IDENTITY, transport }) + expect(transport).not.toHaveBeenCalled() + }) +}) diff --git a/packages/sim-cli/src/embed.ts b/packages/sim-cli/src/embed.ts index f3c52b3a0f0..a13f36747eb 100644 --- a/packages/sim-cli/src/embed.ts +++ b/packages/sim-cli/src/embed.ts @@ -1,5 +1,5 @@ import { getErrorMessage } from '@sim/utils/errors' -import { type Command, CommanderError } from 'commander' +import { type Command, CommanderError, type OutputConfiguration } from 'commander' import { ProfileConfigError } from './config/index' import { type EmbedContext, @@ -9,6 +9,7 @@ import { embedStore, } from './embed-context' import { EmbeddedOutput, EmbeddedOutputLimitError } from './embed-output' +import type { V2OperationName } from './generated/v2-api' import { formatApiErrorDetails, isRequestTimeout, @@ -19,6 +20,8 @@ import { import { writeStderr, writeStdout } from './output/io' import { sanitize } from './output/render' import { buildProgram } from './program' +import { calledOperations } from './runtime/called-operations' +import { commandPath } from './runtime/derive' /** * In-process execution of one CLI invocation, for a server that already knows @@ -40,6 +43,7 @@ export type { ListFilesResponse, ListWorkflowsResponse, ReadFileTextResponse, + V2OperationName, } from './generated/v2-api' export { SimApiError, SimClient } from './http/client' @@ -133,12 +137,7 @@ export async function runEmbeddedCli( let exitCode = 0 try { identity.signal?.throwIfAborted() - const program = buildProgram({ - hostCommands: false, - helpText: - 'The embedding application supplies authentication, endpoint and workspace scope.', - }) - configureEmbeddedOutput(program) + const program = buildEmbeddedProgram({ writeOut: writeStdout, writeErr: writeStderr }) await program.parseAsync(argv, { from: 'user' }) // Commands that soft-fail (a failed run outcome, wait timeout) report through the // context via setSoftExitCode — never process.exitCode, which is shared and raced @@ -203,9 +202,59 @@ function renderEmbeddedError(ctx: EmbedContext, error: unknown): number { return 1 } +function buildEmbeddedProgram(output: OutputConfiguration): Command { + const program = buildProgram({ + hostCommands: false, + helpText: 'The embedding application supplies authentication, endpoint and workspace scope.', + }) + configureEmbeddedOutput(program, output) + return program +} + /** Independently constructed subcommands do not inherit Commander output or exit overrides. */ -function configureEmbeddedOutput(command: Command): void { - command.configureOutput({ writeOut: writeStdout, writeErr: writeStderr }) +function configureEmbeddedOutput(command: Command, output: OutputConfiguration): void { + command.configureOutput(output) command.exitOverride() - for (const child of command.commands) configureEmbeddedOutput(child) + for (const child of command.commands) configureEmbeddedOutput(child, output) +} + +/** Carries the command a parse dispatched to out of the parse, before its action runs. */ +class DispatchedCommand { + constructor(readonly command: Command) {} +} + +const DISCARD_OUTPUT: OutputConfiguration = { writeOut: () => {}, writeErr: () => {} } + +interface EmbeddedCliCommand { + /** The command's canonical path (`['workflows', 'deploy']`), aliases resolved. */ + path: string[] + /** Every v2 operation it may call: a command reaches only those it declares (`connect`). */ + operations: readonly V2OperationName[] +} + +/** + * The command `argv` runs under {@link runEmbeddedCli}, resolved by the same command + * tree and parser and stopped before any action runs. + * + * Null when the parse ends without dispatching (help, a usage error), which calls + * nothing. Undefined when the parse could not be followed to an answer, so a caller + * deciding whether to stop for consent can fail closed. + */ +export function resolveEmbeddedCliCommand(argv: string[]): EmbeddedCliCommand | null | undefined { + try { + const program = buildEmbeddedProgram(DISCARD_OUTPUT) + program.hook('preAction', (_program, action) => { + throw new DispatchedCommand(action) + }) + program.parse(argv, { from: 'user' }) + } catch (error) { + if (error instanceof DispatchedCommand) { + return { + path: commandPath(error.command), + operations: calledOperations(error.command) ?? [], + } + } + if (error instanceof CommanderError) return null + } + return undefined }