Skip to content

Commit 1878ef9

Browse files
fix(plugin-security,platform-objects): an org member is not served a colleague's identity Admin-group fields, directly or through activity (#21237) (#21340)
Fixes #21237 Clause-②: no **Files outside the original surface, named before the change list.** - **Cross-lane declaration (claim revision 1, triage Q1 = B):** one line in `packages/platform-objects/src/identity/sys-user.object.ts`, a `domain:engine` file. The identity object's deactivation flag moves from the `Admin` field group to the `Account` group, so it stays directory data that org peers are served. This is the only `platform-objects` edit. - **Claim surface revision 2 (option A of the dev report's open question):** the ruled behaviour made two existing tests false. Each keeps its intent: - `packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts`: the mixed-payload case of PIN 4 now uses a non-whitelisted column outside the group. It still measures the identity guard's strip. A group field is refused one layer earlier, by the field-level write gate. - `packages/qa/dogfood/test/admin-ledger-decision-metadata.dogfood.test.ts`: its masked and gated classes ride two `Admin`-group fields. The readers' own fixture sets now grant those two fields back as readable, the way an app does, so each class applies only through its own mechanism. ## What changes - **The non-admin sets withhold the group.** `member_default` and `viewer_readonly` declare the identity object's `Admin` field group `readable: false` (with `editable: false`). They use the permission set's existing `fields` mechanism, built from the identity object's declaration, with no hand-kept list. These are the two shipped non-admin sets that open an org peer's identity row through `sys_user_org_members`. - **The admin sets keep the group.** `admin_full_access` and `organization_admin` (and so the derived `organization_admin_no_bypass`) declare the group `readable: true, editable: true`, the same state as a field no set names. That entry is required: `member_default` is the additive `everyone` baseline that every authenticated human resolves, admins included, and field grants merge most-permissively. - **The #11965 neutrality pin.** Its literal in `default-permission-sets.test.ts` gains the admin set's keeping `fields` block, read off the declaration. One docblock line says why, in the shape #21260 used for its own addition. - **New pins.** - `plugin-security/src/identity-admin-field-group.test.ts` holds three: group equality against the declaration plus a completeness check over every shipped set that opens an org peer's identity row; the evaluator's real merge per persona; and the deactivation flag as a served control. - `qa/dogfood/test/identity-admin-fields-org-peer.dogfood.test.ts` pins the HTTP door on a real boot. - The two test edits named above. - A changeset for `@objectstack/plugin-security` and `@objectstack/platform-objects` (patch). ## Measured at the HTTP door (real boot, showcase, `single` posture, head `017857056`) | Persona / door | Result | |:--|:--| | Org member reading a colleague, by id and through the list | 200, 0 group fields; name, email and the deactivation flag served | | The same member through the activity door (object-level read granted) | rows served, 0 group keys in any recorded change | | The member's own row through the generic data API | 0 group fields. Field-level security is row-blind; every reader of the group on a member's own row reads under system or auth context | | The member's user-context write naming a group field, mixed with a profile field | 403 `PERMISSION_DENIED`, naming the field; the profile field does not land | | The member's profile-only write | 200 (control) | | The user picker's candidate query (the not-deactivated filter) as the member | 200. The deactivated peer is excluded, and is served when the query is unfiltered | | The member filtering on a group field | 403 `PERMISSION_DENIED` (the filter oracle stays closed) | | Org owner, and a platform admin holding no org-admin grant | full group on the direct read and in the activity metadata | | The platform admin's write naming a group field | not refused by the field-level gate (admin writes unchanged) | The dogfood file passes 12/12 at `017857056`. In round 1, no reader outside the organization was served the group: a second-org member got 404 by id and 0 rows through the list. The walled posture is NOT MEASURED, because the enterprise organizations package is absent here. ## Ablations Each leg was restored afterwards and checked: blob == HEAD, and `git diff HEAD` empty. **Unit legs** (the subject resolved from `src`): | Mutation | Pins red | |:--|:--| | member set's `fields` entry removed | 3 | | one group field dropped from the built set | 12, incl. the #11965 pin | | org admin set given the withholding entry | 5 | | `admin_full_access` keeping entry removed | 3, incl. the #11965 pin | **Dist legs** (rebuild, then `ablation-dist-preflight` both ways): | Mutation | Pins red | |:--|:--| | member set's entry removed | 6 of 11 dogfood | | `admin_full_access` keeping entry removed | 2 of 11 dogfood (the admin read and the admin write) | | deactivation flag moved back into the `Admin` group (platform-objects rebuilt) | 3 of 12 dogfood (incl. the picker query); 2 unit (the control) | ## Tests and gates **At `017857056`:** | Suite | Test Files | Tests | |:--|:--|:--| | `plugin-security` | 158 passed (158) | 3420 passed, 23 skipped (3443) | | `platform-objects` | 59 passed (59) | 948 passed (948) | | `plugin-auth` | 116 passed (116) | 2484 passed (2484) | | `sys-user-self-service-route.test.ts` | 1 passed | 12/12 | | `admin-ledger-decision-metadata.dogfood.test.ts` | 1 passed | 8/8 | | The new dogfood pin | 1 passed | 12/12 | | Dogfood subset (the 34 files that read the shipped sets or identity rows) | 33 passed, 1 skipped (34) | 313 passed, 3 skipped (316) | **At `77b116888`, before the merge of `origin/main`:** | Suite | Test Files | Tests | |:--|:--|:--| | `rest` | 254 passed | 4805 passed, 316 skipped | | `runtime` | 302 passed | 4329 passed, 11 skipped | | `verify` | 16 passed | 120 passed | | `service-automation` | 162 passed | 2027 passed | | `cli` unit layer (the integration layer is left to CI) | 243 passed | 3439 passed | The full dogfood suite is left to CI's sharded gate. **Typecheck** (exit 0 for each): `plugin-security`, `platform-objects`, `plugin-auth`, `dogfood`. **Gates:** `dispatch-gates --commands` at `017857056` (merge base `222ecc27f`) derived 68 commands. All 68 ran and exited 0, and `--ran` reconciles 68/68 with 0 NOT-MEASURED. ## Acceptance notes - **Docs.** I grepped `content/docs/**` (outside `releases/`) and `skills/**` for what a member reads of the identity object. No sentence is made false. `field-level-security.mdx` already states the most-permissive merge this change relies on. - **Self-service route classifier.** The `route()` helper in the self-service route test labels any middleware refusal raised after the pre-image read `row-scope`. That would misattribute a field-level write-gate refusal. No case in the file sends a group field now, so nothing is misattributed today. A fix needs a fourth layer label, a classifier change and a case producing it (the file's own non-vacuity rule), which is more than one hunk, so it is left as a note. The dogfood pin asserts that this refusal names the field. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 50e1c65 commit 1878ef9

