Skip to content

[config]: Add --require-encryption to refuse starting without a key - #227

Merged
gauravthadani merged 3 commits into
mainfrom
gaurav/require-encryption
Oct 19, 2026
Merged

gauravthadani merged 3 commits into
mainfrom
gaurav/require-encryption

Conversation

@gauravthadani

Copy link
Copy Markdown
Contributor

Summary

Encryption is optional in the proxy config, so an upgrade or config edit can deploy a proxy that forwards payloads unencrypted. This adds an opt-in safety net:

  • New serve flag --require-encryption / env PROXY_REQUIRE_ENCRYPTION. When set, the proxy fails at startup unless the config has encryption.default.
  • It checks that a key is configured, not encryption.enabled. A key with enabled: false is a valid decrypt-only mode (the vault is built when a key is present, internal/kms/fx.go), so it still starts.
  • The guard sits outside the config file on purpose, so editing the config alone can't turn it off.
  • In a rolling update, a rejected pod never becomes ready, so the old pods keep serving.

The Helm chart change that turns this on by default will be a follow-up PR in temporalio/helm-charts.

Test plan

  • TestModule_RequireEncryption: no encryption block → rejected; disabled with no key → rejected; decrypt-only with a key → starts; enabled with a key → starts; flag off with no key → starts
  • Built the binary and ran it against a config with no key, once with --require-encryption and once with PROXY_REQUIRE_ENCRYPTION=true: both exited 1 with encryption: default: a key is required when encryption is required
  • go test ./..., mise run lint

🤖 Generated with Claude Code

A config with no encryption block passes validation, so an upgrade or
config edit can ship a proxy that forwards payloads in plaintext. With
--require-encryption (or PROXY_REQUIRE_ENCRYPTION=true), the proxy fails
at startup unless encryption.default is configured.

The check keys on a configured key rather than encryption.enabled, so a
decrypt-only proxy (a key with enabled false) still starts. It lives
outside the config file so editing the config alone cannot disable it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gauravthadani
gauravthadani requested review from a team and pseudomuto as code owners October 15, 2026 04:03
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pseudomuto pseudomuto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Love this idea 🔥 Simple and effective.

Comment thread internal/config/encryption.go
Comment thread internal/config/fx_test.go Outdated
Comment thread internal/config/fx.go Outdated
gauravthadani and others added 2 commits October 17, 2026 17:55
Replace the named requireEncryption dependency with config.RequireKey,
an fx.Invoke that checks the provided *Config. config.Module and
ConfigParams return to their original shape, and the check now also
works with config.Supply for applications that build their config in
code. A missing key still fails app.Err() at startup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tion

# Conflicts:
#	internal/config/encryption.go
@gauravthadani
gauravthadani marked this pull request as ready for review October 17, 2026 07:07

@pseudomuto pseudomuto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice! LGTM 👍

@gauravthadani
gauravthadani merged commit 0cbd1cc into main Oct 19, 2026
7 checks passed
@gauravthadani
gauravthadani deleted the gaurav/require-encryption branch October 19, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants