Repository navigation
fix(security): close protected billing and identity trust gaps #589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 6 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
ba7554e
test: reproduce unsigned Stripe webhook plan escalation
seonghobae 042830d
fix: authenticate Stripe webhooks before entitlement handling
seonghobae cc0c6ed
test(billing): keep Stripe webhook behind abuse controls
seonghobae 1f5c2b2
fix(billing): keep Stripe verification inside app controls
seonghobae 4488ab9
test(ci): align public-route contract with Stripe repair
seonghobae dce2424
fix(billing): retire unsigned Stripe plan-upgrade handler
seonghobae 5e30072
test(server): prove toast route is unique
seonghobae 4ae65a6
test(a11y): exercise shipped toast asset route
seonghobae 87d34bd
test(security): bind invites to intended identity
seonghobae eaab3f8
fix(security): bind invite redemption to email
seonghobae 5dc72e7
test(ci): match secured route composition
seonghobae a1ad3bc
test(security): fail closed when OIDC is unconfigured
seonghobae 7b19b86
fix(security): require explicit development for mock OIDC
seonghobae 2d39860
ci: run owned-production coverage in server gate
seonghobae 77406a9
test(security): require invite safety in protected route graph
seonghobae d611894
test(security): cover shared OIDC fail-closed boundary
seonghobae 7c1f08c
fix(security): secure shared application boundary
seonghobae 394e3cc
fix(security): align OIDC regression and trust docs
seonghobae 7ef64d2
test(security): expose invite core bypass regression
seonghobae 36c1037
test(security): bind invite guard to live sessions
seonghobae bc50cc9
fix(security): reject revoked sessions before invite binding
seonghobae 40eb4d1
test(security): keep guard rejections behind abuse controls
seonghobae ab7c7fe
test(security): register guard abuse-control regression
seonghobae 72f6acc
fix(security): account rejected guards in abuse controls
seonghobae de0a053
test(stripe): preserve literal signature timestamp
seonghobae 131d79e
fix(stripe): verify literal signed timestamp
seonghobae cf67f2a
test(security): preserve guard rejection method evidence
seonghobae 177996e
fix(observability): retain original guard request method
seonghobae 04363c0
fix(observability): carry original guard method safely
seonghobae 25b95ed
test(security): require core OIDC fail-closed
seonghobae 5807225
test(security): require core invite fail-closed behavior
seonghobae 03cbe9f
fix(security): harden internal invite and OIDC boundaries
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| # Stripe webhook trust boundary | ||
|
|
||
| ## Decision | ||
|
|
||
| Protected `develop` previously accepted an unauthenticated | ||
| `checkout.session.completed` JSON body and upgraded `orgs.plan` to `pro`. That | ||
| is a privilege-escalation window: any caller who can POST to | ||
| `/api/stripe/webhook` can grant paid entitlements. | ||
|
|
||
| The public application now: | ||
|
|
||
| 1. copies the protected route graph, including logging and rate-limit | ||
| middleware, except the historical Stripe handler; | ||
| 2. registers a fail-closed verifier that HMAC-SHA-256-checks | ||
| `timestamp + "." + raw body` against `STRIPE_WEBHOOK_SECRET` before parsing | ||
| JSON; | ||
| 3. acknowledges an authentic delivery with `{ received: true }` and does **not** | ||
| mutate `orgs.plan`; and | ||
| 4. keeps the protected-graph copy of the same route fail-closed so a future | ||
| composer that mounts `application_routes` directly cannot restore unsigned | ||
| plan upgrades. | ||
|
|
||
| Signature validity authenticates the delivery only. Durable event | ||
| deduplication, provider-state reconciliation, and entitlement writes remain | ||
| follow-up work (stacked billing lifecycle), not this hotfix. | ||
|
|
||
| ## Standards rationale | ||
|
|
||
| HMAC-SHA-256 over the exact signed bytes is the Stripe webhook contract and | ||
| matches RFC 2104 / FIPS 198-1 keyed hashing. JSON parsing happens only after | ||
| constant-time comparison so semantically equivalent but byte-different bodies | ||
| cannot be substituted. Replay is bounded by a five-minute timestamp window. | ||
| Missing configuration fails closed with `503 stripe_webhook_not_configured` | ||
| rather than accepting unsigned traffic. | ||
|
|
||
| OAuth bearer-token rules (RFC 6750; RFC 9700) do not apply to this provider | ||
| callback; the webhook secret is a shared HMAC key, not an access token. The | ||
| endpoint remains behind the same abuse-control middleware as the rest of the | ||
| application so unsigned floods are rate-limited instead of skipping the limiter | ||
| through a first-match public route. | ||
|
|
||
| ## Verification contract | ||
|
|
||
| Regression tests must prove: | ||
|
|
||
| - unsigned `checkout.session.completed` JSON never upgrades `orgs.plan`; | ||
| - a correctly signed delivery is acknowledged and still leaves plan unchanged; | ||
| - a stale timestamp or a JSON-equivalent mutated body fails signature checks; | ||
| - when `SCOPEWEAVE_RATE_LIMIT_MAX=1`, the second webhook in the window is `429`; | ||
| - the public app does not `app.route('/', applicationRoutes)`; | ||
| - the public app copies protected routes except `POST /api/stripe/webhook` and | ||
| then registers `verifyStripeWebhookRequest`; and | ||
| - `application_routes.mjs` no longer contains a `checkout.session.completed` | ||
| plan-upgrade path. | ||
|
|
||
| ## Officer next action | ||
|
|
||
| Rotate `STRIPE_WEBHOOK_SECRET` if it may have been exposed while the unsigned | ||
| stub was live. Point the Stripe endpoint at the public `/api/stripe/webhook` | ||
| path. Do not treat a `200 { received: true }` as proof that the organization is | ||
| Pro until reconciliation writes entitlements from Stripe's retrieved session | ||
| state. | ||
|
|
||
| ## References | ||
|
|
||
| Krawczyk, H., Bellare, M., & Canetti, R. (1997). *HMAC: Keyed-hashing for | ||
| message authentication* (RFC 2104). Internet Engineering Task Force. | ||
| https://doi.org/10.17487/RFC2104 | ||
|
|
||
| National Institute of Standards and Technology. (2008). *The keyed-hash | ||
| message authentication code (HMAC)* (FIPS PUB 198-1). U.S. Department of | ||
| Commerce. https://doi.org/10.6028/NIST.FIPS.198-1 | ||
|
|
||
| Stripe. (n.d.). *Webhook signatures*. Stripe Docs. | ||
| https://docs.stripe.com/webhooks/signatures | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.