Skip to content

Re-verify stored plugin signatures during sync - #6399

Merged
samuv merged 2 commits into
mainfrom
plugins-sig/08-sync-reverify
Aug 26, 2026
Merged

samuv merged 2 commits into
mainfrom
plugins-sig/08-sync-reverify

Conversation

@samuv

@samuv samuv commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Sync is where a lock file becomes an enforcement mechanism: it is what CI runs to assert that what is installed is what was pinned. #6397 made install-time verification record a signer identity and persist the Sigstore bundle on the install record, but nothing re-checked either afterwards — an entry whose stored bundle had been tampered with, swapped, or lost still reported as up to date, so the CI gate covered content drift but not signature drift.

Commit 1 — re-verify stored signatures during sync

  • Sync re-verifies each managed entry's stored bundle against the identity its lock entry records, entirely offline via the embedded trust root — no registry access, so --check stays usable in a sandboxed CI runner. A failed re-verification is drift: --check reports it and exits 2, apply mode reinstalls from the pinned reference, where Verify plugin signatures at install time #6397's install-time verification enforces the locked identity and heals the stored bundle.
  • An OCI entry recording a signer identity with no stored bundle fails closed rather than passing — a recorded identity with nothing backing it cannot be verified. Git entries store no bundle by design (their signature lives on the commit and is re-verified when content is re-resolved) and entries recorded unsigned: true have nothing to verify, so both skip the offline check.
  • Adoption is a trust decision, so --adopt back-fills the entry's provenance from the stored bundle when one exists. With no bundle, adopting is the same trust decision as an unsigned install: it now requires the explicit --allow-unsigned exception and records unsigned: true. Without the flag it fails typed as unsigned-rejected. This tightens --adopt, which previously wrote entries with no trust state at all.
  • Adds --allow-unsigned to thv ai-plugin sync and threads the field through the plugins API DTO and the Go HTTP client DTO. plugins.SyncOptions already carried the field (it aliases skills.SyncOptions); nothing was passing it.
  • Signature failures classify through the existing errors.Is sentinel mapping to the typed reasons established in Stack 1 (signature-invalid, signer-mismatch, unsigned-rejected). Exit-code semantics are unchanged.

