Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions FULL_HELP_DOCS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,7 @@ Add a new identity (keypair, ledger, OS specific secure store)

- `--public-key <PUBLIC_KEY>` — Add a public key, ed25519, or muxed account, e.g. G1.., M2..
- `--overwrite` — Overwrite existing identity if it already exists. When combined with --secure-store, also replaces the existing Secure Store entry
- `--hd-path <HD_PATH>` — When importing a seed phrase into the Secure Store, which `hd_path` to derive the key at
- `--hd-path <HD_PATH>` — When importing a seed phrase, which `hd_path` to derive the key at. Persisted on the identity (or its Secure Store entry) so later commands derive the same account without re-passing the flag. Not valid with `--public-key` or a raw secret key

###### **Options (Global):**

Expand Down Expand Up @@ -1207,7 +1207,7 @@ Generate a new identity using a 24-word seed phrase The seed phrase can be store

On Mac this uses Keychain, on Windows it is Secure Store Service, and on \*nix platforms it uses a combination of the kernel keyutils and DBus-based Secret Service.

- `--hd-path <HD_PATH>` — With `--as-secret` or `--secure-store`, which `hd_path` to derive the key at from the seed phrase
- `--hd-path <HD_PATH>` — Which `hd_path` to derive the key at from the seed phrase. Honored across all storage modes: with `--as-secret` it picks which derived key is stored, with `--secure-store` or plain seed-phrase storage it is persisted on the identity so later commands derive the same account without re-passing the flag
- `--fund` — Fund generated key pair

