-
Notifications
You must be signed in to change notification settings - Fork 1
Fix critical security vulnerability in Stripe webhook handler #61
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
base: saas
Are you sure you want to change the base?
Fix critical security vulnerability in Stripe webhook handler #61
Conversation
Prevents usage of test secrets in production environment. Added regression test in `tests/security/test_stripe_webhook_security.py`. Documented findings in `SECURITY_AUDIT.md`.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
Prevents usage of test secrets in production environment. Added regression test in `tests/security/test_stripe_webhook_security.py`. Documented findings in `SECURITY_AUDIT.md`.
Prevents usage of test secrets in production environment. Added regression test in `tests/security/test_stripe_webhook_security.py`. Documented findings in `SECURITY_AUDIT.md`. Configured Vulture to exclude security test file in `pyproject.toml`.
Miyamura80
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls also delete the whitelist and the stripe Webhooks security test. Doesn't feel relevant
Removed the description of the fixed Stripe webhook vulnerability from SECURITY_AUDIT.md as requested in PR review.
Removed `tests/security/test_stripe_webhook_security.py` and `whitelist.tmp` as requested. Reverted Vulture configuration in `pyproject.toml`.
Greptile SummaryThis PR successfully fixes a critical security vulnerability in the Stripe webhook handler where test-signed payloads could be accepted in production environments. Key Changes:
Issues Found:
Non-functional changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Stripe as Stripe Webhook
participant Webhook as /webhook/stripe or /webhook/usage-reset
participant Auth as _try_construct_event
participant Config as global_config
participant DB as Database
Stripe->>Webhook: POST with signature header
Webhook->>Auth: Verify signature
Auth->>Config: Check DEV_ENV
alt DEV_ENV == "prod"
Config-->>Auth: Return STRIPE_WEBHOOK_SECRET only
Note over Auth: ✅ Only prod secret accepted
else DEV_ENV != "prod"
Config-->>Auth: Return STRIPE_TEST_WEBHOOK_SECRET only
Note over Auth: ✅ Only test secret accepted
end
Auth->>Auth: Try stripe.Webhook.construct_event
alt Valid signature
Auth-->>Webhook: Return event data
Webhook->>DB: Process subscription/usage update
DB-->>Webhook: Success
Webhook-->>Stripe: 200 OK
else Invalid signature
Auth-->>Webhook: HTTPException 400
Webhook-->>Stripe: 400 Invalid signature
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (3)
-
src/api/routes/payments/webhooks.py, line 24-27 (link)syntax: Docstring is outdated and contradicts the implementation. No fallback to alternate secret occurs anymore.
-
src/api/routes/payments/webhooks.py, line 79 (link)syntax: Comment is outdated - no longer tries alternate secret, only environment-appropriate secret.
-
src/api/routes/payments/webhooks.py, line 135 (link)syntax: Comment is outdated - no longer tries alternate secret, only environment-appropriate secret.
4 files reviewed, 3 comments
This PR fixes a critical security vulnerability in the Stripe webhook handler where payloads signed with the Test Secret were accepted even when the application was running in Production mode.
Changes:
src/api/routes/payments/webhooks.pyto strictly enforce secret usage based onDEV_ENV.tests/security/test_stripe_webhook_security.pyto verify the fix and prevent regressions.SECURITY_AUDIT.mddocumenting the finding and other low-severity issues.PR created automatically by Jules for task 317896489714379337 started by @Miyamura80