From 77c61f8e54e786a1af7fb897991bafa2de61ef0e Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Mon, 27 Jul 2026 14:10:14 -0300 Subject: [PATCH 1/4] Find a signer when given its public key. --- .../soroban-test/tests/it/integration/auth.rs | 25 +++++++ .../src/commands/contract/arg_parsing.rs | 13 +++- cmd/soroban-cli/src/config/address.rs | 54 ++++++++++++++- cmd/soroban-cli/src/config/locator.rs | 65 +++++++++++++++++++ 4 files changed, 153 insertions(+), 4 deletions(-) diff --git a/cmd/crates/soroban-test/tests/it/integration/auth.rs b/cmd/crates/soroban-test/tests/it/integration/auth.rs index 3d09d980e7..2ff953bc08 100644 --- a/cmd/crates/soroban-test/tests/it/integration/auth.rs +++ b/cmd/crates/soroban-test/tests/it/integration/auth.rs @@ -53,6 +53,31 @@ async fn standard_auth_with_separate_signer() { .stdout("\"hello\"\n"); } +// Regression test for https://github.com/stellar/stellar-cli/issues/2459: +// passing a signer by its public key (G...) must resolve to the stored +// identity that holds it, exactly like passing the identity alias does. +#[tokio::test] +async fn standard_auth_with_separate_signer_by_public_key() { + let sandbox = &TestEnv::new(); + let signer_pubkey = new_account(sandbox, "signer"); + + let (id, _) = deploy_auth_contracts(sandbox).await; + + sandbox + .new_assert_cmd("contract") + .arg("invoke") + .arg("--source=test") + .arg("--id") + .arg(&id) + .arg("--") + .arg("do-auth") + .arg(format!("--addr={signer_pubkey}")) + .arg("--val=hello") + .assert() + .success() + .stdout("\"hello\"\n"); +} + #[tokio::test] async fn root_auth_with_authorized_subcall() { let sandbox = &TestEnv::new(); diff --git a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index b2617ca9a9..631da1c444 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -465,9 +465,16 @@ fn resolve_address(addr_or_alias: &str, config: &config::Args) -> Result Option { - let secret = config.locator.get_secret_key(addr_or_alias).ok()?; - let print = Print::new(false); - let signer = secret.signer(config.hd_path(), print).ok()?; + let account: config::UnresolvedMuxedAccount = addr_or_alias.parse().ok()?; + let secret = account.resolve_secret(&config.locator).ok()?; + // A raw public key was matched to an identity at its persisted derivation, + // so sign at that path (None); an alias or secret honors the global + // --hd-path as before. + let hd_path = match account { + config::UnresolvedMuxedAccount::Resolved(_) => None, + config::UnresolvedMuxedAccount::AliasOrSecret(_) => config.hd_path(), + }; + let signer = secret.signer(hd_path, Print::new(false)).ok()?; Some(signer) } diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index ef037c45b3..dd3cdd79be 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -78,8 +78,19 @@ impl UnresolvedMuxedAccount { pub fn resolve_secret(&self, locator: &locator::Args) -> Result { match &self { + // A literal public key (or muxed account) has no secret on its own, + // but a stored identity may hold the matching key. Scan identities + // by public key so `G...`/`M...` signs like its alias would; fall + // back to `CannotSign` only when nothing matches. UnresolvedMuxedAccount::Resolved(muxed_account) => { - Err(Error::CannotSign(muxed_account.clone())) + let ed25519 = match muxed_account { + xdr::MuxedAccount::Ed25519(xdr::Uint256(key)) => *key, + xdr::MuxedAccount::MuxedEd25519(m) => m.ed25519.0, + }; + let target = stellar_strkey::ed25519::PublicKey(ed25519); + locator + .secret_by_public_key(&target)? + .ok_or_else(|| Error::CannotSign(muxed_account.clone())) } UnresolvedMuxedAccount::AliasOrSecret(alias_or_secret) => { Ok(locator.read_key(alias_or_secret)?.try_into()?) @@ -200,6 +211,47 @@ impl AsRef for ContractName { #[cfg(test)] mod tests { use super::*; + use crate::config::secret::Secret; + + const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ"; + const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH"; + const OTHER_PUBLIC_KEY: &str = "GAKSH6AD2IPJQELTHIOWDAPYX74YELUOWJLI2L4RIPIPZH6YQIFNUSDC"; + + fn locator_with_identity() -> (tempfile::TempDir, locator::Args) { + let dir = tempfile::tempdir().unwrap(); + let locator = locator::Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let secret = Secret::SecretKey { + secret_key: TEST_SECRET_KEY.to_string(), + }; + locator.write_identity("alice", &secret).unwrap(); + (dir, locator) + } + + #[test] + fn resolve_secret_matches_public_key_to_stored_identity() { + let (_dir, locator) = locator_with_identity(); + let account: UnresolvedMuxedAccount = TEST_PUBLIC_KEY.parse().unwrap(); + assert!(matches!(account, UnresolvedMuxedAccount::Resolved(_))); + + let secret = account.resolve_secret(&locator).unwrap(); + assert!(matches!( + secret, + Secret::SecretKey { ref secret_key } if secret_key == TEST_SECRET_KEY + )); + } + + #[test] + fn resolve_secret_errors_when_public_key_has_no_stored_identity() { + let (_dir, locator) = locator_with_identity(); + let account: UnresolvedMuxedAccount = OTHER_PUBLIC_KEY.parse().unwrap(); + + assert!(matches!( + account.resolve_secret(&locator).unwrap_err(), + Error::CannotSign(_) + )); + } #[test] fn ledger_shorthand_is_not_recognized() { diff --git a/cmd/soroban-cli/src/config/locator.rs b/cmd/soroban-cli/src/config/locator.rs index 8a118d084c..2f5bf511e7 100644 --- a/cmd/soroban-cli/src/config/locator.rs +++ b/cmd/soroban-cli/src/config/locator.rs @@ -401,6 +401,26 @@ impl Args { Ok(self.read_key(key_or_name)?.muxed_account(hd_path)?) } + /// Find a stored identity whose public key matches `target`, returning its + /// secret. Best-effort: identities whose public key can't be derived + /// without error (e.g. a disconnected ledger) are skipped rather than + /// failing the whole lookup. Each identity is matched at its own persisted + /// hd_path. + pub fn secret_by_public_key( + &self, + target: &stellar_strkey::ed25519::PublicKey, + ) -> Result, Error> { + for name in self.list_identities()? { + let Ok(Key::Secret(secret)) = self.read_identity(&name) else { + continue; + }; + if secret.public_key(None).is_ok_and(|pk| &pk == target) { + return Ok(Some(secret)); + } + } + Ok(None) + } + pub fn read_network(&self, name: &str) -> Result { utils::validate_name(name)?; let res = KeyType::Network.read_with_global(name, self); @@ -1525,4 +1545,49 @@ mod tests { assert!(matches!(key, Key::PublicKey(_))); } } + + mod secret_by_public_key { + use super::super::*; + + const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ"; + const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH"; + const OTHER_PUBLIC_KEY: &str = "GAKSH6AD2IPJQELTHIOWDAPYX74YELUOWJLI2L4RIPIPZH6YQIFNUSDC"; + + fn locator_with_tempdir() -> (tempfile::TempDir, Args) { + let dir = tempfile::tempdir().unwrap(); + let args = Args { + config_dir: Some(dir.path().to_path_buf()), + }; + (dir, args) + } + + #[test] + fn returns_secret_for_stored_identity() { + let (_dir, locator) = locator_with_tempdir(); + let secret = Secret::SecretKey { + secret_key: TEST_SECRET_KEY.to_string(), + }; + locator.write_identity("alice", &secret).unwrap(); + + let target = stellar_strkey::ed25519::PublicKey::from_string(TEST_PUBLIC_KEY).unwrap(); + let found = locator.secret_by_public_key(&target).unwrap(); + + assert!(matches!( + found, + Some(Secret::SecretKey { ref secret_key }) if secret_key == TEST_SECRET_KEY + )); + } + + #[test] + fn returns_none_for_unknown_public_key() { + let (_dir, locator) = locator_with_tempdir(); + let secret = Secret::SecretKey { + secret_key: TEST_SECRET_KEY.to_string(), + }; + locator.write_identity("alice", &secret).unwrap(); + + let target = stellar_strkey::ed25519::PublicKey::from_string(OTHER_PUBLIC_KEY).unwrap(); + assert!(locator.secret_by_public_key(&target).unwrap().is_none()); + } + } } From f4c1f63a9d6b19a497df737b69684037f8d23717 Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Mon, 27 Jul 2026 14:44:41 -0300 Subject: [PATCH 2/4] Only find a signer for plain account keys. --- cmd/soroban-cli/src/config/address.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index dd3cdd79be..63aab02b20 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -78,16 +78,17 @@ impl UnresolvedMuxedAccount { pub fn resolve_secret(&self, locator: &locator::Args) -> Result { match &self { - // A literal public key (or muxed account) has no secret on its own, - // but a stored identity may hold the matching key. Scan identities - // by public key so `G...`/`M...` signs like its alias would; fall - // back to `CannotSign` only when nothing matches. + // A literal public key has no secret on its own, but a stored + // identity may hold the matching key. Scan identities by public key + // so `G...` signs like its alias would; fall back to `CannotSign` + // when nothing matches. Muxed accounts (`M...`) aren't signable + // end-to-end yet (see the `todo!` in `sign_soroban_authorizations`), + // so they keep returning `CannotSign`. UnresolvedMuxedAccount::Resolved(muxed_account) => { - let ed25519 = match muxed_account { - xdr::MuxedAccount::Ed25519(xdr::Uint256(key)) => *key, - xdr::MuxedAccount::MuxedEd25519(m) => m.ed25519.0, + let xdr::MuxedAccount::Ed25519(xdr::Uint256(key)) = muxed_account else { + return Err(Error::CannotSign(muxed_account.clone())); }; - let target = stellar_strkey::ed25519::PublicKey(ed25519); + let target = stellar_strkey::ed25519::PublicKey(*key); locator .secret_by_public_key(&target)? .ok_or_else(|| Error::CannotSign(muxed_account.clone())) From e602827ac3139841781eb0a62bebb2c5929698d4 Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Mon, 27 Jul 2026 14:49:25 -0300 Subject: [PATCH 3/4] Honor the derivation path when signing by public key. --- .../src/commands/contract/arg_parsing.rs | 15 +++---- cmd/soroban-cli/src/config/address.rs | 12 ++++-- cmd/soroban-cli/src/config/locator.rs | 43 ++++++++++++++++--- cmd/soroban-cli/src/config/mod.rs | 4 +- 4 files changed, 53 insertions(+), 21 deletions(-) diff --git a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index 631da1c444..fd9002493f 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -466,15 +466,12 @@ fn resolve_address(addr_or_alias: &str, config: &config::Args) -> Result Option { let account: config::UnresolvedMuxedAccount = addr_or_alias.parse().ok()?; - let secret = account.resolve_secret(&config.locator).ok()?; - // A raw public key was matched to an identity at its persisted derivation, - // so sign at that path (None); an alias or secret honors the global - // --hd-path as before. - let hd_path = match account { - config::UnresolvedMuxedAccount::Resolved(_) => None, - config::UnresolvedMuxedAccount::AliasOrSecret(_) => config.hd_path(), - }; - let signer = secret.signer(hd_path, Print::new(false)).ok()?; + // A raw public key is matched to an identity at `--hd-path`, so the signer + // is built at that same path; an alias or secret honors it the same way. + let secret = account + .resolve_secret(&config.locator, config.hd_path()) + .ok()?; + let signer = secret.signer(config.hd_path(), Print::new(false)).ok()?; Some(signer) } diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 63aab02b20..04b4bf80f3 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -76,7 +76,11 @@ impl UnresolvedMuxedAccount { } } - pub fn resolve_secret(&self, locator: &locator::Args) -> Result { + pub fn resolve_secret( + &self, + locator: &locator::Args, + hd_path: Option, + ) -> Result { match &self { // A literal public key has no secret on its own, but a stored // identity may hold the matching key. Scan identities by public key @@ -90,7 +94,7 @@ impl UnresolvedMuxedAccount { }; let target = stellar_strkey::ed25519::PublicKey(*key); locator - .secret_by_public_key(&target)? + .secret_by_public_key(&target, hd_path)? .ok_or_else(|| Error::CannotSign(muxed_account.clone())) } UnresolvedMuxedAccount::AliasOrSecret(alias_or_secret) => { @@ -236,7 +240,7 @@ mod tests { let account: UnresolvedMuxedAccount = TEST_PUBLIC_KEY.parse().unwrap(); assert!(matches!(account, UnresolvedMuxedAccount::Resolved(_))); - let secret = account.resolve_secret(&locator).unwrap(); + let secret = account.resolve_secret(&locator, None).unwrap(); assert!(matches!( secret, Secret::SecretKey { ref secret_key } if secret_key == TEST_SECRET_KEY @@ -249,7 +253,7 @@ mod tests { let account: UnresolvedMuxedAccount = OTHER_PUBLIC_KEY.parse().unwrap(); assert!(matches!( - account.resolve_secret(&locator).unwrap_err(), + account.resolve_secret(&locator, None).unwrap_err(), Error::CannotSign(_) )); } diff --git a/cmd/soroban-cli/src/config/locator.rs b/cmd/soroban-cli/src/config/locator.rs index 2f5bf511e7..6141ea2961 100644 --- a/cmd/soroban-cli/src/config/locator.rs +++ b/cmd/soroban-cli/src/config/locator.rs @@ -402,19 +402,21 @@ impl Args { } /// Find a stored identity whose public key matches `target`, returning its - /// secret. Best-effort: identities whose public key can't be derived - /// without error (e.g. a disconnected ledger) are skipped rather than - /// failing the whole lookup. Each identity is matched at its own persisted - /// hd_path. + /// secret. Each identity is derived at `hd_path` (falling back to its own + /// persisted path when `hd_path` is `None`), so a key looked up by strkey + /// resolves the same way it would by alias under the same `--hd-path`. + /// Best-effort: identities whose public key can't be derived without error + /// (e.g. a disconnected ledger) are skipped rather than failing the lookup. pub fn secret_by_public_key( &self, target: &stellar_strkey::ed25519::PublicKey, + hd_path: Option, ) -> Result, Error> { for name in self.list_identities()? { let Ok(Key::Secret(secret)) = self.read_identity(&name) else { continue; }; - if secret.public_key(None).is_ok_and(|pk| &pk == target) { + if secret.public_key(hd_path).is_ok_and(|pk| &pk == target) { return Ok(Some(secret)); } } @@ -1552,6 +1554,8 @@ mod tests { const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ"; const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH"; const OTHER_PUBLIC_KEY: &str = "GAKSH6AD2IPJQELTHIOWDAPYX74YELUOWJLI2L4RIPIPZH6YQIFNUSDC"; + const TEST_SEED_PHRASE: &str = + "depth decade power loud smile spatial sign movie judge february rate broccoli"; fn locator_with_tempdir() -> (tempfile::TempDir, Args) { let dir = tempfile::tempdir().unwrap(); @@ -1570,7 +1574,7 @@ mod tests { locator.write_identity("alice", &secret).unwrap(); let target = stellar_strkey::ed25519::PublicKey::from_string(TEST_PUBLIC_KEY).unwrap(); - let found = locator.secret_by_public_key(&target).unwrap(); + let found = locator.secret_by_public_key(&target, None).unwrap(); assert!(matches!( found, @@ -1587,7 +1591,32 @@ mod tests { locator.write_identity("alice", &secret).unwrap(); let target = stellar_strkey::ed25519::PublicKey::from_string(OTHER_PUBLIC_KEY).unwrap(); - assert!(locator.secret_by_public_key(&target).unwrap().is_none()); + assert!(locator + .secret_by_public_key(&target, None) + .unwrap() + .is_none()); + } + + #[test] + fn matches_identity_at_requested_hd_path() { + let (_dir, locator) = locator_with_tempdir(); + let secret = Secret::SeedPhrase { + seed_phrase: TEST_SEED_PHRASE.to_string(), + hd_path: None, + }; + locator.write_identity("alice", &secret).unwrap(); + + // The account derived at index 5 is only found when the lookup uses + // the same hd_path; the default (index 0) path must not match it. + let at_five = secret.public_key(Some(5)).unwrap(); + assert!(locator + .secret_by_public_key(&at_five, Some(5)) + .unwrap() + .is_some()); + assert!(locator + .secret_by_public_key(&at_five, None) + .unwrap() + .is_none()); } } } diff --git a/cmd/soroban-cli/src/config/mod.rs b/cmd/soroban-cli/src/config/mod.rs index 464c338a3d..cf734df854 100644 --- a/cmd/soroban-cli/src/config/mod.rs +++ b/cmd/soroban-cli/src/config/mod.rs @@ -101,7 +101,9 @@ impl Args { } pub fn key_pair(&self) -> Result { - let key = &self.source_account.resolve_secret(&self.locator)?; + let key = &self + .source_account + .resolve_secret(&self.locator, self.hd_path())?; Ok(key.key_pair(self.hd_path())?) } From dde8297c6da7da28f67ec286e889a1d83a2cd746 Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Tue, 28 Jul 2026 12:14:33 -0300 Subject: [PATCH 4/4] Test signing is refused for unknown and muxed accounts. --- .../tests/it/integration/hello_world.rs | 31 +++++++++++++++++-- cmd/soroban-cli/src/config/address.rs | 21 +++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/cmd/crates/soroban-test/tests/it/integration/hello_world.rs b/cmd/crates/soroban-test/tests/it/integration/hello_world.rs index d779d4c661..394e510a36 100644 --- a/cmd/crates/soroban-test/tests/it/integration/hello_world.rs +++ b/cmd/crates/soroban-test/tests/it/integration/hello_world.rs @@ -134,7 +134,7 @@ async fn invoke_contract() { invoke_auth_with_identity(sandbox, id, "test", &addr); invoke_auth_with_identity(sandbox, id, "testone", &addr_1); invoke_auth_with_non_source_identity(sandbox, id, "test", "testone", &addr_1); - invoke_auth_with_different_test_account_fail(sandbox, id, &addr_1).await; + invoke_auth_with_unknown_account_fail(sandbox, id).await; contract_data_read_failure(sandbox, id); invoke_with_seed(sandbox, id, &seed_phrase).await; invoke_with_sk(sandbox, id, &secret_key).await; @@ -249,10 +249,35 @@ fn invoke_auth_with_non_source_identity( .success(); } -async fn invoke_auth_with_different_test_account_fail(sandbox: &TestEnv, id: &str, addr: &str) { +// A public key that matches no stored identity has no signer, so requiring its +// auth must fail rather than silently succeed. This guards the `CannotSign` +// fallback in `resolve_secret`: signing by public key only works when the key +// belongs to a known identity. +async fn invoke_auth_with_unknown_account_fail(sandbox: &TestEnv, id: &str) { + // Mint a fresh keypair, capture its address, then remove the identity so the + // address is a valid, funded-elsewhere account that we hold no secret for. + sandbox + .new_assert_cmd("keys") + .arg("generate") + .arg("unknown") + .assert() + .success(); + let addr = sandbox + .new_assert_cmd("keys") + .arg("address") + .arg("unknown") + .assert() + .stdout_as_str(); + sandbox + .new_assert_cmd("keys") + .arg("rm") + .arg("--force") + .arg("unknown") + .assert() + .success(); + let res = sandbox .invoke_with_test(&[ - "--hd-path=0", "--id", id, "--", diff --git a/cmd/soroban-cli/src/config/address.rs b/cmd/soroban-cli/src/config/address.rs index 04b4bf80f3..6cc29af5bf 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -258,6 +258,27 @@ mod tests { )); } + #[test] + fn resolve_secret_rejects_muxed_account_even_with_stored_identity() { + let (_dir, locator) = locator_with_identity(); + // A muxed account (`M...`) wrapping alice's ed25519 key. Even though the + // underlying key belongs to a stored identity, muxed accounts aren't + // signable end-to-end yet, so resolution must still return `CannotSign` + // rather than the stored secret. + let pk = stellar_strkey::ed25519::PublicKey::from_string(TEST_PUBLIC_KEY).unwrap(); + let account = UnresolvedMuxedAccount::Resolved(xdr::MuxedAccount::MuxedEd25519( + xdr::MuxedAccountMed25519 { + id: 1, + ed25519: xdr::Uint256(pk.0), + }, + )); + + assert!(matches!( + account.resolve_secret(&locator, None).unwrap_err(), + Error::CannotSign(_) + )); + } + #[test] fn ledger_shorthand_is_not_recognized() { match "ledger".parse::().unwrap() {