Default value: `false`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ async fn invoke_contract() {
let dir = sandbox.config_dir();
let seed_phrase = std::fs::read_to_string(dir.join("identity/test.toml")).unwrap();
let s = toml::from_str::<secret::Secret>(&seed_phrase).unwrap();
let secret::Secret::SeedPhrase { seed_phrase } = s else {
let secret::Secret::SeedPhrase { seed_phrase, .. } = s else {
panic!("Expected seed phrase")
};

Expand Down
73 changes: 73 additions & 0 deletions cmd/crates/soroban-test/tests/it/integration/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,79 @@ async fn rm_with_force_skips_confirmation() {
.failure();
}

// `keys generate --hd-path N` (plain seed-phrase storage) must persist N so
// that later `keys address` calls without `--hd-path` derive at index N rather
// than the default. Guards the user-visible contract from #2538 across CLI
// parsing, identity-file I/O, and key derivation.
#[tokio::test]
async fn hd_path_persists_for_keys_generate() {
let sandbox = &TestEnv::new();
sandbox
.new_assert_cmd("keys")
.args(["generate", "hd-gen", "--hd-path", "5"])
.assert()
.success();

let address_default = pubkey_for_identity(sandbox, "hd-gen");
let address_explicit = sandbox
.new_assert_cmd("keys")
.args(["address", "hd-gen", "--hd-path", "5"])
.assert()
.success()
.stdout_as_str();
let address_zero = sandbox
.new_assert_cmd("keys")
.args(["address", "hd-gen", "--hd-path", "0"])
.assert()
.success()
.stdout_as_str();

assert_eq!(
address_default, address_explicit,
"expected `keys address hd-gen` (no flag) to derive at the persisted hd_path 5"
);
assert_ne!(
address_default, address_zero,
"expected hd_path 5 derivation to differ from hd_path 0"
);
}

#[tokio::test]
async fn hd_path_persists_for_keys_add_seed_phrase() {
let sandbox = &TestEnv::new();
let seed_phrase = "aisle reflect depart add safe fury dress artist bronze abuse warrior clap inquiry ask mandate deputy view trade debate flip priority boy depart recipe";

sandbox
.new_assert_cmd("keys")
.write_stdin(format!("{seed_phrase}\n"))
.args(["add", "hd-add", "--hd-path", "5"])
.assert()
.success();

let address_default = pubkey_for_identity(sandbox, "hd-add");
let address_explicit = sandbox
.new_assert_cmd("keys")
.args(["address", "hd-add", "--hd-path", "5"])
.assert()
.success()
.stdout_as_str();
let address_zero = sandbox
.new_assert_cmd("keys")
.args(["address", "hd-add", "--hd-path", "0"])
.assert()
.success()
.stdout_as_str();

assert_eq!(
address_default, address_explicit,
"expected `keys address hd-add` (no flag) to derive at the persisted hd_path 5"
);
assert_ne!(
address_default, address_zero,
"expected hd_path 5 derivation to differ from hd_path 0"
);
}

#[tokio::test]
async fn rm_nonexistent_key() {
let sandbox = &TestEnv::new();
Expand Down
1 change: 1 addition & 0 deletions cmd/crates/soroban-test/tests/it/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub fn add_key(dir: &Path, name: &str, kind: SecretKind, data: &str) {
let secret = match kind {
SecretKind::Seed => Secret::SeedPhrase {
seed_phrase: data.to_string(),
hd_path: None,
},
SecretKind::Key => Secret::SecretKey {
secret_key: data.to_string(),
Expand Down
91 changes: 87 additions & 4 deletions cmd/soroban-cli/src/commands/keys/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ pub enum Error {
unset STELLAR_SECRET_KEY or provide a seed phrase instead"
)]
SecureStoreRequiresSeedPhrase,

#[error("--hd-path is not valid with a secret key; secret keys cannot be derived")]
HdPathNotSupportedForSecretKey,

#[error("--hd-path is not valid with --public-key; public keys cannot be derived")]
HdPathNotSupportedForPublicKey,
}

#[derive(Debug, clap::Parser, Clone)]
Expand All @@ -62,7 +68,9 @@ pub struct Cmd {
#[arg(long)]
pub overwrite: bool,

/// When importing a seed phrase into the Secure Store, which `hd_path` to derive the key at.
/// When importing a seed phrase, which `hd_path` to derive the key at. Persisted on
/// the identity (or its Secure Store entry) so later commands derive the same account
/// without re-passing the flag. Not valid with `--public-key` or a raw secret key.
Comment thread
fnando marked this conversation as resolved.
#[arg(long)]
pub hd_path: Option<usize>,
}
Expand All @@ -71,6 +79,10 @@ impl Cmd {
pub fn run(&self, global_args: &global::Args) -> Result<(), Error> {
let print = Print::new(global_args.quiet);

if self.public_key.is_some() && self.hd_path.is_some() {
return Err(Error::HdPathNotSupportedForPublicKey);
}

if self.config_locator.read_identity(&self.name).is_ok() {
if !self.overwrite {
return Err(Error::IdentityAlreadyExists(self.name.to_string()));
Expand Down Expand Up @@ -98,7 +110,7 @@ impl Cmd {
return Err(Error::SecureStoreRequiresSeedPhrase);
}
} else if let Ok(secret_key) = std::env::var("STELLAR_SECRET_KEY") {
return Ok(secret_key.parse()?);
return build_secret(&secret_key, self.hd_path);
}

if self.secrets.secure_store {
Expand All @@ -123,8 +135,8 @@ impl Cmd {
} else {
let prompt = "Type a secret key or 12/24 word seed phrase:";
let secret_key = read_password(print, prompt)?;
let secret = secret_key.parse()?;
if let Secret::SeedPhrase { seed_phrase } = &secret {
let secret = build_secret(&secret_key, self.hd_path)?;
if let Secret::SeedPhrase { seed_phrase, .. } = &secret {
if seed_phrase.split_whitespace().count() < 24 {
print.warnln("The provided seed phrase lacks sufficient entropy and should be avoided. Using a 24-word seed phrase is a safer option.".to_string());
print.warnln(
Expand All @@ -138,6 +150,18 @@ impl Cmd {
}
}

fn build_secret(input: &str, hd_path: Option<usize>) -> Result<Secret, Error> {
let secret: Secret = input.parse()?;
match (secret, hd_path) {
(Secret::SecretKey { .. }, Some(_)) => Err(Error::HdPathNotSupportedForSecretKey),
(Secret::SeedPhrase { seed_phrase, .. }, hd_path) => Ok(Secret::SeedPhrase {
seed_phrase,
hd_path,
}),
(secret, _) => Ok(secret),
}
}

fn read_password(print: &Print, prompt: &str) -> Result<String, Error> {
if std::io::stdin().is_terminal() {
// Interactive: prompt and read from TTY
Expand Down Expand Up @@ -190,13 +214,72 @@ mod tests {
(temp_dir, locator, cmd)
}

fn cmd_with_public_key(
public_key: &str,
hd_path: Option<usize>,
) -> (tempfile::TempDir, locator::Args, Cmd) {
let (temp_dir, locator, mut cmd) = set_up_test();
cmd.public_key = Some(public_key.to_string());
cmd.hd_path = hd_path;
(temp_dir, locator, cmd)
}

fn global_args() -> global::Args {
global::Args {
quiet: true,
..Default::default()
}
}

#[test]
fn test_build_secret_persists_hd_path_on_seed_phrase() {
let secret = build_secret(SEED_PHRASE, Some(5)).unwrap();
match secret {
Secret::SeedPhrase {
seed_phrase,
hd_path,
} => {
assert_eq!(seed_phrase, SEED_PHRASE);
assert_eq!(hd_path, Some(5));
}
other => panic!("expected SeedPhrase variant, got {other:?}"),
}
}

#[test]
fn test_build_secret_seed_phrase_without_hd_path() {
let secret = build_secret(SEED_PHRASE, None).unwrap();
match secret {
Secret::SeedPhrase { hd_path, .. } => assert_eq!(hd_path, None),
other => panic!("expected SeedPhrase variant, got {other:?}"),
}
}

#[test]
fn test_build_secret_rejects_hd_path_with_secret_key() {
let result = build_secret(SECRET_KEY, Some(5));
assert!(matches!(result, Err(Error::HdPathNotSupportedForSecretKey)));
}

#[test]
fn test_build_secret_secret_key_without_hd_path() {
let secret = build_secret(SECRET_KEY, None).unwrap();
assert!(matches!(secret, Secret::SecretKey { .. }));
}

#[test]
fn test_run_rejects_hd_path_with_public_key() {
let (_tmp, _locator, cmd) = cmd_with_public_key(PUBLIC_KEY, Some(3));
let result = cmd.run(&global_args());
assert!(matches!(result, Err(Error::HdPathNotSupportedForPublicKey)));
}

#[test]
fn test_run_accepts_public_key_without_hd_path() {
let (_tmp, _locator, cmd) = cmd_with_public_key(PUBLIC_KEY, None);
assert!(cmd.run(&global_args()).is_ok());
}

#[test]
fn public_key_flag_accepts_public_key() {
let (_tmp, locator, mut cmd) = set_up_test();
Expand Down
41 changes: 32 additions & 9 deletions cmd/soroban-cli/src/commands/keys/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ pub struct Cmd {
#[command(flatten)]
pub config_locator: locator::Args,

/// With `--as-secret` or `--secure-store`, which `hd_path` to derive the key at from the seed phrase.
/// Which `hd_path` to derive the key at from the seed phrase. Honored across all
/// storage modes: with `--as-secret` it picks which derived key is stored, with
/// `--secure-store` or plain seed-phrase storage it is persisted on the identity
/// so later commands derive the same account without re-passing the flag.
Comment thread
fnando marked this conversation as resolved.
#[arg(long)]
pub hd_path: Option<usize>,

Expand Down Expand Up @@ -127,7 +130,10 @@ impl Cmd {
let secret: Secret = seed_phrase.into();
Ok(secret.private_key(self.hd_path)?.into())
} else {
Ok(seed_phrase.into())
Ok(Secret::SeedPhrase {
seed_phrase: seed_phrase.seed_phrase.into_phrase(),
hd_path: self.hd_path,
})
Comment thread
fnando marked this conversation as resolved.
}
}

Expand All @@ -140,7 +146,7 @@ impl Cmd {
mod tests {
use crate::config::{address::KeyName, key::Key, secret::Secret};

fn set_up_test() -> (super::locator::Args, super::Cmd) {
fn set_up_test() -> (super::locator::Args, super::Cmd, tempfile::TempDir) {
let temp_dir = tempfile::tempdir().unwrap();
let locator = super::locator::Args {
config_dir: Some(temp_dir.path().to_path_buf()),
Expand All @@ -158,7 +164,7 @@ mod tests {
overwrite: false,
};

(locator, cmd)
(locator, cmd, temp_dir)
}

fn global_args() -> super::global::Args {
Expand All @@ -170,7 +176,7 @@ mod tests {

#[tokio::test]
async fn test_storing_secret_as_a_seed_phrase() {
let (test_locator, cmd) = set_up_test();
let (test_locator, cmd, _temp_dir) = set_up_test();
let global_args = global_args();

let result = cmd.run(&global_args).await;
Expand All @@ -179,9 +185,26 @@ mod tests {
assert!(matches!(identity, Key::Secret(Secret::SeedPhrase { .. })));
}

#[tokio::test]
async fn test_generate_seed_phrase_persists_hd_path() {
let (test_locator, mut cmd, _temp_dir) = set_up_test();
cmd.hd_path = Some(7);
let global_args = global_args();

cmd.run(&global_args).await.unwrap();

Comment thread
fnando marked this conversation as resolved.
let identity = test_locator.read_identity("test_name").unwrap();
match identity {
Key::Secret(Secret::SeedPhrase { hd_path, .. }) => {
assert_eq!(hd_path, Some(7));
}
other => panic!("expected SeedPhrase variant, got {other:?}"),
}
}

#[tokio::test]
async fn test_storing_secret_as_a_secret_key() {
let (test_locator, mut cmd) = set_up_test();
let (test_locator, mut cmd, _temp_dir) = set_up_test();
cmd.as_secret = true;
let global_args = global_args();

Expand All @@ -196,7 +219,7 @@ mod tests {
async fn test_storing_secret_in_secure_store() {
use keyring::{mock, set_default_credential_builder};
set_default_credential_builder(mock::default_credential_builder());
let (test_locator, mut cmd) = set_up_test();
let (test_locator, mut cmd, _temp_dir) = set_up_test();
cmd.secure_store = true;
let global_args = global_args();

Expand All @@ -211,7 +234,7 @@ mod tests {
async fn test_generate_secure_store_caches_public_key_on_disk() {
use keyring::{mock, set_default_credential_builder};
set_default_credential_builder(mock::default_credential_builder());
let (test_locator, mut cmd) = set_up_test();
let (test_locator, mut cmd, _temp_dir) = set_up_test();
cmd.secure_store = true;
let global_args = global_args();

Expand All @@ -237,7 +260,7 @@ mod tests {
#[cfg(not(feature = "additional-libs"))]
#[tokio::test]
async fn test_storing_in_secure_store_returns_error_when_additional_libs_not_enabled() {
let (test_locator, mut cmd) = set_up_test();
let (test_locator, mut cmd, _temp_dir) = set_up_test();
cmd.secure_store = true;
let global_args = global_args();

Expand Down
2 changes: 1 addition & 1 deletion cmd/soroban-cli/src/commands/keys/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Cmd {
pub fn seed_phrase(&self) -> Result<String, Error> {
let key = self.locator.read_identity(&self.name)?;

if let Key::Secret(Secret::SeedPhrase { seed_phrase }) = key {
if let Key::Secret(Secret::SeedPhrase { seed_phrase, .. }) = key {
Ok(seed_phrase)
} else {
Err(Error::UnknownSeedPhrase)
Expand Down
5 changes: 4 additions & 1 deletion cmd/soroban-cli/src/config/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,10 @@ mod test {
#[test]
fn secret_seed_phrase() {
let seed_phrase = "singer swing mango apple singer swing mango apple singer swing mango apple singer swing mango apple".to_string();
let secret = Secret::SeedPhrase { seed_phrase };
let secret = Secret::SeedPhrase {
seed_phrase,
hd_path: None,
};
let key = Key::Secret(secret);
round_trip(&key);
}
Expand Down
Loading
Loading