Stop echoing a mistyped secret given where a contract id is expected

Author: fnando

cmd/crates/soroban-test/tests/it/config.rs

@@ -1312,6 +1312,62 @@ fn token_address_param_error_does_not_leak_secret_bearing_input() {
     }
 }
 
+#[test]
+fn contract_id_param_error_does_not_leak_secret_bearing_input() {
+    // `--id`/`--contract-id`/`STELLAR_CONTRACT_ID` share `UnresolvedContract`; a
+    // mistyped secret pasted there fails to resolve before any network call, so
+    // the "contract not found" error must not echo it. `contract fetch --id`
+    // stands in for the shared resolve path.
+    let sandbox = TestEnv::default();
+    // A seed phrase additionally fails alias-name validation (spaces) before the
+    // lookup, so cover both the mistyped strkey and the seed-phrase paths.
+    for input in [MISTYPED_SECRET, SEED_PHRASE] {
+        sandbox
+            .new_assert_cmd("contract")
+            .args(["fetch", "--id", input, "--network", "testnet"])
+            .assert()
+            .failure()
+            .stdout(predicate::str::contains(input).not())
+            .stderr(predicate::str::contains(input).not());
+    }
+}
+
+#[test]
+fn contract_id_param_error_names_plain_alias() {
+    // A genuine alias typo is not secret-bearing, so the error still names it —
+    // otherwise a multi-argument command gives no clue which value was wrong.
+    let sandbox = TestEnv::default();
+    sandbox
+        .new_assert_cmd("contract")
+        .args(["fetch", "--id", "nosuchalias", "--network", "testnet"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("nosuchalias"));
+}
+
+#[test]
+fn address_param_error_names_plain_alias() {
+    // Same contract for the address params: a mistyped alias (not secret-bearing)
+    // is still echoed so the user can see which value failed.
+    let sandbox = TestEnv::default();
+    sandbox
+        .new_assert_cmd("token")
+        .args([
+            "balance",
+            "--id",
+            "native",
+            "--account",
+            "nosuchalias",
+            "--rpc-url",
+            "http://localhost:1",
+            "--network-passphrase",
+            "Test SDF Network ; September 2015",
+        ])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("nosuchalias"));
+}
+
 #[test]
 fn help_conceals_sensitive_env_var_values() {
     // Each concealed env var, paired with a subcommand whose `--help` renders it.

cmd/soroban-cli/src/config/alias.rs

@@ -3,7 +3,7 @@ use std::{collections::HashMap, convert::Infallible, str::FromStr};
 use serde::{Deserialize, Serialize};
 use stellar_strkey::Contract;
 
-use super::locator;
+use super::{locator, secret};
 use crate::config::token::UnresolvedToken;
 use crate::tx::builder;
 
@@ -60,12 +60,29 @@ pub fn validate_reserved_aliases(alias: &str) -> Result<(), locator::Error> {
 }
 
 /// Address can be either a contract address, C.. or eventually an alias of a contract address.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub enum UnresolvedContract {
     Resolved(stellar_strkey::Contract),
     Alias(String),
 }
 
+impl std::fmt::Debug for UnresolvedContract {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            UnresolvedContract::Resolved(contract) => {
+                f.debug_tuple("Resolved").field(contract).finish()
+            }
+            // A genuine alias is safe to show, but never echo a secret key or
+            // seed phrase pasted where a contract id was expected — e.g. under
+            // `--verbose`, which debug-prints parsed arguments.
+            UnresolvedContract::Alias(alias) if secret::looks_like_secret(alias) => {
+                f.debug_tuple("Alias").field(&"<alias or secret>").finish()
+            }
+            UnresolvedContract::Alias(alias) => f.debug_tuple("Alias").field(alias).finish(),
+        }
+    }
+}
+
 impl Default for UnresolvedContract {
     fn default() -> Self {
         UnresolvedContract::Alias(String::default())
@@ -102,9 +119,21 @@ impl UnresolvedContract {
         locator: &locator::Args,
         network_passphrase: &str,
     ) -> Result<stellar_strkey::Contract, locator::Error> {
-        locator
-            .get_contract_id(alias, network_passphrase)?
-            .ok_or_else(|| locator::Error::ContractNotFound(alias.to_owned()))
+        let result = locator.get_contract_id(alias, network_passphrase);
+
+        // A secret key or seed phrase pasted where a contract id was expected
+        // must never be echoed. Both a lookup miss and name validation (a seed
+        // phrase's spaces fail it) otherwise interpolate the raw input, so
+        // collapse any failure for secret-shaped input to the concealing
+        // not-found error.
+        if secret::looks_like_secret(alias) {
+            return result
+                .ok()
+                .flatten()
+                .ok_or_else(|| locator::Error::ContractNotFound(alias.to_owned()));
+        }
+
+        result?.ok_or_else(|| locator::Error::ContractNotFound(alias.to_owned()))
     }
 }
 
@@ -123,4 +152,23 @@ mod tests {
         assert!(!is_reserved("my-token"));
         assert!(validate_reserved_aliases("my-token").is_ok());
     }
+
+    #[test]
+    fn debug_conceals_secret_bearing_alias() {
+        // `--verbose` debug-prints parsed arguments; a secret pasted where a
+        // contract id was expected must not surface through the `Alias` variant.
+        for input in [
+            "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BX",
+            "illness spike retreat truth genius clock brain pass fit cave bargain xyzzy",
+        ] {
+            let contract = UnresolvedContract::from_str(input).unwrap();
+            assert!(!format!("{contract:?}").contains(input));
+        }
+    }
+
+    #[test]
+    fn debug_echoes_plain_alias() {
+        let contract = UnresolvedContract::from_str("my-token").unwrap();
+        assert!(format!("{contract:?}").contains("my-token"));
+    }
 }

