diff --git a/apps/sim/app/api/v2/workflows/[workflowId]/versions/[version]/route.test.ts b/apps/sim/app/api/v2/workflows/[workflowId]/versions/[version]/route.test.ts index 9c86e7c6b61..863e2f4f495 100644 --- a/apps/sim/app/api/v2/workflows/[workflowId]/versions/[version]/route.test.ts +++ b/apps/sim/app/api/v2/workflows/[workflowId]/versions/[version]/route.test.ts @@ -161,6 +161,22 @@ describe('GET /api/v2/workflows/[workflowId]/versions/[version]', () => { ) }) + it.each(['latest', 'v2'])( + 'names the version path segment when %s is not a version number', + async (version) => { + const request = createMockRequest({ + url: `http://localhost/api/v2/workflows/workflow-1/versions/${version}`, + }) + const response = await GET(request, createRouteContext({ workflowId: 'workflow-1', version })) + + expect(response.status).toBe(400) + expect((await response.json()).error.message).toBe('version must be a positive integer') + expect( + workflowsPersistenceUtilsMockFns.mockGetWorkflowDeploymentVersion + ).not.toHaveBeenCalled() + } + ) + it('never serves credential values in the pinned graph', async () => { const response = await get() diff --git a/apps/sim/lib/api/contracts/primitives.ts b/apps/sim/lib/api/contracts/primitives.ts index 039699e799b..1931a0c6c38 100644 --- a/apps/sim/lib/api/contracts/primitives.ts +++ b/apps/sim/lib/api/contracts/primitives.ts @@ -363,11 +363,14 @@ export const workspaceFileIdSchema = requiredFieldSchema('File ID is required') */ export const INT4_MAX = 2147483647 +/** Also replaces zod's `expected number, received NaN` for a non-numeric path segment. */ +const VERSION_NUMBER_MESSAGE = 'version must be a positive integer' + /** A version number in a body or cursor, bounded to the range its column can hold. */ export const versionNumberSchema = z .number() .int('version must be an integer') - .min(1, 'version must be a positive integer') + .min(1, VERSION_NUMBER_MESSAGE) .max(INT4_MAX, 'version is out of range') /** @@ -377,9 +380,9 @@ export const versionNumberSchema = z * holds. */ export const versionNumberPathSchema = z.coerce - .number() - .int() - .positive() + .number({ error: VERSION_NUMBER_MESSAGE }) + .int(VERSION_NUMBER_MESSAGE) + .positive(VERSION_NUMBER_MESSAGE) .max(INT4_MAX, 'version is out of range') /** diff --git a/apps/sim/lib/internal/function/execute-direct-call.test.ts b/apps/sim/lib/internal/function/execute-direct-call.test.ts new file mode 100644 index 00000000000..a584555f182 --- /dev/null +++ b/apps/sim/lib/internal/function/execute-direct-call.test.ts @@ -0,0 +1,69 @@ +import { createPersonalApiKeyPrincipal } from '@sim/testing/factories/principal.factory' +import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock' +import { + workspaceContextMock, + workspaceContextMockFns, +} from '@sim/testing/mocks/workspace-context.mock' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ runSandbox: vi.fn() })) + +vi.mock('@/lib/function-execution/execute-request', () => ({ + executeFunctionRequest: mocks.runSandbox, +})) +vi.mock('@/lib/workspaces/application/workspace-context', () => workspaceContextMock) +vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock) + +import type { Principal } from '@sim/auth/principal' +import { markCopilotWorkspaceInvocation } from '@/lib/core/application/copilot-workspace-invocation' +import { PrincipalKindAuthorizationError } from '@/lib/core/application/workspace-authorization' +import { executeFunctionTool } from '@/lib/internal/function/execute' +import { createCopilotChatPrincipal } from '@/lib/mothership/auth/application-delegation' +import { TOOL_EXECUTION_DELEGATION_AUDIENCE } from '@/lib/tool-execution/application/operations' + +/** + * `POST /api/v2/tools/{id}/execute` (the embedded CLI's `tools execute`) runs `function_execute` + * with no workflow behind it, so no executor origin exists. The real function-execution policy + * decides from the authenticated caller; only the sandbox is stubbed. + */ +function runDirect(callerPrincipal: Principal) { + return executeFunctionTool({ + body: { code: 'return 1' }, + headers: new Headers(), + requestId: 'request-1', + context: { workflowId: '', workspaceId: 'workspace-1', userId: 'user-1', callerPrincipal }, + }) +} + +describe('executeFunctionTool direct tool calls', () => { + beforeEach(() => { + workspaceContextMockFns.mockResolveActiveWorkspaceApplicationContext.mockResolvedValue({ + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + billedAccountUserId: 'user-1', + allowPersonalApiKeys: true, + }) + workspaceAuthzMockFns.mockResolveEffectiveWorkspacePermission.mockResolvedValue('write') + mocks.runSandbox.mockResolvedValue(Response.json({ success: true, output: { result: 1 } })) + }) + + it('runs the code for an admitted Mothership caller', async () => { + const caller = createCopilotChatPrincipal( + { userId: 'user-1', workspaceId: 'workspace-1', chatId: 'chat-1' }, + TOOL_EXECUTION_DELEGATION_AUDIENCE + ) + markCopilotWorkspaceInvocation(caller) + + const response = await runDirect(caller) + + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ success: true, output: { result: 1 } }) + }) + + it('still refuses a personal API key before any code runs', async () => { + await expect(runDirect(createPersonalApiKeyPrincipal({ keyId: 'key-1' }))).rejects.toThrow( + PrincipalKindAuthorizationError + ) + expect(mocks.runSandbox).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/internal/function/execute.ts b/apps/sim/lib/internal/function/execute.ts index 5383a810faa..6a92e885d6e 100644 --- a/apps/sim/lib/internal/function/execute.ts +++ b/apps/sim/lib/internal/function/execute.ts @@ -1,6 +1,7 @@ -import type { DelegatedPrincipal } from '@sim/auth/principal' +import type { Principal } from '@sim/auth/principal' import type { FunctionExecuteBody } from '@/lib/api/contracts' import type { InternalSandboxProfile } from '@/lib/auth/internal' +import { bindCopilotWorkspaceOperation } from '@/lib/core/application/copilot-workspace-invocation' import { DEFAULT_EXECUTION_TIMEOUT_MS } from '@/lib/core/execution-limits' import { serializeExecutionDeadlineHeader } from '@/lib/execution/execution-deadline-header' import { FUNCTION_EXECUTION_DELEGATION_AUDIENCE } from '@/lib/function-execution/application/authorization' @@ -9,6 +10,7 @@ import { executeFunction } from '@/lib/function-execution/application/execute-fu import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' import type { InternalToolOperationContext } from '@/lib/internal/tool-operations/types' import { createTrustedOrganizationCopilotPrincipal } from '@/lib/mothership/auth/application-delegation' +import { TOOL_EXECUTION_DELEGATION_AUDIENCE } from '@/lib/tool-execution/application/operations' export type TrustedFunctionToolExecutionContext = InternalToolOperationContext @@ -78,8 +80,17 @@ export async function executeFunctionTool(input: ExecuteFunctionToolInput): Prom }, }) } - let principal: DelegatedPrincipal - if (context.copilotToolExecution === true) { + let principal: Principal + if (context.callerPrincipal && !context.executorDelegationOrigin) { + // A direct tool call has no workflow run to bind, so the authenticated caller is the + // authority; the function-execution policy admits or refuses whoever that is. + principal = bindCopilotWorkspaceOperation( + context.callerPrincipal, + context.workspaceId, + [TOOL_EXECUTION_DELEGATION_AUDIENCE], + executeFunction + ) + } else if (context.copilotToolExecution === true) { if (!context.userId) throw new Error('Copilot Function execution requires a user') principal = { kind: 'delegated', diff --git a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts index d29e49c6878..f8c4208cb1f 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts @@ -348,6 +348,17 @@ describe('universal grep', () => { } ) + it.each(['knowledge', 'blocks,KB'])( + 'redirects --scope %s to semantic knowledge search before materializing any world', + async (scope) => { + /** An empty runtime throws on any request, so a clean refusal proves nothing was fetched. */ + const result = await runEngine('grep', ['invoice'], runtimeWith({}), { scope }) + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain('Unknown scope') + expect(result.stderr).toContain('use knowledge search --kb ') + } + ) + it('refuses an --in selector no resource in the searched worlds answers to', async () => { const bare = await runEngine('grep', ['id'], runtimeWith(CATALOG), { scope: 'blocks', diff --git a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts index d4496a4f677..b77400c6cee 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts @@ -445,14 +445,17 @@ function unknownWithin(selector: string): string { } /** - * Heads a model reaches for when it wants to grep a knowledge base. Knowledge is not a - * grep world — chunks are retrieved semantically — so the refusal has to redirect rather - * than just list the worlds, or the next attempt is the same selector spelled differently. + * Names a model reaches for when it wants to grep a knowledge base, as a `--scope` or an + * `--in` head. Knowledge is not a grep world — chunks are retrieved semantically — so the + * refusal has to redirect rather than just list the worlds, or the next attempt is the + * same selector spelled differently. */ const KNOWLEDGE_SELECTOR_HEADS = new Set(['knowledge', 'kb']) +const KNOWLEDGE_REDIRECT = `Knowledge bases are searched semantically — use knowledge search --kb --query "…"; grep covers ${SCOPES.join(', ')}.` + function knowledgeWithin(selector: string): string { - return `${unknownWithin(selector)} Knowledge bases are searched semantically — use knowledge search --kb --query "…"; grep covers ${SCOPES.join(', ')}.` + return `${unknownWithin(selector)} ${KNOWLEDGE_REDIRECT}` } function parseScopes(flags: AgentCliFlags): Scope[] | string { @@ -463,6 +466,9 @@ function parseScopes(flags: AgentCliFlags): Scope[] | string { .split(',') .map((s) => s.trim()) .filter(Boolean)) { + if (KNOWLEDGE_SELECTOR_HEADS.has(part.toLowerCase())) { + return `Unknown scope "${part}". ${KNOWLEDGE_REDIRECT}` + } if (!(SCOPES as readonly string[]).includes(part)) { return `Unknown scope "${part}".${didYouMean(part)} Scopes: ${SCOPES.join(', ')}.` } diff --git a/apps/sim/lib/tool-execution/application/execute-tool.ts b/apps/sim/lib/tool-execution/application/execute-tool.ts index 9612a06e772..51c04479ea6 100644 --- a/apps/sim/lib/tool-execution/application/execute-tool.ts +++ b/apps/sim/lib/tool-execution/application/execute-tool.ts @@ -19,7 +19,10 @@ import { isHosted } from '@/lib/core/config/env-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' import { getEffectiveDecryptedEnv } from '@/lib/environment/utils' import { principalUserId } from '@/lib/integrations/principal-scope.server' -import { toolExecutionOperations } from '@/lib/tool-execution/application/operations' +import { + TOOL_EXECUTION_DELEGATION_AUDIENCE, + toolExecutionOperations, +} from '@/lib/tool-execution/application/operations' import { isEnvVarReference } from '@/executor/constants' import { resolveEnvVarReferences } from '@/executor/utils/reference-validation' import { executeTool as executeRegistryTool } from '@/tools' @@ -286,7 +289,7 @@ export const executeToolForCaller = defineAuthorizedWorkspaceUseCase({ resolveContext: ({ input }: { input: ExecuteToolInput }) => loadCatalogWorkspaceContext(input.workspaceId), authorizationOptions: { - delegation: { audience: 'sim:tool-execution', isWithinScope: () => true }, + delegation: { audience: TOOL_EXECUTION_DELEGATION_AUDIENCE, isWithinScope: () => true }, }, execute: async ({ principal, input, context }): Promise => { const gate = await resolveCatalogGate(principal, context) diff --git a/apps/sim/lib/tool-execution/application/operations.ts b/apps/sim/lib/tool-execution/application/operations.ts index 3dbe325c164..57deb1e284d 100644 --- a/apps/sim/lib/tool-execution/application/operations.ts +++ b/apps/sim/lib/tool-execution/application/operations.ts @@ -1,5 +1,8 @@ import { defineWorkspaceOperation } from '@/lib/core/application/workspace-operation' +/** Audience of the Copilot delegation a direct tool call is admitted under. */ +export const TOOL_EXECUTION_DELEGATION_AUDIENCE = 'sim:tool-execution' + /** * Semantic operation for running one code-defined tool directly. *