diff --git a/AGENTS.md b/AGENTS.md index 1c001ee..fcb878a 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 ``} + + +` + + 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 }) +} + /** * Render a URL as the browser parsed it, without credentials or a fragment. * The host keeps the default text colour and the scheme and path are dimmed, @@ -347,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

@@ -571,12 +646,8 @@ export function renderApprovalDialog(request: Request, options: ApprovalDialogOp
- -
- - ${PAGE_FOOTER_HTML} - - - - -` - - // beginConsent() headers: the browser binding cookie, frame-ancestors 'none', X-Frame-Options DENY - headers.set('Content-Type', 'text/html; charset=utf-8') - return new Response(htmlContent, { headers }) + ` + }, + { headers } + ) } /** @@ -759,15 +826,10 @@ export function renderErrorPage( details?: string, status = 400 ): Response { - const htmlContent = ` - - - - - - ${sanitizeHtml(title)} | Cloudflare - ${PAGE_FONT_LINKS} - - - - ${PAGE_HEADER_HTML} -
-
+ `, + card: `
@@ -811,20 +869,13 @@ export function renderErrorPage(

${sanitizeHtml(title)}

${sanitizeHtml(message)}

${details ? `
${sanitizeHtml(details)}
` : ''} - Close window -
-
- ${PAGE_FOOTER_HTML} - - -` - - return new Response(htmlContent, { - status, - headers: { - 'Content-Security-Policy': "frame-ancestors 'none'", - 'Content-Type': 'text/html; charset=utf-8', - 'X-Frame-Options': 'DENY' - } - }) + + `, + script: ` + document.getElementById('closeWindow').addEventListener('click', () => window.close()); + `, + formAction: "'none'" + }, + { status } + ) } diff --git a/tests/auth/approval-dialog.test.ts b/tests/auth/approval-dialog.test.ts index e9a3f75..a0b031a 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', @@ -26,8 +26,44 @@ function render(options: Partial = {}): Promise { initialScopes: [], ...options }) +} - return response.text() +function render(options: Partial = {}): Promise { + return renderResponse(options).text() +} + +function consentRedirectingTo(redirectUri: string): ApprovalDialogOptions['consent'] { + const { hostname } = new URL(redirectUri) + return { + clientId: 'opaque-client-id', + clientName: 'Test client', + redirectUri, + redirectHost: hostname, + redirectIsLoopback: hostname !== 'callback.example', + scope: [] + } +} + +/** The nonce a page's policy lets scripts run with. */ +function policyNonce(policy: string | null): string { + const nonce = policy?.match(/script-src 'nonce-([^']+)'/)?.[1] + if (!nonce) throw new Error(`No script nonce in the policy: ${policy}`) + 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 +174,72 @@ 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('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('escapes the client name in the title bar', async () => { + const body = await render({ + consent: { + ...consentRedirectingTo('https://callback.example/cb'), + clientName: 'x' + } + }) + + 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.') + 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 27d95d0..05ea0a6 100644 --- a/tests/auth/oauth-routes.test.ts +++ b/tests/auth/oauth-routes.test.ts @@ -253,6 +253,15 @@ describe('GET /authorize', () => { // Consent form with CSRF protection and a session-binding cookie. expect(body).toContain('`) + 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 // 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.