Skip to content

Enforce catalog provenance for plugin installs - #6653

Merged
samuv merged 2 commits into
mainfrom
t3code/bump-toolhive-core
Sep 15, 2026
Merged

samuv merged 2 commits into
mainfrom
t3code/bump-toolhive-core

Conversation

@samuv

@samuv samuv commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Plugin catalog provenance was not enforced on first project-scoped installs, leaving plugin trust weaker than skill trust. This closes that parity gap by carrying catalog provenance from registry search into install-time verification.
  • Supported signer, issuer, repository, ref, and runner constraints are enforced before trust on first use is recorded. Existing lock entries remain authoritative, unsupported constraints fail closed, --allow-unsigned cannot bypass catalog policy, and certificate-shaped policies reject --public-key.

Fixes #6643

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting and vet (task lint)
  • Manual testing

Changes

File Change
pkg/api/server.go Preserve plugin provenance while adapting registry search results.
pkg/api/server_test.go Verify API registry mapping retains provenance.
pkg/plugins/options.go Add internal catalog provenance to plugin install options.
pkg/plugins/pluginsvc/service.go Add provenance to registry search hits.
pkg/plugins/pluginsvc/install.go Carry resolved catalog constraints to the verification boundary.
pkg/plugins/pluginsvc/verify.go Enforce first-use catalog constraints, lock precedence, unsupported-field rejection, and public-key conflicts.
pkg/plugins/pluginsvc/install_registry_test.go Cover end-to-end propagation, rejection side effects, and user scope.
pkg/plugins/pluginsvc/verify_test.go Cover OCI and Git policy enforcement and negative security cases.
docs/arch/14-plugins-system.md Document the plugin catalog trust model and supported constraints.

Does this introduce a user-facing change?

Yes. A first project-scoped plugin install resolved by registry name must satisfy provenance constraints declared by that catalog entry. Direct-reference and user-scoped installs retain their existing behavior.

Implementation plan

Approved implementation plan
  1. Map the core plugin provenance field into plugin registry search results.
  2. Carry provenance internally through install options without exposing it as caller input.
  3. On true first use, verify supported catalog constraints with catalog expectation semantics.
  4. Preserve every existing lock entry as the higher-precedence trust decision and fail closed for unsupported constraints.
  5. Add parity tests with skills and update the plugin architecture documentation.

Special notes for reviewers

main already depends on toolhive-core v0.0.47, which includes the plugin provenance type released in v0.0.46. This PR therefore contains no dependency bump. Any existing plugin lock entry, including a legacy entry without trust state, takes precedence over catalog policy.

@github-actions github-actions Bot added the size/L Large PR: 600-999 lines changed label Sep 14, 2026
@samuv samuv self-assigned this Sep 14, 2026
@codecov

codecov Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.73913% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.93%. Comparing base (aa913cb) to head (3269d8f).

Files with missing lines Patch % Lines
pkg/plugins/pluginsvc/verify.go 96.66% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6653   +/-   ##
=======================================
  Coverage   78.93%   78.93%           
=======================================
  Files         782      782           
  Lines       78065    78136   +71     
=======================================
+ Hits        61620    61678   +58     
- Misses      16440    16453   +13     
  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.

@JAORMX JAORMX left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran a panel review (security, code-quality, architecture) over this diff. Overall this is a well-structured change with tight, faithful parity to the existing skills trust-on-first-use model, and no exploitable bypass was found — CatalogProvenance isn't reachable from the HTTP API or CLI, --allow-unsigned/--public-key correctly can't override catalog policy, and lock-entry precedence (including legacy entries) is correctly enforced with no path to force a re-first-use.

Two inline findings are blocking (a contradictory --public-key remedy for key-signed artifacts under a catalog constraint, and a grammatically broken error string) — the first is compounded by a test that asserts the current, wrong message, so it won't catch a fix. The rest are inline nice-to-haves (duplicated resolution block, minor consistency/doc gaps, a defense-in-depth suggestion around AllowSignerChange).

Also noting: the size/L label undercounts how contained this is — excluding tests/docs (per the repo's own PR-size convention), production code is ~180 changed lines across 5 files, well within the normal guideline.

Comment thread pkg/plugins/pluginsvc/verify.go Outdated
Comment thread pkg/plugins/pluginsvc/verify.go Outdated
Comment thread pkg/plugins/pluginsvc/verify.go Outdated
Comment thread pkg/plugins/pluginsvc/verify.go Outdated
Comment thread pkg/plugins/pluginsvc/verify.go Outdated
Comment thread pkg/plugins/pluginsvc/verify.go
@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 Sep 14, 2026
@samuv
samuv requested a review from JAORMX September 14, 2026 11:26
Signed-off-by: Samuele Verzi <samu@stacklok.com>
Signed-off-by: Samuele Verzi <samu@stacklok.com>
@samuv
samuv force-pushed the t3code/bump-toolhive-core branch from 21179ee to 3269d8f Compare September 14, 2026 12:25
@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 Sep 14, 2026
@samuv
samuv merged commit a734c16 into main Sep 15, 2026
87 of 89 checks passed
@samuv
samuv deleted the t3code/bump-toolhive-core branch September 15, 2026 07:12
@github-actions github-actions Bot mentioned this pull request Sep 18, 2026
2 tasks
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.

Plugins have no catalog-declared provenance (parity gap with skills)

2 participants