fix(mcp): pin OAuth redirect allowlist to exact vendor callbacks + token/log hygiene

Medium finding: the anonymous dynamic-client-registration allowlist accepted any path on whole vendor domains plus wildcard subdomains, so an open redirect (or controllable subdomain) on an allowed domain could exfiltrate authorization codes — PKCE does not help when the attacker runs the flow with their own verifier. Defaults are now the documented callback URIs only (claude.ai + claude.com /api/mcp/auth_callback, chatgpt.com /connector_platform_oauth_redirect, vscode.dev + insiders.vscode.dev /redirect); gemini.google.com dropped (Gemini CLI uses loopback). MCP_ALLOWED_REDIRECT_HOSTS gains optional /path templates (host-only entries still work). redirect_uris with userinfo or fragments are rejected outright, loopback included; localhost stays allowed because Claude Code and Gemini CLI register http://localhost callbacks. Low finding: /oauth/token now sets Cache-Control: no-store + Pragma: no-cache on every response; Entra error logging is reduced to status/error/error_codes/ correlation_id; all user-controlled log fields (client_name, redirect_uri) pass through sanitizeForLog (control-char strip + length cap). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-10-24 12:42:38 +00:00 · 2026-07-11 10:17:09 +02:00
parent 9b023ac1d6
commit c20d9df8dc
2 changed files with 264 additions and 96 deletions
@@ -14,6 +14,8 @@
 * These tests are pure functions; no live backend or token needed.
 */
 import { describe, it, expect } from 'vitest';
+import express from 'express';
+import type { AddressInfo } from 'node:net';
 import { followNextLink } from '../client.js';
 import { SessionIdSchema } from '../tools/shared.js';
 import { ApiError } from '../client.js';
@@ -38,6 +40,8 @@ const {
  isAllowedRedirectUri,
  isAllowedRedirectUriWith,
  parseAllowedHosts,
+  sanitizeForLog,
+  createOAuthRouter,
  signState,
  verifyState,
  MAX_REDIRECT_URIS_PER_CLIENT,
@@ -129,7 +133,7 @@ describe('M2 — SessionIdSchema UUID enforcement', () => {
  });
 });

