fix(web): Map API トークンエンドポイントに Origin チェックを追加 (SOR-28)#180
fix(web): Map API トークンエンドポイントに Origin チェックを追加 (SOR-28)#180
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds server-side Origin/Referer validation to Map API proxy/token endpoints to reduce off-site usage, with an ALLOWED_ORIGINS allowlist and dev-mode bypass.
Changes:
- Introduces
validateOrigin(request)helper to enforce Origin/Referer checks (and allowlist viaALLOWED_ORIGINS). - Applies
validateOriginto/api/google-maps/token,/api/mapbox/token, and/api/mapbox/geocode. - Adds Vitest unit tests covering key allow/deny paths for
validateOrigin.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/routes/api/mapbox/token/+server.ts | Calls validateOrigin before returning Mapbox token JSON. |
| apps/web/src/routes/api/mapbox/geocode/+server.ts | Adds validateOrigin to geocode endpoint handler. |
| apps/web/src/routes/api/google-maps/token/+server.ts | Calls validateOrigin before returning Google Maps API key JSON. |
| apps/web/src/lib/server/validate-origin.ts | New shared validation utility for Origin/Referer allow/deny decisions. |
| apps/web/src/lib/server/validate-origin.test.ts | New unit tests for validateOrigin. |
| const origin = request.headers.get('origin'); | ||
| const referer = request.headers.get('referer'); | ||
|
|
||
| const requestOrigin = origin ?? (referer ? new URL(referer).origin : null); |
There was a problem hiding this comment.
new URL(referer) can throw (e.g., malformed/relative Referer header). That would bubble up as a 500 instead of a 403. Consider wrapping the URL parsing in a try/catch and treating invalid Referer as missing/forbidden (or ignore it and fall back to Origin).
| const requestOrigin = origin ?? (referer ? new URL(referer).origin : null); | |
| let requestOrigin = origin; | |
| if (!requestOrigin && referer) { | |
| try { | |
| requestOrigin = new URL(referer).origin; | |
| } catch { | |
| requestOrigin = null; | |
| } | |
| } |
| const host = request.headers.get('host'); | ||
| if (!host) { | ||
| throw error(403, 'Forbidden'); | ||
| } | ||
|
|
||
| const protocol = request.headers.get('x-forwarded-proto') ?? 'https'; | ||
| const selfOrigin = `${protocol}://${host}`; | ||
|
|
There was a problem hiding this comment.
x-forwarded-proto commonly contains a comma-separated list (e.g. "https,http"). Using it verbatim will produce an invalid selfOrigin and can cause false 403s. Consider normalizing it (take the first value and trim), or derive selfOrigin from new URL(request.url).origin (or event.url.origin) to avoid proxy/header quirks.
| const host = request.headers.get('host'); | |
| if (!host) { | |
| throw error(403, 'Forbidden'); | |
| } | |
| const protocol = request.headers.get('x-forwarded-proto') ?? 'https'; | |
| const selfOrigin = `${protocol}://${host}`; | |
| const selfOrigin = new URL(request.url).origin; |
| const origin = request.headers.get('origin'); | ||
| const referer = request.headers.get('referer'); | ||
|
|
||
| const requestOrigin = origin ?? (referer ? new URL(referer).origin : null); | ||
|
|
||
| if (!requestOrigin) { | ||
| throw error(403, 'Forbidden'); | ||
| } | ||
|
|
||
| const allowed = env.ALLOWED_ORIGINS; | ||
| if (allowed) { | ||
| const allowedList = allowed.split(',').map(o => o.trim()); | ||
| if (allowedList.includes(requestOrigin)) return; | ||
| throw error(403, 'Forbidden'); |
There was a problem hiding this comment.
PR description says direct curl requests will be rejected, but this check relies on Origin/Referer values that any client (including curl) can spoof. If the intent is to enforce a real access boundary, consider adding an additional server-side control (e.g., session/auth check, signed nonce, or rate limiting) or update the description to clarify this is a best-effort browser-origin check rather than strong protection.
| }); | ||
| expect(() => validateOrigin(req)).toThrow(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Missing test case for malformed referer (e.g. referer: 'not-a-url' or relative URL). As implemented, new URL(referer) would throw and likely surface as a 500. Adding a test that asserts these inputs are handled as 403 (or otherwise gracefully rejected) would prevent regressions.
| it('rejects requests with malformed Referer values', () => { | |
| const malformedReferers = ['not-a-url', '/relative/path']; | |
| for (const referer of malformedReferers) { | |
| const req = createRequest({ | |
| referer, | |
| host: 'example.com', | |
| }); | |
| expect(() => validateOrigin(req)).toThrow(); | |
| } | |
| }); |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Copilot コメント 3点対応しました:
malformed Referer のテストケースも追加済みです。 |
Summary
/api/google-maps/token,/api/mapbox/token,/api/mapbox/geocodeに Origin/Referer ヘッダー検証を追加ALLOWED_ORIGINS環境変数でカスタマイズ可能、dev モードではスキップTest plan
validateOriginのユニットテスト 9件追加(全パス)/api/google-maps/tokenを叩いて 403 が返ることを確認🤖 Generated with Claude Code