Skip to content

fix(config): redact secret fields in channel/A2A/MCP config Serialize - #6173

Merged
bug-ops merged 1 commit into
mainfrom
fix/6166-config-serialize-leak
Jul 12, 2026
Merged

fix(config): redact secret fields in channel/A2A/MCP config Serialize#6173
bug-ops merged 1 commit into
mainfrom
fix/6166-config-serialize-leak

Conversation

@bug-ops

@bug-ops bug-ops commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Summary

Closes the Serialize-leak sibling cohort of #6006/#6165. TelegramConfig.token, DiscordConfig.token, SlackConfig.bot_token, and SlackConfig.signing_secret are always None at every --init persist point (the wizard sends real secrets to the vault, not the config field), so they are safe to strip from Serialize entirely — a future diagnostic serialize of a live, runtime-hydrated Config could otherwise leak them despite the existing redacting Debug impls. #[serde(skip_serializing)] closes that gap without touching Deserialize.

DiscordConfig.application_id is a public Discord snowflake, not a secret — left untouched. A2aServerConfig.auth_token, McpServerConfig.env, and McpServerConfig.headers legitimately hold raw values persisted to config.toml by --init, so a blanket redacting Serialize (as used in #6165) would corrupt round-tripped configs; instead each field gets a # Security doc note directing any diagnostic output to the existing redacting Debug impl.

  • Group A (redact): TelegramConfig.token, DiscordConfig.token, SlackConfig.bot_token, SlackConfig.signing_secret#[serde(skip_serializing)]
  • Group B (not a secret): DiscordConfig.application_id → untouched
  • Group C (must stay plaintext for --init round-trip): A2aServerConfig.auth_token, McpServerConfig.env, McpServerConfig.headers → doc note only

Three sibling fields with the identical latent pattern (GatewayConfig.auth_token, AcpConfig.auth_token, and the A2A wizard's raw-token-to-config.toml write) were identified during the audit and intentionally left out of scope — noted in CHANGELOG.md as follow-ups, not yet filed as GitHub issues.

Closes #6166

Test plan

  • cargo +nightly fmt --check
  • cargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings
  • cargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins (13246 passed / 35 skipped / 0 failed)
  • RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --features "desktop,ide,server,chat,pdf,scheduler"
  • New tests: serde_json redaction test per Group A struct (secret value absent from serialized JSON), TOML round-trip test on A2aServerConfig.auth_token (Group C value preserved), deserialize-tolerance test confirming a config omitting a now-skipped field still loads
  • Adversarial critique (rust-critic): approved, verified skip_serializing is unconditional (also closes the runtime-hydrated-Some case, not just None), verified no config-diff/hash path depends on Serialize round-tripping these fields

@github-actions github-actions Bot added bug Something isn't working size/M Medium PR (51-200 lines) documentation Improvements or additions to documentation rust Rust code changes and removed bug Something isn't working labels Jul 12, 2026
@bug-ops
bug-ops enabled auto-merge (squash) July 12, 2026 20:47
@bug-ops
bug-ops force-pushed the fix/6166-config-serialize-leak branch from 4b1cacc to 84bd2ea Compare July 12, 2026 20:47
@github-actions github-actions Bot added the bug Something isn't working label Jul 12, 2026
@bug-ops
bug-ops force-pushed the fix/6166-config-serialize-leak branch from 84bd2ea to 791196e Compare July 12, 2026 20:52
TelegramConfig.token, DiscordConfig.token, SlackConfig.bot_token, and
SlackConfig.signing_secret are vault-only fields, always None at every
--init persist point, so a future diagnostic Serialize of a live Config
(runtime-hydrated with real vault secrets) would have leaked them despite
their Debug impls already redacting. Add #[serde(skip_serializing)] to
each; Deserialize is untouched so inline tokens still load.

DiscordConfig.application_id is a public snowflake, not a secret, and is
left untouched. A2aServerConfig.auth_token, McpServerConfig.env, and
McpServerConfig.headers legitimately hold raw values persisted to
config.toml by --init, so their derived Serialize stays plaintext;
each gets a doc note directing diagnostics to the redacting Debug impl
instead.

Closes #6166
@bug-ops
bug-ops force-pushed the fix/6166-config-serialize-leak branch from 791196e to e1be1c5 Compare July 12, 2026 21:01
@bug-ops
bug-ops merged commit 6ad8ed4 into main Jul 12, 2026
43 checks passed
@bug-ops
bug-ops deleted the fix/6166-config-serialize-leak branch July 12, 2026 21:10
bug-ops added a commit that referenced this pull request Jul 14, 2026
…eaking gateway auth token

PiiFilterConfig and SecretMaskingConfig shipped enabled: false, and the
--init wizard defaulted both prompts to disabled, so an operator accepting
defaults ended up with neither protection active despite both existing
specifically to keep vault secrets and PII out of LLM payloads, message
history, and debug dumps. Flip both to enabled by default (field-level
default_true, not just Default::default(), so the section-present-key-
omitted case resolves correctly too) and fix the two shipped reference
configs that still wrote an explicit enabled = false, which would have
silently overridden the new default.

GatewayConfig.auth_token is vault-hydrated in place at startup but only
had #[serde(default)], not #[serde(skip_serializing)], unlike its already-
fixed Telegram/Discord/Slack siblings (PR #6173) - any future diagnostic
or config-export serialization of a live Config would have leaked the
real gateway bearer token in plaintext.

Closes #6263
Closes #6248
bug-ops added a commit that referenced this pull request Jul 14, 2026
…eaking gateway auth token

PiiFilterConfig and SecretMaskingConfig shipped enabled: false, and the
--init wizard defaulted both prompts to disabled, so an operator accepting
defaults ended up with neither protection active despite both existing
specifically to keep vault secrets and PII out of LLM payloads, message
history, and debug dumps. Flip both to enabled by default (field-level
default_true, not just Default::default(), so the section-present-key-
omitted case resolves correctly too) and fix the two shipped reference
configs that still wrote an explicit enabled = false, which would have
silently overridden the new default.

GatewayConfig.auth_token is vault-hydrated in place at startup but only
had #[serde(default)], not #[serde(skip_serializing)], unlike its already-
fixed Telegram/Discord/Slack siblings (PR #6173) - any future diagnostic
or config-export serialization of a live Config would have leaked the
real gateway bearer token in plaintext.

Closes #6263
Closes #6248
bug-ops added a commit that referenced this pull request Jul 14, 2026
…eaking gateway auth token

PiiFilterConfig and SecretMaskingConfig shipped enabled: false, and the
--init wizard defaulted both prompts to disabled, so an operator accepting
defaults ended up with neither protection active despite both existing
specifically to keep vault secrets and PII out of LLM payloads, message
history, and debug dumps. Flip both to enabled by default (field-level
default_true, not just Default::default(), so the section-present-key-
omitted case resolves correctly too) and fix the two shipped reference
configs that still wrote an explicit enabled = false, which would have
silently overridden the new default.

GatewayConfig.auth_token is vault-hydrated in place at startup but only
had #[serde(default)], not #[serde(skip_serializing)], unlike its already-
fixed Telegram/Discord/Slack siblings (PR #6173) - any future diagnostic
or config-export serialization of a live Config would have leaked the
real gateway bearer token in plaintext.

Closes #6263
Closes #6248
bug-ops added a commit that referenced this pull request Jul 14, 2026
…eaking gateway auth token

PiiFilterConfig and SecretMaskingConfig shipped enabled: false, and the
--init wizard defaulted both prompts to disabled, so an operator accepting
defaults ended up with neither protection active despite both existing
specifically to keep vault secrets and PII out of LLM payloads, message
history, and debug dumps. Flip both to enabled by default (field-level
default_true, not just Default::default(), so the section-present-key-
omitted case resolves correctly too) and fix the two shipped reference
configs that still wrote an explicit enabled = false, which would have
silently overridden the new default.

GatewayConfig.auth_token is vault-hydrated in place at startup but only
had #[serde(default)], not #[serde(skip_serializing)], unlike its already-
fixed Telegram/Discord/Slack siblings (PR #6173) - any future diagnostic
or config-export serialization of a live Config would have leaked the
real gateway bearer token in plaintext.

Closes #6263
Closes #6248
bug-ops added a commit that referenced this pull request Jul 14, 2026
…eaking gateway auth token (#6295)

PiiFilterConfig and SecretMaskingConfig shipped enabled: false, and the
--init wizard defaulted both prompts to disabled, so an operator accepting
defaults ended up with neither protection active despite both existing
specifically to keep vault secrets and PII out of LLM payloads, message
history, and debug dumps. Flip both to enabled by default (field-level
default_true, not just Default::default(), so the section-present-key-
omitted case resolves correctly too) and fix the two shipped reference
configs that still wrote an explicit enabled = false, which would have
silently overridden the new default.

GatewayConfig.auth_token is vault-hydrated in place at startup but only
had #[serde(default)], not #[serde(skip_serializing)], unlike its already-
fixed Telegram/Discord/Slack siblings (PR #6173) - any future diagnostic
or config-export serialization of a live Config would have leaked the
real gateway bearer token in plaintext.

Closes #6263
Closes #6248
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation rust Rust code changes size/M Medium PR (51-200 lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

channel/A2A/MCP config Serialize leak cohort: TelegramConfig.token and 5 siblings still plaintext (Serialize sibling of #6006)

1 participant