Repository navigation
fix(approvals): a snapshot field the reader is served masked is no longer served as stored (#20964) - #20993
Conversation
…r is not served as stored (#20964) Red pins, committed before the fix. - plugin-approvals unit pins: both read doors (the service door and the generic data door) drop a field the security contract reports as served masked to this caller, keep the business fields, keep the derived label map clean, and serve the stored value to a reader who holds what lifts the rule (the control). A source that cannot say which readable fields are masked for the caller fails closed. The approvals plugin's bridge forwards that answer. - A dogfood pin on a real boot: the inbox list, the inbox item and the generic data door against the data plane's reference for the same caller, with an unmasking reader as the control. Fixtures are synthetic. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…ad from the security contract (#20964) The payload snapshot redaction narrowed by the security service's read projection alone. That projection counts a field whose masking rule applies to the reader as readable (the data plane serves it, masked), so the snapshot kept the field and served it as stored. The redaction now also reads the contract's query-side answer (getQueryableFields, landed for #20935), which differs from the read projection by exactly the fields the reader is served masked, and serves only fields in both. No second derivation of the masking rule: who the rule applies to is the contract's answer, asked as the reader. The field is dropped rather than masked, because the contract names which fields are masked, not the masked value. A source that cannot give that answer (absent, undefined, or a throw) fails closed for the object, as the contract obliges. The approvals plugin's bridge to the security service forwards the member for the service door; the generic data door already receives the service itself. The existing redaction pins' source double gains the member the real service has (no field there carries a masking rule). Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d9b226c6ad10e9cf69f0992dee6b5a7bdbb8150c && git checkout d9b226c6ad10e9cf69f0992dee6b5a7bdbb8150c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8d329f02eeb571170bba3b275df3c2fc3cb749b3 e6c9b9d56bfa2fe63ad4481a6b814ab337e2662e && git checkout -B drift-repro 8d329f02eeb571170bba3b275df3c2fc3cb749b3 && git merge --no-ff e6c9b9d56bfa2fe63ad4481a6b814ab337e2662e
node scripts/docs-audit/affected-docs.mjs --json 8d329f02eeb571170bba3b275df3c2fc3cb749b3 |
Contract reviewServed-tier: Rendered 2026-10-01T01:04Z on PR #20993 for card #20964. Inputs: the card body and its four comments (triage, claim, dev report, ACCEPT — the last two read as thread entries, not as conclusions adopted), the PR body and file list, the net diff of the two branch commits against Head state at this reading: every check-run complete — 32 ① Derived judgmentsEach accept-set or public-surface change the diff implies, named and judged.
② Semver levelDeclared: Judgment: the level and the declaration are WRONG; the diff publishes a widening. Item ① 1 is a new accepted key on a published accept set. The level rule (the maintainer's ruling on #15294, as the Check Changeset step states it): a purely additive widening of a published package's public surface — a new exported symbol on an index, or a new accepted key or value — takes at least Open question 3, left to this review: it is NOT Required changeset level: Clause-②: yes (widening) With that, the level axis of ③ Boundary flagsEvery dev flag and
Remedy for the next head (then a new record on that head): changeset level Implemented-by: VERDICT: FAIL |
The field-visibility source that ApprovalServiceOptions.fieldVisibility and ApprovalService.attachFieldVisibility accept gained an optional query-side member, a new accepted key on a published accept set. That is an additive widening, graded minor, as the contract review on this head ruled. The changeset names the addition in one sentence; every other byte, the note to a self-composing host included, is unchanged. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
…e query-side member That double answered the read projection only, so after the masked-field redaction every serve and predicate read in the file took the fail-closed branch. It now answers the query-side member the same way the redaction test's double does: no field in the fixture carries a masking rule, so the answer equals the read projection. No assertion changes. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Rendered 2026-10-01T01:43Z on PR #20993 for card #20964 — the second record on this PR. The first, Head state at this reading: 35 check-runs latest-per-name, all complete — 30 ① Derived judgmentsWhat moved since
② Semver levelDeclared: Judgment: RIGHT, and exactly the remedy the first record asked for. The diff publishes one additive widening (① 1) in one released package; nothing is removed, narrowed or refused. Clause-②: yes (widening) ③ Boundary flagsEvery dev flag and
Implemented-by: VERDICT: PASS |
Fixes #20964
Clause-②: yes (widening)
The approval payload snapshot is redacted at serve time, keyed on the reading caller (#10749). It narrowed by the security service's read projection,
getReadableFields, alone. That projection counts a field whosemaskingRuleapplies to the reader as readable, because the data plane serves the column with its value replaced. So the snapshot kept the field and served it as it was captured at submission.The redaction now also reads the security contract's query-side answer,
getQueryableFields(landed for #20935 as83480c6a), and serves only the fields in both answers. A field served masked to this reader is dropped, with its derived label. There is no second derivation of the masking rule inplugin-approvals: who a rule applies to is the contract's answer, asked as the reader.Measured first (H1), by class
The measurement used a real boot:
bootStackwith the realSecurityPlugin,ObjectQLand SQL driver, REST, automation, the record-change trigger and the approvals plugin. It had one synthetic object with one capability-gated masked field, routed to a position held by two approvers. The evidence is private to the dispatch.The "after" column is the dogfood pin below, green at this PR's head.
The contract member read (H2)
ISecurityService.getQueryableFields, inpackages/spec/src/contracts/security-service.ts.getReadableFields, and "the difference between the two is exactly the fields this caller sees masked".plugin-security's implementation: the read projection keeps a field that is served masked, using the read path's own partial-mask set. The query-side answer reads the one query-guard derivation, which folds every masked-for-this-caller field in as non-queryable. Both start from the same field map (the evaluator, therequiredPermissionsfold and the delegator intersection). Their difference is therefore exactly the read path's masked set.So the member names the masked set by complement, and the redaction reads it. Nothing in
packages/specorplugin-securitychanges.Drop, not mask (H3), and why
The data plane answers this reader with the masked value. This PR drops the field instead, and that is a deliberate divergence in shape:
plugin-security's field masker produces the masked value. Reproducing it here would be the second copy of the masking rule that the triage forbids. Publishing it would be a new contract member, which belongs to the contract lanes and is outside this claim.Fail closed when the answer cannot be had
The contract obliges a consumer that cannot get the query-side answer not to read its absence as "nothing is masked". When the source has no such member, answers
undefinedor throws, the redaction serves no snapshot field for that object and logs it. An unresolvable read projection still passes the snapshot through whole, as before (#3807). With the security plugin wired, the member is always present and answers a list whenever the read projection does.Changes
packages/plugins/plugin-approvals/src/payload-redaction.ts:FieldVisibilitySourcegains the optionalgetQueryableFields.resolveReadableSnapshotFieldsanswers the read projection intersected with it, or fails closed as above. Both doors call this one function, and so does the free-text predicate check (approvals:listRequests' free-text filter pushespayload_json: { $contains: q }into the engine — a probe oracle over snapshot contents the caller may not read #11040), whose behaviour is unchanged.packages/plugins/plugin-approvals/src/approvals-plugin.ts: the service door's bridge to thesecurityservice forwards the member as well. The generic data door was already handed the service itself..changeset/20964-approval-snapshot-masked-field.md:patch. It includes a note for a host that buildsApprovalServicewith its own field-visibility source. Measured: no such producer exists in the tree, and the plugin bridge is the only one.payload-redaction.tsand "payload-redaction-middleware.tsonly where the wiring needs it". The service door's wiring lives inapprovals-plugin.ts, not in the middleware file. The middleware file needed no change. The unit pin and the dogfood pin both read the plugin wiring, and ablation B shows it is load-bearing.Pins (committed red before the fix)
packages/plugins/plugin-approvals/src/approval-payload-masked-field.test.ts, 13 cases. It uses a source double that answers both contract members per reader. Both doors drop the masked-for-this-caller field and keep the business fields. The derived label map is built and does not carry the dropped field. The unmasking reader is served the stored value (the control). The stored column keeps the whole row. Three fail-closed cases cover a missing,undefinedand throwing answer, and the unresolvable read projection still passes through. The plugin's bridge forwards the answer.a961b3310a) source in the tree: 9 red and 4 green. The green cases are the two controls, the audit case and the unresolvable pass-through, which hold both before and after.packages/qa/dogfood/test/approval-snapshot-masked-field.dogfood.test.ts, on a real boot as measured above, beside the inbox's existing route pin. It asserts by class: the reference (the data plane masks the field for this reader), the three approval reads (field absent for the reader the rule applies to, business field present), and the control (stored value on every read). Red at the pins commit and green after.approval-payload-redaction.test.ts(the existing [finding] approvals:sys_approval_request.payload_jsonsnapshots the full raw row and serves it to approvers unfiltered — object-levelhidden: trueand FLS declarations cannot reach inside the JSON blob #10749 pins): its source double gains the member the real service now has. Fixture triage: add the missing declaration. No field there carries a masking rule, so the answer equals the read projection, and every assertion is unchanged.Ablation, predicted before running, at
15f97c152eBoth suites resolve
plugin-approvalsfrom source: the unit suite imports it relatively, and the dogfood isolated project aliases it tosrc. So nodistleg applies. Each mutation went throughscripts/ablation-replace.mjs(anchor hit 1 to 0, blob moved, marker counted on disk). Each was restored to a blob equal to HEAD with an emptygit diff HEAD, and the tree was clean afterwards.payload-redaction.ts). Predicted and observed: 9 red and 4 green in the new unit file. The green cases are the two controls, the audit case and the unresolvable pass-through. The 16 existing redaction cases stayed green, and the dogfood pin went red.approvals-plugin.ts). Predicted and observed: only the bridge case is red (1 red and 28 green across both unit files). The dogfood pin went red: the service door fails closed and serves an empty snapshot to both readers.Verification
Verification ran at
15f97c152e. Line 2 is the measuredClause-②(H4): no export is added, and the one type addition is an optional member on an injected source. Any value of that member can only remove fields from what is served, never add one, and no input is refused. The other results (the derived gate set, the lint narrowing and the--ranreconciliation) are in the dev report on #20964, because this body is written once.@objectstack/plugin-approvals:test52 files and 804 passed.typecheckexit 0, including the test layer (no new debt).Acceptance notes
plugin-security, analytics and PM tooling). None touches this surface, so the branch is not merged with main.Patch rounds (the seat's append from the dev's report on #20964; the dev writes a body only once)
Patch round 1
Head
e6c9b9d56b(was15f97c152e): two commits, no merge ofmain. It applies the remedy in contract review5922625982(FAIL on the semver grade only) and changes nothing else.What changed
.changeset/20964-approval-snapshot-masked-field.md:@objectstack/plugin-approvalsgoes frompatchtominor.Clause-②: nobecomesClause-②: yes (widening), here and on this body's line 2. The seat edited line 2.getQueryableFields(object, context)member on the field-visibility source thatApprovalServiceOptions.fieldVisibilityandApprovalService.attachFieldVisibilityaccept.packages/plugins/plugin-approvals/src/approval-free-text-scope.test.ts: the file's visibility double gains the query-side member, in the same shape the first round gave the redaction test's double. No assertion changed.Verification at
e6c9b9d56b(every run under the shared verify lock)@objectstack/plugin-approvalstests: 52 files and 804 tests passed.typecheckexit 0. The test layer holds exactly at its ledger, and the edited file has 0 errors.15f97c152e): 11 warns over 13 passing tests;check-changeset-no-majorexit 0,check-adr-0087-registrationexit 0 (one non-breaking changeset),check-changeset-fixedexit 0.Clause-②: yes (widening): exit 0 onHEAD, and exit 1 on15f97c152ewith the required-minor refusal.dispatch-gates --commandsover the 7-path branch diff: 67 derived, 67 run, every one exit 0.--ran: 0 not measured.mainis 9 commits past the branch point. None of them touches this diff's paths, somainwas not merged.Generated by Claude Code