plugin: support local-based marketplace.json + install endpoint.#13422
plugin: support local-based marketplace.json + install endpoint.#13422
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 736e85a549
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,14 +1,15 @@ | |||
| mod manager; | |||
| mod manifest; | |||
| mod marketplace; | |||
There was a problem hiding this comment.
Add missing plugins::marketplace module
mod marketplace; is declared, and manager.rs imports super::marketplace::*, but this commit does not add codex-rs/core/src/plugins/marketplace.rs (or plugins/marketplace/mod.rs). The crate will not compile due to unresolved module/imports, so plugin/install cannot work at all.
Useful? React with 👍 / 👎.
sayan-oai
left a comment
There was a problem hiding this comment.
one small comment about errors.
also want to clarify that we will add post-install validation after this? to check if a plugin's apps need to be installed in chatgpt, so we can return that to the client and have them trigger whatever app-install flow they have.
approving to unblock
codex-rs/core/src/plugins/manager.rs
Outdated
| } | ||
|
|
||
| pub fn is_invalid_request(&self) -> bool { | ||
| matches!(self, Self::Marketplace(_) | Self::Store(_)) |
There was a problem hiding this comment.
maybe nit, but MarketplaceError and PluginStoreError include IO error variants, so issues with things like reading marketplace.json and moving plugin to cache would get returned to client as invalid request errors, which is misleading
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78ce0d4ff4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
codex-rs/core/src/plugins/manager.rs
Outdated
| pub fn is_invalid_request(&self) -> bool { | ||
| matches!(self, Self::Marketplace(_) | Self::Store(_)) |
There was a problem hiding this comment.
Classify only client-caused install errors as invalid requests
is_invalid_request() marks all Marketplace and Store failures as client errors. But those enums include Io variants (e.g., unreadable marketplace.json, copy/write failures), so server-side faults are returned as INVALID_REQUEST in plugin_install instead of internal errors. This misleads clients and hides operational failures.
Useful? React with 👍 / 👎.
| let relative_source_path = Path::new(path); | ||
| if relative_source_path | ||
| .components() | ||
| .any(|component| !matches!(component, Component::Normal(_))) | ||
| { |
There was a problem hiding this comment.
Canonicalize local source paths before enforcing directory bounds
The path check is purely lexical (Component::Normal) and does not resolve symlinks. A marketplace entry like ./plugin-link can pass validation even if plugin-link points outside .agents/plugins, bypassing the stated confinement rule and allowing installs from arbitrary directories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This isn't intended to be a strong security boundary. marketplace.json is trusted local configuration, and the current validation is only meant to reject obvious path traversal and malformed relative paths, not to defend against symlink-based escapes.
Support marketplace.json that points to a local file, with
Add a new plugin/install endpoint which add the plugin to the cache folder and enable it in config.toml.