diff --git a/.planning/phases/18.1-payment-gateway-subscriptions/18.1-SECURITY.md b/.planning/phases/18.1-payment-gateway-subscriptions/18.1-SECURITY.md new file mode 100644 index 0000000..04a8ac0 --- /dev/null +++ b/.planning/phases/18.1-payment-gateway-subscriptions/18.1-SECURITY.md @@ -0,0 +1,126 @@ +--- +phase: "18.1" +slug: "payment-gateway-subscriptions" +status: verified +# threats_open = count of OPEN threats at or above workflow.security_block_on severity (the blocking gate) +threats_open: 0 +asvs_level: 1 +created: "2026-10-16" +--- + +# Phase 18.1 — Security + +> Per-phase security contract: threat register, accepted risks, and audit trail. + +Scope: plans 18.1-01..09 plus quick tasks 261016-myi and 261016-nqk. Code in summercms.go (cabana, pact, admin), sm-paymentgatewaysubscriptions-plugin, sm-pgstripe-plugin (root and subscriptions), vue-paymentgateway-layer. + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|---------------| +| Stripe → webhook | Signed events at /webhook/stripe/subscriptions, verified per mode | subscription and invoice state, ids (no PII kept in job args) | +| App → Stripe API | Customer, checkout, portal, subscription, product and price calls | e-mail and name on customer create; secret key (encrypted at rest) | +| Browser (layer) → customer API | JWT Bearer calls for the logged-in user's own subscription | plan code, currency; checkout/portal URLs back | +| Admin SPA → cabana | Plans and subscriptions screens behind manage_plans / manage_subscriptions | plan data, subscriber e-mail/name (read-only) | +| Webhook job → database | Claim-first transaction under a per-subscription session lock | subscription rows, processed-event markers | + +--- + +## Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation | Status | +|-----------|----------|-----------|----------|-------------|------------|--------| +| T-18.1-01 | Tampering | taglist save | medium | mitigate | cabana field_taglist.go type/allow-list/required checks (422) on all save paths; TestTaglistFieldSmoke | closed | +| T-18.1-02 | Tampering (XSS) | TaglistField.vue, PreviewField | high | mitigate | no v-html; TaglistField.test.ts renders script/onerror as text | closed | +| T-18.1-03 | Tampering | flat reorder POST | medium | mitigate | reorder.go parent/duplicate 422, stale set 409, sort column only; TestReorderFlatSmoke | closed | +| T-18.1-04 | EoP | reorder and taglist routes | low | accept | reorderPost under s.protect; taglist rides existing saves (AR-18.1-01) | closed | +| T-18.1-05 | Tampering | Apply with unknown subscriber_type | high | mitigate | apply.go errors, stores nothing (ErrUnknownSubscriberType); core smoke unknown_alias | closed | +| T-18.1-06 | Repudiation/Tampering | duplicate or concurrent deliveries | high | mitigate | ClaimEvent ON CONFLICT in tx, unique event_id; job_test two concurrent jobs fire once | closed | +| T-18.1-07 | Info Disclosure | Apply/ApplyInvoice/Checkout logs | medium | mitigate | PII-free contracts.Invoice; no e-mail in slog calls; provider text never logged | closed | +| T-18.1-08 | Tampering | mass assignment | medium | mitigate | Fillable + Rules; subscription saves refused; TestAdminSubscriptionsSmoke/read_only | closed | +| T-18.1-09 | Tampering | users.stripe_customer_id writes | medium | mitigate | targeted single-column UPDATE; ResolveMany read-only; TestUserSubscribable | closed | +| T-18.1-10 | Spoofing | webhook handler | high | mitigate | body cap, ValidatePayload (hmac.Equal) before decode; wrong_secret/missing_header tests | closed | +| T-18.1-11 | Tampering | replay | high | mitigate | 300 s tolerance; Already processed check; claim-first job; duplicate_event test | closed | +| T-18.1-12 | Tampering/Repudiation | concurrent duplicate jobs | high | mitigate | claim inside lagoon.Transaction; job_test.go:187 | closed | +| T-18.1-13 | Info Disclosure | river_job args and logs | medium | mitigate | minimal args struct; TestJobArgsMinimal key and PII leak lists | closed | +| T-18.1-14 | Info Disclosure | subscription signing secrets | high | mitigate | lagoon.Encrypted, json:"-", password fields; only mode logged; settings/secrets tests | closed | +| T-18.1-15 | Spoofing | wrong-mode secret | medium | mitigate | secret by test_mode only, no payment-secret fallback; live_secret_in_test_mode | closed | +| T-18.1-16 | DoS | webhook floods, large bodies | medium | mitigate | 60/min/IP shared bucket, 1 MiB cap, cheap check, handled() filter; tests | closed | +| T-18.1-17 | Tampering | cross-application events | medium | mitigate | app tag on objects; foreignApp skip keeps marker; app_tag test | closed | +| T-18.1-18 | Tampering | import-winter target column | medium | mitigate | webhookSecretTarget validated before writes; maskSecret; target_* tests | closed | +| T-18.1-19 | EoP | missing secret accepting events | high | mitigate | empty secret answers 500 before signature check; missing_secret test | closed | +| T-18.1-20 | Spoofing | checkout and portal return URLs | high | mitigate | api/urls.go config-only URLs, refuses // javascript: userinfo; TestRedirectURLs | closed | +| T-18.1-21 | EoP | IDOR on subscription routes | high | mitigate | subscriber from JWT on every handler; new cancelIncomplete/expire scoped ForSubscriber; cross_user_isolation, TestCustomerAPIAuthz | closed | +| T-18.1-22 | Tampering | admin plan sync abuse | medium | mitigate | manage_plans enforced (403 tests); toolbar action run not audit-logged with admin id (AR-18.1-03) | closed | +| T-18.1-23 | Info Disclosure | subscriber name/e-mail (preview, list, API) | medium | mitigate | manage_subscriptions via protect on list/show/partials incl. ListExtendRecords/FormRecordTitle; Subscriber json:"-"; authz tests | closed | +| T-18.1-24 | DoS | customer API floods | medium | mitigate | 120/min bucket; cheap 409/404/422 before gateway (residual: per-IP key, no 429 test) | closed | +| T-18.1-25 | Tampering | bulk sync rollback losing ids | high | mitigate | syncOne never errors; ids saved before gateway error; bulk_sync_partial_failure | closed | +| T-18.1-26 | Spoofing (CSRF) | customer API | medium | mitigate | RequireJWT Bearer only, no cookies; bearer_required | closed | +| T-18.1-27 | Info Disclosure | unknown subscriber_type on preview | low | mitigate | danger notice + Error log; alias #id fallback; unknown_alias_is_visible | closed | +| T-18.1-28 | Spoofing | layer open redirect | high | mitigate | isNavigableURL; only checkout_url/portal_url navigated; client sends no URL | closed | +| T-18.1-29 | Info Disclosure | layer token handling | medium | mitigate | token from app hook as Bearer, nothing stored; base URL from runtime config | closed | +| T-18.1-30 | Tampering (XSS) | PgPlans, PgMySubscription | high | mitigate | no v-html in subscription components; subscriptionComponents.test.ts | closed | +| T-18.1-31 | Tampering | double submit | low | mitigate | busy/redirecting guard; subscribe.test.ts | closed | +| T-18.1-32 | Repudiation | gate false green | high | mitigate | check-phase18.1.sh detectors, --self-test, PHASE181_REQUIRE_POSTGRES; gate run exit 0 | closed | +| T-18.1-33 | Info Disclosure | tests leaking secrets or calling Stripe | medium | mitigate | BackendOverride fakes; no api.stripe.com or real keys in tests | closed | +| T-18.1-34 | Tampering | publishing red or unreviewed code | medium | mitigate | tags only after green gate; post-gate fixes unpushed pending go-ahead | closed | +| T-18.1-35 | Tampering | stale snapshot overwrite (CR-01) | high | mitigate | fresh RetrieveSubscription under session lock; Cancel under lock; TestFreshStateSmoke, TestCancelTakesSubscriptionLock | closed | +| T-18.1-36 | EoP | delete-before-create | high | mitigate | ErrSubscriptionNotStored rolls back marker; deleted_before_created (canceled once) | closed | +| T-18.1-37 | Repudiation | unresolved plan after reprice | medium | mitigate | price, product, code resolution; loud failure; plan_resolution_test.go | closed | +| T-18.1-38 | Tampering | SyncSubscription lost update | medium | mitigate | lock + FOR UPDATE re-read + gateway columns only; TestSyncSubscriptionFresh | closed | +| T-18.1-39 | DoS | leaked lock or slow retrieve | low | mitigate | unlock via WithoutCancel, conn discarded on failure (residual: IN-02 pool starvation) | closed | +| T-18.1-40 | Info Disclosure | product_id and plan_code in job args | low | accept | ids and a code only, PII refused by TestJobArgsMinimal (AR-18.1-02) | closed | +| T-18.1-41 | Tampering | cross-currency billing (CR-02) | high | mitigate | CheckoutPriceID no fallback, 422; gateway refuses; checkPrice amount check; TestCheckoutPriceUnavailable | closed | +| T-18.1-42 | Tampering | duplicate Stripe customers | medium | mitigate | idempotency key; first write wins; TestCustomerIDFirstWriteWins | closed | +| T-18.1-43 | Tampering | second billing subscription (WR-03) | high | mitigate | 409 CheckoutBlocker; CheckoutGuarded under SubscriberLockKey; cancelIncomplete + ExpireOpenCheckoutSessions (residual AR-18.1-04) | closed | +| T-18.1-44 | DoS | plan sync holding locks across Stripe calls | medium | mitigate | Detached bulk action; per-plan PlanLockKey; TestAdminSyncDetached, TestSyncPlanSerialised | closed | +| T-18.1-45 | EoP | detached bulk bypassing scope/permissions | medium | mitigate | protect + allowAction before BulkAction; loadScoped; TestDetachedBulkAction | closed | +| T-18.1-46 | DoS | idempotent replay of failures | low | mitigate | 10-min window + parameter hash; keys never logged; customer/product creates only | closed | +| T-18.1-47 | DoS | bfcache-stuck buttons | low | mitigate | watchPageRestore wired in three components; UAT row 4 | closed | +| T-18.1-48 | Repudiation | gate false green on gap-closure tests | high | mitigate | plan 07-09 test names required by gate (residual: quick-task names not pinned) | closed | +| T-18.1-49 | Info Disclosure | tests calling Stripe or leaking secrets | medium | mitigate | same fakes in review_fixes_test.go | closed | +| T-18.1-50 | Tampering | publishing order, immovable tags | medium | mitigate | requires point at existing tags; unreleased HEAD not published | closed | +| T-18.1-SC | Tampering | npm/pip/cargo installs (all plans) | high | mitigate | no new third-party modules or lockfile changes in 18.1 or the quick tasks | closed | + +*Status: open · closed · open — below high threshold (non-blocking)* +*Severity: critical > high > medium > low — only open threats at or above workflow.security_block_on count toward threats_open* +*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)* + +Post-plan attack surface from the quick tasks, mapped to existing entries: `pact.ListExtendRecords` and `pact.FormRecordTitle` (T-18.1-23, gated by protect); `contracts.CheckoutSessionExpirer`, `ExpireOpenCheckoutSessions`, `cancelIncomplete` (T-18.1-21, T-18.1-43); `checkPrice` and `contracts.ErrPriceUnavailable` (T-18.1-41); `contracts.SubscriberBatchResolver.ResolveMany` (T-18.1-09, T-18.1-23). + +--- + +## Accepted Risks Log + +| Risk ID | Threat Ref | Rationale | Accepted By | Date | +|---------|------------|-----------|-------------|------| +| AR-18.1-01 | T-18.1-04 | Reorder POST runs under the controller's protect; taglist adds no route and rides the existing saves | plan 18.1-01 (verified by audit) | 2026-10-16 | +| AR-18.1-02 | T-18.1-40 | product_id and plan_code are Stripe ids and a plan code, not PII; TestJobArgsMinimal pins the key list and refuses PII | plan 18.1-07 (verified by audit) | 2026-10-16 | +| AR-18.1-03 | T-18.1-22 | Toolbar admin actions (sync_all_to_stripe) are not audit-logged with the admin id on success; bulk actions are. The action needs manage_plans and pgsub logs each synced plan. Follow-up todo: admin action audit log (plugin or cabana extension) | user | 2026-10-16 | +| AR-18.1-04 | T-18.1-43 | A Checkout Session already paid but whose webhook is not yet processed is complete, not open, so it is not expired; a second checkout in that window can bill twice. Detected by warnDuplicateOpen (Error log). Replaces the old 10-minute idempotency key, which covered only identical parameters | user (UAT row 5, quick 261016-myi) | 2026-10-16 | +| AR-18.1-05 | T-18.1-39 | Session advisory locks hold a pooled connection; a capped MaxOpenConns or a re-entrant listener could starve the pool (review IN-02, open) | audit residual | 2026-10-16 | +| AR-18.1-06 | T-18.1-24 | The customer API throttle keys on client IP in practice (JWT check runs inside the handler); no 429 test for this bucket | audit residual | 2026-10-16 | +| AR-18.1-07 | T-18.1-48 | check-phase18.1.sh does not pin the quick-task test names, so their removal would not fail the gate | audit residual | 2026-10-16 | + +*Accepted risks do not resurface in future audit runs.* + +--- + +## Security Audit Trail + +| Audit Date | Threats Total | Closed | Open | Run By | +|------------|---------------|--------|------|--------| +| 2026-10-16 | 51 | 50 (+1 accepted: T-18.1-22) | 0 | msd-security-auditor (ASVS 1, block_on high) | + +--- + +## Sign-Off + +- [x] All threats have a disposition (mitigate / accept / transfer) +- [x] Accepted risks documented in Accepted Risks Log +- [x] `threats_open: 0` confirmed +- [x] `status: verified` set in frontmatter + +**Approval:** verified 2026-10-16