Skip to content

feat(server): source encryption keys from a command as well as a file - #312

Merged
BryanFRD merged 1 commit into
mainfrom
feat/key-source-command
Sep 3, 2026
Merged

BryanFRD merged 1 commit into
mainfrom
feat/key-source-command

Conversation

@BryanFRD

@BryanFRD BryanFRD commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Closes #280.

The design the issue asked to have once, not per vendor: envelope encryption keeps working exactly as today, and the key file becomes one implementation of a KeySource. The second implementation is LFSX_ENCRYPTION_KEY_COMMAND, an exec hook run once at boot through the platform shell, whose stdout is read exactly like the key file: hex keys one per line, first line writes. No vendor SDKs, no new dependencies; the command is the interface every KMS, Vault, SOPS and password manager already speaks. Keys never rest on disk, access lands in the source's own audit trail, and rotation stays "the source returns a new first line".

Everything loud stays loud. A hook that exits non-zero fails the boot with the command's own status and stderr in the error; garbage on stdout and duplicate key ids are refused exactly as they are from a file. Setting both variables refuses to start: they are two answers to where the keys live, and picking one silently is how the wrong keys get used.

Config models it as encryption_key: Option<KeySource> (enum File/Command) rather than two stringly fields, so every consumer downstream matches on what it actually has.

Tests: keyring-level equivalence (same bytes from stdout and from a file are the same keyring), the failing hook naming itself in the error, garbage refused, the one-source rule including the both-set panic, and an end-to-end round trip where a command-keyed server stores nothing in the clear and a fresh keyring from the same hook reads back what the first wrote. 367 tests green, clippy clean.

One boundary found on the way and documented rather than papered over: the published container image is distroless and carries no shell, so the hook cannot run in it. docs/encryption.md says so plainly: the command source is for bare-metal, VMs, or an image the operator extends; on Kubernetes the chart's Secret mount stays the right tool. No chart surface added for exactly that reason.

@BryanFRD
BryanFRD enabled auto-merge (squash) September 3, 2026 08:51

@ferrfleet ferrfleet Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clean design: KeySource enum unifies file/command, the both-set panic and empty-string filtering mirror the existing github_app pattern, and the shell hook plus tests (stdout==file equivalence, failing-hook message, garbage refused, e2e round trip) cover the stated contract well. Docs are clear about the distroless-image limitation.

Two nits inline (typo in a panic message, a warning that still hardcodes the file env var name). Neither blocks — approving.

Comment thread server/src/config.rs
(Some(path), None) => Some(KeySource::File(PathBuf::from(path))),
(None, Some(hook)) => Some(KeySource::Command(hook.to_owned())),
(Some(_), Some(_)) => panic!(
"LFSX_ENCRYPTION_KEY_FILE and LFSX_ENCRYPTION_KEY_COMMAND are both set: they are two answers to where the keys live, and picking one for you is how the wrong keys get used"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: stray double space in the panic message ("two answers").

Suggested change
"LFSX_ENCRYPTION_KEY_FILE and LFSX_ENCRYPTION_KEY_COMMAND are both set: they are two answers to where the keys live, and picking one for you is how the wrong keys get used"
"LFSX_ENCRYPTION_KEY_FILE and LFSX_ENCRYPTION_KEY_COMMAND are both set: they are two answers to where the keys live, and picking one for you is how the wrong keys get used"

Comment thread server/src/lib.rs
@@ -277,7 +277,7 @@ fn backends(config: &Config) -> (Store, LockStore) {
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: this warning text still says "LFSX_ENCRYPTION_KEY_FILE is set" unconditionally, but the guarded condition is now config.encryption_key.is_some(), which is also true when LFSX_ENCRYPTION_KEY_COMMAND is the source. An operator using the command source would see a warning naming the wrong env var. Worth wording it generically ("an encryption key is configured") instead.

@BryanFRD
BryanFRD merged commit 778c5ab into main Sep 3, 2026
25 checks passed
@BryanFRD
BryanFRD deleted the feat/key-source-command branch September 3, 2026 08:54
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

SonarQube — aucune nouvelle issue

Comparaison entre le projet bac à sable de cette PR et la branche par défaut : SonarQube Community n'analyse pas les PR, ce delta est calculé côté CI. Détail

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.

Key material from a source other than a file, without marrying a vendor

1 participant