Add plugin REST API, thv ai-plugin CLI, and HTTP client (Phase 4)

[JAORMX]

Summary

Exposes the frozen PluginService interface (Phases 2–3) over REST, CLI, and HTTP client — Phase 4 of the plugin lifecycle management effort (RFC THV-0077).

This is a "mirror the skills system" change: the PluginService interface was already complete from Phases 2–3; this phase adds the exposure layer.

Naming note: the CLI subcommand is thv ai-plugin (not thv plugin as the RFC/issue say) — a deliberate UX decision so users aren't confused whether these are plugins for ToolHive or plugins for AI tools. They are AI-tool plugins.

Changes

REST API (pkg/api/v1/plugins.go + plugins_types.go)

• 10 routes at /api/v1beta/plugins: list, install, uninstall, info, validate, build, push, builds, builds/remove, content
• Input validation on push/build/validate (empty field → 400), nil-slice guard on list
• ValidatePluginName on path-param endpoints (uninstall, info)
• Swagger annotations on all handlers
• /marketplace route deferred to Phase 5c ([plugins] Phase 5c: marketplace generate + signing (THV-0077) #5530)

HTTP client (pkg/plugins/client/)

• Client implementing plugins.PluginService over HTTP
• NewDefaultClient (env var → server discovery → default URL)
• ErrServerUnreachable sentinel error
• Compile-time interface check: var _ plugins.PluginService = (*Client)(nil)

CLI (cmd/thv/app/ai_plugin*.go)

• 11 cobra command files mirroring skill*.go: install, uninstall, list, info, validate, build, push, builds, builds/remove
• ai-plugin info text output surfaces the executable surface inventory (commands/agents/skills/hooks) separately from declared-but-unmanaged components (mcpServers/lspServers) with a Declared (not managed by ToolHive): annotation — per the issue exit gate
• Help text names the supported clients (Claude Code, Codex) and every leaf subcommand carries the "AI-tool plugin" qualifier

Server wiring (pkg/api/server.go)

• pluginManager field on ServerBuilder and Server
• createDefaultPluginManager block: SQLite plugin store, OCI store, packager, registry client, Claude Code + Codex materialization adapters, group manager
• devModePluginRegistryClient mirrors the skills dev-mode OCI registry dispatch
• Route mount at /api/v1beta/plugins + plugin store cleanup

Security hardening (panel review)

• OCI pull paths (install, content) reject localhost/private-IP registry hosts before Pull — mirrors the git path's validateHost SSRF guard, with a TOOLHIVE_DEV bypass and tests asserting Pull is never called for rejected hosts

Tests

• pkg/api/v1/plugins_test.go — table-driven, all 10 handlers + JSON key assertions + edge cases (empty fields, group filter, nil-slice)
• pkg/plugins/client/client_test.go — all 10 methods via httptest.Server + connection error + URL encoding
• pkg/plugins/pluginsvc/oci_ssrf_test.go — OCI registry host validation + dispatch-path coverage

Large PR Justification

This PR is large because it is a templated exposure layer — it mirrors the existing skills system (API + CLI + client) for plugins in a single atomic change. The lines break down as:

• ~3600 lines of generated/swagger docs (docs/server/docs.go, swagger.json, swagger.yaml) — auto-generated by task docs, cannot be split or hand-edited.
• ~1200 lines of CLI doc files (docs/cli/thv_ai-plugin*.md) — auto-generated by task docs.
• ~1000 lines of tests (plugins_test.go + client_test.go + oci_ssrf_test.go) — test-only, exempt per contributing guidelines.
• ~1000 lines of new Go production code across 20 files — this is the actual exposure layer (API handlers, HTTP client, cobra commands, server wiring). These files are structurally identical to their skill counterparts; splitting them would produce non-functional partials (e.g., CLI commands without their server route, or an API handler without the client). The issue ([plugins] Phase 4: REST API + thv plugin CLI + content preview (THV-0077) #5528) explicitly scopes this as one deliverable.

The change is purely additive — no existing files are modified in their logic (only server.go and commands.go get new wiring). Per .claude/rules/pr-creation.md: "Large PRs acceptable for: generated code, dependency updates, docs-only, test-only changes."

Related Issues

Part of #5525 (Epic: Plugin lifecycle management)
Closes #5528 (Phase 4: REST API + thv ai-plugin CLI + content preview)
Refs RFC PR stacklok/toolhive-rfcs#77

Testing

• task build ✅
• task test ✅ (all unit tests pass)
• task lint-fix ✅ (0 issues)
• task docs ✅ (CLI docs + swagger regenerated)
• Multi-agent panel review ✅ (spec, standards, security, architecture, duplication, library-reuse, UX, DevEx) — findings addressed in the top commits

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[codecov]

Codecov Report

❌ Patch coverage is 82.33945% with 77 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.51%. Comparing base (333d71d) to head (0035a8b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/api/server.go	54.08%	34 Missing and 11 partials ⚠️
pkg/plugins/client/client.go	82.66%	21 Missing and 5 partials ⚠️
pkg/api/v1/plugins.go	96.38%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5782      +/-   ##
==========================================
+ Coverage   72.48%   72.51%   +0.03%     
==========================================
  Files         733      735       +2     
  Lines       75842    76255     +413     
==========================================
+ Hits        54971    55294     +323     
- Misses      16960    17052      +92     
+ Partials     3911     3909       -2     

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

Large PR justification has been provided. Thank you!

[github-actions]

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

[JAORMX]

CodeQL false positives

The CodeQL check reports 20 "Uncontrolled data used in path expression" (go/path-injection) alerts. These are false positives — the same pattern that has been dismissed 49 times previously in this repo (see the dismissed alerts list).

Why they're false positives

The taint flow is: REST API request → PluginService.Install → MaterializationAdapter.Materialize → ClientManager.GetPluginPath → filesystem operations (filepath.Join, os.MkdirAll, os.ReadFile, etc.).

CodeQL can't see through the validation barrier, but the input is sanitized at multiple layers:

1. API layer (pkg/api/v1/plugins.go): ValidatePluginName on uninstall/info path params
2. Service layer (pkg/plugins/pluginsvc/install.go:67): ValidatePluginName on plain-name installs
3. Client manager (pkg/client/plugins.go:55): ValidateSkillName in GetPluginPath itself
4. OCI/git install paths: the name comes from the artifact's manifest (not user input), validated at install_oci.go:63 and install_git.go:60

ValidatePluginName delegates to skills.ValidateSkillName which enforces ^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$ — rejecting .., /, and any path traversal characters.

What's affected

All 20 alerts are on pre-existing files from Phases 2-3 (not files changed by this PR):

• pkg/plugins/adapters/claudecode.go (9 alerts)
• pkg/plugins/adapters/codex.go (4 alerts)
• pkg/plugins/parser.go (2 alerts)
• pkg/skills/installer.go (3 alerts)
• pkg/fileutils/atomic.go (2 alerts)

This PR adds new REST API call paths that reach these existing functions, which is why CodeQL reports them as "new."

Resolution

Following the established repo pattern (49 previously dismissed go/path-injection false positives), these alerts will be dismissed as false positives after merge. CodeQL is not a required check (branch protection only requires CRD Schema Compatibility).

[JAORMX]

CodeQL go/path-injection alerts (20) — assessed in depth, dismissed as false positives

CodeQL flagged 20 "Uncontrolled data used in path expression" alerts on this branch. Each was traced source → sink by dedicated security review. No true positives found. Summary of the neutralizing validations:

pkg/plugins/adapters/claudecode.go (9 alerts: 196, 227, 246, 250, 265, 318, 359, 366, 439)
Every sink path derives from GetPluginPath (name validated by ValidateSkillName — strict kebab-case regex that cannot express .., /, or \) or from projectRoot validated by ValidateProjectRoot (absolute, no .. segments, EvalSymlinks equality = no symlinked parents, must be a real git-repo dir). Remaining components are fixed strings (.claude, settings.json, .claude-plugin/marketplace.json).

pkg/plugins/adapters/codex.go (4 alerts: 196, 237, 257, 270)
Same chain: validated projectRoot + fixed .agents/plugins/marketplace.json suffix, or trusted HomeDir() for user scope. The <name> in cache paths is ValidatePluginName-checked before use.

pkg/plugins/parser.go (2 alerts: 106, 121)
pluginDir reaches the sinks only through validateLocalPath (non-empty, no NUL, absolute, no ..). API server binds to localhost; auth middleware applies. Operations are read-only manifest parsing.

pkg/fileutils/atomic.go (2 alerts: 20, 59)
AtomicWriteFile is a shared primitive that deliberately does not validate (per-caller contract, documented by existing #nosec G703). Callers either pass app-rooted fixed paths or go through WriteContainedFile, which enforces prefix containment before delegating.

pkg/skills/installer.go (3 alerts: 143, 157, 167)
Identical alerts previously dismissed as FP on main; rationale still holds — targetDir comes from GetPluginPath (name-validated, config-rooted), never from raw API input or archive entries. Archive entry paths never reach os.Stat/MkdirAll/RemoveAll directly; extraction goes through ExtractTarWithLimit (rejects symlinks/hardlinks/absolute/..) → WriteContainedFile (prefix containment) → post-extraction CheckFilesystem verification.

Why CodeQL fires here: taint tracking sees opts.Name / opts.Scope / opts.ProjectRoot flow from the HTTP body into filesystem calls, but cannot model the cross-package sanitizers (pkg/skills/project_root.go, pkg/skills/validator.go, pkg/plugins/pluginsvc/build.go) that neutralize each input before the sink. This is the same FP pattern as the ~30 identical alerts already dismissed on main for the skills system.

The remaining 9 results in the branch analysis (lockfile/cleanup.go, skills/project_root.go, skills/validator.go, other skills/installer.go lines) are pre-existing and were already dismissed previously — untouched by this PR.

[samuv]

Thanks for putting this together — the skills-system mirroring is executed faithfully, and the security follow-through is genuinely good: validateOCIRegistryHost closes the OCI-pull SSRF gap before Pull is reached, with tests asserting Pull is never invoked for rejected hosts, and the DNS-rebinding limitation honestly documented in the comment. The exposure layer (REST + client + CLI) is consistent with the skills precedent, test coverage is thorough and table-driven, generated docs were regenerated in the same PR, and docs/arch/14-plugins-system.md was updated.

Overall this reads as close to merge-ready — nothing blocking from my side. I've left four inline comments (two small should-fixes, two questions), plus a few PR-level notes below.

PR-level findings

• [SHOULD-FIX] PR body hygiene — the template's two required sections are absent: "Type of change" (exactly one checkbox) and "Test plan" (at least one checkbox). The Testing section covers the substance, so this is just re-adding the two sections to keep the template contract intact.
• [NIT] DCO — none of the 7 commits carry a Signed-off-by trailer, which CONTRIBUTING.md asks for on all commits. CI doesn't gate on it and recent main history suggests it's loosely applied, so noting only for awareness.
• [NIT] follow-up tracking — commit c2c98db notes the skills OCI pull paths have the same SSRF gap, intentionally left out of this PR. A tracking issue would make sure that follow-up doesn't get lost.

What's working well

• The nil-slice guards on listPlugins/listBuilds improve on the skills original, and the client has a compile-time PluginService check plus httptest-driven tests for all 10 methods, including path escaping.
• Swagger and CLI docs regenerated in-PR, arch doc updated, and commit messages are clear what-and-why — nice hygiene throughout.

CI note: everything is green except E2E Test Lifecycle (kindest/node:v1.33.7), which failed on both recent runs while v1.34.3 and v1.35.1 passed. This PR touches no operator code, so it looks unrelated/flaky — probably worth a re-run before merge.

Verdict: approve with nits — the two should-fix items are one-liners, and everything else is optional.

[samuv]

🚀