**Project**: Svault (nim444/Svault)
**Version reviewed**: 0.7.0 (last released tag `v0.7.0`; working tree is `main` at commit `1b79c87`, which opens the 0.8.0 dev cycle with only a version bump + review-intake scaffold — the security-relevant code is identical to the 0.7.0 release)
**Review date**: 2026-05-30
**Reviewed by**: Claude (Opus 4.8)
**Review location**: Reviewed the local working tree at `/Users/nk/Projects/nk-os/repos/svault`; ran `cargo test`, `cargo clippy`, and `cargo audit` against it.

## Executive Summary

**Overall risk rating for medium/large corporate use: MODERATE — conditionally adoptable on Unix as an at-rest secret store and developer convenience; NOT yet suitable as the access-control boundary its "gates agent access" marketing implies.**

The cryptographic core is sound and conservative (AES-256-GCM + Argon2id, HMAC-signed metadata, authenticated decryption, 160-bit recovery codes). The 0.6.0/0.7.0 hardening is real and verifiable: the passphrase no longer crosses the socket, the daemon holds keys in memory only and refuses non-self UIDs, at-rest files are owner-only with a TOCTOU-free umask bind, keys are zeroized on the major lifecycle events, and the supply chain has a `cargo audit` gate plus SLSA provenance. The daemon **materially improves** the posture over the old `.session` file on Unix by keeping keys off disk.

The decisive caveat is the one the project itself flags: **the policy engine is advisory, not enforced.** `svault get`'s policy/scope/tier/caller checks live entirely in a CLI binary that runs as the same user as the agent, and the daemon's `Get` path performs **no** policy or audit check at all. A same-UID process bypasses the entire policy layer trivially — by calling `svault secret get`, by speaking the JSON protocol to the socket directly, or by lying about `--caller`. Caller identity is self-asserted and unauthenticated. For a security committee, this means Svault's policy/tiering features are best understood as **logging and guardrails for cooperative agents**, not as an enforcement boundary against a compromised or adversarial one.

## Scope & Methodology

- Read the full security-relevant source: `crypto.rs`, `vault.rs`, `daemon.rs` (full, incl. tests/stress harness), `client.rs`, `policy.rs`, `secfile.rs`, `session.rs`, `passphrase.rs`, `recovery.rs`, `portable.rs`, `meta.rs`, `config.rs`, `audit.rs`, and the `get`/`create`/`recover`/`unlock` paths in `main.rs`, plus the TUI reveal path.
- Read the documented threat model (`docs/security.md`, `docs/daemon.md`, `docs/policy-engine.md`).
- Reviewed CI/release workflows (`lint.yml`, `release.yml`).
- Ran tooling on the tree:
  - `cargo audit` → **clean**, exit 0, 0 advisories across 257 dependencies.
  - `cargo test --all` → **84 passed, 0 failed, 1 ignored** (the manual stress benchmark).
  - `cargo clippy --all-targets --all-features -- -D warnings` → **clean**.
- Special focus on `daemon.rs` and `client.rs` as instructed, and on independently tracing whether the policy gate actually gates the daemon read path (it does not).

## Security Findings

### Critical / High Severity

**H-1 — The policy engine does not gate the daemon read path; policy is fully bypassable by a same-UID process.**
This is the headline finding, verified in code:
- `cmd_get` (`main.rs:790`) runs `policy::evaluate` and writes audit — but this is in the **client binary**, before it asks the daemon for the value.
- The daemon protocol (`daemon.rs:41`) is `Get { vault, secret }` — no caller, scope, reason, or tier. `handle()`'s `Request::Get` arm (`daemon.rs:223`) clones the key and decrypts with **no policy or audit call**.
- `svault secret get` (`cmd_secret`, `main.rs:624-647`) routes straight to `client::get` → daemon `Get` with **no policy evaluation at all**. This is a documented, first-class path (`docs/daemon.md:31`, `docs/policy-engine.md:5` "the human path… no questions asked").

Consequently a same-UID agent has at least four trivial bypasses: (1) call `svault secret get NAME`; (2) connect to `daemon.sock` and send `{"cmd":"get",...}` directly; (3) read the `.session` key file and decrypt `vault.enc` itself; (4) pass any `--caller`/`SVAULT_CALLER` it likes. The `high`-tier "never handed to an agent" guarantee (`policy.rs:265`) therefore does **not** hold against a same-UID agent — only against one that voluntarily restricts itself to honest `svault get`. **Impact:** the product's central "knows an AI is asking / gates agent access" claim is not an enforceable control. **Recommendation:** for 1.0 "stable," either move evaluation server-side (daemon authenticates caller via a per-agent credential/token and runs policy on `Get`), or re-label the feature honestly as advisory/audit guardrails. This should block a 1.0.0 "enforced policy" claim.

