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

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

domain:server#2 · os-bill · session_01Utp6Cbvcmv8YWFhFJwzjKL

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: d22f921b6a393db376a563ab3c47fbe92374e5df
Local-runs: none
Scope: no hits (reviewFace on the five committed paths names no schema and no governed path; the tier is served above the TIER_DEFAULT floor because triage 6633820272 sent a permission-baseline widening to contract review) — reviewed whole: packages/plugins/plugin-security/src/objects/default-permission-sets.ts, packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts, packages/plugins/plugin-security/src/member-default-feed-grants.integration.test.ts, packages/plugins/plugin-security/src/comment-reaction-own-record-floor.test.ts, .changeset/30108-member-default-feed-grants.md + packages/plugins/plugin-audit/src/comment-access-hooks.ts, packages/plugins/plugin-audit/src/activity-read-visibility.ts, packages/plugins/plugin-audit/src/parent-record-read-gate.ts, packages/plugins/plugin-audit/src/audit-writers.ts (enforceFeedsCapability), packages/core/src/security/resolve-authz-context.ts (§5b), packages/plugins/plugin-security/src/external-baseline.ts, packages/plugins/plugin-security/package.json (exports), content/docs/ui/pages.mdx, content/docs/frontend/plugins/plugin-detail.mdx

Inputs: card #30108 (body and all four comments, the ruling 6633054352 included), PR #30222 (body, file list, git diff origin/main...d22f921b), the head's check-runs read through the API. Not read as evidence: the dispatch order or the seat's own conclusions; the dev report's measurements were re-derived from the code below.

① Derived judgments