Commit 2 — bound stored signature material (closes the #6396 review thread)

@jhrozek flagged on #6396 that nothing caps what gets written into the sigstore_bundle column, and that the git Payload/Signature that PR started carrying are equally unbounded — commit messages have no length limit, so a hostile repo or registry can push a multi-MB blob into SQLite on every install. #6397 deferred the fix here, because this is the PR where sync starts reading those bytes back on every run.

  • All three are capped at capture time in verify.go, before the bytes reach InstallOptions and the store, at 1 MiB — orders of magnitude above a real keyless bundle or gitsign signature (a few KB: certificate chain, signature, inclusion proof).
  • Over-limit material is rejected (422), never truncated: a truncated bundle would fail to verify later and be indistinguishable from tampering. The git payload and signature are checked before the verifier is consulted, so a hostile repo cannot spend our CPU either.
  • The other Store plugin sigstore bundles, carry git signature #6396 comment — nothing binds a stored bundle to the digest it covers — is answered by the two PRs together: capture derives the pair from a single VerifyOCI/VerifyGit call over that digest (Verify plugin signatures at install time #6397), and commit 1 here re-verifies the stored bundle against the entry's digest on every sync, so a mismatched pair now surfaces as drift instead of sitting unnoticed.

Part of #6300. Mirrors the skills counterpart #6131. Stacked on #6397 — this PR targets plugins-sig/07-install-verify.

Type of change

  • New feature

Test plan

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

Sync re-verification tests mirror the skills suite:

  • a stored bundle that no longer verifies reports as drift in --check and is reinstalled at the pin in apply mode;
  • verifyStoredSignature table: unsigned and no-provenance entries skip the check, provenance-without-bundle fails closed for OCI and passes for git, a stored bundle delegates to VerifyBundleOffline and its failure propagates;
  • a missing bundle classifies as signature-invalid, not unknown;
  • adopt back-fills provenance from a stored bundle without needing the flag, rejects an unverifiable bundle rather than silently falling through to unsigned, and without a bundle fails typed unsigned-rejected then records unsigned: true with --allow-unsigned;
  • round-trip tests pin the flag at both DTO boundaries (API request → SyncOptions, client → wire) and the CLI flag's binding to the variable sync passes to the service.

Size-cap tests: a table over all four rejection paths (OCI bundle, git bundle, oversized commit payload, oversized commit signature) asserting 422 and validation-rejected, where the payload/signature cases prove the verifier is never called; plus an end-to-end case showing an oversized bundle leaves neither a lock entry nor a DB record.

task docs output verified with ./cmd/help/verify.sh (exit 0, no diffs).

Two pre-existing conditions on this branch and its base, neither introduced here:

  • TestMCPGoClientInitializeAndPing (pkg/transport/proxy/streamable) fails locally — its server binds a hardcoded 127.0.0.1:8096, which is in use on this machine.
  • task lint-fix was run with golangci-lint v2.12.2, the version CI pins in Pin golangci-lint to avoid nilness panic on main #6393 (v2.13.0 panics in nilness on sentry-go). It reports one gci finding in pkg/authserver/runner/embeddedauthserver.go:249, a file this PR does not touch; the repo is otherwise clean.

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File Change
pkg/plugins/pluginsvc/sync.go Offline re-verification in syncLockedEntry; adoption trust decision in adoptLocked
pkg/plugins/pluginsvc/verify.go 1 MiB ceiling on bundle, commit payload and gitsign signature at capture
cmd/thv/app/ai_plugin_sync.go --allow-unsigned flag
pkg/api/v1/plugins_types.go, pkg/api/v1/plugins.go allow_unsigned on the sync request DTO and handler
pkg/plugins/client/dto.go, pkg/plugins/client/client.go allow_unsigned on the client sync DTO
docs/ Regenerated CLI docs and swagger

Does this introduce a user-facing change?

Yes. thv ai-plugin sync --check now also fails when a plugin's stored signature no longer verifies against the identity pinned in the lock file, not just on content drift. thv ai-plugin sync --adopt now requires --allow-unsigned for installs with no stored Sigstore bundle, and records those entries as unsigned: true; adoptions with a stored bundle back-fill the signer identity into the lock entry instead of leaving trust state empty. An install whose signature material exceeds 1 MiB is rejected.

Special notes for reviewers

  • The OCI/git split keys off whether entry.Digest contains a : — the same discriminator the skills implementation uses. Git digests are bare commit hashes, OCI digests are sha256:…. Local-store pins share the OCI digest shape and store no bundle, but cannot reach the fail-closed branch (a local install is always an unsigned trust decision, so its entry returns at the Unsigned check); verifyStoredSignature's doc comment reasons through that case and through a hand-edited lock entry.
  • In adoptLocked, the restorable-pin check deliberately still runs before the trust decision, so adopting a bare local-store tag keeps reporting validation-rejected rather than changing to unsigned-rejected.
  • ResultFromBundle returning a nil provenance (a key-signed bundle carries no certificate identity) falls through to the unsigned path rather than recording an empty provenance block.
  • The equivalent unbounded-blob exposure exists on the skills side, which has no cap today. Out of scope here (this stack does not touch pkg/skills) but worth a follow-up.
  • Follow-ups in this stack: the signer-change guard on the upgrade path is PR9; push signing and gate removal are PR10.

Generated with Claude Code

@github-actions github-actions Bot added the size/M Medium PR: 300-599 lines changed label Aug 20, 2026
@samuv samuv self-assigned this Aug 20, 2026
@codecov

codecov Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.88%. Comparing base (9a9686c) to head (b10e81d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6399      +/-   ##
==========================================
+ Coverage   77.82%   77.88%   +0.05%     
==========================================
  Files         762      762              
  Lines       73449    73497      +48     
==========================================
+ Hits        57165    57242      +77     
+ Misses      16279    16250      -29     
  Partials        5        5              

☔ 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 force-pushed the plugins-sig/08-sync-reverify branch from 91ff461 to f6dac63 Compare August 25, 2026 12:59
@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 Aug 25, 2026
@samuv
samuv force-pushed the plugins-sig/08-sync-reverify branch from f6dac63 to 32bdb3d Compare August 25, 2026 13:11
@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 Aug 25, 2026
@samuv
samuv force-pushed the plugins-sig/08-sync-reverify branch from 32bdb3d to b1d8e70 Compare August 25, 2026 13:34
@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 Aug 25, 2026
@samuv
samuv force-pushed the plugins-sig/08-sync-reverify branch from b1d8e70 to 5215d7b Compare August 25, 2026 13:34
@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 Aug 25, 2026
@samuv
samuv force-pushed the plugins-sig/08-sync-reverify branch from 5215d7b to f86c395 Compare August 25, 2026 13:38
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/M Medium PR: 300-599 lines changed size/L Large PR: 600-999 lines changed labels Aug 25, 2026
Base automatically changed from plugins-sig/07-install-verify to main August 26, 2026 09:30
samuv and others added 2 commits August 26, 2026 11:30
Plugin 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. 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 signature lives on the commit and is 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 and Go client DTOs), recorded as unsigned in the
entry.

Part of #6300. Mirrors #6131 for skills.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Samuele Verzi <samu@stacklok.com>
The Sigstore bundle persisted with an install and the commit payload and
gitsign signature a git install verifies are all attacker-influenced and
unbounded at their source: a registry serves whatever bundle it likes,
and git imposes no length limit on a commit message. Without a ceiling a
hostile plugin source can push a multi-MB blob into SQLite on every
install, which every subsequent sync then reads back for offline
re-verification.

Cap all three at capture time in verify.go, before the bytes reach
InstallOptions and the store, and reject rather than truncate — a
truncated bundle would fail to verify later and be indistinguishable
from tampering. The git payload and signature are checked before the
verifier is consulted so a hostile repo cannot spend our CPU either.

Closes the size-ceiling review thread on #6396, deferred to this PR.

Part of #6300.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Samuele Verzi <samu@stacklok.com>
@samuv
samuv force-pushed the plugins-sig/08-sync-reverify branch from f86c395 to b10e81d Compare August 26, 2026 09:31
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Aug 26, 2026
@samuv
samuv force-pushed the plugins-sig/08-sync-reverify branch from b10e81d to 0bac2f2 Compare August 26, 2026 09:40
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Aug 26, 2026
@samuv
samuv force-pushed the plugins-sig/08-sync-reverify branch from 0bac2f2 to b10e81d Compare August 26, 2026 09:41
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Aug 26, 2026
@samuv
samuv merged commit 789f2fb into main Aug 26, 2026
88 checks passed
@samuv
samuv deleted the plugins-sig/08-sync-reverify branch August 26, 2026 13:21
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
2 tasks
@samuv samuv mentioned this pull request Sep 1, 2026
29 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants