# Independent Security Review: Svault

**Project**: Svault (nim444/Svault)
**Version reviewed**: 0.5.0
**Review date**: 2026-05-29T12:00:00Z
**Reviewed by**: DeepSeek-V4-Pro (via opencode)
**Review location**: Local clone at `/Users/nk/Projects/nk-os/repos/svault`

---

## Executive Summary

**Overall rating: Low-Medium risk for personal/team use on Unix; Medium-High for server/CI/CD; High for Windows or non-Unix environments.**

The 0.5.0 daemon is a genuine security win on Unix. It moves the session from a passphrase-on-disk file (`.session`) to an in-memory daemon with auto-lock, zeroization, and 0600 socket isolation. Cryptographic primitives (AES-256-GCM, Argon2id, HMAC-SHA256) are correctly applied. The policy engine is well-structured. However, the tool has material gaps for corporate adoption: no daemon on Windows (passphrase lands on disk un-hardened), no transport encryption on the UDS (passphrase travels as JSON string), mutex unwrap calls that could crash the daemon, and no connection rate-limiting. Svault is suitable for individual developer and small-team AI agent workflows on Unix today. For CI/CD servers or mixed-platform environments, the gaps are significant and would require compensating controls.

> **Daemon impact**: The daemon eliminates the passphrase-on-disk exposure on Unix — the most impactful security improvement in this release. Without it, every `unlock` writes the user's passphrase to a file.

---

## Scope & Methodology

- **Full source review** of all 17 Rust source files (~3,200 lines)
- **Tools executed**: `cargo test` (74/74 passing), `cargo clippy --all-targets -- -D warnings` (clean), `cargo fmt --all --check` (clean), `cargo audit` (2 informational warnings via transitive deps)
- **CI/CD review**: 7 workflow files (lint, test/reusable, ubuntu, fedora, macos, windows, release)
- **Documentation review**: daemon.md, security.md, policy-engine.md, recovery.md
- **Special focus**: daemon.rs (937 lines), client.rs (128 lines), session.rs (116 lines), main.rs (1334 lines)

---

## Security Findings

### Medium Severity

