Skip to content

Commit 9e0ee45

Browse files
committed
fix(mothership): mount code secrets explicitly
1 parent b11fc5c commit 9e0ee45

5 files changed

Lines changed: 120 additions & 12 deletions

File tree

apps/sim/lib/function-execution/execute-request.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,43 @@ describe('Function execution request', () => {
567567
)
568568
})
569569

570+
it.each([
571+
{
572+
language: 'javascript',
573+
code: 'return { template: "{{API_KEY}}", name: "environmentVariables" }',
574+
},
575+
{ language: 'python', code: '__sim_result__ = {"template": "{{API_KEY}}"}' },
576+
{ language: 'shell', code: "printf '%s' '{{API_KEY}}'" },
577+
])(
578+
'preserves literal $language templates in trusted Mothership code with an explicitly mounted secret',
579+
async ({ language, code }) => {
580+
envFlagsMock.isMothershipSandboxEnabled = true
581+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValueOnce({
582+
success: true,
583+
userId: 'user-123',
584+
authType: 'internal_jwt',
585+
sandboxProfile: 'mothership',
586+
})
587+
const secret = 'private-test-value-938'
588+
const response = await POST(
589+
createMockRequest('POST', {
590+
code,
591+
language,
592+
envVars: { API_KEY: secret },
593+
secretScope: 'selected',
594+
mountedSecrets: ['API_KEY'],
595+
})
596+
)
597+
expect(response.status).toBe(200)
598+
const request =
599+
language === 'shell'
600+
? mockExecuteShellInSandbox.mock.calls.at(-1)?.[0]
601+
: mockExecuteInSandbox.mock.calls.at(-1)?.[0]
602+
expect(request.code).toContain('{{API_KEY}}')
603+
expect(request.code).not.toContain(secret)
604+
}
605+
)
606+
570607
it.each([
571608
{ language: 'javascript', code: 'return 42' },
572609
{ language: 'python', code: '__sim_result__ = 42' },

apps/sim/lib/function-execution/execute-request.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
CodePlaceholderCompileError,
2121
type CodePlaceholderPrivateInput,
2222
type CodePlaceholderRuntimeBinding,
23+
type CompiledCodePlaceholders,
2324
compileCodePlaceholders,
2425
} from '@/lib/execution/code-placeholders'
2526
import { parseExecutionDeadlineHeader } from '@/lib/execution/execution-deadline-header'
@@ -2413,13 +2414,23 @@ export async function executeFunctionRequest(
24132414
outputSandboxPaths.length > 0 ||
24142415
Boolean(outputSandboxPath)
24152416

2416-
const compilation = await compileCodePlaceholders({
2417-
code: codeResolution.resolvedCode,
2418-
language: lang,
2419-
params: executionParams,
2420-
environmentVariables: envVars,
2421-
reservedNames: Object.keys(contextVariables),
2422-
})
2417+
/** Mothership mounts named secrets explicitly; its code may author literal workflow templates. */
2418+
const compilation: CompiledCodePlaceholders = usesMothershipSandbox
2419+
? {
2420+
code: codeResolution.resolvedCode,
2421+
bindings: [],
2422+
privateInputs: [],
2423+
runtimeBindings: [],
2424+
internalIdentifiers: [],
2425+
resolvedSecretNames: Object.keys(envVars),
2426+
}
2427+
: await compileCodePlaceholders({
2428+
code: codeResolution.resolvedCode,
2429+
language: lang,
2430+
params: executionParams,
2431+
environmentVariables: envVars,
2432+
reservedNames: Object.keys(contextVariables),
2433+
})
24232434
for (const name of compilation.resolvedSecretNames) {
24242435
if (!Object.hasOwn(envVars, name)) continue
24252436
const plaintext = envVars[name]

apps/sim/lib/mothership/generated/workbench.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33

44
import { z } from "zod";
55

6+
/** Secret names explicitly requested for one workbench code call. Values never cross this wire. */
7+
export const WorkbenchSecretNames = z.array(z.string().trim().min(1).max(1024)).max(100);
8+
69
/** Executable bootstrap is served only on the authenticated Sim → worker connection. */
710
export const WorkbenchBootstrap = z.strictObject({
811
version: z.literal(1),

apps/sim/lib/mothership/tools/handlers/function-execute-session.test.ts

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,17 @@
33
*/
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockExecuteTool } = vi.hoisted(() => ({
6+
const { mockExecuteTool, mockMaterializeSecrets } = vi.hoisted(() => ({
77
mockExecuteTool: vi.fn().mockResolvedValue({ success: true, output: {} }),
8+
mockMaterializeSecrets: vi
9+
.fn()
10+
.mockResolvedValue({ envVars: { API_KEY: 'test-value' }, catalogEntries: [] }),
811
}))
912

1013
vi.mock('@/tools', () => ({ executeTool: mockExecuteTool }))
11-
vi.mock('@/executor/utils/code-secret-references', () => ({
12-
extractCodeSecretNames: vi.fn().mockResolvedValue([]),
14+
vi.mock('@/lib/mothership/tools/secret-mount-materializer.server', () => ({
15+
materializeCopilotCodeSecrets: mockMaterializeSecrets,
16+
CopilotCodeSecretAccessError: class extends Error {},
1317
}))
1418
vi.mock('@/executor/utils/resolved-secret-trace-registry', () => ({
1519
ResolvedSecretTraceRegistry: class {
@@ -51,6 +55,7 @@ const BASE_CONTEXT: ToolExecutionContext = {
5155
describe('executeFunctionExecute session plumbing', () => {
5256
beforeEach(() => {
5357
mockExecuteTool.mockClear()
58+
mockMaterializeSecrets.mockClear()
5459
})
5560

5661
it('derives the session key from the chat, one per chat', async () => {
@@ -77,6 +82,57 @@ describe('executeFunctionExecute session plumbing', () => {
7782
expect(params.sandboxSessionKey).toBeUndefined()
7883
})
7984

85+
it.each([
86+
{ language: 'python', code: 'import json\nprint(json.dumps({"apiKey": "{{EXA_API_KEY}}"}))' },
87+
{ language: 'python', code: 'print("{{" + "EXA_API_KEY" + "}}")' },
88+
{ language: 'javascript', code: 'return JSON.stringify({ apiKey: "{{EXA_API_KEY}}" })' },
89+
{ language: 'shell', code: "printf '%s' '{{EXA_API_KEY}}'" },
90+
])('keeps authored $language templates literal without requesting secrets', async (params) => {
91+
await executeFunctionExecute(params, BASE_CONTEXT)
92+
expect(mockMaterializeSecrets).not.toHaveBeenCalled()
93+
expect(mockExecuteTool.mock.calls[0][1]).toMatchObject({
94+
code: params.code,
95+
envVars: {},
96+
secretScope: 'selected',
97+
mountedSecrets: [],
98+
})
99+
})
100+
101+
it('mounts only explicitly named secrets within the caller policy', async () => {
102+
await executeFunctionExecute(
103+
{
104+
code: "return environmentVariables['API_KEY']",
105+
language: 'javascript',
106+
secrets: [' API_KEY ', 'API_KEY'],
107+
},
108+
{
109+
...BASE_CONTEXT,
110+
secretMountPolicy: { secretScope: 'selected', mountedSecrets: ['API_KEY'] },
111+
}
112+
)
113+
expect(mockMaterializeSecrets).toHaveBeenCalledExactlyOnceWith({
114+
actorUserId: 'user-1',
115+
workspaceId: 'ws-1',
116+
requestedNames: ['API_KEY'],
117+
})
118+
expect(mockExecuteTool.mock.calls[0][1]).toMatchObject({
119+
envVars: { API_KEY: 'test-value' },
120+
mountedSecrets: ['API_KEY'],
121+
})
122+
expect(mockExecuteTool.mock.calls[0][1]).not.toHaveProperty('secrets')
123+
})
124+
125+
it('rejects explicit secrets outside the allowlist before materialization or execution', async () => {
126+
await expect(
127+
executeFunctionExecute(
128+
{ code: 'return 1', secrets: ['API_KEY'] },
129+
{ ...BASE_CONTEXT, secretMountPolicy: { secretScope: 'selected', mountedSecrets: [] } }
130+
)
131+
).rejects.toThrow('Secret access is not allowed for: API_KEY')
132+
expect(mockMaterializeSecrets).not.toHaveBeenCalled()
133+
expect(mockExecuteTool).not.toHaveBeenCalled()
134+
})
135+
80136
it('converts second-denominated timeouts, including string values', async () => {
81137
// The catalog doc promises seconds; models also send the number as a string.
82138
// Without the tolerant parse, "90" reached the body schema's z.coerce and

apps/sim/lib/mothership/tools/handlers/function-execute.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import { executeCopilotTableUseCase } from '@/lib/mothership/application/execute-table-use-case'
1818
import { resolveCopilotFilePrincipal } from '@/lib/mothership/auth/file-delegation'
1919
import { messageForCopilotTableError } from '@/lib/mothership/auth/table-delegation'
20+
import { WorkbenchSecretNames } from '@/lib/mothership/generated/workbench'
2021
import { applySecretMountPolicy } from '@/lib/mothership/secret-mount-policy'
2122
import type {
2223
ToolExecutionContext,
@@ -47,7 +48,6 @@ import {
4748
buildWorkspaceFileFolderDisplayPath,
4849
parseWorkspaceFileFolderDisplayPath,
4950
} from '@/lib/workspace-files/folder-display-path'
50-
import { extractCodeSecretNames } from '@/executor/utils/code-secret-references'
5151
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
5252
import { executeTool as executeAppTool } from '@/tools'
5353

@@ -404,6 +404,7 @@ export async function executeFunctionExecute(
404404
context: ToolExecutionContext
405405
): Promise<ToolExecutionResult> {
406406
const enrichedParams = omit(params, [
407+
'secrets',
407408
'sandboxProfile',
408409
'internalSandboxProfile',
409410
// Server-derived below — a model-supplied value must never select a session.
@@ -440,7 +441,7 @@ export async function executeFunctionExecute(
440441
enrichedParams.sandboxId = params.sandboxId.trim()
441442
}
442443
const requestedNames = applySecretMountPolicy(
443-
await extractCodeSecretNames(params.code, params.language),
444+
WorkbenchSecretNames.parse(params.secrets === undefined ? [] : params.secrets),
444445
context.secretMountPolicy
445446
)
446447
const completePendingActivation =

0 commit comments

Comments
 (0)