- 2026-04-18 initial audit verification (all fixed except H8) - 2026-05-01 audit with 8 new findings (B1-B4, I1-I6) - Plan 4: Security Blocker Fixes implementation plan Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1036 lines
28 KiB
Markdown
1036 lines
28 KiB
Markdown
# Plan 4: Security Blocker Fixes
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Clear all 4 audit blockers (B1-B4) + important items (I1-I6) + HOTP bug before v0.3.0
|
|
|
|
**Architecture:** Fix security-theater device keys by removal (not implementation), add missing validations/normalizations, and harden path handling. HOTP is disabled with a clear error since counter persistence requires vault save machinery that doesn't exist.
|
|
|
|
**Tech Stack:** Rust (relicario-core, relicario-cli, relicario-wasm), TypeScript (extension)
|
|
|
|
---
|
|
|
|
## File Structure
|
|
|
|
### Core changes
|
|
- `crates/relicario-core/src/backup.rs` — add NFC normalization (B2)
|
|
- `crates/relicario-core/src/ids.rs` — expand AttachmentId to 128 bits (I2)
|
|
- `crates/relicario-core/src/item_types/totp.rs` — reject HOTP with error (I6)
|
|
- `crates/relicario-core/src/error.rs` — add HotpNotSupported variant
|
|
|
|
### CLI changes
|
|
- `crates/relicario-cli/src/main.rs`:
|
|
- Remove Device subcommand/enum/handler (B1)
|
|
- Remove devices.json from init (B1)
|
|
- Gate test env vars with #[cfg(test)] (B3)
|
|
- Validate IDs on restore (B4)
|
|
- Sanitize titles in commit messages (I1)
|
|
- Enforce per-vault attachment cap (I3)
|
|
- `crates/relicario-cli/src/helpers.rs` — add `sanitize_for_commit()` (I1)
|
|
- `crates/relicario-cli/tests/basic_flows.rs` — remove devices.json assertion (B1)
|
|
|
|
### WASM changes
|
|
- `crates/relicario-wasm/src/lib.rs` — remove `generate_device_keypair` (B1/I5)
|
|
|
|
### Extension changes (B1)
|
|
- Remove `extension/src/popup/components/devices.ts`
|
|
- Remove `extension/src/popup/components/__tests__/devices.test.ts`
|
|
- Remove `extension/src/service-worker/devices.ts`
|
|
- Remove `extension/src/service-worker/__tests__/devices.test.ts`
|
|
- Update `extension/src/shared/messages.ts` — remove device message types
|
|
- Update `extension/src/shared/types.ts` — remove Device type
|
|
- Update `extension/src/popup/popup.ts` — remove devices route
|
|
- Update `extension/src/popup/components/settings.ts` — remove devices link
|
|
- Update `extension/src/service-worker/router/popup-only.ts` — remove device handlers
|
|
- Update `extension/src/wasm.d.ts` — remove generate_device_keypair
|
|
|
|
### Documentation (I4)
|
|
- `docs/SECURITY.md` — document manifest integrity model
|
|
|
|
---
|
|
|
|
## Task 1: Remove device key system from CLI (B1)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-cli/src/main.rs:165-168, 315-320, 388, 500, 518, 2151-2221`
|
|
- Modify: `crates/relicario-cli/tests/basic_flows.rs:11`
|
|
|
|
- [ ] **Step 1: Write test that init does NOT create devices.json**
|
|
|
|
```rust
|
|
// In crates/relicario-cli/tests/basic_flows.rs, modify init_creates_expected_layout:
|
|
#[test]
|
|
fn init_creates_expected_layout() {
|
|
let v = TestVault::init();
|
|
assert!(v.path().join(".relicario/salt").exists());
|
|
assert!(v.path().join(".relicario/params.json").exists());
|
|
// devices.json removed — device key system was security theater
|
|
assert!(!v.path().join(".relicario/devices.json").exists());
|
|
assert!(v.path().join("manifest.enc").exists());
|
|
assert!(v.path().join("settings.enc").exists());
|
|
assert!(v.path().join("reference.jpg").exists());
|
|
assert!(v.path().join(".gitignore").exists());
|
|
assert!(v.path().join(".git").is_dir());
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Run test to verify it fails**
|
|
|
|
Run: `cargo test -p relicario-cli --test basic_flows init_creates_expected_layout`
|
|
Expected: FAIL — devices.json still exists
|
|
|
|
- [ ] **Step 3: Remove DeviceAction enum**
|
|
|
|
In `main.rs`, delete lines 315-320:
|
|
|
|
```rust
|
|
// DELETE THIS BLOCK:
|
|
#[derive(Subcommand)]
|
|
enum DeviceAction {
|
|
Add { #[arg(long)] name: String },
|
|
List,
|
|
Revoke { name: String },
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Remove Device variant from Commands enum**
|
|
|
|
In `main.rs`, delete lines 164-168:
|
|
|
|
```rust
|
|
// DELETE THIS BLOCK:
|
|
/// Device management.
|
|
Device {
|
|
#[command(subcommand)]
|
|
action: DeviceAction,
|
|
},
|
|
```
|
|
|
|
- [ ] **Step 5: Remove Device match arm**
|
|
|
|
In `main.rs`, delete line 388:
|
|
|
|
```rust
|
|
// DELETE THIS LINE:
|
|
Commands::Device { action } => cmd_device(action),
|
|
```
|
|
|
|
- [ ] **Step 6: Remove devices.json creation in cmd_init**
|
|
|
|
In `main.rs`, delete line 500:
|
|
|
|
```rust
|
|
// DELETE THIS LINE:
|
|
fs::write(relicario_dir.join("devices.json"), b"[]")?;
|
|
```
|
|
|
|
- [ ] **Step 7: Remove devices.json from git add in cmd_init**
|
|
|
|
In `main.rs` line 517-519, change:
|
|
|
|
```rust
|
|
// FROM:
|
|
let _ = crate::helpers::git_command(&root, &[
|
|
"add", ".gitignore", ".relicario/params.json", ".relicario/devices.json",
|
|
".relicario/salt", "manifest.enc", "settings.enc",
|
|
]).status()?;
|
|
|
|
// TO:
|
|
let _ = crate::helpers::git_command(&root, &[
|
|
"add", ".gitignore", ".relicario/params.json",
|
|
".relicario/salt", "manifest.enc", "settings.enc",
|
|
]).status()?;
|
|
```
|
|
|
|
- [ ] **Step 8: Remove cmd_device function**
|
|
|
|
Delete the entire `cmd_device` function (lines 2151-2221).
|
|
|
|
- [ ] **Step 9: Run test to verify it passes**
|
|
|
|
Run: `cargo test -p relicario-cli --test basic_flows init_creates_expected_layout`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 10: Run full CLI test suite**
|
|
|
|
Run: `cargo test -p relicario-cli`
|
|
Expected: All tests pass (some may need adjustment if they reference devices)
|
|
|
|
- [ ] **Step 11: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-cli/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(cli): remove device key system (audit B1)
|
|
|
|
The device add/list/revoke commands generated and stored ed25519 keys
|
|
but no code ever signed commits or verified signatures. This gave users
|
|
a false sense of security around device revocation.
|
|
|
|
Removed: Device subcommand, DeviceAction enum, cmd_device handler,
|
|
devices.json creation. Access control remains SSH-key-level only.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 2: Remove device key system from WASM (B1/I5)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-wasm/src/lib.rs:212-227`
|
|
|
|
- [ ] **Step 1: Remove generate_device_keypair function**
|
|
|
|
Delete lines 212-227 in `lib.rs`:
|
|
|
|
```rust
|
|
// DELETE THIS BLOCK:
|
|
/// Generate an ed25519 keypair for device registration.
|
|
/// Returns JSON: { "public_key_hex": "...", "private_key_base64": "..." }
|
|
#[wasm_bindgen]
|
|
pub fn generate_device_keypair() -> Result<JsValue, JsError> {
|
|
let mut rng = rand::thread_rng();
|
|
let signing_key = SigningKey::generate(&mut rng);
|
|
let verifying_key = signing_key.verifying_key();
|
|
|
|
let public_hex = hex::encode(verifying_key.as_bytes());
|
|
let private_b64 = base64::engine::general_purpose::STANDARD.encode(signing_key.as_bytes());
|
|
|
|
js_value_for(&serde_json::json!({
|
|
"public_key_hex": public_hex,
|
|
"private_key_base64": private_b64,
|
|
}))
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Remove ed25519_dalek import if no longer needed**
|
|
|
|
Check if `SigningKey` is used elsewhere. If not, remove the import.
|
|
|
|
- [ ] **Step 3: Build WASM to verify compilation**
|
|
|
|
Run: `cargo build -p relicario-wasm --target wasm32-unknown-unknown`
|
|
Expected: Build succeeds
|
|
|
|
- [ ] **Step 4: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-wasm/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(wasm): remove generate_device_keypair (audit B1/I5)
|
|
|
|
Private key was being returned to JS heap with no Zeroizing protection.
|
|
The entire device key system is removed as security theater.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 3: Remove device UI from extension (B1)
|
|
|
|
**Files:**
|
|
- Delete: `extension/src/popup/components/devices.ts`
|
|
- Delete: `extension/src/popup/components/__tests__/devices.test.ts`
|
|
- Delete: `extension/src/service-worker/devices.ts`
|
|
- Delete: `extension/src/service-worker/__tests__/devices.test.ts`
|
|
- Modify: `extension/src/shared/messages.ts`
|
|
- Modify: `extension/src/shared/types.ts`
|
|
- Modify: `extension/src/popup/popup.ts`
|
|
- Modify: `extension/src/popup/components/settings.ts`
|
|
- Modify: `extension/src/service-worker/router/popup-only.ts`
|
|
- Modify: `extension/src/wasm.d.ts`
|
|
|
|
- [ ] **Step 1: Delete device component files**
|
|
|
|
```bash
|
|
rm extension/src/popup/components/devices.ts
|
|
rm extension/src/popup/components/__tests__/devices.test.ts
|
|
rm extension/src/service-worker/devices.ts
|
|
rm extension/src/service-worker/__tests__/devices.test.ts
|
|
```
|
|
|
|
- [ ] **Step 2: Remove Device type from types.ts**
|
|
|
|
Find and remove the `Device` interface in `extension/src/shared/types.ts`.
|
|
|
|
- [ ] **Step 3: Remove device message types from messages.ts**
|
|
|
|
Find and remove `list_devices`, `register_device`, `revoke_device` message types.
|
|
|
|
- [ ] **Step 4: Remove devices route from popup.ts**
|
|
|
|
Remove the devices case from the route switch and the import.
|
|
|
|
- [ ] **Step 5: Remove devices link from settings.ts**
|
|
|
|
Find and remove the "Devices" button/link in the settings view.
|
|
|
|
- [ ] **Step 6: Remove device handlers from popup-only.ts**
|
|
|
|
Remove the `list_devices`, `register_device`, `revoke_device` handlers.
|
|
|
|
- [ ] **Step 7: Remove generate_device_keypair from wasm.d.ts**
|
|
|
|
Delete line 64 in `extension/src/wasm.d.ts`:
|
|
|
|
```typescript
|
|
// DELETE THIS LINE:
|
|
export function generate_device_keypair(): { public_key_hex: string; private_key_base64: string };
|
|
```
|
|
|
|
- [ ] **Step 8: Run TypeScript check**
|
|
|
|
Run: `cd extension && npm run typecheck`
|
|
Expected: No errors
|
|
|
|
- [ ] **Step 9: Run extension tests**
|
|
|
|
Run: `cd extension && npm test`
|
|
Expected: All tests pass (device tests are deleted)
|
|
|
|
- [ ] **Step 10: Commit**
|
|
|
|
```bash
|
|
git add extension/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(extension): remove device UI (audit B1)
|
|
|
|
Device registration/revocation provided no real security since the
|
|
underlying signing infrastructure was never implemented. Removed all
|
|
device-related components, message handlers, and type definitions.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 4: Fix backup KDF NFC normalization (B2)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-core/src/backup.rs:303-312`
|
|
- Create test in: `crates/relicario-core/tests/backup.rs` (or add to existing)
|
|
|
|
- [ ] **Step 1: Write failing test for NFC normalization**
|
|
|
|
```rust
|
|
// In crates/relicario-core/tests/backup.rs or a new test file:
|
|
#[test]
|
|
fn backup_kdf_normalizes_nfc() {
|
|
use relicario_core::backup::{pack, unpack};
|
|
|
|
// "café" in NFD (decomposed: e + combining acute) vs NFC (composed: é)
|
|
let nfd_passphrase = "caf\u{0065}\u{0301}"; // e + combining accent
|
|
let nfc_passphrase = "caf\u{00E9}"; // precomposed é
|
|
|
|
// Create a minimal backup with NFD passphrase
|
|
let input = BackupInput {
|
|
salt: &[0u8; 32],
|
|
params_json: b"{}",
|
|
devices_json: b"[]",
|
|
manifest_enc: &[1, 2, 3],
|
|
settings_enc: &[4, 5, 6],
|
|
items: vec![],
|
|
attachments: vec![],
|
|
};
|
|
|
|
let packed = pack(input, nfd_passphrase, false, false).unwrap();
|
|
|
|
// Unpack with NFC passphrase — should work because both normalize to same
|
|
let unpacked = unpack(&packed, nfc_passphrase).unwrap();
|
|
assert_eq!(unpacked.manifest_enc, vec![1, 2, 3]);
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Run test to verify it fails**
|
|
|
|
Run: `cargo test -p relicario-core backup_kdf_normalizes_nfc`
|
|
Expected: FAIL — NFD and NFC produce different keys
|
|
|
|
- [ ] **Step 3: Add NFC normalization to derive_backup_key**
|
|
|
|
In `backup.rs`, modify `derive_backup_key`:
|
|
|
|
```rust
|
|
fn derive_backup_key(passphrase: &[u8], salt: &[u8]) -> Result<Zeroizing<[u8; 32]>> {
|
|
use unicode_normalization::UnicodeNormalization;
|
|
|
|
// NFC normalize passphrase (same as derive_master_key in crypto.rs)
|
|
let nfc_passphrase: Vec<u8> = match std::str::from_utf8(passphrase) {
|
|
Ok(s) => s.nfc().collect::<String>().into_bytes(),
|
|
Err(_) => passphrase.to_vec(),
|
|
};
|
|
|
|
let params = Params::new(ARGON2_M_KIB, ARGON2_T, ARGON2_P, Some(32))
|
|
.map_err(|e| RelicarioError::Kdf(format!("argon2 params: {e}")))?;
|
|
let argon = Argon2::new(Algorithm::Argon2id, Version::V0x13, params);
|
|
let mut key = Zeroizing::new([0u8; 32]);
|
|
argon
|
|
.hash_password_into(&nfc_passphrase, salt, key.as_mut_slice())
|
|
.map_err(|e| RelicarioError::Kdf(format!("argon2 hash: {e}")))?;
|
|
Ok(key)
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Run test to verify it passes**
|
|
|
|
Run: `cargo test -p relicario-core backup_kdf_normalizes_nfc`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-core/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(core): NFC normalize backup passphrase (audit B2)
|
|
|
|
Backup KDF was passing raw passphrase bytes to Argon2id without NFC
|
|
normalization, causing cross-platform restore failures for non-ASCII
|
|
passphrases (macOS NFD vs Linux NFC).
|
|
|
|
Now matches derive_master_key behavior from crypto.rs.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 5: Gate test env vars with #[cfg(test)] (B3)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-cli/src/main.rs:417-450, 1425-1430, 1594`
|
|
|
|
- [ ] **Step 1: Identify all test env var locations**
|
|
|
|
Locations found:
|
|
- Lines 417-423: `RELICARIO_TEST_ITEM_SECRET`
|
|
- Lines 443-450: `RELICARIO_TEST_PASSPHRASE`
|
|
- Lines 1425-1430: `RELICARIO_TEST_BACKUP_PASSPHRASE`
|
|
- Line 1594: `RELICARIO_TEST_BACKUP_PASSPHRASE`
|
|
|
|
- [ ] **Step 2: Wrap prompt_item_secret test bypass**
|
|
|
|
```rust
|
|
// Lines 417-423, change:
|
|
fn prompt_item_secret(prompt: &str) -> Result<Zeroizing<String>> {
|
|
#[cfg(test)]
|
|
if let Ok(s) = std::env::var("RELICARIO_TEST_ITEM_SECRET") {
|
|
return Ok(Zeroizing::new(s));
|
|
}
|
|
|
|
let pass = rpassword::prompt_password(prompt)
|
|
.context("failed to read secret")?;
|
|
Ok(Zeroizing::new(pass))
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 3: Wrap prompt_passphrase test bypass**
|
|
|
|
```rust
|
|
// Lines 443-450, change:
|
|
fn prompt_passphrase(prompt: &str, confirm: bool) -> Result<Zeroizing<String>> {
|
|
#[cfg(test)]
|
|
let passphrase = if let Ok(p) = std::env::var("RELICARIO_TEST_PASSPHRASE") {
|
|
Zeroizing::new(p)
|
|
} else {
|
|
Zeroizing::new(rpassword::prompt_password(prompt).context("failed to read passphrase")?)
|
|
};
|
|
|
|
#[cfg(not(test))]
|
|
let passphrase = Zeroizing::new(rpassword::prompt_password(prompt).context("failed to read passphrase")?);
|
|
|
|
#[cfg(test)]
|
|
let confirm_skip = std::env::var_os("RELICARIO_TEST_PASSPHRASE").is_some();
|
|
#[cfg(not(test))]
|
|
let confirm_skip = false;
|
|
|
|
if confirm && !confirm_skip {
|
|
// ... existing confirm logic
|
|
}
|
|
Ok(passphrase)
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Wrap backup passphrase test bypass (export)**
|
|
|
|
Apply same pattern to lines 1425-1430.
|
|
|
|
- [ ] **Step 5: Wrap backup passphrase test bypass (restore)**
|
|
|
|
Apply same pattern to line 1594.
|
|
|
|
- [ ] **Step 6: Build release binary and verify env vars are not present**
|
|
|
|
Run: `cargo build -p relicario-cli --release && strings target/release/relicario | grep RELICARIO_TEST`
|
|
Expected: No output (strings not present in release binary)
|
|
|
|
- [ ] **Step 7: Run tests to verify they still work**
|
|
|
|
Run: `cargo test -p relicario-cli`
|
|
Expected: All tests pass
|
|
|
|
- [ ] **Step 8: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-cli/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(cli): gate test env vars with #[cfg(test)] (audit B3)
|
|
|
|
RELICARIO_TEST_PASSPHRASE and friends were checked in production code,
|
|
exposing the passphrase via /proc/<pid>/environ and shell history.
|
|
|
|
Now only compiled into test binaries.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 6: Validate IDs on backup restore (B4)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-cli/src/main.rs:1619-1626`
|
|
- Modify: `crates/relicario-core/src/ids.rs` (add validation function)
|
|
|
|
- [ ] **Step 1: Write failing test for path traversal**
|
|
|
|
```rust
|
|
// In crates/relicario-cli/tests/attachments.rs or backup tests:
|
|
#[test]
|
|
fn restore_rejects_traversal_ids() {
|
|
// Create a backup with malicious item ID
|
|
// Attempt restore
|
|
// Expect error, not file write outside target
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Add is_valid_id function to ids.rs**
|
|
|
|
```rust
|
|
impl ItemId {
|
|
/// Returns true if this ID is safe for use in filesystem paths.
|
|
/// Valid IDs are 16 hex characters (8 bytes).
|
|
pub fn is_valid(&self) -> bool {
|
|
self.0.len() == 16 && self.0.chars().all(|c| c.is_ascii_hexdigit())
|
|
}
|
|
}
|
|
|
|
impl AttachmentId {
|
|
/// Returns true if this ID is safe for use in filesystem paths.
|
|
/// Valid IDs are 32 hex characters (16 bytes) after I2 fix.
|
|
pub fn is_valid(&self) -> bool {
|
|
self.0.len() == 32 && self.0.chars().all(|c| c.is_ascii_hexdigit())
|
|
}
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 3: Validate IDs during restore**
|
|
|
|
In `main.rs` around line 1619, add validation:
|
|
|
|
```rust
|
|
for item in &unpacked.items {
|
|
if !item.id.is_valid() {
|
|
anyhow::bail!("invalid item ID in backup: {}", item.id.as_str());
|
|
}
|
|
fs::write(target.join("items").join(format!("{}.enc", item.id)), &item.ciphertext)?;
|
|
}
|
|
for a in &unpacked.attachments {
|
|
if !a.item_id.is_valid() || !a.attachment_id.is_valid() {
|
|
anyhow::bail!("invalid attachment ID in backup");
|
|
}
|
|
let dir = target.join("attachments").join(&a.item_id);
|
|
fs::create_dir_all(&dir)?;
|
|
fs::write(dir.join(format!("{}.enc", a.attachment_id)), &a.ciphertext)?;
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Run test to verify it passes**
|
|
|
|
Run: `cargo test -p relicario-cli restore_rejects_traversal`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-core/src/ids.rs crates/relicario-cli/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(cli): validate IDs on backup restore (audit B4)
|
|
|
|
Crafted .relbak files with IDs like "../../.bashrc" could escape the
|
|
target directory. Now validates that item/attachment IDs are hex-only
|
|
before any fs::write.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 7: Sanitize item titles in commit messages (I1)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-cli/src/helpers.rs`
|
|
- Modify: `crates/relicario-cli/src/main.rs:565, 1110, 1327`
|
|
|
|
- [ ] **Step 1: Add sanitize_for_commit to helpers.rs**
|
|
|
|
```rust
|
|
/// Sanitize a string for use in git commit messages.
|
|
/// Removes control characters and truncates to 50 chars.
|
|
pub fn sanitize_for_commit(s: &str) -> String {
|
|
s.chars()
|
|
.filter(|c| !c.is_control())
|
|
.take(50)
|
|
.collect()
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Use sanitize_for_commit in add commit**
|
|
|
|
```rust
|
|
// Line 565, change:
|
|
commit_paths(&vault, &format!("add: {} ({})",
|
|
crate::helpers::sanitize_for_commit(&item.title),
|
|
item.id.as_str()), &path_refs)?;
|
|
```
|
|
|
|
- [ ] **Step 3: Use sanitize_for_commit in edit commit**
|
|
|
|
```rust
|
|
// Line 1110, change:
|
|
commit_paths(&vault, &format!("edit: {} ({})",
|
|
crate::helpers::sanitize_for_commit(&item.title),
|
|
item.id.as_str()),
|
|
```
|
|
|
|
- [ ] **Step 4: Use sanitize_for_commit in trash commit**
|
|
|
|
```rust
|
|
// Line 1327, change:
|
|
commit_paths(&vault, &format!("trash: {} ({})",
|
|
crate::helpers::sanitize_for_commit(&item.title),
|
|
item.id.as_str()),
|
|
```
|
|
|
|
- [ ] **Step 5: Write test for sanitization**
|
|
|
|
```rust
|
|
#[test]
|
|
fn sanitize_for_commit_strips_newlines() {
|
|
assert_eq!(sanitize_for_commit("line1\nline2"), "line1line2");
|
|
assert_eq!(sanitize_for_commit("a\tb"), "ab");
|
|
assert_eq!(sanitize_for_commit("normal"), "normal");
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 6: Run tests**
|
|
|
|
Run: `cargo test -p relicario-cli`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 7: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-cli/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(cli): sanitize item titles in commit messages (audit I1)
|
|
|
|
Control characters (newlines, tabs) in item titles corrupted git log
|
|
output. Now strips control chars and truncates to 50 chars.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 8: Expand AttachmentId to 128 bits (I2)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-core/src/ids.rs:54`
|
|
|
|
- [ ] **Step 1: Write test for 128-bit AttachmentId**
|
|
|
|
```rust
|
|
#[test]
|
|
fn attachment_id_is_128_bits() {
|
|
let id = AttachmentId::from_plaintext(b"test content");
|
|
assert_eq!(id.as_str().len(), 32); // 16 bytes = 32 hex chars
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Run test to verify it fails**
|
|
|
|
Run: `cargo test -p relicario-core attachment_id_is_128_bits`
|
|
Expected: FAIL — currently 16 hex chars (64 bits)
|
|
|
|
- [ ] **Step 3: Change digest slice from 8 to 16 bytes**
|
|
|
|
```rust
|
|
// Line 54, change:
|
|
// FROM:
|
|
Self(hex::encode(&digest[..8]))
|
|
// TO:
|
|
Self(hex::encode(&digest[..16]))
|
|
```
|
|
|
|
- [ ] **Step 4: Run test to verify it passes**
|
|
|
|
Run: `cargo test -p relicario-core attachment_id_is_128_bits`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Update is_valid() in Task 6 if already implemented**
|
|
|
|
Ensure `AttachmentId::is_valid()` checks for 32 chars, not 16.
|
|
|
|
- [ ] **Step 6: Run full test suite**
|
|
|
|
Run: `cargo test`
|
|
Expected: All tests pass
|
|
|
|
- [ ] **Step 7: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-core/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(core): expand AttachmentId to 128 bits (audit I2)
|
|
|
|
64-bit truncation allowed birthday collisions at ~2^32 work. Now uses
|
|
16 bytes (128 bits) of SHA-256, requiring ~2^64 work for collision.
|
|
|
|
No migration needed — only affects new attachments.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 9: Enforce per-vault attachment cap (I3)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-cli/src/main.rs` — cmd_attach function
|
|
|
|
- [ ] **Step 1: Find cmd_attach and calculate vault total**
|
|
|
|
In `cmd_attach`, before calling `encrypt_attachment`, sum existing attachment sizes from manifest.
|
|
|
|
- [ ] **Step 2: Implement vault-level cap check**
|
|
|
|
```rust
|
|
// In cmd_attach, after loading manifest and settings:
|
|
let current_total: u64 = manifest.items.values()
|
|
.flat_map(|entry| &entry.attachments)
|
|
.map(|a| a.size)
|
|
.sum();
|
|
|
|
let new_size = bytes.len() as u64;
|
|
let hard_cap = settings.attachment_caps.per_vault_hard_cap_bytes;
|
|
|
|
if current_total + new_size > hard_cap {
|
|
anyhow::bail!(
|
|
"attachment would exceed vault cap ({} + {} > {} bytes)",
|
|
current_total, new_size, hard_cap
|
|
);
|
|
}
|
|
|
|
let soft_cap = settings.attachment_caps.per_vault_soft_cap_bytes;
|
|
if current_total + new_size > soft_cap {
|
|
eprintln!("warning: vault attachment total exceeds soft cap ({} bytes)", soft_cap);
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 3: Write test for cap enforcement**
|
|
|
|
```rust
|
|
#[test]
|
|
fn attach_enforces_vault_cap() {
|
|
// Create vault with low hard cap in settings
|
|
// Add attachments until cap is hit
|
|
// Verify next attachment fails
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests**
|
|
|
|
Run: `cargo test -p relicario-cli`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-cli/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(cli): enforce per-vault attachment bytes cap (audit I3)
|
|
|
|
per_vault_soft_cap_bytes and per_vault_hard_cap_bytes were defined in
|
|
VaultSettings but never checked. Now enforced in cmd_attach with
|
|
warning at soft cap, error at hard cap.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 10: Disable HOTP with clear error (I6)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-core/src/error.rs`
|
|
- Modify: `crates/relicario-core/src/item_types/totp.rs:71-76`
|
|
|
|
- [ ] **Step 1: Add HotpNotSupported error variant**
|
|
|
|
```rust
|
|
// In error.rs:
|
|
#[derive(Debug, Error)]
|
|
pub enum RelicarioError {
|
|
// ... existing variants ...
|
|
|
|
#[error("HOTP is not supported: counter persistence requires vault save")]
|
|
HotpNotSupported,
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Reject HOTP in compute_totp_code**
|
|
|
|
```rust
|
|
// In totp.rs, compute_totp_code:
|
|
pub fn compute_totp_code(config: &TotpConfig, now_unix_seconds: u64) -> Result<String> {
|
|
let counter = match config.kind {
|
|
TotpKind::Totp => now_unix_seconds / config.period_seconds as u64,
|
|
TotpKind::Hotp { .. } => return Err(RelicarioError::HotpNotSupported),
|
|
TotpKind::Steam => now_unix_seconds / config.period_seconds as u64,
|
|
};
|
|
// ... rest unchanged
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 3: Update HOTP test to expect error**
|
|
|
|
```rust
|
|
#[test]
|
|
fn hotp_returns_not_supported_error() {
|
|
let cfg = TotpConfig { kind: TotpKind::Hotp { counter: 42 }, ..TotpConfig::default() };
|
|
let result = compute_totp_code(&cfg, 0);
|
|
assert!(matches!(result, Err(RelicarioError::HotpNotSupported)));
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Run tests**
|
|
|
|
Run: `cargo test -p relicario-core`
|
|
Expected: PASS (after updating any tests that expected HOTP to work)
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-core/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(core): disable HOTP with clear error (audit I6)
|
|
|
|
HOTP requires incrementing and persisting the counter after each use.
|
|
Without vault-save machinery in compute_totp_code, HOTP would desync
|
|
immediately. Now returns HotpNotSupported error.
|
|
|
|
TOTP and Steam codes continue to work.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 11: Document manifest integrity model (I4)
|
|
|
|
**Files:**
|
|
- Create: `docs/SECURITY.md`
|
|
|
|
- [ ] **Step 1: Write SECURITY.md**
|
|
|
|
```markdown
|
|
# Relicario Security Model
|
|
|
|
## Manifest Integrity
|
|
|
|
The manifest (`manifest.enc`) is encrypted with AEAD (XChaCha20-Poly1305), which provides:
|
|
|
|
- **Confidentiality**: Contents are unreadable without the master key
|
|
- **Integrity**: Any modification is detected and rejected on decrypt
|
|
- **Authenticity**: Only holders of the master key can create valid ciphertexts
|
|
|
|
### What AEAD Does NOT Protect
|
|
|
|
- **Item deletion**: An attacker with write access to the vault can delete
|
|
`.enc` files or git-revert commits. The manifest will decrypt successfully
|
|
but won't contain the deleted items.
|
|
|
|
- **Rollback attacks**: An attacker can replace `manifest.enc` with an older
|
|
valid version. AEAD accepts any ciphertext created with the correct key.
|
|
|
|
### Mitigation
|
|
|
|
Item deletion and rollback are detectable via **git history**. The git log
|
|
provides an append-only audit trail:
|
|
|
|
```bash
|
|
git log --oneline items/
|
|
```
|
|
|
|
For environments where git history could be rewritten (e.g., force-push),
|
|
consider:
|
|
|
|
1. A pre-receive hook that rejects non-fast-forward pushes
|
|
2. Signed commits (not currently implemented)
|
|
3. Regular backups with `relicario backup export`
|
|
|
|
## Access Control
|
|
|
|
Relicario does not implement device-level access control. All access control
|
|
is at the transport layer:
|
|
|
|
- **CLI**: SSH key authentication to the git remote
|
|
- **Extension**: Git credentials stored in browser profile
|
|
|
|
Device registration/revocation was removed in v0.3.0 as it provided no
|
|
actual security benefit without commit signing infrastructure.
|
|
```
|
|
|
|
- [ ] **Step 2: Commit**
|
|
|
|
```bash
|
|
git add docs/SECURITY.md
|
|
git commit -m "$(cat <<'EOF'
|
|
docs: document manifest integrity model (audit I4)
|
|
|
|
Clarifies what AEAD protects (tampering) vs. what it doesn't (deletion,
|
|
rollback). Documents that git history is the audit trail.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 12: Update backup to not include devices.json (B1 cleanup)
|
|
|
|
**Files:**
|
|
- Modify: `crates/relicario-core/src/backup.rs:61, 138`
|
|
- Modify: `crates/relicario-cli/src/main.rs:1447-1448, 1615`
|
|
|
|
- [ ] **Step 1: Remove devices_json from BackupInput struct**
|
|
|
|
In `backup.rs`, remove the `devices_json` field from `BackupInput` and `UnpackedBackup`.
|
|
|
|
- [ ] **Step 2: Update pack/unpack to not include devices**
|
|
|
|
Remove all references to `devices_json` in the pack/unpack logic.
|
|
|
|
- [ ] **Step 3: Update CLI export to not read devices.json**
|
|
|
|
Remove lines 1447-1448 that read devices.json.
|
|
|
|
- [ ] **Step 4: Update CLI restore to not write devices.json**
|
|
|
|
Remove line 1615 that writes devices.json.
|
|
|
|
- [ ] **Step 5: Run backup tests**
|
|
|
|
Run: `cargo test -p relicario-cli --test attachments`
|
|
Run: `cargo test -p relicario-core`
|
|
Expected: PASS
|
|
|
|
- [ ] **Step 6: Commit**
|
|
|
|
```bash
|
|
git add crates/relicario-core/ crates/relicario-cli/
|
|
git commit -m "$(cat <<'EOF'
|
|
fix(backup): remove devices.json from backup format (B1 cleanup)
|
|
|
|
Part of device key system removal. Backup files no longer include
|
|
devices.json since the device registration system is removed.
|
|
|
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
EOF
|
|
)"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 13: Final verification
|
|
|
|
- [ ] **Step 1: Run full test suite**
|
|
|
|
Run: `cargo test`
|
|
Expected: All tests pass
|
|
|
|
- [ ] **Step 2: Build all targets**
|
|
|
|
```bash
|
|
cargo build
|
|
cargo build -p relicario-wasm --target wasm32-unknown-unknown
|
|
cd extension && npm run build
|
|
```
|
|
Expected: All builds succeed
|
|
|
|
- [ ] **Step 3: Manual smoke test**
|
|
|
|
```bash
|
|
# Create new vault
|
|
relicario init test.jpg --output ref.jpg
|
|
|
|
# Verify no devices.json
|
|
ls .relicario/
|
|
# Should show: params.json, salt (no devices.json)
|
|
|
|
# Add item with special chars in title
|
|
relicario add login --title $'Test\nNewline' --username test
|
|
|
|
# Check git log for sanitized commit message
|
|
git log -1 --oneline
|
|
# Should show: add: TestNewline (...)
|
|
```
|
|
|
|
- [ ] **Step 4: Commit verification summary**
|
|
|
|
```bash
|
|
git log --oneline -15
|
|
```
|
|
|
|
Verify all 12+ commits are present with appropriate messages.
|
|
|
|
---
|
|
|
|
## Completion Checklist
|
|
|
|
- [ ] B1: Device key system removed (CLI, WASM, extension, backup)
|
|
- [ ] B2: Backup KDF NFC normalization
|
|
- [ ] B3: Test env vars gated with #[cfg(test)]
|
|
- [ ] B4: Path traversal validation on restore
|
|
- [ ] I1: Item titles sanitized in commits
|
|
- [ ] I2: AttachmentId expanded to 128 bits
|
|
- [ ] I3: Per-vault attachment cap enforced
|
|
- [ ] I4: SECURITY.md documents manifest integrity
|
|
- [ ] I5: (Covered by B1 — generate_device_keypair removed)
|
|
- [ ] I6: HOTP returns HotpNotSupported error
|
|
- [ ] All tests pass
|
|
- [ ] All builds succeed
|