[Orgs] Multi-tenant organizations with a project role ladder (owner/manager/editor/viewer) - #267
Open
amal66 wants to merge 21 commits into
Open
[Orgs] Multi-tenant organizations with a project role ladder (owner/manager/editor/viewer)#267amal66 wants to merge 21 commits into
amal66 wants to merge 21 commits into
Conversation
amal66
force-pushed
the
olp-pr/organizations-rbac
branch
from
July 27, 2026 00:03
64c75cc to
2f0bf7d
Compare
amal66
force-pushed
the
olp-pr/organizations-rbac
branch
from
August 3, 2026 01:57
2f0bf7d to
4d587b9
Compare
4 tasks
amal66
marked this pull request as draft
August 6, 2026 02:37
amal66
force-pushed
the
olp-pr/organizations-rbac
branch
from
August 6, 2026 03:40
8fec9c1 to
e3f465d
Compare
amal66
force-pushed
the
olp-pr/organizations-rbac
branch
from
August 16, 2026 20:56
e3f465d to
cdf6706
Compare
Collaborator
Author
Rebase round, 2026-08-16Rebased onto current
Rebase adaptations folded into the original commits, per the unmerged-branch convention:
Verification
|
amal66
force-pushed
the
olp-pr/organizations-rbac
branch
from
August 18, 2026 13:19
7f19ccb to
305db48
Compare
|
|
Collaborator
Author
Live smoke evidence — 2026-08-18 rebase (backend checks)Recorded live runs against a full local stack after the rebase; the complete flow-by-flow evidence (GIFs, screenshots, videos) is posted on the stacked UI PR: see the evidence comment on #268. Backend-relevant results:
Also re-verified on the rebased tip: 709/709 backend tests, including the 27 supabase-gated stack tests (auth contract + RLS deny-all sweep) against a live Supabase stack. |
amal66
changed the title
Introduce an organizations tenant layer on top of the existing per-user model: every account gets an auto-provisioned personal org, orgs carry owner/admin/member RBAC via org_members, and teams group members inside an org. projects/documents/workflows/tabular_reviews gain a nullable org_id (ON DELETE SET NULL) so org membership becomes a third access branch alongside row ownership and shared_with emails — in the access helpers, the overview RPCs, and the org-aware /orgs REST module. Mechanical port of the organizations/RBAC feature from amal66/mike@main (b3166dd) onto the upstream layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC
…nches Give the three access branches a Drive-style role ladder instead of raw ok/isOwner flags: row owner → owner, shared_with email → editor, org owner/admin → manager, plain org member → viewer. A single capability matrix (lib/permissions.ts) maps roles to what routes may do — view, content.edit, docs.organize, structure.manage, members.manage, container.delete — and every project/document/review write route now declares the capability it needs instead of hand-rolling an owner check. This makes the ADR's 'org membership grants visibility, not ownership' promise real: plain org members are read-only (previously the org branch returned ok:true and most write routes gated on nothing beyond ok), and org owner/admins can curate content (manage folders, sharing, review structure) without being able to delete containers they don't own. Notable tightenings, all fail-closed: - folder rename/move/delete, doc-set/column edits on reviews, and clear-cells are manager+ (generalising the owner-only folder-delete gate that landed upstream in open-legal-products#193) - version pushes, edit resolution, chat, and review generation are editor+ (org viewers excluded) - project PATCH (metadata + sharing) is manager+, so org admins can manage without owning; project/review DELETE stays owner-only - GET /projects/:id and /people now go through checkProjectAccess (the roster previously 404'd for org members who could read the project) Detail responses expose access_role alongside is_owner so the client can render per-role affordances. can() is exhaustively unit-tested (role × capability), and route suites cover the new gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Org viewers could append messages to a colleague's chat — and burn LLM
credits doing it — because the existing-chat path only asked "can you
see this chat?", never "can you write to it?".
WHY THIS MATTERS
The organizations feature gives every plain org member visibility into
their org's projects: they map to the "viewer" project role
(backend/src/lib/access.ts, orgRoleToProjectRole). Visibility is the
point — but the capability matrix (backend/src/lib/permissions.ts) is
explicit that chatting is a WRITE:
capability | min role | covers
content.edit | editor | upload documents, push versions, CHAT, ...
Creating a chat already honored that: validateAccessibleProjectId gates
new project chats on can(projectRole, "content.edit"). But POST /chat
with an existing chat_id, and POST /chat/:chatId/generate-title, gated
only on getAccessibleChat — which returned the chat whenever
checkProjectAccess(...).ok was true. That is true for org viewers. So a
viewer could:
- append user messages into a colleague's project chat,
- trigger LLM generation (spending the owner's configured API keys),
- overwrite the chat's title via generate-title (an UPDATE on chats).
WHAT IS THE ACCESS-VS-CAPABILITY DISTINCTION
"Can you see this resource?" and "can you write to this resource?" are
different questions and must be answered by different checks:
- an ACCESS check resolves whether the caller has any standing at all
(owner / shared editor / org member) — it yields a role;
- a CAPABILITY check asks whether that role clears the bar for the
specific operation: can(role, "content.edit").
A route that stops after the access check silently grants its weakest
role the powers of its strongest. That is exactly the bug class here:
read-only endpoints (GET /chat/:chatId) and write endpoints (POST
/chat) shared one gate, so the gate had to be as permissive as the
reads — and the writes inherited that permissiveness.
HOW THE FIX WORKS
getAccessibleChat now returns the caller's ProjectRole along with the
chat, instead of flattening everything to "found / not found":
type ChatAccess =
| { ok: true; chat: AccessibleChat; projectRole: ProjectRole }
| { ok: false };
The chat owner maps to "owner"; for project chats everyone else
inherits their project role from checkProjectAccess. Chats without a
project_id remain reachable only by their owner (unchanged).
Each caller then declares the capability it needs:
- GET /chat/:chatId — read: any resolved role (project.view
semantics), so org viewers keep read access. Unchanged behavior.
- POST /chat with chat_id — write: rejects with 403 unless
can(projectRole, "content.edit"), mirroring the new-chat path.
- POST /chat/:chatId/generate-title — write (UPDATEs chats.title):
same 403 gate.
403 (not 404) is correct for viewers: they are allowed to know the chat
exists — they can read it — they just cannot modify it.
Note: routes/projectChat.ts does NOT have this hole — it already gates
the entire POST on content.edit before touching any chat row.
Tests (chat.routes.test.ts) add a table-aware supabase stub so the
caller's org role can be varied, and prove: viewer POST to an existing
chat → 403 with no LLM call; viewer generate-title → 403; chat owner
and org admin (manager) still stream successfully; admin generate-title
still works; viewer GET of the same chat still returns 200.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With two or more owners in an org, an admin could demote an owner to
member (or remove them outright), because the membership mutations
checked the ACTOR's rank but never compared it to the TARGET's.
WHY THIS MATTERS
The role ladder is owner > admin > member. updateMember and
removeMember guarded three things:
- the actor can manage members at all (roleCanManage → owner/admin),
- "only an owner may GRANT the owner role" (no self-escalation),
- last-owner protection (never demote/remove the sole owner).
None of those look at who the target IS. So in an org with owners A and
B plus admin C, C could call updateMember(target=A, role="member"):
C passes roleCanManage, "member" is not "owner" so the grant check does
not fire, and B still counts as an owner so last-owner passes. Result:
an admin deposes an owner — and with both owners demoted one at a time,
an admin becomes the effective top of the org without ever holding the
owner role. That inverts the hierarchy the ladder is supposed to
encode.
WHAT IS AN ACTOR/TARGET RANK CHECK
Role-ladder authorization has two halves, and they are easy to conflate:
1. Does the ACTOR's role permit this KIND of operation?
("admins may manage members")
2. Does the ACTOR outrank-or-equal the TARGET of the operation?
("...but not members who outrank them")
Check (1) alone is enough for rank-neutral operations (creating a
team). Any operation aimed at another member also needs check (2),
otherwise every manager-tier role can act on the tier above it. The
existing "only an owner may grant owner" rule is the escalation half of
this idea; what was missing is the demotion half: only an owner may act
AGAINST an owner.
HOW THE FIX WORKS
Both mutations now fetch the target's role (they already did, for
last-owner counting) and add a rank guard before it:
if (targetRole === "owner" && actorRole !== "owner")
return { ok: false, kind: "forbidden" };
- updateMember: an admin demoting an owner → forbidden, regardless of
how many owners exist. Owner-on-owner changes still work.
- removeMember: an admin removing an owner → forbidden. An owner
leaving on their own is unaffected: self-leave implies
actorRole === targetRole === "owner", so the guard passes and the
existing last-owner protection still has the final say.
Since owner/admin/member is a three-rung ladder, this single condition
IS the full outrank-or-equal rule: owners outrank everyone, admins may
act on admins/members, and plain members cannot reach these functions
at all (roleCanManage already rejects them).
Tests (orgs.test.ts) seed a two-owner org so last-owner protection is
provably not what stops the attack: admin-demotes-owner → forbidden,
admin-removes-owner → forbidden, owner-demotes-owner still ok,
owner-self-leave with a second owner still ok, and the existing
last-owner cases stay green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adding an org or team member by email used the Supabase admin API's
listUsers({ perPage: 1000 }) and scanned the page in JavaScript. Any
user beyond the first 1000 accounts silently resolved to null, so the
route answered "No user with that email" for a user who exists.
WHY THIS MATTERS
List-and-scan lookups against an admin API are a scalability trap that
looks fine in every dev and demo environment:
- CORRECTNESS decays with growth. listUsers is paginated; a single
perPage: 1000 call is only "everyone" while the instance has fewer
than 1000 accounts. The day it crosses that line, membership adds
start failing for exactly the newest users — with a misleading 404
and no error anywhere in the logs.
- COST grows linearly. Even while it still works, resolving ONE email
means transferring up to 1000 user records and comparing each in
process, on every add-member call — O(n) network and CPU for what
an indexed table answers in O(log n).
- The admin auth API is a management surface, not a query engine. It
has no "find by email" filter here, which is the hint that
lookup-by-attribute belongs on a queryable table with an index.
The comment above the helper claimed it "mirrors the lookup pattern in
routes/projects.ts /people" — but that route actually uses the
user_profiles-based helpers in lib/userLookup.ts, not the admin API.
The codebase already had the right tool; this route just didn't use it.
WHAT THE INDEXED LOOKUP IS
lib/userLookup.ts maintains lookups over the user_profiles table, which
stores each user's normalized (lowercased, trimmed) email and is kept
in sync on auth events (syncProfileEmail). findProfileUserByEmail is a
single indexed query:
const { data } = await db
.from("user_profiles")
.select("user_id, email, display_name")
.eq("email", normalized)
.maybeSingle();
One row travels over the wire regardless of whether the instance has
100 users or 10 million, and "not found" is an honest answer instead of
an artifact of pagination.
HOW THE FIX WORKS
routes/orgs.ts's resolveUserIdByEmail now delegates to that helper:
const user = await findProfileUserByEmail(db, email);
return user?.id ?? null;
The contract is unchanged — null when no such user — so both callers
(POST /orgs/:orgId/members and POST /orgs/:orgId/teams/:teamId/members)
keep their existing 404 behavior for genuinely unknown emails, and the
stale comment is corrected. This was also the last listUsers scan in
the backend. Email normalization (trim + lowercase) lives inside the
helper, matching the semantics the old loop implemented by hand.
findProfileUserByEmail is covered by unit tests
(lib/__tests__/userLookup.test.ts: found via normalization, not-found,
and blank-input cases); the org routes have no route-level test file,
so the two-line delegation rides on that existing coverage.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…API actually calls
WHY THIS MATTERS
PR migration 20260717_03 added the new org-membership visibility branch
("you can see rows tagged with an org you belong to") to
get_tabular_reviews_overview — but only to its legacy 3-argument
overload. The API never calls that overload. GET /tabular-review builds
nine named arguments (p_user_id, p_user_email, p_project_id, p_scope,
p_limit, p_offset, p_search_term, p_sort_key, p_sort_direction) in
lib/tabularReviewsOverview.ts, so PostgREST resolves the 9-argument
overload from 20260726_01 — which never got the org branch. Result: on
any install upgraded via migrations, org-shared reviews were readable
through the detail endpoints (access.ts allows them) yet invisible in
every list view. Fresh installs, which bootstrap from schema.sql, were
unaffected because schema.sql's 9-argument version already carries the
org branch — so fresh and migrated databases silently diverged, the
worst kind of bug: nothing errors, some deployments just show less data.
WHAT IS FUNCTION OVERLOADING IN POSTGRES
Postgres identifies a function by name AND argument types.
get_tabular_reviews_overview(text, text, text) and
get_tabular_reviews_overview(text, text, text, text, integer, integer,
text, text, text) are two completely independent functions that happen
to share a name. CREATE OR REPLACE matches on the full signature, so
replacing one overload never touches the other:
create or replace function f(a text) ... -- replaces f(text)
create or replace function f(a text, b int) ... -- separate function!
PostgREST resolves an RPC call to the overload whose named parameters
match the JSON body it received. A call with nine named keys can only
ever hit the 9-argument overload. That is why patching the 3-argument
overload in 20260717_03 was a no-op for the API.
HOW THE FIX WORKS
This migration re-declares both overloads exactly as backend/schema.sql
already defines them (the bodies are copied verbatim, not rewritten):
1. The 9-argument overload gains the two org-membership arms:
- accessible_projects: projects whose org_id is in an org the caller
belongs to (EXISTS against org_members) — so in-project reviews of
org colleagues become visible;
- visible_reviews: org-tagged reviews owned by someone else become
visible in the global list (p_project_id is null).
2. The 3-argument overload becomes the thin wrapper schema.sql uses —
it simply delegates to the 9-argument version with scope 'all' and an
effectively unlimited page — replacing 20260717_03's divergent
full-body copy. From now on there is a single source of truth for the
visibility predicate, and this file sorts after every earlier
overview migration, so replaying migrations from scratch also ends in
the correct state (previously 20260726_01 sorted after 20260717_03
and wiped the org branch on replay).
Signatures do not change, so CREATE OR REPLACE suffices — no DROP
FUNCTION, and the migration is safe to re-run. schema.sql needs no
change: it was already correct; this converges migrated installs onto
it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WHY THIS MATTERS
get_tabular_review_ids_overview backs GET /tabular-review/ids, the bulk
"select all matching" action: it returns id + owner for every review the
caller can see, so the client can select the whole filtered set without
fetching full payloads. For performance it does NOT delegate to
get_tabular_reviews_overview — it carries its own copy of the visibility
predicate, and its own migration (20260727_01) warns in its header:
"If the access/visibility rules in get_tabular_reviews_overview's
visible_reviews CTE ever change, mirror the change here too."
The organizations work changed exactly those rules — it added a third
visibility branch, "rows tagged with an org the caller belongs to",
alongside "row owner" and "shared_with email" — but this RPC was never
mirrored, neither in a migration nor in schema.sql. The user-visible
symptom is nasty because it is partial: an org member SEES a colleague's
org-shared reviews in the list, but "select all matching" silently
skips them, so bulk actions run over fewer rows than the visible
selection implies. Nothing errors; rows just go missing.
HOW THE FIX WORKS
Add the same two org-membership arms the paginated overview uses
(compare its 9-argument definition in schema.sql):
1. accessible_projects gains an EXISTS against org_members on
p.org_id — reviews living in an org colleague's project become
visible, exactly like email-shared projects already were:
or (
p.org_id is not null
and p.user_id <> p_user_id
and exists (
select 1 from public.org_members m
where m.org_id = p.org_id and m.user_id::text = p_user_id
)
)
2. The row filter gains the matching arm for org-tagged reviews in the
global list (p_project_id is null), keyed on tr.org_id.
The change is made in BOTH places that define this function — edited
in place in backend/schema.sql (fresh installs bootstrap from it) and
as new migration 20260805_02 (upgrades existing installs) — with
byte-identical function bodies, so the two install paths cannot
diverge. org_members.user_id is a uuid FK to auth.users while content
tables store user_id as text, hence the ::text casts. Org membership
grants visibility only; user_id still identifies the owner.
The signature is unchanged, so the migration is CREATE OR REPLACE only
and safe to re-run.
TESTS
The org-visibility integration suite (tabularPagination.supabase.test.ts)
now seeds two real auth users in one org — the FK to auth.users means
random UUIDs won't do — makes one a plain "member", and asserts the
member sees the colleague's in-project and standalone org reviews via
the ids RPC, plus a lockstep assertion that the ids RPC returns exactly
the set the paginated overview shows, which is the drift this bug class
is about.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…actor
WHY THIS MATTERS
The manager-gate test for POST /tabular-review/:reviewId/clear-cells was
written when the endpoint keyed cell resets on document_ids. After
rebasing onto main it hit the 400 validation branch ("row_ids is
required") before ever reaching the 403 role check it exists to prove,
so the RBAC gate went untested.
WHAT IS THE LOGICAL-ROWS REFACTOR
Main's folder-grouped tabular review work (open-legal-products#274) reshaped reviews around
logical review rows: a row can represent a folder of documents, not just
one document, so cell-level operations now address rows. clear-cells
accordingly takes row_ids instead of document_ids.
HOW THE FIX WORKS
The test now sends { row_ids: ["row-1"] }, a valid payload under the
current API, so the request passes validation and exercises the intended
assertion: an org editor without structure.manage receives 403 "Only a
review manager can clear cells". No production code changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dels Rebasing onto current main surfaced a new instance of the overload trap this branch already fixed once for tabular reviews (20260816_04/05): since the org feature was written, main grew paginated overloads for the projects and workflows overviews (20260807_01), bulk select-all ids RPCs and a sidebar summaries RPC (20260812_01), and a TS-side directory search. Every one of them re-implements the visibility predicate, and every one of them predates the org concept — so an org member would see org content in the detail endpoints and legacy lists but not in the paginated lists, the sidebar, select-all, or the document picker. SQL (migration 20260816_06 + schema.sql in lockstep, byte-identical bodies): get_projects_overview (10-arg), get_project_ids_overview, get_project_summaries gain the same org-membership EXISTS arm the legacy overloads got in 20260816_03; get_workflows_overview (12-arg) and get_workflow_ids_overview gain the org_shared CTE from the 3-arg version, with scope 'shared' including org-shared rows. TS: handleProjectDirectorySearch (GET /projects?view=directory-search) adds the org branch to its inline access derivation via listUserOrgIds, mirroring listAccessibleProjectIds. GET /projects/:id/directory and /export already route through checkProjectAccess and inherit the org branch for free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e on docs and reviews ensureDocAccess and ensureReviewAccess checked the row's own org membership before falling through to the project check — but the project check is what carries the shared_with editor branch. A colleague who is BOTH a plain member of your org AND explicitly shared on your project (the normal in-firm sharing case) matched the org branch first and came out a viewer: every content.edit route (upload versions, rename, chat, accept/reject edits, generate) suddenly 403'd for someone main happily allowed. checkProjectAccess and the file's own precedence note already had it right: owner, then shared_with, then org. Reorder both helpers to match — project fall-through (share-aware) first, the row's own org branch after — while keeping the org branch's upgrade power: an org owner/admin is a manager even where the project branch only found viewer standing, and an org-tagged standalone row (no project) still resolves through its own org_id. Roles can now only strengthen as branches accumulate, never weaken. Also aligns ensureReviewAccess's email comparison with the trim() normalization the other helpers got in the sharing-email-case fix. Regression tests: the overlap case pins editor on both helpers, and an org admin pins manager. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ment used PgArray syntax
GET /projects?view=directory-search builds its shared-projects query
with .contains("shared_with", [email]). supabase-js serializes a JS
array there as PostgREST's PgArray form ({email}), which Postgres
rejects for a jsonb column — "invalid input syntax for type json" —
so the endpoint returned 500 for any authenticated caller (the email
branch always runs). Pre-existing on main, but this branch adds the org
visibility arm to exactly this handler, so the document picker must
actually work to deliver it. Use the JSON containment idiom the sibling
listAccessibleProjectIds already uses:
.filter("shared_with", "cs", JSON.stringify([email])).
Verified live: owner and org-member searches both 500'd before, both
return the project after.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… lists they filter 20260816_06 ported the org-membership arm onto the second-generation paginated/ids/summary read-models, but the 20260812_01 batch it covered also shipped two dropdown-option companions that were missed: get_project_filter_options (the Practice and Owner dropdowns above the projects table) and get_workflow_filter_options (practice / language / jurisdiction above the workflows list). Both derive their option sets from a "visible rows" predicate that still only knew the owner and email-share arms. The symptom is a visible seam for org members: the list shows an org project, but its owner never appears in the Owner filter and its practice never appears in the Practice filter — rows you can see but cannot filter to. Filters and lists must be computed from the same visible set or the UI contradicts itself. The arms are copied verbatim from the 20260816_06 predicates. In the workflows RPC the org CTE keeps the workflow_shares NOT EXISTS dedup and is tagged source='shared', matching the overview's bucketing decision (shared-with-me and shared-via-my-org are one "shared" bucket from the caller's point of view), so p_scope filtering stays consistent between a list and its dropdowns. schema.sql carries the same bodies for fresh installs. Verified live: an org member's Owner dropdown now offers the org project's owner, and an org-shared workflow's practice appears under scope=all and scope=shared but not scope=owned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ant can never demote The previous commit in this area (0096f38) fixed one direction of branch shadowing — an org "viewer" verdict no longer buries a shared_with editor — but the mirror direction survived: the shared_with branch was still an unconditional early return, so it shadowed STRONGER verdicts. Two concrete downgrades: * checkProjectAccess: an org owner/admin (manager tier) whose email someone also put in the project's shared_with resolved as editor — adding a grant stripped their folder-rename/delete, metadata, and sharing rights. * ensureReviewAccess: the review's own shared_with early-returned editor before the project was even consulted, so the PROJECT OWNER, politely added to a review's share list by a collaborator, lost owner-tier standing on that review (403 on title/columns edits). Both are privilege LOSS, not escalation — but they violate the invariant the role ladder promises: grants accumulate, they never subtract. That invariant is safe here precisely because shared_with entries are bare email strings with no per-entry role: there is no way to express a deliberately weaker share that strongest-wins could override. The fix replaces ordered early returns with an explicit merge: every matching branch derives its ProjectRole and the strongest one wins (strongerRole in lib/permissions.ts). ensureDocAccess/ensureReviewAccess skip the extra org lookup when the project verdict already folded in the same org's membership, so the common paths cost the same queries as before. This also retires the old code's dead upgrade ternary — "upgrade-only" is now a property of the merge, not of branch ordering. Regression tests pin the two downgrades above, plus the one fixture where the doc's own org actually decides the outcome (doc org differs from the project's org and only the former makes the caller a manager). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…containment misuses cdf6706 fixed the directory-search 500 caused by calling supabase-js .contains() on a jsonb column (it serializes a JS array as PostgREST's PgArray form {email}, which Postgres rejects for jsonb). A sweep for survivors of the same pattern found two, both with a worse failure mode than a 500 — silent data loss: * routes/audit.ts accessibleProjectIds used .contains on projects.shared_with, and the caller swallows query errors (shared.data ?? []), so GET /audit and GET /audit/export quietly omitted every event from projects shared with the caller — for every caller, on an audit surface whose whole point is completeness. * lib/userDataExport.ts passed the raw login email into the tabular_reviews containment while the projects query two lines up normalized it; shared_with entries are stored lowercased, so a mixed-case account's data export silently missed reviews shared with them. Both now use the JSON containment idiom the rest of the codebase settled on: .filter("shared_with", "cs", JSON.stringify([email])) with a trimmed, lowercased email. The audit test mock deliberately no longer implements .contains — a regression back to PgArray serialization fails the suite loudly instead of passing against a too-forgiving fake — and a new case pins the exact lowercased jsonb literal that goes over the wire. Verified live: an event in a project shared with the caller now appears in their /audit feed (it did not before the fix), and a stranger still sees nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every user gets a personal org at signup, and resolveContentOrgId tags every org-less row they create with it — that is what keeps all content tenant-scoped without demanding an explicit org on every write. The quiet consequence: personal-org membership IS visibility of the owner's entire private library, because all the org visibility arms (lists, detail access, RPCs) key off org_members. addMember had no personal guard. The org owner could POST a colleague into their own personal org — plausibly thinking of it as "my workspace, let me add my assistant" — and thereby share every private project, document, chat, review and workflow they had ever created, with no per-item consent and no UI hint of the blast radius. Explicit sharing (shared_with, real orgs) is the intended channel for that. addMember now refuses with a 400 validation error when the target org is personal. Teams need no equivalent guard: team membership requires org membership first, so a single-user org cannot grow a multi-user team. Verified live: POST /orgs/:personalOrgId/members returns 400; the 57-check RBAC matrix (real orgs) is unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Last-owner protection lived only in lib/orgs.ts as a read-then-act check: count the owners, then demote/remove if more than one remain. Two concurrent requests each removing a DIFFERENT owner both count two, both pass, and the org ends up with zero owners — a terminal state, because granting the owner role itself requires an owner, so no API call can ever repair it. Races between requests cannot be fixed in the application layer without a serialization point, so the invariant moves into the database: org_members_protect_last_owner (BEFORE DELETE OR UPDATE OF role) first locks the organizations row FOR UPDATE — concurrent owner departures on the same org serialize on that lock, and the loser re-counts after the winner committed, sees one owner left, and aborts with errcode 23514. The service layer maps that onto the same 409 the sequential check produces, so callers can't tell which layer stopped them. The trigger deliberately stands aside for the two legitimate cascades: org deletion (the organizations row is already gone inside the transaction, so the lock probe finds nothing) and direct auth-user deletion (the member's auth.users row is gone; security definer, like handle_new_user, so that probe works for any calling role). Account-deletion cleanup is reordered to match — and it's a fix in its own right: it used to DELETE the sole owner's membership first and promote an heir after, a window where the org exists ownerless (and which the trigger would now reject). It now promotes the heir before the membership goes, or deletes a memberless org outright and lets the cascade clean up. Verified live: direct SQL demote/delete of a sole owner both fail with "must keep at least one owner"; the API path still returns its 409. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…map to 409 Three service-layer gaps in the same file: * removeMember deleted only the org_members row. team_members has no FK to org membership (its cascades are team deletion and auth-user deletion), and the org-membership requirement is enforced only at addTeamMember time — so a removed member kept appearing on every one of that org's team rosters, silently violating the "teams group existing members" invariant. Removal now also deletes the target's team_members rows across that org's teams (and only that org's: their seats elsewhere are untouched, pinned by test). * The existence pre-checks in addMember / addTeamMember / createTeam race with concurrent inserts. The unique constraints backstop correctness, but the raw 23505 surfaced as a 500 db_error; it now maps onto the same 409 conflict the sequential path returns. * When the new last-owner trigger (20260816_08) fires under a race, its 23514 arrived as a 500; updateMember / removeMember now translate it into the last_owner result → the standard 409 with the standard message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The signup trigger swallows every exception by design (a provisioning hiccup must never block signup) and the backfill migration ran exactly once. A user who slipped through both — trigger failed after the backfill's moment in time — had no personal org FOREVER: getPersonalOrgId returned null, every piece of content they created landed with org_id null, and GET /orgs returned an empty list, with no code path anywhere that would repair it. The same state reappears when account deletion clears a user's data but the final auth-delete fails. getPersonalOrgId now self-heals: when the lookup comes back empty it creates the org (named after the profile email, like the trigger) plus the owner membership. The partial unique index on (created_by) where personal makes the lazy create race-safe — the loser of a concurrent create re-reads the winner's row. A failed membership insert degrades softly: content tagging and owner-arm visibility never depended on membership, only GET /orgs does, and the next call cannot re-create a duplicate. Verified live: delete a user's personal org, create a workflow — the workflow lands in a freshly healed personal org and GET /orgs shows it again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…accounts POST /orgs/:orgId/members and the team-member variant resolved the target email BEFORE checking the caller's standing in the org, and the two failure responses are distinguishable 404s: unknown email → "No user with that email", known email but caller not a manager → "Organization not found". Any authenticated user could therefore probe arbitrary emails for account existence against any orgId — an oracle, independent of whether the add could ever succeed. The routes now prove the caller holds a manage role in the org first and only then touch the email, so non-managers always get the same organization-not-found response regardless of what they probed with. The service layer re-checks the role afterwards; this commit only fixes the ordering of disclosure. Verified live: an outsider probing a real org with a nonexistent email gets "Organization not found", not email feedback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tail access 20260814_01 lowercased projects.shared_with but left tabular_reviews alone. Review shares have the same jsonb email-list shape and the same split consumers: the TypeScript detail check (ensureReviewAccess) lowercases both sides, but the list/ids RPCs compare the STORED entry against the caller's already-lowercased email via jsonb containment. A pre-normalization mixed-case entry therefore produced a ghost review — openable from a direct link, invisible in every list. New writes have been normalized for a while (routes/tabular.ts), so this is a one-pass data repair using the exact lateral-unnest recipe of 20260814_01: trim, lowercase, dedupe, preserve first-seen order, and touch only rows that actually change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rges The schema-drift check builds a real upgraded deployment (baseline schema.sql + migrations) and a real fresh install (current schema.sql) and diffs their fingerprints — and a function's stored body includes its comments verbatim, so even a rewrapped comment line reads as two deployments disagreeing about a function. Three such seams slipped into the last round: the two filter-options org-arm comments referenced the sibling migration number in the migration copy but not in schema.sql, and the last-owner trigger carried two inline comments in the migration that schema.sql's copy lacked. This aligns the text byte-for-byte (migration 20260816_07 adopts the schema.sql wording; schema.sql adopts the trigger's inline comments). Editing 20260816_07/08 is safe: they are part of this unmerged PR, not shipped history. Verified by replicating the CI check locally — scratch postgres, baseline 9a1277b + added migrations vs current schema.sql, fingerprints identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amal66
force-pushed
the
olp-pr/organizations-rbac
branch
from
August 21, 2026 17:28
305db48 to
4aced37
Compare
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
… has
WHY THIS MATTERS
The org/RBAC migration (20260816_01) added org_id to projects, documents,
workflows and tabular_reviews — and skipped chats. That left chats the only
content family still on the pre-RBAC model: no way to share a standalone
chat at all, and no column for the access helpers or the overview RPC to
derive a role from. It also left a real authorization drift: the chat list
RPC took no email parameter, so a collaborator on a shared project could
open a colleague's chat by URL (the detail route checks the project's
shared_with) while the same chat never appeared in their GET /chat list.
List and detail disagreeing about what exists is exactly the failure mode
the overview-RPC lockstep convention (20260816_03's header) exists to
prevent.
WHAT IS THE SHARED PERMISSION SCHEMA
Every shareable resource carries the same three anchors:
user_id — the hard ON DELETE CASCADE owner anchor (unchanged);
shared_with — a per-row email list, so standalone rows can be shared;
org_id — a nullable FK with ON DELETE SET NULL, so dropping an org
never orphan-deletes a user's rows.
From those, lib/access.ts derives one of four roles (owner / manager /
editor / viewer) and routes gate on a capability matrix instead of ad-hoc
ownership checks.
HOW IT WORKS
- 20260820_01 adds chats.org_id + chats.shared_with (+ index) and
backfills org_id with creation-time semantics: a chat inside a project
inherits the project's org; everything else lands in the owner's
PERSONAL org. A personal org has exactly one member, so this stamping is
the privacy decision in data form — a standalone chat grants nothing to
anyone until it is explicitly shared, and org visibility only ever flows
through project membership.
- 20260820_02 drops and recreates get_chats_overview with p_user_email
and an is_owner output column. The predicate now mirrors
ensureChatAccess branch for branch: chat owner, chat shared_with, chat
org membership, or accessible project (owner / shared_with / org). Drop
rather than overload: the function has exactly one caller, and a
leftover 3-arg overload would ambiguate PostgREST's named-argument
resolution.
- schema.sql is updated byte-identically so fresh installs and upgraded
deployments converge (schema-drift check replicated locally: NO DRIFT).
Part of the chat-permission-parity follow-up to open-legal-products#267.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
WHY THIS MATTERS Authorization bugs are rarely in the policy — they are in the third, slightly different copy of the policy. Chats had exactly that: a private getAccessibleChat helper inside routes/chat.ts that re-implemented part of the access logic (owner, else delegate to the project) and silently lacked the rest (no direct-share branch, no org branch, no strongest-wins merge). Two code paths answering "who can touch this row?" differently is how a collaborator ends up editor on one endpoint and 404 on another. WHAT IS STRONGEST-WINS MERGING A caller can hold several grants at once: org member (viewer tier) AND in the row's shared_with (editor tier). Branches must merge by taking the strongest role, never the first match — otherwise adding someone to a share list can DEMOTE them (the org branch shadowing the share branch was a real bug caught in open-legal-products#267's post-rebase review). The merge lives in strongerRole(); every derivation must route through it. HOW IT WORKS ensureReviewAccess's body — owner short-circuit, direct shared_with → editor, project verdict via checkProjectAccess, own-org membership → manager/viewer, all merged strongest-wins — was already the general algorithm: nothing in it is review-specific. It is extracted verbatim into a private ensureSharedResourceAccess, and both exported functions become three-line delegates: ensureReviewAccess(review, ...) -> ensureSharedResourceAccess(review, ...) ensureChatAccess(chat, ...) -> ensureSharedResourceAccess(chat, ...) Reviews and chats now cannot drift apart: there is one derivation to test, one to read, and one for the overview RPCs to mirror. ensureChatAccess's own-org branch is kept for structural symmetry even though standalone chats are stamped with a single-member personal org (20260820_01), which makes it a practical no-op — the privacy decision lives in the stamping policy, not in a special-cased code path. Part of the chat-permission-parity follow-up to open-legal-products#267. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
… add sharing
WHY THIS MATTERS
Before this commit, three chat endpoints made their own law. PATCH (rename)
and DELETE filtered on `.eq("user_id", userId)` — a raw ownership check
that bypassed the role ladder entirely, so an org admin could rewrite a
chat's title through POST /chat/:id/generate-title (gated content.edit)
but got 404 from PATCH /chat/:id on the very same column. DELETE returned
204 even when it deleted nothing, so a forbidden delete looked like a
successful one. And GET /chat used a list predicate that disagreed with
the detail route, hiding chats the caller could open by URL.
WHAT IS THE DECLARED-CAPABILITY IDIOM
Routes never compare roles or re-derive rights from an isOwner flag; they
declare the capability they need and ask can(role, capability)
(lib/permissions.ts). The whole policy is one table, so tightening or
widening a tier is a one-line diff there — not an audit of every route.
HOW IT WORKS
- getAccessibleChat is now a thin wrapper over ensureChatAccess: it loads
the row and returns {chat, isOwner, projectRole}. Reads gate on the
verdict itself (the same convention project/review reads use).
- GET /chat/:chatId returns access_role + is_owner, mirroring the project
and review detail responses, so clients render per-role affordances
instead of guessing from one boolean.
- PATCH /chat/:chatId now takes {title?, shared_with?}: title needs
content.edit (the tier that already writes titles via generate-title),
shared_with needs members.manage (mirroring the project share PATCH:
emails lowercased + deduped, self-share and unknown users rejected).
Chats — including standalone ones — are shareable for the first time.
- DELETE /chat/:chatId needs container.delete: still the top of the
ladder, but derived — the chat owner, or the owner of the project it
lives in (who could already delete the whole project). Everyone else
gets an honest 403/404 instead of a silent no-op 204.
- GET /chat passes the caller's email to get_chats_overview, whose
predicate now mirrors ensureChatAccess — the list and the detail
endpoint can no longer disagree about what exists.
- GET /chat/:chatId/people exposes the owner + shared_with roster, the
same shape as the project/review people endpoints.
- Both chat-create paths (and projectChat's) stamp org_id via
resolveContentOrgId: project chats inherit the project's org, standalone
chats land in the caller's single-member personal org — private until
explicitly shared.
Part of the chat-permission-parity follow-up to open-legal-products#267.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
…th to the access check
WHY THIS MATTERS
Two authorization holes in the review-chat sub-resource, found while
auditing chat permissions for parity:
1. DELETE and PATCH /tabular-review/:reviewId/chats/:chatId never looked
at reviewId at all — no check that the review exists, that the caller
can access it, or that the chat even belongs to it. Any chat id could
be renamed or deleted through any (or a nonexistent) review path. The
.eq("user_id") filter limited the blast radius to the caller's own
chats, but a URL path that lies about the resource hierarchy is a bug
class in itself (IDOR-shaped: object references must be validated
against their claimed parent, not trusted).
2. Two read routes selected the review row WITHOUT shared_with, which
silently disabled ensureReviewAccess's direct-share branch — the
branch can only match against a column that was actually fetched. A
review-level collaborator could open the review itself (that route
selects *) but got 404 on its chat list and chat messages.
HOW IT WORKS
- ensureReviewChatWriteAccess centralizes the write preamble: load the
review (WITH shared_with), run ensureReviewAccess, then verify
chat.review_id === reviewId. Both writes call it, and their final
UPDATE/DELETE also filters on review_id so the row touched is provably
the one the URL named. Writes stay chat-owner-only — review chats are
personal threads within a shared review, not shared containers.
- The two starved selects gain shared_with, un-breaking direct-share
collaborators on review-chat reads.
Part of the chat-permission-parity follow-up to open-legal-products#267.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amal66
added a commit
to amal66/mike
that referenced
this pull request
Aug 21, 2026
WHY THIS MATTERS A capability matrix is only as trustworthy as the tests that pin it. open-legal-products#267 established the pattern — every role x capability cell asserted, plus regression tests for the overlap-precedence bug its own review caught. This extends the same rigor to the resource family that just joined the schema, so a future refactor that quietly re-adds a user_id filter or drops a merge branch fails a named test instead of shipping. WHAT IS BEING PINNED - ensureChatAccess derivation (lib/__tests__/access.test.ts): owner branch; case-insensitive direct-share -> editor; project inheritance (project owner -> owner, org admin -> manager, org member -> viewer); chat-own-org branch; the overlap-precedence regressions (org member + direct share must be editor, never viewer; project owner + direct share must stay owner); cross-tenant denial; fail-closed with no email. - Chat route gates (integration/chat.routes.test.ts): rename allowed for managers, 403 for viewers; share-list writes manager-only, with the normalization (lowercase/trim/dedupe), self-share and unknown-user rejections asserted on the exact payload persisted; delete 204 for the owner, 403 for an org admin, 404 for a stranger — and the recorded update/delete filters are asserted to be [{id}] alone, which is the machine-checkable form of "the raw user_id filter is gone, the matrix decides now". Also: access_role/is_owner on the detail response, the people roster, the RPC receiving p_user_email, and a standalone direct-share chat walking read/write/delete tiers. - Review-chat path binding (integration/tabular.routes.test.ts): rename and delete 404 when the review is missing, inaccessible, or the chat belongs to a different review; 204 for the owner when review and chat line up. Full backend suite: 712 passed | 27 skipped. Part of the chat-permission-parity follow-up to open-legal-products#267. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
Live verification round — 2026-08-21 (tip
|
| # | Case | Result |
|---|---|---|
| 1 | Org member sees org projects in lists; detail access_role:"viewer" |
200 ✓ |
| 2 | Outsider: empty lists, detail 404, no existence leak | ✓ |
| 3 | Viewer rename/delete denied | ✓ |
| 4 | Org admin ⇒ manager: rename works, container delete still denied |
✓ |
| 5 | Overlap strongest-wins: share upgrades viewer→editor, admin stays manager | ✓ |
| 6 | Last-owner leave → 409, self-demote → 409, org intact | ✓ |
| 7 | Admin cannot demote/remove the owner → 403 | ✓ |
| 8 | Personal org member-add → 400 | ✓ |
| 9 | No email oracle: non-manager add → 403 before email resolution | ✓ |
| 10 | Directory search as member: org rows, no 5xx | ✓ |
| 11 | Full state restore verified | ✓ |
Recorded UI flows (shared with #268, GIFs on assets/orgs-smoke-2026-08-21):
Two style-level observations, recorded not fixed: project-write denials surface as 404 (PATCH /projects/:id → "Project not found") even for callers whose GET on the same id returns 200 — denial is correct, the masking is just self-inconsistent; and GET /projects (list) omits access_role/org_id, so the UI's Owner column renders org rows as the literal "Shared" with no owner identity. Both fine as follow-ups.
amal66
marked this pull request as ready for review
August 21, 2026 21:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Followed by #268
Design (ADR)
Context. The app has been strictly per-user since the baseline: every data table carries a
user_idanchor and access is "row owner OR email inshared_with". A law firm is not one user — firms need a tenant boundary, roles inside it, and content that is visible to colleagues without emailing a share for every row. The fork (amal66/mike) built this as a full feature on its main branch; this PR ports it onto the upstream layout — and gives it the permission model the first revision only promised (see "Permissions model" below).Decision. Introduce a tenant layer without disturbing the per-user anchor:
organizations— a tenant.personal = truemarks the auto-provisioned one-per-user org every account gets (signup trigger + backfill), so single-user usage is completely unchanged: content simply lands in the caller's personal org.org_members—(org_id, user_id, role)withrole in ('owner','admin','member'). This is the RBAC edge: owner/admin manage the org, its members and teams (only an owner can grant the owner role); member gets read access to org content. Last-owner protection prevents demoting/removing the sole owner.teams/team_members— structural intra-org grouping (membership + naming); finer team-scoped permissions are a deliberate future extension point.org_idonprojects/documents/workflows/tabular_reviewsas a nullable FK with ON DELETE SET NULL —user_idremains the hard CASCADE anchor, so account deletion works exactly as before and dropping an org never orphan-deletes user rows.shared_withemail, (3) org membership — but each branch now derives a project role (owner/manager/editor/viewer), and routes gate on a single capability matrix instead of ad-hocisOwnerchecks.isOwnerkeeps meaning "row owner";canManageis now derived from the matrix. The four overview RPCs gain the same third branch in SQL so list views and detail endpoints can never disagree.Consequences. Existing installs are migrated in place: the backfill gives every user a personal org and stamps their existing rows, so nothing becomes invisible when the org branch lands. Org membership grants visibility, not ownership — and unlike the first revision of this PR, that promise is now enforced, not just documented: plain org members are read-only, and destructive/structural operations require manager+ (see the matrix). Account deletion tears down the user's org footprint (personal org dropped; sole-ownership of shared orgs handed off to the earliest remaining member; empty orgs removed), and the GDPR export includes the user's orgs/teams/memberships.
Alternatives considered. (a) Overloading
shared_withwith group emails — no roles, no tenant boundary, O(members) writes per row. (b) Makingorg_idNOT NULL with CASCADE — breaks system workflows (nulluser_id) and turns org deletion into content deletion. (c) SSO/SCIM-first provisioning — intentionally out of scope;organizationsis shaped to growsso_config/scim_tokencolumns and anorg_invitationstable, and the role CHECK can gain roles without a table rewrite. (d) Per-folder/per-document ACL overrides (full Google Drive "My Drive" semantics) — rejected for now: permissions attach at the container root and inherit, like Drive shared drives and legal-matter workspaces; per-item overrides add a lot of model complexity for little demand at this scale.Permissions model
Every access branch resolves to one project role, and every route declares the capability it needs via
can(role, capability)(backend/src/lib/permissions.ts— the whole policy is one table, exhaustively unit-tested). This generalises #193 (owner-only folder delete) from a one-route fix into the policy itself; the same missing-gate class existed on ~ten other destructive/structural routes, all closed here.shared_with)The editor/manager line is the load-bearing one (Drive's writer vs. fileOrganizer): content collaboration stays broad, structural/destructive power is narrow. Deleting containers stays owner-only so org admins can curate without being able to erase a colleague's project.
Behaviour changes vs. upstream
main, all disclosed:shared_with) collaborators can no longer rename/move/delete folders (deletion was already owner-gated by Restrict project folder deletion to owners #193; rename/move had no gate), edit a review's title/column set/document set (removing a document deletes its cells; for title/document set this matches Require review owner for tabular settings edits #175 exactly, generalised to the org tier), or clear extracted cells. They keep full content collaboration (uploads, versions, chat, generation, doc rename/move).PATCH /projects/:iddrops itsuser_idfilter in favour of the manager gate). Container deletion is not widened.GET /projects/:id/peoplepreviously 404'd for org members who could read everything else about the project; the roster now followsproject.view.GET /projects/:idalso routes throughcheckProjectAccessinstead of a hand-rolled inline check.access_rolealongsideis_owner, so a client can render per-role affordances instead of re-deriving policy from one boolean.Summary
A firm is not one user. This PR adds multi-tenant organizations with owner/admin/member roles: every account gets a personal org automatically (so nothing changes for individuals), firms can create shared orgs, add colleagues by email, group them into teams, and everyone in the org can see the org's projects, documents, workflows and tabular reviews — with a four-tier project role ladder (owner/manager/editor/viewer) and a single capability matrix deciding who can change what.
Changes
backend/migrations/, upstream naming convention):20260717_01_organizations_rbac.sql— org/RBAC schema,org_idcolumns + indexes, signup-trigger extension, RLS + grant hardening.20260717_02_backfill_personal_orgs.sql— idempotent personal-org + membership +org_idbackfill for existing data.20260717_03_org_overview_rpcs.sql— org-membership branch added toget_workflows_overview,get_chats_overview,get_projects_overview,get_tabular_reviews_overview.backend/schema.sqlupdated to match (tables, columns, trigger, RPCs).backend/src/lib/permissions.ts—ProjectRole,Capability,can(); the role×capability policy as one data table.backend/src/lib/access.tsderivesprojectRoleon every branch ofcheckProjectAccess/ensureDocAccess/ensureReviewAccess(row owner → owner, shared email → editor, org owner/admin → manager, org member → viewer)./projects,/single-documents,/tabular-review, plus project chat and chat-in-project creation, now declares its needed capability. Read routes stay atproject.view.backend/src/routes/orgs.ts(thin handlers,{detail}error bodies) +backend/src/lib/orgs.ts(service layer enforcing the role model), mounted at/orgsinapp.ts. Endpoints: org CRUD/list, member add/update/remove (by email), team CRUD + team membership.org_idvalidated against membership, else personal org), document uploads/copies/project-assignment, tabular reviews (inherit project org), workflows (personal org). Access-check loads now selectorg_id.deleteUserOrganizations(personal-org teardown, sole-owner handoff) wired intodeleteAllUserData; orgs/teams/memberships added to the user data export.permissions.test.ts(the full role×capability matrix, cell by cell, plus fail-closed on unknown roles),access.test.ts(role derivation on all four branches, cross-tenant denial),orgs.test.ts(service RBAC),userDataCleanup.orgs.test.ts(org teardown/handoff), and route-level gate coverage in the existing integration suites (folder-delete tier walk, review clear-cells/columns gates).No frontend changes are required (everything is additive;
access_roleis new,is_ownerunchanged). Teaching the web UI to useaccess_roleinstead ofis_owneris a natural follow-up PR.Why
Multi-tenant RBAC is the difference between "a tool a lawyer uses" and "a tool a firm can adopt": tenant isolation is enforced in one shared code path (access.ts + RPCs in lockstep), roles come with escalation guards (admins cannot mint owners), and the personal-org design means zero migration burden for existing single users. The capability matrix keeps it honest: without it, adding a colleague to your org would silently grant them destructive power over every project in it — the exact bug class #193 just fixed for
shared_with, at tenant scale. No new runtime dependencies. No new always-on cost: the org branch only adds queries on the access paths that already hit the database.Testing
Rebased on current
main(post-#193/#175/#228–#238), so totals include the merged vitest harness and route suites:cd backend && npm ci && npx tsc --noEmit→ clean.cd backend && npx vitest run→ 307 passed | 5 skipped (23 files), including 30 permission-matrix cells, role-derivation tests on all four branches, and the new route-gate cases (folder-delete allowed for owner/manager, blocked for editor/viewer; clear-cells manager gate; columns manager gate)./healthresponds.Provenance
The schema, migrations, org module, cleanup/export wiring and tenant stamping are mechanical ports of amal66/mike@origin/main (b3166dd) — path moves (
apps/api/src/modules/orgs/*→backend/src/{routes,lib}/orgs.ts), import rewrites, and re-application of the fork's org hunks onto upstream's route files. Exceptions, all mechanical adaptations to upstream's conventions:YYYYMMDD_NN_name.sqlconvention and dated 20260717 so they sort after existing migrations (fork names:20260701000000/1/2_*). Comment cross-references to fork-only migrations adjusted.::textcasts in the backfill and RPC org clauses: upstream stores content-tableuser_idastextwhileorganizations.created_by/org_members.user_idareuuidFKs (the fork migrated its ids to uuid; upstream has not). RPC bodies otherwise reproduce upstream's current definitions plus the fork's org branches verbatim; the fork's unrelated drift (result caps /lower()email normalization from other fork migrations) was NOT carried in.handle_new_userextends upstream's current email-mirror version (20260703_01) with the fork's org-provisioning block (the fork's own merged version, verbatim).routes/*.tsinstead of the fork'smodules/*split; the fork'sfilenamecolumn on document inserts was not carried (upstream droppeddocuments.filenamein 20260602_04).lib/access.tsstarted from the fork's file (a direct descendant of the upstream file); one entangled fork hunk includes a defensiveuserEmail.toLowerCase()inlistAccessibleProjectIds(a no-op upstream —requireAuthalready lowercases).userDataCleanup.orgs.test.tsdrops the fork'svi.mock("../env")(fork-only zod env module; upstream reads process.env).permissions.ts, theprojectRolederivation, the route capability sweep and their tests are new code written for this PR after review discussion, closing the gap between the ADR's stated policy ("visibility, not ownership") and what the first revision actually enforced. The fork will adopt the same model.Credits & prior art
🤖 Generated with Claude Code
Reference: the fork-side ADR PR is amal66#38 (same branch, kept for provenance).