Skip to content

fix(config): bind SP_ENCRYPTION_MASTER_KEY so credentials stop being stored in plaintext - #276

Merged
fclairamb merged 2 commits into
mainfrom
fix/encryption-master-key-env
Aug 27, 2026
Merged

fclairamb merged 2 commits into
mainfrom
fix/encryption-master-key-env

Conversation

@fclairamb

Copy link
Copy Markdown
Owner

The bug

SP_ENCRYPTION_MASTER_KEY never bound anything. koanf's env provider lowercases
the name and turns every underscore after SP_ into a dot, so it arrived as
encryption.master.key while the tag is master_key. The credentials service
therefore stayed in disabled mode and stored secrets in plaintext — while
logging a remediation message naming the exact variable that does not work:

WARN credentials encryption disabled — secrets stored in plaintext
     how_to_fix="set SP_ENCRYPTION_MASTER_KEY (or SP_ENCRYPTION_MASTER_KEY_FILE)"
WARN Unrecognized SP_* environment variable is ignored  name=SP_ENCRYPTION_MASTER_KEY

Both lines, same boot, in production. The warning was right and the advice it
contradicted was not.

This is not a new class of problem — it is project_koanf_env_quirk, and every
other multi-word key already has a hand-written helper: applyACMEEnv,
applyRateLimitingEnv, applyPostHogEnv, applyWhatsAppEnv,
applySchedulingEnv, applyEntitlementsEnv. Encryption is the one that never
got written, and it is the one where the failure mode is silent plaintext
storage rather than a knob that stays at its default.

The fix

applyEncryptionEnv for SP_ENCRYPTION_MASTER_KEY,
SP_ENCRYPTION_MASTER_KEY_FILE and SP_ENCRYPTION_AUTO_MIGRATE, called next to
applyACMEEnv, with the three names registered in manualReaderEnvVars so the
startup check stops reporting a variable that now binds.

Three inverted assertions, and why they are not being silenced

TestRecognizedEnvVars and TestCheckEnvironEncryptionMasterKeyWarns both
asserted these names were not recognized, and both carried a comment saying
that registering them would paper over the bug. That was exactly right while the
binding was broken. The binding is fixed here, so recognition now states a fact
instead of hiding one — the assertions are inverted with that reasoning written
into them, not deleted. envcheck's package doc, which used
SP_ENCRYPTION_MASTER_KEY as its standing example of a documented-but-unbound
name, is updated for the same reason.

Operational note for the hosted deployment

AutoMigrate defaults to true, so supplying a key also sweeps rows still in
plaintext at the next start.

Rows encrypted under a key that is no longer available stay unreadable — a
different key cannot recover them. solidping-prod currently reports
encrypted rows present ... checks=4, and no key exists for them anywhere, so
those four checks will need their credentials re-entered by hand. A prod KEK is
already generated and waiting at gopass k8xp/solidping-prod/encryption/master-key;
it starts working the moment this ships.

Test

  • TestLoadBindsEncryptionMasterKey goes through Load, the path production
    takes. Verified it fails without the fix (expected: "dGVzdC..." actual: "").
  • Unit coverage for all three variables, plus the unset case pinned explicitly so
    it cannot inherit the ambient environment.
  • go test ./internal/config/... ./internal/envcheck/... green, golangci-lint
    0 issues, gofmt clean.

🤖 Generated with Claude Code

…stored in plaintext

The env provider lowercases and turns every underscore after SP_ into a dot, so
SP_ENCRYPTION_MASTER_KEY arrived as "encryption.master.key" and never reached
the koanf tag "master_key". Nothing bound it, so the credentials service stayed
in disabled mode - and its own remediation message told the operator to set
exactly the variable that did not work.

Every other multi-word key already has a hand-written applyXxxEnv helper
(ACME, rate limiting, PostHog, WhatsApp, scheduling...). Encryption never got
one. This adds applyEncryptionEnv for MASTER_KEY, MASTER_KEY_FILE and
AUTO_MIGRATE, and registers the three names in manualReaderEnvVars.

Three assertions are inverted rather than deleted, because they were right when
written: TestRecognizedEnvVars and the envcheck test both asserted the names
were NOT recognized, with comments warning that registering them would paper
over the bug. The bug is fixed at the source, so recognition now states a fact.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@fclairamb
fclairamb enabled auto-merge (squash) August 27, 2026 21:45
@fclairamb
fclairamb merged commit 15615ad into main Aug 27, 2026
12 checks passed
@fclairamb
fclairamb deleted the fix/encryption-master-key-env branch August 27, 2026 22:25
fclairamb added a commit that referenced this pull request Aug 28, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.19.1](v0.19.0...v0.19.1)
(2026-08-28)


### Bug Fixes

* **config:** bind SP_ENCRYPTION_MASTER_KEY so credentials stop being
stored in plaintext
([#276](#276))
([15615ad](15615ad))
* **deps:** update github.com/dop251/goja digest to 8f1c069
([#275](#275))
([550ed79](550ed79))
* **email:** link the check page in incident emails instead of the down
host ([#274](#274))
([0daaf40](0daaf40))
* name the license file LICENSE so pkg.go.dev detects it
([#272](#272))
([f11d7b6](f11d7b6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant