Skip to content

Re-verify stored signatures offline during sync - #6131

Open
samuv wants to merge 1 commit into
skills-sig/11-install-verifyfrom
skills-sig/12-sync-reverify
Open

Re-verify stored signatures offline during sync#6131
samuv wants to merge 1 commit into
skills-sig/11-install-verifyfrom
skills-sig/12-sync-reverify

Conversation

@samuv

@samuv samuv commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Note

Stack 2 of RFC THV-0080 (tracking issue #5899) — stacked on #6129 (install-time verification). Stack: #6091#6121#6129 ← this.

Summary

thv skill sync now closes the verification loop the install PR opened: the trust state recorded at install time is re-checked on every sync, without network access.

  • Offline re-verification before "current": each managed entry's stored Sigstore bundle is re-verified against the lock file's recorded identity via the embedded trust root, before the entry can count as AlreadyCurrent. A failed re-verification is treated as drift: sync --check reports it (exit 2 — the CI gate covers signatures exactly like content drift), and an apply reinstalls from the pinned reference, where install-time verification enforces the locked identity and, on success, heals the stored bundle. A reinstall that fails verification lands in Failed with the typed signature reason (exit 3).
  • Fail-closed for missing bundles: an OCI entry recording a signer identity whose install record has no stored bundle fails re-verification rather than silently passing. Git entries store no bundle by design (the signature lives on the commit and is re-verified whenever content is re-resolved), so they're exempt from the bundle requirement.
  • Adoption is a trust decision: sync --adopt back-fills provenance from the stored bundle when one exists (post-verification installs); an install with no verifiable trust state requires the new --allow-unsigned flag (threaded through the API as allow_unsigned) and is recorded unsigned: true. Refusals classify as unsigned-rejected.

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
  • Linting passes (task lint-fix, 0 issues)
  • New unit tests: verifyStoredSignature matrix (unsigned/no-provenance skip, missing-bundle fails closed for OCI but not git, delegation and failure propagation asserted against the mock); signature-failure-is-drift flow (check mode reports Drifted, apply reinstalls and heals); adoption without verifiable trust fails with unsigned-rejected and writes nothing, then records unsigned: true with the flag.
  • task docs regenerated (new --allow-unsigned sync flag + API field).

Does this introduce a user-facing change?

Yes, behind the experimental gate: sync --check now fails when a stored signature no longer verifies, and sync --adopt requires --allow-unsigned for skills whose signature state cannot be established.

Special notes for reviewers

  • Tampering with the stored bundle is unit-tested via the mock verifier rather than E2E: an E2E would need to reach into the server's SQLite from the test to corrupt the blob, which crosses the process boundary the E2E suite deliberately respects. The real offline-verification cryptography is covered by toolhive-core's tests and the verifier wrapper's tests down-stack.

Generated with Claude Code

@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.36170% with 5 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (skills-sig/11-install-verify@86f35b1). Learn more about missing BASE report.

Files with missing lines Patch % Lines
pkg/skills/skillsvc/sync.go 85.71% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##             skills-sig/11-install-verify    #6131   +/-   ##
===============================================================
  Coverage                                ?   72.49%           
===============================================================
  Files                                   ?      740           
  Lines                                   ?    76317           
  Branches                                ?        0           
===============================================================
  Hits                                    ?    55327           
  Misses                                  ?    17064           
  Partials                                ?     3926           

☔ 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 samuv self-assigned this Jul 29, 2026
@samuv
samuv force-pushed the skills-sig/12-sync-reverify branch from 3350f67 to b5b92a8 Compare July 29, 2026 07:48
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/S Small PR: 100-299 lines changed labels Jul 29, 2026
Sync now re-verifies each managed entry's stored Sigstore bundle
against the identity recorded in the lock file — entirely offline, via
the embedded trust root — before an entry can count as current
(RFC THV-0080). A failed re-verification is drift: check mode reports
it (the CI gate covers signatures like it covers content drift), and
apply mode reinstalls from the pinned reference, where install-time
verification enforces the locked identity and heals the stored state.
An OCI entry recording a signer identity without a stored bundle fails
closed; git entries store no bundle by design — their signatures are
re-verified when content is re-resolved.

Adoption back-fills provenance from the stored bundle when one exists;
otherwise adopting is the same trust decision as an unsigned install
and now requires the explicit --allow-unsigned exception (new flag on
sync, threaded through the API), recorded as unsigned in the entry.

Part of #5899.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@samuv
samuv force-pushed the skills-sig/12-sync-reverify branch from b5b92a8 to 3a2f05e Compare July 29, 2026 08:09
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant