Skip to content

Fix the security audit findings: promo caps, keepalive anon access, error text, import DNS pinning - #230

Merged
Programcode merged 7 commits into
mainfrom
fix/security-audit-findings
Oct 24, 2026
Merged

Programcode merged 7 commits into
mainfrom
fix/security-audit-findings

Conversation

@Programcode

Copy link
Copy Markdown
Owner

Fixes the findings from the security audit in draft #226 (SECURITY-AUDIT.md on cursor/security-audit-ccaa). I checked each one against current main before fixing it.

Warning

Keepalive monitor, Pi backup. After the keepalive migration reaches a project, the Pi's GitHub Actions copy of supabase-keepalive-monitor gets 401/403 on its REST and RPC steps for DebateLab staging and production, because its SUPABASE_KEEPALIVE_PROJECTS_B64 entries for those two projects use the publishable key. Its last run already got 401 on /rest/v1/ for both. The VPS timer, which the monitor's README calls the primary scheduler, uses the service-role key and keeps working. To fix the Pi, switch those two entries to serviceRoleKey, or set restRead, restWrite, rpc and storage to false for them. Staging applies migrations on its own after merge, so the Pi goes red for staging at merge, not at the production deploy.

What changed

1. Discount promo caps (Medium). Confirmed on main. The discount branch of redeem_promo_code_atomic returned before user_promos was written and redemptions_count was incremented, and create-checkout set allow_promotion_codes: true.

  • New migration 20261024120100_discount_promo_claims.sql replaces the function. A discount row must carry its Stripe promotion-code id in config.stripe_promotion_code (^promo_[A-Za-z0-9]+$). Claiming one records the user and counts against max_redemptions. A user asking again for a code they already hold gets the same id back without counting again, even after the cap is reached. A discount row without a valid id answers like an unknown code.
  • New p_discount_only argument (default false). Checkout passes true, so a non-discount code typed into the Checkout path is refused and grants nothing. Adding the argument changes the signature, so the migration drops the old one-argument function and creates the new one in the same transaction. If both existed, PostgREST would refuse every call as ambiguous. redeem-promo still sends only p_code and resolves to the new function.
  • create-checkout claims the code as the user, with the user-scoped client and after the open-subscription check, and sends only discounts: [{ promotion_code }]. It never sets allow_promotion_codes. Every refusal, including the lockout, is one promo_invalid 400. The service-role promo lookup and discountPromoRefusal are gone. If Stripe fails after the claim, the claim stays counted, which errs on the side of the cap.
  • After a discount is claimed in the pricing dialog, the code stays in the field and the dialog stays open, so Upgrade sends it. The message now says the discount is applied on upgrade (en, es, hu).
  • Creating a discount code in Admin requires the promo_ id, in the form and in a server-side check in the admin function.

