Add project-level skills lock file (POC) by samuv · Pull Request #5715 · stacklok/toolhive

samuv · 2026-07-03T14:35:45Z

Summary

Why: Project-scoped skill installs had no reproducible pin, no integrity verification beyond content hashing, and no safe upgrade path when publisher identity changes. Teams could not commit a lock file and trust that sync/upgrade would restore or advance skills safely.
What: Introduce toolhive.lock.yaml for project scope with thv skill sync and thv skill upgrade, then fold RFC THV-0080 v1 Sigstore signing/verification into the same POC: verify on consume, sign on push, pin publisher identity (provenance) or record unsigned: true, and re-verify stored bundles offline in sync --check.

POC scope (RFC THV-0080)

This PR is a feasibility vertical slice, not production-hardening split across many small PRs yet. It proves:

Question	How this PR answers it
Reproducible project installs?	Lock pins `source`, `resolvedReference`, `digest`, `contentDigest`
On-disk tampering?	`contentDigest` dirhash + `sync --check`
Adopt out-of-band installs?	`sync --adopt` writes lock entries (with provenance or `unsigned`)
Transitive deps?	`toolhive.requires` materialized; `requiredBy` + cascade uninstall
Safe upgrades?	Digest re-resolve; immutable pins; `--allow-ref-change`; `--allow-signer-change`
Supply-chain trust?	Sigstore verify on install/sync/upgrade; TOFU identity in lock; offline bundle re-check
Publisher signing?	`thv skill push` signs with `--key` (cosign); `--no-sign` escape hatch
CI/scripting?	Typed failure reasons, `--yes`, exit codes 2/3/4

Trust model (v1)

Three layers (see docs/arch/12-skills-system.md):

Cryptographic — Fulcio + Rekor for OCI/gitsign; offline bundle re-verification in sync --check
Publisher identity (TOFU) — provenance.signerIdentity + provenance.certIssuer pinned at first verified install; signer-mismatch on divergence
Human review — PR review of toolhive.lock.yaml; unsigned: true is an explicit reviewed exception

Deferred: keyless OIDC push (E2E against Sigstore staging); catalog-supplied expected identity (needs toolhive-core Provenance on Skill types).

Lock entry schema (v1)

version: 1
skills:
  - name: code-review
    version: "1.0.0"
    source: ghcr.io/org/code-review
    resolvedReference: ghcr.io/org/code-review:v1.2.0
    digest: sha256:abc...
    contentDigest: sha256:def...
    provenance:
      signerIdentity: https://github.com/org/repo/.github/workflows/release.yml@refs/heads/main
      certIssuer: https://token.actions.githubusercontent.com
      repositoryURI: https://github.com/org/repo
      sigstoreURL: https://rekor.sigstore.dev
    requiredBy: [parent-skill]
    explicit: true
  - name: local-build
    source: local-build
    digest: sha256:...
    contentDigest: sha256:...
    unsigned: true

CLI surface

thv skill sync — --check, --adopt, --prune, --yes, --clients, --project-root

thv skill upgrade [name...] — --preview, --fail-on-changes, --allow-ref-change, --allow-signer-change, --yes

thv skill install — --allow-unsigned (project scope)

thv skill push — --key, --no-sign

Exit codes: 0 success · 2 check/freshness failure · 3 partial failure · 4 policy rejection

Commit structure (19 commits, atomic by layer)

Lock file core

Add project-level skills lock file with sync and upgrade commands
Extend lockfile schema with contentDigest and validation
Split SkillLockService from SkillService
Add managed flag for lock-managed skill installs
Harden install hooks and materialize toolhive.requires
Implement RFC sync with check, adopt, and prune
Implement RFC upgrade with preview and ref-change guard
Wire lock service through API, client, and CLI
Rename SyncResult UpToDate field to fix codespell
Document skills lock file sync and upgrade behavior

Sigstore v1

Add provenance and unsigned fields to skills lockfile
Add Sigstore verifier and signer packages for skills
Store verified Sigstore bundles on installed skills
Verify Sigstore signatures during project-scope installs
Re-verify stored bundles offline during skill sync
Block skill upgrades on signer identity changes
Sign OCI artifacts after skill push
Expose Sigstore install and push flags in API and CLI
Document skills lock Sigstore trust model and update tests

Type of change

New feature

Test plan

Unit tests (task test — pkg/skills/... passes)
Linting (task lint-fix)
E2E against Sigstore staging (deferred)
Manual: signed OCI push + project install + sync --check

API compatibility

Additive: POST /skills/sync, POST /skills/upgrade; new request fields (allowUnsigned, allowSignerChange, key, skipSigning). SkillLockService is separate from SkillService. No operator CRD changes.

Large PR justification

~6.6k lines / 74 files. Kept unified for POC feasibility: lock integrity, sync/upgrade guardrails, and Sigstore trust are coupled — splitting mid-stack leaves non-functional intermediate states. If validated, split into the stack below for production merge.

Review focus

Is contentDigest the right integrity primitive?
Are cascade uninstall semantics (requiredBy / explicit) correct?
Is TOFU provenance the right default until catalog trust exists?
Is offline bundle re-verification in --check sufficient?
Is SkillLockService the right API boundary?

