fix: State what decides channel access, and what allowed_groups is waiting on

[NathanTarbert]

What this changes

Documents allowed_groups as what it is today: a declaration the tenant package carries, not a control that decides anything yet.

Fixes #82, and thanks to @andreolf for a really precise writeup — the "not currently exploitable, but it will be the moment membership is wired up" framing is exactly right, and it's what made this easy to pick up.

synchronizeTenantPackage writes channels.allowed_groups (server/src/tenant-package.ts:488,496), the column is declared (server/src/db/schema/core.ts:224), and a grep across every .ts and .tsx finds only tests. Channel access is membership: every route resolves the caller's row in channel_memberships and refuses without it.

One thing worth adding to the issue's analysis, because it decides which of the two suggested fixes to take: both halves of the control are waiting, not one. users.groups (server/src/db/schema/core.ts:53) is also written by nothing and read by nothing — no sign-in path, no claim mapping, no admin screen, no configuration. There's no group membership anywhere in the deployment for a channel's list to be compared against.

That makes enforcement the bigger piece rather than the quicker one. A check today would compare a declared list against an empty one for every person, which either denies everybody or means nothing depending on which way it's written. Getting group membership to arrive from the identity provider is the real feature underneath, and that's a design call worth having on its own.

So this takes the issue's second suggestion for now, and leaves the first one open:

• docs/configuration.md — the channels.yaml section says the field is validated and stored and read by nothing, points at users.groups as the other half, and notes that channel access is membership alone. Also records that package channels get no membership rows, so today they're unreachable rather than open.
• docs/architecture.md — the same note where "Coworkers and channels" describes what a channel is, since that's where you'd look to find out who can reach one.
• server/src/db/schema/core.ts — a comment on each of the two columns pointing at the other, so the next person to touch either half learns there that it's half of something.

The data stays. Unlike the state #21 took back, neither column is the wrong shape for the rule they're named for — a per-channel list of groups and a per-user list of groups are what a group rule needs, and what's missing is the identity-provider mapping in between. Removing them would also break every existing tenant package, since allowed_groups is required in channels.yaml.

Where it runs

• New state that outlives a request? None. No runtime code changes: documentation and two schema comments.
• What happens on the second replica? Identical behavior everywhere. Nothing changes on any of them.
• Anything serialised? N/A, no writes.
• Anything fanned out to a browser? No.
• New listener, port, or schedule? No.

Boundary and audit

• N/A. No gateway, policy, or audit path touched. The membership check that decides channel access is unchanged.

Changelog

No line — a deployment behaves the same afterwards. What changes is that the documentation stops implying otherwise.

Proof

No failing test to write first, because there's no behavior change to prove: two documents and two columns stop describing something the code doesn't do. What the claim rests on is a grep, and it reproduces:

• channels.allowed_groups / allowedGroups across every .ts and .tsx: writers at server/src/tenant-package.ts:302,488,496, the column at server/src/db/schema/core.ts:224, otherwise only server/tests/tenant-package.test.ts and server/tests/channel-routes.test.ts. No reader on any access path.
• users.groups: the column at server/src/db/schema/core.ts:53 and nothing else in server/src or app/src. No .md, .yaml or .json maps an identity-provider claim onto it either.

Ran on this branch:

• bun run format:check — clean, 363 files.
• bun run lint — 27 warnings, 1 info, identical to unmodified main.
• bun run typecheck — clean across app, server and worker.
• bun run build — clean.
• bun test — 780 pass, 5 skip, 79 fail, 864 tests across 91 files, exactly the unmodified main baseline on this machine. The 79 are the database integration tests; there's no local Postgres here and they fail the same way on a clean main, which is what the CI tests job runs pgvector for. No test added or removed.

On the red migrations check

Not from this branch — it's failing on main too, since #87, and every branch cut from main inherits it. meta/0005_snapshot.json has drifted from core.ts, so drizzle-kit generate writes a migration nobody asked for and the drift probe fails on the dirty tree. Filed as #91 with the details; the two comments added here produce no DDL.

[NathanTarbert]

Carrying one extra commit that is not mine to review: the snapshot regeneration from #93.

migrations was failing here for a reason that has nothing to do with this change — the drift landed on main in #87 and every branch cut from it inherits the red check. Stacking #93's commit underneath is what gets this one green so the actual diff can be looked at on its merits. It disappears the moment #93 lands.

If #93 changes shape in review, say so and I will rework this one to match rather than leave a stale copy of it sitting here.

Review the first commit, fix: State what decides channel access, and what allowed_groups is waiting on- #92 — that is the whole of this PR: two docs and two schema comments.

[davidmckayv]

Right call, and the analysis is what makes it the right call rather than the quick one.

The load-bearing find is that users.groups is also written by nothing and read by nothing, so both halves of the control are waiting rather than one. I checked it independently: allowed_groups has writers in tenant-package.ts and readers only in tests, and the groups that knowledge/acl.ts matches on is a caller-supplied array on KnowledgeActor, not that column. So enforcing today really would compare a declared list against an empty one for every person, and would either deny everybody or mean nothing depending on which way it was written.

Keeping the columns is right for the reason given: a per-channel list of groups and a per-user list of groups are the correct shape for the rule they are named for, and what is missing is the identity-provider mapping between them. That is a different thing from the state #21 took back, which was the wrong shape.

Noting that package channels get no membership rows, so they are unreachable rather than open, is the detail somebody reading #82 needs and it is not in the issue.

Rebased onto main and dropped the snapshot commit this carried, since #90 already landed that repair.

The real work now is getting group membership to arrive from the provider, which is its own design call as you say.