From d051deca15052491268217f45792f0a7d6bde98e Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Mon, 28 Sep 2026 18:09:20 +0100 Subject: [PATCH 1/2] feat(auth): send a nonce-based CSP on the consent and error pages Inline scripts and styles on both pages now run only with a per-response nonce, so markup that slips past escaping cannot execute. The consent form's form-action allows Cloudflare's authorization origin and the client's redirect origin, because Chrome checks form-action on the redirect after each button. The error page's close link moves from an inline onclick to a nonced listener. --- AGENTS.md | 1 + src/auth/oauth-handler.ts | 1 + src/auth/workers-oauth-utils.ts | 70 +++++++++++++++++-- tests/auth/approval-dialog.test.ts | 107 ++++++++++++++++++++++++++++- tests/auth/oauth-routes.test.ts | 11 +++ 5 files changed, 181 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1c001ee6..d0da9081 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -166,6 +166,7 @@ Tool usage is tracked via the `MCP_METRICS` Analytics Engine binding into the sh - OAuth uses PKCE (RFC 7636) for secure authorization - Cookie encryption for OAuth sessions (`MCP_COOKIE_ENCRYPTION_KEY`) - The `/mcp` route validates Host and present browser Origin headers against deployment-static allowlists before authentication +- The consent and error pages send a nonce-based Content-Security-Policy: inline ` ` @@ -822,7 +880,7 @@ export function renderErrorPage( return new Response(htmlContent, { status, headers: { - 'Content-Security-Policy': "frame-ancestors 'none'", + 'Content-Security-Policy': contentSecurityPolicy(nonce, ["'none'"]), 'Content-Type': 'text/html; charset=utf-8', 'X-Frame-Options': 'DENY' } diff --git a/tests/auth/approval-dialog.test.ts b/tests/auth/approval-dialog.test.ts index e9a3f75d..35ce79d9 100644 --- a/tests/auth/approval-dialog.test.ts +++ b/tests/auth/approval-dialog.test.ts @@ -7,8 +7,8 @@ import { type ApprovalDialogOptions } from '../../src/auth/workers-oauth-utils' -function render(options: Partial = {}): Promise { - const response = renderApprovalDialog(new Request('https://mcp.cloudflare.com/authorize'), { +function renderResponse(options: Partial = {}): Response { + return renderApprovalDialog(new Request('https://mcp.cloudflare.com/authorize'), { consent: { clientId: 'opaque-client-id', clientName: 'Test client', @@ -20,14 +20,51 @@ function render(options: Partial = {}): Promise { server: { name: 'Cloudflare API MCP' }, handle: 'test-consent-handle', headers: new Headers({ 'Set-Cookie': '__Host-oauth-consent-0123456789abcdef=test' }), + upstreamOrigin: 'https://dash.cloudflare.com', scopeTemplates: {}, scopeDefinitions: {}, requiredScopes: [], initialScopes: [], ...options }) +} + +function render(options: Partial = {}): Promise { + return renderResponse(options).text() +} + +function loopbackConsent(redirectUri: string): ApprovalDialogOptions['consent'] { + return { + clientId: 'https://client.example/oauth/client.json', + clientDomain: 'client.example', + clientName: 'Native client', + redirectUri, + redirectHost: new URL(redirectUri).hostname, + redirectIsLoopback: true, + scope: [] + } +} - return response.text() +/** The nonce a page's policy lets scripts run with. */ +function policyNonce(policy: string | null): string { + const nonce = policy?.match(/script-src 'nonce-([^']+)'/)?.[1] + expect(nonce).toBeTruthy() + return nonce! +} + +/** + * Every script and style tag carries the nonce, and no tag relies on what a nonce can't allow: + * inline event handlers, `javascript:` URLs and style attributes. Tags built by the page's + * script sit inside it as strings, so they are checked too. + */ +function expectOnlyNoncedInlineCode(body: string, nonce: string): void { + const tags = body.match(/<[a-z][^>]*>/gi) ?? [] + const code = tags.filter((tag) => /^<(script|style)\b/i.test(tag)) + expect(code.length).toBeGreaterThan(0) + for (const tag of code) expect(tag).toContain(`nonce="${nonce}"`) + for (const tag of tags) { + expect(tag).not.toMatch(/\son[a-z]+\s*=|javascript:|\sstyle\s*=/i) + } } /** @@ -138,6 +175,70 @@ describe('OAuth approval dialog identity details', () => { }) }) +describe('OAuth page Content-Security-Policy', () => { + it('runs only the script and styles the consent page ships', async () => { + const response = renderResponse() + const policy = response.headers.get('Content-Security-Policy') + const nonce = policyNonce(policy) + + expect(policy).toContain("default-src 'none'") + expect(policy).toContain(`style-src 'nonce-${nonce}' https://fonts.googleapis.com`) + expect(policy).toContain('font-src https://fonts.gstatic.com') + expect(policy).toContain("base-uri 'none'") + expect(policy).toContain("frame-ancestors 'none'") + expect(policy).not.toContain('unsafe-inline') + expectOnlyNoncedInlineCode(await response.text(), nonce) + // beginConsent()'s binding cookie survives the new policy. + expect(response.headers.get('Set-Cookie')).toContain('__Host-oauth-consent-') + }) + + it('uses a fresh nonce for every page', () => { + const first = policyNonce(renderResponse().headers.get('Content-Security-Policy')) + const second = policyNonce(renderResponse().headers.get('Content-Security-Policy')) + expect(first).not.toBe(second) + }) + + it('lets both buttons reach their redirects: Cloudflare and the client', () => { + // Chrome applies form-action to the redirect that follows a submission. + expect(renderResponse().headers.get('Content-Security-Policy')).toContain( + "form-action 'self' https://dash.cloudflare.com https://callback.example" + ) + const loopback = renderResponse({ + consent: loopbackConsent('http://127.0.0.1:6274/oauth/callback') + }) + expect(loopback.headers.get('Content-Security-Policy')).toContain( + "form-action 'self' https://dash.cloudflare.com http://127.0.0.1:6274" + ) + }) + + it.each(['http://[::1]:6274/oauth/callback', 'cursor://anysphere.cursor-mcp/oauth/callback'])( + 'leaves out form-action for a redirect URI CSP cannot name: %s', + (redirectUri) => { + const policy = renderResponse({ consent: loopbackConsent(redirectUri) }).headers.get( + 'Content-Security-Policy' + ) + + expect(policy).not.toContain('form-action') + expect(policy).not.toContain('null') + policyNonce(policy) + } + ) + + it('runs only the script and styles the error page ships', async () => { + const response = renderErrorPage('Server Error', 'Try again.') + const policy = response.headers.get('Content-Security-Policy') + const nonce = policyNonce(policy) + + expect(policy).toContain("default-src 'none'") + expect(policy).toContain("form-action 'none'") + expect(policy).toContain("frame-ancestors 'none'") + expect(policy).not.toContain('unsafe-inline') + const body = await response.text() + expectOnlyNoncedInlineCode(body, nonce) + expect(body).toContain('id="closeWindow"') + }) +}) + describe('OAuth approval dialog templates', () => { const templates = { scopeTemplates: SCOPE_TEMPLATES, diff --git a/tests/auth/oauth-routes.test.ts b/tests/auth/oauth-routes.test.ts index 27d95d07..078ea5e7 100644 --- a/tests/auth/oauth-routes.test.ts +++ b/tests/auth/oauth-routes.test.ts @@ -253,6 +253,17 @@ describe('GET /authorize', () => { // Consent form with CSRF protection and a session-binding cookie. expect(body).toContain('`) + expect(policy).toContain( + `form-action 'self' https://dash.cloudflare.com ${new URL(REDIRECT_URI).origin}` + ) + expect(policy).toContain("frame-ancestors 'none'") + expect(res.headers.get('X-Frame-Options')).toBe('DENY') // Cloudflare's authorization screen picks individual scopes. This page offers // the built-in templates, plus any saved in the browser by the old picker, and // says where to narrow them. New templates can no longer be saved. From d62eaa26d2c95824c40dd0cf85ce43140b07cada Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Mon, 28 Sep 2026 23:25:44 +0100 Subject: [PATCH 2/2] refactor(auth): one page shell owns the CSP nonce; no form-action on the consent page renderPage() generates the nonce, puts it on the page's only + + + ${PAGE_HEADER_HTML} +
+
${page.card}
+
+ ${PAGE_FOOTER_HTML} + ${page.script === undefined ? '' : ``} + + +` + + headers.set('Content-Security-Policy', contentSecurityPolicy(nonce, page.formAction)) + headers.set('Content-Type', 'text/html; charset=utf-8') + headers.set('X-Frame-Options', 'DENY') + return new Response(html, { status, headers }) } /** @@ -344,7 +382,6 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp consent, handle, headers, - upstreamOrigin, scopeTemplates, scopeDefinitions, requiredScopes, @@ -363,7 +400,6 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp : undefined const isLocalRedirect = consent.redirectIsLoopback const requiredSet = new Set(requiredScopes) - const nonce = generateCspNonce() const templateDataJson = JSON.stringify( Object.fromEntries(Object.entries(scopeTemplates).map(([k, v]) => [k, v.scopes])) @@ -396,15 +432,10 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp ` : '' - const htmlContent = ` - - - - - - Authorize ${clientName} | Cloudflare - ${PAGE_FONT_LINKS} - - - - ${PAGE_HEADER_HTML} - -
-
+ `, + card: `

Authorize application

Grant access to Cloudflare API

@@ -620,12 +646,8 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp
- -
- - ${PAGE_FOOTER_HTML} - - - - -` - - // beginConsent() headers: the browser binding cookie, no-store and X-Frame-Options DENY. This - // policy replaces its frame-ancestors-only CSP and keeps frame-ancestors 'none'. - headers.set( - 'Content-Security-Policy', - contentSecurityPolicy(nonce, consentFormAction(upstreamOrigin, consent.redirectUri)) + ` + }, + { headers } ) - headers.set('Content-Type', 'text/html; charset=utf-8') - return new Response(htmlContent, { headers }) } /** @@ -813,16 +826,10 @@ export function renderErrorPage( details?: string, status = 400 ): Response { - const nonce = generateCspNonce() - const htmlContent = ` - - - - - - ${sanitizeHtml(title)} | Cloudflare - ${PAGE_FONT_LINKS} - - - - ${PAGE_HEADER_HTML} -
-
+ `, + card: `
@@ -867,22 +870,12 @@ export function renderErrorPage(

${sanitizeHtml(message)}

${details ? `
${sanitizeHtml(details)}
` : ''} -
-
- ${PAGE_FOOTER_HTML} - - - -` - - return new Response(htmlContent, { - status, - headers: { - 'Content-Security-Policy': contentSecurityPolicy(nonce, ["'none'"]), - 'Content-Type': 'text/html; charset=utf-8', - 'X-Frame-Options': 'DENY' - } - }) + `, + formAction: "'none'" + }, + { status } + ) } diff --git a/tests/auth/approval-dialog.test.ts b/tests/auth/approval-dialog.test.ts index 35ce79d9..a0b031a6 100644 --- a/tests/auth/approval-dialog.test.ts +++ b/tests/auth/approval-dialog.test.ts @@ -20,7 +20,6 @@ function renderResponse(options: Partial = {}): Response server: { name: 'Cloudflare API MCP' }, handle: 'test-consent-handle', headers: new Headers({ 'Set-Cookie': '__Host-oauth-consent-0123456789abcdef=test' }), - upstreamOrigin: 'https://dash.cloudflare.com', scopeTemplates: {}, scopeDefinitions: {}, requiredScopes: [], @@ -33,14 +32,14 @@ function render(options: Partial = {}): Promise { return renderResponse(options).text() } -function loopbackConsent(redirectUri: string): ApprovalDialogOptions['consent'] { +function consentRedirectingTo(redirectUri: string): ApprovalDialogOptions['consent'] { + const { hostname } = new URL(redirectUri) return { - clientId: 'https://client.example/oauth/client.json', - clientDomain: 'client.example', - clientName: 'Native client', + clientId: 'opaque-client-id', + clientName: 'Test client', redirectUri, - redirectHost: new URL(redirectUri).hostname, - redirectIsLoopback: true, + redirectHost: hostname, + redirectIsLoopback: hostname !== 'callback.example', scope: [] } } @@ -48,8 +47,8 @@ function loopbackConsent(redirectUri: string): ApprovalDialogOptions['consent'] /** The nonce a page's policy lets scripts run with. */ function policyNonce(policy: string | null): string { const nonce = policy?.match(/script-src 'nonce-([^']+)'/)?.[1] - expect(nonce).toBeTruthy() - return nonce! + if (!nonce) throw new Error(`No script nonce in the policy: ${policy}`) + return nonce } /** @@ -198,31 +197,33 @@ describe('OAuth page Content-Security-Policy', () => { expect(first).not.toBe(second) }) - it('lets both buttons reach their redirects: Cloudflare and the client', () => { - // Chrome applies form-action to the redirect that follows a submission. - expect(renderResponse().headers.get('Content-Security-Policy')).toContain( - "form-action 'self' https://dash.cloudflare.com https://callback.example" - ) - const loopback = renderResponse({ - consent: loopbackConsent('http://127.0.0.1:6274/oauth/callback') - }) - expect(loopback.headers.get('Content-Security-Policy')).toContain( - "form-action 'self' https://dash.cloudflare.com http://127.0.0.1:6274" + it('gives every client the same policy, with no form-action to stop Continue or Cancel redirecting', () => { + // Chrome applies form-action to the redirect after a submission: to Cloudflare on Continue, + // to the client on Cancel. CSP can't name an IPv6 literal, and the client picks its origin. + const policies = [ + 'https://callback.example/oauth/callback', + 'http://127.0.0.1:6274/oauth/callback', + 'http://[::1]:6274/oauth/callback' + ].map((redirectUri) => + renderResponse({ consent: consentRedirectingTo(redirectUri) }) + .headers.get('Content-Security-Policy') + ?.replaceAll(/'nonce-[^']+'/g, "'nonce'") ) + + expect(new Set(policies).size).toBe(1) + expect(policies[0]).not.toContain('form-action') }) - it.each(['http://[::1]:6274/oauth/callback', 'cursor://anysphere.cursor-mcp/oauth/callback'])( - 'leaves out form-action for a redirect URI CSP cannot name: %s', - (redirectUri) => { - const policy = renderResponse({ consent: loopbackConsent(redirectUri) }).headers.get( - 'Content-Security-Policy' - ) + it('escapes the client name in the title bar', async () => { + const body = await render({ + consent: { + ...consentRedirectingTo('https://callback.example/cb'), + clientName: 'x' + } + }) - expect(policy).not.toContain('form-action') - expect(policy).not.toContain('null') - policyNonce(policy) - } - ) + expect(body).toContain('Authorize </title><b>x | Cloudflare') + }) it('runs only the script and styles the error page ships', async () => { const response = renderErrorPage('Server Error', 'Try again.') diff --git a/tests/auth/oauth-routes.test.ts b/tests/auth/oauth-routes.test.ts index 078ea5e7..05ea0a60 100644 --- a/tests/auth/oauth-routes.test.ts +++ b/tests/auth/oauth-routes.test.ts @@ -253,15 +253,13 @@ describe('GET /authorize', () => { // Consent form with CSRF protection and a session-binding cookie. expect(body).toContain('`) - expect(policy).toContain( - `form-action 'self' https://dash.cloudflare.com ${new URL(REDIRECT_URI).origin}` - ) + expect(policy).not.toContain('form-action') expect(policy).toContain("frame-ancestors 'none'") expect(res.headers.get('X-Frame-Options')).toBe('DENY') // Cloudflare's authorization screen picks individual scopes. This page offers