From c13875cef8013b9735d28e12acec1557620155d9 Mon Sep 17 00:00:00 2001 From: brunorezio Date: Sat, 25 Jul 2026 22:47:38 +0100 Subject: [PATCH] return 400 for an unparseable body, and cover origin validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bodyParser caught parse failures and did nothing — the throw was commented out and `body` was never set, so handlers destructured undefined and the client got a 500 for a malformed request. Throw BAD_REQUEST instead. Also adds the regression tests for the Host suffix match fixed in 2ca850c. Verified against a running server: malformed JSON now 400, valid credentials path still 401, spoofed Host still 403. Co-Authored-By: Claude Opus 5 --- src/servers/_middlewares/body-parser.ts | 6 ++-- .../_middlewares/origin-validation.test.ts | 32 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 src/servers/_middlewares/origin-validation.test.ts diff --git a/src/servers/_middlewares/body-parser.ts b/src/servers/_middlewares/body-parser.ts index 7fa8df81..2ac1677b 100644 --- a/src/servers/_middlewares/body-parser.ts +++ b/src/servers/_middlewares/body-parser.ts @@ -24,8 +24,10 @@ export const bodyParser: () => MiddlewareHandler = () => async (ctx, next) => { // No recognized content type, set empty body ctx.set('body', {}); } - } catch (ex) { - // throw errors.BAD_REQUEST('Invalid request body'); + } catch { + // Previously swallowed, which left `body` unset — handlers then destructured undefined and the + // client got a 500 for what is squarely a malformed request. + throw errors.BAD_REQUEST('Invalid request body'); } return next(); diff --git a/src/servers/_middlewares/origin-validation.test.ts b/src/servers/_middlewares/origin-validation.test.ts new file mode 100644 index 00000000..f2fa8492 --- /dev/null +++ b/src/servers/_middlewares/origin-validation.test.ts @@ -0,0 +1,32 @@ +import { test, expect } from 'bun:test'; + +// origin-validation reads PUBLIC_URL / PUBLIC_BUILD_ENV at module load, so set them before importing. +process.env.PUBLIC_URL = 'https://officer.example.com'; +process.env.PUBLIC_BUILD_ENV = 'production'; + +const { isOriginAllowed } = await import('./origin-validation'); + +test('accepts the configured origin', () => { + expect(isOriginAllowed('https://officer.example.com', 'officer.example.com')).toBe(true); +}); + +test('accepts the Host forwarded by the reverse proxy when there is no Origin', () => { + expect(isOriginAllowed(undefined, 'officer.example.com')).toBe(true); +}); + +test('rejects a foreign Origin', () => { + expect(isOriginAllowed('https://evil.com', 'officer.example.com')).toBe(false); + expect(isOriginAllowed('https://officer.example.com.evil.com', 'officer.example.com')).toBe(false); +}); + +// Regression: the Host branch used `configuredOrigin.endsWith(host)`, so any suffix of the origin — +// down to a bare TLD — authenticated as the real host. +test('rejects Hosts that are merely suffixes of the configured origin', () => { + for (const host of ['com', 'example.com', 'r.example.com', 'ficer.example.com', 'evil.com']) { + expect(isOriginAllowed(undefined, host)).toBe(false); + } +}); + +test('rejects a request carrying neither Origin nor Host', () => { + expect(isOriginAllowed(undefined, undefined)).toBe(false); +});