An empty MASTER_API_KEY_FILE falls through silently, and the startup log names the wrong source #61
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
From the v2.0.0 security audit.
read_master_key_source(src/main.rs:623) returns the file's content onlyif !content.is_empty()after trimming. An empty-after-trim file falls through toMASTER_API_KEY, and if that is unset, toOk(None). A file that cannot be read is a hard error; a file that is present and empty is not.Separately,
master_key_source(src/main.rs:859) is computed from whether the env var — the path string — is non-empty, not from which value actually won. So it logsMASTER_API_KEY_FILEregardless of where the key came from.Two consequences
A secret mount that comes up present-but-empty (a sealed secret not yet decrypted, a sops/CSI race, an operator who created the file before filling it) does not stop startup. With no
MASTER_API_KEYand a fresh database,decide_master_keytakes the Generate branch and prints a brand-new random key to stdout. Every API client configured with the intended key gets 401, and the key that actually works exists only in journald.If
MASTER_API_KEYis also set, the inline variable silently wins while the log says the key came from the file — the lower-security source in use, reported as the higher one. That inverts the rule that the most secure source wins and rejects lower ones, and it is the same shape as #12 and #27.Also
MASTER_API_KEY_FILE=(declared but empty, the.env.examplestyle) passes theif let Ok(path)guard and failsread_to_string(""), aborting withFailed to read MASTER_API_KEY_FILE : No such file or directory. Fail-closed so not a security defect, but it makes a common deployment shape unbootable with an opaque message.HttpreqConfig::from_envfilters empty vars; this path does not.Fix
Treat a set-but-empty
MASTER_API_KEY_FILEas a hard error naming the file, filter the empty env var the way httpreq does, and derive the logged source from the value that actually won rather than from env presence.Fixed in
055be13, all three parts.MASTER_API_KEYprecisely because it is not indocker inspector/proc/environ, so the higher-security source must reject lower ones rather than silently demote — the rule this issue correctly tied to #12 and #27.MASTER_API_KEY_FILE=is treated as unset, as you suggested and ashttpreq's config already does, rather than aborting onread_to_string("").read_master_key_sourcereturns(value, source), so there is no second derivation to disagree with the first.One structural change beyond the fix: the decision is now a pure
pick_master_key(file, inline), with the env and filesystem reads left inread_master_key_source. The existing test noted it relies on process-global env and that no other test readsMASTER_API_KEY*; adding three more env-mutating tests would have made that assumption load-bearing across parallel tests. The new cases are pure and the original env test stays as the one integration check, with its expectations updated for the new return type.383 tests pass, clippy
--all-targets -D warningsclean, fmt clean. Release-notes entry added, including that the empty-file case is now a refusal to start.