Reworked after the plan changed (new definition of done on #121).
Four reviews found successive ways a shell script could run the host
linter that the hand-written scanner in cmd/vaultik/lintdocker_test.go
did not see — most recently a lone & and an environment assignment
named docker. A hand-written shell parser cannot be complete, and it
is the machinery the workflow rejects: it checks the tree instead of
fixing it, and a newcomer cannot follow it in one reading. The tree is
already right — no script runs the linter outside the container.
Removed TestNoHostLintPathRemains and every helper and helper-test
that served only it: the shell scanner, here-document and command
splitting, and their tests. The file drops from 507 lines on next to
363.
Kept the plain Dockerfile.lint assertions: the linter image is pinned
by digest, config verify runs before run, and the per-run CHECK_EPOCH reaches both steps so neither is replayed from cache.
Corrected the TODO.md sentence that claimed a test proves no script
runs the host linter. Now stated truthfully: script/lint is the only
lint entry point and runs golangci-lint only inside the container;
keeping it so is a review matter.
Judgement call: this drops a guard two reviewers asked to harden, per
the manager's new definition of done; the owner can overrule on #114.
Disclosure: the trailing Model: line follows the repo convention, not
the brief's literal string.
Disclosure: a pre-existing gomodguard deprecation warning surfaces
during the gate; out of scope, untouched.
Model: opus-4-8
Reworked after the plan changed (new definition of done on
https://git.eeqj.de/sneak/vaultik/issues/121).
Four reviews found successive ways a shell script could run the host
linter that the hand-written scanner in `cmd/vaultik/lintdocker_test.go`
did not see — most recently a lone `&` and an environment assignment
named `docker`. A hand-written shell parser cannot be complete, and it
is the machinery the workflow rejects: it checks the tree instead of
fixing it, and a newcomer cannot follow it in one reading. The tree is
already right — no script runs the linter outside the container.
Removed `TestNoHostLintPathRemains` and every helper and helper-test
that served only it: the shell scanner, here-document and command
splitting, and their tests. The file drops from 507 lines on `next` to
363.
Kept the plain `Dockerfile.lint` assertions: the linter image is pinned
by digest, `config verify` runs before `run`, and the per-run
`CHECK_EPOCH` reaches both steps so neither is replayed from cache.
Corrected the `TODO.md` sentence that claimed a test proves no script
runs the host linter. Now stated truthfully: `script/lint` is the only
lint entry point and runs `golangci-lint` only inside the container;
keeping it so is a review matter.
Judgement call: this drops a guard two reviewers asked to harden, per
the manager's new definition of done; the owner can overrule on
https://git.eeqj.de/sneak/vaultik/pulls/114.
Disclosure: the trailing `Model:` line follows the repo convention, not
the brief's literal string.
Disclosure: a pre-existing `gomodguard` deprecation warning surfaces
during the gate; out of scope, untouched.
Model: opus-4-8
The three targeted evasions are genuinely closed — verified by mutating the real script/lint and running make test: a quoted-string <<, a <<-\EOF terminator, and a one-line command -v docker ... || { golangci-lint run ...; } fallback each turn the suite red. make check is green on the head rebased onto current next.
One blocking finding.
High — the inline-comment << case named in #121 finding 1 is only half closed. In cmd/vaultik/lintdocker_test.go, shellCode drops whole-line comments but not inline ones, and heredocTerminator tracks only quotes, not an unquoted #. So a << inside an inline comment still opens a phantom here-document. The end-of-file error catches this only when the terminator word never recurs; when it appears later as a standalone line, the lines between are silently dropped from scanning. Appended to the real script/lint, make test stayed green:
The host golangci-lint run is invisible to TestNoHostLintPathRemains — the silent-disarm class this unit exists to remove, in the exact case the issue's finding 1 names. Consequently the PR body's "opens one only when it is a real redirection" is inaccurate (an inline-comment << is not a redirection yet opens one), and the scanner comment omits inline comments while listing the quoted-string and arithmetic exceptions.
Acceptable: strip an unquoted, word-initial #-to-end-of-line before here-document detection, as whole-line comments already are, or stop heredocTerminator at an unquoted #; add an inline-comment case to TestShellCodeSeesCodeAndNotProse.
Judgement call: definition-of-done item 1's parenthetical says only "not << inside quotes," so a narrow reading puts inline comments out of scope and treats the end-of-file error as sufficient. I treated the issue's finding 1 (which names inline comments) and the demonstrated silent evasion as controlling; overrule if inline comments were intended out of scope.
model: claude-opus-4-8
FAIL (needs-rework)
The three targeted evasions are genuinely closed — verified by mutating the real `script/lint` and running `make test`: a quoted-string `<<`, a `<<-\EOF` terminator, and a one-line `command -v docker ... || { golangci-lint run ...; }` fallback each turn the suite red. `make check` is green on the head rebased onto current `next`.
One blocking finding.
**High — the inline-comment `<<` case named in https://git.eeqj.de/sneak/vaultik/issues/121 finding 1 is only half closed.** In `cmd/vaultik/lintdocker_test.go`, `shellCode` drops whole-line comments but not inline ones, and `heredocTerminator` tracks only quotes, not an unquoted `#`. So a `<<` inside an inline comment still opens a phantom here-document. The end-of-file error catches this only when the terminator word never recurs; when it appears later as a standalone line, the lines between are silently dropped from scanning. Appended to the real `script/lint`, `make test` stayed green:
```
: # housekeeping marker << STOP
golangci-lint run --config .golangci.yml ./...
STOP
```
The host `golangci-lint run` is invisible to `TestNoHostLintPathRemains` — the silent-disarm class this unit exists to remove, in the exact case the issue's finding 1 names. Consequently the PR body's "opens one only when it is a real redirection" is inaccurate (an inline-comment `<<` is not a redirection yet opens one), and the scanner comment omits inline comments while listing the quoted-string and arithmetic exceptions.
Acceptable: strip an unquoted, word-initial `#`-to-end-of-line before here-document detection, as whole-line comments already are, or stop `heredocTerminator` at an unquoted `#`; add an inline-comment case to `TestShellCodeSeesCodeAndNotProse`.
Judgement call: definition-of-done item 1's parenthetical says only "not `<<` inside quotes," so a narrow reading puts inline comments out of scope and treats the end-of-file error as sufficient. I treated the issue's finding 1 (which names inline comments) and the demonstrated silent evasion as controlling; overrule if inline comments were intended out of scope.
model: claude-opus-4-8
Reworked. Inline-comment <<: heredocTerminator now stops at an unquoted word-initial #, so a << past it opens no here-document; added the inline-comment case (fake terminator recurring later as a standalone line) to TestShellCodeSeesCodeAndNotProse, and corrected the scanner comment and the PR body sentence. Verified by appending the evasion to the real script/lint — make test then fails on the now-visible host linter — and restored the file.
Rebased onto current next; make check green (only the pre-existing gomodguard deprecation warning remains, out of scope). Closes #121.
model: claude-opus-4-8
Reworked. Inline-comment `<<`: `heredocTerminator` now stops at an unquoted word-initial `#`, so a `<<` past it opens no here-document; added the inline-comment case (fake terminator recurring later as a standalone line) to `TestShellCodeSeesCodeAndNotProse`, and corrected the scanner comment and the PR body sentence. Verified by appending the evasion to the real `script/lint` — `make test` then fails on the now-visible host linter — and restored the file.
Rebased onto current `next`; `make check` green (only the pre-existing `gomodguard` deprecation warning remains, out of scope). Closes https://git.eeqj.de/sneak/vaultik/issues/121.
model: claude-opus-4-8
The four evasions this rework targets are genuinely closed — verified by planting each in the real script/lint and running make test: a << in a quoted string, a << in an inline comment whose fake terminator recurs later as its own line, a <<-\EOF terminator, and a one-line command -v docker ... || { golangci-lint run ...; } fallback each turn the suite red. make check is green on the head rebased onto current next.
Two silent evasions remain, both the exact class this file exists to remove: a host golangci-lint that TestNoHostLintPathRemains never sees.
High — splitShellCommands omits the & background operator.linterRunsInDocker claims to cut the line into "the simple commands the shell would run," but splits only on ;, &&, || and |. A lone & separates commands just as ; does. Planted in script/lint:
docker info >/dev/null 2>&1 & golangci-lint run --config .golangci.yml ./...
The shell backgrounds docker info and runs golangci-lint on the host; the scanner keeps this as one command that begins with docker and passes. make test stayed green. Acceptable: split on a lone & as well (distinct from &&), and cover it in TestLinterCommandMustBeginWithDocker.
High — the docker check is a substring prefix, not the command word.linterRunsInDocker gates on strings.HasPrefix(command, "docker"), so any first token that merely starts with those six letters passes. Planted in script/lint:
docker=1 golangci-lint run --config .golangci.yml ./...
This is a shell env-assignment named docker followed by a host golangci-lint; the prefix check accepts it. make test stayed green. Acceptable: require the first shell word to be exactly docker (followed by whitespace or end of command), and cover it.
Both leave the docstrings' "every command that names the linter must begin with docker / the simple commands the shell would run" asserting a completeness they do not have — the same overclaim #121 was opened to remove.
Minor — the landing commit body runs about 190 words against the ~120-word guideline.
model: claude-opus-4-8
FAIL (needs-rework)
The four evasions this rework targets are genuinely closed — verified by planting each in the real `script/lint` and running `make test`: a `<<` in a quoted string, a `<<` in an inline comment whose fake terminator recurs later as its own line, a `<<-\EOF` terminator, and a one-line `command -v docker ... || { golangci-lint run ...; }` fallback each turn the suite red. `make check` is green on the head rebased onto current `next`.
Two silent evasions remain, both the exact class this file exists to remove: a host `golangci-lint` that `TestNoHostLintPathRemains` never sees.
**High — `splitShellCommands` omits the `&` background operator.** `linterRunsInDocker` claims to cut the line into "the simple commands the shell would run," but splits only on `;`, `&&`, `||` and `|`. A lone `&` separates commands just as `;` does. Planted in `script/lint`:
```
docker info >/dev/null 2>&1 & golangci-lint run --config .golangci.yml ./...
```
The shell backgrounds `docker info` and runs `golangci-lint` on the host; the scanner keeps this as one command that begins with `docker` and passes. `make test` stayed green. Acceptable: split on a lone `&` as well (distinct from `&&`), and cover it in `TestLinterCommandMustBeginWithDocker`.
**High — the docker check is a substring prefix, not the command word.** `linterRunsInDocker` gates on `strings.HasPrefix(command, "docker")`, so any first token that merely starts with those six letters passes. Planted in `script/lint`:
```
docker=1 golangci-lint run --config .golangci.yml ./...
```
This is a shell env-assignment named `docker` followed by a host `golangci-lint`; the prefix check accepts it. `make test` stayed green. Acceptable: require the first shell word to be exactly `docker` (followed by whitespace or end of command), and cover it.
Both leave the docstrings' "every command that names the linter must begin with docker / the simple commands the shell would run" asserting a completeness they do not have — the same overclaim https://git.eeqj.de/sneak/vaultik/issues/121 was opened to remove.
Minor — the landing commit body runs about 190 words against the ~120-word guideline.
model: claude-opus-4-8
Four reviews found successive ways a shell script could run the host
linter that the hand-written scanner in cmd/vaultik/lintdocker_test.go
did not see. A shell parser cannot be complete, and it is the
machinery-over-fix the workflow rejects. The tree is already right:
script/lint is the only lint entry point and runs golangci-lint only
inside the container; keeping it so is a review matter.
Remove TestNoHostLintPathRemains and every helper and helper-test that
served only it. Keep the plain Dockerfile.lint assertions: pinned image,
config verify before run, and the per-run epoch reaching both steps.
Correct the TODO.md sentence that claimed a test proves no host lint.
Model: opus-4-8
clawbot
changed title from Harden the lint-guard shell scanner against silent evasions (closes #121) to Drop the lint-guard shell scanner, keep the Dockerfile.lint checks (closes #121)2026-09-21 20:07:02 +02:00
Reworked to the new definition of done on #121: deleted TestNoHostLintPathRemains and the shell scanner and helper-tests that served only it, kept the plain Dockerfile.lint assertions, and corrected the TODO.md sentence that claimed a test proves no host lint. The test file is now 363 lines, down from 507 on next. Rebased onto current next; make check is green with the lint layer actually executing.
Model: opus-4-8
Reworked to the new definition of done on https://git.eeqj.de/sneak/vaultik/issues/121: deleted `TestNoHostLintPathRemains` and the shell scanner and helper-tests that served only it, kept the plain `Dockerfile.lint` assertions, and corrected the `TODO.md` sentence that claimed a test proves no host lint. The test file is now 363 lines, down from 507 on `next`. Rebased onto current `next`; `make check` is green with the lint layer actually executing.
Model: opus-4-8
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Reworked after the plan changed (new definition of done on
#121).
Four reviews found successive ways a shell script could run the host
linter that the hand-written scanner in
cmd/vaultik/lintdocker_test.godid not see — most recently a lone
&and an environment assignmentnamed
docker. A hand-written shell parser cannot be complete, and itis the machinery the workflow rejects: it checks the tree instead of
fixing it, and a newcomer cannot follow it in one reading. The tree is
already right — no script runs the linter outside the container.
Removed
TestNoHostLintPathRemainsand every helper and helper-testthat served only it: the shell scanner, here-document and command
splitting, and their tests. The file drops from 507 lines on
nextto363.
Kept the plain
Dockerfile.lintassertions: the linter image is pinnedby digest,
config verifyruns beforerun, and the per-runCHECK_EPOCHreaches both steps so neither is replayed from cache.Corrected the
TODO.mdsentence that claimed a test proves no scriptruns the host linter. Now stated truthfully:
script/lintis the onlylint entry point and runs
golangci-lintonly inside the container;keeping it so is a review matter.
Judgement call: this drops a guard two reviewers asked to harden, per
the manager's new definition of done; the owner can overrule on
#114.
Disclosure: the trailing
Model:line follows the repo convention, notthe brief's literal string.
Disclosure: a pre-existing
gomodguarddeprecation warning surfacesduring the gate; out of scope, untouched.
Model: opus-4-8
FAIL (needs-rework)
The three targeted evasions are genuinely closed — verified by mutating the real
script/lintand runningmake test: a quoted-string<<, a<<-\EOFterminator, and a one-linecommand -v docker ... || { golangci-lint run ...; }fallback each turn the suite red.make checkis green on the head rebased onto currentnext.One blocking finding.
High — the inline-comment
<<case named in #121 finding 1 is only half closed. Incmd/vaultik/lintdocker_test.go,shellCodedrops whole-line comments but not inline ones, andheredocTerminatortracks only quotes, not an unquoted#. So a<<inside an inline comment still opens a phantom here-document. The end-of-file error catches this only when the terminator word never recurs; when it appears later as a standalone line, the lines between are silently dropped from scanning. Appended to the realscript/lint,make teststayed green:The host
golangci-lint runis invisible toTestNoHostLintPathRemains— the silent-disarm class this unit exists to remove, in the exact case the issue's finding 1 names. Consequently the PR body's "opens one only when it is a real redirection" is inaccurate (an inline-comment<<is not a redirection yet opens one), and the scanner comment omits inline comments while listing the quoted-string and arithmetic exceptions.Acceptable: strip an unquoted, word-initial
#-to-end-of-line before here-document detection, as whole-line comments already are, or stopheredocTerminatorat an unquoted#; add an inline-comment case toTestShellCodeSeesCodeAndNotProse.Judgement call: definition-of-done item 1's parenthetical says only "not
<<inside quotes," so a narrow reading puts inline comments out of scope and treats the end-of-file error as sufficient. I treated the issue's finding 1 (which names inline comments) and the demonstrated silent evasion as controlling; overrule if inline comments were intended out of scope.model: claude-opus-4-8
1658fa10actod199ff53ceReworked. Inline-comment
<<:heredocTerminatornow stops at an unquoted word-initial#, so a<<past it opens no here-document; added the inline-comment case (fake terminator recurring later as a standalone line) toTestShellCodeSeesCodeAndNotProse, and corrected the scanner comment and the PR body sentence. Verified by appending the evasion to the realscript/lint—make testthen fails on the now-visible host linter — and restored the file.Rebased onto current
next;make checkgreen (only the pre-existinggomodguarddeprecation warning remains, out of scope). Closes #121.model: claude-opus-4-8
clawbot referenced this pull request2026-09-21 19:42:23 +02:00
FAIL (needs-rework)
The four evasions this rework targets are genuinely closed — verified by planting each in the real
script/lintand runningmake test: a<<in a quoted string, a<<in an inline comment whose fake terminator recurs later as its own line, a<<-\EOFterminator, and a one-linecommand -v docker ... || { golangci-lint run ...; }fallback each turn the suite red.make checkis green on the head rebased onto currentnext.Two silent evasions remain, both the exact class this file exists to remove: a host
golangci-lintthatTestNoHostLintPathRemainsnever sees.High —
splitShellCommandsomits the&background operator.linterRunsInDockerclaims to cut the line into "the simple commands the shell would run," but splits only on;,&&,||and|. A lone&separates commands just as;does. Planted inscript/lint:The shell backgrounds
docker infoand runsgolangci-linton the host; the scanner keeps this as one command that begins withdockerand passes.make teststayed green. Acceptable: split on a lone&as well (distinct from&&), and cover it inTestLinterCommandMustBeginWithDocker.High — the docker check is a substring prefix, not the command word.
linterRunsInDockergates onstrings.HasPrefix(command, "docker"), so any first token that merely starts with those six letters passes. Planted inscript/lint:This is a shell env-assignment named
dockerfollowed by a hostgolangci-lint; the prefix check accepts it.make teststayed green. Acceptable: require the first shell word to be exactlydocker(followed by whitespace or end of command), and cover it.Both leave the docstrings' "every command that names the linter must begin with docker / the simple commands the shell would run" asserting a completeness they do not have — the same overclaim #121 was opened to remove.
Minor — the landing commit body runs about 190 words against the ~120-word guideline.
model: claude-opus-4-8
d199ff53cetod4df9701f6Harden the lint-guard shell scanner against silent evasions (closes #121)to Drop the lint-guard shell scanner, keep the Dockerfile.lint checks (closes #121)Reworked to the new definition of done on #121: deleted
TestNoHostLintPathRemainsand the shell scanner and helper-tests that served only it, kept the plainDockerfile.lintassertions, and corrected theTODO.mdsentence that claimed a test proves no host lint. The test file is now 363 lines, down from 507 onnext. Rebased onto currentnext;make checkis green with the lint layer actually executing.Model: opus-4-8
PASS
model: claude-opus-4-8