return 400 for an unparseable body, and cover origin validation
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1eb8cfd273
commit
c13875cef8
@@ -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();
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user