Skip to content

Commit 8579c49

Browse files
mootz12fnando
andauthored
feat: add user confirmation for keys rm command (#2452)
### What Add a confirmation prompt when deleting keys. The confirmation prompt can be bypassed with a `--force` flag. Confirmation also works by piping `y` into `stdin`. ```bash $ stellar keys rm rmtest ⚠️ Are you sure you want to remove the key 'rmtest' at '/my-config-path/.config/stellar/identity/rmtest.toml'? This action cannot be undone. (y/N) y ℹ️ Removing the key's cli config file ``` ```bash $ stellar keys rm rmtest --force ℹ️ Removing the key's cli config file ``` ```bash $ stellar keys rm rmtest ⚠️ Are you sure you want to remove the key 'rmtest'? This action cannot be undone. (y/N) N ❌ error: removal cancelled by user ``` ### Why Closes: #2420 ### Known limitations None --------- Co-authored-by: Nando Vieira <me@fnando.com>
1 parent cadec3f commit 8579c49

6 files changed

Lines changed: 182 additions & 8 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ exclude = [
2222

2323
[workspace.package]
2424
version = "25.2.0"
25-
rust-version = "1.89.0"
25+
rust-version = "1.92.0"
2626

2727
# Dependencies located in this repo:
2828
[workspace.dependencies.soroban-cli]

FULL_HELP_DOCS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,6 +1283,10 @@ Remove an identity
12831283

12841284
- `<NAME>` — Identity to remove
12851285

1286+
###### **Options:**
1287+
1288+
- `--force` — Skip confirmation prompt
1289+
12861290
###### **Options (Global):**
12871291

12881292
- `--global` — ⚠️ Deprecated: global config is always on

cmd/crates/soroban-test/tests/it/integration/keys.rs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,115 @@ async fn unset_default_identity() {
210210
.stdout(predicate::str::contains("STELLAR_ACCOUNT=").not())
211211
.success();
212212
}
213+
214+
#[tokio::test]
215+
async fn rm_requires_confirmation() {
216+
let sandbox = &TestEnv::new();
217+
sandbox
218+
.new_assert_cmd("keys")
219+
.arg("generate")
220+
.arg("rmtest1")
221+
.assert()
222+
.success();
223+
224+
// Piping "n" should cancel removal
225+
sandbox
226+
.new_assert_cmd("keys")
227+
.arg("rm")
228+
.arg("rmtest1")
229+
.write_stdin("n\n")
230+
.assert()
231+
.stderr(predicate::str::contains("removal cancelled by user"))
232+
.failure();
233+
234+
sandbox
235+
.new_assert_cmd("keys")
236+
.arg("address")
237+
.arg("rmtest1")
238+
.assert()
239+
.success();
240+
241+
// Piping empty input (just Enter) should default to cancel
242+
sandbox
243+
.new_assert_cmd("keys")
244+
.arg("rm")
245+
.arg("rmtest1")
246+
.write_stdin("\n")
247+
.assert()
248+
.stderr(predicate::str::contains("removal cancelled by user"))
249+
.failure();
250+
251+
sandbox
252+
.new_assert_cmd("keys")
253+
.arg("address")
254+
.arg("rmtest1")
255+
.assert()
256+
.success();
257+
258+
// Piping "y" should confirm removal
259+
sandbox
260+
.new_assert_cmd("keys")
261+
.arg("rm")
262+
.arg("rmtest1")
263+
.write_stdin("y\n")
264+
.assert()
265+
.stderr(predicate::str::contains(
266+
"Removing the key's cli config file",
267+
))
268+
.success();
269+
270+
sandbox
271+
.new_assert_cmd("keys")
272+
.arg("address")
273+
.arg("rmtest1")
274+
.assert()
275+
.failure();
276+
}
277+
278+
#[tokio::test]
279+
async fn rm_with_force_skips_confirmation() {
280+
let sandbox = &TestEnv::new();
281+
sandbox
282+
.new_assert_cmd("keys")
283+
.arg("generate")
284+
.arg("rmtest2")
285+
.assert()
286+
.success();
287+
288+
sandbox
289+
.new_assert_cmd("keys")
290+
.arg("rm")
291+
.arg("rmtest2")
292+
.arg("--force")
293+
.assert()
294+
.success();
295+
296+
sandbox
297+
.new_assert_cmd("keys")
298+
.arg("address")
299+
.arg("rmtest2")
300+
.assert()
301+
.failure();
302+
}
303+
304+
#[tokio::test]
305+
async fn rm_nonexistent_key() {
306+
let sandbox = &TestEnv::new();
307+
308+
// Without --force: should fail before prompting
309+
sandbox
310+
.new_assert_cmd("keys")
311+
.arg("rm")
312+
.arg("doesnotexist")
313+
.assert()
314+
.failure();
315+
316+
// With --force: should still fail
317+
sandbox
318+
.new_assert_cmd("keys")
319+
.arg("rm")
320+
.arg("doesnotexist")
321+
.arg("--force")
322+
.assert()
323+
.failure();
324+
}
Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,22 @@
1+
use std::io::{self, BufRead, IsTerminal};
2+
13
use crate::commands::global;
24
use crate::config::address::KeyName;
3-
4-
use super::super::config::locator;
5+
use crate::config::locator::{self, KeyType, Location};
6+
use crate::print::Print;
57

68
#[derive(thiserror::Error, Debug)]
79
pub enum Error {
810
#[error(transparent)]
911
Locator(#[from] locator::Error),
12+
#[error("removal cancelled by user")]
13+
Cancelled,
14+
#[error(transparent)]
15+
Io(#[from] io::Error),
16+
#[error(
17+
"please migrate from local storage using `stellar config migrate` before removing keys"
18+
)]
19+
LocalStorage(),
1020
}
1121

1222
#[derive(Debug, clap::Parser, Clone)]
@@ -15,12 +25,42 @@ pub struct Cmd {
1525
/// Identity to remove
1626
pub name: KeyName,
1727

28+
/// Skip confirmation prompt
29+
#[arg(long)]
30+
pub force: bool,
31+
1832
#[command(flatten)]
1933
pub config: locator::Args,
2034
}
2135

2236
impl Cmd {
2337
pub fn run(&self, global_args: &global::Args) -> Result<(), Error> {
38+
if !self.force {
39+
let print = Print::new(false);
40+
let stdin = io::stdin();
41+
42+
// Check that the key exists before asking for confirmation
43+
let (_, location) = self.config.read_identity_with_location(&self.name)?;
44+
// TODO: Remove check for local storage once it's no longer supported
45+
if let Location::Local(_) = location {
46+
return Err(Error::LocalStorage());
47+
}
48+
49+
// Show the prompt only when the user can see it
50+
if stdin.is_terminal() {
51+
let config_path = KeyType::Identity.path(location.as_ref(), &self.name);
52+
print.warnln(format!(
53+
"Are you sure you want to remove the key '{}' at '{}'? This action cannot be undone. (y/N)",
54+
self.name,
55+
config_path.display()
56+
));
57+
}
58+
let mut response = String::new();
59+
stdin.lock().read_line(&mut response)?;
60+
if !response.trim().eq_ignore_ascii_case("y") {
61+
return Err(Error::Cancelled);
62+
}
63+
}
2464
Ok(self.config.remove_identity(&self.name, global_args)?)
2565
}
2666
}

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

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ pub struct Args {
120120
pub config_dir: Option<PathBuf>,
121121
}
122122

123+
#[derive(Clone)]
123124
pub enum Location {
124125
Local(PathBuf),
125126
Global(PathBuf),
@@ -288,6 +289,12 @@ impl Args {
288289
KeyType::Identity.read_with_global(name, self)
289290
}
290291

292+
// TODO: Remove once local storage is no longer supported
293+
pub fn read_identity_with_location(&self, name: &str) -> Result<(Key, Location), Error> {
294+
utils::validate_name(name)?;
295+
KeyType::Identity.read_with_global_with_location(name, self)
296+
}
297+
291298
pub fn read_key(&self, key_or_name: &str) -> Result<Key, Error> {
292299
key_or_name
293300
.parse()
@@ -643,15 +650,23 @@ impl KeyType {
643650
key: &str,
644651
locator: &Args,
645652
) -> Result<T, Error> {
653+
Ok(self.read_with_global_with_location(key, locator)?.0)
654+
}
655+
656+
pub fn read_with_global_with_location<T: DeserializeOwned>(
657+
&self,
658+
key: &str,
659+
locator: &Args,
660+
) -> Result<(T, Location), Error> {
646661
for location in locator.local_and_global()? {
647662
let path = self.path(location.as_ref(), key);
648663

649664
if let Ok(t) = Self::read_from_path(&path) {
650-
if let Location::Local(config_dir) = location {
665+
if let Location::Local(config_dir) = location.clone() {
651666
print_deprecation_warning(&config_dir);
652667
}
653668

654-
return Ok(t);
669+
return Ok((t, location));
655670
}
656671
}
657672
Err(Error::ConfigMissing(self.to_string(), key.to_string()))
@@ -702,9 +717,12 @@ impl KeyType {
702717
pwd.join(self.to_string())
703718
}
704719

705-
fn path(&self, pwd: &Path, key: &str) -> PathBuf {
720+
pub fn path(&self, pwd: &Path, key: &str) -> PathBuf {
706721
let mut path = self.root(pwd).join(key);
707-
path.set_extension("toml");
722+
match self {
723+
KeyType::Identity | KeyType::Network => path.set_extension("toml"),
724+
KeyType::ContractIds => path.set_extension("json"),
725+
};
708726
path
709727
}
710728

cookbook/stellar-keys.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ You can also fund the account while creating the key by using `stellar keys gene
112112
When you no longer need this identity, remove it using:
113113

114114
```bash
115-
stellar keys rm carol
115+
stellar keys rm carol --force
116116
```
117117

118118
Output:

0 commit comments

Comments
 (0)