From e4bf6402259f4cfc11085720f3e0da29b08ddc12 Mon Sep 17 00:00:00 2001 From: Cornelius Roemer Date: Fri, 31 Jan 2025 00:03:35 +0100 Subject: [PATCH 1/4] fix: respond with 200 to HEAD requests for non-prerendered pages as well Fixes #13079 Inspired by @joshmkennedy's PR #13100 --- .changeset/tricky-toes-drum.md | 5 ++++ packages/astro/src/core/app/middlewares.ts | 12 ++++----- packages/astro/test/csrf-protection.test.js | 28 +++++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 .changeset/tricky-toes-drum.md diff --git a/.changeset/tricky-toes-drum.md b/.changeset/tricky-toes-drum.md new file mode 100644 index 000000000000..4c5435c620d6 --- /dev/null +++ b/.changeset/tricky-toes-drum.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where HEAD requests for non-prerendered pages were incorrectly rejected with 403 FORBIDDEN diff --git a/packages/astro/src/core/app/middlewares.ts b/packages/astro/src/core/app/middlewares.ts index 7c589f0c4dce..f94f130101e0 100644 --- a/packages/astro/src/core/app/middlewares.ts +++ b/packages/astro/src/core/app/middlewares.ts @@ -13,6 +13,8 @@ const FORM_CONTENT_TYPES = [ 'text/plain', ]; +const SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; + /** * Returns a middleware function in charge to check the `origin` header. * @@ -25,15 +27,11 @@ export function createOriginCheckMiddleware(): MiddlewareHandler { if (isPrerendered) { return next(); } - if (request.method === 'GET') { + // Safe methods don't require origin check + if (SAFE_METHODS.includes(request.method)) { return next(); } - const sameOrigin = - (request.method === 'POST' || - request.method === 'PUT' || - request.method === 'PATCH' || - request.method === 'DELETE') && - request.headers.get('origin') === url.origin; + const sameOrigin = request.headers.get('origin') === url.origin; const hasContentType = request.headers.has('content-type'); if (hasContentType) { diff --git a/packages/astro/test/csrf-protection.test.js b/packages/astro/test/csrf-protection.test.js index 5b70e36505f6..0d3776012879 100644 --- a/packages/astro/test/csrf-protection.test.js +++ b/packages/astro/test/csrf-protection.test.js @@ -176,6 +176,34 @@ describe('CSRF origin check', () => { }); }); + it("return a 200 when the origin doesn't match but calling HEAD", async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'multipart/form-data' }, + method: 'HEAD', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), {}); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, + method: 'HEAD', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), {}); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, + method: 'HEAD', + }); + response = await app.render(request); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), {}); + }); + it('return 200 when calling POST/PUT/DELETE/PATCH with the correct origin', async () => { let request; let response; From 1dbf7ec12f0a8d37b47020a6fb54e621c4c1fd2f Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 13 Feb 2025 11:37:43 +0000 Subject: [PATCH 2/4] chore: add more test cases --- packages/astro/src/core/app/middlewares.ts | 9 +++--- packages/astro/test/csrf-protection.test.js | 29 +++++++++++++++++-- .../csrf-check-origin/src/pages/api.ts | 19 ++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/packages/astro/src/core/app/middlewares.ts b/packages/astro/src/core/app/middlewares.ts index f94f130101e0..91aefc2783e4 100644 --- a/packages/astro/src/core/app/middlewares.ts +++ b/packages/astro/src/core/app/middlewares.ts @@ -13,7 +13,8 @@ const FORM_CONTENT_TYPES = [ 'text/plain', ]; -const SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; +// Note: TRACE is unsupported by undici/Node.js +const SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS']; /** * Returns a middleware function in charge to check the `origin` header. @@ -31,18 +32,18 @@ export function createOriginCheckMiddleware(): MiddlewareHandler { if (SAFE_METHODS.includes(request.method)) { return next(); } - const sameOrigin = request.headers.get('origin') === url.origin; + const isSameOrigin = request.headers.get('origin') === url.origin; const hasContentType = request.headers.has('content-type'); if (hasContentType) { const formLikeHeader = hasFormLikeHeader(request.headers.get('content-type')); - if (formLikeHeader && !sameOrigin) { + if (formLikeHeader && !isSameOrigin) { return new Response(`Cross-site ${request.method} form submissions are forbidden`, { status: 403, }); } } else { - if (!sameOrigin) { + if (!isSameOrigin) { return new Response(`Cross-site ${request.method} form submissions are forbidden`, { status: 403, }); diff --git a/packages/astro/test/csrf-protection.test.js b/packages/astro/test/csrf-protection.test.js index 0d3776012879..717cc3081f81 100644 --- a/packages/astro/test/csrf-protection.test.js +++ b/packages/astro/test/csrf-protection.test.js @@ -185,7 +185,6 @@ describe('CSRF origin check', () => { }); response = await app.render(request); assert.equal(response.status, 200); - assert.deepEqual(await response.json(), {}); request = new Request('http://example.com/api/', { headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, @@ -193,7 +192,6 @@ describe('CSRF origin check', () => { }); response = await app.render(request); assert.equal(response.status, 200); - assert.deepEqual(await response.json(), {}); request = new Request('http://example.com/api/', { headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, @@ -201,9 +199,34 @@ describe('CSRF origin check', () => { }); response = await app.render(request); assert.equal(response.status, 200); - assert.deepEqual(await response.json(), {}); }); + it("return a 200 when the origin doesn't match but calling OPTIONS", async () => { + let request; + let response; + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'multipart/form-data' }, + method: 'OPTIONS', + }); + response = await app.render(request); + assert.equal(response.status, 200); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'application/x-www-form-urlencoded' }, + method: 'OPTIONS', + }); + response = await app.render(request); + assert.equal(response.status, 200); + + request = new Request('http://example.com/api/', { + headers: { origin: 'http://loreum.com', 'content-type': 'text/plain' }, + method: 'OPTIONS', + }); + response = await app.render(request); + assert.equal(response.status, 200); + }); + + it('return 200 when calling POST/PUT/DELETE/PATCH with the correct origin', async () => { let request; let response; diff --git a/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts b/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts index 8aa35cc25859..f35abb6a9dcb 100644 --- a/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts +++ b/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts @@ -27,3 +27,22 @@ export const PATCH = () => { something: 'true', }); }; + +export const HEAD = () => { + return Response.json({ + something: 'true', + }); +}; + +export const OPTIONS = () => { + return Response.json({ + something: 'true', + }); +}; + + +export const TRACE = () => { + return Response.json({ + something: 'true', + }); +}; From b64fa2e15944b4e367c622c22917e9439a1ad41d Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 13 Feb 2025 11:40:06 +0000 Subject: [PATCH 3/4] Update .changeset/tricky-toes-drum.md --- .changeset/tricky-toes-drum.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tricky-toes-drum.md b/.changeset/tricky-toes-drum.md index 4c5435c620d6..087e466be54d 100644 --- a/.changeset/tricky-toes-drum.md +++ b/.changeset/tricky-toes-drum.md @@ -2,4 +2,4 @@ 'astro': patch --- -Fixes a bug where HEAD requests for non-prerendered pages were incorrectly rejected with 403 FORBIDDEN +Fixes a bug where `HEAD` and `OPTIONS` requests for non-prerendered pages were incorrectly rejected with 403 FORBIDDEN From 51b1adb6ff0522cb6cc8b3b830f23a57a88752a9 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 13 Feb 2025 11:40:40 +0000 Subject: [PATCH 4/4] chore: remove trace method --- .../astro/test/fixtures/csrf-check-origin/src/pages/api.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts b/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts index f35abb6a9dcb..cffd2c2385ba 100644 --- a/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts +++ b/packages/astro/test/fixtures/csrf-check-origin/src/pages/api.ts @@ -39,10 +39,3 @@ export const OPTIONS = () => { something: 'true', }); }; - - -export const TRACE = () => { - return Response.json({ - something: 'true', - }); -};