feat: custom plugin marketplace support - #3656
diegosouzapw merged 8 commits into
Conversation
- Updated Plugin Marketplace to support custom registry URLs instead of just local seed - Added custom marketplace URL configuration in the UI - Added a full marketplace browser tab in the Plugins dashboard
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Marketplace tab to the plugins dashboard, allowing users to configure a custom marketplace URL and browse or install plugins. The backend is updated to fetch marketplace entries asynchronously from the configured URL with a fallback to a local seed registry. The code review feedback highlights several critical improvements: adding validation for external JSON data fetched from the custom marketplace to prevent runtime crashes, checking the res.ok status on fetch requests to handle API errors gracefully, reverting a potential breaking change that made isMarketplaceAvailable asynchronous, and replacing hardcoded English strings in the UI with the t translation hook to support internationalization.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const handleSaveUrl = async () => { | ||
| setSavingUrl(true); | ||
| try { | ||
| await fetch("/api/settings", { | ||
| method: "PATCH", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ pluginMarketplaceUrl: marketplaceUrl || null }), | ||
| }); | ||
| addNotification({ type: "success", message: "Marketplace URL updated" }); | ||
| await fetchMarketplace(); | ||
| } catch { | ||
| addNotification({ type: "error", message: "Failed to update URL" }); | ||
| } finally { | ||
| setSavingUrl(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The fetch API does not throw an error on non-2xx HTTP status codes (like 400 or 500). If the PATCH request to /api/settings fails (e.g., due to validation errors), the code will still display a success notification and attempt to reload the marketplace. You should check res.ok and throw an error to trigger the catch block.
const handleSaveUrl = async () => {
setSavingUrl(true);
try {
const res = await fetch("/api/settings", {
method: "PATCH",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ pluginMarketplaceUrl: marketplaceUrl || null }),
});
if (!res.ok) {
throw new Error("Failed to update settings");
}
addNotification({ type: "success", message: "Marketplace URL updated" });
await fetchMarketplace();
} catch {
addNotification({ type: "error", message: "Failed to update URL" });
} finally {
setSavingUrl(false);
}
};
| export async function listMarketplacePlugins(): Promise<MarketplaceEntry[]> { | ||
| try { | ||
| const settings = await getSettings(); | ||
| const url = typeof settings.pluginMarketplaceUrl === "string" ? settings.pluginMarketplaceUrl : null; | ||
| if (url) { | ||
| const res = await fetch(url, { signal: AbortSignal.timeout(5000) }); | ||
| if (res.ok) { | ||
| const data = await res.json(); | ||
| return Array.isArray(data) ? data : (data.plugins || []); | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error("Failed to fetch from custom plugin marketplace:", err); | ||
| } | ||
| return [...SEED_REGISTRY]; | ||
| } |
There was a problem hiding this comment.
Fetching external JSON data from a user-configured URL and casting it directly to MarketplaceEntry[] without validation is risky. If the custom marketplace returns malformed data or is missing required fields (like name, version, description, or tags), it can cause runtime crashes (e.g., TypeError: Cannot read properties of undefined) in search, filtering, or UI rendering. \n\nAdditionally, if the response is not successful (res.ok is false), the error is silently ignored and falls back to the seed registry. It would be highly beneficial to log a warning with the status code for better observability.
export async function listMarketplacePlugins(): Promise<MarketplaceEntry[]> {
try {
const settings = await getSettings();
const url = typeof settings.pluginMarketplaceUrl === "string" ? settings.pluginMarketplaceUrl : null;
if (url) {
const res = await fetch(url, { signal: AbortSignal.timeout(5000) });
if (res.ok) {
const data = await res.json();
const rawPlugins = Array.isArray(data) ? data : (data.plugins || []);
return rawPlugins
.filter((p: any) => p && typeof p === "object" && typeof p.name === "string" && typeof p.version === "string")
.map((p: any) => ({
name: p.name,
version: p.version,
description: typeof p.description === "string" ? p.description : "",
author: typeof p.author === "string" ? p.author : "Unknown",
license: typeof p.license === "string" ? p.license : "Unknown",
downloadUrl: typeof p.downloadUrl === "string" ? p.downloadUrl : "",
repository: typeof p.repository === "string" ? p.repository : undefined,
tags: Array.isArray(p.tags) ? p.tags.filter((t: any) => typeof t === "string") : [],
downloads: typeof p.downloads === "number" ? p.downloads : 0,
rating: typeof p.rating === "number" ? p.rating : 0,
verified: Boolean(p.verified),
lastUpdated: typeof p.lastUpdated === "string" ? p.lastUpdated : "",
}));
} else {
console.warn(`Custom plugin marketplace returned status ${res.status}: ${res.statusText}`);
}
}
} catch (err) {
console.error("Failed to fetch from custom plugin marketplace:", err);
}
return [...SEED_REGISTRY];
}| fetch("/api/settings") | ||
| .then((res) => res.json()) | ||
| .then((data) => { | ||
| if (data.pluginMarketplaceUrl) setMarketplaceUrl(data.pluginMarketplaceUrl); | ||
| }); |
There was a problem hiding this comment.
The fetch call to /api/settings does not check if the response is successful (res.ok) before parsing it as JSON. If the request fails (e.g., 401 Unauthorized), res.json() might fail or return an error object, which could lead to unexpected behavior. It is safer to check res.ok first.
fetch("/api/settings")
.then((res) => res.ok ? res.json() : null)
.then((data) => {
if (data?.pluginMarketplaceUrl) setMarketplaceUrl(data.pluginMarketplaceUrl);
})
.catch(() => {});
| export async function isMarketplaceAvailable(): Promise<boolean> { | ||
| try { | ||
| const settings = await getSettings(); | ||
| return true; // Always available (falls back to seed) | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Changing isMarketplaceAvailable to be an async function returning a Promise<boolean> is a potential breaking change for any existing synchronous callers in the codebase. Since the function always returns true (falling back to the seed registry) and does not actually use any values from settings, it is safer to keep it synchronous to maintain backward compatibility.
export function isMarketplaceAvailable(): boolean {
return true; // Local seed always available
}| <div className="flex gap-2"> | ||
| <Button variant={activeTab === "installed" ? "primary" : "secondary"} onClick={() => setActiveTab("installed")}> | ||
| Installed | ||
| </Button> | ||
| <Button variant={activeTab === "marketplace" ? "primary" : "secondary"} onClick={() => setActiveTab("marketplace")}> | ||
| Marketplace | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
The tab labels 'Installed' and 'Marketplace' are hardcoded in English. To support internationalization (i18n) and maintain consistency with the rest of the page, please use the t translation hook.
| <div className="flex gap-2"> | |
| <Button variant={activeTab === "installed" ? "primary" : "secondary"} onClick={() => setActiveTab("installed")}> | |
| Installed | |
| </Button> | |
| <Button variant={activeTab === "marketplace" ? "primary" : "secondary"} onClick={() => setActiveTab("marketplace")}> | |
| Marketplace | |
| </Button> | |
| </div> | |
| <div className="flex gap-2"> | |
| <Button variant={activeTab === "installed" ? "primary" : "secondary"} onClick={() => setActiveTab("installed")}> | |
| {t("installed")} | |
| </Button> | |
| <Button variant={activeTab === "marketplace" ? "primary" : "secondary"} onClick={() => setActiveTab("marketplace")}> | |
| {t("marketplace")} | |
| </Button> | |
| </div> |
| {activeTab === "marketplace" && ( | ||
| <Card className="p-4 flex gap-4 items-end bg-gray-50"> | ||
| <div className="flex-1"> | ||
| <label className="block text-sm font-medium text-gray-700 mb-1">Custom Marketplace URL</label> | ||
| <input | ||
| type="text" | ||
| className="w-full rounded border-gray-300 p-2" | ||
| placeholder="Leave empty for official Omniroute registry" | ||
| value={marketplaceUrl} | ||
| onChange={(e) => setMarketplaceUrl(e.target.value)} | ||
| /> | ||
| </div> | ||
| <Button onClick={handleSaveUrl} disabled={savingUrl}> | ||
| Save & Reload | ||
| </Button> | ||
| </Card> | ||
| )} |
There was a problem hiding this comment.
The form labels, placeholders, and button text are hardcoded in English. Please use the t translation hook to support internationalization (i18n).
| {activeTab === "marketplace" && ( | |
| <Card className="p-4 flex gap-4 items-end bg-gray-50"> | |
| <div className="flex-1"> | |
| <label className="block text-sm font-medium text-gray-700 mb-1">Custom Marketplace URL</label> | |
| <input | |
| type="text" | |
| className="w-full rounded border-gray-300 p-2" | |
| placeholder="Leave empty for official Omniroute registry" | |
| value={marketplaceUrl} | |
| onChange={(e) => setMarketplaceUrl(e.target.value)} | |
| /> | |
| </div> | |
| <Button onClick={handleSaveUrl} disabled={savingUrl}> | |
| Save & Reload | |
| </Button> | |
| </Card> | |
| )} | |
| {activeTab === "marketplace" && ( | |
| <Card className="p-4 flex gap-4 items-end bg-gray-50"> | |
| <div className="flex-1"> | |
| <label className="block text-sm font-medium text-gray-700 mb-1">{t("customMarketplaceUrl")}</label> | |
| <input | |
| type="text" | |
| className="w-full rounded border-gray-300 p-2" | |
| placeholder={t("customMarketplaceUrlPlaceholder")} | |
| value={marketplaceUrl} | |
| onChange={(e) => setMarketplaceUrl(e.target.value)} | |
| /> | |
| </div> | |
| <Button onClick={handleSaveUrl} disabled={savingUrl}> | |
| {t("saveAndReload")} | |
| </Button> | |
| </Card> | |
| )} |
| {marketplacePlugins.length === 0 ? ( | ||
| <div className="text-gray-500 py-4">No plugins found in marketplace.</div> | ||
| ) : ( | ||
| marketplacePlugins.map((plugin) => ( | ||
| <Card key={plugin.name} className="p-4"> | ||
| <div className="flex items-center justify-between"> | ||
| <div> | ||
| <h3 className="font-semibold flex items-center gap-2"> | ||
| {plugin.name} | ||
| {plugin.verified && <Badge variant="success">Verified</Badge>} | ||
| </h3> | ||
| <p className="text-sm text-gray-500"> | ||
| v{plugin.version} by {plugin.author} — {plugin.description} | ||
| </p> | ||
| <div className="mt-1 flex gap-1"> | ||
| {plugin.tags?.map((tag: string) => ( | ||
| <span key={tag} className="rounded bg-gray-100 px-2 py-0.5 text-xs text-gray-600"> | ||
| {tag} | ||
| </span> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| <div className="flex gap-2"> | ||
| <Button | ||
| variant="primary" | ||
| onClick={async () => { | ||
| addNotification({ type: "success", message: `Plugin ${plugin.name} installed! (Mock)` }); | ||
| }} | ||
| > | ||
| Install | ||
| </Button> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
The marketplace list empty state, badge text, and install button text are hardcoded in English. Please use the t translation hook to support internationalization (i18n).
{marketplacePlugins.length === 0 ? (
<div className="text-gray-500 py-4">{t("noMarketplacePlugins")}</div>
) : (
marketplacePlugins.map((plugin) => (
<Card key={plugin.name} className="p-4">
<div className="flex items-center justify-between">
<div>
<h3 className="font-semibold flex items-center gap-2">
{plugin.name}
{plugin.verified && <Badge variant="success">{t("verified")}</Badge>}
</h3>
<p className="text-sm text-gray-500">
v{plugin.version} by {plugin.author} — {plugin.description}
</p>
<div className="mt-1 flex gap-1">
{plugin.tags?.map((tag: string) => (
<span key={tag} className="rounded bg-gray-100 px-2 py-0.5 text-xs text-gray-600">
{tag}
</span>
))}
</div>
</div>
<div className="flex gap-2">
<Button
variant="primary"
onClick={async () => {
addNotification({ type: "success", message: t("pluginInstalledMock", { name: plugin.name }) });
}}
>
{t("install")}
</Button>
</div>
</div>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
|
||
| useEffect(() => { | ||
| fetchPlugins(); | ||
| fetch("/api/settings") |
There was a problem hiding this comment.
WARNING: /api/settings GET has no error handling.
fetch can reject on network failures and this promise chain has no .catch(), causing unhandled promise rejections. Add .catch(() => {}) or an async effect to keep page render stable.
|
|
||
| const fetchMarketplace = useCallback(async () => { | ||
| try { | ||
| const res = await fetch("/api/plugins/marketplace"); |
There was a problem hiding this comment.
CRITICAL: Missing marketplace API endpoint.
fetch("/api/plugins/marketplace") references a route that does not exist under src/app/api/plugins/. The marketplace tab will always fail with 404 until this route is added or an existing endpoint is used.
| const res = await fetch("/api/plugins/marketplace"); | ||
| if (res.ok) { | ||
| const data = await res.json(); | ||
| setMarketplacePlugins(data.plugins || []); |
There was a problem hiding this comment.
WARNING: Marketplace response shape is inconsistent.
listMarketplacePlugins() returns MarketplaceEntry[], but this UI expects data.plugins. If the endpoint returns the helper's array directly, the marketplace list will render empty. Align the response shape or normalize it here.
| const res = await fetch("/api/settings", { | ||
| method: "PATCH", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ pluginMarketplaceUrl: marketplaceUrl || null }), |
There was a problem hiding this comment.
CRITICAL: Marketplace URL setting is not persisted.
pluginMarketplaceUrl is not present in updateSettingsSchema and is not defaulted in getSettings(). Zod strips unknown keys, so PATCH /api/settings can return success while silently ignoring this field, making the save notification misleading.
| const settings = await getSettings(); | ||
| const url = typeof settings.pluginMarketplaceUrl === "string" ? settings.pluginMarketplaceUrl : null; | ||
| if (url) { | ||
| const res = await fetch(url, { signal: AbortSignal.timeout(5000) }); |
There was a problem hiding this comment.
CRITICAL: Arbitrary custom marketplace URL enables SSRF.
Server-side fetch() uses the configured URL without validating scheme, hostname, or length. A management user or compromised setting could make the server request internal metadata endpoints or private network resources. Add allowlisted http/https URL validation and host/IP restrictions before fetch.
| <div className="flex gap-2"> | ||
| <Button | ||
| variant="primary" | ||
| onClick={async () => { |
There was a problem hiding this comment.
CRITICAL: Marketplace install button is a mock.
The button only shows a success notification and never calls an install API or updates installed plugins. Users will see "installed" even though no plugin was installed; either wire up the install flow or label this as unavailable.
Code Review SummaryStatus: 6 New Issues Found (7 existing active comments remain) | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)Existing active comments were already present and were not duplicated. Seven active comments remain at Files Reviewed (3 files)
Reviewed by step-3.7-flash-20260528 · 715,840 tokens |
|
Obrigado pela contribuição, @oyi77! Não dá pra mergear como está — segue o que falta:
Deixo aberta — quando ajustar, reavalio. |
…th coming-soon notification
…able sync - Add isSafeMarketplaceUrl() helper that validates http/https scheme and rejects private/loopback IPs via DNS resolution - Insert SSRF guard in listMarketplacePlugins() before fetch call - Replace async isMarketplaceAvailable() with simple sync function that no longer calls getSettings()
- Add SSRF validation (isSafeMarketplaceUrl) to reject private/loopback IPs
- Clean up isMarketplaceAvailable — remove dead getSettings() call
- Create GET /api/plugins/marketplace route returning { plugins: [...] }
- Add .catch() to settings fetch in plugins page
- Replace mock install button with proper 'coming soon' message
- Add tests for marketplace route, list/search/get/isAvailable
|
Thanks @oyi77 — clean, self-contained feature and a welcome one (it's the real backend behind the marketplace surface that was only documented before). Validated: the |
…contributor credits - Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance - Add bullets for every PR landed since v3.8.23 that was missing: marketplace (#3656), strict-mode CC defaults (#3776), emergency-fallback flag (#3752), xhigh effort (#3756), Codex memory WS (#3749), IPv6 egress (#3777), marketplace SSRF (#3774), CodeQL/Dependabot (#3778), anthropic sampling (#3780), thinking passthrough (#3775), mcp dist entry (#3765), streamed tool args (#3762), logs light-mode (#3760), clean-history purge (#3751), quality-gates (#3757), docs gaps (#3453), file-size re-baseline (#3770), E415 publish guard, i18n prune - Move misplaced #3775 bullet out of [Unreleased] into [3.8.24] - Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
…n stale codex/plugins tests - Date the [3.8.24] section (2026-06-13) and add the missing CHANGELOG bullet for the combo + quota-shared deep audit (#3779). - Align two unit tests left stale by intentional behavior changes this cycle: executor-codex now asserts gpt-5.4-mini xhigh passthrough (#3756 default), and plugins-route-error-sanitization now covers the /api/plugins/marketplace route added by #3656 (verified Hard Rule #12 compliant). No production change.
…leanup (diegosouzapw#3800) Surfaces the Plugins page (marketplace, diegosouzapw#3656) in the sidebar; adds the proxy IP-family selector (auto/ipv4/ipv6) completing diegosouzapw#3777's UI; clears the remaining CodeQL URL-substring alerts; covers diegosouzapw#3799 with tests and fixes the costs-section count.
…leanup (diegosouzapw#3800) Surfaces the Plugins page (marketplace, diegosouzapw#3656) in the sidebar; adds the proxy IP-family selector (auto/ipv4/ipv6) completing diegosouzapw#3777's UI; clears the remaining CodeQL URL-substring alerts; covers diegosouzapw#3799 with tests and fixes the costs-section count.
feat: custom plugin marketplace (GET /api/plugins/marketplace + SSRF-guarded registry). Integrated into release/v3.8.24.
…ding) (diegosouzapw#3774) Harden marketplace SSRF guard (IPv6/AAAA + redirect-block + fail-closed resolve). Follow-up to diegosouzapw#3656. Integrated into release/v3.8.24.
…contributor credits - Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance - Add bullets for every PR landed since v3.8.23 that was missing: marketplace (diegosouzapw#3656), strict-mode CC defaults (diegosouzapw#3776), emergency-fallback flag (diegosouzapw#3752), xhigh effort (diegosouzapw#3756), Codex memory WS (diegosouzapw#3749), IPv6 egress (diegosouzapw#3777), marketplace SSRF (diegosouzapw#3774), CodeQL/Dependabot (diegosouzapw#3778), anthropic sampling (diegosouzapw#3780), thinking passthrough (diegosouzapw#3775), mcp dist entry (diegosouzapw#3765), streamed tool args (diegosouzapw#3762), logs light-mode (diegosouzapw#3760), clean-history purge (diegosouzapw#3751), quality-gates (diegosouzapw#3757), docs gaps (diegosouzapw#3453), file-size re-baseline (diegosouzapw#3770), E415 publish guard, i18n prune - Move misplaced diegosouzapw#3775 bullet out of [Unreleased] into [3.8.24] - Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
…bullet, align stale codex/plugins tests - Date the [3.8.24] section (2026-06-13) and add the missing CHANGELOG bullet for the combo + quota-shared deep audit (diegosouzapw#3779). - Align two unit tests left stale by intentional behavior changes this cycle: executor-codex now asserts gpt-5.4-mini xhigh passthrough (diegosouzapw#3756 default), and plugins-route-error-sanitization now covers the /api/plugins/marketplace route added by diegosouzapw#3656 (verified Hard Rule diegosouzapw#12 compliant). No production change.
…leanup (diegosouzapw#3800) Surfaces the Plugins page (marketplace, diegosouzapw#3656) in the sidebar; adds the proxy IP-family selector (auto/ipv4/ipv6) completing diegosouzapw#3777's UI; clears the remaining CodeQL URL-substring alerts; covers diegosouzapw#3799 with tests and fixes the costs-section count.
2 participants
This PR adds support for Custom Plugin Marketplaces as requested, extending the current local-seed-only implementation.
Changes: