Skip to content

DRAFT security: harden merchant key lookup (do not merge until J5 PASS and owner approval) - #85

Merged
ceedot-rock merged 1 commit into
mainfrom
security/merchant-key-lookup-hardening
Oct 21, 2026
Merged

ceedot-rock merged 1 commit into
mainfrom
security/merchant-key-lookup-hardening

Conversation

@ceedot-rock

Copy link
Copy Markdown
Owner

Status: DRAFT. Not for merge. Waits for J5 PASS and the owner's approval. Nothing has been deployed.

Problem

The merchant key from the X-Merchant-Key header (and the /api/verify body) was pasted unescaped into a Stripe Search query in findSubscriptionByMerchantKey (src/lib/stripe.ts). A crafted value could widen the query and match another merchant's subscription. Callers: /api/rider/issue, /api/verify, /api/v0/citizen-receipts, toll billing (resolveTollPayer).

Change

  1. New src/lib/merchant-key-core.mjs: strict allowlist ^merchant_live_[0-9a-f]{48}$ (exact length) checked before any query is built. A malformed key never reaches Stripe.
  2. findSubscriptionByMerchantKey now goes through that module and, after the search, requires the stored metadata.merchant_key to equal the presented key (constant time). A search hit alone no longer grants access.
  3. /api/verify: rejects malformed keys with 400 before lookup; the monthly usage counter name uses a one-way reference (verify:<sha256 prefix>) instead of the raw key.
  4. Toll billing payer id used the first 16 characters of the key; it now uses the same one-way reference.
  5. Test: src/lib/merchant-key.selftest.mjs (npm run selftest:merchant-key). Offline, fake keys only (single repeated hex digit). 20 hostile headers including ones built to match an active subscription; asserts Stripe is never queried for them, the pre-fix lookup is shown to be widened by the same inputs in an emulated search, and the post-search equality check holds even if Stripe returns the wrong subscription.

Test result

node lib/merchant-key.selftest.mjs passes on Node 20.19 and Node 24.22. tsc --noEmit reports no errors in the touched files.

Known limits

  • The fake Stripe is an emulation of a small slice of the search syntax. It shows the old query shape was widenable and the new code never sends one. It does not prove how live Stripe parses a hostile string.
  • This does not remove plaintext keys from Stripe metadata. That is the follow-up plan (hashed key registry).
  • Behavior change: /api/verify now returns 400 for a malformed key where it used to return valid:false. Other routes return their existing 402.
  • The selftest is not yet wired into the CI workflow (.github/workflows/webpack.yml); editing a workflow is left to the owner.
  • Not covered here: the X-Platform-Key path in /api/rider/issue (see the report in the Slid Phi Labs space).

No real key appears in this PR.

…in counters

The merchant key from X-Merchant-Key / the verify body was pasted unescaped into a Stripe Search query. Allowlist the exact key shape first, require stored metadata to equal the presented key, and use a one-way reference instead of the raw key (or its first 16 chars) in the verify rate limit name and toll payer id. Adds an offline selftest with fake keys.
@ceedot-rock
ceedot-rock marked this pull request as ready for review October 21, 2026 23:15
@ceedot-rock
ceedot-rock merged commit aa9791f into main Oct 21, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant