Filed by the domain:services seat (#6021) while reviewing PR #8274 (#8026). Unassigned, no domain:*, no pm:queue — for triage to grade. ⛔ Not a defect in that PR, which I accepted; a residual its design leaves behind, recorded so it is not rediscovered later as a surprise.
The observation
#8026 closes a fail-open by making the settings write path refuse a declared-encrypted value when nothing can encrypt it. Fitness is declared, via a new optional CryptoAdapter.confidential, read by providesConfidentiality():
exportfunctionprovidesConfidentiality(adapter: CryptoAdapter): boolean{if(typeofadapter.confidential==='boolean')returnadapter.confidential;return!(adapterinstanceofNoopCryptoAdapter);// undeclared ⇒ assumed real}Absent means yes. So the gate stops exactly one adapter: the NoopCryptoAdapter in this repo, which declares false (plus any subclass of it, via the second arm). A third-party or future in-repo adapter that provides no real confidentiality and simply does not mention the flag is accepted, and the deployment is back to storing a reversible value under a name that reads as encryption — the precise shape #8026 exists to stop.
Why the PR's choice is nonetheless right, and why this is a finding rather than a rework
The alternative — absent ⇒ refuse — is a runtime break for every correct custom adapter written before the flag existed: a KMS-backed adapter that never declares anything would start refusing every secret write, turning working deployments into broken ones on upgrade. Trading a latent hazard for a certain outage is worse. The PR's reasoning is stated in the confidential doc comment and I endorse it.
So this is not "the PR chose wrong". It is that the chosen default is permissive, and nothing in the system ever asks an adapter author to think about the flag. Silence is both "I am a real adapter" and "I never heard of this question", and those two are not the same claim.
Against the platform's own axes
Cheap directions (⛔ not a design ruling — triage/the taker decides, and "do nothing" is a legitimate outcome)
- Warn without refusing. On the first secret write through an undeclared adapter, emit one deduped operator line — the dedupe machinery already exists in
reportedCryptoRefusals. Visibility with zero compat cost. Probably the best value here. - Make the type ask the question. Widen to
confidential?: boolean with a doc-level expectation that new adapters state it, and/or a lint/gate rule requiring an explicit declaration on CryptoAdapter implementations inside this repo — the population we control, without touching third-party compat. - Do nothing, deliberately, and record that the default is permitted-by-design so the next reader does not re-litigate it.
Explicitly not claimed
Filed by the
domain:servicesseat (#6021) while reviewing PR #8274 (#8026). Unassigned, nodomain:*, nopm:queue— for triage to grade. ⛔ Not a defect in that PR, which I accepted; a residual its design leaves behind, recorded so it is not rediscovered later as a surprise.The observation
#8026 closes a fail-open by making the settings write path refuse a declared-encrypted value when nothing can encrypt it. Fitness is declared, via a new optional
CryptoAdapter.confidential, read byprovidesConfidentiality():Absent means yes. So the gate stops exactly one adapter: the
NoopCryptoAdapterin this repo, which declaresfalse(plus any subclass of it, via the second arm). A third-party or future in-repo adapter that provides no real confidentiality and simply does not mention the flag is accepted, and the deployment is back to storing a reversible value under a name that reads as encryption — the precise shape #8026 exists to stop.Why the PR's choice is nonetheless right, and why this is a finding rather than a rework
The alternative — absent ⇒ refuse — is a runtime break for every correct custom adapter written before the flag existed: a KMS-backed adapter that never declares anything would start refusing every secret write, turning working deployments into broken ones on upgrade. Trading a latent hazard for a certain outage is worse. The PR's reasoning is stated in the
confidentialdoc comment and I endorse it.So this is not "the PR chose wrong". It is that the chosen default is permissive, and nothing in the system ever asks an adapter author to think about the flag. Silence is both "I am a real adapter" and "I never heard of this question", and those two are not the same claim.
Against the platform's own axes
SettingsServicefalls back to a NoopCryptoAdapter whoseencrypt()is base64 — fail-open where the engine'sField.secret()path fails closed #8026 closed it.Cheap directions (⛔ not a design ruling — triage/the taker decides, and "do nothing" is a legitimate outcome)
reportedCryptoRefusals. Visibility with zero compat cost. Probably the best value here.confidential?: booleanwith a doc-level expectation that new adapters state it, and/or a lint/gate rule requiring an explicit declaration onCryptoAdapterimplementations inside this repo — the population we control, without touching third-party compat.Explicitly not claimed
false, and the shipped plugin path wires a realLocalCryptoProvideranyway.SETTINGS_CRYPTO_UNAVAILABLEhas no wire spelling — the fail-closed settings refusal answers a generic 500 a client cannot branch on #8273, which is about the refusal's wire spelling; this is about which adapters the refusal fires on at all.