fix(db): close the PostgREST Data API surface (migration 027)

docs/decisions/README.md

@@ -62,6 +62,71 @@ It is not part of Helpthread's product or architecture.
 The original charter and its complete amendment record are preserved at
 [docs/history/CHARTER-v1.md](../history/CHARTER-v1.md).
 
+## 2026-07-28 — The Supabase Data API is closed; Postgres is reached directly
+
+**Decided:** 2026-07-28
+
+**Context.** A Supabase advisor alert reported one publicly accessible table. Checking the
+project found the problem was broader: all 19 tables in `public` had Row-Level Security
+disabled *and* full `SELECT/INSERT/UPDATE/DELETE/TRUNCATE` granted to the `anon` and
+`authenticated` roles. Supabase exposes `public` through PostgREST, and the anon key is
+public by design, so this was unauthenticated read and write against production —
+including INSERT into `agents`, `agent_auth_identities`, `agent_mailbox_access`, and
+`webauthn_credentials`, which is enough to self-provision an authenticated Agent.
+`mailbox_oauth_tokens` was partially cushioned: its token columns hold AES-256-GCM
+ciphertext keyed outside the database, so a dump yields ciphertext, not usable Gmail
+credentials. Inspection of the data found no evidence of tampering — this checked state,
+not access logs, so it is not proof of non-access.
+
+**Decision.** The PostgREST Data API is not part of Helpthread's architecture. The
+application reaches Postgres directly over the pooler (`DATABASE_URL`) and uses Supabase
+Storage with the `service_role` key; no anon-key client exists anywhere in the codebase.
+Migration 027 therefore enables RLS on every table and revokes the `anon`/`authenticated`
+grants, including via `ALTER DEFAULT PRIVILEGES` so future tables created by the migrating
+role do not arrive pre-granted. Neither half hardcodes `public`, because `PostgresDb`
+supports a `schema` option; the revokes derive the schema from `'conversations'::regclass`
+— deliberately *not* `current_schema()`, which resolves to search_path's first entry rather
+than to the schema actually holding the tables, and so can diverge from the unqualified
+`ALTER TABLE`s. (An earlier revision of this change used `current_schema()` and was caught
+in review; the divergence fails silently, leaving the grants in place while the migration
+reports success.) The same lockdown was applied directly to the production project ahead of
+the migration landing, to close live exposure rather than wait on review.
+
+Be precise about the limits of the `ALTER DEFAULT PRIVILEGES` layer, which is easy to
+overrate: it *deletes a default-ACL entry* rather than installing a standing deny, and
+without `FOR ROLE` it binds only to the role that ran the migration. It does not survive
+Supabase re-running its stock bootstrap, and it does not touch defaults defined for other
+roles such as `supabase_admin`. The durable protection is the standing rule below.
+
+**Alternatives considered.** Enabling RLS alone was rejected: the grants would remain, so
+a single future permissive policy would reopen everything. Revoking grants alone was
+rejected for the mirror-image reason — anything that re-grants, including Supabase
+re-running its stock bootstrap, restores them, and as noted above the default-privileges
+layer does not prevent that. Writing real per-tenant RLS policies was rejected as solving a problem Helpthread
+does not have: policies exist to make anon access safe, and there is no anon access to
+make safe. Deny-by-default with no policies is the honest expression of that.
+
+**Consequences.** The Supabase security advisor now reports `rls_enabled_no_policy` at
+INFO level for every table. That is the intended end state, not an outstanding item.
+Tables are owned by `postgres`, which bypasses RLS unless `FORCE ROW LEVEL SECURITY` is
+set — deliberately not set — so the application is unaffected. A standing rule follows:
+any migration adding a table must also enable RLS on it.
+
+**This promotes an implicit deployment detail into an invariant: `DATABASE_URL` must
+connect as the role that owns the tables.** It does today — the runbook has the operator
+use the Supabase pooler string (role `postgres`) for both the app and `scripts/migrate.ts`,
+so connecting role and owning role coincide by construction. But pointing `DATABASE_URL` at
+a dedicated least-privilege role — a supported Supabase pattern — now breaks the engine, and
+the two halves break differently: reads go quiet (RLS with no policies returns zero rows
+instead of raising, so the symptom is an empty inbox rather than a database error) while
+every write hard-errors with `new row violates row-level security policy`. The loud signal
+arrives first, via inbound ingest. No test can catch the misconfiguration, because PGlite
+also runs as the owner. Separately,
+`splitStatements` in `src/db/migrate.ts` had to learn about dollar quoting, since
+migration 027's role guard is a `DO $$ ... $$` block whose body contains semicolons.
+
+**Supersedes.** Nothing.
+
 Future material decisions should record:
 
 1. date and scope;

specs/mail/mailbox-connection.md

@@ -154,10 +154,10 @@ Stated explicitly in the operator docs, because it is the quiet advantage: repli
 App passwords are long-lived secrets granting full mailbox access. They must be encrypted at rest via the existing `HELPTHREAD_TOKEN_ENC_KEY` path (`src/store/token-crypto.ts`, AES-256-GCM) already used for OAuth tokens, never logged, never returned by any API read, and write-only in the UI — show a "configured" state, never the value.
 
 **Encryption is not sufficient on its own.** The credential table needs
-deny-by-default server-only authorization, not merely ciphertext at rest — the
-same gap already open on `mailbox_oauth_tokens` (see §8: RLS is disabled on
-every table today). Whatever answer that gets must cover this table from the
-day it exists, rather than inheriting the same debt.
+deny-by-default server-only authorization, not merely ciphertext at rest. That
+answer now exists (see §8): migration 027 enables RLS on every table and revokes
+the PostgREST role grants, so a new table inherits protection only if it does the
+same. This table must enable RLS in the migration that creates it.
 
 **Lifecycle — app passwords die quietly.** Unlike an OAuth grant, there is no
 revocation signal and no refresh failure to classify. They stop working when
@@ -210,7 +210,7 @@ and documentation, not by changing Helpthread's operator-deployed model.
 
 Also found while mapping the current path, each requiring separate follow-up:
 
-- **RLS is disabled on all 19 tables**, including `conversations` and `mailbox_oauth_tokens`. Exposure depends on whether the anon key is distributed; enabling RLS without policies would break the engine's own access, so this needs deliberate policy design.
+- **RLS was disabled on all 19 tables** — addressed by migration 027, which enables RLS on every table and revokes the `anon`/`authenticated` grants. The concern recorded here that "enabling RLS without policies would break the engine's own access" was wrong *given how the engine actually connects*: it connects as the role that owns the tables, and an owner bypasses RLS unless `FORCE ROW LEVEL SECURITY` is set. No policy design was needed, because nothing should reach these tables through PostgREST at all. Note the deployment invariant this promotes: `DATABASE_URL` must connect as the table-owning role. Pointing it at a dedicated least-privilege role instead breaks the engine, and the two halves break differently: reads go *quiet* (RLS with no policies returns zero rows rather than raising, so the symptom is an empty inbox) while every write hard-errors with `new row violates row-level security policy`. Expect the loud signal first — inbound ingest fails immediately — with silently empty reads alongside it. Standing rule: a migration adding a table must also enable RLS on it. See [docs/decisions/README.md](../../docs/decisions/README.md).
 - **Three doc-drift defects** in `specs/deploy/gmail-inbound-runbook.md`: it names an `api/[...path].ts` entrypoint that was tried and abandoned (the real file is `api/index.ts`, which documents why), says "three cron jobs" where `vercel.json` declares four, and omits `HELPTHREAD_UI_BASE_URL` and the entire web-project env set — so an operator following only the runbook gets a working engine and a non-functional UI.
 - **No `.env.example`** anywhere in the repo; the 17-variable contract exists only as a prose table and as validation logic in `src/composition/config.ts`.
 - **History-cursor expiry fallback** not audited. Gmail expires history cursors; the 404 path exists but has not been reviewed for a mailbox that goes quiet for an extended period.