Verify key-signed skill installs against a public key

[samuv]

Summary

A skill pushed with thv skill push --key could be published but never installed project-scoped. The keyless path has no trust root to chain a cosign key-pair signature to, and the signing key is recoverable from neither the artifact nor its attached bundle — cosign's manifest defines no annotation carrying it — so nothing could supply the trust anchor. #6443 made that refusal honest and #6444 gave the lock file somewhere to record a key; this PR gives the install a way through.

• --public-key on thv skill install. The CLI reads the cosign.pub file and sends the encoded key material, not the path: the server may be a different process on a different host, where that path names nothing — or something else. Required on true first use, then pinned into the lock entry, which supplies it on every install thereafter. Trust on first use, with the anchor named explicitly because it cannot be observed.
• Dispatch is lock-first. The recorded expectation picks the verification path, never the artifact. Were the artifact allowed to select its own policy, a republished key-signed artifact could walk an entry out of the certificate identity it is pinned to.
• Conflicts are refused, not resolved by precedence. A supplied key that disagrees with the lock — a different key, an identity pin, a recorded unsigned exception, a catalog-declared identity, or allow_signer_change — is an error. Silently preferring either anchor is how a mistyped --public-key installs as though it had been honored. v1 offers no in-place re-anchor, and each refusal says so.
• ErrKeylessSigned, the mirror of Report key-signed artifacts as such at install #6443's ErrKeySigned: a key aimed at a keylessly-signed artifact is the likeliest way to reach this path by mistake, and a bare "signature verification failed" hides the remedy (drop the key).
• Git and local-build installs refuse a public key outright — a commit signature is made with a Fulcio certificate, and a local build carries no registry signature at all.

Part of #6442. Scoped to install; --public-key on upgrade/sync --adopt follows in the next PR.

Two fixes this exposed

• publicKey shared the 512-character reference bound, but an RSA-4096 SPKI encodes to 736 — the bound would have rejected a legitimate anchor rather than the oversized garbage it guards against. It now has its own.
• validateProvenance ran the anchor's base64 decode before the length check, sizing the allocation off an unchecked value from a hand-editable file. The syntactic checks now run first.

Review fixes

Key verification was not bound to the artifact being installed (CWE-345/347, raised by @JAORMX). The signature covers cosign's simple-signing payload, and RetrieveBundles binds each candidate to the digest of the layer it came from — which proves the signature covers that blob intact, but says nothing about which artifact the blob describes. Signature manifests are discovered by a tag derived from the digest under verification, so attachment was the only link, and it is forgeable by anyone who can push a tag: copying artifact A's signature layer into sha256-<B>.sig made it discoverable as B's, and it still verified. B was then accepted under whatever key legitimately signed A — no key compromise, no forgery.

VerifyOCIWithKey now reconstructs the payload digest from the requested reference and requires each candidate to sign exactly those bytes. Comparing digests rather than parsing the payload keeps the check on the bytes that were actually signed: a payload edited to name B no longer hashes to the layer digest the signature verified against. The offline path already worked this way. Attached-but-unbound material is reported distinctly from unsigned, so it cannot invite an --allow-unsigned override for what is really a rejected trust claim.

The regression test transplants a real signature between two artifacts in an in-process registry. Without the fix it returns a nil error — the hole was live, not theoretical.

Missing-key guidance pointed away from this feature (also @JAORMX). A first install that omitted --public-key was told key-pair signatures cannot be verified and to republish keylessly. That is now split by what the entry pins, because the obvious advice is wrong in one of the two cases: with no anchor recorded, --public-key is exactly the missing input; with a keyless identity pinned, resolveKeyAnchor refuses a supplied key rather than letting it displace the pin, so naming the flag would walk the caller into a conflict one step later. That arm names the pinned signer and says re-anchoring means removing the entry. The remedy is gone from ErrKeySigned itself, which is shared with the plugins surface where --public-key does not exist yet.

Type of change

• New feature (non-breaking change which adds functionality)

Changes

File	Change
pkg/skills/verifier/publickey.go	New. EncodePublicKey/DecodePublicKey between cosign.pub and the stored base64 DER SPKI
pkg/skills/verifier/oci.go, errors.go	ErrKeylessSigned and the onlyKeylessSigned diagnosis
pkg/skills/skillsvc/verify.go	resolveKeyAnchor, the key verification branch, conflict refusals, key-path classification
pkg/skills/skillsvc/install.go	Entry guard: a key this install could never use is bad input
pkg/skills/lockfile/validation.go	Own bound for publicKey; syntactic checks before the anchor decode
pkg/skills/options.go, pkg/api/v1/skills*.go, pkg/skills/client/*	PublicKey through the option, DTO, and client
cmd/thv/app/skill_install.go	--public-key flag and PEM read
pkg/skills/verifier/oci.go	bundleSignsPayload: binds each key-path candidate to the reconstructed payload digest
pkg/skills/verifier/errors.go	ErrKeySigned states the situation and prescribes no remedy (shared with plugins)
cmd/thv/app/skill_push.go, docs/	--key help and architecture docs describe the supported install flow

Test plan

• Unit tests added/updated
• task test passes with no failures
• task lint-fix clean for every file touched (the 6 remaining staticcheck hits are pre-existing, in untouched cmd/thv-operator/ files)
• task docs regenerated

New coverage: PEM↔base64 round-trip and its rejections (private key refused by label, multi-block, non-SPKI, over-bound); every arm of resolveKeyAnchor; the entry guard; key-path error classification; the git and local-build refusals; onlyKeylessSigned including the invariant that it and onlyKeySigned never both hold.

Does this introduce a user-facing change?

Yes. thv skill install --public-key <cosign.pub> installs a skill signed with thv skill push --key. The key is recorded in the lock file and reused on later installs; supplying a conflicting one is refused rather than ignored.

Special notes for reviewers

Why the key must be supplied rather than observed. A key-pair bundle carries no certificate and writes no Rekor entry, so there is nothing to read the signing key off. That makes key provenance a weaker claim than keyless — it says the holder of this key signed this artifact, nothing about who that holder is — which is why the anchor has to come from outside the artifact every time, and why allow_signer_change cannot re-anchor it (re-recording "what was observed" would mean re-recording whatever the caller named).

Wrong key and damaged signature are genuinely indistinguishable. The bundle records no key of its own to compare against, so the error names both causes instead of guessing.

allow_unsigned is not a remedy on any arm of the key path. An install that named a public key asked for that key to be enforced; recording an unsigned exception would file a false trust decision in the lock.

Size. 510 insertions across 14 files of production code, over the repo's 400-line/10-file guideline; 423/13 of that is the original scope and the rest is the two review fixes above. Splitting the ErrKeylessSigned diagnosis out would fit the cap but would ship the key path without the message for its most common failure, and the binding fix is not separable from the path it protects. Happy to split if you'd prefer.

Overlap with toolhive-core#263. That PR fixes the same binding gap at the source and is still open; v0.0.42 is the latest release, so this closes the hole at the version we actually depend on. #263 inverts the contract this rests on — Bundle.DigestHex becomes the artifact digest — so on bump the comparison can never hold and TestVerifyOCIWithKeyRoundTrip fails with it. That tripwire is deliberate, and the code says to delete the helper rather than loosen the comparison.

The keyless path shares the same gap. VerifyBundle binds to the same layer digest, so this predates the PR; activating the key path only newly exposes it. Not fixed here because attestation bundles derive their digest elsewhere and need a distinction the key path does not — worth its own change, and #263 may land first.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 92.81437% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.07%. Comparing base (d1d76c3) to head (56fa28e).

Files with missing lines	Patch %	Lines
pkg/skills/skillsvc/verify.go	94.69%	6 Missing ⚠️
pkg/skills/verifier/publickey.go	87.50%	3 Missing ⚠️
pkg/skills/verifier/oci.go	90.00%	2 Missing ⚠️
pkg/skills/skillsvc/install.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6447      +/-   ##
==========================================
- Coverage   78.11%   78.07%   -0.04%     
==========================================
  Files         767      768       +1     
  Lines       74395    74551     +156     
==========================================
+ Hits        58110    58205      +95     
- Misses      16280    16341      +61     
  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.

[JAORMX]

The install path now reaches key verification, but the online verifier does not bind the verified signature payload to the artifact digest being installed. CI is green, but this trust-boundary issue must be fixed before merge.

[JAORMX]

Re-reviewed the current rebased head against its declared stacked base. The existing digest-binding blocker remains unresolved. I also found that the newly supported first-install flow still tells users who omit --public-key that key-pair signatures cannot be verified and should be republished keylessly; the push help and architecture docs repeat that stale guidance. Please direct them to --public-key and update the generated CLI/docs. CI has completed with no failing or pending checks.

[JAORMX]

Re-reviewed the force-updated head against its declared stacked base across Spec, Standards, Security, Architecture, UX, DevEx, and Reuse. No new duplicate inline comments were added. The two existing blockers remain in the current diff: online key verification is not bound to the requested artifact digest (discussion_r3892856253), and the missing-public-key error/help/docs still direct users to republish keylessly instead of using --public-key (discussion_r3895952200). CI is green.