cmd/soroban-cli/src/config/locator.rs

@@ -22,7 +22,7 @@ use super::{
     alias,
     key::{self, Key},
     network::{self, Network},
-    secret::Secret,
+    secret::{self, Secret},
     utils, Config,
 };
 
@@ -80,7 +80,7 @@ pub enum Error {
     CannotAccessAliasConfigFile,
     #[error("cannot parse contract ID {0}: {1}")]
     CannotParseContractId(String, DecodeError),
-    #[error("contract not found: {0}{hint}", hint = wasm_hash_hint(.0))]
+    #[error("{}", contract_not_found_message(.0))]
     ContractNotFound(String),
     #[error("Failed to read upgrade check file: {path}: {error}")]
     UpgradeCheckReadFailed { path: PathBuf, error: io::Error },
@@ -120,6 +120,18 @@ impl Error {
     }
 }
 
+// Build the `ContractNotFound` message, concealing the input when it looks like
+// a secret key or seed phrase mistyped where a contract id was expected. A
+// genuine alias typo is safe to name so the user can see which value was wrong,
+// but a mistyped secret must not reach the terminal, logs, or JSON output.
+fn contract_not_found_message(value: &str) -> String {
+    if secret::looks_like_secret(value) {
+        "contract not found".to_string()
+    } else {
+        format!("contract not found: {value}{}", wasm_hash_hint(value))
+    }
+}
+
 fn wasm_hash_hint(value: &str) -> &'static str {
     if value.len() == 64 && value.bytes().all(|b| b.is_ascii_hexdigit()) {
         "; expected a contract address (C...), got a hash"
@@ -1084,6 +1096,19 @@ mod error_message_tests {
         let err = Error::ContractNotFound(value.clone());
         assert_eq!(err.to_string(), format!("contract not found: {value}"));
     }
+
+    #[test]
+    fn contract_not_found_conceals_secret_bearing_input() {
+        // A mistyped secret key or seed phrase pasted where a contract id was
+        // expected must not be echoed back in the not-found error.
+        for input in [
+            "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BX",
+            "illness spike retreat truth genius clock brain pass fit cave bargain xyzzy",
+        ] {
+            let err = Error::ContractNotFound(input.to_string());
+            assert_eq!(err.to_string(), "contract not found");
+        }
+    }
 }
 
 #[cfg(all(test, unix))]

cmd/soroban-cli/src/config/sc_address.rs

@@ -2,7 +2,7 @@ use std::str::FromStr;
 
 use crate::xdr;
 
-use super::{alias, key, locator, UnresolvedContract};
+use super::{alias, key, locator, secret, UnresolvedContract};
 
 /// `ScAddress` can be either a resolved `xdr::ScAddress` or an alias of a `Contract` or `MuxedAccount`.
 #[allow(clippy::module_name_repetitions)]
@@ -16,11 +16,12 @@ impl std::fmt::Debug for UnresolvedScAddress {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             UnresolvedScAddress::Resolved(addr) => f.debug_tuple("Resolved").field(addr).finish(),
-            // Never echo the raw input: it may be a secret key or seed phrase
-            // pasted where an alias was expected.
-            UnresolvedScAddress::Alias(_) => {
+            // A genuine alias is safe to show, but never echo a secret key or
+            // seed phrase pasted where an address was expected.
+            UnresolvedScAddress::Alias(alias) if secret::looks_like_secret(alias) => {
                 f.debug_tuple("Alias").field(&"<alias or secret>").finish()
             }
+            UnresolvedScAddress::Alias(alias) => f.debug_tuple("Alias").field(alias).finish(),
         }
     }
 }
@@ -37,10 +38,11 @@ pub enum Error {
     Locator(#[from] locator::Error),
     #[error(transparent)]
     Key(#[from] key::Error),
-    // The input is never echoed: a secret key or seed phrase mistyped where an
-    // address was expected must not reach the terminal, logs, or JSON output.
-    #[error("invalid address or alias")]
-    AccountAliasNotFound,
+    // A genuine alias typo is named so the user can see which value was wrong,
+    // but a secret key or seed phrase mistyped where an address was expected is
+    // concealed so it never reaches the terminal, logs, or JSON output.
+    #[error("{}", address_not_found_message(.0))]
+    AccountAliasNotFound(String),
     #[error("alias '{0}' is reserved for the native asset contract but also matches a stored key; pass an explicit contract (C...) or account (G...) address instead")]
     ReservedAliasShadowsKey(String),
 }
@@ -131,11 +133,22 @@ impl UnresolvedScAddress {
                     xdr::ScAddress::MuxedAccount(xdr::MuxedEd25519Account { id, ed25519 })
                 }
             }),
