From 71d51c0bea4250585e569a0228891cd3147c2170 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Sat, 2 May 2026 00:42:17 -0400 Subject: [PATCH] docs: add security audits and Plan 4 for blocker fixes - 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 --- ...6-04-18-initial-security-audit-opus-4-5.md | 158 +++ .../2026-05-01-security-audit-opus-4-5.md | 113 ++ .../audits/2026-05-01-security-audit.md | 199 ++++ .../2026-05-02-security-blocker-fixes.md | 1035 +++++++++++++++++ 4 files changed, 1505 insertions(+) create mode 100644 docs/superpowers/audits/2026-04-18-initial-security-audit-opus-4-5.md create mode 100644 docs/superpowers/audits/2026-05-01-security-audit-opus-4-5.md create mode 100644 docs/superpowers/audits/2026-05-01-security-audit.md create mode 100644 docs/superpowers/plans/2026-05-02-security-blocker-fixes.md diff --git a/docs/superpowers/audits/2026-04-18-initial-security-audit-opus-4-5.md b/docs/superpowers/audits/2026-04-18-initial-security-audit-opus-4-5.md new file mode 100644 index 0000000..7e8bdf6 --- /dev/null +++ b/docs/superpowers/audits/2026-04-18-initial-security-audit-opus-4-5.md @@ -0,0 +1,158 @@ +# Verification: 2026-04-18 Initial Security Audit + +**Verified by:** Claude Opus 4.5 +**Date:** 2026-05-01 +**Methodology:** Code inspection of referenced file paths and line numbers + +--- + +## Summary + +| Finding | File Exists | Lines Match Current | Vulnerability Status | Confidence | +|---------|-------------|---------------------|---------------------|------------| +| C1 - Setup web-accessible | ✅ | ❌ refactored | **FIXED** | 10/10 | +| C2 - Message router trusts all | ✅ | ❌ refactored | **FIXED** | 10/10 | +| C3 - Capture innerHTML XSS | ✅ | ❌ refactored | **FIXED** | 10/10 | +| C4 - Autofill no origin check | ✅ | ❌ refactored | **FIXED** | 10/10 | +| H1 - KDF unprefixed concat | ✅ | ❌ refactored | **FIXED** | 10/10 | +| H2 - Master key not zeroized | ✅ | ❌ refactored | **FIXED** | 9/10 | +| H3 - Passphrase gate cosmetic | ✅ | ❌ refactored | **FIXED** | 10/10 | +| H4 - Git shells out unsafely | ✅ | ❌ refactored | **FIXED** | 10/10 | +| H5 - WASM Math.random() | ✅ | ❌ refactored | **FIXED** | 10/10 | +| H6 - Modulo bias | ✅ | ❌ refactored | **FIXED** | 10/10 | +| H7 - rpassword outdated | ✅ | ✅ | **FIXED** | 10/10 | +| H8 - Storage plaintext | ✅ | ⚠️ partial | **ACKNOWLEDGED** | 8/10 | + +**Verdict:** All CRITICAL and HIGH findings except H8 have been remediated. The codebase has been significantly refactored since this audit, making line number references obsolete but confirming fixes were applied. + +--- + +## CRITICAL Findings + +### C1: Setup wizard web-accessible + +- **Original claim:** `web_accessible_resources` with `matches: [""]` allows any website to inject vault config. +- **Current state:** `extension/manifest.json` line 38 shows `"web_accessible_resources": []` (empty array). +- **Router validation:** `router/index.ts` lines 29-71 verify sender origins (`isPopup`, `isSetup`, `isContent`) and return `unauthorized_sender` for invalid callers. +- **Status:** ✅ **FIXED** + +### C2: Service-worker trusts every message + +- **Original claim:** `index.ts:116-441` ignores `_sender` and trusts all messages. +- **Current state:** `service-worker/index.ts` is now 100 lines; message handling delegated to modular router. +- **Router checks:** + - `sender.frameId === 0` for content scripts (line 42) + - `sender.id === chrome.runtime.id` (line 43) + - Returns `{ ok: false, error: 'unauthorized_sender' }` for invalid senders +- **Status:** ✅ **FIXED** + +### C3: Capture prompt innerHTML injection + +- **Original claim:** `capture.ts:172-191` uses innerHTML with attacker-controlled strings in page DOM. +- **Current state:** + - Uses `createShadowHost()` (line 128) for closed Shadow DOM + - Builds DOM via `document.createElement` + `.textContent =` (lines 143-216) + - File header (lines 6-9) documents this pattern +- **Status:** ✅ **FIXED** + +### C4: Autofill has no origin check + +- **Original claim:** `get_autofill_candidates` accepts URL from message payload; `get_credentials` returns any entry by ID. +- **Current state in `content-callable.ts`:** + - Line 25: `const senderHost = safeHostname(sender.tab?.url ?? '')` — uses sender tab, not message + - Line 44: `if (!itemHost || itemHost !== senderHost) return { ok: false, error: 'origin_mismatch' }` + - Lines 46-51: TOFU origin-ack check before returning credentials +- **Status:** ✅ **FIXED** + +--- + +## HIGH Findings + +### H1: Argon2id unprefixed concatenation + +- **Original claim:** `crypto.rs:225-227` has `password = passphrase || image_secret` without length prefix. +- **Current state at `crypto.rs:229-236`:** + ```rust + let mut password = Zeroizing::new(Vec::with_capacity(8 + nfc_passphrase.len() + 8 + 32)); + password.extend_from_slice(&(nfc_passphrase.len() as u64).to_be_bytes()); + password.extend_from_slice(&nfc_passphrase); + password.extend_from_slice(&32u64.to_be_bytes()); + password.extend_from_slice(image_secret); + ``` + Also includes NFC normalization (lines 224-227). +- **Status:** ✅ **FIXED** + +### H2: Master key never zeroized + +- **Original claim:** `Vec` from `derive_master_key` and intermediates leak into heap. +- **Current state:** + - `crypto.rs:212`: returns `Zeroizing<[u8; 32]>` + - `crypto.rs:232`: password wrapped in `Zeroizing::new()` + - `session.rs` (WASM/CLI): stores keys as `Zeroizing<[u8; 32]>` + - CLI rpassword calls wrapped in `Zeroizing::new()` +- **Note:** JS string zeroization remains a limitation (acknowledged). +- **Status:** ✅ **FIXED** + +### H3: Passphrase strength gate cosmetic + +- **Original claim:** Extension accepts any non-empty passphrase; CLI only requires 8 chars. +- **Current state:** + - `setup.ts:152,640`: `score < 3` disables button + - `setup.ts:784-789`: server-side re-validation before create + - `generators.rs:124-130`: `validate_passphrase_strength()` requires score >= 3 +- **Status:** ✅ **FIXED** + +### H4: Git shells out without guards + +- **Original claim:** No hooks/gpgsign/editor isolation. +- **Current state in `helpers.rs:41-55`:** + ```rust + cmd.args([ + "-c", "core.hooksPath=/dev/null", + "-c", "commit.gpgsign=false", + "-c", "core.editor=true", + ]); + ``` + Comment explicitly references "Audit H4". +- **Status:** ✅ **FIXED** + +### H5: WASM Math.random() + +- **Original claim:** `lib.rs:240-256` uses `Math.random()` for password generation. +- **Current state:** `generate_password` calls `core_generate_password` from relicario-core. +- **generators.rs:** + - Line 6: `use rand::rngs::OsRng;` + - Lines 61-64: Uses `Uniform::from()` with `OsRng` + - No `Math.random()` anywhere in codebase +- **Status:** ✅ **FIXED** + +### H6: Modulo bias + +- **Original claim:** `main.rs:308-317` uses `% CHARSET.len()`. +- **Current state in `generators.rs`:** + - Line 61: `let dist = Uniform::from(0..charset.len());` + - Line 63: `charset[dist.sample(&mut rng)]` — rejection sampling, no modulo +- **Status:** ✅ **FIXED** + +### H7: rpassword 5.0.1 outdated + +- **Original claim:** Uses deprecated `prompt_password_stderr`. +- **Current state:** `Cargo.toml` shows `rpassword = "7"`, uses `prompt_password`. +- **Status:** ✅ **FIXED** + +### H8: Storage keeps apiToken/imageBase64 plaintext + +- **Original claim:** `chrome.storage.local` stores PAT and reference image unencrypted. +- **Current state:** Still true — `popup-only.ts:139-141` stores `vaultConfig` and `imageBase64`. +- **Mitigation:** Acknowledged as design constraint; spec documents that filesystem access to browser profile compromises both factors. +- **Status:** ⚠️ **ACKNOWLEDGED** (not fixed, documented as acceptable tradeoff) + +--- + +## Conclusion + +The 2026-04-18 audit identified real vulnerabilities that existed at that time. **All CRITICAL and HIGH findings (C1-C4, H1-H7) have since been remediated** with the exact fixes recommended in the audit. The codebase underwent significant refactoring, making the original line number references obsolete. + +H8 remains as an acknowledged design constraint inherent to Chrome extension architecture. + +**The audit was accurate and the remediation was thorough.** diff --git a/docs/superpowers/audits/2026-05-01-security-audit-opus-4-5.md b/docs/superpowers/audits/2026-05-01-security-audit-opus-4-5.md new file mode 100644 index 0000000..a0c3795 --- /dev/null +++ b/docs/superpowers/audits/2026-05-01-security-audit-opus-4-5.md @@ -0,0 +1,113 @@ +# Verification: 2026-05-01 Security Audit + +**Verified by:** Claude Opus 4.5 +**Date:** 2026-05-01 +**Methodology:** Code inspection of referenced file paths and line numbers + +--- + +## Summary + +| Finding | File Exists | Lines Accurate | Vulnerability Real | Confidence | +|---------|-------------|----------------|-------------------|------------| +| 1 - Backup KDF NFC | ✅ | ✅ | ✅ | 10/10 | +| 2 - Commit injection | ✅ | ✅ | ✅ | 10/10 | +| 3 - WASM private key exposure | ✅ | ✅ | ✅ | 10/10 | +| 4 - Test env vars in prod | ✅ | ✅ | ✅ | 10/10 | +| 5 - AttachmentId 64-bit | ✅ | ⚠️ off-by-1 | ✅ | 9/10 | +| 6 - Field history plaintext | ✅ | ✅ | ✅ | 10/10 | +| 7 - Device keys non-functional | ✅ | ✅ | ✅ | 10/10 | +| 8 - Path traversal restore | ✅ | ✅ | ✅ | 10/10 | + +**Verdict:** All 8 findings are verified as real vulnerabilities in the current codebase. + +--- + +## Finding-by-Finding Verification + +### Finding 1 — Backup KDF missing NFC normalization + +- **File:** `crates/relicario-core/src/backup.rs` +- **Claimed lines:** 303-312 +- **Verified:** ✅ `derive_backup_key` at lines 303-312 passes `passphrase` directly to `argon.hash_password_into()` without NFC normalization. Compare to `derive_master_key` in `crypto.rs:224-227` which explicitly normalizes. +- **Impact confirmed:** Cross-platform restore failure for non-ASCII passphrases. + +### Finding 2 — Commit message injection via item titles + +- **File:** `crates/relicario-cli/src/main.rs` +- **Claimed lines:** 565, 899-901, 1110, 1327 +- **Verified:** ✅ + - Line 565: `format!("add: {} ({})", item.title, item.id.as_str())` + - Line 1110: `format!("edit: {} ({})", item.title, item.id.as_str())` + - Line 1327: `format!("trash: {} ({})", item.title, item.id.as_str())` +- **Impact confirmed:** Newlines/control chars in titles corrupt git log output. + +### Finding 3 — WASM `generate_device_keypair` crosses private key to JS + +- **File:** `crates/relicario-wasm/src/lib.rs` +- **Claimed lines:** 215-227 +- **Verified:** ✅ Function returns `{ "private_key_base64": "..." }` as `JsValue`, exposing ed25519 private key to JavaScript heap. +- **Impact confirmed:** Key material accessible to any JS in service worker context. + +### Finding 4 — Test env vars ship in production binary + +- **File:** `crates/relicario-cli/src/main.rs` +- **Claimed lines:** 445-446, 421-423, 1425-1426 +- **Verified:** ✅ + - Lines 421-423: `RELICARIO_TEST_ITEM_SECRET` + - Lines 445-446: `RELICARIO_TEST_PASSPHRASE` + - Lines 1425-1426: `RELICARIO_TEST_BACKUP_PASSPHRASE` +- **Impact confirmed:** All checked in production code without `#[cfg(test)]`. Passphrase visible in `/proc//environ`. + +### Finding 5 — `AttachmentId` truncated to 64 bits + +- **File:** `crates/relicario-core/src/ids.rs` +- **Claimed lines:** 52-57 +- **Actual lines:** 51-56 (off by 1) +- **Verified:** ✅ `&digest[..8]` = 8 bytes = 64 bits. Birthday collision at ~2³² work. +- **Impact confirmed:** Attacker with attachment upload can cause silent overwrites. + +### Finding 6 — `get_field_history` returns plaintext to JS + +- **File:** `crates/relicario-wasm/src/lib.rs` +- **Claimed lines:** 232-265 +- **Verified:** ✅ Returns historical `Password`/`Concealed` values as plaintext JSON via `v.as_str().to_owned()`. +- **Impact confirmed:** Password history exposed to JS heap without Zeroizing. + +### Finding 7 — Device key system is security theater + +- **File:** `crates/relicario-cli/src/main.rs` +- **Claimed lines:** 2151-2221 +- **Verified:** ✅ `cmd_device()` handles Add/List/Revoke but: + - No `sign_commit` or `verify_signature` functions exist anywhere + - `devices.json` is plaintext and unauthenticated + - Revocation has no enforcement mechanism +- **Impact confirmed:** Users falsely believe device revocation provides security. + +### Finding 8 — Path traversal on backup restore + +- **File:** `crates/relicario-cli/src/main.rs` +- **Claimed lines:** 1619-1626 +- **Verified:** ✅ + ```rust + for item in &unpacked.items { + fs::write(target.join("items").join(format!("{}.enc", item.id)), ...)?; + } + ``` + `item.id` and `attachment_id` used directly in path construction with no validation. +- **Impact confirmed:** Crafted `.relbak` with `id = "../../.bashrc"` escapes target directory. + +--- + +## Blockers Assessment + +The audit's "Path to Certifiable Safety" section is accurate: + +| Blocker | Verified | Severity | +|---------|----------|----------| +| B1 - Device key theater | ✅ Real | High | +| B2 - Backup KDF NFC | ✅ Real | Medium | +| B3 - Test env vars | ✅ Real | Medium | +| B4 - Path traversal | ✅ Real | Medium | + +All four blockers are confirmed. B1 is the most dangerous as it misleads users about their security posture. diff --git a/docs/superpowers/audits/2026-05-01-security-audit.md b/docs/superpowers/audits/2026-05-01-security-audit.md new file mode 100644 index 0000000..399f02a --- /dev/null +++ b/docs/superpowers/audits/2026-05-01-security-audit.md @@ -0,0 +1,199 @@ +# Relicario Security Audit — 2026-05-01 + +Scope: full project audit (not a PR diff). Covers crypto correctness, protocol gaps, +implementation reality vs. plans, and a roadmap toward third-party auditability. + +--- + +## Section 1 — Security Findings + +### Finding 1 — Backup KDF missing NFC normalization + +**`crates/relicario-core/src/backup.rs:303-312`** · Severity: **Medium** + +`derive_backup_key` passes raw passphrase bytes to Argon2id. The main vault KDF in +`crypto.rs` uses `u64_be(len) || nfc_passphrase || u64_be(32) || image_secret`. The +backup KDF has neither NFC normalization nor the length-prefix construction. + +**Exploit:** User creates a backup on macOS (NFD normalization) and restores on Linux +(NFC). The Argon2id input differs → wrong key → unrestorable backup. Affects any +non-ASCII passphrase (`"Crêpe-7"`, `"café"`, accented chars). + +**Fix:** Factor out `normalize_passphrase()` and use it in both `derive_master_key` and +`derive_backup_key`. + +--- + +### Finding 2 — Commit message injection via item titles + +**`crates/relicario-cli/src/main.rs:565, 899-901, 1110, 1327`** · Severity: **Medium** + +Item titles (arbitrary user UTF-8) are embedded directly into `-m` commit message strings +via `format!("add: {} ({})", item.title, ...)`. `git_command` uses `Command::args()` (no +shell), so shell injection into `git add` is blocked — but newlines in titles produce +malformed multi-line commit messages that corrupt git log parsers. + +**Fix:** Strip control characters from titles before embedding in commit messages, or omit +the title from the `-m` format entirely and use only the item ID. + +--- + +### Finding 3 — WASM `generate_device_keypair` crosses private key bytes to JS + +**`crates/relicario-wasm/src/lib.rs:215-227`** · Severity: **Medium** + +Returns `{ "private_key_base64": "..." }` as a `JsValue`. The ed25519 private key lives in +the JS heap with no `Zeroizing` protection. The vault master key is protected behind an +opaque `SessionHandle` and never crosses to JS — the device key has no such protection. + +**Exploit:** Any JS running in the extension service worker context (compromised dependency, +content script escalation) that can intercept the return value gets the raw device key. + +**Fix:** Never return the private key to JS. Expose only a `sign(handle, data) → signature` +API; perform the signing in Rust. + +--- + +### Finding 4 — Test env vars ship in production binary + +**`crates/relicario-cli/src/main.rs:445-446, 421-423, 1425-1426`** · Severity: **Medium** + +`RELICARIO_TEST_PASSPHRASE`, `RELICARIO_TEST_ITEM_SECRET`, `RELICARIO_TEST_BACKUP_PASSPHRASE` +are checked in production code (not `#[cfg(test)]`). When set, they bypass the interactive +TTY prompt. + +**Exploit:** On Linux, `/proc//environ` exposes the passphrase in cleartext to +same-UID processes. Shell history captures `RELICARIO_TEST_PASSPHRASE=mysecret relicario unlock ...`. + +**Fix:** Gate behind `#[cfg(test)]` or a `--features testing` build profile. + +--- + +### Finding 5 — `AttachmentId` truncated to 64 bits of SHA-256 + +**`crates/relicario-core/src/ids.rs:52-57`** · Severity: **Medium** + +`AttachmentId::from_plaintext` takes `&digest[..8]` (8 bytes = 64 bits). Standard +content-addressed stores use ≥128 bits. With 64 bits, an attacker who can supply attachment +content can find a second-preimage collision with ~2^32 work, causing a crafted attachment +to silently overwrite an existing one on disk. + +**Fix:** Change `&digest[..8]` → `&digest[..16]` (128 bits). No migration needed for +existing vaults since only new attachments are affected. + +--- + +### Finding 6 — `get_field_history` re-parses item JSON from JS heap + +**`crates/relicario-wasm/src/lib.rs:232-265`** · Severity: **Medium** + +Returns all historical `Password`/`Concealed` values as plaintext `JsValue`. The values +are regular `String` allocations with no `Zeroizing` wrapper before serialization into +`serde_json::Value`. + +**Fix:** Architectural — document that the caller must treat the return value as sensitive. +For strong hygiene: do all history display in Rust, never returning password history bytes +to JS. + +--- + +### Finding 7 — Device key system is non-functional as a security control + +**`crates/relicario-cli/src/main.rs:2151-2221`** · Severity: **High** + +`device add/list/revoke` and `generate_device_keypair` exist, but **no code anywhere signs +git commits with device keys**, and **no code verifies device signatures**. `devices.json` +is plaintext in the repo and unauthenticated by the vault. + +**Exploit:** Users believe "device revocation" prevents unauthorized access after a device +is stolen/compromised. It does nothing. A stolen device continues to have full vault access +via its git remote credentials regardless of revocation. + +**Fix:** Either (a) implement commit signing + server-side pre-receive hook verification, or +(b) remove the `device` subcommands and document that access control is SSH-key-level only. + +--- + +### Finding 8 — Path traversal on backup restore + +**`crates/relicario-cli/src/main.rs:1619-1626`** · Severity: **Medium** + +During restore, item/attachment IDs from the decrypted backup JSON are used directly as +path components with no format validation. IDs are AEAD-authenticated but a user restoring +from a crafted `.relbak` with a known passphrase would execute arbitrary path writes. + +**Exploit (social engineering):** Attacker provides a `.relbak` with item ID +`../../.bashrc` → restore overwrites `~/.bashrc`. + +**Fix:** +```rust +ensure!(id.len() == 16 && id.chars().all(|c| c.is_ascii_hexdigit()), + "invalid id in backup"); +``` + +--- + +## Section 2 — Implementation Status + +| Feature | Status | Notes | +|---|---|---| +| Two-factor decrypt (passphrase + image_secret) | ✅ Implemented | Full crypto pipeline, NFC passphrase, Argon2id m=64MiB t=3 p=4 | +| imgsecret embed | ✅ Implemented | DCT QIM, QUANT_STEP=50, central 70%, 5-50 redundant copies | +| imgsecret extract + crop recovery | ✅ Implemented | Majority voting ≥60%, 4 crop search strategies | +| Manifest browse (schema v2) | ✅ Implemented | Encrypted with master key, search(), title/type/tags/icon | +| Vault CRUD (init/add/edit/rm/trash/restore/purge) | ✅ Implemented | All 7 item types fully handled | +| CLI `init` | ✅ Implemented | zxcvbn ≥3 gate, image embed, Argon2id params, git init | +| CLI `add` / `edit` | ✅ Implemented | All 7 types, TOTP QR decode via rqrr, field history capture | +| CLI `generate` | ✅ Implemented | Random (rejection-sampled) + BIP39, uses vault defaults | +| CLI `sync` | ✅ Implemented | `git pull --rebase && git push` | +| CLI `backup export/restore` | ✅ Implemented | Plan 3A: zstd+AEAD container, optional image + git bundle | +| CLI `import lastpass` | ✅ Implemented | Plan 3B: CSV validation, Login + SecureNote + TOTP mapping | +| WASM bindings (all item/manifest/settings) | ✅ Implemented | Complete symmetric set | +| WASM session handle (opaque master key) | ✅ Implemented | Key never crosses WASM boundary | +| WASM attachment, generator, TOTP, backup, import | ✅ Implemented | All wired | +| Field history tracking + CLI `history` | ✅ Implemented | Password/Concealed/TOTP history, prune policies | +| Trash + retention | ✅ Implemented | `trash list/empty`, TrashRetention window | +| Attachments (CLI + WASM) | ✅ Implemented | File-level AEAD, cap enforcement, Document type | +| Settings / VaultSettings | ✅ Implemented | All retention + generator + cap fields, CLI subcommands | +| Device keys (add/list/revoke) | ⚠️ Partial | Key gen + persistence only — **no signing, no verification** (Finding 7) | +| Per-vault total attachment cap | ⚠️ Partial | Cap defined in settings, per-attachment enforced — per-vault total bytes not checked | +| Browser extension UI | ⚠️ Partial | WASM surface complete; extension TypeScript/HTML is a separate repo | +| Recovery QR | ❌ Plan-only | Spec written; no `recovery_qr.rs` module exists | +| Password coloring | ❌ Plan-only | Spec written; no implementation | +| Passphrase rotation | ❌ Deferred | Explicitly back-burnered | +| Pre-v0.3.0 audit walk | ❌ Not started | Listed as pending before v0.3.0 tag | +| HOTP counter persistence | ❌ Bug | `Hotp { counter }` never incremented/saved — HOTP desynchronizes immediately | + +--- + +## Section 3 — Path to Certifiable Safety + +### Blockers — must fix before any real use + +| # | Item | +|---|---| +| B1 | **Device key system is security theater** — implement signing or remove the commands. This is the most dangerous finding because it misleads users about their security posture. | +| B2 | **Backup KDF NFC normalization** — one-line fix; data loss risk for non-ASCII passphrases. | +| B3 | **Test env vars in production binary** — gate with `#[cfg(test)]`. Exposes passphrase via `/proc`. | +| B4 | **Path traversal on restore** — two-line ID validation before any `fs::write`. | + +### Important — fix before third-party audit + +| # | Item | +|---|---| +| I1 | Sanitize item titles before embedding in commit messages | +| I2 | `AttachmentId`: `&digest[..8]` → `&digest[..16]` (128-bit collision resistance) | +| I3 | Enforce per-vault total attachment bytes cap (already defined, never checked) | +| I4 | Document manifest integrity model: AEAD protects against silent modification, but item deletion is only detectable via git history | +| I5 | Stop crossing device private key bytes to JS (prerequisite for B1 if signing is implemented) | +| I6 | Fix HOTP counter: increment + re-save on each `totp get`, or disable HOTP and return an error | + +### Nice-to-have — audit-friendliness + +| # | Item | +|---|---| +| N1 | Wrap `nfc_passphrase: Vec` in `Zeroizing` in `derive_master_key` | +| N2 | `cargo audit` in CI | +| N3 | Validate Argon2id params on vault load — warn if below production minimums | +| N4 | Broaden steganography recompression tests to use ImageMagick/libjpeg-turbo (not just the `image` crate) | +| N5 | Consider machine-readable audit log encrypted alongside the vault | diff --git a/docs/superpowers/plans/2026-05-02-security-blocker-fixes.md b/docs/superpowers/plans/2026-05-02-security-blocker-fixes.md new file mode 100644 index 0000000..2bb9f5b --- /dev/null +++ b/docs/superpowers/plans/2026-05-02-security-blocker-fixes.md @@ -0,0 +1,1035 @@ +# 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 +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 { + 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 +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 +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> { + use unicode_normalization::UnicodeNormalization; + + // NFC normalize passphrase (same as derive_master_key in crypto.rs) + let nfc_passphrase: Vec = match std::str::from_utf8(passphrase) { + Ok(s) => s.nfc().collect::().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 +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> { + #[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> { + #[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//environ and shell history. + +Now only compiled into test binaries. + +Co-Authored-By: Claude Opus 4.5 +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 +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 +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 +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 +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 { + 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 +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 +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 +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