From f83946148885f5863c9eef6e98c71191101dc84b Mon Sep 17 00:00:00 2001 From: Philip Chmalts Date: Wed, 30 Sep 2026 18:45:11 -0700 Subject: [PATCH 1/3] fix: report credential-safe key validation failures --- src/lib/api.test.ts | 174 +++++++++++++++++++---- src/lib/api.ts | 98 ++++++++++--- src/lib/app.tsx | 28 ++-- src/lib/connection-error.test.ts | 52 +++++++ src/lib/connection-error.ts | 58 ++++++++ src/lib/steps/authenticate.ts | 11 +- src/lib/steps/connect-web.ts | 18 ++- test/app-connection.test.tsx | 189 +++++++++++++++++++++++++ test/steps/authenticate-errors.test.ts | 51 +++++++ 9 files changed, 610 insertions(+), 69 deletions(-) create mode 100644 src/lib/connection-error.test.ts create mode 100644 src/lib/connection-error.ts create mode 100644 test/app-connection.test.tsx create mode 100644 test/steps/authenticate-errors.test.ts diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts index bb55ea2..8d16b7d 100644 --- a/src/lib/api.test.ts +++ b/src/lib/api.test.ts @@ -1,24 +1,19 @@ -import { beforeEach, expect, test, vi } from 'vitest' +import { + SeamHttpApiError, + SeamHttpInvalidTokenError, + SeamHttpUnauthorizedError, +} from '@seamapi/http' +import { afterEach, expect, test, vi } from 'vitest' -const { get, post } = vi.hoisted(() => ({ - get: vi.fn(), - post: vi.fn(), -})) +import { + classifyKeyValidationError, + exchangeWizardInferenceToken, + getWorkspaceForApiKey, +} from './api.js' -vi.mock('@seamapi/http', () => ({ - isSeamHttpApiError: () => false, - isSeamHttpUnauthorizedError: () => false, - SeamHttpInvalidTokenError: class extends Error {}, - SeamHttpWorkspaces: class { - get = get - client = { post } - }, -})) - -import { exchangeWizardInferenceToken, getWorkspaceForApiKey } from './api.js' - -beforeEach(() => vi.clearAllMocks()) +afterEach(() => vi.unstubAllGlobals()) +// Fixed noncredential fixtures; all requests stop at the fetch boundary. test('uses the workspace SDK and its raw client', async () => { const workspace = { workspace_id: 'workspace-1', @@ -33,18 +28,143 @@ test('uses the workspace SDK and its raw client', async () => { embed_customer_portal: null, device_categories: ['locks'], } - get.mockResolvedValue(workspace) - post.mockResolvedValue({ - data: { - wizard_session: { token: 'token', expires_at: 'tomorrow', onboarding }, - }, - }) - - await expect(getWorkspaceForApiKey('seam_key')).resolves.toBe(workspace) + const requests: Request[] = [] + vi.stubGlobal( + 'fetch', + vi.fn(async (request: Request) => { + requests.push(request) + return Response.json( + request.url.endsWith('/session') + ? { + wizard_session: { + token: 'token', + expires_at: 'tomorrow', + onboarding, + }, + } + : { workspace }, + ) + }), + ) + await expect(getWorkspaceForApiKey('seam_key')).resolves.toEqual(workspace) await expect(exchangeWizardInferenceToken('seam_key')).resolves.toEqual({ token: 'token', expires_at: 'tomorrow', onboarding, }) - expect(post).toHaveBeenCalledWith('/seam/wizard/v1/session', {}) + expect(new URL(requests[1]?.url ?? '').pathname).toBe( + '/seam/wizard/v1/session', + ) + expect(await requests[1]?.json()).toEqual({}) +}) + +test('malformed and wrong token types fail locally without claiming 401', async () => { + const fetch = vi.fn() + vi.stubGlobal('fetch', fetch) + for (const token of ['not-a-key', 'seam_pk_not-a-key']) { + await expect(getWorkspaceForApiKey(token)).rejects.toMatchObject({ + category: 'invalid_token_format', + statusCode: null, + message: expect.stringContaining('not a supported Seam API key'), + }) + } + expect(fetch).not.toHaveBeenCalled() +}) + +test('an actual unauthorized response reports 401', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => Response.json({}, { status: 401 })), + ) + await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ + category: 'unauthorized', + statusCode: 401, + }) +}) + +test('a 5xx response reports its status without the API message', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => + Response.json( + { error: { type: 'internal_error', message: 'secret-marker' } }, + { status: 503 }, + ), + ), + ) + await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ + category: 'api_error', + statusCode: 503, + message: 'The Seam API returned 503. Please try again in a moment.', + }) +}) + +test.each([ + [ + new SeamHttpInvalidTokenError('secret-marker'), + 'invalid_token_format', + null, + ], + [new SeamHttpUnauthorizedError('secret-marker'), 'unauthorized', 401], + [ + new SeamHttpApiError( + { + type: 'internal_error', + message: 'secret-marker', + data: { key: 'secret-marker' }, + }, + 500, + 'secret-marker', + ), + 'api_error', + 500, + ], + [ + Object.assign(new Error('secret-marker'), { + code: 'ERR_NETWORK', + config: { url: 'secret-marker' }, + }), + 'transport_error', + null, + ], + [ + Object.assign(new Error('secret-marker'), { code: 'ETIMEDOUT' }), + 'transport_error', + null, + ], + [new Error('secret-marker'), 'unknown', null], + [new TypeError('secret-marker'), 'unknown', null], + ['secret-marker', 'unknown', null], +])('normalizes SDK failures safely (%#)', (error, category, statusCode) => { + const result = classifyKeyValidationError(error) + expect(result).toMatchObject({ category, statusCode }) + expect(`${result.stack} ${JSON.stringify(result)}`).not.toContain( + 'secret-marker', + ) + expect(result).not.toHaveProperty('cause') +}) + +test('fetch transport failures survive the SDK boundary as transport errors', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + throw new TypeError('Failed to fetch secret-marker') + }), + ) + await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ + category: 'transport_error', + statusCode: null, + }) +}) + +test('non-JSON HTTP failures retain their known status', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('secret-marker', { status: 502 })), + ) + await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ + category: 'api_error', + statusCode: 502, + message: 'The Seam API returned 502. Please try again in a moment.', + }) }) diff --git a/src/lib/api.ts b/src/lib/api.ts index 6e3e2e4..05476dd 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -19,6 +19,87 @@ export type SeamWorkspace = Pick< export class ApiKeyError extends Error {} +export type KeyValidationFailure = + | 'invalid_token_format' + | 'unauthorized' + | 'api_error' + | 'transport_error' + | 'unknown' + +// Only fixed messages and a known HTTP status survive the SDK boundary. Never +// retain the original error: it may contain the key, headers or request URL. +export class KeyValidationError extends ApiKeyError { + constructor( + readonly category: KeyValidationFailure, + readonly statusCode: number | null = null, + ) { + const messages: Record = { + invalid_token_format: + 'That value is not a supported Seam API key. Copy the full API key, including the seam_ prefix.', + unauthorized: + 'The Seam API rejected that key (401). Check that the key is valid and active.', + api_error: + statusCode == null + ? 'The Seam API returned an error. Please try again in a moment.' + : `The Seam API returned ${statusCode}. Please try again in a moment.`, + transport_error: + 'Could not reach the Seam API. Check your connection and try again.', + unknown: + 'An unexpected error occurred while verifying the key. Please try again.', + } + super(messages[category]) + } +} + +export function classifyKeyValidationError(error: unknown): KeyValidationError { + if (error instanceof SeamHttpInvalidTokenError) { + return new KeyValidationError('invalid_token_format') + } + if (isSeamHttpUnauthorizedError(error)) { + return new KeyValidationError('unauthorized', 401) + } + if (isSeamHttpApiError(error)) { + return new KeyValidationError( + 'api_error', + knownHttpStatus(error.statusCode), + ) + } + // A non-JSON HTTP failure can remain an Axios error instead of becoming a + // SeamHttpApiError. Its response status is still evidence of an HTTP failure. + if ( + error instanceof Error && + 'isAxiosError' in error && + error.isAxiosError === true && + 'response' in error && + typeof error.response === 'object' && + error.response != null && + 'status' in error.response + ) { + const status = knownHttpStatus(error.response.status) + if (status != null) return new KeyValidationError('api_error', status) + } + // Axios' fetch adapter identifies transport failures by code. An arbitrary + // Error or TypeError is not evidence of a network failure. + if ( + error instanceof Error && + 'code' in error && + typeof error.code === 'string' && + ['ERR_NETWORK', 'ECONNABORTED', 'ETIMEDOUT'].includes(error.code) + ) { + return new KeyValidationError('transport_error') + } + return new KeyValidationError('unknown') +} + +function knownHttpStatus(status: unknown): number | null { + return typeof status === 'number' && + Number.isInteger(status) && + status >= 100 && + status <= 599 + ? status + : null +} + const getApi = (apiKey: string): SeamHttpWorkspaces => new SeamHttpWorkspaces({ apiKey, endpoint: getApiBaseUrl() }) @@ -28,22 +109,7 @@ export async function getWorkspaceForApiKey( try { return await getApi(apiKey).get() } catch (error) { - if ( - error instanceof SeamHttpInvalidTokenError || - isSeamHttpUnauthorizedError(error) - ) { - throw new ApiKeyError( - 'That key was rejected (401). Make sure you copied the full key, including the seam_ prefix.', - ) - } - if (isSeamHttpApiError(error)) { - throw new ApiKeyError( - `The Seam API returned ${error.statusCode}. Please try again in a moment.`, - ) - } - throw new ApiKeyError( - 'Could not reach the Seam API. Check your network connection and try again.', - ) + throw classifyKeyValidationError(error) } } diff --git a/src/lib/app.tsx b/src/lib/app.tsx index 4a82370..35320eb 100644 --- a/src/lib/app.tsx +++ b/src/lib/app.tsx @@ -19,13 +19,13 @@ import { trackScreen, } from './analytics.js' import { - ApiKeyError, exchangeWizardInferenceToken, getInferenceBaseUrl, looksLikeSeamApiKey, type SeamWorkspace, type WizardInferenceSession, } from './api.js' +import { describeConnectionFailure } from './connection-error.js' import { ensureProjectEnvConventions, ENV_EXAMPLE_SYMLINK_REFUSAL_MESSAGE, @@ -649,13 +649,10 @@ export function App({ ) } catch (error) { if (!cancelled) { - const message = - error instanceof Error - ? error.message - : 'Browser connection failed.' + const { message, properties } = describeConnectionFailure(error) track('wizard_connect_failed', { method: 'browser', - reason: message, + ...properties, }) setPhase({ t: 'error', message }) } @@ -665,10 +662,7 @@ export function App({ if (cancelled) return setPhase({ t: 'error', - message: - error instanceof Error - ? error.message - : 'The wizard hit an unexpected error.', + message: describeConnectionFailure(error).message, }) }) return () => { @@ -695,20 +689,17 @@ export function App({ ) } catch (error) { if (cancelled) return - const message = - error instanceof ApiKeyError - ? error.message - : "Couldn't verify the key." + const { message, properties } = describeConnectionFailure(error) track('wizard_connect_failed', { method: 'paste', - reason: message, + ...properties, attempt: attemptRef.current, gave_up: attemptRef.current >= MAX_ATTEMPTS, }) if (attemptRef.current >= MAX_ATTEMPTS) { setPhase({ t: 'error', - message: 'Too many attempts. Re-run with a valid key.', + message: `Too many attempts. ${message} Re-run the wizard to try again.`, }) } else { setPasteError(message) @@ -721,10 +712,7 @@ export function App({ if (cancelled) return setPhase({ t: 'error', - message: - error instanceof Error - ? error.message - : 'The wizard hit an unexpected error.', + message: describeConnectionFailure(error).message, }) }) return () => { diff --git a/src/lib/connection-error.test.ts b/src/lib/connection-error.test.ts new file mode 100644 index 0000000..4c11596 --- /dev/null +++ b/src/lib/connection-error.test.ts @@ -0,0 +1,52 @@ +import { expect, test } from 'vitest' + +import { + ConnectionError, + describeConnectionFailure, +} from './connection-error.js' + +test('unexpected exceptions do not disclose messages or invent a stage', () => { + const error = Object.assign( + new Error('secret-marker https://private.invalid'), + { + api_key: 'secret-marker', + fingerprint: 'secret-marker', + }, + ) + expect(describeConnectionFailure(error)).toEqual({ + message: 'An unexpected error occurred while connecting. Please try again.', + properties: { + reason: 'unknown', + failure_stage: 'unknown', + http_status: null, + }, + }) + expect(JSON.stringify(describeConnectionFailure(error))).not.toContain( + 'secret-marker', + ) +}) + +test('callback and saving failures report their known stage', () => { + expect( + describeConnectionFailure( + new ConnectionError('browser_callback', 'timeout'), + ), + ).toMatchObject({ + message: expect.stringContaining('Timed out'), + properties: { + reason: 'timeout', + failure_stage: 'browser_callback', + http_status: null, + }, + }) + expect( + describeConnectionFailure(new ConnectionError('env_write')), + ).toMatchObject({ + message: expect.stringContaining('key was verified'), + properties: { + reason: 'unknown', + failure_stage: 'env_write', + http_status: null, + }, + }) +}) diff --git a/src/lib/connection-error.ts b/src/lib/connection-error.ts new file mode 100644 index 0000000..37ad7ac --- /dev/null +++ b/src/lib/connection-error.ts @@ -0,0 +1,58 @@ +import { KeyValidationError } from './api.js' + +export type ConnectionFailureStage = + 'browser_callback' | 'key_validation' | 'env_write' | 'unknown' + +// Safe context for failures outside validation; do not retain their cause. +export class ConnectionError extends Error { + constructor( + readonly stage: 'browser_callback' | 'env_write', + readonly category: 'unknown' | 'timeout' = 'unknown', + ) { + super( + stage === 'env_write' + ? 'The key was verified, but the wizard could not save it to the project. Check file permissions and try again.' + : category === 'timeout' + ? 'Timed out waiting for the browser to return a key. Please try again.' + : 'The browser handoff could not complete. Please try again or paste your API key.', + ) + } +} + +export function describeConnectionFailure(error: unknown): { + message: string + properties: { + reason: string + failure_stage: ConnectionFailureStage + http_status: number | null + } +} { + if (error instanceof KeyValidationError) { + return { + message: error.message, + properties: { + reason: error.category, + failure_stage: 'key_validation', + http_status: error.statusCode, + }, + } + } + if (error instanceof ConnectionError) { + return { + message: error.message, + properties: { + reason: error.category, + failure_stage: error.stage, + http_status: null, + }, + } + } + return { + message: 'An unexpected error occurred while connecting. Please try again.', + properties: { + reason: 'unknown', + failure_stage: 'unknown', + http_status: null, + }, + } +} diff --git a/src/lib/steps/authenticate.ts b/src/lib/steps/authenticate.ts index 83914fb..5efae7a 100644 --- a/src/lib/steps/authenticate.ts +++ b/src/lib/steps/authenticate.ts @@ -1,5 +1,6 @@ import { getAuth } from 'lib/adapter.js' import { getWorkspaceForApiKey, type SeamWorkspace } from 'lib/api.js' +import { ConnectionError } from 'lib/connection-error.js' import { findExistingApiKey, type ProjectEnvResult, @@ -60,7 +61,15 @@ export async function verifyAndSaveKey( ): Promise { const trimmed = apiKey.trim() const workspace = await getWorkspaceForApiKey(trimmed) - return { workspace, api_key: trimmed, env: saveProjectApiKey(root, trimmed) } + try { + return { + workspace, + api_key: trimmed, + env: saveProjectApiKey(root, trimmed), + } + } catch { + throw new ConnectionError('env_write') + } } export function saveVerifiedKey( diff --git a/src/lib/steps/connect-web.ts b/src/lib/steps/connect-web.ts index 007a26e..682e594 100644 --- a/src/lib/steps/connect-web.ts +++ b/src/lib/steps/connect-web.ts @@ -8,6 +8,7 @@ import { getWorkspaceForApiKey, type SeamWorkspace, } from 'lib/api.js' +import { ConnectionError } from 'lib/connection-error.js' import { type ProjectEnvResult, saveProjectApiKey } from 'lib/env-file.js' // The dashboard "wizard" page mints a key and posts it back to the local @@ -108,16 +109,23 @@ export async function connectViaWeb( const timeout = setTimeout(() => { server.close() - reject(new Error('Timed out waiting for the browser.')) + reject(new ConnectionError('browser_callback', 'timeout')) }, CALLBACK_TIMEOUT_MS) timeout.unref() + }).catch((error: unknown) => { + if (error instanceof ConnectionError) throw error + throw new ConnectionError('browser_callback') }) const workspace = await getWorkspaceForApiKey(payload.api_key) - return { - workspace, - api_key: payload.api_key, - env: saveProjectApiKey(root, payload.api_key), + try { + return { + workspace, + api_key: payload.api_key, + env: saveProjectApiKey(root, payload.api_key), + } + } catch { + throw new ConnectionError('env_write') } } diff --git a/test/app-connection.test.tsx b/test/app-connection.test.tsx new file mode 100644 index 0000000..bc8346b --- /dev/null +++ b/test/app-connection.test.tsx @@ -0,0 +1,189 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { request } from 'node:http' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { gunzipSync } from 'node:zlib' + +import { render } from 'ink-testing-library' +import { afterEach, beforeEach, expect, test, vi } from 'vitest' + +import { createMemoryAdapter, resetAdapter, setAdapter } from 'lib/adapter.js' +import { + flushAnalytics, + resetAnalytics, + startAnalytics, +} from 'lib/analytics.js' +import { App } from 'lib/app.js' + +const { openBrowser } = vi.hoisted(() => ({ + openBrowser: vi.fn(async (_url: string) => undefined), +})) +vi.mock('open', () => ({ default: openBrowser })) + +let sdkRequests = 0 +let root = '' +let cleanup: (() => void) | undefined +let posted: Array<{ event: string; properties: Record }> = [] + +beforeEach(async () => { + root = mkdtempSync(join(tmpdir(), 'wizard-connect-')) + setAdapter(createMemoryAdapter()) + vi.stubEnv('SEAM_API_KEY', '') + vi.stubEnv('SEAM_WIZARD_POSTHOG_KEY', 'phc_test_project') + posted = [] + const captured = posted + sdkRequests = 0 + openBrowser.mockClear() + vi.stubGlobal( + 'fetch', + vi.fn(async (input: Request | string, init?: RequestInit) => { + if (typeof input !== 'string') { + sdkRequests++ + return Response.json( + { error: { type: 'unauthorized', message: 'secret-marker' } }, + { status: 401 }, + ) + } + const body = JSON.parse( + gunzipSync(init?.body as Uint8Array).toString('utf8'), + ) as { batch: typeof posted } + captured.push(...body.batch) + return new Response('{}', { status: 200 }) + }), + ) + await startAnalytics({ command: 'seam wizard' }) +}) + +afterEach(async () => { + cleanup?.() + cleanup = undefined + await flushAnalytics() + process.exitCode = 0 + resetAnalytics() + resetAdapter() + vi.unstubAllEnvs() + vi.unstubAllGlobals() + rmSync(root, { recursive: true, force: true }) +}) + +test('paste retries and give-up preserve 401 and send only safe failure metadata', async () => { + const reports: string[][] = [] + const { stdin, lastFrame, unmount } = render( + reports.push(lines)} />, + ) + cleanup = unmount + // Wait for Ink to attach its input listener before each input transition. + await vi.waitFor(() => expect(lastFrame()).toContain('Press any key')) + await new Promise((resolve) => setTimeout(resolve, 50)) + stdin.write('x') + await vi.waitFor(() => + expect(lastFrame()).toContain('How do you want to connect'), + ) + await new Promise((resolve) => setTimeout(resolve, 100)) + stdin.write('\u001b[B') + await new Promise((resolve) => setTimeout(resolve, 50)) + stdin.write('\r') + await vi.waitFor(() => + expect(lastFrame()).toContain('Paste your Seam API key'), + ) + for (let attempt = 1; attempt <= 3; attempt++) { + await new Promise((resolve) => setTimeout(resolve, 50)) + stdin.write('seam_fixture_secret-marker') + await new Promise((resolve) => setTimeout(resolve, 50)) + stdin.write('\r') + await vi.waitFor(() => expect(sdkRequests).toBe(attempt)) + if (attempt < 3) { + await vi.waitFor(() => { + expect(lastFrame()).toContain('Paste your Seam API key') + expect(lastFrame()).toContain('rejected that key (401)') + }) + } else { + await vi.waitFor(() => + expect(reports.flat().join(' ')).toContain('Too many attempts'), + ) + expect(reports.flat().join(' ')).toContain('rejected that key (401)') + } + } + unmount() + cleanup = undefined + await flushAnalytics() + const failures = posted.filter( + ({ event }) => event === 'wizard_connect_failed', + ) + expect(failures).toHaveLength(3) + failures.forEach(({ properties }, index) => { + expect(properties).toMatchObject({ + method: 'paste', + reason: 'unauthorized', + failure_stage: 'key_validation', + http_status: 401, + attempt: index + 1, + gave_up: index === 2, + }) + }) + expect(JSON.stringify(posted)).not.toContain('secret-marker') + expect(JSON.stringify(posted)).not.toContain(root) + expect(lastFrame()).not.toContain('secret-marker') +}, 15000) + +test('browser key receipt followed by local validation failure reports the validation stage safely', async () => { + const reports: string[][] = [] + const { stdin, lastFrame, unmount } = render( + reports.push(lines)} />, + ) + cleanup = unmount + await new Promise((resolve) => setTimeout(resolve, 50)) + stdin.write('x') + await vi.waitFor(() => + expect(lastFrame()).toContain('How do you want to connect'), + ) + await new Promise((resolve) => setTimeout(resolve, 100)) + stdin.write('\r') + await vi.waitFor(() => expect(openBrowser.mock.calls).toHaveLength(1)) + const url = new URL(openBrowser.mock.calls[0]?.[0] ?? '') + await new Promise((resolve, reject) => { + const callback = request( + { + hostname: '127.0.0.1', + port: url.searchParams.get('cli_port') ?? '', + method: 'POST', + path: '/', + headers: { 'content-type': 'application/json' }, + }, + (response) => { + response.resume() + response.on('end', () => resolve()) + }, + ) + callback.on('error', reject) + callback.end( + JSON.stringify({ + state: url.searchParams.get('cli_state'), + api_key: 'seam_pk_secret-marker', + }), + ) + }) + await vi.waitFor(() => + expect(reports.flat().join(' ')).toContain('not a supported Seam API key'), + ) + unmount() + cleanup = undefined + await flushAnalytics() + expect(sdkRequests).toBe(0) + expect( + posted.filter(({ event }) => event === 'wizard_browser_key_received'), + ).toHaveLength(1) + expect( + posted.find(({ event }) => event === 'wizard_connect_failed')?.properties, + ).toMatchObject({ + method: 'browser', + reason: 'invalid_token_format', + failure_stage: 'key_validation', + http_status: null, + }) + expect(JSON.stringify(posted)).not.toContain('secret-marker') + expect(JSON.stringify(posted)).not.toContain( + url.searchParams.get('cli_state'), + ) + expect(JSON.stringify(posted)).not.toContain(url.href) +}, 10000) diff --git a/test/steps/authenticate-errors.test.ts b/test/steps/authenticate-errors.test.ts new file mode 100644 index 0000000..95a7c42 --- /dev/null +++ b/test/steps/authenticate-errors.test.ts @@ -0,0 +1,51 @@ +import { mkdirSync, mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterEach, expect, test, vi } from 'vitest' + +import { createMemoryAdapter, resetAdapter, setAdapter } from 'lib/adapter.js' +import { describeConnectionFailure } from 'lib/connection-error.js' +import { verifyAndSaveKey } from 'lib/steps/authenticate.js' + +let root = '' +afterEach(() => { + resetAdapter() + vi.unstubAllGlobals() + rmSync(root, { recursive: true, force: true }) +}) + +test('saving failure after validation is distinct from a rejected key and excludes disk errors', async () => { + setAdapter(createMemoryAdapter()) + root = mkdtempSync(join(tmpdir(), 'wizard-auth-errors-')) + mkdirSync(join(root, '.env')) + vi.stubGlobal( + 'fetch', + vi.fn(async () => + Response.json({ + workspace: { + workspace_id: 'workspace-1', + name: 'Test', + is_sandbox: true, + }, + }), + ), + ) + let failure: unknown + try { + await verifyAndSaveKey(root, 'seam_fixture_secret-marker') + } catch (error) { + failure = error + } + const result = describeConnectionFailure(failure) + expect(result).toMatchObject({ + message: expect.stringContaining('key was verified'), + properties: { + reason: 'unknown', + failure_stage: 'env_write', + http_status: null, + }, + }) + expect(JSON.stringify(result)).not.toContain(root) + expect(JSON.stringify(result)).not.toContain('secret-marker') +}) From 58e9f3ee1dd99f9f828917b26cbcfa7c6341ddb1 Mon Sep 17 00:00:00 2001 From: Philip Chmalts Date: Wed, 30 Sep 2026 18:56:29 -0700 Subject: [PATCH 2/3] fix: preserve native connection errors for safe reporting --- src/lib/api.test.ts | 98 ++++++------------- src/lib/api.ts | 86 +---------------- src/lib/connection-error.test.ts | 88 ++++++++++++++++- src/lib/connection-error.ts | 158 ++++++++++++++++++++++++------- src/lib/steps/authenticate.ts | 6 +- src/lib/steps/connect-web.ts | 17 ++-- 6 files changed, 251 insertions(+), 202 deletions(-) diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts index 8d16b7d..9e2083a 100644 --- a/src/lib/api.test.ts +++ b/src/lib/api.test.ts @@ -1,15 +1,7 @@ -import { - SeamHttpApiError, - SeamHttpInvalidTokenError, - SeamHttpUnauthorizedError, -} from '@seamapi/http' +import { SeamHttpInvalidTokenError } from '@seamapi/http' import { afterEach, expect, test, vi } from 'vitest' -import { - classifyKeyValidationError, - exchangeWizardInferenceToken, - getWorkspaceForApiKey, -} from './api.js' +import { exchangeWizardInferenceToken, getWorkspaceForApiKey } from './api.js' afterEach(() => vi.unstubAllGlobals()) @@ -58,15 +50,13 @@ test('uses the workspace SDK and its raw client', async () => { expect(await requests[1]?.json()).toEqual({}) }) -test('malformed and wrong token types fail locally without claiming 401', async () => { +test('malformed and wrong token types preserve the native local error', async () => { const fetch = vi.fn() vi.stubGlobal('fetch', fetch) for (const token of ['not-a-key', 'seam_pk_not-a-key']) { - await expect(getWorkspaceForApiKey(token)).rejects.toMatchObject({ - category: 'invalid_token_format', - statusCode: null, - message: expect.stringContaining('not a supported Seam API key'), - }) + await expect(getWorkspaceForApiKey(token)).rejects.toBeInstanceOf( + SeamHttpInvalidTokenError, + ) } expect(fetch).not.toHaveBeenCalled() }) @@ -77,12 +67,12 @@ test('an actual unauthorized response reports 401', async () => { vi.fn(async () => Response.json({}, { status: 401 })), ) await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ - category: 'unauthorized', + name: 'SeamHttpUnauthorizedError', statusCode: 401, }) }) -test('a 5xx response reports its status without the API message', async () => { +test('a 5xx response preserves the native SDK exception and details', async () => { vi.stubGlobal( 'fetch', vi.fn(async () => @@ -93,58 +83,13 @@ test('a 5xx response reports its status without the API message', async () => { ), ) await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ - category: 'api_error', + name: 'SeamHttpApiError', statusCode: 503, - message: 'The Seam API returned 503. Please try again in a moment.', + message: 'secret-marker', }) }) -test.each([ - [ - new SeamHttpInvalidTokenError('secret-marker'), - 'invalid_token_format', - null, - ], - [new SeamHttpUnauthorizedError('secret-marker'), 'unauthorized', 401], - [ - new SeamHttpApiError( - { - type: 'internal_error', - message: 'secret-marker', - data: { key: 'secret-marker' }, - }, - 500, - 'secret-marker', - ), - 'api_error', - 500, - ], - [ - Object.assign(new Error('secret-marker'), { - code: 'ERR_NETWORK', - config: { url: 'secret-marker' }, - }), - 'transport_error', - null, - ], - [ - Object.assign(new Error('secret-marker'), { code: 'ETIMEDOUT' }), - 'transport_error', - null, - ], - [new Error('secret-marker'), 'unknown', null], - [new TypeError('secret-marker'), 'unknown', null], - ['secret-marker', 'unknown', null], -])('normalizes SDK failures safely (%#)', (error, category, statusCode) => { - const result = classifyKeyValidationError(error) - expect(result).toMatchObject({ category, statusCode }) - expect(`${result.stack} ${JSON.stringify(result)}`).not.toContain( - 'secret-marker', - ) - expect(result).not.toHaveProperty('cause') -}) - -test('fetch transport failures survive the SDK boundary as transport errors', async () => { +test('fetch transport failures preserve the SDK transport exception', async () => { vi.stubGlobal( 'fetch', vi.fn(async () => { @@ -152,8 +97,7 @@ test('fetch transport failures survive the SDK boundary as transport errors', as }), ) await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ - category: 'transport_error', - statusCode: null, + code: 'ERR_NETWORK', }) }) @@ -163,8 +107,20 @@ test('non-JSON HTTP failures retain their known status', async () => { vi.fn(async () => new Response('secret-marker', { status: 502 })), ) await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ - category: 'api_error', - statusCode: 502, - message: 'The Seam API returned 502. Please try again in a moment.', + response: { status: 502 }, + }) +}) + +test('the SDK transport exception retains its original cause', async () => { + const original = new TypeError('secret-marker') + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + throw original + }), + ) + await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ + isAxiosError: true, + cause: original, }) }) diff --git a/src/lib/api.ts b/src/lib/api.ts index 05476dd..b2756da 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -1,12 +1,11 @@ import { isSeamHttpApiError, - isSeamHttpUnauthorizedError, - SeamHttpInvalidTokenError, SeamHttpWorkspaces, type Workspace, } from '@seamapi/http' import { getAuth } from 'lib/adapter.js' +import { markConnectionFailure } from 'lib/connection-error.js' export function getApiBaseUrl(): string { return getAuth().endpoint.replace(/\/+$/, '') @@ -19,87 +18,6 @@ export type SeamWorkspace = Pick< export class ApiKeyError extends Error {} -export type KeyValidationFailure = - | 'invalid_token_format' - | 'unauthorized' - | 'api_error' - | 'transport_error' - | 'unknown' - -// Only fixed messages and a known HTTP status survive the SDK boundary. Never -// retain the original error: it may contain the key, headers or request URL. -export class KeyValidationError extends ApiKeyError { - constructor( - readonly category: KeyValidationFailure, - readonly statusCode: number | null = null, - ) { - const messages: Record = { - invalid_token_format: - 'That value is not a supported Seam API key. Copy the full API key, including the seam_ prefix.', - unauthorized: - 'The Seam API rejected that key (401). Check that the key is valid and active.', - api_error: - statusCode == null - ? 'The Seam API returned an error. Please try again in a moment.' - : `The Seam API returned ${statusCode}. Please try again in a moment.`, - transport_error: - 'Could not reach the Seam API. Check your connection and try again.', - unknown: - 'An unexpected error occurred while verifying the key. Please try again.', - } - super(messages[category]) - } -} - -export function classifyKeyValidationError(error: unknown): KeyValidationError { - if (error instanceof SeamHttpInvalidTokenError) { - return new KeyValidationError('invalid_token_format') - } - if (isSeamHttpUnauthorizedError(error)) { - return new KeyValidationError('unauthorized', 401) - } - if (isSeamHttpApiError(error)) { - return new KeyValidationError( - 'api_error', - knownHttpStatus(error.statusCode), - ) - } - // A non-JSON HTTP failure can remain an Axios error instead of becoming a - // SeamHttpApiError. Its response status is still evidence of an HTTP failure. - if ( - error instanceof Error && - 'isAxiosError' in error && - error.isAxiosError === true && - 'response' in error && - typeof error.response === 'object' && - error.response != null && - 'status' in error.response - ) { - const status = knownHttpStatus(error.response.status) - if (status != null) return new KeyValidationError('api_error', status) - } - // Axios' fetch adapter identifies transport failures by code. An arbitrary - // Error or TypeError is not evidence of a network failure. - if ( - error instanceof Error && - 'code' in error && - typeof error.code === 'string' && - ['ERR_NETWORK', 'ECONNABORTED', 'ETIMEDOUT'].includes(error.code) - ) { - return new KeyValidationError('transport_error') - } - return new KeyValidationError('unknown') -} - -function knownHttpStatus(status: unknown): number | null { - return typeof status === 'number' && - Number.isInteger(status) && - status >= 100 && - status <= 599 - ? status - : null -} - const getApi = (apiKey: string): SeamHttpWorkspaces => new SeamHttpWorkspaces({ apiKey, endpoint: getApiBaseUrl() }) @@ -109,7 +27,7 @@ export async function getWorkspaceForApiKey( try { return await getApi(apiKey).get() } catch (error) { - throw classifyKeyValidationError(error) + throw markConnectionFailure(error, 'key_validation') } } diff --git a/src/lib/connection-error.test.ts b/src/lib/connection-error.test.ts index 4c11596..ea4fe25 100644 --- a/src/lib/connection-error.test.ts +++ b/src/lib/connection-error.test.ts @@ -1,8 +1,14 @@ +import { + SeamHttpApiError, + SeamHttpInvalidTokenError, + SeamHttpUnauthorizedError, +} from '@seamapi/http' import { expect, test } from 'vitest' import { - ConnectionError, + classifyKeyValidationError, describeConnectionFailure, + markConnectionFailure, } from './connection-error.js' test('unexpected exceptions do not disclose messages or invent a stage', () => { @@ -29,7 +35,11 @@ test('unexpected exceptions do not disclose messages or invent a stage', () => { test('callback and saving failures report their known stage', () => { expect( describeConnectionFailure( - new ConnectionError('browser_callback', 'timeout'), + markConnectionFailure( + new Error('secret-marker'), + 'browser_callback', + 'timeout', + ), ), ).toMatchObject({ message: expect.stringContaining('Timed out'), @@ -40,7 +50,9 @@ test('callback and saving failures report their known stage', () => { }, }) expect( - describeConnectionFailure(new ConnectionError('env_write')), + describeConnectionFailure( + markConnectionFailure(new Error('secret-marker'), 'env_write'), + ), ).toMatchObject({ message: expect.stringContaining('key was verified'), properties: { @@ -50,3 +62,73 @@ test('callback and saving failures report their known stage', () => { }, }) }) +test.each([ + [ + new SeamHttpInvalidTokenError('secret-marker'), + 'invalid_token_format', + null, + ], + [new SeamHttpUnauthorizedError('secret-marker'), 'unauthorized', 401], + [ + new SeamHttpApiError( + { + type: 'internal_error', + message: 'secret-marker', + data: { key: 'secret-marker' }, + }, + 500, + 'secret-marker', + ), + 'api_error', + 500, + ], + [ + Object.assign(new Error('secret-marker'), { + code: 'ERR_NETWORK', + config: { url: 'secret-marker' }, + }), + 'transport_error', + null, + ], + [ + Object.assign(new Error('secret-marker'), { code: 'ETIMEDOUT' }), + 'transport_error', + null, + ], + [new Error('secret-marker'), 'unknown', null], + [new TypeError('secret-marker'), 'unknown', null], + ['secret-marker', 'unknown', null], +])('normalizes SDK failures safely (%#)', (error, category, statusCode) => { + const result = classifyKeyValidationError(error) + expect(result).toMatchObject({ category, statusCode }) + expect(JSON.stringify(result)).not.toContain('secret-marker') + expect(result).not.toHaveProperty('cause') +}) + +test('stage annotation preserves the original exception, cause and own properties', () => { + const cause = new Error('cause-secret-marker') + const error = new SeamHttpApiError( + { + type: 'internal_error', + message: 'secret-marker', + data: { key: 'secret-marker' }, + }, + 503, + 'secret-marker', + ) + error.cause = cause + const originalProperties = Object.getOwnPropertyDescriptors(error) + expect(markConnectionFailure(error, 'key_validation')).toBe(error) + expect(Object.getOwnPropertyDescriptors(error)).toEqual(originalProperties) + const report = describeConnectionFailure(error) + expect(report).toMatchObject({ + message: 'The Seam API returned 503. Please try again in a moment.', + properties: { + reason: 'api_error', + failure_stage: 'key_validation', + http_status: 503, + }, + }) + expect(JSON.stringify(report)).not.toContain('secret-marker') + expect(error.cause).toBe(cause) +}) diff --git a/src/lib/connection-error.ts b/src/lib/connection-error.ts index 37ad7ac..15093dd 100644 --- a/src/lib/connection-error.ts +++ b/src/lib/connection-error.ts @@ -1,58 +1,146 @@ -import { KeyValidationError } from './api.js' +import { + isSeamHttpApiError, + isSeamHttpUnauthorizedError, + SeamHttpInvalidTokenError, +} from '@seamapi/http' export type ConnectionFailureStage = 'browser_callback' | 'key_validation' | 'env_write' | 'unknown' -// Safe context for failures outside validation; do not retain their cause. -export class ConnectionError extends Error { - constructor( - readonly stage: 'browser_callback' | 'env_write', - readonly category: 'unknown' | 'timeout' = 'unknown', +export type KeyValidationCategory = + | 'invalid_token_format' + | 'unauthorized' + | 'api_error' + | 'transport_error' + | 'unknown' + +interface KeyValidationFailure { + category: KeyValidationCategory + statusCode: number | null + message: string +} + +// Associate a stage with the original exception without mutating, wrapping or +// serializing it. Native type, stack, cause and SDK details remain available to +// callers; only the safe descriptor below is used for display and analytics. +const contexts = new WeakMap< + object, + { stage: ConnectionFailureStage; category: 'unknown' | 'timeout' } +>() + +export function markConnectionFailure( + error: T, + stage: ConnectionFailureStage, + category: 'unknown' | 'timeout' = 'unknown', +): T { + if (typeof error === 'object' && error != null && !contexts.has(error)) { + contexts.set(error, { stage, category }) + } + return error +} + +function keyValidationFailure( + category: KeyValidationCategory, + statusCode: number | null = null, +): KeyValidationFailure { + const messages: Record = { + invalid_token_format: + 'That value is not a supported Seam API key. Copy the full API key, including the seam_ prefix.', + unauthorized: + 'The Seam API rejected that key (401). Check that the key is valid and active.', + api_error: + statusCode == null + ? 'The Seam API returned an error. Please try again in a moment.' + : `The Seam API returned ${statusCode}. Please try again in a moment.`, + transport_error: + 'Could not reach the Seam API. Check your connection and try again.', + unknown: + 'An unexpected error occurred while verifying the key. Please try again.', + } + return { category, statusCode, message: messages[category] } +} + +export function classifyKeyValidationError( + error: unknown, +): KeyValidationFailure { + if (error instanceof SeamHttpInvalidTokenError) { + return keyValidationFailure('invalid_token_format') + } + if (isSeamHttpUnauthorizedError(error)) { + return keyValidationFailure('unauthorized', 401) + } + if (isSeamHttpApiError(error)) { + return keyValidationFailure('api_error', knownHttpStatus(error.statusCode)) + } + // A non-JSON HTTP failure can remain an Axios error instead of becoming a + // SeamHttpApiError. Its response status is still evidence of an HTTP failure. + if ( + error instanceof Error && + 'isAxiosError' in error && + error.isAxiosError === true && + 'response' in error && + typeof error.response === 'object' && + error.response != null && + 'status' in error.response ) { - super( - stage === 'env_write' - ? 'The key was verified, but the wizard could not save it to the project. Check file permissions and try again.' - : category === 'timeout' - ? 'Timed out waiting for the browser to return a key. Please try again.' - : 'The browser handoff could not complete. Please try again or paste your API key.', - ) + const status = knownHttpStatus(error.response.status) + if (status != null) return keyValidationFailure('api_error', status) + } + // Axios' fetch adapter identifies transport failures by code. An arbitrary + // Error or TypeError is not evidence of a network failure. + if ( + error instanceof Error && + 'code' in error && + typeof error.code === 'string' && + ['ERR_NETWORK', 'ECONNABORTED', 'ETIMEDOUT'].includes(error.code) + ) { + return keyValidationFailure('transport_error') } + return keyValidationFailure('unknown') +} + +function knownHttpStatus(status: unknown): number | null { + return typeof status === 'number' && + Number.isInteger(status) && + status >= 100 && + status <= 599 + ? status + : null } export function describeConnectionFailure(error: unknown): { message: string properties: { - reason: string + reason: KeyValidationCategory | 'timeout' failure_stage: ConnectionFailureStage http_status: number | null } } { - if (error instanceof KeyValidationError) { + const context = + typeof error === 'object' && error != null ? contexts.get(error) : undefined + const stage = context?.stage ?? 'unknown' + const category = context?.category ?? 'unknown' + if (stage === 'key_validation') { + const failure = classifyKeyValidationError(error) return { - message: error.message, + message: failure.message, properties: { - reason: error.category, - failure_stage: 'key_validation', - http_status: error.statusCode, - }, - } - } - if (error instanceof ConnectionError) { - return { - message: error.message, - properties: { - reason: error.category, - failure_stage: error.stage, - http_status: null, + reason: failure.category, + failure_stage: stage, + http_status: failure.statusCode, }, } } + const message = + stage === 'env_write' + ? 'The key was verified, but the wizard could not save it to the project. Check file permissions and try again.' + : stage === 'browser_callback' + ? category === 'timeout' + ? 'Timed out waiting for the browser to return a key. Please try again.' + : 'The browser handoff could not complete. Please try again or paste your API key.' + : 'An unexpected error occurred while connecting. Please try again.' return { - message: 'An unexpected error occurred while connecting. Please try again.', - properties: { - reason: 'unknown', - failure_stage: 'unknown', - http_status: null, - }, + message, + properties: { reason: category, failure_stage: stage, http_status: null }, } } diff --git a/src/lib/steps/authenticate.ts b/src/lib/steps/authenticate.ts index 5efae7a..aa70345 100644 --- a/src/lib/steps/authenticate.ts +++ b/src/lib/steps/authenticate.ts @@ -1,6 +1,6 @@ import { getAuth } from 'lib/adapter.js' import { getWorkspaceForApiKey, type SeamWorkspace } from 'lib/api.js' -import { ConnectionError } from 'lib/connection-error.js' +import { markConnectionFailure } from 'lib/connection-error.js' import { findExistingApiKey, type ProjectEnvResult, @@ -67,8 +67,8 @@ export async function verifyAndSaveKey( api_key: trimmed, env: saveProjectApiKey(root, trimmed), } - } catch { - throw new ConnectionError('env_write') + } catch (error) { + throw markConnectionFailure(error, 'env_write') } } diff --git a/src/lib/steps/connect-web.ts b/src/lib/steps/connect-web.ts index 682e594..fd46e7e 100644 --- a/src/lib/steps/connect-web.ts +++ b/src/lib/steps/connect-web.ts @@ -8,7 +8,7 @@ import { getWorkspaceForApiKey, type SeamWorkspace, } from 'lib/api.js' -import { ConnectionError } from 'lib/connection-error.js' +import { markConnectionFailure } from 'lib/connection-error.js' import { type ProjectEnvResult, saveProjectApiKey } from 'lib/env-file.js' // The dashboard "wizard" page mints a key and posts it back to the local @@ -109,12 +109,17 @@ export async function connectViaWeb( const timeout = setTimeout(() => { server.close() - reject(new ConnectionError('browser_callback', 'timeout')) + reject( + markConnectionFailure( + new Error('Timed out waiting for the browser.'), + 'browser_callback', + 'timeout', + ), + ) }, CALLBACK_TIMEOUT_MS) timeout.unref() }).catch((error: unknown) => { - if (error instanceof ConnectionError) throw error - throw new ConnectionError('browser_callback') + throw markConnectionFailure(error, 'browser_callback') }) const workspace = await getWorkspaceForApiKey(payload.api_key) @@ -124,8 +129,8 @@ export async function connectViaWeb( api_key: payload.api_key, env: saveProjectApiKey(root, payload.api_key), } - } catch { - throw new ConnectionError('env_write') + } catch (error) { + throw markConnectionFailure(error, 'env_write') } } From 29e7ddd1948b893d15b4d9a3617ec2b7fdfac227 Mon Sep 17 00:00:00 2001 From: Philip Chmalts Date: Wed, 30 Sep 2026 19:11:23 -0700 Subject: [PATCH 3/3] fix: classify status-only unauthorized responses --- src/lib/api.test.ts | 27 +++++++++++++++++---------- src/lib/connection-error.test.ts | 8 ++++++++ src/lib/connection-error.ts | 1 + 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts index 9e2083a..577dc5d 100644 --- a/src/lib/api.test.ts +++ b/src/lib/api.test.ts @@ -61,16 +61,23 @@ test('malformed and wrong token types preserve the native local error', async () expect(fetch).not.toHaveBeenCalled() }) -test('an actual unauthorized response reports 401', async () => { - vi.stubGlobal( - 'fetch', - vi.fn(async () => Response.json({}, { status: 401 })), - ) - await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ - name: 'SeamHttpUnauthorizedError', - statusCode: 401, - }) -}) +test.each(['json', 'text'])( + 'an actual unauthorized %s response reports 401', + async (format) => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => + format === 'json' + ? Response.json({}, { status: 401 }) + : new Response('secret-marker', { status: 401 }), + ), + ) + await expect(getWorkspaceForApiKey('seam_key')).rejects.toMatchObject({ + name: 'SeamHttpUnauthorizedError', + statusCode: 401, + }) + }, +) test('a 5xx response preserves the native SDK exception and details', async () => { vi.stubGlobal( diff --git a/src/lib/connection-error.test.ts b/src/lib/connection-error.test.ts index ea4fe25..2166538 100644 --- a/src/lib/connection-error.test.ts +++ b/src/lib/connection-error.test.ts @@ -95,6 +95,14 @@ test.each([ 'transport_error', null, ], + [ + Object.assign(new Error('secret-marker'), { + isAxiosError: true, + response: { status: 401, data: 'secret-marker' }, + }), + 'unauthorized', + 401, + ], [new Error('secret-marker'), 'unknown', null], [new TypeError('secret-marker'), 'unknown', null], ['secret-marker', 'unknown', null], diff --git a/src/lib/connection-error.ts b/src/lib/connection-error.ts index 15093dd..893e9d3 100644 --- a/src/lib/connection-error.ts +++ b/src/lib/connection-error.ts @@ -84,6 +84,7 @@ export function classifyKeyValidationError( 'status' in error.response ) { const status = knownHttpStatus(error.response.status) + if (status === 401) return keyValidationFailure('unauthorized', 401) if (status != null) return keyValidationFailure('api_error', status) } // Axios' fetch adapter identifies transport failures by code. An arbitrary