Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-security): report a metadata-store outage as an outage, not an absent declaration - #10707
Conversation
…t an absent declaration (#10424)
📓 Docs Drift CheckThis PR changes 1 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 14 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 eb7c0468ed0fc84187276c26bfc21900df0b1559 && git checkout eb7c0468ed0fc84187276c26bfc21900df0b1559
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 86a8ec93e03763f3a35f61a0c91a9c7231b50c6b f14fd1cde35515e71e756ba46d97bf53981e7c65 && git checkout -B drift-repro 86a8ec93e03763f3a35f61a0c91a9c7231b50c6b && git merge --no-ff f14fd1cde35515e71e756ba46d97bf53981e7c65
node scripts/docs-audit/affected-docs.mjs --json 86a8ec93e03763f3a35f61a0c91a9c7231b50c6b
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#10424
SecurityPlugin.getObjectSecurityMetaresolved an object's posture and setunresolved: !obj, whereobjcame fromIMetadataService.get. That read isambiguous by construction — its own TSDoc says
undefinedmeans "not found"and "every loader that could hold it failed", and directs callers to
getDiagnosed(#5840) wherever the difference could change a decision. Theverdict was already being computed and thrown away:
MetadataManagerdefinesgetas(await getDiagnosed(…)).data.So a metadata-store outage and a genuinely absent declaration arrived wearing
the same sentence, and that sentence said "Check that the object is declared
and published on this runtime." Correct for an absent object; actively wrong
mid-incident, where the declaration is fine and the store is not.
Measured before the change, not inferred
Three inputs through one tree in one run — an absent declaration, a store
reporting
degraded: true, and a service with nogetDiagnosedat all —produced byte-identical
messageand log line, withPERMISSION_DENIED/403 in all three. After the change the first and third are unchanged verbatim
and only the outage diverges.
What changed
UnresolvedPostureCausegains a third member,'metadata_unavailable', andthe four wording surfaces in
unresolved-posture.tsstate it distinctly:the refusal names the store, says the declaration may well be fine, and
the operator log line carries a grep-able
DEGRADED/metadata-store OUTAGE. A metadata-store outage is an incident and a query against a missingobject is routine; they were the same line.
probeMetadataOutage, run only on the path that isalready refusing.
What deliberately did NOT change
The resolving read is untouched byte for byte. The tempting shape — swapping
metadata.getforgetDiagnosedand taking.data— was rejected:objisthe input to a fail-closed deny (#3545), so sourcing it from an optional
member would put an externally observable accept/reject decision at the mercy
of every third-party
IMetadataServicewhosegetDiagnoseddisagrees with itsown
getor throws wheregetsucceeds. An object that resolves today wouldthen be refused. This is an explanation change and stays one. The same trade
was made once before, for the same reason (#6055, written down in #6505): buy a
second read on the miss path rather than presume an equivalence the contract
never made.
The deny is unchanged in every case — same
PermissionDeniedError, samePERMISSION_DENIED, same 403, still fail-closed — and the accept/reject setdoes not move. Pinned separately from the wording, in both directions.
The fail-safe direction
getDiagnosedis optional, and the contract is explicit that a consumerprobing for it must keep reading
getwhen it is absent. A service that doesnot implement it reports
'unknown'and keeps the previous wording — never'metadata_unavailable'. Publishing "I don't know" as "the store is down"would manufacture an incident out of a missing capability and send an operator
to read healthy dashboards.
'metadata_unavailable'is asserted on a positivedegraded === trueand on nothing else; a probe that throws, returns anon-boolean
degraded, or resolves to nothing lands on'unknown'.A degraded read also outranks the draft probe: "a draft exists but no published
one" has a second half the outage made unknowable, since the store is exactly
what could not answer.
Tests
21 new cases. The three-way pin (absent → existing wording, outage → new
wording, no-capability →
'unknown') plus the fail-safe variants, theprecedence rule, the #3545 refusal envelope in all three cases, and four
accept-side cases proving the resolving read did not move.
Two ablations, predicted before running, both matching exactly:
The second is the one that matters: a suite pinning only absent and outage goes
green on an implementation that reports an outage for everything. Both files
restored byte-identically (
git hash-objectequal before and after).Gates
Union derived with
node scripts/pm/dispatch-gates.mjs(no path arguments)after the final commit on a clean tree, at
f14fd1cde. All green: the 11path-derived families, the six convention-triggered ones (including
check:type-check-debt --re-measurewith the workspace closure built, andcheck:i18nreportingplugins/plugin-security in sync (4 bundle(s))), andcheck:nul-bytes.Note for the reviewer: the derivation named neither
check:route-envelopenorcheck:dispatcher-error-vocabulary; both were run explicitly, with--self-test, and both pass.Generated by Claude Code
Generated by Claude Code