From 5cc4e4ef20545ccdb550a1fa0c99894d3b73d8a8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 2 May 2026 03:58:55 +0000 Subject: [PATCH 1/2] Load secret values in account editor Co-authored-by: Kent C. Dodds --- e2e/connect-secret.spec.ts | 2 +- .../worker/client/routes/account-secrets.tsx | 7 +++-- .../app/handlers/account-secrets.node.test.ts | 17 +++++++++-- .../src/app/handlers/account-secrets.ts | 28 ++++++++++++++++--- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/e2e/connect-secret.spec.ts b/e2e/connect-secret.spec.ts index 68bdfe6a48..eaf35d40e8 100644 --- a/e2e/connect-secret.spec.ts +++ b/e2e/connect-secret.spec.ts @@ -49,6 +49,6 @@ test('connect secret shows editable name and scope and saves the edited name', a await expect(page.getByLabel('Description')).toHaveValue(description) await expect( page.getByPlaceholder('Enter the secret value').first(), - ).toHaveValue('') + ).toHaveValue(secretValue) await expect(page.getByPlaceholder('saved package id')).toHaveValue(packageId) }) diff --git a/packages/worker/client/routes/account-secrets.tsx b/packages/worker/client/routes/account-secrets.tsx index 45740ee595..fd7f9f2c89 100644 --- a/packages/worker/client/routes/account-secrets.tsx +++ b/packages/worker/client/routes/account-secrets.tsx @@ -66,7 +66,9 @@ type SecretListItem = { ttlMs: number | null } -type SecretDetail = SecretListItem +type SecretDetail = SecretListItem & { + value: string +} type AccountSecretsPayload = { ok: true @@ -176,7 +178,7 @@ function createEditorStateFromSecret(secret: SecretDetail): EditorState { scope: secret.scope, appId: secret.appId ?? '', description: secret.description, - value: '', + value: secret.value, allowedHosts: allowedHosts.length > 0 ? allowedHosts : [''], allowedCapabilities: allowedCapabilities.length > 0 ? allowedCapabilities : [''], @@ -510,6 +512,7 @@ export function AccountSecretsRoute(handle: Handle) { createEditorStateFromSecret(selectedSecret), capabilityPrefill, ) + revealState = 'revealed' return } editorState = applyCapabilityPrefill( diff --git a/packages/worker/src/app/handlers/account-secrets.node.test.ts b/packages/worker/src/app/handlers/account-secrets.node.test.ts index 66678da761..4176032ea6 100644 --- a/packages/worker/src/app/handlers/account-secrets.node.test.ts +++ b/packages/worker/src/app/handlers/account-secrets.node.test.ts @@ -35,7 +35,7 @@ const mockModule = vi.hoisted(() => ({ listSavedPackagesByUserId: vi.fn(async () => []), listSecrets: vi.fn(async () => []), listAppSecretsByAppIds: vi.fn(async () => []), - resolveSecret: vi.fn(async () => null), + resolveSecret: vi.fn(async () => ({ found: false, value: null })), deleteSecret: vi.fn(async () => false), setSecretAllowedCapabilities: vi.fn(async () => undefined), setSecretAllowedPackages: vi.fn(async () => undefined), @@ -564,7 +564,7 @@ test('package approval approve deduplicates allowed package ids', async () => { ) }) -test('GET /account/secrets.json does not include value in selectedSecret', async () => { +test('GET /account/secrets.json includes decrypted value in selectedSecret', async () => { mockModule.listSecrets.mockResolvedValueOnce([ { name: 'myApiKey', @@ -581,6 +581,10 @@ test('GET /account/secrets.json does not include value in selectedSecret', async ]) mockModule.listSavedPackagesByUserId.mockResolvedValueOnce([]) mockModule.listAppSecretsByAppIds.mockResolvedValueOnce(new Map()) + mockModule.resolveSecret.mockResolvedValueOnce({ + found: true, + value: 'the-actual-secret-value', + }) const handler = createAccountSecretsApiHandler(createEnv()) const response = await handler.handler({ @@ -596,7 +600,14 @@ test('GET /account/secrets.json does not include value in selectedSecret', async expect(payload.ok).toBe(true) expect(payload.selectedSecret).toBeDefined() expect(payload.selectedSecret.name).toBe('myApiKey') - expect(payload.selectedSecret).not.toHaveProperty('value') + expect(payload.selectedSecret.value).toBe('the-actual-secret-value') + expect(mockModule.resolveSecret).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'myApiKey', + scope: 'user', + storageContext: { appId: null, sessionId: null }, + }), + ) }) test('POST /account/secrets/reveal without password returns 401', async () => { diff --git a/packages/worker/src/app/handlers/account-secrets.ts b/packages/worker/src/app/handlers/account-secrets.ts index 6233601bca..612167b476 100644 --- a/packages/worker/src/app/handlers/account-secrets.ts +++ b/packages/worker/src/app/handlers/account-secrets.ts @@ -67,7 +67,9 @@ type AccountSecretListItem = { ttlMs: number | null } -type AccountSecretDetail = AccountSecretListItem +type AccountSecretDetail = AccountSecretListItem & { + value: string +} type SecretApprovalView = { name: string @@ -735,7 +737,9 @@ async function buildAccountSecretsPayload(input: { packageApps, }) const selectedSecret = input.selectedSecretId - ? resolveAccountSecretDetail({ + ? await resolveAccountSecretDetail({ + env: input.env, + userId: input.user.mcpUser.userId, secretId: input.selectedSecretId, secrets, }) @@ -896,7 +900,9 @@ async function resolveSecretApprovalView(input: { } satisfies SecretApprovalView } -function resolveAccountSecretDetail(input: { +async function resolveAccountSecretDetail(input: { + env: Env + userId: string secretId: string secrets: Array }) { @@ -904,7 +910,21 @@ function resolveAccountSecretDetail(input: { if (!parsed) return null const selected = input.secrets.find((secret) => secret.id === input.secretId) - return selected ?? null + if (!selected) return null + + const resolved = await resolveSecret({ + env: input.env, + userId: input.userId, + name: parsed.name, + scope: parsed.scope, + storageContext: getSecretContextForAccountSecret(parsed), + }) + if (!resolved?.found || resolved.value == null) return null + + return { + ...selected, + value: resolved.value, + } satisfies AccountSecretDetail } function toAccountSecretListItem( From 7e1f251af97ff8338e1dcbcfff948ea9fb70b0bc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 2 May 2026 04:43:20 +0000 Subject: [PATCH 2/2] Preserve secret reveal reauth boundary Co-authored-by: Kent C. Dodds --- e2e/connect-secret.spec.ts | 2 +- .../worker/client/routes/account-secrets.tsx | 7 ++--- .../app/handlers/account-secrets.node.test.ts | 17 +++-------- .../src/app/handlers/account-secrets.ts | 28 +++---------------- 4 files changed, 11 insertions(+), 43 deletions(-) diff --git a/e2e/connect-secret.spec.ts b/e2e/connect-secret.spec.ts index eaf35d40e8..68bdfe6a48 100644 --- a/e2e/connect-secret.spec.ts +++ b/e2e/connect-secret.spec.ts @@ -49,6 +49,6 @@ test('connect secret shows editable name and scope and saves the edited name', a await expect(page.getByLabel('Description')).toHaveValue(description) await expect( page.getByPlaceholder('Enter the secret value').first(), - ).toHaveValue(secretValue) + ).toHaveValue('') await expect(page.getByPlaceholder('saved package id')).toHaveValue(packageId) }) diff --git a/packages/worker/client/routes/account-secrets.tsx b/packages/worker/client/routes/account-secrets.tsx index fd7f9f2c89..45740ee595 100644 --- a/packages/worker/client/routes/account-secrets.tsx +++ b/packages/worker/client/routes/account-secrets.tsx @@ -66,9 +66,7 @@ type SecretListItem = { ttlMs: number | null } -type SecretDetail = SecretListItem & { - value: string -} +type SecretDetail = SecretListItem type AccountSecretsPayload = { ok: true @@ -178,7 +176,7 @@ function createEditorStateFromSecret(secret: SecretDetail): EditorState { scope: secret.scope, appId: secret.appId ?? '', description: secret.description, - value: secret.value, + value: '', allowedHosts: allowedHosts.length > 0 ? allowedHosts : [''], allowedCapabilities: allowedCapabilities.length > 0 ? allowedCapabilities : [''], @@ -512,7 +510,6 @@ export function AccountSecretsRoute(handle: Handle) { createEditorStateFromSecret(selectedSecret), capabilityPrefill, ) - revealState = 'revealed' return } editorState = applyCapabilityPrefill( diff --git a/packages/worker/src/app/handlers/account-secrets.node.test.ts b/packages/worker/src/app/handlers/account-secrets.node.test.ts index 4176032ea6..3a01737ecc 100644 --- a/packages/worker/src/app/handlers/account-secrets.node.test.ts +++ b/packages/worker/src/app/handlers/account-secrets.node.test.ts @@ -564,7 +564,7 @@ test('package approval approve deduplicates allowed package ids', async () => { ) }) -test('GET /account/secrets.json includes decrypted value in selectedSecret', async () => { +test('GET /account/secrets.json keeps selected metadata without decrypted value', async () => { mockModule.listSecrets.mockResolvedValueOnce([ { name: 'myApiKey', @@ -581,10 +581,6 @@ test('GET /account/secrets.json includes decrypted value in selectedSecret', asy ]) mockModule.listSavedPackagesByUserId.mockResolvedValueOnce([]) mockModule.listAppSecretsByAppIds.mockResolvedValueOnce(new Map()) - mockModule.resolveSecret.mockResolvedValueOnce({ - found: true, - value: 'the-actual-secret-value', - }) const handler = createAccountSecretsApiHandler(createEnv()) const response = await handler.handler({ @@ -600,14 +596,9 @@ test('GET /account/secrets.json includes decrypted value in selectedSecret', asy expect(payload.ok).toBe(true) expect(payload.selectedSecret).toBeDefined() expect(payload.selectedSecret.name).toBe('myApiKey') - expect(payload.selectedSecret.value).toBe('the-actual-secret-value') - expect(mockModule.resolveSecret).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'myApiKey', - scope: 'user', - storageContext: { appId: null, sessionId: null }, - }), - ) + expect(payload.selectedSecret.description).toBe('API key') + expect(payload.selectedSecret).not.toHaveProperty('value') + expect(mockModule.resolveSecret).not.toHaveBeenCalled() }) test('POST /account/secrets/reveal without password returns 401', async () => { diff --git a/packages/worker/src/app/handlers/account-secrets.ts b/packages/worker/src/app/handlers/account-secrets.ts index 612167b476..6233601bca 100644 --- a/packages/worker/src/app/handlers/account-secrets.ts +++ b/packages/worker/src/app/handlers/account-secrets.ts @@ -67,9 +67,7 @@ type AccountSecretListItem = { ttlMs: number | null } -type AccountSecretDetail = AccountSecretListItem & { - value: string -} +type AccountSecretDetail = AccountSecretListItem type SecretApprovalView = { name: string @@ -737,9 +735,7 @@ async function buildAccountSecretsPayload(input: { packageApps, }) const selectedSecret = input.selectedSecretId - ? await resolveAccountSecretDetail({ - env: input.env, - userId: input.user.mcpUser.userId, + ? resolveAccountSecretDetail({ secretId: input.selectedSecretId, secrets, }) @@ -900,9 +896,7 @@ async function resolveSecretApprovalView(input: { } satisfies SecretApprovalView } -async function resolveAccountSecretDetail(input: { - env: Env - userId: string +function resolveAccountSecretDetail(input: { secretId: string secrets: Array }) { @@ -910,21 +904,7 @@ async function resolveAccountSecretDetail(input: { if (!parsed) return null const selected = input.secrets.find((secret) => secret.id === input.secretId) - if (!selected) return null - - const resolved = await resolveSecret({ - env: input.env, - userId: input.userId, - name: parsed.name, - scope: parsed.scope, - storageContext: getSecretContextForAccountSecret(parsed), - }) - if (!resolved?.found || resolved.value == null) return null - - return { - ...selected, - value: resolved.value, - } satisfies AccountSecretDetail + return selected ?? null } function toAccountSecretListItem(