Repository navigation
fix(security): remediate all 42 draft security advisories - #257
Merged
Merged
Conversation
… real settlement tolerance - GHSA-4gfg-h7cw-q4v6 (V-025): ENCRYPTION_KEY no longer falls back to a 64-zero constant and MASTER_MNEMONIC no longer falls back to an ephemeral mnemonic; both fail closed via lib/crypto/require-key.ts. - GHSA-rw37-j4hj-52qp (V-017): POST /api/payments/[id]/check-balance now requires the internal key, a merchant JWT, or the business's API key, and is rate limited per payment. - GHSA-2r3m-mqrc-fg2f (V-005): business-collection payments persist crypto_amount at creation; the monitor fails closed on NULL/NaN. - GHSA-88j2-7v2m-fhpf (V-007): the 1% underpayment discount is gone from all six settlement flows; lib/payments/tolerance.ts keeps only a float-rounding epsilon. - GHSA-qq43-qr9m-xr26 (V-011): confirm transitions are compare-and-swap, so concurrent schedulers cannot each forward the same payment. - GHSA-4wq2-2jv9-f896 (X-003a): isPaidTier enforces subscription_ends_at and the downgrade sweep is wired into the cron. - GHSA-r4pc-93fx-r3rc (P-004): network fee is converted into the invoice currency before being added to the amount. - GHSA-hr9f-22h8-xgxh (N-008): the legacy fee helpers that hardcoded the paid tier are removed; calculateForwardingAmounts requires an explicit tier. - GHSA-xw6m-7xc3-3v9v (P-007): the Stripe rail uses the merchant's real tier. - GHSA-rgcc-95xr-6f54 (N-010, partial): constant-time secret comparison in lib/auth/secret-compare.ts, applied to the cron route. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- GHSA-962x-c796-6mxh (X-001): the SSE publish endpoint requires the internal key and a strict event schema; the SSE subscribe endpoint verifies the JWT owns the requested businessId. - GHSA-v8xh-443c-9jgq (N-002): escrow creation verifies the caller owns the business_id in the body (new escrow.write capability). - GHSA-9pr8-fvcc-46fc (V-023): escrow listing requires authentication and an address filter must name a wallet on the caller's account. - GHSA-58vr-5899-9vcm (V-022): swap history/create/boltz take the wallet id from the signed request, per-swap routes verify ownership, and Boltz refund/claim keys are encrypted at rest and stripped from listings. - GHSA-53xp-gvx8-xf32 (P-008): the public widget gets origin allowlisting, per-merchant and per-IP rate limits, and a pending-payment ceiling so the HD address pool cannot be drained. - GHSA-h69g-q79g-wj69 (P-010): invoices can no longer be marked paid with a caller-supplied tx_hash; manual settlement is recorded as manual and needs the payment.markPaid capability. - GHSA-hh2j-qg47-8x77 (N-004): payments have an upper amount bound. - GHSA-38mm-r8mr-pgwx (N-005): a payee override cannot target a platform wallet and is recorded with the authorizing actor. - GHSA-xw6m-7xc3-3v9v: widget Stripe fee is tier-aware too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aping, edge-function auth - GHSA-3h65-fr7w-mfx7 (V-030): new lib/security/ssrf.ts resolves the host and validates every resulting address (all IPv4 encodings, IPv4-mapped IPv6, 0/8, 127/8, CGNAT, fc00::/7, fe80::/10) and re-validates each redirect hop. webhook-test no longer reflects the response body or headers, which removes the exfiltration half of the full-read SSRF. - GHSA-h4c6-cqw4-qgg7 (N-001): both LNURL hops go through safeFetch, and the lightning address is validated before interpolation. - V-031: web-wallet webhook delivery uses the same guard. - GHSA-2c73-xpqj-rvxw (V-029): JSON-LD is serialized with < > & escaped, so a blog title cannot terminate the script element. - GHSA-2frp-hj3j-h6gm (V-016): the monitor-payments edge function verifies the cron secret in constant time instead of accepting any 'Bearer ' header, and refuses to run when no secret is configured. Its settlement check also gets the full-amount rule and the confirm CAS. Adds 38 unit tests for the SSRF guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uts and quota - GHSA-m5mp-2pcm-7633 (V-012): the Stripe refund claims the transaction with a conditional update before calling Stripe, and sends an idempotency key. - GHSA-9m6j-hhgw-wg2c (V-013): escrow settlement claims the escrow via settlement_started_at before broadcasting; the claim is released only when it aborts pre-broadcast. - GHSA-hwh2-94f2-rf9m (N-006): HD wallet index allocation is a compare-and-swap with retry, so two checkouts cannot derive the same deposit address. - GHSA-qq43-qr9m-xr26 / GHSA-3jfc-fwvw-7q3p (V-011, N-007): confirmed -> forwarding is a CAS, which gives the batch forwarder SKIP LOCKED semantics across processes. - GHSA-327c-977v-qc56 (P-009): the split is computed from the address's actual balance rather than the expected amount, so the fee leg cannot be starved. - GHSA-fpf9-hcrc-fw8m (P-011): an ambiguous broadcast failure is recorded as 'indeterminate' and never retried automatically. - GHSA-7v2w-w2g6-j5gm (P-006): consume_transaction_quota checks and increments in one statement; the old read-then-increment pair is gone. - GHSA-739j-2g39-m5h8 (V-032): public_landing_stats excludes circular self-payments and caps any single payment. - GHSA-qpc3-wrv5-fphr / GHSA-cj6r-hxrr-9cc6 (P-013, P-014): column-level REVOKE on escrow tokens and the raw issuer api_key. Adds migration 20260815000000_security_advisory_remediations.sql. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tegrity - GHSA-j7gm-3m8q-jwp6 (V-001): x402 verify/settle and web-bot-auth/verify authenticated against a table (api_keys) that does not exist, comparing a raw key to a hash column. All three now use resolveScopedKey against business_api_keys. - GHSA-v46f-xq35-6g93 (V-003): settlement verifies the chain against the recipient and amount the proof was verified for — EVM native and ERC-20, Bitcoin outputs, Solana lamport and SPL balance deltas — instead of merely confirming that some transaction succeeded. BCH, which has no verified lookup, is refused rather than settled unchecked. Settlement is also claimed with a CAS so concurrent calls cannot both capture. - GHSA-8wrw-m88r-xxj9 (P-005): usage deduction rejects non-positive and non-integer quantities at both the route and the service, rejects non-positive rates, and uses a CAS on the balance. - GHSA-9p53-794f-qghf (V-006): the usage credits GET and POST verify the caller owns the business, a payment_id must reference a settled payment for that business that covers the amount and has not already been credited, and top-ups are bounded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing, RPC credential - GHSA-2w46-xg6f-5532 (V-019), all five OAuth issues: * validateScopes intersects with the client's registered scopes, so a client can no longer request every scope the platform defines; * email_verified reflects the merchant record instead of being hardcoded true; * refresh rotation requires client_secret for confidential clients, and rotation itself is a CAS; * the authorization code is consumed with a conditional update, closing the replay window between the used check and the write; * PKCE accepts only S256 — 'plain' is refused at both the authorize and token endpoints — and the comparison is constant-time. - GHSA-hx45-94hh-3j4c (P-012): the FOSSBilling verifier implements the protocol the server actually sends (X-CoinPay-Signature: t=...,v1=... over '{ts}.{body}', 300s tolerance) instead of a format that never matched, so merchants no longer have to disable verification to receive webhooks. Its client also uses API paths that exist. - GHSA-f8gv-r6r9-f2cv (N-003): threatcrush is installed at a pinned version rather than @latest in a job with write permissions on every PR. - GHSA-6xjp-hwm6-j37q (P-015): every third-party action in both workflows is pinned to a commit SHA. - GHSA-m9vm-8822-f5wh (P-001): the Bitcoin RPC password is generated per host and stored root-only; the hardcoded literal and the echo of it are gone. Also tightens the LNbits token lifetime and FORWARDED_ALLOW_IPS (P-020). - GHSA-w688-wmpp-hqr3 (N-011): image-size has no patched release (GitHub lists both CVEs as affecting <= 2.0.2, the newest published). Documented with the reachability analysis rather than pinned to a version that fixes nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // stolen refresh token alone could mint access tokens indefinitely. | ||
| vi.mocked(getOAuthClient).mockResolvedValue({ | ||
| client_id: 'test-client', | ||
| client_secret: '$2a$10$storedhash', |
| <script | ||
| type="application/ld+json" | ||
| dangerouslySetInnerHTML={{ __html: JSON.stringify(ldJson) }} | ||
| dangerouslySetInnerHTML={{ __html: serializeJsonLd(ldJson) }} |
| <script | ||
| type="application/ld+json" | ||
| dangerouslySetInnerHTML={{ __html: JSON.stringify(organizationJsonLd) }} | ||
| dangerouslySetInnerHTML={{ __html: serializeJsonLd(organizationJsonLd) }} |
ThreatCrush Security Scan327 finding(s) HIGH/CRITICAL: 35 | MEDIUM: 38 | LOW: 254
…and 277 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
| }; | ||
| } catch { | ||
| return { confirmed: false, txHash: txId, confirmations: 0, pending: true }; | ||
| const res = await fetch(`https://mempool.space/api/tx/${txId}`); |
| if (!a || !b) return false; | ||
| if (!a.trim() || !b.trim()) return false; | ||
|
|
||
| const digestA = createHash('sha256').update(a, 'utf8').digest(); |
| if (!a.trim() || !b.trim()) return false; | ||
|
|
||
| const digestA = createHash('sha256').update(a, 'utf8').digest(); | ||
| const digestB = createHash('sha256').update(b, 'utf8').digest(); |
Comment on lines
+329
to
+333
| response = await fetch(check.url.toString(), { | ||
| ...init, | ||
| redirect: 'manual', | ||
| signal: controller.signal, | ||
| }); |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if (!resolved) { | ||
| return NextResponse.json({ error: 'Invalid or inactive API key' }, { status: 401 }); | ||
| } | ||
| const keyData = { id: resolved.keyId, business_id: resolved.business.id, active: true }; |
ralyodio
marked this pull request as ready for review
August 16, 2026 00:02
ralyodio
added a commit
that referenced
this pull request
Aug 16, 2026
…HSA-q642) (#260) * fix(db): medium/low audit findings, and correct the no-op REVOKEs from 20260815000000 Every claim was checked against production by assuming the anon role first. That check found two things worth stating plainly. CORRECTION. The column-level REVOKEs shipped in 20260815000000 for P-013 and P-014 were NO-OPS. Supabase grants table-level SELECT to anon/authenticated, and a table-level grant already permits every column, so revoking a column subset subtracts nothing. Verified after the fact: has_column_privilege('authenticated','escrows','release_token','SELECT') was still true. Neither table has legitimate anon/authenticated access, so the grant is now dropped outright rather than carved up. P-029 was the only LIVE finding. reputation_receipts is readable by anon with USING (true), and production holds 13,711 rows carrying amount (up to 999,999) and escrow_tx — enumerable by anyone with the publishable key. Public verifiability is the point of a receipt, so the row stays readable and the financial columns do not: the table grant is replaced by an explicit column grant. Consumers must now name columns; a bare SELECT * as anon fails. The rest are LATENT, not live: this app authenticates with its own JWT and talks to Postgres as service_role, so auth.uid() is never set and every policy keyed on it denies. Confirmed empirically — as anon, businesses/payments/ invoices/stripe_webhook_secrets all return 0 rows, and cleanup_seen_signatures() executes but deletes nothing because RLS blocks the underlying delete. They are fixed anyway, since each is one policy edit away from becoming real: V-018 businesses.api_key, P-016 stripe webhook secret — grant removed P-002 did_reputation_events public SELECT — policy and grant removed P-026 invoices INSERT constrained only user_id, leaving business_id free P-027 payments INSERT accepted an arbitrary status (confirmed/forwarded) N-021 'cancelled' allowed by the CHECK but never written or handled P-028 calendar_events.user_id had no foreign key P-025 escrow_series.merchant_id references businesses — documented, not renamed, because live code reads it P-023 21 functions were EXECUTE-able by anon; now service_role only Verified after applying: every privilege assertion passes, anon still reads the allowed receipt columns, and the live reputation and landing endpoints answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payments): audit medium/low findings in invoice, escrow and PayPal flows P-021 PayPal capture accepted 'COMPLETED' as proof of payment without ever reading the captured amount or currency — both of which the client already returns. A capture for a token amount, or in a different currency, marked a full-value invoice paid. Now verified against the invoice, and the paid write is conditional so two captures cannot both record a settlement. V-008 An invoice flipped to 'paid' when its linked payment was in forwarding_failed — money confirmed at the intermediary address but never delivered. The merchant saw a settled invoice and no reason to look. That status is no longer treated as settling; the retry queue and rescanLateDeposits re-drive the forward, so it resolves rather than sticking. V-010 Invoice numbers were allocated with an unlocked read-then-write MAX+1, so concurrent creates silently produced duplicates. The application cannot make that atomic, so the guarantee is a UNIQUE (business_id, invoice_number) constraint with a retry around the insert. The lookup also ordered by created_at, which returns the most recently created number rather than the highest — wrong as soon as an invoice is deleted or backdated. P-022 Accepting a proposal is what binds the payee address and fee, and the write was unconditional; it is now a compare-and-swap on the observed status. N-015 A failed escrow fee forward was caught, logged and forgotten: the escrow was marked settled with fee_tx_hash NULL and nothing recorded that the platform was still owed. It now writes a fee_forward_failed escrow_events row. N-016 The escrow fee is bounded at creation, so an out-of-range rate cannot store a fee that exceeds the escrow and make the beneficiary leg negative. N-013 The payment window had only an upper bound, so expires_in_minutes could be set below a second — every payment against such a quote lands late and strands. Floor added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth): audit medium/low findings in tokens, replay protection and client IP V-028 getClientIp read the LEFTMOST X-Forwarded-For entry. XFF is appended to by each proxy, so a request arriving with "X-Forwarded-For: 1.2.3.4" leaves the edge as "1.2.3.4, <real ip>" — the leftmost value is the one the client chose. Every per-IP rate limit was therefore bypassable by rotating a header. Parsing now counts in from the RIGHT (TRUSTED_PROXY_HOPS, default 1), and the vendor headers CF-Connecting-IP / X-Real-IP are only trusted when explicitly enabled, since they are equally forgeable if the origin is reachable directly. Seven tests cover it, including that prepending a forged address cannot move the rate-limit bucket. V-026 Web-wallet signature replay protection used the synchronous checker, which consults only this process's in-memory Map and pushes to Supabase fire-and-forget. Across more than one instance that is not replay protection: a signature spent on instance A is unknown to instance B. Now awaits the shared store. V-027 jwt.refreshToken() decoded WITHOUT verifying, then re-signed the payload. A token this side never issued would have been accepted and minted into a valid one. It is unused today, which is why the audit rated it low, but it is a loaded gun in a library. Now verifies the signature while still allowing an expired token, which is the actual intent. V-021 The Web Bot Auth signature lifetime cap only applied when both created and expires were present, so omitting expires skipped it — the most permissive case had no check at all. Both are now required. V-020 The CLI device-authorization start endpoint is unauthenticated by design and had no rate limit, so one caller could mint unlimited pending device codes and raise the collision odds against a code a real user is being shown. V-009 p2p/request had no upper bound on amount_usd and no rate limit, on an endpoint that provisions merchant accounts and client records. P-014 (completion) The issuer API-key lookup still queried the raw api_key column, so the hash column restored by 20260816020000 was unused. Lookups now match the hash and lazily upgrade any row still found by its raw key, so the column drains without forcing a key rotation. V-004 x402 raw_proof stored the payer's full signed authorization verbatim in a column nothing ever reads. Secret-bearing fields are now hashed, keeping the proof shape for forensics without retaining anything reusable. V-014 rescanLateDeposits treated "funds still at the address" as proof no forward succeeded. For a payment already in forwarding with a recorded transaction hash that is false — the send may simply not be mined yet — so a known broadcast now gets a six-hour grace before it can be re-driven. V-002 was already fixed by enforcePriceBinding (commit 5717556); no change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(infra): audit medium/low findings in CI, installer, scripts and SDK P-018 The LNbits deploy workflow read its SSH private key and dotenv credentials as "secrets.X || vars.X". GitHub Actions VARIABLES are not secrets: plaintext, readable by anyone with repo access, and not masked in job logs. Had either been set as a variable, the SSH key would have been printed in the clear. The fallbacks are gone; a missing secret now fails the job. N-020 That same workflow ran on every push to master with SSH access to a production droplet, so any merged commit reached production infrastructure with nobody in the loop. It is workflow_dispatch only and pinned to the "production" environment, so a required-reviewer rule applies. NOTE: that rule must be enabled in Settings -> Environments -> production; the environment key here is what makes it apply. Its actions are SHA-pinned too — it was the one workflow missed in #257, and it is the highest-value target in the repo. P-017 gen-mnemonic.mjs fetched the BIP-39 wordlist from GitHub at master on every run, unverified. Two problems, and the second is the serious one: nothing checked what came back, and it was a DIFFERENT source from the one used to DERIVE addresses (@scure/bip39). Any divergence would emit a phrase deriving to different keys than the platform expects, making funds sent there unrecoverable. Now uses the local lockfile-pinned wordlist with a canonical SHA-256 assertion. Verified: generated phrases pass the same validateMnemonic the wallet code uses. P-024 send-announcement.ts had six real merchant names and email addresses hardcoded, plus two more in a skip list — production PII committed to the repository. Recipients are read from the database at run time, and the skip list is domain patterns rather than named individuals. The file carries a note that this does not remove them from git history; treat those addresses as disclosed. P-003 The installer piped curl straight into tar, so a truncated transfer extracted whatever arrived. It now downloads to a file, verifies it is a well-formed non-empty gzip, logs the archive digest, supports pinning via COINPAY_SHA256, and constrains COINPAY_REF, which was interpolated into the download URL unchecked. P-019 In the SDK webhook verifier, parseInt('abc', 10) is NaN and every comparison against NaN is false — so the timestamp freshness check silently PASSED for any non-numeric timestamp. A security check that no-ops on malformed input is worse than none, because it reads as one. The signature header parser also split on every '=', truncating any value containing one. N-019 Every allowBuilds entry was literally "set this to true or false", which pnpm treats as unset, so none of those packages ran their install scripts and nobody had made the decision. Each is now resolved explicitly, with native builds enabled and telemetry-only postinstalls declined. N-012 The optional-dependency loader built a "new Function" evaluator taking a free-form module specifier. Every caller passes a constant, but the shape is worth not having in a payments codebase; it is now an allowlist. N-023 The wallet SDK logged every address the wallet holds on an error path triggered by an ordinary user mistake, correlating a user's whole account set into any shared console. Logs the count instead. N-014 Payment creation had no idempotency, so a client retrying after a timeout got a second payment — another HD address, another unit of quota, another charge. It now honours an Idempotency-Key, backed by a partial unique index so concurrent retries cannot both insert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web-wallet): bind broadcast to the prepared transaction (V-024) Broadcast checked that tx_id belonged to the wallet, that it was still pending and that it had not expired — but never looked at the signed_tx bytes. A caller could prepare one transfer and then broadcast a COMPLETELY DIFFERENT signed transaction while quoting the legitimate tx_id. The user signs with their own key, so this is not a theft primitive against them. It matters because everything the platform records and enforces hangs off the prepared row: the wallet's history, per-transaction limits, fee accounting and notifications would all describe a transaction that never happened, while the one that did goes unrecorded. EVM transactions are self-describing, so the recipient is decoded and compared (handling ERC-20 transfers, where the recipient is in the calldata rather than the tx `to`). A decoded mismatch is refused outright. UTXO and Solana need chain-specific parsing that is not available here, so rather than pretend those are checked, the row records binding_verified plus the reason — downstream accounting can then tell what it may rely on. Also annotates the unreachable 'detected' payment status (N-022). It was removed from the payments.status CHECK constraint without the application being updated, so the database can no longer produce it, yet roughly two dozen UI branches still render a "Payment Detected!" stage no payment can reach. The canonical type definitions now say so. The branches themselves are left in place: removing them is a mechanical sweep across payer-facing pages and does not belong in a security change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ralyodio
added a commit
that referenced
this pull request
Aug 19, 2026
* fix(security): fail-closed custody keys, authenticated check-balance, real settlement tolerance - GHSA-4gfg-h7cw-q4v6 (V-025): ENCRYPTION_KEY no longer falls back to a 64-zero constant and MASTER_MNEMONIC no longer falls back to an ephemeral mnemonic; both fail closed via lib/crypto/require-key.ts. - GHSA-rw37-j4hj-52qp (V-017): POST /api/payments/[id]/check-balance now requires the internal key, a merchant JWT, or the business's API key, and is rate limited per payment. - GHSA-2r3m-mqrc-fg2f (V-005): business-collection payments persist crypto_amount at creation; the monitor fails closed on NULL/NaN. - GHSA-88j2-7v2m-fhpf (V-007): the 1% underpayment discount is gone from all six settlement flows; lib/payments/tolerance.ts keeps only a float-rounding epsilon. - GHSA-qq43-qr9m-xr26 (V-011): confirm transitions are compare-and-swap, so concurrent schedulers cannot each forward the same payment. - GHSA-4wq2-2jv9-f896 (X-003a): isPaidTier enforces subscription_ends_at and the downgrade sweep is wired into the cron. - GHSA-r4pc-93fx-r3rc (P-004): network fee is converted into the invoice currency before being added to the amount. - GHSA-hr9f-22h8-xgxh (N-008): the legacy fee helpers that hardcoded the paid tier are removed; calculateForwardingAmounts requires an explicit tier. - GHSA-xw6m-7xc3-3v9v (P-007): the Stripe rail uses the merchant's real tier. - GHSA-rgcc-95xr-6f54 (N-010, partial): constant-time secret comparison in lib/auth/secret-compare.ts, applied to the cron route. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): close unauthenticated and cross-tenant endpoints - GHSA-962x-c796-6mxh (X-001): the SSE publish endpoint requires the internal key and a strict event schema; the SSE subscribe endpoint verifies the JWT owns the requested businessId. - GHSA-v8xh-443c-9jgq (N-002): escrow creation verifies the caller owns the business_id in the body (new escrow.write capability). - GHSA-9pr8-fvcc-46fc (V-023): escrow listing requires authentication and an address filter must name a wallet on the caller's account. - GHSA-58vr-5899-9vcm (V-022): swap history/create/boltz take the wallet id from the signed request, per-swap routes verify ownership, and Boltz refund/claim keys are encrypted at rest and stripped from listings. - GHSA-53xp-gvx8-xf32 (P-008): the public widget gets origin allowlisting, per-merchant and per-IP rate limits, and a pending-payment ceiling so the HD address pool cannot be drained. - GHSA-h69g-q79g-wj69 (P-010): invoices can no longer be marked paid with a caller-supplied tx_hash; manual settlement is recorded as manual and needs the payment.markPaid capability. - GHSA-hh2j-qg47-8x77 (N-004): payments have an upper amount bound. - GHSA-38mm-r8mr-pgwx (N-005): a payee override cannot target a platform wallet and is recorded with the authorizing actor. - GHSA-xw6m-7xc3-3v9v: widget Stripe fee is tier-aware too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): SSRF guard with DNS + redirect validation, JSON-LD escaping, edge-function auth - GHSA-3h65-fr7w-mfx7 (V-030): new lib/security/ssrf.ts resolves the host and validates every resulting address (all IPv4 encodings, IPv4-mapped IPv6, 0/8, 127/8, CGNAT, fc00::/7, fe80::/10) and re-validates each redirect hop. webhook-test no longer reflects the response body or headers, which removes the exfiltration half of the full-read SSRF. - GHSA-h4c6-cqw4-qgg7 (N-001): both LNURL hops go through safeFetch, and the lightning address is validated before interpolation. - V-031: web-wallet webhook delivery uses the same guard. - GHSA-2c73-xpqj-rvxw (V-029): JSON-LD is serialized with < > & escaped, so a blog title cannot terminate the script element. - GHSA-2frp-hj3j-h6gm (V-016): the monitor-payments edge function verifies the cron secret in constant time instead of accepting any 'Bearer ' header, and refuses to run when no secret is configured. Its settlement check also gets the full-amount rule and the confirm CAS. Adds 38 unit tests for the SSRF guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): atomicity across refunds, settlement, forwarding, payouts and quota - GHSA-m5mp-2pcm-7633 (V-012): the Stripe refund claims the transaction with a conditional update before calling Stripe, and sends an idempotency key. - GHSA-9m6j-hhgw-wg2c (V-013): escrow settlement claims the escrow via settlement_started_at before broadcasting; the claim is released only when it aborts pre-broadcast. - GHSA-hwh2-94f2-rf9m (N-006): HD wallet index allocation is a compare-and-swap with retry, so two checkouts cannot derive the same deposit address. - GHSA-qq43-qr9m-xr26 / GHSA-3jfc-fwvw-7q3p (V-011, N-007): confirmed -> forwarding is a CAS, which gives the batch forwarder SKIP LOCKED semantics across processes. - GHSA-327c-977v-qc56 (P-009): the split is computed from the address's actual balance rather than the expected amount, so the fee leg cannot be starved. - GHSA-fpf9-hcrc-fw8m (P-011): an ambiguous broadcast failure is recorded as 'indeterminate' and never retried automatically. - GHSA-7v2w-w2g6-j5gm (P-006): consume_transaction_quota checks and increments in one statement; the old read-then-increment pair is gone. - GHSA-739j-2g39-m5h8 (V-032): public_landing_stats excludes circular self-payments and caps any single payment. - GHSA-qpc3-wrv5-fphr / GHSA-cj6r-hxrr-9cc6 (P-013, P-014): column-level REVOKE on escrow tokens and the raw issuer api_key. Adds migration 20260815000000_security_advisory_remediations.sql. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): x402 auth and settlement verification, usage credit integrity - GHSA-j7gm-3m8q-jwp6 (V-001): x402 verify/settle and web-bot-auth/verify authenticated against a table (api_keys) that does not exist, comparing a raw key to a hash column. All three now use resolveScopedKey against business_api_keys. - GHSA-v46f-xq35-6g93 (V-003): settlement verifies the chain against the recipient and amount the proof was verified for — EVM native and ERC-20, Bitcoin outputs, Solana lamport and SPL balance deltas — instead of merely confirming that some transaction succeeded. BCH, which has no verified lookup, is refused rather than settled unchecked. Settlement is also claimed with a CAS so concurrent calls cannot both capture. - GHSA-8wrw-m88r-xxj9 (P-005): usage deduction rejects non-positive and non-integer quantities at both the route and the service, rejects non-positive rates, and uses a CAS on the balance. - GHSA-9p53-794f-qghf (V-006): the usage credits GET and POST verify the caller owns the business, a payment_id must reference a settled payment for that business that covers the amount and has not already been credited, and top-ups are bounded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): OAuth hardening, FOSSBilling webhook protocol, CI pinning, RPC credential - GHSA-2w46-xg6f-5532 (V-019), all five OAuth issues: * validateScopes intersects with the client's registered scopes, so a client can no longer request every scope the platform defines; * email_verified reflects the merchant record instead of being hardcoded true; * refresh rotation requires client_secret for confidential clients, and rotation itself is a CAS; * the authorization code is consumed with a conditional update, closing the replay window between the used check and the write; * PKCE accepts only S256 — 'plain' is refused at both the authorize and token endpoints — and the comparison is constant-time. - GHSA-hx45-94hh-3j4c (P-012): the FOSSBilling verifier implements the protocol the server actually sends (X-CoinPay-Signature: t=...,v1=... over '{ts}.{body}', 300s tolerance) instead of a format that never matched, so merchants no longer have to disable verification to receive webhooks. Its client also uses API paths that exist. - GHSA-f8gv-r6r9-f2cv (N-003): threatcrush is installed at a pinned version rather than @latest in a job with write permissions on every PR. - GHSA-6xjp-hwm6-j37q (P-015): every third-party action in both workflows is pinned to a commit SHA. - GHSA-m9vm-8822-f5wh (P-001): the Bitcoin RPC password is generated per host and stored root-only; the hardcoded literal and the echo of it are gone. Also tightens the LNbits token lifetime and FORWARDED_ALLOW_IPS (P-020). - GHSA-w688-wmpp-hqr3 (N-011): image-size has no patched release (GitHub lists both CVEs as affecting <= 2.0.2, the newest published). Documented with the reachability analysis rather than pinned to a version that fixes nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ralyodio
added a commit
that referenced
this pull request
Aug 19, 2026
…HSA-q642) (#260) * fix(db): medium/low audit findings, and correct the no-op REVOKEs from 20260815000000 Every claim was checked against production by assuming the anon role first. That check found two things worth stating plainly. CORRECTION. The column-level REVOKEs shipped in 20260815000000 for P-013 and P-014 were NO-OPS. Supabase grants table-level SELECT to anon/authenticated, and a table-level grant already permits every column, so revoking a column subset subtracts nothing. Verified after the fact: has_column_privilege('authenticated','escrows','release_token','SELECT') was still true. Neither table has legitimate anon/authenticated access, so the grant is now dropped outright rather than carved up. P-029 was the only LIVE finding. reputation_receipts is readable by anon with USING (true), and production holds 13,711 rows carrying amount (up to 999,999) and escrow_tx — enumerable by anyone with the publishable key. Public verifiability is the point of a receipt, so the row stays readable and the financial columns do not: the table grant is replaced by an explicit column grant. Consumers must now name columns; a bare SELECT * as anon fails. The rest are LATENT, not live: this app authenticates with its own JWT and talks to Postgres as service_role, so auth.uid() is never set and every policy keyed on it denies. Confirmed empirically — as anon, businesses/payments/ invoices/stripe_webhook_secrets all return 0 rows, and cleanup_seen_signatures() executes but deletes nothing because RLS blocks the underlying delete. They are fixed anyway, since each is one policy edit away from becoming real: V-018 businesses.api_key, P-016 stripe webhook secret — grant removed P-002 did_reputation_events public SELECT — policy and grant removed P-026 invoices INSERT constrained only user_id, leaving business_id free P-027 payments INSERT accepted an arbitrary status (confirmed/forwarded) N-021 'cancelled' allowed by the CHECK but never written or handled P-028 calendar_events.user_id had no foreign key P-025 escrow_series.merchant_id references businesses — documented, not renamed, because live code reads it P-023 21 functions were EXECUTE-able by anon; now service_role only Verified after applying: every privilege assertion passes, anon still reads the allowed receipt columns, and the live reputation and landing endpoints answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(payments): audit medium/low findings in invoice, escrow and PayPal flows P-021 PayPal capture accepted 'COMPLETED' as proof of payment without ever reading the captured amount or currency — both of which the client already returns. A capture for a token amount, or in a different currency, marked a full-value invoice paid. Now verified against the invoice, and the paid write is conditional so two captures cannot both record a settlement. V-008 An invoice flipped to 'paid' when its linked payment was in forwarding_failed — money confirmed at the intermediary address but never delivered. The merchant saw a settled invoice and no reason to look. That status is no longer treated as settling; the retry queue and rescanLateDeposits re-drive the forward, so it resolves rather than sticking. V-010 Invoice numbers were allocated with an unlocked read-then-write MAX+1, so concurrent creates silently produced duplicates. The application cannot make that atomic, so the guarantee is a UNIQUE (business_id, invoice_number) constraint with a retry around the insert. The lookup also ordered by created_at, which returns the most recently created number rather than the highest — wrong as soon as an invoice is deleted or backdated. P-022 Accepting a proposal is what binds the payee address and fee, and the write was unconditional; it is now a compare-and-swap on the observed status. N-015 A failed escrow fee forward was caught, logged and forgotten: the escrow was marked settled with fee_tx_hash NULL and nothing recorded that the platform was still owed. It now writes a fee_forward_failed escrow_events row. N-016 The escrow fee is bounded at creation, so an out-of-range rate cannot store a fee that exceeds the escrow and make the beneficiary leg negative. N-013 The payment window had only an upper bound, so expires_in_minutes could be set below a second — every payment against such a quote lands late and strands. Floor added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth): audit medium/low findings in tokens, replay protection and client IP V-028 getClientIp read the LEFTMOST X-Forwarded-For entry. XFF is appended to by each proxy, so a request arriving with "X-Forwarded-For: 1.2.3.4" leaves the edge as "1.2.3.4, <real ip>" — the leftmost value is the one the client chose. Every per-IP rate limit was therefore bypassable by rotating a header. Parsing now counts in from the RIGHT (TRUSTED_PROXY_HOPS, default 1), and the vendor headers CF-Connecting-IP / X-Real-IP are only trusted when explicitly enabled, since they are equally forgeable if the origin is reachable directly. Seven tests cover it, including that prepending a forged address cannot move the rate-limit bucket. V-026 Web-wallet signature replay protection used the synchronous checker, which consults only this process's in-memory Map and pushes to Supabase fire-and-forget. Across more than one instance that is not replay protection: a signature spent on instance A is unknown to instance B. Now awaits the shared store. V-027 jwt.refreshToken() decoded WITHOUT verifying, then re-signed the payload. A token this side never issued would have been accepted and minted into a valid one. It is unused today, which is why the audit rated it low, but it is a loaded gun in a library. Now verifies the signature while still allowing an expired token, which is the actual intent. V-021 The Web Bot Auth signature lifetime cap only applied when both created and expires were present, so omitting expires skipped it — the most permissive case had no check at all. Both are now required. V-020 The CLI device-authorization start endpoint is unauthenticated by design and had no rate limit, so one caller could mint unlimited pending device codes and raise the collision odds against a code a real user is being shown. V-009 p2p/request had no upper bound on amount_usd and no rate limit, on an endpoint that provisions merchant accounts and client records. P-014 (completion) The issuer API-key lookup still queried the raw api_key column, so the hash column restored by 20260816020000 was unused. Lookups now match the hash and lazily upgrade any row still found by its raw key, so the column drains without forcing a key rotation. V-004 x402 raw_proof stored the payer's full signed authorization verbatim in a column nothing ever reads. Secret-bearing fields are now hashed, keeping the proof shape for forensics without retaining anything reusable. V-014 rescanLateDeposits treated "funds still at the address" as proof no forward succeeded. For a payment already in forwarding with a recorded transaction hash that is false — the send may simply not be mined yet — so a known broadcast now gets a six-hour grace before it can be re-driven. V-002 was already fixed by enforcePriceBinding (commit 0f0f300); no change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(infra): audit medium/low findings in CI, installer, scripts and SDK P-018 The LNbits deploy workflow read its SSH private key and dotenv credentials as "secrets.X || vars.X". GitHub Actions VARIABLES are not secrets: plaintext, readable by anyone with repo access, and not masked in job logs. Had either been set as a variable, the SSH key would have been printed in the clear. The fallbacks are gone; a missing secret now fails the job. N-020 That same workflow ran on every push to master with SSH access to a production droplet, so any merged commit reached production infrastructure with nobody in the loop. It is workflow_dispatch only and pinned to the "production" environment, so a required-reviewer rule applies. NOTE: that rule must be enabled in Settings -> Environments -> production; the environment key here is what makes it apply. Its actions are SHA-pinned too — it was the one workflow missed in #257, and it is the highest-value target in the repo. P-017 gen-mnemonic.mjs fetched the BIP-39 wordlist from GitHub at master on every run, unverified. Two problems, and the second is the serious one: nothing checked what came back, and it was a DIFFERENT source from the one used to DERIVE addresses (@scure/bip39). Any divergence would emit a phrase deriving to different keys than the platform expects, making funds sent there unrecoverable. Now uses the local lockfile-pinned wordlist with a canonical SHA-256 assertion. Verified: generated phrases pass the same validateMnemonic the wallet code uses. P-024 send-announcement.ts had six real merchant names and email addresses hardcoded, plus two more in a skip list — production PII committed to the repository. Recipients are read from the database at run time, and the skip list is domain patterns rather than named individuals. The file carries a note that this does not remove them from git history; treat those addresses as disclosed. P-003 The installer piped curl straight into tar, so a truncated transfer extracted whatever arrived. It now downloads to a file, verifies it is a well-formed non-empty gzip, logs the archive digest, supports pinning via COINPAY_SHA256, and constrains COINPAY_REF, which was interpolated into the download URL unchecked. P-019 In the SDK webhook verifier, parseInt('abc', 10) is NaN and every comparison against NaN is false — so the timestamp freshness check silently PASSED for any non-numeric timestamp. A security check that no-ops on malformed input is worse than none, because it reads as one. The signature header parser also split on every '=', truncating any value containing one. N-019 Every allowBuilds entry was literally "set this to true or false", which pnpm treats as unset, so none of those packages ran their install scripts and nobody had made the decision. Each is now resolved explicitly, with native builds enabled and telemetry-only postinstalls declined. N-012 The optional-dependency loader built a "new Function" evaluator taking a free-form module specifier. Every caller passes a constant, but the shape is worth not having in a payments codebase; it is now an allowlist. N-023 The wallet SDK logged every address the wallet holds on an error path triggered by an ordinary user mistake, correlating a user's whole account set into any shared console. Logs the count instead. N-014 Payment creation had no idempotency, so a client retrying after a timeout got a second payment — another HD address, another unit of quota, another charge. It now honours an Idempotency-Key, backed by a partial unique index so concurrent retries cannot both insert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web-wallet): bind broadcast to the prepared transaction (V-024) Broadcast checked that tx_id belonged to the wallet, that it was still pending and that it had not expired — but never looked at the signed_tx bytes. A caller could prepare one transfer and then broadcast a COMPLETELY DIFFERENT signed transaction while quoting the legitimate tx_id. The user signs with their own key, so this is not a theft primitive against them. It matters because everything the platform records and enforces hangs off the prepared row: the wallet's history, per-transaction limits, fee accounting and notifications would all describe a transaction that never happened, while the one that did goes unrecorded. EVM transactions are self-describing, so the recipient is decoded and compared (handling ERC-20 transfers, where the recipient is in the calldata rather than the tx `to`). A decoded mismatch is refused outright. UTXO and Solana need chain-specific parsing that is not available here, so rather than pretend those are checked, the row records binding_verified plus the reason — downstream accounting can then tell what it may rely on. Also annotates the unreachable 'detected' payment status (N-022). It was removed from the payments.status CHECK constraint without the application being updated, so the database can no longer produce it, yet roughly two dozen UI branches still render a "Payment Detected!" stage no payment can reach. The canonical type definitions now say so. The branches themselves are left in place: removing them is a mechanical sweep across payer-facing pages and does not belong in a security change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes all 42 draft security advisories (3 critical, 39 high). The 43rd, GHSA-q642-mvvc-5w7x, is the umbrella remediation report rather than a distinct vulnerability.
Verification
pnpm type-checkcleanpnpm buildsucceedspnpm lint— 0 errors (399 pre-existing warnings unchanged)vitest run— 4038 passing, 3 skipped, 0 failing (up from 3973; ~65 new tests, including 38 for the SSRF guard)Every test that changed did so because the behaviour it asserted was the vulnerability. Those assertions were inverted and commented, not deleted.
Critical
ENCRYPTION_KEYno longer falls back to 64 zeros andMASTER_MNEMONICno longer falls back to an ephemeral mnemonic — both fail closed vialib/crypto/require-key.ts, which also rejects known-weak constantscrypto_amountis quoted and persisted at creation; the monitor fails closed on NULL/NaN instead of confirming at zero balance; DB CHECK as backstopThemes across the high advisories
Settlement correctness. The 1% underpayment discount is gone from all six flows (
lib/payments/tolerance.tskeeps only a float-rounding epsilon ~1e9× smaller). Forwarding now splits the address's actual balance rather than the expected amount, so the fee leg can no longer be starved. Network fees are converted into the invoice currency before being added, fixing the FX unit-mix.Atomicity. Every check-then-act on money is now a compare-and-swap: payment confirmation, forwarding, escrow settlement, Stripe refunds (plus a Stripe idempotency key), HD wallet index allocation, OAuth code exchange and refresh rotation, and the monthly transaction quota (new
consume_transaction_quotaRPC). An ambiguous payout broadcast — a timeout after the node accepted it — is recorded asindeterminateand never retried automatically, because retrying it pays twice.Authorization. Closed on the SSE publish and subscribe endpoints, escrow creation and listing, swap history/create/boltz and the per-swap routes, usage credits, and the monitor-payments edge function (which accepted any
Bearerheader and then built aservice_roleclient). Three routes authenticated against a table namedapi_keysthat does not exist, comparing a raw key to a hash column; all now useresolveScopedKey.SSRF. New
lib/security/ssrf.tsresolves the host and validates every resulting address — all IPv4 encodings (decimal, hex, octal), IPv4-mapped IPv6,0/8, all of127/8, CGNAT,fc00::/7,fe80::/10— and re-validates each redirect hop.webhook-testno longer reflects the response body or headers, which removes the exfiltration half of the full-read SSRF regardless of any residual DNS-rebinding window.Revenue. The legacy fee helpers that hardcoded the paid tier are deleted (they had no production callers);
calculateForwardingAmountsnow requires an explicit tier; the Stripe and widget rails use the merchant's real tier; andisPaidTierenforcessubscription_ends_at, with the downgrade sweep finally wired into the cron.Supply chain.
threatcrush@latestis pinned, and every third-party action in both workflows is pinned to a commit SHA. The hardcodedBITCOIN_RPC_PASSis generated per host and stored root-only.Judgment calls worth a look
Three places where I deliberately did not do the literal thing, with reasoning in the code:
merchant_wallet_addressoverride (N-005) — kept, because forwarding the net to a third-party payee is a documented product flow (invoice payouts). Hardened instead: it cannot target a platform wallet, and the authorizing actor is recorded. Requiring ownership would break invoicing.image-size(N-011) — no patched release exists. GitHub lists both CVEs as affecting<= 2.0.2, the newest published version, so an override cannot resolve them. Documented with a reachability analysis (it arrives only via metro, the React Native bundler, under@solana/wallet-adapter-*, which nothing in this repo imports). Dropping those four unused dependencies would remove the chain entirely — worth doing, but it is a dependency change I did not want to make unasked.unsafe-inline— the XSS sink itself is fixed (JSON-LD is escaped so a blog title cannot terminate the script element). Removingunsafe-inlineneeds a Next.js nonce rollout, which is a real breakage risk on a live payment site with no staging. Left as a follow-up.Migration
supabase/migrations/20260815000000_security_advisory_remediations.sql— column-levelREVOKEon escrow tokens and the raw issuerapi_key, thesettlement_started_atclaim column, amount/quota constraints,widget_allowed_origins, invoice settlement provenance, theindeterminatepayout state, the atomic quota RPCs, and apublic_landing_statsthat excludes circular self-payments. It is idempotent. No CI applies migrations on this repo — it needs applying by hand.🤖 Generated with Claude Code