Verify key-signed skill installs against a public key

cmd/thv/app/skill_install.go

@@ -5,11 +5,14 @@ package app
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/spf13/cobra"
 
 	"github.com/stacklok/toolhive/pkg/skills"
+	"github.com/stacklok/toolhive/pkg/skills/verifier"
 )
 
 var (
@@ -19,6 +22,7 @@ var (
 	skillInstallProjectRoot   string
 	skillInstallGroup         string
 	skillInstallAllowUnsigned bool
+	skillInstallPublicKey     string
 )
 
 var skillInstallCmd = &cobra.Command{
@@ -46,6 +50,10 @@ func init() {
 	skillInstallCmd.Flags().StringVar(&skillInstallGroup, "group", "", "Group to add the skill to after installation")
 	skillInstallCmd.Flags().BoolVar(&skillInstallAllowUnsigned, "allow-unsigned", false,
 		"Allow installing a project-scoped skill without a verified signature (recorded in the lock file)")
+	skillInstallCmd.Flags().StringVar(&skillInstallPublicKey, "public-key", "",
+		"Path to the cosign public key (cosign.pub) a key-pair-signed skill must verify against."+
+			" Required the first time such a skill is installed project-scoped; the key is then pinned"+
+			" in the lock file and reused automatically")
 }
 
 func skillInstallCmdFunc(cmd *cobra.Command, args []string) error {
@@ -56,6 +64,11 @@ func skillInstallCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
+	publicKey, err := readInstallPublicKey(skillInstallPublicKey)
+	if err != nil {
+		return err
+	}
+
 	result, err := c.Install(cmd.Context(), skills.InstallOptions{
 		Name:          args[0],
 		Scope:         skills.Scope(skillInstallScope),
@@ -64,6 +77,7 @@ func skillInstallCmdFunc(cmd *cobra.Command, args []string) error {
 		ProjectRoot:   projectRoot,
 		Group:         skillInstallGroup,
 		AllowUnsigned: skillInstallAllowUnsigned,
+		PublicKey:     publicKey,
 	})
 	if err != nil {
 		return formatSkillError("install skill", err)
@@ -73,6 +87,28 @@ func skillInstallCmdFunc(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+// readInstallPublicKey turns a --public-key file path into the encoded key
+// material the API carries. The CLI reads the file rather than forwarding its
+// path because the server is a separate process, possibly on another host,
+// where that path names nothing — or something else.
+func readInstallPublicKey(path string) (string, error) {
+	if path == "" {
+		return "", nil
+	}
+	cleanPath := filepath.Clean(path)
+	// #nosec G304 - the path is a CLI flag the user chose; reading the file
+	// they named is the operation.
+	pemBytes, err := os.ReadFile(cleanPath)
+	if err != nil {
+		return "", fmt.Errorf("read public key: %w", err)
+	}
+	encoded, err := verifier.EncodePublicKey(pemBytes)
+	if err != nil {
+		return "", fmt.Errorf("read public key %s: %w", cleanPath, err)
+	}
+	return encoded, nil
+}
+
 // printInstallTrust shows the trust state the install recorded — RFC
 // THV-0080 wants the pinned identity displayed prominently, not discovered
 // weeks later inside a signer-mismatch error.

cmd/thv/app/skill_push.go

@@ -35,10 +35,10 @@ func init() {
 	skillPushCmd.Flags().StringVar(&skillPushKey, "key", "",
 		"Path to a cosign private key to sign the pushed artifact. "+
 			"Encrypted keys are decrypted with COSIGN_PASSWORD read from the 'thv serve' process, "+
-			"which performs the signing. NOTE: ToolHive cannot verify key-pair signatures at "+
-			"install time, so a project-scoped install of the result is refused and "+
-			"--allow-unsigned does not override it — use keyless signing for artifacts that "+
-			"need to be installable")
+			"which performs the signing. Consumers installing the result project-scoped must pass "+
+			"--public-key with the matching cosign public key the first time; distribute it "+
+			"alongside the artifact. Keyless signing needs no such out-of-band step, since the "+
+			"signer identity is verifiable from the artifact itself")
 	skillPushCmd.Flags().StringVar(&skillPushIdentityToken, "identity-token", "",
 		"OIDC identity token (or a path to a file containing one) for keyless signing. "+
 			"Mutually exclusive with --key. If omitted, one is acquired automatically: from the "+

docs/arch/12-skills-system.md

@@ -430,7 +430,15 @@ What is still trusted on faith, deliberately and visibly:
 
 Publishing is signed by default: `thv skill push` requires `--key` (a cosign private key), an OIDC identity token for keyless signing (supplied with `--identity-token` or acquired automatically), or an explicit `--no-sign`. Either signing path attaches the signature manifest next to the artifact, and the bundle is retrievable at install. See [Publishing](#3-publishing) for the full ladder.
 
-Only the **keyless** path produces an installable artifact. Install-time verification checks the keyless (Fulcio) trust root, and a cosign key pair carries no certificate to chain to it — nor is the signing public key recoverable from the artifact, since the cosign manifest defines no annotation for it. A project-scoped install of a `--key`-signed artifact is therefore refused, and `--allow-unsigned` does **not** override the refusal: the artifact *is* signed, so it never produces the unsigned verdict that exception applies to. Tracked as [#6442](https://github.com/stacklok/toolhive/issues/6442).
+Both paths produce an installable artifact, but they differ in what the consumer must supply. A cosign key pair carries no certificate to chain to the keyless (Fulcio) trust root, and the signing public key is recoverable neither from the artifact nor from the attached bundle — the cosign manifest defines no annotation for it. So the key has to arrive from outside the artifact: a project-scoped install of a `--key`-signed skill requires `--public-key` on first use, which verifies the signature and pins that key in the lock entry as `publicKey:`. Later installs read it back from the lock and need no flag.
+
+`--allow-unsigned` does **not** substitute for the key, in either direction: the artifact *is* signed, so it never produces the unsigned verdict that exception applies to. Nor does `--allow-signer-change` re-anchor an entry to a new key — a key-pair bundle carries no identity to observe, so honoring one would re-anchor on the strength of the caller having named it. Re-anchoring means removing the lock entry and reinstalling.
+
+Dispatch between the two paths is decided by the **lock entry, never the artifact**: a `publicKey:` pin selects the key path, a certificate pin the keyless one. Letting the artifact choose would let a republished key-signed artifact walk out of the identity its entry is pinned to. A supplied key that disagrees with what the entry pins is refused up front rather than silently ignored.
+
+Verifying a key-pair signature binds it to the artifact explicitly. The signature covers cosign's simple-signing payload, and signature manifests are discovered by a tag derived from the digest being verified — so attachment proves nothing about which artifact a signature describes. The payload digest is reconstructed from the requested reference and each candidate must sign exactly those bytes, which is what stops one artifact's signature from being replayed onto another by copying its signature layer into that artifact's `.sig` manifest (`bundleSignsPayload`).
+
+Scope for v1 (issue [#6442](https://github.com/stacklok/toolhive/issues/6442)): install only. `--public-key` is not yet accepted on `upgrade` or `sync --adopt`, and the plugins surface does not accept it at all yet.
 
 ### Schema
 

go.mod

@@ -258,7 +258,7 @@ 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.2 // indirect
+	github.com/sigstore/protobuf-specs v0.5.2
 	github.com/sigstore/rekor v1.5.4 // indirect
 	github.com/sigstore/rekor-tiles/v2 v2.3.0 // indirect
 	github.com/sigstore/sigstore v1.10.9

pkg/api/v1/skills.go

@@ -129,6 +129,7 @@ func (s *SkillsRoutes) installSkill(w http.ResponseWriter, r *http.Request) erro
 		Force:         req.Force,
 		Group:         req.Group,
 		AllowUnsigned: req.AllowUnsigned,
+		PublicKey:     req.PublicKey,
 	})
 	if err != nil {
 		return err

pkg/api/v1/skills_types.go

@@ -35,6 +35,11 @@ type installSkillRequest struct {
 	// verified signature; the exception is recorded in the project's lock
 	// file.
 	AllowUnsigned bool `json:"allow_unsigned,omitempty"`
+	// PublicKey is the base64-encoded DER SPKI cosign public key the artifact
+	// must verify against, for artifacts signed with a cosign key pair rather
+	// than keylessly. Required the first time such an artifact is installed
+	// project-scoped, and pinned in the lock file from then on.
+	PublicKey string `json:"public_key,omitempty"`
 	// Group is the group name to add the skill to after installation
 	Group string `json:"group,omitempty"`
 }

pkg/skills/client/client.go

@@ -212,6 +212,7 @@ func (c *Client) Install(ctx context.Context, opts skills.InstallOptions) (*skil
 		Force:         opts.Force,
 		Group:         opts.Group,
 		AllowUnsigned: opts.AllowUnsigned,
+		PublicKey:     opts.PublicKey,
 	}
 
 	var resp installResponse

pkg/skills/client/dto.go

@@ -18,6 +18,10 @@ type installRequest struct {
 	// AllowUnsigned mirrors skills.InstallOptions.AllowUnsigned; without it
 	// here the CLI flag would silently never reach the server.
 	AllowUnsigned bool `json:"allow_unsigned,omitempty"`
+	// PublicKey mirrors skills.InstallOptions.PublicKey: the base64 DER SPKI
+	// the CLI encoded from the --public-key file, since a path would not
+	// resolve on a server in another process or on another host.
+	PublicKey string `json:"public_key,omitempty"`
 }
 
 type validateRequest struct {

pkg/skills/lockfile/validation.go

@@ -134,6 +134,14 @@ func findRequiredByCycle(entries []Entry) []string {
 // from a corrupted or hostile lock file reaching the fetch path.
 const maxReferenceLength = 512
 
+// MaxEncodedPublicKeyLength bounds the base64 DER SPKI in a provenance block.
+// Key material needs more room than an identifier: the largest key cosign can
+// import, RSA-4096, encodes to 736 characters, so the reference bound would
+// reject a legitimate anchor rather than the oversized garbage these limits
+// exist to stop. Exported because the same value must bound the field before
+// it is ever written — see verifier.EncodePublicKey.
+const MaxEncodedPublicKeyLength = 1024
+
 func validateEntry(entry Entry) error {
 	if err := skills.ValidateSkillName(entry.Name); err != nil {
 		return fmt.Errorf("entry name: %w", err)
@@ -239,9 +247,6 @@ func validateDigestKind(entry Entry) error {
 // well-formed graphic strings of bounded length. Validation is purely
 // syntactic — whether the identity is trustworthy is the verifier's job.
 func validateProvenance(p *Provenance) error {
-	if err := validateProvenanceAnchor(p); err != nil {
-		return err
-	}
 	fields := map[string]string{
 		"signerIdentity":    p.SignerIdentity,
 		"certIssuer":        p.CertIssuer,
@@ -255,8 +260,8 @@ func validateProvenance(p *Provenance) error {
 		if value == "" {
 			continue
 		}
-		if len(value) > maxReferenceLength {
-			return fmt.Errorf("%s exceeds %d characters", name, maxReferenceLength)
+		if limit := provenanceFieldLimit(name); len(value) > limit {
+			return fmt.Errorf("%s exceeds %d characters", name, limit)
 		}
 		if strings.TrimSpace(value) != value {
 			return fmt.Errorf("%s has leading or trailing whitespace", name)
@@ -267,7 +272,20 @@ func validateProvenance(p *Provenance) error {
 			}
 		}
 	}
-	return nil
+	// Ordered after the syntactic checks so publicKey is length-bounded before
+	// it is base64-decoded: this file is hand-editable, and decoding first
+	// would size the allocation off a value that has not been checked yet.
+	return validateProvenanceAnchor(p)
+}
+
+// provenanceFieldLimit returns the length bound for one provenance field.
+// Every field is a short identifier except publicKey, which carries encoded
+// key material and needs the wider bound.
+func provenanceFieldLimit(name string) int {
+	if name == "publicKey" {
+		return MaxEncodedPublicKeyLength
+	}
+	return maxReferenceLength
 }
 
 // validateProvenanceAnchor enforces that an entry records exactly one trust

pkg/skills/lockfile/validation_test.go

@@ -275,7 +275,10 @@ func TestValidateLockfile(t *testing.T) {
 			name: "publicKey must be base64",
 			lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
 				{Name: "keyed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
-					PublicKey: "-----BEGIN PUBLIC KEY-----",
+					// Graphic and whitespace-free, so the syntactic checks pass
+					// and the base64 decode is genuinely what rejects it. PEM
+					// armor would be caught earlier, by its embedded spaces.
+					PublicKey: "-----BEGINPUBLICKEY-----",
 				}},
 			}},
 			wantErr: "not valid base64",
@@ -419,6 +422,32 @@ func TestValidateLockfile(t *testing.T) {
 				},
 			}},
 		},
+		{
+			// Key material needs more room than an identifier: an RSA-4096
+			// SPKI encodes to 736 characters, so the reference bound would
+			// reject a legitimate anchor rather than the garbage it guards
+			// against. Reaching the SPKI parse at this length proves the
+			// narrower bound is not the one being applied.
+			name: "publicKey is bounded above the reference limit",
+			lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
+				{Name: "keyed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
+					PublicKey: strings.Repeat("A", maxReferenceLength+4),
+				}},
+			}},
+			wantErr: "not a DER SPKI public key",
+		},
+		{
+			// The length must be rejected before the value is base64-decoded,
+			// so the allocation is bounded by a checked number rather than by
+			// whatever the lock file happens to contain.
+			name: "publicKey beyond its own bound rejected before decoding",
+			lf: Lockfile{Version: CurrentVersion, Skills: []Entry{
+				{Name: "keyed", Source: "s", Digest: validSHA256Digest, Provenance: &Provenance{
+					PublicKey: strings.Repeat("A", MaxEncodedPublicKeyLength+4),
+				}},
+			}},
+			wantErr: "exceeds",
+		},
 		{
 			name: "requiredBy diamond is not a cycle",
 			lf: Lockfile{Version: CurrentVersion, Skills: []Entry{

pkg/skills/options.go

@@ -40,6 +40,15 @@ type InstallOptions struct {
 	// "unsigned: true". Skill content is AI-executed instructions, so this
 	// is an explicit per-install trust decision, never a default.
 	AllowUnsigned bool `json:"allow_unsigned,omitempty"`
+	// PublicKey is the base64-encoded DER SPKI cosign public key a
+	// project-scoped install must verify the artifact against, for artifacts
+	// signed with a cosign key pair rather than keylessly. Required on true
+	// first use of such an artifact — the signing key is recoverable from
+	// neither the artifact nor its bundle, so nothing else can supply the
+	// trust anchor — and pinned into the lock entry, which supplies it on
+	// every install thereafter. A value that conflicts with what the lock
+	// already pins is rejected, never ignored.
+	PublicKey string `json:"public_key,omitempty"`
 	// LayerData is the tar.gz content from an OCI layer. Internal use only — NOT exposed via HTTP API.
 	LayerData []byte `json:"-"`
 	// Reference is the full OCI reference (e.g. ghcr.io/org/skill:v1).

pkg/skills/skillsvc/install.go

@@ -47,6 +47,14 @@ func (s *service) Install(ctx context.Context, opts skills.InstallOptions) (*ski
 	// the same lock key and DB record.
 	opts.ProjectRoot = projectRoot
 
+	// Checked here, before any resolve or fetch work: this is the only entry
+	// point a caller-supplied public key arrives through, and rejecting it now
+	// means a key that could never be used is reported as bad input rather
+	// than as a verification failure after the artifact has been pulled.
+	if err := validateInstallPublicKey(opts, scope); err != nil {
+		return nil, err
+	}
+
 	// When the caller supplies `version` separately and the name is a tag-less
 	// OCI-like reference (contains '/' but no ':' or '@'), splice the version
 	// in as the tag. Without this, parseOCIReference + qualifiedOCIRef would