fix(plugin-security): every member reads and posts comments, and reads the activity history, of the records they can read

.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.

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.

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',