fix: keep test scaffolding out of the binary and bump the toolchain - #254
Merged
Conversation
gosec became blocking in the shared workflow and flagged G103 (audit unsafe) in internal/server/test_helpers.go. The file has no _test suffix, so it compiled into package server: the mock LDAP client, the fixture builder and its reflect+unsafe write to ldap.Object's unexported dn field all shipped in the production binary, reachable by nothing but reachable all the same. Split it. LDAPClient is the only declaration production code uses (2 call sites), so it moves to ldap_client.go; MockLDAP, NewMockLDAP, setObjectDN and newTestServer move to test_helpers_test.go. unsafe and reflect are now compiled into the test binary only. No suppression comment: gosec runs without -tests, so the finding is gone because the code is gone from the build, not because it was silenced. The unsafe write itself stays as-is. simple-ldap-go writes Object.dn only in objectFromEntry when decoding a directory response and exposes no constructor taking a DN, so a fixture cannot be built through its public API. govulncheck was failing too, on four stdlib advisories. Move the module to go 1.26 with toolchain go1.26.5, matching ofelia. Verified with go1.26.5: gosec 0 issues, govulncheck reports 0 affecting vulnerabilities, go vet clean, tests pass, and no production file imports unsafe. Signed-off-by: Sebastian Mendel <[email protected]>
|
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
|
github-actions
Bot
added
dependencies
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #254 +/- ##
==========================================
- Coverage 75.39% 74.70% -0.69%
==========================================
Files 11 10 -1
Lines 512 427 -85
==========================================
- Hits 386 319 -67
+ Misses 101 88 -13
+ Partials 25 20 -5
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:
|
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.
gosec became blocking in the shared workflow (netresearch/.github#312) and immediately failed here, as did govulncheck. Both are fixed.
gosec: test scaffolding was in the production binary
G103 (audit unsafe) pointed at
internal/server/test_helpers.go. The file has no_testsuffix, so it compiled intopackage server— the mock LDAP client, the fixture builder, and itsreflect+unsafewrite toldap.Object's unexporteddnfield all shipped in the binary. Nothing called them, but they were there.Split by what production actually needs:
LDAPClientis the only declaration used outside tests (2 call sites) and moves toldap_client.go;MockLDAP,NewMockLDAP,setObjectDNandnewTestServermove totest_helpers_test.go.unsafeandreflectare now compiled into the test binary only.No suppression comment. gosec runs without
-tests, so the finding is gone because the code left the build — not because it was silenced. A#nosecthere would have suppressed nothing while implying it had.The unsafe write itself is unchanged and still justified: simple-ldap-go writes
Object.dnonly inobjectFromEntrywhen decoding a directory response and exposes no constructor taking a DN, so a fixture cannot be built through its public API. That library gap is worth closing separately; it is not this PR's job.govulncheck: four stdlib advisories
GO-2026-5856 (crypto/tls), GO-2026-5039 (net/textproto), GO-2026-5037 (crypto/x509), GO-2026-4971 (net). The module was on
go 1.25.1with notoolchain. It now usesgo 1.26withtoolchain go1.26.5, matching ofelia.Verification
Run with go1.26.5:
Issues : 0, exit 0Your code is affected by 0 vulnerabilities, exit 0go vet ./...clean,go test ./...passesgrep '"unsafe"'over non-test files returns nothing