Phase log 12: 2026-08-07 - Audit and phases 7–10.1

Full-codebase audit followed by four implementation passes. Versions moved 0.5.0/0.6.0 (mixed) → 0.6.0 everywhere, phases 7 → 10.1.

Goal

Audit the whole codebase, then: fix all bugs found, delete dead weight, remove TOTP and third-party API dependencies, unify version drift, and work through the tier 1–4 improvement list. Mid-session the scope grew to include an Open to LAN feature and a boolean permission expression language.

Current Progress - all complete and verified

Verification at handoff:

cargo test --workspace        83 passed, 0 failed   (76 vault-core + 7 envv-server)
cargo check --workspace --all-targets   clean, zero warnings
npx tsc --noEmit              clean
npx vite build                ✓ built
element-id audit              no unresolved lookups

Started from 0 tests and no CI. Now 83 tests plus a GitHub Actions workflow (cargo test --workspace, cargo check --all-targets, tsc --noEmit, vite build).

The bugs that mattered

Listed because each was silent - nothing surfaced them at runtime.

1. escAttr did not escape & (utils.ts). A secret containing & was HTML-decoded on read-back, so copy-to-clipboard returned the wrong secret. Worst bug in the codebase.
2. Two disagreeing entry identities. save_vault keyed on provider|account_name, the RBAC merge on provider|account_name|key_id. Version history and audit rows could land on the wrong entry. Unified on a stable id, backfilled on load.
3. Master password stored as a sub-user hash. The desktop seeded an admin row whose password_hash was the master password, turning the sub-user login into an oracle for the vault key. Removed.
4. Username enumeration oracle. verify_user_password returned Err for a passwordless user (→ 500) but Ok(None) for a nonexistent one (→ 401), and only the 401 path incremented the rate limiter. Unthrottled. Both are ordinary credential failures now.
5. "Lock" did not lock. DELETE /api/unlock dropped only the caller's session while every user session held its own copy of the vault key. Owner lock is now global.
6. Sessions never expired. A leaked token was valid until process restart. Now an idle TTL (--session-ttl-mins, default 480) that every authenticated request slides - which is what finally made /api/ping mean anything.
7. Keep-alive never worked. It read a _token field that does not exist (the field is token), so every ping went out as Bearer and was rejected.
8. Settings save disconnected your remote vault. saveSettings() called applyRemoteConfig(), which reassigned st.store from an unchecked toggle. Opening and closing Settings while connected silently dropped you to local.
9. The chunk secret type could never be saved. It was in the dropdown, but dynamicSecretFields hid its key input while saveModal still required api_key. Removed; existing entries relabel to env_var.
10. Lost-update window (see Pass 3.1 below).

Also: remote status checks used bare fetch() and reported "Unreachable" for any self-signed HTTPS server; expand/reveal state was array-index-keyed so it jumped cards after deletes; lock-on-hide fired on every alt-tab; cron month names were off by one and nextFireTimes could not see past ~7 days; Exporter.yaml emitted invalid YAML for quoted values; card rendering was O(n²).

Decisions made this session

Recorded so they are not re-litigated. All were the user's explicit choices.

Pass 3.1 - the lost-update fix (most recent work)

Opening to LAN made the desktop and its peers concurrent writers on one database. The desktop wrote the whole blob unconditionally, so a peer's edit landing between the desktop's load and its next save vanished silently.

The check now lives inside save_vault, in the same BEGIN IMMEDIATE transaction as the write:

#![allow(unused)]
fn main() {
pub struct SaveCtx<'a> {
    pub actor: Option<&'a str>,
    pub expect_version: Option<&'a str>,   // None = unconditional
}
}

Placement is the whole point. A caller that reads, compares, then writes has a race between the compare and the write - which is what the server's If-Match handling was. A caller that forgets is silently unprotected - which is what the desktop was.

Three things surfaced while doing it:

• Audit rows were written outside the transaction, so a rejected write still logged changes that never happened. Now inside; test asserts no trace.
• The GET ETag and the CAS value could disagree. The ETag re-hashed parsed JSON; the CAS compares the stored data_hash. They match today only because serde_json::Value iterates sorted. The ETag now reads the stored value.
• Read ordering is load-bearing. Version-then-data can only pin an older version, which fails the CAS and retries. Data-then-version passes the check while merging a stale base - the clobber itself. Commented at the site.

What Worked

• Mechanical id auditing. Before/after diffing every id="..." against git show HEAD:index.html, cross-checked against every getElementById, caught what types and builds cannot. Used for the index.html split and the crossed-DOM-id rename. Reuse this for any markup move.
• Verbatim extraction over retyping. Both large splits (chunk-ops.ts → chunks/, tool panes → tools-markup.ts) used sed line ranges, so no transcription errors were possible. The compiler then found the four missing imports.
• Letting tests correct the plan. Three RBAC tests failed on first run and the tests were wrong, not the code - entry_write_allowed genuinely required every declared scope. That surfaced the usability problem that led to the ANY-match relaxation.
• Structs over positional Option<&str>. SaveCtx exists because swapping actor and version would compile and pass every test.
• Asking before implementing. The user's "clear all confusions first" instruction avoided building the wrong permission model twice.

What Didn't Work

• rm -rf .claude/worktrees was blocked by the sandbox classifier. Not worked around. Still pending - see Next Steps.
• */n inside a JSDoc block closed the comment early and produced ~30 bogus TS errors. Rephrase step syntax in prose inside doc comments.
• Migration ordering, caught before it ran. The row→expression migration was initially placed before class seeding, which on a fresh vault would have left the built-in Admin/Moderator/Viewer classes with no rules - silently denying everything. Now after seeding, with a test.
• Read ordering in the merge path, first version had it backwards (above).

Next Steps

1. Run the blocked cleanup (needs you - reclaims 1.9 GB)

Verified safe: the worktree is orphaned (registered under the old /Code/APIV path, gitdir link broken) and its branch commit 6284b35 is already an ancestor of main - zero unique commits.

rm -rf .claude/worktrees; git worktree prune; git branch -D worktree-agent-a3723c0e15ae0afb5; rm -f architecture.dot

(target/ is a further 4.6 GB - cargo clean if wanted, at the cost of a full rebuild.)

2. Smoke-test - nothing below has ever been run

This is the single biggest gap. Everything is verified by tests, type checks, builds and id audits. No UI was rendered and no server was started at any point in this session. Run cargo tauri dev and walk:

1. Unlock - creates the owner row, migrates permission rows to expressions, backfills entry ids. Confirm Users shows owner.
2. Sidebar - Categories and Projects sections render (DOM ids were renamed).
3. Tools panel - panes are injected from tools-markup.ts, not in index.html any more. Check several, especially Audit Log (new).
4. Users → a user → Permissions - entirely new expression editor. Check validation, the predicate builder, and the "matches N of M" preview.
5. Open to LAN - Remote panel. Should refuse to start until a password-capable user exists.
6. Conflict path - with LAN open, edit the same entry from desktop and peer. Confirm the conflict dialog appears rather than silent loss.

3. Then commit

Large diff. Suggested split: dead-code deletion + versions · correctness fixes · auth model · permission expressions · LAN server · lost-update fix. Per the project rule, hand over a pwsh one-liner rather than committing.

Notes for whoever picks this up

• vault-core/src/permex.rs is authoritative for permissions. src/ts/permex.ts is an advisory mirror for editor validation and preview only. If you change semantics, change both - they can drift.
• SaveCtx::default() writes unconditionally. Only correct where nothing else can be writing. Everything reachable while LAN hosting must pass a version.
• entry_ck in vault-core/src/lib.rs is the one entry identity. Two disagreeing versions of this was bug 2.
• Two unrelated things are called "chunk". SecretType 'chunk' is gone; SecretChunk (WireGuard/Docker/nginx config sections) is alive and central.
• CLAUDE.md is stale in places - it still documents TOTP and the old phase list. README.md is current through Phase 10.1.
• The sidebar element ids were crossed (#project-tree held categories) and are now #category-tree / #project-list. The types.ts comments claiming a UI-label inversion were wrong and have been corrected.