Isolate the lint cache per worktree and context-gate the native lint path #102
Reference in New Issue
Block a user
Delete Branch "fix-lint-isolation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #99. Closes #80.
One defect seen twice:
script/lintdecided whether it could skip thepinned image by asking what version was on
PATHrather than where itwas running, and cache isolation is part of that same question.
What changed
${XDG_CACHE_HOME:-~/.cache}/vaultik-lint/<slug>-<12-hex digest of $ROOT>,still persistent, so a warm run stays seconds. The old single
per-repo directory is what let two checkouts with identical Go
contents collide and replay each other's stored analysis.
script/lint-audit, run onevery lint. Rejects output citing any file not in the tree being
linted: absolute paths outside the root,
..components, andrelative paths that do not exist here. Runs on clean output too,
because the unearned-pass direction is the silent one. It never
certifies that a run passed — it does not look at whether there were
findings — so it cannot itself become a gate reporting a green.
Split out as its own script so it is directly exercisable against a
saved capture, which is how it was verified.
cache does not remove the contention.
parallel golangci-lint is runningis retried up to 6 times, 15s apart; exhausting them failssaying explicitly that the tree was never analysed.
collected when that worktree disappears, so throwaway worktrees do
not accumulate caches. The tree sits under
XDG_CACHE_HOMEand isdisposable; the removal command is in the script's comment and in the
audit's rejection message.
VAULTIK_LINT_IN_CONTAINER=1, set only by theDockerfilelintstage, and a matching version.
/.dockerenvwas rejected as thesignal:
dockerdcreates it fordocker run, but it is not reliablypresent during a BuildKit
docker build, which is exactly the casethe exception exists for. Inside the container a version mismatch is
now a hard error rather than a fall-through — there is no daemon
there to fall through to.
now a hard failure naming
script/lint,script/check,script/precommitandscript/cibuild, instead of a warningfollowed by
bootstrap complete. Installing docker from bootstrapwas rejected: root, a daemon, and on macOS a GUI cask, so the attempt
would itself fail in the common case and trade one false success for
a second failure mode.
version --short(#80.3) replaces the banner scrape, whichsurvives only as a fallback.
TODO.md's "as trustworthy asscript/cibuild"corrected to match
README.md(only the lint leg is equivalent);README.mdrequirements gained docker andsqlite3.Verification (reproduction, not inspection)
/tmp/impl-99(clean) and/tmp/impl-99-dirty(one file with areviveand anunusedfinding), started together. Clean:0 issues., exit 0. Dirty: exactly 2 findings, both citinginternal/blobgen/lintbait.goin its own tree, exit 2. Neither logcontained
parallel golangci-lint is running. No cross-contaminationin either direction.
script/lintrun in thedirty worktree made to report outside paths reproduced the #99
signature (
../impl-91/internal/blobgen/lintbait.go) and exited 1with the REJECTED diagnostic, instead of reporting the findings. The
auditor was also run directly against the verbatim output from #99
(exit 1), a foreign path without a
..component (exit 1), a realclean capture (exit 0), and a container-absolute
/src/...path(exit 0).
golangci-lintshim onPATHreporting2.12.2and logging every invocation,make lintnever invoked itand ran the pinned image. With
VAULTIK_LINT_IN_CONTAINER=1the shimwas invoked (
version --short, thenrun ./...), confirming thatvariable is the only door. A mismatched version with that variable set
exits 1 naming both versions.
PATHwithout docker:bootstrap: FAILED - docker is not installed., exit 1, zero occurrences ofbootstrap complete. Adockershim whoseinfofails:bootstrap: FAILED - the docker daemon is not reachable., exit 1. Unchanged happy path on this host:exit 0,
bootstrap complete.script/cibuildexit 0, 3m24s wall. Cachecontrol: no prune, no
--no-cache; theENVadded to the lint stageinvalidates everything below it, and the 11
CACHEDlayers are alldependency/module layers.
[lint 9/9] ... make lintexecuted for78.7s inside the pinned image and printed
0 issues., which is whatproves the native path still works with no daemon;
[builder 9/10] ... make testexecuted with all 14oklines and no(cached).make check— exit 0, 18.6s: 14oklines, zero(cached),0 issues.,fmt-checkclean.The added ~1.4s is the
teecapture plus the audit pass over theoutput; the inner loop is still seconds.
.golangci.ymlunchanged, sha256 verified before push:021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.The lint-stage
FROMline and digest, theARG CHECK_EPOCHguardstructure,
REPO_POLICIES.mdandscript/testare untouched.Known limitation, deliberately not papered over
script/lintbind-mounts the tree at/src, so two containerized runsof different checkouts both call themselves
/src. Contaminationbetween two container runs is therefore not distinguishable by path
alone, and the audit cannot catch it — which is why the per-worktree
cache key, not the audit, is the primary fix. The audit catches the
signature actually observed in #99 and any host-side path leakage. This
is documented at the top of
script/lint-audit.--allow-parallel-runnerswas considered instead of the retry andrejected: it disables the lock that protects a cache shared by two runs
of the same worktree, which is a real configuration, and it would trade
a delay for corruption.
426a8d7645to043aabbd27Addendum to the verification above, because the cache-collection item
(#99.4) failed its own negative control before it passed one, and the
failure is worth recording rather than quietly fixing.
The first cut collected a stale cache with a plain
rm -rf. Go's modulecache inside it is deliberately read-only, so
rmcould not unlinkfiles out of directories it may not write; it exited non-zero,
set -etook the whole script down, and
make lintreturned an error on a treewith zero findings — a gate failing for a housekeeping reason, which is
its own bug and exactly the class this repo keeps paying for. It also
left a half-removed cache with no marker file, which every future run
would have skipped forever.
Fixed: the tree is made writable before removal, a removal that still
fails prints a warning and restores the marker so the cache stays
collectable, and no tidy-up failure can fail a lint.
Re-verified against a stale cache containing a read-only
module-cache-like tree with its marker pointing at a removed worktree:
the cache was collected and the lint exited 0 with
0 issues..make checkre-run: exit 0, 16.2s, 14oklines, zero(cached),0 issues..script/cibuildre-run: exit 0, 2m43s,[lint 9/9]executed for 53.3s in the pinned image printing
0 issues., no--no-cacheand no prune used..golangci.ymlsha256 re-verifiedbefore the force-push:
021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.The branch was force-updated to keep this as one commit (
426a8d7to043aabb).Review: PASS
Independent review of #102 against
#99 and
#80. Verified by reproduction on this
host, not by inspection. Nothing found that can produce an unearned green; two
fail-closed defects and one accuracy nit below.
Probes run (each reproduced, not assumed)
one carrying a planted
unused+nlreturnbait,make lintstartedconcurrently under one shared
XDG_CACHE_HOME: two distinct cache directories(
...-9fdd462e03dc,...-c679b0c15419), clean exit 0 /0 issues., dirtyexit 1 with exactly 2 findings both citing its own
internal/blobgen/rv102bait.go, zero occurrences of the lock message, zeroforeign paths in either direction. Issue
#99 does not reproduce on this branch.
script/lint-audit, 23 vectors. Rejects the verbatim #99 signature,absolute-foreign with no
.., mid-path../, whitespace- and tab-prefixedforeign paths, and
/src/- or$ROOT/-prefixed paths naming files not presenthere. Accepts
/src/-,$ROOT/- and relative-prefixed paths that do exist,clean output, empty output. Crucially it also exits 0 on a capture containing
three real findings, so the "never certifies a pass" design genuinely holds and
it cannot become a second gate reporting green. Missing/extra arguments exit 2
and
audit_outputtreats any non-zero as reject, so the auditor fails closed.golangci-lintshim reporting2.12.2firston
PATH,make lintnever invoked it (the shim's log file was never created)and ran the pinned image. With
VAULTIK_LINT_IN_CONTAINER=1the shim wasinvoked (
version --short, thenrun ./...). Var set plus shim reporting2.10.1: exit 1 naming both versions. Var set totruerather than1: dockerpath. The variable is the only door. Rejecting
/.dockerenvis sound - it is adockerdruntime artifact, absent during a BuildKitdocker build, which isthe only case the exception serves.
golangci-lint2.12.2 onPATH, so onmainscript/lintexecs the host binary against the global default~/.cache/golangci-lintand never runs the pinned image at all. That is verylikely the actual mechanism behind the
../impl-91/paths in#99 - host, cwd-relative rendering of
another worktree's absolute paths - and this PR closes it at the root, so the
fix is stronger than the issue's own diagnosis.
exit 1 with the explicit "NOT a lint verdict: the tree was never analysed"
text. Non-zero and self-describing; not mistakable for a lint result.
script/cibuild: exit 0, 185s wall, no prune and no--no-cache. 14CACHEDlayers, all dependency/setup; the check layers executed -#16 make fmt-check,#17 make lintDONE 52.8s printing0 issues.inside the pinnedimage with no daemon,
#25 make testDONE 89.6s with 14oklines and 0(cached). FreshCHECK_EPOCHin both stages. Disclosure: my first attemptexited 127 because
/usr/bin/timeis absent here; that run proved nothing andwas discarded, the figures above are the re-run.
.golangci.ymlsha256021cc83f4e6f...matches;REPO_POLICIES.md,script/test,script/check,script/cibuild,script/precommit,script/fmt,Makefileand.gitea/byte-identical tomain; lint-stageFROM+ digest and bothARG CHECK_EPOCHguards intact; noGo file touched;
ENV VAULTIK_LINT_IN_CONTAINER=1is confined to thelintstage and inherited by neither
buildernor the runtime image.043aabb(3m11s); mergeable, merges cleanly againstmainatc51f693;make fmta no-op; commit title ends(closes #99)and the bodycloses #80; no attribution trailers or vendor references anywhere; inclusive
terminology, naming and idiom consistent with the surrounding scripts; no scope
creep.
Non-blocking findings
1.
script/lint:174- a tidy-up failure can still fail a lint, contradictingthe commit message.
This is the one unguarded command in
prune_dead_caches; every other operationin the collection path is
|| trueorif !-guarded. Underset -euanassignment whose command substitution fails aborts the script. The commit
message states the opposite ("never lets a failure to tidy up fail the lint"),
and this is the same class as the self-reported
rm -rfanomaly.Demonstrated: with a stale cache whose marker file is unreadable,
./script/lintexits 1 having printed onlycat: .../worktree: Permission denied, never reaching the linter. The realistictrigger is two concurrent lints collecting the same stale cache -
[ -f "$dir/worktree" ]true for B, then A'srm -rfunlinks the marker before B'scat. I could not win that race in 36 concurrent runs, so the window is narrow,and the failure direction is closed (a loud lint error, never a false green) -
which is why this is not blocking.
Acceptable:
owner="$(cat "$dir/worktree" 2>/dev/null || true)". The existing[ -z "$owner" ]check already handles the empty result.2.
script/lint-audit:63-66- a genuine dependency error is reported as cachecontamination.
Any absolute path not under
$ROOTor/srcis classed foreign. Inside thecontainer
GOMODCACHE=/cache/go-mod, so atypecheckfinding citing adependency source file is rejected: verified,
/cache/go-mod/example.com/x@v1.0.0/a.go:3:1: msg (typecheck)exits 1 with "thelinter reported findings for files that are not in this tree ... clear this
tree's lint cache". That is the wrong diagnosis for a real dependency
compile error, and it sends the reader to clear a cache that is not the cause.
Fails closed, so not blocking.
Acceptable: exempt
/cache/go-mod/and/cache/go-build/(the pathsscript/lintitself sets) from the foreign set, or soften the message when everyforeign path lies under the cache mount.
Nit
the dirty worktree exiting 2. Observed here: exit 1 with 2 findings,
which is golangci-lint's default
issues-exit-code. Immaterial to the outcome,but a verification narrative in this repo should match what the tool actually
did.