Repository navigation
fix(crypto): fail loud instead of silently minting an ephemeral key (#1507) - #1509
Merged
Merged
Conversation
…1507) The default ICryptoProvider backs every secret-at-rest in the platform (encrypted settings, ObjectQL secret fields, datasource credentials). Its key resolution silently fell back to a per-process random key (or auto-minted a new on-disk key each boot) when no stable key was available, making every sys_secret value undecryptable after a restart in containers or on a second node — with no error at encrypt or boot time. - Rename InMemoryCryptoProvider -> LocalCryptoProvider (old name wrongly implied an ephemeral key); keep InMemoryCryptoProvider as a deprecated alias. - Add OS_SECRET_KEY as the canonical production master key (32-byte hex/base64), the documented production default. - Fail loud in production: when NODE_ENV=production and no stable key source (env var or pre-existing persisted file) exists, throw an actionable error at construction instead of generating a key. Never auto-mint a key in production. Dev/test keep the ergonomic fallback. - serve surfaces the production-key error verbatim and refuses to wire an unstable provider for secret fields. - Document the env-key default and reframe the provider in the README; update spec contract and downstream comments. - Add LocalCryptoProvider test coverage for every key-resolution tier. https://claude.ai/code/session_01Qzri77Foepw1hnw52Vm3pP
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…VGl6 # Conflicts: # packages/cli/src/commands/serve.ts
os-zhuang
marked this pull request as ready for review
June 2, 2026 03:20
The datasource-admin block is the first to touch `sharedCryptoProvider` (declared `undefined` just above), so the `?? new LocalCryptoProvider()` left operand was always nullish — a useless conditional. Assign directly; the secret-field wiring below still reuses the one instance. https://claude.ai/code/session_01Qzri77Foepw1hnw52Vm3pP
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1507.
Problem
The default
ICryptoProviderbacks every secret-at-rest in the platform — encrypted settings (sys_setting.value_enc), ObjectQLsecretfields, and runtime datasource credentials. Its key resolution fell back, silently, to a fresh per-processrandomBytes(32)key (or auto-minted a new on-disk key on every boot) when no stable key was available.In an ephemeral-FS container or a multi-node cluster, each restart / each node then encrypts under a different key, and every previously-written
sys_secretvalue becomes undecryptable. The failure was invisible at encrypt and boot time and only surfaced later as "all my saved passwords / API keys / DB credentials fail to decrypt".Changes
Implements all three proposals from the issue:
Fail loud instead of going ephemeral. When
NODE_ENV=productionand no stable key source (env var or a pre-existing persisted file) is available, the provider now throws an actionable error at construction instead of generating a key. It never auto-mints a key in production. Development and test keep the ergonomic fallback (persisted dev key / ephemeral test key, with loud warnings).Ship a persistent env-master-key default. Added
OS_SECRET_KEYas the canonical production master key (32-byte hex or base64), the documented production default.OS_DEV_CRYPTO_KEYremains the dev convenience key. KMS / Vault stay as future plug-ins behind the sameICryptoProviderseam.Reframed the provider. Renamed
InMemoryCryptoProvider→LocalCryptoProvider(the old name wrongly implied an ephemeral key).InMemoryCryptoProviderstays as a deprecated alias for backward compatibility, so existing importers (serve, datasource binder, tests) keep working.Key resolution order (first match wins)
opts.key(explicit)OS_SECRET_KEYOS_DEV_CRYPTO_KEY(legacyOBJECTSTACK_DEV_CRYPTO_KEY)~/.objectstack/dev-crypto-keyA present-but-malformed
OS_SECRET_KEYis treated as an explicit operator error and throws in every mode (never silently diverges to a different key).Boundary note
Per the issue: secrets surviving a restart is correctness, not a premium feature.
LocalCryptoProviderand the env-key path stay open-source; KMS, automatic rotation, and per-tenant key isolation remain the paid layer.Files
packages/services/service-settings/src/local-crypto-provider.ts— new implementation (renamed + fail-loud +OS_SECRET_KEY).…/in-memory-crypto-provider.ts— now a backward-compat re-export shim.…/local-crypto-provider.test.ts— coverage for every key-resolution tier (env hex/base64, prod fail-loud, prod pre-existing file, no-mint-in-prod, dev persist, test ephemeral, legacy alias, AAD, rotation, digest).settings-service-plugin.ts,cli/serve.ts,objectql/engine.ts,datasource-secret-binder.ts,spec/crypto-provider.ts— wiring + docs updated toLocalCryptoProvider;servesurfaces the production-key error verbatim.README.md— documents theOS_SECRET_KEYdefault and fail-loud behaviour.Testing
@objectstack/service-settings: 93 tests pass (incl. the new suite).@objectstack/service-datasource-admin: 29 pass.@objectstack/clibuild (51 packages, incl.serve.ts) and@objectstack/objectqlbuild succeed.Compatibility / behaviour change
Only deployments explicitly running
NODE_ENV=productionwithout a key and without a pre-existing key file will now refuse to boot (with actionable guidance) — which is exactly the silently-broken case this fixes. Dev (NODE_ENVunset/development) is unchanged apart from louder warnings.InMemoryCryptoProviderimports keep working via the alias.https://claude.ai/code/session_01Qzri77Foepw1hnw52Vm3pP
Generated by Claude Code