Add commit-signature plumbing and sigstore bundle storage

[samuv]

Note

Stack 2 of RFC THV-0080 (tracking issue #5899) — PR 10 of the Sigstore signing/verification series. Follows #6015 (schema) and #6023 (signer), both merged. This is the core-independent half of the "git + storage" step; the gitsign verification logic lands with the verifier work once stacklok/toolhive-core#192 is released, since it consumes core's exported trusted material.

Summary

Install-time signature verification needs two pieces of groundwork that have no dependency on the toolhive-core release, so they land now to keep the stack moving:

• Git installs must surface the commit signature. git.Client's HeadCommitHash is collapsed into a new HeadCommit method returning hash and signature from a single lookup (per review — both always describe the same commit), with the signature empty for unsigned commits and explicitly documented as UNVERIFIED until the install-time verifier checks it. gitresolver.ResolveResult gains CommitSignature alongside CommitHash. Unlike the POC (Add project-level skills lock file (POC) #5715), the lookup error is propagated, not silently swallowed.
• OCI installs must persist the Sigstore bundle. Migration 004_add_skill_sigstore_bundle.sql adds installed_skills.sigstore_bundle BLOB DEFAULT NULL; InstalledSkill.SigstoreBundle []byte (never serialized to API responses, json:"-") is wired through the SQLite store's Create/Update/scan paths. NULL means unsigned, so rows created by earlier schema versions read back as unsigned rather than acquiring phantom provenance. This is what lets sync re-verify signatures offline later in the stack, without re-contacting the registry.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Test improvements
• CI/CD or build system changes

Test plan

• Unit tests pass locally (task test)
• Linting passes (task lint-fix, 0 issues)
• New unit tests added: git client signature retrieval (nil repo / unsigned commit / signed commit surviving clone verbatim); resolver propagation of the signature with a signed HEAD (and empty for unsigned); migration 004 applied over pre-004 rows defaults to NULL and Down drops the column cleanly; skill store bundle round trip including replace-on-update and clear-to-nil.

Does this introduce a user-facing change?

No — nothing consumes the new fields yet. Verification behavior arrives with the install-verification PR later in the stack.

Special notes for reviewers

• Nothing consumes the new fields on this branch — ResolveResult.CommitSignature and InstalledSkill.SigstoreBundle are wiring points for the install-verification PR (PR11 in the stack), which threads them through InstallOptions so git and OCI installs share one construction point.
• The signed-commit tests attach a synthetic armored signature by rewriting HEAD through go-git's object storer — the same object layout gitsign produces — because generating a real gitsign signature requires a Fulcio flow that doesn't belong in unit tests. Cryptographic verification of these signatures is deliberately out of scope here.
• Migration 004 mirrors 003's structure (ALTER TABLE ... ADD COLUMN with a NULL default; Down drops the column).

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 86.84211% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.41%. Comparing base (53908ef) to head (6e8e888).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/git/client.go	70.00%	2 Missing and 1 partial ⚠️
pkg/plugins/pluginsvc/install_git.go	66.66%	1 Missing ⚠️
pkg/skills/gitresolver/resolver.go	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6084      +/-   ##
==========================================
+ Coverage   72.30%   72.41%   +0.10%     
==========================================
  Files         728      731       +3     
  Lines       75496    75753     +257     
==========================================
+ Hits        54590    54854     +264     
+ Misses      16987    16983       -4     
+ Partials     3919     3916       -3     

☔ 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]

Panel review of #6084 — three orthogonal axes (Spec vs RFC THV-0080/#5899, Standards vs repo rules, Domain panel: security / architecture / duplication / devex / library-reuse). Verified locally: branch builds, go test ./pkg/git/... ./pkg/skills/... ./pkg/storage/... green, no new dependencies.

Spec axis (RFC THV-0080 + tracking issue #5899, PR10 scope)

Scope is exactly PR10 ("migration for sigstore bundle blob + git commit-signature plumbing") — no creep. Two borderline notes, no blockers:

• Nothing consumes the new fields yet. ResolveResult.CommitSignature and the store's SigstoreBundle round-trip have zero producers/consumers on this branch (same for the pre-existing InstallOptions.SigstoreBundle, which nothing populates). Consistent with the stack plan, but worth calling out in the PR description that these are PR11 wiring points.
• Shape verified against the RFC: HEAD is the pinned commit at read time (Clone checks out the pinned commit before Resolve calls HeadCommitSignature), so the signature/hash pair is read from the same ref. BLOB column is one of the two options the RFC's data-model section explicitly offers ("adjacent blob or dedicated column/table"). opts.SigstoreBundle deferral to PR11 is legitimate.

Standards axis (CLAUDE.md + .claude/rules/)

• Hard violation: commit subject is 58 chars — Add commit-signature plumbing and sigstore bundle storage exceeds the 50-char limit (CLAUDE.md § Commit Guidelines). Otherwise compliant (imperative, no conventional prefix, explains what/why; the Co-Authored-By trailer matches recent merged precedent).
• Clean otherwise: migration 004 mirrors 003's structure exactly; the hand-written blockingCloneClient stub predates this PR (#4051) and extending it is forced by the interface change; PR size is within limits (10 files, well under 400 code lines); tests follow testing.md (t.Cleanup in parallel tests, table-driven where applicable); doc comments are accurate and synchronized.

Domain axis

Cross-confirmed (architecture + standards + duplication, 3 axes independently): the HeadCommitHash/HeadCommitSignature split. See inline comment — the one finding worth acting on before PR11 hardens the contract.

Security (secure-code-reviewer): no actionable findings. Signature is transported as an opaque string — never parsed, logged, or formatted into errors; all SQL parameterized; NULL→nil scan semantics tested; json:"-" verified as the sole and effective control against API leakage (pkg/api/v1/skills_types.go embeds InstalledSkill directly); migration is transactional, metadata-only, and old binaries tolerate the extra column. One residual risk lives in the follow-up verifier PR, not here: the armored text is the only place the gitsign identity lives, so PR11/12 must verify the signature over the commit object and not trust self-described payload contents.

Architecture (software-architect): layering is right — PGPSignature is commit metadata honestly reported by git.Client, verification logic correctly absent from this layer; the column choice is sound (1:1, never independently queried). Watch-items for PR11: (1) CommitSignature (armored gitsign string, git path) and SigstoreBundle (bytes, OCI path) are different artifacts from different provenance flows — don't pour one into the other; (2) buildInstalledSkill has no seam for the bundle yet — thread it via InstallOptions so git and OCI share one construction point.

Duplication / library-reuse / devex: nothing actionable. All duplication candidates are at the two-site threshold (Rule of Three not met) or essential; the code uses go-git's API properly rather than hand-parsing; empty-string-for-unsigned is conventional; interface addition is a theoretical break but git.Client has only in-tree implementations. Polish notes as inline comments.

Verdict: approve — the commit-subject length is the only hard item; the interface-collapse suggestion is optional but cheapest now.

[JAORMX]

Inline findings from the panel review (summary in the review above). Only the HeadCommitHash/HeadCommitSignature collapse suggestion rises above polish — everything else is optional.