From 14186d6dc513b6fe80f8ca518ec67b6a70be8ac1 Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Mon, 27 Jul 2026 16:04:03 +0200 Subject: [PATCH 1/2] Add Sigstore signer package for skill OCI artifacts Skills pushed to OCI registries need signatures so installs can be verified against the lock file's recorded provenance (RFC THV-0080). This adds pkg/skills/signer: cosign key-pair signing that signs the simple-signing payload (binding the artifact's manifest digest), pushes the classic cosign signature manifest at the sha256-.sig tag for interop with `cosign verify --key`, and returns the serialized Sigstore bundle for durable storage and offline re-verification. The signature is computed once over the payload and reused for both the registry manifest annotation and the returned bundle, so a verifier reconstructing the bundle from the registry checks exactly the signature that was attached. Encrypted keys are decrypted via COSIGN_PASSWORD, matching the cosign CLI. Key paths are canonicalized (absolute, symlinks resolved, regular-file check) before opening. Part of #5899; consumed by the push-signing and install-verification PRs in this stack. Co-Authored-By: Claude Fable 5 --- go.mod | 9 +- go.sum | 12 ++ pkg/skills/signer/cosign_attach.go | 134 ++++++++++++++++ pkg/skills/signer/key.go | 127 +++++++++++++++ pkg/skills/signer/signer.go | 108 +++++++++++++ pkg/skills/signer/signer_test.go | 240 +++++++++++++++++++++++++++++ 6 files changed, 627 insertions(+), 3 deletions(-) create mode 100644 pkg/skills/signer/cosign_attach.go create mode 100644 pkg/skills/signer/key.go create mode 100644 pkg/skills/signer/signer.go create mode 100644 pkg/skills/signer/signer_test.go diff --git a/go.mod b/go.mod index 4346886b7a..0a99729f85 100644 --- a/go.mod +++ b/go.mod @@ -89,10 +89,13 @@ require go.starlark.net v0.0.0-20260630144053-529d8e869a14 require ( github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.32 // indirect github.com/go-openapi/runtime/server-middleware v0.30.0 // indirect + github.com/jedisct1/go-minisign v0.0.0-20230811132847-661be99b8267 // indirect github.com/klauspost/cpuid/v2 v2.3.0 // indirect github.com/modelcontextprotocol/go-sdk v1.7.0-pre.3 // indirect github.com/oklog/ulid/v2 v2.1.1 // indirect + github.com/sassoftware/relic v7.2.1+incompatible // indirect github.com/segmentio/encoding v0.5.4 // indirect + github.com/theupdateframework/go-tuf v0.7.0 // indirect github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 // indirect ) @@ -246,11 +249,11 @@ require ( github.com/sergi/go-diff v1.4.0 // indirect github.com/sethvargo/go-retry v0.3.0 // indirect github.com/shibumi/go-pathspec v1.3.0 // indirect - github.com/sigstore/protobuf-specs v0.5.1 // indirect + github.com/sigstore/protobuf-specs v0.5.1 github.com/sigstore/rekor v1.5.3 // indirect github.com/sigstore/rekor-tiles/v2 v2.3.0 // indirect - github.com/sigstore/sigstore v1.10.8 // indirect - github.com/sigstore/sigstore-go v1.2.2 // indirect + github.com/sigstore/sigstore v1.10.8 + github.com/sigstore/sigstore-go v1.2.2 github.com/sigstore/timestamp-authority/v2 v2.1.2 // indirect github.com/sirupsen/logrus v1.9.4 // indirect github.com/skeema/knownhosts v1.3.1 // indirect diff --git a/go.sum b/go.sum index 8a58454cd7..5541fd97fd 100644 --- a/go.sum +++ b/go.sum @@ -312,6 +312,8 @@ github.com/go-openapi/testify/v2 v2.6.0 h1:5PKH2HE7YJ/LuRPQGvSxBRlFXNQhSetBLlGAg github.com/go-openapi/testify/v2 v2.6.0/go.mod h1:SgsVHtfooshd0tublTtJ50FPKhujf47YRqauXXOUxfw= github.com/go-openapi/validate v0.26.0 h1:dxWzQ3F+vb1SajqUxHjwb5T4mTpSHmdrtv5Bi7+ZNhw= github.com/go-openapi/validate v0.26.0/go.mod h1:b4o00uq7fJeJA+wWhVFCJpKTctzeFwzZImGGmHsl2JA= +github.com/go-rod/rod v0.116.2 h1:A5t2Ky2A+5eD/ZJQr1EfsQSe5rms5Xof/qj296e+ZqA= +github.com/go-rod/rod v0.116.2/go.mod h1:H+CMO9SCNc2TJ2WfrG+pKhITz57uGNYU43qYHh438Mg= github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= github.com/go-sql-driver/mysql v1.7.0/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= @@ -872,6 +874,16 @@ github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zI github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 h1:ilQV1hzziu+LLM3zUTJ0trRztfwgjqKnBWNtSRkbmwM= github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78/go.mod h1:aL8wCCfTfSfmXjznFBSZNN13rSJjlIOI1fUNAtF7rmI= +github.com/ysmood/fetchup v0.2.3 h1:ulX+SonA0Vma5zUFXtv52Kzip/xe7aj4vqT5AJwQ+ZQ= +github.com/ysmood/fetchup v0.2.3/go.mod h1:xhibcRKziSvol0H1/pj33dnKrYyI2ebIvz5cOOkYGns= +github.com/ysmood/goob v0.4.0 h1:HsxXhyLBeGzWXnqVKtmT9qM7EuVs/XOgkX7T6r1o1AQ= +github.com/ysmood/goob v0.4.0/go.mod h1:u6yx7ZhS4Exf2MwciFr6nIM8knHQIE22lFpWHnfql18= +github.com/ysmood/got v0.40.0 h1:ZQk1B55zIvS7zflRrkGfPDrPG3d7+JOza1ZkNxcc74Q= +github.com/ysmood/got v0.40.0/go.mod h1:W7DdpuX6skL3NszLmAsC5hT7JAhuLZhByVzHTq874Qg= +github.com/ysmood/gson v0.7.3 h1:QFkWbTH8MxyUTKPkVWAENJhxqdBa4lYTQWqZCiLG6kE= +github.com/ysmood/gson v0.7.3/go.mod h1:3Kzs5zDl21g5F/BlLTNcuAGAYLKt2lV5G8D1zF3RNmg= +github.com/ysmood/leakless v0.9.0 h1:qxCG5VirSBvmi3uynXFkcnLMzkphdh3xx5FtrORwDCU= +github.com/ysmood/leakless v0.9.0/go.mod h1:R8iAXPRaG97QJwqxs74RdwzcRHT1SWCGTNqY8q0JvMQ= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= diff --git a/pkg/skills/signer/cosign_attach.go b/pkg/skills/signer/cosign_attach.go new file mode 100644 index 0000000000..fa1d3e97a4 --- /dev/null +++ b/pkg/skills/signer/cosign_attach.go @@ -0,0 +1,134 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package signer + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "strings" + + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/google/go-containerregistry/pkg/v1/static" + "github.com/google/go-containerregistry/pkg/v1/types" + "github.com/opencontainers/go-digest" +) + +const ( + mediaTypeCosignSimpleSigningV1JSON = "application/vnd.dev.cosign.simplesigning.v1+json" + annotationCosignSignature = "dev.cosignproject.cosign/signature" +) + +// cosignSimpleSigning is the payload cosign signs: it embeds the artifact's +// manifest digest, binding the signature to the exact artifact content. +type cosignSimpleSigning struct { + Critical cosignCritical `json:"critical"` +} + +type cosignCritical struct { + Identity cosignIdentity `json:"identity"` + Image cosignImage `json:"image"` + Type string `json:"type"` +} + +type cosignIdentity struct { + DockerReference string `json:"docker-reference"` +} + +type cosignImage struct { + DockerManifestDigest string `json:"docker-manifest-digest"` +} + +// simpleSigningPayload builds the canonical simple-signing payload for the +// artifact at ref pinned to digestStr. This payload — not the manifest +// digest — is what gets signed, per the cosign convention: a verifier +// recovers the payload from the signature manifest's layer, checks the +// signature over it, and reads the bound manifest digest out of it. +func simpleSigningPayload(imageRef, digestStr string) ([]byte, error) { + ref, err := name.ParseReference(imageRef) + if err != nil { + return nil, fmt.Errorf("parsing image reference: %w", err) + } + d, err := parseManifestDigest(digestStr) + if err != nil { + return nil, err + } + payload := cosignSimpleSigning{ + Critical: cosignCritical{ + Identity: cosignIdentity{DockerReference: ref.Context().Name()}, + Image: cosignImage{DockerManifestDigest: d.String()}, + Type: "cosign container image signature", + }, + } + return json.Marshal(payload) +} + +// parseManifestDigest validates and normalizes an artifact manifest digest +// string, defaulting a bare hex value to sha256. +func parseManifestDigest(digestStr string) (digest.Digest, error) { + digestStr = strings.TrimSpace(digestStr) + if digestStr == "" { + return "", fmt.Errorf("digest is required for signing") + } + if !strings.Contains(digestStr, ":") { + digestStr = "sha256:" + digestStr + } + d, err := digest.Parse(digestStr) + if err != nil { + return "", fmt.Errorf("parsing digest: %w", err) + } + return d, nil +} + +// attachCosignSignature writes the cosign signature manifest for the +// artifact: an OCI image at the "sha256-.sig" tag whose single layer is +// the simple-signing payload, carrying the signature in the layer's +// annotations. This is the classic cosign layout, chosen deliberately for +// interop — "cosign verify --key" and any Sigstore-aware registry tooling +// can discover and verify it. +func attachCosignSignature( + ctx context.Context, + keychain authn.Keychain, + imageRef, digestStr string, + payload, signatureBytes []byte, +) error { + ref, err := name.ParseReference(imageRef) + if err != nil { + return fmt.Errorf("parsing image reference: %w", err) + } + d, err := parseManifestDigest(digestStr) + if err != nil { + return err + } + + layer := static.NewLayer(payload, mediaTypeCosignSimpleSigningV1JSON) + img, err := mutate.Append(empty.Image, mutate.Addendum{ + Layer: layer, + Annotations: map[string]string{ + annotationCosignSignature: base64.StdEncoding.EncodeToString(signatureBytes), + }, + MediaType: mediaTypeCosignSimpleSigningV1JSON, + }) + if err != nil { + return fmt.Errorf("building signature manifest: %w", err) + } + img = mutate.MediaType(img, types.OCIManifestSchema1) + + h, err := v1.NewHash(d.String()) + if err != nil { + return fmt.Errorf("parsing digest hash: %w", err) + } + sigTag := ref.Context().Tag(fmt.Sprint(h.Algorithm, "-", h.Hex, ".sig")) + remoteOpts := []remote.Option{remote.WithAuthFromKeychain(keychain), remote.WithContext(ctx)} + if err := remote.Write(sigTag, img, remoteOpts...); err != nil { + return fmt.Errorf("pushing signature manifest: %w", err) + } + return nil +} diff --git a/pkg/skills/signer/key.go b/pkg/skills/signer/key.go new file mode 100644 index 0000000000..1475dd12aa --- /dev/null +++ b/pkg/skills/signer/key.go @@ -0,0 +1,127 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package signer + +import ( + "context" + "crypto" + "crypto/rand" + "fmt" + "os" + "path/filepath" + + protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + "github.com/sigstore/sigstore-go/pkg/sign" + "github.com/sigstore/sigstore/pkg/cryptoutils" +) + +// resolveKeyPath canonicalizes and vets a user-supplied signing key path: +// absolute, cleaned, symlinks resolved, and a regular file. The value comes +// straight from a --key flag, so this is the recognized sanitization +// barrier before the path is opened — no string-inspection shortcuts. +func resolveKeyPath(path string) (string, error) { + if path == "" { + return "", ErrKeyRequired + } + abs, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("resolving signing key path: %w", err) + } + resolved, err := filepath.EvalSymlinks(filepath.Clean(abs)) + if err != nil { + return "", fmt.Errorf("resolving signing key path: %w", err) + } + info, err := os.Stat(resolved) + if err != nil { + return "", fmt.Errorf("reading signing key: %w", err) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("signing key %q is not a regular file", resolved) + } + return resolved, nil +} + +// loadKeypair reads a cosign PEM private key from path. Encrypted keys are +// decrypted with the COSIGN_PASSWORD environment variable, matching the +// cosign CLI's behavior. +func loadKeypair(path string) (sign.Keypair, error) { + resolved, err := resolveKeyPath(path) + if err != nil { + return nil, err + } + pemBytes, err := os.ReadFile(resolved) //nolint:gosec // resolved by resolveKeyPath from an explicit --key flag + if err != nil { + return nil, fmt.Errorf("reading signing key: %w", err) + } + priv, err := cryptoutils.UnmarshalPEMToPrivateKey(pemBytes, cosignPassFunc()) + if err != nil { + return nil, fmt.Errorf("decoding signing key: %w", err) + } + signerKey, ok := priv.(crypto.Signer) + if !ok { + return nil, fmt.Errorf("signing key type %T cannot sign", priv) + } + return &fileKeypair{priv: signerKey}, nil +} + +func cosignPassFunc() cryptoutils.PassFunc { + if pw := os.Getenv("COSIGN_PASSWORD"); pw != "" { + return cryptoutils.StaticPasswordFunc([]byte(pw)) + } + return func(_ bool) ([]byte, error) { return nil, nil } +} + +// fileKeypair adapts a file-loaded private key to sigstore-go's signing +// Keypair interface (ECDSA P-256 / SHA-256, the cosign default). +type fileKeypair struct { + priv crypto.Signer +} + +func (*fileKeypair) GetHashAlgorithm() protocommon.HashAlgorithm { + return protocommon.HashAlgorithm_SHA2_256 +} + +func (*fileKeypair) GetSigningAlgorithm() protocommon.PublicKeyDetails { + return protocommon.PublicKeyDetails_PKIX_ECDSA_P256_SHA_256 +} + +func (k *fileKeypair) GetHint() []byte { + pubKeyBytes, err := cryptoutils.MarshalPublicKeyToPEM(k.priv.Public()) + if err != nil { + return nil + } + return pubKeyBytes +} + +func (*fileKeypair) GetKeyAlgorithm() string { + return "ecdsa" +} + +func (k *fileKeypair) GetPublicKey() crypto.PublicKey { + return k.priv.Public() +} + +func (k *fileKeypair) GetPublicKeyPem() (string, error) { + pemBytes, err := cryptoutils.MarshalPublicKeyToPEM(k.priv.Public()) + if err != nil { + return "", err + } + return string(pemBytes), nil +} + +// SignData hashes data with SHA-256 and signs the digest, returning +// (signature, digest) — the order sigstore-go's Keypair contract requires +// (see sign.EphemeralKeypair for the reference implementation). +func (k *fileKeypair) SignData(_ context.Context, data []byte) ([]byte, []byte, error) { + hash := crypto.SHA256.New() + if _, err := hash.Write(data); err != nil { + return nil, nil, err + } + digest := hash.Sum(nil) + sig, err := k.priv.Sign(rand.Reader, digest, crypto.SHA256) + if err != nil { + return nil, nil, err + } + return sig, digest, nil +} diff --git a/pkg/skills/signer/signer.go b/pkg/skills/signer/signer.go new file mode 100644 index 0000000000..59d13efd22 --- /dev/null +++ b/pkg/skills/signer/signer.go @@ -0,0 +1,108 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package signer signs skill OCI artifacts with Sigstore, following the +// cosign key-pair convention: a "simple signing" payload binding the +// artifact's manifest digest is signed with the user's key, attached to the +// registry as a cosign signature manifest (the "sha256-.sig" tag), and +// returned as a serialized Sigstore bundle for durable storage and offline +// re-verification. +package signer + +import ( + "context" + "errors" + "fmt" + + "github.com/google/go-containerregistry/pkg/authn" + verifybundle "github.com/sigstore/sigstore-go/pkg/bundle" + "github.com/sigstore/sigstore-go/pkg/sign" +) + +// ErrKeyRequired indicates no signing key was provided. Keyless (OIDC) +// signing is not implemented yet, so a cosign private key is the only +// supported signing method. +var ErrKeyRequired = errors.New("signing key required: pass --key with a cosign private key") + +// Options configures OCI signing. +type Options struct { + // Key is the path to a cosign PEM-encoded private key file. An + // encrypted key is decrypted with the COSIGN_PASSWORD environment + // variable, matching the cosign CLI. + Key string +} + +// Signer signs skill OCI artifacts and attaches the signature to the +// registry. +type Signer interface { + // SignOCI signs the artifact at ref pinned to the given manifest digest + // ("sha256:..."), attaches the signature as a cosign signature manifest + // next to the artifact, and returns the serialized Sigstore bundle for + // storage. + SignOCI(ctx context.Context, ref, digest string, opts Options) ([]byte, error) +} + +// Default implements Signer with file-based cosign keys. +type Default struct { + keychain authn.Keychain +} + +var _ Signer = (*Default)(nil) + +// NewDefault creates a signer using the given registry auth keychain for +// pushing the signature manifest. A nil keychain falls back to the default +// keychain. +func NewDefault(keychain authn.Keychain) *Default { + if keychain == nil { + keychain = authn.DefaultKeychain + } + return &Default{keychain: keychain} +} + +// SignOCI signs the artifact following the cosign convention: the signature +// is computed over the simple-signing payload (which embeds the manifest +// digest, binding the signature to the artifact), the SAME signature is +// attached to the registry as a cosign signature manifest, and the returned +// bundle carries it with the payload's digest as the signed message. A +// verifier reconstructing the bundle from the registry manifest (or +// re-verifying the stored bundle offline) therefore checks exactly the +// signature that was attached — one signature, two representations. +func (d *Default) SignOCI(ctx context.Context, ref, digestStr string, opts Options) ([]byte, error) { + if opts.Key == "" { + return nil, ErrKeyRequired + } + keypair, err := loadKeypair(opts.Key) + if err != nil { + return nil, err + } + + payload, err := simpleSigningPayload(ref, digestStr) + if err != nil { + return nil, err + } + + // sign.Bundle invokes the keypair's SignData over the payload and + // records both the signature and the payload digest in the bundle. + pb, err := sign.Bundle(&sign.PlainData{Data: payload}, keypair, sign.BundleOptions{Context: ctx}) + if err != nil { + return nil, fmt.Errorf("building sigstore bundle: %w", err) + } + msgSig := pb.GetMessageSignature() + if msgSig == nil || len(msgSig.GetSignature()) == 0 { + return nil, errors.New("signing produced no message signature") + } + + if err := attachCosignSignature(ctx, d.keychain, ref, digestStr, payload, msgSig.GetSignature()); err != nil { + return nil, fmt.Errorf("attaching signature manifest: %w", err) + } + + bun, err := verifybundle.NewBundle(pb) + if err != nil { + return nil, fmt.Errorf("finalizing sigstore bundle: %w", err) + } + raw, err := bun.MarshalJSON() + if err != nil { + return nil, fmt.Errorf("serializing sigstore bundle: %w", err) + } + return raw, nil +} diff --git a/pkg/skills/signer/signer_test.go b/pkg/skills/signer/signer_test.go new file mode 100644 index 0000000000..83cf9319c9 --- /dev/null +++ b/pkg/skills/signer/signer_test.go @@ -0,0 +1,240 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package signer + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "encoding/json" + "encoding/pem" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/google/go-containerregistry/pkg/crane" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/random" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/sigstore/sigstore-go/pkg/bundle" + "github.com/sigstore/sigstore-go/pkg/root" + "github.com/sigstore/sigstore-go/pkg/verify" + "github.com/sigstore/sigstore/pkg/cryptoutils" + "github.com/sigstore/sigstore/pkg/signature" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// writeTestKey generates an ECDSA P-256 key pair, writes the private key as +// a cosign-style PEM file, and returns its path plus the public key PEM. +func writeTestKey(t *testing.T) (keyPath string, pubPEM []byte) { + t.Helper() + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + privPEM, err := cryptoutils.MarshalPrivateKeyToPEM(priv) + require.NoError(t, err) + keyPath = filepath.Join(t.TempDir(), "cosign.key") + require.NoError(t, os.WriteFile(keyPath, privPEM, 0o600)) + + pubPEM, err = cryptoutils.MarshalPublicKeyToPEM(priv.Public()) + require.NoError(t, err) + return keyPath, pubPEM +} + +// pushTestArtifact pushes a random OCI image to the in-process registry and +// returns its reference and manifest digest. +func pushTestArtifact(t *testing.T, registryHost string) (ref string, digestStr string) { + t.Helper() + img, err := random.Image(256, 1) + require.NoError(t, err) + ref = registryHost + "/test/skill:v1" + parsed, err := name.ParseReference(ref) + require.NoError(t, err) + require.NoError(t, remote.Write(parsed, img)) + d, err := img.Digest() + require.NoError(t, err) + return ref, d.String() +} + +// verifyKeyBundle verifies a serialized bundle against the public key and +// the given payload digest — the same semantics toolhive-core's +// VerifyBundleWithKey exposes (used directly here until the core release +// this stack's verifier PR consumes). +func verifyKeyBundle(t *testing.T, raw, pubPEM []byte, payloadDigest []byte) error { + t.Helper() + pub, err := cryptoutils.UnmarshalPEMToPublicKey(pubPEM) + require.NoError(t, err) + sigVerifier, err := signature.LoadVerifier(pub, crypto.SHA256) + require.NoError(t, err) + tm := root.NewTrustedPublicKeyMaterial(func(string) (root.TimeConstrainedVerifier, error) { + return root.NewExpiringKey(sigVerifier, time.Time{}, time.Time{}), nil + }) + sev, err := verify.NewVerifier(tm, verify.WithNoObserverTimestamps()) + require.NoError(t, err) + + parsed := &bundle.Bundle{} + require.NoError(t, parsed.UnmarshalJSON(raw)) + _, err = sev.Verify(parsed, verify.NewPolicy( + verify.WithArtifactDigest("sha256", payloadDigest), + verify.WithKey(), + )) + return err +} + +//nolint:paralleltest // uses t.Setenv indirectly via subtests sharing a registry +func TestSignOCIRoundTrip(t *testing.T) { + reg := httptest.NewServer(registry.New()) + t.Cleanup(reg.Close) + host := strings.TrimPrefix(reg.URL, "http://") + + keyPath, pubPEM := writeTestKey(t) + ref, digestStr := pushTestArtifact(t, host) + + raw, err := NewDefault(nil).SignOCI(t.Context(), ref, digestStr, Options{Key: keyPath}) + require.NoError(t, err) + require.NotEmpty(t, raw) + + // The returned bundle verifies against the signing key over the + // simple-signing payload digest. + payload, err := simpleSigningPayload(ref, digestStr) + require.NoError(t, err) + payloadDigest := sha256.Sum256(payload) + require.NoError(t, verifyKeyBundle(t, raw, pubPEM, payloadDigest[:]), + "the returned bundle must verify against the signing key") + + // The attached signature manifest reconstructs to the SAME signature: + // this is the layout toolhive-core's bundle retrieval (and cosign) + // discover — one signature, two representations. + h, err := v1.NewHash(digestStr) + require.NoError(t, err) + sigRef := host + "/test/skill:" + h.Algorithm + "-" + h.Hex + ".sig" + manifestBytes, err := crane.Manifest(sigRef) + require.NoError(t, err, "the signature manifest must exist at the cosign .sig tag") + + var manifest struct { + Layers []struct { + MediaType string `json:"mediaType"` + Digest string `json:"digest"` + Annotations map[string]string `json:"annotations"` + } `json:"layers"` + } + require.NoError(t, json.Unmarshal(manifestBytes, &manifest)) + require.Len(t, manifest.Layers, 1) + layer := manifest.Layers[0] + assert.Equal(t, mediaTypeCosignSimpleSigningV1JSON, layer.MediaType) + + // The layer content digest is the payload digest the bundle signed. + wantLayerDigest := "sha256:" + hex.EncodeToString(payloadDigest[:]) + assert.Equal(t, wantLayerDigest, layer.Digest, + "the signature manifest layer must be the exact signed payload") + + // The annotation signature matches the bundle's message signature. + parsed := &bundle.Bundle{} + require.NoError(t, parsed.UnmarshalJSON(raw)) + bundleSig := parsed.Bundle.GetMessageSignature().GetSignature() + annotationSig, err := base64.StdEncoding.DecodeString(layer.Annotations[annotationCosignSignature]) + require.NoError(t, err) + assert.Equal(t, bundleSig, annotationSig, + "the attached annotation and the returned bundle must carry the same signature") +} + +//nolint:paralleltest // shares process env semantics with the round-trip test +func TestSignOCIRejectsWrongKeyVerification(t *testing.T) { + reg := httptest.NewServer(registry.New()) + t.Cleanup(reg.Close) + host := strings.TrimPrefix(reg.URL, "http://") + + keyPath, _ := writeTestKey(t) + _, otherPub := writeTestKey(t) + ref, digestStr := pushTestArtifact(t, host) + + raw, err := NewDefault(nil).SignOCI(t.Context(), ref, digestStr, Options{Key: keyPath}) + require.NoError(t, err) + + payload, err := simpleSigningPayload(ref, digestStr) + require.NoError(t, err) + payloadDigest := sha256.Sum256(payload) + require.Error(t, verifyKeyBundle(t, raw, otherPub, payloadDigest[:]), + "a different key must not verify the bundle") +} + +func TestSignOCIRequiresKey(t *testing.T) { + t.Parallel() + _, err := NewDefault(nil).SignOCI(t.Context(), "ghcr.io/org/skill:v1", "sha256:abc", Options{}) + require.ErrorIs(t, err, ErrKeyRequired) +} + +func TestSignOCIEncryptedKey(t *testing.T) { + reg := httptest.NewServer(registry.New()) + t.Cleanup(reg.Close) + host := strings.TrimPrefix(reg.URL, "http://") + + // Generate an encrypted cosign-style key: the "ENCRYPTED SIGSTORE + // PRIVATE KEY" PEM type wrapping password-encrypted DER, which is what + // `cosign generate-key-pair` produces. + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + encDER, err := cryptoutils.MarshalPrivateKeyToEncryptedDER( + priv, cryptoutils.StaticPasswordFunc([]byte("test-password"))) + require.NoError(t, err) + encPEM := pem.EncodeToMemory(&pem.Block{ + Type: string(cryptoutils.EncryptedSigstorePrivateKeyPEMType), + Bytes: encDER, + }) + keyPath := filepath.Join(t.TempDir(), "cosign-enc.key") + require.NoError(t, os.WriteFile(keyPath, encPEM, 0o600)) + + ref, digestStr := pushTestArtifact(t, host) + + t.Setenv("COSIGN_PASSWORD", "test-password") + raw, err := NewDefault(nil).SignOCI(t.Context(), ref, digestStr, Options{Key: keyPath}) + require.NoError(t, err, "an encrypted key must decrypt via COSIGN_PASSWORD") + require.NotEmpty(t, raw) + + t.Setenv("COSIGN_PASSWORD", "wrong-password") + _, err = NewDefault(nil).SignOCI(t.Context(), ref, digestStr, Options{Key: keyPath}) + require.Error(t, err, "a wrong password must fail key decryption") +} + +func TestResolveKeyPath(t *testing.T) { + t.Parallel() + + t.Run("regular file resolves", func(t *testing.T) { + t.Parallel() + p := filepath.Join(t.TempDir(), "key.pem") + require.NoError(t, os.WriteFile(p, []byte("x"), 0o600)) + resolved, err := resolveKeyPath(p) + require.NoError(t, err) + assert.True(t, filepath.IsAbs(resolved)) + }) + + t.Run("directory rejected", func(t *testing.T) { + t.Parallel() + _, err := resolveKeyPath(t.TempDir()) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a regular file") + }) + + t.Run("missing file rejected", func(t *testing.T) { + t.Parallel() + _, err := resolveKeyPath(filepath.Join(t.TempDir(), "nope.pem")) + require.Error(t, err) + }) + + t.Run("empty path rejected", func(t *testing.T) { + t.Parallel() + _, err := resolveKeyPath("") + require.ErrorIs(t, err, ErrKeyRequired) + }) +} From 6c104c87e06eccf5773e03899c1cdd756427fc1e Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Mon, 27 Jul 2026 17:33:33 +0200 Subject: [PATCH 2/2] Extend signer tests to error paths and keypair metadata Cover input-validation failures (garbage key, bad reference, malformed digest), bare-hex digest normalization, the payload's digest/repository binding, and the sigstore Keypair metadata accessors, raising patch coverage flagged by codecov. Co-Authored-By: Claude Fable 5 --- pkg/skills/signer/signer_test.go | 96 ++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/pkg/skills/signer/signer_test.go b/pkg/skills/signer/signer_test.go index 83cf9319c9..91e2b68b19 100644 --- a/pkg/skills/signer/signer_test.go +++ b/pkg/skills/signer/signer_test.go @@ -26,6 +26,7 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/random" "github.com/google/go-containerregistry/pkg/v1/remote" + protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" "github.com/sigstore/sigstore-go/pkg/bundle" "github.com/sigstore/sigstore-go/pkg/root" "github.com/sigstore/sigstore-go/pkg/verify" @@ -207,6 +208,101 @@ func TestSignOCIEncryptedKey(t *testing.T) { require.Error(t, err, "a wrong password must fail key decryption") } +func TestSignOCIInputErrors(t *testing.T) { + t.Parallel() + keyPath, _ := writeTestKey(t) + + tests := []struct { + name string + key string + ref string + digest string + wantErr string + }{ + { + name: "garbage key file", + key: writeGarbageKey(t), + ref: "example.com/org/skill:v1", + digest: "sha256:" + strings.Repeat("a", 64), + wantErr: "decoding signing key", + }, + { + name: "invalid image reference", + key: keyPath, + ref: "NOT a valid ref!!", + digest: "sha256:" + strings.Repeat("a", 64), + wantErr: "parsing image reference", + }, + { + name: "empty digest", + key: keyPath, + ref: "example.com/org/skill:v1", + digest: "", + wantErr: "digest is required", + }, + { + name: "malformed digest", + key: keyPath, + ref: "example.com/org/skill:v1", + digest: "sha256:tooshort", + wantErr: "parsing digest", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := NewDefault(nil).SignOCI(t.Context(), tc.ref, tc.digest, Options{Key: tc.key}) + require.ErrorContains(t, err, tc.wantErr) + }) + } +} + +func writeGarbageKey(t *testing.T) string { + t.Helper() + p := filepath.Join(t.TempDir(), "garbage.key") + require.NoError(t, os.WriteFile(p, []byte("not a pem key"), 0o600)) + return p +} + +func TestParseManifestDigestNormalizesBareHex(t *testing.T) { + t.Parallel() + hexDigest := strings.Repeat("a", 64) + d, err := parseManifestDigest(hexDigest) + require.NoError(t, err) + assert.Equal(t, "sha256:"+hexDigest, d.String()) +} + +func TestSimpleSigningPayloadBindsDigestAndRepo(t *testing.T) { + t.Parallel() + digestStr := "sha256:" + strings.Repeat("b", 64) + payload, err := simpleSigningPayload("example.com/org/skill:v1", digestStr) + require.NoError(t, err) + + var got cosignSimpleSigning + require.NoError(t, json.Unmarshal(payload, &got)) + assert.Equal(t, digestStr, got.Critical.Image.DockerManifestDigest) + // The identity is the repository, not the tag — retagging must not + // invalidate the signature's digest binding. + assert.Equal(t, "example.com/org/skill", got.Critical.Identity.DockerReference) + assert.Equal(t, "cosign container image signature", got.Critical.Type) +} + +func TestFileKeypairMetadata(t *testing.T) { + t.Parallel() + keyPath, pubPEM := writeTestKey(t) + kp, err := loadKeypair(keyPath) + require.NoError(t, err) + + assert.Equal(t, "ecdsa", kp.GetKeyAlgorithm()) + assert.Equal(t, protocommon.HashAlgorithm_SHA2_256, kp.GetHashAlgorithm()) + assert.Equal(t, protocommon.PublicKeyDetails_PKIX_ECDSA_P256_SHA_256, kp.GetSigningAlgorithm()) + gotPEM, err := kp.GetPublicKeyPem() + require.NoError(t, err) + assert.Equal(t, string(pubPEM), gotPEM) + assert.Equal(t, pubPEM, kp.GetHint()) + assert.NotNil(t, kp.GetPublicKey()) +} + func TestResolveKeyPath(t *testing.T) { t.Parallel()