Report key-signed artifacts as such at install

[samuv]

Summary

thv skill push --key and thv ai-plugin push --key produce a validly key-signed artifact, but install-time verification is keyless-only — so the result cannot be installed project-scoped. Worse than failing as "unsigned", it failed inexplicably:

• A key-pair signature is a real signature referrer, so retrieveBundles finds it and ErrUnsigned never fires — that sentinel means no signature material at all.
• verifyKeylessBundles then fails for want of a Fulcio certificate chain, and classifyVerifyFailure fell through to wrapInvalid → ErrSignatureInvalid.
• isAllowedUnsigned requires errors.Is(err, ErrUnsigned), so --allow-unsigned could not override it.

Net effect: a hard 403 blaming the signature for what is actually a missing trust anchor, with no flag that could resolve it.

This PR makes the diagnosis truthful. It does not yet add key verification — that needs a lock-schema change and is the rest of the stack.

• Adds verifier.ErrKeySigned, classified when every retrieved bundle is certificate-less (via core's already-exported Bundle.HasCertificate(), which had zero callers here).
• Install now names the key-pair layout, states that keyless signing is the remedy, and says plainly that allow_unsigned does not apply — for both skills and plugins.
• Both classification routes share one formatter. A catalog-constrained first install (classifyCatalogVerifyError) is classified separately from a lock-constrained one, and previously reported anything other than unsigned as a mismatch against catalog-declared provenance — wrong twice over for a key-signed artifact, since nothing was compared and the remedy was lost.
• Adds FailureReasonKeySigned for sync/upgrade, distinct from signature-invalid because nothing is wrong with the signature.
• Documents the gap on thv skill push --key, which advertised key signing with no caveat while the equivalent plugin warning shipped separately in Sign plugin pushes and remove the lock feature gate #6438.

Refs #6442 (first of ~7 PRs; the issue stays open).

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Full unit suite passes for every package this PR touches. Two caveats, stated plainly:

• One unrelated failure persists — pkg/transport/proxy/streamable's TestMCPGoClientInitializeAndPing, caused by a stray local process holding :8096. Confirmed pre-existing on clean main in a detached worktree; untouched by this PR.
• task lint-fix reports issues only in cmd/thv-operator/ files this PR does not touch (deprecated ctrl.Result{Requeue} / PrimaryUpstreamProvider). All 15 files here are clean.

New coverage: a real key-signed artifact (pushed to an in-process registry via the existing signArtifact helper) classifying as ErrKeySigned and not as ErrSignatureInvalid or ErrUnsigned; the same artifact against a keyless-pinned lock entry; the 403 wording; the failure-reason mapping; and isAllowedUnsigned refusing to rescue a key-signed artifact even with AllowUnsigned: true.

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

No operator API surface is touched. The docs/server/* churn is the skills FailureReason enum, which is swagger-documented — an additive enum value.

Changes

File	Change
pkg/skills/verifier/errors.go	New ErrKeySigned sentinel, with the rationale for not wrapping ErrSignatureInvalid
pkg/skills/verifier/oci.go	onlyKeySigned() helper; classified before the generic invalid verdict
pkg/skills/skillsvc/verify.go	403 naming the key-pair layout; failure-reason mapping
pkg/plugins/pluginsvc/verify.go	Same, for plugins
pkg/skills/options.go	FailureReasonKeySigned
pkg/plugins/options.go	Alias, per the shared THV-0080 contract
cmd/thv/app/skill_push.go	The missing --key caveat
docs/arch/12-skills-system.md	Why only the keyless path yields an installable artifact
docs/cli/*, docs/server/*	Regenerated (task docs)
3 × *_test.go	New coverage; one pre-existing test refined (see notes)
pkg/skills/skillsvc/verify.go	keySignedInstallError, shared by both classification routes

Does this introduce a user-facing change?

Yes. Installing a key-signed artifact project-scoped still fails — that is unchanged and needs the rest of the stack — but the error now explains why and what to do, instead of reporting a signature verification failure. thv skill push --key also now warns at the point of use that its output is not installable. Sync and upgrade report key-signed-unverifiable rather than signature-invalid.

Implementation plan

Approved implementation plan

Resolved through a design interview covering the whole of #6442. The chosen arm is support key verification, install-only for v1; --key stays on push. Full write-up posted to the issue: #6442 (comment)

Decisions relevant to this PR:

• Lock-first dispatch — the expectation picks the verify path, never the artifact. A key-signed artifact must never be able to dodge an identity pin by choosing its own policy.
• New ErrKeySigned sentinel, and --allow-unsigned must not override it: the artifact is signed, so recording it as an unsigned exception would file a false trust decision in the lock.
• Detection via HasCertificate() — no toolhive-core change or release is needed anywhere in this stack.

Facts established while planning that shaped later PRs, and explain why this one stops where it does:

1. Key signing writes no Rekor entry (toolhive-core/container/signer/signer.go:99). A --key bundle is a bare detached signature: no certificate, no transparency log, no signing time, no revocation.
2. The public key is not recoverable from the artifact or the bundle. signatureAnnotations writes no annotation for it, and the retrieve side reconstructs with a fixed placeholder hint (keySignedPublicKeyHint = "cosign-keypair"). So a sha256: digest in the lock is impossible, resultFromKey must not grow an observed-key field, and "wrong key" vs "broken signature" are genuinely indistinguishable.
3. lockfile.CurrentVersion must not be bumped — validation.go:35 compares !=, not <=, so a bump makes every existing v1 lock unreadable. Unnecessary regardless: validateProvenance already requires signerIdentity, so an older thv fails a future key-pinned entry closed for free.

Remaining sequence: (2) lock schema Provenance.publicKey, identity XOR key; (3) skills install --public-key; (4) skills sync/upgrade; (5–6) plugins mirrors; (7) E2E + docs.

Special notes for reviewers

ErrKeySigned deliberately does not wrap ErrSignatureInvalid, unlike the ErrProvenanceFieldMismatch precedent directly below it. Reporting a valid signature as a verification failure is the exact misclassification being removed. I checked the narrowing cannot fail open: every consumer of ErrSignatureInvalid is either one of the two classifySignatureError switches or a sync.go message construction — nothing treats it as permission to proceed.

A pre-existing test pinned the old behavior. TestVerifyOCIKeylessRejectsKeySignedArtifact asserted ErrSignatureInvalid. Its comment showed the intent was only "not as unsigned, and never as a panic" — ErrSignatureInvalid was simply the classification available at the time, not a claim the signature was bad. I refined it in place (keeping both original invariants as explicit negative assertions) rather than treating it as a contract, and folded my duplicate test into it.

Mixed artifacts keep the keyless diagnosis. onlyKeySigned requires every bundle to be certificate-less; if one carries a certificate, that bundle genuinely failed the keyless policy and the existing error is correct.

Two adjacent paths are already broken for key-signed artifacts and are addressed in PR 4/6, not here — worth knowing they are not regressions: probeCandidateSigner (upgrade.go:275) is keyless-only, so a key-signed upgrade candidate reports UpgradeStatusFailed rather than SignerChangeBlocked; and adoptSkill/adoptLocked back-fill via ResultFromBundle, which requires a certificate, so a key-signed install cannot be adopted at all.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.94%. Comparing base (8fa9220) to head (ddea141).

Files with missing lines	Patch %	Lines
pkg/skills/verifier/oci.go	75.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6443      +/-   ##
==========================================
- Coverage   78.02%   77.94%   -0.08%     
==========================================
  Files         766      766              
  Lines       74077    74102      +25     
==========================================
- Hits        57795    57760      -35     
- Misses      16277    16337      +60     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.