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,