diff --git a/cmd/crates/soroban-test/tests/it/integration/auth.rs b/cmd/crates/soroban-test/tests/it/integration/auth.rs index 3d09d980e..2ff953bc0 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/crates/soroban-test/tests/it/integration/hello_world.rs b/cmd/crates/soroban-test/tests/it/integration/hello_world.rs index d779d4c66..394e510a3 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/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index b2617ca9a..fd9002493 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -465,9 +465,13 @@ 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()?; + // 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 ef037c45b..6cc29af5b 100644 --- a/cmd/soroban-cli/src/config/address.rs +++ b/cmd/soroban-cli/src/config/address.rs @@ -76,10 +76,26 @@ 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 + // 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) => { - Err(Error::CannotSign(muxed_account.clone())) + let xdr::MuxedAccount::Ed25519(xdr::Uint256(key)) = muxed_account else { + return Err(Error::CannotSign(muxed_account.clone())); + }; + let target = stellar_strkey::ed25519::PublicKey(*key); + locator + .secret_by_public_key(&target, hd_path)? + .ok_or_else(|| Error::CannotSign(muxed_account.clone())) } UnresolvedMuxedAccount::AliasOrSecret(alias_or_secret) => { Ok(locator.read_key(alias_or_secret)?.try_into()?) @@ -200,6 +216,68 @@ 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, None).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, None).unwrap_err(), + Error::CannotSign(_) + )); + } + + #[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() { diff --git a/cmd/soroban-cli/src/config/locator.rs b/cmd/soroban-cli/src/config/locator.rs index 8a118d084..6141ea296 100644 --- a/cmd/soroban-cli/src/config/locator.rs +++ b/cmd/soroban-cli/src/config/locator.rs @@ -401,6 +401,28 @@ 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. 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(hd_path).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 +1547,76 @@ 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"; + 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(); + 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, None).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, 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 464c338a3..cf734df85 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())?) }