RFC: Project-level skills lock file

[samuv]

Summary

Proposes a project-level lock file (toolhive.lock.yaml) that pins the name, version, source, resolved reference, and digest of every project-scoped skill install, bringing skills the reproducibility guarantees that package-lock.json, Cargo.lock, and go.sum provide elsewhere.

• Problem: thv skill install --scope project leaves no shareable, version-controlled pin, so teammates cloning a repo get whatever the catalog currently serves.
• Lock file: committed at the project root, client-agnostic, entries sorted for stable diffs; designed as the general ToolHive project lock (skills: today, room for plugins: later).
• New commands:
  • thv skill sync — restore the pinned skill set (drift reported distinctly; --check gives CI a npm ci-style gate; --prune removes unmanaged skills).
  • thv skill upgrade — re-resolve each entry's original source and rewrite the pin on digest change; immutable pins (OCI digests, full commit hashes) are never re-resolved.
• Out of scope: manifest/constraint resolution, per-client pinning, dependency graphs, user-scope installs.

POC implementation: stacklok/toolhive#5715

[JAORMX]

Panel review — cross-cutting summary

Conducted a three-axis panel review (spec / standards / domain) with specialist reviewers (secure-code-reviewer, software-architect, library-reuse-reviewer, devex-reviewer, ux-reviewer) plus verified research against go.sum, Cargo.lock, npm, pnpm, and poetry.lock. All factual claims were verified against source code/docs before posting. Inline comments below cover location-specific findings; this comment captures the cross-cutting picture and the non-location-specific ones.

Overall

The RFC is well-researched and consciously cites the right prior art. It correctly avoids several npm mistakes (pins content digests not registry names, refuses to rewrite source, treats --prune as opt-in, hard-errors on unknown schema versions). But it inherits npm's exact supply-chain gaps while claiming parity with go.sum/Cargo.lock — and go.sum/Cargo.lock each close those gaps with an external trust root (GOSUMDB / curated registry) that this design lacks. For skills (instructions an AI executes), the stakes are higher than inert code.

Critical findings (inline comments posted)

1. No binding between source and digest — a committed digest is an assertion, not a verification. A tampered PR can pin any digest that passes format/name validation, and sync faithfully installs malicious AI instructions. go.sum has GOSUMDB (Trillian Merkle transparency log, Google-operated); Cargo.lock has crates.io's curated content-addressed registry. This RFC has neither.

2. SHA-1 git commit hashes accepted for a new 2026 supply-chain system — all three comparators (go.sum SHA-256, Cargo SHA-256, npm SHA-512) avoid SHA-1 for content integrity. The "inherited from git" defense doesn't hold for a new system when git offers sha256 and no comparator uses SHA-1.

3. Transitive skill dependencies (toolhive.requires) not locked — two machines with identical locks can install different transitive skill content. Every mature ecosystem (npm v2/v3, Cargo.lock) locks the full transitive graph. This is the canonical npm reproducibility mistake.

4. sync/--check compares against SQLite, not re-hashed content — npm ci re-hashes files against the lock; go mod verify re-hashes the module cache. This RFC's CI gate can pass on tampered installs. For prompt-instruction artifacts, undetected on-disk tamper is high-impact.

5. sync is a one-command prompt-injection amplifier with no pre-install gate — the RFC's own threat model names this, but the mitigation is post-install reporting, not pre-install gating. For AI-instruction artifacts, reporting after install is too late.

High findings (inline comments posted)

• source re-resolution on upgrade is a catalog-redirect/typosquat vector (TOCTOU) — same shape as npm's resolved URL malleability.
• No signature/transparency log/provenance in v1 — "future Sigstore" is a forward pointer, not a v1 control. THV-0030:472 commits to Sigstore; this RFC never reconciles.
• sync --check ≠ cargo --locked — the analogy oversells the guarantee. There's no command that catches a stale lock (dev bumped catalog, forgot to commit lock update).

Medium findings (inline comments posted)

• SkillService Go interface break is real — verified: exported interface with implementers in toolhive-git-skills and stacklok-enterprise-platform. "If any exist" should be "yes." §Compatibility's "fully backward compatible" contradicts §API Changes.
• Best-effort lock write → silent stale lock / silent downgrade on sync — Cargo makes lock write atomic with install (fails the build). This RFC's "warn and continue" is strictly weaker.
• --dry-run is a misnomer — fetches to cache/clones. cargo/npm/git --dry-run are side-effect-free.
• --prune conflates "never-managed" with "removed-from-lock-by-tampered-PR" — both collapse to unmanaged; a habitual sync --prune deletes legitimate skills indistinguishably.
• projectRoot validation underspecified at the trust boundary (CWE-22) — "validated the same way existing paths are" is vague for the one new API-body trust boundary.
• failed collapses unrecoverable/retryable/policy-rejected causes — callers can't branch; exit codes undocumented.
• General project lock defined under a skills RFC — THV-0077 has zero mention of toolhive.lock.yaml; this RFC silently commits it to a contract. Layering violation.
• Lockfile discovery breaks on worktrees — .git is a file, not a dir, in worktrees. Use git rev-parse semantics.
• No --adopt path — first-run treats existing installs as unmanaged (the same flag --prune deletes).

