VC-59682: Align the go directive with the vendored Go 1.27.1 toolchain

[FelixPhipps]

Problem

go.mod declares go 1.26.4 while VENDORED_GO_VERSION is 1.27.1, the toolchain CI builds with. Richard's 0c29720 (#831) moved the toolchain and left the directive behind. This fixes no bug — the agent was never broken for ML-DSA, since it ships raw PEM unparsed and its one customer-certificate parse point reads only IsCA and ExtKeyUsage. But the mismatch has two costs:

• go vet gates standard library symbols on the go directive, so nothing in this repo can import crypto/mldsa and the agent's ML-DSA behaviour cannot be pinned by a test at all. (Correction to the original wording, which said "the compiler": go build succeeds at any directive level with a 1.27 toolchain. It is go vet that reports mldsa.GenerateKey requires go1.27 or later (file is go1.26), and go test runs vet, so the test is what cannot exist. Same practical conclusion, different mechanism.)
• Nothing stops a Go 1.26 build. Once anyone reads cert.PublicKeyAlgorithm to report a key type, which is where PQC discovery is heading, that build silently reports 0 instead of failing to compile.

Solution

Move the directive to 1.27.1 and add ML-DSA-65 cases to TestIsExcludableSecret pinning that a post-quantum chain is classified exactly like the P-256 one. The helper imports crypto/mldsa, so the suite now also fails to build on an older toolchain, which gives the directive teeth. The bump activates modernize/embedlit, which would rewrite ~250 lines of unrelated Kubernetes object literals and whose --fix leaves a tree that does not compile; disabled in .golangci.yaml. Remaining go.mod churn is go mod tidy canonicalising three require blocks into two.

Test plan

• TestIsExcludableSecret — ML-DSA-65 client-auth leaf kept, server-auth leaf excluded, matching the existing P-256 cases. All 11 subtests pass.
• The agent's parse path run against a real ML-DSA-65 chain on go1.26.5 and go1.27.1 gives identical classification, confirming this is a guard, not a fix.
• make -j verify green — golangci-lint 0 issues at GOVERSION=1.27.1, tree clean.
• make -j test-unit green — 465 tests, 4 skipped, 0 failures.

[wallrj-cyberark]

Four points, none blocking the direction. I checked out the branch and ran it: make verify-golangci-lint is 0 issues, make verify-govulncheck is clean, go mod tidy and make generate-golangci-lint-config are both idempotent, and TestIsExcludableSecret passes all 11 subtests. Your --fix claim reproduces too: enabling embedlit and running --fix rewrites five files and leaves an unused metav1 import in pkg/agent/run.go, so the tree does not compile.

The four comments are about how durable the change is, not whether it works today.

[wallrj-cyberark]

An unknown analyzer name here is silently ignored, so this suppression can rot without telling you.

I changed embedlit to embedlitXXX on this branch and ran GOVERSION=1.27.1 golangci-lint run -c .golangci.yaml. No error, no warning, and the embedlit findings came straight back. golangci-lint v2.13.2 does not validate the names in settings.modernize.disable.

So if x/tools renames or splits embedlit in a later bump, this entry becomes a no-op and CI goes red with the ~200 lines you describe, with nothing pointing at the dead config line.

Please use an exclusions.rules entry instead. This file already sets exclusions.warn-unused: true, and golangci-lint reports how many issues each exclusion rule actually skipped. In the same run it prints Skipped 0 issues by rules: [Path: "third_party", ...] for the paths that match nothing. That gives you a signal when the suppression stops doing anything; settings.modernize.disable gives you none.

[FelixPhipps]

Done in 5453b8e — moved to exclusions.rules.

Reproduced your embedlitXXX result, and confirmed the replacement behaves the way you expect:

level=warning msg="[runner/exclusion_rules] Skipped 0 issues by rules: [Text: \"^embedlitXXX:\", Linters: \"modernize\"]"
46 issues:

So a typo'd rule announces itself and turns CI red, where the disable list did neither. (46, not the ~200 I claimed — that number was the line count --fix rewrites, and the 3 findings a plain run reports is max-same-issues: 3 truncating.)

The one thing I wanted to prove before making the swap: exclusions.rules only filters the report, so I checked it does not reopen the --fix hole that the disable list was closing. golangci-lint run --fix -c with the new rule leaves the tree untouched — exclusions are applied before fixes. make fix-golangci-lint is a documented target here, so that mattered.

make generate-golangci-lint-config is still idempotent over two runs with the comment block in its new position.

[Generated with claude code]

[wallrj-cyberark]

These two cases do not pin what the helper comment says they pin.

isExcludableTLSSecret branches only on isClientCertificate(cert), which reads ExtKeyUsage. The subtests assert only the exclude boolean. Nothing in the path touches the key.

So take the scenario from your own description: someone adds cert.PublicKeyAlgorithm key-type reporting and gets it wrong for ML-DSA. Both of these still pass. The regression they are meant to catch goes through untouched.

The crypto/mldsa import is the part that earns its place, and it does so at compile time, not at assert time. Please either assert something algorithm-specific — parse the leaf and check PublicKeyAlgorithm == x509.MLDSA65, which is UnknownPublicKeyAlgorithm on 1.26 — or drop the claim from the comment and say plainly that the import exists to hold the toolchain floor.

[FelixPhipps]

Agreed, and fixed in 5453b8e. The helper now parses the leaf and asserts on it.

One correction to the suggested check: x509.MLDSA65 is a SignatureAlgorithm (enum 18), not a PublicKeyAlgorithm. The PublicKeyAlgorithm constant added in 1.27 is x509.MLDSA (enum 5). Comparing PublicKeyAlgorithm == x509.MLDSA65 would compile — both are int underneath — and never hold. So the helper asserts both, each against the right type:

require.Equal(t, x509.MLDSA, leaf.PublicKeyAlgorithm)
require.Equal(t, x509.MLDSA65, leaf.SignatureAlgorithm)

and sampleCertificateChain asserts x509.ECDSA for symmetry.

Checked the assertion is live rather than vacuous: swapping the expectation to x509.ECDSA fails with expected: 3, actual: 5.

[Generated with claude code]

[wallrj-cyberark]

Confirmed against the 1.27.1 stdlib: x509.MLDSA is a PublicKeyAlgorithm (5), x509.MLDSA65 a SignatureAlgorithm (18). My version would have compared across the two types through require.Equal's any parameters and never held. Asserting both is better than what I asked for.

Mutation-checked all three assertions and each fails as it should, including x509.ECDSA → x509.MLDSA on the P-256 helper.

[wallrj-cyberark]

This is a 60-line copy of sampleCertificateChain directly above it. The only differences are the two key generations, two subject strings, and caPrivKey.Public() in place of &caPrivKey.PublicKey.

It will drift. Someone adds a SAN or changes NotAfter in the ECDSA helper to fix an unrelated test, this copy keeps the old shape, and "classified exactly like the elliptic curve one" quietly stops comparing like with like. Nothing fails when that happens.

Please take the two keys as parameters and keep one body. crypto.Signer plus Public() covers *ecdsa.PrivateKey and *mldsa.PrivateKey alike, so the ECDSA caller just passes ecdsa.GenerateKey(...) and the new one passes mldsa.GenerateKey(mldsa.MLDSA65()).

[FelixPhipps]

Done in 5453b8e. Both samples are now wrappers over one certificateChain(t, newKey func() (crypto.Signer, error), usages ...x509.ExtKeyUsage) (string, *x509.Certificate).

*mldsa.PrivateKey documents itself as implementing crypto.Signer, so caPrivKey.Public() works for both and the &caPrivKey.PublicKey special case disappears. The two subject strings are gone too — nothing asserts on them, so both chains now use Test CA / example.com, which removes the other axis you could drift along.

It returns the parsed leaf so each wrapper can assert its own algorithm, which is what your other comment asked for.

[Generated with claude code]

[wallrj-cyberark]

Please cut this down to a line or two. "crypto/mldsa exists only in Go 1.27+, so this also fails to build under an older toolchain, which is the requirement the go directive in go.mod states" is the PR description, not a fact about the helper. It goes stale the moment the directive moves again.

