[Programcode]

Batch T15 from the 22 Oct QA report: Supabase security and database hygiene (SC-04, SC-08, DB-03, DB-04, DB-05).

Every new migration is non-destructive. The destructive steps are proposals in supabase/proposed/, which db push and db reset never run. Nothing has been applied to production (uziwdtkzofptihahfooo) or staging (brifeyypjauyusmkptkx). No Edge Function changed, so there is nothing to deploy.

The QA snapshot files (/workspace/debatelab-qa/supabase/*) aren't on this VPS. The evidence below comes from the repo migrations, the local stack's own security advisor, and catalog queries on a fresh local replay. Where production can differ from the repo, I say so.

What changed, by issue

SC-04: share views counted on every reload

20261023060000_share_view_dedup.sql replaces get_shared_item(text), keeping the same signature, result columns and grants. It now counts each viewer at most once per share per UTC day:

• The viewer is the signed-in user (auth.uid()). Otherwise it's the client IP from PostgREST's request.headers: cf-connecting-ip first (the edge sets it), then the first x-forwarded-for entry, then x-real-ip, then 'unknown'.
• A new public.share_views (share_id, viewed_on, viewer_key) table records sha256(share id : day : viewer), so rows can't be matched across shares or days. RLS is on, there are no policies, and clients have no grants; only the function writes it.
• An expired or unknown code still returns no row. SharedPage.tsx is unchanged.
• Product change: the "views" number on the share page now means distinct viewers per day, not page loads. People sharing one NAT address (a school network, for example) count once.
• Each counted view deletes at most 500 earlier-day records, so the first view after midnight never runs a large delete inside a visitor's request.
• This stops reload inflation. It is not abuse protection: a client that really changes IP still counts again.

SC-08: advisor warnings

Leaked-password protection is a dashboard setting. It's listed under "For Adam" below. The app signs in with an email code, Google or a demo code, so the impact is low.

SECURITY DEFINER RPCs executable by authenticated. The local replay has 22 (21 in public plus private.is_current_workspace_member). Each one has a browser caller (or the collaboration service, which forwards the user's own JWT) and checks auth.uid(), ownership or membership in its body. Every one pins its search_path: ai_usage_today and clear_current_user_data use public (only postgres can create objects there), and the rest, including the rewritten get_shared_item, use "". So no revokes and no body fixes were needed.

Function	Callers	Guard	Decision
ai_usage_today	src/hooks/use-subscription.ts	reads only auth.uid()'s row	keep
attach_to_debate	sparring-store.ts, session-store.ts	owner of item and debate	keep
change_workspace_access	src/lib/workspace/client.ts, services/collaboration (user JWT)	member role check	keep
change_workspace_state	workspace/client.ts	member role check	keep
clear_current_user_data	session-store.ts	auth.uid() only	keep
create_workspace	WorkspacesPage.tsx	auth.uid() becomes owner	keep
create_workspace_comment / reply_ / resolve_	workspace/client.ts	private.require_workspace_document_member	keep
create_workspace_document / update_	workspace/client.ts	member check	keep
create_workspace_proposal	workspace/client.ts	require_workspace_document_member	keep
create_workspace_thread, save_workspace_source / _artifact / _thread_message	workspace/practice-client.ts	require_active_workspace_member	keep
create_workspace_version / restore_ / get_workspace_version_snapshot	workspace/client.ts	member check	keep
get_shared_item	SharedPage.tsx (anon)	code-scoped capability	keep, rewritten for SC-04
redeem_promo_code_atomic	redeem-promo Edge Function via userClient (user JWT)	auth.uid()	keep (T4 owns it; unchanged)
private.is_current_workspace_member	every workspace RLS policy	auth.uid()	keep

The existing pgTAP files already cover each kept RPC: it works for its owner or member and is refused for another user or for anon (current_debates_test, ai_quota_sessions_test, workspace_*_test, promo_redemption_test, account_deletion_test). Since nothing was revoked, there was no service-role-only path to test.

Tables with RLS and no policies. These are deny-all for clients by design: every reader is the service role or a SECURITY DEFINER function, and no client code reads them. Accepted as they are. On the local replay there are 18: private.opus_default_backup, private.promo_redeem_failures, private.workspace_document_state, private.workspace_document_updates, private.workspace_invite_attempts, private.workspace_invites, private.workspace_version_data, public.ai_quota_sessions, public.daily_usage_counters, public.demo_access_attempts, public.demo_access_codes, public.global_prompts, public.promo_codes, public.shared_items, public.stripe_customers, public.user_data_clear_markers, public.webhook_events, and the new public.share_views.

Broad table grants that only RLS protects. Tightened only where no client caller exists, one migration per table:

Migration	Change	Why it's safe
…060100_revoke_client_writes_user_roles	no INSERT/UPDATE/DELETE for anon or authenticated, no anon SELECT	written only by the admin and demo-code-login Edge Functions (service role); the browser reads its own row (SettingsPage)
…060200_revoke_client_writes_subscriptions	same	written only by stripe-webhook and create-checkout (service role); the browser reads its own rows
…060300_revoke_client_writes_user_promos	same	written by redeem_promo_code_atomic (definer); read by create-checkout (service role)
…060400_revoke_client_writes_admin_grants	same	written by the admin Edge Function (service role)
…060500_revoke_client_access_promo_codes	all client access	admin and create-checkout use the service role; redeem RPC is definer; no policy existed
…060600_revoke_client_access_daily_usage_counters	all client access	quota RPCs use the service role; the browser reads through ai_usage_today() (definer); no policy existed
…060800_revoke_client_select_ai_requests_anon	no anon SELECT	no signed-out reader; signed-in users keep reading their own rows

The user_roles one matters most. QA saw prod-only permissive policies on user_roles. With a write grant plus a policy like FOR ALL USING (user_id = auth.uid()), a user could set their own role to admin. server_written_tables_grants_test.sql creates exactly that policy inside the test and checks the update changes nothing. With the old grant restored, that test fails 3 of 6 (the role becomes admin).

Left as they are, listed only: anon CRUD grants on the owner-scoped tables (cases, attacks, brainstorms, factoids, chats, messages, sources, source_files, source_folders, flowcharts, debates, evidence_cards, current_debates, api_keys) and on curated_sources; anon SELECT on user_settings. Their policies need auth.uid(), which is null for anon, so anon sees and changes nothing. keepalive_* stays anon-writable for the external uptime monitor.

DB-03: prod-only legacy policies

• supabase/proposed/20261023060000_drop_legacy_policies.sql drops every policy on shared_items, every policy on user_roles except user_roles_select_own, and every policy on ai_requests except ai_requests_select_own.
  • The QA policy names aren't in the repo, so it drops by exclusion and prints each policy's USING and CHECK before dropping it.
  • It stops before dropping anything unless both kept policies exist and read only the caller's own rows. It prints both kept policies too.
  • Write policies on these tables are already inert, because clients have no write grants. Legacy SELECT policies on user_roles and ai_requests stay live for signed-in users until this runs.
  • Do a dry run in BEGIN … ROLLBACK first.
• The repo defines no policy on shared_items (20260819000000 dropped both and revoked client grants), so its two auth_rls_initplan warnings come from prod-only policies. The proposed drop resolves them; no migration in this PR can.
• Drift worth knowing: the local replay still has about 20 repo policies with bare auth.uid() (api_keys_all, cases_all, flowcharts_*, user_settings_*, and others). The QA run flagged only the two on shared_items, so production's policy set differs from the repo's. I didn't touch them, because ALTER POLICY on a name production may not have would fail the migration.

DB-04: staging_auto_admin() on staging

supabase/proposed/20261023060100_drop_staging_auto_admin.sql: DROP FUNCTION IF EXISTS public.staging_auto_admin(), with a pre-check query for leftover triggers. Run it by hand on staging only. It is not in migrations/.

DB-05: index and key hygiene

• Unindexed foreign keys. The local replay has 28. 20261023060700_index_foreign_keys.sql adds 21 indexes covering 23 of them: source_files (source_id, user_id), api_keys.user_id, user_promos.promo_code_id, shared_items.created_by, demo_access_codes.created_by, and 15 on the workspace tables (two composite (workspace_id, …) indexes also cover the single-column workspace_id FKs).
  • Skipped: created_by on global_prompts, promo_codes and curated_sources, which are admin-authored tables with a handful of rows.
  • Also skipped: the (workspace_id, document_id) keys of private.workspace_document_state and private.workspace_document_updates. Their primary keys already lead with document_id, and workspace_document_updates takes a write on every collaborative edit. The advisor will still list these five.
  • Plain CREATE INDEX IF NOT EXISTS with a squawk-ignore-file require-concurrent-index-creation. Each table's writes are blocked until the file's transaction commits. The largest tables here are source_files and demo_access_codes; the workspace tables are empty in production because the feature is hidden.
• Duplicate indexes. supabase/proposed/20261023060200_drop_duplicate_indexes.sql drops idx_subscriptions_stripe_customer (duplicate of idx_subscriptions_stripe_customer_id) and five more exact duplicates of unique-constraint indexes (idx_subscriptions_stripe_sub, idx_promo_codes_code, idx_global_prompts_feature, idx_user_roles_user_id, demo_access_codes_email_idx). Each drop runs only if the index that stays exists, is valid, is unique whenever the dropped one is, and has the same columns, operator classes and collations.
• Unused indexes (39 on prod). Nothing is dropped. The QA list isn't available here, and a fresh local replay has no usage stats to reproduce it.
• Table without a primary key. private.promo_redeem_failures. supabase/proposed/20261023060300_promo_redeem_failures_primary_key.sql adds a surrogate id bigint GENERATED ALWAYS AS IDENTITY PRIMARY KEY. That can't fail on duplicate rows, unlike a key on (user_id, failed_at). It's a proposal because it rewrites the table.

Tooling fix outside the batch scope

scripts/agent-stack.sh stripped only the magic-link template from its overlay config. T2 added a confirmation template, so agent-stack.sh start failed with content_path … resolves outside the project root. It now strips every [auth.email.template.*] block. Without this I couldn't start the stack.

Migrations (all non-destructive)

1. 20261023060000_share_view_dedup.sql: new table and index, CREATE OR REPLACE FUNCTION with the same signature
2. 20261023060100_revoke_client_writes_user_roles.sql
3. 20261023060200_revoke_client_writes_subscriptions.sql
4. 20261023060300_revoke_client_writes_user_promos.sql
5. 20261023060400_revoke_client_writes_admin_grants.sql
6. 20261023060500_revoke_client_access_promo_codes.sql
7. 20261023060600_revoke_client_access_daily_usage_counters.sql
8. 20261023060700_index_foreign_keys.sql
9. 20261023060800_revoke_client_select_ai_requests_anon.sql

Each REVOKE can be undone on its own with the matching GRANT. Migrations 2 to 7 and 9 are independent of each other.

Proposed, never applied (supabase/proposed/)

• 20261023060000_drop_legacy_policies.sql: production (and staging, if the same policies exist there)
• 20261023060100_drop_staging_auto_admin.sql: staging only
• 20261023060200_drop_duplicate_indexes.sql
• 20261023060300_promo_redeem_failures_primary_key.sql

I dry-ran each one on the local replay inside BEGIN … ROLLBACK, with two fake legacy policies added. The drops removed exactly the fakes and the six duplicates, and kept the intended policies.

Edge Functions to deploy

None.

For Adam (dashboard and manual steps)

1. In the dashboard, open Auth, then Passwords, and turn on leaked-password protection on production and on staging.
2. In the staging SQL editor, run supabase/proposed/20261023060100_drop_staging_auto_admin.sql after its trigger pre-check.
3. Decide on the other proposed files. For the policy drop, dry-run it first and read its NOTICE lines.
4. When this merges, apply the 9 migrations to staging, then production (npm run backend:status first). The frontend doesn't depend on them.

Verification

CI won't run (no Actions minutes left), so everything below ran locally. A local pass covers only the local commit.

Full npm run verify on the final head 5c8144b1:

• Docs check, lint (including the squawk migration lint), typecheck.
• Unit tests: 80 files, 581 tests. Collaboration: 30 tests. Edge Functions: 457 passed, 0 failed.
• Build with prerender, sitemap and check-site: 48 HTML files, no problems. test:sw: all offline checks true.
• E2e smoke: 43 passed.
• The e2e port was pinned to 18100. The T11 worktree had a vite preview running on this worktree's registry port (18115), and I left it alone; otherwise the script ran as written.

./.superset/db-gate.sh (full, under the shared lock):

• Final head 5c8144b1 (gate run 3): pgTAP 516/516, all database and concurrency scripts passed, full-stack e2e 71 passed and 1 skipped. That covers billing, admin, practice, case generation, the signed-out share link and the workspace RPC flows, on desktop and mobile Chromium.
• Previous head 6061ca32 (gate run 2): pgTAP 516/516, all database scripts passed, full-stack e2e 71 passed and 1 skipped. That includes the signed-out share link (public.spec.ts), billing and the workspace flows on both viewports.
• Gate run 1 on the first commit: 2 desktop case-generation specs failed. One hit the app's "Unable to load DebateLab" boot screen before any database call. The other read sources before the save landed, and this PR doesn't touch sources. Both passed on mobile in the same run, and on both viewports in run 2.

Fresh local stack (scripts/agent-stack.sh):

• After db reset: pgTAP 516/516 and the concurrency scripts.
• supabase db lint: only 2 existing unused-variable warnings, both in workspace functions this PR doesn't touch.
• Security advisor before and after: 17 rls_enabled_no_policy INFO entries became 18 (the new share_views). Nothing else appeared.

Mutation checks (do the new tests catch the bug?):

• Restoring the old get_shared_item makes share_views_test.sql fail.
• Re-granting UPDATE on user_roles makes server_written_tables_grants_test.sql fail 3 of 6, and the user's role becomes admin.

Proposed files: each was dry-run inside BEGIN … ROLLBACK. The policy-drop guard raised as intended when user_roles_select_own was missing.

Not verified:

• Anything on staging or production.
• The real Cloudflare cf-connecting-ip header on hosted Supabase. The tests set request.headers by hand.
• A real browser against the deployed share page.

Manual check after deploy: open a share link twice from the same browser and confirm the count goes up once, then once more from another network.

Review log

Each round was one fresh-context Claude Opus subagent reading the full diff against main.

Round 1 (on 4fb9ab35): 2 Medium, all fixed in 6061ca32.

• Medium: anon kept SELECT on ai_requests. Fixed with new migration 20261023060800, plus a test.
• Medium: the policy-drop proposal didn't check that the kept policies exist. It now raises before dropping anything.
• Lows and nits fixed:
  • a test for pruning old share views;
  • honest wording about the viewer hash;
  • a stronger duplicate-index guard;
  • corrected comments on lock duration and on who writes user_roles;
  • a DateStyle-independent hash;
  • has_any_column_privilege checks;
  • dropped two composite workspace FK indexes the primary keys already serve;
  • notes on the staging-drop and primary-key proposals.

Round 2 (on 6061ca32): no Medium or High. All round-1 fixes confirmed. Lows fixed in 5c8144b1:

• the share-view prune is bounded to 500 rows per call;
• the policy proposal also checks what the kept policies allow;
• column-level SELECT is checked;
• this description states the "views" meaning change.

Left open (Nits):

• A non-counting call returns the count read before a concurrent increment.
• A deleted user's hashed view key lives until the next day's prune.
• The duplicate-index guard doesn't check indimmediate; none of the six pairs is deferrable.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Shared-item view counts now count each viewer only once per day, providing more consistent engagement totals.

• Security

  • Access to sensitive account, subscription, promotion, and usage data is more restricted.

• Documentation

  • Added guidance for reviewing and authorizing proposed database changes; these changes are not applied automatically.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Programcode/DebateLab
• Branch: fix/t15-db-security-hygiene
• File: .coderabbit.yaml
• Commit: 5c8144b
• Layers: Repository YAML (base), Organization UI (inherited)
• Review profile: CHILL
• Plan: Free
• Run ID: 8f86603b-2046-4719-adbf-a5d853cf0c24

📥 Commits

Reviewing files that changed from the base of the PR and between 7407139 and 5c8144b.

📒 Files selected for processing: 20

• scripts/agent-stack.sh
• supabase/README.md
• supabase/migrations/20261023060000_share_view_dedup.sql
• supabase/migrations/20261023060100_revoke_client_writes_user_roles.sql
• supabase/migrations/20261023060200_revoke_client_writes_subscriptions.sql
• supabase/migrations/20261023060300_revoke_client_writes_user_promos.sql
• supabase/migrations/20261023060400_revoke_client_writes_admin_grants.sql
• supabase/migrations/20261023060500_revoke_client_access_promo_codes.sql
• supabase/migrations/20261023060600_revoke_client_access_daily_usage_counters.sql
• supabase/migrations/20261023060700_index_foreign_keys.sql
• supabase/migrations/20261023060800_revoke_client_select_ai_requests_anon.sql
• supabase/proposed/20261023060000_drop_legacy_policies.sql
• supabase/proposed/20261023060100_drop_staging_auto_admin.sql
• supabase/proposed/20261023060200_drop_duplicate_indexes.sql
• supabase/proposed/20261023060300_promo_redeem_failures_primary_key.sql
• supabase/proposed/README.md
• supabase/tests/admin_tables_rls_test.sql
• supabase/tests/server_written_tables_grants_test.sql
• supabase/tests/share_views_test.sql
• supabase/tests/user_roles_rls_test.sql

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The changes update an agent-stack configuration filter and Supabase database behavior. They add daily deduplication for shared-item views, revoke specified client privileges, add foreign-key indexes, and introduce proposed SQL scripts for manual application.

Changes: Agent stack configuration

Layer / File(s)	Summary
Filter auth email template sections scripts/agent-stack.sh	The overlay filter now skips any section with an [auth.email.template.*] header. The comment now describes removing auth email template blocks.

Supabase database changes

Layer / File(s)	Summary
Deduplicate shared-item views supabase/migrations/20261023060000_share_view_dedup.sql, supabase/tests/share_views_test.sql	get_shared_item(text) counts a viewer once per share per UTC day. It identifies viewers from the authenticated user or request headers, then falls back to unknown. The migration stores hashed viewer keys and deletes up to 500 earlier-day records when it counts a new view. Tests cover viewer identification, access, and cleanup.
Restrict client table privileges supabase/migrations/20261023060100_revoke_client_writes_user_roles.sql, supabase/migrations/20261023060200_revoke_client_writes_subscriptions.sql, supabase/migrations/20261023060300_revoke_client_writes_user_promos.sql, supabase/migrations/20261023060400_revoke_client_writes_admin_grants.sql, supabase/migrations/20261023060500_revoke_client_access_promo_codes.sql, supabase/migrations/20261023060600_revoke_client_access_daily_usage_counters.sql, supabase/migrations/20261023060800_revoke_client_select_ai_requests_anon.sql, supabase/tests/server_written_tables_grants_test.sql, supabase/tests/admin_tables_rls_test.sql, supabase/tests/user_roles_rls_test.sql	The migrations revoke specified read or write privileges from anon and authenticated. Tests check these privileges, service-role access, and that a signed-in user cannot change their own role to admin. Anonymous SELECT tests now expect a 42501 privilege error for admin_grants and user_roles.
Add foreign-key indexes supabase/migrations/20261023060700_index_foreign_keys.sql	The migration adds 22 indexes for foreign-key columns across source, user, promo, shared-item, demo-access, and workspace tables. Comments describe exclusions and note that transactional index builds block writes until commit.
Validate and remove selected policies supabase/proposed/20261023060000_drop_legacy_policies.sql	The proposed script checks that two named policies exist and match the required SELECT type and USING expression. If both checks pass, it drops all policies on shared_items and all but the named policies on user_roles and ai_requests.
Add proposed SQL operations and application guidance supabase/proposed/20261023060100_drop_staging_auto_admin.sql, supabase/proposed/20261023060200_drop_duplicate_indexes.sql, supabase/proposed/20261023060300_promo_redeem_failures_primary_key.sql, supabase/README.md, supabase/proposed/README.md	The proposed scripts drop a zero-argument staging function, conditionally drop duplicate indexes, and add an identity primary key when the id column is absent. The README files describe the proposed directory and state that its scripts require authorization and manual application; they are not applied by the listed database commands or CI.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant get_shared_item
  participant share_views
  participant shared_items
  Caller->>get_shared_item: Submit share code and request context
  get_shared_item->>share_views: Insert daily viewer key if absent
  get_shared_item->>shared_items: Increment view_count for a new key
  get_shared_item->>share_views: Delete up to 500 earlier-day records
  get_shared_item->>Caller: Return shared item details and view_count

Loading

• 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.}