Mask secret-looking config values in TTY output - #175
Open
blsmth wants to merge 1 commit into
Open
Conversation
apppack config get/list printed secret values (SECRET_KEY, DATABASE_URL, API tokens, etc.) in plaintext with no guardrails, leaking into shell scrollback, CI logs, and screen shares. Add a best-effort, name-based heuristic that masks values in interactive terminals only, with --reveal to opt out. Piped/scripted usage, --json, and config export stay unmasked so automation keeps working unchanged. Closes #174
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
apppack config getandapppack config listpreviously printed secret values (SECRET_KEY,DATABASE_URL, API tokens, etc.) in plaintext, unconditionally.app/mask.go) that masks values which look sensitive: a case-insensitive deny-list substring match on the variable name, plus URL-userinfo password masking forDATABASE_URL/REDIS_URL-style values (host/db stay readable).config get/config list— never onconfig export,--json, or piped output — so scripts and$(apppack config get FOO)keep working unchanged.--revealflag on bothgetandlistprints plaintext even in a TTY.N value(s) masked — use --reveal to show) is printed to stderr so it never pollutes captured/piped stdout.This is a heuristic, not a security boundary — it will miss things like
STRIPE_SKand over-mask false positives likeAUTH_ENABLED(test asserts this known tradeoff explicitly). See apppack#174 for the design discussion.Closes #174
Test plan
make fmt,make test,make lintall pass (no new lint issues; pre-existing repo lint debt in unrelated files left untouched).app/mask_test.goandapp/config_test.gocover: sensitive names, short/long/empty values,DATABASE_URLpassword-only masking,redis://host:6379(no userinfo) untouched, plain vars untouched, and theAUTH_ENABLEDfalse-positive documented as expected behavior.TestConfigExportRemainsUnmaskedguardsToJSON/ToJSONUnmanagedagainst ever being masked.MaskConfigValue:apppack config get SECRET_KEYin a TTY → prints masked value +value masked — use --reveal to showon stderr.apppack config get SECRET_KEY --reveal→ plaintext.apppack config get SECRET_KEY | cat/$(apppack config get SECRET_KEY)(non-TTY stdout) → plaintext, unchanged.apppack config list→ masked table +N value(s) masked — use --reveal to showhint on stderr;--revealand non-TTY behave the same asget.apppack config export/--json→ always plaintext, untouched by this change.