Skip to content

fix: hide camelCase secrets in --dry-run and --debug output - #42

Closed
Bradenream wants to merge 3 commits into
masterfrom
braden/redact-camelcase-secrets/COR-0
Closed

Bradenream wants to merge 3 commits into
masterfrom
braden/redact-camelcase-secrets/COR-0

Conversation

@Bradenream

Copy link
Copy Markdown
Contributor

Summary

--dry-run and --debug print request and response bodies to stderr. They hid only a fixed list of lowercase names: password, secret, token, api_key and a few more. The Voiceflow API names its fields in camelCase, so most of the secrets it carries printed in full:

  • integration credentials: apiKeySecret, keySecret, secretKey, oauthSecretKey, webhookSecret, webhookSecretKey, authHeaderValue
  • a secret's value in secret create and secret set-value
  • Authorization values in API tool and MCP server headers. In responses this applies to API tool headers, which the API returns as saved; MCP header values come back already masked.

Coding agents keep stderr in their transcripts, so a dry run or a debug run leaked these.

The rule now comes from a survey of all 868 property names in the OpenAPI spec (internal/client/redact.go):

  • Names are compared without case or separators. Those ending in secret, secretkey, password, token, apikey or privatekey are hidden, which covers every name the old list had. Counts such as maxTokens and identifiers such as accountSid, apiKeySid, keyId and accessTokenID stay visible.
  • credentials is hidden whole.
  • A {key, value} pair whose key names a credential, such as an Authorization header, hides its value.
  • The secret endpoints have defaultValue and value hidden. Elsewhere they stay visible, so a variable's default still shows.
  • Booleans, numbers and null are never hidden, so hasPassword: true still shows.

Before and after

master this PR
Secret names the rule hides, out of the spec's 868 4 11, plus hasPassword, which stays visible as a boolean
test/redaction.test.ts (4 --dry-run and 2 --debug leak routes) 6 leak 6 hidden, ordinary fields kept

Test plan

#39 also edits internal/client/diagnostics.go, and the two apply cleanly together.

Not covered: if the API ever sent a malformed response, the SDK's own parse error could quote a raw field value. Only the API could cause that, so it is left as is.

--dry-run and --debug print request and response bodies to stderr, and
hid only a fixed list of lowercase names: password, secret, token,
api_key and a few more. The Voiceflow API names its fields in camelCase,
so most of the secrets it carries printed in full:
- integration credentials: apiKeySecret, keySecret, secretKey,
  oauthSecretKey, webhookSecret, webhookSecretKey, authHeaderValue
- a secret's value in secret create and set-value
- Authorization values in API tool and MCP server headers
Coding agents keep stderr in their transcripts.

The rule now comes from a survey of all 868 property names in the
OpenAPI spec:
- Names are compared without case or separators. Those ending in
  secret, secretkey, password, token, apikey or privatekey are hidden,
  which covers every name the old list had. Counts like maxTokens and
  identifiers like accountSid, apiKeySid and keyId stay visible.
- credentials is hidden whole.
- A {key, value} pair whose key names a credential, such as an
  Authorization header, hides its value.
- On the secret endpoints, defaultValue and value are hidden.
- Booleans, numbers and null are never hidden.

The rule lives in internal/client/redact.go. diagnostics.go calls it
from redactJSON and from both request-body previews.
Copilot AI balanced review requested due to automatic review settings October 1, 2026 22:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Credential-pair redaction incorrectly converts null, boolean, and numeric values into redaction strings.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Improves diagnostic redaction to prevent camelCase credentials and secret endpoint values from leaking through stderr.

Changes:

  • Adds normalized, suffix-based secret detection.
  • Redacts credential pairs and secret endpoint fields.
  • Adds Go and CLI behavior tests.
File Description
internal/​client/​redact.go Defines expanded redaction rules.
internal/​client/​diagnostics.go Applies rules to diagnostic bodies.
internal/​client/​redact_test.go Tests redaction logic.
test/​redaction.test.ts Tests CLI dry-run/debug output.
Files not reviewed (1)
  • internal/client/diagnostics.go: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/client/diagnostics.go Outdated
A {key, value} pair whose key names a credential, such as an
Authorization header, had its value replaced whatever it held, so
{"key": "Authorization", "value": null} printed as "[REDACTED]". That
broke this change's own rule that booleans, numbers and null are never
hidden, and changed the value's JSON type. The pair now hides a value
only when it could hold a secret, as named fields already did. Copilot
raised this in review.

effervescentia commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Merge activity

  • Oct 2, 6:41 PM UTC: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Oct 2, 6:41 PM UTC: effervescentia added this pull request to the Graphite merge queue.
  • Oct 2, 6:42 PM UTC: CI is running for this pull request on a draft pull request (#49) due to your merge queue CI optimization settings.
  • Oct 2, 6:42 PM UTC: Merged by the Graphite merge queue via draft PR: #49.

@graphite-app graphite-app Bot closed this Oct 2, 2026
@graphite-app
graphite-app Bot deleted the braden/redact-camelcase-secrets/COR-0 branch October 2, 2026 18:42
@graphite-app graphite-app Bot removed the merge label Oct 2, 2026
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.

3 participants