Skip to content

fix(stripe-webhook): auto-detect test/live mode via dual-secret verification - #60

Merged
therajanmaurya merged 1 commit into
developmentfrom
feat/stripe-webhook-dual-mode
May 14, 2026
Merged

fix(stripe-webhook): auto-detect test/live mode via dual-secret verification#60
therajanmaurya merged 1 commit into
developmentfrom
feat/stripe-webhook-dual-mode

Conversation

@therajanmaurya

@therajanmaurya therajanmaurya commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • stripe-webhook now auto-detects test vs live mode by trying PAYCRAFT_STRIPE_TEST_WEBHOOK_SECRET first, then PAYCRAFT_STRIPE_LIVE_WEBHOOK_SECRET — whichever verifies wins
  • Uses the mode-appropriate Stripe API key (PAYCRAFT_STRIPE_TEST/LIVE_SECRET_KEY) for all downstream calls — fixes the reels-downloader outage where a test key was trying to retrieve a live subscription
  • Passes mode + eventType into handleSubscriptionEvent for proper DB rows and webhook_logs
  • subscription-handler: makes tenantId optional to support self-hosted single-tenant deployments
  • paycraft-adopt-verify: adds test/live arg; Step 5.2B validates HMAC-SHA256 sig secrets against Supabase; test row and is_premium() calls include RESOLVED_MODE

Test plan

  • Trigger a test-mode Stripe webhook event — verify mode=test in DB row and 200 response
  • Trigger a live-mode Stripe webhook event — verify mode=live in DB row and 200 response
  • Run /paycraft-adopt-verify test — all steps pass
  • Run /paycraft-adopt-verify live — all steps pass
  • Verify is_premium() with correct mode returns correct premium status

Summary by CodeRabbit

  • New Features

    • Enhanced PayCraft adoption verification with expanded schema checks (9 queries) and webhook signature validation.
    • CLI now accepts optional mode parameter to explicitly select test or live environment for verification.
    • Improved test vs live Stripe environment support with separate credential handling and detection.
  • Documentation

    • Updated PayCraft adoption guides with new verification steps and CLI usage instructions.

Review Change Stack

…ication

- stripe-webhook now tries PAYCRAFT_STRIPE_TEST_WEBHOOK_SECRET first, then
  PAYCRAFT_STRIPE_LIVE_WEBHOOK_SECRET; whichever verifies sets mode + API key
- Uses PAYCRAFT_STRIPE_TEST/LIVE_SECRET_KEY accordingly — test key can't
  retrieve live subscriptions (was the root cause of the reels-downloader outage)
- Passes mode + eventType into handleSubscriptionEvent for DB + webhook_logs
- subscription-handler: make tenantId optional (self-hosted single-tenant path)
- paycraft-adopt-verify: add test/live param; Step 5.2B validates sig secrets;
  test row + is_premium() calls include RESOLVED_MODE field
@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR extends PayCraft to support separate test and live Stripe webhook signing secrets with automatic dual-stage verification, mode-aware event routing through webhook handlers, and comprehensive adoption verification steps that include the new webhook signature test and updated Phase 5 integration.

Changes

Dual Test/Live Stripe Webhook Secrets and Verification

