Scan for vulnerabilities in CI, and cache the compile that pays for it - #29
Merged
Conversation
rethab
force-pushed
the
worktree-ci-static-analysis-and-caching
branch
2 times, most recently
from
July 12, 2026 09:19
385acfd to
799e468
Compare
The lint target only installed the pinned linter when no golangci-lint was found on $PATH, so anyone who already had one linted against that instead. This is not a theoretical drift: a v2.12.2 on the author's machine reports findings that v2.6.2 in CI never sees, and would equally hide findings that CI does gate on. Check the version of the binary we have and reinstall when it differs, and invoke it by absolute path so $PATH cannot pick another. Expose the pinned version as a target so CI can key a cache on it without restating the number, and let the test target take extra go test flags so CI can ask for a coverage profile without duplicating the command.
The linter set caught style and obvious bugs but nothing about security or
error handling. Adds gosec, errorlint, noctx, testifylint, nolintlint and a
dozen quieter ratchets that currently report nothing but will hold the line.
The findings worth calling out:
- 34 assertions in pkg/jira read assert.Error(t, &ErrUnexpectedResponse{}, err).
The second argument of assert.Error is the error under test, not an expected
value, so these asserted that a freshly built error is non-nil, which it
always is. They passed no matter what the client returned. They now check
the error with ErrorAs, and still pass, so nothing was hiding behind them.
- errorlint found four type switches and assertions on errors that would stop
recognising a jira error the moment anyone wrapped one.
- gosec found `jira man` creating its output directory 0777, under /tmp by
default, where anyone could then tamper with the generated pages.
- TestGetConfigHome called os.Clearenv and os.Setenv while marked parallel,
racing every other parallel test that reads the environment.
Also lifts golangci-lint's default issue caps. It reports at most 3 copies of
an identical finding, which is how the 34 vacuous assertions above showed up
as 3, and reads as a clean tail rather than a truncated one.
The gosec suppressions are deliberate and narrow: a CLI shells out to $EDITOR
and reads paths the user hands it, and --insecure is a documented flag.
testifylint's require-error check is left off. It is a good change and a
mechanical one, rewriting ~100 error assertions across 27 test files, which
has nothing to do with the above and belongs in its own commit.
Nothing watched the dependencies. Dependabot only tracked GitHub Actions, so Go modules moved only when someone bumped them by hand, and no advisory ever raised an alarm. Adds govulncheck (call-graph aware, so a finding is reachable rather than merely present), CodeQL for the cross-function paths gosec cannot see, and gomod and docker to Dependabot. The workflows are the most privileged code here and nothing checked them either: actionlint and zizmor now do. govulncheck and CodeQL also run weekly. A dependency becomes vulnerable when somebody else publishes an advisory, not when we push. On caching: the Makefile compiles into $(CURDIR)/.gocache, while setup-go asks the toolchain where GOCACHE is and dutifully caches ~/.cache/go-build, which the build never writes to. Every run recompiled the whole tree from scratch, twice, the race-enabled test build included. Naming that same path in the workflow env is the entire fix. The linter binary and its analysis cache are now cached too, keyed on the pinned version. The deliberate no-cache in release, snapshot and docker stays as it is. Those publish artifacts, and a cache any branch can write to is a way into them. Also adds a concurrency group so a superseded push stops burning a runner, and a coverage profile (34.3% today, reported, not gated). The cross-compile job covers the two release targets that can fail on their own. There is no OS-specific source in the tree and cgo is off, so every 64-bit unix target type-checks exactly like the linux/amd64 that lint and test already build. What is left is linux/386, where an int is 32 bits wide and a narrowing conversion that is harmless on amd64 is a real bug, and windows, the one GOOS that is not unix. Nothing compiled either until a tag was pushed.
Pinning the version means reinstalling on mismatch, and doing that in GOPATH/bin silently downgrades whatever golangci-lint the developer keeps there for other projects. bin/ is already gitignored and nothing else competes for it.
setup-go's cache key has no job discriminator, so every job in this workflow raced to save one entry per go.sum hash and the first to finish won. That was always a fast cross-compile: the saved entry held GOARCH=arm artifacts, the test job restored them, recompiled its race-enabled build from scratch every run, and could never save the result. The one compile the caching was meant to pay for was the one compile it never cached. Each job now saves its own entry, keyed by what it builds, and a trailing SHA in the key makes every run save so the cache tracks source changes instead of freezing at whatever the first run compiled.
rethab
force-pushed
the
worktree-ci-static-analysis-and-caching
branch
from
July 12, 2026 12:01
d3e90b3 to
f26b335
Compare
rethab
added a commit
that referenced
this pull request
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.