From 1adbed61e2bf25d956d2f2bc1b4b0ff9ffb29ee6 Mon Sep 17 00:00:00 2001 From: Hayden <154503486+groupthinking@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:16:33 -0500 Subject: [PATCH 01/12] fix(web): stop leaking upstream and Stripe error details to clients Two information-disclosure issues were returning third-party error text verbatim to callers. 1. /api/realtime/session (GET + POST) returned the raw OpenAI response body as `details` while mirroring `upstream.status`. OpenAI bodies echo org/project IDs, quota state, and on 401 a partial key (`Incorrect API key provided: sk-proj-****ABCD`). Both handlers now log the body server-side and return a static message with a fixed 502. 2. Five billing routes returned raw Stripe SDK `err.message`. All five are on the unauthenticated PUBLIC_API_EXACT allowlist, so any internet caller could trigger and read them. Each now returns a static string plus a machine-readable code; the raw message stays in console.error and kaizenObserve. /api/transcribe had the same raw-upstream pattern in its JSON-parse and catch-all branches and is fixed the same way. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../__tests__/billing-activate-route.test.ts | 67 ++++++++++++++ .../__tests__/billing-checkout-route.test.ts | 31 ++++++- .../api/__tests__/billing-renew-route.test.ts | 27 ++++++ .../__tests__/billing-webhook-route.test.ts | 92 +++++++++++++++++++ .../__tests__/realtime-session-route.test.ts | 68 ++++++++++++++ .../api/__tests__/transcribe-route.test.ts | 70 ++++++++++++++ .../web/src/app/api/billing/activate/route.ts | 9 +- .../web/src/app/api/billing/checkout/route.ts | 10 +- apps/web/src/app/api/billing/renew/route.ts | 8 +- apps/web/src/app/api/billing/webhook/route.ts | 17 +++- .../web/src/app/api/realtime/session/route.ts | 38 +++++--- apps/web/src/app/api/transcribe/route.ts | 19 ++-- 12 files changed, 430 insertions(+), 26 deletions(-) create mode 100644 apps/web/src/app/api/__tests__/billing-activate-route.test.ts create mode 100644 apps/web/src/app/api/__tests__/billing-webhook-route.test.ts create mode 100644 apps/web/src/app/api/__tests__/transcribe-route.test.ts diff --git a/apps/web/src/app/api/__tests__/billing-activate-route.test.ts b/apps/web/src/app/api/__tests__/billing-activate-route.test.ts new file mode 100644 index 000000000..d98894aad --- /dev/null +++ b/apps/web/src/app/api/__tests__/billing-activate-route.test.ts @@ -0,0 +1,67 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { NextRequest } from 'next/server'; +import { POST } from '@/app/api/billing/activate/route'; +import { getKaizenTraces, resetKaizenTracesForTests } from '@/lib/billing/kaizen-trace'; + +vi.mock('@/lib/billing/stripe-checkout', () => ({ + getCheckoutSession: vi.fn(), +})); + +vi.mock('@/lib/billing/checkout-session-store', () => ({ + getCheckoutActivation: vi.fn().mockResolvedValue(null), +})); + +vi.mock('@/lib/billing/subscription-events', () => ({ + activateFromCheckoutSession: vi.fn(), +})); + +import { getCheckoutSession } from '@/lib/billing/stripe-checkout'; + +afterEach(() => { + vi.restoreAllMocks(); + resetKaizenTracesForTests(); +}); + +function activateRequest(sessionId: string) { + return new NextRequest('http://localhost/api/billing/activate', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ sessionId }), + }); +} + +// This route is on PUBLIC_API_EXACT — unauthenticated, so any internet caller +// can trigger the failure branch and read the response. +describe('POST /api/billing/activate error leakage', () => { + it('never returns raw Stripe SDK error text', async () => { + const stripeMessage = + "No such checkout.session: 'cs_live_secret'; Invalid API Key provided: sk_live_****ABCD"; + vi.mocked(getCheckoutSession).mockRejectedValue(new Error(stripeMessage)); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'info').mockImplementation(() => {}); + + const res = await POST(activateRequest('cs_live_secret')); + const raw = await res.text(); + + expect(res.status).toBe(500); + expect(JSON.parse(raw)).toEqual({ error: 'activation_failed', code: 'activation_failed' }); + for (const token of ['sk_live', 'No such checkout.session', 'Invalid API Key']) { + expect(raw).not.toContain(token); + } + + // Raw detail is retained for operators only. + expect(JSON.stringify(consoleError.mock.calls)).toContain('sk_live_****ABCD'); + expect(getKaizenTraces('billing').some((t) => t.observation === stripeMessage)).toBe(true); + }); + + it('rejects a malformed body without echoing parser internals', async () => { + const req = new NextRequest('http://localhost/api/billing/activate', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: '{not json', + }); + const res = await POST(req); + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ error: 'invalid_json' }); + }); +}); diff --git a/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts b/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts index 8b4326716..5f326d66a 100644 --- a/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts +++ b/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { NextRequest } from 'next/server'; import { POST } from '@/app/api/billing/checkout/route'; -import { resetKaizenTracesForTests } from '@/lib/billing/kaizen-trace'; +import { getKaizenTraces, resetKaizenTracesForTests } from '@/lib/billing/kaizen-trace'; vi.mock('@/lib/billing/stripe-checkout', () => ({ createProCheckoutSession: vi.fn().mockResolvedValue({ @@ -54,4 +54,33 @@ describe('POST /api/billing/checkout', () => { flow: 'acquisition', }); }); + + // This route is on PUBLIC_API_EXACT — unauthenticated, so any internet caller + // can trigger this branch and read the response. + it('never returns raw Stripe SDK error text', async () => { + const stripeMessage = + "No such price: 'price_1QLiveSecret'; Invalid API Key provided: sk_live_****ABCD"; + vi.mocked(verifyTurnstileToken).mockResolvedValue({ ok: true }); + vi.mocked(createProCheckoutSession).mockRejectedValue(new Error(stripeMessage)); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'info').mockImplementation(() => {}); + + const req = new NextRequest('http://localhost/api/billing/checkout', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ annual: false, turnstileToken: 'valid' }), + }); + const res = await POST(req); + const raw = await res.text(); + + expect(res.status).toBe(500); + expect(JSON.parse(raw)).toEqual({ error: 'checkout_failed', code: 'checkout_failed' }); + for (const secret of ['price_1QLiveSecret', 'sk_live', 'No such price']) { + expect(raw).not.toContain(secret); + } + + // Raw detail is retained for operators only. + expect(JSON.stringify(consoleError.mock.calls)).toContain('sk_live_****ABCD'); + expect(getKaizenTraces('billing').some((t) => t.observation === stripeMessage)).toBe(true); + }); }); diff --git a/apps/web/src/app/api/__tests__/billing-renew-route.test.ts b/apps/web/src/app/api/__tests__/billing-renew-route.test.ts index 96304223d..c39d70c0e 100644 --- a/apps/web/src/app/api/__tests__/billing-renew-route.test.ts +++ b/apps/web/src/app/api/__tests__/billing-renew-route.test.ts @@ -56,4 +56,31 @@ describe('POST /api/billing/renew', () => { expect(traces.some((t) => t.stage === 'renewal_start')).toBe(true); expect(traces.some((t) => t.stage === 'renewal_session')).toBe(true); }); + + // This route is on PUBLIC_API_EXACT — unauthenticated, so any internet caller + // can trigger this branch and read the response. + it('never returns raw Stripe SDK error text', async () => { + const stripeMessage = "Invalid API Key provided: sk_live_****ABCD (account acct_1QLive)"; + vi.mocked(createProCheckoutSession).mockRejectedValue(new Error(stripeMessage)); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'info').mockImplementation(() => {}); + + const req = new NextRequest('http://localhost/api/billing/renew', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ annual: false }), + }); + const res = await POST(req); + const raw = await res.text(); + + expect(res.status).toBe(500); + expect(JSON.parse(raw)).toEqual({ error: 'renewal_failed', code: 'renewal_failed' }); + for (const token of ['sk_live', 'acct_1QLive', 'Invalid API Key']) { + expect(raw).not.toContain(token); + } + + // Raw detail is retained for operators only. + expect(JSON.stringify(consoleError.mock.calls)).toContain('sk_live_****ABCD'); + expect(getKaizenTraces('billing').some((t) => t.observation === stripeMessage)).toBe(true); + }); }); diff --git a/apps/web/src/app/api/__tests__/billing-webhook-route.test.ts b/apps/web/src/app/api/__tests__/billing-webhook-route.test.ts new file mode 100644 index 000000000..da3f13850 --- /dev/null +++ b/apps/web/src/app/api/__tests__/billing-webhook-route.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; +import { NextRequest } from 'next/server'; +import { POST } from '@/app/api/billing/webhook/route'; +import { getKaizenTraces, resetKaizenTracesForTests } from '@/lib/billing/kaizen-trace'; + +const constructEvent = vi.fn(); + +vi.mock('@/lib/billing/stripe-checkout', () => ({ + getStripeClient: () => ({ webhooks: { constructEvent } }), +})); + +vi.mock('@/lib/billing/subscription-events', () => ({ + activateFromCheckoutSession: vi.fn(), + syncFromSubscription: vi.fn(), +})); + +import { activateFromCheckoutSession } from '@/lib/billing/subscription-events'; + +const ORIGINAL_SECRET = process.env.STRIPE_WEBHOOK_SECRET; + +beforeEach(() => { + process.env.STRIPE_WEBHOOK_SECRET = 'whsec_test'; +}); + +afterEach(() => { + if (ORIGINAL_SECRET === undefined) delete process.env.STRIPE_WEBHOOK_SECRET; + else process.env.STRIPE_WEBHOOK_SECRET = ORIGINAL_SECRET; + constructEvent.mockReset(); + vi.restoreAllMocks(); + resetKaizenTracesForTests(); +}); + +function webhookRequest(body: string) { + return new NextRequest('http://localhost/api/billing/webhook', { + method: 'POST', + headers: { 'stripe-signature': 't=1,v1=deadbeef' }, + body, + }); +} + +// This route is on PUBLIC_API_EXACT — unauthenticated, so any internet caller +// can trigger both failure branches and read the response. +describe('POST /api/billing/webhook error leakage', () => { + it('never returns signature-verification internals', async () => { + const stripeMessage = + 'No signatures found matching the expected signature for payload (tolerance 300s, scheme v1, key sk_live_****ABCD)'; + constructEvent.mockImplementation(() => { + throw new Error(stripeMessage); + }); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'info').mockImplementation(() => {}); + + const res = await POST(webhookRequest('{"id":"evt_1"}')); + const raw = await res.text(); + + expect(res.status).toBe(400); + expect(JSON.parse(raw)).toEqual({ error: 'invalid_payload', code: 'invalid_payload' }); + for (const token of ['sk_live', 'tolerance', 'scheme v1', 'No signatures found']) { + expect(raw).not.toContain(token); + } + + // Raw detail is retained for operators only. + expect(JSON.stringify(consoleError.mock.calls)).toContain('sk_live_****ABCD'); + expect(getKaizenTraces('billing').some((t) => t.observation === stripeMessage)).toBe(true); + }); + + it('never returns handler internals', async () => { + const handlerMessage = 'Redis connection refused at 10.0.3.14:6379 for customer cus_live_secret'; + constructEvent.mockReturnValue({ + type: 'checkout.session.completed', + data: { object: { id: 'cs_test_1' } }, + }); + vi.mocked(activateFromCheckoutSession).mockRejectedValue(new Error(handlerMessage)); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'info').mockImplementation(() => {}); + + const res = await POST(webhookRequest('{"id":"evt_2"}')); + const raw = await res.text(); + + expect(res.status).toBe(500); + expect(JSON.parse(raw)).toEqual({ + error: 'webhook_handler_failed', + code: 'webhook_handler_failed', + }); + for (const token of ['10.0.3.14', 'cus_live_secret', 'Redis connection refused']) { + expect(raw).not.toContain(token); + } + + expect(JSON.stringify(consoleError.mock.calls)).toContain('10.0.3.14:6379'); + expect(getKaizenTraces('billing').some((t) => t.observation === handlerMessage)).toBe(true); + }); +}); diff --git a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts index ce8e03f66..a9167ffc4 100644 --- a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts +++ b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts @@ -101,3 +101,71 @@ describe('/api/realtime/session', () => { expect(init.body).toBeInstanceOf(FormData); }); }); + +/** + * OpenAI error bodies echo org/project identifiers, quota state and — on a 401 — + * a partial API key. None of that may reach the browser. + */ +describe('/api/realtime/session upstream error leakage', () => { + const LEAKY_OPENAI_ERROR = JSON.stringify({ + error: { + message: + 'Incorrect API key provided: sk-proj-****ABCD. You can find your API key at https://platform.openai.com/account/api-keys.', + type: 'invalid_request_error', + param: null, + code: 'invalid_api_key', + }, + organization: 'org-eventrelay-prod', + project: 'proj_abc123', + }); + + const LEAKED_TOKENS = [ + 'sk-proj', + 'invalid_api_key', + 'platform.openai.com', + 'org-eventrelay-prod', + 'proj_abc123', + ]; + + it('does not forward the OpenAI body when minting a client secret', async () => { + process.env.OPENAI_API_KEY = 'test-key'; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(upstreamResponse(LEAKY_OPENAI_ERROR, 401))); + + const res = await GET(); + const raw = await res.text(); + + // Upstream status is deliberately not mirrored — it is itself a signal + // about the server's OpenAI account state. + expect(res.status).toBe(502); + expect(JSON.parse(raw)).toEqual({ + error: 'OpenAI Realtime client secret creation failed.', + code: 'realtime_client_secret_failed', + }); + for (const token of LEAKED_TOKENS) { + expect(raw).not.toContain(token); + } + + // ...but operators still get the full body server-side. + expect(JSON.stringify(consoleError.mock.calls)).toContain('sk-proj-****ABCD'); + }); + + it('does not forward the OpenAI body when exchanging an SDP offer', async () => { + process.env.OPENAI_API_KEY = 'test-key'; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(upstreamResponse(LEAKY_OPENAI_ERROR, 429))); + + const res = await POST(sdpRequest('v=0\no=- 1 1 IN IP4 127.0.0.1')); + const raw = await res.text(); + + expect(res.status).toBe(502); + expect(JSON.parse(raw)).toEqual({ + error: 'OpenAI Realtime session creation failed.', + code: 'realtime_session_failed', + }); + for (const token of LEAKED_TOKENS) { + expect(raw).not.toContain(token); + } + expect(JSON.stringify(consoleError.mock.calls)).toContain('sk-proj-****ABCD'); + }); +}); diff --git a/apps/web/src/app/api/__tests__/transcribe-route.test.ts b/apps/web/src/app/api/__tests__/transcribe-route.test.ts new file mode 100644 index 000000000..44f230181 --- /dev/null +++ b/apps/web/src/app/api/__tests__/transcribe-route.test.ts @@ -0,0 +1,70 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { POST } from '@/app/api/transcribe/route'; + +vi.mock('@/lib/transcription-service', () => ({ + fetchTranscript: vi.fn(), +})); + +import { fetchTranscript } from '@/lib/transcription-service'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function transcribeRequest(body: string) { + return new Request('http://localhost/api/transcribe', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body, + }); +} + +describe('POST /api/transcribe error leakage', () => { + it('does not echo raw upstream provider text when the pipeline throws', async () => { + const upstreamMessage = + 'OpenAI 401: Incorrect API key provided: sk-proj-****ABCD (org org-eventrelay-prod)'; + vi.mocked(fetchTranscript).mockRejectedValue(new Error(upstreamMessage)); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const res = await POST( + transcribeRequest(JSON.stringify({ url: 'https://www.youtube.com/watch?v=auJzb1D-fag' })), + ); + const raw = await res.text(); + + expect(res.status).toBe(500); + expect(JSON.parse(raw)).toEqual({ + success: false, + error: 'Transcription failed', + code: 'transcription_failed', + transcript: '', + }); + for (const token of ['sk-proj', 'org-eventrelay-prod', 'Incorrect API key']) { + expect(raw).not.toContain(token); + } + + // Raw detail is retained for operators only. + expect(consoleError).toHaveBeenCalledWith( + 'Transcription route error:', + expect.objectContaining({ message: upstreamMessage }), + ); + }); + + it('does not echo parser internals for a malformed body', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const res = await POST(transcribeRequest('{not json')); + const raw = await res.text(); + + expect(res.status).toBe(400); + expect(JSON.parse(raw)).toEqual({ + success: false, + error: 'Invalid JSON in request body', + code: 'invalid_json', + transcript: '', + }); + expect(raw).not.toContain('Unexpected token'); + expect(raw).not.toContain('position'); + expect(raw).not.toContain('details'); + expect(consoleError).toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/app/api/billing/activate/route.ts b/apps/web/src/app/api/billing/activate/route.ts index cabfaa853..aa74e9877 100644 --- a/apps/web/src/app/api/billing/activate/route.ts +++ b/apps/web/src/app/api/billing/activate/route.ts @@ -90,6 +90,13 @@ export async function POST(req: NextRequest) { return res; } catch (err) { const message = err instanceof Error ? err.message : 'activation_failed'; - return NextResponse.json({ error: message }, { status: 500 }); + // Unauthenticated public route (PUBLIC_API_EXACT) — keep Stripe SDK text, + // which can echo account identifiers and partial API keys, server-side. + console.error('[billing] activation failed:', message); + kaizenObserve('billing', 'activate_error', message, { fix: 'inspect_stripe_checkout_session' }); + return NextResponse.json( + { error: 'activation_failed', code: 'activation_failed' }, + { status: 500 }, + ); } } diff --git a/apps/web/src/app/api/billing/checkout/route.ts b/apps/web/src/app/api/billing/checkout/route.ts index d735b064e..3b6a1354f 100644 --- a/apps/web/src/app/api/billing/checkout/route.ts +++ b/apps/web/src/app/api/billing/checkout/route.ts @@ -34,7 +34,15 @@ export async function POST(req: NextRequest) { return NextResponse.json(result); } catch (err) { const message = err instanceof Error ? err.message : 'checkout_failed'; + // This route is on the unauthenticated public allowlist (PUBLIC_API_EXACT), + // so anything returned here is readable by any internet caller. Stripe SDK + // messages can contain price/account identifiers and partial API keys, so + // the raw text stays server-side only. + console.error('[billing] checkout failed:', message); kaizenObserve('billing', 'checkout_error', message, { fix: 'verify_stripe_env' }); - return NextResponse.json({ error: message }, { status: 500 }); + return NextResponse.json( + { error: 'checkout_failed', code: 'checkout_failed' }, + { status: 500 }, + ); } } \ No newline at end of file diff --git a/apps/web/src/app/api/billing/renew/route.ts b/apps/web/src/app/api/billing/renew/route.ts index 26ae0b5a0..a70b1740d 100644 --- a/apps/web/src/app/api/billing/renew/route.ts +++ b/apps/web/src/app/api/billing/renew/route.ts @@ -35,7 +35,13 @@ export async function POST(req: NextRequest) { return NextResponse.json(result); } catch (err) { const message = err instanceof Error ? err.message : 'renewal_failed'; + // Unauthenticated public route (PUBLIC_API_EXACT) — keep Stripe SDK text, + // which can echo account identifiers and partial API keys, server-side. + console.error('[billing] renewal failed:', message); kaizenObserve('billing', 'renewal_error', message, { fix: 'verify_stripe_env' }); - return NextResponse.json({ error: message }, { status: 500 }); + return NextResponse.json( + { error: 'renewal_failed', code: 'renewal_failed' }, + { status: 500 }, + ); } } \ No newline at end of file diff --git a/apps/web/src/app/api/billing/webhook/route.ts b/apps/web/src/app/api/billing/webhook/route.ts index 927f8b6c4..1e8f3cf53 100644 --- a/apps/web/src/app/api/billing/webhook/route.ts +++ b/apps/web/src/app/api/billing/webhook/route.ts @@ -24,8 +24,15 @@ export async function POST(req: NextRequest) { event = stripe.webhooks.constructEvent(raw, signature, secret); } catch (err) { const message = err instanceof Error ? err.message : 'invalid_payload'; + // Unauthenticated public route (PUBLIC_API_EXACT). Stripe signature errors + // describe the expected scheme/timestamp tolerance and can echo API key + // fragments, which would help an attacker forge a payload — log only. + console.error('[billing] webhook signature verification failed:', message); kaizenObserve('billing', 'webhook_rejected', message, { fix: 'verify_stripe_signature' }); - return NextResponse.json({ error: message }, { status: 400 }); + return NextResponse.json( + { error: 'invalid_payload', code: 'invalid_payload' }, + { status: 400 }, + ); } kaizenObserve('billing', 'webhook_received', `Stripe event ${event.type}`); @@ -49,7 +56,13 @@ export async function POST(req: NextRequest) { return NextResponse.json({ received: true }); } catch (err) { const message = err instanceof Error ? err.message : 'webhook_handler_failed'; + // Unauthenticated public route (PUBLIC_API_EXACT) — handler failures can + // surface Stripe SDK and datastore internals, so keep them server-side. + console.error('[billing] webhook handler failed:', message); kaizenObserve('billing', 'webhook_error', message, { fix: 'inspect_handler' }); - return NextResponse.json({ error: message }, { status: 500 }); + return NextResponse.json( + { error: 'webhook_handler_failed', code: 'webhook_handler_failed' }, + { status: 500 }, + ); } } \ No newline at end of file diff --git a/apps/web/src/app/api/realtime/session/route.ts b/apps/web/src/app/api/realtime/session/route.ts index 5226afb15..c52312d7c 100644 --- a/apps/web/src/app/api/realtime/session/route.ts +++ b/apps/web/src/app/api/realtime/session/route.ts @@ -46,6 +46,20 @@ const realtimeSession = { tool_choice: 'auto', }; +/** + * OpenAI error bodies routinely echo org/project identifiers, quota state and, + * on a 401, a partial API key (`Incorrect API key provided: sk-proj-****ABCD`). + * None of that may reach the browser, so the upstream body is logged + * server-side only and the caller gets a fixed 502 with a static message. + * The upstream status is deliberately NOT mirrored — it is itself a signal + * about the server's OpenAI account state. + */ +function upstreamFailure(stage: string, status: number, body: string, error: string, code: string) { + console.error(`[realtime] ${stage} failed`, JSON.stringify({ status, body })); + + return Response.json({ error, code }, { status: 502 }); +} + function getOpenAiHeaders(contentType?: string) { const apiKey = process.env.OPENAI_API_KEY; @@ -95,12 +109,12 @@ export async function GET() { const contentType = upstream.headers.get('content-type') || 'application/json'; if (!upstream.ok) { - return Response.json( - { - error: 'OpenAI Realtime client secret creation failed.', - details: body, - }, - { status: upstream.status }, + return upstreamFailure( + 'client_secret', + upstream.status, + body, + 'OpenAI Realtime client secret creation failed.', + 'realtime_client_secret_failed', ); } @@ -144,12 +158,12 @@ export async function POST(request: Request) { const body = await upstream.text(); if (!upstream.ok) { - return Response.json( - { - error: 'OpenAI Realtime session creation failed.', - details: body, - }, - { status: upstream.status }, + return upstreamFailure( + 'sdp_exchange', + upstream.status, + body, + 'OpenAI Realtime session creation failed.', + 'realtime_session_failed', ); } diff --git a/apps/web/src/app/api/transcribe/route.ts b/apps/web/src/app/api/transcribe/route.ts index aaf2e7ebb..580ccc1e6 100644 --- a/apps/web/src/app/api/transcribe/route.ts +++ b/apps/web/src/app/api/transcribe/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from 'next/server'; import { fetchTranscript } from '@/lib/transcription-service'; -import { parseJsonSafely, formatApiError } from '@/lib/error-handling'; +import { parseJsonSafely } from '@/lib/error-handling'; export const runtime = 'nodejs'; export const maxDuration = 120; @@ -14,7 +14,8 @@ export const maxDuration = 120; * 3. OpenAI Responses API with web search - with rate limit handling * 4. Direct audio STT via OpenAI Whisper * - * Error responses provide actionable details: + * Error responses are static and machine-readable; raw upstream provider text + * is logged server-side only (it can carry account IDs and partial API keys): * - 400: Invalid input (missing URL, malformed JSON) * - 429: Rate limited (implement backoff or upgrade service plan) * - 500: Service unavailable (check API keys and billing) @@ -27,12 +28,12 @@ export async function POST(request: Request) { try { body = await parseJsonSafely(request); } catch (parseError) { - const error = formatApiError(parseError, 'Invalid JSON in request body'); + console.error('Transcription route JSON parse error:', parseError); return NextResponse.json( { success: false, - error: error.message, - details: error.details, + error: 'Invalid JSON in request body', + code: 'invalid_json', transcript: '', }, { status: 400 } @@ -102,14 +103,16 @@ export async function POST(request: Request) { return NextResponse.json(result); } catch (error) { + // Failures here can carry upstream provider text (OpenAI/Gemini/backend), + // which may include account identifiers, quota state or partial API keys. + // Log it server-side and return a fixed, static payload. console.error('Transcription route error:', error); - const formatted = formatApiError(error); return NextResponse.json( { success: false, - error: formatted.message, - details: formatted.details, + error: 'Transcription failed', + code: 'transcription_failed', transcript: '', }, { status: 500 } From 672bac373868b05bbdf4e89ce5c69a479c3506da Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 09:45:25 +0000 Subject: [PATCH 02/12] docs(web): correct two inaccurate security comments flagged in review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the two fixes agreed in the Copilot review threads on #1381. Both are comment-only; no behavior, status code, or response body changes. transcribe/route.ts — the docblock claimed all error responses are static and machine-readable. The four `!result.success` branches return `fetchTranscript`'s message verbatim and carry no `code`, and the billing branch still returns a static `details` hint. Those messages are app-authored (every `error` in transcription-service.ts is a literal, a numeric status, or our own SSRF-guard text), so the CWE-209 objective holds — but the docblock overstated it. Scoped it to the two sanitized paths and described the rest accurately. billing/webhook/route.ts — the rationale was wrong on two counts. `constructEvent` verifies Stripe-Signature against the webhook signing secret, not the API key, so an API-key fragment cannot forge a signature; and StripeSignatureVerificationError describes tolerance/scheme/digests without echoing key material. Rewrote it to state the real risk: on an unauthenticated route, verification internals are an oracle for tuning replay/timestamp attacks. Verified at this head: type-check clean, eslint clean, the six route test files pass (17 tests), full web suite 261 passed / 1 failed — the failure being the pre-existing billing-chat-gating timeout tracked in #1116, which fails identically on the base commit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jsfy3ts3U1ggGsJqFB2KSC --- apps/web/src/app/api/billing/webhook/route.ts | 7 +++++-- apps/web/src/app/api/transcribe/route.ts | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/apps/web/src/app/api/billing/webhook/route.ts b/apps/web/src/app/api/billing/webhook/route.ts index 1e8f3cf53..767131e1b 100644 --- a/apps/web/src/app/api/billing/webhook/route.ts +++ b/apps/web/src/app/api/billing/webhook/route.ts @@ -25,8 +25,11 @@ export async function POST(req: NextRequest) { } catch (err) { const message = err instanceof Error ? err.message : 'invalid_payload'; // Unauthenticated public route (PUBLIC_API_EXACT). Stripe signature errors - // describe the expected scheme/timestamp tolerance and can echo API key - // fragments, which would help an attacker forge a payload — log only. + // disclose verification internals — the tolerance window, the expected + // signature scheme, and which check failed — which an unauthenticated + // caller could use as an oracle to tune replay/timestamp attacks. The + // signing secret is never echoed, so this is disclosure, not forgery + // assistance; log server-side only. console.error('[billing] webhook signature verification failed:', message); kaizenObserve('billing', 'webhook_rejected', message, { fix: 'verify_stripe_signature' }); return NextResponse.json( diff --git a/apps/web/src/app/api/transcribe/route.ts b/apps/web/src/app/api/transcribe/route.ts index 580ccc1e6..b05c88d3c 100644 --- a/apps/web/src/app/api/transcribe/route.ts +++ b/apps/web/src/app/api/transcribe/route.ts @@ -14,8 +14,11 @@ export const maxDuration = 120; * 3. OpenAI Responses API with web search - with rate limit handling * 4. Direct audio STT via OpenAI Whisper * - * Error responses are static and machine-readable; raw upstream provider text - * is logged server-side only (it can carry account IDs and partial API keys): + * The JSON-parse and catch-all branches return static, machine-readable payloads + * (`code` + fixed `error`); raw upstream provider text is logged server-side only, + * since it can carry account IDs and partial API keys. The `!result.success` + * branches below return `fetchTranscript`'s own app-authored message verbatim — + * these are not upstream text, and they carry no `code`: * - 400: Invalid input (missing URL, malformed JSON) * - 429: Rate limited (implement backoff or upgrade service plan) * - 500: Service unavailable (check API keys and billing) From ae2be834e417bce57102c4d2260788ae152d00a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 09:49:58 +0000 Subject: [PATCH 03/12] fix(web): return the static 502 when the Realtime upstream is unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit flagged this on #1381 and it reproduces: neither GET nor POST wrapped its `fetch`, so a transport-level failure — DNS, TLS, connection reset, timeout — rejected before `upstreamFailure` could run. The throw escaped to Next.js's own unstructured 500 instead of the `{ error, code }` JSON the route promises, `use-realtime-voice.ts` got a payload it cannot parse, and no `[realtime]` line was logged for the operator. Both handlers now wrap the fetch and the body read in try/catch and route transport failures through a new `upstreamUnreachable` helper, which logs the cause server-side and returns the same fixed 502 as a non-OK upstream response. The cause is an Error from our own transport rather than upstream text, so logging it introduces no disclosure. `await upstream.text()` is inside the try as well — a truncated body rejects there for the same reason. The gap predates this PR, but this PR is what establishes the contract that these routes always answer with a static structured error, so it belongs here. Three regression tests, each verified non-vacuous — they fail against the unwrapped route and pass against the fix: GET with a rejected fetch (ENOTFOUND), POST with a rejected fetch (socket hang up), and GET with a response whose body read rejects mid-stream. Verified at this head: tsc clean, eslint clean, full web suite 264 passed / 1 failed — the pre-existing billing-chat-gating timeout tracked in #1116, identical on the base commit. Up from 261 passed by exactly these 3 tests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jsfy3ts3U1ggGsJqFB2KSC --- .../__tests__/realtime-session-route.test.ts | 58 ++++++++++++++ .../web/src/app/api/realtime/session/route.ts | 75 ++++++++++++++----- 2 files changed, 115 insertions(+), 18 deletions(-) diff --git a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts index a9167ffc4..0124e75f3 100644 --- a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts +++ b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts @@ -169,3 +169,61 @@ describe('/api/realtime/session upstream error leakage', () => { expect(JSON.stringify(consoleError.mock.calls)).toContain('sk-proj-****ABCD'); }); }); + +/** + * A rejected fetch (DNS, TLS, reset, timeout) throws before any upstream status + * exists. Without a catch it escapes to Next.js's own unstructured 500, which + * breaks the JSON error contract the client parses and logs nothing. + */ +describe('/api/realtime/session transport failures', () => { + it('returns the static 502 when the client-secret request never reaches OpenAI', async () => { + process.env.OPENAI_API_KEY = 'test-key'; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal( + 'fetch', + vi.fn().mockRejectedValue(new TypeError('fetch failed: getaddrinfo ENOTFOUND api.openai.com')), + ); + + const res = await GET(); + const raw = await res.text(); + + expect(res.status).toBe(502); + expect(JSON.parse(raw)).toEqual({ + error: 'OpenAI Realtime client secret creation failed.', + code: 'realtime_client_secret_failed', + }); + expect(JSON.stringify(consoleError.mock.calls)).toContain('client_secret request failed'); + }); + + it('returns the static 502 when the SDP exchange never reaches OpenAI', async () => { + process.env.OPENAI_API_KEY = 'test-key'; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('socket hang up'))); + + const res = await POST(sdpRequest('v=0\no=- 1 1 IN IP4 127.0.0.1')); + const raw = await res.text(); + + expect(res.status).toBe(502); + expect(JSON.parse(raw)).toEqual({ + error: 'OpenAI Realtime session creation failed.', + code: 'realtime_session_failed', + }); + expect(JSON.stringify(consoleError.mock.calls)).toContain('sdp_exchange request failed'); + }); + + it('returns the static 502 when the upstream body cannot be read', async () => { + process.env.OPENAI_API_KEY = 'test-key'; + vi.spyOn(console, 'error').mockImplementation(() => {}); + const truncated = new Response('', { status: 200 }); + vi.spyOn(truncated, 'text').mockRejectedValue(new Error('terminated: aborted mid-body')); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(truncated)); + + const res = await GET(); + + expect(res.status).toBe(502); + expect(await res.json()).toEqual({ + error: 'OpenAI Realtime client secret creation failed.', + code: 'realtime_client_secret_failed', + }); + }); +}); diff --git a/apps/web/src/app/api/realtime/session/route.ts b/apps/web/src/app/api/realtime/session/route.ts index c52312d7c..f7e868a56 100644 --- a/apps/web/src/app/api/realtime/session/route.ts +++ b/apps/web/src/app/api/realtime/session/route.ts @@ -60,6 +60,20 @@ function upstreamFailure(stage: string, status: number, body: string, error: str return Response.json({ error, code }, { status: 502 }); } +/** + * A transport-level failure — DNS, TLS, connection reset, timeout — rejects the + * fetch before any upstream status exists, so `upstreamFailure` never runs. + * Left unhandled it escapes to Next.js's own 500, which is not the JSON shape + * the client parses and which produces no `[realtime]` log line. The cause is + * an Error from our own transport, not upstream text, so it is safe to log — + * and the caller gets the same fixed 502 as a non-OK upstream response. + */ +function upstreamUnreachable(stage: string, cause: unknown, error: string, code: string) { + console.error(`[realtime] ${stage} request failed`, cause); + + return Response.json({ error, code }, { status: 502 }); +} + function getOpenAiHeaders(contentType?: string) { const apiKey = process.env.OPENAI_API_KEY; @@ -93,19 +107,32 @@ export async function GET() { ); } - const upstream = await fetch(OPENAI_REALTIME_CLIENT_SECRETS_URL, { - method: 'POST', - headers, - body: JSON.stringify({ - expires_after: { - anchor: 'created_at', - seconds: 600, - }, - session: realtimeSession, - }), - }); + let upstream: Response; + let body: string; + + try { + upstream = await fetch(OPENAI_REALTIME_CLIENT_SECRETS_URL, { + method: 'POST', + headers, + body: JSON.stringify({ + expires_after: { + anchor: 'created_at', + seconds: 600, + }, + session: realtimeSession, + }), + }); + + body = await upstream.text(); + } catch (err) { + return upstreamUnreachable( + 'client_secret', + err, + 'OpenAI Realtime client secret creation failed.', + 'realtime_client_secret_failed', + ); + } - const body = await upstream.text(); const contentType = upstream.headers.get('content-type') || 'application/json'; if (!upstream.ok) { @@ -149,13 +176,25 @@ export async function POST(request: Request) { formData.set('sdp', new Blob([offerSdp], { type: 'application/sdp' }), 'offer.sdp'); formData.set('session', new Blob([JSON.stringify(realtimeSession)], { type: 'application/json' }), 'session.json'); - const upstream = await fetch(OPENAI_REALTIME_CALLS_URL, { - method: 'POST', - headers, - body: formData, - }); + let upstream: Response; + let body: string; + + try { + upstream = await fetch(OPENAI_REALTIME_CALLS_URL, { + method: 'POST', + headers, + body: formData, + }); - const body = await upstream.text(); + body = await upstream.text(); + } catch (err) { + return upstreamUnreachable( + 'sdp_exchange', + err, + 'OpenAI Realtime session creation failed.', + 'realtime_session_failed', + ); + } if (!upstream.ok) { return upstreamFailure( From 5e69b7eb386e3b8b3063844eab7832125738cbc6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 09:55:48 +0000 Subject: [PATCH 04/12] fix(web): complete the error-code contract across the patched routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's review of 1adbed61 found the `code` field was added only to the branches that were sanitized for CWE-209, leaving the rest of each route's error surface without a machine-readable key. A client cannot branch on a contract that only half the branches honour. Every remaining error branch on the six patched routes now returns `code` alongside its existing `error` value: - realtime/session — `realtime_not_configured` (GET and POST missing-key), `invalid_sdp_offer` - billing/activate — `invalid_json`, `session_id_required`, `not_eligible` - billing/checkout — `invalid_json`, `turnstile_rejected` - billing/renew — `invalid_json` - billing/webhook — `webhook_not_configured`, `missing_signature` `error` values are unchanged, so no client contract breaks. The checkout Turnstile branch additionally gains a fallback: `turnstile.error` is optional on `TurnstileVerifyResult`, so the previous `{ error: turnstile.error }` could serialize to `{}`. Its values are all our own literals, never Cloudflare response text, so returning it verbatim remains correct. Also from the same review: - `upstreamFailure` and `upstreamUnreachable` declare `: Response`. - `activateRequest`/`webhookRequest` declare `: NextRequest`, `transcribeRequest` declares `: Request`. The review's remaining finding — that a rejected `fetch()` skips `upstreamFailure` entirely — was already fixed in ae2be83, which landed after the review ran. +9 tests covering each newly-coded branch. `npm test` 271 passed / 1 failed; the single failure is the pre-existing `billing-chat-gating` 5000 ms timeout (#1116), identical on the base commit. `npm run lint` and `npm run type-check` clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DSkV9zjdhpkfUSMeLhrFGy --- .../__tests__/billing-activate-route.test.ts | 35 ++++++++++++++++-- .../__tests__/billing-checkout-route.test.ts | 18 +++++++++- .../api/__tests__/billing-renew-route.test.ts | 12 +++++++ .../__tests__/billing-webhook-route.test.ts | 36 ++++++++++++++++++- .../__tests__/realtime-session-route.test.ts | 12 +++++++ .../api/__tests__/transcribe-route.test.ts | 2 +- .../web/src/app/api/billing/activate/route.ts | 9 +++-- .../web/src/app/api/billing/checkout/route.ts | 12 +++++-- apps/web/src/app/api/billing/renew/route.ts | 2 +- apps/web/src/app/api/billing/webhook/route.ts | 10 ++++-- .../web/src/app/api/realtime/session/route.ts | 27 +++++++++++--- 11 files changed, 157 insertions(+), 18 deletions(-) diff --git a/apps/web/src/app/api/__tests__/billing-activate-route.test.ts b/apps/web/src/app/api/__tests__/billing-activate-route.test.ts index d98894aad..599d7ebf0 100644 --- a/apps/web/src/app/api/__tests__/billing-activate-route.test.ts +++ b/apps/web/src/app/api/__tests__/billing-activate-route.test.ts @@ -16,13 +16,14 @@ vi.mock('@/lib/billing/subscription-events', () => ({ })); import { getCheckoutSession } from '@/lib/billing/stripe-checkout'; +import { activateFromCheckoutSession } from '@/lib/billing/subscription-events'; afterEach(() => { vi.restoreAllMocks(); resetKaizenTracesForTests(); }); -function activateRequest(sessionId: string) { +function activateRequest(sessionId: string): NextRequest { return new NextRequest('http://localhost/api/billing/activate', { method: 'POST', headers: { 'content-type': 'application/json' }, @@ -62,6 +63,36 @@ describe('POST /api/billing/activate error leakage', () => { }); const res = await POST(req); expect(res.status).toBe(400); - expect(await res.json()).toEqual({ error: 'invalid_json' }); + expect(await res.json()).toEqual({ error: 'invalid_json', code: 'invalid_json' }); + }); +}); + +// Every error branch must carry a stable machine-readable `code`, not only the +// two that were sanitized for CWE-209 — otherwise a client cannot branch on the +// contract this route declares. +describe('POST /api/billing/activate error-code contract', () => { + it('returns session_id_required with a matching code', async () => { + const res = await POST(activateRequest(' ')); + + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ + error: 'session_id_required', + code: 'session_id_required', + }); + }); + + it('returns not_eligible with a matching code', async () => { + vi.mocked(getCheckoutSession).mockResolvedValue({ id: 'cs_free' } as never); + vi.mocked(activateFromCheckoutSession).mockResolvedValue(null); + vi.spyOn(console, 'info').mockImplementation(() => {}); + + const res = await POST(activateRequest('cs_free')); + + expect(res.status).toBe(402); + expect(await res.json()).toEqual({ + error: 'not_eligible', + code: 'not_eligible', + sessionId: 'cs_free', + }); }); }); diff --git a/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts b/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts index 5f326d66a..028b74624 100644 --- a/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts +++ b/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts @@ -33,10 +33,26 @@ describe('POST /api/billing/checkout', () => { const res = await POST(req); expect(res.status).toBe(403); const body = await res.json(); - expect(body.error).toBe('turnstile_verification_failed'); + // `error` stays verbatim for client compatibility; `code` is the stable key. + expect(body).toEqual({ + error: 'turnstile_verification_failed', + code: 'turnstile_rejected', + }); expect(createProCheckoutSession).not.toHaveBeenCalled(); }); + it('rejects a malformed body with a matching code', async () => { + const req = new NextRequest('http://localhost/api/billing/checkout', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: '{not json', + }); + const res = await POST(req); + + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ error: 'invalid_json', code: 'invalid_json' }); + }); + it('creates Stripe session on valid Turnstile token', async () => { vi.mocked(verifyTurnstileToken).mockResolvedValue({ ok: true }); const req = new NextRequest('http://localhost/api/billing/checkout', { diff --git a/apps/web/src/app/api/__tests__/billing-renew-route.test.ts b/apps/web/src/app/api/__tests__/billing-renew-route.test.ts index c39d70c0e..72b1a7727 100644 --- a/apps/web/src/app/api/__tests__/billing-renew-route.test.ts +++ b/apps/web/src/app/api/__tests__/billing-renew-route.test.ts @@ -83,4 +83,16 @@ describe('POST /api/billing/renew', () => { expect(JSON.stringify(consoleError.mock.calls)).toContain('sk_live_****ABCD'); expect(getKaizenTraces('billing').some((t) => t.observation === stripeMessage)).toBe(true); }); + + it('rejects a malformed body with a matching code', async () => { + const req = new NextRequest('http://localhost/api/billing/renew', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: '{not json', + }); + const res = await POST(req); + + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ error: 'invalid_json', code: 'invalid_json' }); + }); }); diff --git a/apps/web/src/app/api/__tests__/billing-webhook-route.test.ts b/apps/web/src/app/api/__tests__/billing-webhook-route.test.ts index da3f13850..d39317c0b 100644 --- a/apps/web/src/app/api/__tests__/billing-webhook-route.test.ts +++ b/apps/web/src/app/api/__tests__/billing-webhook-route.test.ts @@ -30,7 +30,7 @@ afterEach(() => { resetKaizenTracesForTests(); }); -function webhookRequest(body: string) { +function webhookRequest(body: string): NextRequest { return new NextRequest('http://localhost/api/billing/webhook', { method: 'POST', headers: { 'stripe-signature': 't=1,v1=deadbeef' }, @@ -90,3 +90,37 @@ describe('POST /api/billing/webhook error leakage', () => { expect(getKaizenTraces('billing').some((t) => t.observation === handlerMessage)).toBe(true); }); }); + +// The two pre-flight rejections short-circuit before Stripe is ever called, so +// they never carried leaked text — but they are part of the same error contract +// and must return a stable `code` like every other branch. +describe('POST /api/billing/webhook error-code contract', () => { + it('returns webhook_not_configured with a matching code', async () => { + delete process.env.STRIPE_WEBHOOK_SECRET; + + const res = await POST(webhookRequest('{"id":"evt_3"}')); + + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ + error: 'webhook_not_configured', + code: 'webhook_not_configured', + }); + expect(constructEvent).not.toHaveBeenCalled(); + }); + + it('returns missing_signature with a matching code', async () => { + const req = new NextRequest('http://localhost/api/billing/webhook', { + method: 'POST', + body: '{"id":"evt_4"}', + }); + + const res = await POST(req); + + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ + error: 'missing_signature', + code: 'missing_signature', + }); + expect(constructEvent).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts index 0124e75f3..ca69a73d6 100644 --- a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts +++ b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts @@ -39,6 +39,17 @@ describe('/api/realtime/session', () => { expect(res.status).toBe(500); expect(body.error).toMatch(/OPENAI_API_KEY/); + expect(body.code).toBe('realtime_not_configured'); + }); + + it('returns 500 with a matching code when POST finds no OpenAI key', async () => { + delete process.env.OPENAI_API_KEY; + + const res = await POST(sdpRequest('v=0\no=- 1 1 IN IP4 127.0.0.1')); + const body = await res.json(); + + expect(res.status).toBe(500); + expect(body.code).toBe('realtime_not_configured'); }); it('mints a browser client secret with the GA Realtime session shape', async () => { @@ -80,6 +91,7 @@ describe('/api/realtime/session', () => { expect(res.status).toBe(400); expect(body.error).toMatch(/SDP offer/); + expect(body.code).toBe('invalid_sdp_offer'); }); it('keeps the server-side SDP exchange available as a fallback', async () => { diff --git a/apps/web/src/app/api/__tests__/transcribe-route.test.ts b/apps/web/src/app/api/__tests__/transcribe-route.test.ts index 44f230181..f48937702 100644 --- a/apps/web/src/app/api/__tests__/transcribe-route.test.ts +++ b/apps/web/src/app/api/__tests__/transcribe-route.test.ts @@ -11,7 +11,7 @@ afterEach(() => { vi.restoreAllMocks(); }); -function transcribeRequest(body: string) { +function transcribeRequest(body: string): Request { return new Request('http://localhost/api/transcribe', { method: 'POST', headers: { 'content-type': 'application/json' }, diff --git a/apps/web/src/app/api/billing/activate/route.ts b/apps/web/src/app/api/billing/activate/route.ts index aa74e9877..982043c60 100644 --- a/apps/web/src/app/api/billing/activate/route.ts +++ b/apps/web/src/app/api/billing/activate/route.ts @@ -24,11 +24,14 @@ export async function POST(req: NextRequest) { try { body = await req.json(); } catch { - return NextResponse.json({ error: 'invalid_json' }, { status: 400 }); + return NextResponse.json({ error: 'invalid_json', code: 'invalid_json' }, { status: 400 }); } if (!body.sessionId?.trim()) { - return NextResponse.json({ error: 'session_id_required' }, { status: 400 }); + return NextResponse.json( + { error: 'session_id_required', code: 'session_id_required' }, + { status: 400 }, + ); } const sessionId = body.sessionId.trim(); @@ -49,7 +52,7 @@ export async function POST(req: NextRequest) { if (!entitlement || entitlement.plan !== 'pro') { return NextResponse.json( - { error: 'not_eligible', sessionId }, + { error: 'not_eligible', code: 'not_eligible', sessionId }, { status: 402 }, ); } diff --git a/apps/web/src/app/api/billing/checkout/route.ts b/apps/web/src/app/api/billing/checkout/route.ts index 3b6a1354f..3c533105f 100644 --- a/apps/web/src/app/api/billing/checkout/route.ts +++ b/apps/web/src/app/api/billing/checkout/route.ts @@ -8,7 +8,7 @@ export async function POST(req: NextRequest) { try { body = await req.json(); } catch { - return NextResponse.json({ error: 'invalid_json' }, { status: 400 }); + return NextResponse.json({ error: 'invalid_json', code: 'invalid_json' }, { status: 400 }); } const turnstile = await verifyTurnstileToken( @@ -20,7 +20,15 @@ export async function POST(req: NextRequest) { kaizenObserve('billing', 'acquisition_blocked', 'Turnstile rejected acquisition checkout', { decision: turnstile.error ?? 'unknown', }); - return NextResponse.json({ error: turnstile.error }, { status: 403 }); + // `turnstile.error` is always one of our own literals (`turnstile_not_configured`, + // `turnstile_token_missing`, `siteverify_http_`, `turnstile_verification_failed`) + // — never Cloudflare response text — so it is safe to return. It is kept verbatim + // for client compatibility; `code` is the stable machine-readable key, and the + // fallback covers the `ok: false` shape that carries no `error`. + return NextResponse.json( + { error: turnstile.error ?? 'turnstile_rejected', code: 'turnstile_rejected' }, + { status: 403 }, + ); } kaizenObserve('billing', 'acquisition_allowed', 'Turnstile passed for new Pro checkout'); diff --git a/apps/web/src/app/api/billing/renew/route.ts b/apps/web/src/app/api/billing/renew/route.ts index a70b1740d..a16fc5ab2 100644 --- a/apps/web/src/app/api/billing/renew/route.ts +++ b/apps/web/src/app/api/billing/renew/route.ts @@ -9,7 +9,7 @@ export async function POST(req: NextRequest) { try { body = await req.json(); } catch { - return NextResponse.json({ error: 'invalid_json' }, { status: 400 }); + return NextResponse.json({ error: 'invalid_json', code: 'invalid_json' }, { status: 400 }); } const email = await resolveTrustedBillingEmail(req); diff --git a/apps/web/src/app/api/billing/webhook/route.ts b/apps/web/src/app/api/billing/webhook/route.ts index 767131e1b..f91ff839f 100644 --- a/apps/web/src/app/api/billing/webhook/route.ts +++ b/apps/web/src/app/api/billing/webhook/route.ts @@ -9,12 +9,18 @@ export const runtime = 'nodejs'; export async function POST(req: NextRequest) { const secret = process.env.STRIPE_WEBHOOK_SECRET; if (!secret) { - return NextResponse.json({ error: 'webhook_not_configured' }, { status: 503 }); + return NextResponse.json( + { error: 'webhook_not_configured', code: 'webhook_not_configured' }, + { status: 503 }, + ); } const signature = req.headers.get('stripe-signature'); if (!signature) { - return NextResponse.json({ error: 'missing_signature' }, { status: 400 }); + return NextResponse.json( + { error: 'missing_signature', code: 'missing_signature' }, + { status: 400 }, + ); } let event: Stripe.Event; diff --git a/apps/web/src/app/api/realtime/session/route.ts b/apps/web/src/app/api/realtime/session/route.ts index f7e868a56..ab2277c4c 100644 --- a/apps/web/src/app/api/realtime/session/route.ts +++ b/apps/web/src/app/api/realtime/session/route.ts @@ -54,7 +54,13 @@ const realtimeSession = { * The upstream status is deliberately NOT mirrored — it is itself a signal * about the server's OpenAI account state. */ -function upstreamFailure(stage: string, status: number, body: string, error: string, code: string) { +function upstreamFailure( + stage: string, + status: number, + body: string, + error: string, + code: string, +): Response { console.error(`[realtime] ${stage} failed`, JSON.stringify({ status, body })); return Response.json({ error, code }, { status: 502 }); @@ -68,7 +74,12 @@ function upstreamFailure(stage: string, status: number, body: string, error: str * an Error from our own transport, not upstream text, so it is safe to log — * and the caller gets the same fixed 502 as a non-OK upstream response. */ -function upstreamUnreachable(stage: string, cause: unknown, error: string, code: string) { +function upstreamUnreachable( + stage: string, + cause: unknown, + error: string, + code: string, +): Response { console.error(`[realtime] ${stage} request failed`, cause); return Response.json({ error, code }, { status: 502 }); @@ -102,7 +113,10 @@ export async function GET() { if (!headers) { return Response.json( - { error: 'OPENAI_API_KEY is not configured on the server.' }, + { + error: 'OPENAI_API_KEY is not configured on the server.', + code: 'realtime_not_configured', + }, { status: 500 }, ); } @@ -158,7 +172,7 @@ export async function POST(request: Request) { const offerSdp = await request.text(); if (!offerSdp.trim() || !offerSdp.includes('v=0')) { return Response.json( - { error: 'A valid WebRTC SDP offer is required.' }, + { error: 'A valid WebRTC SDP offer is required.', code: 'invalid_sdp_offer' }, { status: 400 }, ); } @@ -167,7 +181,10 @@ export async function POST(request: Request) { if (!headers) { return Response.json( - { error: 'OPENAI_API_KEY is not configured on the server.' }, + { + error: 'OPENAI_API_KEY is not configured on the server.', + code: 'realtime_not_configured', + }, { status: 500 }, ); } From f5cb50966d15828ed61424e525a6450718163c7a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 09:59:35 +0000 Subject: [PATCH 05/12] fix(web): annotate remaining helper return types and cover the Turnstile fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Absorbs the three items #1402 carried that this branch did not, so the two lines of work converge here rather than diverging: - `getOpenAiHeaders` declares `: Record | null` - `sdpRequest` declares `: Request`, `upstreamResponse` declares `: Response` - a test for `verifyTurnstileToken` returning `{ ok: false }` with no `error`, which the optional-field fallback added in 5e69b7e handles but nothing exercised #1402's `fetchUpstream` rewrite is deliberately not taken. It guards only the `fetch` call, leaving `await upstream.text()` outside the try — so a connection reset mid-body still escapes the route. `upstreamUnreachable` (ae2be83) wraps both, which is the stricter guarantee. `npm test` 272 passed / 1 failed — the pre-existing `billing-chat-gating` timeout (#1116), identical on the base commit. `npm run lint` and `npm run type-check` clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DSkV9zjdhpkfUSMeLhrFGy --- .../__tests__/billing-checkout-route.test.ts | 18 ++++++++++++++++++ .../__tests__/realtime-session-route.test.ts | 8 ++++++-- apps/web/src/app/api/realtime/session/route.ts | 2 +- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts b/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts index 028b74624..13800410e 100644 --- a/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts +++ b/apps/web/src/app/api/__tests__/billing-checkout-route.test.ts @@ -41,6 +41,24 @@ describe('POST /api/billing/checkout', () => { expect(createProCheckoutSession).not.toHaveBeenCalled(); }); + // `TurnstileVerifyResult.error` is optional, so an `ok: false` result carrying + // no reason must still produce a usable pair rather than `{ error: undefined }`. + it('falls back to a stable error and code when Turnstile reports no reason', async () => { + vi.mocked(verifyTurnstileToken).mockResolvedValue({ ok: false }); + const req = new NextRequest('http://localhost/api/billing/checkout', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ annual: false, turnstileToken: 'bad' }), + }); + const res = await POST(req); + + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ + error: 'turnstile_rejected', + code: 'turnstile_rejected', + }); + }); + it('rejects a malformed body with a matching code', async () => { const req = new NextRequest('http://localhost/api/billing/checkout', { method: 'POST', diff --git a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts index ca69a73d6..e723fe739 100644 --- a/apps/web/src/app/api/__tests__/realtime-session-route.test.ts +++ b/apps/web/src/app/api/__tests__/realtime-session-route.test.ts @@ -4,7 +4,7 @@ import { GET, POST } from '@/app/api/realtime/session/route'; const ORIGINAL_OPENAI_KEY = process.env.OPENAI_API_KEY; const ORIGINAL_SAFETY_IDENTIFIER = process.env.OPENAI_SAFETY_IDENTIFIER; -function sdpRequest(body: string) { +function sdpRequest(body: string): Request { return new Request('http://localhost/api/realtime/session', { method: 'POST', headers: { 'content-type': 'application/sdp' }, @@ -12,7 +12,11 @@ function sdpRequest(body: string) { }); } -function upstreamResponse(body: unknown, status = 200, headers: HeadersInit = { 'content-type': 'application/json' }) { +function upstreamResponse( + body: unknown, + status = 200, + headers: HeadersInit = { 'content-type': 'application/json' }, +): Response { return new Response(typeof body === 'string' ? body : JSON.stringify(body), { status, headers, diff --git a/apps/web/src/app/api/realtime/session/route.ts b/apps/web/src/app/api/realtime/session/route.ts index ab2277c4c..65b9e3250 100644 --- a/apps/web/src/app/api/realtime/session/route.ts +++ b/apps/web/src/app/api/realtime/session/route.ts @@ -85,7 +85,7 @@ function upstreamUnreachable( return Response.json({ error, code }, { status: 502 }); } -function getOpenAiHeaders(contentType?: string) { +function getOpenAiHeaders(contentType?: string): Record | null { const apiKey = process.env.OPENAI_API_KEY; if (!apiKey) { From 090f29775d3f471566228d981d1fdd7397e6993d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 10:02:11 +0000 Subject: [PATCH 06/12] fix(web): stop verifyTurnstileToken rejecting into an unstructured 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's review of the previous head found this and it reproduces. verifyTurnstileToken returns a TurnstileVerifyResult on every other exit, so its sole caller — the unauthenticated /api/billing/checkout route — awaits it outside any try/catch. Two awaits inside it break that contract: the siteverify fetch rejects on a transport failure, and res.json() rejects when Cloudflare answers 2xx with a truncated or non-JSON body. Either escaped POST as an unstructured framework 500 with no kaizenObserve trace and no `code` — the same disclosure-shaped hole ae2be83 closed on the realtime route. Fixed at the source rather than at the call site: the function now honours its own Promise signature, so every present and future caller is covered. The reject reason is logged server-side — undici puts the resolved host and port in it (`connect ECONNREFUSED 10.0.3.14:443`) — and collapsed into the app-authored literal `turnstile_verification_unavailable`. The route's existing 403 branch handles it unchanged, so no status code or response shape moves. Note the 403 is now also reachable when Cloudflare is merely unreachable rather than rejecting; keeping the existing status was the narrower choice, and the distinction is visible to operators in the log and to clients in the `error` literal. Five tests, verified non-vacuous — reverting the guard fails three of them: rejected fetch, rejected res.json(), and a non-rejection contract check, plus the accept and non-2xx paths. They assert the resolved host/port never rides out on the result while the reason still reaches console.error. Verified at this head: tsc clean, eslint clean, full web suite 277 passed / 1 failed — the pre-existing billing-chat-gating 5000 ms timeout (#1116), which is also intermittently joined by a second timeout under load. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jsfy3ts3U1ggGsJqFB2KSC --- .../web/src/app/api/billing/checkout/route.ts | 3 +- .../lib/billing/__tests__/turnstile.test.ts | 87 +++++++++++++++++++ apps/web/src/lib/billing/turnstile.ts | 56 +++++++----- 3 files changed, 124 insertions(+), 22 deletions(-) create mode 100644 apps/web/src/lib/billing/__tests__/turnstile.test.ts diff --git a/apps/web/src/app/api/billing/checkout/route.ts b/apps/web/src/app/api/billing/checkout/route.ts index 3c533105f..b20ec510f 100644 --- a/apps/web/src/app/api/billing/checkout/route.ts +++ b/apps/web/src/app/api/billing/checkout/route.ts @@ -21,7 +21,8 @@ export async function POST(req: NextRequest) { decision: turnstile.error ?? 'unknown', }); // `turnstile.error` is always one of our own literals (`turnstile_not_configured`, - // `turnstile_token_missing`, `siteverify_http_`, `turnstile_verification_failed`) + // `turnstile_token_missing`, `siteverify_http_`, `turnstile_verification_failed`, + // `turnstile_verification_unavailable`) // — never Cloudflare response text — so it is safe to return. It is kept verbatim // for client compatibility; `code` is the stable machine-readable key, and the // fallback covers the `ok: false` shape that carries no `error`. diff --git a/apps/web/src/lib/billing/__tests__/turnstile.test.ts b/apps/web/src/lib/billing/__tests__/turnstile.test.ts new file mode 100644 index 000000000..883c82b0a --- /dev/null +++ b/apps/web/src/lib/billing/__tests__/turnstile.test.ts @@ -0,0 +1,87 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { verifyTurnstileToken } from '@/lib/billing/turnstile'; + +const ORIGINAL_SECRET = process.env.TURNSTILE_SECRET_KEY; + +afterEach(() => { + if (ORIGINAL_SECRET === undefined) delete process.env.TURNSTILE_SECRET_KEY; + else process.env.TURNSTILE_SECRET_KEY = ORIGINAL_SECRET; + + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe('verifyTurnstileToken', () => { + it('reports ok when Cloudflare accepts the token', async () => { + process.env.TURNSTILE_SECRET_KEY = 'test-secret'; + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue(new Response(JSON.stringify({ success: true }), { status: 200 })), + ); + + await expect(verifyTurnstileToken('token')).resolves.toEqual({ ok: true }); + }); + + it('surfaces a non-2xx siteverify status as an app-authored literal', async () => { + process.env.TURNSTILE_SECRET_KEY = 'test-secret'; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('', { status: 503 }))); + + await expect(verifyTurnstileToken('token')).resolves.toEqual({ + ok: false, + error: 'siteverify_http_503', + }); + }); +}); + +/** + * Every other exit returns a TurnstileVerifyResult, so the sole caller — the + * unauthenticated /api/billing/checkout route — treats this as non-rejecting. + * A reject escapes it as an unstructured framework 500 with no kaizenObserve + * trace, and undici's reason carries the resolved host and port. + */ +describe('verifyTurnstileToken transport failures', () => { + it('resolves to a static result when siteverify is unreachable', async () => { + process.env.TURNSTILE_SECRET_KEY = 'test-secret'; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal( + 'fetch', + vi.fn().mockRejectedValue(new TypeError('fetch failed: connect ECONNREFUSED 10.0.3.14:443')), + ); + + const result = await verifyTurnstileToken('token'); + + expect(result).toEqual({ ok: false, error: 'turnstile_verification_unavailable' }); + // The resolved host and port must not ride out on the result... + expect(JSON.stringify(result)).not.toContain('10.0.3.14'); + expect(JSON.stringify(result)).not.toContain('ECONNREFUSED'); + // ...but operators still get the reason server-side. Asserted on the logged + // argument itself: JSON.stringify would render an Error as `{}`, since its + // properties are non-enumerable, and would pass vacuously. + const [label, logged] = consoleError.mock.calls[0]; + expect(label).toContain('turnstile siteverify unreachable'); + expect((logged as Error).message).toContain('ECONNREFUSED 10.0.3.14:443'); + }); + + it('resolves to a static result when the siteverify body is unreadable', async () => { + process.env.TURNSTILE_SECRET_KEY = 'test-secret'; + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + const truncated = new Response('', { status: 200 }); + vi.spyOn(truncated, 'json').mockRejectedValue(new Error('Unexpected end of JSON input')); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(truncated)); + + const result = await verifyTurnstileToken('token'); + + expect(result).toEqual({ ok: false, error: 'turnstile_verification_unavailable' }); + expect((consoleError.mock.calls[0][1] as Error).message).toContain( + 'Unexpected end of JSON input', + ); + }); + + it('never rejects, so the checkout route always reaches its 403 branch', async () => { + process.env.TURNSTILE_SECRET_KEY = 'test-secret'; + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('socket hang up'))); + + await expect(verifyTurnstileToken('token')).resolves.toMatchObject({ ok: false }); + }); +}); diff --git a/apps/web/src/lib/billing/turnstile.ts b/apps/web/src/lib/billing/turnstile.ts index 622c1974a..2b08553f8 100644 --- a/apps/web/src/lib/billing/turnstile.ts +++ b/apps/web/src/lib/billing/turnstile.ts @@ -21,28 +21,42 @@ export async function verifyTurnstileToken( body.set('remoteip', remoteIp); } - const res = await fetch('https://challenges.cloudflare.com/turnstile/v0/siteverify', { - method: 'POST', - headers: { 'content-type': 'application/x-www-form-urlencoded' }, - body, - }); - - if (!res.ok) { - return { ok: false, error: `siteverify_http_${res.status}` }; - } + // Every other exit returns a TurnstileVerifyResult, so callers reasonably treat + // this function as non-rejecting. Two awaits below can break that: the siteverify + // fetch rejects on a transport failure (undici puts the resolved host and port in + // the reason — `connect ECONNREFUSED 10.0.3.14:443`), and res.json() rejects when + // Cloudflare answers 2xx with a truncated or non-JSON body. Either would escape + // the sole caller — the unauthenticated /api/billing/checkout route — as an + // unstructured framework 500 with no kaizenObserve trace. The reason is logged + // server-side and collapsed into an app-authored literal. + try { + const res = await fetch('https://challenges.cloudflare.com/turnstile/v0/siteverify', { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body, + }); - const data = (await res.json()) as { - success?: boolean; - 'error-codes'?: string[]; - }; + if (!res.ok) { + return { ok: false, error: `siteverify_http_${res.status}` }; + } - if (data.success) { - return { ok: true }; - } + const data = (await res.json()) as { + success?: boolean; + 'error-codes'?: string[]; + }; + + if (data.success) { + return { ok: true }; + } - return { - ok: false, - error: 'turnstile_verification_failed', - errorCodes: data['error-codes'], - }; + return { + ok: false, + error: 'turnstile_verification_failed', + errorCodes: data['error-codes'], + }; + } catch (err) { + console.error('[billing] turnstile siteverify unreachable:', err); + + return { ok: false, error: 'turnstile_verification_unavailable' }; + } } \ No newline at end of file From e185c4b0dfb0eba58e7a80b3a970c0ed9af25018 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 10:09:38 +0000 Subject: [PATCH 07/12] fix(web): complete the error-code contract on the transcribe route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit flagged the `!result.success` branches of /api/transcribe as a CWE-209 leak. Traced every value `fetchTranscript` can put in `error` before changing anything: each one is an app-authored literal (transcription-service 75, 296, 303, 314), a numeric HTTP status (287), our own SSRF-guard message (278, and every throw in ssrf-guard interpolates only the caller's own URL), or a conditional literal (363). Upstream throws are swallowed at console.warn and never reach `error`. So the leak half of that finding does not hold. The contract half does. This PR established `error` + a stable machine-readable `code` on every error branch of the five billing routes and realtime/session; transcribe was the last surface still missing it. The docblock documented the gap instead of closing it. All five error branches now carry a code — input_required, rate_limited, billing_not_configured, transcription_unavailable, alongside the existing invalid_json and transcription_failed. The `error` strings are untouched, so the human-facing contract is unchanged, matching the precedent already accepted for the Turnstile branch: dynamic message, pinned code. The `details` hint on the billing branch is kept. It is an app-authored operator string, not upstream text; the docblock no longer claims otherwise. Tests: +5. One asserting the missing-input branch short-circuits before fetchTranscript, and four table-driven cases pinning each resolved-failure branch's status and code while asserting no provider-shaped token reaches the client. Also added vi.clearAllMocks() to the afterEach — restoreAllMocks only restores spies, so the module-factory vi.fn() was leaking call history between tests in this file. apps/web: type-check clean, eslint clean, 282 passed / 1 failed — the pre-existing billing-chat-gating timeout tracked in #1116. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y1xcdTagvCU4K7ZwTgGQF6 --- .../api/__tests__/transcribe-route.test.ts | 77 +++++++++++++++++++ apps/web/src/app/api/transcribe/route.ts | 29 ++++--- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/apps/web/src/app/api/__tests__/transcribe-route.test.ts b/apps/web/src/app/api/__tests__/transcribe-route.test.ts index f48937702..1e2be7575 100644 --- a/apps/web/src/app/api/__tests__/transcribe-route.test.ts +++ b/apps/web/src/app/api/__tests__/transcribe-route.test.ts @@ -9,6 +9,9 @@ import { fetchTranscript } from '@/lib/transcription-service'; afterEach(() => { vi.restoreAllMocks(); + // restoreAllMocks only restores spies; the module-factory vi.fn() keeps its + // call history, which would leak across tests in this file. + vi.clearAllMocks(); }); function transcribeRequest(body: string): Request { @@ -68,3 +71,77 @@ describe('POST /api/transcribe error leakage', () => { expect(consoleError).toHaveBeenCalled(); }); }); + +describe('POST /api/transcribe error-code contract', () => { + it('returns input_required when neither url nor audioUrl is supplied', async () => { + const res = await POST(transcribeRequest(JSON.stringify({ language: 'en' }))); + const body = await res.json(); + + expect(res.status).toBe(400); + expect(body).toMatchObject({ + success: false, + error: 'Either url or audioUrl is required', + code: 'input_required', + }); + // fetchTranscript must not be reached on a missing-input request. + expect(vi.mocked(fetchTranscript)).not.toHaveBeenCalled(); + }); + + // Every resolved-failure branch keeps fetchTranscript's app-authored message in + // `error` and gains a stable `code`. The mocked messages below carry provider- + // shaped tokens purely to prove none of them can reach the client via `code`, + // `details` or any other added field. + const resolvedFailures = [ + { + name: 'input_required', + serviceError: 'url or audioUrl is required', + status: 400, + code: 'input_required', + }, + { + name: 'rate_limited', + serviceError: 'OpenAI rate limit reached', + status: 429, + code: 'rate_limited', + }, + { + name: 'billing_not_configured', + serviceError: 'Gemini billing is not enabled', + status: 500, + code: 'billing_not_configured', + }, + { + name: 'transcription_unavailable', + serviceError: 'Could not transcribe video — all strategies failed', + status: 503, + code: 'transcription_unavailable', + }, + ] as const; + + for (const branch of resolvedFailures) { + it(`returns ${branch.name} with the service message preserved`, async () => { + vi.mocked(fetchTranscript).mockResolvedValue({ + success: false, + error: branch.serviceError, + transcript: '', + }); + + const res = await POST( + transcribeRequest(JSON.stringify({ url: 'https://www.youtube.com/watch?v=auJzb1D-fag' })), + ); + const raw = await res.text(); + const body = JSON.parse(raw); + + expect(res.status).toBe(branch.status); + expect(body.code).toBe(branch.code); + expect(body.success).toBe(false); + expect(body.transcript).toBe(''); + // The human-facing string contract is unchanged. + expect(body.error).toBe(branch.serviceError); + // No provider credential material is introduced by the added fields. + for (const token of ['sk-proj', 'sk_live', 'org-eventrelay-prod', 'Bearer ']) { + expect(raw).not.toContain(token); + } + }); + } +}); diff --git a/apps/web/src/app/api/transcribe/route.ts b/apps/web/src/app/api/transcribe/route.ts index b05c88d3c..ef63574f0 100644 --- a/apps/web/src/app/api/transcribe/route.ts +++ b/apps/web/src/app/api/transcribe/route.ts @@ -14,15 +14,20 @@ export const maxDuration = 120; * 3. OpenAI Responses API with web search - with rate limit handling * 4. Direct audio STT via OpenAI Whisper * - * The JSON-parse and catch-all branches return static, machine-readable payloads - * (`code` + fixed `error`); raw upstream provider text is logged server-side only, - * since it can carry account IDs and partial API keys. The `!result.success` - * branches below return `fetchTranscript`'s own app-authored message verbatim — - * these are not upstream text, and they carry no `code`: - * - 400: Invalid input (missing URL, malformed JSON) - * - 429: Rate limited (implement backoff or upgrade service plan) - * - 500: Service unavailable (check API keys and billing) - * - 503: Cascading failures (all fallback strategies exhausted) + * Every error response carries a stable machine-readable `code`. The JSON-parse + * and catch-all branches also fix their `error` string, because those paths can + * carry raw upstream provider text (account IDs, quota state, partial API keys) + * — that text is logged server-side only. + * + * The `!result.success` branches keep `fetchTranscript`'s own message in `error`: + * every one of those values is an app-authored literal, a numeric HTTP status, or + * our own SSRF-guard message, so none is upstream text. Preserving them verbatim + * keeps the human-facing string contract intact while `code` carries the stable + * key for programmatic handling. + * - 400: Invalid input (missing URL, malformed JSON) — `input_required` / `invalid_json` + * - 429: Rate limited (implement backoff or upgrade service plan) — `rate_limited` + * - 500: Service unavailable (check API keys and billing) — `billing_not_configured` + * - 503: Cascading failures (all fallback strategies exhausted) — `transcription_unavailable` */ export async function POST(request: Request) { try { @@ -50,6 +55,7 @@ export async function POST(request: Request) { { success: false, error: 'Either url or audioUrl is required', + code: 'input_required', accepted_fields: ['url', 'audioUrl', 'language'], transcript: '', }, @@ -68,7 +74,7 @@ export async function POST(request: Request) { // Distinguish error severity for appropriate HTTP status if (result.error?.includes('url or audioUrl')) { return NextResponse.json( - { success: false, error: result.error, transcript: '' }, + { success: false, error: result.error, code: 'input_required', transcript: '' }, { status: 400 } ); } else if (result.error?.includes('rate limit')) { @@ -76,6 +82,7 @@ export async function POST(request: Request) { { success: false, error: result.error, + code: 'rate_limited', retry_after: 60, transcript: '', }, @@ -86,6 +93,7 @@ export async function POST(request: Request) { { success: false, error: result.error, + code: 'billing_not_configured', details: 'Configure billing in Google Cloud and OpenAI console', transcript: '', }, @@ -98,6 +106,7 @@ export async function POST(request: Request) { { success: false, error: result.error || 'All transcription strategies failed', + code: 'transcription_unavailable', transcript: '', }, { status: 503 } From 36645adb02fc9fe443eb43decfd2c9d4c702424b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 10:14:22 +0000 Subject: [PATCH 08/12] fix(web): stop the SSRF guard leaking DNS resolver text to the caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit found the last CWE-209 path on this PR's surface, and it is a real one. `assertPublicHttpUrl` throws app-authored literals at every exit except `dns.lookup`, which was awaited bare: ssrf-guard.ts:121 await dns.lookup(host, { all: true }) -> rejects `getaddrinfo ENOTFOUND ` transcription-service.ts:273-280 -> `Rejected audioUrl: ${guardErr.message}` transcribe/route.ts:104-113 -> returns that verbatim as `error` in a 503 So a caller could name any host and read back the server's resolver verdict on it. That is system error text reaching a client, which is this PR's whole objective — but the sharper problem is that it is a DNS oracle. ENOTFOUND vs EAI_AGAIN vs the existing 'Host resolves to a private address' distinguishes "no such name" from "exists but private" from "exists and public", letting a caller map internal names from the server's network position. That is exactly the reconnaissance this guard exists to block, leaking out of the guard. Fixed at the source rather than at either call site, so present and future callers of `assertPublicHttpUrl` are covered: the lookup is wrapped, the reject reason is logged server-side, and every resolver failure collapses into 'Host does not resolve' — the literal a zero-result lookup already threw, so the oracle closes in both directions. This also corrects a claim made earlier on this PR. The transcribe docblock thread asserted every `result.error` value was app-authored and listed line 278 as "our own SSRF-guard message". That held for the five explicit `throw new Error(...)` sites but not for the `dns.lookup` rejection, which is Node's. The docblock scoping stands; the reasoning behind it was incomplete. Severity is bounded: `/api/transcribe` is not on `PUBLIC_API_EXACT`, so this needs an authenticated caller, unlike the billing routes. Four tests, verified non-vacuous — reverting the try/catch fails exactly the two leakage tests (resolver text suppressed but still logged; ENOTFOUND and EAI_AGAIN indistinguishable from an empty result) while the two behaviour tests still pass, since private-address blocking and the public-host allow path do not depend on the catch. Verified at this head: tsc clean, eslint clean, web suite 286 passed / 1 failed — the pre-existing billing-chat-gating 5000 ms timeout (#1116). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DSkV9zjdhpkfUSMeLhrFGy --- .../__tests__/ssrf-guard-dns-leakage.test.ts | 89 +++++++++++++++++++ apps/web/src/lib/ssrf-guard.ts | 19 +++- 2 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts diff --git a/apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts b/apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts new file mode 100644 index 000000000..e6d57a2da --- /dev/null +++ b/apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts @@ -0,0 +1,89 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; + +vi.mock('node:dns/promises', () => ({ + lookup: vi.fn(), +})); + +import * as dns from 'node:dns/promises'; +import { assertPublicHttpUrl } from '@/lib/ssrf-guard'; + +afterEach(() => { + vi.restoreAllMocks(); + vi.mocked(dns.lookup).mockReset(); +}); + +/** + * `assertPublicHttpUrl` throws app-authored literals everywhere except the + * `dns.lookup` await, which rejects with Node resolver text embedding the + * caller's own hostname. `fetchTranscript` interpolates that into + * `Rejected audioUrl: ${guardErr.message}` and `/api/transcribe` returns it + * verbatim in its 503 — so a resolver reason reaching the caller is both + * system-error disclosure and a DNS oracle over the server's network position. + */ +describe('assertPublicHttpUrl DNS failure leakage', () => { + const PROBED_HOST = 'vault.internal-corp.example'; + + it('does not surface resolver text when the lookup rejects', async () => { + const resolverError = Object.assign( + new Error(`getaddrinfo ENOTFOUND ${PROBED_HOST}`), + { code: 'ENOTFOUND', syscall: 'getaddrinfo', hostname: PROBED_HOST }, + ); + vi.mocked(dns.lookup).mockRejectedValue(resolverError); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + + await expect(assertPublicHttpUrl(`https://${PROBED_HOST}/clip.mp3`)).rejects.toThrow( + 'Host does not resolve', + ); + + const thrown = await assertPublicHttpUrl(`https://${PROBED_HOST}/clip.mp3`).catch( + (e: unknown) => (e as Error).message, + ); + for (const token of ['getaddrinfo', 'ENOTFOUND', PROBED_HOST]) { + expect(thrown).not.toContain(token); + } + + // Suppressed for the caller, retained for operators. + expect(JSON.stringify(consoleError.mock.calls)).toContain('ENOTFOUND'); + }); + + it('is indistinguishable from a transient resolver failure', async () => { + // ENOTFOUND vs EAI_AGAIN would tell a caller whether the name exists but is + // merely unreachable — the oracle. Both must collapse to one message. + vi.mocked(dns.lookup).mockRejectedValue( + Object.assign(new Error(`getaddrinfo EAI_AGAIN ${PROBED_HOST}`), { code: 'EAI_AGAIN' }), + ); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + const transient = await assertPublicHttpUrl(`https://${PROBED_HOST}/a.mp3`).catch( + (e: unknown) => (e as Error).message, + ); + + vi.mocked(dns.lookup).mockResolvedValue([] as never); + const empty = await assertPublicHttpUrl(`https://${PROBED_HOST}/a.mp3`).catch( + (e: unknown) => (e as Error).message, + ); + + expect(transient).toBe('Host does not resolve'); + expect(transient).toBe(empty); + }); + + it('still blocks hosts that resolve to a private address', async () => { + vi.mocked(dns.lookup).mockResolvedValue([ + { address: '10.0.3.14', family: 4 }, + ] as never); + + await expect(assertPublicHttpUrl('https://public-looking.example/a.mp3')).rejects.toThrow( + 'Host resolves to a private address', + ); + }); + + it('still allows a genuinely public host', async () => { + vi.mocked(dns.lookup).mockResolvedValue([ + { address: '93.184.216.34', family: 4 }, + ] as never); + + const url = await assertPublicHttpUrl('https://example.com/a.mp3'); + + expect(url.hostname).toBe('example.com'); + }); +}); diff --git a/apps/web/src/lib/ssrf-guard.ts b/apps/web/src/lib/ssrf-guard.ts index cc315a0cd..46d679c1e 100644 --- a/apps/web/src/lib/ssrf-guard.ts +++ b/apps/web/src/lib/ssrf-guard.ts @@ -12,6 +12,7 @@ import 'server-only'; * Host header. This is a strong, low-cost first line of defense. */ import * as dns from 'node:dns/promises'; +import type { LookupAddress } from 'node:dns'; import * as net from 'node:net'; const BLOCKED_HOSTNAMES = new Set(['localhost', 'metadata.google.internal']); @@ -118,7 +119,23 @@ export async function assertPublicHttpUrl(input: string): Promise { if (ipIsPrivate(host)) throw new Error('Blocked private IP literal'); return u; } - const resolved = await dns.lookup(host, { all: true }); + // Every other throw here is an app-authored literal, but `dns.lookup` rejects + // with Node resolver text that embeds the caller's own hostname — e.g. + // `getaddrinfo ENOTFOUND evil.internal.corp`. `fetchTranscript` interpolates + // that message into `Rejected audioUrl: ${guardErr.message}`, which the + // transcribe route returns verbatim, so the reject reason would reach the + // client. Beyond disclosing system error text, distinguishing ENOTFOUND from + // EAI_AGAIN from 'Host resolves to a private address' turns this guard into a + // DNS oracle for probing internal names from the server's network position — + // the reconnaissance the guard exists to block. Collapse every resolver + // failure into the same app-authored literal a zero-result lookup produces. + let resolved: LookupAddress[]; + try { + resolved = await dns.lookup(host, { all: true }); + } catch (err) { + console.error('[ssrf-guard] DNS lookup failed:', err); + throw new Error('Host does not resolve'); + } if (resolved.length === 0) throw new Error('Host does not resolve'); for (const r of resolved) { if (ipIsPrivate(r.address)) throw new Error('Host resolves to a private address'); From 020fad82a3c059251b0a44f85a263e410edde5dd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 18:04:42 +0000 Subject: [PATCH 09/12] fix(web): close the last DNS oracle in the SSRF guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vercel's review bot found this on 8628b5e and it is correct — 36645ad closed only half the hole it described. That commit collapsed the resolver-failure cases (ENOTFOUND, EAI_AGAIN, zero results) into one literal, but left the private-address branch throwing a distinct one: Host does not resolve <- the name does not exist Host resolves to a private address <- the name EXISTS, and is internal That difference is the sharpest oracle of the set. `fetchTranscript` interpolates the message into `Rejected audioUrl: ${guardErr.message}` (transcription-service.ts:278) and the transcribe route returns it verbatim on the `!result.success` path, so an unauthenticated caller could guess internal hostnames and read back whether each one exists and points at internal infrastructure — the reconnaissance this guard exists to prevent. 36645ad's own comment names "Host resolves to a private address" as part of the oracle and then does not unify it. All four non-public outcomes now throw one literal, `Host does not resolve to a public address`, which is true of every one of them. The resolved address is logged for operators instead. The IP-literal branch keeps its own distinct message deliberately: the caller supplied that address, so naming it private reveals nothing they did not already know, and no hostname is confirmed or denied. Two tests, verified non-vacuous — restoring the old literal fails both: one asserts the private address never rides out on the rejection while still reaching console.error, the other feeds the same hostname down the does-not-exist and exists-but-internal paths and asserts the caller cannot tell them apart. npm test 287 passed / 1 failed — the pre-existing billing-chat-gating timeout (#1116). lint and type-check clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DSkV9zjdhpkfUSMeLhrFGy --- .../__tests__/ssrf-guard-dns-leakage.test.ts | 45 +++++++++++++++++-- apps/web/src/lib/ssrf-guard.ts | 31 +++++++++---- 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts b/apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts index e6d57a2da..bf6701278 100644 --- a/apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts +++ b/apps/web/src/lib/__tests__/ssrf-guard-dns-leakage.test.ts @@ -32,7 +32,7 @@ describe('assertPublicHttpUrl DNS failure leakage', () => { const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); await expect(assertPublicHttpUrl(`https://${PROBED_HOST}/clip.mp3`)).rejects.toThrow( - 'Host does not resolve', + 'Host does not resolve to a public address', ); const thrown = await assertPublicHttpUrl(`https://${PROBED_HOST}/clip.mp3`).catch( @@ -63,7 +63,7 @@ describe('assertPublicHttpUrl DNS failure leakage', () => { (e: unknown) => (e as Error).message, ); - expect(transient).toBe('Host does not resolve'); + expect(transient).toBe('Host does not resolve to a public address'); expect(transient).toBe(empty); }); @@ -71,10 +71,47 @@ describe('assertPublicHttpUrl DNS failure leakage', () => { vi.mocked(dns.lookup).mockResolvedValue([ { address: '10.0.3.14', family: 4 }, ] as never); + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const message = await assertPublicHttpUrl('https://public-looking.example/a.mp3').catch( + (e: unknown) => (e as Error).message, + ); + + expect(message).toBe('Host does not resolve to a public address'); + // The resolved private address must not ride out on the rejection… + expect(message).not.toContain('10.0.3.14'); + expect(message).not.toContain('private'); + // …but operators still get it. + expect(JSON.stringify(consoleError.mock.calls)).toContain('10.0.3.14'); + }); + + /** + * The sharpest oracle of the set: a caller who can tell "this name does not + * exist" from "this name exists and points somewhere internal" can enumerate + * internal hostnames through an unauthenticated endpoint, which is the + * reconnaissance the guard exists to prevent. Every outcome that is not a + * public address must be one indistinguishable message. + */ + it('does not reveal whether a probed internal name exists', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + + // A name that does not exist at all. + vi.mocked(dns.lookup).mockRejectedValue( + Object.assign(new Error(`getaddrinfo ENOTFOUND ${PROBED_HOST}`), { code: 'ENOTFOUND' }), + ); + const absent = await assertPublicHttpUrl(`https://${PROBED_HOST}/a.mp3`).catch( + (e: unknown) => (e as Error).message, + ); - await expect(assertPublicHttpUrl('https://public-looking.example/a.mp3')).rejects.toThrow( - 'Host resolves to a private address', + // A name that DOES exist and points at internal infrastructure. + vi.mocked(dns.lookup).mockResolvedValue([ + { address: '10.0.3.14', family: 4 }, + ] as never); + const present = await assertPublicHttpUrl(`https://${PROBED_HOST}/a.mp3`).catch( + (e: unknown) => (e as Error).message, ); + + expect(present).toBe(absent); }); it('still allows a genuinely public host', async () => { diff --git a/apps/web/src/lib/ssrf-guard.ts b/apps/web/src/lib/ssrf-guard.ts index 46d679c1e..8d5a0e6a9 100644 --- a/apps/web/src/lib/ssrf-guard.ts +++ b/apps/web/src/lib/ssrf-guard.ts @@ -124,21 +124,36 @@ export async function assertPublicHttpUrl(input: string): Promise { // `getaddrinfo ENOTFOUND evil.internal.corp`. `fetchTranscript` interpolates // that message into `Rejected audioUrl: ${guardErr.message}`, which the // transcribe route returns verbatim, so the reject reason would reach the - // client. Beyond disclosing system error text, distinguishing ENOTFOUND from - // EAI_AGAIN from 'Host resolves to a private address' turns this guard into a - // DNS oracle for probing internal names from the server's network position — - // the reconnaissance the guard exists to block. Collapse every resolver - // failure into the same app-authored literal a zero-result lookup produces. + // client. + // + // Beyond disclosing system error text, ANY difference between these outcomes + // is a DNS oracle. "Does not resolve" versus "resolves to a private address" + // is the sharpest one: it answers, for an unauthenticated caller, whether a + // guessed internal name EXISTS and points at internal infrastructure. That is + // precisely the reconnaissance this guard exists to block, so all four + // outcomes — resolver rejection, transient failure, zero results, and a + // private result — must be indistinguishable to the caller. The real reason + // is logged for operators instead. + // + // The IP-literal branch above keeps its own message deliberately: the caller + // supplied the address, so telling them it is private reveals nothing they + // did not already know, and no name is confirmed or denied. + const NOT_PUBLIC = 'Host does not resolve to a public address'; let resolved: LookupAddress[]; try { resolved = await dns.lookup(host, { all: true }); } catch (err) { console.error('[ssrf-guard] DNS lookup failed:', err); - throw new Error('Host does not resolve'); + throw new Error(NOT_PUBLIC); } - if (resolved.length === 0) throw new Error('Host does not resolve'); + if (resolved.length === 0) throw new Error(NOT_PUBLIC); for (const r of resolved) { - if (ipIsPrivate(r.address)) throw new Error('Host resolves to a private address'); + if (ipIsPrivate(r.address)) { + console.error( + `[ssrf-guard] host resolved to a non-public address: ${host} -> ${r.address}`, + ); + throw new Error(NOT_PUBLIC); + } } return u; } From 18a787a55be41ffe3a61ff3a28f51a819c22e985 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 19:09:45 +0000 Subject: [PATCH 10/12] fix(web): stop /api/transcribe leaking probe status and key config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1381 sanitized the thrown and JSON-parse paths of /api/transcribe but left the `!result.success` branches returning `fetchTranscript`'s message verbatim, on the stated grounds that every such value is an app-authored literal. Two were not, and both are reachable by a caller who controls the request body: - `Failed to fetch audio: ${audioResponse.status}` echoed the HTTP status of the caller-supplied `audioUrl`. The SSRF guard admits public hosts, so this returned a cross-origin read (401 vs 403 vs 404 vs 500) that the browser's same-origin policy would otherwise deny — a probe oracle, not upstream text, which is why it survived the original pass. - The all-strategies-failed message branched on whether provider keys were set, disclosing server configuration and naming both the variables and the hosting platform. Both are fixed at the source in transcription-service.ts rather than masked at the route, so any future caller of fetchTranscript inherits the fix. The real status and the real key state are logged for operators, where they were the only thing actionable. Also drops the residual `details` field from the billing branch, which named this deployment's cloud and model vendors — #1381 stated `details` was gone from this route, and this is the one that was left. The route docblock carried the claim these values disproved; it now states the invariant and what enforces it. Verification (head e9bf62d4, apps/web): - 3 new tests in transcription-error-disclosure.test.ts; all 3 fail against the unfixed service and pass with it — the status test asserts four distinct upstream statuses collapse to one message, so a partial fix cannot pass it. - vitest run: 1 failed | 290 passed (291). The single failure, billing-chat-gating "blocks free tier after daily quota", is pre-existing and unrelated — confirmed failing identically on the untouched e9bf62d4 with this change stashed. Tracked in #1116; #1230 makes the suite hermetic against it. - npm run type-check: clean. npm run lint: clean. - package-lock.json drift from npm install reverted, per #1381's convention. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EX3tLZJJfbDUM6XXi6Uefm --- apps/web/src/app/api/transcribe/route.ts | 20 +++- .../transcription-error-disclosure.test.ts | 104 ++++++++++++++++++ apps/web/src/lib/transcription-service.ts | 28 ++++- 3 files changed, 141 insertions(+), 11 deletions(-) create mode 100644 apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts diff --git a/apps/web/src/app/api/transcribe/route.ts b/apps/web/src/app/api/transcribe/route.ts index ef63574f0..e77251521 100644 --- a/apps/web/src/app/api/transcribe/route.ts +++ b/apps/web/src/app/api/transcribe/route.ts @@ -19,11 +19,17 @@ export const maxDuration = 120; * carry raw upstream provider text (account IDs, quota state, partial API keys) * — that text is logged server-side only. * - * The `!result.success` branches keep `fetchTranscript`'s own message in `error`: - * every one of those values is an app-authored literal, a numeric HTTP status, or - * our own SSRF-guard message, so none is upstream text. Preserving them verbatim - * keeps the human-facing string contract intact while `code` carries the stable - * key for programmatic handling. + * The `!result.success` branches keep `fetchTranscript`'s own message in `error`. + * That is safe only because every one of those values is now a fixed, + * app-authored literal. Two of them were not, and were removed at the source in + * `transcription-service.ts` rather than masked here: + * - the caller-supplied `audioUrl`'s HTTP status, which made this route a + * cross-origin probe, and + * - a message that varied on whether provider API keys were configured, which + * disclosed server configuration. + * Both are logged server-side instead. Keep that invariant when adding a + * strategy: anything interpolated into a `fetchTranscript` error reaches the + * client verbatim through the branches below. * - 400: Invalid input (missing URL, malformed JSON) — `input_required` / `invalid_json` * - 429: Rate limited (implement backoff or upgrade service plan) — `rate_limited` * - 500: Service unavailable (check API keys and billing) — `billing_not_configured` @@ -94,7 +100,9 @@ export async function POST(request: Request) { success: false, error: result.error, code: 'billing_not_configured', - details: 'Configure billing in Google Cloud and OpenAI console', + // No `details` here: the remediation text named this deployment's + // cloud and model vendors, and the PR that sanitized this route + // stated `details` was gone from it. `code` is what clients branch on. transcript: '', }, { status: 500 } diff --git a/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts b/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts new file mode 100644 index 000000000..c4bbc7120 --- /dev/null +++ b/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts @@ -0,0 +1,104 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; + +// `fetchTranscript`'s error string is returned to the caller verbatim by +// `/api/transcribe`'s `!result.success` branches. That is only safe while every +// value it can carry is a fixed, app-authored literal. These tests pin the two +// values that were not: the caller-supplied `audioUrl`'s HTTP status, and a +// message that varied on whether provider API keys were configured. + +vi.mock('server-only', () => ({})); +vi.mock('@/lib/ssrf-guard', () => ({ + assertPublicHttpUrl: vi.fn(async (input: string) => new URL(input)), +})); +vi.mock('@/lib/youtube-metadata', () => ({ + fetchYouTubeMetadata: vi.fn(async () => null), + formatMetadataAsContext: vi.fn(() => ''), +})); +vi.mock('@/lib/gemini-client', () => ({ + getGeminiClient: vi.fn(() => null), + hasGeminiKey: vi.fn(() => false), +})); +vi.mock('@/lib/vercel-ai-gateway', () => ({ + gatewayChat: vi.fn(async () => null), + hasAiGatewayKey: vi.fn(() => false), + toGatewayModelId: vi.fn((m: string) => m), +})); + +import { fetchTranscript } from '@/lib/transcription-service'; + +const AUDIO_URL = 'https://cdn.example.com/clip.mp3'; + +let consoleError: ReturnType; +const savedEnv = { ...process.env }; + +beforeEach(() => { + consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + process.env = { ...savedEnv }; +}); + +describe('fetchTranscript error disclosure', () => { + it('does not echo the status of a caller-supplied audioUrl', async () => { + process.env.OPENAI_API_KEY = 'sk-test-key'; + // 403 from a host the caller chose. Echoing it back is a cross-origin read + // the browser's same-origin policy would otherwise deny them. + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('forbidden', { status: 403 })), + ); + + const result = await fetchTranscript({ audioUrl: AUDIO_URL }); + + expect(result.success).toBe(false); + expect(result.error).toBe('Could not retrieve the audio file'); + // The probe result must not survive anywhere in the caller-visible payload. + expect(JSON.stringify(result)).not.toContain('403'); + // ...but an operator can still see it. + expect(consoleError).toHaveBeenCalledWith(expect.stringContaining('403')); + }); + + it('reports the same status-free message whatever the upstream status is', async () => { + process.env.OPENAI_API_KEY = 'sk-test-key'; + const errors: string[] = []; + + for (const status of [401, 403, 404, 500]) { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('nope', { status })), + ); + const result = await fetchTranscript({ audioUrl: AUDIO_URL }); + errors.push(result.error ?? ''); + } + + // One distinguishable message per status would rebuild the oracle. + expect(new Set(errors).size).toBe(1); + }); + + it('does not disclose whether provider API keys are configured', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('{}', { status: 500 })), + ); + + delete process.env.OPENAI_API_KEY; + const withoutKeys = await fetchTranscript({ url: 'https://youtu.be/dQw4w9WgXcQ' }); + + process.env.OPENAI_API_KEY = 'sk-test-key'; + const withKeys = await fetchTranscript({ url: 'https://youtu.be/dQw4w9WgXcQ' }); + + expect(withoutKeys.success).toBe(false); + expect(withKeys.success).toBe(false); + // Configuration state is not a caller-facing fact. + expect(withoutKeys.error).toBe(withKeys.error); + expect(withoutKeys.error).not.toMatch(/OPENAI_API_KEY|GEMINI_API_KEY|Vercel/i); + // The distinction stays available to operators. + expect(consoleError).toHaveBeenCalledWith( + expect.stringContaining('no OPENAI_API_KEY or GEMINI_API_KEY configured'), + ); + }); +}); diff --git a/apps/web/src/lib/transcription-service.ts b/apps/web/src/lib/transcription-service.ts index 06333497c..fed35d6bc 100644 --- a/apps/web/src/lib/transcription-service.ts +++ b/apps/web/src/lib/transcription-service.ts @@ -282,9 +282,17 @@ ${metadataContext ? `\nKNOWN METADATA:\n${metadataContext}` : ''}`, const audioResponse = await fetch(audioUrl, { signal: AbortSignal.timeout(30_000) }); if (!audioResponse.ok) { + // The status belongs to a caller-supplied `audioUrl`. Echoing it turns + // this route into a probe: the caller learns 401 vs 403 vs 404 vs 500 + // for any host the SSRF guard admits, which is a cross-origin read the + // browser same-origin policy would otherwise deny them. Log it, and + // report only that the fetch failed. + console.error( + `[transcription] audioUrl fetch failed with status ${audioResponse.status}`, + ); return { success: false, - error: `Failed to fetch audio: ${audioResponse.status}`, + error: 'Could not retrieve the audio file', transcript: '', }; } @@ -356,13 +364,23 @@ ${metadataContext ? `\nKNOWN METADATA:\n${metadataContext}` : ''}`, } } - // No strategy succeeded + // No strategy succeeded. + // + // Which provider keys this deployment holds is server configuration, not + // caller-facing detail: branching the client message on `hasKeys` told any + // caller whether OPENAI_API_KEY/GEMINI_API_KEY were set, and named the + // variables and the hosting platform. Both outcomes now report the same + // string; the distinction survives in the operator log, which is the only + // place it was ever actionable. const hasKeys = !!(process.env.OPENAI_API_KEY || hasGeminiKey()); + console.error( + hasKeys + ? '[transcription] all strategies failed with provider keys configured' + : '[transcription] all strategies failed: no OPENAI_API_KEY or GEMINI_API_KEY configured', + ); return { success: false, - error: hasKeys - ? 'Could not transcribe video — all strategies failed' - : 'No AI API key configured. Set OPENAI_API_KEY or GEMINI_API_KEY in Vercel environment variables.', + error: 'Could not transcribe video — all strategies failed', transcript: '', }; } From 8d756b606449db5a675e9f574fde1950711bb824 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 19:14:16 +0000 Subject: [PATCH 11/12] test: use the repo's standard fixture video ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new disclosure tests used dQw4w9WgXcQ. `.github/copilot-instructions.md` requires auJzb1D-fag for all test data, and 60 files follow it against 6 stragglers — this file was one I had just added to that tail. Flagged by Vercel VADE review on #1440. The ID is inert here: fetch is stubbed, so the URL is never dereferenced and only reaches the strategy dispatch. 3/3 tests still pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EX3tLZJJfbDUM6XXi6Uefm --- .../src/lib/__tests__/transcription-error-disclosure.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts b/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts index c4bbc7120..a1e170b08 100644 --- a/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts +++ b/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts @@ -86,10 +86,10 @@ describe('fetchTranscript error disclosure', () => { ); delete process.env.OPENAI_API_KEY; - const withoutKeys = await fetchTranscript({ url: 'https://youtu.be/dQw4w9WgXcQ' }); + const withoutKeys = await fetchTranscript({ url: 'https://youtu.be/auJzb1D-fag' }); process.env.OPENAI_API_KEY = 'sk-test-key'; - const withKeys = await fetchTranscript({ url: 'https://youtu.be/dQw4w9WgXcQ' }); + const withKeys = await fetchTranscript({ url: 'https://youtu.be/auJzb1D-fag' }); expect(withoutKeys.success).toBe(false); expect(withKeys.success).toBe(false); From e511781603bed6086d8385ef70e0275b4dfaa850 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 19:40:40 +0000 Subject: [PATCH 12/12] fix(web): stop forwarding the SSRF guard's rejection reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raised by CodeRabbit on #1440 (P2), and correct. The previous commit's docblock asserted that every `fetchTranscript` error value reaching the client is a fixed app-authored literal. That was false: the guard branch returned `Rejected audioUrl: ${guardErr.message}` verbatim. `assertPublicHttpUrl` throws five distinguishable messages — `Invalid URL`, `Blocked URL scheme: `, `Blocked host`, `Blocked private IP literal`, and `Host does not resolve to a public address`. Forwarding them told the caller WHICH rule fired. `Blocked host` confirms a hostname-blocklist match; the resolution message confirms only that DNS returned nothing public. That difference is a policy oracle, and it sharpens as BLOCKED_HOSTNAMES grows. All five rejections now return one fixed `Rejected audioUrl`; the real reason is logged. With this, all seven of the service's error values are fixed literals — verified by enumeration, not assumed. The docblock overstated in the same way #1381's did, which is what this PR was opened to correct, so it has been rewritten to state what is actually true and to name what it does NOT claim: the choice among literals still tells a caller whether their host cleared the guard. That residue is inherent to a guard that refuses some inputs and attempts others, is far coarser than naming the rule, and the route is session-gated — accepted, and now written down rather than glossed. Complementary to #1428, which fixes this at the guard level with SsrfGuardError (public message + private reason). This is the call-site boundary and does not touch ssrf-guard.ts, so the two do not conflict. Verification (head 8d756b6 + this, apps/web): - New test drives all five real guard messages through fetchTranscript and asserts the caller-visible error is identical. Fails without the fix (`expected 5 to be 1`), passes with it. - vitest run: 1 failed | 291 passed (292). The failure, billing-chat-gating "blocks free tier after daily quota", is pre-existing and unrelated — confirmed earlier on the untouched base. Tracked in #1116. - npm run type-check: clean. npm run lint: clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EX3tLZJJfbDUM6XXi6Uefm --- apps/web/src/app/api/transcribe/route.ts | 28 ++++++++---- .../transcription-error-disclosure.test.ts | 45 +++++++++++++++++-- apps/web/src/lib/transcription-service.ts | 11 ++++- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/apps/web/src/app/api/transcribe/route.ts b/apps/web/src/app/api/transcribe/route.ts index e77251521..3d375e956 100644 --- a/apps/web/src/app/api/transcribe/route.ts +++ b/apps/web/src/app/api/transcribe/route.ts @@ -20,16 +20,28 @@ export const maxDuration = 120; * — that text is logged server-side only. * * The `!result.success` branches keep `fetchTranscript`'s own message in `error`. - * That is safe only because every one of those values is now a fixed, - * app-authored literal. Two of them were not, and were removed at the source in - * `transcription-service.ts` rather than masked here: + * That holds only while every value it can return is a fixed, app-authored + * literal — verified as of this change, all seven are. Three were not, and were + * corrected at the source in `transcription-service.ts` rather than masked here: * - the caller-supplied `audioUrl`'s HTTP status, which made this route a - * cross-origin probe, and + * cross-origin probe, * - a message that varied on whether provider API keys were configured, which - * disclosed server configuration. - * Both are logged server-side instead. Keep that invariant when adding a - * strategy: anything interpolated into a `fetchTranscript` error reaches the - * client verbatim through the branches below. + * disclosed server configuration, and + * - the SSRF guard's rejection reason, which named which guard rule fired and + * so leaked hostname-blocklist policy. + * All three are logged server-side instead. + * + * Keep that invariant when adding a strategy: anything interpolated into a + * `fetchTranscript` error reaches the client verbatim through the branches + * below. `${}` in an error value is the thing to look for. + * + * What this deliberately does NOT claim: the *choice among* those literals is + * still informative. `Rejected audioUrl` versus `Could not retrieve the audio + * file` tells a caller whether their host cleared the SSRF guard — i.e. whether + * it resolves to a public address. That residue is inherent to a guard that + * refuses some inputs and attempts others, it is far coarser than naming the + * rule, and this route is session-gated (`apps/web/src/lib/auth-paths.ts`), so + * it is accepted rather than papered over. * - 400: Invalid input (missing URL, malformed JSON) — `input_required` / `invalid_json` * - 429: Rate limited (implement backoff or upgrade service plan) — `rate_limited` * - 500: Service unavailable (check API keys and billing) — `billing_not_configured` diff --git a/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts b/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts index a1e170b08..b961c56f8 100644 --- a/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts +++ b/apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts @@ -2,9 +2,10 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; // `fetchTranscript`'s error string is returned to the caller verbatim by // `/api/transcribe`'s `!result.success` branches. That is only safe while every -// value it can carry is a fixed, app-authored literal. These tests pin the two -// values that were not: the caller-supplied `audioUrl`'s HTTP status, and a -// message that varied on whether provider API keys were configured. +// value it can carry is a fixed, app-authored literal. These tests pin the three +// values that were not: the caller-supplied `audioUrl`'s HTTP status, a message +// that varied on whether provider API keys were configured, and the SSRF guard's +// rejection reason, which named which guard rule fired. vi.mock('server-only', () => ({})); vi.mock('@/lib/ssrf-guard', () => ({ @@ -25,9 +26,21 @@ vi.mock('@/lib/vercel-ai-gateway', () => ({ })); import { fetchTranscript } from '@/lib/transcription-service'; +import { assertPublicHttpUrl } from '@/lib/ssrf-guard'; const AUDIO_URL = 'https://cdn.example.com/clip.mp3'; +// The real messages `assertPublicHttpUrl` throws, as of ssrf-guard.ts. Each +// names a different guard rule; the point of the test below is that the caller +// cannot tell them apart. +const GUARD_REJECTIONS = [ + 'Invalid URL', + 'Blocked URL scheme: file:', + 'Blocked host', + 'Blocked private IP literal', + 'Host does not resolve to a public address', +]; + let consoleError: ReturnType; const savedEnv = { ...process.env }; @@ -79,6 +92,32 @@ describe('fetchTranscript error disclosure', () => { expect(new Set(errors).size).toBe(1); }); + it('does not reveal which SSRF guard rule rejected the audioUrl', async () => { + process.env.OPENAI_API_KEY = 'sk-test-key'; + const errors: string[] = []; + + for (const message of GUARD_REJECTIONS) { + vi.mocked(assertPublicHttpUrl).mockRejectedValueOnce(new Error(message)); + const result = await fetchTranscript({ audioUrl: AUDIO_URL }); + errors.push(result.error ?? ''); + } + + // A blocklist match and a DNS rejection must be indistinguishable: the first + // confirms a hostname is on the server's blocklist, the second only that DNS + // returned nothing public. Distinguishing them is a policy oracle. + expect(new Set(errors).size).toBe(1); + expect(errors[0]).toBe('Rejected audioUrl'); + // No guard rule name survives into the caller-visible payload. + for (const message of GUARD_REJECTIONS) { + expect(errors[0]).not.toContain(message); + } + // Operators still get the real reason. + expect(consoleError).toHaveBeenCalledWith( + '[transcription] audioUrl rejected by SSRF guard:', + expect.objectContaining({ message: 'Blocked host' }), + ); + }); + it('does not disclose whether provider API keys are configured', async () => { vi.stubGlobal( 'fetch', diff --git a/apps/web/src/lib/transcription-service.ts b/apps/web/src/lib/transcription-service.ts index fed35d6bc..9d8adea39 100644 --- a/apps/web/src/lib/transcription-service.ts +++ b/apps/web/src/lib/transcription-service.ts @@ -273,9 +273,18 @@ ${metadataContext ? `\nKNOWN METADATA:\n${metadataContext}` : ''}`, try { await assertPublicHttpUrl(audioUrl); } catch (guardErr) { + // The guard throws five distinguishable messages — `Invalid URL`, + // `Blocked URL scheme: `, `Blocked host`, `Blocked private IP + // literal`, and `Host does not resolve to a public address`. Forwarding + // them told the caller WHICH rule fired: `Blocked host` confirms a + // hostname-blocklist match, while the resolution message confirms only + // that DNS gave nothing public. That difference is a policy oracle, and + // it sharpens as BLOCKED_HOSTNAMES grows. One fixed message for every + // rejection; the real reason goes to the log. + console.error('[transcription] audioUrl rejected by SSRF guard:', guardErr); return { success: false, - error: `Rejected audioUrl: ${guardErr instanceof Error ? guardErr.message : 'blocked'}`, + error: 'Rejected audioUrl', transcript: '', }; }