Repository navigation
Commit 6f57888
fix(plugin-audit)!: an engine read of sys_activity returns only the rows whose parent record the caller can read (#21069)
Part of #20833
Clause-②: no (narrowing) — an engine read of one platform object returns
fewer rows to a caller that is not system context. No schema, accept
set, export or stored shape moves: `src/index.ts`, `package.json` and
`src/objects/**` are unchanged, and none of the new module-level names
appears in the built `dist/index.d.ts` / `dist/index.d.mts` (positive
control: `installCommentReadVisibility` does).
## What this changes
`AuditPlugin` mounts a read middleware on `sys_activity`, beside the one
`sys_comment` already has. On `find`, `findOne`, `count` and `aggregate`
it pre-scans the parent pairs (`object_name`, `record_id`) the query
would touch under system context, asks which of those parent records the
caller can read, and ANDs one branch per parent object into the query. A
row is returned only when the caller can read the record it is about.
- **One readability answer, shared.** The comment gate's per-object,
caller-scoped engine read of the parent is extracted from
`comment-access-hooks.ts` into `resolveReadableParentIds`, and both
gates call it. The parent object's own sharing, RLS and object-level
CRUD decide. Nothing derives readability a second way.
- **Batched.** One parent read per parent object per request, never one
per row. A pin counts them.
- **Fails closed**, with the same row classes the comment gate uses for
threads: a row about a record the caller cannot read, about a record
that no longer exists, naming no record or an object the engine does not
know, or naming `sys_activity` itself.
- **Scan window.** The pre-scan keeps the comment gate's 2,000-row bound
and its fail-closed warning. It also carries the caller's own `orderBy`,
so on a larger table the window it scans is the one the caller pages
through, such as a feed sorted by `timestamp`.
- System-context and context-less reads are not narrowed.
## Per door, by class
A real org-bound boot (`bootStack`, `orgContext: true`) with
plugin-audit's CRUD mirror writing the rows. The member holds
object-level `sys_activity` read through one explicit permission set.
The admin is the seeded admin. "Parent readable" means the same caller's
read of the parent record through the data door.
| door | caller | parent readable by the caller | before | after |
|:--|:--|:--|:--|:--|
| list `GET /data/sys_activity` | member | no | rows returned | absent |
| list | member | yes | returned | returned |
| list | member | record no longer exists | returned | absent |
| list `total` | member | (any) | every row (43) | the rows returned
(38) |
| by id `GET /data/sys_activity/:id` | member | no | 200 | 404 |
| by id | member | yes | 200 | 200 |
| filtered query `POST /data/sys_activity/query` (`where`) | member | no
| rows returned | none |
| grouped count `POST /data/sys_activity/query` (`groupBy`) | member |
no | group present | group absent |
| list, by id, filtered query, grouped count | admin | yes (every
existing record) | returned | returned |
| list, grouped count | admin | record no longer exists | returned |
absent |
| export `GET /data/sys_activity/export` | member, admin | (any) | 403 |
403 (the object's API methods, not this gate) |
Measured counts in that boot: member list 43 rows before, 38 after.
Admin list 43 before, 41 after. The admin's two missing rows are both
about one record deleted before the read.
## Pins (committed red, before the fix)
-
`packages/plugins/plugin-audit/src/activity-read-visibility.integration.test.ts`,
commit 97fdaa4. It uses a real kernel, the real `AuditPlugin`, the
real CRUD mirror and a real SQLite driver, and it covers find, findOne,
count, aggregate, a scoped query, the batching count, the fail-closed
classes and an admin. At the pin commit: 8 failed, 4 passed of 12. After
the fix: 12 of 12.
- `packages/qa/dogfood/test/activity-parent-read-gate.dogfood.test.ts`,
commit 2e261af. It covers the public data doors above. Each returned
row is checked by opening its parent through the same door as the same
member. Against the pre-fix build: 4 failed, 2 passed of 6. After the
fix: 6 of 6.
- The fix commit 2afc6a0 adds `activity-read-visibility.test.ts` for
the fail-closed branches, the scan bound, the order pass-through and the
envelope keys. It also adds a mounting assertion to
`audit-plugin.test.ts`.
## Ablation: gate off, red, restore (from the committed fix 2afc6a0)
The mutation deletes the one `installActivityReadVisibility(...)` call
in `audit-plugin.ts` through `scripts/ablation-replace.mjs`: anchor
count 1 → 0, blob `952d943e` → `84a26115`. A trap restores it by
absolute path.
- **Leg A (plugin read path, resolved from source):** the integration
pin plus `audit-plugin.test.ts` gave 10 failed, 12 passed of 22. The
restore was proven: blob equals HEAD `952d943e` and `git diff HEAD` is
empty. Re-run: 22 of 22.
- **Leg B (dogfood door pin, resolved from `dist/`):** after the
rebuild, the JS was emitted. The DTS step refused the now-unused import
(TS6133), and the suite consumes the JS. `ablation-dist-preflight
--absent` proved the call gone from all 4 built files. The dogfood pin
then gave 4 failed, 2 passed of 6. The restore was proven, the rebuild
exited 0, and the preflight proved the call present again. Re-run: 6 of
6.
## Verification
At HEAD 22cc56d, run as one locked script. Each exit code was
captured before any pipe.
- `pnpm --filter @objectstack/plugin-audit test`: 28 files, 392 tests
passed. `pnpm --filter @objectstack/plugin-audit typecheck`: exit 0,
test layer included (`check:test-typecheck` OK, 0 errors).
- Dogfood, at the fix commit's build:
`activity-parent-read-gate.dogfood.test.ts` passed 6 of 6, and
`comments-permission-matrix.dogfood.test.ts` passed 10 of 10. The
comment gate is the one whose readability answer is now shared.
- Gate families derived by `dispatch-gates --commands` (95): 91 ran
green. 4 are NOT MEASURED, each refusing on its own PREREQUISITE NOT MET
(exit 3): `check:dual-build-cjs-loads` and `check:type-check-debt` need
a whole-repo build, `check:i18n` needs the built CLI, and `spec
check:skill-examples` needs the built client SDK. `dispatch-gates
--ran`: 95 accounted for, 0 unrun.
- Lint, a declared narrowing: `eslint --no-inline-config --format json`
over the 7 changed TypeScript files reports 7 files linted, with 0
errors and 0 warnings. An ignored file would report a warning, so none
of them falls outside the config's population. `eslint.config.mjs` sets
no `parserOptions.project` and no cross-file import rule, so this diff
cannot move a verdict on an untouched file. The whole-repo `pnpm lint`
runs in CI.
## Acceptance notes
- **The card stays open, so this PR says `Part of`.** One read door of
this object answers outside the engine's middleware chain, so an engine
middleware does not reach it. That door was measured on the same boot
and reported to the dispatching seat. It is not described here, and
closing it is outside this package.
- **Admins.** An admin reads every row about a record that exists,
exactly as before. Rows about a record that no longer exists are now
excluded for every caller, admins included. The comment gate does the
same with a deleted record's thread. The deletion itself stays in
`sys_audit_log`, which this gate does not touch. Whether admins should
keep those rows is raised with the seat. The security service gives this
object no per-object read-bypass answer to key on, measured on
`/auth/me/permissions` for the seeded admin.
- **Surface addition:** `content/docs/permissions/system-context.mdx`,
row 45. `check-system-context-census` requires every `isSystem` read
site to carry an anchor, and the new middleware's early return is one.
The declared counts were regenerated by `gen:system-context-census`.
- Not measured: whether a caller who can read a parent record, but not
every field of it, sees the before and after values an activity row
records. That is field-level and is not this record-level gate's
question.
- Not measured: `GET /search` returned no `sys_activity` hits for either
principal, before or after, in this boot. That reading says nothing
about the gate.
- Not measured: a boot with more than one organization.
- Not measured, whole-repo build prerequisites (CI runs them):
`check:dual-build-cjs-loads`, `check:type-check-debt`, `check:i18n`
(built CLI), and `pnpm --filter @objectstack/dogfood typecheck`. Its 40
errors locally are all TS2307 for unbuilt packages plus one downstream
TS7006, and none is in the added file.
- Main moved after the branch point. The four gate inputs it changed are
not on this PR's paths. The gates ran against this branch's copies.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent dff98c1 commit 6f57888
9 files changed
Lines changed: 886 additions & 31 deletions
File tree
- .changeset
- content/docs/permissions
- packages
- plugins/plugin-audit/src
- qa/dogfood/test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
138 | 138 | | |
139 | 139 | | |
140 | 140 | | |
141 | | - | |
| 141 | + | |
142 | 142 | | |
143 | 143 | | |
144 | 144 | | |
| |||
161 | 161 | | |
162 | 162 | | |
163 | 163 | | |
164 | | - | |
| 164 | + | |
165 | 165 | | |
166 | 166 | | |
167 | 167 | | |
| |||
281 | 281 | | |
282 | 282 | | |
283 | 283 | | |
284 | | - | |
| 284 | + | |
285 | 285 | | |
286 | 286 | | |
287 | 287 | | |
| |||
355 | 355 | | |
356 | 356 | | |
357 | 357 | | |
358 | | - | |
| 358 | + | |
359 | 359 | | |
360 | 360 | | |
361 | 361 | | |
362 | | - | |
363 | | - | |
| 362 | + | |
| 363 | + | |
364 | 364 | | |
365 | 365 | | |
366 | | - | |
367 | | - | |
| 366 | + | |
| 367 | + | |
368 | 368 | | |
369 | 369 | | |
370 | 370 | | |
| |||
428 | 428 | | |
429 | 429 | | |
430 | 430 | | |
431 | | - | |
| 431 | + | |
432 | 432 | | |
433 | 433 | | |
434 | 434 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
0 commit comments