| # | Finding | Location | Impact |
|---|---------|----------|--------|
| 1 | **Passphrase travels in plaintext over UDS** | `daemon.rs:133` — `Request::Unlock { passphrase: String }` | The raw passphrase is transmitted as a JSON field over the Unix socket. While UDS is local-only and the socket is 0600, the passphrase is recoverable from a kernel buffer or by a process that can read the socket fd. The daemon was designed to avoid passphrase-on-disk; ironclad would avoid it on-wire too by having the client derive the key and send only a pre-authenticated op. |
| 2 | **Mutex poisoning panics kill the daemon** | `daemon.rs:143,152,158,165,186,280,305` — `store.lock().unwrap()` | 7 production-path calls use bare `.unwrap()` on the Mutex. If a handler thread panics while holding the lock, the mutex is poisoned and every subsequent connection — including `LockAll` and `Shutdown` — will panic. The daemon dies without zeroizing keys (the `LockAll` fallback in `serve`'s `store.clear()` happens at normal shutdown, not on panic). A `PoisonError::into_inner()` recovery would prevent this. |
| 3 | **No per-connection authentication or rate limiting** | `daemon.rs:301-302` — `std::thread::spawn(move || serve_conn(...))` | Any process of the same user can connect and issue arbitrary commands with no further authentication. A compromised process running as the same user can exfiltrate all unlocked secrets, keep keys alive by periodic pings, or DoS the daemon by opening many connections. No `max_connections` or per-source rate limit exists. |
| 4 | **Windows session file has no permission hardening** | `session.rs:32` — `std::fs::write(&path, passphrase)?` | On non-Unix, the `.session` file is written without any permission control. On Windows this means the passphrase is accessible to any process running as the same user with no `0600` equivalent enforced. Combined with no daemon on Windows, this is a material gap. |

### Low / Informational

| # | Finding | Location |
|---|---------|----------|
| 5 | **Unlock passphrase String not zeroized** | `daemon.rs:133` — the `passphrase` in the `Unlock` variant is a plain `String` with no zeroization. It persists in memory until the JSON deserialization buffer is reused. |
| 6 | **No Unix socket SO_PASSCRED peer check** | The daemon doesn't verify the connecting process's PID/UID via `SO_PASSCRED` or `peer_cred()`. Currently `socket_mode() == 0o600` is the only access control. |
| 7 | **Daemon log file unrotated** | `daemon.rs:412-416` — `daemon.log` grows unbounded with `append(true)`. No rotation or size cap. On a long-running CI server, this could fill the disk. |
| 8 | **`setsid()` has no `SIGCHLD` handler** | `daemon.rs:425-429` — The daemon spawns via `setsid()` but the parent doesn't handle `SIGCHLD`. Orphaned daemon children could accumulate zombies in the parent process. |
| 9 | **No `--locked` flag in local workflow** | Release builds use `--locked`; CI test builds do not, meaning CI tests could pick up dependency versions not pinned in `Cargo.lock`. |
| 10 | **`cargo audit`: transitive advisories** | `RUSTSEC-2024-0436` (paste 1.0.15 unmaintained) and `RUSTSEC-2026-0002` (lru 0.12.5 unsound) — both via ratatui 0.29. These affect the TUI code path only, not the core daemon/crypto. The `lru` advisory (Stacked Borrows violation in `IterMut`) is technically unsound but only triggered through ratatui's internal layout cache; low practical risk. Upgrading ratatui would resolve both. |
| 11 | **`vault.rs:152` — raw slice without bounds check** | `encrypted[..SALT_SIZE].try_into().unwrap()` will panic if `encrypted.len() < SALT_SIZE`. Guarded by a prior `decrypt()` call which checks length, but the invariant isn't locally visible. |

---

## Positive Security Properties Observed

1. **AES-256-GCM with random per-encryption nonces** (`crypto.rs:50` — `Aes256Gcm::generate_nonce(&mut OsRng)`) — correctly generates fresh nonces for each encryption, preventing nonce reuse.
2. **Argon2id with strong parameters** (64 MB memory, 3 iterations) — GPU-resistant and tuned for interactive use. The 64 MB memory cost makes parallel brute force expensive.
3. **HMAC-SHA256 with constant-time comparison** (`meta.rs:198-206` — `constant_time_eq`) for `meta.yaml` integrity — prevents timing attacks on the signature check.
4. **Zeroize/ZeroizeOnDrop** on `VaultKey` (`crypto.rs:18`), `SecretStore` (`vault.rs:13`), and `Held` key in daemon (`daemon.rs:119`) — key material and decrypted secrets are wiped from memory on drop.
5. **Dual auto-lock timers** (`daemon.rs:80-82,274-287`) — idle timeout (15 min default) + hard max (8 h default) with a 10-second ticker. Correctly factored as a pure function (`is_expired`) for testability.
6. **Minimal critical section for concurrent Gets** (`daemon.rs:181-194`) — the mutex is held only long enough to clone the 32-byte key and bump `last_used`; AES-GCM decryption happens outside the lock.
7. **Shutdown zeroizes all keys** (`daemon.rs:305` — `store.lock().unwrap().clear()`) — on both graceful `Shutdown` and the accept-loop exit, all cached keys are dropped (zeroized).
8. **Secrets never logged** — `main.rs:812` ("never log the secret value"), audit log stores only decision metadata, usage log stores action type + target name only.
9. **Stale socket/pid cleanup** — `daemon.rs:369-373,596-601,606-621` — `run()` removes stale sockets before binding; `doctor` detects and can `--fix` stale artifacts left by crashes.
10. **Export bundle checksum integrity** (`portable.rs:28-38,94-98`) — SHA-256 over all included files, verified on import.
11. **Recovery code: 160 bits, shown once** (`recovery.rs:29-39`) — generated from OS CSPRNG, formatted as 10 groups of 4 hex chars, never stored in plaintext (only as a key-wrapping-key under Argon2id).
12. **Test suite**: 74 passing tests covering encryption roundtrips, wrong-key rejection, bit-flip tampering, HMAC tampering, policy engine (capabilities, tiers, rate limits, burst detection, fallback), recovery, export/import, daemon protocol + unlock-get-lock-shutdown + concurrent reads, usage logging, TUI key dispatch.
13. **CI on 5 platforms**: Ubuntu, Fedora, macOS, Windows, plus a lint job (rustfmt + clippy). Release builds use `--locked`.
14. **Passphrase strength advisory** (`passphrase.rs:3-40`) — warns on <12 chars, single character class, or common words, with user override.

---

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

### Architecture (daemon.rs:84-815, client.rs:16-128)

The daemon is a single-process, multi-threaded Unix socket server bound to `.svault/daemon.sock` (mode 0600). It holds derived `VaultKey`s (32-byte AES-256 keys) in an `Arc<Mutex<HashMap<String, Held>>>`. The protocol is newline-delimited JSON with 7 request types.

**Lifecycle**:
1. `svault daemon run` — foreground; binds socket, writes PID, spawns ticker, enters accept loop
2. `svault daemon start` — spawns `svault daemon run` in a detached session (`setsid()`), waits up to 5s for the socket to respond
3. `svault daemon stop` — sends `Shutdown`, SIGTERM fallback, cleans up socket/pid files

### Security Benefits

- **No passphrase on disk**: `svault unlock` with daemon running sends the passphrase to the daemon, which derives and caches the key in memory. No `.session` file is written. On lock/shutdown/auto-lock, the key is zeroized.
- **Auto-lock is real**: The background ticker (`daemon.rs:276-287`) runs every 10 seconds and drops keys past their idle or hard-max limit. Without the daemon, auto-lock is imaginary — the `.session` file sits on disk until manually locked.
- **Key-only, no passphrase exposure**: The daemon holds derived keys, not passphrases. A `Get` request returns a decrypted secret value, but the daemon never exposes the passphrase or the vault key over the socket. Write operations (`secret add`, `secret remove`) require the passphrase and bypass the daemon.
- **Zeroization on drop**: `Held.key` uses `Zeroizing<[u8; 32]>`, and when the `HashMap` entry is removed (by lock, auto-lock eviction, or shutdown), the key bytes are zeroized.

### Remaining Risks

| Risk | Detail |
|------|--------|
| Passphrase on wire | The `Unlock` request carries the raw passphrase as a JSON `String` field. While UDS 0600 limits access to the same user, the passphrase is in kernel socket buffers and in the daemon's deserialization buffer until Argon2 derivation completes. |
| No caller authentication | Any process of the same UID can send commands. A compromised child process can drain all secrets from unlocked vaults or keep keys alive with periodic `Ping`/`Status`. |
| Mutex poisoning | A single thread panic while holding the store lock poisons the mutex; all subsequent operations (including `Shutdown`) panic. |
| No transport encryption | The UDS carries JSON in plaintext. While UDS traffic doesn't cross a network boundary, `ptrace` or a kernel module could observe it. |
| Single daemon, single project | One daemon per `.svault/` directory. Multiple projects need multiple daemons or a single daemon with multi-root support (not implemented). |

### Permission Model

```
.svault/
  daemon.sock    mode 0600  (owner read/write only)
  daemon.pid     mode 0600  (owner read/write only)
  daemon.log     append mode (no explicit mode set, inherits umask)
  <vault>/
    vault.enc    encrypted at rest (AES-256-GCM)
    meta.yaml    HMAC-signed, safe to commit
    recovery.enc wrapped vault key
    audit.log    mode 0600
    usage.log    mode 0600
```

The `daemon.log` file is opened without an explicit `mode(0o600)` (lines 412-416) — it inherits the process umask, which may be more permissive than 0600.

### Auto-Lock

Correctly implemented as a pure function `is_expired(idle_secs, age_secs, idle_timeout, max_unlocked)` tested with three cases (idle timeout, hard max, within limits). The 10-second ticker granularity means a key could remain unlocked up to 10 seconds past its actual expiration. Acceptable for the threat model.

---

## Infrastructure, Server & CI/CD Deployment Considerations

### Unix Servers and CI/CD

With the daemon, Svault becomes viable for CI use on Unix:
- Start the daemon at the CI job start, unlock vaults, run the build, stop the daemon.
- The daemon's `start` → `unlock` → build → `stop` pattern keeps secrets in memory for the job duration.
- The hard-max timer (default 8h) covers long CI runs. The idle timer won't fire during active CI `get` calls.

**Recommended pattern**:
```bash
svault daemon start
svault unlock myvault
# ... CI steps using svault get ...
svault lock --all
svault daemon stop
```

**Gaps**:
- No CI-specific mode (the daemon is the same binary, same protocol). An env variable like `SVAULT_DAEMON_NO_IDLE` for CI jobs that should keep keys alive for the entire pipeline would be useful.
- No health check endpoint suitable for Docker health checks (would need a `SVAULT_HEALTHCHECK` env or similar).

### Windows and Non-Unix Environments

On Windows, the daemon does not compile (stubs return "Unix-only"). The file `.session` contains the passphrase in plaintext with no permission hardening (`session.rs:32` — `std::fs::write`). This makes Windows deployments strictly weaker:

| Aspect | Unix + Daemon | Windows |
|--------|---------------|---------|
| Session storage | In-memory, zeroized | File on disk, no permission control |
| Auto-lock | Real (daemon ticker) | None (file stays until manual lock) |
| Concurrent reads | Threaded, non-blocking | File-based, serial |
| Passphrase at rest | Never on disk | On disk in `.session` |

### High-Value Secrets

For production secrets (cloud API keys, signing keys, database credentials):
- The daemon reduces the exposure window but doesn't eliminate it. A kernel compromise or a process running as the same user can still extract unlocked secrets.
- Consider adding a `SVAULT_CONFIRM_HIGH` mode where `high`-tier secrets require explicit human confirmation even when the daemon is unlocked.
- No HSM integration. All key material lives in process memory.

---

## Recommendations for Corporate Adoption

1. **Eliminate passphrase-on-wire in Unlock (Medium → High priority).** Have the client derive the vault key via Argon2id and send a pre-authenticated `Unlock` token (HMAC of a server-provided challenge). This would make passphrase interception impossible even for a root process on the same machine.

2. **Add per-connection peer credential checking (Medium).** Use `SO_PASSCRED` / `peer_cred()` on the Unix socket to verify the connecting process's UID matches the daemon's UID. This is a low-cost defense against accidental cross-user access.

3. **Replace Mutex unwrap with poison recovery (Medium).** All `store.lock().unwrap()` calls in the daemon should use `lock().unwrap_or_else(PoisonError::into_inner)` to survive a poisoned mutex. Critical for a long-lived daemon.

4. **Set mode 0600 on daemon.log (Low).** Add `.mode(0o600)` to the `OpenOptions` in `start_quiet()` to prevent log file information disclosure.

5. **Add connection/rate limiting (Medium).** Cap concurrent connections (e.g., 128) and throttle `Unlock` retries to prevent brute force or DoS from a compromised same-user process.

6. **Zeroize the Unlock passphrase String (Low).** Wrap the `passphrase` field or explicitly zeroize the deserialized string buffer after `VaultKey::derive()` completes.

7. **Consider a CI/automation mode (Medium).** Add `SVAVLT_DAEMON_PASSPHRASE` env or a `--ci` flag that disables idle timeout for CI pipelines.

8. **Upgrade ratatui (Low).** Upgrading to ratatui ~0.30 would resolve both transitive advisory warnings (`paste` unmaintained, `lru` unsound).

9. **Add `--locked` to CI test builds (Low).** Align CI test builds with release builds by adding `--locked` to catch dependency drift.

10. **Windows hardening (High priority for mixed-platform teams).** Add Windows-equivalent permission controls (ACL-based) on `.session` or implement a named-pipe daemon for Windows.

---

## Overall Risk Assessment by Context

| Context | Risk | Notes |
|---------|------|-------|
| Personal use (Unix + daemon) | **Low** | Daemon keeps keys in memory, auto-lock works, zeroization on shutdown. Strong for individual dev workflows. |
| Personal use (Windows) | **Medium-High** | Passphrase on disk un-hardened, no daemon. Acceptable only with full-disk encryption + manual lock discipline. |
| Small team (Unix, all using daemon) | **Low-Medium** | Policy engine provides audit trail. Same-user processes can access daemon — team members must not share Unix accounts. |
| Corporate CI/CD with AI agents (Unix) | **Medium** | Daemon `start`/`unlock`/`stop` pattern works. No CI-specific mode. Hard-max timer (8h) may be exceeded by long pipelines. |
| High-security production infrastructure | **High** | No HSM, no per-connection auth, secrets travel plaintext over UDS, no TEE. Not recommended for production secrets without compensating controls. |
| Mixed Windows/Unix environments | **High** | Windows fallback weakens the overall posture. Teams must mandate Unix-only for secret access or accept the file-based risk on Windows. |
| Developer laptop + AI coding agent | **Low-Medium** | The core use case. Daemon provides reasonable isolation. Policy engine prevents agent from requesting high-tier secrets. Passphrase-on-wire risk is low on a single-user laptop. |

---

## References

- **Source files analyzed**:
  - `src/daemon.rs` (937 lines) — Daemon protocol, lifecycle, tests
  - `src/client.rs` (128 lines) — Daemon-aware routing layer
  - `src/crypto.rs` (159 lines) — AES-256-GCM + Argon2id + VaultKey
  - `src/vault.rs` (350 lines) — Vault open/init/rekey/secrets
  - `src/policy.rs` (569 lines) — Policy engine, tiers, rate/burst detection
  - `src/meta.rs` (225 lines) — Metadata with HMAC integrity
  - `src/session.rs` (116 lines) — File-based session (non-daemon fallback)
  - `src/recovery.rs` (177 lines) — Recovery code, key wrapping
  - `src/portable.rs` (249 lines) — Export/import bundles
  - `src/audit.rs` (203 lines) — Audit log
  - `src/usage.rs` (296 lines) — Usage timeline
  - `src/main.rs` (1334 lines) — CLI commands, glue logic
  - `src/config.rs` (66 lines) — Daemon config
  - `src/passphrase.rs` (67 lines) — Passphrase strength checks
- **Configuration**: `Cargo.toml`
- **CI workflows**: `.github/workflows/*.yml`
- **Documentation**: `docs/daemon.md`, `docs/security.md`, `docs/policy-engine.md`
- **Tool output**: `cargo test` (74/74 passed), `cargo clippy` (clean), `cargo fmt` (clean), `cargo audit` (2 informational advisories)