Skip to content

fix(approvals): a snapshot field the reader is served masked is no longer served as stored (#20964) - #20993

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20964-approval-snapshot-mask
Oct 1, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20964-approval-snapshot-mask

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

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 whose maskingRule applies 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 as 83480c6a), 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 in plugin-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: bootStack with the real SecurityPlugin, ObjectQL and 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.

Read, same record Reader the rule applies to: before after Reader who lifts the rule (control): before after
Approvals inbox, list stored absent stored stored
Approvals inbox, item stored absent stored stored
Generic data door on the request object stored absent stored stored
Data plane read of the subject record (reference) masked masked stored stored

The "after" column is the dogfood pin below, green at this PR's head.

The contract member read (H2)

ISecurityService.getQueryableFields, in packages/spec/src/contracts/security-service.ts.

  • By its declaration: it is a subset of getReadableFields, and "the difference between the two is exactly the fields this caller sees masked".
  • By 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, the requiredPermissions fold 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/spec or plugin-security changes.

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:

  • The contract publishes which fields are masked for a reader, not the masked value. Only 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.
  • Dropping is the fail-closed side of the data plane's answer. It discloses strictly less than the masked value would (no kept characters, no length). It is also the shape this seam already serves for a field the reader may not read at all, so a drawer sees one "not for you" shape.

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 undefined or 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: FieldVisibilitySource gains the optional getQueryableFields. resolveReadableSnapshotFields answers 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 pushes payload_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 the security service 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 builds ApprovalService with its own field-visibility source. Measured: no such producer exists in the tree, and the plugin bridge is the only one.
  • Surface note: the claim's file list names payload-redaction.ts and "payload-redaction-middleware.ts only where the wiring needs it". The service door's wiring lives in approvals-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, undefined and throwing answer, and the unresolvable read projection still passes through. The plugin's bridge forwards the answer.
    • Measured with the pins commit's (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_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 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 15f97c152e

Both suites resolve plugin-approvals from source: the unit suite imports it relatively, and the dogfood isolated project aliases it to src. So no dist leg applies. Each mutation went through scripts/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 empty git diff HEAD, and the tree was clean afterwards.

  • A, the redaction ignores the query-side answer (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.
  • B, the plugin bridge stops forwarding the member (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 measured Clause-② (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 --ran reconciliation) are in the dev report on #20964, because this body is written once.

  • @objectstack/plugin-approvals: test 52 files and 804 passed. typecheck exit 0, including the test layer (no new debt).
  • The dogfood pin and the existing inbox override pin: 2 files and 2 passed.

Acceptance notes

  • Main moved by four commits after this branch was cut (formula, the explain engine in plugin-security, analytics and PM tooling). None touches this surface, so the branch is not merged with main.
  • The masked value is not reproduced, so an approver who may see a field masked on the data plane sees no value for it in the approval drawer. Whether a contract member that serves the masked value is wanted is left to the seat.

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 (was 15f97c152e): two commits, no merge of main. It applies the remedy in contract review 5922625982 (FAIL on the semver grade only) and changes nothing else.

What changed

  • .changeset/20964-approval-snapshot-masked-field.md:
    • @objectstack/plugin-approvals goes from patch to minor.
    • Clause-②: no becomes Clause-②: yes (widening), here and on this body's line 2. The seat edited line 2.
    • One sentence is added. It names the one public-surface addition: an optional getQueryableFields(object, context) member on the field-visibility source that ApprovalServiceOptions.fieldVisibility and ApprovalService.attachFieldVisibility accept.
    • Every other byte is unchanged, including the FROM → TO note to a self-composing host.
  • 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.
  • Not touched: the fix, the service-door bridge, the unit pins and the dogfood pin.

Verification at e6c9b9d56b (every run under the shared verify lock)

  • @objectstack/plugin-approvals tests: 52 files and 804 tests passed. typecheck exit 0. The test layer holds exactly at its ledger, and the edited file has 0 errors.
  • The fail-closed branch in the free-text file, measured once with a recording logger:
    • before (the double at 15f97c152e): 11 warns over 13 passing tests;
    • after: 0 warns over the same 13.
  • Changeset gates: check-changeset-no-major exit 0, check-adr-0087-registration exit 0 (one non-breaking changeset), check-changeset-fixed exit 0.
  • The level axis, driven offline with a synthetic pull-request payload that declares Clause-②: yes (widening): exit 0 on HEAD, and exit 1 on 15f97c152e with the required-minor refusal.
  • dispatch-gates --commands over the 7-path branch diff: 67 derived, 67 run, every one exit 0. --ran: 0 not measured.
  • main is 9 commits past the branch point. None of them touches this diff's paths, so main was not merged.

Generated by Claude Code

claude added 2 commits October 1, 2026 00:23
…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>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 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
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8d329f02eeb571170bba3b275df3c2fc3cb749b3 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from d9b226c6ad10e9cf69f0992dee6b5a7bdbb8150c — the merge of head e6c9b9d56bfa2fe63ad4481a6b814ab337e2662e into base 8d329f02eeb571170bba3b275df3c2fc3cb749b3, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 15f97c152ee12ab9b527a6db38fb0b72a3bb7f6f
Local-runs: none

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 main at the branch point 5f6b63a6fd, and the latest check-run per name on this head. Nothing was built, run or re-run; the diff was read with git fetch and git diff only. The security family's disclosure discipline holds below: every fact is stated by class and position.

Head state at this reading: every check-run complete — 32 success, 3 skipped (Build Docs, Console Pin Gate, the opt-in tarball smoke), 0 failed, 0 in progress. All seven required contexts are green, every Test Core and Dogfood Regression Gate shard included. Those conclusions are the gate verdicts; the verdict below does not turn on them.

① Derived judgments

Each accept-set or public-surface change the diff implies, named and judged.

  1. Public surface — the field-visibility source gains an optional query-side member (payload-redaction.ts :88–104, FieldVisibilitySource.getQueryableFields?). The interface is not exported by name, but it is reachable from the package entry: ApprovalService.attachFieldVisibility takes it and ApprovalServiceOptions.fieldVisibility carries it, and both symbols are on index.ts. By the review rule (a type reachable from the entry type graph is a published accept set), this is an additive widening: the source a host hands in may now carry one more key, and the seam reads it. The design is RIGHT — optional, mirroring the contract member's own optionality on ISecurityService, with the contract's prescribed fail-closed reading when absent. Its grading is judged in ②.

  2. Served set — the snapshot is narrowed to the read projection intersected with the query-side answer, on both doors (resolveReadableSnapshotFields :186–224; the service door's redaction and the generic door's row rewrite both call it, and the derived label and display maps are built from the redacted keys by the existing ordering). RIGHT. The contract declares the two answers differ by exactly the fields the caller is served masked; plugin-security computes both from one field map (resolveProjectionFieldMask), so the intersection names the masked set without any masking rule being read in plugin-approvals. This is a runtime security behaviour change, which the standing rules place outside clause ② (the negative boundary), so it does not enter ②.

  3. Fail closed when the query-side answer cannot be had — member absent, undefined, or a throw ⇒ [], logged at warn. RIGHT in direction: the contract forbids reading absence as "nothing is masked". [] is stricter than the contract's floor (treat every field that declares a masking rule as not queryable) and is the only derivation-free answer open to a seam the claim forbids from reading masking declarations. Reached only by a source that predates the member: at this head attachFieldVisibility is called by the plugin bridge and by tests alone, so no producer in the tree reaches it. warn is the right level — a visible functional degradation; nothing claimed persisted is lost.

  4. Fail open preserved for the read projection — undefined or a throw from getReadableFields still serves the snapshot whole (approvals: a department approver never resolves when the business unit has organization_id = null (every seeded BU) #3807; docblock case 3 now names the undefined answer explicitly, no behaviour change). RIGHT. The asymmetry with item 3 is deliberate: falling open on a query-side failure would serve exactly the values the mask hides, and since both answers share one derivation in plugin-security, a shared outage still fails open through the first.

  5. Free-text predicate (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) unchanged. freeTextMayMatchSnapshot distinguishes only undefined from a list, and the new resolver answers undefined in exactly the cases it did before. RIGHT.

  6. Service-door bridge forwards the member (approvals-plugin.ts :249–260); the generic door is handed the service itself and reads the member directly. RIGHT and load-bearing by code reading: the attached source is a wrapper object, so without the forwarded member the service door would answer [] for every reader while the generic door narrowed correctly.

  7. Drop, not mask. RIGHT within the claim: the contract publishes which fields are masked for a caller, not the masked value; reproducing the mask would be the second derivation the triage forbids, and publishing it would be a new contract member outside this claim. Dropping discloses strictly less than the data plane's answer and is the shape the seam already serves for an unreadable field.

  8. Snapshot at rest unchanged (the Option B ruling the seam's header records: redact at serve time, keep the audit evidence). RIGHT; pinned.

  9. Hard stops honoured. No path under packages/spec/src, plugin-security/src, or any governed surface. main moved four commits after the branch point; their 18 files are disjoint from this diff (verified on the file lists), so not merging is sound.

  10. Pins. The unit pins cover both doors, the derived label map, the control reader, the at-rest column, three fail-closed shapes, the pass-through shape and the bridge; the dogfood pin asserts the data-plane reference, the three approval reads and the control on a real boot. The existing redaction fixture gains the member (no masked field there, so its assertions stand). Fixtures are synthetic, and the pins land in the same head as the fix, so they exercise no open gap. RIGHT.

② Semver level

Declared: @objectstack/plugin-approvals: patch, with Clause-②: no on PR body line 2 and in the changeset, no arm.

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 minor, and a fix( type never lowers what the act requires. The fleet graded this same act the same way, twice, in the #20935 landing this PR builds on (its changesets are still pending on main): ISecurityService.getQueryableFields? (an optional member on a published interface) shipped minor under Clause-②: yes (widening), and AnalyticsServiceConfig.getQueryableFields (an optional hook on a host-composition surface, the plugin wiring it itself) shipped minor under a yes whose body names the hook as "the one public-surface addition" (its (narrowing) arm came from that PR's refusals, which this one has none of). Grading the same optional-hook addition patch / no here is the two-reviews-opposite-bumps defect the #15294 ruling exists to end.

Open question 3, left to this review: it is NOT no (narrowing). The arm reads accept-set narrowings; here nothing that was accepted is refused — every source that type-checked still type-checks and is still accepted. What shrinks for a host-built source lacking the member is the served content, which is (a) the fail-closed default the contract itself prescribes for a consumer of this member and (b) a runtime security behaviour, the negative boundary clause ② does not read. So no BREAKING banner and no ADR-0087 disposition marker are owed. The changeset's FROM → TO note to a self-composing host is right to keep.

Required changeset level: minor. Required declaration, in both carriers (PR body line 2 and the changeset body):

Clause-②: yes (widening)

With that, the level axis of check-changeset-no-major is satisfied by the one moved package graded minor, and check-adr-0087-registration sees a non-breaking changeset, as it does now.

③ Boundary flags

Every dev flag and open_questions entry, answered or escalated.

  • OQ1 — the bridge edit in approvals-plugin.ts is outside the claim's literal file list. Answered: accept on the merits, judged independently of the ACCEPT comment. It is the wiring the claim provided for, in the same package the claim's serial constraints cleared, load-bearing by code reading (① 6), and no other open claim or PR names the file. Reporting it as an open question was the prescribed handling of a surface breach.
  • OQ2 — keep drop, or open a contract card for a masked-value member. Answered: keep drop (① 7). A member that serves the masked value is the contract lanes' route and would be a widening of packages/spec; nothing here blocks it and no pull is measured. No card from this review. The acceptance-note line "left to the seat" is escalated to the seat as it stands.
  • OQ3 — no versus no (narrowing). Answered in ②: neither; yes (widening) at minor.
  • Dev flag — "Surface note" in the PR body. The same matter as OQ1; answered.
  • Dev flag — main not merged (four commits). Verified disjoint by file; no re-check owed.
  • Dev flag — the "measured" Clause-② no. The instrument judged every file NOT MEASURED (no declared surface), so the no was a semantic reading, not a measurement; ② replaces it. Consequence for the seat: the claim's own Clause-②: no line is superseded by the PR's measured grammar, and the standing clause-② routing rule for a card that widens a public surface is the seat's to apply, not this review's.
  • out_of_scope_findings[0] — the contract text and the masking-rule describe disagree on a caller with zero permission sets. Not this PR's to close; the card thread records it as filed for triage. The approval seam inherits whatever the data plane does for that caller, so this PR neither widens nor closes it.
  • out_of_scope_findings[1] — no dedicated pin for a masked lookup field in the derived display map. Covered by the shared redact-before-enrich ordering; acceptance-note material, no pin owed by this card.
  • Reviewer's flag (non-blocking): the free-text scope test's own visibility double (approval-free-text-scope.test.ts :102–110) was not given the member, so in that file the serve path now takes the fail-closed branch and serves empty snapshots with a warn per read — unasserted there, so green, and the predicate pins are unaffected (① 5). The fixture triage the dev applied to the redaction test's double belongs here too; fold it into the fix-up push.
  • Check-runs: all green on this head at this reading (above). The next head re-owes its own check-runs; a PASS there is conditional on every required context green, as the standing rule requires.

Remedy for the next head (then a new record on that head): changeset level minor; Clause-②: yes (widening) on PR body line 2 and in the changeset; optionally the test-double member above. Nothing else is asked of the diff — items ① 2 to 10 stand.

Implemented-by: claude/issue-20964-approval-snapshot-mask
Reviewed-by: session_01XY5uCwTjZj7884yYtyur4H (the contract-review subagent of the dispatching session, served at CONTRACT_REVIEW_TIER)

VERDICT: FAIL

claude added 2 commits October 1, 2026 01:09
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>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: e6c9b9d56bfa2fe63ad4481a6b814ab337e2662e
Local-runs: none

Rendered 2026-10-01T01:43Z on PR #20993 for card #20964 — the second record on this PR. The first, 5922625982 at 15f97c152e, was FAIL on ② only. Inputs: the card body and its six comments (triage, claim, two dev reports, ACCEPT, the patch-round dispatch — the last four read as thread entries, not as conclusions adopted), the PR body and file list, the net diff against main at this head (merge base 5f6b63a6fd), the delta between the two heads, and the latest check-run per name on this head. Nothing was built, run or re-run; every read was a REST GET or a git show of an object already in the local store. The security family's disclosure discipline holds below: every fact is stated by class and position.

Head state at this reading: 35 check-runs latest-per-name, all complete — 30 success, 5 skipped, 0 failed, 0 in progress. All seven required contexts are green, every Test Core and Dogfood Regression Gate shard included. The five skipped: Build Docs, Console Pin Gate and the opt-in tarball smoke (path-filtered, as on the first head), plus Auto Label and Check PR Size on the edited re-fire — their push-triggered runs on this same head are success, and their if: excludes edited by design. Check Changeset ran twice on this head: once on the push (PR body line 2 then still no) and again on the body edit (line 2 now yes (widening)); both success. The second is the run that judged the declaration this record judges, since that job reads the clause-② line out of the event's PR body. Those conclusions are the gate verdicts; the verdict below does not turn on them.

① Derived judgments

What moved since 15f97c152e: two commits touching two files — the changeset (level, declaration, one added sentence) and the free-text scope test's visibility double. The other five files of the net diff are byte-identical to the head the first record judged, so items ① 2 to 10 of 5922625982 stand unchanged. Re-judged here: the item the new commits touch, and the one they bear on.

  1. Public surface — the field-visibility source's optional query-side member (payload-redaction.ts :96–104). Unchanged in code; re-read because the changeset's new sentence must name it correctly. It does: ApprovalServiceOptions (a type) and ApprovalService (a class with the public attachFieldVisibility) are both on index.ts, so the member is a new accepted key on a published accept set; the interface itself is not exported by name, and the sentence does not claim it is. RIGHT.

  2. The intersection names exactly the masked-for-this-caller set — re-verified at the implementation rather than taken from the declaration, since the fix's correctness rides on it. In plugin-security (security-plugin.ts, which main has not moved since the branch point): the read projection is the shared field map's readable fields plus the read path's effective partial-mask set; the query-side answer is the same field map (same requiredPermissions fold, same delegator intersection) with every applicable masking rule forced non-queryable. Their difference is therefore the read path's partial-mask set and nothing else — a field unreadable for a non-mask reason is in neither answer, so the intersection never drops a readable, unmasked field. The settled answers (undefined, system, zero permission sets, []) come from one shared resolver, so the two cannot disagree on them. RIGHT; the contract's own docblock (security-service.ts, byte-identical between the branch point and main) states the same relation.

  3. Free-text scope test double (approval-free-text-scope.test.ts :150–160). Gains the member in the redaction test's exact shape (system context → every key; otherwise the fixture's per-object answer), not recorded in _calls. Effect: that file's serve path no longer takes the fail-closed branch, which was the first record's non-blocking flag. The predicate pins are unaffected: freeTextMayMatchSnapshot distinguishes only undefined from a list, and the resolver answers undefined in exactly the cases it did before the member existed. The _calls ceiling (:361) keeps counting read-projection asks, which is what it pinned. RIGHT — fixture triage, no behaviour change, no assertion moved.

  4. Nothing else moved. The fix, the service-door bridge, both unit pins and the dogfood pin are identical to the first head. main is now 10 commits past the branch point (74 files); 0 of them overlap the 7 paths, and plugin-security's moves are the explain engine and its tests, not the projection code this fix reads. Not merging main stands. RIGHT. Both new commits carry the model-free trailer pair and no model identifier.

② Semver level

Declared: '@objectstack/plugin-approvals': minor; Clause-②: yes (widening) in the changeset body (bare, on a line of its own) and on PR body line 2. No (narrowing) arm, no BREAKING banner, no ADR-0087 marker.

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. yes (widening) is the fixed grammar's spelling for that, minor is the floor it takes, and a fix( type cannot lower it. The dogfood package is private and publishes nothing; the test files publish nothing; the fixed group's lockstep is unaffected by one minor. The gate readings on this head agree: Check Changeset (the level axis reads line 2 against the grade of the package the diff moved) is success on the edited body, and the ADR-0087 registration gate sees a non-breaking changeset. no (narrowing) remains wrong for the reason the first record gave: no accepted input is refused; what shrinks for a self-composing host whose source lacks the member is served content — the fail-closed default the contract itself prescribes for a consumer of this member, a runtime security behaviour outside clause ②. The changeset keeps its FROM → TO note to such a host, and the one added sentence names the addition without a field spelling or a value.

Clause-②: yes (widening)

③ Boundary flags

Every dev flag and open_questions entry, answered or escalated.

  • Remedy from 5922625982. minor: done. yes (widening) in both carriers: done — the changeset by the dev, PR body line 2 by the seat, as the patch-round comment said it would be. The test double: done. The remedy is complete and correct, and the round changed nothing else (① 4).
  • Round-2 dev report. open_questions: [], out_of_scope_findings: []. Two declared deviations: the report's first line is the bare marker because the fleet's reader does not recognise the parenthesised form (a measured reason, the right call), and the commit trailers are the model-free pair rather than the harness-suggested one (verified on both commits). Both answered: correct.
  • "PR body line 2 is the seat's to patch." Verified patched; the edited event re-fired Check Changeset, which is green.
  • Stale prose in the PR body — non-blocking, no gate reads it. The "Changes" bullet still says the changeset is patch, and the "Verification" paragraph still argues the reading that produced no. The appended "Patch round 1" section supersedes both and says so, and line 2 is the carrier the gates read. Nothing is owed for this verdict; the seat may strike or annotate those two spots if it edits the body again.
  • Carried from the first record, unchanged by this round: OQ1 (the bridge edit in approvals-plugin.ts) accepted on the merits; OQ2 (drop, not mask) keep drop — the acceptance-note line "left to the seat" stands escalated to the seat as it was; out_of_scope_findings[0] is now security(plugin-security): for a caller who resolves no permission set, the field-projection answers say no masking rule reaches it, while maskingRule's describe and the result masker mask it — which one a public door serves is not measured #20995, filed abstract by the seat for triage; [1] is acceptance-note material. No new card from this review.
  • Check-runs: all green on this head (above). This record binds to e6c9b9d56b; a further push re-owes a record unless it is a pure regeneration.

Implemented-by: claude/issue-20964-approval-snapshot-mask
Reviewed-by: session_01XY5uCwTjZj7884yYtyur4H (the contract-review subagent of the dispatching session, served at CONTRACT_REVIEW_TIER)

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 1, 2026 01:44
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 1, 2026 01:44
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 1, 2026
Merged via the queue into main with commit 2f2fa11 Oct 1, 2026
44 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-20964-approval-snapshot-mask branch October 1, 2026 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants