Skip to content

chore: bump Go to 1.25.8 and fix CI and gosec issues#378

Open
maxlambrecht wants to merge 4 commits intospiffe:mainfrom
maxlambrecht:chore/update-golangci-lint-gosec
Open

chore: bump Go to 1.25.8 and fix CI and gosec issues#378
maxlambrecht wants to merge 4 commits intospiffe:mainfrom
maxlambrecht:chore/update-golangci-lint-gosec

Conversation

@maxlambrecht
Copy link
Copy Markdown
Member

@maxlambrecht maxlambrecht commented Mar 25, 2026

What

  • Bump the minimum Go version to 1.25.8 in go.mod, add the unreleased changelog entry, and align the Makefile toolchain version with that minimum.
  • Update golangci-lint to v2.11.4 and fix the Makefile Go download URLs to use the full Go version so Windows CI can fetch the toolchain correctly.
  • Move SPIFFE TLS peer authorization from VerifyPeerCertificate to VerifyConnection in spiffetls/tlsconfig, and update related tests and smaller gosec-driven example/test cleanups.

Why

  • Keep the repository’s minimum supported Go version aligned with its support policy.
  • Make the lint/toolchain setup work with the newer Go target and restore Windows CI toolchain downloads.
  • Ensure custom SPIFFE TLS authorization also runs on resumed sessions, which VerifyPeerCertificate alone does not cover.

How tested

  • make lint
  • make test

@maxlambrecht maxlambrecht changed the title fix: update golangci-lint and address gosec findings chore: bump Go to 1.25.8 and fix CI and gosec issues Mar 25, 2026
Copy link
Copy Markdown
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, @maxlambrecht! I have some backcompat concerns that I'm not sure how to resolve right away...

}
}

func wrapVerifyConnection(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried about backwards compatibility. There are two situations:

  1. somebody passes a tls.Config where either VeriyPeerCertificate/VerifyConnection is set. This code handles that fine.
  2. somebody takes the returned tls.Config and wraps VerifyPeerCertificate. We'd break the latter here, because we no longer set VerifyPeerCertificate.

I need to think about how we handle this. Open to suggestions. It may be the case that we have to accept a small breaking change. In any case, whatever we pick, the nuances should be documented on the functions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. The implementation already preserves the input-side case by capturing any existing config.VerifyPeerCertificate / config.VerifyConnection callbacks and invoking them after SPIFFE auth succeeds. The remaining change is the narrow returned-config case: callers that previously wrapped VerifyPeerCertificate on the returned config now need to extend VerifyConnection instead, since SPIFFE auth has to live on VerifyConnection to also run on resumed sessions. I updated the function docs to call out that nuance explicitly, and I added focused tests for the preserved input-callback behavior and chaining/error propagation. So this is a narrow breaking change, but I think it is the right tradeoff here.

@maxlambrecht maxlambrecht force-pushed the chore/update-golangci-lint-gosec branch from f505442 to 6c7af92 Compare March 26, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants