# Svault 0.5.0 — Consolidated Findings Register

This is the **document of record** for the 0.5.0 security review. It de-duplicates the
findings from all five independent reviews into one list, shows which models raised each
one (cross-model agreement is the strongest prioritization signal), and records the
**maintainer's disposition and justification** for each. The full reviews live in
[`../reviews/`](../reviews/).

The AI reviews are **inputs** — they give breadth and independence. The decisions are
**human**: every finding below was read, triaged, and dispositioned by the maintainer,
who owns the outcome. Nothing here is auto-generated or accepted without human judgment.

- **Version:** 0.5.0
- **Review date:** 2026-05-29
- **Automated reviewers (5):** Grok 4.3 (xAI), GLM-5-1 (z-ai), Gemini 3.5 Flash, DeepSeek-V4-Pro, Claude Opus 4.8 (Anthropic) — used for breadth, not as the final word
- **Tooling pass:** `cargo test --locked` (74 pass), `cargo clippy -D warnings` (clean), `cargo audit` (0 vulns, 2 transitive advisories — see #11)

> ## Human review & sign-off
> Every one of the 22 findings below was reviewed and dispositioned by a human:
> **Nima Karimi (maintainer)** — 2026-05-29. The Status, Disposition, and Notes columns
> are his decisions and his words; the AI reviews informed them but did not make them.

### Legend

**Severity** — highest rating any reviewer assigned.

**Status:**
- **Approved** — reviewed and signed off; the current behavior is intentional and accepted.
- **OK** — accepted, no action needed for now.
- **Planned** — accepted as real and scheduled for a future release (see [roadmap](../../roadmap.md)).
- **To fix** — will be fixed.
- **To check** — needs investigation; decision pending.

> The **Status** and **Disposition** columns are the maintainer's decisions for 0.5.0,
> grounded in the five reviews and the documented [threat model](../../security.md). The
> **Notes** column holds the maintainer's own working notes.

---

## Cross-model agreement matrix

A `Y` means that reviewer independently raised the finding. Findings flagged by **≥3
models** (marked `*`) are the consensus priorities.

| # | Finding | Sev | Grok | GLM | Gem | DSk | Cla |
|---|---------|-----|:----:|:---:|:---:|:---:|:---:|
| 1 * | No socket authentication / peer-credential check — daemon trusts any process of the same UID | High | Y | Y | Y | Y | Y |
| 2 | Policy engine is advisory, not enforcing — daemon `Get` runs no policy/audit; caller/scope/reason are self-asserted | High |  |  |  |  | Y |
| 3 * | Master passphrase transmitted in cleartext over the Unix socket (`Unlock`) | High | Y | Y | Y | Y | Y |
| 4 * | File-session fallback writes the raw passphrase to disk (non-daemon, Windows, **and TUI on every platform**); Windows applies no `0600`/ACL equivalent | High | Y | Y | Y | Y | Y |
| 5 * | Policy file is unsigned and `find_policy_file()` walks to `/` — a writable ancestor dir allows policy injection | High | Y | Y | Y |  |  |
| 6 * | Decrypted secret values (and the `Unlock` passphrase) handled as un-zeroized `String` — heap residue | Med |  | Y | Y | Y | Y |
| 7 | Static salt reuse on vault writes (`save_secrets` re-uses existing salt instead of a fresh one) | Med |  | Y | Y |  |  |
| 8 * | No connection limit / rate-limit on the daemon — unbounded thread-per-connection (DoS) | Med |  | Y | Y | Y |  |
| 9 * | No dependency/SCA scanning in CI (`cargo audit` / `cargo-deny`) | Med | Y | Y | Y | Y | Y |
| 10 * | Transitive advisories via `ratatui` (TUI only): `lru` unsound (RUSTSEC-2026-0002), `paste` unmaintained (RUSTSEC-2024-0436) | Med | Y | Y | Y | Y | Y |
| 11 * | No supply-chain hardening — no binary signing, SBOM, checksums, or provenance on release artifacts | Med | Y | Y |  | Y | Y |
| 12 * | Passphrase strength is advisory + shallow heuristic — no hard floor / entropy estimate | Med | Y | Y | Y |  | Y |
| 13 | Mutex poisoning kills the daemon — 7 production paths use bare `lock().unwrap()` | Med |  |  |  | Y |  |
| 14 | `recovery.enc` & export bundles written world-readable (umask, not `0600`); export contains a key-equivalent; gitignore guard is best-effort (already leaked once in git history) | Med |  |  |  |  | Y |
| 15 | Audit/usage logs are best-effort, attacker-mutable, and not tamper-evident — and the rate-limiter reads them back (defeatable by editing logs or rotating `--caller`) | Med |  | Y |  |  | Y |
| 16 | Socket-permission TOCTOU (umask window between `bind` and `chmod 0600`) + world-traversable `.svault/` (`0755`) | Med |  | Y |  |  | Y |
| 17 * | Daemon hygiene: no `SIGTERM`/`SIGCHLD` handler, no full double-fork, `daemon.log` unrotated, PID-file start race, coarse 10 s auto-lock ticker | Low |  | Y |  | Y | Y |
| 18 | Recovery code has no checksum/parity digit (typos only caught after Argon2id) | Low |  | Y | Y |  |  |
| 19 | `Vault::open` decrypts the entire secret DB just to verify the passphrase | Low |  | Y | Y |  |  |
| 20 | `unwrap()` on a length slice (`vault.rs:152`) panics if `vault.enc` is truncated below `SALT_SIZE` | Low |  |  |  | Y | Y |
| 21 | GCM uses no AAD — the prepended salt is not authenticated as associated data | Low |  |  |  |  | Y |
| 22 | Fallback policy reads `meta.yaml` **unverified** (`load_unverified`) — HMAC guard isn't on the `get` path, so a same-UID actor can flip `allow_agent` | Low |  |  |  |  | Y |

**Consensus priorities (≥3 models):** #1, #3, #4, #5, #6, #8, #9, #10, #11, #12, #17.
**All five models agreed on:** #1, #3, #4, #9, #10.

---

## Maintainer dispositions (human review)

Decided and signed off by **Nima Karimi (maintainer), 2026-05-29.** Status and
justification are the maintainer's call; the **Notes** column is his own running notes.

| # | Status | Disposition & justification | Maintainer notes |
|---|--------|-----------------------------|------------------|
| 1 | Approved | Same-UID trust is the documented threat model ([security.md](../../security.md)): Svault protects secrets at rest and gives cooperating agents an audit trail — it is not a sandbox against a hostile same-UID process. The `SO_PEERCRED`/`getpeereid` peer-UID bond is the accepted approach. | Peer-UID bond — OK. |
| 2 | Planned | Policy engine work is planned — evaluate moving policy evaluation + audit into the daemon so the socket becomes the enforced choke point, and state plainly that the policy layer is an honesty/audit layer today. | Policy engine planned. |
| 3 | Planned | Socket improvement planned — derive the key client-side and send only the derived key / a session token, so the passphrase never crosses the socket. | Socket improvement. |
| 4 | To fix | Update the TUI so the daemon is always preferred; deprecate `.session` on Unix. On Windows, add DPAPI-backed protection or refuse-to-cache (require re-entry). Until then, document loudly that `.session` = cleartext passphrase at rest. | Update TUI — daemon always preferred. |
| 5 | Planned | Policy config work planned — sign the policy file (or pin it to the vault) and stop the unbounded upward search / restrict to a trusted root. | Policy config. |
| 6 | To check | Investigate extending zeroization to the passphrase `String` and returned secret values (`Zeroizing`/`secrecy`); derived keys are already `ZeroizeOnDrop`. | TBD — to check. |
| 7 | OK | Random 96-bit nonce per write makes key+nonce reuse risk negligible, and re-key rotates the salt. Per-write salt rotation can be done as cleanup; not a blocker. | OK. |
| 8 | Planned | Add a max-connections ceiling + per-connection timeout, and include performance / max-out tests across many threads. | Planned — perf + max-out multi-thread tests. |
| 9 | Planned | Add a `cargo audit` / `cargo-deny` gate to CI. Schema/config still to be defined. | Planned — TBD, define schema. |
| 10 | Planned | TUI-only transitive advisories, not in the crypto/daemon path; a `ratatui` bump clears both. Will be fixed. | Planned — will be fixed. |
| 11 | To check | Verify what's feasible for release-artifact integrity — SHA-256 checksums first, then evaluate signing/SBOM/provenance. | To check — to verify. |
| 12 | To check | Create flow already warns + confirms. Investigate an entropy-based estimate with an enforced floor + `--force` for non-interactive use. | To check. |
| 13 | To fix | Replace bare `lock().unwrap()` with `lock().unwrap_or_else(PoisonError::into_inner)` so a panicking handler can't kill the daemon without zeroizing. Check zeroization on that path. | To fix — check zeroizing. |
| 14 | Planned | Create `recovery.enc` / export bundles `0600`, make the export gitignore guard non-optional, consider a distinct export passphrase. Cover import/export hardening more fully later. | Planned — cover in/out more later. |
| 15 | OK | Needs improvement, OK for now: local logs are advisory and never contain secret values. For real assurance, ship them to an append-only/remote sink; document that local logs are not tamper-evident. | Needs improvement; OK for now. |
| 16 | Planned | Create `.svault/` as `0700` and set the socket mode atomically (umask around `bind`, or bind in a private temp dir). Also explore binding a machine ID at vault creation so the vault is stuck to the hardware it was created on — check options. | Planned — also: machine-ID bind to hardware at vault creation; check options. |
| 17 | Planned | Daemon hygiene cleanup — `SIGTERM` handler that zeroizes, log rotation/size cap, tighten the start race, tune the ticker interval. | Planned — Rust/runtime hygiene. |
| 18 | OK | 160-bit code is strong; a checksum digit is a UX nicety to catch typos before Argon2id. Backlog. | OK. |
| 19 | OK | Minor perf/exposure nit; a separate HMAC/known-payload verification step would avoid decrypting all secrets just to check the passphrase. Backlog. | OK. |
| 20 | To fix | Replace the `unwrap()` on the length slice (`vault.rs:152`) with a checked length guard returning an error. Availability-only, same-user. | To fix — Rust things. |
| 21 | OK | Salt change yields the wrong key (auth failure) and `meta.yaml` is separately HMAC-protected, so impact is minimal. Bind salt/context as AAD as cleanup. | OK. |
| 22 | Planned | Config-related: verify `meta.yaml` (HMAC) on the `get`/fallback path too, so the advertised metadata-integrity guarantee covers the policy gate. | Planned — config related. |

---

## Carry-forward to the next release

When 0.6.0 is reviewed, restate each item above as **fixed / improved / unchanged** in
`findings/0.6.0.md`. Working order for 0.6.0, by status and cross-model agreement:

1. **To fix first** — #4 (route the TUI through the daemon / Windows story), #13 (mutex
   poison recovery + zeroizing), #20 (checked length guard).
2. **Approved direction to land** — #1 peer-UID bond (`SO_PEERCRED`/`getpeereid`).
3. **Planned, high consensus** — #9 / #10 (CI `cargo audit` gate + `ratatui` bump),
   #2 / #5 (policy engine + signed/pinned policy config), #3 (client-side key derivation).
4. **To check / decide** — #6 (zeroize secret/passphrase strings), #11 (release-artifact
   integrity), #12 (passphrase entropy floor).
5. **Remaining planned** — #8, #14, #16 (incl. machine-ID-to-hardware bind idea), #17, #22.
