From 531f2f23b103515c9c4252ef331c8b6188760515 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:48:47 +0200 Subject: [PATCH 01/12] git: add Signer alias for go-git interface Re-exports the go-git Signer interface from the signature package so consumers can refer to it without importing go-git directly. Prepares for the upcoming NewOpenPGPSigner and NewSSHSigner constructors. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/signature/signature.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/git/signature/signature.go b/git/signature/signature.go index 0dc829631..492a49a9f 100644 --- a/git/signature/signature.go +++ b/git/signature/signature.go @@ -19,8 +19,18 @@ package signature import ( "slices" "strings" + + gogit "github.com/go-git/go-git/v5" ) +// Signer is the interface used to sign Git commits and tags. It is +// re-exported from go-git so that consumers do not need to import the +// go-git package directly. +// +// Concrete implementations are returned by [NewOpenPGPSigner] and +// [NewSSHSigner] in this package. +type Signer = gogit.Signer + // signatureType is the canonical string returned by GetSignatureType for // each recognised signature category. The values are unexported on // purpose: callers should compare against the strings returned by From 1ed3c9f6ddcda141cf738c97e161650216d88a48 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:49:00 +0200 Subject: [PATCH 02/12] git: add NewOpenPGPSigner constructor Wraps an *openpgp.Entity in a Signer so it can be used through the new generic signing path on CommitOptions. Produces the same ASCII-armored detached signature as go-git's internal gpgSigner, preserving existing commit-signing output bit-for-bit. The concrete OpenPGPSigner type is exported so callers can type-assert a Signer returned by NewOpenPGPSigner and distinguish it from other Signer implementations. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/signature/gpg_signature.go | 37 ++++++++++++++++++ git/signature/gpg_signer_test.go | 66 ++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 git/signature/gpg_signer_test.go diff --git a/git/signature/gpg_signature.go b/git/signature/gpg_signature.go index 90ee2112b..db7589c7d 100644 --- a/git/signature/gpg_signature.go +++ b/git/signature/gpg_signature.go @@ -18,7 +18,9 @@ package signature import ( "bytes" + "errors" "fmt" + "io" "strings" "github.com/ProtonMail/go-crypto/openpgp" @@ -88,3 +90,38 @@ func VerifyPGPSignature(signature string, payload []byte, keyRings ...string) (s return "", fmt.Errorf("unable to verify payload with any of the given key rings: %w", ErrNoMatchingKey) } + +// OpenPGPSigner adapts an [openpgp.Entity] to the [Signer] interface so it +// can be used as a generic Git commit signer. Callers may type-assert a +// [Signer] returned by [NewOpenPGPSigner] back to *OpenPGPSigner to inspect +// or distinguish it from other Signer implementations. +type OpenPGPSigner struct { + entity *openpgp.Entity +} + +// Sign produces an ASCII-armored detached OpenPGP signature over the +// message read from r. The output matches what go-git's internal gpgSigner +// produces, so it is interchangeable with the previous typed-entity +// signing path. +func (s *OpenPGPSigner) Sign(r io.Reader) ([]byte, error) { + var buf bytes.Buffer + if err := openpgp.ArmoredDetachSign(&buf, s.entity, r, nil); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// NewOpenPGPSigner returns a [Signer] that signs commits with the given +// OpenPGP entity. The entity's private key must be present and decrypted; +// callers that load keys from passphrase-protected key rings are +// responsible for decryption before constructing the signer. +// +// The constructor returns an error only when the entity is nil; today no +// other validation is performed, but the (Signer, error) shape leaves room +// to add validation later without an API break. +func NewOpenPGPSigner(e *openpgp.Entity) (Signer, error) { + if e == nil { + return nil, errors.New("nil openpgp entity") + } + return &OpenPGPSigner{entity: e}, nil +} diff --git a/git/signature/gpg_signer_test.go b/git/signature/gpg_signer_test.go new file mode 100644 index 000000000..169b26fab --- /dev/null +++ b/git/signature/gpg_signer_test.go @@ -0,0 +1,66 @@ +/* +Copyright 2026 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package signature_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/armor" + . "github.com/onsi/gomega" + + "github.com/fluxcd/pkg/git/signature" +) + +func TestNewOpenPGPSigner(t *testing.T) { + t.Run("nil entity returns error", func(t *testing.T) { + g := NewWithT(t) + + _, err := signature.NewOpenPGPSigner(nil) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("nil openpgp entity")) + }) + + t.Run("sign produces detached armored signature verifiable by VerifyPGPSignature", func(t *testing.T) { + g := NewWithT(t) + + entity, err := openpgp.NewEntity("Test", "test signing key", "test@example.com", nil) + g.Expect(err).ToNot(HaveOccurred()) + + signer, err := signature.NewOpenPGPSigner(entity) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(signer).ToNot(BeNil()) + + payload := []byte("hello world") + sig, err := signer.Sign(bytes.NewReader(payload)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(sig).ToNot(BeEmpty()) + g.Expect(string(sig)).To(HavePrefix("-----BEGIN PGP SIGNATURE-----")) + + // Round-trip: armor the public key and verify the produced signature. + var sb strings.Builder + armorWriter, err := armor.Encode(&sb, "PGP PUBLIC KEY BLOCK", nil) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(entity.Serialize(armorWriter)).To(Succeed()) + g.Expect(armorWriter.Close()).To(Succeed()) + + _, err = signature.VerifyPGPSignature(string(sig), payload, sb.String()) + g.Expect(err).ToNot(HaveOccurred()) + }) +} From b2cbaf3fac5134013579e2fe6e8b5d9a1e875be1 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:49:51 +0200 Subject: [PATCH 03/12] git: add NewSSHSigner constructor (happy path) Wraps an SSH private key in a Signer that produces SSHSIG-armored signatures with namespace "git" and SHA-512 hash, matching Git's defaults for SSH-signed commits. The concrete SSHSigner type is exported so callers can type-assert a Signer returned by NewSSHSigner. This commit covers the unencrypted- key happy path; encrypted keys and the algorithm allowlist land in follow-up commits. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/signature/ssh_signature.go | 39 +++++++++++ git/signature/ssh_signer_test.go | 107 +++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 git/signature/ssh_signer_test.go diff --git a/git/signature/ssh_signature.go b/git/signature/ssh_signature.go index 896c3b2d1..7b8c0c264 100644 --- a/git/signature/ssh_signature.go +++ b/git/signature/ssh_signature.go @@ -20,6 +20,7 @@ import ( "bytes" "errors" "fmt" + "io" "strings" "github.com/hiddeco/sshsig" @@ -137,6 +138,44 @@ func VerifySSHSignature(signature string, payload []byte, authorizedKeys ...stri errors.Join(append([]error{ErrNoMatchingKey}, verifyErrors...)...)) } +// SSHSigner adapts a [gossh.Signer] to the [Signer] interface, producing +// SSHSIG-armored signatures with namespace [SSHSignatureNamespace] ("git") +// and SHA-512 as the hash algorithm, matching Git's defaults for SSH-signed +// commits. Callers may type-assert a [Signer] returned by [NewSSHSigner] +// back to *SSHSigner to inspect or distinguish it from other Signer +// implementations. +type SSHSigner struct { + inner gossh.Signer +} + +// Sign produces an SSHSIG-armored signature over the message read from r, +// using SHA-512 and the "git" namespace. +func (s *SSHSigner) Sign(r io.Reader) ([]byte, error) { + sig, err := sshsig.Sign(r, s.inner, sshsig.HashSHA512, SSHSignatureNamespace) + if err != nil { + return nil, err + } + return sshsig.Armor(sig), nil +} + +// NewSSHSigner returns a [Signer] that signs commits with the given SSH +// private key. The pem argument is the private key in any format accepted +// by [gossh.ParsePrivateKey], typically the OpenSSH "-----BEGIN OPENSSH +// PRIVATE KEY-----" format produced by ssh-keygen. The passphrase argument +// is consulted only when the private key is encrypted; pass nil for an +// unencrypted key. +// +// Signatures use namespace [SSHSignatureNamespace] ("git") and SHA-512, +// which match Git's defaults for SSH-signed commits. See +// https://git-scm.com/docs/gitformat-signature. +func NewSSHSigner(pem, passphrase []byte) (Signer, error) { + inner, err := gossh.ParsePrivateKey(pem) + if err != nil { + return nil, fmt.Errorf("could not parse SSH signing key: %w", err) + } + return &SSHSigner{inner: inner}, nil +} + // appendUniqueSentinel appends err to dst only when no existing element of // dst matches err under errors.Is. This keeps the joined error chain from // growing linearly with the number of keys when every key fails with the diff --git a/git/signature/ssh_signer_test.go b/git/signature/ssh_signer_test.go new file mode 100644 index 000000000..244466f5b --- /dev/null +++ b/git/signature/ssh_signer_test.go @@ -0,0 +1,107 @@ +/* +Copyright 2026 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package signature_test + +import ( + "bytes" + "crypto" + "crypto/ed25519" + "crypto/rand" + "encoding/pem" + "testing" + + . "github.com/onsi/gomega" + gossh "golang.org/x/crypto/ssh" + + "github.com/fluxcd/pkg/git/signature" +) + +// sshKeyFactory returns a private key and the OpenSSH-PEM-encoded form of +// it. It accepts a passphrase argument: when non-nil the PEM is encrypted +// with it, otherwise it is unencrypted. +type sshKeyFactory func(t *testing.T, passphrase []byte) (crypto.PrivateKey, []byte) + +func ed25519Key(t *testing.T, passphrase []byte) (crypto.PrivateKey, []byte) { + t.Helper() + g := NewWithT(t) + _, priv, err := ed25519.GenerateKey(rand.Reader) + g.Expect(err).ToNot(HaveOccurred()) + return priv, marshalPEM(t, priv, passphrase) +} + +func marshalPEM(t *testing.T, priv crypto.PrivateKey, passphrase []byte) []byte { + t.Helper() + g := NewWithT(t) + var ( + block *pem.Block + err error + ) + if len(passphrase) == 0 { + block, err = gossh.MarshalPrivateKey(priv, "test key") + } else { + block, err = gossh.MarshalPrivateKeyWithPassphrase(priv, "test key", passphrase) + } + g.Expect(err).ToNot(HaveOccurred()) + return pem.EncodeToMemory(block) +} + +func TestNewSSHSigner(t *testing.T) { + tests := []struct { + name string + key sshKeyFactory + pemPassphrase []byte // passphrase used to encrypt the PEM; nil = unencrypted + callPassphrase []byte // passphrase passed to NewSSHSigner; nil = none + expectErr string // substring expected in the error; empty = expect success + }{ + { + name: "ed25519 unencrypted", + key: ed25519Key, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + priv, pemBytes := tt.key(t, tt.pemPassphrase) + + signer, err := signature.NewSSHSigner(pemBytes, tt.callPassphrase) + if tt.expectErr != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.expectErr)) + g.Expect(signer).To(BeNil()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(signer).ToNot(BeNil()) + + // Round-trip: signer.Sign produces SSHSIG armor that verifies + // against the matching authorized_keys-formatted public key. + payload := []byte("hello sshsig world") + sig, err := signer.Sign(bytes.NewReader(payload)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(string(sig)).To(HavePrefix("-----BEGIN SSH SIGNATURE-----")) + + gosshSigner, err := gossh.NewSignerFromKey(priv) + g.Expect(err).ToNot(HaveOccurred()) + authorizedKey := gossh.MarshalAuthorizedKey(gosshSigner.PublicKey()) + + _, err = signature.VerifySSHSignature(string(sig), payload, string(authorizedKey)) + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} From 693a9c97c1dac20fdd88e3e75301ed2b857ebf53 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:50:16 +0200 Subject: [PATCH 04/12] git: handle encrypted SSH signing keys OpenSSH-format private keys cannot be classified as encrypted before attempting to parse. Detect *gossh.PassphraseMissingError, require a passphrase, and retry through ParsePrivateKeyWithPassphrase. Wrong- passphrase parse failures surface with the generic "could not parse" wrap; missing-passphrase errors are explicit. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/signature/ssh_signature.go | 12 +++++++++++- git/signature/ssh_signer_test.go | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/git/signature/ssh_signature.go b/git/signature/ssh_signature.go index 7b8c0c264..228709a0b 100644 --- a/git/signature/ssh_signature.go +++ b/git/signature/ssh_signature.go @@ -171,7 +171,17 @@ func (s *SSHSigner) Sign(r io.Reader) ([]byte, error) { func NewSSHSigner(pem, passphrase []byte) (Signer, error) { inner, err := gossh.ParsePrivateKey(pem) if err != nil { - return nil, fmt.Errorf("could not parse SSH signing key: %w", err) + var missingErr *gossh.PassphraseMissingError + if !errors.As(err, &missingErr) { + return nil, fmt.Errorf("could not parse SSH signing key: %w", err) + } + if len(passphrase) == 0 { + return nil, errors.New("SSH signing key is encrypted; passphrase required") + } + inner, err = gossh.ParsePrivateKeyWithPassphrase(pem, passphrase) + if err != nil { + return nil, fmt.Errorf("could not parse SSH signing key: %w", err) + } } return &SSHSigner{inner: inner}, nil } diff --git a/git/signature/ssh_signer_test.go b/git/signature/ssh_signer_test.go index 244466f5b..e2710abf8 100644 --- a/git/signature/ssh_signer_test.go +++ b/git/signature/ssh_signer_test.go @@ -60,6 +60,8 @@ func marshalPEM(t *testing.T, priv crypto.PrivateKey, passphrase []byte) []byte } func TestNewSSHSigner(t *testing.T) { + const passphrase = "correct horse battery staple" + tests := []struct { name string key sshKeyFactory @@ -71,6 +73,25 @@ func TestNewSSHSigner(t *testing.T) { name: "ed25519 unencrypted", key: ed25519Key, }, + { + name: "ed25519 encrypted with passphrase", + key: ed25519Key, + pemPassphrase: []byte(passphrase), + callPassphrase: []byte(passphrase), + }, + { + name: "ed25519 encrypted without passphrase", + key: ed25519Key, + pemPassphrase: []byte(passphrase), + expectErr: "SSH signing key is encrypted; passphrase required", + }, + { + name: "ed25519 encrypted with wrong passphrase", + key: ed25519Key, + pemPassphrase: []byte("right"), + callPassphrase: []byte("wrong"), + expectErr: "could not parse SSH signing key", + }, } for _, tt := range tests { From 2ace4e76357539de3f661857972f87ca4c38a3cf Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:51:05 +0200 Subject: [PATCH 05/12] git: restrict NewSSHSigner to safe algorithms Allowlist ssh-ed25519, ecdsa-sha2-nistp256/384/521, and ssh-rsa with key size at least 2048 bits. DSA and undersized RSA produce signatures modern OpenSSH refuses to verify, so reject them at construction time instead of letting downstream verify silently fail. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/signature/ssh_signature.go | 39 ++++++++++++++++++++++++++++++++ git/signature/ssh_signer_test.go | 34 ++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/git/signature/ssh_signature.go b/git/signature/ssh_signature.go index 228709a0b..328280315 100644 --- a/git/signature/ssh_signature.go +++ b/git/signature/ssh_signature.go @@ -18,6 +18,7 @@ package signature import ( "bytes" + "crypto/rsa" "errors" "fmt" "io" @@ -183,9 +184,47 @@ func NewSSHSigner(pem, passphrase []byte) (Signer, error) { return nil, fmt.Errorf("could not parse SSH signing key: %w", err) } } + if err := validateSSHSigningKey(inner.PublicKey()); err != nil { + return nil, err + } return &SSHSigner{inner: inner}, nil } +// validateSSHSigningKey rejects SSH public keys whose algorithm is not in +// the allowlist for commit signing. +// +// Allowlist: +// - ssh-ed25519 +// - ecdsa-sha2-nistp256, ecdsa-sha2-nistp384, ecdsa-sha2-nistp521 +// - ssh-rsa (require >= 2048-bit key) +// +// DSA and undersized RSA are rejected because they produce signatures that +// modern OpenSSH (>= 8.7) refuses to verify. +func validateSSHSigningKey(pub gossh.PublicKey) error { + switch pub.Type() { + case gossh.KeyAlgoED25519, + gossh.KeyAlgoECDSA256, + gossh.KeyAlgoECDSA384, + gossh.KeyAlgoECDSA521: + return nil + case gossh.KeyAlgoRSA: + ck, ok := pub.(gossh.CryptoPublicKey) + if !ok { + return fmt.Errorf("unable to inspect RSA public key: type %T does not expose crypto.PublicKey", pub) + } + rsaPub, ok := ck.CryptoPublicKey().(*rsa.PublicKey) + if !ok { + return errors.New("unable to inspect RSA public key: not an *rsa.PublicKey") + } + if bits := rsaPub.Size() * 8; bits < 2048 { + return fmt.Errorf("RSA key size %d bits is below the minimum supported by NewSSHSigner; must be at least 2048", bits) + } + return nil + default: + return fmt.Errorf("unsupported SSH signing key algorithm: %s", pub.Type()) + } +} + // appendUniqueSentinel appends err to dst only when no existing element of // dst matches err under errors.Is. This keeps the joined error chain from // growing linearly with the number of keys when every key fails with the diff --git a/git/signature/ssh_signer_test.go b/git/signature/ssh_signer_test.go index e2710abf8..58ab62668 100644 --- a/git/signature/ssh_signer_test.go +++ b/git/signature/ssh_signer_test.go @@ -19,8 +19,11 @@ package signature_test import ( "bytes" "crypto" + "crypto/ecdsa" "crypto/ed25519" + "crypto/elliptic" "crypto/rand" + "crypto/rsa" "encoding/pem" "testing" @@ -43,6 +46,24 @@ func ed25519Key(t *testing.T, passphrase []byte) (crypto.PrivateKey, []byte) { return priv, marshalPEM(t, priv, passphrase) } +func ecdsaP256Key(t *testing.T, passphrase []byte) (crypto.PrivateKey, []byte) { + t.Helper() + g := NewWithT(t) + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + g.Expect(err).ToNot(HaveOccurred()) + return priv, marshalPEM(t, priv, passphrase) +} + +func rsaKey(bits int) sshKeyFactory { + return func(t *testing.T, passphrase []byte) (crypto.PrivateKey, []byte) { + t.Helper() + g := NewWithT(t) + priv, err := rsa.GenerateKey(rand.Reader, bits) + g.Expect(err).ToNot(HaveOccurred()) + return priv, marshalPEM(t, priv, passphrase) + } +} + func marshalPEM(t *testing.T, priv crypto.PrivateKey, passphrase []byte) []byte { t.Helper() g := NewWithT(t) @@ -92,6 +113,19 @@ func TestNewSSHSigner(t *testing.T) { callPassphrase: []byte("wrong"), expectErr: "could not parse SSH signing key", }, + { + name: "ecdsa-sha2-nistp256 accepted", + key: ecdsaP256Key, + }, + { + name: "rsa 2048 accepted", + key: rsaKey(2048), + }, + { + name: "rsa 1024 rejected", + key: rsaKey(1024), + expectErr: "RSA key size 1024 bits is below the minimum supported by NewSSHSigner; must be at least 2048", + }, } for _, tt := range tests { From 903bfa829ed0371ab0e4a022b9350b4c6dfa0bb2 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:51:30 +0200 Subject: [PATCH 06/12] git: migrate CommitOptions.Signer to interface CommitOptions.Signer changes from *openpgp.Entity to the go-git Signer interface re-exported from the signature package, and WithSigner takes the same type. The gogit client forwards the value to go-git's generic Signer field, which takes precedence over the legacy typed SignKey. This is a breaking API change in pkg/git/repository; consumers must migrate to signature.NewOpenPGPSigner or signature.NewSSHSigner. Since pkg/git is still pre-1.0, the change lands in a coordinated minor. Refs fluxcd/pkg#398[1]. [1]: https://github.com/fluxcd/pkg/issues/398 Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/gogit/client.go | 2 +- git/repository/options.go | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/git/gogit/client.go b/git/gogit/client.go index bd6baba82..689c52c10 100644 --- a/git/gogit/client.go +++ b/git/gogit/client.go @@ -376,7 +376,7 @@ func (g *Client) Commit(info git.Commit, commitOpts ...repository.CommitOption) } if options.Signer != nil { - opts.SignKey = options.Signer + opts.Signer = options.Signer } commit, err := wt.Commit(info.Message, opts) diff --git a/git/repository/options.go b/git/repository/options.go index e2abd9379..c3d2850bb 100644 --- a/git/repository/options.go +++ b/git/repository/options.go @@ -19,7 +19,7 @@ package repository import ( "io" - "github.com/ProtonMail/go-crypto/openpgp" + "github.com/fluxcd/pkg/git/signature" ) const ( @@ -98,8 +98,10 @@ type CheckoutStrategy struct { // CommitOptions provides options to configure a Git commit operation. type CommitOptions struct { - // Signer can be used to sign a commit using OpenPGP. - Signer *openpgp.Entity + // Signer signs the resulting commit. May be nil for unsigned commits. + // Use [signature.NewOpenPGPSigner] or [signature.NewSSHSigner] to + // construct one. + Signer signature.Signer // Files contains file names mapped to the file's content. // Its used to write files which are then included in the commit. Files map[string]io.Reader @@ -109,10 +111,12 @@ type CommitOptions struct { type CommitOption func(*CommitOptions) // WithSigner allows for the commit to be signed using the provided -// OpenPGP signer. -func WithSigner(signer *openpgp.Entity) CommitOption { +// [signature.Signer]. Passing a nil interface is a no-op; the commit will +// be unsigned. See [signature.NewOpenPGPSigner] and +// [signature.NewSSHSigner] for the supported constructors. +func WithSigner(s signature.Signer) CommitOption { return func(co *CommitOptions) { - co.Signer = signer + co.Signer = s } } From 4466a4119c0c0505689473fc49c28ca523ef6271 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:52:38 +0200 Subject: [PATCH 07/12] git: cover commit with OpenPGP signer end-to-end Calls gogit.Client.Commit with WithSigner(NewOpenPGPSigner(...)) and verifies the resulting commit's gpgsig header through signature.VerifyPGPSignature. The table-driven harness leaves room for SSH and unsigned cases to land as new rows in follow-up commits. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/gogit/client_test.go | 118 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index 9094e3898..d5d6b5820 100644 --- a/git/gogit/client_test.go +++ b/git/gogit/client_test.go @@ -17,6 +17,7 @@ limitations under the License. package gogit import ( + "bytes" "context" "io" "os" @@ -25,12 +26,16 @@ import ( "testing" "time" + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/armor" extgogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/object" . "github.com/onsi/gomega" "github.com/fluxcd/pkg/git" "github.com/fluxcd/pkg/git/repository" + "github.com/fluxcd/pkg/git/signature" "github.com/fluxcd/pkg/gittestserver" ) @@ -173,6 +178,119 @@ func TestCommit(t *testing.T) { g.Expect(cc).ToNot(Equal(hash)) } +func TestCommit_WithSigner(t *testing.T) { + // signerVerifier returns the [signature.Signer] to install on the + // commit and a verifyFn that checks the resulting commit's gpgsig + // header. verifyFn is nil for the unsigned-commit case. + type verifyFn func(t *testing.T, commit *object.Commit) + type signerSetup func(t *testing.T) (signature.Signer, verifyFn) + + openpgpSetup := func(t *testing.T) (signature.Signer, verifyFn) { + t.Helper() + g := NewWithT(t) + entity, err := openpgp.NewEntity("Test", "openpgp test", "test@example.com", nil) + g.Expect(err).ToNot(HaveOccurred()) + signer, err := signature.NewOpenPGPSigner(entity) + g.Expect(err).ToNot(HaveOccurred()) + return signer, func(t *testing.T, commit *object.Commit) { + g := NewWithT(t) + g.Expect(commit.PGPSignature).To(HavePrefix("-----BEGIN PGP SIGNATURE-----")) + payload := commitPayload(t, commit) + var pubBuf bytes.Buffer + g.Expect(entity.Serialize(&pubBuf)).To(Succeed()) + armored, err := armorPGPPublicKey(&pubBuf) + g.Expect(err).ToNot(HaveOccurred()) + _, err = signature.VerifyPGPSignature(commit.PGPSignature, payload, armored) + g.Expect(err).ToNot(HaveOccurred()) + } + } + + tests := []struct { + name string + setup signerSetup + file string + message string + }{ + { + name: "openpgp", + setup: openpgpSetup, + file: "signed-openpgp", + message: "signed by openpgp", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + server, err := gittestserver.NewTempGitServer() + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(server.Root()) + + g.Expect(server.InitRepo("../testdata/git/repo", git.DefaultBranch, "test.git")).To(Succeed()) + + tmp := t.TempDir() + repo, err := extgogit.PlainClone(tmp, false, &extgogit.CloneOptions{ + URL: filepath.Join(server.Root(), "test.git"), + }) + g.Expect(err).ToNot(HaveOccurred()) + + ggc, err := NewClient(tmp, nil) + g.Expect(err).ToNot(HaveOccurred()) + ggc.repository = repo + + signer, verify := tt.setup(t) + + hash, err := ggc.Commit( + git.Commit{ + Author: git.Signature{Name: "Test User", Email: "test@example.com"}, + Message: tt.message, + }, + repository.WithFiles(map[string]io.Reader{ + tt.file: strings.NewReader("payload for " + tt.name), + }), + repository.WithSigner(signer), + ) + g.Expect(err).ToNot(HaveOccurred()) + + commit, err := repo.CommitObject(plumbing.NewHash(hash)) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(commit.PGPSignature).ToNot(BeEmpty()) + verify(t, commit) + }) + } +} + +// commitPayload returns the canonical commit-without-signature payload that +// signature.Verify{PGP,SSH}Signature expect for a verification round-trip. +func commitPayload(t *testing.T, commit *object.Commit) []byte { + t.Helper() + g := NewWithT(t) + encoded := &plumbing.MemoryObject{} + g.Expect(commit.EncodeWithoutSignature(encoded)).To(Succeed()) + r, err := encoded.Reader() + g.Expect(err).ToNot(HaveOccurred()) + b, err := io.ReadAll(r) + g.Expect(err).ToNot(HaveOccurred()) + return b +} + +// armorPGPPublicKey ASCII-armors r as a "PGP PUBLIC KEY BLOCK". +func armorPGPPublicKey(r io.Reader) (string, error) { + var sb strings.Builder + w, err := armor.Encode(&sb, "PGP PUBLIC KEY BLOCK", nil) + if err != nil { + return "", err + } + if _, err := io.Copy(w, r); err != nil { + return "", err + } + if err := w.Close(); err != nil { + return "", err + } + return sb.String(), nil +} + func TestPush(t *testing.T) { g := NewWithT(t) From ed303618c54ff33a7c54d1b020684fdcc9cf137f Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:53:13 +0200 Subject: [PATCH 08/12] git: cover commit with SSH ed25519 signer e2e Adds an ed25519 row to TestCommit_WithSigner that exercises NewSSHSigner and verifies the SSHSIG-armored gpgsig header through signature.VerifySSHSignature. Asserts the signature uses the "git" namespace and round-trips against the corresponding authorized_keys public key. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/gogit/client_test.go | 41 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index d5d6b5820..8575f6b10 100644 --- a/git/gogit/client_test.go +++ b/git/gogit/client_test.go @@ -19,6 +19,10 @@ package gogit import ( "bytes" "context" + "crypto" + "crypto/ed25519" + "crypto/rand" + "encoding/pem" "io" "os" "path/filepath" @@ -32,6 +36,7 @@ import ( "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" . "github.com/onsi/gomega" + gossh "golang.org/x/crypto/ssh" "github.com/fluxcd/pkg/git" "github.com/fluxcd/pkg/git/repository" @@ -205,6 +210,36 @@ func TestCommit_WithSigner(t *testing.T) { } } + sshSetup := func(keyFn func(t *testing.T) (crypto.PublicKey, []byte)) signerSetup { + return func(t *testing.T) (signature.Signer, verifyFn) { + t.Helper() + g := NewWithT(t) + pub, pemBytes := keyFn(t) + signer, err := signature.NewSSHSigner(pemBytes, nil) + g.Expect(err).ToNot(HaveOccurred()) + return signer, func(t *testing.T, commit *object.Commit) { + g := NewWithT(t) + g.Expect(commit.PGPSignature).To(HavePrefix("-----BEGIN SSH SIGNATURE-----")) + payload := commitPayload(t, commit) + gosshPub, err := gossh.NewPublicKey(pub) + g.Expect(err).ToNot(HaveOccurred()) + authorizedKey := gossh.MarshalAuthorizedKey(gosshPub) + _, err = signature.VerifySSHSignature(commit.PGPSignature, payload, string(authorizedKey)) + g.Expect(err).ToNot(HaveOccurred()) + } + } + } + + ed25519Key := func(t *testing.T) (crypto.PublicKey, []byte) { + t.Helper() + g := NewWithT(t) + pub, priv, err := ed25519.GenerateKey(rand.Reader) + g.Expect(err).ToNot(HaveOccurred()) + pemBlock, err := gossh.MarshalPrivateKey(priv, "test ed25519 key") + g.Expect(err).ToNot(HaveOccurred()) + return pub, pem.EncodeToMemory(pemBlock) + } + tests := []struct { name string setup signerSetup @@ -217,6 +252,12 @@ func TestCommit_WithSigner(t *testing.T) { file: "signed-openpgp", message: "signed by openpgp", }, + { + name: "ssh ed25519", + setup: sshSetup(ed25519Key), + file: "signed-ssh-ed25519", + message: "signed by ssh ed25519", + }, } for _, tt := range tests { From 20f68c9b6a6118e033b634ef0d8f7ab0b16cf94b Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:53:39 +0200 Subject: [PATCH 09/12] git: cover commit with SSH ECDSA P-256 signer e2e Adds an ecdsa-sha2-nistp256 row to TestCommit_WithSigner so the non-ed25519 SSH-signing path is exercised explicitly. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/gogit/client_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index 8575f6b10..6a40aed94 100644 --- a/git/gogit/client_test.go +++ b/git/gogit/client_test.go @@ -20,7 +20,9 @@ import ( "bytes" "context" "crypto" + "crypto/ecdsa" "crypto/ed25519" + "crypto/elliptic" "crypto/rand" "encoding/pem" "io" @@ -240,6 +242,16 @@ func TestCommit_WithSigner(t *testing.T) { return pub, pem.EncodeToMemory(pemBlock) } + ecdsaP256Key := func(t *testing.T) (crypto.PublicKey, []byte) { + t.Helper() + g := NewWithT(t) + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + g.Expect(err).ToNot(HaveOccurred()) + pemBlock, err := gossh.MarshalPrivateKey(priv, "test ecdsa p256 key") + g.Expect(err).ToNot(HaveOccurred()) + return &priv.PublicKey, pem.EncodeToMemory(pemBlock) + } + tests := []struct { name string setup signerSetup @@ -258,6 +270,12 @@ func TestCommit_WithSigner(t *testing.T) { file: "signed-ssh-ed25519", message: "signed by ssh ed25519", }, + { + name: "ssh ecdsa-sha2-nistp256", + setup: sshSetup(ecdsaP256Key), + file: "signed-ssh-ecdsa-p256", + message: "signed by ssh ecdsa p256", + }, } for _, tt := range tests { From 48fa8da7867fff0147700846c499bbb2cd7d5cae Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:54:10 +0200 Subject: [PATCH 10/12] git: assert WithSigner(nil) yields unsigned commit Adds a nil-signer row to TestCommit_WithSigner that pins the nil- interface guard at gogit/client.go: a typed-nil reaching go-git's commit path would panic on signer.Sign. The row also documents the contract that WithSigner(nil) produces an empty gpgsig header. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/gogit/client_test.go | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index 6a40aed94..c166bc3db 100644 --- a/git/gogit/client_test.go +++ b/git/gogit/client_test.go @@ -253,10 +253,11 @@ func TestCommit_WithSigner(t *testing.T) { } tests := []struct { - name string - setup signerSetup - file string - message string + name string + setup signerSetup // nil = pass nil to WithSigner + file string + message string + unsigned bool // true when WithSigner(nil) should yield an empty gpgsig }{ { name: "openpgp", @@ -276,6 +277,12 @@ func TestCommit_WithSigner(t *testing.T) { file: "signed-ssh-ecdsa-p256", message: "signed by ssh ecdsa p256", }, + { + name: "nil signer yields unsigned commit", + file: "unsigned", + message: "unsigned despite WithSigner(nil)", + unsigned: true, + }, } for _, tt := range tests { @@ -298,7 +305,13 @@ func TestCommit_WithSigner(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) ggc.repository = repo - signer, verify := tt.setup(t) + var ( + signer signature.Signer + verify verifyFn + ) + if tt.setup != nil { + signer, verify = tt.setup(t) + } hash, err := ggc.Commit( git.Commit{ @@ -314,6 +327,11 @@ func TestCommit_WithSigner(t *testing.T) { commit, err := repo.CommitObject(plumbing.NewHash(hash)) g.Expect(err).ToNot(HaveOccurred()) + + if tt.unsigned { + g.Expect(commit.PGPSignature).To(BeEmpty()) + return + } g.Expect(commit.PGPSignature).ToNot(BeEmpty()) verify(t, commit) }) From 22b9a5964ca778e537252b3865a12391e34fc75e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 11:54:30 +0200 Subject: [PATCH 11/12] git: document SSH allowlist on NewSSHSigner Adds the supported-algorithms paragraph to the NewSSHSigner godoc so the public surface signals up front which keys it will accept. No behaviour change; validateSSHSigningKey already enforces this. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/signature/ssh_signature.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/git/signature/ssh_signature.go b/git/signature/ssh_signature.go index 328280315..0ad35ce30 100644 --- a/git/signature/ssh_signature.go +++ b/git/signature/ssh_signature.go @@ -166,6 +166,11 @@ func (s *SSHSigner) Sign(r io.Reader) ([]byte, error) { // is consulted only when the private key is encrypted; pass nil for an // unencrypted key. // +// Supported algorithms: ssh-ed25519, ecdsa-sha2-nistp256/384/521, and +// ssh-rsa with key size at least 2048 bits. DSA and undersized RSA keys +// are rejected at construction time because they produce signatures that +// modern OpenSSH refuses to verify. +// // Signatures use namespace [SSHSignatureNamespace] ("git") and SHA-512, // which match Git's defaults for SSH-signed commits. See // https://git-scm.com/docs/gitformat-signature. From 7bce1f280b318b0bd99493c5bb1916a57b581ad8 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 29 May 2026 14:27:59 +0200 Subject: [PATCH 12/12] git: expose ErrSSHPassphraseRequired sentinel NewSSHSigner now returns a typed sentinel error instead of a bare errors.New for the encrypted-key-without-passphrase case, so consumers can branch on it via errors.Is rather than matching the literal error string. Assisted-by: claude-code/claude-opus-4-7 Signed-off-by: Hidde Beydals --- git/signature/errors.go | 5 +++++ git/signature/ssh_signature.go | 2 +- git/signature/ssh_signer_test.go | 18 +++++++++--------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/git/signature/errors.go b/git/signature/errors.go index 1b17760e2..02c4f0a3a 100644 --- a/git/signature/errors.go +++ b/git/signature/errors.go @@ -42,4 +42,9 @@ var ( // at least one key ring or authorized_keys input and none of them could // verify the signature against the payload. ErrNoMatchingKey = errors.New("no matching key") + + // ErrSSHPassphraseRequired is returned by NewSSHSigner when the + // provided private key is encrypted but no passphrase was supplied. + // Callers may branch on it via errors.Is. + ErrSSHPassphraseRequired = errors.New("SSH signing key is encrypted; passphrase required") ) diff --git a/git/signature/ssh_signature.go b/git/signature/ssh_signature.go index 0ad35ce30..f899f510c 100644 --- a/git/signature/ssh_signature.go +++ b/git/signature/ssh_signature.go @@ -182,7 +182,7 @@ func NewSSHSigner(pem, passphrase []byte) (Signer, error) { return nil, fmt.Errorf("could not parse SSH signing key: %w", err) } if len(passphrase) == 0 { - return nil, errors.New("SSH signing key is encrypted; passphrase required") + return nil, ErrSSHPassphraseRequired } inner, err = gossh.ParsePrivateKeyWithPassphrase(pem, passphrase) if err != nil { diff --git a/git/signature/ssh_signer_test.go b/git/signature/ssh_signer_test.go index 58ab62668..96bc1e16c 100644 --- a/git/signature/ssh_signer_test.go +++ b/git/signature/ssh_signer_test.go @@ -28,6 +28,7 @@ import ( "testing" . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" gossh "golang.org/x/crypto/ssh" "github.com/fluxcd/pkg/git/signature" @@ -86,9 +87,9 @@ func TestNewSSHSigner(t *testing.T) { tests := []struct { name string key sshKeyFactory - pemPassphrase []byte // passphrase used to encrypt the PEM; nil = unencrypted - callPassphrase []byte // passphrase passed to NewSSHSigner; nil = none - expectErr string // substring expected in the error; empty = expect success + pemPassphrase []byte // passphrase used to encrypt the PEM; nil = unencrypted + callPassphrase []byte // passphrase passed to NewSSHSigner; nil = none + expectErr types.GomegaMatcher // nil = expect success }{ { name: "ed25519 unencrypted", @@ -104,14 +105,14 @@ func TestNewSSHSigner(t *testing.T) { name: "ed25519 encrypted without passphrase", key: ed25519Key, pemPassphrase: []byte(passphrase), - expectErr: "SSH signing key is encrypted; passphrase required", + expectErr: MatchError(signature.ErrSSHPassphraseRequired), }, { name: "ed25519 encrypted with wrong passphrase", key: ed25519Key, pemPassphrase: []byte("right"), callPassphrase: []byte("wrong"), - expectErr: "could not parse SSH signing key", + expectErr: MatchError(ContainSubstring("could not parse SSH signing key")), }, { name: "ecdsa-sha2-nistp256 accepted", @@ -124,7 +125,7 @@ func TestNewSSHSigner(t *testing.T) { { name: "rsa 1024 rejected", key: rsaKey(1024), - expectErr: "RSA key size 1024 bits is below the minimum supported by NewSSHSigner; must be at least 2048", + expectErr: MatchError(ContainSubstring("RSA key size 1024 bits is below the minimum supported by NewSSHSigner; must be at least 2048")), }, } @@ -135,9 +136,8 @@ func TestNewSSHSigner(t *testing.T) { priv, pemBytes := tt.key(t, tt.pemPassphrase) signer, err := signature.NewSSHSigner(pemBytes, tt.callPassphrase) - if tt.expectErr != "" { - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(tt.expectErr)) + if tt.expectErr != nil { + g.Expect(err).To(tt.expectErr) g.Expect(signer).To(BeNil()) return }