Layer / File(s) Summary
Subscription event optional tenantId
supabase/functions/_shared/subscription-handler.ts
SubscriptionEvent.tenantId is now optional to denote single-tenant (self-hosted) behavior when omitted or null.
Stripe webhook handler dual-secret verification
supabase/functions/stripe-webhook/index.ts
Added dedicated environment variables for test/live webhook signing secrets and Stripe API keys; introduced stripeClient() helper; replaced single-secret verification with two-stage logic that attempts test secret first, retries with live secret on failure, and tracks detected mode for subsequent API calls.
Webhook event routing with mode context
supabase/functions/stripe-webhook/index.ts
Extended all webhook event handlers (checkout.session.completed, customer.subscription.updated/deleted, invoice.paid) to receive and forward detected mode and eventType to handleSubscriptionEvent; response now includes detected mode.
PayCraft adoption: mode resolution and CLI support
.claude/commands/paycraft-adopt-verify.md, layers/paycraft/commands/paycraft-adopt-verify.md
Updated adoption documentation to support optional CLI [mode] argument that overrides .env via RESOLVED_MODE, with fallback to PAYCRAFT_MODE (defaulting to test); all Phase 5 steps consistently reference RESOLVED_MODE.
PayCraft adoption: webhook signature verification step
layers/paycraft/commands/paycraft-adopt-verify.md
Added Phase 5.2B step to verify Stripe webhook signature validation by generating minimal signed ping events for both test/live secrets and classifying HTTP responses to detect secret mismatches.
PayCraft adoption: Phase 5 step integration with mode
layers/paycraft/commands/paycraft-adopt-verify.md
Updated Phase 5 steps to include mode column in test subscription row, pass stripe_mode: RESOLVED_MODE to is_premium() RPC, add failure diagnostics for mode mismatch, and update deployment state and Live Mode Upgrade Checklist references.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Two secrets in the test and live,
Now verified, so keen and swift,
Mode flows through each webhook's call,
Dual-stage checks cover all.
Adoption steps now sing along,
Test and live both proven strong! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: automatic test/live mode detection via dual-secret verification in the Stripe webhook handler, which is the primary feature across the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/stripe-webhook-dual-mode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@layers/paycraft/commands/paycraft-adopt-verify.md`:
- Line 230: The code is still reading mode from PAYCRAFT_MODE in a few places
(e.g., the JSON field "mode": "[PAYCRAFT_MODE — 'test' or 'live']" and the step
blocks around lines referenced in the review) which breaks CLI overrides; update
all mode-dependent reads/writes in this file to use the resolved runtime value
RESOLVED_MODE instead of PAYCRAFT_MODE (replace the literal/documentation usage
and any lookup expressions that reference PAYCRAFT_MODE), ensuring every
write/lookup that depends on environment mode consistently references
RESOLVED_MODE so CLI-provided mode takes effect across Step 5.5/5.10C and the
rest of the flow.
- Around line 180-184: Update the HTTP 400 handling in the signature-failure
classification logic so that "No signatures found" is treated as REJECT (secret
mismatch) unless the response body explicitly contains timestamp-related text;
only accept 400 responses that contain timestamp-tolerance messages such as
"timestamp wasn't within tolerance" or "Timestamp outside the tolerance zone"
(or other explicit clock-skew phrasing). Modify the branch that currently
accepts 400 bodies containing "No signatures found" to instead check for the
timestamp phrases first and accept only when one of those phrases is present;
otherwise classify as REJECT for secret mismatch.

In `@supabase/functions/stripe-webhook/index.ts`:
- Around line 5-15: The code currently forces creation of a Stripe client using
testSecretKey (and uses non-null assertions on test/live env vars) which
prevents falling back to live credentials; change stripeClient usage so you do
not instantiate a client with testSecretKey at module load: remove the
unconditional use of testSecretKey and instead defer creating a Stripe instance
until handling the webhook payload, choose the secretKey at runtime by checking
PAYCRAFT_STRIPE_TEST_SECRET_KEY and falling back to
PAYCRAFT_STRIPE_LIVE_SECRET_KEY (handle both possibly undefined without using
!), and use the stripeClient(secretKey) factory when you actually need to verify
the webhook or call Stripe so live-only deployments work and no hard runtime
dependency on the test secret exists.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97ef70cf-e4c7-4dc9-ae72-f76b0da87e0e

📥 Commits

Reviewing files that changed from the base of the PR and between aa045e2 and be32c35.

⛔ Files ignored due to path filters (1)
  • supabase/.temp/cli-latest is excluded by !**/.temp/**
📒 Files selected for processing (4)
  • .claude/commands/paycraft-adopt-verify.md
  • layers/paycraft/commands/paycraft-adopt-verify.md
  • supabase/functions/_shared/subscription-handler.ts
  • supabase/functions/stripe-webhook/index.ts

Comment on lines +180 to +184
ACCEPT : HTTP 400 with body containing "No signatures found" OR "timestamp" (clock skew only)
NOTE: 400 with clock-skew message means the secret IS correct but the timestamp
is too old -- this counts as a PASS for secret verification.
REJECT : HTTP 400 with body NOT containing any signature-related message -> secret mismatch
REJECT : HTTP 500 -> function crashed after signature passed -> code bug

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Stripe webhook signature verification, what exact error messages are returned for (1) wrong webhook signing secret, (2) missing Stripe-Signature header, and (3) timestamp outside tolerance when using Stripe SDK webhook signature verification?

💡 Result:

In Stripe SDK webhook signature verification (e.g., stripe.webhooks.constructEvent in Node.js, stripe.Webhook.construct_event in Python/Ruby/Java/Go), the exact error messages vary slightly by language but follow consistent patterns for the three cases: 1. Wrong webhook signing secret: This causes a signature mismatch after computing the HMAC with the incorrect secret. The error is "No signatures found matching the expected signature for payload." (or similar, e.g., "#<Stripe::SignatureVerificationError: No signatures found matching the expected signature for payload>" in Ruby).[1][2][3][4][5][6] 2. Missing Stripe-Signature header: This triggers an error when parsing the header, such as "webhook has no Stripe-Signature header" (Go),[7] "Unable to extract timestamp and signatures from header" (Java/Python),[3][8][6] or equivalent in other SDKs when no header is provided or invalid.[1][5] 3. Timestamp outside tolerance: This is explicitly checked after parsing the header but before or during signature match, e.g., "timestamp wasn't within tolerance" (Go),[7] "Timestamp outside the tolerance zone" (Java),[3] "timestamp outside the tolerance" (implied in Node.js validation).[5] Note: Wrong secret and tampered payload both yield the same generic mismatch error in most SDKs, as the computed signature won't match any provided.[1][9][2] Official docs highlight this as the common failure message.[1][10] SDK source code confirms distinct errors for missing header and timestamp across languages (Node.js: StripeSignatureVerificationError with specific messages; Python: SignatureVerificationError; Java: SignatureVerificationException; Go: specific ErrWebhook* vars; Ruby: SignatureVerificationError).[7][3][5][6][11]

Citations:


Tighten signature-failure classification to correctly reject wrong secrets.

Treating HTTP 400 containing "No signatures found" as accepted is incorrect for secret verification. According to Stripe SDK implementation, "No signatures found matching the expected signature for payload" is the error returned when the webhook signing secret is wrong. Timestamp tolerance errors produce distinct messages like "timestamp wasn't within tolerance" or "Timestamp outside the tolerance zone". The current logic incorrectly passes secret mismatches that do not include explicit timestamp-related text.

Update the logic to reject "No signatures found" unless the response explicitly contains timestamp-related language indicating a clock-skew failure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@layers/paycraft/commands/paycraft-adopt-verify.md` around lines 180 - 184,
Update the HTTP 400 handling in the signature-failure classification logic so
that "No signatures found" is treated as REJECT (secret mismatch) unless the
response body explicitly contains timestamp-related text; only accept 400
responses that contain timestamp-tolerance messages such as "timestamp wasn't
within tolerance" or "Timestamp outside the tolerance zone" (or other explicit
clock-skew phrasing). Modify the branch that currently accepts 400 bodies
containing "No signatures found" to instead check for the timestamp phrases
first and accept only when one of those phrases is present; otherwise classify
as REJECT for secret mismatch.

