Skip to content

feat(mcp-proxy): add web dashboard for monitoring connected MCPs#16

Merged
ricardoraposo merged 11 commits into
mainfrom
feat/mcp-proxy-dashboard
Mar 16, 2026
Merged

feat(mcp-proxy): add web dashboard for monitoring connected MCPs#16
ricardoraposo merged 11 commits into
mainfrom
feat/mcp-proxy-dashboard

Conversation

@ricardoraposo

@ricardoraposo ricardoraposo commented Mar 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds a localhost HTTP dashboard (default port 9100) for real-time monitoring of connected MCP servers
  • Shows connection status (connected/error/connecting), registered tools with descriptions, error messages, and connection logs per upstream
  • Includes a /api/status JSON endpoint with auto-refresh every 5s
  • Configurable via MCP_PROXY_DASHBOARD_PORT env var

Changes

  • src/dashboard.ts (new): HTTP server serving an HTML dashboard and JSON API
  • src/connector.ts: Added UpstreamStatus tracking with per-upstream logs and getStatuses() method
  • src/types.ts: Added UpstreamStatus interface
  • src/server.ts: Wired dashboard into proxy lifecycle (start/stop)
  • src/index.ts: Export Dashboard class

Summary by CodeRabbit

  • New Features

    • Added a lightweight HTTP dashboard (default port 9100) with an HTML UI and a /api/status JSON endpoint showing upstreams, tools, recent logs, and audit activity.
    • Exposed upstream status data (transport, state, tool counts, recent logs) for easier monitoring.
  • Bug Fixes / Enhancements

    • Improved upstream error handling and real-time log capture (including runtime stderr) to surface clearer error messages and recent events.

Adds a localhost HTTP dashboard (default port 9100) that shows:
- Connected MCP servers with status (connected/error/connecting)
- Tools registered per upstream with descriptions
- Error messages and connection logs for broken upstreams
- Recent audit log entries

Auto-refreshes every 5 seconds via /api/status JSON endpoint.
@coderabbitai

coderabbitai Bot commented Mar 16, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@ricardoraposo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 49 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1fa8b365-7887-47f5-8a2b-509787ec1d50

📥 Commits

Reviewing files that changed from the base of the PR and between e6e869a and 7046ef5.

📒 Files selected for processing (1)
  • packages/mcp-proxy/src/connector.ts
📝 Walkthrough

Walkthrough

Adds upstream status tracking and per-upstream logs to the connector, introduces a lightweight HTTP dashboard (JSON + HTML) that surfaces statuses, tools and recent audit logs, and integrates the dashboard lifecycle into the proxy server.

Changes

Cohort / File(s) Summary
Types
packages/mcp-proxy/src/types.ts
Adds exported UpstreamStatus interface (name, transport, status, toolCount, optional error, logs).
Connector
packages/mcp-proxy/src/connector.ts
Adds statuses map, getStatuses() accessor, addLog() helper, stderr piping (pipeStderr) for stdio transports, initializes/updates per-upstream status on connect/success/failure, captures stack traces and log-derived error text, enforces max logs.
Dashboard
packages/mcp-proxy/src/dashboard.ts, packages/mcp-proxy/src/index.ts
New Dashboard class exposing /api/status and an HTML UI; added export in index. Aggregates connector statuses, tool registry data, and recent audit logs; configurable port (default 9100).
Server integration
packages/mcp-proxy/src/server.ts
Instantiates Dashboard in McpProxyServer, starts it on server start and stops it during cleanup; uses env var MCP_PROXY_DASHBOARD_PORT.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Dashboard
    participant Connector as McpConnectorManager
    participant Registry as ToolRegistry
    participant Logger as AuditLogger

    Client->>Dashboard: GET /api/status
    activate Dashboard
    Dashboard->>Connector: getStatuses()
    activate Connector
    Connector-->>Dashboard: UpstreamStatus[]
    deactivate Connector
    Dashboard->>Registry: query tools per upstream
    activate Registry
    Registry-->>Dashboard: tools list
    deactivate Registry
    Dashboard->>Logger: recent audit logs
    activate Logger
    Logger-->>Dashboard: log entries
    deactivate Logger
    Dashboard-->>Client: JSON status + tools + logs
    deactivate Dashboard
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nibble logs and hop in delight,
A dashboard glowing through the night,
Upstreams chatter, tools parade,
I tuck their stories in a neat brigade,
Hooray for visibility—hop, hop, hooray!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: adding a web dashboard for monitoring connected MCPs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/mcp-proxy-dashboard
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Pipe stdio transport stderr instead of inheriting it, so the actual
error output (e.g. auth failures, crash stack traces) shows up in
the dashboard logs instead of only in the parent terminal.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/mcp-proxy/src/dashboard.ts (1)

92-118: Missing error handling in the client-side fetch.

If /api/status fails (network error, non-JSON response), the dashboard will silently break without feedback to the user.

♻️ Suggested fix with error handling
 async function load(){
+  try{
   const r=await fetch('/api/status');
+  if(!r.ok)throw new Error('HTTP '+r.status);
   const d=await r.json();
   // ... rest of rendering
+  }catch(e){
+    document.getElementById('grid').innerHTML='<div class="card"><h2>Error loading status</h2><div class="error-msg">'+esc(e.message)+'</div></div>';
+  }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/dashboard.ts` around lines 92 - 118, The client-side
load() function calls fetch('/api/status') without error handling; wrap the
fetch/response parsing in a try/catch, verify response.ok before calling
response.json(), and handle JSON parse errors so the UI doesn’t silently break;
on error, set meaningful content into the existing DOM targets (e.g., grid
and/or audit elements) and log the error (use esc where needed) so users see a
friendly error message instead of a blank/failed dashboard.
packages/mcp-proxy/src/connector.ts (1)

17-17: Statuses map is not cleared during disconnectAll().

The statuses map retains entries after disconnectAll() is called (lines 134-141), which could cause stale status data to persist if the server is reused or if connectAll is called again without recreating the manager.

♻️ Suggested fix
  async disconnectAll(): Promise<void> {
    for (const [name, upstream] of this.upstreams) {
      try { await upstream.client.close(); } catch (e) {
        console.error(`[connector] Error disconnecting ${name}:`, e instanceof Error ? e.message : e);
      }
    }
    this.upstreams.clear();
+   this.statuses.clear();
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/connector.ts` at line 17, The statuses Map (private
readonly statuses) is not cleared when disconnectAll() runs, leaving stale
UpstreamStatus entries; update disconnectAll() (the method that iterates
upstream disconnect logic) to remove status entries as each upstream is
disconnected or simply call this.statuses.clear() at the end of disconnectAll()
so the map is empty for future connectAll() calls or manager reuse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/mcp-proxy/src/dashboard.ts`:
- Around line 16-29: The dashboard's start() creates an HTTP server bound to all
interfaces; update start() to bind only to localhost by passing a host argument
(e.g., '127.0.0.1' or 'localhost') into this.server.listen so the server is not
exposed externally, and make the host configurable with a secure default of
localhost; additionally, add simple access control for sensitive endpoints (the
request handler around /api/status and the HTML root that calls getData() and
getHtml())—for example require a configurable Basic Auth token or check an
Authorization header before returning data—to prevent unauthenticated remote
access.

In `@packages/mcp-proxy/src/server.ts`:
- Around line 51-56: The port passed to the Dashboard constructor is parsed with
parseInt(process.env.MCP_PROXY_DASHBOARD_PORT || "9100", 10) without validating
the result, so an invalid env value yields NaN and breaks server binding; update
the code where this.dashboard is constructed (the Dashboard(...) call) to parse
the env var into a number, verify Number.isInteger and that the value is within
valid TCP port range (1–65535), and if invalid fall back to the default 9100 (or
throw a clear error/log via this.logger) before passing the port into Dashboard
to ensure a safe, valid port is always used.

---

Nitpick comments:
In `@packages/mcp-proxy/src/connector.ts`:
- Line 17: The statuses Map (private readonly statuses) is not cleared when
disconnectAll() runs, leaving stale UpstreamStatus entries; update
disconnectAll() (the method that iterates upstream disconnect logic) to remove
status entries as each upstream is disconnected or simply call
this.statuses.clear() at the end of disconnectAll() so the map is empty for
future connectAll() calls or manager reuse.

In `@packages/mcp-proxy/src/dashboard.ts`:
- Around line 92-118: The client-side load() function calls fetch('/api/status')
without error handling; wrap the fetch/response parsing in a try/catch, verify
response.ok before calling response.json(), and handle JSON parse errors so the
UI doesn’t silently break; on error, set meaningful content into the existing
DOM targets (e.g., grid and/or audit elements) and log the error (use esc where
needed) so users see a friendly error message instead of a blank/failed
dashboard.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 62ca5b26-edee-4993-83f9-e6f25c2c6479

📥 Commits

Reviewing files that changed from the base of the PR and between fe844be and 4dc0447.

📒 Files selected for processing (5)
  • packages/mcp-proxy/src/connector.ts
  • packages/mcp-proxy/src/dashboard.ts
  • packages/mcp-proxy/src/index.ts
  • packages/mcp-proxy/src/server.ts
  • packages/mcp-proxy/src/types.ts

Comment on lines +16 to +29
start(): void {
this.server = createServer((req, res) => {
if (req.url === "/api/status") {
res.writeHead(200, { "Content-Type": "application/json", "Access-Control-Allow-Origin": "*" });
res.end(JSON.stringify(this.getData()));
return;
}
res.writeHead(200, { "Content-Type": "text/html" });
res.end(this.getHtml());
});
this.server.listen(this.port, () => {
console.error(`[dashboard] http://localhost:${this.port}`);
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Dashboard binds to all network interfaces without authentication.

The HTTP server listens on 0.0.0.0 by default (no host specified), exposing the dashboard and status API to any reachable network. This could leak sensitive information (tool configurations, error messages, audit logs) in shared or production environments.

Consider binding only to localhost or adding basic authentication.

🛡️ Suggested fix to bind to localhost only
-    this.server.listen(this.port, () => {
+    this.server.listen(this.port, "127.0.0.1", () => {
       console.error(`[dashboard] http://localhost:${this.port}`);
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
start(): void {
this.server = createServer((req, res) => {
if (req.url === "/api/status") {
res.writeHead(200, { "Content-Type": "application/json", "Access-Control-Allow-Origin": "*" });
res.end(JSON.stringify(this.getData()));
return;
}
res.writeHead(200, { "Content-Type": "text/html" });
res.end(this.getHtml());
});
this.server.listen(this.port, () => {
console.error(`[dashboard] http://localhost:${this.port}`);
});
}
start(): void {
this.server = createServer((req, res) => {
if (req.url === "/api/status") {
res.writeHead(200, { "Content-Type": "application/json", "Access-Control-Allow-Origin": "*" });
res.end(JSON.stringify(this.getData()));
return;
}
res.writeHead(200, { "Content-Type": "text/html" });
res.end(this.getHtml());
});
this.server.listen(this.port, "127.0.0.1", () => {
console.error(`[dashboard] http://localhost:${this.port}`);
});
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/dashboard.ts` around lines 16 - 29, The dashboard's
start() creates an HTTP server bound to all interfaces; update start() to bind
only to localhost by passing a host argument (e.g., '127.0.0.1' or 'localhost')
into this.server.listen so the server is not exposed externally, and make the
host configurable with a secure default of localhost; additionally, add simple
access control for sensitive endpoints (the request handler around /api/status
and the HTML root that calls getData() and getHtml())—for example require a
configurable Basic Auth token or check an Authorization header before returning
data—to prevent unauthenticated remote access.

Comment on lines +51 to +56
this.dashboard = new Dashboard(
this.connector,
this.registry,
this.logger,
parseInt(process.env.MCP_PROXY_DASHBOARD_PORT || "9100", 10)
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Port parsing lacks validation for invalid values.

If MCP_PROXY_DASHBOARD_PORT is set to a non-numeric string, parseInt returns NaN, which would cause the HTTP server to fail silently or bind to an unexpected port.

🛡️ Suggested fix with validation
-    this.dashboard = new Dashboard(
-      this.connector,
-      this.registry,
-      this.logger,
-      parseInt(process.env.MCP_PROXY_DASHBOARD_PORT || "9100", 10)
-    );
+    const dashboardPort = parseInt(process.env.MCP_PROXY_DASHBOARD_PORT || "9100", 10);
+    if (Number.isNaN(dashboardPort) || dashboardPort < 1 || dashboardPort > 65535) {
+      throw new ProxyError("Invalid MCP_PROXY_DASHBOARD_PORT", "INVALID_CONFIG");
+    }
+    this.dashboard = new Dashboard(
+      this.connector,
+      this.registry,
+      this.logger,
+      dashboardPort
+    );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.dashboard = new Dashboard(
this.connector,
this.registry,
this.logger,
parseInt(process.env.MCP_PROXY_DASHBOARD_PORT || "9100", 10)
);
const dashboardPort = parseInt(process.env.MCP_PROXY_DASHBOARD_PORT || "9100", 10);
if (Number.isNaN(dashboardPort) || dashboardPort < 1 || dashboardPort > 65535) {
throw new ProxyError("Invalid MCP_PROXY_DASHBOARD_PORT", "INVALID_CONFIG");
}
this.dashboard = new Dashboard(
this.connector,
this.registry,
this.logger,
dashboardPort
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/server.ts` around lines 51 - 56, The port passed to
the Dashboard constructor is parsed with
parseInt(process.env.MCP_PROXY_DASHBOARD_PORT || "9100", 10) without validating
the result, so an invalid env value yields NaN and breaks server binding; update
the code where this.dashboard is constructed (the Dashboard(...) call) to parse
the env var into a number, verify Number.isInteger and that the value is within
valid TCP port range (1–65535), and if invalid fall back to the default 9100 (or
throw a clear error/log via this.logger) before passing the port into Dashboard
to ensure a safe, valid port is always used.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
packages/mcp-proxy/src/connector.ts (1)

29-31: Return defensive copies from getStatuses() to avoid external mutation.

Current code exposes internal status objects/arrays by reference, so external callers can mutate connector state indirectly.

💡 Proposed fix
   getStatuses(): UpstreamStatus[] {
-    return Array.from(this.statuses.values());
+    return Array.from(this.statuses.values(), (s) => ({
+      ...s,
+      logs: [...s.logs],
+    }));
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/connector.ts` around lines 29 - 31, getStatuses()
currently returns references to internal UpstreamStatus objects stored in
this.statuses, allowing external mutation; modify getStatuses() to return
defensive copies by mapping Array.from(this.statuses.values()) to a new array
and cloning each UpstreamStatus (e.g., shallow copy each status object) before
returning, so callers receive independent objects and cannot mutate the
connector's internal state.
packages/mcp-proxy/src/dashboard.ts (1)

16-36: Make start() idempotent (or explicitly reject double-start).

If start() is called twice, this.server gets overwritten and lifecycle/port handling becomes inconsistent.

💡 Proposed fix
   start(): void {
+    if (this.server) return;
     this.server = createServer((req, res) => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/dashboard.ts` around lines 16 - 36, start() currently
overwrites this.server when called multiple times causing lifecycle/port
handling bugs; make start() idempotent or explicitly reject double-start by
checking for an existing active server before creating a new one (use a boolean
flag like this.started or test this.server and its listening state), and if
already started either return early or throw an error; ensure the existing
server/error handler (server.on("error") and server.listen) are only set up once
and that this.port increment/retry logic operates on the single server instance
(refer to start(), this.server, server.on("error"), server.listen, and
this.port).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/mcp-proxy/src/connector.ts`:
- Around line 101-104: The code currently mirrors upstream stderr via
console.error in the connector where this.addLog(name, line) is called; stop
directly printing raw `line` to process stderr — instead create/ reuse a
redactSensitiveInfo(line) helper and use a debug flag (e.g. this.options.debug
or process.env.DEBUG) to gate console output; keep this.addLog(name, line) for
storing the raw or processed trace as required, but before any external logging
run line through redactSensitiveInfo and only call console.error or
console.debug when the debug flag is enabled, ensuring any redaction of tokens,
emails, or IPs is applied consistently (update all places that call
console.error with the same pattern, including the other block that mirrors
stderr).
- Around line 91-113: pipeStderr currently listens only for "data" and "end" on
transport.stderr, which can emit "error" and crash the process; add a
stream.on("error", ...) handler inside pipeStderr (where stream is derived from
transport.stderr) that captures the Error, calls this.addLog(name, error.message
|| String(error)) and console.error(`[${name}]`, error) (or similar safe
logging), so I/O errors are handled and won't cause an unhandled exception;
reference the pipeStderr method, the stream variable, transport.stderr and
this.addLog when making the change.

In `@packages/mcp-proxy/src/dashboard.ts`:
- Line 19: The response in the /api/status handler currently sets
Access-Control-Allow-Origin: "*" (via the res.writeHead call in
packages/mcp-proxy/src/dashboard.ts); change this to avoid the wildcard by
either removing the header for same-origin use or restricting it to a safe
origin (e.g., read ALLOWED_ORIGIN from config/env and only set
Access-Control-Allow-Origin to that value when the request Origin matches), and
ensure the logic is implemented in the same handler that calls res.writeHead so
untrusted web pages cannot read dashboard status/logs.

---

Nitpick comments:
In `@packages/mcp-proxy/src/connector.ts`:
- Around line 29-31: getStatuses() currently returns references to internal
UpstreamStatus objects stored in this.statuses, allowing external mutation;
modify getStatuses() to return defensive copies by mapping
Array.from(this.statuses.values()) to a new array and cloning each
UpstreamStatus (e.g., shallow copy each status object) before returning, so
callers receive independent objects and cannot mutate the connector's internal
state.

In `@packages/mcp-proxy/src/dashboard.ts`:
- Around line 16-36: start() currently overwrites this.server when called
multiple times causing lifecycle/port handling bugs; make start() idempotent or
explicitly reject double-start by checking for an existing active server before
creating a new one (use a boolean flag like this.started or test this.server and
its listening state), and if already started either return early or throw an
error; ensure the existing server/error handler (server.on("error") and
server.listen) are only set up once and that this.port increment/retry logic
operates on the single server instance (refer to start(), this.server,
server.on("error"), server.listen, and this.port).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2485be46-f676-4149-a675-d8adbd1301eb

📥 Commits

Reviewing files that changed from the base of the PR and between 4dc0447 and e6e869a.

📒 Files selected for processing (2)
  • packages/mcp-proxy/src/connector.ts
  • packages/mcp-proxy/src/dashboard.ts

Comment on lines +91 to +113
private pipeStderr(name: string, transport: StdioClientTransport): void {
const stream = transport.stderr as unknown as Readable | null;
if (!stream || typeof stream.on !== "function") return;
let buf = "";
stream.on("data", (chunk: Buffer | string) => {
buf += chunk.toString();
let nl: number;
while ((nl = buf.indexOf("\n")) !== -1) {
const line = buf.slice(0, nl).trimEnd();
buf = buf.slice(nl + 1);
if (line) {
this.addLog(name, line);
console.error(`[${name}] ${line}`);
}
}
});
stream.on("end", () => {
if (buf.trim()) {
this.addLog(name, buf.trim());
console.error(`[${name}] ${buf.trim()}`);
}
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether pipeStderr has an "error" listener in connector.ts
rg -n -C3 'pipeStderr|stream\.on\("error"' packages/mcp-proxy/src/connector.ts

Repository: arvoreeducacao/arvore-mcp-servers

Length of output: 579


🏁 Script executed:

#!/bin/bash
# Get the complete pipeStderr implementation
sed -n '91,113p' packages/mcp-proxy/src/connector.ts

Repository: arvoreeducacao/arvore-mcp-servers

Length of output: 821


🏁 Script executed:

#!/bin/bash
# Check if there's any error handling around pipeStderr calls
rg -n -B5 -A5 'pipeStderr' packages/mcp-proxy/src/connector.ts | head -40

Repository: arvoreeducacao/arvore-mcp-servers

Length of output: 730


🏁 Script executed:

#!/bin/bash
# Check if there's a try-catch in the connect method or elsewhere that might handle stream errors
rg -n -B10 -A10 'pipeStderr|await client\.connect' packages/mcp-proxy/src/connector.ts | head -50

Repository: arvoreeducacao/arvore-mcp-servers

Length of output: 2189


Add error handler to stderr stream to prevent unhandled process crashes.

pipeStderr only subscribes to "data" and "end" events. Node.js streams emit "error" on I/O failures (permission issues, closed descriptors, etc.), and an unhandled "error" event crashes the process. Add an error listener:

  stream.on("end", () => {
    if (buf.trim()) {
      this.addLog(name, buf.trim());
      console.error(`[${name}] ${buf.trim()}`);
    }
  });
+ stream.on("error", (err: Error) => {
+   const msg = `stderr stream error: ${err.message}`;
+   this.addLog(name, msg);
+   console.error(`[${name}] ${msg}`);
+ });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/connector.ts` around lines 91 - 113, pipeStderr
currently listens only for "data" and "end" on transport.stderr, which can emit
"error" and crash the process; add a stream.on("error", ...) handler inside
pipeStderr (where stream is derived from transport.stderr) that captures the
Error, calls this.addLog(name, error.message || String(error)) and
console.error(`[${name}]`, error) (or similar safe logging), so I/O errors are
handled and won't cause an unhandled exception; reference the pipeStderr method,
the stream variable, transport.stderr and this.addLog when making the change.

Comment on lines +101 to +104
if (line) {
this.addLog(name, line);
console.error(`[${name}] ${line}`);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid mirroring raw upstream stderr directly to process logs.

Forwarding every stderr line to console.error can leak credentials/PII/secrets from upstream traces into centralized logs. Prefer redaction and/or a debug flag gate.

Also applies to: 108-111

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/connector.ts` around lines 101 - 104, The code
currently mirrors upstream stderr via console.error in the connector where
this.addLog(name, line) is called; stop directly printing raw `line` to process
stderr — instead create/ reuse a redactSensitiveInfo(line) helper and use a
debug flag (e.g. this.options.debug or process.env.DEBUG) to gate console
output; keep this.addLog(name, line) for storing the raw or processed trace as
required, but before any external logging run line through redactSensitiveInfo
and only call console.error or console.debug when the debug flag is enabled,
ensuring any redaction of tokens, emails, or IPs is applied consistently (update
all places that call console.error with the same pattern, including the other
block that mirrors stderr).

start(): void {
this.server = createServer((req, res) => {
if (req.url === "/api/status") {
res.writeHead(200, { "Content-Type": "application/json", "Access-Control-Allow-Origin": "*" });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Drop wildcard CORS from /api/status (localhost data exfil risk).

Access-Control-Allow-Origin: "*" allows any website opened in the browser to read local dashboard data and exfiltrate status/log content. Restrict origin or omit CORS for same-origin use.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/mcp-proxy/src/dashboard.ts` at line 19, The response in the
/api/status handler currently sets Access-Control-Allow-Origin: "*" (via the
res.writeHead call in packages/mcp-proxy/src/dashboard.ts); change this to avoid
the wildcard by either removing the header for same-origin use or restricting it
to a safe origin (e.g., read ALLOWED_ORIGIN from config/env and only set
Access-Control-Allow-Origin to that value when the request Origin matches), and
ensure the logic is implemented in the same handler that calls res.writeHead so
untrusted web pages cannot read dashboard status/logs.

Comment thread packages/mcp-proxy/src/connector.ts Outdated
Comment on lines +65 to +67
// Use captured stderr logs as the error if available — they contain
// the actual reason (auth failures, missing config, crash traces).
// Strip the timestamp prefix from log lines for readability.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Use captured stderr logs as the error if available — they contain
// the actual reason (auth failures, missing config, crash traces).
// Strip the timestamp prefix from log lines for readability.

@ricardoraposo ricardoraposo merged commit 92cbd5f into main Mar 16, 2026
5 checks passed
@Joao208 Joao208 deleted the feat/mcp-proxy-dashboard branch April 4, 2026 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants