Repository navigation
fix(core): encrypt module credential fields, and read plaintext safely - #680
seanspeaks wants to merge 11 commits into
Conversation
Field-level encryption used to be skipped whenever STAGE was dev, test or local, and silently turned off (with a warning) when no key was configured. `frigg deploy` defaults to --stage dev, so a plain deploy stored credentials in plaintext. The rule now keys off where the code runs, not the stage name, and lives in one place (database/encryption/encryption-config.js) shared by the Prisma client, DocumentDBEncryptionService, the integration-mapping helpers and the encryption health check: - Deployed (in Lambda, not serverless-offline / invoke local / Jest): KMS_KEY_ARN or AES_KEY_ID+AES_KEY encrypts on every stage, dev included. No key throws EncryptionConfigurationError at Prisma client creation, naming the fix, unless FRIGG_ENCRYPTION_DISABLED=true opts out explicitly (warned once per cold start). A configured key wins over the opt-out. - Local (frigg start, tests, scripts): unchanged. dev/test/local skip encryption; other stages encrypt when a key is set. The Prisma client also stops continuing without encryption when the extension fails to initialise. /health/detailed reports a deployed stage without a key as unhealthy and an opt-out as disabled; it no longer reads BYPASS_ENCRYPTION_STAGE, which never switched encryption off. BREAKING CHANGE: a deployed stage with no KMS_KEY_ARN and no AES_KEY_ID/AES_KEY now refuses to start instead of writing plaintext, including stages named dev, test or local. Set encryption.fieldLevelEncryptionMethod: 'kms' (or provide AES keys), or opt out with FRIGG_ENCRYPTION_DISABLED=true. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`encryption: { fieldLevelEncryptionMethod: 'none' }` is the declarative
way to run a deployed stage without field-level encryption now that core
refuses to start without a key. The environment builder turns it into
FRIGG_ENCRYPTION_DISABLED=true on every function, and the SSM offload
blocklist keeps that variable in the Lambda environment, since Prisma
reads it at INIT before SSM values load.
The KMS wiring already ignores the stage name, so an app with
fieldLevelEncryptionMethod: 'kms' deployed with --stage dev gets
KMS_KEY_ARN and encrypts.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the "dev, test and local stages bypass encryption" guidance in CLAUDE.md, the core encryption README, the DocumentDB encryption design note, the frigg and canary skills, the infrastructure README and the website assistant prompt with the deployed/local rule, the explicit opt-out, and the startup error and its fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
loadModuleEncryptionSchemas required a path that does not exist, so module credentialFields were never registered for encryption (the error was swallowed). Fix the path, consume the entity registry so registry-only modules count, filter fields core already encrypts, and add module fields to the app's own custom schema instead of replacing it (other models included). Because that makes more fields encrypted, make reads tolerate plaintext: _isEncrypted now requires the exact Cryptor shape (keyId:iv:ciphertext: encryptedKey, base64/32-hex/hex/base64) instead of any string with three colons, and a field that fails to decrypt is returned as stored with a warning instead of failing the whole read, findMany included. Existing rows stay plaintext until rewritten. See CHANGELOG (Unreleased). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
seanspeaks
added
release
Claude
|
ADR reconciliation needed after Stack A/B — drafted with Claude Code at Sean's request Severity: must-reconcile with #663 (fail-closed encryption). No ADR owns field-level encryption today. The rules live in Conflict
Proposed change
Merge order: #663 → this PR rebased. #686's encryption-registry changes come after both. |
|
Decision (Sean, 22 Oct): decryption fails closed in deployed runtimes. This replaces the review guide's decision 4 ("decrypt tolerates plaintext"):
Please use #663's shared 🤖 Generated with Claude Code |
…vel encryption isDeployedRuntime read only AWS Lambda's variables, so a Netlify function with STAGE=dev counted as a local run and skipped encryption. A non-aws FRIGG_PROVIDER (set by provider-netlify's generated entry points) now marks a deployed function, and NETLIFY_DEV marks `netlify dev` as local. Netlify's SITE_ID, CONTEXT and NETLIFY are build-time variables and are not used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
…pted in a deployed runtime A value with Cryptor's keyId:iv:ciphertext:encryptedKey shape that fails to decrypt (wrong, rotated or missing key) now throws FieldDecryptionError, with the cause, when the process runs deployed (encryption-config's isDeployedRuntime: AWS Lambda or a non-aws FRIGG_PROVIDER such as Netlify). Returning it as stored would hand ciphertext to an API module as a token. Legacy plaintext never takes this path, and local runs keep the warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
GitHub's ubuntu-latest is now Ubuntu 24.04, which has no libcrypto.so.1.1, so mongodb-memory-server's default MongoDB 5.0.19 could not start and core's test step failed before any test ran. Pin MongoDB 7.0.14 (published for Ubuntu 24.04; MONGOMS_VERSION still overrides it) and the wiredTiger storage engine, since the library's default ephemeralForTest was removed in MongoDB 7. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
7.0.14 has no ubuntu2404 build (fastdl answers 403), so the in-memory server still could not start on GitHub's runners. 8.0.4's ubuntu2404 build downloads and starts with the wiredTiger engine; checked with an empty binary cache. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
|
Why core never ran: GitHub's
What is still red:
A re-run would not change this result. Generated by Claude Code |
Frigg 2.0.1 review (Group 2) — COMMENT onlyChecklist
Verdict (would): would approve Findings
No blocking issues found on the Prisma path for the 2.0.1 fail-closed decrypt requirement. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // missing key fails the read rather than reaching an API | ||
| // module as a token. Legacy plaintext never gets here: it does | ||
| // not look like ciphertext (_isEncrypted). | ||
| if (this.failClosedOnDecryptError()) { |
There was a problem hiding this comment.
Checklist note (pass): deployed decrypt fail-closed is correctly implemented here — ciphertext-shaped values that fail cryptor.decrypt throw FieldDecryptionError when failClosedOnDecryptError() is true (default isDeployedRuntime()), so API modules never receive raw ciphertext as a “token”.
Frigg 2.0.1 review (Stack A) — COMMENT onlyVerdict (would): would approve Decrypt-fails-closed on the primary Prisma / BlockingNone on the primary Prisma decrypt path. Should-fix
Nits
No formal approve / request-changes on this review. |
| } | ||
| } catch (error) { | ||
| // A bad entity registry is a configuration error, not "no custom schema". | ||
| if (error instanceof EntityRegistryError) throw error; |
There was a problem hiding this comment.
should-fix — module encryption registration can still fail open
Only EntityRegistryError is rethrown; other failures fall through to logger.debug and are ignored (“custom schema is optional”).
Scenario: Module credentialFields registration throws (unexpected shape / validation edge) → app starts with only core Credential fields encrypted; module fields stay plaintext — recreating the silent registration hole under a different exception.
When encryption is enabled / deployed, treat failure to load module encryption schemas as fatal.
…stered From the Stack A review on #680: - loadCustomEncryptionSchema swallowed every error, so a schema the app declares (encryption.schema, encryption.disable, module credentialFields) that failed to register left those fields in plaintext with only a debug line. Not finding an app definition is still quiet; a registration failure now throws in a deployed runtime and warns on a local run. - Local runs enabled AES on AES_KEY_ID alone (deployed needs both). An incomplete pair now keeps a local run plaintext and warns that AES_KEY is missing. Tests that turned encryption on with the id alone set both. - The DocumentDB decrypt error no longer logs a ciphertext prefix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
|
On the Stack A review's should-fix items. Commit d670696 is not yet pushed.
The DocumentDB tolerance from the Group 2 review stays a follow-up, as the PR body says. The Generated by Claude Code |
|
Summary
Stacked on #679.
The bug. API-module
credentialFieldshave never been encrypted.loadModuleEncryptionSchemasrequired../integrations/utils/map-integration-dtofromdatabase/encryption/, which is a path that does not exist, and the error was swallowed. The schemas were never registered, so module credential fields were stored in plaintext.Why it isn't a one-line fix. Fixing the path alone breaks reads of existing rows:
_isEncryptedtreated any string with 3 or more colons as ciphertext.https://u:p@host:443ora:b:c:d, in a field that is now marked encrypted, therefore went to decrypt and was rejected.findManyacross every row.Changes
credentialFieldsnow register with the Prisma encryption extension.encryption.schema(Credential and every other model) instead of replacing it. Fields core already encrypts are skipped._isEncryptedrequires exactly four parts shapedbase64 keyId : 32-hex iv : hex ciphertext : base64 encryptedKey.database/encryption/README.md.Upgrade impact
credentialFields, so only external modules that declare them see a change.Not changed
documentdb-encryption-service.js) has its own_isEncryptedValue, which requires 4 base64 parts of at least 50 chars, and it still rethrows on decrypt failure.Test plan
decryptFieldsanddecryptFieldsInBulk. An app schema on a second model (Integration) is tested alongside entities.test-support/fake-ciphertext.js. The old "throws on decryption error" test now asserts the tolerant behaviour.packages/corerun: 57 tests fail, against 65 on feat(core): app-level entity registry (ADR-059) #679. The only suite that changes isencryption-schema-registry, which goes from failing to passing....customSchemaspread, replace instead of merge, the core-field filter, and a swallowed registry error.🤖 Generated with Claude Code
https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
Generated by Claude Code
📦 Published PR as canary version:
2.0.0--canary.680.d670696.0✨ Test out this PR locally via:
npm install @friggframework/admin-scripts@2.0.0--canary.680.d670696.0 npm install @friggframework/core@2.0.0--canary.680.d670696.0 npm install @friggframework/devtools@2.0.0--canary.680.d670696.0 npm install @friggframework/eslint-config@2.0.0--canary.680.d670696.0 npm install @friggframework/prettier-config@2.0.0--canary.680.d670696.0 npm install @friggframework/schemas@2.0.0--canary.680.d670696.0 npm install @friggframework/serverless-plugin@2.0.0--canary.680.d670696.0 npm install @friggframework/test@2.0.0--canary.680.d670696.0 npm install @friggframework/ui@2.0.0--canary.680.d670696.0 # or yarn add @friggframework/admin-scripts@2.0.0--canary.680.d670696.0 yarn add @friggframework/core@2.0.0--canary.680.d670696.0 yarn add @friggframework/devtools@2.0.0--canary.680.d670696.0 yarn add @friggframework/eslint-config@2.0.0--canary.680.d670696.0 yarn add @friggframework/prettier-config@2.0.0--canary.680.d670696.0 yarn add @friggframework/schemas@2.0.0--canary.680.d670696.0 yarn add @friggframework/serverless-plugin@2.0.0--canary.680.d670696.0 yarn add @friggframework/test@2.0.0--canary.680.d670696.0 yarn add @friggframework/ui@2.0.0--canary.680.d670696.0Note
High Risk
Changes credential encryption registration and deployed startup/read behavior for sensitive fields; deployed dev/staging without keys will fail until KMS/AES or an explicit
noneopt-out is configured.Overview
Fixes API module
credentialFieldsnever being registered (broken require path with swallowed errors) by loading schemas through the entity registry (integrations +appDefinition.entities), merging module fields into the app’s customencryption.schemainstead of replacing it.Encryption behavior changes: shared
encryption-config.jsdecides by runtime (deployed Lambda/Netlify vs localfrigg start/Jest), not stage name alone. Deployed functions without KMS or complete AES keys refuse to start (EncryptionConfigurationError) unlessFRIGG_ENCRYPTION_DISABLED=true(wired fromfieldLevelEncryptionMethod: 'none'in the app definition). Prisma no longer continues without encryption if the extension fails to initialize.Read/write safety: ciphertext detection requires the exact four-part envelope shape so colon-heavy plaintext is not decrypted. Deployed reads throw
FieldDecryptionErrorwhen real ciphertext cannot be decrypted; local runs return the stored value with a warning. Existing credential rows stay plaintext until rewritten.Docs, health checks, DocumentDB encryption, SSM blocklist, and test harness Mongo 8.0.4 (CI OpenSSL 3) are updated to match.
Reviewed by Cursor Bugbot for commit d670696. Bugbot is set up for automated code reviews on this repo. Configure here.