Host-lint guard scanner goes blind on a phantom heredoc and accepts a || host fallback #115

New Issue

clawbot · 2026-08-11T13:54:29+02:00

clawbot commented

2026-08-11 13:54:29 +02:00

TestNoHostLintPathRemains in cmd/vaultik/lintdocker_test.go (on next, commit d257f8f) asserts that no script invokes golangci-lint except through docker. It does not hold that property. Two evasions, each demonstrated on the third review of pull request #114 by mutating the real script/lint with the suite left green.

(a) Phantom here-document silently blinds the rest of the file. heredocTerminator treats any << on a joined code line as a here-document opener, and shellCode then discards every following line until one equals that word. So a << in a quoted string or an inline comment (only whole-line # comments are stripped), or a cat <<-\EOF whose terminator word becomes \EOF, yields a word that never appears — and scanning of everything below it in that file stops, permanently and silently. Appending to the real script/lint:

echo "bitmask note: 1&lt;&lt;3"
golangci-lint run --config .golangci.yml ./...

leaves make test green. This needs no adversary: an ordinary later edit disarms the guard.

(b) assertLinterIsContainerised accepts any line where docker appears anywhere before the linter, so a one-line native fallback passes. Inserted into script/lint's main():

command -v docker &gt;/dev/null 2&gt;&amp;1 || { golangci-lint run --config .golangci.yml ./...; return; }

Same run, same green — the escape hatch in its most natural spelling.

Not a live false green on the tree as it stands: all 17 current scripts were confirmed reachable by the scanner. This is a guard asserting a property it does not hold, which is what a later reader trusts instead of re-deriving the invariant.

Implementation requirements

In shellCode, fail loudly when a here-document is still open at end of file (terminate != "" after the loop). That alone converts all three forms in (a) from silent blindness into a named failure. shellCode will need to report the error to its caller rather than swallow it. Tightening the opener test to an actual redirection is a bonus, not a substitute for this.
In assertLinterIsContainerised, split the joined line on ;, &&, || and |, and require the command containing the linter to begin with docker. script/lint-fix's continuation-joined docker run … golangci-lint run … is a single command and must still pass.
Extend TestShellCodeSeesCodeAndNotProse to cover an unterminated here-document and a << that is not one.
Fix the citation at Dockerfile.lint:73-76: it says the PR records script/lint "exits 1 naming the lll finding", but that evidence block names a revive finding and says why. One word.

Do not re-litigate the containerised gate itself; it is verified and out of scope here.

Definition of done

Both mutations above — the 1<<3 echo plus host lint call, and the command -v docker || { …; return; } fallback — inserted into the real script/lint, each shown failing make test with a message naming the offending script and line, then reverted. Output recorded verbatim on the PR.
The <<-\EOF and quoted-<< forms likewise shown to produce a named failure rather than silence.
script/lint-fix still passes the tightened assertion, and the existing 17 scripts still scan clean.
make check green, evidence on the PR. script/cibuild not required — this touches test code only.
Commit title ends (closes #N) with this issue's number; branch off next, PR based on next.

`TestNoHostLintPathRemains` in `cmd/vaultik/lintdocker_test.go` (on `next`, commit `d257f8f`) asserts that no script invokes `golangci-lint` except through `docker`. It does not hold that property. Two evasions, each demonstrated on the third review of [pull request #114](https://git.eeqj.de/sneak/vaultik/pulls/114) by mutating the real `script/lint` with the suite left green. **(a) Phantom here-document silently blinds the rest of the file.** `heredocTerminator` treats any `<<` on a joined code line as a here-document opener, and `shellCode` then discards every following line until one equals that word. So a `<<` in a quoted string or an inline comment (only whole-line `#` comments are stripped), or a `cat <<-\EOF` whose terminator word becomes `\EOF`, yields a word that never appears — and scanning of everything below it in that file stops, permanently and silently. Appending to the real `script/lint`: ```sh echo "bitmask note: 1<<3" golangci-lint run --config .golangci.yml ./... ``` leaves `make test` green. This needs no adversary: an ordinary later edit disarms the guard. **(b) `assertLinterIsContainerised` accepts any line where `docker` appears anywhere before the linter**, so a one-line native fallback passes. Inserted into `script/lint`'s `main()`: ```sh command -v docker >/dev/null 2>&1 || { golangci-lint run --config .golangci.yml ./...; return; } ``` Same run, same green — the escape hatch in its most natural spelling. Not a live false green on the tree as it stands: all 17 current scripts were confirmed reachable by the scanner. This is a guard asserting a property it does not hold, which is what a later reader trusts instead of re-deriving the invariant. ## Implementation requirements 1. In `shellCode`, **fail loudly when a here-document is still open at end of file** (`terminate != ""` after the loop). That alone converts all three forms in (a) from silent blindness into a named failure. `shellCode` will need to report the error to its caller rather than swallow it. Tightening the opener test to an actual redirection is a bonus, not a substitute for this. 2. In `assertLinterIsContainerised`, **split the joined line on `;`, `&&`, `||` and `|`, and require the command containing the linter to _begin_ with `docker`.** `script/lint-fix`'s continuation-joined `docker run … golangci-lint run …` is a single command and must still pass. 3. Extend `TestShellCodeSeesCodeAndNotProse` to cover an unterminated here-document and a `<<` that is not one. 4. Fix the citation at `Dockerfile.lint:73-76`: it says the PR records `script/lint` "exits 1 naming the `lll` finding", but that evidence block names a `revive` finding and says why. One word. Do not re-litigate the containerised gate itself; it is verified and out of scope here. ## Definition of done - Both mutations above — the `1<<3` echo plus host lint call, and the `command -v docker || { …; return; }` fallback — inserted into the real `script/lint`, each shown **failing** `make test` with a message naming the offending script and line, then reverted. Output recorded verbatim on the PR. - The `<<-\EOF` and quoted-`<<` forms likewise shown to produce a named failure rather than silence. - `script/lint-fix` still passes the tightened assertion, and the existing 17 scripts still scan clean. - `make check` green, evidence on the PR. `script/cibuild` not required — this touches test code only. - Commit title ends ` (closes #N)` with this issue's number; branch off `next`, PR based on `next`.

clawbot added this to the 1.0.0 milestone 2026-08-11 13:54:29 +02:00

clawbot self-assigned this 2026-08-11 13:54:29 +02:00

clawbot commented

2026-08-11 13:56:58 +02:00

Plan

Branch fix-115-lint-scanner off next. All changes in
cmd/vaultik/lintdocker_test.go plus one word in Dockerfile.lint.

shellCode becomes func shellCode(contents string) ([]string, error).
After the loop, a non-empty terminate is an error naming the line
number and text of the opening line. TestNoHostLintPathRemains wraps
it with require.NoError so the failure names the script. The opener
test in heredocTerminator is deliberately left conservative: any
<< still opens a here-document, so a quoted <<, an
inline-comment << and <<-\EOF all reach end of file
unterminated and fail loudly rather than being silently skipped.
assertLinterIsContainerised splits the joined line on &&,
||, ; and |, then requires the segment containing the linter to
begin with docker (after stripping leading {/(). script/lint-fix's
continuation-joined docker run … golangci-lint run … contains none of
those operators, so it is one segment beginning with docker and still
passes on its merits; no exemption is added. The predicate is extracted
as a pure function so it gets its own table test covering the ||
fallback spelling.
TestShellCodeSeesCodeAndNotProse gains cases for an unterminated
here-document and for a << that is not one.
Dockerfile.lint:75 — the cited evidence names a revive finding, not
lll. One word.

Evidence for the PR: both mutations from the third review plus the
<<-\EOF and quoted-<< forms inserted into the real
script/lint, each shown failing make test verbatim and reverted; all 17
current scripts confirmed still scanning clean; make check green.

## Plan Branch `fix-115-lint-scanner` off `next`. All changes in `cmd/vaultik/lintdocker_test.go` plus one word in `Dockerfile.lint`. 1. `shellCode` becomes `func shellCode(contents string) ([]string, error)`. After the loop, a non-empty `terminate` is an error naming the line number and text of the opening line. `TestNoHostLintPathRemains` wraps it with `require.NoError` so the failure names the script. The opener test in `heredocTerminator` is deliberately left conservative: any `<<` still opens a here-document, so a quoted `<<`, an inline-comment `<<` and `<<-\EOF` all reach end of file unterminated and fail loudly rather than being silently skipped. 2. `assertLinterIsContainerised` splits the joined line on `&&`, `||`, `;` and `|`, then requires the segment containing the linter to *begin* with `docker` (after stripping leading `{`/`(`). `script/lint-fix`'s continuation-joined `docker run … golangci-lint run …` contains none of those operators, so it is one segment beginning with `docker` and still passes on its merits; no exemption is added. The predicate is extracted as a pure function so it gets its own table test covering the `||` fallback spelling. 3. `TestShellCodeSeesCodeAndNotProse` gains cases for an unterminated here-document and for a `<<` that is not one. 4. `Dockerfile.lint:75` — the cited evidence names a `revive` finding, not `lll`. One word. Evidence for the PR: both mutations from the third review plus the `<<-\EOF` and quoted-`<<` forms inserted into the real `script/lint`, each shown failing `make test` verbatim and reverted; all 17 current scripts confirmed still scanning clean; `make check` green.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/vaultik#115

Host-lint guard scanner goes blind on a phantom heredoc and accepts a || host fallback #115

Implementation requirements

Definition of done

Plan

Host-lint guard scanner goes blind on a phantom heredoc and accepts a `||` host fallback #115