From 683195e6a97c82396324382e9f66630db541ff68 Mon Sep 17 00:00:00 2001
From: satyakwok <satyakwok@users.noreply.github.com>
Date: Tue, 12 May 2026 00:31:35 +0200
Subject: [PATCH] fix(wallet): custom Debug impl redacts seed and private_key
 (#287)

`Wallet` was deriving `Debug`, so any code path that formatted it with
`{:?}`, `{:#?}`, `dbg!(wallet)`, or `tracing::debug!(?wallet)` dumped the
plaintext seed and private key. The CLI's faucet command was the most
visible victim: `println!("Generated faucet wallet: {:#?}", wallet)`
already printed the full secret to stdout on the happy path.

`Display` was already redacting both fields. Replace the derive with a
hand-written `Debug` that delegates to `Display`, so the two formatters
behave identically.

Adds three regression tests under `mod tests`:
- `debug_does_not_leak_private_key_or_seed` (the failing test from #287)
- `pretty_debug_also_redacts` (covers the `{:#?}` form the CLI uses)
- `display_redacts_private_key_and_seed` (pins the existing `Display`
  output so a future edit to it can't reintroduce the leak through the
  `Debug -> Display` delegation)

All three fail on the prior commit and pass after.
---
 src/wallet/mod.rs | 66 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs
index 08c67a8f..4358dc6d 100644
--- a/src/wallet/mod.rs
+++ b/src/wallet/mod.rs
@@ -10,7 +10,7 @@ use crate::core::keypairs::derive_classic_address;
 use crate::core::keypairs::derive_keypair;
 use crate::core::keypairs::generate_seed;
 use alloc::string::String;
-use core::fmt::Display;
+use core::fmt::{Debug, Display};
 use exceptions::XRPLWalletResult;
 use zeroize::Zeroize;
 
@@ -19,7 +19,10 @@ use zeroize::Zeroize;
 ///
 /// See Cryptographic Keys:
 /// `<https://xrpl.org/cryptographic-keys.html>`
-#[derive(Debug)]
+///
+/// Both `Debug` and `Display` redact the `seed` and `private_key` fields so that
+/// `dbg!(wallet)`, `format!("{wallet:?}")`, and `tracing::debug!(?wallet)` cannot leak
+/// secrets into logs or terminal scrollback (issue #287).
 pub struct Wallet {
     /// The seed from which the public and private keys
     /// are derived.
@@ -98,3 +101,62 @@ impl Display for Wallet {
         )
     }
 }
+
+// `Debug` delegates to `Display` so `{:?}`, `{:#?}`, `dbg!(...)`, and
+// `tracing::debug!(?wallet)` all redact the `seed` and `private_key` fields the same
+// way the human-readable formatter does. The default derive would print the raw
+// secrets — see issue #287 (CLI faucet path printed `{:#?}` on the generated wallet).
+impl Debug for Wallet {
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+        Display::fmt(self, f)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use alloc::format;
+
+    const TEST_SEED: &str = "sEdSkooMk31MeTjbHVE7vLvgCpEMAdB";
+
+    #[test]
+    fn debug_does_not_leak_private_key_or_seed() {
+        // Regression for #287. With the default `#[derive(Debug)]`, this output would
+        // include the raw `seed` and `private_key`. The custom impl mirrors `Display`,
+        // which redacts both.
+        let wallet = Wallet::new(TEST_SEED, 0).unwrap();
+        let dbg_output = format!("{wallet:?}");
+        assert!(
+            !dbg_output.contains(wallet.private_key.as_str()),
+            "Debug output must not contain the raw private_key. Got: {dbg_output}",
+        );
+        assert!(
+            !dbg_output.contains(wallet.seed.as_str()),
+            "Debug output must not contain the raw seed. Got: {dbg_output}",
+        );
+        // Sanity: redacted marker must be present and public fields must still appear.
+        assert!(dbg_output.contains("-HIDDEN-"));
+        assert!(dbg_output.contains(wallet.public_key.as_str()));
+        assert!(dbg_output.contains(wallet.classic_address.as_str()));
+    }
+
+    #[test]
+    fn pretty_debug_also_redacts() {
+        // The CLI faucet path uses `{:#?}` (alternate Debug). It must redact too.
+        let wallet = Wallet::new(TEST_SEED, 0).unwrap();
+        let dbg_output = format!("{wallet:#?}");
+        assert!(!dbg_output.contains(wallet.private_key.as_str()));
+        assert!(!dbg_output.contains(wallet.seed.as_str()));
+    }
+
+    #[test]
+    fn display_redacts_private_key_and_seed() {
+        // Pin the pre-existing Display behaviour so a future change to it can't silently
+        // reintroduce the leak via the `Debug -> Display` delegation above.
+        let wallet = Wallet::new(TEST_SEED, 0).unwrap();
+        let display_output = format!("{wallet}");
+        assert!(!display_output.contains(wallet.private_key.as_str()));
+        assert!(!display_output.contains(wallet.seed.as_str()));
+        assert!(display_output.contains("-HIDDEN-"));
+    }
+}