Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 54 additions & 10 deletions internal/handlers/billing.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ func detectBillingMisconfiguration(cfgEnvironment, razorpayKeyID string) (code,

// BillingHandler handles billing and Razorpay webhook endpoints.
type BillingHandler struct {
db *sql.DB
cfg *config.Config
db *sql.DB
cfg *config.Config
// email is the Mailer used for all webhook-triggered sends (payment
// receipts, payment-failed dunning, etc.). The interface lets main.go
// wrap the underlying *email.Client in a *email.BreakingClient — a
Expand Down Expand Up @@ -1115,12 +1115,12 @@ type rzpSubscriptionEntity struct {
}

type rzpPaymentEntity struct {
ID string `json:"id"`
Amount int64 `json:"amount"`
Currency string `json:"currency"`
Email string `json:"email"`
AttemptCount int `json:"attempt_count"`
ErrorDescription string `json:"error_description"`
ID string `json:"id"`
Amount int64 `json:"amount"`
Currency string `json:"currency"`
Email string `json:"email"`
AttemptCount int `json:"attempt_count"`
ErrorDescription string `json:"error_description"`
// SubscriptionID + OrderID + Notes (B11-P1, 2026-05-20): used to
// resolve the team server-side instead of trusting payload.email
// verbatim. A payment.failed entity carries `subscription_id` for
Expand Down Expand Up @@ -2100,11 +2100,48 @@ func (h *BillingHandler) handleSubscriptionCancelled(ctx context.Context, c *fib
return fmt.Errorf("subscription.cancelled team resolve: %w", err)
}

// Snapshot the prior tier so the audit row can capture from→to. Failure
// to read it is non-fatal — we just emit with from_tier="".
// Snapshot the prior tier so the audit row can capture from→to, and the
// team's CURRENT live subscription id so we can reject a stale/superseded
// cancellation (see the guard below). Failure to read the team is non-fatal
// — we just emit with from_tier="" and an empty live sub id (which falls
// through to the historical always-downgrade behaviour).
fromTier := ""
liveSubID := ""
if team, lookupErr := models.GetTeamByID(ctx, h.db, teamID); lookupErr == nil && team != nil {
fromTier = team.PlanTier
if team.RazorpaySubscriptionID.Valid {
liveSubID = strings.TrimSpace(team.RazorpaySubscriptionID.String)
}
}

// STALE-SUB GUARD (MONEY-SENSITIVE, 2026-06-04): a cancel/halt/deauth
// webhook carries notes.team_id verbatim from WHATEVER subscription fired
// it — including a SUPERSEDED one. After a hobby→pro plan change the old
// hobby subscription stays alive in Razorpay carrying the same notes.team_id;
// its eventual cancellation (or a halt/deauth) would otherwise downgrade the
// team that is now actively paying on a DIFFERENT live Pro subscription.
//
// Mirror the charged-path lower_tier guard (handleSubscriptionCharged): if
// the webhook's sub.ID does NOT match the team's stored live subscription id
// AND that live id is non-empty, this is a stale event for an old
// subscription. Skip the downgrade entirely, log a loud WARN + a
// billing.charge_undeliverable audit row for operator reconciliation, and
// keep the higher tier. An empty live id (never stored a sub id, or a
// lookup miss above) falls through to the historical behaviour — we cannot
// prove the event is stale, and a never-paid team should still downgrade.
if liveSubID != "" && sub.ID != "" && sub.ID != liveSubID {
slog.Warn("billing.subscription.cancelled.stale_subscription_skip",
"team_id", teamID,
"event_subscription_id", sub.ID,
"live_subscription_id", liveSubID,
"current_tier", fromTier,
"action", "cancel/halt/deauth carried a subscription_id that is NOT the team's live subscription — "+
"NOT downgrading (likely a superseded subscription from a prior plan change). Operator: verify "+
"the team's live subscription is healthy; if this WAS the live sub, reconcile manually",
)
emitChargeUndeliverableAudit(ctx, h.db, teamID, sub, event,
chargeUndeliverableReasonStaleSubscriptionCancel, fromTier)
return nil
}

// Downgrade behaviour: a cancellation with zero paid invoices means the
Expand Down Expand Up @@ -3236,6 +3273,13 @@ const (
// downgrades flow through cancellation/plan-change), so the tier was kept
// and the charge flagged for operator reconciliation.
chargeUndeliverableReasonLowerTierCharge = "lower_tier_charge"
// chargeUndeliverableReasonStaleSubscriptionCancel — 2026-06-04: a
// subscription.cancelled/halted/deauthenticated webhook carried a
// subscription_id that does NOT match the team's stored live subscription.
// The event is for a superseded subscription (e.g. the old sub left alive
// after a hobby→pro plan change), so the downgrade was SKIPPED to protect an
// actively-paying customer. Flagged for operator reconciliation.
chargeUndeliverableReasonStaleSubscriptionCancel = "stale_subscription_cancel"
)

// F11 (billing-trust audit 2026-05-19) — cancellation copy.
Expand Down
153 changes: 153 additions & 0 deletions internal/handlers/billing_stale_sub_cancel_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
package handlers_test

import (
"net/http"
"testing"

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"instant.dev/internal/testhelpers"
)

// Stale-sub downgrade guard (MONEY-SENSITIVE, 2026-06-04).
//
// A subscription.cancelled/halted/deauthenticated webhook carries
// notes.team_id verbatim from WHATEVER subscription fired it — including a
// SUPERSEDED one. After a hobby→pro plan change the old hobby subscription
// stays alive in Razorpay; its eventual cancellation must NOT downgrade the
// team that is now actively paying on a different live Pro subscription.
//
// These tests pin three behaviours exercised through the real webhook handler
// path (signature verify → dispatch → handleSubscriptionCancelled):
// (a) a cancel for the team's LIVE subscription still downgrades.
// (b) a cancel for a STALE/non-matching subscription id is IGNORED (no
// UpdatePlanTier, a billing.charge_undeliverable audit row IS emitted).
// (c) an empty live subscription id falls through to historical behaviour
// (the team is downgraded — we cannot prove the event is stale).

// TestBillingWebhook_SubscriptionCancelled_LiveSub_StillDowngrades verifies
// behaviour (a): when the cancelled webhook's subscription_id MATCHES the
// team's stored live subscription, the historical downgrade still happens.
func TestBillingWebhook_SubscriptionCancelled_LiveSub_StillDowngrades(t *testing.T) {
db, cleanDB := billingStateNeedsDB(t)
defer cleanDB()

app, _ := billingWebhookDBApp(t, db)

teamID := testhelpers.MustCreateTeamDB(t, db, "pro")
defer db.Exec(`DELETE FROM teams WHERE id = $1::uuid`, teamID)

liveSub := "sub_live_" + uuid.NewString()
_, err := db.Exec(`UPDATE teams SET stripe_customer_id = $1 WHERE id = $2::uuid`, liveSub, teamID)
require.NoError(t, err)

// Webhook fires for the SAME (live) subscription → downgrade proceeds.
payload := makeSubscriptionCancelledPayload(t, teamID, liveSub)
req := signedWebhookRequest(t, payload)
resp, err := app.Test(req, 5000)
require.NoError(t, err)
defer resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode)

// Tier dropped to hobby (paid_count omitted → courtesy floor).
var newTier string
require.NoError(t, db.QueryRow(`SELECT plan_tier FROM teams WHERE id = $1::uuid`, teamID).Scan(&newTier))
assert.Equal(t, "hobby", newTier, "cancel for the LIVE sub must downgrade")

// No stale-skip audit row — this was a legitimate cancellation.
var staleCount int
require.NoError(t, db.QueryRow(`
SELECT count(*) FROM audit_log
WHERE team_id = $1::uuid
AND kind = 'billing.charge_undeliverable'
AND metadata->>'reason' = 'stale_subscription_cancel'`, teamID).Scan(&staleCount))
assert.Equal(t, 0, staleCount, "live-sub cancel must NOT emit a stale-skip audit row")
}

// TestBillingWebhook_SubscriptionCancelled_StaleSub_IsIgnored verifies
// behaviour (b): a cancelled webhook whose subscription_id does NOT match the
// team's stored live subscription is treated as a superseded event — the tier
// is KEPT and a billing.charge_undeliverable (reason=stale_subscription_cancel)
// audit row is emitted for operator reconciliation.
func TestBillingWebhook_SubscriptionCancelled_StaleSub_IsIgnored(t *testing.T) {
db, cleanDB := billingStateNeedsDB(t)
defer cleanDB()

app, _ := billingWebhookDBApp(t, db)

teamID := testhelpers.MustCreateTeamDB(t, db, "pro")
defer db.Exec(`DELETE FROM teams WHERE id = $1::uuid`, teamID)

liveSub := "sub_live_pro_" + uuid.NewString()
_, err := db.Exec(`UPDATE teams SET stripe_customer_id = $1 WHERE id = $2::uuid`, liveSub, teamID)
require.NoError(t, err)

// Webhook fires for a DIFFERENT (superseded hobby) subscription.
staleSub := "sub_stale_hobby_" + uuid.NewString()
payload := makeSubscriptionCancelledPayload(t, teamID, staleSub)
req := signedWebhookRequest(t, payload)
resp, err := app.Test(req, 5000)
require.NoError(t, err)
defer resp.Body.Close()
// Non-retryable skip keeps the 200 — Razorpay must not redeliver.
require.Equal(t, http.StatusOK, resp.StatusCode)

// Tier UNCHANGED — the actively-paying Pro customer must not be downgraded.
var newTier string
require.NoError(t, db.QueryRow(`SELECT plan_tier FROM teams WHERE id = $1::uuid`, teamID).Scan(&newTier))
assert.Equal(t, "pro", newTier, "stale-sub cancel must NOT downgrade a live paying customer")

// A stale-skip audit row IS emitted for operator reconciliation.
var staleCount int
require.NoError(t, db.QueryRow(`
SELECT count(*) FROM audit_log
WHERE team_id = $1::uuid
AND kind = 'billing.charge_undeliverable'
AND metadata->>'reason' = 'stale_subscription_cancel'`, teamID).Scan(&staleCount))
assert.Equal(t, 1, staleCount, "stale-sub cancel must emit exactly one stale-skip audit row")

// No subscription.canceled row — the cancellation email must NOT fire.
var canceledCount int
require.NoError(t, db.QueryRow(`
SELECT count(*) FROM audit_log
WHERE team_id = $1::uuid AND kind = 'subscription.canceled'`, teamID).Scan(&canceledCount))
assert.Equal(t, 0, canceledCount, "stale-sub cancel must NOT emit a customer cancellation row")
}

// TestBillingWebhook_SubscriptionCancelled_EmptyLiveSub_FallsThrough verifies
// behaviour (c): when the team has NO stored live subscription id (never paid,
// or a sub-id write that never landed), the guard cannot prove the event is
// stale, so it falls through to the historical downgrade behaviour.
func TestBillingWebhook_SubscriptionCancelled_EmptyLiveSub_FallsThrough(t *testing.T) {
db, cleanDB := billingStateNeedsDB(t)
defer cleanDB()

app, _ := billingWebhookDBApp(t, db)

teamID := testhelpers.MustCreateTeamDB(t, db, "pro")
defer db.Exec(`DELETE FROM teams WHERE id = $1::uuid`, teamID)

// stripe_customer_id left NULL (MustCreateTeamDB does not set it).
payload := makeSubscriptionCancelledPayload(t, teamID, "sub_any_"+uuid.NewString())
req := signedWebhookRequest(t, payload)
resp, err := app.Test(req, 5000)
require.NoError(t, err)
defer resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode)

// Historical behaviour: tier downgraded to the hobby courtesy floor.
var newTier string
require.NoError(t, db.QueryRow(`SELECT plan_tier FROM teams WHERE id = $1::uuid`, teamID).Scan(&newTier))
assert.Equal(t, "hobby", newTier, "empty live sub must fall through to historical downgrade")

// No stale-skip audit row — the guard did not trip.
var staleCount int
require.NoError(t, db.QueryRow(`
SELECT count(*) FROM audit_log
WHERE team_id = $1::uuid
AND kind = 'billing.charge_undeliverable'
AND metadata->>'reason' = 'stale_subscription_cancel'`, teamID).Scan(&staleCount))
assert.Equal(t, 0, staleCount, "empty-live-sub fall-through must NOT emit a stale-skip audit row")
}
Loading