**H-2 — Caller identity is unauthenticated (self-asserted).**
`caller` resolves from `--caller` → `$SVAULT_CALLER` → `"default"` (`main.rs:800-803`). There is no binding between the asserted caller and any OS-level identity. Per-caller `scopes` and `rate_limit` in the policy are therefore an honor system; an agent grants itself any caller's privileges by setting one string. Even within the cooperative-agent model this undermines per-caller differentiation.

### Medium Severity

**M-1 — Policy parse failure fails *open* to a permissive default.**
`policy::load()` (`policy.rs:127-131`) uses `serde_yaml::from_str(&content).ok()` — any YAML syntax error returns `None`, which silently drops the engine into `evaluate_fallback` (`policy.rs:279`) against `meta.yaml`, whose `allow_agent` **defaults to `Bool(true)` = allow all agents** (`meta.rs:18-21`). So an administrator who writes a restrictive `svault.policy.yaml` but introduces a typo gets a **silent downgrade to allow-all**, not an error. This affects the honest/cooperative deployment too (independent of the same-UID issue). **Recommendation:** distinguish "no policy file" from "policy file present but unparseable"; the latter should be a hard error (fail closed), not a fallback.

**M-2 — Policy is unsigned and located by an upward directory walk; CWD chooses the policy.**
`find_policy_file()` (`policy.rs:134-145`) walks from CWD to root. An agent that controls its working directory selects which policy applies — e.g. running from a directory with no ancestor policy file yields fallback allow-all. Combined with M-1's permissive default, "escape the project policy" is `cd` away. The file is also unsigned, so any same-UID write changes the rules. **Recommendation:** anchor the policy to the vault/`.svault` location rather than CWD, and consider signing it under the vault key (as `meta.yaml` already is).

**M-3 — Audit coverage gap: daemon/`secret get` reads are not audited as policy events.**
Only `cmd_get` writes `audit.log` entries. Reads via the daemon `Get` (whether through `svault secret get` or direct socket access) write at most a `usage` "secret.get" entry on the *human* timeline (`main.rs:630`) and **no `audit.log` policy entry**. A same-UID agent reading via `secret get` thus evades the agent audit trail entirely. Since audit is the realistic value of this product today, this is a real detection gap. **Recommendation:** have the daemon emit an audit record on every `Get`, tagged with the connection's peer UID and the request surface.