Something like // sampleCertificateChain with ML-DSA-65 keys. The crypto/mldsa import holds the Go 1.27 floor: this package will not build on an older toolchain. carries everything the next reader needs.

[FelixPhipps]

Done in 5453b8e, using your wording with the name corrected to match the function:

// sampleMLDSACertificateChain is certificateChain with ML-DSA-65 keys. The
// crypto/mldsa import holds the Go 1.27 floor: this package will not build on
// an older toolchain.

The reasoning about what isExcludableTLSSecret reads has gone; it belongs in the PR, as you say.

[Generated with claude code]

[FelixPhipps]

Thanks — all four taken, pushed as 5453b8e.

• .golangci.yaml: embedlit suppression moved to exclusions.rules. Also checked --fix still leaves the tree alone with the exclusion in place, since make fix-golangci-lint is the reason the disable list existed.
• The ML-DSA helper now asserts x509.MLDSA / x509.MLDSA65 on the parsed leaf (note MLDSA65 is a SignatureAlgorithm, not a PublicKeyAlgorithm — details in that thread).
• One certificateChain body parameterised on a crypto.Signer factory serves both samples; the copy is gone.
• Doc comment trimmed to your wording.

Separately, I have corrected the PR description: I wrote that the compiler gates stdlib symbols on the go directive. It does not — go build succeeds at any directive level with a 1.27 toolchain. It is go vet that reports mldsa.GenerateKey requires go1.27 or later (file is go1.26), and go test runs vet, which is why the test is the thing that cannot exist below the directive. The conclusion stands, the mechanism was wrong.

make -j verify green, make test-unit green at 465 tests / 4 skipped.

[Generated with claude code]

[wallrj-cyberark]

Approving. All four are addressed at 5453b8e, and you are right on both points where you corrected me.

The exclusion-rule swap was the change worth proving, because exclusions.rules filters reported issues where settings.modernize.disable switched the analyser off. Those are not the same mechanism, so I checked it does not reopen the --fix hole. It does not, and a typo now fails loudly, which was the whole point of the original comment.

You are right about the constants. x509.MLDSA is a PublicKeyAlgorithm (5) and x509.MLDSA65 a SignatureAlgorithm (18), confirmed against the 1.27.1 stdlib. My suggested check would have compared across the two types through require.Equal's any parameters, compiled, and never held. Asserting both, each against the right type, is better than what I asked for.

Your correction to the description is also right. With the directive lowered to go 1.26.4, go build ./pkg/client/ exits 0, while go vet and go test both report mldsa.GenerateKey requires go1.27 or later (file is go1.26). The test is what cannot exist below the directive, not the build.

Nothing further from me.

What I ran at 5453b8e, including the --fix negative control

Detached worktree at 5453b8e, vendored Go 1.27.1.

Exclusion rule behaviour:

Check	Result
Rule present	0 issues
Rule removed	3 issues, all modernize (max-same-issues: 3 is capping, not the true count)
Rule typo'd to ^embedlitXXX:	Skipped 0 issues by rules: [Text: "^embedlitXXX:", Linters: "modernize"] and 3 issues, so CI goes red
--fix with the rule	no files modified
--fix with the rule removed	5 files, 214 lines, unused metav1 import in pkg/agent/run.go, tree no longer builds

The last two rows are the pair that matters: the protection survives the swap, and the negative control shows the test is not passing vacuously.

The rule is also live rather than merely unreported. warn-unused only warns about rules that match nothing, and the embedlit rule draws no warning on a clean run while the three formatter path rules do.

Mutation tests on the new assertions, each failing as it should:

Mutation	Failure
x509.MLDSA → x509.ECDSA	expected: 3, actual: 5
x509.MLDSA65 → x509.ECDSAWithSHA256	expected: 10, actual: 18
x509.ECDSA → x509.MLDSA on the P-256 helper	expected: 5, actual: 3

Everything else:

• make verify-golangci-lint — 0 issues
• TestIsExcludableSecret — 11/11 subtests pass
• go mod tidy — no drift in go.mod or go.sum
• make generate-golangci-lint-config — idempotent, the comment block survives in its new position inside exclusions.rules
• certificateChain does not collide with any existing identifier, and nothing in the package asserts on the subject strings you dropped