From 34811f58a31b5e3d2b5e3da88e35346826ac239f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 29 Mar 2026 22:11:34 +0000 Subject: [PATCH 1/5] Add secure codemode secret_set capability Co-authored-by: Kent C. Dodds --- docs/agents/adding-capabilities.md | 9 + docs/agents/secret-host-approval.md | 4 + .../src/mcp/capabilities/secrets/domain.ts | 9 +- .../mcp/capabilities/secrets/secret-delete.ts | 2 +- .../mcp/capabilities/secrets/secret-list.ts | 2 +- .../mcp/capabilities/secrets/secret-set.ts | 73 ++++++++ packages/worker/src/mcp/index.ts | 7 +- .../worker/src/mcp/mcp-server.mcp-e2e.test.ts | 74 +++++++++ .../mcp/run-codemode-registry.node.test.ts | 155 ++++++++++++++++- .../worker/src/mcp/run-codemode-registry.ts | 156 +++++++++++++++++- .../src/mcp/secrets/capability-inputs.ts | 14 +- .../infer-codemode-capabilities.node.test.ts | 15 ++ packages/worker/src/mcp/tools/execute.ts | 2 + 13 files changed, 504 insertions(+), 18 deletions(-) create mode 100644 packages/worker/src/mcp/capabilities/secrets/secret-set.ts diff --git a/docs/agents/adding-capabilities.md b/docs/agents/adding-capabilities.md index 5907001bfd..a81ca781dc 100644 --- a/docs/agents/adding-capabilities.md +++ b/docs/agents/adding-capabilities.md @@ -104,6 +104,12 @@ runs. Missing secrets still fail with the same "secret not found" guidance used elsewhere, and secret-bearing capability inputs still require an authenticated user. +Those inputs are also treated as write-only for the rest of that execution: +once plaintext crosses an `x-kody-secret` capability boundary, Kody redacts that +plaintext from later execute results and logs before returning them to the +caller. Capability authors should still avoid returning or logging secret +material, but the runtime adds this extra defense-in-depth layer. + When a secret has an `allowed_capabilities` policy, Kody also checks that the current capability name is explicitly listed before resolving the placeholder. An empty `allowed_capabilities` list means no capability is allowed to resolve @@ -132,6 +138,8 @@ How to annotate: 3. Leave non-secret fields unannotated. 4. Document the intended use in the capability description when it may not be obvious. +5. If the capability persists or hands off a secret value, make the description + say that the input is write-only and must never be returned to chat. Example: @@ -289,6 +297,7 @@ scope narrow: - annotate only the exact credential fields, not the whole object - prefer this for local persistence or device-side credential flows +- treat those inputs as write-only even if the runtime redacts accidental echoes - tell users which capability names should be added to a secret's `allowed_capabilities` policy when a workflow depends on restricted secrets - avoid using it for generic remote API wrappers where fetch-time host approval diff --git a/docs/agents/secret-host-approval.md b/docs/agents/secret-host-approval.md index ca3fa0ce46..507d29c713 100644 --- a/docs/agents/secret-host-approval.md +++ b/docs/agents/secret-host-approval.md @@ -73,6 +73,8 @@ When writing capability descriptions or agent-facing docs: - say explicitly that only the authenticated account admin UI can approve hosts - say explicitly that only the authenticated account admin UI can restrict which capabilities may consume a secret directly +- say explicitly that secret-bearing capability inputs are write-only from the + model's perspective and must not be echoed in execute results or logs - tell agents to inspect secret metadata before making a secret-bearing request - tell agents to surface the approval link and stop on deny @@ -102,6 +104,8 @@ Use it narrowly: - mark only the exact sensitive fields - prefer it for local persistence or device-side credential handoff - require an authenticated user, just like fetch-time secret placeholders +- never return or log the resolved plaintext after it crosses an + `x-kody-secret` capability boundary Do not treat `x-kody-secret` as a generic replacement for execute-time `fetch(...)` placeholders. diff --git a/packages/worker/src/mcp/capabilities/secrets/domain.ts b/packages/worker/src/mcp/capabilities/secrets/domain.ts index 9dbc2727c6..63b9ded374 100644 --- a/packages/worker/src/mcp/capabilities/secrets/domain.ts +++ b/packages/worker/src/mcp/capabilities/secrets/domain.ts @@ -2,11 +2,16 @@ import { defineDomain } from '#mcp/capabilities/define-domain.ts' import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' import { secretDeleteCapability } from './secret-delete.ts' import { secretListCapability } from './secret-list.ts' +import { secretSetCapability } from './secret-set.ts' export const secretsDomain = defineDomain({ name: capabilityDomainNames.secrets, description: - 'Server-side secret references that can be discovered by name, listed with allowed-host metadata, and deleted without placing raw secret values into prompts or saved app source.', + 'Server-side secret references that can be created, listed, and deleted without placing raw secret values into prompts, execute results, or saved app source.', keywords: ['secret', 'credentials', 'reference', 'secure input'], - capabilities: [secretListCapability, secretDeleteCapability], + capabilities: [ + secretListCapability, + secretSetCapability, + secretDeleteCapability, + ], }) diff --git a/packages/worker/src/mcp/capabilities/secrets/secret-delete.ts b/packages/worker/src/mcp/capabilities/secrets/secret-delete.ts index cfa0d78692..c49ad0dbcd 100644 --- a/packages/worker/src/mcp/capabilities/secrets/secret-delete.ts +++ b/packages/worker/src/mcp/capabilities/secrets/secret-delete.ts @@ -11,7 +11,7 @@ export const secretDeleteCapability = defineDomainCapability( { name: 'secret_delete', description: - 'Delete an existing secret reference for the signed-in user. Use this when a secret should no longer be available to execute-time code.', + 'Delete an existing secret reference for the signed-in user without revealing its plaintext value. Never ask the user to paste a secret, token, API key, password, or credential into chat; use generated UI to collect missing user-provided secrets safely. Use this when a secret should no longer be available to execute-time code.', keywords: ['secret', 'delete', 'remove', 'revoke', 'credential'], readOnly: false, idempotent: false, diff --git a/packages/worker/src/mcp/capabilities/secrets/secret-list.ts b/packages/worker/src/mcp/capabilities/secrets/secret-list.ts index 977ed8b133..ef0fea9b04 100644 --- a/packages/worker/src/mcp/capabilities/secrets/secret-list.ts +++ b/packages/worker/src/mcp/capabilities/secrets/secret-list.ts @@ -12,7 +12,7 @@ export const secretListCapability = defineDomainCapability( { name: 'secret_list', description: - 'List available secret references for the signed-in user without revealing secret values. When scope is omitted, results include every accessible scope in precedence order. Use `codemode.secret_list({ scope })` inside execute-time code when you want the same metadata, including allowed hosts and allowed capabilities, from the sandbox.', + 'List available secret references for the signed-in user without revealing secret values. When scope is omitted, results include every accessible scope in precedence order. Use `codemode.secret_list({ scope })` inside execute-time code when you want the same metadata, including allowed hosts and allowed capabilities, from the sandbox. Never return a secret value from execute, and never ask the user to paste a secret, token, API key, password, or credential into chat; use generated UI to collect missing secrets safely.', keywords: ['secret', 'list', 'discovery', 'metadata', 'credentials'], readOnly: true, idempotent: true, diff --git a/packages/worker/src/mcp/capabilities/secrets/secret-set.ts b/packages/worker/src/mcp/capabilities/secrets/secret-set.ts new file mode 100644 index 0000000000..cf5236fe8f --- /dev/null +++ b/packages/worker/src/mcp/capabilities/secrets/secret-set.ts @@ -0,0 +1,73 @@ +import { markSecretInputFields } from '@kody-internal/shared/secret-input-schema.ts' +import { z } from 'zod' +import { defineDomainCapability } from '#mcp/capabilities/define-domain-capability.ts' +import { capabilityDomainNames } from '#mcp/capabilities/domain-metadata.ts' +import { requireMcpUser } from '#mcp/capabilities/meta/require-user.ts' +import { type CapabilityContext } from '#mcp/capabilities/types.ts' +import { saveSecret } from '#mcp/secrets/service.ts' +import { secretScopeValues } from '#mcp/secrets/types.ts' +import { secretMetadataSchema } from './shared.ts' + +const secretSetInputSchema = z.object({ + name: z.string().min(1).describe('Secret name to create or update.'), + value: z + .string() + .min(1) + .describe( + 'Secret value to persist. This field is write-only and must never be returned to the caller.', + ), + description: z + .string() + .optional() + .describe('Optional human-readable description of the secret.'), + scope: z + .enum(secretScopeValues) + .describe('Storage scope that owns the secret.'), +}) + +const secretSetCapabilityInputJsonSchema = markSecretInputFields( + z.toJSONSchema(secretSetInputSchema) as Record, + ['value'], +) as Record + +export const secretSetCapability = defineDomainCapability( + capabilityDomainNames.secrets, + { + name: 'secret_set', + description: + 'Create or update a stored secret reference for the signed-in user without ever returning the plaintext value. Use this only for server-side persistence of secret values that are already available inside trusted execution, such as refreshed OAuth tokens. Never ask the user to paste a secret, token, API key, password, or credential into chat; use generated UI to collect missing user-provided secrets safely. Saving a secret value does not authorize outbound host use or direct capability access.', + keywords: ['secret', 'persist', 'store', 'oauth', 'token', 'credential'], + readOnly: false, + idempotent: false, + destructive: false, + inputSchema: secretSetCapabilityInputJsonSchema, + outputSchema: secretMetadataSchema, + async handler(args, ctx: CapabilityContext) { + const parsed = secretSetInputSchema.parse(args) + const user = requireMcpUser(ctx.callerContext) + const saved = await saveSecret({ + env: ctx.env, + userId: user.userId, + scope: parsed.scope, + name: parsed.name, + value: parsed.value, + description: parsed.description ?? '', + storageContext: { + sessionId: ctx.callerContext.storageContext?.sessionId ?? null, + appId: ctx.callerContext.storageContext?.appId ?? null, + }, + }) + return { + name: saved.name, + scope: saved.scope, + description: saved.description, + app_id: saved.appId, + allowed_hosts: saved.allowedHosts, + allowed_capabilities: saved.allowedCapabilities, + created_at: saved.createdAt, + updated_at: saved.updatedAt, + ttl_ms: saved.ttlMs, + } + }, + }, +) diff --git a/packages/worker/src/mcp/index.ts b/packages/worker/src/mcp/index.ts index ce8ef2e7fa..acc3bf0af4 100644 --- a/packages/worker/src/mcp/index.ts +++ b/packages/worker/src/mcp/index.ts @@ -37,6 +37,7 @@ Quick start - When a saved skill declares parameters, pass values via meta_run_skill params; the codemode can read them from the params variable. - Use 'ui_save_app' to persist reusable UI source for later reopening via 'app_id'. Saved apps are user-scoped UI artifacts, not codemode skills. - Use \`codemode.secret_list(args)\` during execute-time code to list secret metadata only; it does not return plaintext values. +- Use \`codemode.secret_set(args)\` only to persist secret values that are already available inside trusted execution, such as refreshed OAuth tokens. It returns metadata only and never returns plaintext values. Kody source repository - Kody (this app and MCP server) is developed at https://github.com/kentcdodds/kody. When you launch a Cursor Cloud Agent to improve Kody itself, use that repository URL (unless the user explicitly points you at another fork or repo). @@ -81,13 +82,15 @@ How to use execute - When chaining calls, read fields from the previous result using its outputSchema. - Chain multiple calls, use conditionals, and return structured results. - Use \`await codemode.secret_list({})\` or \`await codemode.secret_list({ scope: 'app' })\` when you need secret metadata such as names, descriptions, scopes, allowed hosts, and allowed capabilities from the sandbox. +- Use \`await codemode.secret_set({ name, value, scope, description? })\` only when a capability or fetch flow has already produced a secret value inside execution and you need to persist it without showing it to the model or returning it to the caller. - Use \`await codemode.value_get({ name })\` or \`await codemode.value_list({ scope })\` for readable non-secret configuration that generated UI code should be able to store and read back later. - Execute-time code includes \`refreshAccessToken(providerName)\` and \`createAuthenticatedFetch(providerName)\` helpers for connector OAuth flows. - Use normal \`fetch(...)\` for outbound HTTP. To inject a stored secret, place a placeholder such as \`{{secret:cloudflareToken}}\` or \`{{secret:cloudflareToken|scope=user}}\` in the URL, headers, or request body; the host resolves it server-side and blocks unapproved destinations. - Some capability input fields also accept secret placeholders. When an input schema marks a string field with \`x-kody-secret: true\`, you may pass \`{{secret:name}}\` or \`{{secret:name|scope=user}}\` there instead of a raw value. If that secret has an allowed-capabilities policy, the current capability name must be on the allowlist. - Secret placeholders are not general-purpose string interpolation. Do not use \`execute\` to build a string or object that merely returns \`{{secret:...}}\`; those placeholders only resolve in secret-aware fetch paths or capability inputs that explicitly opt into \`x-kody-secret\`. -- Saving or updating a secret does not authorize sending it anywhere. If a fetch fails because a host is not approved for that secret, ask the user whether to open the approval link and approve that host in the web app. -- Secrets are intentionally not readable or updatable through \`codemode\`. Never ask the user to paste a secret into chat; use generated UI flows such as \`saveSecret(...)\` when the user needs to provide or rotate a value, and use \`codemode.secret_delete(...)\` only when removing a stored secret reference. +- Saving or updating a secret does not authorize sending it anywhere or passing it into arbitrary capabilities. If a fetch fails because a host is not approved for that secret, ask the user whether to open the approval link and approve that host in the web app. +- Never ask the user to paste a secret into chat. Use generated UI flows when the user needs to provide or rotate a value themselves. \`codemode.secret_set(...)\` is for persisting secret values that are already available inside execution, not for collecting them from chat. +- Secret values that pass through \`x-kody-secret\` capability inputs are treated as write-only: they may be persisted or handed to the host-side capability, but they must never be returned in execute results or logs. - Your code must be an async arrow function that returns the result. - Example: const result = await codemode[capabilityName](args) diff --git a/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts b/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts index 777cd6aa25..a16fcb2e33 100644 --- a/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts +++ b/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts @@ -1494,6 +1494,76 @@ test('generated ui sessions support secret storage, execute-time resolution, and expect(executePayload.result?.userSecretNames).toContain('globalApiKey') expect(executePayload.result?.sessionSecretNames).toContain('ephemeralCode') + const refreshedAccessToken = `spotify-access-${crypto.randomUUID()}` + const refreshedRefreshToken = `spotify-refresh-${crypto.randomUUID()}` + const secretSetExecuteResponse = await fetch(executeUrl!, { + method: 'POST', + headers: { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json', + Accept: 'application/json', + }, + body: JSON.stringify({ + code: `async () => { + const savedAccessToken = await codemode.secret_set({ + name: 'spotifyAccessToken', + value: ${JSON.stringify(refreshedAccessToken)}, + description: 'Spotify OAuth access token', + scope: 'user', + }) + const savedRefreshToken = await codemode.secret_set({ + name: 'spotifyRefreshToken', + value: ${JSON.stringify(refreshedRefreshToken)}, + description: 'Spotify OAuth refresh token', + scope: 'user', + }) + return { + savedAccessToken, + savedRefreshToken, + leakedValue: ${JSON.stringify(refreshedAccessToken)}, + } + }`, + }), + }) + if (!secretSetExecuteResponse.ok) { + throw new Error(await secretSetExecuteResponse.text()) + } + const secretSetExecutePayload = + (await secretSetExecuteResponse.json()) as { + ok?: boolean + result?: { + savedAccessToken?: Record + savedRefreshToken?: Record + leakedValue?: string + } + logs?: Array + } + expect(secretSetExecutePayload.ok).toBe(true) + expect(secretSetExecutePayload.result?.savedAccessToken).toEqual({ + name: 'spotifyAccessToken', + scope: 'user', + description: 'Spotify OAuth access token', + app_id: null, + allowed_hosts: [], + allowed_capabilities: [], + created_at: expect.any(String), + updated_at: expect.any(String), + ttl_ms: null, + }) + expect(secretSetExecutePayload.result?.savedRefreshToken).toEqual({ + name: 'spotifyRefreshToken', + scope: 'user', + description: 'Spotify OAuth refresh token', + app_id: null, + allowed_hosts: [], + allowed_capabilities: [], + created_at: expect.any(String), + updated_at: expect.any(String), + ttl_ms: null, + }) + expect(secretSetExecutePayload.result?.leakedValue).toBe('[REDACTED SECRET]') + expect(secretSetExecutePayload.logs ?? []).toEqual([]) + const listSecretsResponse = await fetch(`${secretsUrl!}?scope=app`, { headers: { Authorization: `Bearer ${token}`, @@ -1814,6 +1884,10 @@ test('generated ui sessions support secret storage, execute-time resolution, and (match) => match.type === 'secret' && match.name === 'globalApiKey', ) expect(userSecretMatch?.description).toBe('Reusable cross-app API key') + const spotifySecretMatch = matches.find( + (match) => match.type === 'secret' && match.name === 'spotifyAccessToken', + ) + expect(spotifySecretMatch?.description).toBe('Spotify OAuth access token') const sessionSecretMatch = matches.find( (match) => match.type === 'secret' && match.name === 'ephemeralCode', ) diff --git a/packages/worker/src/mcp/run-codemode-registry.node.test.ts b/packages/worker/src/mcp/run-codemode-registry.node.test.ts index 654670ffcc..61370bcebf 100644 --- a/packages/worker/src/mcp/run-codemode-registry.node.test.ts +++ b/packages/worker/src/mcp/run-codemode-registry.node.test.ts @@ -1,6 +1,10 @@ import { expect, test, vi } from 'vitest' +import { type getCapabilityRegistryForContext } from '#mcp/capabilities/registry.ts' import { createMcpCallerContext } from '#mcp/context.ts' -import { buildCodemodeFns } from './run-codemode-registry.ts' +import { + buildCodemodeFns, + runCodemodeWithRegistry, +} from './run-codemode-registry.ts' import * as secretService from '#mcp/secrets/service.ts' test('buildCodemodeFns resolves annotated home capability secret placeholders', async () => { @@ -180,3 +184,152 @@ test('buildCodemodeFns denies capability secret placeholders for disallowed capa resolveSecretSpy.mockRestore() } }) + +test('runCodemodeWithRegistry redacts values that crossed secret-marked capability inputs', async () => { + const env = { + LOADER: {}, + } as unknown as Env + const executorExports = { + CodemodeFetchGateway() { + return {} + }, + } as unknown as typeof import('cloudflare:workers').exports + const callerContext = createMcpCallerContext({ + baseUrl: 'https://heykody.dev', + user: { userId: 'user-123' }, + }) + + const getRegistrySpy = vi + .spyOn(await import('#mcp/capabilities/registry.ts'), 'getCapabilityRegistryForContext') + .mockResolvedValue({ + capabilityDomains: [], + capabilityDomainDescriptionsByName: {} as Record, + capabilityHandlers: {}, + capabilityList: [ + { + name: 'secret_set', + domain: 'secrets', + description: 'Store a secret.', + keywords: [], + readOnly: false, + idempotent: false, + destructive: false, + inputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + value: { type: 'string', 'x-kody-secret': true }, + }, + required: ['name', 'value'], + }, + outputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + async handler(args: Record) { + return { + name: args.name, + } + }, + }, + ], + capabilityMap: { + secret_set: { + name: 'secret_set', + domain: 'secrets', + description: 'Store a secret.', + keywords: [], + readOnly: false, + idempotent: false, + destructive: false, + inputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + value: { type: 'string', 'x-kody-secret': true }, + }, + required: ['name', 'value'], + }, + outputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + async handler(args: Record) { + return { + name: args.name, + } + }, + }, + }, + capabilitySpecs: {}, + capabilityToolDescriptors: { + secret_set: { + description: 'Store a secret.', + inputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + value: { type: 'string', 'x-kody-secret': true }, + }, + required: ['name', 'value'], + }, + outputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + }, + }, + } as Awaited>) + + const executeSpy = vi + .spyOn(await import('#mcp/executor.ts'), 'createExecuteExecutor') + .mockReturnValue({ + async execute() { + return { + result: { + saved: true, + echoedValue: 'fresh-access-token', + nested: { + message: 'Bearer fresh-access-token', + }, + }, + logs: ['saved fresh-access-token'], + } + }, + } as Awaited>) + + try { + const result = await runCodemodeWithRegistry( + env, + callerContext, + `async () => { + await codemode.secret_set({ + name: 'spotifyAccessToken', + value: 'fresh-access-token', + }) + return { ok: true } + }`, + undefined, + executorExports, + ) + + expect(result.error).toBeUndefined() + expect(result.result).toEqual({ + saved: true, + echoedValue: '[REDACTED SECRET]', + nested: { + message: 'Bearer [REDACTED SECRET]', + }, + }) + expect(result.logs).toEqual(['saved [REDACTED SECRET]']) + } finally { + getRegistrySpy.mockRestore() + executeSpy.mockRestore() + } +}) diff --git a/packages/worker/src/mcp/run-codemode-registry.ts b/packages/worker/src/mcp/run-codemode-registry.ts index 4dc301d5d9..18c770e265 100644 --- a/packages/worker/src/mcp/run-codemode-registry.ts +++ b/packages/worker/src/mcp/run-codemode-registry.ts @@ -1,11 +1,19 @@ import { resolveProvider, + type ExecuteResult, type ResolvedProvider, type ToolProvider, } from '@cloudflare/codemode' import { exports as workerExports } from 'cloudflare:workers' import { type McpCallerContext } from '@kody-internal/shared/chat.ts' -import { resolveCapabilityInputSecrets } from '#mcp/secrets/capability-inputs.ts' +import { + getAdditionalPropertiesSchema, + getArrayItemSchema, + getSchemaProperties, + isRecord, + isSecretInputSchema, + resolveCapabilityInputSecrets, +} from '#mcp/secrets/capability-inputs.ts' import { capabilityInputSecretAuthRequiredMessage, createCapabilitySecretAccessDeniedMessage, @@ -27,6 +35,7 @@ export async function buildCodemodeFns( secret: ReferencedSecret, capabilityName: string, ) => Promise + trackSecretInputValue?: (value: string) => void }, ) { const { capabilityMap } = await getCapabilityRegistryForContext({ @@ -50,6 +59,11 @@ export async function buildCodemodeFns( resolveSecretValue: (secret) => resolveSecretValue(secret, capability.name), }) + collectSecretInputValues({ + schema: capability.inputSchema, + value: resolvedArgs, + track: options?.trackSecretInputValue, + }) return capability.handler(resolvedArgs as Record, { env, callerContext, @@ -62,8 +76,11 @@ export async function buildCodemodeFns( export async function buildCodemodeProvider( env: Env, callerContext: McpCallerContext, + options?: { + trackSecretInputValue?: (value: string) => void + }, ): Promise { - const tools = await buildCodemodeFns(env, callerContext) + const tools = await buildCodemodeFns(env, callerContext, options) const provider: ToolProvider = { name: 'codemode', tools: Object.fromEntries( @@ -127,9 +144,10 @@ export async function runCodemodeWithRegistry( code: string, params?: Record, executorExports?: typeof workerExports, -) { +): Promise { const { createExecuteExecutor } = await import('#mcp/executor.ts') const { normalizeCode } = await import('@cloudflare/codemode') + const secretRedactor = createExecutionSecretRedactor() const normalizedStorageContext = normalizeStorageContext( callerContext.storageContext ?? null, ) @@ -142,7 +160,11 @@ export async function runCodemodeWithRegistry( storageContext: normalizedStorageContext, }, }) - const provider = await buildCodemodeProvider(env, callerContext) + const provider = await buildCodemodeProvider(env, callerContext, { + trackSecretInputValue: (value) => { + secretRedactor.track(value) + }, + }) const wrappedCode = params !== undefined ? await buildParameterizedSkillCode(code, params) @@ -154,14 +176,19 @@ ${createExecuteHelperPrelude()} return await __kodyUserCode(); }` const result = await executor.execute(wrapped, [provider]) - if (!result.error) return result + const sanitizedResult = secretRedactor.sanitizeExecuteResult(result) + if (!result.error) return sanitizedResult const batchMessage = await rewriteCapabilitySecretError({ error: result.error, code: wrapped, env, callerContext, }) - return batchMessage ? { ...result, error: batchMessage } : result + if (!batchMessage) return sanitizedResult + return { + ...sanitizedResult, + error: secretRedactor.redactErrorMessage(batchMessage), + } } async function rewriteCapabilitySecretError(input: { @@ -216,6 +243,123 @@ function normalizeSecretNameList(names: Array) { ).sort((left, right) => left.localeCompare(right)) } +const redactedSecretText = '[REDACTED SECRET]' + +function createExecutionSecretRedactor() { + const secretValues = new Set() + return { + track(value: string) { + if (value.length > 0) { + secretValues.add(value) + } + }, + redactErrorMessage(value: string) { + return redactSecretValuesInString(value, secretValues) + }, + sanitizeExecuteResult(result: ExecuteResult): ExecuteResult { + return { + ...result, + result: redactUnknownSecretValues(result.result, secretValues), + logs: Array.isArray(result.logs) + ? result.logs.map((entry) => + redactSecretValuesInString(entry, secretValues), + ) + : result.logs, + error: + typeof result.error === 'string' + ? redactSecretValuesInString(result.error, secretValues) + : result.error, + } + }, + } +} + +function collectSecretInputValues(input: { + schema: unknown + value: unknown + track?: (value: string) => void +}) { + if (!input.track) return + visitSecretInputValue(input.schema, input.value, input.track) +} + +function visitSecretInputValue( + schema: unknown, + value: unknown, + track: (value: string) => void, +) { + if (typeof value === 'string' && isSecretInputSchema(schema)) { + track(value) + return + } + if (Array.isArray(value)) { + const itemSchema = getArrayItemSchema(schema) + if (!itemSchema) return + for (const item of value) { + visitSecretInputValue(itemSchema, item, track) + } + return + } + if (!isRecord(value)) return + const propertySchemas = getSchemaProperties(schema) + const additionalProperties = getAdditionalPropertiesSchema(schema) + if (!propertySchemas && !additionalProperties) return + for (const [key, entryValue] of Object.entries(value)) { + const entrySchema = propertySchemas?.[key] ?? additionalProperties + if (!entrySchema) continue + visitSecretInputValue(entrySchema, entryValue, track) + } +} + +function redactUnknownSecretValues( + value: unknown, + secretValues: ReadonlySet, +): unknown { + if (secretValues.size === 0) return value + if (typeof value === 'string') { + return redactSecretValuesInString(value, secretValues) + } + if (value instanceof Error) { + const next = new Error( + redactSecretValuesInString(value.message, secretValues), + value.cause !== undefined + ? { cause: redactUnknownSecretValues(value.cause, secretValues) } + : undefined, + ) + next.name = value.name + if (value.stack) { + next.stack = redactSecretValuesInString(value.stack, secretValues) + } + return next + } + if (Array.isArray(value)) { + return value.map((entry) => redactUnknownSecretValues(entry, secretValues)) + } + if (isRecord(value)) { + return Object.fromEntries( + Object.entries(value).map(([key, entry]) => [ + key, + redactUnknownSecretValues(entry, secretValues), + ]), + ) + } + return value +} + +function redactSecretValuesInString( + value: string, + secretValues: ReadonlySet, +) { + if (secretValues.size === 0 || value.length === 0) return value + let nextValue = value + for (const secretValue of [...secretValues].sort( + (left, right) => right.length - left.length, + )) { + nextValue = nextValue.replaceAll(secretValue, redactedSecretText) + } + return nextValue +} + function normalizeStorageContext( storageContext: McpCallerContext['storageContext'] | null, ) { diff --git a/packages/worker/src/mcp/secrets/capability-inputs.ts b/packages/worker/src/mcp/secrets/capability-inputs.ts index 9de9af08a6..8145701cbc 100644 --- a/packages/worker/src/mcp/secrets/capability-inputs.ts +++ b/packages/worker/src/mcp/secrets/capability-inputs.ts @@ -81,29 +81,33 @@ async function resolveStringPlaceholders( return replaceSecretPlaceholders(value, replacements) } -function isSecretInputSchema(schema: unknown) { +export function isSecretInputSchema(schema: unknown): boolean { if (!isRecord(schema)) return false return schema[secretInputSchemaFlag] === true } -function getSchemaProperties(schema: unknown) { +export function getSchemaProperties( + schema: unknown, +): Record | null { if (!isRecord(schema)) return null const properties = schema.properties return isRecord(properties) ? properties : null } -function getArrayItemSchema(schema: unknown) { +export function getArrayItemSchema(schema: unknown): unknown | null { if (!isRecord(schema)) return null const items = schema.items return items === undefined || Array.isArray(items) ? null : items } -function getAdditionalPropertiesSchema(schema: unknown) { +export function getAdditionalPropertiesSchema( + schema: unknown, +): Record | null { if (!isRecord(schema)) return null const additionalProperties = schema.additionalProperties return isRecord(additionalProperties) ? additionalProperties : null } -function isRecord(value: unknown): value is Record { +export function isRecord(value: unknown): value is Record { return value !== null && typeof value === 'object' && !Array.isArray(value) } diff --git a/packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.ts b/packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.ts index 7f25f4df31..cea696a732 100644 --- a/packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.ts +++ b/packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.ts @@ -24,6 +24,21 @@ test('infers bracket string literal access', () => { expect(staticNames).toContain('github_rest') }) +test('infers secret_set member access', () => { + const src = `async () => { + const refreshedToken = 'token-from-refresh'; + await codemode.secret_set({ + name: 'spotifyAccessToken', + value: refreshedToken, + scope: 'user', + description: 'Spotify OAuth access token', + }); + }` + const { staticNames, inferencePartial } = inferCodemodeCapabilities(src) + expect(inferencePartial).toBe(false) + expect(staticNames).toContain('secret_set') +}) + test('marks partial for computed non-literal codemode access', () => { const program = parse( `async () => { const x = 'ui_save_app'; return await codemode[x]({}) }`, diff --git a/packages/worker/src/mcp/tools/execute.ts b/packages/worker/src/mcp/tools/execute.ts index 1da0353d8b..11f981a920 100644 --- a/packages/worker/src/mcp/tools/execute.ts +++ b/packages/worker/src/mcp/tools/execute.ts @@ -70,7 +70,9 @@ Network access: Secrets: - Never ask the user to paste secrets, tokens, API keys, passwords, OAuth codes, client secrets, or other credentials into chat. If a workflow needs a secret value that is not already stored, use generated UI to collect and save it instead. - Use \`await codemode.secret_list({})\` to inspect available secret metadata before building a request. The result is metadata only and does not reveal secret values; it includes allowed hosts and allowed capability names. +- Use \`await codemode.secret_set({ name, value, scope, description })\` only to persist a secret value that is already available inside trusted execution, such as a refreshed OAuth token. The \`value\` input is write-only; execute results and logs redact secret-marked capability inputs before they are returned. - Pass \`scope\` to narrow results, for example \`await codemode.secret_list({ scope: 'app' })\`. +- Deleting or saving a secret never grants outbound host approval or capability approval. Those policies are still managed through the authenticated account secrets UI. - Do not expect \`codemode.secret_get(...)\`, \`codemode.secret_require(...)\`, or any injected \`secrets\` helper to be available in execute-time code. Values: From 15d08bc3f4be1ca3a77a6335b05bd6789c716b84 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 29 Mar 2026 22:15:53 +0000 Subject: [PATCH 2/5] Fix secret_set redaction unit coverage Co-authored-by: Kent C. Dodds --- .../mcp/run-codemode-registry.node.test.ts | 66 +++++-------------- 1 file changed, 17 insertions(+), 49 deletions(-) diff --git a/packages/worker/src/mcp/run-codemode-registry.node.test.ts b/packages/worker/src/mcp/run-codemode-registry.node.test.ts index 61370bcebf..e82f89e419 100644 --- a/packages/worker/src/mcp/run-codemode-registry.node.test.ts +++ b/packages/worker/src/mcp/run-codemode-registry.node.test.ts @@ -1,10 +1,7 @@ import { expect, test, vi } from 'vitest' import { type getCapabilityRegistryForContext } from '#mcp/capabilities/registry.ts' import { createMcpCallerContext } from '#mcp/context.ts' -import { - buildCodemodeFns, - runCodemodeWithRegistry, -} from './run-codemode-registry.ts' +import { buildCodemodeFns } from './run-codemode-registry.ts' import * as secretService from '#mcp/secrets/service.ts' test('buildCodemodeFns resolves annotated home capability secret placeholders', async () => { @@ -185,19 +182,13 @@ test('buildCodemodeFns denies capability secret placeholders for disallowed capa } }) -test('runCodemodeWithRegistry redacts values that crossed secret-marked capability inputs', async () => { - const env = { - LOADER: {}, - } as unknown as Env - const executorExports = { - CodemodeFetchGateway() { - return {} - }, - } as unknown as typeof import('cloudflare:workers').exports +test('buildCodemodeFns tracks values that crossed secret-marked capability inputs', async () => { + const env = {} as Env const callerContext = createMcpCallerContext({ baseUrl: 'https://heykody.dev', user: { userId: 'user-123' }, }) + const trackedSecretValues: Array = [] const getRegistrySpy = vi .spyOn(await import('#mcp/capabilities/registry.ts'), 'getCapabilityRegistryForContext') @@ -287,49 +278,26 @@ test('runCodemodeWithRegistry redacts values that crossed secret-marked capabili }, } as Awaited>) - const executeSpy = vi - .spyOn(await import('#mcp/executor.ts'), 'createExecuteExecutor') - .mockReturnValue({ - async execute() { - return { - result: { - saved: true, - echoedValue: 'fresh-access-token', - nested: { - message: 'Bearer fresh-access-token', - }, - }, - logs: ['saved fresh-access-token'], - } - }, - } as Awaited>) - try { - const result = await runCodemodeWithRegistry( + const codemode = await buildCodemodeFns( env, callerContext, - `async () => { - await codemode.secret_set({ - name: 'spotifyAccessToken', - value: 'fresh-access-token', - }) - return { ok: true } - }`, - undefined, - executorExports, + { + trackSecretInputValue(value) { + trackedSecretValues.push(value) + }, + }, ) + const result = await codemode.secret_set({ + name: 'spotifyAccessToken', + value: 'fresh-access-token', + }) - expect(result.error).toBeUndefined() - expect(result.result).toEqual({ - saved: true, - echoedValue: '[REDACTED SECRET]', - nested: { - message: 'Bearer [REDACTED SECRET]', - }, + expect(result).toEqual({ + name: 'spotifyAccessToken', }) - expect(result.logs).toEqual(['saved [REDACTED SECRET]']) + expect(trackedSecretValues).toEqual(['fresh-access-token']) } finally { getRegistrySpy.mockRestore() - executeSpy.mockRestore() } }) From 6314a710d762a65a1ac2d77ae220446cb798f615 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 29 Mar 2026 23:15:44 +0000 Subject: [PATCH 3/5] Fix execute result typing for secret redaction Co-authored-by: Kent C. Dodds --- packages/worker/src/mcp/run-codemode-registry.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/worker/src/mcp/run-codemode-registry.ts b/packages/worker/src/mcp/run-codemode-registry.ts index 18c770e265..a9d1fbc58c 100644 --- a/packages/worker/src/mcp/run-codemode-registry.ts +++ b/packages/worker/src/mcp/run-codemode-registry.ts @@ -190,7 +190,6 @@ ${createExecuteHelperPrelude()} error: secretRedactor.redactErrorMessage(batchMessage), } } - async function rewriteCapabilitySecretError(input: { error: unknown code: string From d56ba2c20e8b7c68953457ccaae8a673e4503b7c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 30 Mar 2026 00:46:10 +0000 Subject: [PATCH 4/5] Rebase secret_set on main and address review feedback Co-authored-by: Kent C. Dodds --- docs/agents/adding-capabilities.md | 4 +- docs/agents/secret-host-approval.md | 4 +- package.json | 2 +- .../worker/public/mcp-apps/kody-ui-utils.css | 228 +++++++++--------- .../worker/src/mcp/mcp-server.mcp-e2e.test.ts | 23 +- .../mcp/run-codemode-registry.node.test.ts | 21 +- packages/worker/src/mcp/ui-artifacts-repo.ts | 6 +- 7 files changed, 137 insertions(+), 151 deletions(-) diff --git a/docs/agents/adding-capabilities.md b/docs/agents/adding-capabilities.md index a81ca781dc..5668d9da90 100644 --- a/docs/agents/adding-capabilities.md +++ b/docs/agents/adding-capabilities.md @@ -104,8 +104,8 @@ runs. Missing secrets still fail with the same "secret not found" guidance used elsewhere, and secret-bearing capability inputs still require an authenticated user. -Those inputs are also treated as write-only for the rest of that execution: -once plaintext crosses an `x-kody-secret` capability boundary, Kody redacts that +Those inputs are also treated as write-only for the rest of that execution: once +plaintext crosses an `x-kody-secret` capability boundary, Kody redacts that plaintext from later execute results and logs before returning them to the caller. Capability authors should still avoid returning or logging secret material, but the runtime adds this extra defense-in-depth layer. diff --git a/docs/agents/secret-host-approval.md b/docs/agents/secret-host-approval.md index 507d29c713..f8ef0db7f8 100644 --- a/docs/agents/secret-host-approval.md +++ b/docs/agents/secret-host-approval.md @@ -104,8 +104,8 @@ Use it narrowly: - mark only the exact sensitive fields - prefer it for local persistence or device-side credential handoff - require an authenticated user, just like fetch-time secret placeholders -- never return or log the resolved plaintext after it crosses an - `x-kody-secret` capability boundary +- never return or log the resolved plaintext after it crosses an `x-kody-secret` + capability boundary Do not treat `x-kody-secret` as a generic replacement for execute-time `fetch(...)` placeholders. diff --git a/package.json b/package.json index ac70f80ce2..7a25890881 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "dev:mock-cloudflare": "node --env-file=packages/worker/.env ./wrangler-env.ts dev --local --config packages/mock-servers/cloudflare/wrangler.jsonc", "deploy": "node --input-type=module -e \"if (['preview','test'].includes(process.env.CLOUDFLARE_ENV ?? '')) { await import('./tools/prepare-e2e-env.ts'); }\" && npm run build && node --env-file=packages/worker/.env ./wrangler-env.ts deploy --outdir .wrangler/sentry-bundle --upload-source-maps", "sentry:upload-sourcemaps": "node tools/sentry-upload-sourcemaps.ts", - "build:mcp-apps": "esbuild packages/worker/client/mcp-apps/kody-ui-utils.ts --bundle --format=esm --target=es2022 --outfile=packages/worker/public/mcp-apps/kody-ui-utils.js --platform=browser --banner:js='/* eslint-disable no-undef */'", + "build:mcp-apps": "esbuild packages/worker/client/mcp-apps/kody-ui-utils.ts --bundle --format=esm --target=es2022 --outfile=packages/worker/public/mcp-apps/kody-ui-utils.js --platform=browser --banner:js='/* eslint-disable no-undef */' && oxfmt packages/worker/public/mcp-apps/kody-ui-utils.css", "build:client:web": "esbuild packages/worker/client/entry.tsx --bundle --format=esm --target=es2022 --outdir=packages/worker/public --entry-names=client-entry --chunk-names=assets/[name] --asset-names=assets/[name] --jsx=automatic --jsx-import-source=remix/component", "build:client": "npm run build:client:web && npm run build:mcp-apps", "build": "nx run worker:build", diff --git a/packages/worker/public/mcp-apps/kody-ui-utils.css b/packages/worker/public/mcp-apps/kody-ui-utils.css index 74106bae9a..f9791fb19a 100644 --- a/packages/worker/public/mcp-apps/kody-ui-utils.css +++ b/packages/worker/public/mcp-apps/kody-ui-utils.css @@ -1,176 +1,172 @@ /* packages/worker/client/mcp-apps/kody-ui-utils.css */ :root { - color-scheme: light dark; - --font-body: - ui-sans-serif, - system-ui, - sans-serif; - --font-mono: - ui-monospace, - SFMono-Regular, - Menlo, - Monaco, - Consolas, - monospace; - --spacing-1: 0.25rem; - --spacing-2: 0.5rem; - --spacing-3: 0.75rem; - --spacing-4: 1rem; - --spacing-6: 1.5rem; - --radius-2: 0.5rem; - --radius-3: 0.75rem; - --shadow-1: 0 1px 2px rgb(15 23 42 / 0.08); - --color-bg: #ffffff; - --color-surface: #f8fafc; - --color-fg: #0f172a; - --color-muted: #475569; - --color-border: #dbe2ea; - --color-accent: #2563eb; - --color-accent-contrast: #ffffff; - --color-code-bg: rgb(15 23 42 / 0.06); + color-scheme: light dark; + --font-body: ui-sans-serif, system-ui, sans-serif; + --font-mono: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace; + --spacing-1: 0.25rem; + --spacing-2: 0.5rem; + --spacing-3: 0.75rem; + --spacing-4: 1rem; + --spacing-6: 1.5rem; + --radius-2: 0.5rem; + --radius-3: 0.75rem; + --shadow-1: 0 1px 2px rgb(15 23 42 / 0.08); + --color-bg: #ffffff; + --color-surface: #f8fafc; + --color-fg: #0f172a; + --color-muted: #475569; + --color-border: #dbe2ea; + --color-accent: #2563eb; + --color-accent-contrast: #ffffff; + --color-code-bg: rgb(15 23 42 / 0.06); } @media (prefers-color-scheme: dark) { - :root { - --color-bg: #0f172a; - --color-surface: #162033; - --color-fg: #e5eef8; - --color-muted: #a5b4c7; - --color-border: #2a3950; - --color-accent: #60a5fa; - --color-accent-contrast: #0f172a; - --color-code-bg: rgb(148 163 184 / 0.16); - } + :root { + --color-bg: #0f172a; + --color-surface: #162033; + --color-fg: #e5eef8; + --color-muted: #a5b4c7; + --color-border: #2a3950; + --color-accent: #60a5fa; + --color-accent-contrast: #0f172a; + --color-code-bg: rgb(148 163 184 / 0.16); + } } :where(html) { - min-height: 100%; - background: var(--color-bg); - color: var(--color-fg); + min-height: 100%; + background: var(--color-bg); + color: var(--color-fg); } :where(body) { - min-height: 100%; - margin: 0; - padding: var(--spacing-4); - background: var(--color-bg); - color: var(--color-fg); - font: 400 14px/1.5 var(--font-body); + min-height: 100%; + margin: 0; + padding: var(--spacing-4); + background: var(--color-bg); + color: var(--color-fg); + font: 400 14px/1.5 var(--font-body); } -:where(body[data-kody-runtime=javascript]) { - padding: 0; +:where(body[data-kody-runtime='javascript']) { + padding: 0; } :where(*, *::before, *::after) { - box-sizing: border-box; + box-sizing: border-box; } :where(a) { - color: var(--color-accent); + color: var(--color-accent); } :where(p, ul, ol, dl, pre, table, blockquote, fieldset) { - margin: 0 0 var(--spacing-4); + margin: 0 0 var(--spacing-4); } :where(h1, h2, h3, h4, h5, h6) { - margin: 0 0 var(--spacing-3); - line-height: 1.2; + margin: 0 0 var(--spacing-3); + line-height: 1.2; } :where(h1) { - font-size: 1.875rem; + font-size: 1.875rem; } :where(h2) { - font-size: 1.5rem; + font-size: 1.5rem; } :where(h3) { - font-size: 1.25rem; + font-size: 1.25rem; } :where(ul, ol) { - padding-left: 1.5rem; + padding-left: 1.5rem; } :where(hr) { - border: 0; - border-top: 1px solid var(--color-border); - margin: var(--spacing-6) 0; + border: 0; + border-top: 1px solid var(--color-border); + margin: var(--spacing-6) 0; } :where(code, kbd, samp) { - font-family: var(--font-mono); - font-size: 0.95em; + font-family: var(--font-mono); + font-size: 0.95em; } :where(code) { - background: var(--color-code-bg); - border-radius: 0.375rem; - padding: 0.1rem 0.35rem; + background: var(--color-code-bg); + border-radius: 0.375rem; + padding: 0.1rem 0.35rem; } :where(pre) { - overflow-x: auto; - padding: var(--spacing-3); - background: var(--color-code-bg); - border: 1px solid var(--color-border); - border-radius: var(--radius-2); + overflow-x: auto; + padding: var(--spacing-3); + background: var(--color-code-bg); + border: 1px solid var(--color-border); + border-radius: var(--radius-2); } :where(pre code) { - background: transparent; - padding: 0; + background: transparent; + padding: 0; } :where(form) { - display: grid; - gap: var(--spacing-4); + display: grid; + gap: var(--spacing-4); } :where(label) { - display: grid; - gap: var(--spacing-2); - font-weight: 600; + display: grid; + gap: var(--spacing-2); + font-weight: 600; } :where(input, button, textarea, select) { - font: inherit; + font: inherit; } -:where(input:not([type=checkbox]):not([type=radio]), textarea, select) { - width: 100%; - padding: var(--spacing-2) var(--spacing-3); - background: var(--color-surface); - color: var(--color-fg); - border: 1px solid var(--color-border); - border-radius: var(--radius-2); - box-shadow: inset 0 1px 2px rgb(15 23 42 / 0.04); +:where(input:not([type='checkbox']):not([type='radio']), textarea, select) { + width: 100%; + padding: var(--spacing-2) var(--spacing-3); + background: var(--color-surface); + color: var(--color-fg); + border: 1px solid var(--color-border); + border-radius: var(--radius-2); + box-shadow: inset 0 1px 2px rgb(15 23 42 / 0.04); } :where(textarea) { - min-height: 7rem; - resize: vertical; + min-height: 7rem; + resize: vertical; } -:where(input[type=checkbox], input[type=radio]) { - accent-color: var(--color-accent); +:where(input[type='checkbox'], input[type='radio']) { + accent-color: var(--color-accent); } :where(button) { - width: fit-content; - padding: var(--spacing-2) var(--spacing-4); - background: var(--color-accent); - color: var(--color-accent-contrast); - border: 1px solid transparent; - border-radius: var(--radius-2); - box-shadow: var(--shadow-1); - cursor: pointer; + width: fit-content; + padding: var(--spacing-2) var(--spacing-4); + background: var(--color-accent); + color: var(--color-accent-contrast); + border: 1px solid transparent; + border-radius: var(--radius-2); + box-shadow: var(--shadow-1); + cursor: pointer; } :where(button:disabled, input:disabled, textarea:disabled, select:disabled) { - opacity: 0.7; - cursor: not-allowed; -} -:where(button:focus-visible, input:focus-visible, textarea:focus-visible, select:focus-visible) { - outline: 2px solid var(--color-accent); - outline-offset: 2px; + opacity: 0.7; + cursor: not-allowed; +} +:where( + button:focus-visible, + input:focus-visible, + textarea:focus-visible, + select:focus-visible +) { + outline: 2px solid var(--color-accent); + outline-offset: 2px; } :where(table) { - width: 100%; - border-collapse: collapse; + width: 100%; + border-collapse: collapse; } :where(th, td) { - padding: var(--spacing-2) var(--spacing-3); - border: 1px solid var(--color-border); - text-align: left; + padding: var(--spacing-2) var(--spacing-3); + border: 1px solid var(--color-border); + text-align: left; } :where(th) { - background: var(--color-surface); + background: var(--color-surface); } :where(blockquote) { - margin-left: 0; - padding-left: var(--spacing-4); - border-left: 3px solid var(--color-border); - color: var(--color-muted); + margin-left: 0; + padding-left: var(--spacing-4); + border-left: 3px solid var(--color-border); + color: var(--color-muted); } :where([data-generated-ui-root]) { - min-height: 100%; + min-height: 100%; } diff --git a/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts b/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts index a16fcb2e33..873fa124e1 100644 --- a/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts +++ b/packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts @@ -583,11 +583,7 @@ test('mcp server searches capabilities', async () => { (m) => m.type === 'capability' && m.name === 'ui_save_app', ) expect(saveApp?.domain).toBe('apps') - expect(saveApp?.requiredInputFields).toEqual([ - 'title', - 'description', - 'code', - ]) + expect(saveApp?.requiredInputFields).toEqual(['title', 'description', 'code']) expect(saveApp?.readOnly).toBeUndefined() const textOutput = @@ -1528,16 +1524,15 @@ test('generated ui sessions support secret storage, execute-time resolution, and if (!secretSetExecuteResponse.ok) { throw new Error(await secretSetExecuteResponse.text()) } - const secretSetExecutePayload = - (await secretSetExecuteResponse.json()) as { - ok?: boolean - result?: { - savedAccessToken?: Record - savedRefreshToken?: Record - leakedValue?: string - } - logs?: Array + const secretSetExecutePayload = (await secretSetExecuteResponse.json()) as { + ok?: boolean + result?: { + savedAccessToken?: Record + savedRefreshToken?: Record + leakedValue?: string } + logs?: Array + } expect(secretSetExecutePayload.ok).toBe(true) expect(secretSetExecutePayload.result?.savedAccessToken).toEqual({ name: 'spotifyAccessToken', diff --git a/packages/worker/src/mcp/run-codemode-registry.node.test.ts b/packages/worker/src/mcp/run-codemode-registry.node.test.ts index e82f89e419..eb09c28231 100644 --- a/packages/worker/src/mcp/run-codemode-registry.node.test.ts +++ b/packages/worker/src/mcp/run-codemode-registry.node.test.ts @@ -191,7 +191,10 @@ test('buildCodemodeFns tracks values that crossed secret-marked capability input const trackedSecretValues: Array = [] const getRegistrySpy = vi - .spyOn(await import('#mcp/capabilities/registry.ts'), 'getCapabilityRegistryForContext') + .spyOn( + await import('#mcp/capabilities/registry.ts'), + 'getCapabilityRegistryForContext', + ) .mockResolvedValue({ capabilityDomains: [], capabilityDomainDescriptionsByName: {} as Record, @@ -279,21 +282,17 @@ test('buildCodemodeFns tracks values that crossed secret-marked capability input } as Awaited>) try { - const codemode = await buildCodemodeFns( - env, - callerContext, - { - trackSecretInputValue(value) { - trackedSecretValues.push(value) - }, + const codemode = await buildCodemodeFns(env, callerContext, { + trackSecretInputValue(value) { + trackedSecretValues.push(value) }, - ) - const result = await codemode.secret_set({ + }) + const result = await codemode.secret_set({ name: 'spotifyAccessToken', value: 'fresh-access-token', }) - expect(result).toEqual({ + expect(result).toEqual({ name: 'spotifyAccessToken', }) expect(trackedSecretValues).toEqual(['fresh-access-token']) diff --git a/packages/worker/src/mcp/ui-artifacts-repo.ts b/packages/worker/src/mcp/ui-artifacts-repo.ts index 653237da67..c1e2cbdcd2 100644 --- a/packages/worker/src/mcp/ui-artifacts-repo.ts +++ b/packages/worker/src/mcp/ui-artifacts-repo.ts @@ -92,11 +92,7 @@ export async function updateUiArtifact( updates: Partial< Pick< UiArtifactRow, - | 'title' - | 'description' - | 'code' - | 'runtime' - | 'parameters' + 'title' | 'description' | 'code' | 'runtime' | 'parameters' > >, ): Promise { From d6562c7cdeca632c9de383da221cca1dea6e7251 Mon Sep 17 00:00:00 2001 From: "Kent C. Dodds" Date: Sun, 29 Mar 2026 18:08:10 -0700 Subject: [PATCH 5/5] Fix cyclic execute result redaction. Sanitize secret-bearing keys as well as values in execute results, and make the redaction walker cycle-safe so self-referential results cannot recurse forever. Made-with: Cursor --- .../mcp/run-codemode-registry.node.test.ts | 173 +++++++++++++++++- .../worker/src/mcp/run-codemode-registry.ts | 35 +++- 2 files changed, 197 insertions(+), 11 deletions(-) diff --git a/packages/worker/src/mcp/run-codemode-registry.node.test.ts b/packages/worker/src/mcp/run-codemode-registry.node.test.ts index eb09c28231..4a3b5ab98c 100644 --- a/packages/worker/src/mcp/run-codemode-registry.node.test.ts +++ b/packages/worker/src/mcp/run-codemode-registry.node.test.ts @@ -1,7 +1,10 @@ import { expect, test, vi } from 'vitest' import { type getCapabilityRegistryForContext } from '#mcp/capabilities/registry.ts' import { createMcpCallerContext } from '#mcp/context.ts' -import { buildCodemodeFns } from './run-codemode-registry.ts' +import { + buildCodemodeFns, + runCodemodeWithRegistry, +} from './run-codemode-registry.ts' import * as secretService from '#mcp/secrets/service.ts' test('buildCodemodeFns resolves annotated home capability secret placeholders', async () => { @@ -300,3 +303,171 @@ test('buildCodemodeFns tracks values that crossed secret-marked capability input getRegistrySpy.mockRestore() } }) + +test('runCodemodeWithRegistry redacts secret keys and survives cyclic results', async () => { + const env = {} as Env + const callerContext = createMcpCallerContext({ + baseUrl: 'https://heykody.dev', + user: { userId: 'user-123' }, + }) + const getRegistrySpy = vi + .spyOn( + await import('#mcp/capabilities/registry.ts'), + 'getCapabilityRegistryForContext', + ) + .mockResolvedValue({ + capabilityDomains: [], + capabilityDomainDescriptionsByName: {} as Record, + capabilityHandlers: {}, + capabilityList: [ + { + name: 'secret_set', + domain: 'secrets', + description: 'Store a secret.', + keywords: [], + readOnly: false, + idempotent: false, + destructive: false, + inputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + value: { type: 'string', 'x-kody-secret': true }, + }, + required: ['name', 'value'], + }, + outputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + async handler(args: Record) { + return { + name: args.name, + } + }, + }, + ], + capabilityMap: { + secret_set: { + name: 'secret_set', + domain: 'secrets', + description: 'Store a secret.', + keywords: [], + readOnly: false, + idempotent: false, + destructive: false, + inputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + value: { type: 'string', 'x-kody-secret': true }, + }, + required: ['name', 'value'], + }, + outputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + async handler(args: Record) { + return { + name: args.name, + } + }, + }, + }, + capabilitySpecs: {}, + capabilityToolDescriptors: { + secret_set: { + description: 'Store a secret.', + inputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + value: { type: 'string', 'x-kody-secret': true }, + }, + required: ['name', 'value'], + }, + outputSchema: { + type: 'object', + properties: { + name: { type: 'string' }, + }, + }, + }, + }, + } as Awaited>) + const createExecuteExecutorSpy = vi + .spyOn(await import('#mcp/executor.ts'), 'createExecuteExecutor') + .mockReturnValue({ + async execute(_wrapped, providers) { + const provider = providers[0] as { + fns: Record Promise> + } + await provider.fns.secret_set({ + name: 'spotifyAccessToken', + value: 'fresh-access-token', + }) + + const objectResult: Record = { + 'fresh-access-token key': 'fresh-access-token value', + } + objectResult.self = objectResult + + const arrayResult: Array = ['fresh-access-token array'] + arrayResult.push(arrayResult) + + const errorResult = new Error('fresh-access-token error') as Error & { + cause?: unknown + } + errorResult.cause = errorResult + + return { + result: { + objectResult, + arrayResult, + errorResult, + }, + logs: ['fresh-access-token log'], + } + }, + } as never) + + try { + const result = await runCodemodeWithRegistry( + env, + callerContext, + `async () => { + await codemode.secret_set({ + name: 'spotifyAccessToken', + value: 'fresh-access-token', + }) + return null + }`, + ) + const sanitized = result.result as { + objectResult: Record + arrayResult: Array + errorResult: Error & { cause?: unknown } + } + + expect(sanitized.objectResult['[REDACTED SECRET] key']).toBe( + '[REDACTED SECRET] value', + ) + expect(sanitized.objectResult.self).toBe(sanitized.objectResult) + + expect(sanitized.arrayResult[0]).toBe('[REDACTED SECRET] array') + expect(sanitized.arrayResult[1]).toBe(sanitized.arrayResult) + + expect(sanitized.errorResult.message).toBe('[REDACTED SECRET] error') + expect(sanitized.errorResult.cause).toBe(sanitized.errorResult) + + expect(result.logs).toEqual(['[REDACTED SECRET] log']) + } finally { + createExecuteExecutorSpy.mockRestore() + getRegistrySpy.mockRestore() + } +}) diff --git a/packages/worker/src/mcp/run-codemode-registry.ts b/packages/worker/src/mcp/run-codemode-registry.ts index a9d1fbc58c..32f6fab6e3 100644 --- a/packages/worker/src/mcp/run-codemode-registry.ts +++ b/packages/worker/src/mcp/run-codemode-registry.ts @@ -313,18 +313,23 @@ function visitSecretInputValue( function redactUnknownSecretValues( value: unknown, secretValues: ReadonlySet, + seen = new WeakMap(), ): unknown { if (secretValues.size === 0) return value if (typeof value === 'string') { return redactSecretValuesInString(value, secretValues) } if (value instanceof Error) { + const existing = seen.get(value) + if (existing) return existing const next = new Error( redactSecretValuesInString(value.message, secretValues), - value.cause !== undefined - ? { cause: redactUnknownSecretValues(value.cause, secretValues) } - : undefined, + value.cause !== undefined ? { cause: undefined } : undefined, ) + seen.set(value, next) + if (value.cause !== undefined) { + next.cause = redactUnknownSecretValues(value.cause, secretValues, seen) + } next.name = value.name if (value.stack) { next.stack = redactSecretValuesInString(value.stack, secretValues) @@ -332,15 +337,25 @@ function redactUnknownSecretValues( return next } if (Array.isArray(value)) { - return value.map((entry) => redactUnknownSecretValues(entry, secretValues)) + const existing = seen.get(value) + if (existing) return existing + const next: Array = [] + seen.set(value, next) + for (const entry of value) { + next.push(redactUnknownSecretValues(entry, secretValues, seen)) + } + return next } if (isRecord(value)) { - return Object.fromEntries( - Object.entries(value).map(([key, entry]) => [ - key, - redactUnknownSecretValues(entry, secretValues), - ]), - ) + const existing = seen.get(value) + if (existing) return existing + const next: Record = {} + seen.set(value, next) + for (const [key, entry] of Object.entries(value)) { + const redactedKey = redactSecretValuesInString(key, secretValues) + next[redactedKey] = redactUnknownSecretValues(entry, secretValues, seen) + } + return next } return value }