Prevent stale keyring entry when overwriting a key.

FULL_HELP_DOCS.md

@@ -1137,7 +1137,7 @@ Add a new identity (keypair, ledger, OS specific secure store)
   This only supports seed phrases for now.
 
 - `--public-key <PUBLIC_KEY>` — Add a public key, ed25519, or muxed account, e.g. G1.., M2..
-- `--overwrite` — Overwrite existing identity if it already exists
+- `--overwrite` — Overwrite existing identity if it already exists. When combined with --secure-store, also replaces the existing Secure Store entry
 
 ###### **Options (Global):**
 
@@ -1211,7 +1211,7 @@ Generate a new identity using a 24-word seed phrase The seed phrase can be store
 
   Default value: `false`
 
-- `--overwrite` — Overwrite existing identity if it already exists
+- `--overwrite` — Overwrite existing identity if it already exists. When combined with --secure-store, also replaces the existing Secure Store entry
 
 ###### **Options (Global):**
 

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

@@ -74,4 +74,28 @@ async fn secure_store_key_management() {
 
     let new_account = client.get_account(&new_address).await.unwrap();
     assert_eq!(new_account.balance, starting_balance);
+
+    // generating the same key again without --overwrite must fail
+    sandbox
+        .new_assert_cmd("keys")
+        .args(["generate", secure_key_name, "--secure-store"])
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("already exists"));
+
+    // generating the same key again with --overwrite must succeed and replace the entry
+    sandbox
+        .new_assert_cmd("keys")
+        .args(["generate", secure_key_name, "--secure-store", "--overwrite"])
+        .assert()
+        .success();
+
+    // the address should still be a valid public key (new key was written)
+    let new_secure_store_address = sandbox
+        .new_assert_cmd("keys")
+        .args(["address", secure_key_name])
+        .assert()
+        .success()
+        .stdout_as_str();
+    assert!(new_secure_store_address.starts_with('G'));
 }

cmd/soroban-cli/src/commands/keys/add.rs

@@ -57,7 +57,8 @@ pub struct Cmd {
     #[arg(long, conflicts_with = "seed_phrase", conflicts_with = "secret_key")]
     pub public_key: Option<String>,
 
-    /// Overwrite existing identity if it already exists.
+    /// Overwrite existing identity if it already exists. When combined with
+    /// --secure-store, also replaces the existing Secure Store entry.
     #[arg(long)]
     pub overwrite: bool,
 }
