Severity: P3 — Security
Where: src/core/systemConfig.ts:85 and src/core/systemConfig.ts:124 (JSON.stringify(res.data.details)), src/core/keychain.ts:118-144 (redactToken/scrubTokens)
Follow-up to #99, which wired scrubTokens into bootstrapAdmin (#114). That closed the only place a raw response body was passed straight to console.error. This covers the one remaining echo path and two gaps in the scrubber itself.
Problem
1. Validation details are echoed unscrubbed. Both patchSystemConfig and providerMutationError splice a server 400 payload into the thrown ConfigApiError:
const details = res.data?.details ? ` ${JSON.stringify(res.data.details)}` : "";
throw new ConfigApiError(`${reason}.${details}`);runConfig prints that message to stderr. The payload describes a request the CLI just sent, and validation errors commonly echo the offending value back. For seamless config oauth-providers add|update that request body carries clientSecret (src/core/oauthProviders.ts:68), so a rejected provider config can print a client secret — in CI, into the build log.
2. TOKEN_KEYS is too narrow. It covers token, accesstoken, refreshtoken, verificationtoken, authorization. It does not cover secret, clientsecret, password, apikey, code, or otp — including the clientSecret field the CLI itself sends.
3. scrubTokens only walks objects and arrays. Secrets embedded in a string value pass through untouched, because the scrubber matches on key name only:
scrubTokens({ error: "invalid token eyJhbGciOi..." }) // returned verbatimA bare-string body is returned as-is for the same reason. This limits how much the bootstrapAdmin fix from #114 actually buys us, since res.json() there can resolve to a string.
Suggested fix
- Scrub before stringifying at both
systemConfig.ts sites: JSON.stringify(scrubTokens(res.data.details)).
- Widen
TOKEN_KEYS to include secret, clientsecret, client_secret, password, apikey, api_key, otp.
- Add value-level scrubbing for strings: mask JWT-shaped (
eyJ...) and Bearer <...> substrings so a secret inside a message is caught regardless of its key.
- Move
redactToken/scrubTokens out of keychain.ts into a src/core/redact.ts. They are a logging concern with no keychain dependency, and every caller so far is a log path, not a credential path.
Out of scope (deliberately)
The --json output paths (apps.ts:58, users.ts:55, org.ts:86, config.ts:92, config.ts:245) should not be scrubbed. That output is a machine-readable contract meant to be piped into jq; rewriting values to [redacted] would break scripts and hide data the caller explicitly asked for. If the control plane ever returns a secret on those endpoints, the fix is server-side redaction.
Acceptance
- A rejected
config oauth-providers add with a clientSecret in the echoed details prints [redacted], covered by a test.
scrubTokens has tests for a JWT inside a string value and for a bare-string input.
Severity: P3 — Security
Where:
src/core/systemConfig.ts:85andsrc/core/systemConfig.ts:124(JSON.stringify(res.data.details)),src/core/keychain.ts:118-144(redactToken/scrubTokens)Follow-up to #99, which wired
scrubTokensintobootstrapAdmin(#114). That closed the only place a raw response body was passed straight toconsole.error. This covers the one remaining echo path and two gaps in the scrubber itself.Problem
1. Validation
detailsare echoed unscrubbed. BothpatchSystemConfigandproviderMutationErrorsplice a server 400 payload into the thrownConfigApiError:runConfigprints that message to stderr. The payload describes a request the CLI just sent, and validation errors commonly echo the offending value back. Forseamless config oauth-providers add|updatethat request body carriesclientSecret(src/core/oauthProviders.ts:68), so a rejected provider config can print a client secret — in CI, into the build log.2.
TOKEN_KEYSis too narrow. It coverstoken,accesstoken,refreshtoken,verificationtoken,authorization. It does not coversecret,clientsecret,password,apikey,code, orotp— including theclientSecretfield the CLI itself sends.3.
scrubTokensonly walks objects and arrays. Secrets embedded in a string value pass through untouched, because the scrubber matches on key name only:A bare-string body is returned as-is for the same reason. This limits how much the
bootstrapAdminfix from #114 actually buys us, sinceres.json()there can resolve to a string.Suggested fix
systemConfig.tssites:JSON.stringify(scrubTokens(res.data.details)).TOKEN_KEYSto includesecret,clientsecret,client_secret,password,apikey,api_key,otp.eyJ...) andBearer <...>substrings so a secret inside a message is caught regardless of its key.redactToken/scrubTokensout ofkeychain.tsinto asrc/core/redact.ts. They are a logging concern with no keychain dependency, and every caller so far is a log path, not a credential path.Out of scope (deliberately)
The
--jsonoutput paths (apps.ts:58,users.ts:55,org.ts:86,config.ts:92,config.ts:245) should not be scrubbed. That output is a machine-readable contract meant to be piped intojq; rewriting values to[redacted]would break scripts and hide data the caller explicitly asked for. If the control plane ever returns a secret on those endpoints, the fix is server-side redaction.Acceptance
config oauth-providers addwith aclientSecretin the echoeddetailsprints[redacted], covered by a test.scrubTokenshas tests for a JWT inside a string value and for a bare-string input.