Skip to content

Prevent stale keyring entry when overwriting a key.#2505

Open
fnando wants to merge 2 commits intomainfrom
fix-2492
Open

Prevent stale keyring entry when overwriting a key.#2505
fnando wants to merge 2 commits intomainfrom
fix-2492

Conversation

@fnando
Copy link
Copy Markdown
Member

@fnando fnando commented Apr 20, 2026

What

Prevent stale keyring entry when overwriting a key

Why

Close #2492

Known limitations

N/A

Copilot AI review requested due to automatic review settings April 20, 2026 23:39
@fnando fnando requested review from mootz12 and removed request for Copilot April 20, 2026 23:39
@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Apr 20, 2026
@fnando fnando self-assigned this Apr 20, 2026
@fnando fnando moved this from Backlog (Not Ready) to Needs Review in DevX Apr 20, 2026
@fnando fnando requested review from Copilot April 20, 2026 23:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #2492 by ensuring Secure Store (keyring) entries are actually updated when a key is overwritten, preventing the CLI from continuing to reference stale key material after a successful-looking overwrite.

Changes:

  • Thread an overwrite: bool flag through secure_store::save_secret into StellarEntry::write.
  • Change keyring write behavior to error on existing entries unless overwrite is explicitly allowed, and to always persist the new seed phrase when overwriting.
  • Add unit tests covering overwrite and non-overwrite behavior for keyring writes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
cmd/soroban-cli/src/signer/secure_store.rs Adds overwrite parameter and forwards it to keyring entry writes.
cmd/soroban-cli/src/signer/keyring.rs Implements overwrite-aware keyring writes and adds tests for the new semantics.
cmd/soroban-cli/src/commands/keys/generate.rs Passes CLI --overwrite through to Secure Store save path.
cmd/soroban-cli/src/commands/keys/add.rs Passes CLI --overwrite through to Secure Store save path.

Comment thread cmd/soroban-cli/src/signer/keyring.rs Outdated
Comment thread cmd/soroban-cli/src/signer/keyring.rs Outdated
Comment thread cmd/soroban-cli/src/commands/keys/generate.rs
Comment thread cmd/soroban-cli/src/commands/keys/add.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

Secure-store overwrite keeps a stale keyring entry bound to the alias

2 participants