8 files changed

Lines changed: 585 additions & 6 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
'@objectstack/platform-objects': patch
4+
---
5+
6+
fix(plugin-security,platform-objects): an org member reading a colleague's `sys_user` row is no longer served the identity object's `Admin` field group, directly or through the activity stream (#21237)
7+
8+
Clause-②: no
9+
10+
- **What a member was served.** The platform baseline `member_default` opens every org peer's `sys_user` row (the `sys_user_org_members` policy) and declared no field-level security on it. An org member reading a colleague's row was therefore served the whole `Admin` field group: the sign-in trail, the lockout state, the ban reason and expiry, the password and MFA stamps, the legacy platform role scalar and the AI-seat flag. With object-level read on `sys_activity`, the colleague's activity metadata carried the same fields, because the activity field redaction serves exactly what the data plane serves.
11+
- **What changes.** `member_default` and `viewer_readonly` now declare the `Admin` group `readable: false` through the permission set's existing `fields` entries. The withheld set is built from the identity object's declaration, so a field the declaration adds to the group is withheld from the day it is declared. `admin_full_access` and `organization_admin` (and so `organization_admin_no_bypass`) declare the group readable and editable, the same state as a field no set names, so an administrator's reads and writes are unchanged. `member_default` is the additive baseline every authenticated user resolves, and field grants merge most-permissively, which is why the admin sets carry that keeping entry.
12+
- **What a member sees now.** On the direct read, the list read and the activity metadata, a member is served no `Admin`-group field of a colleague's row. The directory fields (name, email, image) are still served. Field-level security does not distinguish rows, so the member's own row read through the generic data API is withheld the group too; every platform reader of those fields on a member's own row (the auth gates, the sign-in stamps, the session, the AI-seat resolution) reads under system or auth context and is unaffected. A member's query that filters or sorts on a withheld field is refused (`403 PERMISSION_DENIED`, the filter-oracle rule). A member's user-context write that names a withheld field is refused by the field-level write gate (`403 PERMISSION_DENIED`), and a payload mixing such a field with profile fields no longer lands partially.
13+
- **The deactivation flag is directory data.** `sys_user.banned` moves from the `Admin` field group to the `Account` group in `@objectstack/platform-objects`, so members are still served it. Every user picker filters its candidates on it, and a filter on a withheld field would be refused. Its reason and expiry stay in the `Admin` group. In a record form the field now renders in the `Account` section.
14+
15+
**Migration.** None for shipped apps. A custom permission set that grants an org member read on `sys_user` and is meant to show them the `Admin` group must name those fields `readable: true` in its `fields` entries. A client that filtered members' `sys_user` queries on an `Admin`-group field must drop that predicate or run it with an administrator's grant.

‎packages/platform-objects/src/identity/sys-user.object.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,7 @@ export const SysUser = ObjectSchema.create({
708708
label: 'Banned',
709709
defaultValue: false,
710710
readonly: true, // ADR-0092 — toggled via Ban/Unban actions (session side effects)
711-
group: 'Admin',
711+
group: 'Account',
712712
description: 'When true, the user cannot sign in. Toggle via Ban User / Unban User actions.',
713713
}),
714714

‎packages/plugins/plugin-auth/src/sys-user-self-service-route.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,9 @@ describe('sys_user self-service — the four pins, each attributed to a layer',
428428
// Worth pinning next to the refusal: the two behaviours are one branch apart
429429
// and a change that made stripping silent-refuse (or refusal silent-strip)
430430
// would be invisible to either test alone.
431-
const r = await route(ME, { locale: 'zh-CN', role: 'admin' });
431+
// A non-whitelisted column OUTSIDE the identity object's `Admin` group: a group field in the
432+
// payload is refused one layer earlier, by the field-level write gate (#21237).
433+
const r = await route(ME, { locale: 'zh-CN', email: 'attacker@example.com' });
432434
expect(r.refusedBy).toBeNull();
433435
expect(r.data).toEqual({ id: ME, locale: 'zh-CN' });
434436
});
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#21237] The identity object's `Admin` field group is served to an org peer
4+
// only when the reader holds an admin set — the shipped sets' half, pinned
5+
// against the identity object's DECLARATION rather than against a list.
6+
//
7+
// What the card measured: `member_default` opens every org peer's identity row
8+
// (`sys_user_org_members`) and declared no field-level security on it, so an
9+
// org member was served the whole `Admin` group of a colleague's row — and,
10+
// through the activity field redaction that reads the same served projection,
11+
// every colleague's history of those fields. The ruling (triage, direction A):
12+
// the permission set's existing `fields` → `readable: false`, on the shipped
13+
// non-admin sets, with the admin sets keeping the group. ⛔ No hand-kept list:
14+
// the withheld set is held EQUAL to the declared group here, so a field the
15+
// declaration adds to the group cannot slip through.
16+
//
17+
// Three pins, each read off a source the module under test does not own:
18+
//
19+
// 1. GROUP EQUALITY — every shipped set's identity-object field entries are
20+
// exactly the group the declaration names, read here from `SysUser`
21+
// itself (never from the module's own helper), with the posture each set
22+
// class must carry;
23+
// 2. COMPLETENESS — every shipped set that opens an org peer's identity row
24+
// is classified by an INDEPENDENT property (does it carry an org-peer
25+
// `sys_user` select policy?), so a new non-admin set opening that read
26+
// without withholding the group is red here, not silently served;
27+
// 3. COMPOSITION — the evaluator's REAL most-permissive merge, over the sets
28+
// each persona resolves. `member_default` is the additive `everyone`
29+
// baseline (ADR-0090 D5): an admin resolves it too, so the admin half is
30+
// only real if the merge keeps the group for them.
31+
//
32+
// The HTTP door (direct read, the activity metadata through the field
33+
// redaction, the member's own row, the field-level write gate and the user
34+
// picker's candidate query) is pinned on a real boot in
35+
// `packages/qa/dogfood/test/identity-admin-fields-org-peer.dogfood.test.ts`.
36+
37+
import { describe, it, expect } from 'vitest';
38+
import type { PermissionSet } from '@objectstack/spec/security';
39+
import { ADMIN_FULL_ACCESS, ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec';
40+
import { SysUser } from '@objectstack/platform-objects/identity';
41+
import { PermissionEvaluator } from './permission-evaluator.js';
42+
import { defaultPermissionSets } from './objects/default-permission-sets.js';
43+
44+
const IDENTITY = 'sys_user';
45+
46+
/** The declared group, read off the declaration — the pin's independent side. */
47+
const DECLARED_ADMIN_GROUP: string[] = Object.entries(SysUser.fields as Record<string, { group?: string }>)
48+
.filter(([, field]) => field.group === 'Admin')
49+
.map(([name]) => name)
50+
.sort();
51+
52+
/**
53+
* Fields a member already reads on a peer's row — the controls that must stay
54+
* served. `banned` is among them by ruling (triage on #21237, Q1 = B): the
55+
* deactivation flag is directory status, declared outside the `Admin` group,
56+
* because every user picker filters candidates on it and a filter on a withheld
57+
* field is refused. Moving it back into the group is red here.
58+
*/
59+
const PEER_DIRECTORY_FIELDS = ['name', 'email', 'image', 'banned'];
60+
61+
const ADMIN_SETS = [ADMIN_FULL_ACCESS, ...ORGANIZATION_ADMIN_GRANTS];
62+
const WITHHOLDING_SETS = ['member_default', 'viewer_readonly'];
63+
64+
const setByName = (name: string): PermissionSet => {
65+
const set = defaultPermissionSets.find((s) => s.name === name);
66+
if (!set) throw new Error(`shipped set ${name} not found`);
67+
return set;
68+
};
69+
70+
/** One set's identity-object field entries, keyed by field name. */
71+
const identityFieldEntries = (set: PermissionSet): Record<string, { readable?: boolean; editable?: boolean }> =>
72+
Object.fromEntries(
73+
Object.entries(set.fields ?? {})
74+
.filter(([key]) => key.startsWith(`${IDENTITY}.`))
75+
.map(([key, perm]) => [key.slice(IDENTITY.length + 1), perm]),
76+
);
77+
78+
/** Does this set open an org peer's identity row? Read off its row-level security. */
79+
const opensOrgPeerIdentityRead = (set: PermissionSet): boolean =>
80+
(set.rowLevelSecurity ?? []).some(
81+
(p) => p.object === IDENTITY && (p.operation === 'select' || p.operation === 'all') && /org_user_ids/.test(p.using ?? ''),
82+
);
83+
84+
describe('[#21237] the identity object Admin group — withheld from org peers, kept by admin sets', () => {
85+
it('the declared group is non-empty and names the field the card measured (vacuity guard)', () => {
86+
// Without this, a renamed group would make BOTH sides of the equality
87+
// below empty, and every pin in this file green over nothing.
88+
expect(DECLARED_ADMIN_GROUP.length).toBeGreaterThan(0);
89+
expect(DECLARED_ADMIN_GROUP).toContain('last_login_ip');
90+
for (const control of PEER_DIRECTORY_FIELDS) expect(DECLARED_ADMIN_GROUP).not.toContain(control);
91+
});
92+
93+
it.each(WITHHOLDING_SETS)('%s withholds exactly the declared group (readable: false), and nothing else on the identity object', (name) => {
94+
const entries = identityFieldEntries(setByName(name));
95+
expect(Object.keys(entries).sort()).toEqual(DECLARED_ADMIN_GROUP);
96+
for (const field of DECLARED_ADMIN_GROUP) {
97+
expect(entries[field], field).toEqual({ readable: false, editable: false });
98+
}
99+
});
100+
101+
it.each(ADMIN_SETS)('%s keeps exactly the declared group (readable: true, editable: true — the no-entry state)', (name) => {
102+
const entries = identityFieldEntries(setByName(name));
103+
expect(Object.keys(entries).sort()).toEqual(DECLARED_ADMIN_GROUP);
104+
for (const field of DECLARED_ADMIN_GROUP) {
105+
expect(entries[field], field).toEqual({ readable: true, editable: true });
106+
}
107+
});
108+
109+
it('every other shipped set names no identity-object field at all', () => {
110+
const others = defaultPermissionSets
111+
.filter((s) => !ADMIN_SETS.includes(s.name) && !WITHHOLDING_SETS.includes(s.name))
112+
.filter((s) => Object.keys(identityFieldEntries(s)).length > 0)
113+
.map((s) => s.name);
114+
expect(others).toEqual([]);
115+
});
116+
117+
it('completeness: every shipped set opening the identity row of an org peer is an admin set or withholds the group', () => {
118+
const openers = defaultPermissionSets.filter(opensOrgPeerIdentityRead).map((s) => s.name).sort();
119+
// The classification itself must have found something — an empty opener
120+
// list would make the loop below vacuous.
121+
expect(openers).toEqual(expect.arrayContaining(['member_default', 'viewer_readonly']));
122+
for (const name of openers) {
123+
const entries = identityFieldEntries(setByName(name));
124+
const expected = ADMIN_SETS.includes(name);
125+
for (const field of DECLARED_ADMIN_GROUP) {
126+
expect(entries[field]?.readable, `${name} → ${field}`).toBe(expected);
127+
}
128+
}
129+
});
130+
131+
describe('composition — the real most-permissive merge of the evaluator over the sets each persona resolves', () => {
132+
const evaluator = new PermissionEvaluator();
133+
const mask = (names: string[]) => evaluator.getFieldPermissions(IDENTITY, names.map(setByName));
134+
135+
it('an org member (the `everyone` baseline alone) is withheld every group field, and served the directory fields', () => {
136+
const perms = mask(['member_default']);
137+
for (const field of DECLARED_ADMIN_GROUP) expect(perms[field]?.readable, field).toBe(false);
138+
// The controls: no entry at all means the field masker serves them.
139+
for (const field of PEER_DIRECTORY_FIELDS) expect(perms[field], field).toBeUndefined();
140+
});
141+
142+
it('a read-only viewer (baseline + viewer_readonly) is withheld every group field', () => {
143+
const perms = mask(['member_default', 'viewer_readonly']);
144+
for (const field of DECLARED_ADMIN_GROUP) expect(perms[field]?.readable, field).toBe(false);
145+
});
146+
147+
it.each(ADMIN_SETS)('an admin holding %s on top of the baseline keeps every group field, readable and editable', (name) => {
148+
const perms = mask(['member_default', name]);
149+
for (const field of DECLARED_ADMIN_GROUP) {
150+
expect(perms[field], field).toEqual({ readable: true, editable: true });
151+
}
152+
});
153+
});
154+
});

‎packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,8 @@ describe('sys_comment delete is moderation-shaped, not ownership-shaped (#8839)'
365365
* One capability change has landed on purpose since, on its own card, and is
366366
* written into the literal below: `view_all_audit_log` (#21260, ruling B on
367367
* #21175 — platform administrators hold the compliance ledger's audit
368-
* capability by default).
368+
* capability by default). And one more, on #21237: the `fields` block keeping the
369+
* identity object's `Admin` field group, which the `everyone` baseline withholds.
369370
*/
370371
describe('admin_full_access imports the kernel capability declaration unchanged (#11965)', () => {
371372
it('parsed declaration deep-equals the pre-#11965 inline literal', () => {
@@ -394,6 +395,13 @@ describe('admin_full_access imports the kernel capability declaration unchanged
394395
// [#21260] added on purpose — see the docblock above.
395396
'view_all_audit_log',
396397
],
398+
// [#21237] added on purpose — see the docblock above. The group is read off
399+
// the identity object's declaration, never listed here.
400+
fields: Object.fromEntries(
401+
Object.entries(PlatformObjects.SysUser.fields as Record<string, { group?: string }>)
402+
.filter(([, field]) => field.group === 'Admin')
403+
.map(([name]) => [`sys_user.${name}`, { readable: true, editable: true }]),
404+
),
397405
});
398406
expect(setByName('admin_full_access')).toEqual(preMove);
399407
});

0 commit comments

Comments
 (0)