1. What the baseline now grants — exactly the ruling, nothing more. member_default.objects gains sys_comment: { allowRead, allowCreate, allowEdit: true, allowDelete: false } and sys_activity: { allowRead: true, allowCreate/Edit/Delete: false } (default-permission-sets.ts, the two lines after the new [#30108] block). No allowExport, viewAllRecords or modifyAllRecords key on either; no '*' entry added (the [#5491] NO-WILDCARD note and maintainer ruling of 2026-08-07 in the same object literal still hold, and the pre-existing pin expect(Object.keys(MEMBER_DEFAULT.objects)).not.toContain('*') is untouched). ADR-0090 D5 anchor-safety holds: no delete bit anywhere in the set; the new explicit-allow block pins all four of these per object. Correct.
2. Row access stays derived from the parent record; no row policy is added. Read side: installCommentReadVisibility (comment-access-hooks.ts) is a data middleware on find/findOne/count/aggregate that ANDs computeThreadVisibilityFilter into ctx.ast.where and, on any failure, ANDs READ_DENY_ALL (fails closed); installActivityReadVisibility (activity-read-visibility.ts) does the same over PARENT_GATE_READ_OPS with computeParentRecordFilter, excluding rows that name no parent. Write side: onInsert refuses unless callerCanRead(target) (RECORD_NOT_ACCESSIBLE), stamps author_id from the session; the beforeUpdate hook requires author-or-parent-EDIT and re-checks the new parent on a re-thread. The baseline adds no select/insert policy for either object (pinned: policiesFor(object,'select'|'insert') is []), which is right: neither object has a column a using predicate could scope by, so a baseline row policy would have replaced the gate's answer with a wrong one. Correct.
3. The edit limb. The only update-class policy on sys_comment is the wildcard owner_only_writes (created_by == current_user.id, positions: ['org_member']); sys_comment_moderation is the delete limb only (operation: 'delete', by the file's own c25b2d5 note). So for an org_member an edit reaches the comments they wrote, as the PR claims and block 1 of the integration test measures (403 PERMISSION_DENIED on another's comment). One reading to carry: the floor binds org_member only; outside that domain (admin sets, which already hold '*' edit; an org-less everyone-only session) the bound is the comment gate's own author-or-parent-EDIT rule, which is plugin-audit's documented rule and not something this diff widens. Not a must-fix.
4. External members are not reached — verified in code, and the ruling's premise is wrong, not the implementation. resolve-authz-context.ts §5b: if (!holdsExternalIdentity(grants.positions) && !grants.positions.includes('everyone')) grants.positions.push('everyone'); humanBaselineFor (external-baseline.ts) returns [external_default] for a holder of org_external in place of the configured baseline; external_default names neither object (pinned in the explicit-allow test). So "member_default also reaches everyone, including external members" (card risk paragraph, ruling condition 1) is false on main and on this head; the condition's intent (the grant widens nothing for externals) is met more strongly than it was written. Reporting this as a corrected premise rather than a fork is right: the grant is not narrowed, option B is not taken, and the external path is measured both ways (baseline-only external refused on both objects; explicit-grant external scoped to its own ticket through the same gates).
5. The feeds switch and the ruling's "both verbs". enforceFeedsCapability (audit-writers.ts) is registered on beforeInsert and beforeUpdate for sys_comment only, keyed off the thread_id target object's enable.feeds === false; it has never been registered on a read, and neither installCommentReadVisibility nor commentThreadGate() consults enable.feeds. The PR's statement that reads are not gated by the switch, before or after, is true. Judgment under ③.
6. Log noise (Every console page load by a non-admin logs ~16 "[Security] Access denied" WARN lines: the header activity feed's GET /api/v1/data/sys_activity fans out to objects the caller cannot read and logs each refusal #23279). parent-record-read-gate.ts asks the security service's canReadObject for each parent object before any caller-scoped read of it ("A skipped object is not an access attempt"), so a member's sys_activity feed read refuses unreadable parent objects as $in: [] without logging [Security] Access denied. Integration block 5 arms the measurement both ways (a direct refused read logs exactly once; the feed with canReadObject withdrawn logs per parent). Holds.
7. Public accept set / export. @objectstack/plugin-security exports . only; no symbol is added (EXTERNAL_DEFAULT_PERMISSION_SET already exists on main, line 227) and no type, schema, route or packages/spec path changes. What changes is the value of the shipped defaultPermissionSets constant: a runtime permission behaviour, ruled by the maintainer. Clause-②: no is the right declaration and Metadata-protocol: none is right. Permission sets merge most-permissively, so an app already naming the two objects (HotCRM) keeps its own bits, delete included; no regression path.
8. Residual widening to name plainly. viewer_readonly and the admin sets already read both objects through their '*' read; after this PR every org_member does too. The row set they see is the same gate-derived set. The one observable new reach is item ③.1.

② Semver level

.changeset/30108-member-default-feed-grants.md: '@objectstack/plugin-security': patch. The diff publishes no API shape change — a changed default value of a shipped constant, under a maintainer ruling that fixes a declared-not-enforced capability (ADR-0049 shape, as the card frames it). patch is consistent with AGENTS.md's "a package whose shipped bytes change takes a patch changeset" and with the fix framing; minor was arguable for an access-widening default but is not owed. The Clause-②: and Metadata-protocol: lines are present and identical in the PR body and the changeset; Check Changeset is green on the head. Accepted.

③ Boundary flags

1. Open question — does "both verbs" allow reads of pre-existing comments on an enable.feeds: false object? Ruling: the dev's reading satisfies the condition; it is not a fork. The condition reads "An object with enable.feeds: false still refuses on both verbs" — still, i.e. a verification that the switch's existing refusals survive the grant. The switch has refused exactly two verbs since [spec/cli] 对象 enable.files/trackHistory/activities/feeds 为 dead property:schema 接受、liveness 仅告警不拦 #2707/enforceFilesCapability / enforceFeedsCapability are beforeInsert-only — a re-point via update dodges the capability opt-in on the NEW parent object #10170, create and re-thread, and those are the write verbs this grant opens; both are measured refusing a member with 403 FEEDS_DISABLED on a record that member reads and edits (integration block 4). It never refused a read, so no refusal was lost, and the documented behaviour (content/docs/ui/pages.mdx: the object "suppresses the panel ... and the server refuses comments on such an object") speaks of refusing comments, not of hiding grandfathered rows. Adding a read refusal to the switch would narrow it for every role (viewer_readonly, admin sets, every app that grants sys_comment today), which the ruling did not order and a PR executing option A must not bundle (Prime Directive chore: version packages #10, no scope expansion). What IS new after this PR: an ordinary member can list a feeds-off object's pre-existing comments through the data API (never through the record page, which suppresses the panel), under the parent gate. That is a residual the maintainer has not ruled on explicitly, not a failed condition. Disposition: dev option B — the lane seat files a separate decision card on the switch ("should enable.feeds: false also hide grandfathered comments from readers?") naming this record, and names it in the landing ACCEPT. No hold on this PR; option C (put it to the maintainer before landing) is not warranted because landing changes nothing about that read for any role that already held it.
2. Reactions. member_default does not name sys_comment_reaction; the ruling names two objects and a third would be scope expansion. The record page keeps the reaction control gated on create on sys_comment_reaction (plugin-detail.mdx, "A mounted discussion context"). Correct to leave; the dev's dedupe to the open Comment reactions (ruling A amended on #22505): a reaction is the reactor's own sys_comment_reaction record, and sys_comment.reactions retires with no aggregate and no data migration #22566 decision is right, and the gap (members can comment but not react) is a UX residual the card filed there should carry.
3. The corrected premise of condition 1 (external members) — judged in ①.4: holds; the deviation line is honest and the grant is not narrowed.
4. Ruling conditions, one by one: (a) external parent gates hold — yes, and the baseline does not reach externals at all; (b) enable.feeds: false still refuses both registered verbs — yes; (c) no Every console page load by a non-admin logs ~16 "[Security] Access denied" WARN lines: the header activity feed's GET /api/v1/data/sys_activity fans out to objects the caller cannot read and logs each refusal #23279 log noise — yes. No fork to report; the grant is the ruling's grant.
5. Author: PR user.login is objectstack-fleet[bot] (relay-written); no off-relay author to name.
6. Governance tier: the committed file list touches no GOVERNED_SURFACES row (Governed Surface Queue Guard success); this record is the contract review triage owed for a permission-boundary widening, not a Tier S landing key. The PR is a draft with no review and no auto-merge armed.
7. Checks on d22f921b, read through the commits check-runs API: all 48 runs concluded — 35 success, 13 skipped (path-filtered UI, docs, dogfood-matrix, opt-in and schedule-gate jobs), 0 failing, 0 in progress — at the last read before this post (all eight Test Core shards and Type Check · workspace were still in progress at the first read, 02:34Z, and concluded success by 02:45Z). Nothing was re-run locally. The PASS below is not conditioned on any check.

④ Design judgment

• default-permission-sets.ts: the change holds against the code it cites. The new block names the two gates by file and states each one's rule accurately (comment list/read/post on a readable parent; edit author-or-parent-editor under the owner_only_writes floor; activity rows served only for readable records, unreadable parent objects skipped through canReadObject), names the switch's two verbs correctly, states the anchor rule and the external baseline correctly (§5b, humanBaselineFor). The rewritten inbox-pair sentence removes the contradiction the old line 804 would have left ("deliberately NOT included") without erasing why sys_activity got no _self policy. The set remains explicit-allow and anchor-safe.
• member-default-explicit-allow.test.ts: the flipped sys_activity negative is replaced by a _self-policy pin and a positive block that pins the shape that would quietly widen: no delete/export/view-all/modify-all, no baseline select/insert policy on either object, edit limb exactly ['owner_only_writes'], external_default names neither. That is the right thing to pin at this layer.
• member-default-feed-grants.integration.test.ts: drives the real SecurityPlugin, AuditPlugin, SQL driver and the real resolveAuthzContext / assembleExecutionContext with an app set naming only the app's own objects (the card's create-objectstack shape). Blocks 1–5 are the card's scenario (requester on own ticket by row scope; unreadable by row scope and by object; both external cases; feeds-off create and re-thread; the console feed's denial count armed both ways). The technician-with-viewAllRecords row of the card's table is not driven, but the gates defer to the parent's read scope (ReadableIds leaves), so nothing in this diff decides it differently; not a must-fix.
• comment-reaction-own-record-floor.test.ts: header prose only, and the new sentence is accurate (baseline grants read/create/edit; the delete bit stays app-granted).
• Changeset: the five bullets restate ①.1–②; the "an app that already grants the two objects keeps working" bullet is the most-permissive-merge fact. No (narrowing) arm, correctly.
• No must-fix found. Follow-ups for the lane seat: the ③.1 decision card; the ③.2 residual stays on Comment reactions (ruling A amended on #22505): a reaction is the reactor's own sys_comment_reaction record, and sys_comment.reactions retires with no aggregate and no data migration #22566.

Implemented-by: claude/project-thread-24iijl
Reviewed-by: session_01Utp6Cbvcmv8YWFhFJwzjKL

VERDICT: PASS

domain:server#2 · os-bill · session_01Utp6Cbvcmv8YWFhFJwzjKL

维护者速读

• 改了什么： 平台基线 member_default 加两行授权：sys_comment（读、建、改）和 sys_activity（读）；无删除位、无导出、无行策略。
• 为什么改： 评论和动态在每个对象上默认开启，但没有任何出厂权限集授予这两个对象，新应用的普通成员在自己能看的记录上只看到「没有权限」。您已「同意 A」。
• 风险与回滚： 能看哪些行仍由父记录门决定；外部成员拿不到这个基线；enable.feeds: false 的对象仍拒新评论和挪线程；动态读取不刷拒绝日志。回滚即撤本 PR。
• 先看： default-permission-sets.ts 新增两行及上方注释，再看 member-default-feed-grants.integration.test.ts。
• 两分钟验证： pnpm --filter @objectstack/plugin-security exec vitest run src/member-default-feed-grants.integration.test.ts，20 条全绿。
• 需要您做的： 无，席位经合并队列落地（非受管面，620 行）。
• 席位意见： ACCEPT。三条裁决条件逐条实测成立，契约复审 PASS（6648282410）。一个残留写在下方：关了 feeds 的对象上，关之前留下的旧评论现在普通成员可经 API 读到（页面不显示），这是开关本来就不管读，不是本 PR 收窄或放宽的问题。

席位核对 — ACCEPT · domain:server#2 · head d22f921b6a393db376a563ab3c47fbe92374e5df

• 形态: draft, author objectstack-fleet[bot] (relay pr_create), no assignee, no review request; 5 files, +607 −13; check-governed-merges --pr 30222: NOT governed, 620 lines ≤ 3000. Write tally reported by the dev: 4 writes, mcp_calls 0.
• 前提实测: at 31fe289f member_default.objects named neither object (claim 6646463911 Premise: holds); the diff adds exactly the two rows the ruling names. Seat read the diff of default-permission-sets.ts, the changeset, and the flipped pins; owner_only_writes is the only update-class limb on sys_comment; no '*', no allowDelete, no export / view-all / modify-all.
• 门禁 check-runs: 48 runs on the head, 35 success, 13 skipped, 0 failing. Dev: dispatch-gates 60/60, --ran 0 NOT-MEASURED, pnpm lint green, 8 ablations red-then-restored.
• 契约复审: CONTRACT_REVIEW_TIER PASS on this head, comment 6648282410 (served above the path floor because triage 6633820272 sent a permission-baseline widening to review).
• 接受的偏差: condition 1's premise corrected (the baseline never reaches org_external; measured on the real resolver), not a fork. Condition 2 read as the switch's two registered verbs (create, re-thread); the reviewer ruled that reading satisfies "still refuses", and the seat agrees: the switch never gated reads, so no refusal was lost and the grant is not narrowed. Full pnpm build ran outside the verify lock (single-agent container).
• 范围外去向: (1) reads of pre-existing comments on a feeds-off object by a member — Acceptance notes (documented switch behaviour; record page hides the panel; a read refusal would change the switch for every role, which is not this card); (2) sys_comment_reaction not in the baseline — carried by the open decision Comment reactions (ruling A amended on #22505): a reaction is the reactor's own sys_comment_reaction record, and sys_comment.reactions retires with no aggregate and no data migration #22566, not filed again; (3) none else. Skills-docs: not owed (Metadata-protocol: none).
• 落地条款: Clause-②: no (claim line-start key), Metadata-protocol: none, changeset patch for @objectstack/plugin-security. Ordinary queue landing by this seat: pr_ready + automerge_enable on this head.