Skip to content

fix(security): redact secret fields in IbctKey(Config)/McpTransport Serialize - #6165

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

fix(security): redact secret fields in IbctKey(Config)/McpTransport Serialize#6165
bug-ops merged 1 commit into
mainfrom
fix/6006-serialize-secret-leak

Conversation

@bug-ops

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

Copy link
Copy Markdown
Owner

Summary

  • Replaces the derived Serialize on IbctKeyConfig, IbctKey, and McpTransport with hand-written impls that redact their secret fields (key_hex, key_bytes, env values, headers values) to "[REDACTED]", closing the residual gap PR fix(config,mcp,a2a): redact three more secret-bearing Debug/Serialize leaks #6005 intentionally left when it fixed only Debug.
  • Deserialize is untouched on all three types — config load, --init/--migrate-config, and ACP mcp/add are unaffected.
  • Deletes the now-dead hex_bytes::serialize function in crates/zeph-a2a/src/ibct.rs (only deserialize is referenced once Serialize is no longer derived on IbctKey).
  • ServerEntry needs no change — it nests McpTransport and inherits the redaction automatically (verified, not assumed).

A security audit confirmed this was a latent risk, not a live leak: no code path today serializes these types to JSON/logs/status output (ACP mcp/list only serializes server IDs as Vec<String>; mcp/add only deserializes). No round-trip/persistence path exists for any of the three types, so the redaction is behavior-preserving.

A same-defect-class cohort (TelegramConfig.token, DiscordConfig.token, SlackConfig.bot_token/signing_secret, A2aServerConfig.auth_token, McpServerConfig.env/headers) was identified but intentionally scoped out of this PR — those legitimately serialize to TOML via the --init wizard and need per-field assessment before a fix (blanket redaction would corrupt generated configs). Filing a follow-up tracking issue.

Closes #6006

Test plan

  • 6 new regression tests (IbctKeyConfig, IbctKey, McpTransport::Stdio, McpTransport::Http) asserting the serialized output contains "[REDACTED]" and does not contain the raw secret value
  • cargo +nightly fmt --check — clean
  • cargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins — 13213 passed, 0 failed
  • RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --features "desktop,ide,server,chat,pdf,scheduler" — clean
  • cargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings — fails, but on a pre-existing, unrelated clippy::large_futures issue in src/serve/handlers.rs/src/serve/agent_factory.rs. Confirmed via git stash + clippy against plain origin/main HEAD that the failure predates this branch; this PR touches neither file. Will file separately.

@github-actions github-actions Bot added documentation Improvements or additions to documentation rust Rust code changes bug Something isn't working size/M Medium PR (51-200 lines) labels Jul 12, 2026
…erialize

Serialize on IbctKeyConfig, IbctKey, and McpTransport left key_hex,
key_bytes, env, and headers exposed in plaintext to any JSON/log/status
output, mirroring the Debug leak fixed in #6005. Replace derived
Serialize with hand-written impls that redact the secret fields while
keeping Deserialize intact for config load and ACP mcp/add.

No live leak existed (no serialize-to-output path reaches these types
today), but the gap was latent and shared the same defect class as
prior Debug leaks.

Closes #6006
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.

Serialize path still leaks secret material for IbctKey(Config)/McpTransport after PR #6005

1 participant