Skip to content

fix: one stat-identity model for the target-secret filesystem store - #12

Merged
apresmoi merged 4 commits into
mainfrom
fix/target-secret-fs-identity
Sep 4, 2026
Merged

fix: one stat-identity model for the target-secret filesystem store#12
apresmoi merged 4 commits into
mainfrom
fix/target-secret-fs-identity

Conversation

@apresmoi

@apresmoi apresmoi commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

src/auth/targetSecretSourceFsPublishImmutable.test.ts — "fails closed when the
final inode is replaced" — was red on roughly a third of full-suite runs on
main, blocking #9 and #11. It is not a flaky test. It was a correct test
sitting on top of an identity model that could not answer the question it asked.

Two identity models for one store

The auth-owned target-secret store has two filesystem paths, and both of them
re-observe a pathname they already stated and decide whether they are still
looking at the same node.

                         birthtimeNs   ctimeNs   dev/ino/mode/nlink/uid   size
read   sameFile              yes         yes             yes              yes
publish same                 NO          no              yes              yes
publish sameNode             NO          no              yes (no size)     -
publish sameInode            NO          no              yes (no nlink)   yes

The read path compared birthtimeNs. The publish path did not. One store, two
models, and the weaker one on the side that writes.

birthtimeNs is the field that matters here because ext4 recycles inode
numbers freely. The publisher allocates the election token (inode A), then the
final (inode B), then frees A. When the test unlinks the final and recreates it,
ext4 normally hands back A — so ino differs and the swap is caught by
accident. Under parallel vitest load a sibling worker takes A first, the
replacement lands back on B, and every field the publish path compared was
identical. Measured on Linux 6.8 / overlayfs, the count of undetected swaps and
the count of recycled inode numbers were the same number in every run.

This change adds src/auth/targetSecretSourceFsIdentity.ts: one
FileIdentity/DirectoryIdentity pair and four comparators, used by both
paths. birthtimeNs is in all four. Everything else is the behaviour that was
already there, moved.

Why ctime is not the fix

Two independent reasons, both measured rather than reasoned about.

  1. It does not discriminate. Linux file timestamps come from the coarse tick
    clock — 1 ms on the diagnostic host, 4 ms on GitHub's CONFIG_HZ=250
    runners. A ~50 µs unlink/recreate lands in the same tick almost every time.
    The "196/200" figure that made ctime look promising came from a tight probe
    with no work between the unlink and the recreate, not from this code path.
  2. It is deterministically wrong. The final is legitimately mutated while
    being observed: a cooperating peer publishing identical bytes appends the
    second half of the record between two observations. That is the same reason
    sameNode already omitted size. Adding ctimeMs to it makes
    joins identical writers and honors actual short writes fail every time.

birthtimeNs has neither problem. It is fixed for the life of an inode and
changes on every reallocation. ctimeNs stays where it was already safe —
sameFileExact only — and never goes near sameDirectory, whose ctime the
publisher moves itself on every link and unlink.

On a filesystem that reports no birth time libuv yields 0n or the ctime for
every file, so the comparison degrades to exactly today's behaviour. It can
never produce a false failure, only fail to add one.

What this actually guarantees

Stated exactly, in the module and here:

Any replacement that is not byte-, mode-, uid-, and nlink-identical to the
original is rejected by the content proof that already runs; a byte-identical
replacement is rejected as well, unless it was created within one kernel tick
of the original, in which case it is indistinguishable from the original by
any stat field and is observably harmless.

That is not full inode-replacement detection, and no stat-based scheme can
deliver it: nothing holds a file descriptor pinning the inode across the
lstat-then-open window. The old test title claimed the stronger property, so
it is retitled to what the design can actually deliver —
"fails closed when the final is unlinked and recreated with identical bytes
after the named observation"
— and now sleeps 25 ms before recreating, which
guarantees a tick boundary at HZ >= 100, and asserts that the replacement's
birth time really differs so the run cannot pass green on a filesystem that
lacks the discriminator.

Tests

src/auth/targetSecretSourceFsIdentity.test.ts is the real guard: synthetic
BigIntStats pairs, no filesystem, so it behaves identically on ext4, APFS,
tmpfs and overlayfs. Mutation-checked — deleting birthtimeNs from any one of
the four comparators turns it red (verified for all four).

Measured on Linux 6.8 / overlayfs (Node 22), replaying this exact scenario 200
times with two background processes churning inodes in the shared tmpdir:

before after
swaps undetected, no sleep 49/200 0/200
swaps undetected, 25 ms sleep 7/200 0/200
full targetSecretSourceFsPublishImmutable file, 25 vitest runs 4 red 0 red

Isolated, with no churn, the pre-fix code detected 200/200 — which is exactly
why an isolated green run proved nothing here.

Full gate green (3399 tests, 950 suites, coverage thresholds met), plus the auth
suite run separately on APFS, where nanosecond ctime resolution makes
sameFileExact maximally strict, to confirm the added field causes no false
rejection. node check-boundaries.mjs clean.

Scope

Per specs/TARGETS.md this is a same-uid trusted local-state boundary and a
process already running as that uid is explicitly outside the threat model, so
this is a P4 security issue and a P1 engineering one. It is on the second count
that it is worth fixing: it unblocks #9 and #11, and it removes a divergence
that would have kept producing surprises.

@apresmoi
apresmoi merged commit fe3b888 into main Sep 4, 2026
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant