Two fail-closed defects in the new lint isolation #103

New Issue

clawbot · 2026-08-09T17:15:23+02:00

clawbot commented

2026-08-09 17:15:23 +02:00

Non-blocking findings from the PR #102
review. Both fail closed — they can abort or misdiagnose a lint run,
never report a false green — which is why they did not block the merge.
Both are one-to-three-line fixes.

1. `script/lint` — `prune_dead_caches` can still fail a lint

script/lint:174:

owner="$(cat "$dir/worktree")"

The only unguarded command in the function. Under set -eu a failed
command substitution in an assignment aborts the script, so a stale cache
with an unreadable marker makes ./script/lint exit 1 printing only
cat: .../worktree: Permission denied — without linting anything.

This contradicts the intent recorded in the PR, that tidy-up can never
fail a lint. It is the same class of defect the author already caught and
fixed once during implementation (an rm -rf on a read-only Go module
cache aborting the script), surviving in one remaining line.

Realistic trigger is two concurrent lints collecting the same stale
cache: [ -f ] passes, then A's rm -rf unlinks the marker before B's
cat. The reviewer could not win that race in 36 concurrent attempts, so
it is narrow.

Fix: 2>/dev/null || true on the substitution. The existing
[ -z "$owner" ] branch already handles the empty result.

2. `script/lint-audit` — dependency paths misdiagnosed as contamination

script/lint-audit:63-66 classes any absolute path outside $ROOT or
/src as foreign. Inside the container GOMODCACHE=/cache/go-mod, so a
legitimate typecheck finding citing a dependency's source is rejected
as cache contamination, telling the user to clear their lint cache when
the real problem is a dependency error.

Fix: exempt /cache/go-mod and /cache/go-build from the foreign
set. Keep everything else rejecting — the guard's value is that it
refuses to certify a pass.

Definition of done

Both fixed.
Verified by negative control, not inspection: a stale cache with an
unreadable marker lints normally, and a capture containing a
/cache/go-mod/... path no longer produces the contamination message.
script/lint-audit still rejects the original #99
signature and still never certifies a pass.
script/cibuild exits 0.

Also worth correcting

The PR #102 body and its
issue comment state the dirty worktree exited 2; the reviewer observed
1, golangci-lint's default issues-exit-code. Minor, but the number
is quoted as evidence.

Non-blocking findings from the [PR #102](https://git.eeqj.de/sneak/vaultik/pulls/102) review. Both fail **closed** — they can abort or misdiagnose a lint run, never report a false green — which is why they did not block the merge. Both are one-to-three-line fixes. ## 1. `script/lint` — `prune_dead_caches` can still fail a lint `script/lint:174`: ```sh owner="$(cat "$dir/worktree")" ``` The only unguarded command in the function. Under `set -eu` a failed command substitution in an assignment aborts the script, so a stale cache with an unreadable marker makes `./script/lint` exit 1 printing only `cat: .../worktree: Permission denied` — without linting anything. This contradicts the intent recorded in the PR, that tidy-up can never fail a lint. It is the same class of defect the author already caught and fixed once during implementation (an `rm -rf` on a read-only Go module cache aborting the script), surviving in one remaining line. Realistic trigger is two concurrent lints collecting the same stale cache: `[ -f ]` passes, then A's `rm -rf` unlinks the marker before B's `cat`. The reviewer could not win that race in 36 concurrent attempts, so it is narrow. **Fix:** `2>/dev/null || true` on the substitution. The existing `[ -z "$owner" ]` branch already handles the empty result. ## 2. `script/lint-audit` — dependency paths misdiagnosed as contamination `script/lint-audit:63-66` classes any absolute path outside `$ROOT` or `/src` as foreign. Inside the container `GOMODCACHE=/cache/go-mod`, so a legitimate `typecheck` finding citing a dependency's source is rejected as cache contamination, telling the user to clear their lint cache when the real problem is a dependency error. **Fix:** exempt `/cache/go-mod` and `/cache/go-build` from the foreign set. Keep everything else rejecting — the guard's value is that it refuses to certify a pass. ## Definition of done 1. Both fixed. 2. Verified by negative control, not inspection: a stale cache with an unreadable marker lints normally, and a capture containing a `/cache/go-mod/...` path no longer produces the contamination message. 3. `script/lint-audit` still rejects the original [#99](https://git.eeqj.de/sneak/vaultik/issues/99) signature and still never certifies a pass. 4. `script/cibuild` exits 0. ## Also worth correcting The [PR #102](https://git.eeqj.de/sneak/vaultik/pulls/102) body and its issue comment state the dirty worktree exited **2**; the reviewer observed **1**, golangci-lint's default `issues-exit-code`. Minor, but the number is quoted as evidence.

clawbot added this to the 1.0.0 milestone 2026-08-09 17:15:23 +02:00

clawbot referenced this issue

2026-08-09 17:43:11 +02:00

Make the tagged-release path work on Gitea (closes #65) #104

clawbot referenced this issue

2026-08-09 18:02:25 +02:00

Make the tagged-release path work on Gitea (closes #65) #104

clawbot referenced this issue

2026-08-10 14:27:53 +02:00

Run all linting in Docker via Dockerfile.lint + script/lint #113

Sign in to join this conversation.