signing-keys create: reject a duplicate name before generating

[mobileoverlord]

What

avocado signing-keys create <name> checked for a name collision only after the key had been generated. Re-running it with an existing name ran the full generation first and then failed:

• --algorithm rsa2048/rsa4096: shelled out to openssl and wrote a .key/.crt pair into the signing-keys directory before bailing, leaving the orphaned pair behind.
• --pkcs11-device --generate: created a key on the hardware token, consuming a slot, before bailing.
• hmac-sha256 and ed25519: wrote key material to disk before bailing.

How

Move the check to the top of execute(), before any generation, when a name is given. When no name is given it defaults to the key id, which is not known until after generation; that case is still covered by registry.add_key.

import already checked before doing any work, so it needed no change.

Results

New tests/signing_keys_create.rs: creates a key, then re-runs create with the same name and --algorithm rsa2048, and asserts the error is raised and the keys directory is unchanged. Verified the test fails against the previous behavior. cargo fmt --check and cargo clippy --all-targets clean.

[Copilot]

🟡 Changes recommended

The new test compares unsorted read_dir results, which can be flaky because directory iteration order is not guaranteed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR prevents avocado signing-keys create <name> from performing any key generation work when <name> already exists in the signing-keys registry, avoiding orphaned files and unintended PKCS#11 key creation.

Changes:

• Move duplicate-name detection to the start of SigningKeysCreateCommand::execute() when an explicit name is provided.
• Add an integration test ensuring a second create with the same name fails before RSA/openssl generation and leaves the keys directory unchanged.

File summaries

File	Description
src/commands/signing_keys/create.rs	Checks for duplicate explicit names before any key generation occurs.
tests/signing_keys_create.rs	Adds a regression test that verifies duplicate-name failure doesn’t leave new files behind.

Review details

Suppressed comments (1)

tests/signing_keys_create.rs:40

• Same ordering issue here: read_dir iteration order is filesystem-dependent, so the Vec may not match after_first even when no files were added/removed. Sort before comparing.

    let after_second: Vec<_> = std::fs::read_dir(dir.path())
        .unwrap()
        .map(|e| e.unwrap().file_name())
        .collect();
    assert_eq!(after_first, after_second, "second create left files behind");

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.