Enforce TamperLog single-writer invariant: add exclusive file lock in koinon and serialize concurrent appends in kryphos

[forkwright]

Finding

The TamperLog hash chain depends on a strict single-writer invariant that is enforced at neither the library (koinon) nor the consumer (kryphos) layer. At the library level, TamperLog::open_with_config reads the current tail to recover (prev_hash, sequence, bytes_written) and then reopens the file with O_APPEND, taking no exclusive lock. At the consumer level, kryphos::Vault::append_vault_audit opens a fresh TamperLog handle and appends on every call, with no in-process synchronization across concurrent callers. Because Vault is Sync and its mutators take &self, two threads sharing one Arc<Vault> can each open the same log file and independently recover the same tail state, then both emit entries chained from that same predecessor — forking or clobbering the chain. Rotation is an additional check-then-act race: two writers can compute the same rotation number and both rename into {stem}.N.log, clobbering the segment the other just created.

Evidence

crates/koinon/src/tamper_log.rs:437 — state recovery with no lock held:

let (prev_hash, sequence, bytes_written) = Self::recover_state(&path)?;

crates/koinon/src/tamper_log.rs:439-443 — file reopened for append without any exclusive lock:

let file = OpenOptions::new()
    .create(true)
    .append(true)
    .open(&path)
    .context(IoSnafu { path: &path })?;

crates/koinon/src/tamper_log.rs:480-496 — append derives each entry's hash from the shared, now-stale prev_hash and assigns self.sequence; two concurrent writers emit entries carrying the same prev_hash and sequence.

crates/koinon/src/tamper_log.rs:578-581 — rotation is a check-then-act race:

let n = Self::next_rotation_number(&self.path);
let rotated = rotation_path(&self.path, n);
std::fs::rename(&self.path, &rotated).context(IoSnafu { path: &self.path })?;

On Unix rename replaces the target, so two concurrent writers computing the same n clobber the segment the other just rotated.

crates/kryphos/src/storage.rs:459 — the fixed log path is computed, then crates/kryphos/src/storage.rs:463 opens a new TamperLog handle on every mutation with no in-process mutex:

let mut log = TamperLog::open(self.tamper_log_path()).context(TamperLogSnafu)?;

crates/kryphos/src/storage.rs:99-103 — the documented concurrency posture claims protection only at the cross-process directory level, not across threads sharing one Vault handle:

/// The vault directory is advisory-locked to prevent concurrent access.

Why this matters

The tamper-evident audit log is the artifact an operator relies on to detect unauthorized credential mutation; chain integrity is its entire value. Under concurrent open, O_APPEND prevents byte-level write interleaving but not chain forking: two writers that both recover the same tail emit entries with the same prev_hash and sequence, so verify_chain returns ChainStatus::Broken for a log that was never tampered with. That cuts both ways: benign concurrency raises a false tamper alarm, and a genuine tamper can be dismissed as "just a concurrency artifact," destroying the evidentiary value the log exists to provide. The rotation rename race can additionally delete a just-rotated sibling segment outright, losing audit entries permanently.

Desired correction

In koinon, acquire an exclusive advisory lock (flock/fcntl via a lock crate) on the log path in open_with_config, failing fast if another writer already holds it, and hold the lock through rotation so the rename is atomic with respect to concurrent openers. In kryphos, guard the append_vault_audit open+append critical section with an in-process Mutex (or hold a single long-lived locked TamperLog handle on the Vault) so concurrent vault mutations append strictly serially. Done when: a second TamperLog::open on a path already held by a live writer returns an error or blocks rather than producing an interleaved verify_chain-Broken file; concurrent vault mutations on a shared Arc<Vault> produce a single verifiable non-forked chain; both behaviors are covered by multi-threaded tests.