diff --git a/Cargo.toml b/Cargo.toml index 3bdbcf39e..44cafa236 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,7 @@ exclude = [ [workspace.package] version = "25.2.0" -rust-version = "1.89.0" +rust-version = "1.92.0" # Dependencies located in this repo: [workspace.dependencies.soroban-cli] diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index e197e0238..6f21219ee 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -1283,6 +1283,10 @@ Remove an identity - `` — Identity to remove +###### **Options:** + +- `--force` — Skip confirmation prompt + ###### **Options (Global):** - `--global` — ⚠️ Deprecated: global config is always on diff --git a/cmd/crates/soroban-test/tests/it/integration/keys.rs b/cmd/crates/soroban-test/tests/it/integration/keys.rs index 311a60bc2..604782731 100644 --- a/cmd/crates/soroban-test/tests/it/integration/keys.rs +++ b/cmd/crates/soroban-test/tests/it/integration/keys.rs @@ -210,3 +210,115 @@ async fn unset_default_identity() { .stdout(predicate::str::contains("STELLAR_ACCOUNT=").not()) .success(); } + +#[tokio::test] +async fn rm_requires_confirmation() { + let sandbox = &TestEnv::new(); + sandbox + .new_assert_cmd("keys") + .arg("generate") + .arg("rmtest1") + .assert() + .success(); + + // Piping "n" should cancel removal + sandbox + .new_assert_cmd("keys") + .arg("rm") + .arg("rmtest1") + .write_stdin("n\n") + .assert() + .stderr(predicate::str::contains("removal cancelled by user")) + .failure(); + + sandbox + .new_assert_cmd("keys") + .arg("address") + .arg("rmtest1") + .assert() + .success(); + + // Piping empty input (just Enter) should default to cancel + sandbox + .new_assert_cmd("keys") + .arg("rm") + .arg("rmtest1") + .write_stdin("\n") + .assert() + .stderr(predicate::str::contains("removal cancelled by user")) + .failure(); + + sandbox + .new_assert_cmd("keys") + .arg("address") + .arg("rmtest1") + .assert() + .success(); + + // Piping "y" should confirm removal + sandbox + .new_assert_cmd("keys") + .arg("rm") + .arg("rmtest1") + .write_stdin("y\n") + .assert() + .stderr(predicate::str::contains( + "Removing the key's cli config file", + )) + .success(); + + sandbox + .new_assert_cmd("keys") + .arg("address") + .arg("rmtest1") + .assert() + .failure(); +} + +#[tokio::test] +async fn rm_with_force_skips_confirmation() { + let sandbox = &TestEnv::new(); + sandbox + .new_assert_cmd("keys") + .arg("generate") + .arg("rmtest2") + .assert() + .success(); + + sandbox + .new_assert_cmd("keys") + .arg("rm") + .arg("rmtest2") + .arg("--force") + .assert() + .success(); + + sandbox + .new_assert_cmd("keys") + .arg("address") + .arg("rmtest2") + .assert() + .failure(); +} + +#[tokio::test] +async fn rm_nonexistent_key() { + let sandbox = &TestEnv::new(); + + // Without --force: should fail before prompting + sandbox + .new_assert_cmd("keys") + .arg("rm") + .arg("doesnotexist") + .assert() + .failure(); + + // With --force: should still fail + sandbox + .new_assert_cmd("keys") + .arg("rm") + .arg("doesnotexist") + .arg("--force") + .assert() + .failure(); +} diff --git a/cmd/soroban-cli/src/commands/keys/rm.rs b/cmd/soroban-cli/src/commands/keys/rm.rs index c0ba97909..173640fc3 100644 --- a/cmd/soroban-cli/src/commands/keys/rm.rs +++ b/cmd/soroban-cli/src/commands/keys/rm.rs @@ -1,12 +1,22 @@ +use std::io::{self, BufRead, IsTerminal}; + use crate::commands::global; use crate::config::address::KeyName; - -use super::super::config::locator; +use crate::config::locator::{self, KeyType, Location}; +use crate::print::Print; #[derive(thiserror::Error, Debug)] pub enum Error { #[error(transparent)] Locator(#[from] locator::Error), + #[error("removal cancelled by user")] + Cancelled, + #[error(transparent)] + Io(#[from] io::Error), + #[error( + "please migrate from local storage using `stellar config migrate` before removing keys" + )] + LocalStorage(), } #[derive(Debug, clap::Parser, Clone)] @@ -15,12 +25,42 @@ pub struct Cmd { /// Identity to remove pub name: KeyName, + /// Skip confirmation prompt + #[arg(long)] + pub force: bool, + #[command(flatten)] pub config: locator::Args, } impl Cmd { pub fn run(&self, global_args: &global::Args) -> Result<(), Error> { + if !self.force { + let print = Print::new(false); + let stdin = io::stdin(); + + // Check that the key exists before asking for confirmation + let (_, location) = self.config.read_identity_with_location(&self.name)?; + // TODO: Remove check for local storage once it's no longer supported + if let Location::Local(_) = location { + return Err(Error::LocalStorage()); + } + + // Show the prompt only when the user can see it + if stdin.is_terminal() { + let config_path = KeyType::Identity.path(location.as_ref(), &self.name); + print.warnln(format!( + "Are you sure you want to remove the key '{}' at '{}'? This action cannot be undone. (y/N)", + self.name, + config_path.display() + )); + } + let mut response = String::new(); + stdin.lock().read_line(&mut response)?; + if !response.trim().eq_ignore_ascii_case("y") { + return Err(Error::Cancelled); + } + } Ok(self.config.remove_identity(&self.name, global_args)?) } } diff --git a/cmd/soroban-cli/src/config/locator.rs b/cmd/soroban-cli/src/config/locator.rs index 9237f582d..b07ae3a9d 100644 --- a/cmd/soroban-cli/src/config/locator.rs +++ b/cmd/soroban-cli/src/config/locator.rs @@ -120,6 +120,7 @@ pub struct Args { pub config_dir: Option, } +#[derive(Clone)] pub enum Location { Local(PathBuf), Global(PathBuf), @@ -288,6 +289,12 @@ impl Args { KeyType::Identity.read_with_global(name, self) } + // TODO: Remove once local storage is no longer supported + pub fn read_identity_with_location(&self, name: &str) -> Result<(Key, Location), Error> { + utils::validate_name(name)?; + KeyType::Identity.read_with_global_with_location(name, self) + } + pub fn read_key(&self, key_or_name: &str) -> Result { key_or_name .parse() @@ -643,15 +650,23 @@ impl KeyType { key: &str, locator: &Args, ) -> Result { + Ok(self.read_with_global_with_location(key, locator)?.0) + } + + pub fn read_with_global_with_location( + &self, + key: &str, + locator: &Args, + ) -> Result<(T, Location), Error> { for location in locator.local_and_global()? { let path = self.path(location.as_ref(), key); if let Ok(t) = Self::read_from_path(&path) { - if let Location::Local(config_dir) = location { + if let Location::Local(config_dir) = location.clone() { print_deprecation_warning(&config_dir); } - return Ok(t); + return Ok((t, location)); } } Err(Error::ConfigMissing(self.to_string(), key.to_string())) @@ -702,9 +717,12 @@ impl KeyType { pwd.join(self.to_string()) } - fn path(&self, pwd: &Path, key: &str) -> PathBuf { + pub fn path(&self, pwd: &Path, key: &str) -> PathBuf { let mut path = self.root(pwd).join(key); - path.set_extension("toml"); + match self { + KeyType::Identity | KeyType::Network => path.set_extension("toml"), + KeyType::ContractIds => path.set_extension("json"), + }; path } diff --git a/cookbook/stellar-keys.mdx b/cookbook/stellar-keys.mdx index bccf61bef..1356f12c4 100644 --- a/cookbook/stellar-keys.mdx +++ b/cookbook/stellar-keys.mdx @@ -112,7 +112,7 @@ You can also fund the account while creating the key by using `stellar keys gene When you no longer need this identity, remove it using: ```bash -stellar keys rm carol +stellar keys rm carol --force ``` Output: