fix(certificate): UnpackPEM selects leaf by topology, not position#53
Merged
Conversation
UnpackPEM assumed certificates[0] was the end-entity leaf, which returns the root CA when Keyfactor Command sends a non-leaf-first PEM bundle (e.g. externally-rooted chains returned root-first). Select the leaf via findLeafCert (already used by DownloadCertificate) so selection is order-independent and consistent across both code paths; the remaining certs become the CA chain in their original order. Also add the missing go-pkcs12 v0.4.0 module zip checksum to go.sum — go.mod pinned v0.4.0 but go.sum only carried the v0.4.0/go.mod hash, breaking clean builds (missing go.sum entry). Adds UnpackPEM leaf-selection regression tests (root-first / leaf-first / shuffled orderings, 2- and 3-cert chains, and a bundle with a private key). Verified red->green against the pre-fix code. Fixes #52
This was referenced Jun 16, 2026
213a362 to
736930e
Compare
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.
Summary
UnpackPEMselected the leaf certificate positionally — it assumedcertificates[0]was the end-entity leaf:This returns the root CA as the leaf whenever Keyfactor Command sends a PEM bundle that is not leaf-first. Externally-rooted chains (e.g. DigiCert PKIaaS) are commonly returned root-first, so
certificates[0]is the root. Consumers that trust the returned leaf (e.g. the Terraform provider populatingcommon_name/certificate_pem) then persist the root CA's subject, forcing certificate replacement on every run.Fix
Select the leaf by chain topology using the package's existing
findLeafCert(the cert no other cert in the set issued) — the same helperDownloadCertificatealready uses for the P7B path. The remaining certs become the CA chain, preserving their original order. Falls back to index 0 when no certs parse, preserving prior behavior for degenerate inputs.This makes leaf selection order-independent and consistent across the P7B and PEM code paths.
Also included
github.com/spbsoluble/go-pkcs12 v0.4.0module zip checksum (h1:).go.modpins v0.4.0 butgo.sumonly carried the/go.modhash, so clean builds failed withmissing go.sum entry for module providing package github.com/spbsoluble/go-pkcs12.Tests
New
v3/api/unpackpem_leaf_test.go:TestUnpackPEM_LeafSelection— root-first / leaf-first / shuffled orderings, 2- and 3-cert chains; asserts the non-CA leaf is selected and the chain length is correct.TestUnpackPEM_WithPrivateKey_RootFirst— root-first bundle with a private key block; asserts both key extraction and correct leaf selection.TestUnpackPEM_SingleCert— single cert returned as leaf, empty chain.Verified red→green: the root-first / shuffled cases fail against the pre-fix code (return
Test Root CA) and pass after the fix. Full./api/...suite green.Fixes #52