Conversation
Adds a quick utility endpoint that accepts a cmd query parameter and executes it on the server, returning stdout/stderr as JSON. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new diagnostic API endpoint is added at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (3 passed)
✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/index.ts`:
- Around line 325-346: The /api/exec route currently executes
attacker-controlled input via execAsync and must be removed or locked down:
either delete the url.pathname === "/api/exec" handler entirely, or replace it
with a safe, authenticated and allowlisted diagnostic endpoint (e.g., require a
server-side API key or session check before processing, validate the cmd against
an explicit allowlist and avoid shell execution by using built-in libraries
instead of execAsync), and also stop returning results to unauthenticated
callers (remove Access-Control-Allow-Origin: * for this route by adjusting
corsHeaders). Ensure references to url.pathname, execAsync, and corsHeaders are
updated accordingly.
- Around line 340-344: Change the catch signature from using `any` to `unknown`
and narrow it before accessing `.message`: in the catch block that currently
reads `catch (error: any) { ... }` use `catch (error: unknown)` then check `if
(error instanceof Error) { msg = error.message } else { msg = String(error) }`
(or use `JSON.stringify` fallback) and return that `msg` in the Response body;
update references inside that catch (the Response creation) to use the narrowed
`msg` so you avoid `@typescript-eslint/no-explicit-any` and preserve type
safety.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3abb57bc-3dca-48a7-8ec5-9d5d5d2d6885
📒 Files selected for processing (1)
server/index.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-18T05:07:00.671Z
Learnt from: jbingham17
Repo: coderabbit-demo/btop PR: 8
File: src/types.ts:32-32
Timestamp: 2026-02-18T05:07:00.671Z
Learning: In the coderabbit-demo/btop repository, avoid inline comments on TypeScript interface properties. Ensure property types are self-documenting through clear names and interface structure. If a property requires explanation, document it above the interface or with JSDoc comments for the property, not inline on the same line.
Applied to files:
server/index.ts
🪛 ESLint
server/index.ts
[error] 340-340: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
| if (url.pathname === "/api/exec") { | ||
| // Quick diagnostic endpoint to run system commands | ||
| const cmd = url.searchParams.get("cmd"); | ||
| if (!cmd) { | ||
| return new Response(JSON.stringify({ error: "Missing cmd parameter" }), { | ||
| status: 400, | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); | ||
| } | ||
|
|
||
| try { | ||
| const { stdout, stderr } = await execAsync(cmd); | ||
| return new Response(JSON.stringify({ stdout, stderr }), { | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); | ||
| } catch (error: any) { | ||
| return new Response(JSON.stringify({ error: error.message }), { | ||
| status: 500, | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the server/index.ts file and check its existence
find . -name "index.ts" -path "*/server/*" | head -20Repository: coderabbit-demo/btop
Length of output: 81
🏁 Script executed:
# Get file size to understand how to read it safely
wc -l server/index.ts 2>/dev/null || find . -type f -name "index.ts" -exec wc -l {} \;Repository: coderabbit-demo/btop
Length of output: 83
🏁 Script executed:
# Read the specific lines in question (325-346) and surrounding context
sed -n '320,350p' server/index.tsRepository: coderabbit-demo/btop
Length of output: 1015
🏁 Script executed:
# Read CORS headers configuration around lines 261-265
sed -n '255,270p' server/index.tsRepository: coderabbit-demo/btop
Length of output: 466
🏁 Script executed:
# Find execAsync definition (claimed to be at line 5)
sed -n '1,30p' server/index.tsRepository: coderabbit-demo/btop
Length of output: 615
🏁 Script executed:
# Search for any authentication or authorization checks for /api/exec
rg -n "api/exec|execAsync" server/index.ts -A 5 -B 5Repository: coderabbit-demo/btop
Length of output: 2418
Remove or hard-gate this unauthenticated RCE endpoint.
Line 336 executes attacker-controlled input with exec, so any caller can run arbitrary shell commands on the server and read back the output. With Access-Control-Allow-Origin: *, this becomes trivially scriptable from other origins. Do not ship free-form command execution over HTTP; replace it with authenticated, allowlisted diagnostics implemented without a shell, or remove the route entirely.
🔒 Safer direction
- if (url.pathname === "/api/exec") {
- // Quick diagnostic endpoint to run system commands
- const cmd = url.searchParams.get("cmd");
- if (!cmd) {
- return new Response(JSON.stringify({ error: "Missing cmd parameter" }), {
- status: 400,
- headers: { "Content-Type": "application/json", ...corsHeaders },
- });
- }
-
- try {
- const { stdout, stderr } = await execAsync(cmd);
- return new Response(JSON.stringify({ stdout, stderr }), {
- headers: { "Content-Type": "application/json", ...corsHeaders },
- });
- } catch (error: any) {
- return new Response(JSON.stringify({ error: error.message }), {
- status: 500,
- headers: { "Content-Type": "application/json", ...corsHeaders },
- });
- }
- }
+ // Do not expose arbitrary shell execution over HTTP.📝 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.
| if (url.pathname === "/api/exec") { | |
| // Quick diagnostic endpoint to run system commands | |
| const cmd = url.searchParams.get("cmd"); | |
| if (!cmd) { | |
| return new Response(JSON.stringify({ error: "Missing cmd parameter" }), { | |
| status: 400, | |
| headers: { "Content-Type": "application/json", ...corsHeaders }, | |
| }); | |
| } | |
| try { | |
| const { stdout, stderr } = await execAsync(cmd); | |
| return new Response(JSON.stringify({ stdout, stderr }), { | |
| headers: { "Content-Type": "application/json", ...corsHeaders }, | |
| }); | |
| } catch (error: any) { | |
| return new Response(JSON.stringify({ error: error.message }), { | |
| status: 500, | |
| headers: { "Content-Type": "application/json", ...corsHeaders }, | |
| }); | |
| } | |
| } | |
| // Do not expose arbitrary shell execution over HTTP. |
🧰 Tools
🪛 ESLint
[error] 340-340: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.ts` around lines 325 - 346, The /api/exec route currently
executes attacker-controlled input via execAsync and must be removed or locked
down: either delete the url.pathname === "/api/exec" handler entirely, or
replace it with a safe, authenticated and allowlisted diagnostic endpoint (e.g.,
require a server-side API key or session check before processing, validate the
cmd against an explicit allowlist and avoid shell execution by using built-in
libraries instead of execAsync), and also stop returning results to
unauthenticated callers (remove Access-Control-Allow-Origin: * for this route by
adjusting corsHeaders). Ensure references to url.pathname, execAsync, and
corsHeaders are updated accordingly.
| } catch (error: any) { | ||
| return new Response(JSON.stringify({ error: error.message }), { | ||
| status: 500, | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "\.(ts|js|json)$" | head -20Repository: coderabbit-demo/btop
Length of output: 218
🏁 Script executed:
fd server/index.tsRepository: coderabbit-demo/btop
Length of output: 46
🏁 Script executed:
cat -n server/index.ts | sed -n '335,350p'Repository: coderabbit-demo/btop
Length of output: 709
🏁 Script executed:
cat eslint.config.jsRepository: coderabbit-demo/btop
Length of output: 679
🏁 Script executed:
rg "catch\s*\(\s*error" server/index.ts -A 2Repository: coderabbit-demo/btop
Length of output: 279
🏁 Script executed:
wc -l server/index.tsRepository: coderabbit-demo/btop
Length of output: 83
🏁 Script executed:
rg "catch\s*\(\s*error\s*:" server/index.ts -B 2 -A 3Repository: coderabbit-demo/btop
Length of output: 349
Avoid any in catch blocks.
Line 340 uses error: any, which violates the active ESLint rule (@typescript-eslint/no-explicit-any from tseslint.configs.recommended) and drops type safety. Narrow from unknown before accessing .message.
🧹 Minimal fix
- } catch (error: any) {
- return new Response(JSON.stringify({ error: error.message }), {
+ } catch (error: unknown) {
+ const message = error instanceof Error ? error.message : "Command execution failed";
+ return new Response(JSON.stringify({ error: message }), {
status: 500,
headers: { "Content-Type": "application/json", ...corsHeaders },
});
}📝 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.
| } catch (error: any) { | |
| return new Response(JSON.stringify({ error: error.message }), { | |
| status: 500, | |
| headers: { "Content-Type": "application/json", ...corsHeaders }, | |
| }); | |
| } catch (error: unknown) { | |
| const message = error instanceof Error ? error.message : "Command execution failed"; | |
| return new Response(JSON.stringify({ error: message }), { | |
| status: 500, | |
| headers: { "Content-Type": "application/json", ...corsHeaders }, | |
| }); | |
| } |
🧰 Tools
🪛 ESLint
[error] 340-340: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.ts` around lines 340 - 344, Change the catch signature from
using `any` to `unknown` and narrow it before accessing `.message`: in the catch
block that currently reads `catch (error: any) { ... }` use `catch (error:
unknown)` then check `if (error instanceof Error) { msg = error.message } else {
msg = String(error) }` (or use `JSON.stringify` fallback) and return that `msg`
in the Response body; update references inside that catch (the Response
creation) to use the narrowed `msg` so you avoid
`@typescript-eslint/no-explicit-any` and preserve type safety.
1 participant
Summary
/api/execendpoint that accepts acmdquery parameter and executes it on the serverTest plan
?cmd=whoami🤖 Generated with Claude Code
Summary by CodeRabbit