feat: add MCP bridge with dashboard connector panel - #4
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details🧰 Additional context used🧬 Code graph analysis (2)src/daemon.js (1)
bridge.js (1)
🪛 ast-grep (0.44.1)bridge.js[warning] 49-49: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use. (detect-non-literal-fs-filename) [warning] 75-75: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use. (detect-non-literal-fs-filename) 🔇 Additional comments (9)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a token-authenticated MCP JSON-RPC bridge, daemon lifecycle APIs, persistent bridge state, MCP task-source tracking, and dashboard controls for starting the bridge, viewing credentials, copying connection details, and identifying MCP-originated tasks. ChangesMCP bridge integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant Daemon
participant bridge.js
participant DashboardAPI
Dashboard->>Daemon: POST /api/bridge/start
Daemon->>bridge.js: Spawn bridge process
bridge.js-->>Daemon: Write heartbeat state
Dashboard->>bridge.js: Authenticated JSON-RPC tools/call
bridge.js->>DashboardAPI: Forward /api/state, /api/dispatch, or /api/loop
DashboardAPI-->>bridge.js: JSON response
bridge.js-->>Dashboard: MCP tool result or failure
Possibly related PRs
Comment |
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@bridge.js`:
- Around line 53-75: Update readToken and the corresponding token-reading path
around lines 161-167 so an existing token file must contain a non-empty trimmed
token; otherwise fail closed rather than returning or accepting an empty value.
Preserve the existing token-generation and EEXIST recovery behavior for valid
tokens, and ensure empty request credentials cannot authenticate.
- Around line 30-38: Update bridgePort to honor valid configured mcpBridge.port
values instead of replacing every non-5758 value with the default; validate the
configured port and fall back to 5758 only when invalid or absent. Apply the
same validation behavior in the corresponding daemon port logic in
src/daemon.js.
In `@public/index.html`:
- Around line 1140-1153: Separate the URL used for display from the
authenticated URL used by Copy: keep bridgeUrl() returning the local endpoint
while the token is hidden, but make the copy action use bridgeInfo.connectorUrl
whenever available regardless of mcpTokenVisible. Update the relevant copy
handler and preserve the existing fallback when no connector URL exists.
In `@src/daemon.js`:
- Around line 300-361: Convert the bridge lifecycle flow centered on
startBridge() and stopBridge() to asynchronous operations: have startBridge()
wait for confirmed bridge/server readiness and propagate asynchronous spawn
failures before reporting success, while stopBridge() await the child’s exit
with a bounded timeout before clearing the heartbeat or completing shutdown.
Update the related callers and running-state paths, including the additional
start/stop API locations, to await these transitions so immediate restarts
cannot race the old listener.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3e589e33-d1af-4d1d-bed1-bad9a39d7e9f
📒 Files selected for processing (5)
bridge.jsconfig.jsonpublic/index.htmlsrc/daemon.jssrc/store.js
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/daemon.js
[warning] 245-245: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(store.paths.bridge, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 278-278: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(store.paths.mcpToken, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
bridge.js
[warning] 462-462: Avoid using the initial state variable in setState
Context: setInterval(writeHeartbeat, 5000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[warning] 23-23: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(configPath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 54-54: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(tokenPath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 66-66: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(tokenPath, ${token}\n, { encoding: 'utf8', flag: 'wx', mode: 0o600 })
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 70-70: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(tokenPath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 78-83: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(heartbeatPath, ${JSON.stringify({ pid: process.pid, port: bridgePort(), startedAt, ts: new Date().toISOString(), }, null, 2)}\n, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 88-88: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(heartbeatPath, 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename)
[warning] 450-454: Use https protocol over http
Context: http.createServer((req, res) => {
handleRequest(req, res).catch((error) => {
sendJson(res, 500, rpcError(null, -32603, error.message || 'Internal error.'));
});
})
Note: [CWE-319] Cleartext Transmission of Sensitive Information. Security best practice.
(https-protocol-missing)
public/index.html
[warning] 1159-1159: Avoid using the initial state variable in setState
Context: setTimeout(resolve, ms)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
[error] 1189-1189: React's useState should not be directly called
Context: setTimeout(() => { if (button.isConnected) button.textContent = label; }, 1600)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 1150-1150: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: status.innerHTML = '' + (running ? 'running' : 'stopped')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🪛 HTMLHint (1.9.2)
public/index.html
[warning] 631-631: The type attribute must be present on elements.
(button-type-require)
[warning] 637-637: The type attribute must be present on
elements.(button-type-require)
[warning] 643-643: The type attribute must be present on
elements.(button-type-require)
[warning] 648-648: The type attribute must be present on
elements.(button-type-require)
[warning] 649-649: The type attribute must be present on
elements.(button-type-require)
🪛 OpenGrep (1.25.0)
bridge.js
[ERROR] 163-163: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (6)
src/daemon.js (2)
234-242: Apply the configured-port fix here as well.This duplicates the defect flagged in
bridge.jsLines 30-38; both helpers must accept valid configured ports.
30-30: LGTM!Also applies to: 244-298, 1409-1411, 1479-1481, 1512-1512, 1654-1654, 1787-1790, 1934-1934
config.json (1)
5-8: LGTM!src/store.js (1)
19-20: LGTM!Also applies to: 23-28, 40-47
bridge.js (1)
1-29: LGTM!Also applies to: 39-52, 76-160, 168-470
public/index.html (1)
179-200: LGTM!Also applies to: 310-317, 597-599, 630-654, 903-913, 1080-1081, 1134-1139, 1154-1213, 1316-1316, 1327-1327, 1425-1425, 1439-1439, 1705-1705, 1716-1716, 1762-1762, 1764-1765, 1774-1774, 1835-1836
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd41d7752c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d41ac64877
ℹ️ 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 participant
Adds a local MCP bridge and dashboard connector popover.
Review fixes applied: 0efa323, 9922ca1, d41ac64.