diff --git a/.gitignore b/.gitignore index c6c2687..7126e14 100644 --- a/.gitignore +++ b/.gitignore @@ -12,10 +12,13 @@ # Misc .DS_Store -.env.local -.env.development.local -.env.test.local -.env.production.local + +# Environment files hold keys (Supabase service role, Resend, …); never commit +# them. Covers .env, .env.production, supabase/functions/.env and the *.local +# variants. Only a placeholder-only example may be committed. +.env +.env.* +!.env.example npm-debug.log* yarn-debug.log* diff --git a/dev-plans/supabase-pending-work-2026-08.md b/dev-plans/supabase-pending-work-2026-08.md index 967cfb1..fa2f887 100644 --- a/dev-plans/supabase-pending-work-2026-08.md +++ b/dev-plans/supabase-pending-work-2026-08.md @@ -1,350 +1,193 @@ -# Supabase: work that must be finished from a machine with project access +# Supabase: production state and remaining work -Written 2026-08-16, updated 2026-08-17. Everything below needs the Supabase CLI -logged in against the production project; none of it can be done from CI or from -a checkout alone. +First written 2026-08-16. Rewritten 2026-09-16, once the tasks it originally +tracked were done. Production project: `bkcimygtsnckzexbfqxh`. -Read task 1 first — it is the only item where production is currently in a -half-applied state, and it is losing data every hour it stays that way. - -The underlying theme, worth holding in mind throughout: **`supabase/migrations/` -and the production database have drifted apart in both directions.** Objects -exist in production that the repository never defines (`is_admin()`, -`public.profiles`), and migration versions are recorded in production that the -repository does not contain. Every failure below is a symptom of that one -problem, and the work is as much about reconciling the two as about any -individual fix. +The underlying theme still holds: **`supabase/migrations/` and the production +migration history do not line up.** The schema itself now matches the +repository. What is left is bookkeeping (task A), and until it is done nothing +reaches production automatically. --- -## Task 1 — finish landing the client_error_log hardening (PR #164, merged) - -### What is already true - -PR #164 is merged and Vercel has deployed `main`. The browser bundle in -production now calls `supabase.functions.invoke('report-client-error', …)` -instead of inserting into `client_error_log` directly. - -That function does not exist yet, and the migration has not been applied. - -### What that means right now - -- **Client error reports are being dropped.** The invoke fails, the failure is - caught in `src/lib/errorReporting.ts` and logged to the console, and nothing - is stored. Nothing crashes — reporting is deliberately incapable of throwing — - but production errors are invisible. -- **The hole PR #164 exists to close is still open.** `anon` keeps its `INSERT` - grant on `client_error_log` until `20260816000000` is applied, so the public - anon key can still write unbounded rows straight to PostgREST. - -**Reporting needs both halves — neither one alone restores it.** The function is -only a front door: it stores nothing by itself, it calls -`record_client_error()`, and that function is created by the migration. Deploy -the function without the migration and the RPC fails, the function swallows the -error and still answers `{"ok":true}`, and no row is written. - -Do the function first anyway. It is a one-liner and nothing depends on it, -whereas the migration is entangled with the version mismatch described in 1d and -may take real work. Just do not read a successful deploy as reporting being -fixed — that is only true after 1d. - -Neither problem is an emergency, but the window should be short. - -### 1a. Link the project - -Every `supabase db …` and `supabase migration …` command below acts on the -*linked* project. They select their target with `--linked` / `--db-url` / -`--local` and do **not** accept `--project-ref`, so link once up front: - -```bash -supabase link --project-ref -``` - -The project ref is in the Supabase dashboard URL, or in `supabase projects list`. -Do not use a ref taken from a PR check link — those are ephemeral preview -branches and differ on every PR. - -(`functions deploy` and `secrets set` do take `--project-ref`, and it is spelled -out below so those two steps work whether or not the link succeeded.) - -### 1b. Deploy the Edge Function - -```bash -supabase functions deploy report-client-error --project-ref -``` - -Verify it is reachable: - -```bash -curl -s -X POST "https://.supabase.co/functions/v1/report-client-error" -H "Authorization: Bearer " -H "Content-Type: application/json" -d '{"message":"deploy smoke test","source":"/manual-check"}' -``` - -Expect `{"ok":true}`. That is all this proves. The function answers `{"ok":true}` -for every outcome by design — stored, throttled, malformed, or RPC-missing — so -it cannot tell you the row landed, and at this point the row will *not* have -landed, because `record_client_error()` does not exist until 1d. The end-to-end -check that does confirm storage is at the end of 1d. - -If you want to see why nothing was stored, the function says so in its logs — -Edge Function logs live in the dashboard, under Edge Functions → -`report-client-error` → Logs. Expect a `[report-client-error] record failed` -line naming the missing function. (There is no `supabase functions logs` -subcommand; the CLI has `list`, `deploy`, `download`, `delete`, `new` and -`serve`.) - -### 1c. Set the IP salt (optional) - -```bash -supabase secrets set ERROR_LOG_IP_SALT="$(openssl rand -hex 32)" --project-ref -``` - -Skipping this is safe: the function falls back to the service role key as the -salt, which is unguessable and never leaves the server. Set it if you would -rather the rate-limit hashes not be derived from the service key. Changing it -later only resets the current hour's quota buckets. - -### 1d. Apply the migration - -Read the next section before running this — `db push` will probably refuse. - -```bash -supabase db push --linked -``` - -The intent is to apply `20260816000000_harden_client_error_log.sql` and nothing -else: earlier versions are already recorded as applied and are not re-run, -including the historical files edited in PR #166. - -#### Expect a version mismatch first - -The `Supabase Preview` check has been failing on every push to `main` since at -least 7 August — before any of the recent work — with: - -``` -Remote migration versions not found in local migrations directory. -``` - -The production database has migration versions recorded in -`supabase_migrations.schema_migrations` that do not exist as files in -`supabase/migrations/`. Someone applied migrations directly against the project -and they were never committed. This is the same problem as `is_admin()` and -`profiles`, seen from the other side: the repository is not a faithful record of -production. - -Two consequences worth being clear about: - -- **Nothing has been applied automatically.** The GitHub integration has not - successfully synced `main` to production for weeks, so `20260816000000` is - certainly not applied — and possibly neither are others. -- **`db push` will hit the same wall** until the histories agree. - -Start by seeing exactly where they diverge: - -```bash -supabase migration list --linked -``` - -That prints local and remote versions side by side. For each version that is -remote-only, decide deliberately: - -- **Capture it** — the right default. The migration represents real schema that - exists in production and is missing from the repo. `supabase db pull --linked` - writes the current remote schema into a new migration file; use it to recover - the definitions (this is also where `profiles` and `is_admin()` will come from, - so it doubles up with task 2). -- **Discard the record** — `supabase migration repair --linked --status reverted ` - tells the CLI to forget a remote entry. Only do this for versions you have - confirmed are obsolete or were superseded. It changes bookkeeping only; it - does not undo any schema change that migration made. - -Do not reach for `migration repair` to make the error go away quickly. Marking -real, applied migrations as reverted is how the repo drifts further from -production, and the drift is what caused every problem in this document. - -Verify the outcome. All four should hold: - -```sql --- 1. anon has no table-level or column-level INSERT -select grantee, privilege_type from information_schema.role_table_grants -where table_name = 'client_error_log' and grantee in ('anon', 'authenticated'); -select count(*) from information_schema.column_privileges -where table_name = 'client_error_log' and grantee in ('anon', 'authenticated') - and privilege_type = 'INSERT'; - --- 2. no INSERT policy remains -select * from pg_policies where tablename = 'client_error_log' and cmd = 'INSERT'; - --- 3. the quota-checked path exists -select proname from pg_proc where proname = 'record_client_error'; -select tablename from pg_tables where tablename = 'client_error_report_quota'; - --- 4. reporting now works end to end — re-run the 1b curl, then: -select message, source, created_at from public.client_error_log -order by created_at desc limit 5; -``` - -Expected: `authenticated: SELECT` only; `0` column grants; no INSERT policy; -both objects present; and the smoke-test row appearing for the first time — this -is the check that proves reporting is restored, which the deploy in 1b could not. - -Delete the smoke-test row afterwards if you care about a clean table. - -### 1e. Confirm the direct path is actually closed +## Current state (2026-09-16) -The point of the change. This must now fail: - -```bash -curl -s -X POST "https://.supabase.co/rest/v1/client_error_log" -H "apikey: " -H "Authorization: Bearer " -H "Content-Type: application/json" -d '{"message":"should be rejected","source":"/direct"}' -``` - -Expect a permission-denied error, not `201`. If it succeeds, the migration did -not apply — recheck 1d before assuming otherwise. +| Item | State | +|---|---| +| Schema | Every file in `supabase/migrations/` is applied to production. | +| `public.profiles` / `is_admin()` | Bootstrapped, guarded, in `20260613000001`. A database built from the repo alone now applies every migration. | +| `20260816000000_harden_client_error_log` | **Applied.** anon/authenticated have no INSERT on `client_error_log`; `record_client_error()` and `client_error_report_quota` exist. | +| Edge Function `report-client-error` | **Deployed** (`verify_jwt` on). Checked end to end: a report through the function is stored, and a direct PostgREST insert returns 401. `ERROR_LOG_IP_SALT` is not set; the function falls back to the service role key. | +| `20260915000000_create_workspaces` (PR #175) | **Applied.** | +| `20260916000000_harden_profiles` (PR #177) | **Applied.** Users can no longer set their own `role`; `profiles.email` is no longer readable by anon/authenticated. | +| Edge Function `newsletter-subscribe` | **Not deployed** — see task B. | +| `Supabase Preview` on PRs | Green (first time since July). | +| `Supabase Preview` on `main` | Still red, with `Remote migration versions not found in local migrations directory` — see task A. | + +## How production changes are applied until task A is done + +The GitHub integration does not sync, so migrations were applied by hand +through the Supabase connector (`execute_sql`). Each one went through the same +steps: + +1. **Check for collisions.** Look for existing objects with the same names — + `create table if not exists` silently keeps a different table, and + `create or replace` silently overwrites a function. +2. **Dry run.** Inside one transaction: + - apply the migration; + - exercise the real call paths as `anon`, `authenticated` and + `service_role`. Set `set local role …` and + `request.jwt.claims` to impersonate real users; + - end with a `raise exception` that carries the results, so everything + rolls back; + - confirm afterwards that nothing was left behind. +3. **Apply.** Apply the file verbatim in one transaction, and insert the + **repo's** version into `supabase_migrations.schema_migrations` in the same + transaction: + + ```sql + insert into supabase_migrations.schema_migrations (version, name, statements) + values ('', '', array['-- applied from supabase/migrations/']); + ``` + + Do not use the connector's `apply_migration` for repo files. It records a + fresh timestamp as the version, which is exactly how the drift in task A + came about. +4. **Verify.** + - Check catalog state: grants, policies, functions. + - Make real REST/function calls with the anon key. + - Run the security advisor. + +Edge Functions deploy fine through the connector (`deploy_edge_function`). +Pass the file from `supabase/functions//` verbatim, then read it back +with `get_edge_function`. --- -## Task 2 — close the `public.profiles` gap in the migration history - -**Done 2026-09-16.** The definition was read from production and bootstrapped, -guarded, in `20260613000001`; every migration now applies to a bare scaffold. -Reading it also showed that any signed-in user could set their own `role` to -`admin`, and that anon could read every e-mail address — -`20260916000000_harden_profiles.sql` closes both and must be applied to -production. The notes below are kept for the record. - -### Background - -The `Supabase Preview` check builds a database from `supabase/migrations/` -alone. It has been failing since 2 July. PR #166 fixed the first cause -(`is_admin()` was defined by hand in production and never entered the migration -history); the check now gets three migrations further and fails on the next -instance of the same problem: - -``` -ERROR: relation "public.profiles" does not exist (SQLSTATE 42P01) -At statement: 2 -drop trigger if exists trg_log_role_change on public.profiles -``` - -`public.profiles` is an application table created by hand in the dashboard. It -is referenced by five migrations and defined by none: - -- `20260613000001_add_admin_utility_usage_fn.sql` — first mention (inside a - function body, so it resolves lazily and does not fail at apply time) -- `20260702000000_create_admin_audit_log.sql` -- `20260702000001_account_self_service_fns.sql` -- `20260702000002_admin_analytics_fns.sql` -- `20260702000003_role_change_audit_trigger.sql` — first hard failure, because a - trigger names its table at creation time - -This is why the check fails on exactly the PRs that touch `supabase/` and is -skipped on all others: **no migration has been validated by CI since 2 July.** - -### Why this needs the real database - -The migrations only reveal the columns they happen to touch — `id`, `email`, -`full_name`, `username`, `role`. The real table certainly has more, and it -certainly has RLS policies and grants that the migrations never mention. - -Writing `create table if not exists public.profiles (…)` from those five columns -would not affect production, where the table already exists — but every preview -branch and any future rebuild would get a truncated, probably unprotected -`profiles`, and the check would go green while describing a schema that does not -exist. A green check that lies is worse than the red one. - -### Dump the real definition - -```bash -supabase db dump --linked --schema public > /tmp/public-schema.sql -``` - -From that file, extract everything about `profiles`, not just the table: - -- `CREATE TABLE public.profiles (…)` with all columns, defaults and constraints -- indexes -- `ALTER TABLE … ENABLE ROW LEVEL SECURITY` -- every `CREATE POLICY … ON public.profiles` -- `GRANT` / `REVOKE` statements naming `profiles` -- any trigger or function attached to it that is not already in - `supabase/migrations/` (`handle_new_user` and similar are common) - -Also dump `is_admin()` itself while you are there and compare it against the -deny-all stub PR #166 adds — the stub is only a fallback for fresh databases, -but knowing the real definition is worth having written down: - -```bash -supabase db dump --linked --schema public | grep -A 20 "FUNCTION public.is_admin" -``` - -### Where to put it - -Follow the pattern PR #166 established: bootstrap the object in the earliest -migration that references it, guarded so production is never touched. - -That is `20260613000001_add_admin_utility_usage_fn.sql` — the same file, which -already carries the `is_admin()` guard and is the first to mention `profiles`. -Add a guarded `create table if not exists public.profiles (…)` plus its RLS, -policies and grants above the existing function definition. - -Editing an applied migration is safe: Supabase records migrations by version and -will not re-run them, and `if not exists` means the block is inert against a -database that already has the table. - -### Verify locally before pushing - -No Supabase access needed for this part — it is the same harness used to verify -PR #166: +## Task A — reconcile the migration history + +Production records most migrations under the timestamps the connector +assigned when they were applied. The repository uses rounded versions. The +schema is the same; only the version keys differ. The CLI compares keys, so +`db push` and the `main` preview check both refuse. + +| Repo file | Production version | Notes | +|---|---|---| +| `20260512000000_create_user_utility_usage` | — | no matching record; the table exists in production | +| `20260514000000_create_user_settings` | `20260514161254` | | +| `20260613000000_add_utility_popularity_fn` | `20260613085540` | recorded as `add_global_utility_popularity_fn` | +| `20260613000001_add_admin_utility_usage_fn` | `20260613193429` | | +| `20260702000000` … `20260702000003` | `20260702161444` … `20260702161538` | same order | +| — | `20260702161609 lock_down_role_change_trigger_fn` | the repo does this inside `20260702000003` | +| `20260720000000_create_client_error_log` | `20260720073213` | | +| `20260720100000_security_advisor_cleanup` | `20260720091511` | | +| `20260720120000_create_guest_utility_usage` | `20260720100542` | | +| `20260720130000_harden_guest_usage_fn` | `20260720103715` | | +| `20260723000000_create_user_calculation_history` | `20260723205925` | | +| `20260729000000_create_newsletter_subscribers` | `20260729105337` | | +| `20260816000000`, `20260915000000`, `20260916000000` | same | already aligned | + +Remote-only records with no repo file: + +- `20260511190354` … `20260511194904` — seven migrations named `focus_*` and + `user_app_documents*`; +- `20260512080754 lockdown_admin_definer_functions`. + +They fall into two groups: + +- **`user_app_documents` and `user_app_documents_realtime`.** This repository + uses this table: `src/shared/user-data/` reads and writes it. The table + therefore belongs in `supabase/migrations/`. +- **`focus_*`.** These create the Focus-Planner sync tables (`focus_tasks`, + `focus_timer_sessions`, …). The spec lives in the Focus-Planner repository + (see `src/shared/user-data/README.md`). No code here uses them. +- **`lockdown_admin_definer_functions`.** Not yet checked. Read its + `statements` in `supabase_migrations.schema_migrations` before deciding. + +### Steps + +1. **Deal with the remote-only records.** + - **Capture what this repository owns.** At least `user_app_documents`, + plus whatever `lockdown_admin_definer_functions` turns out to be. The + original SQL is stored in the `statements` column of + `supabase_migrations.schema_migrations`. Commit each file under its + **existing** remote version, so no repair is needed for it. + - **Move the rest.** The `focus_*` migrations belong in the Focus-Planner + repository. Once they live there, mark them `reverted` here, or keep + copies in this repository if both apps are meant to share one migration + history. + + **Confirm before repairing.** `migration repair` only changes bookkeeping; + it never undoes schema. +2. **Re-key the renamed migrations to the repo versions.** This is + bookkeeping only: + + ```bash + supabase link --project-ref bkcimygtsnckzexbfqxh + supabase migration repair --linked --status reverted 20260514161254 20260613085540 20260613193429 20260702161444 20260702161503 20260702161526 20260702161538 20260720073213 20260720091511 20260720100542 20260720103715 20260723205925 20260729105337 + supabase migration repair --linked --status applied 20260512000000 20260514000000 20260613000000 20260613000001 20260702000000 20260702000001 20260702000002 20260702000003 20260720000000 20260720100000 20260720120000 20260720130000 20260723000000 20260729000000 + ``` + + `20260512000000` is marked applied because its table already exists in + production. Diff the definition before relying on that. +3. **Check the result.** `supabase migration list --linked` should show every + version on both sides. After that, `supabase db push --linked --dry-run` + should report nothing to apply. + +When the next push to `main` turns `Supabase Preview` green, migrations sync +automatically again. Then the manual procedure above is no longer needed. + +## Task B — deploy `newsletter-subscribe` + +The site calls `supabase.functions.invoke('newsletter-subscribe')` from +`src/shared/newsletter/`, but the function is not deployed. The subscription +form therefore cannot work in production. The table and its RPCs +(`20260729000000`) are in place. + +The function reads these environment variables: + +| Variable | Required | Default | +|---|---|---| +| `RESEND_API_KEY` | yes | — | +| `SITE_URL` | no | `https://cadautoscript.com` | +| `NEWSLETTER_FROM` | no | `CAD AutoScript ` | + +Set the secret first, then deploy: ```bash -docker run -d --name mig-check -e POSTGRES_PASSWORD=test -p 55440:5432 postgres:17-alpine -``` - -Scaffold **only** what Supabase itself provides, so anything missing from the -migration history shows up as a failure rather than being papered over: - -```sql -create role anon; create role authenticated; create role service_role; -create schema if not exists auth; -create table auth.users (id uuid primary key default gen_random_uuid(), email text); -create function auth.uid() returns uuid language sql stable as $$ select null::uuid $$; +supabase secrets set RESEND_API_KEY= --project-ref bkcimygtsnckzexbfqxh +supabase functions deploy newsletter-subscribe --project-ref bkcimygtsnckzexbfqxh ``` -Do **not** create `profiles` or `is_admin()` in the scaffold — those are the -things under test. Then apply every migration in filename order and confirm all -of them succeed. When that passes with nothing but the four lines above, the -preview check will pass too. - ---- +Check that the sending domain is verified in Resend before announcing the +form. -## Task 3 — make the check mean something +## Task C — make the preview check required -Once it goes green, make `Supabase Preview` required on `main` in the branch -protection settings. It is the only thing that would have caught any of these -gaps, and while it fails it silently protects nothing. +Once task A turns `Supabase Preview` green on `main`, make it a required check +in the branch protection for `main`. It is the only check that catches +migrations missing from the repository. -Note that the check reports two different failures depending on where it runs, -and both must be fixed: +The check reports two different failures depending on where it runs: -- **on a pull request** it builds a database from `supabase/migrations/` alone — - this is the one that fails on `profiles` (task 2); -- **on `main`** it syncs to production — this is the one that fails on the - version mismatch (task 1d). +- **on a pull request**, it builds a fresh database from + `supabase/migrations/`. This is now green. +- **on `main`**, it syncs to production. This fails until task A is done. ---- +**Stale preview branches.** A PR's preview branch keeps its migration history +between pushes. If a migration file that the branch has already recorded is +edited, the edit is never re-run there, and the check keeps failing on old +content. Reset or delete that preview branch in the dashboard (Branches). A +branch created afterwards starts clean. -## Status as of 2026-08-17 +## Smaller follow-ups (from the security advisor) -| Item | State | -|---|---| -| PR #164 — client_error_log via Edge Function | merged; **function not deployed, migration not applied** | -| PR #165 — camera scoped to QR Master | merged; smoke-test QR Master scanning on `/utilities/qr-master/` and `/ru/utilities/qr-master/`, plus the whisper microphone on a non-English page | -| PR #166 — `is_admin()` bootstrap order | merged; PR-side preview now gets past it and fails on `profiles` (task 2) | -| PR #169 — related tools use the catalog card | merged | -| PR #168 — nanoid pinned to a patched 3.x | merged | -| PR #167 — this document | open | -| PR #163 — dompurify 3.4.12 → 3.4.13 | merged | -| dependabot alerts | **none open.** dompurify closed by #163, nanoid by #168, and the two `image-size` highs dismissed as not-used (build-time only, no patch exists) | -| `Supabase Preview` on `main` | failing since ≥ 7 August on the version mismatch — nothing is syncing to production automatically | - -Everything that could be finished from a checkout is finished. What is left in -this document is exactly the part that needs project access, and task 1 is the -one carrying a cost while it waits. +- **`trim_calculation_history()` is a trigger function.** It is still + executable by anon and authenticated. Calling it over RPC only errors, but + revoking `execute` from `public, anon, authenticated` would clear the + warning, as the workspace migration does for its triggers. +- **Leaked password protection is disabled.** Supabase offers it on paid plans + only, so it is deferred while the project runs on the free tier. +- **"RLS enabled, no policy"** on `workspace_invites` and + `client_error_report_quota` is intentional. Both are reachable only through + security definer functions or the service role.