fix: mask sensitive values in env override logging#1441
Open
mvanhorn wants to merge 9 commits into
Open
Conversation
…ix/1429-mask-sensitive-env-overrides
…ix/1429-mask-sensitive-env-overrides
The env-override masking added for OverrideEnvVars only covered the config.go log line. When an action is submitted via POST /backup/actions with the command string carrying --env=NAME=VALUE, the raw command was logged, stored in the async status list (exposed via GET /backup/actions and system.backup_actions), and echoed back in error/ack responses with the secret in cleartext. Add config.MaskEnvOverrideCommand to redact only the value of sensitive --env/--environment-override tokens (=, space, and quoted forms) while preserving the rest of the command, and apply it to row.Command in the actions() handler after args are parsed from the raw string, so all downstream logging/status/response uses inherit the masked form while execution still applies the real values. Also mask the value on the "can't override" warn branch in OverrideEnvVars, which the original fix missed. Refs Altinity#1429
When clickhouse fails to become healthy on an Azure-backed disk, the azurite log dump (added in 350fcde) emitted ~13m of debug-level output as one unprefixed blob, burying the actual "--- FAIL: TestX" line under thousands of idle GC lines and making the failing test hard to spot in interleaved parallel CI output. Thread testName into DumpContainerLogsSince so every dumped line is prefixed with the owning test, and filter azurite's idle maintenance chatter (GC mark-sweep loops, per-extent skip lines, account re-init) so the request/error lines that explain the timeout stay visible. --debug is left on; the fix is noise reduction, not less detail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slach
approved these changes
Jul 1, 2026
queryWithNoError dumped clickhouse container logs with an empty testName on query failure, so in interleaved parallel CI output the dump could not be tied to the owning test. Pass t through and feed t.Name() into DumpContainerLogsSince (same fix as the azurite dump in 3a48b39), and add t.Helper() so failures point at the caller. Swept all call sites (r, -> t, r,) and threaded t through the handful of helpers that call it but didn't receive it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When environment variables are overridden via
--env, the override is logged at info level asoverride NAME=VALUEwith the value in cleartext. For secret-bearing variables (passwords, access keys, storage credentials, encryption keys) that writes the raw secret into the log, where it can persist in log files, aggregators, or CI output. This masks the value in that log line when the variable name looks sensitive, printing[MASKED]instead, while leaving non-sensitive overrides readable.Why this matters
Issue #1429 asks for the env-override logging to stop leaking secrets. The override log is otherwise useful for debugging which variables were applied, so the fix keeps the line but redacts only the value, and only when the variable name matches a sensitive token. The token list covers the documented credential and encryption-key variables across backends (passwords, secrets, access/secret keys,
AZBLOB_ACCOUNT_KEY/ SSE keys,S3_SSE_CUSTOMER_KEY,GCS_ENCRYPTION_KEY, tokens, SAS, connection strings), so common Azure/GCS/S3 SSE configurations no longer expose credentials in logs.Changes
pkg/config/config.gogains amaskSensitiveEnvValue(name, value)helper backed by asensitiveEnvNameTokenslist and uses it only at theoverride NAME=VALUEinfo-log call site. The override behavior itself is unchanged; only the logged representation of the value changes for sensitive names.Testing
Added tests in
pkg/config/config_test.goasserting that sensitive overrides (including the Azure/GCS/SSE key variables) log[MASKED]and never leak the raw value, that a non-sensitive variable still logs its value, and that the env override is applied regardless. The log-capture test pinsLOG_LEVEL=infoso it is independent of the surrounding process's log level.go test ./pkg/configpasses (including underLOG_LEVEL=error);go vetandgofmtare clean.Closes #1429