Skip to content

Add project-level skills lock file schema package#5892

Open
samuv wants to merge 2 commits into
mainfrom
skills-lock/01-lockfile
Open

Add project-level skills lock file schema package#5892
samuv wants to merge 2 commits into
mainfrom
skills-lock/01-lockfile

Conversation

@samuv

@samuv samuv commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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

RFC THV-0080 pins every project-scoped skill install to a
committed toolhive.lock.yaml so teams get reproducible installs
across machines, the same guarantee package-lock.json/Cargo.lock
provide elsewhere. This package defines the v1 schema (entries,
content digest, requiredBy) with no callers yet; skillsvc wiring
follows in a later PR of the stack.

Access is confined to the validated project root via os.Root
(OS-enforced containment), not string validation alone.
@samuv
samuv requested a review from JAORMX as a code owner July 21, 2026 13:51
@github-actions github-actions Bot added the size/XL Extra large PR: 1000+ lines changed label Jul 21, 2026
@samuv samuv self-assigned this Jul 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.90244% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.61%. Comparing base (fbf2f6d) to head (4997552).

Files with missing lines Patch % Lines
pkg/skills/lockfile/lockfile.go 78.70% 12 Missing and 11 partials ⚠️
pkg/skills/lockfile/contentdigest.go 86.66% 3 Missing and 3 partials ⚠️
pkg/skills/lockfile/validation.go 92.30% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5892      +/-   ##
==========================================
- Coverage   71.63%   71.61%   -0.02%     
==========================================
  Files         698      701       +3     
  Lines       71541    71746     +205     
==========================================
+ Hits        51245    51383     +138     
- Misses      16598    16668      +70     
+ Partials     3698     3695       -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.

Path validation rejected traversal but not control characters, so a
crafted Path containing an embedded NUL and newline could reproduce
the byte stream of an unrelated multi-file tree and collide with its
digest. Save also never validated before writing, so a caller bug
could persist an invalid lock file with no recovery through the
package API. Also normalize repeated "./" prefixes idempotently.
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jul 21, 2026
@samuv

samuv commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant