Report key-signed artifacts as such at install

cmd/thv/app/skill_push.go

@@ -35,7 +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")
+			"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")
 	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

@@ -426,6 +426,8 @@ 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).
+
 ### Schema
 
 ```yaml

pkg/plugins/options.go

@@ -194,6 +194,7 @@ const (
 	FailureReasonLockWriteFailed     = skills.FailureReasonLockWriteFailed
 	FailureReasonSignatureInvalid    = skills.FailureReasonSignatureInvalid
 	FailureReasonSignerMismatch      = skills.FailureReasonSignerMismatch
+	FailureReasonKeySigned           = skills.FailureReasonKeySigned
 	FailureReasonUnsignedRejected    = skills.FailureReasonUnsignedRejected
 	FailureReasonUnknown             = skills.FailureReasonUnknown
 

pkg/plugins/pluginsvc/verify.go

@@ -321,6 +321,17 @@ func classifyInstallVerifyError(
 				pluginName, verifyErr),
 			http.StatusForbidden,
 		)
+	// Reported before the default arm so a key-signed artifact is named as
+	// such rather than as a failed verification. allow_unsigned is
+	// deliberately no remedy here: the artifact IS signed, and recording it
+	// as an unsigned exception would file a false trust decision in the lock.
+	case errors.Is(verifyErr, verifier.ErrKeySigned):
+		return httperr.WithCode(
+			fmt.Errorf("plugin %q: %w; re-publish it with keyless signing"+
+				" (allow_unsigned does not apply — the artifact is signed)",
+				pluginName, verifyErr),
+			http.StatusForbidden,
+		)
 	default:
 		return httperr.WithCode(
 			fmt.Errorf("signature verification failed for %q: %w", pluginName, verifyErr),
@@ -341,6 +352,8 @@ func classifySignatureError(err error) plugins.FailureReason {
 		return plugins.FailureReasonSignerMismatch
 	case errors.Is(err, verifier.ErrUnsigned):
 		return plugins.FailureReasonUnsignedRejected
+	case errors.Is(err, verifier.ErrKeySigned):
+		return plugins.FailureReasonKeySigned
 	case errors.Is(err, verifier.ErrSignatureInvalid):
 		return plugins.FailureReasonSignatureInvalid
 	default:

pkg/plugins/pluginsvc/verify_test.go

@@ -481,3 +481,45 @@ func TestInstallVerification_OversizedBundleIsNotStored(t *testing.T) {
 	})
 	require.Error(t, err, "an oversized bundle must not produce a DB record")
 }
+
+// TestClassifyInstallVerifyErrorNamesKeySigned pins the user-facing half of
+// #6442: a key-signed artifact must be diagnosed as key-signed, must not be
+// reported as a verification failure, and must say plainly that allow_unsigned
+// is no remedy — the artifact is signed, so recording an unsigned exception
+// would file a false trust decision in the lock.
+func TestClassifyInstallVerifyErrorNamesKeySigned(t *testing.T) {
+	t.Parallel()
+
+	err := classifyInstallVerifyError(verifier.ErrKeySigned, "some-plugin", nil)
+	assert.Contains(t, err.Error(), "cosign key pair")
+	assert.Contains(t, err.Error(), "re-publish it with keyless signing",
+		"the message must state the remedy, not merely the refusal")
+	assert.Contains(t, err.Error(), "allow_unsigned does not apply")
+	assert.NotContains(t, err.Error(), "signature verification failed for",
+		"the generic invalid-signature wording is the misdiagnosis this replaces")
+}
+
+// TestClassifySignatureErrorNamesKeySigned keeps the sync/upgrade failure
+// reason distinct from signature-invalid for the same reason.
+func TestClassifySignatureErrorNamesKeySigned(t *testing.T) {
+	t.Parallel()
+
+	assert.Equal(t, plugins.FailureReasonKeySigned, classifySignatureError(verifier.ErrKeySigned))
+	assert.Equal(t, plugins.FailureReasonSignatureInvalid,
+		classifySignatureError(verifier.ErrSignatureInvalid),
+		"the pre-existing mapping must be unaffected")
+}
+
+// TestIsAllowedUnsignedRejectsKeySigned is the guard that closes #6442's
+// actual escape-hatch gap: --allow-unsigned must not rescue a key-signed
+// artifact even on true first use with the flag explicitly set.
+func TestIsAllowedUnsignedRejectsKeySigned(t *testing.T) {
+	t.Parallel()
+
+	assert.False(t, isAllowedUnsigned(verifier.ErrKeySigned,
+		plugins.InstallOptions{AllowUnsigned: true}, nil),
+		"a signed artifact must never be recordable as an unsigned exception")
+	assert.True(t, isAllowedUnsigned(verifier.ErrUnsigned,
+		plugins.InstallOptions{AllowUnsigned: true}, nil),
+		"the genuine unsigned case must still be allowed through")
+}

pkg/skills/options.go

@@ -315,6 +315,11 @@ const (
 	// is pinned — a narrower case than FailureReasonSignerMismatch, whose
 	// remediation (--allow-signer-change) is nonetheless the same.
 	FailureReasonProvenanceFieldMismatch FailureReason = "provenance-field-mismatch"
+	// FailureReasonKeySigned means the artifact carries only cosign
+	// key-pair signatures, which install-time verification cannot check.
+	// Distinct from FailureReasonSignatureInvalid: nothing is wrong with
+	// the signature, there is simply no trust anchor to check it against.
+	FailureReasonKeySigned FailureReason = "key-signed-unverifiable"
 	// FailureReasonUnsignedRejected means the artifact is unsigned and the
 	// operation did not permit unsigned installs.
 	FailureReasonUnsignedRejected FailureReason = "unsigned-rejected"

pkg/skills/skillsvc/verify.go

@@ -292,6 +292,12 @@ func classifyInstallVerifyError(
 				" and reinstall, or upgrade with allow_signer_change)", skillName, verifyErr),
 			http.StatusForbidden,
 		)
+	// Reported before the default arm so a key-signed artifact is named as
+	// such rather than as a failed verification. allow_unsigned is
+	// deliberately no remedy here: the artifact IS signed, and recording it
+	// as an unsigned exception would file a false trust decision in the lock.
+	case errors.Is(verifyErr, verifier.ErrKeySigned):
+		return keySignedInstallError(skillName, verifyErr)
 	default:
 		return httperr.WithCode(
 			fmt.Errorf("signature verification failed for %q: %w", skillName, verifyErr),
@@ -309,12 +315,35 @@ func classifyCatalogVerifyError(verifyErr error, skillName string) error {
 			http.StatusForbidden,
 		)
 	}
+	// A key-signed artifact is not a provenance mismatch — nothing was
+	// compared, because the keyless policy cannot check a key-pair signature
+	// at all. The catalog constraint is beside the point, so this reports the
+	// same diagnosis and remedy the non-catalog route does.
+	if errors.Is(verifyErr, verifier.ErrKeySigned) {
+		return keySignedInstallError(skillName, verifyErr)
+	}
 	return httperr.WithCode(
 		fmt.Errorf("skill %q does not match its catalog-declared provenance: %w", skillName, verifyErr),
 		http.StatusForbidden,
 	)
 }
 
+// keySignedInstallError reports a key-signed artifact identically wherever it
+// is detected. A lock-constrained install and a catalog-constrained first
+// install reach classification by different routes, but neither could verify
+// the artifact and both have the same remedy, so the wording is shared rather
+// than duplicated — including the note that allow_unsigned is not a way out,
+// since the artifact IS signed and recording it as an unsigned exception
+// would file a false trust decision in the lock.
+func keySignedInstallError(skillName string, verifyErr error) error {
+	return httperr.WithCode(
+		fmt.Errorf("skill %q: %w; re-publish it with keyless signing"+
+			" (allow_unsigned does not apply — the artifact is signed)",
+			skillName, verifyErr),
+		http.StatusForbidden,
+	)
+}
+
 // classifySignatureError maps verifier sentinels to typed failure reasons
 // for sync/upgrade results. Returns "" when err is not a signature failure.
 func classifySignatureError(err error) skills.FailureReason {
@@ -327,6 +356,8 @@ func classifySignatureError(err error) skills.FailureReason {
 		return skills.FailureReasonSignerMismatch
 	case errors.Is(err, verifier.ErrUnsigned):
 		return skills.FailureReasonUnsignedRejected
+	case errors.Is(err, verifier.ErrKeySigned):
+		return skills.FailureReasonKeySigned
 	case errors.Is(err, verifier.ErrSignatureInvalid):
 		return skills.FailureReasonSignatureInvalid
 	default:

pkg/skills/skillsvc/verify_test.go

@@ -991,3 +991,79 @@ func TestClassifyInstallVerifyErrorDistinguishesProvenanceField(t *testing.T) {
 	assert.Contains(t, identityMismatch.Error(), "signer identity mismatch for",
 		"a genuine signer-identity mismatch keeps its existing wording")
 }
+
+// TestClassifyInstallVerifyErrorNamesKeySigned pins the user-facing half of
+// #6442: a key-signed artifact must be diagnosed as key-signed, must not be
+// reported as a verification failure, and must say plainly that allow_unsigned
+// is no remedy — the artifact is signed, so recording an unsigned exception
+// would file a false trust decision in the lock.
+func TestClassifyInstallVerifyErrorNamesKeySigned(t *testing.T) {
+	t.Parallel()
+
+	err := classifyInstallVerifyError(verifier.ErrKeySigned, "some-skill", nil)
+	assert.Contains(t, err.Error(), "cosign key pair")
+	assert.Contains(t, err.Error(), "re-publish it with keyless signing",
+		"the message must state the remedy, not merely the refusal")
+	assert.Contains(t, err.Error(), "allow_unsigned does not apply")
+	assert.NotContains(t, err.Error(), "signature verification failed for",
+		"the generic invalid-signature wording is the misdiagnosis this replaces")
+}
+
+// TestClassifySignatureErrorNamesKeySigned keeps the sync/upgrade failure
+// reason distinct from signature-invalid for the same reason.
+func TestClassifySignatureErrorNamesKeySigned(t *testing.T) {
+	t.Parallel()
+
+	assert.Equal(t, skills.FailureReasonKeySigned, classifySignatureError(verifier.ErrKeySigned))
+	assert.Equal(t, skills.FailureReasonSignatureInvalid,
+		classifySignatureError(verifier.ErrSignatureInvalid),
+		"the pre-existing mapping must be unaffected")
+}
+
+// TestIsAllowedUnsignedRejectsKeySigned is the guard that closes #6442's
+// actual escape-hatch gap: --allow-unsigned must not rescue a key-signed
+// artifact even on true first use with the flag explicitly set.
+func TestIsAllowedUnsignedRejectsKeySigned(t *testing.T) {
+	t.Parallel()
+
+	assert.False(t, isAllowedUnsigned(verifier.ErrKeySigned,
+		skills.InstallOptions{AllowUnsigned: true}, nil),
+		"a signed artifact must never be recordable as an unsigned exception")
+	assert.True(t, isAllowedUnsigned(verifier.ErrUnsigned,
+		skills.InstallOptions{AllowUnsigned: true}, nil),
+		"the genuine unsigned case must still be allowed through")
+}
+
+// TestCatalogInstallNamesKeySignedArtifact covers the second route to
+// classification. A first install resolved from a catalog entry that declares
+// provenance is classified by classifyCatalogVerifyError, not
+// classifyInstallVerifyError, so a key-signed artifact arriving that way would
+// otherwise be reported as failing to match its catalog-declared provenance —
+// which is doubly wrong: nothing was compared, because the keyless policy
+// cannot check a key-pair signature at all, and the report would carry neither
+// the re-publish remedy nor the note that allow_unsigned cannot help.
+func TestCatalogInstallNamesKeySignedArtifact(t *testing.T) {
+	t.Parallel()
+
+	projectRoot := makeProjectRoot(t)
+	mv := verifiermocks.NewMockVerifier(gomock.NewController(t))
+	svc := &service{sigVerifier: mv}
+	opts := skills.InstallOptions{
+		ProjectRoot:       projectRoot,
+		CatalogProvenance: &regtypes.Provenance{SignerIdentity: testSignerIdentity},
+	}
+	mv.EXPECT().VerifyOCI(
+		gomock.Any(), gomock.Any(), gomock.Any(),
+		gomock.Eq(verifier.NewCatalogExpectation(opts.CatalogProvenance))).
+		Return(nil, verifier.ErrKeySigned)
+
+	_, err := svc.verifyOCIInstall(
+		t.Context(), opts, "catalog-skill", "ghcr.io/test/catalog-skill:v1", "sha256:digest")
+
+	require.Error(t, err)
+	assert.Equal(t, http.StatusForbidden, httperr.Code(err))
+	assert.Contains(t, err.Error(), "re-publish it with keyless signing")
+	assert.Contains(t, err.Error(), "allow_unsigned does not apply")
+	assert.NotContains(t, err.Error(), "does not match its catalog-declared provenance",
+		"a key-signed artifact was never compared against the catalog constraint")
+}

pkg/skills/verifier/errors.go

@@ -15,6 +15,21 @@ var (
 	// ErrSignerMismatch indicates the signature verifies, but against an
 	// identity other than the expected one.
 	ErrSignerMismatch = errors.New("signer identity mismatch")
+	// ErrKeySigned indicates the artifact carries only cosign key-pair
+	// signatures, which install-time verification cannot check: the keyless
+	// (Fulcio) trust root has nothing to chain them to, and the signing
+	// public key is recoverable neither from the artifact nor from the
+	// attached bundle — cosign's manifest defines no annotation carrying it,
+	// and the reconstructed bundle holds a fixed placeholder hint in its
+	// place.
+	//
+	// Deliberately NOT wrapping ErrSignatureInvalid, unlike
+	// ErrProvenanceFieldMismatch below: the signature may be perfectly
+	// valid, so reporting it as a verification failure is precisely the
+	// misclassification this sentinel exists to end. That narrowing cannot
+	// fail open, because no caller treats ErrSignatureInvalid as permission
+	// to proceed — it only selects a failure reason.
+	ErrKeySigned = errors.New("artifact is signed with a cosign key pair, which cannot be verified at install time")
 	// ErrProvenanceFieldMismatch indicates the signature verifies against
 	// the expected signer identity and issuer, but a certificate field the
 	// Sigstore policy cannot itself express — the repository ref or runner

pkg/skills/verifier/oci.go

@@ -217,9 +217,35 @@ func classifyVerifyFailure(
 			return signerMismatchError(vr, expected)
 		}
 	}
+	// Every bundle being certificate-less is the cosign key-pair layout, not
+	// a broken keyless signature — the keyless policy could never have
+	// accepted it. Falling through to wrapInvalid would report
+	// ErrSignatureInvalid, sending the user hunting a corrupt signature; and
+	// because --allow-unsigned only overrides ErrUnsigned, it would leave
+	// them a 403 with no available remedy. A mixed artifact keeps the
+	// keyless diagnosis: one of its bundles genuinely failed the policy.
+	if onlyKeySigned(bundles) {
+		return ErrKeySigned
+	}
 	return wrapInvalid(lastErr)
 }
 
+// onlyKeySigned reports whether every retrieved bundle uses the cosign
+// key-pair layout. An empty slice is not key-signed: retrieveBundles already
+// reports having found no signature material as ErrUnsigned, so classify is
+// never reached with one.
+func onlyKeySigned(bundles []coreverifier.Bundle) bool {
+	if len(bundles) == 0 {
+		return false
+	}
+	for _, b := range bundles {
+		if b.HasCertificate() {
+			return false
+		}
+	}
+	return true
+}
+
 // signerMismatchError builds the ErrSignerMismatch error, naming both the
 // expected identity tuple and the identity the artifact actually verifies
 // with (when extractable).