From 44675491341482c492a647be8a020d4fb546f6cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?=
 <66887028+NathanTarbert@users.noreply.github.com>
Date: Fri, 21 Aug 2026 13:29:47 -0400
Subject: [PATCH] fix: stop calling allowed_groups an access control (#82)

No code reads channels.allowed_groups. Channel access is membership: every
route resolves the caller in channel_memberships and refuses without a row,
and the column is not consulted on the way.

Both halves of the control are missing rather than one. users.groups is
written by nothing and read by nothing either, so there is no group membership
anywhere for a channel's list to be compared against. Enforcing the field today
would compare a declared list against an empty one for every person, which is
not an access control; making groups arrive from the identity provider is a
feature and a design call rather than a bug fix.

The columns stay. Neither is the wrong shape for the rule they are named for,
and allowed_groups is required in channels.yaml, so removing it would break
every existing tenant package. The docs and both columns now say what decides
channel access and what does not.
---
 docs/architecture.md         |  6 ++++++
 docs/configuration.md        | 12 ++++++++++++
 server/src/db/schema/core.ts | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/docs/architecture.md b/docs/architecture.md
index 9c108b30..2709a62d 100644
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -103,6 +103,12 @@ A coworker is a durable Bot profile:
 
 A channel is a conversation with one coworker and a CopilotKit Intelligence thread mapping. Starting a new channel creates a new thread.
 
+Who may reach one is decided by membership: every channel route resolves the caller in
+`channel_memberships` and refuses without a row. `channels.allowed_groups` is declared in the
+tenant package and stored, and is not part of that decision — `users.groups` is never populated by
+any sign-in path, so a group-based rule has nothing to evaluate. Treat it as a declaration waiting
+on group membership from the identity provider, not as a control that is running.
+
 See [coworkers.md](coworkers.md).
 
 ## Components
diff --git a/docs/configuration.md b/docs/configuration.md
index 4836f0d4..75b56e16 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -306,6 +306,18 @@ channels:
 
 Each channel requires `id`, `name`, `description`, `permitted_agents`, and `allowed_groups`. Every `permitted_agents` entry must match an agent id.
 
+`allowed_groups` is validated and stored, and nothing reads it. It decides nothing today, and a
+deployment that writes one must not treat it as an access control. Both halves of that control are
+missing, not one: `users.groups` exists as a column and no sign-in path, claim mapping or admin
+screen ever populates it, so there is nothing for a channel's list to be compared against. Channel
+access is decided by membership alone — every channel route resolves the caller's row in
+`channel_memberships` and refuses without it.
+
+Package-declared channels get no membership rows from `synchronizeTenantPackage`, so today they
+are unreachable rather than open. The field is kept because the enforcement it is named for needs
+the declaration and needs group membership arriving from the identity provider, and neither this
+column nor `users.groups` is the wrong shape for it.
+
 ### `model.yaml`
 
 ```yaml
diff --git a/server/src/db/schema/core.ts b/server/src/db/schema/core.ts
index 4dace4b9..87fe6a86 100644
--- a/server/src/db/schema/core.ts
+++ b/server/src/db/schema/core.ts
@@ -50,6 +50,14 @@ export const users = pgTable("users", {
   name: text("name"),
   image: text("image"),
   emailVerified: boolean("email_verified").notNull().default(false),
+  /**
+   * The person's groups, for a group-based rule to be evaluated against.
+   *
+   * Empty on every row: no sign-in path, claim mapping or admin screen writes this, and nothing
+   * reads it. It is the other half of `channels.allowedGroups`, and #82 is about the pair. Anything
+   * that starts deciding access on a group has to populate this first, or it decides on an empty
+   * list for everybody.
+   */
   groups: text("groups").array().notNull().default([]),
   createdAt: createdAt(),
   updatedAt: updatedAt(),
@@ -221,6 +229,16 @@ export const channels = pgTable(
     name: text("name").notNull(),
     description: text("description").notNull(),
     suggestedPrompts: text("suggested_prompts").array().notNull().default([]),
+    /**
+     * Which groups the tenant package says this channel is for.
+     *
+     * Written by `synchronizeTenantPackage` and read by nothing. Channel access is membership: every
+     * route resolves the caller in `channelMemberships` and refuses without a row, and this column is
+     * not consulted on the way. It is not currently a hole, because package channels get no
+     * membership rows either and so are unreachable rather than open, but it is a control the name
+     * promises and nothing keeps. See #82, and `users.groups`, which is the half that has to arrive
+     * from the identity provider before this one can decide anything.
+     */
     allowedGroups: text("allowed_groups").array().notNull().default([]),
     packageId: uuid("package_id").references(() => deploymentPackages.id, {
       onDelete: "set null",