"current_period_end": "[30 days from now in UTC — e.g. 2026-05-25T10:00:00Z]",
"cancel_at_period_end": false
"cancel_at_period_end": false,
"mode": "[PAYCRAFT_MODE — 'test' or 'live']"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use RESOLVED_MODE consistently in all mode-dependent writes and lookups.

Line 230 and Line 618 still source mode from PAYCRAFT_MODE, which breaks CLI override behavior and can make Step 5.5/5.10C operate on the wrong mode context.

Suggested doc fix
-            "mode": "[PAYCRAFT_MODE — 'test' or 'live']"
+            "mode": "[RESOLVED_MODE — 'test' or 'live']"
-  mode            = PAYCRAFT_MODE
+  mode            = RESOLVED_MODE

Also applies to: 618-633

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@layers/paycraft/commands/paycraft-adopt-verify.md` at line 230, The code is
still reading mode from PAYCRAFT_MODE in a few places (e.g., the JSON field
"mode": "[PAYCRAFT_MODE — 'test' or 'live']" and the step blocks around lines
referenced in the review) which breaks CLI overrides; update all mode-dependent
reads/writes in this file to use the resolved runtime value RESOLVED_MODE
instead of PAYCRAFT_MODE (replace the literal/documentation usage and any lookup
expressions that reference PAYCRAFT_MODE), ensuring every write/lookup that
depends on environment mode consistently references RESOLVED_MODE so
CLI-provided mode takes effect across Step 5.5/5.10C and the rest of the flow.

Comment on lines +5 to +15
const testWebhookSecret = Deno.env.get("PAYCRAFT_STRIPE_TEST_WEBHOOK_SECRET")!;
const liveWebhookSecret = Deno.env.get("PAYCRAFT_STRIPE_LIVE_WEBHOOK_SECRET")!;
const testSecretKey = Deno.env.get("PAYCRAFT_STRIPE_TEST_SECRET_KEY")!;
const liveSecretKey = Deno.env.get("PAYCRAFT_STRIPE_LIVE_SECRET_KEY")!;

const webhookSecret = Deno.env.get("STRIPE_WEBHOOK_SECRET")!;
function stripeClient(secretKey: string): Stripe {
return new Stripe(secretKey, {
apiVersion: "2023-10-16",
httpClient: Stripe.createFetchHttpClient(),
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "index.ts" | grep stripe-webhook

Repository: MobileByteLabs/PayCraft

Length of output: 155


🏁 Script executed:

cat -n supabase/functions/stripe-webhook/index.ts

Repository: MobileByteLabs/PayCraft

Length of output: 5769


Defer verifier instantiation to allow fallback to live secrets.

Line 27 instantiates the Stripe client with testSecretKey before any error handling, making it a hard runtime dependency. The ! assertions on lines 5–8 provide no runtime protection; if PAYCRAFT_STRIPE_TEST_SECRET_KEY is missing, the function fails immediately before the fallback logic can attempt live-secret verification. This breaks live-only or partially migrated deployments.

Suggested fix
-const testWebhookSecret = Deno.env.get("PAYCRAFT_STRIPE_TEST_WEBHOOK_SECRET")!;
-const liveWebhookSecret = Deno.env.get("PAYCRAFT_STRIPE_LIVE_WEBHOOK_SECRET")!;
-const testSecretKey = Deno.env.get("PAYCRAFT_STRIPE_TEST_SECRET_KEY")!;
-const liveSecretKey = Deno.env.get("PAYCRAFT_STRIPE_LIVE_SECRET_KEY")!;
+const testWebhookSecret = Deno.env.get("PAYCRAFT_STRIPE_TEST_WEBHOOK_SECRET");
+const liveWebhookSecret = Deno.env.get("PAYCRAFT_STRIPE_LIVE_WEBHOOK_SECRET");
+const testSecretKey = Deno.env.get("PAYCRAFT_STRIPE_TEST_SECRET_KEY");
+const liveSecretKey = Deno.env.get("PAYCRAFT_STRIPE_LIVE_SECRET_KEY");

 function stripeClient(secretKey: string): Stripe {
   return new Stripe(secretKey, {
     apiVersion: "2023-10-16",
     httpClient: Stripe.createFetchHttpClient(),
   });
 }

 serve(async (req) => {
+  const verifierKey = testSecretKey ?? liveSecretKey;
+  if (!verifierKey) {
+    console.error("Missing Stripe secret key configuration for webhook verification");
+    return new Response("Webhook misconfigured", { status: 500 });
+  }
+
   const signature = req.headers.get("stripe-signature");
   if (!signature) {
     return new Response("Missing stripe-signature", { status: 400 });
   }

   const body = await req.text();

   // Auto-detect test vs live by trying both secrets.
   // Whichever verifies determines which Stripe key to use for API calls.
-  const verifier = stripeClient(testSecretKey); // any instance works for sig verification
+  const verifier = stripeClient(verifierKey); // any configured key works for sig verification
   let event: Stripe.Event;
   let mode: "test" | "live";
   let stripe: Stripe;
   try {
+    if (!testWebhookSecret || !testSecretKey) throw new Error("test webhook config missing");
     event = await verifier.webhooks.constructEventAsync(body, signature, testWebhookSecret);
     mode = "test";
     stripe = stripeClient(testSecretKey);
   } catch {
     try {
+      if (!liveWebhookSecret || !liveSecretKey) throw new Error("live webhook config missing");
       event = await verifier.webhooks.constructEventAsync(body, signature, liveWebhookSecret);
       mode = "live";
       stripe = stripeClient(liveSecretKey);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@supabase/functions/stripe-webhook/index.ts` around lines 5 - 15, The code
currently forces creation of a Stripe client using testSecretKey (and uses
non-null assertions on test/live env vars) which prevents falling back to live
credentials; change stripeClient usage so you do not instantiate a client with
testSecretKey at module load: remove the unconditional use of testSecretKey and
instead defer creating a Stripe instance until handling the webhook payload,
choose the secretKey at runtime by checking PAYCRAFT_STRIPE_TEST_SECRET_KEY and
falling back to PAYCRAFT_STRIPE_LIVE_SECRET_KEY (handle both possibly undefined
without using !), and use the stripeClient(secretKey) factory when you actually
need to verify the webhook or call Stripe so live-only deployments work and no
hard runtime dependency on the test secret exists.

@therajanmaurya
therajanmaurya merged commit 10a2a43 into development May 14, 2026
7 checks passed
@mobilebytesenseicommunity
mobilebytesenseicommunity deleted the feat/stripe-webhook-dual-mode branch June 17, 2026 08:05
@mobilebytesenseicommunity
mobilebytesenseicommunity restored the feat/stripe-webhook-dual-mode branch June 17, 2026 08:05
@coderabbitai coderabbitai Bot mentioned this pull request Jul 13, 2026
17 tasks
@therajanmaurya
therajanmaurya deleted the feat/stripe-webhook-dual-mode branch July 29, 2026 12:28
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