Add project-level skills lock file schema package

[samuv]

Summary

• Why: RFC THV-0080 (merged) proposes a project-level toolhive.lock.yaml that pins the exact content of every project-scoped skill install, giving skills the reproducibility guarantee package-lock.json/Cargo.lock/go.sum provide elsewhere. This is the first PR of a stack productionizing the validated POC in Add project-level skills lock file (POC) #5715 into reviewable, independently-tested changes.
• What: Adds pkg/skills/lockfile, a new leaf package with no callers yet: the lock file schema (Entry, Lockfile), a deterministic content-hash algorithm (ContentDigest) used for on-disk integrity verification, and load/save/upsert/remove operations. All filesystem access is confined to the validated project root via os.Root (OS-enforced containment), not string path validation alone — this avoids the CodeQL "uncontrolled data used in path expression" findings that were flagged on the POC.

Part of the production stack for #5715 (RFC THV-0080). Stack: 1/6 — lockfile → lock-service → install-hooks → sync → upgrade → cli-exitcodes. This PR has no consuming callers; wiring into skillsvc.Install lands in PR3 of the stack.

Type of change

• New feature

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

Table-driven tests cover round-trip load/save, unknown-version hard error, requiredBy cross-reference validation, golden contentDigest vectors (freezing the algorithm), a goroutine-hammer concurrency test against UpsertEntry, and traversal/escape rejection. 89.9% statement coverage.

Does this introduce a user-facing change?

No — this package has no callers yet.

Special notes for reviewers

• Entry deliberately omits provenance/unsigned (Sigstore fields) — those land in PR7 of the Sigstore stack, once Stack 1 (this stack) merges.
• The whole feature (this PR through the final Sigstore PR) stays inert on main behind an env-var gate introduced in PR3, per the incremental-rollout plan — see that PR's description.
• Golden vectors in contentdigest_test.go freeze the dirhash algorithm; treat any future change to it as a lock-file format break, not a routine test update.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 85.17110% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.82%. Comparing base (3dee7d6) to head (141d980).

Files with missing lines	Patch %	Lines
pkg/skills/lockfile/lockfile.go	76.72%	14 Missing and 13 partials ⚠️
pkg/skills/lockfile/contentdigest.go	86.66%	3 Missing and 3 partials ⚠️
pkg/skills/lockfile/validation.go	94.11%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5892      +/-   ##
==========================================
+ Coverage   71.77%   71.82%   +0.04%     
==========================================
  Files         702      705       +3     
  Lines       71901    72164     +263     
==========================================
+ Hits        51608    51829     +221     
- Misses      16593    16619      +26     
- Partials     3700     3716      +16     

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

Tracked in #5899 (RFC THV-0080 stack).

[JAORMX]

🤖 Automated panel review — PR 1/6 (lockfile schema)

Multi-agent review (correctness / security / architecture lenses), each finding adversarially verified against the diff. Advisory — worth a human pass. The whole stack is gated behind TOOLHIVE_SKILLS_LOCK_ENABLED (off by default), so none of these are live regressions.

🟠 Major (forward-compat — flagged here because this is the schema PR)

• The v1 schema bakes in a hazard for the deferred Sigstore fields. Load/Save unmarshal into fixed Entry/Lockfile structs with no ,inline catch-all / KnownFields, and validateLockfile hard-fails any version != 1. When the Sigstore stack adds provenance, it's lose-lose: keep version: 1 and add omitempty fields → an older binary from this stack silently strips the signing metadata on the next Save (a teammate commits an unsigned lock); or bump to version: 2 → this stack's binary hard-fails reading the file for the whole mixed-version team. Reserve/round-trip unknown fields (inline map) or define the version-skew contract now, while the schema is new.

🟡 Minor

• ResolvedReference is completely unvalidated (validation.go:57). It's the exact field sync fetches from without re-resolving source. In an unauthenticated, hand-editable YAML (Sigstore deferred), a lock-diff edit that swaps only resolvedReference — leaving source/digest plausible — steers the fetch origin (attacker registry → cred exfil; or an internal http://169.254.169.254/… SSRF target). Constrain scheme/host/control-chars/length in the schema package.
• Exported Save uses a fixed temp filename and takes no lock (lockfile.go:~210). Only Update/UpsertEntry/RemoveEntry acquire WithFileLock; the package's own tests call lf.Save directly. Two concurrent direct Saves both write .toolhive.lock.tmp → interleave/truncate, promote a half-written temp, or the post-rename Remove deletes the other's temp. The comment's "already serialized by the caller's file lock" is false for any direct caller — use a random suffix or lock inside Save.
• contentDigest ignores file mode and skips symlinks/non-regular files (contentdigest.go). A pure chmod (exec bit flip) or symlink swap is invisible to sync --check. Precedented by go.sum and partly covered by the source Digest pin, so minor — but a real limit worth noting for the Sigstore/verify stack. Also contentDigest is optional in the schema, so an entry can be pinned with no on-disk integrity primitive at all.
• No requiredBy cycle detection at load (validation.go:41). The install-time Visited guard prevents this via normal installs, but a hand-edited/merge-resolved lock with a mutual-requiredBy ring of non-explicit entries passes validation and can never be cascade-removed (each keeps the other's RequiredBy non-empty). Load-time validation is the right place to reject it.

🟢 Verified praise

• The control-char/NUL/newline rejection in ContentDigest paths with an explicit forged-collision test (TestContentDigestRejectsForgedPathCollision) is a genuinely careful serialization-ambiguity defense, and the frozen golden vectors are the right call.

[JAORMX]

🤖 Automated panel review — follow-up on 0a45fce → approving

The "Harden lock file schema against skew, forgery, and races" commit addresses every finding from the earlier review. Verified each fix against the code, not just the commit message:

• ✅ Forward-compat (major): Entry.Extra/Lockfile.Extra as yaml:",inline" maps round-trip unknown keys through Load→Save, so an older binary no longer strips the deferred Sigstore provenance fields — and the comment correctly notes that a freshly-rewritten entry drops its stale extras. This is the right resolution of the schema-skew hazard.
• ✅ resolvedReference validation (minor): validateResolvedReference bounds length, rejects whitespace/non-graphic runes, and requires a strict git:// or name.ParseReference(…, StrictValidation) OCI ref — rejecting URL-shaped SSRF targets like http://169.254.169.254/…. Exactly the syntactic guard requested.
• ✅ Save race (minor): per-call crypto/rand temp suffix; concurrent direct Saves can no longer promote or delete each other's half-written file, and the doc comment is now honest.
• ✅ requiredBy cycle detection (minor): findRequiredByCycle (3-color DFS) rejects hand-edited mutual-requires rings at validation.
• ✅ contentDigest mode/symlink (minor): documented as a known limitation — appropriate here; the real enforcement belongs to the verification (Sigstore) stack.

Approving. One trivial non-blocking nit for a future touch: there's a stale leftover comment line above tmpFileName() in lockfile.go (// tmpFileName is the temporary file Save writes before renaming it into) dangling above the new doc comment — safe to delete.

Note this approval is for PR 1/6 on its own merits; the rest of the stack still has open blocker/major items in #5894–#5897.

[samuv]

Thanks for the thorough panel review — every finding was verified against the code and addressed. Fixed in the latest push:

• Forward-compat hazard (major): Entry and Lockfile now carry inline catch-all maps, so unknown fields (e.g. the future Sigstore provenance block under version 1) round-trip through an older binary's Load→Save instead of being stripped. Frozen by TestUnknownFieldsSurviveLoadModifySave. Version negotiation beyond that is deliberately left for the Sigstore stack to define — this keeps the door open without pre-committing a skew contract.
• ResolvedReference unvalidated: now syntactically validated in the schema package — bounded length, no control/non-graphic characters or whitespace, and it must parse as a git:// reference or a strict OCI reference (explicit tag or digest, matching what install actually records via qualifiedOCIRef). Strict parsing also rejects URL-shaped strings like http://169.254.169.254/… outright.
• Save fixed temp filename / no lock: per-call random suffix now; the false "already serialized by the caller's file lock" comment is gone and the doc comment states plainly that direct Save does not serialize read-modify-write.
• requiredBy cycle detection at load: validateLockfile now runs a DFS over the requiredBy graph and hard-fails naming the cycle — and since Save validates too, one can no longer be written.
• contentDigest mode/symlink limitation: documented on ContentDigest as an accepted, go.sum-style limitation; the algorithm is frozen by golden vectors, and file-set provenance is the Sigstore layer's job.