The top three highest-leverage fixes (per the operator's stated priority: avoid npm, adopt Go/Rust rigor)

1. Lock transitive dependencies (write lock entries for materialized toolhive.requires skills with a requiredBy provenance marker — Cargo.lock records, doesn't resolve).
2. Make sync re-hash content + always-on digest verification on every install path including cache hits (go mod verify / npm ci rigor).
3. Add a publisher/provenance field with change-detection on upgrade — a cheap, local mitigation for the event-stream/ua-parser-js class ("digest correct, content malicious") that needs no Sigstore in v1, while explicitly naming GOSUMDB-style transparency logging + Sigstore (THV-0030) as the v2 trust layer.

Verified comparators (sources cited in inline comments)

Mechanism	Go	Cargo	npm	RFC-0080
Hash algorithm	SHA-256 (h1:)	SHA-256 (tarball)	SHA-512 (SRI)	SHA-256 (OCI) or SHA-1 (git) ❌
Transitive deps locked	✅	✅	✅ (v2/v3)	❌
Re-hash on verify	✅ (go mod verify)	✅ (registry check)	✅ (npm ci)	❌ (SQLite only)
External trust root	✅ GOSUMDB (Trillian)	✅ curated registry	⚠️ opt-in provenance	❌
Lock-write atomic	✅	✅ (fails build)	✅	❌ (best-effort warn)
CI frozen gate	✅ -mod=readonly	✅ --locked/--frozen	✅ npm ci	⚠️ --check (weaker)
Executable code on install	N/A	N/A	⚠️ postinstall runs by default	❌ undefined for skill scripts

Happy to drill into any specific finding or help draft the source→digest binding design.

[samuv]

@JAORMX Thanks for the thorough panel review — I've pushed a revision (0ed7a65) and replied inline to each finding. Cross-cutting summary of what changed:

Finding	Resolution
Unauthenticated trust root	Reframed honestly in §Trust model: v1 trust = PR review of lock diff; digest is an assertion verified against itself, not an external record. v2 milestone for Sigstore + transparency binding. Rejected sync-time re-derivation (defeats pinning) — see Alternative 4.
SHA-1 / content integrity	Added contentDigest: sha256:... dirhash on every entry; digest is checkout pin only.
Transitive deps not locked	Cargo record-not-resolve: lock entries with requiredBy: [parent] for materialized toolhive.requires skills.
--check / SQLite-only drift	--check re-hashes on-disk content against contentDigest + file-presence; always-on digest verification on all install paths. Dropped npm-ci/cargo-locked analogy.
Pre-install gate	Interactive sync: pre-flight summary + Install? [y/N] default No; --yes for scripts.
Upgrade TOCTOU	Refuse reference changes without --allow-ref-change; catalog pinning deferred to v2.
Stale lock CI signal	upgrade --preview --fail-on-changes as optional freshness gate (distinct from --check).
Lock write atomicity	Project-scope install exits non-zero on lock write failure.
Prune hardening	managed SQLite marker; prune only removed-from-lock; split report categories.
Go interface break	New SkillLockService; SkillService unchanged; public PreserveSource option.
Layering / THV-0041	RFC owns skills: only; sibling keys are a contract proposal; reconciled with THV-0041 dual-store model.
Bundle (preview, projectRoot, exit codes, worktrees, --adopt)	All incorporated — see inline replies.

Happy to iterate further, especially on v2 trust-layer scope or POC alignment in toolhive#5715.

[JAORMX]

I know you're explicitly calling out a v1 and v2, but can we just do v1? I'd really want to ship this with signing to begin with.

[samuv]

I was trying to defer this to a future update because I don't know much about Sigstore, but I'll try to do a full deep dive on it

[samuv]

@JAORMX Yes — pushed cd35106 folding Sigstore into v1. The v2 trust-layer milestone is gone.

What ships in v1:

• Sign on publish: thv skill push keyless Sigstore signing (Fulcio + Rekor, OCI referrer); --key for key-based setups. Catalog skills signed in CI (coordinated workstream).
• Verify on consume: install/sync/upgrade verify signatures by default. Unsigned artifacts rejected unless --allow-unsigned (recorded as unsigned: true in the lock, visible in diffs).
• Identity pinning: each lock entry carries a provenance block (signerIdentity, certIssuer, optional repositoryUri/sigstoreUrl) — mirrors the existing registry provenance shape. Catalog-first on first install, TOFU for direct refs. Upgrade refuses signer changes without --allow-signer-change.
• Git sources: gitsign verification on pinned commits (unsigned git still needs --allow-unsigned).
• Offline CI: Sigstore bundles stored with install state; sync --check re-hashes contentDigest and re-verifies bundle + locked identity offline (hermetic, no Rekor round-trip).
• Implementation: extends toolhive-core container/verifier (sigstore-go, same stack as MCP image verification).

This directly closes the unauthenticated-trust-root finding via signature + pinned publisher identity + Rekor transparency, rather than the earlier "PR review only until v2" framing. See §Trust model, §Signing on publish, §Verification on consume, and Phase 5 in the Implementation Plan.

Happy to walk through any of the enforcement edge cases (--allow-unsigned, catalog-first vs TOFU, gitsign scope).