[pull] main from Open-Legal-Products:main

✅ merged · #26 · admariner/mike ← Open-Legal-Products/mike · opened 26d ago by pull[bot] · merged 26d ago by pull[bot] · +5,365-155 across 39 files · ↗ on GitHub

From the PR description

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

Our analysis

Merge Open Legal Products fork updates — read the full analysis →

Think the analysis missed something the PR description covers?

Commits in this PR (23)

SHA Subject Author Date
2bf6f9f8 feat: workspace history of actions (History page + audit_events) JJ 2026-07-28 ↗ GitHub
commit body
Adds an append-only audit trail of user actions with a top-level History
page (sidebar entry): User, Created, Title (linked), Status, Type,
Application and Model columns, with search, type filter, date range, CSV
export and pagination.

Backend: audit_events table (schema.sql + dated migration), a
fire-and-forget recorder (lib/audit.ts) that can never throw or block a
user-facing path, and call sites for chat turns (model, cancelled status,
plus per-artifact rows for generated/edited documents and applied
workflows mined from the persisted stream events), document uploads,
tabular review creation/generation and data exports. GET /audit lists
events (visibility: own actions plus activity in projects owned by or
shared with the caller); GET /audit/export streams the same view as CSV.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ec34667f fix(db): lock down audit_events to the backend (revoke + RLS + grant order) Amal 2026-08-01 ↗ GitHub
commit body
WHY THIS MATTERS
This repo's threat model explicitly includes direct PostgREST access with the
public anon key - which is why schema.sql revokes anon/authenticated on every
backend-owned table. audit_events shipped with neither a revoke nor RLS, so on a
hosted Supabase deployment its default privileges leave the whole table readable
AND writable from the browser: any visitor could dump every user's email, chat
titles and prompt excerpts, or forge/delete audit rows - which defeats the whole
point of an append-only audit trail.

WHAT IS RLS / PostgREST default access
Supabase exposes every table in schema `public` over PostgREST. Whether the
browser `anon`/`authenticated` roles can touch a table is governed by two
things: (1) SQL table GRANTs (managed Supabase's default ACLs grant these roles
broad privileges on new tables), and (2) Row-Level Security. With RLS disabled
and the default grants in place, the table is wide open. The repo's convention
is defense-in-depth: `revoke all ... from anon, authenticated` removes the
grant, and `enable row level security` (with no policies) means even if a grant
slips back the rows are invisible. service_role bypasses RLS, so the backend
path is unaffected.

HOW IT WORKS
- schema.sql: audit_events now has `revoke all ... from anon, authenticated` in
  the revoke block and `enable row level security`, matching every sibling
  table.
- Grant ordering (F4): `grant ... on all tables in schema public to
  service_role` only covers tables that already exist when it runs. The table
  was defined *after* that block, so a fresh plain-Postgres install created it
  with no service_role privileges and the backend's inserts failed
  permission-denied - silently, because recordAudit swallows errors. The DDL is
  moved above the grant block so the blanket grant covers it.
- migration 20260728: adds the same revoke + RLS, plus an explicit
  `grant select, insert, update, delete ... to service_role` so a fresh apply
  works even where service_role has no default ACL for new tables. The old
  comment ("no RLS policies needed, like other app tables") inverted the
  convention and is corrected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3d4e0d52 fix(privacy): purge audit_events on account deletion and include them in export Amal 2026-08-01 ↗ GitHub
commit body
WHY THIS MATTERS
audit_events stores personal data: the user's id, email, chat/document titles
and prompt excerpts. Account deletion erased chats, projects, documents and
workflows but left the audit rows behind forever - a GDPR "right to erasure"
gap, and the rows also became orphans pointing at chats/projects that no longer
exist. Separately, GET /user/export (the user's own copy of their data) omitted
audit rows, so the export was incomplete.

WHAT IS "erasure completeness"
When a user deletes their account, every table keyed by their identity must be
swept - not just the primary feature tables. Any table carrying user_id (or
their email, titles, excerpts) is in scope. Audit trails are easy to overlook
precisely because they're written by a fire-and-forget side path, but they hold
some of the most sensitive text in the system (prompt excerpts).

HOW IT WORKS
- deleteUserAccountData: adds `audit_events.delete().eq("user_id", userId)` to
  the batched deletion set, alongside workflows/projects/etc. Keyed by user_id so
  it removes exactly the departing user's rows.
- buildUserAccountExport: adds an `audit_events` section (the user's own rows,
  ordered by created_at) so the export mirrors what deletion removes.
- Test: the account-deletion fixture gains audit rows for two users and asserts
  only the other user's row survives (a1/a2 purged, a-other kept).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
484dadcf fix(audit): harden the /audit route - CSV escaping, export limiter/MFA, input bounds Amal 2026-08-01 ↗ GitHub
commit body
WHY THIS MATTERS
The history route had four independent weaknesses: CSV export could smuggle a
formula into a victim's spreadsheet, /audit/export lacked the export limiter and
MFA gate that every other data export has, an out-of-range ?page= crashed with a
500, and a malformed from/to date crashed with a 500. Titles are attacker-
controllable across shared projects, so these are reachable by another user.

WHAT IS CSV FORMULA INJECTION
Excel/Google Sheets evaluate any cell whose text begins with =, +, -, @, a tab
or a carriage return as a *formula* when the file is opened. A chat titled
=HYPERLINK("http://evil","invoice") therefore executes on export in the
victim's spreadsheet - data exfiltration / phishing with no macro prompt. The
fix (OWASP's recommendation) prefixes a single quote to any value starting with
a trigger char, forcing the value to be treated as literal text. The quote-
trigger regex also gains \r so a leading carriage return is both escaped and
quoted.

HOW IT WORKS
- csvCell (F3): prefixes ' when the value matches /^[=+\-@\t\r]/, and the
  CSV-quote test now includes \r.
- Export limiter + MFA (F5): app.ts adds app.get("/audit/export",
  exportLimiter) (10/hr) and the route gains requireMfaIfEnrolled, matching
  /user/export. A 2000-row export that can include other users' emails no longer
  runs under only the general limiter and plain auth.
- Page clamp (F7): parseQuery clamps page into [1, 100000]. Previously
  ?page=99999999999999 produced a ~5e15 OFFSET that PostgREST rejected as a 500;
  the clamp keeps the offset inside Postgres' integer range.
- Date validation (F8): from/to must match ^\d{4}-\d{2}-\d{2}$ (they come from
  <input type="date">). parseQuery now returns a discriminated result and the
  handlers reply 400 on bad input instead of building "...ZT23:59:59.999Z" and
  500ing.

Tests pin csvCell escaping for every trigger char, page clamping/flooring, date
rejection/acceptance, and (with queryEvents/accessibleProjectIds exported)
visibility scoping: own-events OR accessible-project events for owned+shared
projects, own-only when none, and owned/shared id de-duplication.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
97478819 fix(audit): mine doc_replicated copies and audit project document uploads Amal 2026-08-01 ↗ GitHub
commit body
WHY THIS MATTERS
The history feature under-recorded reality in two ways. Replicated documents
were attributed to the wrong file and lost their id, and project document
uploads were never recorded at all - so the audit trail silently disagreed with
what the user actually did.

WHAT IS EVENT MINING
recordChatTurn derives one audit row per artifact a chat turn produced by
walking the persisted assistant-event stream (doc_created, doc_edited,
doc_replicated, workflow_applied). Each event type stores its payload
differently, so the miner has to read each shape correctly.

HOW IT WORKS
- doc_replicated (F6): per chat/streaming.ts, this event's top-level `filename`
  is the SOURCE document and there is no top-level document_id - the produced
  copies live in a `copies: [{ new_filename, document_id, version_id }]` array,
  and one event can produce several. The miner previously read the top-level
  fields, so it logged one row titled with the source filename and a null id.
  It now iterates `copies`, emitting one document.generated row per copy with
  that copy's new_filename and document_id.
- Project uploads (F6): POST /projects/:id/documents calls the project-scoped
  handleDocumentUpload in projects.ts, a duplicate of the instrumented one in
  documents.ts, and it had no recordAudit call - so project uploads never
  appeared in history. It now records a document.uploaded event
  (surface: "project", the project id, the new document id) on success,
  fire-and-forget like the sibling handler.

Tests exercise the miner directly: a mixed turn (created/edited/workflow) maps
to the right actions, a doc_replicated with two copies yields exactly two
document.generated rows carrying the copies' filenames/ids (and never the source
filename), and an empty-copies event yields only the chat.message row.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3cba69ae fix(history): add an error state and cancel out-of-order loads on the History page Amal 2026-08-01 ↗ GitHub
commit body
WHY THIS MATTERS
Two UX/correctness bugs. When the fetch failed the catch cleared the list, so a
backend outage rendered the "No history yet" empty state - telling the user they
have no history when the truth is the request failed. And rapid filter changes
raced: because responses can arrive out of order, a slow earlier request could
land after a faster later one and overwrite it with stale rows, and a "Load
more" issued mid-filter-change appended a page from the old filter (wrong rows
and duplicate React key={e.id} warnings).

WHAT IS AN OUT-OF-ORDER RESPONSE RACE
The browser does not guarantee that fetches resolve in the order they were
started. If you fire request A (filter=all) then request B (filter=chat), B may
resolve first and A second, leaving the UI showing "all" results under a "chat"
filter. The standard fix is an AbortController: each new load aborts the
previous in-flight request, and the handler ignores any response whose signal
was aborted so a superseded request can neither overwrite fresher state nor
surface a false error.

HOW IT WORKS
- getAuditHistory (mikeApi) gains an optional AbortSignal, threaded into the
  underlying fetch via RequestInit.
- HistoryTable keeps the live controller in a ref. Each load() aborts the
  previous controller, starts a new one, and passes its signal down. Responses
  are dropped when controller.signal.aborted; the aborted-request rejection
  (AbortError) is swallowed rather than treated as an error. The effect's
  cleanup aborts on unmount.
- A dedicated error state renders "Couldn't load your history - Try again"
  (with a retry button) instead of the misleading empty state; the genuine
  empty state only shows when there was no error.
- Nit (F12): drops a leaked private-fork "(Clue custom)" comment from
  HistorySkeuoIcon.

Frontend tsc is clean and the existing suite passes; the History page has no
pre-existing component test to extend.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e12b5cf9 Merge remote-tracking branch 'upstream/main' into upstream-pr/audit-history JJ 2026-08-04 ↗ GitHub
# Conflicts:
#	backend/src/app.ts
f80722a7 test: mutation testing (security libs) + SSE load harness, on-demand Amalanand Muthukumaran 2026-07-25 ↗ GitHub
commit body
Two on-demand depth tools, neither a merge gate:

- Stryker mutation testing scoped to the security-critical backend libs
  (access.ts, downloadTokens.ts, safeError.ts, chat/citations.ts).
  Measured 74.0-76.4% mutation score across runs; thresholds.break=69
  fails only on real regressions. `npm run test:mutation` locally (~3
  min), .github/workflows/mutation.yml on demand + monthly cron, HTML
  report uploaded as artifact.

- k6 load harness for the SSE chat stream (loadtest/sse-stream.js):
  ramps to N concurrent POST /chat streams, checks TTFB and that every
  stream delivers events through to the [DONE] sentinel - the past
  incident class (streams timing out on long tool calls). Lenient,
  documented thresholds. .github/workflows/loadtest.yml is
  workflow_dispatch-only and boots nothing: point it at a staging
  stack (PR #210).

docs/test-depth.md explains how to read the mutation report, how to run
the k6 harness against the local stack, and why neither gates merges.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
64ddcf4c docs: replace dead PR #210 staging-stack references with self-contained target guidance Amal 2026-08-05 ↗ GitHub
commit body
WHY THIS MATTERS
The load-test harness (workflow, doc, and k6 script) all told operators
to point it at "the staging stack from PR #210". PR #210 was CLOSED
unmerged - it was rejected for leaking admin credentials - so the one
piece of infrastructure the harness claims to require does not exist.
A newcomer following the docs would hit a dead end: the reference reads
as "there is a blessed staging stack somewhere", when in reality there
is no such stack and never will be from that PR. Worse, linking to a
credential-leaking PR as recommended reading is itself a small security
smell. Docs that point at dead artifacts erode trust in all the other
docs around them.

WHAT IS A SELF-CONTAINED REFERENCE
Documentation can either point at an artifact ("use the stack from PR
#210") or describe a contract ("use any stack that serves X behind Y").
Pointers are fragile: PRs get closed, branches get deleted, staging
environments get torn down, and the doc silently rots. A contract-style
reference survives all of that because it tells the operator what the
target must PROVIDE rather than where one specific instance lived. For
an on-demand load harness - which by design boots nothing itself - the
contract is the only stable thing to document.

HOW THE FIX WORKS
Every "#210" reference in the three harness files is replaced with the
actual contract the target stack must satisfy:

  - a deployed, NON-production backend the operator owns,
  - serving the backend API's streaming endpoint:
      POST {BASE_URL}/chat
      Authorization: Bearer <supabase access token>
  - with real LLM provider keys configured (each iteration performs a
    real chat completion).

Concretely:
  - .github/workflows/loadtest.yml: the header comment and the
    `target_url` input description now describe that contract, e.g.
      description: "Backend base URL of a non-production stack you
      deployed (serving the backend API with auth), ..."
  - docs/test-depth.md: the "Running from GitHub Actions" section
    describes the same contract instead of naming PR #210.
  - loadtest/sse-stream.js: the header comment does likewise.

No behavior changes - the workflow inputs, k6 scenario, thresholds, and
safety warnings ("never point at production") are untouched. Verified by
grepping the three files for "#210" (no matches), YAML-parsing the
workflow, and `node --check` on the k6 script.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2288419a ci: run the Supabase RLS/stack integration suite on every PR Amalanand Muthukumaran 2026-07-25 ↗ GitHub
commit body
The gated stack tests (backend/src/__tests__/integration/*.supabase.test.ts)
prove the deny-all RLS firewall and the auth<->API contract against a real
local Supabase stack, but no CI trigger ever set the SUPABASE_TEST_* env
vars, so they silently self-skipped on every PR. Add a workflow that boots
the stack on the runner (supabase CLI pinned, minimal service set: db, auth,
rest, kong), bootstraps the database the way backend/scripts/test-stack.sh
does, and runs the suite. No secrets: everything is local to the runner.

The bootstrap loads schema.sql and then applies every dated migration on
top in filename order, which doubles as a schema-drift smoke test: the
snapshot and the migrations must apply cleanly together. Running it
surfaced five migrations that could not apply on top of the current
schema.sql - three whose backfills read documents columns that later
migrations moved to document_versions, and two overview RPCs whose return
row type `create or replace` cannot change. Guard the backfills on the
historical columns' existence and drop-before-create the RPCs (the pattern
20260703_02 already uses); behavior on era deployments is unchanged, and
the full sequence now applies cleanly end to end (verified locally: fresh
stack, schema + 44 migrations, 5/5 stack tests green).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ab716822 ci: bootstrap the stack from schema.sql only; restore historical migrations Amal 2026-08-05 ↗ GitHub
commit body
Addresses willchen96's review on PR #256.

WHY THIS MATTERS

The previous revision of this workflow built the CI database as:

    empty database -> current schema.sql -> every dated migration

and called it a "schema-drift smoke test". But that sequence is not a real
installation path. Per README.md, this repo has two deliberately separate
database artifacts:

  - backend/schema.sql      -> the COMPLETE shape for FRESH databases
  - backend/migrations/     -> incremental steps that move OLDER, already-
                               deployed databases forward from the version
                               they are on

A fresh install runs schema.sql and stops. An existing deployment runs only
the migrations dated after its version. Nobody ever replays the full
migration history on top of the current snapshot - so when five old
migrations "failed" under that replay, they were not broken; the harness
was. They had run correctly on the era-appropriate schemas they were
written for.

WHAT IS MIGRATION IMMUTABILITY

Once a migration has shipped and real deployments have executed it, the
file becomes a historical record of "the change that was required at that
point in time". Editing it afterwards cannot help any database that already
ran it - it only makes the repo's history diverge from what production
actually executed, which is exactly the record you need intact when
debugging a deployment later. (Tools like Flyway enforce this with
checksums: a modified applied migration is a hard error.) The previous
revision rewrote five historical migrations to satisfy the artificial
replay; worse, the column-existence guards it added made those backfills
silently no-op on unexpected schemas - converting the loud failure a drift
check exists to produce into a silent skip.

HOW THIS COMMIT FIXES IT

1. The five historical migrations are restored byte-for-byte to their
   state on main (20260424_01, 20260427_01, 20260602_01, 20260613_02,
   20260613_05).
2. The workflow's bootstrap step now loads schema.sql only - the same
   thing backend/scripts/test-stack.sh does locally and the same thing the
   README documents for a fresh deployment. The suite therefore tests the
   real contract: "a fresh Mike database enforces deny-all RLS and the
   auth<->API contract."

Real drift protection (does baseline-plus-migrations equal the current
snapshot?) needs a pinned baseline dump from an older release, the
migrations dated after it, and a pg_dump schema diff against a
schema.sql-built database. That is a separate change, designed separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4e136af6 ci: prove fresh installs and upgraded deployments converge on one schema Amal 2026-08-05 ↗ GitHub
commit body
The follow-up promised in PR #256's review thread: a real drift check,
designed the way willchen96 described - a pinned baseline from an older
point in history, only the migrations added since, and a schema comparison
against a fresh install. It also fixes the first drift it caught.

WHY THIS MATTERS

This repo maintains the database shape twice, on purpose: schema.sql is
what a FRESH install runs; backend/migrations/ is what an EXISTING
deployment applies to move forward. They are edited by hand, in parallel,
and nothing forces them to agree. When they disagree, the two classes of
real deployment silently diverge - and no ordinary test notices, because
test databases are always built fresh.

This is not hypothetical. Commit b2dbb39 ("narrow service role schema
grants") tightened service_role from GRANT ALL to select/insert/update/
delete - in schema.sql only, with no migration. Every deployment created
before 2026-07-23 and upgraded by the book still lets service_role
TRUNCATE any table, create TRIGGERs, and reset sequences. Fresh installs
do not. Same codebase, two different security postures.

HOW THE CHECK WORKS

The new "Schema drift" workflow builds both REAL installation paths in one
disposable Supabase stack and demands they converge:

  upgraded:  schema.sql as of a pinned baseline commit (9a1277b, the
             commit that introduced dated migrations) + only the
             migrations git says were ADDED since (git diff
             --diff-filter=A) - the documented upgrade path, exactly
  fresh:     today's schema.sql

Each build is reduced to a canonical fingerprint
(backend/scripts/schema-fingerprint.sql): tables, columns, constraints,
indexes, RLS policies, function definitions, triggers, views, enums, and
exploded per-privilege ACLs, every section totally ordered.

WHY A FINGERPRINT INSTEAD OF DIFFING pg_dump

Two reasons, both lessons from PR #256:

1. Column order. Migrations append columns; schema.sql may declare them
   anywhere. Upgraded and fresh databases therefore differ in physical
   column order forever - a benign difference a raw pg_dump diff would
   flag on every table. The fingerprint sorts columns by name: benign
   difference ignored, every real difference kept.
2. Grant order. An ACL array is ordered by GRANT execution order, which
   legitimately differs between the two paths. Exploding to one row per
   (object, grantee, privilege) and sorting compares the meaning, not the
   history.

And unlike the replay-history approach reverted in PR #256, this never
runs an old migration against a schema from its future, and it can never
be "fixed" by editing shipped migrations - the failure message explicitly
forbids that.

THE ACCOMPANYING MIGRATION

20260805_01_narrow_service_role_grants.sql is the forward fix for the
b2dbb39 drift: it revokes service_role's excess table/sequence privileges
so upgraded deployments land on the same least-privilege grants a fresh
install gets. Note the difference from what PR #256 reverted: adding a NEW
dated migration to move deployments forward is exactly what migrations are
for; editing already-shipped ones is what they must never suffer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
128464e1 fix: cast "char" catalog columns to text in the schema fingerprint Amal 2026-08-05 ↗ GitHub
commit body
First CI run of the drift check confirmed the whole upgrade path works -
baseline schema plus all twelve since-added migrations applied cleanly -
and then died inside the fingerprint itself:

    ERROR:  operator is not unique: text || "char"

WHAT IS "char" (WITH QUOTES)

Postgres catalog columns like pg_class.relkind, pg_attribute.attidentity,
and pg_default_acl.defaclobjtype use the internal single-byte type "char"
(quoted) - a different type from char(1). Concatenating text with "char"
is ambiguous: the parser finds more than one usable || operator via
implicit casts and refuses to guess. An explicit ::text cast picks one.

Only the "char" columns needed casts; booleans and reals concatenate fine
(anynonarray || text is unique for them).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2f667486 fix: fold workflow_open_source_submissions into schema.sql Amal 2026-08-05 ↗ GitHub
commit body
The drift check's first complete run found exactly the class of bug it was
built for - and it is a real one affecting every fresh install today.

WHAT THE CHECK FOUND

Migration 20260629_01 created workflow_open_source_submissions, and commit
a5fe6d6 (2026-07-04) merged it - but the table was never folded into
backend/schema.sql (git log -S confirms it has never appeared there). The
two installation paths therefore diverge:

  upgraded deployment:  has the table (migration applied)
  fresh install:        does not - schema.sql never creates it

This is not dead weight: backend/src/routes/workflows.ts,
lib/userDataCleanup.ts, and lib/userDataExport.ts all query the table. On
a fresh install, submitting a workflow to the open-source queue, exporting
user data, or cleaning up a user hits "relation does not exist".

THE FIX

Add the table to schema.sql verbatim from the migration - same columns,
check constraints, the three indexes, RLS enabled - placed with the other
workflow tables, plus its row in the deny-all revoke block. The file's
closing "grant select, insert, update, delete on all tables" already
covers service_role, matching what the migration's era grants produce on
the upgraded path, so the ACL fingerprints converge too.

With this fold-in, the fingerprint diff between "baseline + migrations
since" and "current schema.sql" should be empty - the check's green state.

WHY THIS DIRECTION AND NOT A MIGRATION

The failure message offers two legitimate fixes: fold a missing change
into schema.sql, or ship a new dated migration. Here the migration already
exists and upgraded deployments are correct; it is the snapshot that is
missing the change. So the snapshot gets the fix - the mirror image of the
service_role grants drift fixed in the previous commit, where schema.sql
was right and deployments needed a new migration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
317c5559 fix: apply the service_role grant migration in one transaction Amal 2026-08-05 ↗ GitHub
commit body
WHY THIS MATTERS

This migration REVOKEs every privilege service_role holds on the
application's tables and sequences, then GRANTs back the narrower set the
backend actually needs. On a live deployment those two steps were not
atomic: CI (and the documented upgrade path) apply migrations with

    psql --set ON_ERROR_STOP=1 --file <migration>

and plain psql, without the -1/--single-transaction flag, runs each SQL
statement in its own autocommitted transaction. That means there was a
real moment - after `revoke all ... from service_role` committed and
before the following `grant select, insert, update, delete` committed -
when service_role had ZERO privileges on every table. Any backend query
racing through that window fails with "permission denied", i.e. a brief
production outage caused by a security-hardening migration.

WHAT IS AUTOCOMMIT VS. AN EXPLICIT TRANSACTION

PostgreSQL always runs statements inside transactions. If you do not open
one yourself, each statement gets its own ("autocommit"), and its effects
become visible to every other session the instant it completes:

    revoke all privileges on all tables ... ;  -- visible immediately!
    -- <-- other sessions now see service_role with no privileges
    grant select, insert, update, delete ... ; -- visible only now

Wrapping the statements in `begin; ... commit;` changes when other
sessions see the effects: nothing is visible until COMMIT, and then
everything is visible at once. DDL and privilege changes are fully
transactional in PostgreSQL (unlike some other databases), so this is a
supported and standard pattern:

    begin;
    revoke all privileges on all tables ... ;
    grant select, insert, update, delete ... ;
    commit;

Concurrent queries either see the old grants (before commit) or the final
narrowed grants (after commit) - never the empty in-between state.

HOW THE FIX WORKS

The migration file now opens with `begin;` and ends with `commit;`. The
revoke+regrant pairs for tables and for sequences all sit inside that one
transaction, so applying the file with plain psql is atomic. A bonus:
with ON_ERROR_STOP=1, a failure partway through now rolls the whole file
back instead of leaving service_role stripped of privileges with no
re-grant applied.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
66877127 fix: use the sequence ACL default for sequences in the fingerprint Amal 2026-08-05 ↗ GitHub
commit body
WHY THIS MATTERS

The schema-drift fingerprint explodes each relation's ACL into one line
per (object, grantee, privilege) so CI can diff a fresh install against
an upgraded deployment. When a relation has never had an explicit GRANT
or REVOKE, its pg_class.relacl is NULL - PostgreSQL stores nothing and
applies built-in defaults - so the script synthesizes the effective ACL
with acldefault(). The relation query admits sequences (relkind 'S'),
but it called acldefault('r', ...) - the TABLE default - for every row.
A sequence with a NULL relacl would therefore be fingerprinted as
holding INSERT, DELETE, TRUNCATE, REFERENCES and TRIGGER: privileges a
sequence cannot hold at all. Today no sequence in the schema has a NULL
relacl, so the check passes either way - but the first time one does,
the failure diff would show phantom table privileges on a sequence,
sending whoever debugs the drift down a false trail.

WHAT IS acldefault()

acldefault(objtype, ownerid) answers "what ACL does PostgreSQL treat a
NULL acl column as meaning for this kind of object?". The first argument
is a one-character object-type code, and each code maps to a different
default privilege set:

    acldefault('r', owner)  -- 'r'elation: SELECT, INSERT, UPDATE,
                            -- DELETE, TRUNCATE, REFERENCES, TRIGGER
    acldefault('s', owner)  -- 's'equence: USAGE, SELECT, UPDATE
    acldefault('f', owner)  -- 'f'unction: EXECUTE (incl. PUBLIC)

Note the code is about the object KIND, not pg_class.relkind: relkind
'S' (a sequence row in pg_class) corresponds to acldefault kind 's'.
Passing 'r' for a sequence does not error - it just fabricates a
table-shaped ACL the sequence could never actually have.

HOW THE FIX WORKS

The lateral aclexplode now picks the acldefault kind per row:

    aclexplode(coalesce(c.relacl,
        acldefault((case when c.relkind = 'S' then 's' else 'r' end)::"char",
                   c.relowner)))

Sequences get the sequence default (USAGE, SELECT, UPDATE); tables,
partitioned tables, views and materialized views - everything else the
WHERE clause admits - keep the relation default. The explicit ::"char"
cast matches acldefault's parameter type (the internal one-byte "char",
not char(1)). Verified against a live Postgres 15: a fresh sequence with
NULL relacl now fingerprints as exactly SELECT|UPDATE|USAGE, and the
full drift check (baseline schema + migrations vs. current schema.sql)
still reports identical fingerprints.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2d442b15 Merge branch 'main' into olp-pr/schema-drift-check Will Chen 2026-08-06 ↗ GitHub
d3f41718 feat: refine workspace history experience willchen96 2026-08-13 ↗ GitHub
bebbc713 Merge main into upstream-pr/audit-history willchen96 2026-08-13 ↗ GitHub
4ddcd5a7 fix: clear history CI and security checks willchen96 2026-08-13 ↗ GitHub
53cb54e8 Merge pull request #271 from jmooves/upstream-pr/audit-history Will Chen 2026-08-13 ↗ GitHub
feature: add workspace history of actions (History page + audit_events)
9df42324 Merge pull request #254 from amal66/olp-pr/test-depth-stretch Will Chen 2026-08-13 ↗ GitHub
[Testing 21] test depth: mutation testing + SSE load harness (on-demand)
1af92310 Merge pull request #293 from amal66/olp-pr/schema-drift-check Amalanand Muthukumaran 2026-08-12 ↗ GitHub
[Testing 17] ci: schema-drift check - fresh install vs upgraded deployment must converge

Capture this PR into my fork

Download a Markdown prompt that tells Claude how to port every commit in this PR into your working tree. Run it via claude -p < capture-pull-26.md from inside the repo you want the changes in.

⬇ Download capture-pull-26.md