Feature/improve password security

[abd0-omar]

Description

Add compile time protection against password leakage by using Password type that wraps SecretString from the secrecy crate. This ensures passwords cannot be accidentally logged.

Related Issues

Partially addresses #282 - Add protection for passwords to prevent accidental loggging

Changes Made

• Added secrecy crate to prevent leaking passwords
• Created Password type wrapper in libs/shared/src/models/password.rs
• Enforces minimum 8 character constraint and can't be empty using new type pattern
• Updated String passwords with Password type

---

• Updated generate_password() to return Result<Password, PasswordGenerationError>
• Refactored generate_password() to use byte slices instead of String for better performance
• Added conflict detection with retry mechanism (max 10 attempts), although it's very rare for 8 characters password to conflict
• Added proper error types: TooShort, Conflict, NonUTF8

---

• Renamed parameter no_symbols to include_symbols

---

• added docs in README.md

---

• Upgraded schemars from 0.8.22 to 1.1.0 (with chrono04 feature, even though chrono is not used)

Screenshots

testing creating a password less than 8 characters

password redaction int the TUI

passwords in the secret.json

Breaking Changes

Minor breaking changes:

• generate_password() now returns Result<Password, PasswordGenerationError> instead of String
• Password fields in structs changed from Option<String> to Option<Password>
• redact_password() changed to accept &Password instead of &str
• redact_and_store_password() accepts Password instead of &str

if this got approved I'll work on the rest of the #282 issue

[shehab299]

@abd0-omar thank u

under review

[ahmedhesham6]

Good work on adding compile-time protection against password leakage using the secrecy crate. The Password newtype pattern is clean and the performance improvements using byte slices are nice.

Issues to Address

1. Collision detection logic only shuffles, doesn't regenerate

for attempt in 0..MAX_RETRIES {
    if redaction_map.values().any(|v| v.as_bytes() == password) {
        password.shuffle(&mut rng);  // Just shuffles same chars
        continue;
    }
    // ...
}

Shuffling the same characters is unlikely to resolve a collision since you're checking exact byte match. Consider regenerating the entire password with fresh random characters instead.

2. Missing validation in Password deserialization

The Password type validates length in new(), but serde deserialization bypasses this:

#[derive(Deserialize)]
pub struct Password(SecretString);  // Deserializes directly

A password shorter than 8 chars could be deserialized from JSON. Consider adding a custom deserializer:

impl<'de> Deserialize<'de> for Password {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where D: Deserializer<'de> {
        let s = String::deserialize(deserializer)?;
        Password::new(s).map_err(serde::de::Error::custom)
    }
}

3. Potential underflow in generate_password

If length is less than the number of required characters (4 with symbols, 3 without), this could underflow:

password.extend((0..(length - password.len())).map(...));

The minimum is enforced in Password::new(), but that happens after generation. Consider validating early:

pub fn generate_password(length: usize, ...) -> Result<Password, PasswordGenerationError> {
    if length < 8 {
        return Err(PasswordGenerationError::TooShort);
    }
    // ...
}

4. redact_password() behavioral change

The old function replaced password occurrences within content. The new function just returns a redaction key. This is fine, but make sure all callers are updated to handle this change correctly.

---

Thanks for the thorough work on this! The PR description and screenshots are helpful.

[abd0-omar]

Thanks for the detailed review @ahmedhesham6 !

the current fixes:

• Collision detection logic: Fixed in c0b44a1 and 8438d6d. Instead of shuffling, I've refactored the population logic into its own function.
• Missing validation in Password deserialization: Fixed in 661e30c.
  Potential underflow in generate_password: Fixed in c412def. Added early validation.
• redact_password behavioral change: I've kept the function as it is to just redact a password not a password within content as it was before this changes

Please let me know for any changes!

[ahmedhesham6]

@abd0-omar can you sync with beta branch

[abd0-omar]

@ahmedhesham6 I've merged "actor-model", which is already merged with branch "beta", into the current branch