Commit 90b0ccd
authored
fix(security): replace forgeable header trust with HMAC session token (koala73#3541) (koala73#3557)
* fix(security): replace forgeable header trust with HMAC session token (koala73#3541)
The previous validateApiKey trusted the Origin header (and a Referer
fallback) to grant trusted-browser access without an API key. Both
headers are entirely client-controlled at the wire level — curl can set
either with one flag, bypassing the gate. Closed PR koala73#3554 attempted to
move the trust to Sec-Fetch-Site; same problem (Forbidden Header is a
browser-side JS API restriction, not a wire-level guarantee). Only
cryptographic proof closes that bypass class.
The fix:
- Add api/_session.js: HMAC-SHA256 (Web Crypto) token issuer/validator.
Tokens are wms_<base64url(payload)>.<sig>, payload {iat,exp,n}, 12h TTL.
Fails closed when WM_SESSION_SECRET is unset (operator-visible 503).
- Add api/wm-session.js: POST endpoint that mints a token, rate-limited
per IP via the existing checkRateLimit. Returns 503 if secret unset.
- Rewrite api/_api-key.js: drop the Origin/Referer no-key trust paths
entirely. Browsers now authenticate via wms_ token, wm_ user key,
enterprise key, or (via the gateway) a valid Clerk JWT.
- Add server-side gateway override: tier-gated routes with a resolved
Clerk session pass without a key (the Clerk JWT is the auth).
- Add src/services/wm-session.ts: thin client that fetches+caches the
token and patches globalThis.fetch ONCE at boot to attach
X-WorldMonitor-Key on calls to our API origin. Avoids touching ~50
individual fetch sites. Skips when caller already set Authorization /
X-WorldMonitor-Key / X-Api-Key (Clerk Bearer takes precedence).
- Wire into App.ts boot before fetchBootstrapData (skipped on desktop
which uses its own enterprise-key path).
- Migrate validateApiKey to async + update all 7 call sites; add 35
unit tests covering the bypass regression, defense-in-depth (combined
header forgeries), and existing flows (Tauri, Bearer, forceKey).
- Update tests/premium-stock-gateway, tests/gateway-cdn-origin-policy,
tests/usage-telemetry-emission to provide a valid wms_ token where
they previously relied on the trusted-origin bypass — mirrors the
production fetch-interceptor flow.
Operator action required before merge: set WM_SESSION_SECRET (≥32 chars)
in Vercel env. Without it, /api/wm-session returns 503 and anonymous
browser users cannot authenticate.
Closes koala73#3541. Replaces closed PR koala73#3554.
* fix(security): close wms_ entitlement-bypass + canonical b64url defence (koala73#3557 review)
Two BLOCKING findings from PR koala73#3557 review:
1. Premium-endpoint bypass via anonymous session token
----------------------------------------------------
validateApiKey returned valid:true for any wms_ token; gateway
treated any non-wm_ valid key as enterprise/admin and skipped
entitlement checks. Result: a freshly minted anonymous wms_ token
got 200 on /api/market/v1/analyze-stock and
/api/resilience/v1/get-resilience-score with no Clerk session, no
Pro entitlement, no enterprise key.
Fix:
- validateApiKey result now carries kind: 'session' | 'user' |
'enterprise'. Only kind='enterprise' bypasses entitlement.
- validateApiKey rejects wms_ when forceKey=true (premium /
tier-gated endpoints set forceKey exactly because they need
user-bound auth).
- gateway.ts entitlement-skip condition tightened to require
kind === 'enterprise', not just "valid && not isUserApiKey".
2. Tamper test was non-deterministic (intermittent CI failure)
-----------------------------------------------------------
_session.test.mjs flipped the last base64url char of the signature.
For SHA-256 (32 bytes -> 43 b64url chars, no padding), the last
char encodes 2 high bits of byte 32 plus 4 unused padding bits --
flipping only padding bits decodes to identical signature bytes
and passes HMAC verification. Whether the test caught tampering
depended on the random nonce of that run.
Fix:
- Tamper test now decodes the signature bytes, XORs the first
byte, and re-encodes -- guaranteed to differ.
- Added a defence-in-depth canonical-encoding check inside
validateSessionToken: re-encode decoded bytes and require an
exact match. Rejects any non-canonical b64url that would
otherwise sneak through atob's permissive decoder.
- Added an explicit non-canonical-padding-bit-flip test.
Regression coverage:
- _api-key.test.mjs: forceKey=true + wms_ -> reject; wms_ result
kind !== 'enterprise'; enterprise key kind === 'enterprise'.
- premium-stock-gateway.test.mts: explicit gateway-level test that
anonymous wms_ MUST NOT unlock /analyze-stock or
/get-resilience-score (locks the entitlement-bypass anti-regression).
All checks: typecheck, typecheck:api, lint, bundle, test:data
(7820 / 7820 pass), api session/key/wm-session unit tests
(37 / 37 pass).
* chore(api-contract): list api/wm-session.js as ops-admin exception
Sebuf-API-contract lint flagged api/wm-session.js as a non-proto JSON
endpoint without an exception entry. Add it under ops-admin: a session-
bootstrap mint with a 2-field response is below the bar where a sebuf
RPC adds value, and shaped by Vercel Edge / fetch interceptor needs
rather than a product surface.
Reviewer: @SebastienMelki per CODEOWNERS.
Context: PR koala73#3557, issue koala73#3541.
Fixes the biome-job lint:api-contract failure on PR koala73#3557.
* fix(security): wms_ wrapper steps aside on premium routes + probe mints token (koala73#3557 review)
Two BLOCKING findings from PR koala73#3557 review:
1. wms_ wrapper suppressed Clerk auth on premium routes
----------------------------------------------------
installWebApiRedirect (main.ts:712) installs the premium-auth
injector first; my installWmSessionFetchInterceptor (App.ts:946)
wraps that patched fetch second. Result: wms_ wrapper runs as the
OUTER layer and sets X-WorldMonitor-Key: wms_... on every API call
before delegating. The inner premium injector (enrichInitForPremium)
then sees an existing X-WorldMonitor-Key and returns early, so it
never adds the Clerk Authorization: Bearer ... header. Plain or
generated premium RPC fetches from signed-in Pro users were being
sent with only the anonymous wms_ token and got 401 from the
server (combined with the previous review fix that rejects wms_
on forceKey=true premium endpoints).
Fix: wms_ wrapper now early-returns for any path in
PREMIUM_RPC_PATHS (the same canonical set the premium injector
consumes), letting the inner layer attach Clerk Bearer / API key /
tester key as appropriate.
2. seed-contract-probe public-boundary check now 401s /api/bootstrap
------------------------------------------------------------------
checkPublicBoundary() sent /api/bootstrap with only an Origin
header, relying on the now-removed trusted-browser path. Post-PR,
/api/bootstrap requires a wms_ token, so the probe marked the
boundary check as bad status 401 and returned 503.
Fix: probe now mints its own wms_ token in-process via
issueSessionToken() and passes it as X-WorldMonitor-Key. Same
WM_SESSION_SECRET environment as /api/wm-session, so this is
equivalent to the round-trip without the extra hop. Falls back
gracefully if the secret is unset — the resulting 401 surfaces as
the right operator signal for missing config.
All checks: typecheck x 2, lint, lint:api-contract, bundle (clean),
test:data 7820/7820, sidecar 37/37.
* fix(telemetry): require kind=enterprise to set usage.enterpriseApiKey (koala73#3557 review)
The enterprise-key marker condition (gateway.ts:475) was:
keyCheck.valid && wmKey && !isUserApiKey && !wmKey.startsWith('wm_')
Anonymous wms_ tokens slip through every clause: validateApiKey returns
valid:true, wmKey is set, isUserApiKey only flips for wm_ user keys,
and 'wms_' does NOT start with 'wm_' (third char is 's', not '_').
Result: usage.enterpriseApiKey gets the wms_ string and downstream
telemetry emits auth_kind:'enterprise_api_key' with
customer_id:'enterprise-unmapped' for every anonymous browser request.
Fix: tighten the condition to require keyCheck.kind === 'enterprise'
-- the kind field already added in the previous review pass for the
entitlement-bypass fix. Telemetry now correctly emits auth_kind:'anon'
for anonymous wms_ traffic.
Regression test added in tests/usage-telemetry-emission.test.mts that
locks the contract: anon wms_ token MUST telemeter as anon, MUST NOT
telemeter as enterprise-unmapped.1 parent 26ed611 commit 90b0ccd
24 files changed
Lines changed: 951 additions & 75 deletions
File tree
- api
- v2/shipping/webhooks
- [subscriberId]
- server
- worldmonitor/shipping/v2
- src
- services
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
1 | 3 | | |
2 | 4 | | |
3 | 5 | | |
4 | 6 | | |
5 | 7 | | |
6 | 8 | | |
7 | 9 | | |
8 | | - | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | 10 | | |
18 | 11 | | |
19 | 12 | | |
20 | 13 | | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
32 | 18 | | |
33 | 19 | | |
34 | | - | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
35 | 40 | | |
36 | 41 | | |
37 | | - | |
38 | | - | |
39 | | - | |
| 42 | + | |
40 | 43 | | |
41 | | - | |
| 44 | + | |
42 | 45 | | |
43 | 46 | | |
44 | | - | |
45 | | - | |
46 | | - | |
| 47 | + | |
| 48 | + | |
47 | 49 | | |
48 | 50 | | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
53 | 60 | | |
54 | | - | |
55 | | - | |
56 | | - | |
| 61 | + | |
| 62 | + | |
57 | 63 | | |
58 | | - | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
59 | 72 | | |
60 | 73 | | |
61 | | - | |
| 74 | + | |
62 | 75 | | |
63 | | - | |
64 | | - | |
65 | | - | |
| 76 | + | |
| 77 | + | |
66 | 78 | | |
67 | 79 | | |
68 | | - | |
| 80 | + | |
69 | 81 | | |
70 | 82 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
75 | | - | |
| 75 | + | |
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
| |||
0 commit comments