From e7226bbc4804ee94017053a3903e66693e4e94d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 05:10:50 +0000 Subject: [PATCH 1/3] fix(plugin-security): re-land member_default's sys_comment / sys_activity grants This reverts the revert 119c2db1, restoring 490d136f byte for byte: the platform baseline member_default grants sys_comment (read, create, edit) and sys_activity (read), with its tests and changeset. It lands together with the permission-set clone door fix that the revert was waiting for. Claude-Session: https://claude.ai/code/session_01Utp6CatT2ajve5PJf1Hhzj Co-authored-by: Claude --- .../30108-member-default-feed-grants.md | 16 + .../comment-reaction-own-record-floor.test.ts | 5 +- .../src/member-default-explicit-allow.test.ts | 67 ++- ...er-default-feed-grants.integration.test.ts | 481 ++++++++++++++++++ .../src/objects/default-permission-sets.ts | 51 +- 5 files changed, 607 insertions(+), 13 deletions(-) create mode 100644 .changeset/30108-member-default-feed-grants.md create mode 100644 packages/plugins/plugin-security/src/member-default-feed-grants.integration.test.ts diff --git a/.changeset/30108-member-default-feed-grants.md b/.changeset/30108-member-default-feed-grants.md new file mode 100644 index 00000000000..91be2f6bb0a --- /dev/null +++ b/.changeset/30108-member-default-feed-grants.md @@ -0,0 +1,16 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): every member reads and posts comments, and reads the activity history, of the records they can read — the platform baseline `member_default` now grants `sys_comment` (read, create, edit) and `sys_activity` (read) + +Clause-②: no — runtime permission behaviour (human-floor security/permission class, ruled by the maintainer), not a published accept-set or public-surface widening +Metadata-protocol: none + +Comments (`enable.feeds`) and the activity timeline (`enable.activities`) are on by default on every object, but no shipped permission set named `sys_comment` or `sys_activity`. A member of an app whose permission sets name only the app's own objects — what `create-objectstack` generates — was refused both (`403` on `GET /api/v1/data/sys_comment` and `GET /api/v1/data/sys_activity`), on records they can read: the record page showed "no permission" for comments and activity, and offered no comment box. An app author no longer needs to grant the two objects by hand. + +- **Which rows is unchanged.** The grant opens the two objects; plugin-audit's parent-record gates still decide the rows, so a member reads and posts comments, and reads the history, of exactly the records they can read, and nothing of any other record — a record another user owns under the app's row scope, or a record of an object the member does not read. A member edits the comments they wrote. The grant carries no delete bit (deleting a comment stays the moderation path, through a delete bit an app grants), no export and no view-all or modify-all. +- **`enable.feeds: false` still closes comments** on that object: creating a comment there, or moving one into it, answers `403 FEEDS_DISABLED`. `enable.activities: false` still writes no activity row for the object. +- **External members are not reached.** A holder of the `external` organization role does not hold the `everyone` anchor and resolves `external_default` in place of `member_default`, so a portal user still gets comments and activity only through an explicit grant, under the same two gates. +- **No log noise.** A member's activity read skips the parent objects they cannot read without reading them, so the console header's feed logs no `[Security] Access denied` line per skipped object. +- An app that already grants the two objects to its members keeps working unchanged: permission sets merge most-permissively, so its own lines (a delete bit included) still apply beside the baseline's. diff --git a/packages/plugins/plugin-security/src/comment-reaction-own-record-floor.test.ts b/packages/plugins/plugin-security/src/comment-reaction-own-record-floor.test.ts index b9473b85fd3..9cb68c4866c 100644 --- a/packages/plugins/plugin-security/src/comment-reaction-own-record-floor.test.ts +++ b/packages/plugins/plugin-security/src/comment-reaction-own-record-floor.test.ts @@ -33,8 +33,9 @@ * ## Who grants the bits * * The object-level create / read / delete bits come from an ordinary - * position-distributed set an application ships (as an app grants - * `sys_comment` today), never from the `everyone`-bound baseline, which + * position-distributed set an application ships (as an app grants the + * `sys_comment` delete bit; the baseline grants comments' read, create and + * edit, never delete), never from the `everyone`-bound baseline, which * ADR-0090 D5 keeps free of delete bits. Which shipped set — if any — carries * them is an open decision on #22566; the floor pinned here holds whatever set * hands the delete bit out. diff --git a/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts b/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts index 68beec7aece..b665092348a 100644 --- a/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts +++ b/packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts @@ -28,7 +28,7 @@ import { PermissionSetSchema } from '@objectstack/spec/security'; import type { PermissionSet } from '@objectstack/spec/security'; import { PermissionEvaluator } from './permission-evaluator.js'; import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; -import { defaultPermissionSets, BETTER_AUTH_MANAGED_OBJECTS } from './objects/default-permission-sets.js'; +import { defaultPermissionSets, BETTER_AUTH_MANAGED_OBJECTS, EXTERNAL_DEFAULT_PERMISSION_SET } from './objects/default-permission-sets.js'; const evaluator = new PermissionEvaluator(); const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; @@ -267,13 +267,10 @@ describe('[#7344] the personal inbox is readable by a member, scoped to their ow expect(rls.compileFilter(policiesFor(object, 'select') as any, {} as any)).toEqual(RLS_DENY_FILTER); }); - it('`sys_activity` is deliberately NOT granted — the ruling excludes it', () => { - // Named as an explicit negative so a future "while we are here" sweep has to - // argue with the ruling rather than quietly widen past it. It is not a - // per-user-scoped shape; a separate question if it ever matters. - for (const { operation } of AXES) { - expect(allows(operation, [MEMBER_DEFAULT], 'sys_activity'), `sys_activity ${operation}`).toBe(false); - } + it('`sys_activity` rides on its own ruling, not on this one: no `_self` policy scopes it to the caller', () => { + // The inbox ruling left it out because it has no `user_id` to scope by. Its + // grant (the `[#30108]` block below) is scoped by its parent-record gate, so + // no inbox-shaped `_self` carve-out may appear for it. const names = (MEMBER_DEFAULT.rowLevelSecurity ?? []).map((p: any) => p.name); expect(names).not.toContain('sys_activity_self'); }); @@ -291,6 +288,60 @@ describe('[#7344] the personal inbox is readable by a member, scoped to their ow }); }); +// [#30108 — maintainer ruling 6633054352, 「同意 A」] A record's comments and its +// activity timeline, for every member who can read the record. The baseline +// opens the two OBJECTS and nothing more; which rows is plugin-audit's +// parent-record gates' answer, driven on a real stack in +// `member-default-feed-grants.integration.test.ts`. What is pinned here is the +// shape of the opening: the part that would quietly widen if it moved. +const FEED_OBJECTS = ['sys_comment', 'sys_activity'] as const; + +describe('[#30108] the baseline opens comments and activity, and leaves the rows to the parent gates', () => { + it('sys_comment: a member reads, posts and edits — and holds no delete bit', () => { + expect(allows('find', [MEMBER_DEFAULT], 'sys_comment'), 'read').toBe(true); + expect(allows('insert', [MEMBER_DEFAULT], 'sys_comment'), 'create').toBe(true); + expect(allows('update', [MEMBER_DEFAULT], 'sys_comment'), 'edit').toBe(true); + expect(allows('delete', [MEMBER_DEFAULT], 'sys_comment'), 'delete').toBe(false); + }); + + it('sys_activity: a member reads — the audit mirror is its only writer', () => { + expect(allows('find', [MEMBER_DEFAULT], 'sys_activity'), 'read').toBe(true); + for (const operation of ['insert', 'update', 'delete']) { + expect(allows(operation, [MEMBER_DEFAULT], 'sys_activity'), operation).toBe(false); + } + }); + + it.each(FEED_OBJECTS)('%s: no read or create row policy of the baseline stands in for the parent gate', (object) => { + // A select- or insert-class policy here would answer the gate's question with + // a column predicate neither object can answer (no owner or user column, and + // a different parent object on every row). + expect(policiesFor(object, 'select').map((p: any) => p.name), 'select').toEqual([]); + expect(policiesFor(object, 'insert').map((p: any) => p.name), 'insert').toEqual([]); + }); + + it('sys_comment: an edit stays under the ownership floor — a member edits the comments they wrote', () => { + // The moderation carve-out is the DELETE limb only; the edit limb is the + // wildcard floor, so a member's edit reaches their own comments alone. + expect(policiesFor('sys_comment', 'update').map((p: any) => p.name)).toEqual(['owner_only_writes']); + }); + + it.each(FEED_OBJECTS)('%s: no export, view-all or modify-all rides along', (object) => { + const perm = (MEMBER_DEFAULT.objects as any)[object]; + expect(perm, `${object} is named`).toBeTruthy(); + expect(perm.allowDelete ?? false).toBe(false); + expect(perm.allowExport ?? false).toBe(false); + expect(perm.viewAllRecords ?? false).toBe(false); + expect(perm.modifyAllRecords ?? false).toBe(false); + }); + + it.each(FEED_OBJECTS)('%s: the external baseline does not name it — the grant reaches no external member', (object) => { + const external = defaultPermissionSets.find((p) => p.name === EXTERNAL_DEFAULT_PERMISSION_SET)!; + expect(external, `${EXTERNAL_DEFAULT_PERMISSION_SET} is shipped`).toBeTruthy(); + expect(allows('find', [external], object)).toBe(false); + expect(Object.keys(external.objects ?? {})).not.toContain(object); + }); +}); + describe('[#5491] the admin sets keep their wildcards (this is a BASELINE change only)', () => { it.each(['admin_full_access', 'organization_admin', 'viewer_readonly'])( '%s still carries a `*` entry', diff --git a/packages/plugins/plugin-security/src/member-default-feed-grants.integration.test.ts b/packages/plugins/plugin-security/src/member-default-feed-grants.integration.test.ts new file mode 100644 index 00000000000..ac307a2cad8 --- /dev/null +++ b/packages/plugins/plugin-security/src/member-default-feed-grants.integration.test.ts @@ -0,0 +1,481 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30108 — maintainer ruling 6633054352, 「同意 A」] Every member reads and + * posts comments, and reads the activity history, of the records they can + * read — with no grant from the app author — and of no other record. + * + * The platform baseline `member_default` names `sys_comment` (read, create, + * edit) and `sys_activity` (read). Those lines open the OBJECTS; the rows are + * decided by plugin-audit's parent-record gates. This suite drives the two + * together, as shipped, on a real stack: + * + * - the REAL `SecurityPlugin` with the shipped permission sets, and an app + * set that names the app's own objects and nothing else — the shape of a + * `create-objectstack` project, where the refusal was measured; + * - the REAL `AuditPlugin`: `sys_comment` and `sys_activity`, the CRUD mirror + * that writes the activity rows, the comment gate, the activity gate and + * the `enable.feeds` capability gate; + * - the REAL caller resolution (`resolveAuthzContext`, then + * `assembleExecutionContext`) over the identity rows seeded below, so a + * member's `everyone` anchor and an external's lack of it are the + * resolver's answer, not this file's. + * + * What the ruling asked to be measured before landing, one block each: + * + * 1. a member uses both on a record they can read; + * 2. a member gets nothing of a record they cannot read — a row in an object + * they read (row scope) and a row in an object they do not read at all; + * 3. an external member: the grant does not reach one (`org_external` holds + * no `everyone` anchor and resolves `external_default` in this set's + * place), and one holding an explicit grant meets the same two gates; + * 4. an object with `enable.feeds: false` refuses a comment's create and its + * re-thread into that object; + * 5. the console header's feed read (`GET /data/sys_activity?top=20&sort=-timestamp`, + * its rows and its `total`) logs no `[Security] Access denied` line for + * the parent objects it skips — armed by the read that does log one. + * + * `@objectstack/plugin-audit` reads from source here (this package's vitest + * alias); `@objectstack/core` from its built output, as this package's other + * suites read it. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { ObjectKernel, resolveAuthzContext, assembleExecutionContext, type Plugin, type PluginContext } from '@objectstack/core'; +import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { AuditPlugin } from '@objectstack/plugin-audit'; +import { SysMember, SysOrganization, SysUser } from '@objectstack/platform-objects/identity'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { mapDataError } from '@objectstack/types'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const COMMENT = 'sys_comment'; +const ACTIVITY = 'sys_activity'; +/** The app's record object. A member reads the tickets they requested. */ +const TICKET = 'fdg_ticket'; +/** An app object no member reads at all. */ +const VAULT = 'fdg_vault'; +/** An app object that turns comments and the activity mirror off. */ +const QUIET = 'fdg_quiet'; +const ORG = 'org_fdg'; + +const ALICE = 'usr_alice'; +const BOB = 'usr_bob'; +/** An external member holding an explicit portal grant. */ +const EVE = 'usr_eve'; +/** An external member holding nothing but its own baseline. */ +const EZRA = 'usr_ezra'; + +/** plugin-security's denial line, as a matcher (its developer sentence's opener). */ +const DENIAL = '[Security] Access denied'; + +/** + * The app's every-member set — the set a `create-objectstack` project ships: + * its own objects and NOTHING else. No `sys_comment`, no `sys_activity`: the + * baseline is what has to carry them. + */ +const APP_SET = PermissionSetSchema.parse({ + name: 'fdg_app_member', + label: 'Help desk member (the app set)', + objects: { + [TICKET]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: false }, + [QUIET]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: false }, + }, + rowLevelSecurity: [ + { name: 'fdg_ticket_requester', object: TICKET, operation: 'select', using: 'requester_id == current_user.id' }, + ], +}); + +/** + * A portal grant an app hands its external users explicitly: their own + * tickets, and the two feed objects named outright. + */ +const PORTAL_SET = PermissionSetSchema.parse({ + name: 'fdg_portal', + label: 'Help desk portal (explicit external grant)', + objects: { + [TICKET]: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, + [COMMENT]: { allowRead: true, allowCreate: true, allowEdit: false, allowDelete: false }, + [ACTIVITY]: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, + }, + rowLevelSecurity: [ + { name: 'fdg_ticket_portal_requester', object: TICKET, operation: 'select', using: 'requester_id == current_user.id' }, + ], +}); + +const SYS = { isSystem: true, tenantId: ORG } as const; + +function sqliteDriverPlugin(): Plugin { + return { + name: 'test.driver.sqlite', + type: 'standard', + version: '1.0.0', + async init(ctx: PluginContext) { + ctx.registerService( + 'driver.default', + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + ); + }, + }; +} + +function objectsPlugin(): Plugin { + const appObject = (name: string, label: string, extra: Record = {}) => ({ + name, + label, + fields: { + name: { type: 'text', label: 'Name' }, + requester_id: { type: 'text', label: 'Requester' }, + }, + ...extra, + }); + return { + name: 'test.objects', + type: 'standard', + version: '1.0.0', + dependencies: ['com.objectstack.engine.objectql'], + async init(ctx: PluginContext) { + ctx.getService<{ register(m: unknown): unknown }>('manifest').register({ + id: 'test.member-default-feed-grants', + namespace: 'fdg', + version: '1.0.0', + type: 'plugin', + name: 'Feed grants fixture', + objects: [ + SysUser, + SysMember, + SysOrganization, + appObject(TICKET, 'Ticket'), + appObject(VAULT, 'Vault'), + appObject(QUIET, 'Quiet', { enable: { feeds: false, activities: false } }), + ], + }); + }, + }; +} + +type Row = Record; +type Ctx = Record; +const ids = (rows: Row[]) => rows.map((r) => String(r.id)).sort(); + +/** The wire answer a refused engine call maps to, as the data door sends it. */ +async function refusal(run: () => Promise, object: string): Promise<{ status: number; code: string }> { + let thrown: unknown; + try { + await run(); + } catch (e) { + thrown = e; + } + expect(thrown, `the ${object} call was refused`).toBeDefined(); + const wire = mapDataError(thrown, object); + return { status: wire.status, code: String(wire.body.code) }; +} + +describe('member_default: comments and activity on the records a member can read', () => { + let kernel: ObjectKernel; + let ql: ObjectQL; + /** The kernel logger every plugin context shares — plugin-security's `ctx.logger`. */ + let logger: { warn: (...args: unknown[]) => void }; + /** The registered `security` service: the object the gates resolve on every read. */ + let security: { canReadObject?: unknown }; + const as: Record = {}; + const rec: Record = {}; + /** Comments seeded as the system, by the record they are about. */ + const seeded: Record = {}; + + /** The caller's context, as a request assembles it from the session. */ + const contextOf = async (userId: string): Promise => { + const authz = await resolveAuthzContext({ + ql, + headers: {}, + getSession: async () => ({ + user: { id: userId }, + session: { id: `ses_${userId}`, userId, activeOrganizationId: ORG }, + }), + } as never); + return assembleExecutionContext({ + authz, + oauth: undefined, + localization: undefined, + requestLocale: undefined, + accessToken: undefined, + } as never) as unknown as Ctx; + }; + + const thread = (object: string, id: string) => `${object}:${id}`; + const comments = async (ctx: Ctx, where?: Row) => (await ql.find(COMMENT, { ...(where ? { where } : {}), context: ctx } as never)) as Row[]; + const activity = async (ctx: Ctx, where?: Row) => (await ql.find(ACTIVITY, { ...(where ? { where } : {}), context: ctx } as never)) as Row[]; + const post = (ctx: Ctx, threadId: string, body: string) => + ql.insert(COMMENT, { thread_id: threadId, body }, { context: ctx } as never) as Promise; + const stored = async (object: string, where: Row) => (await ql.find(object, { where, context: SYS } as never)) as Row[]; + /** The stored activity rows about one record. */ + const historyOf = (object: string, id: string) => stored(ACTIVITY, { object_name: object, record_id: id }); + + /** Every WARN plugin-security logs as a denial while `run` is in flight. */ + const denialsDuring = async (run: () => Promise): Promise<{ value: T; denials: string[] }> => { + const spy = vi.spyOn(logger, 'warn'); + try { + const value = await run(); + return { value, denials: spy.mock.calls.map((args) => String(args[0])).filter((w) => w.includes(DENIAL)) }; + } finally { + spy.mockRestore(); + } + }; + + beforeAll(async () => { + kernel = new ObjectKernel({ logger: { level: 'silent' } }); + await kernel.use(sqliteDriverPlugin()); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(objectsPlugin()); + await kernel.use(new AuditPlugin()); + await kernel.use( + new SecurityPlugin({ + defaultPermissionSets: [...defaultPermissionSets, APP_SET, PORTAL_SET], + // The every-member baseline: the app's set composed with `member_default`. + fallbackPermissionSet: APP_SET.name, + }), + ); + await kernel.bootstrap(); + ql = kernel.getService('objectql'); + security = kernel.getService<{ canReadObject?: unknown }>('security'); + logger = (kernel as unknown as { logger: { warn: (...args: unknown[]) => void } }).logger; + + await ql.insert('sys_organization', { id: ORG, name: 'Help Desk Co', slug: 'help-desk' }, { context: SYS } as never); + const role: Record = { [ALICE]: 'member', [BOB]: 'member', [EVE]: 'external', [EZRA]: 'external' }; + for (const userId of [ALICE, BOB, EVE, EZRA]) { + await ql.insert( + 'sys_user', + { id: userId, name: userId, email: `${userId}@help-desk.test`, email_verified: true }, + { context: SYS } as never, + ); + await ql.insert('sys_member', { user_id: userId, organization_id: ORG, role: role[userId] }, { context: SYS } as never); + } + await ql.insert('sys_user_permission_set', { user_id: EVE, permission_set: PORTAL_SET.name }, { context: SYS } as never); + + // The records, written as the system: the CRUD mirror records each create. + const write = async (object: string, name: string, requester: string) => + String((await ql.insert(object, { name, requester_id: requester }, { context: SYS } as never)).id); + rec.alice = await write(TICKET, "alice's printer", ALICE); + rec.bob = await write(TICKET, "bob's laptop", BOB); + rec.eve = await write(TICKET, "eve's delivery", EVE); + rec.vault = await write(VAULT, 'payroll', BOB); + rec.quiet = await write(QUIET, 'quiet note', ALICE); + // Bob, the technician, has answered on every record but the quiet one. + for (const [key, object] of [['alice', TICKET], ['bob', TICKET], ['eve', TICKET], ['vault', VAULT]] as const) { + seeded[key] = String( + (await ql.insert(COMMENT, { thread_id: thread(object, rec[key]), author_id: BOB, body: `on ${key}` }, { context: SYS } as never)).id, + ); + } + + for (const [name, userId] of [['alice', ALICE], ['bob', BOB], ['eve', EVE], ['ezra', EZRA]] as const) { + as[name] = await contextOf(userId); + } + }, 120_000); + + afterAll(async () => { + if (kernel) { + await Promise.race([kernel.shutdown(), new Promise((r) => setTimeout(r, 10_000))]); + } + }, 30_000); + + it('the fixture is armed: the resolver gives a member the everyone anchor and an external none, and the stores hold what the cases read', async () => { + expect(as.alice.positions).toEqual(expect.arrayContaining(['org_member', 'everyone'])); + expect(as.eve.positions).toContain('org_external'); + expect(as.eve.positions).not.toContain('everyone'); + expect(as.ezra.positions).not.toContain('everyone'); + // The member reads their own ticket and not bob's: the row scope the gates defer to. + expect(ids((await ql.find(TICKET, { context: as.alice } as never)) as Row[])).toEqual([rec.alice]); + // Every record the cases read has history, written by the real mirror — + // except the quiet one, whose `enable.activities: false` writes none. + for (const [object, key] of [[TICKET, 'alice'], [TICKET, 'bob'], [TICKET, 'eve'], [VAULT, 'vault']] as const) { + expect((await historyOf(object, rec[key])).length, `${object} ${key} history`).toBeGreaterThan(0); + } + expect(await historyOf(QUIET, rec.quiet)).toEqual([]); + }); + + // ── 1. a record the member can read ───────────────────────────────────── + describe('1. on a record the member can read', () => { + it('reads its comments', async () => { + expect(ids(await comments(as.alice, { thread_id: thread(TICKET, rec.alice) }))).toEqual([seeded.alice]); + }); + + it('posts a comment, stamped as the member', async () => { + const row = await post(as.alice, thread(TICKET, rec.alice), 'the printer is jammed again'); + const [written] = await stored(COMMENT, { id: String(row.id) }); + expect(written?.author_id).toBe(ALICE); + expect(ids(await comments(as.alice, { thread_id: thread(TICKET, rec.alice) }))).toContain(String(row.id)); + }); + + it('edits the comment they wrote, and not one somebody else wrote', async () => { + const own = String((await post(as.alice, thread(TICKET, rec.alice), 'first draft')).id); + await ql.update(COMMENT, { body: 'second draft' }, { where: { id: own }, context: as.alice } as never); + expect((await stored(COMMENT, { id: own }))[0]?.body).toBe('second draft'); + + const theirs = await refusal( + () => ql.update(COMMENT, { body: 'rewritten' }, { where: { id: seeded.alice }, context: as.alice } as never), + COMMENT, + ); + expect(theirs).toEqual({ status: 403, code: 'PERMISSION_DENIED' }); + expect((await stored(COMMENT, { id: seeded.alice }))[0]?.body).toBe('on alice'); + }); + + it('reads its activity history', async () => { + const served = ids(await activity(as.alice, { object_name: TICKET, record_id: rec.alice })); + expect(served).toEqual(ids(await historyOf(TICKET, rec.alice))); + expect(served.length).toBeGreaterThan(0); + }); + + it('deletes nothing: the grant carries no delete bit', async () => { + const own = String((await post(as.alice, thread(TICKET, rec.alice), 'to be kept')).id); + const answer = await refusal(() => ql.delete(COMMENT, { where: { id: own }, context: as.alice } as never), COMMENT); + expect(answer).toEqual({ status: 403, code: 'PERMISSION_DENIED' }); + expect((await stored(COMMENT, { id: own })).length).toBe(1); + }); + }); + + // ── 2. records the member cannot read ─────────────────────────────────── + describe('2. on a record the member cannot read', () => { + const unreadable = () => + [ + { what: "another requester's ticket (row scope)", object: TICKET, key: 'bob' }, + { what: 'a record of an object the member does not read', object: VAULT, key: 'vault' }, + ] as const; + + it('a list of every comment serves none of its comments, and its count agrees', async () => { + const served = ids(await comments(as.alice)); + for (const { what, key } of unreadable()) expect(served, what).not.toContain(seeded[key]); + expect(served).toContain(seeded.alice); + expect(await ql.count(COMMENT, {}, { context: as.alice } as never)).toBe(served.length); + }); + + it('a direct read of one of its comments finds nothing', async () => { + for (const { what, key } of unreadable()) { + expect(await ql.findOne(COMMENT, { where: { id: seeded[key] }, context: as.alice } as never), what).toBeNull(); + } + }); + + it('a post to its thread is refused, and nothing is written', async () => { + for (const { what, object, key } of unreadable()) { + const before = (await stored(COMMENT, { thread_id: thread(object, rec[key]) })).length; + expect(await refusal(() => post(as.alice, thread(object, rec[key]), 'let me in'), COMMENT), what).toEqual({ + status: 403, + code: 'RECORD_NOT_ACCESSIBLE', + }); + expect((await stored(COMMENT, { thread_id: thread(object, rec[key]) })).length, what).toBe(before); + } + }); + + it('a list of every activity row serves none of its history, and its count agrees', async () => { + const served = await activity(as.alice); + for (const { what, object, key } of unreadable()) { + expect( + served.filter((r) => r.object_name === object && String(r.record_id) === rec[key]), + what, + ).toEqual([]); + } + expect(served.some((r) => r.object_name === TICKET && String(r.record_id) === rec.alice)).toBe(true); + expect(await ql.count(ACTIVITY, {}, { context: as.alice } as never)).toBe(served.length); + }); + + it('a direct read of one of its activity rows finds nothing', async () => { + for (const { what, object, key } of unreadable()) { + const [row] = await historyOf(object, rec[key]); + expect(await ql.findOne(ACTIVITY, { where: { id: String(row.id) }, context: as.alice } as never), what).toBeNull(); + } + }); + }); + + // ── 3. external members ───────────────────────────────────────────────── + describe('3. an external member', () => { + it('with only its own baseline: the grant does not reach it — both objects stay refused', async () => { + expect(await refusal(() => comments(as.ezra), COMMENT)).toEqual({ status: 403, code: 'PERMISSION_DENIED' }); + expect(await refusal(() => activity(as.ezra), ACTIVITY)).toEqual({ status: 403, code: 'PERMISSION_DENIED' }); + }); + + it('with an explicit grant: the comments and history of its own ticket only', async () => { + expect(ids(await comments(as.eve))).toEqual([seeded.eve]); + for (const key of ['alice', 'bob', 'vault'] as const) { + expect(await ql.findOne(COMMENT, { where: { id: seeded[key] }, context: as.eve } as never), key).toBeNull(); + } + const served = await activity(as.eve); + expect(served.some((r) => r.object_name === TICKET && String(r.record_id) === rec.eve)).toBe(true); + for (const [object, key] of [[TICKET, 'alice'], [TICKET, 'bob'], [VAULT, 'vault']] as const) { + expect(served.filter((r) => r.object_name === object && String(r.record_id) === rec[key]), key).toEqual([]); + } + }); + + it('with an explicit grant: posts on its own ticket, and is refused on a ticket it cannot read', async () => { + const row = await post(as.eve, thread(TICKET, rec.eve), 'when will it arrive?'); + expect((await stored(COMMENT, { id: String(row.id) }))[0]?.author_id).toBe(EVE); + const before = (await stored(COMMENT, { thread_id: thread(TICKET, rec.alice) })).length; + expect(await refusal(() => post(as.eve, thread(TICKET, rec.alice), 'hello?'), COMMENT)).toEqual({ + status: 403, + code: 'RECORD_NOT_ACCESSIBLE', + }); + expect((await stored(COMMENT, { thread_id: thread(TICKET, rec.alice) })).length).toBe(before); + }); + }); + + // ── 4. enable.feeds: false ────────────────────────────────────────────── + describe('4. an object with `enable.feeds: false`, on a record the member reads and edits', () => { + it('the member reads the record (the refusals below are the switch, not the row scope)', async () => { + expect(ids((await ql.find(QUIET, { context: as.alice } as never)) as Row[])).toEqual([rec.quiet]); + }); + + it('refuses a comment create: 403 FEEDS_DISABLED, and nothing is written', async () => { + expect(await refusal(() => post(as.alice, thread(QUIET, rec.quiet), 'psst'), COMMENT)).toEqual({ + status: 403, + code: 'FEEDS_DISABLED', + }); + expect(await stored(COMMENT, { thread_id: thread(QUIET, rec.quiet) })).toEqual([]); + }); + + it('refuses re-threading a comment into it: 403 FEEDS_DISABLED, and the comment stays where it was', async () => { + const own = String((await post(as.alice, thread(TICKET, rec.alice), 'move me')).id); + expect( + await refusal( + () => ql.update(COMMENT, { thread_id: thread(QUIET, rec.quiet) }, { where: { id: own }, context: as.alice } as never), + COMMENT, + ), + ).toEqual({ status: 403, code: 'FEEDS_DISABLED' }); + expect((await stored(COMMENT, { id: own }))[0]?.thread_id).toBe(thread(TICKET, rec.alice)); + }); + }); + + // ── 5. the feed read logs no denial per skipped object ────────────────── + describe('5. the console header feed read logs no access denial for the objects it skips', () => { + /** `GET /api/v1/data/sys_activity?top=20&sort=-timestamp`: its rows, then its `total`. */ + const headerFeed = async (ctx: Ctx) => ({ + rows: (await ql.find(ACTIVITY, { orderBy: [{ field: 'timestamp', order: 'desc' }], limit: 20, context: ctx } as never)) as Row[], + total: await ql.count(ACTIVITY, {}, { context: ctx } as never), + }); + + it('armed: a read the member does make of a refused object logs its denial, exactly once', async () => { + const { denials } = await denialsDuring(() => refusal(() => ql.find(VAULT, { context: as.alice } as never), VAULT)); + expect(denials).toHaveLength(1); + expect(denials[0]).toContain(VAULT); + }); + + it('armed: without the object-level admission the same read logs one per refused parent', async () => { + const own = Object.getOwnPropertyDescriptor(security, 'canReadObject'); + expect(own, 'the registered service carries canReadObject as its own member').toBeTruthy(); + delete security.canReadObject; + try { + const { denials } = await denialsDuring(() => headerFeed(as.alice)); + expect(denials.some((d) => d.includes(VAULT)), denials.join('\n')).toBe(true); + } finally { + Object.defineProperty(security, 'canReadObject', own!); + } + }); + + it('the member reads the feed with no denial logged, and it serves only rows about records they read', async () => { + const { value, denials } = await denialsDuring(() => headerFeed(as.alice)); + expect(denials).toEqual([]); + expect(value.rows.length).toBeGreaterThan(0); + expect(value.rows.filter((r) => r.object_name === VAULT)).toEqual([]); + expect(value.total).toBeGreaterThanOrEqual(value.rows.length); + }); + }); +}); diff --git a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts index 9d257f01074..af3bc8d250f 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts +++ b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts @@ -801,13 +801,58 @@ const baseDefaultPermissionSets: PermissionSet[] = [ // access, and mark-read is served by `/api/v1/notifications` rather than the // generic data API — so no create/edit bit is needed by the flow, and // `allowDelete` stays false like everything else in this anchor-bound set. - // `sys_activity` is deliberately NOT included: it is not a per-user-scoped - // shape (no `user_id` to scope by), and it is a separate question if it ever - // matters. Two NAMED additions in #5491's explicit-allow shape — the + // `sys_activity` was left out of THIS pair because it has no `user_id` for + // a `_self` policy to scope by; it is granted further down, on its own + // ruling and under its own row scope (the parent-record gate, not a `_self` + // policy). Two NAMED additions in #5491's explicit-allow shape — the // `_self` policies below scope both to the caller — not a widening pattern // and not a step back toward a wildcard. sys_inbox_message: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, sys_notification_receipt: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, + // [#30108 — maintainer ruling 6633054352, verbatim 「同意 A」] A record's + // comments and its activity timeline, for every member who can read the + // record. `enable.feeds` and `enable.activities` are opt-OUT (on unless an + // object says `false`), so the record page offers both on every object of + // every app; with neither object named by any shipped set, a member of an + // app whose sets name only the app's own objects was refused both (`403` + // on `sys_comment` and on `sys_activity`), on records they can read — the + // "declared, not reachable" shape ADR-0049 forbids. The rule the comment + // gate already states is "a user who may READ a record may discuss it". + // + // These two lines open the OBJECTS. WHICH ROWS is not decided here, and ⛔ + // no row policy is added for either: neither object has an owner or user + // column to scope by, and each row's parent is a different object. The + // rows are decided by plugin-audit's parent-record gates, which ask the + // engine and this plugin "can this caller read that record": + // - `comment-access-hooks.ts`: a comment is listed, read and posted only + // on a thread whose parent record the caller can read; an edit needs + // the comment's author or an editor of the parent record, and the + // `owner_only_writes` floor below keeps a member's edit to the + // comments they wrote; + // - `activity-read-visibility.ts`: an activity row is served only when + // the record it is about is readable. A parent object the caller + // holds no read on is skipped through `canReadObject`, never read as + // the caller, so a feed read logs no access denial per skipped object. + // So a member reads and writes the comments, and reads the history, of + // exactly the records they can read, and nothing on any other record. + // + // The capability switches stay orthogonal and in force: on an object + // with `enable.feeds: false`, plugin-audit's `enforceFeedsCapability` + // refuses a comment's create and its re-thread into that object + // (`403 FEEDS_DISABLED`), and `enable.activities: false` writes no + // activity row for that object to read. + // + // ⛔ No `allowDelete` on either, for the anchor reason above. Removing a + // comment stays the moderation path (`sys_comment_moderation` below, with + // a delete bit an ordinary set grants). `sys_activity` is READ only: its + // writer is the audit mirror, and the object serves `get` / `list` alone. + // + // An external member (`org_external`) holds no `everyone` anchor and + // resolves `external_default` in place of this set, so neither line + // reaches a portal user: theirs come only through an explicit grant, under + // the same two gates. + sys_comment: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: false }, + sys_activity: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, }, // [#21237] The identity object's `Admin` group is admin-review data, not a // staff directory: `sys_user_org_members` below opens every org peer's row, From 591198e4e1052010cefbce18714abc7ec526b92e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 04:02:14 +0000 Subject: [PATCH 2/3] fix(runtime, metadata-protocol): the permission-set clone door copies a set whose grants name an object this deployment does not install The save door's unknown-grant refusal already leaves alone a grant the set itself already holds. A clone's grants are the source's, copied verbatim, so the clone door now passes the copy's grant keys to saveMetaItem as inheritedGrantObjects, and the refusal does not judge those. An unknown object the save does not say it inherits is still refused. member_default, which grants sys_comment and sys_activity, copies into its package's customization pair again on a boot without @objectstack/plugin-audit. Claude-Session: https://claude.ai/code/session_01Utp6CatT2ajve5PJf1Hhzj Co-authored-by: Claude --- .../30108-member-default-feed-grants.md | 1 + .../30255-clone-inherits-held-grants.md | 13 +++++++++ .../src/protocol-door-requests.ts | 12 ++++++++- .../src/protocol-save-door-refusals.ts | 9 +++++++ .../metadata-protocol/src/protocol-save.ts | 5 +++- packages/metadata-protocol/src/protocol.ts | 14 ++++++++-- ...tocol.unknown-grant-object-refusal.test.ts | 27 ++++++++++++++++--- .../src/domains/permission-set-clone.test.ts | 10 ++++--- .../src/domains/permission-set-clone.ts | 20 +++++++++++++- 9 files changed, 100 insertions(+), 11 deletions(-) create mode 100644 .changeset/30255-clone-inherits-held-grants.md diff --git a/.changeset/30108-member-default-feed-grants.md b/.changeset/30108-member-default-feed-grants.md index 91be2f6bb0a..2a40eb8bd50 100644 --- a/.changeset/30108-member-default-feed-grants.md +++ b/.changeset/30108-member-default-feed-grants.md @@ -14,3 +14,4 @@ Comments (`enable.feeds`) and the activity timeline (`enable.activities`) are on - **External members are not reached.** A holder of the `external` organization role does not hold the `everyone` anchor and resolves `external_default` in place of `member_default`, so a portal user still gets comments and activity only through an explicit grant, under the same two gates. - **No log noise.** A member's activity read skips the parent objects they cannot read without reading them, so the console header's feed logs no `[Security] Access denied` line per skipped object. - An app that already grants the two objects to its members keeps working unchanged: permission sets merge most-permissively, so its own lines (a delete bit included) still apply beside the baseline's. +- **Lands with the permission-set clone door fix** (`.changeset/30255-clone-inherits-held-grants.md`). This grant first landed alone and was reverted: on a deployment that does not install `@objectstack/plugin-audit`, neither object is registered, and cloning `member_default` answered `422 INVALID_METADATA`. The clone door now copies such a set, so the grant and the fix ship together. diff --git a/.changeset/30255-clone-inherits-held-grants.md b/.changeset/30255-clone-inherits-held-grants.md new file mode 100644 index 00000000000..3e209ce5f2c --- /dev/null +++ b/.changeset/30255-clone-inherits-held-grants.md @@ -0,0 +1,13 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/runtime': patch +--- + +fix(runtime, metadata-protocol): the permission-set clone door copies a platform set whose grants name an object this deployment does not install — `member_default` copies into its package's customization pair again on a boot without `@objectstack/plugin-audit` + +Clause-②: yes (widening) — `ObjectStackProtocolImplementation.saveMetaItem`'s request gains one optional key, `inheritedGrantObjects`, reachable from `@objectstack/metadata-protocol`'s `.` entry types +Metadata-protocol: none + +- **What failed:** cloning a set through `POST /api/v1/security/permission-sets/:name/clone` answered `422 INVALID_METADATA` when the source grants on an object no package registers here. The commonest source is the platform baseline `member_default`, which grants `sys_comment` and `sys_activity`. `@objectstack/plugin-audit` registers both objects, and it is an optional package: a deployment that does not install it could not clone `member_default` at all. +- **The rule it now follows:** the save door's unknown-grant refusal (#28990) already leaves alone a grant the set itself already holds, because that grant is not the current write's. A clone's grants are the source's, copied verbatim, so the same holds for a copy. The clone door now tells the save which grant keys the copy inherits (`inheritedGrantObjects`), and the refusal does not judge those. +- **Still refused:** an unknown object that the save does not say it inherits is this write's grant, and is refused exactly as before. That covers a `/meta` save, a re-save that adds a grant, and a draft's publish. No HTTP door forwards the new key. diff --git a/packages/metadata-protocol/src/protocol-door-requests.ts b/packages/metadata-protocol/src/protocol-door-requests.ts index 4e38539800b..984378c69f7 100644 --- a/packages/metadata-protocol/src/protocol-door-requests.ts +++ b/packages/metadata-protocol/src/protocol-door-requests.ts @@ -18,7 +18,17 @@ import type { MetadataWriteFace } from './protocol-write-face.js'; */ /** The request of {@link ObjectStackProtocolImplementation.saveMetaItem}. */ -type SaveMetaItemRequest = { type: string, name: string, item?: any, parentVersion?: string | null, storedParentVersion?: string | null, actor?: string, force?: boolean, mode?: 'draft' | 'publish', packageId?: string | null, source?: string, writeFace?: MetadataWriteFace, surface?: MetadataAuthoringSurface, client?: MetadataAuthoringClient }; +type SaveMetaItemRequest = { + type: string, name: string, item?: any, parentVersion?: string | null, storedParentVersion?: string | null, actor?: string, force?: boolean, mode?: 'draft' | 'publish', packageId?: string | null, source?: string, writeFace?: MetadataWriteFace, surface?: MetadataAuthoringSurface, client?: MetadataAuthoringClient, + /** + * [#30255] The grant keys of a `permission` item that this save copies + * verbatim from a set that already holds them — the permission-set clone + * door's copy. Not this write's grants, so the unknown-grant refusal does + * not judge them, exactly as it does not judge a re-save's grants the set + * already holds. Set by server code only; no HTTP door forwards it. + */ + inheritedGrantObjects?: readonly string[], +}; /** The request of {@link ObjectStackProtocolImplementation.publishMetaItem}. */ type PublishMetaItemRequest = { diff --git a/packages/metadata-protocol/src/protocol-save-door-refusals.ts b/packages/metadata-protocol/src/protocol-save-door-refusals.ts index 89d62d1e99d..53d38458874 100644 --- a/packages/metadata-protocol/src/protocol-save-door-refusals.ts +++ b/packages/metadata-protocol/src/protocol-save-door-refusals.ts @@ -579,6 +579,7 @@ export async function unknownGrantObjectSaveRefusal( pending?: 'drafts' | ReadonlySet, writeFace?: MetadataWriteFace, door: 'save' | 'publish' = 'save', + inherited?: readonly string[], ): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> { if (singularType !== 'permission') return undefined; if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined; @@ -625,6 +626,14 @@ export async function unknownGrantObjectSaveRefusal( : new Set(); unknown = unknown.filter((key) => !held.has(key)); if (unknown.length === 0) return undefined; + // [#30255] A grant this save copies from a set that already holds it (the + // clone door's copy) is the source's, not this save's — the rule above, + // for a copy: judged where the source introduced it. + if (inherited && inherited.length > 0) { + const copied = new Set(inherited); + unknown = unknown.filter((key) => !copied.has(key)); + if (unknown.length === 0) return undefined; + } // [#29279] What was looked for, as this door looked: a save also counts an // object saved here; a draft's publish counts only what goes live with it. const looked = door === 'publish' diff --git a/packages/metadata-protocol/src/protocol-save.ts b/packages/metadata-protocol/src/protocol-save.ts index c24e666d4cf..8873e784807 100644 --- a/packages/metadata-protocol/src/protocol-save.ts +++ b/packages/metadata-protocol/src/protocol-save.ts @@ -1575,10 +1575,13 @@ export class ProtocolSaveFacet { // already holds and is not judged: a draft-only set has no active row // to hold its grants, so judging it refused the pass's own rewrite of // a draft whose object draft was gone; the draft is judged at its - // publish. See {@link unknownGrantObjectSaveRefusal}. + // publish. [#30255] Grants the item copies from a set that already + // holds them (`inheritedGrantObjects`, the clone door's copy) are not + // this save's either. See {@link unknownGrantObjectSaveRefusal}. if (writeSource !== 'migrate-stored') { const grantRefusal = await host.unknownGrantObjectSaveRefusal( singularType, request.item, request.name, mode === 'draft' ? 'drafts' : undefined, request.writeFace, + undefined, request.inheritedGrantObjects, ); if (grantRefusal) throw grantRefusal; } diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 2451f7138cc..f49c98d3dea 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -10302,7 +10302,16 @@ export class ObjectStackProtocolImplementation implements * a set), since the batch promotes as one unit in no type order; * - the set's current definition already grants on it: the grant is not * this write's. A re-save or a restore that keeps it is not judged, and - * neither is an object that has gone since the grant was made. + * neither is an object that has gone since the grant was made; + * - [#30255] the save copies it verbatim from a set that already holds it + * (`inherited`, the request's `inheritedGrantObjects`: the permission-set + * clone door's copy). The grant is the source's, not this write's — the + * rule above, for a copy. A platform set may grant on an object of an + * optional platform plugin this deployment does not install + * (`member_default` on `sys_comment` / `sys_activity`, which + * `@objectstack/plugin-audit` registers): the grant applies to nothing + * here, in the source and in the copy alike, and the copy is what the + * clone door promises. * * Asked in draft and in publish mode by {@link saveMetaItem} after the * schema gate (except a `migrate-stored` re-save, the platform healing @@ -10323,6 +10332,7 @@ export class ObjectStackProtocolImplementation implements pending?: 'drafts' | ReadonlySet, writeFace?: MetadataWriteFace, door?: 'save' | 'publish', + inherited?: readonly string[], ): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> { const readEngine = () => this.engine; const host: ThisParameterType = { @@ -10330,7 +10340,7 @@ export class ObjectStackProtocolImplementation implements listDrafts: (...a) => this.listDrafts(...a), readMetaItem: (...a) => this.readMetaItem(...a), }; - return unknownGrantObjectSaveRefusal.call(host, singularType, item, name, pending, writeFace, door); + return unknownGrantObjectSaveRefusal.call(host, singularType, item, name, pending, writeFace, door, inherited); } /** diff --git a/packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts b/packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts index 31bd46448db..2c5dd2a81da 100644 --- a/packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts +++ b/packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts @@ -22,9 +22,11 @@ * - controls, each saved, published and promoted: a registered object, a * platform object (an RBAC link table), the `'*'` wildcard, an object * stored in this environment, an object drafted in the same package and - * published in the same batch (promoted after the set), and a re-save - * that keeps a grant the set already holds; a registry that cannot look - * an object up refuses nothing. + * published in the same batch (promoted after the set), a re-save + * that keeps a grant the set already holds, and [#30255] a copy whose + * grants a set already holds (`inheritedGrantObjects`, the permission-set + * clone door's copy); a registry that cannot look an object up refuses + * nothing. * * Each refusal is asserted by its code, its status, its path and the first * sentence of its finding, spelled out here. @@ -363,6 +365,25 @@ describe('[#28990] controls — a grant on an object that exists is not refused' expectRefusal(err, 'qa_old', ['qa_typo']); }); + it('[#30255] a copy that inherits a grant on an object not registered here (the clone door): saved; a grant the copy adds is still refused', async () => { + const { protocol, rows } = makeSession({ objects: ['qa_widget'] }); + + const result = await protocol.saveMetaItem({ + type: 'permission', name: 'qa_copy', item: set('qa_copy', ['qa_widget', 'qa_plugin_object']), + inheritedGrantObjects: ['qa_widget', 'qa_plugin_object'], + }); + + expect(result.success).toBe(true); + expect(stored(rows, 'permission', 'qa_copy')).toEqual(['active|-|qa_widget,qa_plugin_object']); + // …while an unknown object the copy does not inherit is this save's grant, and refused. + const err = await refusalOf(protocol.saveMetaItem({ + type: 'permission', name: 'qa_copy2', item: set('qa_copy2', ['qa_plugin_object', 'qa_typo']), + inheritedGrantObjects: ['qa_plugin_object'], + })); + expectRefusal(err, 'qa_copy2', ['qa_typo']); + expect(stored(rows, 'permission', 'qa_copy2')).toEqual([]); + }); + it('a registry that cannot look an object up refuses nothing', async () => { const { protocol, rows } = makeSession({ looksUpObjects: false }); expect((await protocol.saveMetaItem({ type: 'permission', name: 'qa_probe', item: set('qa_probe', ['qa_no_such_object']) })).success).toBe(true); diff --git a/packages/runtime/src/domains/permission-set-clone.test.ts b/packages/runtime/src/domains/permission-set-clone.test.ts index b5008f627ef..63d307abd73 100644 --- a/packages/runtime/src/domains/permission-set-clone.test.ts +++ b/packages/runtime/src/domains/permission-set-clone.test.ts @@ -84,7 +84,7 @@ const PACKAGES: Record }> = { 'com.vendor.app': { manifest: { id: 'com.vendor.app', scope: 'cloud' } }, }; -interface Saved { type: string; name: string; item: Record; parentVersion?: string | null; packageId?: string } +interface Saved { type: string; name: string; item: Record; parentVersion?: string | null; packageId?: string; inheritedGrantObjects?: readonly string[] } interface BootOptions { unbound?: boolean; @@ -178,8 +178,11 @@ describe('the permission-set clone door — success', () => { expect(save.parentVersion).toBeNull(); // [ADR-0146 D3/D6] No organization; the package is the SAVE's — the // read-only source's pair, ensured through the ensure door. - expect(Object.keys(save).sort()).toEqual(['item', 'name', 'packageId', 'parentVersion', 'type']); + expect(Object.keys(save).sort()).toEqual(['inheritedGrantObjects', 'item', 'name', 'packageId', 'parentVersion', 'type']); expect(save.packageId).toBe('crm.custom'); + // [#30255] The copy's grant keys are the source's, and the save is told + // so: its unknown-grant refusal does not judge them as this write's. + expect(save.inheritedGrantObjects).toEqual(Object.keys(PACKAGED.objects)); expect(h.ensures).toEqual(['crm']); // Whole-definition copy: the source minus the declared keys, the two @@ -226,7 +229,8 @@ describe('the permission-set clone door — success', () => { expect(statusOf(res)).toBe(200); expect(h.saves[0].item).toEqual({ ...ENV_SET, name: 'env_reader_copy', label: 'Env Reader Copy' }); - expect(Object.keys(h.saves[0]).sort()).toEqual(['item', 'name', 'parentVersion', 'type']); + expect(Object.keys(h.saves[0]).sort()).toEqual(['inheritedGrantObjects', 'item', 'name', 'parentVersion', 'type']); + expect(h.saves[0].inheritedGrantObjects).toEqual(Object.keys(ENV_SET.objects)); expect(h.ensures).toEqual([]); expect((res as any).response.body.data.package).toBeNull(); // [#29727] The environment-wide variant's code. diff --git a/packages/runtime/src/domains/permission-set-clone.ts b/packages/runtime/src/domains/permission-set-clone.ts index 01d8332ae6b..d06436f9bd2 100644 --- a/packages/runtime/src/domains/permission-set-clone.ts +++ b/packages/runtime/src/domains/permission-set-clone.ts @@ -242,6 +242,16 @@ export function clonePermissionSetDefinition( return copy; } +/** + * [#30255] The grant keys of a copy's `objects` map — what the save is told the + * copy inherits ({@link handlePermissionSetCloneWrite} step 7). Every one is the + * source's own key, carried by {@link clonePermissionSetDefinition} unchanged. + */ +export function inheritedGrantObjectsOf(copy: Readonly>): string[] { + const objects = copy.objects; + return objects && typeof objects === 'object' && !Array.isArray(objects) ? Object.keys(objects) : []; +} + /** * Every object a permission set grants on: the keys of its `objects` map, the * object half of each `fields` key (`object.field`), and each row-level rule's @@ -392,7 +402,9 @@ function readCloneBody(body: unknown): { name: string; label: string; package?: * `packageId` (none for environment-wide): the same save the `/meta` door * makes, so the same schema, publish gates and hooks judge it. A save that * loses the race for the name (a row written under it after step 5's read) - * is step 5's 409, in step 5's words. + * is step 5's 409, in step 5's words. [#30255] The copy's grant keys go + * with it as `inheritedGrantObjects`: they are the source's grants, so the + * save's unknown-grant refusal does not judge them as this write's. */ export async function handlePermissionSetCloneWrite( deps: DomainHandlerDeps, @@ -537,6 +549,12 @@ export async function handlePermissionSetCloneWrite( item: clone, parentVersion: null, ...(landing.packageId !== null ? { packageId: landing.packageId } : {}), + // [#30255] The copy's grants are the source's, so the save's + // unknown-grant refusal does not judge them as this write's: a + // platform set may grant on an object of a platform plugin this + // deployment does not install (`member_default` on + // `sys_comment` / `sys_activity`, `@objectstack/plugin-audit`). + inheritedGrantObjects: inheritedGrantObjectsOf(clone), }); } catch (err: unknown) { // [#23577] The create-only pin refused: a row was written under the From d770fe2150f293162ce906de2fb910370edcc6fc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 5 Nov 2026 06:29:52 +0000 Subject: [PATCH 3/3] fix(runtime): the clone door reads the inherited grant keys from the source set, not from the copy Only a key the source already holds is ever passed to the save as inheritedGrantObjects, whatever the copy step does to the copy's objects map (contract review S1). Claude-Session: https://claude.ai/code/session_01Utp6CatT2ajve5PJf1Hhzj Co-authored-by: Claude --- .../src/domains/permission-set-clone.ts | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/runtime/src/domains/permission-set-clone.ts b/packages/runtime/src/domains/permission-set-clone.ts index d06436f9bd2..a46dc08a5e4 100644 --- a/packages/runtime/src/domains/permission-set-clone.ts +++ b/packages/runtime/src/domains/permission-set-clone.ts @@ -243,12 +243,13 @@ export function clonePermissionSetDefinition( } /** - * [#30255] The grant keys of a copy's `objects` map — what the save is told the - * copy inherits ({@link handlePermissionSetCloneWrite} step 7). Every one is the - * source's own key, carried by {@link clonePermissionSetDefinition} unchanged. + * [#30255] The grant keys of the SOURCE's `objects` map — what the save is told + * the copy inherits ({@link handlePermissionSetCloneWrite} step 7). Read from the + * source, not from the copy, so only a key the source already holds is ever + * passed, whatever {@link clonePermissionSetDefinition} does to the copy. */ -export function inheritedGrantObjectsOf(copy: Readonly>): string[] { - const objects = copy.objects; +export function inheritedGrantObjectsOf(source: Readonly>): string[] { + const objects = source.objects; return objects && typeof objects === 'object' && !Array.isArray(objects) ? Object.keys(objects) : []; } @@ -402,9 +403,9 @@ function readCloneBody(body: unknown): { name: string; label: string; package?: * `packageId` (none for environment-wide): the same save the `/meta` door * makes, so the same schema, publish gates and hooks judge it. A save that * loses the race for the name (a row written under it after step 5's read) - * is step 5's 409, in step 5's words. [#30255] The copy's grant keys go - * with it as `inheritedGrantObjects`: they are the source's grants, so the - * save's unknown-grant refusal does not judge them as this write's. + * is step 5's 409, in step 5's words. [#30255] The source's grant keys go + * with it as `inheritedGrantObjects`: the copy's grants are the source's, + * so the save's unknown-grant refusal does not judge them as this write's. */ export async function handlePermissionSetCloneWrite( deps: DomainHandlerDeps, @@ -549,12 +550,13 @@ export async function handlePermissionSetCloneWrite( item: clone, parentVersion: null, ...(landing.packageId !== null ? { packageId: landing.packageId } : {}), - // [#30255] The copy's grants are the source's, so the save's - // unknown-grant refusal does not judge them as this write's: a + // [#30255] The copy's grants are the source's — the keys are + // read from the source itself — so the save's unknown-grant + // refusal does not judge them as this write's: a // platform set may grant on an object of a platform plugin this // deployment does not install (`member_default` on // `sys_comment` / `sys_activity`, `@objectstack/plugin-audit`). - inheritedGrantObjects: inheritedGrantObjectsOf(clone), + inheritedGrantObjects: inheritedGrantObjectsOf(source.definition), }); } catch (err: unknown) { // [#23577] The create-only pin refused: a row was written under the