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

CHANGELOG.md

@@ -97,6 +97,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- `zeph-config`/`zeph-a2a`/`zeph-mcp`: `IbctKeyConfig`, `IbctKey`, and `McpTransport` derived
+  `Serialize` unredacted, so any future `serde_json`/`toml::to_string`/log/status/ACP path
+  serializing one of these types would have leaked the raw secret even though their `Debug`
+  impls were already redacted by #6005 (#6006). Replaced the derived `Serialize` on all three
+  with hand-written impls mirroring the existing `Debug` redaction: `IbctKeyConfig.key_hex` and
+  `IbctKey.key_bytes` emit `"[REDACTED]"`; `McpTransport::Stdio.env` and `McpTransport::Http
+  .headers` redact values only, keeping keys for diagnostics (`ServerEntry` inherits this
+  automatically since it nests `McpTransport`). `Deserialize` is untouched on all three — config
+  load, the ACP `mcp/add` handler, and IBCT token decoding still need the real values on the way
+  in. No live leak existed before this fix (audited: `--migrate-config` is text-based, the
+  `--init` wizard never populates these fields with raw secrets, and the runtime types have no
+  serialize-to-output path today) — this closes the latent risk for any future caller.
 - `zeph-orchestration`/`zeph-subagent`/`zeph-core`: `TaskNode::network_scope: Deny` was
   advisory-only — the field was never read at dispatch time, so a planner-emitted
   `network_scope: Deny` silently left a task's network egress unrestricted (spec

crates/zeph-a2a/src/ibct.rs

@@ -86,7 +86,10 @@ pub enum IbctError {
 ///
 /// Multiple entries allow key rotation: old keys are kept until all in-flight tokens
 /// signed with them expire.
-#[derive(Clone, Serialize, Deserialize)]
+///
+/// `Serialize` is hand-written and redacts `key_bytes` to `"[REDACTED]"` (mirroring the
+/// `Debug` impl below); `Deserialize` is derived and reads the real key bytes untouched.
+#[derive(Clone, Deserialize)]
 pub struct IbctKey {
     /// Unique key identifier. Embedded in the token so the verifier can look it up.
     pub key_id: String,
@@ -104,6 +107,16 @@ impl std::fmt::Debug for IbctKey {
     }
 }
 
+impl Serialize for IbctKey {
+    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        use serde::ser::SerializeStruct;
+        let mut s = serializer.serialize_struct("IbctKey", 2)?;
+        s.serialize_field("key_id", &self.key_id)?;
+        s.serialize_field("key_bytes", "[REDACTED]")?;
+        s.end()
+    }
+}
+
 /// An Invocation-Bound Capability Token.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Ibct {
@@ -307,11 +320,7 @@ fn unix_now() -> u64 {
 
 /// Serde helper for hex-encoded byte vectors.
 mod hex_bytes {
-    use serde::{Deserialize, Deserializer, Serializer};
-
-    pub fn serialize<S: Serializer>(bytes: &Vec<u8>, ser: S) -> Result<S::Ok, S::Error> {
-        ser.serialize_str(&hex::encode(bytes))
-    }
+    use serde::{Deserialize, Deserializer};
 
     pub fn deserialize<'de, D: Deserializer<'de>>(de: D) -> Result<Vec<u8>, D::Error> {
         let s = String::deserialize(de)?;
@@ -539,4 +548,14 @@ mod tests {
         assert!(debug.contains("k1"));
         assert!(debug.contains("REDACTED"));
     }
+
+    #[cfg(feature = "ibct")]
+    #[test]
+    fn ibct_key_serialize_redacts_key_bytes() {
+        let key = test_key();
+        let json = serde_json::to_string(&key).unwrap();
+        assert!(!json.contains(&hex::encode(b"super-secret-key-for-testing-only")));
+        assert!(json.contains("k1"));
+        assert!(json.contains("REDACTED"));
+    }
 }

crates/zeph-config/src/channels.rs

@@ -344,6 +344,18 @@ max_bot_chain_depth = 5
         assert!(debug.contains("primary"));
         assert!(debug.contains("REDACTED"));
     }
+
+    #[test]
+    fn ibct_key_config_serialize_redacts_key_hex() {
+        let key = IbctKeyConfig {
+            key_id: "primary".into(),
+            key_hex: "deadbeefdeadbeefdeadbeefdeadbeef".into(),
+        };
+        let json = serde_json::to_string(&key).unwrap();
+        assert!(!json.contains("deadbeefdeadbeefdeadbeefdeadbeef"));
+        assert!(json.contains("primary"));
+        assert!(json.contains("REDACTED"));
+    }
 }
 
 fn default_slack_port() -> u16 {
@@ -559,7 +571,23 @@ impl std::fmt::Debug for SlackConfig {
 /// An IBCT signing key entry in the A2A server configuration.
 ///
 /// Multiple entries allow key rotation: keep old keys until all tokens signed with them expire.
-#[derive(Clone, Deserialize, Serialize)]
+///
+/// `Serialize` is hand-written and redacts `key_hex` to `"[REDACTED]"` (mirroring the
+/// `Debug` impl below); `Deserialize` is derived and reads the real hex key untouched, since
+/// config loading and the `--init` wizard both need the real value on the way in.
+///
+/// # Tradeoff
+///
+/// A future "load config → mutate → save TOML" flow that persists an inline
+/// `[a2a] ibct_keys[].key_hex` would round-trip through this redacting `Serialize` and write
+/// back `key_hex = "[REDACTED]"`, corrupting the key. This is acceptable today: no such flow
+/// exists, `--migrate-config` operates on the TOML text directly (never through
+/// `Config`/`Serialize`), and the documented direction is vault-resolved keys via
+/// [`A2aServerConfig::ibct_signing_key_vault_ref`](crate::channels::A2aServerConfig::ibct_signing_key_vault_ref)
+/// (which takes precedence over `ibct_keys[0]`), making inline `key_hex` a legacy path. If a
+/// struct-based config save flow is ever added, this type should graduate to a split
+/// config/diagnostic-shape design instead of redacting in place.
+#[derive(Clone, Deserialize)]
 pub struct IbctKeyConfig {
     /// Unique key identifier. Must match the `key_id` field in issued IBCT tokens.
     pub key_id: String,
@@ -576,6 +604,16 @@ impl std::fmt::Debug for IbctKeyConfig {
     }
 }
 
+impl Serialize for IbctKeyConfig {
+    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        use serde::ser::SerializeStruct;
+        let mut s = serializer.serialize_struct("IbctKeyConfig", 2)?;
+        s.serialize_field("key_id", &self.key_id)?;
+        s.serialize_field("key_hex", "[REDACTED]")?;
+        s.end()
+    }
+}
+
 fn default_ibct_ttl() -> u64 {
     300
 }

crates/zeph-mcp/src/manager/mod.rs

@@ -44,8 +44,12 @@ pub(crate) use zeph_config::McpTrustLevel;
 const MAX_INJECTION_PENALTIES_PER_REGISTRATION: usize = 3;
 
 /// Transport type for MCP server connections.
+///
+/// `Serialize` is hand-written and redacts `Stdio.env` / `Http.headers` values to
+/// `"[REDACTED]"` (keys are kept for diagnostics), mirroring the `Debug` impl below;
+/// `Deserialize` is derived and reads real values untouched (needed for ACP `mcp/add`).
 #[non_exhaustive]
-#[derive(Clone, serde::Serialize, serde::Deserialize)]
+#[derive(Clone, serde::Deserialize)]
 pub enum McpTransport {
     /// Stdio: spawn child process with command + args.
     Stdio {
@@ -109,6 +113,44 @@ impl std::fmt::Debug for McpTransport {
     }
 }
 
+impl serde::Serialize for McpTransport {
+    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
+        use serde::ser::SerializeStructVariant;
+        match self {
+            Self::Stdio { command, args, env } => {
+                let redacted: HashMap<&str, &str> =
+                    env.keys().map(|k| (k.as_str(), "[REDACTED]")).collect();
+                let mut v = serializer.serialize_struct_variant("McpTransport", 0, "Stdio", 3)?;
+                v.serialize_field("command", command)?;
+                v.serialize_field("args", args)?;
+                v.serialize_field("env", &redacted)?;
+                v.end()
+            }
+            Self::Http { url, headers } => {
+                let redacted: HashMap<&str, &str> =
+                    headers.keys().map(|k| (k.as_str(), "[REDACTED]")).collect();
+                let mut v = serializer.serialize_struct_variant("McpTransport", 1, "Http", 2)?;
+                v.serialize_field("url", url)?;
+                v.serialize_field("headers", &redacted)?;
+                v.end()
+            }
+            Self::OAuth {
+                url,
+                scopes,
+                callback_port,
+                client_name,
+            } => {
+                let mut v = serializer.serialize_struct_variant("McpTransport", 2, "OAuth", 4)?;
+                v.serialize_field("url", url)?;
+                v.serialize_field("scopes", scopes)?;
+                v.serialize_field("callback_port", callback_port)?;
+                v.serialize_field("client_name", client_name)?;
+                v.end()
+            }
+        }
+    }
+}
+
 /// Connection parameters for a single MCP server consumed by [`McpManager`].
 ///
 /// Deserialized from the `[[mcp.servers]]` TOML config table or constructed

crates/zeph-mcp/src/manager/tests.rs

@@ -244,6 +244,24 @@ fn transport_stdio_debug_redacts_env_values() {
     assert!(dbg.contains("REDACTED"));
 }
 
+#[test]
+fn transport_stdio_serialize_redacts_env_values() {
+    let mut env = HashMap::new();
+    env.insert(
+        "GITHUB_PERSONAL_ACCESS_TOKEN".to_string(),
+        "ghp_super_secret_token".to_string(),
+    );
+    let transport = McpTransport::Stdio {
+        command: "npx".into(),
+        args: vec![],
+        env,
+    };
+    let json = serde_json::to_string(&transport).unwrap();
+    assert!(!json.contains("ghp_super_secret_token"));
+    assert!(json.contains("GITHUB_PERSONAL_ACCESS_TOKEN"));
+    assert!(json.contains("REDACTED"));
+}
+
 #[test]
 fn transport_http_debug() {
     let transport = McpTransport::Http {
@@ -272,6 +290,23 @@ fn transport_http_debug_redacts_header_values() {
     assert!(dbg.contains("REDACTED"));
 }
 
+#[test]
+fn transport_http_serialize_redacts_header_values() {
+    let mut headers = HashMap::new();
+    headers.insert(
+        "Authorization".to_string(),
+        "Bearer sk-super-secret-token".to_string(),
+    );
+    let transport = McpTransport::Http {
+        url: "http://example.com".into(),
+        headers,
+    };
+    let json = serde_json::to_string(&transport).unwrap();
+    assert!(!json.contains("sk-super-secret-token"));
+    assert!(json.contains("Authorization"));
+    assert!(json.contains("REDACTED"));
+}
+
 #[test]
 fn server_entry_debug_redacts_http_header_values() {
     let mut entry = make_http_entry("secret-header-test");