@@ -108,7 +109,8 @@ impl Cmd {
 
             let seed_phrase: SeedPhrase = secret_key.parse()?;
 
-            let secret = secure_store::save_secret(print, &self.name, &seed_phrase)?;
+            let secret =
+                secure_store::save_secret(print, &self.name, &seed_phrase, self.overwrite)?;
             Ok(secret.parse()?)
         } else {
             let prompt = "Type a secret key or 12/24 word seed phrase:";

cmd/soroban-cli/src/commands/keys/generate.rs

@@ -61,7 +61,8 @@ pub struct Cmd {
     #[arg(long, default_value = "false")]
     pub fund: bool,
 
-    /// Overwrite existing identity if it already exists.
+    /// Overwrite existing identity if it already exists. When combined with
+    /// --secure-store, also replaces the existing Secure Store entry.
     #[arg(long)]
     pub overwrite: bool,
 }
@@ -115,7 +116,8 @@ impl Cmd {
     fn secret(&self, print: &Print) -> Result<Secret, Error> {
         let seed_phrase = self.seed_phrase()?;
         if self.secure_store {
-            let secret = secure_store::save_secret(print, &self.name, &seed_phrase)?;
+            let secret =
+                secure_store::save_secret(print, &self.name, &seed_phrase, self.overwrite)?;
             Ok(secret.parse()?)
         } else if self.as_secret {
             let secret: Secret = seed_phrase.into();

cmd/soroban-cli/src/signer/keyring.rs

@@ -15,6 +15,9 @@ pub enum Error {
 
     #[error("Secure Store keys are not allowed: additional-libs feature must be enabled")]
     FeatureNotEnabled,
+
+    #[error("An entry for '{0}' already exists in the secure store")]
+    EntryAlreadyExists(String),
 }
 
 pub struct StellarEntry {
@@ -31,19 +34,32 @@ impl StellarEntry {
         })
     }
 
-    pub fn write(&self, seed_phrase: SeedPhrase, print: &Print) -> Result<(), Error> {
-        if let Ok(key) = self.get_public_key(None) {
+    pub fn write(
+        &self,
+        seed_phrase: SeedPhrase,
+        print: &Print,
+        overwrite: bool,
+    ) -> Result<(), Error> {
+        let exists = match self.keyring.get_password() {
+            Ok(_) => true,
+            Err(keyring::Error::NoEntry) => false,
+            Err(e) => return Err(Error::Keyring(e)),
+        };
+        if exists {
+            if !overwrite {
+                return Err(Error::EntryAlreadyExists(self.name.clone()));
+            }
             print.warnln(format!(
-                "A key for {0} already exists in your operating system's secure store: {1}",
-                self.name, key
+                "Overwriting existing key in secure store: {0}",
+                self.name
             ));
         } else {
             print.infoln(format!(
                 "Saving a new key to your operating system's secure store: {0}",
                 self.name
             ));
-            self.set_seed_phrase(seed_phrase)?;
         }
+        self.set_seed_phrase(seed_phrase)?;
         Ok(())
     }
 
@@ -184,6 +200,58 @@ mod test {
         assert!(sign_tx_env_result.is_ok());
     }
 
+    #[test]
+    fn write_with_overwrite_updates_existing_entry() {
+        set_default_credential_builder(mock::default_credential_builder());
+
+        let seed_phrase_1 =
+            crate::config::secret::seed_phrase_from_seed(Some("0123456789abcdef")).unwrap();
+        let seed_phrase_2 =
+            crate::config::secret::seed_phrase_from_seed(Some("fedcba9876543210")).unwrap();
+
+        let pubkey_1 = seed_phrase_1.from_path_index(0, None).unwrap().public().0;
+        let pubkey_2 = seed_phrase_2.from_path_index(0, None).unwrap().public().0;
+        assert_ne!(pubkey_1, pubkey_2, "test seeds must produce different keys");
+
+        let entry = StellarEntry::new("test-overwrite").unwrap();
+        let print = print::Print::new(true);
+
+        entry.write(seed_phrase_1, &print, false).unwrap();
+        assert_eq!(entry.get_public_key(None).unwrap().0, pubkey_1);
+
+        // overwrite=true must replace the entry with the new seed phrase
+        entry.write(seed_phrase_2, &print, true).unwrap();
+        assert_eq!(
+            entry.get_public_key(None).unwrap().0,
+            pubkey_2,
+            "overwrite should have replaced the keyring entry"
+        );
+    }
+
+    #[test]
+    fn write_without_overwrite_errors_on_existing_entry() {
+        set_default_credential_builder(mock::default_credential_builder());
+
+        let seed_phrase_1 =
+            crate::config::secret::seed_phrase_from_seed(Some("0123456789abcdef")).unwrap();
+        let seed_phrase_2 =
+            crate::config::secret::seed_phrase_from_seed(Some("fedcba9876543210")).unwrap();
+
+        let entry = StellarEntry::new("test-no-overwrite").unwrap();
+        let print = print::Print::new(true);
+
+        entry.write(seed_phrase_1, &print, false).unwrap();
+
+        // overwrite=false must fail with EntryAlreadyExists when an entry already exists
+        let err = entry
+            .write(seed_phrase_2, &print, false)
+            .expect_err("write without overwrite should fail on existing entry");
+        assert!(
+            matches!(err, Error::EntryAlreadyExists(_)),
+            "expected EntryAlreadyExists, got {err:?}"
+        );
+    }
+
     #[test]
     fn test_delete_seed_phrase() {
         set_default_credential_builder(mock::default_credential_builder());

cmd/soroban-cli/src/signer/secure_store.rs

@@ -45,12 +45,13 @@ mod secure_store_impl {
         print: &Print,
         entry_name: &str,
         seed_phrase: &SeedPhrase,
+        overwrite: bool,
     ) -> Result<String, Error> {
         // secure_store:org.stellar.cli:<key name>
         let entry_name_with_prefix = format!("{ENTRY_PREFIX}{ENTRY_SERVICE}-{entry_name}");
 
         let entry = StellarEntry::new(&entry_name_with_prefix)?;
-        entry.write(seed_phrase.clone(), print)?;
+        entry.write(seed_phrase.clone(), print, overwrite)?;
 
         Ok(entry_name_with_prefix)
     }
@@ -81,6 +82,7 @@ mod secure_store_impl {
         _print: &Print,
         _entry_name: &str,
         _seed_phrase: &SeedPhrase,
+        _overwrite: bool,
     ) -> Result<String, Error> {
         Err(Error::FeatureNotEnabled)
     }