Skip to content

Conversation

@google-labs-jules
Copy link
Contributor

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:

  • Modified src/api/routes/payments/webhooks.py to strictly enforce secret usage based on DEV_ENV.
  • Added tests/security/test_stripe_webhook_security.py to verify the fix and prevent regressions.
  • Added SECURITY_AUDIT.md documenting the finding and other low-severity issues.

PR created automatically by Jules for task 317896489714379337 started by @Miyamura80

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`.
@google-labs-jules
Copy link
Contributor Author

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!


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`.
Copy link
Owner

@Miyamura80 Miyamura80 left a 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`.
@Miyamura80 Miyamura80 marked this pull request as ready for review December 28, 2025 05:36
@greptile-apps
Copy link

greptile-apps bot commented Dec 28, 2025

Greptile Summary

This PR successfully fixes a critical security vulnerability in the Stripe webhook handler where test-signed payloads could be accepted in production environments.

Key Changes:

  • Modified _try_construct_event() in webhooks.py to strictly enforce environment-based secret selection - production now only accepts production secrets, non-production only accepts test secrets
  • Removed dangerous cross-environment fallback logic that allowed test secrets to validate in prod
  • Added SECURITY_AUDIT.md documenting remaining low-severity issues (DB session handling, rate limiting, idempotency)
  • Test file was created then removed per request (visible in commit history)

Issues Found:

  • Outdated docstrings and comments still reference "fallback to alternate secret" behavior that no longer exists (lines 24-27, 79, 135)

Non-functional changes:

  • Minor formatting improvements to alembic/versions/33ae457b2ddf_add_referral_columns.py and src/db/utils/users.py (whitespace, quotes)

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk after updating outdated comments
  • The security fix is correct and addresses the critical vulnerability effectively. The implementation properly enforces environment-based secret selection. Only issue is outdated documentation (docstrings/comments) that contradicts the implementation - these should be fixed to prevent future confusion but don't affect runtime behavior. Score is 4/5 instead of 5/5 due to documentation inconsistencies.
  • Pay attention to src/api/routes/payments/webhooks.py - update the outdated docstrings and comments that still mention fallback behavior

Important Files Changed

Filename Overview
src/api/routes/payments/webhooks.py Correctly fixes critical security vulnerability by removing cross-environment secret fallback, ensuring prod only accepts prod secrets
SECURITY_AUDIT.md Documents remaining low-severity security issues after critical vulnerability fix

Sequence Diagram

sequenceDiagram
    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
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (3)

  1. 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.

  2. src/api/routes/payments/webhooks.py, line 79 (link)

    syntax: Comment is outdated - no longer tries alternate secret, only environment-appropriate secret.

  3. 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

Edit Code Review Agent Settings | Greptile

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.

2 participants