fix(approvals): apply the subject object's field-level read controls to the payload snapshot at serve time - #11039

Part of #10749.

Applies the subject object's field-level read controls to the approval
payload snapshot at serve time, so an approver no longer receives fields the app
author declared they may not read. Implements the maintainer's 2026-08-22 ruling
(Option B — read-time masking, consumer inventory first). Remaining scope, and
why it is not discharged here, is in Open question at the bottom.

Draft, with needs:contract-review on the card: what a published read returns to
a non-admin changes. It is a narrowing — nothing here widens what any caller
receives.

Mechanism

sys_approval_request.payload_json stores the submitted record's raw row,
captured from the flow's $record variable, which the automation layer hands
over with the record's own FLS never applying. The column is a textarea on
sys_approval_request, so to every read door it is an opaque string: the
field-visibility machinery governs columns of objects and cannot see inside a
JSON column. Every field-level read control declared on the SUBJECT object —
requiredPermissions (ADR-0066 D3), a permission set marking a field
non-readable, a maskingRule — was therefore unenforceable on the approval
path, for every app.

The full snapshot stays at rest. Redaction happens at serve time, keyed on
the reading caller, so the same row answers an admin with the whole snapshot and
a restricted approver with only the fields they may read. The readable set is
not recomputed: it comes from the security service's getReadableFields,
documented as the same field mask the read middleware applies, so this seam
cannot drift from data-plane FLS.

Object-level access is deliberately untouched. An approver commonly holds no
read grant on the object under approval at all — that is the normal shape of an
approval, and the snapshot is how they see what they are approving.
getReadableFields answers a caller with no field-permission entries with the
full field set, so field-level declarations are enforced while every approval
drawer shipping today keeps rendering.

STEP 1 — the inventory

Consumers of sys_approval_request.payload_json

A repo-wide scan for payload_json returns 53 hits, and that is not a
consumer count: the identifier is a field name on three unrelated objects —
sys_job_queue (service-queue), http_delivery (service-messaging) and
sys_approval_request. Classified individually, hits on the approval object's
column are these, and only these:

Consumers the ruling asked about specifically, each checked rather than assumed:

• Notifications — the object's own doc comment claims the snapshot is "used
  by notifications so they can render before the record is locked or changed".
  Measured against all 12 this.notify(...) call sites: every payload is
  {title, message, actionUrl} (two also actions). None reads the
  snapshot. The doc comment is stale; filed as [finding] sys_approval_request.payload_json's doc comment claims notifications render from the snapshot — measured false against all 12 notify call sites #11041 rather than edited here.
• Actionable-link pages (ADR-0043 magic links) — renderConfirmPage /
  renderResultPage render summaryRows, which reads record_title,
  object_label, submitter_name, record_id, object_name. Not a payload
  door.
• Replay / resume — the flow-resume path correlates on
  flow_run_id/flow_node_id and reads node_config_json, not payload_json.
• Reporting / audit export — no reader. Searching all of packages/,
  apps/ and examples/ for sys_approval_request returns 16 non-test files
  outside plugin-approvals; none of them appears in the payload_json
  scan — they are constants, i18n, lint rules, nav contributions and seeds.
  The instrument was proven on a corpus where the term is known to exist (68
  hits inside plugin-approvals) before the zero elsewhere was read as
  evidence.

Verdict: no existing consumer loses a field it depends on.

Serve doors, and how they were enumerated

Enumerated by asking what can return this column's bytes to a caller, then
tracing each candidate to its producer, rather than by counting matches.

Door 2 is registered as object-scoped engine middleware, not an afterFind
hook, and that choice is load-bearing: a hook receives Engine.buildSession's
output, which carries no onBehalfOf. getReadableFields intersects the
delegator's field mask on an on-behalf-of read (ADR-0090 D10, fail-closed on a
dangling delegator), so a hook-based seam would silently drop that intersection
and answer a delegated read more permissively than the service door.
Middleware carries the real ExecutionContext, so both doors narrow
identically.

The two doors are pinned independently — proven by ablation below, not
asserted.

What is pinned

packages/plugins/plugin-approvals/src/approval-payload-redaction.test.ts,
16 tests. The four directions, each asserted in the direction that can fail:

1. Leak closed — a restricted approver does not receive salary, ssn,
   internal_notes. Asserted by name; a shape assertion would pass against
   a redactor that removed the wrong keys.
2. Audit preserved — after a restricted read, the stored column still parses
   to the whole row, asserted against the table separately from anything
   served. A write-time trim would pass a leak-only suite while silently taking
   Option A.
3. Still-served — the approver still receives id/title/amount/
   submitted_by. A seam that redacted everything would also score green on a
   leak-only suite and break every drawer shipping today.
4. Admin unchanged — a caller who may read every field still gets the whole
   snapshot, which is what makes this masking rather than trimming. Modelled as a
   participant on purpose: getRequest enforces participation (审批请求的可见性是「按租户」而非「按参与者」——getRequest 用 SYSTEM_CTX 读、只按 organization_id 收窄 #3590) first, so
   a non-participant "admin" answers null and the pin would have been measuring
   visibility instead of field masking. That was caught by the pin failing.

Plus: the list door narrows (its own assertion); the generic door
narrows (its own assertions); the derived payload_display / payload_labels
maps never carry a restricted key — redaction runs before enrichment, and
those maps are built by walking the snapshot's own keys, so redacting after
would still ship a restricted field's name, its authored label, and the title of
the record it points at; the visibility question is asked as the caller and
about the subject object, never as SYSTEM_CTX; undefined is not a denial
(#3807) while [] is; and a throwing authority serves unredacted rather than
blanking every drawer.

src vs dist resolution

Three independent arguments, from the files:

1. All three subject modules are imported relatively (./approval-service.js,
   ./payload-redaction.js, ./payload-redaction-middleware.js); a relative
   specifier resolves to the sibling source and can never reach exports/dist.
2. The package has no dist/ directory at all — only the dependency closure
   was built ('…^...'), never the package itself, so there is no artifact for a
   subject import to resolve to even in principle.
3. The only alias in the package's vitest.config.ts is for
   @objectstack/metadata-protocol; it does not touch the subject modules, and
   there is no root vitest config adding others.

Ablation — prediction written before mutating

Two legs, chosen to test the claim that one door's pin does not stand for
another's. Predictions were written to a file first; the tests to redden were
named individually.

The prediction also called derived maps are clean as uncertain, predicted
green — under this engine double there is no metadata registry, so
resolveFieldLabels returns {} and the map is never attached. It is
defence-in-depth here, not a discriminating pin. It behaved as predicted, and is
recorded as such rather than claimed as a bite it does not have.

Restores proved byte-identical:

approval-service.ts             baseline dd38fe823aed762be5307b98577c776f7ebb9436
                                restored dd38fe823aed762be5307b98577c776f7ebb9436
payload-redaction-middleware.ts baseline 66e957c6e8305004f3937ad3b7f1503fe0673daf
                                restored 66e957c6e8305004f3937ad3b7f1503fe0673daf

Breaking or not — argued in the open

A non-admin caller that was reading restricted keys out of the snapshot now
receives fewer keys. That is a real behaviour change for such a consumer, and it
is shipped as a patch fix, not a breaking change, for three reasons stated
here rather than decided silently:

1. Those fields were never that caller's to read. The platform enforces the
   declaration on every other read door; the approval path was the bypass.
   Removing a bypass is a security fix.
2. The served type is payload?: unknown — never a promised field set, so no
   declared contract narrows.
3. Precedent: the __search companion strip (The __search companion column, declared client-invisible, is echoed in every record body #7642) shipped the same way on the
   same reasoning — a column that was never supposed to be served.

The inventory above is what backs claim 1 empirically: no existing consumer
depends on a trimmed field.

Gates

Union derived on the final commit 9ea05bf11, clean tree,
node scripts/pm/dispatch-gates.mjs with no path arguments (6 paths, 136
families discovered). Every exit code captured before any pipe; each verdict
line below is the gate's own.

Two notes kept honest rather than tidied:

• check:i18n first returned PREREQUISITE NOT MET — the workspace CLI is not built … Nothing was checked. That is not measured, not a pass; the CLI was
  built and the gate re-run to the real OK above.
• check:engine-double-contract first went red because the new fake engine
  declared update()/delete(). Rather than growing the pinned ledger, the two
  verbs were removed — this file's subject is the read path and never reaches a
  write verb — so the ledger is untouched and the gate is green on its own terms.

Class #10309: the derivation named check:nul-bytes nowhere; it was run
explicitly on judgment, and is green.

Out of scope, recorded not fixed

• Search oracle — filed as approvals: listRequests' free-text filter pushes payload_json: { $contains: q } into the engine — a probe oracle over snapshot contents the caller may not read #11040. listRequests' free-text filter pushes
  { payload_json: { $contains: q } } into the engine, so a caller can still
  probe snapshot contents they cannot see — the same oracle shape
  maskingRule closes by making masked fields non-filterable. Untouched here
  because it is a filter, not a serve door, and closing it changes search
  semantics.
• Stale doc comment — filed as [finding] sys_approval_request.payload_json's doc comment claims notifications render from the snapshot — measured false against all 12 notify call sites #11041. The column's docstring claims
  notifications render from the snapshot; measured false against all 12 notify
  call sites. Left alone here because the string is extracted into generated
  i18n bundles, so rewording it is a regeneration change rather than a comment
  edit.

Downstream

objectui#5565 (the approval drawer) unblocks in this direction. Nothing in
objectui was touched. Note for that card: the server now omits restricted keys
from payload, and payload_labels / payload_display are already consistent
with what is served, so the drawer can render whatever it receives without a
second visibility rule of its own.

Open question — not decided here

The card's headline also names hidden: true. That half is not implemented,
and deliberately so: hidden is a UI contract ("Hidden from default UI") which,
in the spec's own words, "has never governed serialization". Measured — a
scan for consumers of a field's hidden returns 8 sites (instrument proven on a
known-positive first), and none of them strips a value from a read result:
they are search-companion eligibility, search-field eligibility, two UI layout
helpers, two auto-view column pickers, and two lint rules.

So enforcing hidden on the approval path alone would make approvals stricter
than a direct read of the very same row — closing no leak, since the approver
can simply read the record — while breaking every drawer that renders a hidden
business column. Giving hidden a serialization semantic is a packages/spec
question, which this lane has zero ownership of and which is precisely why
Option C was deferred. Flagged for the contract review rather than decided.

A second, smaller question for the same review: this seam fails open on a
throw from the visibility authority (logged loudly), so a metadata hiccup
serves unredacted rather than blanking every approval drawer. It does honour the
fail-closed tier getReadableFields itself computes ([]). Both directions are
defensible; the choice is stated rather than buried.

Generated by Claude Code