Skip to content

Commit 3c6ddc2

Browse files
committed
fix(core): union the first membership read into the org-members completion
The completion read's rows replaced Leg 1's orgMembersLeg, so a completion that saw fewer members than the first read shortened org_user_ids below what the first read alone answered. Union them; both reads are filtered to the active organization, so no non-member can be added. Claude-Session: https://claude.ai/code/session_01JQwxbZDZLmVprAaL7DLh1K Co-authored-by: Claude <noreply@anthropic.com>
1 parent 399df5b commit 3c6ddc2

3 files changed

Lines changed: 43 additions & 3 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/core': patch
3+
---
4+
5+
fix(core): the fellow-org peer list keeps every member the first membership read returned, even when the completion read sees fewer
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
In an organization of 1,000 members or more, the resolver reads the membership twice: a first read bounded at 1,000 rows, then a completion read in id order. The completion's rows used to replace the first read's, so a completion that saw fewer members than the first read (an older snapshot on another connection) left `org_user_ids` shorter than the first read alone would have, and those members vanished from ordinary members' `sys_user` reads with no signal. The two reads are now unioned. Both are filtered to the active organization, so the union cannot add anyone outside it. No read is added or changed.

‎packages/core/src/security/resolve-authz-context.org-members-past-first-read.test.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ const PAGE_ROWS = 10000;
3535

3636
interface Read { object: string; where: Record<string, unknown>; limit?: number; orderBy?: unknown }
3737

38-
function makeQl(tables: Record<string, any[]>, opts: { ignoreCursor?: boolean; failAfter?: number } = {}) {
38+
function makeQl(
39+
tables: Record<string, any[]>,
40+
opts: { ignoreCursor?: boolean; failAfter?: number; staleForOrderedReads?: ReadonlySet<string> } = {},
41+
) {
3942
const reads: Read[] = [];
4043
const holds = (row: any, where: Record<string, unknown>) =>
4144
Object.entries(where).every(([k, v]) => {
@@ -53,8 +56,11 @@ function makeQl(tables: Record<string, any[]>, opts: { ignoreCursor?: boolean; f
5356
if (opts.failAfter !== undefined && object === 'sys_member' && reads.filter((r) => r.object === 'sys_member').length > opts.failAfter) {
5457
throw new Error('connection reset');
5558
}
56-
let rows = (tables[object] ?? []).filter((r) => holds(r, q?.where ?? {}));
5759
const order = Array.isArray(q?.orderBy) ? q.orderBy[0] : undefined;
60+
// An ordered read (the completion's) answering from an older snapshot: the
61+
// members named here had not joined yet as far as it can see.
62+
const stale = order && object === 'sys_member' ? opts.staleForOrderedReads : undefined;
63+
let rows = (tables[object] ?? []).filter((r) => holds(r, q?.where ?? {}) && !stale?.has(r.user_id));
5864
if (order) {
5965
rows = [...rows].sort((a, b) => (a[order.field] < b[order.field] ? -1 : a[order.field] > b[order.field] ? 1 : 0));
6066
if (order.order === 'desc') rows.reverse();
@@ -163,6 +169,23 @@ describe('[#29281] org_user_ids holds the whole active organization', () => {
163169
expect(lines[0]).toContain('org_user_ids');
164170
});
165171

172+
it('a completion that sees fewer members than the first read still yields every member the first read returned', async () => {
173+
vi.spyOn(console, 'warn').mockImplementation(() => undefined);
174+
const tables = tablesOf(FIRST_READ_BOUND + 5);
175+
// Five members the unordered first read returns (it answers in insertion
176+
// order) that the completion's older snapshot does not hold.
177+
const unseen = new Set(['u000001', 'u000002', 'u000003', 'u000004', 'u000005']);
178+
const ql = makeQl(tables, { staleForOrderedReads: unseen });
179+
const grants = await resolveUserAuthzGrants(ql, ME, { tenantId: ORG });
180+
181+
// The completion did run, and did come back short of them.
182+
expect(orgReads(ql.reads)).toHaveLength(2);
183+
for (const u of unseen) expect(grants.org_user_ids).toContain(u);
184+
expect([...grants.org_user_ids].sort()).toEqual(membersOf(tables));
185+
expect(grants.org_user_ids).not.toContain('u_elsewhere_1');
186+
expect(grants.org_user_ids).not.toContain('u_elsewhere_2');
187+
});
188+
166189
it('a page that fails to read fails the resolution loudly — never a shorter list', async () => {
167190
vi.spyOn(console, 'warn').mockImplementation(() => undefined);
168191
const tables = tablesOf(FIRST_READ_BOUND + 5);

‎packages/core/src/security/resolve-authz-context.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1922,9 +1922,16 @@ export async function resolveUserAuthzGrants(
19221922
// membership only when it came back under its bound; at the bound it is a
19231923
// first page and the membership is read to completion here. Under the
19241924
// bound nothing more is read.
1925+
//
1926+
// The completion's rows are UNIONED with Leg 1's, never put in their
1927+
// place: a completion that comes back with fewer members than Leg 1
1928+
// already held (a snapshot older than Leg 1's on another connection) would
1929+
// otherwise shorten the list below what the first read alone answered.
1930+
// The union cannot add a non-member — both reads are filtered to the
1931+
// active organization — so the list is never shorter than Leg 1's.
19251932
if (tenantId) {
19261933
const orgMembers = orgMembersLeg.length >= ORG_MEMBERS_FIRST_READ_ROWS
1927-
? await readOrganizationMembersToCompletion(ql, tenantId)
1934+
? [...orgMembersLeg, ...await readOrganizationMembersToCompletion(ql, tenantId)]
19281935
: orgMembersLeg; // (read in Leg 1 above)
19291936
const ids = new Set<string>(
19301937
orgMembers

0 commit comments

Comments
 (0)