From c7ce63ff35bac7cb8ecaa9978b2d5e4304776139 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Mon, 27 Jul 2026 12:58:47 -0400 Subject: [PATCH 1/4] fix(mcp-worker): restore dynamic client registration endpoint Signed-off-by: Jonathan Norris --- mcp-worker/src/index.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mcp-worker/src/index.ts b/mcp-worker/src/index.ts index 69c10841..736d5522 100644 --- a/mcp-worker/src/index.ts +++ b/mcp-worker/src/index.ts @@ -167,6 +167,11 @@ export default { defaultHandler: app, authorizeEndpoint: '/oauth/authorize', tokenEndpoint: '/oauth/token', + // Enables RFC 7591 Dynamic Client Registration. MCP clients such as + // Claude Code and `mcp-remote` require a `registration_endpoint` in the + // OAuth discovery metadata to obtain a client_id; without it they fail + // with "does not support dynamic client registration". See PR #577. + clientRegistrationEndpoint: '/oauth/register', tokenExchangeCallback: createTokenExchangeCallback(env), }) From e3786be4eb41af92c03f8bfcdca2d83bf14bc16c Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Thu, 30 Jul 2026 15:00:41 -0400 Subject: [PATCH 2/4] test(mcp-worker): verify consent screen escapes client_name/logo_uri XSS Signed-off-by: Jonathan Norris --- mcp-worker/src/consentScreen.test.ts | 102 +++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 mcp-worker/src/consentScreen.test.ts diff --git a/mcp-worker/src/consentScreen.test.ts b/mcp-worker/src/consentScreen.test.ts new file mode 100644 index 00000000..69d4c879 --- /dev/null +++ b/mcp-worker/src/consentScreen.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, test } from 'vitest' +import { renderConsentScreen } from './consentScreen' + +/** + * The consent screen is the only text/html surface that reflects + * attacker-controlled OAuth client metadata (client_name, logo_uri). Because + * anonymous Dynamic Client Registration lets anyone set those fields, these + * tests lock in that the values are HTML-escaped and can never break out of + * their rendering context into executable markup. + * + * See DevCycleHQ/cli#581 and the DCR stored-XSS report. + */ + +const render = (clientName: string, clientLogo = '') => + renderConsentScreen({ + clientName, + clientLogo, + requestedScopes: ['openid', 'profile'], + transactionState: 'txn', + consentToken: 'csrf', + }).toString() + +// Mirrors the HTML entity escaping applied by hono/html's tagged template. +const htmlEscape = (s: string) => + s + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''') + +// Payloads spanning the contexts common XSS-bypass lists target: raw tags, +// quote/attribute breakout, SVG, case-mangling, null bytes, unclosed tags. +const XSS_PAYLOADS: [name: string, payload: string][] = [ + ['script tag', ''], + ['quote breakout + script', '">'], + ['svg onload (slash)', ''], + ['svg onload (space)', ''], + ['img onerror', ''], + ['case-mangled img', ''], + ['quote+single breakout svg', `'">`], + ['body onload', ''], + ['null-byte script', '<%00script>alert(1)'], + ['unclosed img onerror', 'x'], +] + +describe('renderConsentScreen — client_name XSS escaping', () => { + test.each(XSS_PAYLOADS)( + 'escapes %s and never emits it raw', + (_name, payload) => { + const html = render(payload) + // The raw payload (with unescaped `<`/`"`) must never survive into + // the rendered HTML, otherwise it would execute in the browser. + expect(html).not.toContain(payload) + // ...and the value must actually be present, in escaped form, so + // this test fails loudly if the reflection point is ever removed. + expect(html).toContain(htmlEscape(payload)) + }, + ) + + test('reflects the reported payload as inert, escaped text', () => { + const html = render('') + expect(html).toContain( + '<img src=x onerror=alert(document.domain)>', + ) + expect(html).not.toContain( + '', + ) + }) + + test('escapes the HTML metacharacters used for breakout', () => { + const html = render(`"'<>&`) + expect(html).toContain('"') + expect(html).toContain('<') + expect(html).toContain('>') + expect(html).toContain('&') + }) +}) + +describe('renderConsentScreen — logo_uri handling', () => { + test('does not produce a live that breaks out of the src attribute', () => { + const html = render('Acme', 'https://x/l.png" onerror="alert(1)') + // The logo block is emitted as escaped text, not live markup, so no + // executable onerror handler is ever attached to a rendered element. + expect(html).not.toMatch(/]*onerror=/i) + expect(html).not.toContain('onerror="alert(1)"') + }) + + test('a javascript: logo URI never becomes a live attribute', () => { + const html = render('Acme', 'javascript:alert(1)') + expect(html).not.toMatch(/]*src="javascript:/i) + }) +}) + +describe('renderConsentScreen — happy path', () => { + test('renders a normal client name as visible text', () => { + const html = render('Claude Code') + expect(html).toContain('Claude Code') + expect(html).toContain('Authorization Request') + }) +}) From 56b0d8e9f7d65f83993805a32b643025796eedc3 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Thu, 30 Jul 2026 15:13:22 -0400 Subject: [PATCH 3/4] test(mcp-worker): remove consent screen escaping tests Signed-off-by: Jonathan Norris --- mcp-worker/src/consentScreen.test.ts | 102 --------------------------- 1 file changed, 102 deletions(-) delete mode 100644 mcp-worker/src/consentScreen.test.ts diff --git a/mcp-worker/src/consentScreen.test.ts b/mcp-worker/src/consentScreen.test.ts deleted file mode 100644 index 69d4c879..00000000 --- a/mcp-worker/src/consentScreen.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -import { describe, expect, test } from 'vitest' -import { renderConsentScreen } from './consentScreen' - -/** - * The consent screen is the only text/html surface that reflects - * attacker-controlled OAuth client metadata (client_name, logo_uri). Because - * anonymous Dynamic Client Registration lets anyone set those fields, these - * tests lock in that the values are HTML-escaped and can never break out of - * their rendering context into executable markup. - * - * See DevCycleHQ/cli#581 and the DCR stored-XSS report. - */ - -const render = (clientName: string, clientLogo = '') => - renderConsentScreen({ - clientName, - clientLogo, - requestedScopes: ['openid', 'profile'], - transactionState: 'txn', - consentToken: 'csrf', - }).toString() - -// Mirrors the HTML entity escaping applied by hono/html's tagged template. -const htmlEscape = (s: string) => - s - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"') - .replace(/'/g, ''') - -// Payloads spanning the contexts common XSS-bypass lists target: raw tags, -// quote/attribute breakout, SVG, case-mangling, null bytes, unclosed tags. -const XSS_PAYLOADS: [name: string, payload: string][] = [ - ['script tag', ''], - ['quote breakout + script', '">'], - ['svg onload (slash)', ''], - ['svg onload (space)', ''], - ['img onerror', ''], - ['case-mangled img', ''], - ['quote+single breakout svg', `'">`], - ['body onload', ''], - ['null-byte script', '<%00script>alert(1)'], - ['unclosed img onerror', 'x'], -] - -describe('renderConsentScreen — client_name XSS escaping', () => { - test.each(XSS_PAYLOADS)( - 'escapes %s and never emits it raw', - (_name, payload) => { - const html = render(payload) - // The raw payload (with unescaped `<`/`"`) must never survive into - // the rendered HTML, otherwise it would execute in the browser. - expect(html).not.toContain(payload) - // ...and the value must actually be present, in escaped form, so - // this test fails loudly if the reflection point is ever removed. - expect(html).toContain(htmlEscape(payload)) - }, - ) - - test('reflects the reported payload as inert, escaped text', () => { - const html = render('') - expect(html).toContain( - '<img src=x onerror=alert(document.domain)>', - ) - expect(html).not.toContain( - '', - ) - }) - - test('escapes the HTML metacharacters used for breakout', () => { - const html = render(`"'<>&`) - expect(html).toContain('"') - expect(html).toContain('<') - expect(html).toContain('>') - expect(html).toContain('&') - }) -}) - -describe('renderConsentScreen — logo_uri handling', () => { - test('does not produce a live that breaks out of the src attribute', () => { - const html = render('Acme', 'https://x/l.png" onerror="alert(1)') - // The logo block is emitted as escaped text, not live markup, so no - // executable onerror handler is ever attached to a rendered element. - expect(html).not.toMatch(/]*onerror=/i) - expect(html).not.toContain('onerror="alert(1)"') - }) - - test('a javascript: logo URI never becomes a live attribute', () => { - const html = render('Acme', 'javascript:alert(1)') - expect(html).not.toMatch(/]*src="javascript:/i) - }) -}) - -describe('renderConsentScreen — happy path', () => { - test('renders a normal client name as visible text', () => { - const html = render('Claude Code') - expect(html).toContain('Claude Code') - expect(html).toContain('Authorization Request') - }) -}) From 3eafec4dc881dd939d634c55f857ebb0f3be060d Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Thu, 30 Jul 2026 15:13:22 -0400 Subject: [PATCH 4/4] feat(mcp-worker): set strict CSP on OAuth consent page Signed-off-by: Jonathan Norris --- mcp-worker/src/auth.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/mcp-worker/src/auth.ts b/mcp-worker/src/auth.ts index 30b99df7..3d3a5cb8 100644 --- a/mcp-worker/src/auth.ts +++ b/mcp-worker/src/auth.ts @@ -105,7 +105,14 @@ export async function authorize( const clientLogo = client.logoUri || '' // No default logo const requestedScopes = (c.env.AUTH0_SCOPE || '').split(' ') - // Render the consent screen with CSRF protection + // Render the consent screen with CSRF protection. + // + // The consent page reflects client-supplied metadata (client_name, logo_uri) + // from Dynamic Client Registration. It contains no first-party JavaScript, so + // we lock it down with a strict CSP: `script-src 'none'` blocks every script + // execution path (inline