Add local-first plugin marketplace#653
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis change adds a local-first plugin marketplace with catalog validation, signatures, caching, managed installation, lifecycle controls, integrity enforcement, CLI workflows, and a ChangesPlugin marketplace and management
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PluginManager
participant CLI
participant Marketplace
participant PluginRuntime
User->>PluginManager: Select marketplace install
PluginManager->>CLI: Run plugins install with scope and confirmation flags
CLI->>Marketplace: Load and verify catalog release
Marketplace-->>CLI: Release metadata and source
CLI->>PluginRuntime: Install and record lock metadata
PluginRuntime-->>CLI: Installation result
CLI-->>PluginManager: Snapshot and restart status
PluginManager-->>User: Render installed plugin state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/tui/model.go (1)
1329-1374: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCancel the in-flight plugin command on Esc.
handlePluginManagerKeyonly clearsm.pluginManager; callm.cancelPluginCommand()first so/plugins ...and manager actions can actually be interrupted, like the MCP branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tui/model.go` around lines 1329 - 1374, Update the Esc handling branch for m.pluginManager to call m.cancelPluginCommand() before clearing the manager state, matching the cancellation behavior in the MCP branch and ensuring in-flight /plugins commands and manager actions are interrupted.
🧹 Nitpick comments (1)
internal/marketplace/registry_test.go (1)
25-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify all persisted fields in the round-trip test.
TestRegistryLoadSaveRoundTripsetsPublicKeyPathandVerificationStatusbut only assertsIDandSourceafter reload. Adding assertions for the persisted fields would catch serialization bugs.✅ Suggested additions
if len(loaded.Catalogs) != 1 || loaded.Catalogs[0].ID != "team" || loaded.Catalogs[0].Source != "./catalog.json" { t.Fatalf("unexpected registry: %#v", loaded) } + if loaded.Catalogs[0].PublicKeyPath != "./catalog.pub" { + t.Fatalf("publicKeyPath = %q, want ./catalog.pub", loaded.Catalogs[0].PublicKeyPath) + } + if loaded.Catalogs[0].VerificationStatus != VerificationUnsigned { + t.Fatalf("verificationStatus = %q, want %q", loaded.Catalogs[0].VerificationStatus, VerificationUnsigned) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/marketplace/registry_test.go` around lines 25 - 47, Update TestRegistryLoadSaveRoundTrip to assert that the reloaded catalog’s PublicKeyPath and VerificationStatus match the values originally provided, in addition to the existing ID and Source checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 295-300: Clarify the marketplace documentation sentence so that
--allow-unverified is explicitly required only for installs from unsigned
catalogs, while --yes remains universally required; update the wording in the
Marketplace catalogs section of AGENTS.md.
In `@internal/cli/app.go`:
- Around line 891-906: Thread the TUI command context through the plugin
execution path: update tuiPluginCommand to pass ctx into runPlugins, then
propagate it through runPlugins and all marketplace browse/install/update
helpers, replacing their hard-coded context.Background() calls with the received
context. Preserve existing behavior for non-TUI callers by supplying an
appropriate background context at the entry point.
In `@internal/cli/plugin_marketplace.go`:
- Around line 1172-1220: Update compareSemver to apply SemVer prerelease
precedence after comparing the numeric core: when cores match, treat a version
without a prerelease identifier as greater than one with a prerelease
identifier, and only use a deterministic tie-breaker for equivalent prerelease
status. Adjust semverCore or add a helper near compareSemver to preserve
prerelease information, ensuring selectRelease chooses the stable release over
prereleases when --version is omitted.
- Around line 861-888: Bound each catalog operation in resolveMarketplacePlugin
with a per-catalog timeout context before calling loadCatalogForRegistration,
updateCatalogCache, or loadCatalogEntry. Derive the context from the command
context, use the existing catalog timeout convention if available, and ensure
each cancel function is released. Pass the timed context through all git-backed
loads so stalled remotes return an error instead of blocking the command
indefinitely.
In `@internal/marketplace/catalog.go`:
- Around line 271-285: Update ValidateRemoteCatalogReleaseSources to explicitly
reject file:// repository URLs in addition to CatalogSourceLocal paths. Use the
parsed source’s identifying fields or the original release.Repository value to
detect the file scheme, and return the same field-specific validation error;
ensure file:// sources cannot pass as CatalogSourceGit.
- Around line 481-485: Add table-driven tests for the HTTP source handling in
the catalog parsing test suite, covering loopback hosts 127.0.0.1, ::1,
::ffff:127.0.0.1, and 0.0.0.0 as accepted, plus localhost.example.com as
rejected. Exercise the relevant catalog parsing function and assert each case’s
success or error outcome.
In `@internal/marketplace/registry.go`:
- Around line 293-305: Add a default 30-second timeout in fetchGitCatalog when
the supplied context has no deadline, using context.WithTimeout and ensuring the
derived cancel function is deferred; preserve any existing caller-provided
deadline and continue passing the resulting context to exec.CommandContext.
In `@internal/plugins/root_lock_windows.go`:
- Around line 45-63: Update recoverStalePluginRootLock to perform a real Windows
process-liveness check using syscall.OpenProcess with
PROCESS_QUERY_LIMITED_INFORMATION before removing a stale lock; retain the lock
when the recorded PID is still running, handle invalid or unreadable PIDs
safely, and verify the API/constants are supported by the project’s Go
toolchain.
In `@internal/tui/plugin_manager.go`:
- Around line 93-95: Closing the plugin manager must invalidate any in-flight
command before clearing the overlay. Update the Esc handler and
prefillPluginManagerCommand to call m.cancelPluginCommand() before setting
m.pluginManager = nil, ensuring pluginCommandSeq/cancellation state marks
pending results stale and prevents applyPluginCommandResultMessage from
reopening the manager or appending transcript rows.
In `@internal/zerocommands/backend_snapshots.go`:
- Around line 268-269: Update pluginQuarantined to detect both path segments
containing "/.disabled/" and paths beginning with ".disabled/"; normalize the
path with filepath.ToSlash and combine the existing strings.Contains check with
a strings.HasPrefix check.
---
Outside diff comments:
In `@internal/tui/model.go`:
- Around line 1329-1374: Update the Esc handling branch for m.pluginManager to
call m.cancelPluginCommand() before clearing the manager state, matching the
cancellation behavior in the MCP branch and ensuring in-flight /plugins commands
and manager actions are interrupted.
---
Nitpick comments:
In `@internal/marketplace/registry_test.go`:
- Around line 25-47: Update TestRegistryLoadSaveRoundTrip to assert that the
reloaded catalog’s PublicKeyPath and VerificationStatus match the values
originally provided, in addition to the existing ID and Source checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3bb943b-6487-4ca7-ad83-b4cf1b2a75c3
📒 Files selected for processing (41)
AGENTS.mdREADME.mddocs/HOW_ZERO_WORKS.mddocs/PLUGIN_MARKETPLACE.mdinternal/cli/app.gointernal/cli/distribution.gointernal/cli/extensions.gointernal/cli/hook_dispatch.gointernal/cli/marketplace_cli_test.gointernal/cli/marketplace_install_cli_test.gointernal/cli/plugin_disable_cli_test.gointernal/cli/plugin_marketplace.gointernal/cli/plugin_pin_cli_test.gointernal/marketplace/catalog.gointernal/marketplace/catalog_test.gointernal/marketplace/registry.gointernal/marketplace/registry_test.gointernal/plugins/disable_test.gointernal/plugins/install.gointernal/plugins/install_test.gointernal/plugins/integrity_test.gointernal/plugins/plugins.gointernal/plugins/root_lock_unix.gointernal/plugins/root_lock_unix_test.gointernal/plugins/root_lock_windows.gointernal/tui/autocomplete.gointernal/tui/clipboard.gointernal/tui/commands.gointernal/tui/commands_test.gointernal/tui/composer.gointernal/tui/files_panel.gointernal/tui/model.gointernal/tui/mouse.gointernal/tui/options.gointernal/tui/plan_step_detail.gointernal/tui/plugin_manager.gointernal/tui/scroll_test.gointernal/tui/sidebar.gointernal/tui/transcript_selection.gointernal/zerocommands/backend_snapshots.gointernal/zerocommands/backend_snapshots_test.go
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Reviewed this against current main — build, vet, gofmt, and the marketplace/plugins/cli tests all pass. The security model is sound and fail-closed: ed25519 catalog signatures are verified, invalid or missing signatures are rejected (unsigned needs an explicit --allow-unverified), install never executes fetched content, copyTree skips .git and refuses symlinks, subdir and plugin ids are validated against path traversal, the tree hash is checked against the catalog before publish, project-scoped catalogs are gated behind workspace trust, and install/update require --yes. It composes with the existing activation + permission gating rather than bypassing it.
Holding for two owner calls, both kevin's. First, issue #652 is still open with no maintainer sign-off — this is a sizable feature and I'd want your go-ahead on the scope before it lands. Second, the official catalog ships a hardcoded ed25519 public key and points at Gitlawb/zero-plugins, which doesn't exist yet. That trust anchor and repo are yours to provision and sign, so I don't want to merge a built-in "official" catalog whose signing key a contributor chose — until that repo exists and is signed with a key you control, zero plugins browse against the official catalog will fail. Minor non-blocker: the root lock removes its lockfile on release (the usual flock+remove race), fine for a low-concurrency per-user plugins dir.
|
@gnanam1990 when you have time, #653 is a big one worth a second pair of eyes — a local-first plugin marketplace (+7946, 43 files) with ed25519 catalog signing, git/HTTP catalog fetch, hash-verified installs, and workspace-trust gating for project scope. I reviewed it inline and the security model looks fail-closed and sound, but I'd value your independent read on the install/verify path (signature enforcement, copyTree symlink/.git handling, subdir path safety) before it goes anywhere. I'm holding it for an owner call on issue #652 and the official-catalog trust anchor, not on the code. |
Summary
Closes #652
Summary by CodeRabbit
zero pluginslifecycle commands: enable/disable and pin/unpin (including JSON output and restart-required behavior in the interactive UI)./pluginsmanager with search, marketplace/catalog browsing, risk/integrity details, and disabled-plugin quarantine visibility.