Skip to content

feat(core)!: fail closed on field-level encryption in deployed stages - #663

Open
seanspeaks wants to merge 5 commits into
nextfrom
fix/encryption-fail-closed
Open

seanspeaks wants to merge 5 commits into
nextfrom
fix/encryption-fail-closed

Conversation

@seanspeaks

@seanspeaks seanspeaks commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Field-level encryption now fails closed in deployed stages. Frigg used to skip encryption whenever STAGE was dev, test or local, and it silently wrote plaintext when no key was configured. Because frigg deploy defaults to --stage dev, a plain deploy stored OAuth tokens and API keys unencrypted.

Encryption now depends on where the code runs, not on the stage name:

Runtime Key Opt-out (FRIGG_ENCRYPTION_DISABLED=true) Result
Deployed (any stage, including dev) KMS_KEY_ARN any KMS (an opt-out is ignored with a warning, so existing ciphertext stays readable)
Deployed AES_KEY_ID + AES_KEY any AES
Deployed AES_KEY_ID without AES_KEY no EncryptionConfigurationError
Deployed none no EncryptionConfigurationError
Deployed none yes plaintext, with a warning on every cold start
Local, dev/test/local stage any any skipped (unchanged)
Local, other stage key set / none any encrypts / plaintext with warning (unchanged)

Deployed means running in AWS Lambda (AWS_LAMBDA_FUNCTION_NAME, LAMBDA_TASK_ROOT or AWS_EXECUTION_ENV=AWS_Lambda_*) and not under serverless-offline (IS_OFFLINE), serverless invoke local (IS_LOCAL) or Jest. Stage names alone aren't enough, because serverless-offline sets AWS_LAMBDA_FUNCTION_NAME itself, so frigg start keeps working. CloudShell, CodeBuild and ECS count as local, so CLI commands that open a Prisma client in CI aren't affected.

Changes

  • packages/core/database/encryption/encryption-config.js holds the rule in one place. prisma.js, the DocumentDB encryption service, integration-mapping encryption and the encryption health check all use it; the DocumentDB service previously had its own copy of the stage check.
  • Prisma client creation throws EncryptionConfigurationError in the deployed no-key case. The message says what's missing and how to fix it, and the failed client isn't cached. A failed encryption-extension setup now re-throws instead of continuing unencrypted.
  • Explicit opt-out: encryption: { fieldLevelEncryptionMethod: 'none' } in the app definition (new schema enum value) sets FRIGG_ENCRYPTION_DISABLED=true. That variable is kept out of SSM offload, because Prisma reads it before SSM values load.
  • /health/detailed reports a deployed stage with no key as unhealthy, and an opt-out as disabled. BYPASS_ENCRYPTION_STAGE was only ever read by the health check, and is no longer read.
  • Docs: CLAUDE.md, the core encryption README, the DocumentDB encryption doc, the devtools infrastructure README, the frigg and canary skills, and the website assistant's knowledge.

Breaking change (for the 2.0.1 release notes, see #660)

A deployed stage with no key refuses to start: database access returns 500 and the logs show EncryptionConfigurationError with the fix. To fix it, add encryption: { fieldLevelEncryptionMethod: 'kms' } and redeploy (Frigg creates or discovers the key on every stage), provide AES_KEY_ID and AES_KEY, or opt out explicitly. Rows already written in plaintext stay readable and are encrypted the next time they're written.

Known limits

  • A key reference that is set but invalid (e.g. an empty discovered KMS key ID) passes the startup check and fails on the first KMS call.
  • Non-Lambda hosts (ECS and similar) count as local, so the fail-closed check doesn't cover them.
  • The schema still annotates fieldLevelEncryptionMethod with default: "aes", which nothing applies. It's left unchanged here.

Test plan

  • New encryption-config.test.js runtime matrix: offline, invoke-local, Jest, script, Lambda, ECS and CloudShell, each with KMS, AES, none and opt-out
  • New prisma.encryption.test.js: startup throws and nothing is cached; KMS on dev installs the extension; opt-out warns; offline bypasses; a failed extension setup throws
  • DocumentDB service, health use case, environment-builder and schema cases added
  • Core: 2,571 pass / 64 fail. The same 64 tests fail on next.
  • devtools infrastructure: 89 failures before and after, none new. Schemas 126/126.

🤖 Generated with Claude Code

📦 Published PR as canary version: 2.0.0--canary.663.8de669c.0

✨ Test out this PR locally via:

npm install @friggframework/admin-scripts@2.0.0--canary.663.8de669c.0
npm install @friggframework/core@2.0.0--canary.663.8de669c.0
npm install @friggframework/devtools@2.0.0--canary.663.8de669c.0
npm install @friggframework/eslint-config@2.0.0--canary.663.8de669c.0
npm install @friggframework/prettier-config@2.0.0--canary.663.8de669c.0
npm install @friggframework/schemas@2.0.0--canary.663.8de669c.0
npm install @friggframework/serverless-plugin@2.0.0--canary.663.8de669c.0
npm install @friggframework/test@2.0.0--canary.663.8de669c.0
npm install @friggframework/ui@2.0.0--canary.663.8de669c.0
# or 
yarn add @friggframework/admin-scripts@2.0.0--canary.663.8de669c.0
yarn add @friggframework/core@2.0.0--canary.663.8de669c.0
yarn add @friggframework/devtools@2.0.0--canary.663.8de669c.0
yarn add @friggframework/eslint-config@2.0.0--canary.663.8de669c.0
yarn add @friggframework/prettier-config@2.0.0--canary.663.8de669c.0
yarn add @friggframework/schemas@2.0.0--canary.663.8de669c.0
yarn add @friggframework/serverless-plugin@2.0.0--canary.663.8de669c.0
yarn add @friggframework/test@2.0.0--canary.663.8de669c.0
yarn add @friggframework/ui@2.0.0--canary.663.8de669c.0

Note

High Risk
Changes authentication-adjacent credential encryption startup behavior for all deployed Lambdas; misconfigured stages will refuse to start until KMS/AES or an explicit opt-out is set.

Overview
Field-level encryption now fails closed in deployed runtimes instead of skipping encryption when STAGE is dev/test/local or silently writing plaintext without keys.

A new shared module encryption-config.js centralizes the rule: deployed (Lambda, Netlify via FRIGG_PROVIDER, etc., minus offline/Jest) requires KMS or complete AES keys or throws EncryptionConfigurationError; local runs still bypass on dev/test/local stages. prisma.js, DocumentDB encryption, integration-mapping encryption, and the encryption health check all use this module. Prisma no longer continues without encryption if the extension fails to initialize.

Explicit opt-out: encryption.fieldLevelEncryptionMethod: 'none' in the app definition (new schema enum) sets FRIGG_ENCRYPTION_DISABLED=true via the environment builder; that var is blocklisted from SSM offload. Health checks report deployed misconfiguration as unhealthy and drop BYPASS_ENCRYPTION_STAGE.

Docs, skills, and assistant copy are updated to match. Tests cover the runtime matrix, Prisma startup, DocumentDB env config, and infrastructure wiring.

Reviewed by Cursor Bugbot for commit 8de669c. Bugbot is set up for automated code reviews on this repo. Configure here.

seanspeaks and others added 4 commits October 2, 2026 16:01
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>
@seanspeaks seanspeaks added release Create a release when this pr is merged prerelease This change is available in a prerelease. labels Oct 2, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

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, comment @claude review on this pull request, or push a new commit, to trigger a review.

@netlify

netlify Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for friggframework-org ready!

Name Link
🔨 Latest commit 8de669c
🔍 Latest deploy log https://app.netlify.com/projects/friggframework-org/deploys/6adae49812adc9378489b52c
😎 Deploy Preview https://deploy-preview-663--friggframework-org.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@seanspeaks

Copy link
Copy Markdown
Contributor Author

ADR reconciliation needed after Stack A/B — drafted with Claude Code at Sean's request

Severity: must-reconcile (with #680 and #686; no ADR owns field-level encryption yet).

Merge order: this PR → #680 → #686.

seanspeaks pushed a commit that referenced this pull request Oct 19, 2026
"Deployed" was inferred only from AWS Lambda variables, so a Netlify
Function looked like a local run and a deployed STAGE=dev site could accept
unsigned queue messages with FRIGG_QUEUE_ALLOW_UNSIGNED=true.

core/providers/runtime-detection.js adds isDeployedRuntime(env): not under a
local emulator or test runner (IS_OFFLINE, IS_LOCAL, NETLIFY_DEV,
JEST_WORKER_ID), and either AWS Lambda's variables or the active provider's
detect() (appDefinition.provider or FRIGG_PROVIDER, resolved to its
@friggframework/provider-<name> package). isProductionEnvironment, which
gates the unsigned-queue opt-in, now treats any deployed function as
production whatever STAGE says.

provider-netlify: the generated function preamble sets
FRIGG_PROVIDER=netlify, the one runtime signal Frigg controls (Netlify
documents NETLIFY, SITE_ID and CONTEXT as build-time variables), and
detect(env) returns true for it or for NETLIFY.

PR #663's fail-closed encryption config is not on this branch; its
deployed check can call isDeployedRuntime() instead of the AWS-only one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6
@seanspeaks

Copy link
Copy Markdown
Contributor Author

Decision (Sean, 22 Oct): keep fail-closed, including deployed dev stages. A deployed function with no key refuses to start, whatever the stage name. Local runs (frigg start, tests) still skip encryption.

New requirement from that decision: frigg-internal runs on Netlify and is part of the 2.0.1 dogfood gate. "Deployed" detection here only reads AWS Lambda variables (AWS_LAMBDA_FUNCTION_NAME, LAMBDA_TASK_ROOT, AWS_EXECUTION_ENV), so a Netlify function would count as local and could write plaintext.

This PR should merge before #680 and #686.

🤖 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
seanspeaks pushed a commit that referenced this pull request Oct 23, 2026
…thentication

encryption-config's isDeployedRuntime now delegates to
providers/runtime-detection, which asks the active provider's detect() and
fails closed when a named non-AWS provider cannot be loaded. The matrix from
#663 (with its Netlify rows) still passes unchanged against it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HyrF4oqcTdK2AtvcrocxE6

Copy link
Copy Markdown
Contributor Author

The Netlify requirement from the 22 Oct decision is ready as one commit on top of this PR's head: 8de669cb on claude/internal-mcp-09-deployed-runtime. Merge that branch into this one (a fast-forward is fine) to take it.

  • Deployed: isDeployedRuntime also counts a non-aws FRIGG_PROVIDER as deployed. feat: Netlify provider on the v2 stack (port of #545) (F7) #686's generated Netlify functions set FRIGG_PROVIDER=netlify. Netlify's SITE_ID, CONTEXT and NETLIFY are build-time variables, so they are not used.
  • Local: NETLIFY_DEV (netlify dev) counts as local, alongside IS_OFFLINE, IS_LOCAL and Jest.
  • Matrix: new Netlify rows. A deployed Netlify function at any stage with no key raises EncryptionConfigurationError; with AES it encrypts; under netlify dev with STAGE=dev it skips encryption. 82/82 pass, and four mutations of the new rules each fail a test.
  • Docs: CLAUDE.md, the encryption README and the DocumentDB doc updated.

The same commit is already merged into #680 and #686, so merging it here first keeps the stack conflict-free. #680 also now fails closed on a ciphertext-shaped value that cannot be decrypted in a deployed runtime (daae24ec).


Generated by Claude Code

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization has no extra usage available to pay for this review.

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8de669c. Configure here.

if (appDefinition?.encryption?.fieldLevelEncryptionMethod === 'none') {
envVars.FRIGG_ENCRYPTION_DISABLED = 'true';
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt-out ignored after KMS discovery

Medium Severity

fieldLevelEncryptionMethod: 'none' only sets FRIGG_ENCRYPTION_DISABLED=true. buildEnvironment still writes KMS_KEY_ARN when discovery returns kmsKeyId, including isolated-mode KMS lookup that runs even for 'none'. A configured key then wins over the opt-out, so encryption stays on and KmsBuilder never grants KMS IAM.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8de669c. Configure here.

@sonarqubecloud

Copy link
Copy Markdown

@cursor

cursor Bot commented Oct 24, 2026

Copy link
Copy Markdown

Frigg 2.0.1 review (Group 2) — COMMENT only (not an approval / not a formal request-changes)

Checklist

  • Provider-aware deployed detection (Netlify via FRIGG_PROVIDER, NETLIFY_DEV local): pass
  • Deployed dev stages encrypt / fail closed: pass
  • 'none' explicit opt-out (FRIGG_ENCRYPTION_DISABLED): pass

Verdict (would): changes needed (one should-fix that becomes blocking once #666 applies schema defaults)

Findings

  1. should-fix — packages/schemas/schemas/app-definition.schema.json:316

    • fieldLevelEncryptionMethod still has "default": "aes" while adding 'none'.
    • Scenario: After #666’s applyAppDefinitionDefaults(), an app that omits encryption receives aes with no AES_KEY_ID/AES_KEY → deploy looks “configured for AES” in the definition, but the runtime has no key and throws EncryptionConfigurationError (or, if someone later wires AES discovery from the default alone, silently diverges from fail-closed). #666’s own merge note says to keep 'none' and drop this default.
  2. should-fix — packages/core/database/encryption/encryption-config.js (resolveEncryptionConfig, local branch ~L141–147)

    • Local non-bypass stages treat AES_KEY_ID alone as “has AES” (hasAES = isSet(AES_KEY_ID)), unlike the deployed path which requires AES_KEY too.
    • Scenario: STAGE=prod under frigg start with only AES_KEY_ID set → enabled: true, method: 'aes' → first encrypt/decrypt throws a low-level crypto/KMS error instead of the clear incomplete-AES message used when deployed.

General correctness/security otherwise looks solid; Netlify matrix tests cover the intended runtime split. PR is currently CONFLICTING with next — rebase before merge.

"enum": ["kms", "aes"],
"description": "Method for field-level encryption. 'kms' creates or discovers a KMS key and sets KMS_KEY_ARN; 'aes' expects AES_KEY_ID and AES_KEY in the Lambda environment; 'none' sets FRIGG_ENCRYPTION_DISABLED=true and stores sensitive fields in plaintext. A deployed stage with no key and no 'none' opt-out refuses to start.",
"enum": ["kms", "aes", "none"],
"default": "aes"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should-fix: "default": "aes" conflicts with fail-closed and with #666’s merge guidance (keep 'none', drop the default).

Scenario: Once applyAppDefinitionDefaults() lands, an omitted encryption block becomes aes with no keys → definition claims AES while Prisma still has no usable key and refuses to start on deploy. Drop the default here (enum may keep kms|aes|none).

@cursor

cursor Bot commented Oct 24, 2026

Copy link
Copy Markdown

Follow-up findings (COMMENT)

  1. should-fix — encryption-config.js Netlify path depends on FRIGG_PROVIDER=netlify being set at runtime. That wiring is on the Netlify provider stack (#686), not this PR. Until those entry points ship, a deployed Netlify function with STAGE=dev and no Lambda markers still counts as local and can skip encryption.

    • Scenario: Netlify dev stage deploy without #686 preamble → plaintext credentials despite this PR’s intent.
  2. should-fix — 'none' / FRIGG_ENCRYPTION_DISABLED=true is ignored when a usable key is still present (optOutIgnored: true, key wins). Schema/docs say 'none' stores plaintext.

    • Scenario: App sets fieldLevelEncryptionMethod: 'none' but leftover KMS_KEY_ARN remains → fields stay encrypted; operator expected plaintext. Either document “key wins” as the contract or fail validation when both are set.

seanspeaks added a commit that referenced this pull request Oct 24, 2026
…when deployed

Review of #710: loadCustomEncryptionSchema wrapped everything in one
catch that logged at debug level, the same handler that hid the broken
require fixed in 2a2adff. One non-string entry in a module's
`encryption.credentialFields` (`field.startsWith is not a function`) or an
invalid app `encryption.schema` dropped every module field, and those
credentials were written in plaintext with nothing in the logs.

- credentialFields are validated entry by entry: an invalid entry is
  skipped with a warning naming the module; valid entries from the same
  and other modules are still registered.
- Only "no backend / no app definition" stays at debug level. Once an app
  definition is found, an invalid encryption.schema, encryption.disable or
  module schema is a configuration error: in deployed stages it is logged
  at error level and thrown as EncryptionSchemaError; in dev/test/local it
  is a warning and loading continues.
- prisma.js rethrows EncryptionSchemaError instead of "Continuing without
  encryption", so startup fails rather than silently writing plaintext.
  Other extension-initialisation failures are unchanged here (#663 covers
  them).

New tests fail on the previous source (4 of 5).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
@seanspeaks seanspeaks mentioned this pull request Oct 24, 2026
3 of 4 tasks
seanspeaks added a commit that referenced this pull request Oct 24, 2026
- app-definition schema: database.postgres.management accepts 'external'
  (the frigg init scaffold, #639), and fieldLevelEncryptionMethod accepts
  'none' (the explicit opt-out, #663) with no default.
- adopterJwt: a jwtConfig without a `secret` key is still an error, but an
  empty value (`secret: process.env.JWT_SECRET` with the variable unset in
  the shell; build and deploy validate before .env is loaded) is now a
  warning that the secret must be set at runtime. The schema no longer
  marks `secret` required, since it cannot tell the two apart.
- unreachable-database: under managementMode 'managed' or 'existing' the
  Aurora builder ignores postgres.management 'use-existing', so the check
  now warns in that case too.
- resolveBackendDir stops at the nearest backend/package.json like core's
  findNearestBackendPackageJson, and reports a missing index.js there
  instead of validating a farther app.
- 'none' is not warned about as plaintext or as a bypassed stage.
- Contract matrix: the frigg init scaffold and an external/none case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013b2UUFaQ5Lk6TFPqzRr1j7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

prerelease This change is available in a prerelease. release Create a release when this pr is merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants