chore: fix the lint gate, CI duplication, Docker context, and release trigger - #8
Merged
Conversation
The config was still in v1 format, so golangci-lint v2 refused to load it at all: "unsupported version of the configuration". Every `make lint` and the matching pull request checklist item had been failing on load rather than reporting anything. gosimple has since folded into staticcheck and tenv into usetesting, so both entries are gone. With the config loading, 15 findings surfaced. Fixed the real ones: unused parameters renamed to _, an unchecked w.Write, and a missing package comment. The two G704 SSRF findings are the purpose of the service and carry a nosec note pointing at the checks that do gate it. redactURL now strips control characters. It returned its input unchanged when parsing failed, so it was not the sanitiser the G706 suppressions would be claiming it was; a newline in a caller-supplied URL could have forged log lines on that branch.
The push trigger had been widened to every branch so the required status check would fire on pull requests. That was working around a name mismatch in the branch ruleset, which required "build / test" while the job reports as "test"; the ruleset now names the check correctly, so the trigger can go back to main and pull requests. Branch pushes were otherwise producing a duplicate run of the whole workflow, including the Docker build. golangci-lint had config but nothing ran it, even though CONTRIBUTING and the pull request template both ask for it. Add a lint job and make publish depend on it. Tests now run with -race, matching the Makefile.
The Dockerfile named its sources one by one, so a second .go file would have been dropped from the image with no error — the build would simply produce a binary missing that code. Copy go.mod and *.go instead, and add a .dockerignore that excludes everything else. The context was 11 MB, almost all of it the local image-proxy binary and .git, neither of which the build ever needed. Also add -trimpath so the image matches the release binaries.
The only tag in the repository is V1.0, and the trigger glob was 'v*', which is case sensitive. The workflow never ran for it, which is why that release carries no binaries at all. Match both cases. Also add --verify-tag so the release step fails rather than creating a tag of its own if the ref is missing, and run the tests with -race here too.
codeql.yml was removed in favour of CodeQL default setup, so the badge pointed at a workflow that no longer exists and rendered as an error. Default setup has no badge endpoint; scanning results live under the Security tab. Add a Development section covering the Makefile targets, note that lint needs golangci-lint v2, and mention that the release trigger accepts an uppercase tag.
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.
Description
A pass over the repository's tooling. Each commit is one fix.
unsupported version of the configuration), somake lintand the PR checklist item had never actually run. Migrated to the v2 schema;gosimplefolded intostaticcheckandtenvintousetesting. The 15 findings that surfaced are fixed or annotated.redactURLwas not a sanitiser. It returned its input unchanged when parsing failed, so a newline in a caller-supplied URL could forge log lines on that branch. It now strips control characters, which is what theG706suppressions rely on.lintjob;publishnow depends on it.pushtrigger had been widened to all branches to work around the ruleset requiringbuild / testwhile the job reports astest. The ruleset now names it correctly, so the trigger is back tomain+pull_request..gofile would have been dropped from the image with no error. Now copiesgo.mod+*.go, with a.dockerignorethat cuts the context from 11 MB (mostly the local binary and.git) to the sources.V1.0; the glob wasv*, which is case sensitive — hence a release with no binaries. Matches both cases now.Verification
go test -racepasses;golangci-lint run(v2.12.2) reports 0 issues./health200,169.254.169.254403,file://400.Security implications
No change to the SSRF, allowlist, or rate-limit logic. The one behavioural change is
redactURLstripping control characters, which strictly narrows what can reach the log.