-            _ => Err(Error::AccountAliasNotFound),
+            _ => Err(Error::AccountAliasNotFound(alias)),
         }
     }
 }
 
+// Build the address "not found" message, concealing the input when it looks like
+// a secret key or seed phrase mistyped where an address was expected. A genuine
+// alias typo is named so the user can tell which argument and value were wrong.
+fn address_not_found_message(value: &str) -> String {
+    if secret::looks_like_secret(value) {
+        "invalid address or alias".to_string()
+    } else {
+        format!("address alias '{value}' not found")
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -360,4 +373,27 @@ mod tests {
             assert!(!format!("{address:?}").contains(input));
         }
     }
+
+    #[test]
+    fn resolve_names_plain_alias() {
+        let dir = tempfile::tempdir().unwrap();
+        let locator = locator::Args {
+            config_dir: Some(dir.path().to_path_buf()),
+        };
+        let network_passphrase = "Test Network";
+
+        // A genuine alias typo is not secret-bearing, so it is still named in the
+        // error — otherwise a multi-address command gives no clue which value
+        // failed.
+        let err = UnresolvedScAddress::Alias("nosuchalias".to_string())
+            .resolve(&locator, network_passphrase, None)
+            .unwrap_err();
+        assert!(err.to_string().contains("nosuchalias"), "got: {err}");
+    }
+
+    #[test]
+    fn debug_echoes_plain_alias() {
+        let address = UnresolvedScAddress::from_str("nosuchalias").unwrap();
+        assert!(format!("{address:?}").contains("nosuchalias"));
+    }
 }

cmd/soroban-cli/src/config/secret.rs

@@ -102,6 +102,30 @@ pub enum HardwareKind {
     Ledger,
 }
 
+/// Whether `value` could be a secret key or seed phrase mistyped where an alias
+/// or address was expected. Callers use it to decide whether a "not found" error
+/// may echo the raw input: a genuine alias is safe to name so the user can see
+/// which value was wrong, but secret-bearing input must be concealed. The check
+/// is shape-based on purpose — a one-character typo of a real secret no longer
+/// decodes yet still reveals nearly all of it, so matching a successful parse
+/// would miss exactly the case that matters.
+#[must_use]
+pub fn looks_like_secret(value: &str) -> bool {
+    // A secret seed strkey is `S…`, 56 uppercase base32 characters. Allow slack
+    // on the length so a typo that drops or adds a character still matches; no
+    // real alias is a 50+ character all-base32 string beginning with `S`.
+    let strkey_shaped = value.len() >= 50
+        && value.starts_with('S')
+        && value
+            .bytes()
+            .all(|b| b.is_ascii_uppercase() || (b'2'..=b'7').contains(&b));
+    // A seed phrase is 12 or 24 words and a mistyped one keeps the count; an
+    // alias or address never contains whitespace, so any multi-word input here
+    // is secret-shaped.
+    let mnemonic_shaped = value.split_whitespace().count() >= 12;
+    strkey_shaped || mnemonic_shaped
+}
+
 impl FromStr for Secret {
     type Err = Error;
 
@@ -277,6 +301,34 @@ mod tests {
     const TEST_SEED_PHRASE: &str =
         "depth decade power loud smile spatial sign movie judge february rate broccoli";
 
+    #[test]
+    fn looks_like_secret_matches_secret_shaped_input() {
+        assert!(looks_like_secret(TEST_SECRET_KEY));
+        assert!(looks_like_secret(TEST_SEED_PHRASE));
+        // A valid strkey charset with a bad checksum still looks like a secret.
+        assert!(looks_like_secret(
+            "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BX"
+        ));
+        // A seed phrase with a bad final word keeps its 12-word shape.
+        assert!(looks_like_secret(
+            "illness spike retreat truth genius clock brain pass fit cave bargain xyzzy"
+        ));
+    }
+
+    #[test]
+    fn looks_like_secret_rejects_aliases_and_ids() {
+        assert!(!looks_like_secret("alice"));
+        assert!(!looks_like_secret("my-token"));
+        assert!(!looks_like_secret("native"));
+        // A contract id (C…) and a public key (G…) are not secrets.
+        assert!(!looks_like_secret(
+            "CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE"
+        ));
+        assert!(!looks_like_secret(TEST_PUBLIC_KEY));
+        // A 64-char hex wasm hash begins with a hex digit, never `S`.
+        assert!(!looks_like_secret(&"a".repeat(64)));
+    }
+
     #[test]
     fn test_from_str_for_secret_key() {
         let secret = Secret::from_str(TEST_SECRET_KEY).unwrap();