Add real fault-injection crash-safety testing; investigate TSan for real

Prompted by a direct question about data-integrity trustworthiness: the
crash-safety design (self-checking journal records, atomic-rename
compaction, Section 4.3/4.4) had never actually been tested against a real
crash -- only reasoned about statically and tested against an
already-corrupted file (test_pack_overlay.c, a different scenario).

Added tests/test_crash_consistency.c: real fork()+SIGKILL fault injection
against an actual child process, not a hand-truncated file standing in
for a crash, with empirically-calibrated kill-delay sampling (measured via
throwaway scripts, not guessed) so trials land genuine mid-operation
interruptions rather than always completing first:

- Journal-write crash injection (25 trials): kills a child mid-burst of
  individually-journaled, individually-fsynced writes; verifies a strict
  clean-prefix recovery (every completed write present and correct, every
  write after the kill cleanly absent, no gaps). 20-22/25 trials per run
  land a genuine interruption; zero corruption found.
- Compaction crash injection (40 trials): kills a child mid-vfs_sync;
  verifies every write durably journaled *before* vfs_sync was called
  survives regardless of whether compaction itself completed. 40/40
  trials per run land a genuine interruption; zero corruption found.

A real finding from building this test, not from the code under test: the
first draft reported 128 failures, every one a bug in the test itself --
it couldn't distinguish "the child was killed before ever attempting this
write" from "the write completed and was then lost," since SIGKILL can't
be caught to report progress. Fixed with an independent progress
side-channel (plain write()+fsync() on a separate file, entirely outside
packfs) as ground truth. Documented in CLAUDE.md's new "Known reliability
characteristics" section, including this finding, because a claim is only
as trustworthy as what's measuring it.

Also investigated whether this sandbox's TSan block is actually
unfixable, rather than re-asserting the known limitation: tried
personality(ADDR_NO_RANDOMIZE) directly (EPERM), setarch -R (fails
identically), searched TSAN_OPTIONS for a bypass (none exists), and
checked capabilities/seccomp/unshare --user (zero effective caps, active
seccomp filter, namespace escape also blocked). Confirmed categorical and
documented as such in CLAUDE.md/CONTRIBUTING.md, rather than left as an
unexamined "it doesn't work here."

Also closed a real documentation gap SECURITY.md had: the pack integrity
checksum's non-cryptographic nature (FNV-1a64, already documented in the
context of the compaction-dedup bug) had never been explicitly connected
to its *other* use, load-time pack integrity validation -- added, since
it's a real, relevant caveat for anyone relying on that checksum against
a deliberate adversary rather than accidental corruption.

Verified: clean make all + make test (all 7 binaries), full ASan/UBSan
sweep of all 7 binaries with zero real findings, and the new test run
standalone 4+ times (this session) with stable, consistent results.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqJpkdJ6Njnt1pw3CbghzB

CLAUDE.md

