security: audit tail-truncation detection, writeRoots confinement, constant-time token

[askalf]

What

Three hardening fixes from a security review of redstamp's trust boundary. None is a classifier bypass (the risk classifier held up to an adversarial read); each closes a defense-in-depth gap where a stated guarantee did not fully hold, verified by exploit repro before the fix and by regression tests after.

1. Audit tail-truncation is now detectable (the substantive one)

A hash chain proves no past record was edited or deleted from the middle, but a valid prefix still verifies — so an attacker who can write the log could delete the most-recent entries (the ones recording their own action) undetected, and a restart re-seeded from the truncated tail. SECURITY.md listed exactly this ("deletion of a past audit entry that verifyAuditFile() fails to detect") as in-scope.

• ChainedFileAudit can anchor the chain head in a 0600 <audit>.chk checkpoint ({ checkpoint: true }, which the streaming daemon now enables).
• verifyAuditFile(path, { head, count }) pins the checkpointed prefix: fewer records than promised → truncated; the count-th record's hash not matching → rollback (catches truncate-and-regrow). Growth past the checkpoint still verifies, so a stale checkpoint never false-alarms.
• Honest bound (in SECURITY.md + CHANGELOG): the same-fs sidecar catches accidental truncation and a naive attacker; a same-directory attacker who rewrites both is defeated only by a checkpoint retained on separate-trust storage (now expressible via the expected arg).

2. writeRoots confinement no longer bypassable by .. / shared-prefix sibling

The gate compared paths with a raw startsWith, so src/../../etc/x (traverses out) and a data root admitting database/… (shared string prefix) both slipped the review gate. Paths are now normalized (./.. collapsed) and matched on a separator boundary.

3. Daemon capability token compared in constant time

req.token !== token returned early on the first differing byte — a timing oracle a local process on the loopback listener could walk. Now a fixed-length SHA-256 digest compare via crypto.timingSafeEqual (constant-time and length-independent).

Verification

• Exploit repro before → gated/detected after, for all three (traversal + sibling now flagged; truncation → { ok:false, reason:'truncated' }).
• Tests: +11 (audit truncation / rollback / growth-past-checkpoint / regrow, writeRoots traversal + sibling, token accept/reject/length-mismatch). Full suite 199/199.
• npm run bench unchanged: 97% recall, 100% precision.
• Back-compat: verifyAuditFile(path) with no checkpoint returns the exact legacy shape.

Not addressed (noted for follow-up)

The 64 KB input scan cap (index.mjs) can drop a secret past detection on a padded NET body — a deliberate DoS tradeoff; a targeted fix (scan a window around each detected external host) is a separate change.

[sprayberry-reviewer]

Automated review from the Sprayberry Labs fleet code reviewer.

Verdict: no blocking issues — approving.

Reviewed: src/audit.mjs, src/daemon.mjs, src/index.mjs, src/cli.mjs, and all three test files. All CI checks pass (199/199 tests, CodeQL clean, arena benchmark unchanged).

---

What I checked

1. Audit tail-truncation (src/audit.mjs)

The fix is sound. The key logic path:

• chainStateOf counts only records with both prev and hash present — exactly the same filter verifyAuditFile uses for n. So checkpoint.count and the verification counter n are always over the same population; a mixed chained/unchained log doesn't produce a false truncation alarm.
• After appendFileSync succeeds, this.prev and this.count are updated before writeCheckpoint is called. If appendFileSync throws, neither state nor sidecar advances — no partial write.
• wantCount > 0 is enforced on line 162, making wantCount always null or a positive integer. The truthy guard on line 169 (if (wantCount)) is therefore consistent and can't misfire on 0.
• The wantHead-only branch (lines 190–191) correctly handles callers who pass { head } without a count — it checks prev !== wantHead after the full scan, which is the right sentinel for a head-only checkpoint.
• Growth past a stale checkpoint (n > wantCount) correctly falls through without flagging; only the pinned prefix is verified.

Adversarial trace verified: attacker truncates a 3-record log to 1 record; n ends at 1, wantCount is 3; line 188 fires { ok: false, reason: 'truncated' }. ✓

2. writeRoots path confinement (src/index.mjs)

normPath uses path.posix.normalize after unifying separators, then strips a trailing / without a regex (correctly noted as ReDoS-avoidance). withinRoots uses root + '/' as the separator boundary, not a bare startsWith. Verified manually:

src/../../etc/x  → normPath → '../etc/x'   → startsWith('src/') = false ✓
database/evil    → normPath → 'database/evil' → startsWith('data/') = false ✓
src/deep/a.ts    → normPath → 'src/deep/a.ts' → startsWith('src/') = true ✓
/ root           → target.startsWith('/') = true for any path ✓

The absolute-root case (root === '/') is handled by the ternary on line 42, avoiding a double-slash comparison. Clean.

3. Constant-time token comparison (src/daemon.mjs)

tokenDigest is a 32-byte Buffer (SHA-256). Any presented token — regardless of its string length — also produces a 32-byte digest, so crypto.timingSafeEqual never throws on a length mismatch. The typeof presented === 'string' ? presented : '' guard prevents a TypeError if a non-string token arrives from a malformed request. Verified: absent (undefined), short, and same-length-but-wrong tokens all produce a digest that does not compare equal. ✓

4. Tests

Eleven new tests cover exactly the scenarios addressed:

• Truncation, restart-across-checkpoint, file-deleted, rollback, truncate-and-regrow, stale-checkpoint growth, and back-compat (no sidecar written, legacy return shape preserved).
• Traversal, sibling directory, normal inside-root write.
• Token accept, wrong same-length, length-mismatch, and absent.

Every attack scenario the PR describes in its changelog has a corresponding regression test that would have caught the pre-fix behaviour.

---

What's good

The threat model is stated honestly and precisely in SECURITY.md: the sidecar catches accidental truncation and a naive attacker; a same-directory attacker who rewrites both files is explicitly documented as out of scope for the sidecar, with a clear path to full protection via separate-trust storage. This is exactly the right level of precision for a SECURITY.md clause. The "not addressed" note on the 64 KB scan cap is also a good-faith disclosure.

No blocking issues found.