fix(ci): make the linters and secret scanning actually able to fail - #756
Merged
Conversation
An audit of the quality gates found several that were configured but inert. This sharpens the repo-local ones and fixes what they immediately surfaced. Secret scanning found nothing because it had no rules. A gitleaks config file REPLACES the built-in ruleset unless it extends it, and .gitleaks.toml declared only an allowlist. Verified: the same file with [extend] useDefault = true reports a planted Slack token, without it reports "no leaks found". Every secret pattern was invisible to CI. Turning it on flagged four doc examples. The JWT ones were truncated placeholders; docs/webhooks.md and middlewares/presets/ntfy-token.yaml carried a token-shaped ntfy value that is indistinguishable from a real one. All are now unmistakable placeholders. If that value was ever real it has been in public history since 2025-12 and needs rotating — replacing the file does not undo that. The blanket staticcheck exclusions hid a real defect. `text: "QF"` matched every QF check by message text, and the ST1018 exclusion was suppressing a finding about invisible Unicode: the HTML mail template contained five U+200B zero-width spaces, shipped in every notification mail, which is a known spam-filter signal. Removed, along with the exclusions; staticcheck now selects checks explicitly with QF1008 as the one documented exception. Also: uniq-by-line disabled, so a second linter's finding on a line another linter already reported is no longer dropped; five error strings lowercased to match both ST1005 and this codebase's own convention; eleven mechanical staticcheck findings fixed. gosec is about to become blocking in the shared workflow. Its three findings here are false positives and are annotated rather than worked around: both auth cookies set HttpOnly and SameSite=Strict and derive Secure from the request scheme deliberately, since the UI is commonly served over plain HTTP on a private network; the third marshals a password because the Docker API requires exactly that payload. Verified: golangci-lint 0 issues, gosec exit 0, gitleaks clean, build and vet clean under the default, integration and e2e tags, go test -race 14/14. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
github-actions
Bot
added
the
documentation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #756 +/- ##
==========================================
- Coverage 87.65% 87.64% -0.01%
==========================================
Files 90 90
Lines 12058 12054 -4
==========================================
- Hits 10569 10565 -4
Misses 1198 1198
Partials 291 291
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Repairing the gitleaks ruleset made CI red, correctly: gitleaks scans every commit, not just the working tree, so correcting the files does not remove what was committed. Five findings remain reachable in history. Each was reviewed. Two are the ntfy placeholder from 2025-12, two are a truncated JWT header used as an Authorization example, one is a curl auth header in the security docs. None is a live credential; if one had been, the answer would be rotation rather than an ignore entry, since a rewrite of public history is not a remedy anyone can rely on. Verified that this silences only those five: a freshly planted Slack token is still reported (leaks found: 1) with the ignore file in place, and the tree is clean again once it is removed. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
The previous commit rewrote .gitleaksignore instead of appending to it, dropping the test-file and test-constant entries that were already there. Restored verbatim. While restoring: those entries are bare paths and substrings, which is not the .gitleaksignore format — it matches finding fingerprints of the form commit:file:rule:line — so they suppress nothing. The suppression that actually works for those files lives in .gitleaks.toml's allowlist. Left in place and annotated rather than removed, since deleting them is a separate decision. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
golangci-lint analysed nothing behind a build tag: run.build-tags was unset, so the entire e2e suite and the Docker integration adapters — 18 files — were outside every linter. The config even carried exclusion rules written for those paths (path: integration_test\.go, path: e2e/.*_test\.go) that could never match, which is how long this went unnoticed. Enabling the tags exposed two findings, both fixed rather than suppressed: client.NewClientWithOpts in the benchmark is deprecated in favour of client.New, and the copyright header in core/integration_test_main.go abutted the package clause so it was being read as the package doc comment. It also exposed a latent platform problem. e2e/helpers_test.go uses syscall.Setpgid, Getpgid and Kill, which do not exist on Windows, but the suite was tagged e2e without unix — it had simply never been type-checked for that platform. Tagging only that file breaks the package, since it provides the helpers the other e2e files call, so the whole suite is now e2e && unix. It is unix-only in substance: it drives process groups and signals. Verified on both platforms, which the earlier verification did not do: GOOS=linux and GOOS=windows both report 0 issues. The suite still runs — 13 e2e tests found and passing — and build/vet are clean under the default, integration and e2e tags. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
QUICK_REFERENCE.md told operators to add max-runtime to a [job-exec] section. The key exists on RunJob, RunServiceJob and [global], but ExecJob has no such field, so the documented configuration is rejected. Point at the global default instead, and mark the block as ini rather than bash so a config-parsing check can see it — the snippet was invisible to the docs gate precisely because of the fence language. Found by the documentation-drift gate being built for this repo, which is the kind of defect it exists to catch. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
This was referenced Jul 28, 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
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.
An audit of this repo's quality gates found several that were configured but inert — the same pattern as the documentation rule that sat
disabled: truefor years. This sharpens the repo-local ones and fixes what they immediately surfaced.Secret scanning found nothing because it had no rules
A gitleaks config file replaces the built-in ruleset unless it extends it.
.gitleaks.tomldeclared only an[allowlist]— so there were zero detection rules and every scan reported "no leaks found" regardless of input.Proven by A/B on the same file:
no leaks found[extend] useDefault = trueleaks found: 1Turning it on immediately flagged four doc examples. The JWT ones are truncated placeholders.
docs/webhooks.mdandmiddlewares/presets/ntfy-token.yamlcarriedtk_AgQdq7mVBoFD37zQVN29RhuMzNIz2— indistinguishable in shape from a real ntfy access token. All are now unmistakable placeholders.The blanket staticcheck exclusions were hiding a real defect
text: "QF"is an unanchored regex over the message, so it dropped everyQF####check. Alongside it,text: "ST1018:"was suppressing a finding about invisible Unicode — and it was right: the HTML mail body template contained five U+200B zero-width spaces, shipped in every notification mail this daemon sends. Zero-width characters are a well-known spam-filter signal.Removed. staticcheck now selects checks explicitly (
allminusQF1008, the one pure-style check that fires widely here) instead of filtering by message text.Also
uniq-by-line: false— the default drops a second linter's finding whenever another linter already reported on that line.docker unavailable/creating docker clienteverywhere else. These five were the outliers.WriteString(fmt.Sprintf(…))→fmt.Fprintf, De Morgan, inferrable types).gosec annotations
netresearch/.github#312 makes gosec blocking. Its three findings here are false positives and are annotated with reasons rather than worked around:
HttpOnlyandSameSite=Strict, and deriveSecurefrom the request scheme on purpose — a hardtruewould silently break the common plain-HTTP-on-a-private-network deployment;X-Registry-Auth.Verification
golangci-lint0 issues ·gosecexit 0 ·gitleaksclean · build and vet clean under the default,integrationande2etags ·go test -race ./...14/14.