Does this introduce a user-facing change?

Yes — new sync/upgrade commands, lock file on project install, Sigstore flags, push signing.

github-actions

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

This review will be automatically dismissed once you add the justification section.

JAORMX · 2026-07-03T14:36:59Z

Love it! Let's talk on Monday about this. Shall we have an RFC too? Just for docs

codecov · 2026-07-03T14:42:40Z

Codecov Report

❌ Patch coverage is 29.44855% with 1241 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.75%. Comparing base (9662681) to head (18d7dd1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/skills/verifier/oci_bundle.go	0.00%	181 Missing ⚠️
pkg/skills/skillsvc/sync.go	25.32%	160 Missing and 11 partials ⚠️
pkg/skills/skillsvc/upgrade.go	35.09%	116 Missing and 19 partials ⚠️
pkg/skills/skillsvc/verify.go	23.80%	104 Missing and 8 partials ⚠️
pkg/skills/skillsvc/lock.go	13.08%	91 Missing and 2 partials ⚠️
pkg/skills/signer/cosign_attach.go	0.00%	50 Missing ⚠️
pkg/skills/signer/key.go	0.00%	45 Missing ⚠️
pkg/skills/signer/signer.go	0.00%	43 Missing ⚠️
pkg/skills/verifier/git.go	0.00%	36 Missing ⚠️
pkg/skills/verifier/oci.go	0.00%	32 Missing ⚠️
... and 21 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5715      +/-   ##
==========================================
- Coverage   70.74%   69.75%   -1.00%     
==========================================
  Files         683      704      +21     
  Lines       69194    70884    +1690     
==========================================
+ Hits        48950    49443     +493     
- Misses      16663    17741    +1078     
- Partials     3581     3700     +119

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

samuv · 2026-07-03T15:00:11Z

Love it! Let's talk on Monday about this. Shall we have an RFC too? Just for docs

Yeah 🚀 , I opened this RFC: stacklok/toolhive-rfcs#80. I just refined it a bit.

Large PR justification has been provided. Thank you!

github-actions · 2026-07-07T10:03:14Z

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

Introduce toolhive.lock.yaml, committed at the project root, that pins the name, version, source, resolved reference, and digest of every project-scoped skill install. This gives teams reproducible skill installs (restorable via "thv skill sync") and a path to refresh pinned content when the catalog moves ("thv skill upgrade"), mirroring the role package-lock.json plays for npm. - New pkg/skills/lockfile package: schema, load/save, and file-locked upsert/remove using pkg/fileutils.WithFileLock; entries sorted by name for stable diffs. - skillsvc install/uninstall hooks: project-scope installs upsert a lock entry (recording the original source string for later re-resolution); uninstalls remove it. User-scope installs never touch the lock. - Sync: installs each entry strictly at its pinned resolvedReference@digest, reports unmanaged project-scope skills, and prunes them with --prune. - Upgrade: re-resolves each entry's original source, compares digests, and installs + rewrites the entry on change; immutable sources (OCI digest, git commit) reported as not-upgradable; --dry-run supported. - API: POST /skills/sync and POST /skills/upgrade plus skills client methods. - CLI: thv skill sync and thv skill upgrade with git-root auto-detection and --project-root override; JSON/text output. - Docs: regenerated CLI/swagger docs; updated docs/arch/12-skills-system.md with a Project Lock File section, diagram, and endpoint/CLI tables. Co-authored-by: Cursor <cursoragent@cursor.com>

samuv · 2026-07-21T15:20:22Z

Splitting this into the stacked PR chain proposed above, matching the merged RFC THV-0080. Closing this POC in favor of:

Stack 1 — lock file (open now):

Add project-level skills lock file schema package #5892 — lockfile schema package
Add SkillLockService interface and managed install flag #5893 — SkillLockService interface + managed install flag
Record project-scope skill installs in the lock file #5894 — install/uninstall lock hooks, toolhive.requires materialization, cascade
Add thv skill sync: restore project skills from the lock file #5895 — thv skill sync
Add thv skill upgrade: re-resolve pinned skills to newer content #5896 — thv skill upgrade
Add typed exit codes and confirmation gate to skill sync/upgrade #5897 — typed exit codes, confirmation gate, arch docs

Stack 2 — Sigstore signing/verification: to follow once Stack 1 is fully merged, per the RFC's trust model. A prerequisite PR to toolhive-core (exporting Sigstore bundle retrieval, offline verification, and identity extraction from container/verifier) will be filed separately.

The whole feature (both stacks) stays gated behind TOOLHIVE_SKILLS_LOCK_ENABLED until the full v1 lands, so each PR in the chain is independently mergeable without exposing partial behavior — see #5894's description for the rationale.

Thanks for validating the approach here — the stack is a from-scratch rewrite against the RFC (not a mechanical re-split of these commits), so it also folds in a few hardening fixes this review process and testing surfaced (a real drift-detection bug in the install fast path, CodeQL path-injection findings, typed failure-reason classification instead of string matching).

RFC THV-0080 gives CI a scriptable contract for sync/upgrade: exit 2 for check/freshness failures, 3 for partial failures, 4 for policy rejections, distinct from the generic 1 cobra already uses. It also requires a pre-install confirmation gate — skill content is a set of AI-followed instructions, so a non-interactive run must refuse outright without --yes rather than silently proceeding. - cmd/thv/app/exitcode.go: exitCodeError carries a specific exit code through a RunE return; main.go maps it via ExitCodeFromError instead of a hardcoded os.Exit(1) - cmd/thv/app/skill_confirm.go: requireConfirmation prompts on a TTY, refuses with ExitCodePolicyRejection off one - sync/upgrade gain --yes, skipped automatically by --check/ --preview (which never install anything); their exit-code mapping prioritizes partial failure over drift/ref-change-blocked - docs/arch/12-skills-system.md: new Project Lock File section covering rollout gating, schema, install/uninstall hooks, sync, upgrade, and the exit-code contract - CLI E2E coverage for all four exit codes (0/2/4, plus --yes) This is the last PR of Stack 1 (lock file + sync + upgrade) for RFC THV-0080. Stack 2 (Sigstore signing/verification) builds on top once this stack is fully merged; TOOLHIVE_SKILLS_LOCK_ENABLED stays off by default until that full v1 lands. Part of the production stack for #5715 (RFC THV-0080). Stack: 6/6.

RFC THV-0080's sync and upgrade commands operate over a project's whole lock file rather than a single named skill, so they need their own service boundary (SkillLockService) instead of extending SkillService. Installed skills also need a durable "is this tracked in the lock file" marker so sync --prune can distinguish lock-managed installs from ones a user manages by hand. - SkillLockService{Sync,Upgrade} interface with generated mock - Sync/Upgrade option and result types, RFC's typed failure reasons - installed_skills.managed column (migration 003) Part of the production stack for #5715 (RFC THV-0080). Stack: 2/6. No consumers yet — skillsvc wiring lands in PR3.

RFC THV-0080 requires every project-scoped skill install to be pinned in toolhive.lock.yaml, including transitively materialized toolhive.requires dependencies, with a fail-hard guarantee so a lock-write failure never leaves an install silently unpinned. The whole feature stays inert on main until the full RFC v1 (this lock-file stack plus the later Sigstore stack) has landed — gated behind TOOLHIVE_SKILLS_LOCK_ENABLED, following the existing TOOLHIVE_DEV precedent for staged rollouts. Each PR in the stack is independently mergeable without exposing partial behavior. - Install hooks into the single choke point (installAndRegister): compute contentDigest, upsert the lock entry, roll back the DB record if the lock write fails - toolhive.requires dependencies materialize recursively (visited set guards cycles, skills.MaxDependencies bounds the whole tree), with RequiredBy merged across shared dependencies - Uninstall cascades to orphaned, non-explicit dependencies via Lockfile.RemoveParentFromRequiredBy, itself cycle-safe - E2E coverage for the real HTTP API + thv serve subprocess path Part of the production stack for #5715 (RFC THV-0080). Stack: 3/6.

RFC THV-0080 needs a way to restore a project's pinned skill set on any machine (a fresh clone, CI, a teammate's laptop) and to verify on-disk content still matches the lock file without installing anything. This adds the full sync vertical slice: service logic, HTTP endpoint, client, and CLI command. - Sync reconciles installed skills against toolhive.lock.yaml: missing/drifted entries reinstall at the pinned reference (never re-resolving source, see buildPinnedReference), --check reports drift read-only, --adopt/--prune handle unlocked installs - Fixes a real bug this surfaced: reinstalling at an unchanged pinned digest hit both the OCI and git install paths' "same digest means content is already correct" fast path, silently skipping re-extraction — exactly the drift sync exists to repair. A new internal SyncRestore option bypasses that fast path. - POST /api/v1beta/skills/sync via a narrow skillSyncer interface (pkg/api/v1), so this PR doesn't need to ship an Upgrade stub - `thv skill sync` CLI (--check/--adopt/--prune/--clients/ --project-root, auto-detected via .git when omitted) Part of the production stack for #5715 (RFC THV-0080). Stack: 4/6.

RFC THV-0080 needs a controlled way to pull newer content for skills pinned by source, without silently drifting the lock file on every install. This adds the full upgrade vertical slice: service logic, HTTP endpoint, client, and CLI command. - Upgrade re-resolves each targeted entry's Source (via the same git/OCI/registry dispatch order Install uses, stopping short of extraction — resolveLatestState) and installs newer content when the digest changed; Source itself is never rewritten - Entries pinned to an immutable reference (an OCI digest or full git commit hash) report not-upgradable — there's nothing newer to resolve to - --preview reports what would change without persisting it; --allow-ref-change permits the resolved reference itself changing; --fail-on-changes gives CI a freshness gate - var _ skills.SkillLockService = (*service)(nil) lands here now that both Sync and Upgrade exist; SkillsRouter widens from the narrow skillSyncer interface (PR4) to the full interface - `thv skill upgrade [name...]` CLI Part of the production stack for #5715 (RFC THV-0080). Stack: 5/6.