Repository navigation
[finding] approvals: sys_approval_request.payload_json snapshots the full raw row and serves it to approvers unfiltered — object-level hidden: true and FLS declarations cannot reach inside the JSON blob #10749
Description
Split out of #10734 (MEMBER-filed console defect, moved to objectstack-ai/objectui#5553 for its UI half) during triage. This card is the server-side residual: even after the UI stops rendering its "Raw data (JSON)" panel, the data itself still crosses the wire.
Observation
@objectstack/plugin-approvals writes the submitted record's complete raw row into sys_approval_request.payload_json unconditionally at request-creation time (per the #10734 locator). That snapshot includes internal columns (id, created_by, updated_by, owner_id, organization_id, bare lookup ids) and fields the source object's metadata declares hidden: true.
Any approver who can read the approval request receives the whole blob — the field-visibility machinery (object metadata hidden, FLS) governs columns of objects, and cannot see inside a JSON column of sys_approval_request. So the app author's "these fields are not for users" declaration is structurally unenforceable on the approval path, for every app, today (measured on 17.1.0 by the #10734 reporter's deployment).
Why filed as a finding rather than queued
The right treatment has real design weight and at least three shapes with different audit consequences:
- Trim at write time (snapshot only non-hidden, business-relevant fields) — cheapest, but the approval record then no longer proves what was actually submitted (audit/replay cost);
- Trim at serve time by requester (full snapshot at rest, metadata-driven redaction when a non-admin reads it) — preserves audit, needs a serve-path seam;
- Snapshot an explicit projection declared by the approval process shape — contract-first, but touches
ApprovalNodeConfigSchema($strict, currently has no such field), i.e. spec surface.
Grading should also answer the standing triage question "who is the producer?" — the producer is plugin-approvals' request-creation path, so the domain is services; option 3 would pull domain:spec in.
Not queued pending first-touch grading; no user-visible fix should ride the objectui#5553 UI card.
Activity
huangyiirene commentedon Aug 21, 2026
Triage (first-touch grading):
finding→needs-user-decision,domain:services, type Bug. 安全/权限边界属人工地板,不代裁;且有 open 下游依赖 —— objectui#5565(审批抽屉渲染 hidden 字段)正Blocked-by:本卡等方向。决策分析(中文):一句话问题:提交审批的记录会把整行原始数据 —— 包括 app 作者声明「对用户隐藏」的字段和内部列 —— 原样发给每个审批人;字段可见性规则管不进审批快照的 JSON 里,每个 app、今天都如此(真实客户 17.1.0 部署实测,#10734)。
选项:
<!-- os-decision-facets -->
payload_json的现有消费者(回放、报表、审计导出)是否依赖会被裁掉的字段未清点;serve 路径有几扇门(REST/ObjectQL/导出)未逐一数过 —— B 的实现面大小取决于它。回一个字母即可(A/B/C;「B,但先清点消费者」也可)。裁后执行:裁定后本席出箱入队 services 席派发(B ⇒ 附 serve 门清点为第一步;若涉 spec 投影则拆 spec 半边转该席),objectui#5565 依裁定方向解锁。
Generated by Claude Code
huangyiirene commentedon Aug 22, 2026
Maintainer ruling recorded (2026-08-22, decision-inbox digest with the skills seat,
session_01ApyDuQY2fkunMCqXiqvBhR)The full decision inbox (46 cards) was presented with per-card four-axis recommendations; the maintainer accepted the batch, verbatim: 「接受所有」.
Ruled: Option B — read-time masking, with the consumer inventory first. The full snapshot stays at rest in
sys_approval_request.payload_json(preserving the approval record as audit evidence of what was actually submitted), and metadata-driven redaction is applied at serve time when a non-admin reads it, sohidden: true/FLS declarations become structurally enforceable on the approval path for all existing apps at once. Per triage's post-ruling plan, the first step of the dispatched work is the inventory triage flagged as unmeasured: enumeratepayload_json's existing consumers (replay, reporting, audit export) and count the serve doors (REST/ObjectQL/export) the redaction seam must cover. Option A (write-time trimming) remains the fallback; C (a contract-declared projection) is deferred until a real projection need appears. objectui#5565 unblocks in this direction.State:
needs-user-decisionremoved →pm:queue(ready for dispatch).Generated by Claude Code
os-warren commentedon Aug 22, 2026
Claim: dispatched.
claude/issue-10749-approval-payload-read-masking../objectstack-10749claude-opus-5(this seat's contract-review-tier allocation is exhausted;CONTRACT_REVIEW_TIERreadsclaude-fable-5atscripts/pm/dispatch-gates.mjs:1932). PR stays draft;needs:contract-reviewhung as the compensating control. ⛔ This seat does not clear it.packages/plugins/plugin-approvals/src/approval-service.tsand whatever serve doors the inventory names. Free.The ruling is settled
Maintainer, 2026-08-22 decision-inbox digest (46 cards, accepted verbatim 「接受所有」): Option B — read-time masking, with the consumer inventory first.
The full snapshot stays at rest in
sys_approval_request.payload_json, preserving the approval record as audit evidence of what was actually submitted. Metadata-driven redaction is applied at serve time when a non-admin reads it, sohidden: true/ FLS declarations become structurally enforceable on the approval path for all existing apps at once.⛔ Option A (write-time trimming) is the fallback, not a shortcut — it buys safety with an audit gap, which is the thing this ruling declined. Option C (a contract-declared projection) is deferred until a real projection need appears; it is not yours to add.
Step 1 is the inventory, and it is not a formality
Triage flagged its own blind spot and the ruling makes closing it the first step:
So, before writing the seam:
payload_json— replay, reporting, audit export, the Console approval drawer, anything else. For each, say whether it would lose a field it depends on.⛔ Do not report either count from a grep hit count. This lane got grep-count-as-fact-count wrong four times in one day. Enumerate, then say what each hit actually is.
If the inventory shows B is materially larger than the ruling assumed, that is a
needs_decisionreturn with the measurement — not a licence to fall back to A. The fallback is the maintainer's to invoke.What must be pinned — both directions
hidden: true/ FLS-restricted fields does not receive them. Assert the absent keys by name, not just a shape.payload_jsonstill contains the full row. An implementation that trimmed at write time would pass a leak-only suite while silently taking the option the maintainer declined. Assert the row at rest, separately from what is served.Downstream
objectui#5565 (the approval drawer rendering hidden fields) is
Blocked-by:this card and unblocks in this direction. Do not reach intoobjectui— if your work implies a change there, put it innotes_for_other_cards.Standing proof standard
Ablation with the signature predicted first; restore proved byte-identical by
git hash-object. Provesrc/vsdist/resolution argued from the files. Zero-hit counter-check with the positive control run first, so the instrument is proven on the corpus before its silence is read as evidence. Gate union derived on the final commit, clean tree,dispatch-gates.mjswith no path arguments, exit codes captured before any pipe. Class #10309 is live — run what you judge relevant explicitly and say whether the derivation named it. Changeset required.PREREQUISITE NOT MET/--re-measure cannot run) is NOT MEASURED, never a pass — build the closure and re-run. Nevergit stash(shared stack).⛔ Do not touch
content/docs/releases/**orpackages/spec— zero ownership in this lane, and note that Option C would have been a spec change, which is precisely why it is deferred. If you conclude the fix needs spec, stop and report (precedent: #10999).Refs
#10734 (the customer report, measured on a real 17.1.0 deployment) · objectui#5565 (downstream, blocked on this) · the triage four-facet analysis and the maintainer ruling are both on this card · ADR-0112
Generated by Claude Code
fix(approvals): redact the payload snapshot at serve time (#10749)
os-warren commentedon Aug 22, 2026
os-dev-report
{ "issue": 10749, "status": "rework", "branch": "claude/issue-10749-approval-payload-read-masking", "pr": "https://github.com/objectstack-ai/objectstack/pull/11039", "premise_still_valid": "partial", "summary": "The FLS half of the card is implemented and pinned; the `hidden: true` half rests on a premise measurement falsifies, and is escalated rather than decided. Implemented Option B exactly as ruled: the full snapshot stays at rest in `sys_approval_request.payload_json`, and the SUBJECT object's field-level read controls are applied at serve time via plugin-security's `getReadableFields` (the same mask the read middleware applies, so the seam cannot drift from data-plane FLS). Two serve doors, each with its own seam and its own pins: the service door (`getRequest`/`listRequests`, redacting BEFORE enrichment so the derived `payload_display`/`payload_labels` maps — which are built by walking the snapshot's own keys — cannot leak a restricted field's name, label, or referenced-record title), and the generic data door (object-scoped engine middleware on `sys_approval_request`, covering the REST-data/ObjectQL/export/MCP family that shares `find` as its one producer). Middleware rather than an `afterFind` hook because `Engine.buildSession` carries no `onBehalfOf`, so a hook-based seam would silently drop the ADR-0090 D10 delegator intersection and answer a delegated read MORE permissively than the service door. Object-level access is deliberately untouched: an approver commonly holds no read grant on the object under approval, and `getReadableFields` answers a caller with no field-permission entries with the full set — so field-level declarations are enforced while every shipping drawer keeps rendering. ⚠️ The `hidden: true` half of the card is NOT implemented: see open_questions[0]. Reported as `rework` for that reason, not because anything shipped is incomplete.", "inventory_consumers": "Repo-wide `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`, `http_delivery`, `sys_approval_request`). Classified individually, the approval column's readers are exactly: (1) `approval-service.ts:415` rowFromRequest -> the served `payload` (this IS the leak, door 1); (2) `approval-service.ts:2267` approver re-resolution at decide time — `raw` comes from `loadPendingRow` under SYSTEM_CTX, so the isSystem skip leaves it whole, NOT affected; (3) `approval-service.ts:4096` free-text filter `{payload_json:{$contains:q}}` — a predicate, not a serve, NOT affected (filed #11040); (4) the object's own field declaration + docstring; (5) two test fixtures, both re-checked and green. Consumers the ruling named specifically: NOTIFICATIONS — the column's own docstring claims notifications render from the snapshot; measured against all 12 `this.notify(...)` call sites, every payload is {title,message,actionUrl} (two also `actions`) and NONE reads the snapshot, so the docstring is stale (filed #11041). ACTIONABLE-LINK PAGES — `summaryRows` renders record_title/object_label/submitter_name/record_id/object_name only; not a payload door. REPLAY/RESUME — correlates on flow_run_id/flow_node_id and reads `node_config_json`, not payload_json. REPORTING/AUDIT EXPORT — no reader: `sys_approval_request` across packages/apps/examples returns 16 non-test files outside plugin-approvals and NONE appears in the payload_json scan (constants, i18n, lint, nav, seeds). Instrument proven on a known-positive corpus (68 hits inside plugin-approvals) BEFORE the zero elsewhere was read as evidence. VERDICT: no existing consumer loses a field it depends on.", "inventory_doors": "Enumerated by asking what can return this column's bytes to a caller, then tracing each candidate to its PRODUCER — not by counting matches. DOOR 1 service: GET /api/v1/approvals/requests[/:id] -> listRequests/getRequest -> rowFromRequest -> parsed `payload` (+ payload_display/payload_labels). COVERED. DOOR 2 generic data: the object declares `enable.apiMethods:['get','list']`, so a plain find/findOne returns the raw string with the service never running — this is a FAMILY (REST data routes, ObjectQL, CSV/XLSX export as a read-derived projection, MCP data tool, AI-context interceptor) all sharing ONE producer, `find` on the engine. COVERED at that producer. NOT doors, each checked rather than assumed: actionable-link pages (renders summary fields only), notifications (carries no snapshot), system/replay reads (SYSTEM_CTX — deliberately whole, this is the audit channel). Method limit stated so a missed door shows as a limit rather than a gap: door 2 is covered at the engine producer, so any future reader that bypasses `find` on this object (a raw driver call, a direct SQL export) would not be narrowed.", "tests": "All commands run through `scripts/pm/os-verify-lock.sh`; every exit code captured BEFORE any pipe; verdicts quoted are the gates' own lines. Package suite on final commit 9ea05bf11: `Test Files 29 passed (29) · Tests 552 passed (552)`, os-verify-lock `VERDICT command-exit 0`. Package typecheck `tsc --noEmit` exit 0. NEW FILE: approval-payload-redaction.test.ts, 16 tests. FOUR PINNED DIRECTIONS — (1) leak closed: restricted approver does not receive `salary`/`ssn`/`internal_notes`, asserted BY NAME not by shape; (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: approver still receives id/title/amount/submitted_by (a redact-everything seam would also pass a leak-only suite and break every shipping drawer); (4) admin unchanged: a caller who may read every field still gets the whole snapshot — modelled as a PARTICIPANT because getRequest enforces #3590 participation first, which the pin caught by returning null on the first attempt. Plus per-door pins (list door; three generic-door pins), derived-map cleanliness, caller-keying (asked as the caller about the SUBJECT object, never SYSTEM_CTX), `undefined` is not a denial (#3807) while `[]` is, and throw-serves-unredacted. SRC-vs-DIST, argued from the files: (a) all three subject modules imported RELATIVELY (`./approval-service.js` etc), which can never reach exports/dist; (b) the package has NO dist/ directory at all — only the dependency closure was built (`'@objectstack/plugin-approvals^...'`), never the package itself; (c) the sole vitest alias is for @objectstack/metadata-protocol and does not touch the subject, and there is no root vitest config. ABLATION, prediction written to a file BEFORE mutating, tests named individually. LEG A (drop the service-door seam from loadRequest only) predicted exactly 3 red — leak-closed+still-served, `[]`-is-a-denial, keyed-on-reading-caller — and predicted the list-door and ALL generic-door pins stay GREEN; observed exactly those 3 red, 549 pass, other doors green. LEG B (drop the write-back in redactRowsInPlace only) predicted exactly 2 red — generic-read-narrowed, registered-against-sys_approval_request-only — and every service-door pin green; observed exactly those 2 red, 550 pass. The prediction ALSO called `derived maps are clean` as UNCERTAIN/predicted-green (no metadata registry under this double, so resolveFieldLabels returns {} and the map never attaches) — it behaved as predicted and is recorded as defence-in-depth, not claimed as a bite it does not have. RESTORES byte-identical: approval-service.ts baseline dd38fe823aed762be5307b98577c776f7ebb9436 / restored dd38fe823aed762be5307b98577c776f7ebb9436; payload-redaction-middleware.ts baseline 66e957c6e8305004f3937ad3b7f1503fe0673daf / restored 66e957c6e8305004f3937ad3b7f1503fe0673daf. Tree clean after both.", "gates": "Union derived on FINAL commit 9ea05bf11, clean tree, `node scripts/pm/dispatch-gates.mjs` with NO path arguments (6 paths, 136 families discovered, DERIVE_EXIT=0). check:engine-double-contract EXIT=0 `OK — 377 pinned, 133 in the DEBT ledger, 2 exempt.` · check:where-matcher EXIT=0 `277 matcher(s) discovered, 277 answer the combinator battery correctly or refuse it loudly` · check:query-options-erasure EXIT=0 `67 unswept non-test site(s) in 17 file(s), none new` · check:type-check-coverage EXIT=0 `OK — 65/78 workspace packages type-checked (plus the root)` · check:type-check-debt EXIT=0 `--re-measure: OK — 33 ledger entr(ies) re-measured in 244.7s, 1908 raw tsc error(s) total, none above its recorded number.` · check:i18n EXIT=0 `OK (9 package(s) — all bundles in sync, no undeclared authoring keys).` · check:nul-bytes EXIT=0 `OK (scanned 6393 text file(s) ... no raw ASCII control bytes).` · check:test-source-alias EXIT=0 · check:type-source-resolution EXIT=0 · check:slot-lookup EXIT=0 · check-ci-filter-parity EXIT=0 · check-plugin-teardown-shape EXIT=0 · check-empty-changeset EXIT=0 · check-changeset-no-major EXIT=0 · check-adr-0087-registration EXIT=0 · check:objectui-changeset EXIT=0 · check:changeset-gate-self-tests EXIT=0 · check-affected-docs EXIT=0. TWO GATES INITIALLY REFUSED/FAILED AND WERE DRIVEN TO A REAL VERDICT: check:i18n first returned `PREREQUISITE NOT MET — the workspace CLI is not built ... Nothing was checked` (NOT MEASURED, never a pass) — built the CLI and re-ran 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 unexercised verbs were REMOVED (this file's subject is the read path), so the ledger is untouched and the gate is green on its own terms. CLASS #10309: the derivation named check:nul-bytes nowhere; run explicitly on judgment, green.", "declared_narrowings": "None on gate coverage — the derived union plus the convention-triggered families were all run to real verdicts, including the two heavy ratchets (full workspace build 70/70 tasks, then the 244.7s type-check-debt re-measure). Behavioural narrowings, all deliberate and all stated in the PR body: (a) the seam narrows FIELD-level visibility only, never object-level access, so an approver with no grant on the subject object still receives the business fields; (b) `undefined` from getReadableFields serves the snapshot WHOLE (#3807 — it is the 'schema unresolvable' tier, not a denial), while `[]` IS honoured as the security plugin's own fail-closed tier; (c) a THROW from the visibility authority serves unredacted with a loud warn rather than blanking every drawer — see open_questions[1].", "open_questions": [ { "question": "The card's headline names `hidden: true` as an enforceable author declaration. Measurement contradicts that premise: `packages/spec/src/data/field.zod.ts` states in the `internal` docstring that `hidden` is a UI contract ('Hidden from default UI') and 'has never governed serialization' — and that is true in the tree, not just in prose. A scan for consumers of a field's `hidden` (instrument proven on a known-positive first) returns 8 sites and NONE strips a value from a read result: search-companion eligibility, search-field eligibility, two UI layout helpers, two auto-view column pickers, two lint rules. So: should `hidden: true` be given a serialization semantic on the approval path?", "options": [ "A — do NOT enforce `hidden` (shipped): keep `hidden` a UI hint. Enforcing it on the approval path alone would make approvals STRICTER than a direct read of the same row, which closes no leak (the approver can read the record directly and get the hidden field) while breaking every drawer that renders a `hidden` business column.", "B — give `hidden` a read-side serialization semantic platform-wide. Genuinely closes the author's intent everywhere, but it is a `packages/spec` contract change with a blast radius across every object and every read door, and it collides with the existing `internal: true` key that was introduced (maintainer ruling 2026-08-12) precisely BECAUSE `hidden` does not do this.", "C — tell app authors to use `requiredPermissions` / `internal` / `maskingRule` for anything that must not reach a reader, and treat the card's `hidden` clause as a documentation fix. Zero code, but leaves authors who already wrote `hidden: true` believing it protects them." ], "recommendation": "A as shipped, plus C as the follow-up — and NOT decided by me. Real business need: the customer-measured leak is the FLS half, which is now closed; no evidence any deployment relies on `hidden` as a security control, and if one did, option A leaves them exactly as exposed as they are on every other read door rather than newly exposed. Long-term soundness: `internal: true` already exists as the read-side omission primitive and was created because `hidden` is not one — giving `hidden` a second, overlapping meaning is the workaround, not the contract-first move. AI-written-metadata safety: this is the axis that argues loudest for B (an AI author writing `hidden: true` and believing it protects data is exactly the silent-failure class the platform tries to make structurally impossible) — but the honest fix for that is a loud authoring-time diagnostic steering `hidden` -> `internal`/`requiredPermissions`, which is option C's shape and is cheaper and more general than B. Startup scope: B expands a public contract's meaning across every object for no measured pull; declined by default. ⛔ This is spec surface and this lane has zero ownership of it (and Option C of the original grading was deferred for exactly this reason), so it is escalated, not taken." }, { "question": "When the visibility authority THROWS (metadata briefly unavailable), should the snapshot be served unredacted or blanked?", "options": [ "Fail open, logged loudly (shipped): a metadata hiccup serves today's behaviour rather than emptying every approval drawer in the deployment. Leaves the pre-existing leak open for the duration of the outage — no regression, but no protection either.", "Fail closed: redact everything on a throw. Consistent with the platform's stated posture that consumers turning `getReadableFields` into an access decision must fail closed, but converts a transient metadata blip into a deployment-wide approvals outage." ], "recommendation": "Fail open as shipped, flagged for the contract review rather than settled by me. The seam DOES honour the fail-closed tier the security plugin itself computes (`[]` for an unresolvable posture or a dangling delegator) — the only thing treated as non-denial is an exception, which is the tier `api-exposure.ts` already reasons about in the same fail-open direction. Both directions are defensible and the choice is stated in the PR body rather than buried; `needs:contract-review` is the right place to overturn it." } ], "out_of_scope_findings": [ "filed as #11040: `listRequests` pushes `{payload_json:{$contains:q}}` into the engine, so a caller can PROBE snapshot contents they may not read (row membership is the oracle) — the same shape `maskingRule` closes by making masked fields non-filterable. A predicate, not a serve door; fixing it changes search semantics, so it wants its own grading.", "filed as #11041: `sys_approval_request.payload_json`'s docstring claims notifications render from the snapshot — measured false against all 12 notify call sites. Left alone because the string is extracted into generated i18n bundles, so rewording it is a regeneration change rather than a comment edit." ], "notes_for_other_cards": "objectui#5565 (approval drawer rendering hidden fields) unblocks in this direction; nothing in objectui was touched. 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. ⚠️ It must NOT assume `hidden: true` fields are absent — see open_questions[0]; if the drawer needs to stop rendering `hidden` columns, that is a client-side presentation decision on objectui#5565, not something this server change delivers.", "pr_posture": "Draft, as dispatched. `needs:contract-review` left hanging — this seat did not clear it, did not mark ready, did not arm auto-merge. `Part of #10749` (not `Fixes`) because the `hidden` half is escalated rather than discharged, so merging must not close the card." }Generated by Claude Code
Generated by Claude Code
os-warren commentedon Aug 22, 2026
PM review of PR #11039. Not flipping ready — Clause-② means a non-dispatching seat PASSes first, and this seat does not clear
needs:contract-review.Part of #10749, correctly, because half of this card's headline turns out to rest on a premise measurement falsifies.⛔ This card's title is half wrong, and that is the finding
The title says "object-level
hidden: trueand FLS declarations cannot reach inside the JSON blob." The FLS half is real, was the customer-measured leak (#10734), and is now closed. Thehiddenhalf assumeshidden: trueis a data-protection declaration. It is not, and never has been.Verified by content on
origin/main, because a claim that contradicts a card's own title deserves checking rather than accepting:field.zod.ts:1237hidden: z.boolean().default(false).describe('Hidden from default UI')— one clause, UI onlyfield.zod.ts:1243field.zod.ts:1270internalis the read-side omission primitive: "the engine OMITS the key fromfind/findOneresults, the 201 create body and the by-id update body… on the default projection AND when a client names the field in?select=" — ADR-0049, "enforced from landing day, atEngine.maskSecretFields"And the consumer scan holds: every reader of a field's
hiddenis an eligibility or column-picker decision — two lint rules, threemetadata-protocolcolumn pickers,search-companion— plus generated translation bundles, which are not code consumers at all. None strips a value from a read result.So
internal: trueexists becausehiddendoes not do this (maintainer ruling 2026-08-12). Givinghiddena serialization meaning on the approval path alone would make approvals stricter than a direct read of the same row — closing no leak, since the approver can read the record directly, while breaking every drawer that renders ahiddenbusiness column.⛔ Not mine to decide, and it is spec surface this lane has zero ownership of. Option C of the original grading was deferred for exactly this reason.
hiddena UI hint (shipped)hidden-relying deployment exactly as exposed as on every other read door, rather than newly exposedhiddena read-side serialization semantic platform-widepackages/speccontract change across every object and read door, and it collides head-on withinternal, which was created becausehiddenis not thatinternal/requiredPermissions/maskingRule, treat the card'shiddenclause as a docs fixhidden: truebelieving it protects data is the silent-failure class this platform exists to make impossible, and a doc that nobody re-reads does not reach themA (shipped) + C as a follow-up card. Say the word and I file C.
What shipped, and the two seam choices worth review's attention
Option B as ruled: full snapshot at rest, field-level redaction at serve time via plugin-security's
getReadableFields— the same mask the read middleware applies, so the seam cannot drift from data-plane FLS. Two doors, each with its own pins:payload_display/payload_labelsmaps (built by walking the snapshot's own keys) cannot leak a restricted field's name, label, or referenced-record title. Leaking the key while hiding the value would have been the easy miss.afterFindhook. The reason is measured and is the sharper of the two calls:Engine.buildSessioncarries noonBehalfOf, so a hook-based seam would silently drop the ADR-0090 D10 delegator intersection and answer a delegated read more permissively than the service door. A seam that is more permissive on one door than another is the shape this card exists to end.Object-level access is deliberately untouched — an approver commonly holds no read grant on the object under approval, and
getReadableFieldsanswers a caller with no field-permission entries with the full set. So field declarations are enforced while every shipping drawer keeps rendering.The inventory was done as an inventory, not as a grep
The ruling made this step 1 and it was the right call:
payload_jsonreturns 53 repo-wide hits, and that is not a consumer count — the identifier is a field name on three unrelated objects (sys_job_queue,http_delivery,sys_approval_request). Classified individually, the approval column has exactly one serve reader (the leak), one SYSTEM-context read at decide time (unaffected — deliberately whole, it is the audit channel), and one predicate use.Two of the ruling's named consumers turned out not to be consumers at all:
this.notify(...)call sites: every payload is{title, message, actionUrl}and none reads the snapshot. The docstring is stale → [finding]sys_approval_request.payload_json's doc comment claims notifications render from the snapshot — measured false against all 12 notify call sites #11041.sys_approval_requestappears in 16 non-test files outsideplugin-approvals, none in the payload scan (constants, i18n, lint, nav, seeds).Instrument proven on a known-positive corpus (68 hits inside
plugin-approvals) before the zero elsewhere was read as evidence. Verdict: no existing consumer loses a field it depends on.find), so a future reader bypassingfindon this object — a raw driver call, a direct SQL export — would not be narrowed. That is exactly the shape I asked for: a missed door shows up as a limit of the method, not as an unstated gap.Two things done right that are easy to get wrong
The template was deviated from, out loud.
premise_still_validwas reported as"partial"rather than forced to a boolean, with the reason:falsewould have read as "no PR, re-triage", which is not the state. Bending a schema and saying so beats fitting a true answer into a false shape.A gate was driven to a verdict twice rather than narrowed.
check:i18nfirst returnedPREREQUISITE NOT MET … Nothing was checked— NOT MEASURED — so the CLI was built and it was re-run to a real OK. Andcheck:engine-double-contractwent red because the new fake engine declaredupdate()/delete(); rather than growing the pinned ledger to accommodate it, the two unexercised verbs were removed. The ledger is untouched and the gate is green on its own terms. Growing a ledger to fit your test is the move this lane bans by name.Ablation: two legs, each predicted in writing with the failing tests named individually, each observed exactly (3 red / 549 pass, then 2 red / 550 pass), restores byte-identical on both files. The prediction also marked one case UNCERTAIN-but-expected-green and it behaved as predicted — recorded as defence-in-depth rather than claimed as a bite it does not have.
⛔ One shipped behaviour that needs review's explicit sign-off
When the visibility authority throws, the snapshot is served unredacted with a loud warn (fail open). The seam does honour the fail-closed tier the security plugin itself computes (
[]for an unresolvable posture or a dangling delegator); only an exception is treated as non-denial, matching howapi-exposure.tsalready reasons.Both directions are defensible — fail-closed converts a transient metadata blip into a deployment-wide approvals outage; fail-open leaves the pre-existing leak open for the duration of that blip. It is a no-regression, no-protection trade during an outage. The seat flagged it rather than settling it, which is right. Contract review is the place to overturn it.
Also filed
listRequests' free-text filter pushespayload_json: { $contains: q }into the engine — a probe oracle over snapshot contents the caller may not read #11040 —listRequestspushes{payload_json: {$contains: q}}into the engine, so a caller can probe snapshot contents they may not read; row membership is the oracle. A predicate, not a serve door, and fixing it changes search semantics — its own grading.sys_approval_request.payload_json's doc comment claims notifications render from the snapshot — measured false against all 12 notify call sites #11041 — the stale docstring above. Left alone here because the string is extracted into generated i18n bundles, so rewording it is a regeneration change, not a comment edit.objectui#5565 unblocks in this direction. Note for whoever takes it: the server now omits restricted keys from
payload, andpayload_labels/payload_displayare consistent with what is served — but it must not assumehidden: truefields are absent. See thehiddenquestion above.Generated by Claude Code
os-zhuang commentedon Aug 22, 2026
Contract review — PASS (clause-② compensation review of PR #11039's actual diff, not its report).
Reviewer: skills seat, session
session_01MsbKEG4LtERSLaDrbehM3e. Tier fuse machine reading, not self-report:get_session→external_metadata.last_served_model = claude-fable-5=CONTRACT_REVIEW_TIER.getReadableFields— the same mask the data-plane read middleware applies, so the seam cannot drift from FLS. The full snapshot is pinned intact at rest (a write-time trim — the declined Option A — fails the suite while passing every leak-only assertion).payload_display/payload_labelscannot ship a restricted field's name or label; the generic door is object-scoped engine middleware carrying the realExecutionContext, preserving the ADR-0090 D10 on-behalf-of intersection a hook would drop. System/audit reads and writes untouched.undefined≠ denial (approvals: adepartmentapprover never resolves when the business unit hasorganization_id = null(every seeded BU) #3807) and the[]fail-closed tier both honoured; object-level access deliberately untouched so every shipping approval drawer keeps rendering.Part of, notFixes— thehidden: trueserialization question (apackages/specsemantics decision, correctly not decided in a services PR) and the$containssearch oracle (approvals:listRequests' free-text filter pushespayload_json: { $contains: q }into the engine — a probe oracle over snapshot contents the caller may not read #11040) remain on this card.needs:contract-reviewcleared from this card in the same stroke. PR #11039 proceeds by its lane's normal landing rules.Generated by Claude Code
fix(approvals): stop pushing a free-text predicate onto a masked payl…
fix(approvals): apply the subject object's field-level read controls …
fix(approvals): stop pushing a free-text predicate onto a masked payl…
os-zhuang commentedon Aug 24, 2026
Maintainer ruling recorded —
hidden: truestays UI-only;internal: trueis the serialization primitive; closingProvenance: maintainer, 2026-08-24, live PM chat, batch acceptance, verbatim: 「四维分析一致的，接手你的建议。」 This applies the existing 2026-08-12 ruling lineage (recorded in
field.zod.ts:internalexists becausehiddenis not a serialization primitive) rather than making a new rule — the recommendation inherits that ruling with its reason.Ruled:
hiddengains no serialization semantic. The approval-snapshot serve path is governed by FLS viagetReadableFields(landed, PR #11039, Option B, contract-review PASS); an app author who wants a field kept out of read results declaresinternal: true— that is the documented, enforced answer. The accepted fail-open-on-throw trade stands as reviewed. Residual doc sentence (hidden ≠ serialization, internal is) rides the next docs touch in the services lane, not its own card.Closing as completed: the FLS half landed, the
hiddenhalf is ruled out, and #11040 ($contains oracle) / #11041 (stale docstring) carry their own residues. DownstreamBlocked-by: #10749lines (#11041, objectui#5565) unlock via the standard scan — seats re-verify surfaces on the post-#11039 ref before dispatch.Generated by Claude Code
docs(plugin-approvals): correct what the approval snapshot column is …
fix(approvals): a snapshot field the reader is served masked is no lo…