@@ -19,7 +19,7 @@ A single test: `make build/test_mem && ./build/test_mem` (substitute any `test_*
 
 Sanitizer builds are not wired into `make test` (they need per-file compilation with sanitizer flags plus `-D_GNU_SOURCE -Iinclude -Isrc`); see `.gitea/workflows/ci.yml` (Gitea Actions) for the exact invocation, which CI runs on every push. **Any change to `upper.c`, `overlay.c`, or `vfs.c` must be verified under `-fsanitize=address,undefined` and `-fsanitize=thread` before being considered done** — this is not a formality: exactly this process caught a real use-after-free in the snapshot-reclamation logic during initial development (see the `reclaim_gate` note below), which the plain build and even repeated plain test runs never surfaced.
 
-**If your environment cannot run ThreadSanitizer at all, say so rather than skipping it silently.** TSan needs `personality(ADDR_NO_RANDOMIZE)` to disable ASLR for itself; some sandboxes block that syscall outright, in which case *every* TSan build fails identically (`FATAL: ThreadSanitizer: unexpected memory mapping`), including a trivial unrelated pthread program — that is the confirming test, not a PackFS-specific symptom. **This project's own CI runner is one of them, confirmed by its first real run** (see `CONTRIBUTING.md` for the details and for exactly how `.gitea/workflows/ci.yml`'s TSan step now tells that specific, known failure apart from a real finding rather than either hiding it or failing the build over it) — a green CI run is therefore not proof TSan executed. In that situation, ASan/UBSan are still run and still matter (they are what actually caught the `reclaim_gate` bug above), but they do not perform TSan's happens-before race analysis and are not a substitute for it; a change is TSan-verified only once it has actually passed on a machine confirmed able to execute it, never merely because ASan/UBSan passed or because a CI step reported success.
+**If your environment cannot run ThreadSanitizer at all, say so rather than skipping it silently.** TSan needs `personality(ADDR_NO_RANDOMIZE)` to disable ASLR for itself; some sandboxes block that syscall outright, in which case *every* TSan build fails identically (`FATAL: ThreadSanitizer: unexpected memory mapping`), including a trivial unrelated pthread program — that is the confirming test, not a PackFS-specific symptom. **This project's own CI runner is one of them, confirmed by its first real run** (see `CONTRIBUTING.md` for the details and for exactly how `.gitea/workflows/ci.yml`'s TSan step now tells that specific, known failure apart from a real finding rather than either hiding it or failing the build over it) — a green CI run is therefore not proof TSan executed. In that situation, ASan/UBSan are still run and still matter (they are what actually caught the `reclaim_gate` bug above), but they do not perform TSan's happens-before race analysis and are not a substitute for it; a change is TSan-verified only once it has actually passed on a machine confirmed able to execute it, never merely because ASan/UBSan passed or because a CI step reported success. **This has been actively investigated, not just accepted at face value:** in this sandbox, `personality(ADDR_NO_RANDOMIZE)` itself returns `EPERM` when called directly (confirmed via a raw syscall, not just observed via TSan's own error); `setarch -R` (which just calls the same syscall) fails identically; there is no `TSAN_OPTIONS` flag that relaxes the startup memory-mapping check (checked against the runtime's own `help=1` option listing); the process holds zero effective capabilities (`CapEff` all-zero) under an active seccomp filter (`Seccomp_filters: 1`) that also blocks `unshare --user`, ruling out working around it via a nested, less-restricted namespace. This is a categorical, syscall-level restriction with no userspace workaround available in this environment, not a configuration this project has simply failed to find yet.
 
 **A related but separate flake affects ASan/UBSan themselves in that kind of sandbox, not just TSan:** a sanitizer-built test binary can non-deterministically fail to start with `AddressSanitizer:DEADLYSIGNAL`, occasionally as an unbounded repeating loop rather than a single line — a sandbox startup race, confirmed by it hitting different, unrelated binaries across repeated runs, each of which then passes cleanly on retry. See `CONTRIBUTING.md`'s workflow section for the full description and the required mitigation (`timeout`-wrap sanitizer runs in such an environment; treat `DEADLYSIGNAL` alone, without an actual `ERROR: AddressSanitizer` or `runtime error:` string, as inconclusive and re-run rather than as a finding).
 
@@ -34,7 +34,7 @@ Sanitizer builds are not wired into `make test` (they need per-file compilation
 - `src/containment.c` — `dir`-mount path containment (`openat2`/`O_NOFOLLOW` fallback) and opt-in Landlock hardening.
 - `src/path.c` — virtual-namespace `.`/`..` canonicalization.
 - `src/hash.c` — FNV-1a64, used for pack checksums and compaction dedup.
-- `tests/` — one binary per concern (`test_mem`, `test_dir`, `test_pack_overlay`, `test_concurrency`, `test_index_stress` — the treap's correctness under thousands of randomized structural operations, cross-checked against an independent reference model); `test_harness.h` is a small assertion-macro header, not a framework, consistent with the zero-dependency constraint.
+- `tests/` — one binary per concern (`test_mem`, `test_dir`, `test_pack_overlay`, `test_concurrency`, `test_index_stress` — the treap's correctness under thousands of randomized structural operations, cross-checked against an independent reference model; `test_pack_write_perf` — the `pack_write` dedup O(n²) regression tripwire; `test_crash_consistency` — real fork()+`SIGKILL` fault injection against a real process, not a hand-truncated file, verifying Section 4.3/4.4's crash-safety claims for both journaled writes and compaction, with an independent progress side-channel so the oracle can tell "never attempted before the kill" apart from "attempted and lost" — see its own file header before touching journal or compaction code); `test_harness.h` is a small assertion-macro header, not a framework, consistent with the zero-dependency constraint.
 
 **One architectural fact spans every mutable structure and is easy to miss reading any single file in isolation:** `MountSnapshot` (`vfs.c`), `UpperSnapshot` (`upper.c`), and a `mem`-backed `MutCell`'s buffer (`upper.c`, Section 5.7) are all retired through the same two-step pattern — publish the replacement via `atomic_store_explicit(..., memory_order_release)`, then free the superseded object only under a dedicated `reclaim_gate` rwlock's *write* side, while every acquirer takes that same gate's *read* side around its own load-then-increment. This exists because a plain "load a pointer, then atomically increment its refcount" leaves a real gap between those two steps in which a concurrent writer can free the very object being acquired — confirmed by ASan as an actual heap-use-after-free during development, not a theoretical concern. See the comment on `UpperStore.reclaim_gate` in `internal.h` for the full reasoning. Any new refcounted, concurrently-reclaimed structure added to this codebase needs the same gate, not just an atomic pointer and a naive refcount. The one exception, and the reason it's an exception rather than a hole: `TreapNode`'s own per-node refcounting (`upper.c`) does *not* need its own `reclaim_gate`, because nothing ever acquires a `TreapNode*` independently the way `upper_acquire` acquires an `UpperSnapshot*` — a reader only ever reaches tree nodes by walking `s->root` after already holding a valid `UpperSnapshot` reference, which transitively keeps the whole tree alive for the reader's purposes; `node_ref`/`node_unref` are called only by the single writer (under `writer_lock`) and by `upper_release`'s cascade (already inside `reclaim_gate`'s protection at the snapshot level), never by a reader racing a writer the way the snapshot pointer itself is raced.
 
@@ -95,6 +95,17 @@ These are hard requirements stated in the spec, not stylistic suggestions — an
 - **RESOLVED: `pack_write`'s compaction-time exact-duplicate elimination (Section 9.2) was O(n²) in entry count, plus a latent correctness bug, both fixed together.** The dedup check was a linear scan of every previously-seen `(hash, size)` pair per entry, comparing only the hash and size — never the actual bytes — before trusting a match, so two different-content entries that happened to collide on `(hash, size)` under FNV-1a64 (explicitly not collision-resistant, `src/hash.c`) would have been silently merged, corrupting one of them; this was never observed in practice but was true of the code as written. `bench/bench.c`'s own compaction benchmark never surfaced either problem because its test files are byte-identical, which made every scan match on the first comparison (the scan's best case, not its worst case). Measuring with unique content instead (`pack_write` called directly, bypassing the journal/fsync path) showed 80,000 entries taking 1.74s, with a 20,000→80,000 (4x N) step showing 15.6x — matching O(n²)'s 16x prediction. Fixed with an open-addressing hash table (`DedupSlot`, load factor 1/2, linear probing) plus a `memcmp` verification before ever reusing a `data_off`, closing both the complexity issue and the correctness gap at once; post-fix, the same 80,000-entry case takes 0.044s (39.6x faster) and the 20,000→80,000 ratio drops to 3.35x, consistent with O(n). `BENCH.md`'s "Resolution #2" has the full before/after numbers. Correctness is covered permanently by `tests/test_pack_overlay.c` (asserts duplicate-content entries share one `data_off` and distinct-content entries do not); the performance fix is covered permanently by `tests/test_pack_write_perf.c` (a regression tripwire against 10,000 unique entries, run as part of `make test`).
 - **NOT FIXED, by deliberate decision: `vfs.c`'s mount table (`MountSnapshot`) is O(n²) in mount count, using the same full-array-copy-per-write pattern the file index used to.** Confirmed via `bench/bench.c` category 10 (500/2,000/8,000 mounts, both 4x-N steps showing 15–20x, matching O(n²)'s 16x prediction, for mount, resolve, and unmount alike). Unlike the file index, this is not being converted to a persistent tree: mount points are created by calls written into a program's own source code, not driven by workload or user data, so realistic mount counts do not reach the scale that made the file index's O(n²) an actual problem — `concept.md` Section 5.3's own stated trigger for needing a persistent structure ("not required until this assumption is empirically violated") has not been violated here the way it was for the file index. `BENCH.md`'s "Finding: mount table scaling" has the full numbers and reasoning; revisit only if a real use case needs thousands of runtime mount points (e.g. one mount per tenant).
 
+## Known reliability characteristics
+
+Section 4.3/4.4's crash-safety claims (self-checking journal records; atomic-rename compaction that leaves the existing pack + journal untouched on failure) were, until now, verified only by static reasoning about the design and by `test_pack_overlay.c`'s test of loading an *already*-corrupted pack file — never by actually killing a process mid-write and checking what a fresh reopen recovers. `tests/test_crash_consistency.c` closes that gap with real fault injection (`fork()` + `SIGKILL` at randomized, empirically-calibrated delays against an actual child process, not a hand-truncated file standing in for a crash):
+
+- **Journal-write crash injection** (25 trials, ~6ms/journaled-write on this build, delays sampled across the whole burst): kills a child mid-way through a sequence of individually-journaled, individually-fsynced writes. Verifies a strict "clean prefix" after reopening — every write the child actually completed is present with exact content, every write after the kill is cleanly absent, no gaps. 20–22 of 25 trials land a genuine interruption (`WIFSIGNALED`, not `WIFEXITED`) each run; **zero corruption or gaps found**.
+- **Compaction crash injection** (40 trials, delays sampled across a calibrated ~0.08s window covering pre-compaction durable writes through post-rename): kills a child mid-`vfs_sync`. Verifies that every write durably journaled *before* `vfs_sync` was even called survives regardless of whether compaction itself completed, and that reopening never fails. Uses an independent progress side-channel (a plain `write()`+`fsync()` on a dedicated file, entirely outside packfs) as ground truth for how many writes the child actually completed before dying — without it, a write the child simply never reached in time is indistinguishable from one that completed and was then lost, which is exactly the bug the test itself had on its first draft (see below). 40/40 trials land a genuine interruption each run; **zero corruption or loss found**.
+
+**A real finding from building this test, recorded here per this file's own "document literally all" standard: the test's first draft reported 128 failures, and every one of them was a bug in the test, not in PackFS.** It checked "is extra write *i* present after the crash" without distinguishing "the child was killed before it ever attempted write *i*" (expected, not a bug) from "write *i* completed and was then lost" (would be a real bug) — SIGKILL cannot be caught, so the child has no way to report its own progress through the normal API. Fixed by adding the independent progress side-channel described above; the corrected test has passed cleanly across every run so far (multiple full runs, plus 3 clean runs under ASan/UBSan). This is the same category of lesson as `BENCH.md`'s own history of findings: a claim (here, "PackFS's crash safety" — there, "the benchmark's numbers") is only as trustworthy as the thing measuring it, and that has to be checked too, not just the subject under test.
+
+**Still not covered, stated plainly:** these two scenarios don't exercise cross-process contention (two processes crashing/racing against the same files — not implemented in v0, see "Load-bearing constraints" above), disk-full/`ENOSPC` mid-write, or a crash during journal *truncation* specifically (the second atomic-rename step after a successful compaction, `.jnl.tmp` → `.jnl`) as its own isolated case. TSan remains unable to run in this sandbox for the concurrency side of reliability — see the sanitizer section above, now including the specific workarounds tried (not just the limitation) before concluding it's genuinely unfixable here.
+
 ## Self-evaluation
 
 **Methodology.** This file was checked against the repository's actual state (`make test` passing, including two new tests, `test_pack_write_perf` and the dedup-verification block added to `test_pack_overlay`; `include/`, `src/`, `tests/`, `bench/` present and matching the description below; confirmed by directory listing and a live build), against `concept.md` as frozen (for factual consistency of the constraints and architecture summarized above), and against the documentation standard stated in this file. This revision followed a deliberate audit for other instances of the file index's O(n²) shape elsewhere in the codebase — prompted by a request to leave no caveats undocumented — which found two more real issues, not zero: `pack_write`'s compaction dedup (O(n²), plus a latent hash-collision correctness bug, both now fixed) and the mount table (O(n²), confirmed and deliberately left unfixed, with the reasoning recorded). The audit also found and fixed a CI gap the new tests exposed — the CI config's (now `.gitea/workflows/ci.yml`; at the time of this finding it lived at `.github/workflows/ci.yml`, before this project's scaffolding was set up for Gitea hosting instead of the GitHub-Actions convention it started from — this repository was never actually hosted on GitHub) sanitizer-build steps never passed `-D_GNU_SOURCE` when compiling test files, which was harmless while no test included `internal.h` and became a real link failure once two did (`internal.h` needs it for `pthread_rwlock_t`) — and documented a sandbox flake (ASan/UBSan's own `DEADLYSIGNAL` startup race, distinct from and in addition to the already-documented TSan limitation) observed directly during this audit's own sanitizer runs, in `CONTRIBUTING.md` and cross-referenced here. Every finding below was confirmed empirically (direct `pack_write` timing, `bench/bench.c`'s new mount-scaling category, repeated sanitizer runs with `timeout`), not asserted.