Conversation
lifi-action-bot
changed the title
WalkthroughThis PR introduces comprehensive MCP (Model Context Protocol) infrastructure to the project, including three new MCP server implementations (Foundry, Tenderly, Explorer), configuration files, environment templates, and extensive documentation. It updates command workflows to prioritize MCP-first approaches and adds dependencies for MCP SDK, tsx, and zod. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
19-57: Update engines field:@modelcontextprotocol/sdkand tsx both require Node >= 18.The package.json currently specifies
"node": ">= 12.18.0", but@modelcontextprotocol/sdkv1.25.2 and tsx v4.21.0 both require Node >= 18. Update the engines field to"node": ">= 18"to reflect the actual minimum Node version needed by these new dependencies.
🤖 Fix all issues with AI agents
In `@docs/MCP.md`:
- Around line 116-118: The docs mention the wrong file extension: replace
references to "foundry.server.mjs" with the actual filename "foundry.server.ts"
in the MCP documentation so the example and instructions match the repository;
search for the string "foundry.server.mjs" and update it to "foundry.server.ts"
wherever it appears (keeping surrounding wording intact) so other doc references
remain consistent.
In `@script/mcp/explorer.server.ts`:
- Around line 430-479: Add regex validation to the txHash parameter schemas for
explorer proxy and cast tools: update the txHash z.string() schema in the
explorer_proxy_tx_by_hash and explorer_proxy_tx_receipt_by_hash server.tool
definitions (in explorer.server.ts) and in the cast_tx_by_hash and
cast_receipt_by_hash tools (in foundry.server.ts) to include a
.regex(/^0x[a-fA-F0-9]{64}$/) (with a clear .describe message) so only valid
0x-prefixed 64-hex-character transaction hashes are accepted.
- Around line 248-267: In the smokeTest function replace all console.error
usages with consola.error and remove the corresponding eslint-disable-next-line
no-console comments; add an import for consola from 'consola' at the top of the
file so the smokeTest function (and any other places using console.error in this
file) call consola.error(...) instead of console.error(...). Ensure you update
the three console.error calls inside smokeTest and any similar console.error
usages elsewhere in the file so they use consola.error and no longer rely on
eslint-disable comments.
- Around line 618-689: The explorer_get_logs handler currently allows arbitrary
fromBlock/toBlock and topic0 values which can create malformed API requests; add
strict validation in the server.tool schema for explorer_get_logs: constrain
fromBlock/toBlock to either the string "latest" or a non-negative integer (use
z.union of z.literal('latest') and z.preprocess to coerce/validate numeric input
and refine to Number.isInteger & >=0), and validate topic0 with a regex
refinements (z.string().optional().refine(s => /^0x[a-fA-F0-9]{64}$/.test(s))).
If validation fails, return the same structured error response used elsewhere
(content with a text message) before calling resolveExplorerApi/buildUrl so
invalid values never reach buildUrl or httpsJson.
- Around line 308-324: The baseExplorerArgsSchema needs stricter validation for
external inputs: update the network field in baseExplorerArgsSchema (the
z.string() for key "network") to reject empty strings by adding .min(1), and
update the chainId field (the z.number() for key "chainId") to enforce integer
positive IDs by adding .int().positive(); keep both as .optional() but apply
these constraints so empty/invalid values are rejected upfront.
- Around line 143-178: The code reads process.env.EXPLORER_NETWORK directly in
resolveNetwork; add an explicit optional env validation layer instead: create or
call an optional validator (e.g., getOptionalEnvVar or validateOptionalEnv) to
read EXPLORER_NETWORK and return undefined when empty/invalid rather than
accessing process.env directly, then use that validated value as fallbackNetwork
in resolveNetwork; also apply the same optional validation for
EXPLORER_API_BASE_URL and EXPLORER_API_KEY where they are used so env values are
normalized/checked (e.g., ensure returned string is non-empty and matches
expected keys before using in logic within resolveNetwork and related
functions).
In `@script/mcp/foundry.server.ts`:
- Around line 79-125: The forge_test tool (server.tool 'forge_test') currently
forwards user-supplied extraArgs directly into run('forge', args), which allows
dangerous flags like --ffi; update the handler to validate/sanitize extraArgs
before appending: implement a blocklist (e.g., ['--ffi', '--allow-remote-exec',
...]) or an allowlist of safe flags and reject or strip any disallowed entries,
returning a clear error to the caller; apply the same validation logic to the
other MCP server tools mentioned (forge_build, cast_tx, cast_receipt) by
factoring the check into a shared helper (e.g.,
validateForgeArgs/validateCastArgs) that is called before
args.push(...extraArgs) so dangerous flags are never passed to run().
- Around line 33-46: The smokeTest function is using console.error and an
eslint-disable comment; update it to use consola.error instead and remove the
eslint-disable comment: locate the smokeTest function (which awaits run('forge',
...) and run('cast', ...), referencing the forge and cast result variables) and
replace the console.error call with consola.error(...) while removing the
preceding eslint-disable-next-line no-console so the script follows logging
conventions for scripts/tasks.
- Around line 175-245: Replace direct env access and loose address handling in
the cast_call tool: use the shared getEnvVar() helper to fetch RPC URL (instead
of process.env[envVar]) and fail with the helper error if missing; tighten the
request validation by updating the zod schema for the "to" field to validate
Ethereum addresses (or call viem's getAddress() inside the handler) so malformed
addresses are rejected before calling cast. Locate the cast_call server.tool
handler and change RPC lookup to getEnvVar(rpcUrlEnvVar ?? 'RPC_URL'), and
validate/normalize the "to" value via getAddress(to) (or a zod refinment using
viem's getAddress) before constructing callArgs and running run('cast', ...).
In `@script/mcp/tenderly.server.ts`:
- Around line 17-59: The httpsJson helper lacks a request timeout, so add
timeout protection inside the httpsJson function: set a timeout (e.g. using
req.setTimeout or a manual timer) after creating the `req` in `httpsJson`, and
on timeout destroy the request and reject the Promise with a clear timeout
error; ensure you clear the timer on `res.on('end')` and on `req.on('error')` to
avoid leaks, and keep existing behavior for sending `body` and resolving with `{
statusCode, headers, raw, json }` in the `res` end handler.
♻️ Duplicate comments (10)
script/mcp/foundry.server.ts (4)
247-310: Same RPC input/env validation concern ascast_call.
As per coding guidelines.
312-375: Same RPC input/env validation concern ascast_call.
As per coding guidelines.
377-382: Same logging standard issue as smokeTest.
Useconsolafor startup logs. As per coding guidelines.
384-387: Same logging standard issue as smokeTest.
Useconsola.errorfor fatal logging. As per coding guidelines.script/mcp/explorer.server.ts (6)
180-246: Same env validation concern asresolveNetwork.
As per coding guidelines.
378-427: Same address validation concern asexplorer_get_abi.
As per coding guidelines.
482-532: Same txHash validation concern asexplorer_proxy_tx_by_hash.
As per coding guidelines.
534-615: Same address validation concern asexplorer_get_abi.
As per coding guidelines.
692-697: Same logging standard issue as smokeTest.
Useconsolafor startup logs. As per coding guidelines.
699-702: Same logging standard issue as smokeTest.
Useconsola.errorfor fatal logging. As per coding guidelines.
🧹 Nitpick comments (5)
script/mcp/run.ts (1)
41-49: Spawning without shell is secure.Good choice to not use
shell: truewhich prevents shell injection vulnerabilities. The exit handling correctly propagates both exit codes and signal terminations.Consider whether a timeout might be useful for debugging scenarios where a command hangs, though this may be intentionally omitted for long-running MCP servers.
.cursor/mcp.json (1)
18-50: Consider pinning third-party MCP server versions.Using
@latestfor jira, notion, and slack servers means these could break unexpectedly when packages are updated. Pinning to specific versions would improve reproducibility.💡 Example: pin a specific version
"jira": { "command": "bunx", "args": [ "tsx", "script/mcp/run.ts", "--", "npx", "-y", - "@aashari/mcp-server-atlassian-jira@latest" + "@aashari/mcp-server-atlassian-jira@1.0.0" ] },However, given that MCP is emerging technology with rapid updates, using
@latestmay be intentional to stay current. This is a tradeoff between stability and getting new features/fixes.script/mcp/tenderly.server.ts (1)
349-359: Main function and error handling are correct.Proper error propagation with
process.exit(1)on fatal errors. The smoke test path is cleanly separated from normal operation.Note: Per coding guidelines, new TypeScript helpers should have colocated
*.test.tsfiles. Consider addingtenderly.server.test.tsto cover the helper functions (httpsJson,collectCallTraceNodes, etc.).script/mcp/foundry.server.ts (1)
10-31: Add a timeout/output cap for spawned commands.
A hungforge/castprocess can stall the MCP server and unbounded output can grow memory; add a kill timer (or centralize this in the shared runner) to keep the server responsive.♻️ Example timeout guard
return new Promise((resolve, reject) => { const child = spawn(cmd, args, { cwd, stdio: ['ignore', 'pipe', 'pipe'], env: process.env, }) + const timeoutMs = 5 * 60_000 + const killTimer = setTimeout(() => child.kill('SIGKILL'), timeoutMs) let stdout = '' let stderr = '' child.stdout.on('data', (d) => (stdout += d.toString())) child.stderr.on('data', (d) => (stderr += d.toString())) child.on('error', reject) - child.on('close', (code) => resolve({ code: code ?? 1, stdout, stderr })) + child.on('close', (code) => { + clearTimeout(killTimer) + resolve({ code: code ?? 1, stdout, stderr }) + }) }) }script/mcp/explorer.server.ts (1)
34-66: Add timeout/size limits to outbound HTTPS calls.
Without guards, a slow or large response can stall the MCP server or blow memory.♻️ Example hardening
return new Promise((resolve, reject) => { const u = new URL(url) const req = https.request( @@ (res) => { let raw = '' - res.on('data', (d) => (raw += d.toString())) + const MAX_BYTES = 2 * 1024 * 1024 + res.on('data', (d) => { + raw += d.toString() + if (raw.length > MAX_BYTES) { + req.destroy(new Error('Explorer response too large')) + } + }) @@ ) + req.setTimeout(10_000, () => + req.destroy(new Error('Explorer request timeout')) + ) req.on('error', reject) req.end() }) }
| - **Repo-owned servers fail immediately**: | ||
| - Ensure you ran `bun install` | ||
| - Ensure you have required local tooling (e.g. Foundry for `foundry.server.mjs`) |
There was a problem hiding this comment.
Minor inconsistency: file extension mismatch.
Line 118 references foundry.server.mjs, but the actual file is foundry.server.ts based on the PR context and other documentation references.
📝 Suggested fix
- **Repo-owned servers fail immediately**:
- Ensure you ran `bun install`
- - Ensure you have required local tooling (e.g. Foundry for `foundry.server.mjs`)
+ - Ensure you have required local tooling (e.g. Foundry for `foundry.server.ts`)🤖 Prompt for AI Agents
In `@docs/MCP.md` around lines 116 - 118, The docs mention the wrong file
extension: replace references to "foundry.server.mjs" with the actual filename
"foundry.server.ts" in the MCP documentation so the example and instructions
match the repository; search for the string "foundry.server.mjs" and update it
to "foundry.server.ts" wherever it appears (keeping surrounding wording intact)
so other doc references remain consistent.
| type ResolveNetworkResult = | ||
| | { ok: true; key: string; net: INetworkConfig } | ||
| | { ok: false; error: string } | ||
|
|
||
| function resolveNetwork({ | ||
| network, | ||
| chainId, | ||
| }: { | ||
| network?: string | ||
| chainId?: number | ||
| }): ResolveNetworkResult { | ||
| const fallbackNetwork = process.env.EXPLORER_NETWORK | ||
| const effectiveNetwork = network ?? fallbackNetwork | ||
|
|
||
| if (effectiveNetwork && networks[effectiveNetwork]) { | ||
| return { | ||
| ok: true, | ||
| key: effectiveNetwork, | ||
| net: asNetworkConfig(networks[effectiveNetwork]), | ||
| } | ||
| } | ||
|
|
||
| if (chainId !== undefined && chainId !== null) { | ||
| const cid = Number(chainId) | ||
| for (const [k, v] of Object.entries(networks)) { | ||
| const net = asNetworkConfig(v) | ||
| if (Number(net.chainId) === cid) return { ok: true, key: k, net } | ||
| } | ||
| return { ok: false, error: `Unknown chainId: ${cid}` } | ||
| } | ||
|
|
||
| return { | ||
| ok: false, | ||
| error: 'Missing network/chainId (set tool args or EXPLORER_NETWORK)', | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for getEnvVar helper definition
rg "export.*getEnvVar|function getEnvVar|const getEnvVar" --type tsRepository: lifinance/contracts
Length of output: 182
🏁 Script executed:
# Check if getEnvVar is imported/used in explorer.server.ts
fd explorer.server.ts -x cat {} | head -50Repository: lifinance/contracts
Length of output: 182
🏁 Script executed:
# Look for env var validation patterns in script/ directory
rg "getEnvVar\(" --type ts -l | head -10Repository: lifinance/contracts
Length of output: 526
🏁 Script executed:
# Check the full structure of script/mcp/explorer.server.ts
fd explorer.server.tsRepository: lifinance/contracts
Length of output: 92
🏁 Script executed:
# Read the explorer.server.ts file to see the implementation
cat -n script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 23264
🏁 Script executed:
# Find getEnvVar definition
rg -A 5 "^export (function|const) getEnvVar" --type tsRepository: lifinance/contracts
Length of output: 534
🏁 Script executed:
# Search for where getEnvVar is typically located in script utils
fd -t f "*.ts" script/utils script/common -x grep -l "getEnvVar" {}Repository: lifinance/contracts
Length of output: 346
🏁 Script executed:
# Search for optional env var helpers
rg "getOptional|getEnvVarOptional|getEnvVar.*optional" --type ts -A 3Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Check if there are other files in script/mcp/ directory
fd -t f "*.ts" script/mcp/Repository: lifinance/contracts
Length of output: 346
🏁 Script executed:
# Look for any env var validation patterns in the codebase
rg "process\.env\[" --type ts -B 2 -A 1 | head -40Repository: lifinance/contracts
Length of output: 2361
🏁 Script executed:
# Check .eslintrc configuration for script paths
grep -A 5 -B 5 "script" .eslintrc.cjs 2>/dev/null || echo "File not found or no matches"Repository: lifinance/contracts
Length of output: 3465
🏁 Script executed:
# Check if there's an optional variant of getEnvVar or similar helper
rg "export.*getEnv|export.*readEnv" --type ts -A 3 script/Repository: lifinance/contracts
Length of output: 715
🏁 Script executed:
# Look at foundry.server.ts to see how it handles env vars (also in script/mcp/)
grep -n "process.env\|EXPLORER\|RPC_URL" script/mcp/foundry.server.ts | head -20Repository: lifinance/contracts
Length of output: 655
🏁 Script executed:
# Check viemScriptHelpers to see optional env var patterns
cat -n script/utils/viemScriptHelpers.ts | grep -A 5 -B 5 "process.env"Repository: lifinance/contracts
Length of output: 635
Add explicit validation for EXPLORER_NETWORK instead of direct process.env access.
Per coding guidelines, env variables in script/**/*.ts should be validated explicitly. However, since EXPLORER_NETWORK is optional (used as a fallback), getEnvVar() (which throws on missing) isn't suitable here. Consider adding an optional env var validator or explicit validation that handles the fallback gracefully (e.g., process.env.EXPLORER_NETWORK ?? undefined). The same applies to other env accesses in this function (EXPLORER_API_BASE_URL, EXPLORER_API_KEY).
🤖 Prompt for AI Agents
In `@script/mcp/explorer.server.ts` around lines 143 - 178, The code reads
process.env.EXPLORER_NETWORK directly in resolveNetwork; add an explicit
optional env validation layer instead: create or call an optional validator
(e.g., getOptionalEnvVar or validateOptionalEnv) to read EXPLORER_NETWORK and
return undefined when empty/invalid rather than accessing process.env directly,
then use that validated value as fallbackNetwork in resolveNetwork; also apply
the same optional validation for EXPLORER_API_BASE_URL and EXPLORER_API_KEY
where they are used so env values are normalized/checked (e.g., ensure returned
string is non-empty and matches expected keys before using in logic within
resolveNetwork and related functions).
| function smokeTest(): void { | ||
| if (!networksLoad.ok) { | ||
| // eslint-disable-next-line no-console | ||
| console.error(`explorer smoke test failed: ${networksLoad.error}`) | ||
| process.exit(1) | ||
| } | ||
| const supported = Object.values(networks) | ||
| .map(asNetworkConfig) | ||
| .filter(isSupportedNetworkConfig).length | ||
| if (supported === 0) { | ||
| // eslint-disable-next-line no-console | ||
| console.error( | ||
| 'explorer smoke test failed: no supported networks found in config/networks.json' | ||
| ) | ||
| process.exit(1) | ||
| } | ||
| // eslint-disable-next-line no-console | ||
| console.error(`explorer smoke test ok (supported networks: ${supported})`) | ||
| process.exit(0) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if file exists and examine imports
head -30 script/mcp/explorer.server.ts | cat -nRepository: lifinance/contracts
Length of output: 182
🏁 Script executed:
# Get total line count to understand file structure
wc -l script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 96
🏁 Script executed:
# Check the actual code at lines 248-267
sed -n '248,267p' script/mcp/explorer.server.ts | cat -nRepository: lifinance/contracts
Length of output: 853
🏁 Script executed:
# Search for all console usage in this file
rg -n "console\." script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 433
🏁 Script executed:
# Check if consola is imported anywhere in the file
rg -n "consola\|from ['\"]consola" script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Check if consola is a dependency in package.json
rg -A 2 -B 2 "consola" package.jsonRepository: lifinance/contracts
Length of output: 198
🏁 Script executed:
# Look at the imports section of the explorer.server.ts file (first 50 lines)
head -50 script/mcp/explorer.server.ts | cat -nRepository: lifinance/contracts
Length of output: 1862
🏁 Script executed:
# Check if there are other script files using consola correctly
rg -l "from ['\"]consola" script/ | head -5Repository: lifinance/contracts
Length of output: 221
🏁 Script executed:
# Look at one example of proper consola usage for reference
rg -A 3 -B 3 "from ['\"]consola" script/ --max-count=1Repository: lifinance/contracts
Length of output: 21265
Use consola for all logging in this file.
The file matches the {script,tasks}/**/*.ts pattern requiring consola per coding guidelines. Replace all console.error calls with consola.error() (add import { consola } from 'consola' at the top). This includes lines 251, 259, 265, and similar calls elsewhere in the file. Remove the associated eslint-disable-next-line no-console comments.
🤖 Prompt for AI Agents
In `@script/mcp/explorer.server.ts` around lines 248 - 267, In the smokeTest
function replace all console.error usages with consola.error and remove the
corresponding eslint-disable-next-line no-console comments; add an import for
consola from 'consola' at the top of the file so the smokeTest function (and any
other places using console.error in this file) call consola.error(...) instead
of console.error(...). Ensure you update the three console.error calls inside
smokeTest and any similar console.error usages elsewhere in the file so they use
consola.error and no longer rely on eslint-disable comments.
| const baseExplorerArgsSchema = { | ||
| network: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| 'Network key from config/networks.json (e.g. "mainnet", "base").' | ||
| ), | ||
| chainId: z.number().optional().describe('Chain ID (e.g. 1).'), | ||
| explorerApiUrlOverride: z | ||
| .string() | ||
| .optional() | ||
| .describe('Optional override for explorer API URL (advanced).'), | ||
| apiKeyOverride: z | ||
| .string() | ||
| .optional() | ||
| .describe('Optional override for API key (advanced).'), | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, confirm the file exists and check the lines in question
wc -l script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 182
🏁 Script executed:
# Read the specific lines around 308-324
sed -n '300,330p' script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 982
🏁 Script executed:
# Check if there are similar Zod schema validation patterns in script/ for reference
rg -A 5 'z\.string\(\)' script/ -t ts | head -60Repository: lifinance/contracts
Length of output: 3390
🏁 Script executed:
# Search for how chainId is validated elsewhere in the codebase
rg 'chainId.*z\.' script/ -t ts -A 2 -B 2Repository: lifinance/contracts
Length of output: 398
🏁 Script executed:
# Check how network parameters are validated elsewhere
rg 'network.*z\.string' script/ -t ts -A 3 -B 1Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Check how network and chainId parameters are used in the tools that follow this schema
sed -n '325,400p' script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 1753
🏁 Script executed:
# Search for any validation helpers or patterns specific to network/chainId in explorer.server.ts
rg -B 5 -A 10 'baseExplorerArgsSchema' script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 2783
🏁 Script executed:
# Check the coding guideline context more carefully - are there validation helpers for network keys?
rg 'getEnvVar|Validatable' script/mcp/ -t ts | head -20Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Find and examine the resolveExplorerApi function to see how it handles network/chainId
rg -A 30 'function resolveExplorerApi|const resolveExplorerApi' script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 833
🏁 Script executed:
# Also check the imports and dependencies at the top of the file
head -50 script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 1512
🏁 Script executed:
# Find and examine the resolveNetwork function to see if it performs validation
rg -A 20 'function resolveNetwork|const resolveNetwork' script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 607
🏁 Script executed:
# Check if there's a resolveNetwork import from elsewhere
rg 'resolveNetwork' script/mcp/explorer.server.ts | head -10Repository: lifinance/contracts
Length of output: 137
Add stricter Zod constraints for external inputs (network/chainId). Per coding guidelines, script files must validate external inputs explicitly. Add .min(1) to network string to reject empty strings, and .int().positive() to chainId to enforce valid chain IDs upfront.
♻️ Suggested changes
- network: z
- .string()
+ network: z
+ .string()
+ .min(1)
.optional()
.describe(
'Network key from config/networks.json (e.g. "mainnet", "base").'
),
- chainId: z.number().optional().describe('Chain ID (e.g. 1).'),
+ chainId: z.number().int().positive().optional().describe('Chain ID (e.g. 1).'),🤖 Prompt for AI Agents
In `@script/mcp/explorer.server.ts` around lines 308 - 324, The
baseExplorerArgsSchema needs stricter validation for external inputs: update the
network field in baseExplorerArgsSchema (the z.string() for key "network") to
reject empty strings by adding .min(1), and update the chainId field (the
z.number() for key "chainId") to enforce integer positive IDs by adding
.int().positive(); keep both as .optional() but apply these constraints so
empty/invalid values are rejected upfront.
| server.tool( | ||
| 'explorer_proxy_tx_by_hash', | ||
| 'Fetch transaction by hash via explorer "proxy" API (if supported by the explorer).', | ||
| { | ||
| txHash: z.string().describe('Transaction hash (0x...).'), | ||
| ...baseExplorerArgsSchema, | ||
| }, | ||
| async ({ | ||
| txHash, | ||
| network, | ||
| chainId, | ||
| explorerApiUrlOverride, | ||
| apiKeyOverride, | ||
| }: { | ||
| txHash: string | ||
| network?: string | ||
| chainId?: number | ||
| explorerApiUrlOverride?: string | ||
| apiKeyOverride?: string | ||
| }) => { | ||
| const cfg = resolveExplorerApi({ | ||
| network, | ||
| chainId, | ||
| explorerApiUrlOverride, | ||
| apiKeyOverride, | ||
| }) | ||
| if (!cfg.ok) | ||
| return { | ||
| content: [{ type: 'text', text: `${cfg.error}\nSee docs/MCP.md.` }], | ||
| } | ||
|
|
||
| const url = buildUrl(cfg.baseUrl, { | ||
| module: 'proxy', | ||
| action: 'eth_getTransactionByHash', | ||
| txhash: txHash, | ||
| apikey: cfg.apiKey, | ||
| }) | ||
|
|
||
| const res = await httpsJson('GET', url) | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: | ||
| `statusCode: ${res.statusCode}\n\n` + | ||
| (res.json ? JSON.stringify(res.json, null, 2) : res.raw), | ||
| }, | ||
| ], | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and verify the file exists
fd "explorer.server.ts" --type fRepository: lifinance/contracts
Length of output: 182
🏁 Script executed:
# Check the line count and read the relevant section
wc -l script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 182
🏁 Script executed:
# Read the specific lines 430-479
sed -n '430,479p' script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 1214
🏁 Script executed:
# Search for existing tx hash validation patterns in the codebase
rg "0x\[a-fA-F0-9\]" script/ --type ts -A 2 -B 2Repository: lifinance/contracts
Length of output: 5615
🏁 Script executed:
# Look for validation helpers in script/utils and script/common
fd ".*\.(ts|tsx)$" script/utils script/common script/demoScripts/utils --type f | head -20Repository: lifinance/contracts
Length of output: 507
🏁 Script executed:
# Check what validation patterns exist for hex values or transaction data
rg "eth_getTransaction|txHash|tx hash" script/mcp/ --type ts -B 2 -A 2Repository: lifinance/contracts
Length of output: 4303
Add tx hash validation to explorer proxy calls.
Transaction hashes are external inputs that must be validated. Add regex validation /^0x[a-fA-F0-9]{64}$/ to the txHash parameter schema per coding guidelines.
♻️ Suggested validation pattern
- txHash: z.string().describe('Transaction hash (0x...).'),
+ txHash: z
+ .string()
+ .regex(/^0x[a-fA-F0-9]{64}$/, 'Invalid tx hash format')
+ .describe('Transaction hash (0x...).'),Note: The same validation should also be applied to explorer_proxy_tx_receipt_by_hash in the same file and the cast_tx_by_hash / cast_receipt_by_hash tools in script/mcp/foundry.server.ts.
📝 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.
| server.tool( | |
| 'explorer_proxy_tx_by_hash', | |
| 'Fetch transaction by hash via explorer "proxy" API (if supported by the explorer).', | |
| { | |
| txHash: z.string().describe('Transaction hash (0x...).'), | |
| ...baseExplorerArgsSchema, | |
| }, | |
| async ({ | |
| txHash, | |
| network, | |
| chainId, | |
| explorerApiUrlOverride, | |
| apiKeyOverride, | |
| }: { | |
| txHash: string | |
| network?: string | |
| chainId?: number | |
| explorerApiUrlOverride?: string | |
| apiKeyOverride?: string | |
| }) => { | |
| const cfg = resolveExplorerApi({ | |
| network, | |
| chainId, | |
| explorerApiUrlOverride, | |
| apiKeyOverride, | |
| }) | |
| if (!cfg.ok) | |
| return { | |
| content: [{ type: 'text', text: `${cfg.error}\nSee docs/MCP.md.` }], | |
| } | |
| const url = buildUrl(cfg.baseUrl, { | |
| module: 'proxy', | |
| action: 'eth_getTransactionByHash', | |
| txhash: txHash, | |
| apikey: cfg.apiKey, | |
| }) | |
| const res = await httpsJson('GET', url) | |
| return { | |
| content: [ | |
| { | |
| type: 'text', | |
| text: | |
| `statusCode: ${res.statusCode}\n\n` + | |
| (res.json ? JSON.stringify(res.json, null, 2) : res.raw), | |
| }, | |
| ], | |
| } | |
| } | |
| server.tool( | |
| 'explorer_proxy_tx_by_hash', | |
| 'Fetch transaction by hash via explorer "proxy" API (if supported by the explorer).', | |
| { | |
| txHash: z | |
| .string() | |
| .regex(/^0x[a-fA-F0-9]{64}$/, 'Invalid tx hash format') | |
| .describe('Transaction hash (0x...).'), | |
| ...baseExplorerArgsSchema, | |
| }, | |
| async ({ | |
| txHash, | |
| network, | |
| chainId, | |
| explorerApiUrlOverride, | |
| apiKeyOverride, | |
| }: { | |
| txHash: string | |
| network?: string | |
| chainId?: number | |
| explorerApiUrlOverride?: string | |
| apiKeyOverride?: string | |
| }) => { | |
| const cfg = resolveExplorerApi({ | |
| network, | |
| chainId, | |
| explorerApiUrlOverride, | |
| apiKeyOverride, | |
| }) | |
| if (!cfg.ok) | |
| return { | |
| content: [{ type: 'text', text: `${cfg.error}\nSee docs/MCP.md.` }], | |
| } | |
| const url = buildUrl(cfg.baseUrl, { | |
| module: 'proxy', | |
| action: 'eth_getTransactionByHash', | |
| txhash: txHash, | |
| apikey: cfg.apiKey, | |
| }) | |
| const res = await httpsJson('GET', url) | |
| return { | |
| content: [ | |
| { | |
| type: 'text', | |
| text: | |
| `statusCode: ${res.statusCode}\n\n` + | |
| (res.json ? JSON.stringify(res.json, null, 2) : res.raw), | |
| }, | |
| ], | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@script/mcp/explorer.server.ts` around lines 430 - 479, Add regex validation
to the txHash parameter schemas for explorer proxy and cast tools: update the
txHash z.string() schema in the explorer_proxy_tx_by_hash and
explorer_proxy_tx_receipt_by_hash server.tool definitions (in
explorer.server.ts) and in the cast_tx_by_hash and cast_receipt_by_hash tools
(in foundry.server.ts) to include a .regex(/^0x[a-fA-F0-9]{64}$/) (with a clear
.describe message) so only valid 0x-prefixed 64-hex-character transaction hashes
are accepted.
| server.tool( | ||
| 'explorer_get_logs', | ||
| 'Query logs via explorer API (module=logs&action=getLogs). Compatibility varies across explorers.', | ||
| { | ||
| fromBlock: z | ||
| .union([z.string(), z.number()]) | ||
| .describe('From block (number or "latest").'), | ||
| toBlock: z | ||
| .union([z.string(), z.number()]) | ||
| .describe('To block (number or "latest").'), | ||
| address: z | ||
| .string() | ||
| .optional() | ||
| .describe('Optional contract address filter.'), | ||
| topic0: z | ||
| .string() | ||
| .optional() | ||
| .describe('Optional topic0 (event signature hash).'), | ||
| ...baseExplorerArgsSchema, | ||
| }, | ||
| async ({ | ||
| fromBlock, | ||
| toBlock, | ||
| address, | ||
| topic0, | ||
| network, | ||
| chainId, | ||
| explorerApiUrlOverride, | ||
| apiKeyOverride, | ||
| }: { | ||
| fromBlock: string | number | ||
| toBlock: string | number | ||
| address?: string | ||
| topic0?: string | ||
| network?: string | ||
| chainId?: number | ||
| explorerApiUrlOverride?: string | ||
| apiKeyOverride?: string | ||
| }) => { | ||
| const cfg = resolveExplorerApi({ | ||
| network, | ||
| chainId, | ||
| explorerApiUrlOverride, | ||
| apiKeyOverride, | ||
| }) | ||
| if (!cfg.ok) | ||
| return { | ||
| content: [{ type: 'text', text: `${cfg.error}\nSee docs/MCP.md.` }], | ||
| } | ||
|
|
||
| const url = buildUrl(cfg.baseUrl, { | ||
| module: 'logs', | ||
| action: 'getLogs', | ||
| fromBlock, | ||
| toBlock, | ||
| address, | ||
| topic0, | ||
| apikey: cfg.apiKey, | ||
| }) | ||
|
|
||
| const res = await httpsJson('GET', url) | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: | ||
| `statusCode: ${res.statusCode}\n\n` + | ||
| (res.json ? JSON.stringify(res.json, null, 2) : res.raw), | ||
| }, | ||
| ], | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file path and examine the explorer.server.ts file
wc -l script/mcp/explorer.server.ts && echo "---" && head -50 script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 1550
🏁 Script executed:
# Look at the specific lines mentioned (618-689)
sed -n '618,689p' script/mcp/explorer.server.tsRepository: lifinance/contracts
Length of output: 1727
🏁 Script executed:
# Search for existing validation helpers in the codebase
fd -t f -e ts script/utils script/common | head -20Repository: lifinance/contracts
Length of output: 416
🏁 Script executed:
# Look for similar validation patterns with Zod in the codebase
rg "z\.(literal|int|nonnegative|regex)" -t ts script/ | head -20Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Check how explorer API is used elsewhere
rg "explorer.*getLogs\|buildUrl.*logs" -t ts script/ -A 3 -B 3Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Look at the buildUrl function to understand how it processes inputs
rg "buildUrl" -t ts script/mcp/ -A 10 -B 2Repository: lifinance/contracts
Length of output: 4611
🏁 Script executed:
# Check for topic validation or block number validation patterns
rg "topic0?\|fromBlock\|toBlock" -t ts script/ -A 2 -B 2Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Search for other topic0/topic validations in codebase
rg "topic" -t ts script/ -B 2 -A 2 | grep -v node_modules | head -30Repository: lifinance/contracts
Length of output: 1436
🏁 Script executed:
# Search for block number validation patterns
rg "block.*int\|nonnegative\|latest" -t ts script/ | head -20Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Check if address validation uses regex or other patterns
rg "0x[a-fA-F0-9]" -t ts script/ | head -15Repository: lifinance/contracts
Length of output: 1683
🏁 Script executed:
# Look at resolveExplorerApi to see what validation it does
sed -n '1,650p' script/mcp/explorer.server.ts | grep -A 30 "resolveExplorerApi"Repository: lifinance/contracts
Length of output: 4338
🏁 Script executed:
# Check how the buildUrl result is used and if there's any documentation about explorer API constraints
grep -n "getLogs\|module.*logs" script/mcp/explorer.server.ts -B 3 -A 10 | head -50Repository: lifinance/contracts
Length of output: 852
🏁 Script executed:
# Check Zod documentation/usage patterns for number validation in the codebase
rg "z\.number.*int\|\.number.*nonnegative" -t ts script/ | head -10Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Look at how address validation is done elsewhere
rg "address.*z\.|z\..*address" -t ts script/ -A 2 | head -20Repository: lifinance/contracts
Length of output: 1100
🏁 Script executed:
# Check explorer API documentation or comments about block ranges
grep -n "block.*range\|fromBlock\|toBlock" script/mcp/explorer.server.ts -i -B 2 -A 2Repository: lifinance/contracts
Length of output: 800
🏁 Script executed:
# Verify the baseExplorerArgsSchema to understand what's already being validated
rg "baseExplorerArgsSchema" -t ts script/mcp/ -B 5 -A 15Repository: lifinance/contracts
Length of output: 8140
Validate log query inputs (fromBlock, toBlock, topic0) to prevent malformed explorer API requests.
The current validation accepts arbitrary strings and numbers. Restrict to valid formats:
- Block parameters:
"latest"or non-negative integers - Topic0: valid hex format (0x-prefixed 64 hex characters)
Required validation
- fromBlock: z
- .union([z.string(), z.number()])
+ fromBlock: z
+ .union([z.literal('latest'), z.number().int().nonnegative()])
.describe('From block (number or "latest").'),
- toBlock: z
- .union([z.string(), z.number()])
+ toBlock: z
+ .union([z.literal('latest'), z.number().int().nonnegative()])
.describe('To block (number or "latest").'),
address: z
.string()
.optional()
.describe('Optional contract address filter.'),
- topic0: z
- .string()
- .optional()
+ topic0: z
+ .string()
+ .regex(/^0x[a-fA-F0-9]{64}$/, 'Invalid topic0')
+ .optional()
.describe('Optional topic0 (event signature hash).'),🤖 Prompt for AI Agents
In `@script/mcp/explorer.server.ts` around lines 618 - 689, The explorer_get_logs
handler currently allows arbitrary fromBlock/toBlock and topic0 values which can
create malformed API requests; add strict validation in the server.tool schema
for explorer_get_logs: constrain fromBlock/toBlock to either the string "latest"
or a non-negative integer (use z.union of z.literal('latest') and z.preprocess
to coerce/validate numeric input and refine to Number.isInteger & >=0), and
validate topic0 with a regex refinements (z.string().optional().refine(s =>
/^0x[a-fA-F0-9]{64}$/.test(s))). If validation fails, return the same structured
error response used elsewhere (content with a text message) before calling
resolveExplorerApi/buildUrl so invalid values never reach buildUrl or httpsJson.
| async function smokeTest(): Promise<void> { | ||
| const forge = await run('forge', ['--version']) | ||
| const cast = await run('cast', ['--version']) | ||
|
|
||
| // eslint-disable-next-line no-console | ||
| console.error( | ||
| 'foundry smoke test ok\n' + | ||
| `forge: exitCode=${forge.code}\n` + | ||
| `cast: exitCode=${cast.code}\n` | ||
| ) | ||
|
|
||
| if (forge.code !== 0 || cast.code !== 0) process.exit(1) | ||
| process.exit(0) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n script/mcp/foundry.server.tsRepository: lifinance/contracts
Length of output: 182
🌐 Web query:
consola npm package v3 named import syntax
💡 Result:
In consola v3 you use named exports (ESM) or destructuring (CJS). Examples:
-
ESM:
import { consola, createConsola } from "consola"; -
CommonJS:
const { consola, createConsola } = require("consola");
(utilities example)
- ESM utils:
import { stripAnsi } from "consola/utils"; - CJS utils:
const { stripAnsi } = require("consola/utils");
Sources: [1][2]
Sources
[1] consola README / GitHub.
[2] consola package page (npm).
Use consola for smoke-test logging.
Replace console.error with consola.error() and remove the eslint disable comment to match script logging conventions per coding guidelines. Files in {script,tasks}/**/*.ts must use consola for logging.
♻️ Example update
+import { consola } from 'consola'
@@
- // eslint-disable-next-line no-console
- console.error(
+ consola.error(
'foundry smoke test ok\n' +
`forge: exitCode=${forge.code}\n` +
`cast: exitCode=${cast.code}\n`
)🤖 Prompt for AI Agents
In `@script/mcp/foundry.server.ts` around lines 33 - 46, The smokeTest function is
using console.error and an eslint-disable comment; update it to use
consola.error instead and remove the eslint-disable comment: locate the
smokeTest function (which awaits run('forge', ...) and run('cast', ...),
referencing the forge and cast result variables) and replace the console.error
call with consola.error(...) while removing the preceding
eslint-disable-next-line no-console so the script follows logging conventions
for scripts/tasks.
| server.tool( | ||
| 'forge_test', | ||
| 'Run `forge test` (can be long-running).', | ||
| { | ||
| matchPath: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| "Optional `--match-path` filter, e.g. 'test/solidity/Foo.t.sol'." | ||
| ), | ||
| matchTest: z | ||
| .string() | ||
| .optional() | ||
| .describe('Optional `--match-test` filter (regex).'), | ||
| extraArgs: z | ||
| .array(z.string()) | ||
| .optional() | ||
| .describe('Extra args appended to forge test.'), | ||
| }, | ||
| async ({ | ||
| matchPath, | ||
| matchTest, | ||
| extraArgs, | ||
| }: { | ||
| matchPath?: string | ||
| matchTest?: string | ||
| extraArgs?: string[] | ||
| }) => { | ||
| const args = ['test'] | ||
| if (matchPath) args.push('--match-path', matchPath) | ||
| if (matchTest) args.push('--match-test', matchTest) | ||
| args.push(...(extraArgs ?? [])) | ||
|
|
||
| const { code, stdout, stderr } = await run('forge', args) | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: | ||
| `exitCode: ${code}\n\n` + | ||
| (stdout ? `stdout:\n${stdout}\n\n` : '') + | ||
| (stderr ? `stderr:\n${stderr}\n` : ''), | ||
| }, | ||
| ], | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if file exists and view the forge_test tool implementation
fd "foundry.server.ts" --type fRepository: lifinance/contracts
Length of output: 182
🏁 Script executed:
# Once we confirm the file, let's examine the full context around lines 79-125
cat -n script/mcp/foundry.server.ts | sed -n '60,140p'Repository: lifinance/contracts
Length of output: 2598
🏁 Script executed:
# Search for other similar tools in the same file to see if there's a pattern for handling extraArgs
rg "extraArgs" script/mcp/foundry.server.ts -B 3 -A 3Repository: lifinance/contracts
Length of output: 2336
🌐 Web query:
Foundry forge --ffi flag security implications command execution
💡 Result:
Short answer
- The forge cheatcode vm.ffi and the CLI flag --ffi let tests call arbitrary external commands; if enabled, anyone who can change tests/scripts can run commands on the machine running forge (arbitrary command execution / full RCE risk). [1][2]
Why this is dangerous (load-bearing facts)
- vm.ffi “calls an arbitrary command if ffi is enabled” and Foundry docs explicitly advise not enabling it by default because “anyone who can change the tests of a project will be able to execute arbitrary commands on devices that run the tests.” [1][2]
- There are real-world bug reports/issue discussions showing FFI-related surprises and behavior differences when used in tests, reinforcing that it’s a high-risk capability in test suites. [3]
Immediate security recommendations
- Never enable --ffi for untrusted or third‑party repositories. (Default: keep it off.)
- Audit any tests/scripts that use vm.ffi before running them.
- Run forge in isolated environments: ephemeral CI runners, containers (Docker), VMs, or dedicated low‑privilege users. Do not run on your host as root.
- Limit the execution environment: minimal PATH, no secrets in env, mount workdirs read‑only where possible.
- Prefer secure alternatives (mocking, native Rust/JS helpers) instead of calling external programs.
- In CI, restrict who can modify tests and treat FFI usage as a code-review checkpoint.
If you want, I can:
- scan a repo or specific test file for vm.ffi/--ffi usage and list risky calls, or
- provide a small Docker/CI example that safely runs forge tests without exposing the host.
Sources
- Foundry cheatcodes: ffi — getfoundry.sh. [1]
- ffi doc / warning mirrored in Foundry docs (learnblockchain page). [2]
- Example GitHub issue discussing FFI-related test behavior. [3]
References
[1] getfoundry.sh — ffi docs.
[2] Foundry cheatcode docs (mirror).
[3] foundry-rs/foundry GitHub issue discussing FFI usage.
Add an allowlist for safe Forge flags in MCP server tools to prevent --ffi and other dangerous flags.
User-supplied extraArgs enable --ffi and similar flags that execute arbitrary commands. Foundry explicitly warns that enabling --ffi creates a remote code execution vector—anyone who can pass arguments to the tool can run arbitrary shell commands. MCP servers expose these tools to external clients, making input validation mandatory per coding guidelines. This affects forge_test, forge_build, cast_tx, and cast_receipt tools.
Implement a blocklist (e.g., ['--ffi']) or require explicit opt-in for dangerous flags, especially in client-exposed MCP contexts.
🤖 Prompt for AI Agents
In `@script/mcp/foundry.server.ts` around lines 79 - 125, The forge_test tool
(server.tool 'forge_test') currently forwards user-supplied extraArgs directly
into run('forge', args), which allows dangerous flags like --ffi; update the
handler to validate/sanitize extraArgs before appending: implement a blocklist
(e.g., ['--ffi', '--allow-remote-exec', ...]) or an allowlist of safe flags and
reject or strip any disallowed entries, returning a clear error to the caller;
apply the same validation logic to the other MCP server tools mentioned
(forge_build, cast_tx, cast_receipt) by factoring the check into a shared helper
(e.g., validateForgeArgs/validateCastArgs) that is called before
args.push(...extraArgs) so dangerous flags are never passed to run().
| server.tool( | ||
| 'cast_call', | ||
| 'Perform an eth_call via `cast call` using `RPC_URL` from env (read-only).', | ||
| { | ||
| to: z.string().describe('Target contract address.'), | ||
| signature: z | ||
| .string() | ||
| .describe('Function signature, e.g. "decimals()(uint8)".'), | ||
| args: z | ||
| .array(z.string()) | ||
| .optional() | ||
| .describe('Function arguments as strings, e.g. ["0xabc..."].'), | ||
| block: z | ||
| .string() | ||
| .optional() | ||
| .describe('Optional block tag/number (passed to cast call).'), | ||
| rpcUrlEnvVar: z | ||
| .string() | ||
| .optional() | ||
| .describe('Env var name for RPC URL (default: RPC_URL).'), | ||
| }, | ||
| async ({ | ||
| to, | ||
| signature, | ||
| args, | ||
| block, | ||
| rpcUrlEnvVar, | ||
| }: { | ||
| to: string | ||
| signature: string | ||
| args?: string[] | ||
| block?: string | ||
| rpcUrlEnvVar?: string | ||
| }) => { | ||
| const envVar = rpcUrlEnvVar ?? 'RPC_URL' | ||
| const rpcUrl = process.env[envVar] | ||
| if (!rpcUrl) { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: `Missing RPC URL env var: ${envVar}\nSet it in .env.mcp.local (see docs/MCP.md).`, | ||
| }, | ||
| ], | ||
| } | ||
| } | ||
|
|
||
| const callArgs = [ | ||
| 'call', | ||
| '--rpc-url', | ||
| rpcUrl, | ||
| to, | ||
| signature, | ||
| ...(args ?? []), | ||
| ] | ||
| if (block) callArgs.push('--block', block) | ||
|
|
||
| const { code, stdout, stderr } = await run('cast', callArgs) | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: | ||
| `exitCode: ${code}\n\n` + | ||
| (stdout ? `stdout:\n${stdout}\n\n` : '') + | ||
| (stderr ? `stderr:\n${stderr}\n` : ''), | ||
| }, | ||
| ], | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for getEnvVar() helper
fd -e ts -e tsx script/utils | xargs rg "getEnvVar" -A 2 -B 2 | head -50Repository: lifinance/contracts
Length of output: 3451
🏁 Script executed:
# Check if getEnvVar is exported from script/utils/
rg "export.*getEnvVar" script/utils/ -A 3Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Look at the current foundry.server.ts implementation
wc -l script/mcp/foundry.server.tsRepository: lifinance/contracts
Length of output: 95
🏁 Script executed:
# Find address validation patterns in the codebase
rg "regex.*0x\[a-fA-F0-9\]" script/ -A 1 -B 1 | head -30Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Read the getEnvVar() implementation
sed -n '1,50p' script/demoScripts/utils/demoScriptHelpers.tsRepository: lifinance/contracts
Length of output: 1524
🏁 Script executed:
# Read the actual code in foundry.server.ts lines 175-245
sed -n '175,245p' script/mcp/foundry.server.tsRepository: lifinance/contracts
Length of output: 1697
🏁 Script executed:
# Search for zod address validation patterns
rg "z\.string.*regex.*0x" script/ -A 2Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Look for existing address validation helpers
rg "address.*regex\|0x.*[a-fA-F0-9].*40" script/ -t ts -A 2 -B 2Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Search for other tool implementations in foundry.server.ts to see pattern
sed -n '1,174p' script/mcp/foundry.server.ts | tail -50Repository: lifinance/contracts
Length of output: 1245
🏁 Script executed:
# Look for other tools that might use getEnvVar or similar patterns
rg "server\.tool\(" script/mcp/foundry.server.ts -A 30 | head -100Repository: lifinance/contracts
Length of output: 2577
🏁 Script executed:
# Check if getEnvVar is already imported in foundry.server.ts
head -50 script/mcp/foundry.server.tsRepository: lifinance/contracts
Length of output: 1450
🏁 Script executed:
# Search for any address validation helper in script/utils or script/common
rg "address.*regex\|Address.*validation\|validateAddress" script/ -t ts -B 2 -A 2Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Read the security rules to understand validation requirements
cat .cursor/rules/105-security.mdcRepository: lifinance/contracts
Length of output: 786
🏁 Script executed:
# Check if there are MCP-specific guidelines or patterns
rg "MCP\|mcp" .cursor/rules/ -A 5 -B 5Repository: lifinance/contracts
Length of output: 45
🏁 Script executed:
# Look for how viem's getAddress (address checksum validator) is used
rg "getAddress" script/ -t ts -B 2 -A 2 | head -40Repository: lifinance/contracts
Length of output: 2070
🏁 Script executed:
# Check if getEnvVar throws or handles gracefully, and if it's compatible with MCP tools
cat script/demoScripts/utils/demoScriptHelpers.ts | head -100Repository: lifinance/contracts
Length of output: 4094
🏁 Script executed:
# Look at the full getEnvVar implementation
rg "export const getEnvVar" script/demoScripts/utils/demoScriptHelpers.ts -A 10Repository: lifinance/contracts
Length of output: 397
Use getEnvVar() helper and validate address input.
RPC URL must be fetched via getEnvVar() (per coding guidelines for {script,tasks}/**/*.ts). Address input should be validated strictly—use viem's getAddress() which validates checksums, or add regex validation to the zod schema.
♻️ Example validation
+import { getAddress } from 'viem'
+import { getEnvVar } from '../../demoScripts/utils/demoScriptHelpers'
+
{
to: z
.string()
+ .refine(
+ (addr) => {
+ try {
+ getAddress(addr)
+ return true
+ } catch {
+ return false
+ }
+ },
+ 'Invalid Ethereum address'
+ )
.describe('Target contract address.'),
signature: z
- .string()
+ .string()
+ .min(1)
.describe('Function signature, e.g. "decimals()(uint8)".'), async ({
to,
signature,
args,
block,
rpcUrlEnvVar,
}: {
to: string
signature: string
args?: string[]
block?: string
rpcUrlEnvVar?: string
}) => {
- const envVar = rpcUrlEnvVar ?? 'RPC_URL'
- const rpcUrl = process.env[envVar]
- if (!rpcUrl) {
- return {
- content: [
- {
- type: 'text',
- text: `Missing RPC URL env var: ${envVar}\nSet it in .env.mcp.local (see docs/MCP.md).`,
- },
- ],
- }
- }
+ try {
+ const envVar = rpcUrlEnvVar ?? 'RPC_URL'
+ const rpcUrl = getEnvVar(envVar)
+ // ... rest of function
+ } catch (error) {
+ return {
+ content: [
+ {
+ type: 'text',
+ text: `${error instanceof Error ? error.message : 'Failed to retrieve RPC URL'}\nSet it in .env.mcp.local (see docs/MCP.md).`,
+ },
+ ],
+ }
+ }🤖 Prompt for AI Agents
In `@script/mcp/foundry.server.ts` around lines 175 - 245, Replace direct env
access and loose address handling in the cast_call tool: use the shared
getEnvVar() helper to fetch RPC URL (instead of process.env[envVar]) and fail
with the helper error if missing; tighten the request validation by updating the
zod schema for the "to" field to validate Ethereum addresses (or call viem's
getAddress() inside the handler) so malformed addresses are rejected before
calling cast. Locate the cast_call server.tool handler and change RPC lookup to
getEnvVar(rpcUrlEnvVar ?? 'RPC_URL'), and validate/normalize the "to" value via
getAddress(to) (or a zod refinment using viem's getAddress) before constructing
callArgs and running run('cast', ...).
| function httpsJson( | ||
| method: string, | ||
| url: string, | ||
| { | ||
| headers = {}, | ||
| body, | ||
| }: { headers?: Record<string, string>; body?: unknown } = {} | ||
| ): Promise<IHttpJsonResponse> { | ||
| return new Promise((resolve, reject) => { | ||
| const u = new URL(url) | ||
| const req = https.request( | ||
| { | ||
| method, | ||
| protocol: u.protocol, | ||
| hostname: u.hostname, | ||
| port: u.port || 443, | ||
| path: u.pathname + u.search, | ||
| headers: { | ||
| accept: 'application/json', | ||
| ...(body ? { 'content-type': 'application/json' } : {}), | ||
| ...headers, | ||
| }, | ||
| }, | ||
| (res) => { | ||
| let raw = '' | ||
| res.on('data', (d) => (raw += d.toString())) | ||
| res.on('end', () => { | ||
| const statusCode = res.statusCode ?? 0 | ||
| let parsed: unknown = null | ||
| try { | ||
| parsed = raw ? JSON.parse(raw) : null | ||
| } catch { | ||
| // keep parsed as null | ||
| } | ||
| resolve({ statusCode, headers: res.headers, raw, json: parsed }) | ||
| }) | ||
| } | ||
| ) | ||
| req.on('error', reject) | ||
| if (body) req.write(JSON.stringify(body)) | ||
| req.end() | ||
| }) | ||
| } |
There was a problem hiding this comment.
HTTP helper lacks timeout protection.
The httpsJson function has no timeout, which could cause MCP server hangs if the Tenderly API is slow or unresponsive. This affects the reliability of the MCP tool.
💡 Suggested fix: add timeout
function httpsJson(
method: string,
url: string,
{
headers = {},
body,
+ timeoutMs = 30000,
- }: { headers?: Record<string, string>; body?: unknown } = {}
+ }: { headers?: Record<string, string>; body?: unknown; timeoutMs?: number } = {}
): Promise<IHttpJsonResponse> {
return new Promise((resolve, reject) => {
const u = new URL(url)
const req = https.request(
{
method,
protocol: u.protocol,
hostname: u.hostname,
port: u.port || 443,
path: u.pathname + u.search,
+ timeout: timeoutMs,
headers: {
accept: 'application/json',
...(body ? { 'content-type': 'application/json' } : {}),
...headers,
},
},
// ... rest of handler
)
+ req.on('timeout', () => {
+ req.destroy()
+ reject(new Error(`Request timed out after ${timeoutMs}ms`))
+ })
req.on('error', reject)
// ...
})
}🤖 Prompt for AI Agents
In `@script/mcp/tenderly.server.ts` around lines 17 - 59, The httpsJson helper
lacks a request timeout, so add timeout protection inside the httpsJson
function: set a timeout (e.g. using req.setTimeout or a manual timer) after
creating the `req` in `httpsJson`, and on timeout destroy the request and reject
the Promise with a clear timeout error; ensure you clear the timer on
`res.on('end')` and on `req.on('error')` to avoid leaks, and keep existing
behavior for sending `body` and resolving with `{ statusCode, headers, raw, json
}` in the `res` end handler.
2 participants
Which Jira task belongs to this PR?
Why did I implement it this way?
Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)