-describe('H1 — OAuth redirect_uri allowlist (default Multi-Vendor)', () => {
+describe('H1 — OAuth redirect_uri allowlist (default = exact vendor callback URIs)', () => {
  it('accepts loopback over http with arbitrary port', () => {
    expect(isAllowedRedirectUri('http://localhost:1234/callback')).toBe(true);
    expect(isAllowedRedirectUri('http://127.0.0.1:54321/cb')).toBe(true);
@@ -139,27 +143,50 @@ describe('H1 — OAuth redirect_uri allowlist (default Multi-Vendor)', () => {
    expect(isAllowedRedirectUri('https://localhost:8443/cb')).toBe(true);
  });

-  it('accepts Anthropic hosts over https (claude.ai, *.claude.ai, *.anthropic.com)', () => {
-    expect(isAllowedRedirectUri('https://claude.ai/api/oauth/cb')).toBe(true);
-    expect(isAllowedRedirectUri('https://api.claude.ai/cb')).toBe(true);
-    expect(isAllowedRedirectUri('https://accounts.anthropic.com/cb')).toBe(true);
+  it('accepts the documented Claude web/desktop callback (claude.ai + claude.com)', () => {
+    expect(isAllowedRedirectUri('https://claude.ai/api/mcp/auth_callback')).toBe(true);
+    expect(isAllowedRedirectUri('https://claude.com/api/mcp/auth_callback')).toBe(true);
  });

-  it('accepts OpenAI/ChatGPT hosts over https', () => {
-    expect(isAllowedRedirectUri('https://chatgpt.com/connector/oauth/cb')).toBe(true);
-    expect(isAllowedRedirectUri('https://app.chatgpt.com/cb')).toBe(true);
-    expect(isAllowedRedirectUri('https://platform.openai.com/cb')).toBe(true);
-    expect(isAllowedRedirectUri('https://chat.openai.com/cb')).toBe(true);
+  it('accepts the documented ChatGPT connector callback', () => {
+    expect(isAllowedRedirectUri('https://chatgpt.com/connector_platform_oauth_redirect')).toBe(true);
  });

-  it('accepts Google Gemini host over https', () => {
-    expect(isAllowedRedirectUri('https://gemini.google.com/oauth/cb')).toBe(true);
+  it('accepts the documented VS Code stable/insiders callbacks', () => {
+    expect(isAllowedRedirectUri('https://vscode.dev/redirect')).toBe(true);
+    expect(isAllowedRedirectUri('https://insiders.vscode.dev/redirect')).toBe(true);
+  });
+
+  it('tolerates a trailing slash and a query string on a path-template match', () => {
+    expect(isAllowedRedirectUri('https://vscode.dev/redirect/')).toBe(true);
+    expect(isAllowedRedirectUri('https://vscode.dev/redirect?quality=stable')).toBe(true);
+  });
+
+  it('rejects other paths on allowlisted hosts (open-redirect containment)', () => {
+    expect(isAllowedRedirectUri('https://claude.ai/api/oauth/cb')).toBe(false);
+    expect(isAllowedRedirectUri('https://claude.ai/')).toBe(false);
+    expect(isAllowedRedirectUri('https://chatgpt.com/connector/oauth/cb')).toBe(false);
+    expect(isAllowedRedirectUri('https://vscode.dev/redirect/extra')).toBe(false);
+  });
+
+  it('rejects dot-segment traversal into a template path (URL normalizes it away)', () => {
+    // new URL() collapses "/api/mcp/auth_callback/../evil" to "/api/mcp/evil",
+    // which must then fail the exact-path compare.
+    expect(isAllowedRedirectUri('https://claude.ai/api/mcp/auth_callback/../evil')).toBe(false);
+  });
+
+  it('rejects subdomains and unrelated vendor hosts no longer in the defaults', () => {
+    expect(isAllowedRedirectUri('https://api.claude.ai/cb')).toBe(false);
+    expect(isAllowedRedirectUri('https://accounts.anthropic.com/cb')).toBe(false);
+    expect(isAllowedRedirectUri('https://app.chatgpt.com/cb')).toBe(false);
+    expect(isAllowedRedirectUri('https://platform.openai.com/cb')).toBe(false);
+    expect(isAllowedRedirectUri('https://chat.openai.com/cb')).toBe(false);
+    expect(isAllowedRedirectUri('https://gemini.google.com/oauth/cb')).toBe(false);
  });

  it('rejects allowed hosts over plain http (downgrade attempt)', () => {
-    expect(isAllowedRedirectUri('http://claude.ai/cb')).toBe(false);
-    expect(isAllowedRedirectUri('http://chatgpt.com/cb')).toBe(false);
-    expect(isAllowedRedirectUri('http://gemini.google.com/cb')).toBe(false);
+    expect(isAllowedRedirectUri('http://claude.ai/api/mcp/auth_callback')).toBe(false);
+    expect(isAllowedRedirectUri('http://chatgpt.com/connector_platform_oauth_redirect')).toBe(false);
  });

  it('rejects arbitrary hosts', () => {
@@ -167,17 +194,21 @@ describe('H1 — OAuth redirect_uri allowlist (default Multi-Vendor)', () => {
    expect(isAllowedRedirectUri('https://evil.example.com/cb')).toBe(false);
  });

-  it('rejects look-alike hosts (suffix match must be on the dot boundary)', () => {
-    // "evilclaude.ai" ends with "claude.ai" naïvely, but the suffix list
-    // stores ".claude.ai" with a leading dot, anchoring matches to a real
-    // subdomain boundary.
-    expect(isAllowedRedirectUri('https://evilclaude.ai/cb')).toBe(false);
-    expect(isAllowedRedirectUri('https://notanthropic.com/cb')).toBe(false);
-    expect(isAllowedRedirectUri('https://fakechatgpt.com/cb')).toBe(false);
-    expect(isAllowedRedirectUri('https://notopenai.com/cb')).toBe(false);
-    // gemini.google.com is exact-match only — google.com itself is NOT allowed.
+  it('rejects look-alike hosts', () => {
+    expect(isAllowedRedirectUri('https://evilclaude.ai/api/mcp/auth_callback')).toBe(false);
+    expect(isAllowedRedirectUri('https://fakechatgpt.com/connector_platform_oauth_redirect')).toBe(false);
    expect(isAllowedRedirectUri('https://google.com/cb')).toBe(false);
-    expect(isAllowedRedirectUri('https://accounts.google.com/cb')).toBe(false);
+  });
+
+  it('rejects userinfo in the URI (host-confusion primitive), including loopback', () => {
+    expect(isAllowedRedirectUri('https://user@claude.ai/api/mcp/auth_callback')).toBe(false);
+    expect(isAllowedRedirectUri('https://claude.ai:pw@evil.tld/cb')).toBe(false);
+    expect(isAllowedRedirectUri('http://user@localhost:1234/callback')).toBe(false);
+  });
+
+  it('rejects fragments (forbidden by RFC 6749 §3.1.2), including loopback', () => {
+    expect(isAllowedRedirectUri('https://claude.ai/api/mcp/auth_callback#frag')).toBe(false);
+    expect(isAllowedRedirectUri('http://localhost:1234/callback#frag')).toBe(false);
  });

  it('rejects javascript:, data:, file:, ftp: schemes', () => {
@@ -196,50 +227,77 @@ describe('H1 — OAuth redirect_uri allowlist (default Multi-Vendor)', () => {
 });

 describe('H1 — OAuth allowlist via MCP_ALLOWED_REDIRECT_HOSTS env override', () => {
+  const find = (r: { entries: { host: string; wildcard: boolean; path: string | null }[] }, host: string) =>
+    r.entries.find((e) => e.host === host);
+
  it('parseAllowedHosts splits exact hosts and *.wildcards', () => {
    const r = parseAllowedHosts('foo.example.com,*.bar.example.com,baz.org');
-    expect(r.exact.has('foo.example.com')).toBe(true);
-    expect(r.exact.has('baz.org')).toBe(true);
-    expect(r.exact.has('bar.example.com')).toBe(false); // wildcard, not exact
-    expect(r.suffixes).toContain('.bar.example.com');
+    expect(find(r, 'foo.example.com')).toMatchObject({ wildcard: false, path: null });
+    expect(find(r, 'baz.org')).toMatchObject({ wildcard: false, path: null });
+    expect(find(r, 'bar.example.com')).toMatchObject({ wildcard: true, path: null });
+  });
+
+  it('parseAllowedHosts parses optional path templates', () => {
+    const r = parseAllowedHosts('a.example.com/oauth/cb,*.b.example.com/redirect,c.example.com');
+    expect(find(r, 'a.example.com')).toMatchObject({ wildcard: false, path: '/oauth/cb' });
+    expect(find(r, 'b.example.com')).toMatchObject({ wildcard: true, path: '/redirect' });
+    expect(find(r, 'c.example.com')).toMatchObject({ wildcard: false, path: null });
  });

  it('parseAllowedHosts normalizes case', () => {
    const r = parseAllowedHosts('Foo.EXAMPLE.com');
-    expect(r.exact.has('foo.example.com')).toBe(true);
+    expect(find(r, 'foo.example.com')).toBeDefined();
  });

  it('parseAllowedHosts trims whitespace and drops empty entries', () => {
    const r = parseAllowedHosts(' a.com ,  , b.com ');
-    expect(r.exact.size).toBe(2);
-    expect(r.exact.has('a.com')).toBe(true);
-    expect(r.exact.has('b.com')).toBe(true);
+    expect(r.entries).toHaveLength(2);
+    expect(find(r, 'a.com')).toBeDefined();
+    expect(find(r, 'b.com')).toBeDefined();
  });

-  it('parseAllowedHosts falls back to defaults when env is undefined', () => {
+  it('parseAllowedHosts falls back to path-pinned defaults when env is undefined', () => {
    const r = parseAllowedHosts(undefined);
-    // Default baseline includes all three vendors.
-    expect(r.exact.has('claude.ai')).toBe(true);
-    expect(r.exact.has('chatgpt.com')).toBe(true);
-    expect(r.exact.has('gemini.google.com')).toBe(true);
+    expect(find(r, 'claude.ai')).toMatchObject({ path: '/api/mcp/auth_callback' });
+    expect(find(r, 'claude.com')).toMatchObject({ path: '/api/mcp/auth_callback' });
+    expect(find(r, 'chatgpt.com')).toMatchObject({ path: '/connector_platform_oauth_redirect' });
+    expect(find(r, 'vscode.dev')).toMatchObject({ path: '/redirect' });
+    expect(find(r, 'insiders.vscode.dev')).toMatchObject({ path: '/redirect' });
+    // No host-wide or wildcard entries remain in the defaults.
+    expect(r.entries.every((e) => e.path !== null && !e.wildcard)).toBe(true);
  });

-  it('custom allowlist accepts only the configured hosts (defaults are NOT additive)', () => {
+  it('custom allowlist accepts only the configured entries (defaults are NOT additive)', () => {
    const customOnly = parseAllowedHosts('mycompany.example.com,*.partner.example.org');
    expect(isAllowedRedirectUriWith('https://mycompany.example.com/cb', customOnly)).toBe(true);
    expect(isAllowedRedirectUriWith('https://api.partner.example.org/cb', customOnly)).toBe(true);
    // Defaults must NOT leak through — operator-supplied list fully replaces them.
-    expect(isAllowedRedirectUriWith('https://claude.ai/cb', customOnly)).toBe(false);
+    expect(isAllowedRedirectUriWith('https://claude.ai/api/mcp/auth_callback', customOnly)).toBe(false);
    expect(isAllowedRedirectUriWith('https://chatgpt.com/cb', customOnly)).toBe(false);
  });

+  it('custom path-template entries pin the path; host-only entries allow any path', () => {
+    const r = parseAllowedHosts('pinned.example.com/oauth/cb,open.example.com');
+    expect(isAllowedRedirectUriWith('https://pinned.example.com/oauth/cb', r)).toBe(true);
+    expect(isAllowedRedirectUriWith('https://pinned.example.com/other', r)).toBe(false);
+    expect(isAllowedRedirectUriWith('https://open.example.com/anything/goes', r)).toBe(true);
+  });
+
+  it('wildcard entries match subdomains on the dot boundary only', () => {
+    const r = parseAllowedHosts('*.partner.example.org/cb');
+    expect(isAllowedRedirectUriWith('https://api.partner.example.org/cb', r)).toBe(true);
+    // Bare apex does not match a `*.` entry, and look-alikes stay out.
+    expect(isAllowedRedirectUriWith('https://partner.example.org/cb', r)).toBe(false);
+    expect(isAllowedRedirectUriWith('https://evilpartner.example.org.attacker.tld/cb', r)).toBe(false);
+  });
+
  it('custom allowlist still requires HTTPS for non-loopback', () => {
    const r = parseAllowedHosts('mycompany.example.com');
    expect(isAllowedRedirectUriWith('http://mycompany.example.com/cb', r)).toBe(false);
  });

  it('loopback is always allowed even with empty allowlist', () => {
-    const empty = parseAllowedHosts('');
+    const empty = { entries: [] };
    expect(isAllowedRedirectUriWith('http://localhost:9000/cb', empty)).toBe(true);
    expect(isAllowedRedirectUriWith('http://127.0.0.1:9000/cb', empty)).toBe(true);
    // But arbitrary hosts are not.
@@ -247,6 +305,53 @@ describe('H1 — OAuth allowlist via MCP_ALLOWED_REDIRECT_HOSTS env override', (
  });
 });

+describe('sanitizeForLog — user-controlled log-field hygiene', () => {
+  it('strips CR/LF so a hostile value cannot forge log lines', () => {
+    expect(sanitizeForLog('evil\r\n[oauth] fake line')).toBe('evil[oauth] fake line');
+  });
+
+  it('strips other control characters', () => {
+    expect(sanitizeForLog('a\x00b\x1bc\x7fd')).toBe('abcd');
+  });
+
+  it('caps the length', () => {
+    const out = sanitizeForLog('x'.repeat(500), 200);
+    expect(out).toHaveLength(203); // 200 chars + '...'
+    expect(out.endsWith('...')).toBe(true);
+  });
+
+  it('stringifies null/undefined without throwing', () => {
+    expect(sanitizeForLog(undefined)).toBe('');
+    expect(sanitizeForLog(null)).toBe('');
+  });
+});
+
+describe('/oauth/token response hygiene', () => {
+  it('sets Cache-Control: no-store and Pragma: no-cache on token responses', async () => {
+    const app = express();
+    app.use(express.urlencoded({ extended: false }));
+    app.use(createOAuthRouter());
+    const server = app.listen(0, '127.0.0.1');
+    await new Promise<void>((resolve) => server.once('listening', resolve));
+    const { port } = server.address() as AddressInfo;
+    try {
+      // Missing grant_type → the proxy's own 400, no outbound Entra call.
+      // The anti-caching headers are set route-wide at handler entry, so this
+      // cheap path proves they are present on every response shape.
+      const res = await fetch(`http://127.0.0.1:${port}/oauth/token`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
+        body: '',
+      });
+      expect(res.status).toBe(400);
+      expect(res.headers.get('cache-control')).toBe('no-store');
+      expect(res.headers.get('pragma')).toBe('no-cache');
+    } finally {
+      await new Promise<void>((resolve) => server.close(() => resolve()));
+    }
+  });
+});
+
 describe('L3 — error-handler 5xx body redaction', () => {
  it('does not leak the response body on 500', () => {
    const apiErr = new ApiError(500, 'Stack frame at /usr/src/app/secrets.js:42 with token sk-leak');
@@ -128,6 +128,19 @@ export const MAX_REDIRECT_URIS_PER_CLIENT = 10;
 export const MAX_REDIRECT_URI_LENGTH = 1024;
 export const MAX_CLIENT_NAME_LENGTH = 256;

+/**
+ * Makes a caller-controlled value safe for single-line text logs: strips
+ * control characters (CR/LF would let a hostile client_name or redirect_uri
+ * forge additional log lines) and caps the length. Everything user-supplied
+ * that reaches console.error in this module must pass through here.
+ */
+export function sanitizeForLog(value: unknown, maxLength = 200): string {
+  const s = String(value ?? '');
+  // eslint-disable-next-line no-control-regex
+  const cleaned = s.replace(/[\x00-\x1f\x7f]/g, '');
+  return cleaned.length > maxLength ? `${cleaned.slice(0, maxLength)}...` : cleaned;
+}
+
 // ---- Client-id signing (HMAC) ---------------------------------------------
 //
 // The dynamic-registration client_id is a self-describing signed token rather
@@ -196,69 +209,86 @@ export function verifyClientId(clientId: string | undefined | null): ClientIdPay
 }

 /**
- * Strict allowlist of host patterns acceptable for OAuth redirect_uri.
- * Defense-in-depth on top of the per-client redirect_uri match: even if an
- * attacker calls the unauthenticated /oauth/register endpoint to register a
- * client with a hostile redirect_uri, the URI must still pass this filter.
+ * Strict allowlist of redirect_uri templates. Defense-in-depth on top of the
+ * per-client redirect_uri match: even if an attacker calls the unauthenticated
+ * /oauth/register endpoint to register a client with a hostile redirect_uri,
+ * the URI must still pass this filter.
 *
 * Loopback (localhost / 127.0.0.1 / [::1]) is always allowed for native CLI
- * flows (RFC 8252 §7.3). Hosted-AI-client hosts are configurable via the
- * MCP_ALLOWED_REDIRECT_HOSTS env var (comma-separated). When unset, a
- * baseline of widely-deployed AI vendors is used.
+ * flows (RFC 8252 §7.3). `localhost` is deliberately kept alongside the IP
+ * literals — Claude Code and Gemini CLI both register `http://localhost:...`
+ * callbacks, so an IP-literal-only rule would break real clients.
 *
- * Entry syntax: bare host (`claude.ai`) for exact match, or `*.example.com`
- * for any subdomain. The leading-dot suffix check anchors the match to a
- * real subdomain boundary so `evilclaude.ai` does not slip past `claude.ai`.
+ * Hosted-AI-client entries are configurable via the MCP_ALLOWED_REDIRECT_HOSTS
+ * env var (comma-separated). When unset, the baseline below allows exactly the
+ * documented callback URIs of widely-deployed AI vendors — full host+path, not
+ * whole domains. Scoping to the vendor's dedicated OAuth callback path keeps an
+ * open redirect elsewhere on the same domain (or a compromised sibling
+ * subdomain) from being usable to exfiltrate authorization codes; PKCE does not
+ * help in that scenario because the attacker runs the whole flow with their own
+ * verifier.
+ *
+ * Entry syntax: `host` (any path — operator escape hatch), `host/path`
+ * (exact path, trailing-slash tolerant), and `*.host` / `*.host/path` for
+ * subdomain wildcards. The dot-anchored suffix check keeps `evilclaude.ai`
+ * from slipping past `*.claude.ai`.
 */
 const DEFAULT_ALLOWED_HOSTS: readonly string[] = [
-  // Anthropic
-  'claude.ai',
-  '*.claude.ai',
-  '*.anthropic.com',
-  // OpenAI / ChatGPT
-  'chatgpt.com',
-  '*.chatgpt.com',
-  '*.openai.com',
-  // Google / Gemini
-  'gemini.google.com',
-  // Microsoft / VS Code (GitHub Copilot, Continue, etc.)
-  // Stable uses https://vscode.dev/redirect; Insiders uses
-  // https://insiders.vscode.dev/redirect. The loopback URI VS Code also
-  // registers (http://127.0.0.1:<port>) is already accepted by the
-  // RFC 8252 loopback rule below.
-  'vscode.dev',
-  '*.vscode.dev',
+  // Anthropic — Claude web / Desktop / mobile connector callback (both the
+  // claude.ai and claude.com front doors). Claude Code uses RFC 8252 loopback,
+  // which the loopback rule below already covers.
+  'claude.ai/api/mcp/auth_callback',
+  'claude.com/api/mcp/auth_callback',
+  // OpenAI — ChatGPT connector-platform callback.
+  'chatgpt.com/connector_platform_oauth_redirect',
+  // Microsoft / VS Code (GitHub Copilot, Continue, etc.) — Stable and Insiders
+  // web redirect endpoints. The loopback URI VS Code also registers
+  // (http://127.0.0.1:<port>) is accepted by the loopback rule below.
+  'vscode.dev/redirect',
+  'insiders.vscode.dev/redirect',
+  // Google Gemini has no documented hosted-web MCP callback; Gemini CLI uses
+  // loopback redirects, which are always allowed. No entry needed.
 ];

-export function parseAllowedHosts(envValue: string | undefined): { exact: Set<string>; suffixes: string[] } {
-  const exact = new Set<string>();
-  const suffixes: string[] = [];
-  const entries = envValue
+export interface AllowedRedirectEntry {
+  host: string; // lowercase, no leading `*.`
+  wildcard: boolean; // true → match any subdomain of host (dot-anchored)
+  path: string | null; // null → any path; else exact pathname (trailing-slash tolerant)
+}
+
+export function parseAllowedHosts(envValue: string | undefined): { entries: AllowedRedirectEntry[] } {
+  const entries: AllowedRedirectEntry[] = [];
+  const raw = envValue
    ? envValue.split(',').map((s) => s.trim()).filter(Boolean)
    : DEFAULT_ALLOWED_HOSTS;
-  for (const entry of entries) {
-    const lower = entry.toLowerCase();
-    if (lower.startsWith('*.')) {
-      // Store with the leading dot so the suffix check is anchored to a real
-      // subdomain boundary — `*.claude.ai` matches `api.claude.ai` but not
-      // `evilclaude.ai`.
-      suffixes.push(lower.slice(1));
-    } else {
-      exact.add(lower);
-    }
+  for (const item of raw) {
+    let rest = item.toLowerCase();
+    const wildcard = rest.startsWith('*.');
+    if (wildcard) rest = rest.slice(2);
+    const slash = rest.indexOf('/');
+    const host = slash >= 0 ? rest.slice(0, slash) : rest;
+    const path = slash >= 0 ? rest.slice(slash) : null;
+    if (!host) continue;
+    entries.push({ host, wildcard, path });
  }
-  return { exact, suffixes };
+  return { entries };
 }

 const ALLOWED_HOSTS = parseAllowedHosts(process.env.MCP_ALLOWED_REDIRECT_HOSTS);

 if (process.env.MCP_ALLOWED_REDIRECT_HOSTS) {
  console.error(
-    `[oauth] MCP_ALLOWED_REDIRECT_HOSTS set — using ${ALLOWED_HOSTS.exact.size} exact + ` +
-    `${ALLOWED_HOSTS.suffixes.length} wildcard host(s) (loopback always allowed)`,
+    `[oauth] MCP_ALLOWED_REDIRECT_HOSTS set — using ${ALLOWED_HOSTS.entries.length} ` +
+    'allowlist entrie(s) (loopback always allowed)',
  );
 }

+/** Exact pathname compare, tolerant of a single trailing slash on either side. */
+function pathMatchesTemplate(templatePath: string, pathname: string): boolean {
+  const norm = (p: string) => (p.length > 1 && p.endsWith('/') ? p.slice(0, -1) : p);
+  return norm(templatePath) === norm(pathname);
+}
+
 /**
 * Internal worker that applies the host check against an arbitrary allowlist.
 * Exposed for tests so the ENV-override path can be exercised without
@@ -266,7 +296,7 @@ if (process.env.MCP_ALLOWED_REDIRECT_HOSTS) {
 */
 export function isAllowedRedirectUriWith(
  uri: string | undefined | null,
-  allowed: { exact: Set<string>; suffixes: string[] },
+  allowed: { entries: AllowedRedirectEntry[] },
 ): boolean {
  if (!uri) return false;
  let parsed: URL;
@@ -278,6 +308,14 @@ export function isAllowedRedirectUriWith(
  // Reject anything other than http/https. Blocks file:, javascript:, data:, etc.
  if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return false;

+  // Userinfo and fragments have no legitimate place in a redirect_uri
+  // (RFC 6749 §3.1.2 forbids fragments outright) and both are classic
+  // confusion primitives — `https://claude.ai@evil.tld/` reads as claude.ai
+  // to a human, and a fragment can smuggle state past server-side parsers.
+  // Applies to loopback URIs too.
+  if (parsed.username !== '' || parsed.password !== '') return false;
+  if (parsed.hash !== '') return false;
+
  const host = parsed.hostname.toLowerCase();

  // Loopback (any port, http or https) — RFC 8252 §7.3 native-app pattern.
@@ -291,9 +329,17 @@ export function isAllowedRedirectUriWith(
  // tricks where an attacker gets the user to follow `http://claude.ai/...`.
  if (parsed.protocol !== 'https:') return false;

-  if (allowed.exact.has(host)) return true;
-  for (const suffix of allowed.suffixes) {
-    if (host.endsWith(suffix)) return true;
+  // Host must match (exact, or dot-anchored subdomain for wildcards) AND the
+  // pathname must match the entry's path template when one is set. `new URL()`
+  // has already collapsed dot-segments, so `/cb/../evil` cannot masquerade as
+  // a template path. Query strings are permitted — the host+path pair is what
+  // pins the vendor's callback handler.
+  for (const entry of allowed.entries) {
+    const hostMatches = entry.wildcard
+      ? host.endsWith(`.${entry.host}`)
+      : host === entry.host;
+    if (!hostMatches) continue;
+    if (entry.path === null || pathMatchesTemplate(entry.path, parsed.pathname)) return true;
  }
  return false;
 }
@@ -436,7 +482,7 @@ export function createOAuthRouter(): Router {
    const invalidUris = uris.filter((u: unknown) => typeof u !== 'string' || !isAllowedRedirectUri(u));
    if (invalidUris.length > 0) {
      console.error(
-        `[oauth/register] Rejected client ${client_name ?? 'unknown'}: ` +
+        `[oauth/register] Rejected client ${sanitizeForLog(client_name ?? 'unknown')}: ` +
        `${invalidUris.length} redirect_uri(s) outside allowlist`,
      );
      res.status(400).json({
@@ -454,7 +500,7 @@ export function createOAuthRouter(): Router {
    // on any replica (see signClientId).
    const clientId = signClientId(uris, client_name ?? 'unknown');

-    console.error(`[oauth/register] Registered dynamic client (${client_name ?? 'unknown'}), ${uris.length} redirect_uri(s)`);
+    console.error(`[oauth/register] Registered dynamic client (${sanitizeForLog(client_name ?? 'unknown')}), ${uris.length} redirect_uri(s)`);

    res.status(201).json({
      client_id: clientId,
@@ -512,7 +558,7 @@ export function createOAuthRouter(): Router {
      return;
    }
    if (!isAllowedRedirectUri(redirect_uri)) {
-      console.error(`[oauth/authorize] Rejected redirect_uri (host not in allowlist): ${redirect_uri}`);
+      console.error(`[oauth/authorize] Rejected redirect_uri (not in allowlist): ${sanitizeForLog(redirect_uri)}`);
      res.status(400).json({
        error: 'invalid_request',
        error_description: 'redirect_uri host is not in the allowlist',
@@ -547,7 +593,7 @@ export function createOAuthRouter(): Router {
    }
    if (!redirectUriMatches(registered.redirectUris, redirect_uri)) {
      console.error(
-        `[oauth/authorize] redirect_uri not registered for this client_id: ${redirect_uri}`,
+        `[oauth/authorize] redirect_uri not registered for this client_id: ${sanitizeForLog(redirect_uri)}`,
      );
      res.status(400).json({
        error: 'invalid_request',
@@ -628,7 +674,7 @@ export function createOAuthRouter(): Router {
    // /oauth/authorize already gated the URI, but the state value transits the
    // user-agent — re-running the host allowlist here closes any replay gap.
    if (!isAllowedRedirectUri(redirectUri)) {
-      console.error(`[oauth/callback] Rejected redirectUri from state (host not in allowlist): ${redirectUri}`);
+      console.error(`[oauth/callback] Rejected redirectUri from state (not in allowlist): ${sanitizeForLog(redirectUri)}`);
      res.status(400).json({
        error: 'invalid_request',
        error_description: 'redirect_uri host is not in the allowlist',
@@ -642,7 +688,7 @@ export function createOAuthRouter(): Router {
      // redirectUri. A forged client_id fails closed.
      const registered = verifyClientId(clientId);
      if (!registered || !redirectUriMatches(registered.redirectUris, redirectUri)) {
-        console.error(`[oauth/callback] client_id invalid or redirectUri not registered: ${redirectUri}`);
+        console.error(`[oauth/callback] client_id invalid or redirectUri not registered: ${sanitizeForLog(redirectUri)}`);
        res.status(400).json({
          error: 'invalid_request',
          error_description: 'redirect_uri does not match any URI registered for this client_id',
@@ -664,6 +710,13 @@ export function createOAuthRouter(): Router {
  router.post('/oauth/token', oauthTokenRateLimit, async (req, res) => {
    const baseUrl = getPublicBaseUrl(req);

+    // RFC 6749 §5.1: responses carrying tokens MUST NOT be cached. Set on
+    // every response from this route (success, Entra error passthrough, our
+    // own 400s/502) — Entra sends these headers but the proxy's res.json()
+    // does not copy upstream headers.
+    res.set('Cache-Control', 'no-store');
+    res.set('Pragma', 'no-cache');
+
    const params = req.body as Record<string, string>;

    // Allowlist the grant before anything is forwarded. Every request to this
@@ -723,7 +776,17 @@ export function createOAuthRouter(): Router {

      const data = await tokenResponse.json();
      if (tokenResponse.status !== 200) {
-        console.error(`[oauth/token] Entra error: status=${tokenResponse.status}`, JSON.stringify(data));
+        // Log only stable identifiers — never the full body. error_description
+        // can carry user/tenant details, and a full dump would also capture any
+        // token material Entra chooses to include in edge-case errors. The
+        // correlation_id is what Microsoft support needs to trace the request.
+        const err = (typeof data === 'object' && data !== null ? data : {}) as Record<string, unknown>;
+        const codes = Array.isArray(err.error_codes) ? err.error_codes.join(',') : err.error_codes;
+        console.error(
+          `[oauth/token] Entra error: status=${tokenResponse.status} ` +
+          `error=${sanitizeForLog(err.error)} error_codes=${sanitizeForLog(codes)} ` +
+          `correlation_id=${sanitizeForLog(err.correlation_id)}`,
+        );
      }
      res.status(tokenResponse.status).json(data);
    } catch (err) {