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 b513343
Browse filesBrowse the repository at this point in the historyBrowse files
fix(plugin-approvals): lookup-target titles and the record-lock refusal title are served only from fields the caller may read (#23629)
Fixes#23618
Clause-②: no
The two approval-path titles this card enumerates are now served only
from fields the caller may be served, by the served-fields rule PR
#23614 applied to the subject's `record_title`, asked through one seam.
Function level only, per the card.
## What changed
All in `packages/plugins/plugin-approvals`.
- **Row 1: lookup-target titles (`payload_display`).** `enrichRows` now
titles each record a readable lookup points at from the TARGET record as
served to the reader.
- `ApprovalService.servedFields` is now the one place a row read asks
`resolveReadableSnapshotFields` (readable, and not masked for this
caller), as the reading caller, once per object per read.
`servedSubjectFields` is that method for the rows' subject objects.
- `enrichRows` asks it for the target objects into the same map, so a
target that is also a subject object (a self-lookup) reuses the
subject's answer and is not asked twice.
- `titlesAsServed` is now the one spelling of how an approval read
titles a record: read under the system context, `redactSnapshot(record,
served)`, then the existing `pickTitle`. The subject's `record_title`
(`resolveSubjectDisplays`) and the targets' `payload_display` both call
it, so the two cannot drift.
- What a reader gets: a target display field the reader may not read, or
is served masked (#20964), gives that key no display value, and the
snapshot's stored id stands. That is the answer a deleted or
exposure-refused target (#22738) already gets. A title-eligible field
the reader may read on the target still names the record, as for the
subject (PR #23614, pin f). A reader who may read the display field gets
the same title as before. The exposure gate is unchanged and still runs
first, so a refused target is neither read nor asked.
- **Row 2: the `RECORD_LOCKED` refusal (`lifecycle-hooks.ts`).** The
per-record sentence now picks its title from the update pre-image as
served to the WRITER.
- The ask is the same seam (`resolveReadableSnapshotFields`, then
`redactSnapshot`), made with the writer's own execution envelope: the
one riding on the operation (`input.options.context`), stripped of its
operation-private keys with `@objectstack/core`'s
`withoutOperationPrivateKeys`. ⛔ It is never rebuilt from the hook
session, which drops `permissions` and `posture`; a question asked with
that projection would be a second derivation of the writer's authority.
- A display field the writer may not be served gives no title, and the
sentence degrades to the object's label (`This Opportunity is locked
...`). That is the degradation #18153 already defines; ⛔ never the id. A
writer who may read the field is told the title as before.
`RECORD_LOCKED` and 409 do not move.
- The cost is one field-visibility question on the deny path only, and
no record read. A write the lock does not refuse asks nothing.
- `bindApprovalLockHook` takes a lazy field-visibility getter.
`approvals-plugin.ts` passes the same lazily resolved `security`
authority the snapshot redaction middleware reads. With none wired, the
title passes through as before, exactly as the payload snapshot does.
- **Comment only (bounded in-place change, triage ruling `6185594767` on
#23556, option B).** `nameSubjectRecord`'s inline `[#23594]` comment
named a condition PR #23614 already meets in the inbox. It now gives the
real reason the `approval.requested` body stays label-and-id: it is
written once at send time, with no per-recipient reader to narrow for.
Two comment lines; the return value is unchanged. No test and no
changeset line of its own.
## The dispatch's mechanism assumptions, measured
1. **Row 1: held.** The lookup keys come from the reader-redacted
snapshot. On the base, the target record was titled from the whole
record under the system context, gated only by the target's declared
exposure. A restricted reader now gets no display value for that key.
The console's inbox card drops a reference row the server could not
resolve when it has the object's field metadata, and otherwise shows the
stored text: the same answer a deleted target gets (read in `objectui`
at `28fb2a0`, `payloadSummary`). This change does not move that.
2. **Row 2: the pre-image is the raw row, and the leak is reachable.**
- By source, `readUpdatePriorRow` (`@objectstack/objectql`) is a raw
driver read shaped to the declared columns. No read middleware runs on
it and nothing in `plugin-security` touches `previous`.
- Measured with the real `SecurityPlugin` and a real engine and driver,
in a throwaway rig in `plugin-security`'s tree (deleted, never
committed).
- A writer whose permission set withholds the display field (the read
door confirmed it withheld) was refused, on the base's wiring, with that
field's value in the sentence. The hook's `previous` carried the field.
`input.options.context` carried `permissions` and `posture`, which the
session does not.
- With this change: that writer is refused with the label-only sentence,
on a by-id and on a predicate write. A writer who may read the field
still gets the title.
3. **No whole-record or cross-principal leak in either row.** Both are a
title-level disclosure to a caller already on the path: a routed
approver reading the inbox, or a writer the security middleware already
admitted to the row. A by-id or predicate write on a row the caller
cannot read is answered not-found, or does not match, before the hook
runs. `plugin-security`'s `by-id-write-unreadable-not-found` and
`predicate-write-unreadable-not-matched` pins hold that.
## Pins
- New `lookup-target-title-reader-fls.integration.test.ts`: the real
`ApprovalService` on a real `ObjectQL` over `SqlDriver` (in-memory
better-sqlite3), with a field-visibility source standing in for
`plugin-security`'s.
- (a) A reader who may not read a target's display field gets no title
for that key on `getRequest` and `listRequests`, and the stored id
stands. A self-lookup is narrowed by the reader's answer for the subject
object.
- (a-masked) A target display field served masked is withheld the same
way.
- (b) Control: a reader who may read every target field gets every
title.
- (c) A withheld target display field lends nothing; the title is picked
from what the reader is served on the target.
- (d) One ask per object per read, subject and targets together, on the
single read and a multi-row list.
- New `record-lock-refusal-writer-fls.integration.test.ts`: real engine,
real driver, the real `beforeUpdate` hook.
- (a) A writer who may not read the display field is refused without the
title, by-id and on a predicate write.
- (a-masked) The same, served masked.
- (b) Control: a writer who may read it gets the title.
- (c) One ask per refusal, made with the writer's own envelope, and none
for a write the lock does not refuse.
- `RECORD_LOCKED` and 409 are asserted on every refusal.
- Updated: `payload-display-target-exposure.test.ts` and
`implicit-reference-target-lookup.test.ts` pass `enrichRows`' new
required context. No security service is wired there, so nothing is
narrowed, which is the posture those pins measure.
## Red before, ablations after
- **Before the fix**, the two new files on the base's source (commit
`c0c5a0ba28`): 8 failed / 2 passed. The two passes are the controls (b);
every other pin was red.
- **Ablations**, at `d3a9c5f12a`, via `scripts/ablation-replace.mjs` in
wrap mode, under an EXIT/INT/TERM restore trap. Each run covers the two
new files plus `subject-title-reader-fls.integration.test.ts`, 18 tests.
- **1a**: the targets' served answer replaced by `undefined` (the
whole-target pick, asks kept). Predicted red: row 1 (a), (a-masked),
(c). Observed: 3 failed / 15 passed, exactly those. Restore proven: blob
`0a8b5fc6121d` equals HEAD and `git diff HEAD` is empty.
- **1b**: the targets' served-fields ask deleted. Predicted red: row 1
(a), (a-masked), (c), (d). Observed: 4 failed / 14 passed, exactly
those. Restore proven: blob `0a8b5fc6121d`.
- **2**: the refusal titled from the raw pre-image again. Predicted red:
row 2 (a) by-id, (a) predicate, (a-masked). Observed: 3 failed / 15
passed, exactly those. Restore proven: blob `25e7a598c358`.
- No `dist` step: every subject is imported from `src` by relative path.
## Tests and gates
- `pnpm --filter @objectstack/plugin-approvals exec vitest run
--maxWorkers=2`, the whole package, at the final head `c5115facf0`
(after merging `origin/main` `72329942f9`, then `pnpm install
--frozen-lockfile` and a rebuild of the package and its closure, 32
tasks, exit 0): Test Files 77 passed (77), Tests 1072 passed (1072),
lock `VERDICT command-exit 0`. It read the same at `d3a9c5f12a`, before
the merge.
- `pnpm --filter @objectstack/plugin-approvals typecheck`: exit 0.
`check:test-typecheck` OK (8 files / 324 errors / 27 signatures held; no
new signature). `tsc --listFiles` on `tsconfig.test.json` compiles all
four new or edited test files.
- Gates, at `c5115facf0`: `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` derived 67 commands off this change
set. All 67 ran, each exit code captured before any pipe, and all 67
exit 0.
- `check:dual-build-cjs-loads` and `check:i18n` first answered 3,
PREREQUISITE NOT MET (no workspace dist). After a workspace build (75
tasks, exit 0), both answered 0, and they are recorded at that measured
0, not as failures.
- `--ran` reconciliation, with every exit code recorded: 67 derived, 67
run, 0 NOT-MEASURED (derived), 0 UNRUN, exit 0.
- eslint (`--no-inline-config`) over the 7 changed TS files: exit 0, no
findings.
- Not owed locally: `check:dev-prereqs` and
`check:skill-frame-freshness`, because no `packages/spec` build input or
frame file is touched. The full farm is CI's.
## Changeset
`.changeset/23618-approval-title-family-served-fields.md`:
`@objectstack/plugin-approvals` **patch**. It narrows served values
only. No schema, route, error code, translation key, export or
notification field moves. `payload_display` and its values are already
optional, and `bindApprovalLockHook`'s new parameter is optional. With
no field-visibility source wired, every title is served as before.
`Clause-②: no`, on this body, the changeset and the claim.
## Deviations
1. **Push order.** The pins, the fix and the changeset were committed
locally and went up in one push, after the empty-branch probe, so
nothing red reached the remote before the fix. That follows the card's
security rule; AGENTS.md otherwise asks for a WIP push before long
steps.
2. **File surface.** Beyond the claim's two sources and their tests:
`approvals-plugin.ts`, one wiring line that hands the lock hook the
authority the redaction already reads, and the two pass-through test
files named above.
3. **The bounded comment change** above, by triage ruling `6185594767`.
4. **Attribution.** Commit trailers are the repository's model-free
pair, and this body ends with the repository's session footer, rather
than the harness reminder's lines.
5. **The throwaway measurement** lived in
`packages/plugins/plugin-security/src` for one run and was deleted. The
tree was verified clean.
## Acceptance notes
- **Read, not measured, not filed (no carrier):** a lookup target is
still read under the system context. Its title is therefore not narrowed
by the reader's row-level or sharing access to the TARGET record, only
by its declared exposure (#22738, which takes no caller) and now by
field-level security. Whether a referenced record's title should follow
the reader's row-level access is a design question beside #7497 ruling
C, not a defect this card names.
- **Not measured:** row 1 with `plugin-security` composed. The pins hold
the seam to the field-visibility contract's three answers, as the
payload and subject-title pins do. Row 2 was measured composed
(assumption 2).
- **The seam's posture carries over unchanged to both rows.** A read
projection that throws, or answers `undefined`, passes the title
through, logged at `warn` as for the snapshot. A source with no
masked-for-this-caller answer fails closed (`[]`, no title). A write
that carries no execution envelope asks the security service with none,
and its answer for no caller decides.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(plugin-approvals): the inbox's referenced-record titles (`payload_display`) and the record-lock refusal's title are served only from fields the caller may read
6
+
7
+
Clause-②: no
8
+
9
+
-**Referenced-record titles follow the reader's field-level security on the referenced object.**`listRequests` and `getRequest` now pick each `payload_display` title from the referenced record as served to the reading caller: the same answer the payload snapshot and `record_title` are narrowed by (readable, and not masked for that caller), asked for the referenced object, once per object per read. A display field the reader may not read on the referenced object, or is served masked, gives that key no display value, so the snapshot's stored id stands, as it already does for a deleted referenced record. A title-eligible field the reader may read still names the record, and a reader who may read the display field gets the same title as before.
10
+
-**The `RECORD_LOCKED` refusal names the record only from what the writer may read.** The per-record refusal sentence takes its title from the record as served to the refused writer, asked with the writer's own execution context. When the writer may not read the display field, or is served it masked, the sentence names the object's label alone (`This Opportunity is locked …`), as it already does for a record with no title; it never names the id. A writer who may read the field is told the title as before. The code (`RECORD_LOCKED`) and status (409) are unchanged.
11
+
12
+
A patch: a narrowing of served values only. Nothing is granted, and no schema, route, error code, translation key or export moves. With no field-visibility source wired, every title is served as before.
0 commit comments