Skip to content

fix(security): explain's field-mask layer reports partial masking — hidden / partially masked / readable (#9127) - #9328

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-9127-explain-partial-mask-reporting
Aug 17, 2026
Merged

fix(security): explain's field-mask layer reports partial masking — hidden / partially masked / readable (#9127)#9328
os-project-manager merged 1 commit into
mainfrom
claude/issue-9127-explain-partial-mask-reporting

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes#9127

What was wrong

#8993 landed partial masking on the enforcement channel: a field declaring maskingRule is no longer deleted from a masked caller's response — its value is replaced (13812345678 becomes 138****5678), with the field's requiredPermissions acting as the unmask gate.

The explain engine's field-mask layer predates that change and read only the binary mask from getFieldMask. On the one surface whose entire job is to describe enforcement, it stated two things that were not true — both reproduced verbatim by the ablation below:

caseshipped reportwhat the caller actually gets
maskingRule + a requiredPermissions gate the caller lacks1 field(s) masked from responses: [phone]the key IS served, carrying 138****5678
maskingRule with no gateNo field-level masking applies. (verdict not_applicable)masked for every non-system caller

They fail in opposite directions: the first overstates the protection in place, the second hides that any applies at all. security-service.ts's module contract claims explain "runs the same resolution/evaluator/compiler the enforcement path uses, so the explanation matches enforcement by construction" — the partial-mask dimension had fallen outside that construction.

The fix

The fls layer now reports the three states enforcement actually produces — hidden (key deleted), partially masked (key served, value replaced, applicable rule named) and readable — and answers narrows whenever either dimension bites.

Mirrored, not re-derived (the binding instruction from triage). The composition that decides which rules apply to a caller is computePartialMaskRulesplus the explicit-deny exclusion — a permission-set readable: false still wins outright, and that exclusion must consult the RAW evaluator map, because foldFieldRequiredPermissions forces readable: false on exactly the capability-gated fields a rule is meant to soften. That two-part composition was already duplicated at two enforcement sites; it is lifted into one SecurityPlugin.computeReadPartialMaskRules that result masking (step 4), the readable-field projection (getReadableFields) and now explain all call.

Explain reaches it through a new requiredExplainEngineDeps.getPartialMaskRules, and the hidden/partial split in the report is FieldMasker.maskResults' own rule (a field carrying a surviving rule is never in the delete set). The engine reports the three states; it derives none of them.

getPartialMaskRules is required rather than optional deliberately: the field-mask decision has three outcomes and the existing binary getFieldMask can express only two, so an engine wired without it would silently reproduce both misreports above. A compile error is the right way for that omission to surface. Every consumer in this repo goes through SecurityPlugin / the security service's explain() and needs no change.

Reverse verification — direction predicted before running

Predicted red on the two misreports, green on the states the old code already got right. The ablation kept the dep plumbing and reverted only the reporting rule, so nothing could fail for a missing-symbol reason. Observed, against the committed fix:

× PARTIAL (gated) … expected '1 field(s) masked from responses: [ph…' not to contain 'masked from responses'
× PARTIAL (gate-less) … expected 'not_applicable' to be 'narrows'
× explain reports the partially masked fields the read path actually serves masked (integration)
× explain drops the gated field from the masked set once the caller holds the unmask capability
… 8 failed | 1284 passed (1292)

Both card misreports reproduced exactly. Green in both directions, as predicted: the HIDDEN control, stays not_applicable when neither dimension applies, and both explicit-permission-set-deny cases — the state where a rule must NOT demote a deletion. The ablation was reverted from the commit and the tree re-verified clean (zero ABLATION markers).

Tests

New coverage pins all three states, in two places:

metadata-unresolvable-posture.test.ts builds its own deps literal and gained the same default.

Verification, at 032321660 (the final commit)

  • pnpm --filter @objectstack/plugin-security test66 files, 1292 tests, all passing
  • pnpm --filter @objectstack/plugin-security typecheck — clean
  • Gates derived with node scripts/pm/dispatch-gates.mjs against the actual changed paths, all run and passing: check:nul-bytes, check:authz-resolver, check:changeset-gate-self-tests, check:cross-package-test-inputs, check:objectui-changeset, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check-adr-0087-registration.mjs, check-changeset-no-major.mjs, check-cross-package-test-inputs.mjs, check-empty-changeset.mjs, check-affected-docs.mjs
  • The two gates needing a built workspace were run on a real build (turbo run build, 70/70 successful) rather than skipped: check:i18n — 9 packages, all bundles in sync; check:type-check-debt --re-measure — 33 ledger entries re-measured, 1926 raw tsc errors total, none above its recorded number. plugin-security carries a TEST_DEBT entry (errors: 11), so edits to its test files genuinely move this ratchet — hence the full run rather than a reasoned skip.

Changeset added (minor — the required dep is type-breaking for a direct embedder of the engine).


Generated by Claude Code

)
#8993 landed partial masking (`field.maskingRule`) on the enforcement
channel — a masked-for-caller field's value is REPLACED, not deleted, with
the field's `requiredPermissions` as the unmask gate. The explain engine's
`fls` layer predates it and read only the binary mask, so it misreported in
both directions: a gated rule field was listed as "masked from responses"
(the caller actually receives `138****5678`), and a gate-less rule field was
reported under "No field-level masking applies" (every non-system caller sees
it masked).
The layer now reports three states — hidden / partially masked (naming the
applicable rule) / readable — and answers `narrows` whenever either dimension
bites.
Mirrored, not re-derived: `computePartialMaskRules` plus the explicit-deny
exclusion is lifted into one `computeReadPartialMaskRules` that result
masking, the readable-field projection and explain all call, injected through
a new required `ExplainEngineDeps.getPartialMaskRules`. The hidden/partial
split is `FieldMasker.maskResults`' own, so the report cannot disagree with
the masking it describes — restoring `security-service.ts`'s own claim that
explain "matches enforcement by construction".
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 10 documentable anchor(s).

8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/authorization.mdx(via not_applicable (literal))
  • content/docs/permissions/explain.mdx(via not_applicable (literal))
  • content/docs/permissions/field-level-security.mdx(via SecurityPlugin (symbol))
  • content/docs/permissions/index.mdx(via SecurityPlugin (symbol))
  • content/docs/permissions/rls.mdx(via not_applicable (literal))
  • content/docs/plugins/packages.mdx(via SecurityPlugin (symbol))
  • content/docs/ui/forms.mdx(via SecurityPlugin (symbol), /forms/:slug/submit (route))
  • content/docs/ui/public-data-collection.mdx(via /forms/:slug/submit (route))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx(via SecurityPlugin (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 13 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 origin/mainpackageMentionDocs.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-project-manager@claude