diff --git a/.changeset/30108-member-default-feed-grants.md b/.changeset/30108-member-default-feed-grants.md deleted file mode 100644 index 91be2f6bb0a..00000000000 --- a/.changeset/30108-member-default-feed-grants.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -'@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 9cb68c4866c..b9473b85fd3 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,9 +33,8 @@ * ## 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 the - * `sys_comment` delete bit; the baseline grants comments' read, create and - * edit, never delete), never from the `everyone`-bound baseline, which + * position-distributed set an application ships (as an app grants + * `sys_comment` today), 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 b665092348a..68beec7aece 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, EXTERNAL_DEFAULT_PERMISSION_SET } from './objects/default-permission-sets.js'; +import { defaultPermissionSets, BETTER_AUTH_MANAGED_OBJECTS } from './objects/default-permission-sets.js'; const evaluator = new PermissionEvaluator(); const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; @@ -267,10 +267,13 @@ 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` 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. + 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); + } const names = (MEMBER_DEFAULT.rowLevelSecurity ?? []).map((p: any) => p.name); expect(names).not.toContain('sys_activity_self'); }); @@ -288,60 +291,6 @@ 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 deleted file mode 100644 index ac307a2cad8..00000000000 --- a/packages/plugins/plugin-security/src/member-default-feed-grants.integration.test.ts +++ /dev/null @@ -1,481 +0,0 @@ -// 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 af3bc8d250f..9d257f01074 100644 --- a/packages/plugins/plugin-security/src/objects/default-permission-sets.ts +++ b/packages/plugins/plugin-security/src/objects/default-permission-sets.ts @@ -801,58 +801,13 @@ 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` 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 + // `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 // `_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,