fix(web): stop /api/transcribe leaking probe status and key config

apps/web/src/app/api/transcribe/route.ts

@@ -19,11 +19,29 @@ 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 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,
+ *   - a message that varied on whether provider API keys were configured, which
+ *     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`
@@ -94,7 +112,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 }

apps/web/src/lib/__tests__/transcription-error-disclosure.test.ts

@@ -0,0 +1,143 @@
+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 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', () => ({
+  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';
+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<typeof vi.spyOn>;
+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 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',
+      vi.fn(async () => new Response('{}', { status: 500 })),
+    );
+
+    delete process.env.OPENAI_API_KEY;
+    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/auJzb1D-fag' });
+
+    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'),
+    );
+  });
+});

apps/web/src/lib/transcription-service.ts

@@ -273,18 +273,35 @@ ${metadataContext ? `\nKNOWN METADATA:\n${metadataContext}` : ''}`,
       try {
         await assertPublicHttpUrl(audioUrl);
       } catch (guardErr) {
+        // The guard throws five distinguishable messages — `Invalid URL`,
+        // `Blocked URL scheme: <protocol>`, `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: '',
         };
       }
 
       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 +373,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: '',
   };
 }