**M-4 — The gate reads *unverified* metadata.**
`cmd_get` uses `VaultMeta::load_unverified` (`main.rs:798`), and `evaluate_fallback` keys its allow/deny on `meta.access.allow_agent` from that unverified copy. The HMAC machinery exists (`meta.rs:157`, and a test proves tampering is caught via `load_verified`) but is not used on the decision path. A same-UID attacker can flip `allow_agent` in `meta.yaml` to defeat fallback denial without knowing the key. (Severity is bounded by the broader same-UID model, but the project ships the integrity primitive and then doesn't apply it where it gates access.)

**M-5 — Windows at-rest protection is weaker and has a creation-time window.**
`secfile::restrict_to_owner` (`secfile.rs:48-58`) writes the file first with default/inherited ACLs, *then* runs `icacls /inheritance:r`. There is a TOCTOU window where the file (incl. the `.session` key-equivalent, since Windows has no daemon) carries inherited ACLs. It is also best-effort: it silently no-ops if `%USERNAME%` is unset or `icacls` fails/returns nonzero (the result is discarded). On a multi-user Windows host this is materially weaker than the Unix `0600`-at-open guarantee. **Recommendation:** create the file with a restrictive DACL atomically (e.g. `CreateFile` with a SECURITY_ATTRIBUTES owner-only ACL) rather than write-then-chmod, and surface failures.

### Low / Informational

- **L-1 — Incomplete zeroization (plaintext residue).** `Vault::load_secrets` (`vault.rs:152-157`) deserializes the entire secret set into a `HashMap<String,String>` of plain (non-`Zeroizing`) `String`s on every get/add/list/remove; only the single returned value (`get_secret`, `vault.rs:130`) and the transient JSON wrapper (`SecretStore`) are wiped. The daemon's `Response::Secret { value }` and the serialized JSON reply buffer (`reply`, `daemon.rs:259-265`) are likewise plain `String`s. Zeroization is therefore partial — the whole-vault plaintext and transport copies linger in freed heap until overwritten. Defensible given the at-rest/cooperative model, but the "secrets are Zeroizing" claim is stronger than the implementation.
- **L-2 — AES-GCM uses random 96-bit nonces** (`crypto.rs:50`) and re-encrypts the whole vault per mutation. Safe for realistic vault mutation volumes (birthday bound ~2³² encryptions/key is far away); noting it for completeness — there is no nonce counter, so very high write volumes would eventually matter.
- **L-3 — Same-UID DoS of the daemon is possible but bounded.** A same-UID process can hold `max_connections` (default 512) idle connections; the per-connection 30s read timeout and `ConnGuard` slot accounting (`daemon.rs:112-117`) bound it, and real clients fall back to a prompt on "busy." No secret exposure; degraded availability only. Documented.
- **L-4 — `.session` is key-equivalent at rest.** Owner-only, zeroized-then-deleted on lock — but a stolen `.session` opens that vault outright (it caches the raw derived key). This is the correct trade-off (it no longer leaks the reusable passphrase) and is clearly documented; flagging so adopters size the residual risk.
- **L-5 — `recovery.enc` / export bundles are key-equivalent ciphertext.** Safe to keep at rest (wrapped under a 160-bit code), owner-only. Export bundles are gitignored best-effort. Sound.

## Positive Security Properties Observed

- **Cryptography is conservative and correct.** AES-256-GCM with authenticated decryption (tamper test passes), Argon2id at 64 MiB / t=3 (GPU-resistant, sensible params), per-encryption random nonce, 160-bit CSPRNG recovery codes, HMAC-SHA256 on metadata with a constant-time comparison (`meta.rs:198`). `VaultKey`/`SecretStore` are `ZeroizeOnDrop`.
- **Passphrase no longer crosses the socket (#3) — verified.** `client::unlock` (`client.rs:38-57`) opens the vault locally, sends only the hex key; the daemon validates it opens the vault before caching (`daemon.rs:176-186`). Wrong passphrase fails client-side.
- **Peer-UID bond (#1) is correct and fail-closed.** `peer_is_self` (`daemon.rs:370-406`) uses `SO_PEERCRED` on Linux and `getpeereid` elsewhere, returns `false` on any error, and is checked before spawning a handler (`daemon.rs:430`).
- **TOCTOU-free socket creation (#16).** Bind under `umask(0o077)` then explicit `0600` (`daemon.rs:556-560`) — no world-readable window. PID file is `0600`.
- **Graceful shutdown (#17)** zeroizes keys and removes socket/pid on SIGTERM/SIGINT via an async-signal-safe atomic flag (`daemon.rs:338-363`, `457-460`); mutex poison recovery prevents one panicking handler from taking down all held keys (`lock_store`, `daemon.rs:149-153`).
- **Entropy floor (#12)** is enforced at create and recover with a `--force` escape (`main.rs:279-291`, `1089-1098`); the estimate is deliberately conservative (no dictionary credit) and layered with a separate common-word/variety check.
- **Supply chain.** `cargo audit --deny warnings` CI gate (verified clean locally), `--locked` release builds, SHA-256 checksums, and SLSA build-provenance attestation (`release.yml`). Clippy `-D warnings` and `rustfmt --check` gates.
- **Test discipline.** 84 passing tests including tamper/truncation/wrong-key negative cases, daemon concurrency, slot-leak, and poison-recovery tests.

## The Unix Daemon (0.5.0+) — Detailed Analysis

**Architecture.** One daemon per project `.svault/`, listening on a `0600` Unix socket, thread-per-connection, with an `Arc<Mutex<HashMap<vault, Held>>>` key store. The critical section is genuinely minimal — `Get` clones the 32-byte key and bumps `last_used` under the lock, then decrypts outside it (`daemon.rs:223-253`), so parallel reads don't serialize. A ticker thread evicts idle/hard-max keys every ~10s (`daemon.rs:324-335`).

**Security benefits over the file session.** Keys live in memory only; no `.session` key-equivalent is written when the daemon is up. Keys are zeroized on lock, auto-lock eviction, signal, and shutdown. Defense in depth: `0600` socket + umask bind + peer-UID bond. The socket exposes only scoped reads — no write operations — so even a same-UID caller can't *mutate* the vault through the daemon (mutations still require the passphrase).

**Remaining risks.** (1) The `Get` path is unauthenticated beyond same-UID and runs no policy/audit (H-1, M-3) — the daemon is the very component that should host enforcement and currently hosts none. (2) Same-UID is the entire trust boundary; the daemon is explicitly *not* a sandbox against a hostile same-UID process. (3) Auto-lock is wall-clock/idle based and effective, but a busy agent keeps a vault unlocked up to the 8h hard cap by default — reasonable, tune per environment. (4) Transport copies of secrets aren't zeroized (L-1).

**Operational implications.** The connection ceiling (#8), per-connection read timeout, `ConnGuard` slot accounting, connect-retry with backoff, and `daemon doctor [--fix]` for stale socket/pid cleanup are all production-minded and tested. The stress harness exists (ignored by default) and asserts zero wrong values under load. This is a well-engineered daemon; its gap is policy, not robustness.

## Infrastructure, Server & CI/CD Deployment Considerations

- **Improvements from the daemon (Unix).** On a server/CI box, the daemon keeps keys off disk between uses and auto-locks them — a real improvement over a persistent `.session`. Scoped-read-only socket means a leaked socket handle can't add/remove secrets.
- **Recommended safe usage on Unix today.** Treat Svault as an **encrypted at-rest store + audit/guardrail layer for cooperative agents**, not as an enforcement gate. Run one dedicated low-privilege UID per trust domain (the daemon's whole boundary is same-UID — co-locating a hostile workload under the same UID defeats it). Keep the hard-max/idle timers short on shared hosts. Do not rely on `high`-tier or per-caller scopes to stop a determined or compromised same-UID agent; they won't (H-1, H-2). Validate that your `svault.policy.yaml` actually parses (a typo silently means allow-all, M-1).
- **CI/CD with AI agents.** Acceptable for low/medium-sensitivity secrets where the audit trail and cooperative gating add value, *provided* you understand any step running as the build UID can read everything the daemon holds. For high-value secrets, prefer a short-lived-token broker / cloud secret manager that authenticates the caller out-of-process.
- **Windows and non-Unix.** No daemon — falls back to the `.session` key file, protected only by best-effort `icacls` with a creation-time window (M-5). On multi-user Windows hosts, treat at-rest protection as weaker; prefer Unix for anything sensitive.
- **High-value production infrastructure.** Not recommended as the primary control today, precisely because the access decision is not enforced where the key lives.

## Recommendations for Corporate Adoption (prioritized)

1. **(Blocker for a 1.0 "enforced policy" claim)** Move policy evaluation and audit into the daemon `Get` path, and authenticate the caller out-of-process (e.g. per-agent token, or bind caller→peer credentials). Until then, document the policy engine as advisory. (H-1, H-2, M-3)
2. **Fail closed on policy parse errors** and anchor policy discovery to the vault, signed under the vault key. (M-1, M-2)
3. **Verify metadata signatures on the decision path** (`load_verified`, not `load_unverified`). (M-4)
4. **Harden the Windows at-rest path** with an atomic owner-only DACL at create time; surface failures. (M-5)
5. **Complete zeroization** of the in-memory secret map and daemon transport buffers, or soften the "secrets are zeroized" claim. (L-1)
6. Keep the strong supply-chain posture; consider a `cargo-deny` license/duplicate gate alongside `cargo audit`.

## Overall Risk Assessment by Context

| Context | Rating | Notes |
|---|---|---|
| Personal use (Unix + daemon) | **Low** | Strong crypto, keys in memory, good ergonomics. Fit for purpose. |
| Small trusted team (Unix) | **Low–Moderate** | Fine as encrypted store + audit; don't lean on policy enforcement. |
| Corporate CI/CD with AI agents (Unix) | **Moderate** | Useful audit/guardrails; **not** an enforcement boundary — any same-UID step reads all held secrets. Low/medium-sensitivity secrets only. |
| High-security production infrastructure | **High (not recommended as primary control)** | Access decision isn't enforced where the key resides; prefer an authenticating broker. |
| Mixed / Windows environments | **Moderate–High** | No daemon; best-effort `icacls` with a creation window. Weaker at rest. |

## References

- `src/daemon.rs` — protocol (`:24-71`), unauthenticated `Get` handler (`:223-253`), peer-UID bond (`:370-406`), umask/`0600` bind (`:556-560`), signal/shutdown (`:338-363`, `:457-461`).
- `src/client.rs` — client-side key derivation (`:38-57`), `get` routing (`:88-102`).
- `src/policy.rs` — pipeline (`:193-277`), fail-open `load()` (`:127-131`), CWD-walk `find_policy_file` (`:134-145`).
- `src/main.rs` — agent gate `cmd_get` (`:790-890`), unaudited `secret get` via daemon (`:624-647`), self-asserted caller (`:800-803`), entropy-floor enforcement (`:279-291`, `:1089-1098`).
- `src/crypto.rs`, `src/vault.rs` (`load_secrets` residue `:152-157`), `src/meta.rs` (HMAC `:148-206`), `src/secfile.rs` (Windows ACL `:48-58`), `src/session.rs`, `src/recovery.rs`, `src/passphrase.rs`.
- `docs/security.md`, `docs/daemon.md`, `docs/policy-engine.md`.
- `.github/workflows/lint.yml` (audit/clippy/fmt gates), `.github/workflows/release.yml` (checksums + SLSA provenance).
- Tooling run locally: `cargo audit` (clean), `cargo test` (84 pass / 1 ignored), `cargo clippy -D warnings` (clean).