2. Anon keepalive tables (Medium). Confirmed. The tables had TO anon insert/select/update policies and GRANT ALL to anon. Before locking anything down I found the only writer, Programcode/supabase-keepalive-monitor. A VPS systemd timer runs it as the primary scheduler with the service-role key, and a backup workflow on the Pi runs it with the publishable key for DebateLab (see the warning above). Nothing in this repo reads or writes these tables.

  • New migration 20261024120000_keepalive_server_only.sql revokes all client grants on both tables and the event sequence, drops the five anon policies, and keeps keepalive_touch(text, jsonb) service-role only when it exists. That function exists only in the hosted projects, where the monitor's bootstrap SQL creates it, so the revoke is guarded. No rows or tables are dropped.
  • I didn't change the monitor's optional publishable-access.sql also adds anon policies on storage.objects for keepalive-monitor/checks/*. Those policies live in the hosted projects, not this repo. They have the same problem, since the publishable key can read and overwrite those objects, and should be removed on the hosted side.

3. share error text (Low). On main the quota failure the audit cites is already caught with a fixed message. The outer catch still copied any other thrown message into the 500 body. It now answers Failed to create share link and logs the detail, truncated.

4. create-portal error text (Low). Confirmed. Stripe failures now answer with a fixed portal_failed error, which the browser translates with the existing serverErrors.portalFailed text. Stripe's type, code, request id and message go to the log.

5. import-source DNS rebinding (unranked in the audit). Confirmed. validateResolvedPublicHost resolved the name, then fetch resolved it again. I ran a spike in the pinned edge-runtime image, supabase/edge-runtime:v1.77.4. Deno.connect to an IP plus Deno.startTls with the hostname works there, so I implemented the pinned dial.

  • validateResolvedPublicHost now returns the addresses it checked, and new import-source/pinned-fetch.ts connects only to one of them. It opens TCP to the address, then TLS with SNI and certificate checks for the hostname, and finishes the handshake before sending anything. It reads the answer with a small HTTP/1.1 client that handles one GET per connection, Content-Length, chunked and close-delimited bodies, gzip and deflate, 1xx skipped, a 32 KB head cap. Anything else is refused. Socket and TLS errors reach the user as fixed messages. Each redirect hop gets the same check and pin.
  • docs/standards/security.md drops the recorded exception and states the rule.

Open

  • When a TLS handshake stalls and is cancelled, Deno keeps that TCP socket until the worker exits. I reproduced this in Deno 2.9.6 and in edge-runtime v1.77.4, and found no way to release it from JS. The 3 s per-address budget and the 50-a-day import quota cap how many such sockets one user can hold.
  • I did not check hosted Supabase Edge behaviour (no deploy), a real Stripe Checkout with discounts, or a rebinding DNS server. Unit tests show the pin, and so does a connection to an unresolvable hostname in the runtime.
  • The monitor's storage.objects anon policies on the hosted projects are still open (see finding 2).

Deploy later (not done here)

No migration, function deploy or production change was made.

  1. Migrations: 20261024120000_keepalive_server_only.sql and 20261024120100_discount_promo_claims.sql.
  2. Then the functions create-checkout, create-portal, share, import-source and admin. Deploy create-checkout only after the promo migration. The new function calls the RPC with p_discount_only, which the old database function does not accept, so every promo checkout would fail until the migration lands. redeem-promo itself is unchanged and works with both.
  3. Before or right after the promo migration, give each active discount code its Stripe promotion code, or it stops working: UPDATE public.promo_codes SET config = config || '{"stripe_promotion_code": "promo_..."}' WHERE code = '...' AND type = 'discount';
  4. Update the Pi monitor's DebateLab entries (see the warning at the top).

Tests

  • pgTAP (./.superset/db-gate.sh --db-only): all 31 files, 541 tests passed, including the new discount_promo_claims_test.sql (cap reached by the last distinct user, the next user refused with the unknown-code message, redemptions_count equal to the cap, a holder's repeat call not counted, the Checkout path refusing a non-discount code and granting nothing, a single function overload) and keepalive_server_only_test.sql (anon and authenticated hold no privilege and no policy; anon insert, select and heartbeat update fail with 42501; the service role can still upsert the heartbeat and insert an event). legacy_table_grants_test.sql now asserts that anon has no keepalive sequence access.
  • Edge (npm run test:edge): 482 passed. New: Checkout sends discounts[0][promotion_code] and no allow_promotion_codes, and claims as the user; a refused claim creates no session; a Stripe portal failure answers with the fixed body (the old handler leaked the customer id); pinnedGet connects to the given address without resolving the hostname, skips 103, decodes chunked gzip, reads close-delimited bodies, refuses malformed answers, falls through to the next checked address (including one that never answers, after a 3 s per-address budget), and closes on abort, including mid-TLS-handshake; the import handler dials exactly the addresses each hop checked across a redirect. I ran the checkout and portal tests against the old handlers, and they failed.
  • Edge runtime image v1.77.4, real network: pinnedGet fetched example.com, en.wikipedia.org (http to https redirect), bbc.com, gov.uk and github.com. A certificate for the wrong name was refused with the fixed message. A hostname that cannot resolve still connected to the given address, which shows no second lookup happens. localtest.me (127.0.0.1) is refused.
  • npm run verify: passed (vitest 651, edge 480 at that point, Playwright smoke 47).
  • Real app (verify-debatelab, fake provider), desktop and mobile Chromium: recipes/billing and recipes/billing-states, 18 tests, all passed (16 in the first run, where the new bill-discount failed on my selector for a trial account; after the fix, bill-discount and bill-promo passed on both viewports). bill-discount shows the code claimed once, a second Apply not counted again, the code kept in the dialog, and Upgrade sending it to create-checkout. Also recipes/library 12/12, including lib-import, which imports a live Wikipedia page through the local Edge runtime and so goes through the pinned client; e2e/full-stack/billing 4/4.
  • Local PostgREST with the anon key: insert, select and heartbeat upsert on the keepalive tables all get 401. The service-role key gets 201, 200 and 201, the requests the monitor makes.
  • Codex review (gpt-6.1-sol, xhigh): round 1 found two medium issues in pinned-fetch.ts, both fixed in cad752f9: a stalled first address used up the whole deadline, and a stalled TLS handshake wasn't cancelled on abort. Round 2 on cad752f9 found nothing.

GitHub Actions is billing-blocked, so CI here is red without running. The results above are local runs, not a CI pass. After merging the latest main into the branch I re-ran docs check, lint, typecheck, npm test (651), npm run test:edge (482) and the build at the pushed head; all passed.

🤖 Generated with Claude Code

Programcode and others added 7 commits October 24, 2026 17:08
share's outer catch copied the thrown message (PostgREST or Postgres
detail) into the 500 body. create-portal copied Stripe's message, which
can name the customer id or the rejected parameter. Both now answer with
a fixed error and log the detail server-side. create-portal sends
code portal_failed, which the browser translates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
keepalive_events and keepalive_heartbeat had TO anon policies and GRANT
ALL to anon, so the publishable key could insert unbounded event rows,
read them, and overwrite the heartbeat without a session. Revoke client
grants on both tables and the event sequence, drop the anon policies,
and keep keepalive_touch() service-role only where it exists.

The external monitor's VPS timer already uses the service-role key and
keeps working. A monitor run with the publishable key now gets 401/403.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
redeem_promo_code_atomic returned early for discount codes, before
user_promos was written and redemptions_count incremented, so
max_redemptions and single use never applied. create-checkout then set
allow_promotion_codes, which lets Stripe accept any promotion code on
the account.

A discount row now carries its Stripe promotion-code id in
config.stripe_promotion_code. Claiming it records the user and counts
against the cap; asking again for a held code returns the same id
without counting. create-checkout claims the code as the user with
p_discount_only (so a non-discount code typed there grants nothing) and
sends only discounts[0].promotion_code. Every refusal is one
promo_invalid answer.

The pricing dialog keeps a claimed discount code in the field so the
upgrade sends it. Admin requires the promo_ id when creating a discount
code.

Deploy order: the migration before create-checkout. Existing discount
rows need a stripe_promotion_code before they work again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fetch() resolves the hostname again when it connects, so a name whose
DNS answer changed between validateResolvedPublicHost and the
connection (rebinding) could reach a private range. import-source now
dials one of the checked addresses directly (TCP, then TLS with SNI and
certificate checks for the hostname) and reads the answer with a small
HTTP/1.1 client: Content-Length, chunked and close-delimited bodies,
gzip and deflate, 1xx skipped, a 32 KB head cap, and anything else
refused. Each redirect hop is checked and pinned the same way.

Checked in the pinned edge-runtime image (v1.77.4) against real HTTPS
sites, an http-to-https redirect, and a certificate for the wrong name.
Closes the recorded exception in docs/standards/security.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rade to Pro

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 1: one address that never answered spent the whole import
deadline, so a healthy second address was never tried, and a TLS
handshake that stalled was not cancelled when the deadline aborted.
Each address now gets 3 s to connect and finish TLS before the next is
tried, and an abort closes the connection mid-handshake. Deno still
holds the TCP socket of a cancelled handshake until the worker exits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ndings

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing.

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

@Programcode
Programcode marked this pull request as ready for review October 24, 2026 18:43
Copilot AI balanced review requested due to automatic review settings October 24, 2026 18:43
@Programcode
Programcode merged commit 6f93aa5 into main Oct 24, 2026
3 of 14 checks passed

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants