You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit c3fb185
Browse filesBrowse the repository at this point in the historyBrowse files
fix(plugin-audit): a mention notification reaches a mentioned user only when they may read the comment it previews (#26497)
Fixes#26453
Clause-②: no
Metadata-protocol: none
## 维护者速读
- **改了什么**:@提及通知现在要过两道读权限:被提及者既要能读这条评论所在的记录,也要能读这条评论本身。读不到评论的人收不到这条通知。
-
**为什么改**:通知里带着评论正文的摘要。能读记录不等于能读评论,因为评论对象有自己的对象权限、租户墙和行级范围。铃铛里的这份副本不能送到评论区本身不给看的人手里。这是安全类卡,细节按维护者裁决不公开。
- **风险与回滚**:如果某个应用给成员的权限集没有授予评论读权限,这些成员以后收不到 @提及通知,这是本修复的预期效果,changeset
里写了管理员该怎么调整。回滚就是 revert 本 PR:只动 `@objectstack/plugin-audit`
一个包,没有数据迁移,也没有 spec 变更。
- **先看**:① `packages/plugins/plugin-audit/src/mention-recipient-gate.ts`
里逐收件人判定循环新增的评论读取那一行(`readableIn(..., COMMENT_OBJECT, commentId)`);②
`mention-recipient-gate.integration.test.ts` 里新增的 DAVE 用例和 `sys_comment`
读权限替身;③ `.changeset/26453-mention-recipient-reads-comment.md`。
- **两分钟验证**:先构建依赖闭包 `pnpm --filter '@objectstack/plugin-audit^...'
build`,再跑 `pnpm --filter @objectstack/plugin-audit exec vitest run
--maxWorkers=2 src/mention-recipient-gate.integration.test.ts`,应该看到
15/15 通过。
- **需要你做的**:无,席位落地。
## What changes
- **`mention-recipient-gate.ts`**: the gate used to decide each
mentioned user on one read, the record the comment is on. It now also
runs that user's own read of the comment, by id, through
`resolveReadableParentIds`, the same evaluator the comment-reaction
create door asks of a comment. The read runs in the user's resolved
envelope, with their organization claim vetted exactly as before. When
that read does not return the comment, the user is withheld and logged
with the new reason `cannot_read_thread`. A comment with no id withholds
every candidate (`no_comment_id`). The record read stays in place and
runs first, for two reasons: the deep link opens the record, and an
engine with no middleware seam has no thread read gate to fold the
record question into the comment read. The module header is rewritten so
that what it claims is true. The old header said a thread is readable
exactly when its record is, and that is wrong, because the comment
object carries its own grant.
- **`comment-access-hooks.ts`**: `COMMENT_OBJECT` is exported from this
internal module so the gate names the object from one source. The
package index is unchanged.
- **`audit-writers.ts`, `audit-plugin.ts`**: the doc comments now
describe the two-read rule. No code changes in either file.
- **Changeset**: `patch` on `@objectstack/plugin-audit`.
## Tests
`mention-recipient-gate.integration.test.ts` runs a real kernel,
ObjectQL, SQLite, `AuditPlugin` (including its comment read gate) and
the real grant resolver.
- **New stand-in**: plugin-security's object-level read grant on
`sys_comment`. It is keyed on a position assigned through
`sys_user_position`, which the real resolver reads, and it refuses the
way the real layer does: a throw carrying `PERMISSION_DENIED` / 403. A
new fellow member, DAVE, can read the open record but does not hold that
position.
- **New pins**:
- a control: the resolver projects the grant to CAROL and not to DAVE,
both read the record, and only CAROL reads the comment, while DAVE's
read is refused with `code`/`status`;
- a recipient who can read the record but not the comment is withheld,
by id and by email;
- only the recipient who can read both is notified, and the author never
is;
- the single posture behaves like a walled one;
- the withheld-recipient log carries `cannot_read_thread` and
`no_comment_id`, and never the comment text.
- **Adjusted pin**: the existing direct-call log pin now reads a real
comment id. Before, it passed a non-existent one, which the comment read
now correctly refuses.
Ablation: deleting the comment-read limb turns exactly the 5 new gate
pins red. The other 10 stay green, including the stand-in control. The
file was then restored, and its blob matches HEAD.
<details><summary>Pin, package suite, typecheck (head
88a2b6c)</summary>
```text
pnpm --filter @objectstack/plugin-audit exec vitest run --maxWorkers=2 src/mention-recipient-gate.integration.test.ts
Test Files 1 passed (1) · Tests 15 passed (15) · VERDICT command-exit 0 (912eeac; file unchanged since)
pnpm --filter @objectstack/plugin-audit test
Test Files 64 passed (64) · Tests 1028 passed (1028) · VERDICT command-exit 0 (88a2b6c)
pnpm --filter @objectstack/plugin-audit typecheck
tsc --noEmit · tsc -p tsconfig.scripts.json · check:test-typecheck OK · VERDICT command-exit 0 (88a2b6c)
tsc --listFiles: tsconfig.test.json compiles both the gate and its integration test
```
</details>
<details><summary>Ablation, engine layer</summary>
The test imports its subject through relative `src` paths, so no rebuild
was involved.
```text
node scripts/ablation-replace.mjs --delete (the comment-read `else if` line of the verdict loop)
anchor 1 -> 0 · blob 1ee01f8 -> 47e854c54865
Tests 5 failed | 10 passed (15) the five new gate pins red; the stand-in control green
restore: blob == HEAD (1ee01f8) · git diff HEAD empty
```
</details>
<details><summary>One-time check in a real composition (throwaway, not
committed)</summary>
A throwaway dogfood file booted `bootStack` with the real
plugin-security, plugin-audit and messaging, org-bound, and drove the
gate through the REST layer. All three legs ran at 88a2b6c:
- **With the fix**: the gate admitted exactly the recipient who may read
both the comment and the record.
- **Mutated leg**: the comment-read limb was deleted and plugin-audit
rebuilt. `ablation-dist-preflight --absent` showed the limb absent from
`dist/`. The gate admitted the record-only recipient too, and that was
the one red assertion. The rebuild exited 1 at its DTS stage after
writing the JS bundles the suite consumes, so the `dist/` preflight
reading, not the build's exit code, certifies this leg.
- **Restore leg**: the rebuild exited 0, the limb was present in `dist/`
again, and the result was green again.
The throwaway file was then deleted, and `git status --porcelain` is
empty.
</details>
## Acceptance notes
- **Behaviour for deployments**: a mentioned user is notified only when
the sets they hold grant read on the comment as well as on the record.
The changeset tells administrators how to restore notifications for a
population that should take part in record discussions: grant comment
read in a set that population holds.
- **Producer sweep**: the mention writer in `audit-writers.ts` is the
only producer that copies `sys_comment` text into a notification.
`approval.comment` in plugin-approvals notifies an approval request's
own participants about approval-action comments. That is a different
object with its own audience, and it is not in scope here.
- **Cost**: one extra caller-scoped read per mentioned candidate who
passes the record read.
- **Timing**: the comment is read from inside the author's write, in
`afterInsert`. The engine binds reads there to the write's own unit of
work (the ambient transaction store), so the row is visible. A read that
cannot see the row withholds, which is the fail-closed direction. The
existing CAROL pins exercise this path end to end.
- **Not run locally**:
- `test:repo` (the vitest `repo` project): it belongs to CI's `Test
Core`, and this diff changes no test in that project.
- Full dogfood: the title has no `!` and there is no `(narrowing)` arm.
- `packages/cli` integration tier: not touched.
## Verification
Local subset, run at head `88a2b6cd2d`. The families CI runs are read
from this PR's check-runs, not re-run here.
<details><summary>Derived gate list: 64 of 64 exit 0</summary>
`node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--commands` derived 64 commands for the 6 changed paths. Each ran
sequentially from the worktree root, with its exit code captured before
any pipe. Reconciliation (`--ran`, exit 0): `64 derived famil(ies)
accounted for — 64 run, 0 NOT-MEASURED`.
- `check:dual-build-cjs-loads` first read exit 1. At that moment a
concurrent closure build of mine was rewriting `dist/`, and the finding
was a missing CommonJS file of a package this diff does not touch.
Re-run on the quiet tree, it exited 0: 112 entry points across 69
packages load.
- An earlier run at `428f1f9c20`, before the merge of `main`, had 62
commands at exit 0 and 2 at exit 3 (PREREQUISITE NOT MET:
`check:dual-build-cjs-loads` and `check:i18n`). Both are measured green
at `88a2b6cd2d`.
```text
node scripts/check-adr-0087-registration.mjs --base origin/main 0
node scripts/check-adr-0087-registration.mjs --self-test 0
node scripts/check-changeset-no-major.mjs --base origin/main 0
node scripts/check-ci-filter-parity.mjs 0
node scripts/check-closing-keyword-parity.mjs (+ --self-test) 0 0
node scripts/check-comment-mask-adoption.mjs (+ --self-test) 0 0
node scripts/check-comment-mask-corpus.mjs 0
node scripts/check-dts-emitted.mjs --self-test 0
node scripts/check-empty-changeset.mjs --base origin/main (+ --self-test) 0 0
node scripts/check-issue-citations.mjs 0
node scripts/check-plugin-teardown-shape.mjs (+ --self-test) 0 0
node scripts/check-tenant-audit-census.mjs (+ --self-test) 0 0
node scripts/ci/assert-turbo-remote-cache.mjs --self-test 0
node scripts/ci/plan-test-core-shards.mjs --self-test 0
node scripts/ci/prune-turbo-cache.mjs --self-test 0
node scripts/docs-audit/check-affected-docs.mjs 0
node scripts/docs-audit/check-drift-comment.mjs 0
node scripts/pm/release-rehearsal-clone.mjs --self-test 0
node scripts/release-pending-publish.mjs --self-test 0
pnpm --filter @objectstack/spec run check:duration-unit-keys 0
pnpm check:changeset-gate-self-tests · check:closed-ceremony-text 0 0
pnpm check:cross-package-test-inputs · check:dispatcher-error-vocabulary 0 0
pnpm check:doc-authoring · check:driver-memory-census · check:dts-closure 0 0 0
pnpm check:dual-build-cjs-loads (re-run on the quiet tree) 0
pnpm check:durability-log-level · check:engine-double-contract 0 0
pnpm check:error-status-conformance · check:gitlink-declared 0 0
pnpm check:i18n · check:i18n-stale-fill · check:issue-citations 0 0 0
pnpm check:keyed-text-bounds · check:lean-entry-closure 0 0
pnpm check:logger-receiver-detach · check:nul-bytes 0 0
pnpm check:objectql-double-limit · check:org-identifier 0 0
pnpm check:page-declaration-shape · check:platform-object-tenancy-census 0 0
pnpm check:pm-changeset-deadline-census · check:published-files 0 0
pnpm check:query-options-erasure · check:refd-timer-probe 0 0
pnpm check:registry-log-declared · check:rest-log-spy-declared 0 0
pnpm check:slot-lookup · check:sourcemap-no-sources-content 0 0
pnpm check:system-context-census · check:test-source-alias 0 0
pnpm check:tier-file-adoption · check:type-check-coverage 0 0
pnpm check:type-check-debt · check:undeclared-dep-imports 0 0
pnpm check:watch-hint-literal · check:where-matcher 0 0
```
</details>
<details><summary>Narrowed spot readings</summary>
```text
require('packages/plugins/plugin-audit/dist/index.js') -> AuditPlugin, installAuditWriters: function exit 0
import('packages/plugins/plugin-audit/dist/index.mjs') -> AuditPlugin: function exit 0
grep -naP control bytes over the six changed files no match
```
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_012YjcDon6BvR9MN93ZsFq3U)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
plugin-audit: a mention notification reaches a mentioned user only when they may read the comment it previews, as well as the record it links
6
+
7
+
Clause-②: no
8
+
Metadata-protocol: none
9
+
10
+
The `@mention` recipient gate decided each mentioned user on one read: may they open the record the comment is on. The notification also carries a preview of the comment's text, and being able to read a record does not mean being able to read its comments, because the comment object has its own object-level grant, tenant wall and row scope. The gate now also runs each recipient's own read of the comment, by id. It uses the same evaluator as the comment read path (`resolveReadableParentIds`, in the recipient's resolved envelope), and it withholds the notification when that read does not return the comment. Each withheld recipient is logged at `warn` with a new reason: `cannot_read_thread`, or `no_comment_id` when the comment has no id to read it by. The comment text is never logged.
11
+
12
+
For administrators: a mentioned user is notified only when a permission set they hold grants read on comments. To notify a population that should take part in record discussions, grant comment read in a set it holds.
0 commit comments