Skip to content

SettingsService falls back to a NoopCryptoAdapter whose encrypt() is base64 — fail-open where the engine's Field.secret() path fails closed #8026

Description

@huangyiirene

Filed by the domain:services PM seat from the #7902 credential-persistence survey (report: comment 5264546868). Unassigned, no pm:queue. For triage to grade.

⚠️This is NOT a live leak, and the card should not be read as one. On the shipped plugin path a real LocalCryptoProvider is always wired (settings-service-plugin.ts:204). Filed on direction, not on impact.

The observation

SettingsService constructed without a cryptoProvider + secretStore falls back to NoopCryptoAdapter (settings-service.ts:537), whose encrypt() is:

'b64:' + base64(plaintext)

crypto-adapter.ts:30-33. That is encoding, not encryption: trivially reversible, and it produces a value that looks protected (prefixed, opaque at a glance) while being plaintext with extra steps.

Why it is worth recording despite not being live

The direction is the opposite of the engine's. The Field.secret() path refuses the write when no CryptoProvider is present — measured on #7799/PR #7901, where the fail-closed behaviour is what makes the webhook secret migration safe on a provider-less deployment (the row stays cleartext and is reported, rather than being silently "encrypted" into something reversible).

So the platform currently has two credential-encryption paths with opposite failure modes:

pathno CryptoProvider ⇒
engine Field.secret()refuses the write (fail-closed)
SettingsServicewrites base64 (fail-open)

The survey's whole subject is credentials at rest. A fallback that yields a reversible value under a name that reads as encryption is the shape most likely to be mistaken for protection by the next author — and by the next audit, since value_enc being populated looks like success from the storage side.

What a fix would be, if wanted (⛔ not decided here)

Make the settings path fail closed like the engine's: refuse to persist an encryptedKeys value when no real provider is wired, and say so loudly at boot. The measured fact that the shipped path always wires one is what makes this cheap — if nothing legitimate depends on the Noop fallback, removing it costs nothing and closes the divergence.

The card should first establish that: who actually constructs SettingsService without a provider? If the answer is "only tests", the fix is small and the tests move to an explicit fake. If some real host relies on it, that is a different and more interesting conversation.

Explicitly NOT claimed

  • No live leak. Shipped plugin path wires LocalCryptoProvider unconditionally. This is a hazard-by-construction card.
  • No accusation that settings credentials are unprotected today — the survey verified the opposite: everyservice-settings credential specifier is shape ③ (manifest type: 'password'encryptedKeyssys_secret handle in value_enc, value nulled). That is the mechanism that makes settings the cleanest family in the whole 26-surface sweep.
  • ⚠️ Note for whoever picks this up: the manifest's type: 'password' is not the objectql Field type and has nothing to do with ADR-0100 — it means "encrypt this". Easy to misread as a password-field hit; it is the opposite.

Routing note

Landing site is packages/services/service-settings ⇒ plausibly domain:services, but the divergence it is measured against lives in packages/objectql, so a triage that routes it to domain:engine-core as "make the two paths agree" would also be defensible. Not guessed here.

Evidence

Measured 2026-08-12 by the dev on #7902 (read-only survey, pr: null). Full 26-surface verdict table in comment 5264546868.

Source

Split out of #7902 (survey residual (c)). Siblings: #7986, #7987, #7990, #8009, #8011, #8025.

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions