Skip to content

bug: clipwallet vault-rotate isn't atomic—an interrupted key rotation can leave the vault encrypted with mixed keys, causing permanent data loss. #78

Description

@divyanshim27

🐛 Critical Data Safety Issue

clipwallet vault-rotate is documented as: "Rotate the encryption key (re-encrypts all entries)". A key rotation operation must:

  1. Generate a new AES-256-GCM key
  2. For each .vlt file: decrypt with old key → encrypt with new key → write new ciphertext
  3. Store the new key in macOS Keychain
  4. Delete the old key from Keychain

If this is implemented as a simple sequential loop (decrypt old → write new → next file), an interruption at step 2 leaves the vault in a split state: some files use the old key, some use the new key. After the interruption:

  • The Keychain may have the new key (if step 3 ran) → old-key files fail to decrypt
  • The Keychain may have the old key (if step 3 didn't run yet) → new-key files fail to decrypt

Either way, some clipboard entries are permanently unrecoverable.

Proposed Fix

Implement a two-phase commit pattern for key rotation:

// src/vault.rs

pub fn rotate_key(vault_dir: &Path, keychain: &dyn KeyStore) -> anyhow::Result<()> {
    let old_key = keychain.load_key()?;
    let new_key = generate_aes256_key();

    let vlt_files: Vec<_> = std::fs::read_dir(vault_dir)?
        .filter_map(|e| e.ok())
        .filter(|e| e.path().extension().map_or(false, |ext| ext == "vlt"))
        .collect();

    // Phase 1: Write ALL re-encrypted files to a staging directory
    let staging_dir = vault_dir.join(".rotate_staging");
    std::fs::create_dir_all(&staging_dir)?;

    for entry in &vlt_files {
        let path = entry.path();
        let ciphertext = std::fs::read(&path)?;
        let plaintext = aes_gcm_decrypt(&ciphertext, &old_key)
            .map_err(|e| anyhow::anyhow!("Failed to decrypt {:?} with old key: {e}", path))?;

        let new_ciphertext = aes_gcm_encrypt(&plaintext, &new_key)?;
        let staging_path = staging_dir.join(path.file_name().unwrap());
        std::fs::write(&staging_path, &new_ciphertext)?;
    }

    // Phase 2: All re-encryption succeeded — atomically swap staging into vault
    // Store new key in Keychain BEFORE moving files (recovery path: if move fails, use new key)
    keychain.store_key(&new_key)?;

    for entry in &vlt_files {
        let fname = entry.path().file_name().unwrap().to_owned();
        let staging_path = staging_dir.join(&fname);
        let final_path = vault_dir.join(&fname);
        std::fs::rename(&staging_path, &final_path)?; // atomic on same filesystem
    }

    // Phase 3: Cleanup staging directory
    std::fs::remove_dir_all(&staging_dir)?;

    // Revoke old key from Keychain (best-effort — not fatal if this fails)
    let _ = keychain.delete_old_key();

    println!("✅ Key rotation complete. {} entries re-encrypted.", vlt_files.len());
    Ok(())
}

Recovery on interrupted rotation: On startup, if ~/.clipwallet/vault/.rotate_staging/ exists:

  • If Keychain has the new key: complete the rename pass (Phase 2 was interrupted mid-rename)
  • If Keychain has the old key: delete staging directory (Phase 1 was interrupted — no data lost)
// src/vault.rs — add to init/startup
pub fn recover_interrupted_rotation(vault_dir: &Path, keychain: &dyn KeyStore) {
    let staging_dir = vault_dir.join(".rotate_staging");
    if staging_dir.exists() {
        eprintln!("[clipwallet] Detected interrupted key rotation. Attempting recovery...");
        // ... recovery logic as described
    }
}

Files to Modify

File Change
src/vault.rs (or equivalent) Implement two-phase commit rotation with staging directory
src/main.rs or init path Call recover_interrupted_rotation() at startup
SECURITY.md Document key rotation safety guarantees
README.md Update vault-rotate command description

Suggested labels: bug, data-safety, encryption, rust, critical

I would like to work on this. Could you please assign it to me?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions