Automated Audit - Reliability Improvements

[0xRahim]

Reliability & data loss audit

Private key expiry — no, it will never expire

The SecretKeyParamsBuilder in keys.rs has no .expires_at() call. PGP keys support an expiry field, but it's simply never set here. The key has no TTL. This is correct for a local password manager — you don't want your vault to spontaneously lock itself out.

---

Data loss risks — several real ones

1. Forgotten master password = permanent total loss

There is no recovery path. No backup code, no recovery phrase, no secondary unlock method. If the user forgets their master password, the S2K unlock fails, the X25519 scalar can never be derived, and every single encrypted entry is irrecoverable forever. This is the largest single-point failure mode for any user.

The standard mitigation is a recovery key — a randomly generated 128-bit code shown once at vault creation, stored offline by the user, that can unlock the private key independently of the master password. This is what 1Password's Emergency Kit does. In your architecture it would mean wrapping the private key twice: once with Argon2id(master_password) and once with Argon2id(recovery_key), storing both blobs in the vault row.

2. The test suite does not compile and covers the wrong pipeline

This is a reliability finding, not just a code quality one. The tests in pipeline_tests.rs, session_key_tests.rs, and keys_tests.rs were written against an older version of the API. They import functions that no longer exist (decrypt_private_key_with_mpass, encrypt_private_key) and call the functions that do exist with the wrong signatures:

// tests expect this signature (PGP-based, 3 args):
decrypt_session_key_with_private_key(&msg, &plain_sk, "")
//   plain_sk: SignedSecretKey  ↑
// actual production signature (X25519 raw bytes, 2 args):

decrypt_session_key_with_private_key(blob: &[u8], private_key_bytes: &[u8; 32])

// tests expect this signature (ASC string):
encrypt_session_key_with_public_key(&session_key, &pk_asc)
//   pk_asc: String (PGP ASC)  ↑

// actual production signature (raw bytes):
encrypt_session_key_with_public_key(session_key: &[u8; 32], recipient_public_key: &[u8; 32])

These tests cannot compile. The production path in commands.rs is entirely untested. If you broke the ECIES round-trip tomorrow (e.g. changed the HKDF label, altered blob layout), no test would catch it and users would silently write entries they can never read back. This needs to be fixed before any public release.

3. No alias uniqueness constraint in the database

The entries table has no UNIQUE(vault_id, alias) constraint. You can create two entries both named gmail. The get_entry_by_alias query does WHERE alias=?1 with no vault scoping and returns the first match — which could be from a different vault. cred get gmail and cred rm gmail would silently hit whichever row SQLite returns first. Add this to the migration:

CREATE UNIQUE INDEX IF NOT EXISTS idx_entries_vault_alias 
ON entries(vault_id, alias);

4. Encrypted file entries store absolute paths inside encrypted blobs

For EntryType::File, the encrypted file's path on disk (e.g. /home/rahim/.local/share/phrase/uploads/secret.pdf.phrased) is stored as a field inside the secret_data JSON, which is then AES-encrypted. This creates a serious portability problem: if you move the vault to a different machine, rename your home directory, or reinstall the OS with a different username, the stored path resolves to nothing. The display_entry function calls std::fs::read(&enc_path) on the stored path and dies if the file isn't there. The entry exists in the database but is permanently inaccessible.

The fix is to store only the filename (not the absolute path), and always resolve it relative to paths::uploads_dir() at read time.

5. File name collision in uploads silently overwrites

If you store two different files both named report.pdf, encrypted_file_path() produces uploads/report.pdf.phrased for both. The second write silently overwrites the first. The DB entries both point to the same path, so one of them decrypts into the wrong file. Use the entry UUID as the filename prefix to guarantee uniqueness: {entry_id}_{original_name}.phrased.

6. Debug build uses a different database than release

paths.rs switches between phrase-dev/ and phrase/ based on cfg!(debug_assertions). Any data added while running cargo run (debug) is invisible when running the release binary, and vice versa. This isn't a loss risk for production users, but it's a footgun during development that can make you think your data disappeared.

7. No vault backup or export command

The entire vault is one SQLite file plus the uploads/ directory. There is no phrase vault export or phrase backup command, no documentation on where the file lives, and no reminder to back it up. A single rm -rf ~/.local/share/phrase/ or a disk failure loses everything. Even a simple phrase backup <destination> that copies the DB and uploads directory would significantly reduce real-world loss risk.

8. cred edit destroys the old entry with no confirmation

edit calls update_entry() which does a SET alias=..., secret_data=... replacement with no undo. If the user types the wrong password into the edit form, the old ciphertext is gone. At minimum, edit should ask for the master password first and display the current (decrypted) values so the user can decide what to keep.

---

Summary

Risk	Impact	Fix
Forgotten master password	All data permanently lost	Recovery key (second blob wrapped with offline code)
Test suite doesn't compile, wrong API	Bugs in crypto path go undetected	Rewrite tests against actual commands.rs pipeline
No alias uniqueness constraint	Silent wrong-entry hits on get/rm	Add UNIQUE(vault_id, alias) index
File entries store absolute paths	Inaccessible after OS reinstall or home dir change	Store filename only, resolve from uploads_dir() at runtime
File name collision in uploads	Silent overwrite	Prefix with entry UUID
No backup/export command	Total loss on disk failure	phrase backup copying DB + uploads
edit with no confirmation	Accidental permanent overwrite	Show current values, require master password

The private key will never expire, so that specific concern is fine. The real risks are almost all around data durability and recovery — which is the other half of security that password managers often get wrong.

[0xRahim]

Only important fixes will be pushed in next update