SmartConfig

Review of M2 chunk B (steps 6-10)

Date: 2026-09-30 to 10-01. Normal review, as the plan asks for chunk B: two reviewers (AI agents, each in its own clone, running the code and trying to break it), one on the store (record, restore), one on the file system, the CLI and M1 compatibility. Claude then acted as the skeptic and reproduced each finding before fixing it. Nothing touched the repo, /etc (read only) or the real store.

Held up: across about 350,000 reads racing a swap between a file, a FIFO and a symlink to a secret, ReadState never followed the last component and never blocked; a watcher process racing a file, link or deletion restore never wrote a row after the restore row; make m1-compat passes and leaves the checkout untouched.

Findings

Id Severity Finding Skeptic Outcome
D1 medium The fingerprint rule was matched against the path as typed, so sc snapshot /proc/self/root/etc/machine-id (or a path through a symlink to /) stored the file’s content and sc cat printed it; as root the same works for host keys. Found by both reviewers. Reproduced with the real binary on /etc/machine-id: 33 bytes stored, one object. Fixed cae8992: the rule also applies to the path with its directories resolved. Same commands now give “(fingerprint only)” and no object.
D2 low A file row followed by a digest row of the same content (an M1 store with machine-id, once the rule exists) got the intent “changed”. Reproduced in a test. Fixed 3db3c1e: “now fingerprint only” / “now with content”; “changed” only when the content changed.
D3 low A link replaced by a file mid-read gave “read link: invalid argument”, a file replaced by a link “is a symlink, refusing”; neither was retried. Follows from the code; tests swap at the exact point. Fixed 4cdb7d6: both are ErrReplaced; the identity check also compares the file type (a new FIFO can reuse the inode).
D4 low A newline in a link target forged extra stdout lines in sc snapshot and sc restore. Reproduced. Fixed c56fe24: names and targets with control characters or invalid UTF-8 are quoted, also in sc log.
D5 low riscv64 and other 64-bit targets no longer built (only amd64 and arm64 had fstatat). Reproduced. Fixed bae267e: amd64, arm64, riscv64, loong64, ppc64le, s390x vet clean (ppc64’s SQLite does not build).
T1-T10 test gaps safeDir above the parent; the stamp re-check for link and deletion restores; “gone meanwhile”; a restored link’s owner; the identity check in ReadState; temp-link tracking, cleanup after a failed chown and the collision retry; an unchanged link diff; the group name in the link line; a goroutine calling t.Fatal; a root test that could chown /dev/null. Each confirmed by the reviewer’s mutation. 4cdb7d6, d93083e: each of those mutations now fails a test.

Not defects