Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 7 Skipped Deployments
|
![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: 64694656d5
ℹ️ 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".
| ["get_symbols", "get_historical_price", "get_candlestick_data", "get_latest_price"].map( | ||
| (name) => [name, (arg: unknown) => hostCall(name, arg)], | ||
| ), |
There was a problem hiding this comment.
The codemode methods are host-realm functions injected directly into untrusted execute code, which lets callers escape the sandbox by using a function constructor chain (for example codemode.get_symbols.constructor("return process.env")()). In the public Code Mode endpoint this gives arbitrary access to process, environment variables (including PYTH_PRO_ACCESS_TOKEN), and host capabilities, so token injection is no longer isolated to server-side code.
Useful? React with 👍 / 👎.
apps/mcp/src/codemode/executor.ts
Outdated
| const result = await runInNewContext(wrapped, sandbox, { | ||
| timeout: timeoutMs, | ||
| }); |
There was a problem hiding this comment.
The timeout passed to runInNewContext only limits synchronous script execution; if user code returns a never-settling promise (for example await new Promise(() => {})), await runInNewContext(...) can hang indefinitely. Because execute accepts untrusted code, this allows callers to pin requests and degrade service capacity despite the configured timeoutMs.
Useful? React with 👍 / 👎.
apps/mcp/src/codemode/registry.ts
Outdated
| : JSON.stringify(result.result); | ||
| const resultSizeBytes = Buffer.byteLength(resultText); |
There was a problem hiding this comment.
When executed code does not explicitly return a value, result.result is undefined, so JSON.stringify(result.result) also returns undefined; passing that into Buffer.byteLength throws a type error and turns an otherwise valid execution into an internal failure. This is easy to trigger with common snippets that omit return, so execute should normalize undefined results before logging/response construction.
Useful? React with 👍 / 👎.
Document a Cloudflare-first Code Mode path so API evolution does not require frequent MCP tool reshaping. Capture token-injection security and observability requirements to guide rollout.
- Add codemode executor (isolated-vm, 30s timeout, no network) - Add codemode bindings: search, execute, get_latest_price (token-injected) - Add createServerCodeModeOnly and pyth-mcp-codemode entrypoint - Add pythProAccessToken config and PYTH_PRO_ACCESS_TOKEN env - Add redact utils and logger serializers for token/sensitive data - Add unit, integration, and security tests for Code Mode Made-with: Cursor
Simplify search tool (remove code execution path), include full types in execute description, fix redactSecrets false positives on keys like author/authority/token_count, extract shared setupProcessCleanup in server.ts, complete return types in types.ts, and add sandbox boundary tests for timeout/process/require. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sults
- Sever prototype chain on injected host functions to prevent sandbox
escape via .constructor("return process")()
- Race vm execution with a timer so never-settling promises also respect
the configured timeoutMs
- Normalize undefined results before Buffer.byteLength to avoid TypeError
when user code omits a return statement
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b188620186
ℹ️ 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".
apps/mcp/src/codemode/executor.ts
Outdated
| const codemode = Object.fromEntries( | ||
| ["get_symbols", "get_historical_price", "get_candlestick_data", "get_latest_price"].map( | ||
| (name) => { | ||
| const fn = (arg: unknown) => hostCall(name, arg); |
There was a problem hiding this comment.
Setting Object.setPrototypeOf(fn, null) blocks codemode.*.constructor, but untrusted code can still escape through host objects returned by hostCall: const x = await codemode.get_symbols({}); x.constructor.constructor("return process")() yields host process in Node vm. Because bindings return plain host-realm objects, this keeps arbitrary environment/process access possible even after the function-constructor hardening.
Useful? React with 👍 / 👎.
apps/mcp/package.json
Outdated
| "build": "tsup src/index.ts src/http.ts --format esm --dts --clean", | ||
| "build": "tsup src/index.ts src/index-codemode.ts --format esm --dts --clean", |
There was a problem hiding this comment.
The scripts object defines build twice, and JSON keeps only the last one, so src/http.ts is no longer built. start still runs node dist/http.js, so the normal build → start flow now fails at runtime because dist/http.js is missing.
Useful? React with 👍 / 👎.
Code mode uses node:vm instead of isolated-vm. The lockfile still referenced the removed dependency, causing CI frozen-lockfile failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Host-realm async functions return host-realm Promises whose prototype
chain allows escape: p.constructor.constructor("return process")().
Replace async binding functions with a bridge function created inside
the sandbox context that produces sandbox-realm Promises, preventing
prototype chain traversal back to host Function/process.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| }); | ||
|
|
||
| const trimmed = code.trim(); | ||
| const isFnExpr = /^async\s+(?:\(|function\b)/.test(trimmed); |
There was a problem hiding this comment.
🟡 Regex fails to detect async() arrow functions without space, causing incorrect code wrapping
The isFnExpr regex /^async\s+(?:\(|function\b)/ requires one or more whitespace characters (\s+) between async and (. When an LLM generates async(params) => { ... } (valid JavaScript with no space), the regex doesn't match, so the executor wraps it as (async () => { async(params) => { ... } })() instead of (async(params) => { ... })(). This produces a syntax error because async(params) is parsed as a function call to an undefined async identifier, followed by an invalid => token. The fix is to use \s* for the ( alternative while keeping \s+ for function: /^async(?:\s*\(|\s+function\b)/.
| const isFnExpr = /^async\s+(?:\(|function\b)/.test(trimmed); | |
| const isFnExpr = /^async(?:\s*\(|\s+function\b)/.test(trimmed); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| Code Mode exposes these via `codemode.get_symbols`, `codemode.get_latest_price`, etc. inside `execute()`. | ||
|
|
||
| ## Setup |
There was a problem hiding this comment.
🟡 README has misplaced Code Mode text under ### Build and broken heading hierarchy with ## Setup
The ### Build subsection (under ## Local Development) now contains an orphaned Code Mode description sentence (Code Mode exposes these via...) instead of the build command. The build command was moved into a new ## Setup top-level section, which breaks the document hierarchy — ## Setup sits at the same level as ## Local Development but the build instructions logically belong under ### Build. The Code Mode sentence appears to belong in the ## Modes or ## Tools (Legacy mode) section instead.
Prompt for agents
In apps/mcp/README.md, lines 98-101, the sentence 'Code Mode exposes these via codemode.get_symbols, codemode.get_latest_price, etc. inside execute().' appears to be misplaced under the '### Build' heading. It should be moved to the '## Modes' section (around line 51-64) or the '## Tools (Legacy mode)' section (around line 66). Additionally, the '## Setup' heading on line 101 should be removed or changed to '### Build' to restore the original subsection hierarchy under '## Local Development'. The build command on line 103-105 should remain under the '### Build' subsection.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Rationale
How has this been tested?