Conversation
Greptile SummaryThis PR adds an MCP server catalog to the
Confidence Score: 4/5Safe to merge after addressing the wildcard CORS issue in the supergateway install command. The PR is well-structured with good test coverage on the Rust side and a clean React component architecture. Most issues raised in prior review rounds are addressed (shell injection fixed with single-quote escaping, loadingMore stuck state fixed with unconditional finally reset, required headers now emit placeholders). One new P1 security finding remains: ui/src/services/mcpRegistry/client.ts —
|
| Filename | Overview |
|---|---|
| ui/src/services/mcpRegistry/client.ts | New registry client: search/lookup, install-command builder, header materializer. buildInstallCommand emits --cors "*" in the supergateway wrapper, exposing the local server to all web origins. Shell-injection and required-header-placeholder issues from previous threads are now addressed. |
| ui/src/components/MCPConfigModal/MCPCatalog.tsx | New catalog component: favorites section, registry search with pagination, RemoteCard/LocalCard/FavoriteCard rendering. Load-more abort and loadingMore reset issues from prior threads are now fixed. React.ReactNode type references without the React namespace import remain (flagged in previous thread). |
| src/config/ui.rs | Adds McpUiConfig and FavoriteMcpServer Rust structs with deny_unknown_fields, Default, and a set of well-known favorite servers. Clean and consistent with the rest of the config module. |
| src/routes/admin/ui_config.rs | Adds McpUiResponse and FavoriteMcpServerResponse types plus From impls; wires them into the existing UiConfigResponse. Straightforward mapping, well-covered by the new integration tests. |
| ui/src/components/MCPConfigModal/MCPConfigModal.tsx | Wires MCPCatalog into the modal's view state machine (list → catalog → form). The catalog-prefill/form-origin flow is handled correctly; favorites is read from config and passed through. |
Sequence Diagram
sequenceDiagram
participant User
participant MCPConfigModal
participant MCPCatalog
participant RegistryAPI as MCP Registry API
participant ServerForm
participant LocalProxy as Local Supergateway Proxy
User->>MCPConfigModal: Click "Add Server"
MCPConfigModal->>MCPCatalog: show(favorites)
MCPCatalog->>RegistryAPI: searchRegistry({limit:30})
RegistryAPI-->>MCPCatalog: {servers[], nextCursor}
MCPCatalog->>MCPCatalog: dedupeLatest() + categorize()
alt Remote server selected
User->>MCPCatalog: Click "Add"
MCPCatalog->>ServerForm: onPick(CatalogPrefill{url, headers, authType})
else Local/stdio server selected
User->>MCPCatalog: Click "Set up"
MCPCatalog->>ServerForm: onPick(CatalogPrefill{url=localhost:8000, localInstall.command})
Note over ServerForm: Shows install banner with npx supergateway command
User->>LocalProxy: Runs generated command locally
else Favorite (registry ID)
MCPCatalog->>RegistryAPI: getRegistryEntry(registryId)
RegistryAPI-->>MCPCatalog: MCPRegistryEntry
MCPCatalog->>MCPCatalog: categorize() → RemoteCard or LocalCard
end
ServerForm->>MCPConfigModal: onSubmit(values)
MCPConfigModal->>MCPConfigModal: addServer()
Prompt To Fix All With AI
This is a comment left during a code review.
Path: ui/src/services/mcpRegistry/client.ts
Line: 190
Comment:
**`--cors "*"` exposes the local MCP server to every web origin**
The generated supergateway command passes `--cors "*"`, which instructs supergateway to respond with `Access-Control-Allow-Origin: *`. While the server binds to `localhost`, CORS headers are what the *browser* enforces — any website the user visits while this proxy is running can send `fetch("http://localhost:8000/mcp", ...)` requests and read the responses. Local MCP servers routinely hold API keys in environment variables and may have filesystem or shell access, making this a meaningful cross-site exfiltration surface.
Consider narrowing the CORS origin to the gateway's own origin so only the Hadrian UI can reach the proxy. Since `buildInstallCommand` doesn't know the UI origin at call time, one approach is to derive the origin from `window.location.origin` and embed it:
```ts
const origin = window.location.origin;
command: `npx -y supergateway --cors "${origin}" --outputTransport streamableHttp --stdio ${quoted}`,
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (3): Last reviewed commit: "Review fixes" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| title: string; | ||
| icon: React.ReactNode; | ||
| hint: string; | ||
| footer?: React.ReactNode; | ||
| children: React.ReactNode; | ||
| }) { |
There was a problem hiding this comment.
React.ReactNode referenced without a React namespace import
Section's prop types reference React.ReactNode (lines 440, 441, 442), but the file only has named React-hook imports (import { useEffect, useRef, useState } from "react"). With the new JSX transform, React doesn't need to be in scope for JSX, but React.ReactNode as a type still requires either import React from "react" or the type written as ReactNode with import type { ReactNode } from "react". This will cause a TypeScript compile error in standard modern React project configurations.
| title: string; | |
| icon: React.ReactNode; | |
| hint: string; | |
| footer?: React.ReactNode; | |
| children: React.ReactNode; | |
| }) { | |
| import { type ReactNode, useEffect, useRef, useState } from "react"; |
Then update the three type annotations to use ReactNode instead of React.ReactNode.
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/components/MCPConfigModal/MCPCatalog.tsx
Line: 438-443
Comment:
**`React.ReactNode` referenced without a React namespace import**
`Section`'s prop types reference `React.ReactNode` (lines 440, 441, 442), but the file only has named React-hook imports (`import { useEffect, useRef, useState } from "react"`). With the new JSX transform, React doesn't need to be in scope for JSX, but `React.ReactNode` as a type still requires either `import React from "react"` or the type written as `ReactNode` with `import type { ReactNode } from "react"`. This will cause a TypeScript compile error in standard modern React project configurations.
```suggestion
import { type ReactNode, useEffect, useRef, useState } from "react";
```
Then update the three type annotations to use `ReactNode` instead of `React.ReactNode`.
How can I resolve this? If you propose a fix, please make it concise.
| // closing the quote, inserting an escaped quote, and reopening: `'\''`. | ||
| const quoted = `'${raw.replace(/'/g, "'\\''")}'`; | ||
| return { | ||
| command: `npx -y supergateway --cors "*" --outputTransport streamableHttp --stdio ${quoted}`, |
There was a problem hiding this comment.
--cors "*" exposes the local MCP server to every web origin
The generated supergateway command passes --cors "*", which instructs supergateway to respond with Access-Control-Allow-Origin: *. While the server binds to localhost, CORS headers are what the browser enforces — any website the user visits while this proxy is running can send fetch("http://localhost:8000/mcp", ...) requests and read the responses. Local MCP servers routinely hold API keys in environment variables and may have filesystem or shell access, making this a meaningful cross-site exfiltration surface.
Consider narrowing the CORS origin to the gateway's own origin so only the Hadrian UI can reach the proxy. Since buildInstallCommand doesn't know the UI origin at call time, one approach is to derive the origin from window.location.origin and embed it:
const origin = window.location.origin;
command: `npx -y supergateway --cors "${origin}" --outputTransport streamableHttp --stdio ${quoted}`,Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/services/mcpRegistry/client.ts
Line: 190
Comment:
**`--cors "*"` exposes the local MCP server to every web origin**
The generated supergateway command passes `--cors "*"`, which instructs supergateway to respond with `Access-Control-Allow-Origin: *`. While the server binds to `localhost`, CORS headers are what the *browser* enforces — any website the user visits while this proxy is running can send `fetch("http://localhost:8000/mcp", ...)` requests and read the responses. Local MCP servers routinely hold API keys in environment variables and may have filesystem or shell access, making this a meaningful cross-site exfiltration surface.
Consider narrowing the CORS origin to the gateway's own origin so only the Hadrian UI can reach the proxy. Since `buildInstallCommand` doesn't know the UI origin at call time, one approach is to derive the origin from `window.location.origin` and embed it:
```ts
const origin = window.location.origin;
command: `npx -y supergateway --cors "${origin}" --outputTransport streamableHttp --stdio ${quoted}`,
```
How can I resolve this? If you propose a fix, please make it concise.
1 participant
No description provided.