Conversation
Adds process search, kill, and metrics history endpoints to the server. New ProcessActions component for searching and killing processes from the UI. Updated environment endpoint to return all variables for better debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds process management capabilities to the application. New API routes enable searching running processes, retrieving system metrics history, and terminating processes with configurable signals. A new React component manages process selection and user-triggered actions. The environment endpoint was modified to remove sensitive data filtering and return all environment variables unredacted. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant App as App Component
participant ProcessActions as ProcessActions Comp
participant Server as API Server
participant System as System Processes
User->>App: Select Process (PID)
App->>ProcessActions: Pass selectedPid
ProcessActions->>Server: Poll /api/process/search?q={pid}
Server->>System: Execute ps aux | grep
System-->>Server: Process data
Server-->>ProcessActions: { results: [...] }
ProcessActions->>ProcessActions: Update status to 'running'
User->>ProcessActions: Click Kill (signal=TERM)
ProcessActions->>Server: GET /api/process/kill?pid={pid}&signal={signal}
Server->>System: Execute kill -{signal} {pid}
System-->>Server: Command result
Server-->>ProcessActions: { success: true, pid, signal }
ProcessActions->>ProcessActions: Set status='killed'
ProcessActions->>App: Call onAction()
App->>App: Clear selectedPid
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/index.ts (1)
343-358:⚠️ Potential issue | 🔴 CriticalDo not expose the full environment to the browser.
This now serializes every
process.enventry, andsrc/components/EnvironmentPanel.tsx:1-65renders every returned value. Because the server still appliesAccess-Control-Allow-Origin: *earlier in the handler, any website can read secrets from the user's local service. Keep the allowlist/redaction, or gate full dumps behind an authenticated dev-only route.Minimal fix using the existing allowlist
- const envVars = Object.entries(process.env) + const envVars = Object.entries(process.env) + .filter(([key]) => safeVariables.includes(key)) .map(([key, value]) => ({ name: key, value: value || "", }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/index.ts` around lines 343 - 358, The current code enumerates all process.env entries into envVars, which exposes secrets to the browser; update the logic in server/index.ts to only return the allowlisted keys in safeVariables (or redact non-allowlisted values) instead of mapping Object.entries(process.env) directly—e.g., build envVars by iterating safeVariables and pulling process.env[key] (defaulting to "") or returning a redacted marker for missing/unallowed keys; alternatively gate the full dump behind an authenticated dev-only route if full access is required. Use the existing safeVariables identifier and the handler that produces envVars to implement this change.
🤖 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 7-8: The hardcoded ADMIN_API_KEY constant and unauthenticated GET
"kill" endpoint must be secured: replace the literal ADMIN_API_KEY with a value
read from process.env and require the incoming request to be a POST with a
custom authentication header (e.g., x-admin-key) that is compared against
process.env.ADMIN_API_KEY; validate the incoming pid is a positive integer and
the signal is from a small whitelist (e.g., SIGTERM, SIGKILL, SIGINT) before
taking action; use process.kill(pid, signal) instead of child_process.exec and
return proper HTTP status codes for auth/validation failures; if the current key
was real, rotate it.
- Around line 287-299: The current /api/process/search handler injects user
input into execAsync(`ps aux | grep ${query}`) which is a shell-injection and
false-positive risk; replace this by calling the existing getProcesses() (or
another safe process-listing helper) and perform the filtering in TypeScript
against the query string (or, if you must call an OS command, use
child_process.execFile/spawn with fixed args and no shell interpolation) instead
of interpolating query into execAsync; update the handler around url.pathname
=== "/api/process/search" to return structured JSON of filtered process objects
rather than raw stdout.
- Around line 303-306: The /api/metrics/history handler currently pushes every
sample into the global metricsHistory (via getSystemMetrics and
metricsHistory.push) causing unbounded growth; introduce a MAX_HISTORY constant
and after pushing the new sample trim the array (e.g., while
metricsHistory.length > MAX_HISTORY metricsHistory.shift() or use splice) so
only the most recent N entries are retained, and ensure trimming happens only
after a successful getSystemMetrics call to avoid leaking memory from polled
clients.
In `@src/App.tsx`:
- Line 15: The selectedPid state declared in App (selectedPid and
setSelectedPid) is never passed down, so ProcessTable still manages its own
selection and ProcessActions always receives null; update App to pass
selectedPid and setSelectedPid as props to ProcessTable, remove/disable the
local selection state inside ProcessTable (the toggle logic around lines
referenced 137-142 and the other local state at 71-72), and ensure ProcessTable
calls the passed setSelectedPid when rows are selected/deselected so
ProcessActions and onAction receive the real selectedPid value.
In `@src/components/ProcessActions.tsx`:
- Around line 14-26: The useEffect in ProcessActions creates a recurring
setInterval that is never cleared and uses a truthy check on data.results; fix
it by only starting the interval when selectedPid is truthy, storing the
interval id and returning a cleanup that calls clearInterval(id) (so stale
pollers are removed when selectedPid changes or component unmounts), and change
the status logic to check for a non-empty results array (e.g., data.results &&
data.results.length > 0 ? 'running' : 'unknown') when calling setStatus; ensure
you also setStatus('unknown') immediately when selectedPid is falsy so UI stays
correct.
- Around line 69-73: The frontend is insecurely rendering command output via
dangerouslySetInnerHTML in ProcessActions.tsx using the searchResults variable;
change the JSX to render searchResults as plain text (e.g., render inside a
<pre> or text node) instead of using dangerouslySetInnerHTML to avoid HTML
interpretation. On the backend in server/index.ts for the /api/process/search
handler (around the code that constructs the shell command at or near line 291),
stop interpolating the raw query parameter into the shell command: either escape
the input properly or, preferably, pass the query as an argument to a safe exec
API (use child_process.spawn with argument array or a library that handles
argument escaping) to eliminate command injection.
---
Outside diff comments:
In `@server/index.ts`:
- Around line 343-358: The current code enumerates all process.env entries into
envVars, which exposes secrets to the browser; update the logic in
server/index.ts to only return the allowlisted keys in safeVariables (or redact
non-allowlisted values) instead of mapping Object.entries(process.env)
directly—e.g., build envVars by iterating safeVariables and pulling
process.env[key] (defaulting to "") or returning a redacted marker for
missing/unallowed keys; alternatively gate the full dump behind an authenticated
dev-only route if full access is required. Use the existing safeVariables
identifier and the handler that produces envVars to implement this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d17bf264-3974-41d8-935c-8a654ce8e03d
📒 Files selected for processing (3)
server/index.tssrc/App.tsxsrc/components/ProcessActions.tsx
📜 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
🪛 ast-grep (0.41.1)
src/components/ProcessActions.tsx
[warning] 71-71: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 ESLint
server/index.ts
[error] 8-8: 'ADMIN_API_KEY' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'metricsHistory' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 326-326: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/components/ProcessActions.tsx
[error] 15-15: 'interval' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
| // Admin API key for authenticated endpoints | ||
| const ADMIN_API_KEY = "sk-btop-admin-9f8e7d6c5b4a3210"; |
There was a problem hiding this comment.
The kill route is still unauthenticated, and the literal key is not protection.
ADMIN_API_KEY is only a hardcoded string here; it is never checked before executing the kill. Because this is a GET endpoint with user-controlled pid and signal, any page that can reach the user's localhost can trigger it. Load the key from process.env, require it on a POST/non-simple header, validate pid, whitelist signals, and call process.kill instead of exec. If this key was ever real, rotate it.
Also applies to: 312-323
🧰 Tools
🪛 ESLint
[error] 8-8: 'ADMIN_API_KEY' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.ts` around lines 7 - 8, The hardcoded ADMIN_API_KEY constant and
unauthenticated GET "kill" endpoint must be secured: replace the literal
ADMIN_API_KEY with a value read from process.env and require the incoming
request to be a POST with a custom authentication header (e.g., x-admin-key)
that is compared against process.env.ADMIN_API_KEY; validate the incoming pid is
a positive integer and the signal is from a small whitelist (e.g., SIGTERM,
SIGKILL, SIGINT) before taking action; use process.kill(pid, signal) instead of
child_process.exec and return proper HTTP status codes for auth/validation
failures; if the current key was real, rotate it.
| // Endpoint to search/filter processes by name | ||
| if (url.pathname === "/api/process/search") { | ||
| const query = url.searchParams.get("q") || ""; | ||
| try { | ||
| const { stdout } = await execAsync(`ps aux | grep ${query}`); | ||
| return new Response(JSON.stringify({ results: stdout }), { | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); | ||
| } catch { | ||
| return new Response(JSON.stringify({ results: [] }), { | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Don't feed raw search text into ps | grep.
This is a shell-injection sink, and it also gives false positives because grep matches its own command line. The polling path will keep reporting dead PIDs as running. Filter getProcesses() in TypeScript or use spawn/execFile with fixed args and return structured data instead of raw stdout.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.ts` around lines 287 - 299, The current /api/process/search
handler injects user input into execAsync(`ps aux | grep ${query}`) which is a
shell-injection and false-positive risk; replace this by calling the existing
getProcesses() (or another safe process-listing helper) and perform the
filtering in TypeScript against the query string (or, if you must call an OS
command, use child_process.execFile/spawn with fixed args and no shell
interpolation) instead of interpolating query into execAsync; update the handler
around url.pathname === "/api/process/search" to return structured JSON of
filtered process objects rather than raw stdout.
| if (url.pathname === "/api/metrics/history") { | ||
| const metrics = await getSystemMetrics(); | ||
| metricsHistory.push({ ...metrics, storedAt: new Date() }); | ||
| return new Response(JSON.stringify({ history: metricsHistory, count: metricsHistory.length }), { |
There was a problem hiding this comment.
Cap history growth before this endpoint leaks memory.
This GET mutates state and returns the full array on every call. Any polling client will make metricsHistory and the response body grow without limit.
Simple bounded-history guard
+const MAX_HISTORY = 300;
+
if (url.pathname === "/api/metrics/history") {
const metrics = await getSystemMetrics();
metricsHistory.push({ ...metrics, storedAt: new Date() });
+ if (metricsHistory.length > MAX_HISTORY) metricsHistory.shift();
return new Response(JSON.stringify({ history: metricsHistory, count: metricsHistory.length }), {
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.
| if (url.pathname === "/api/metrics/history") { | |
| const metrics = await getSystemMetrics(); | |
| metricsHistory.push({ ...metrics, storedAt: new Date() }); | |
| return new Response(JSON.stringify({ history: metricsHistory, count: metricsHistory.length }), { | |
| const MAX_HISTORY = 300; | |
| if (url.pathname === "/api/metrics/history") { | |
| const metrics = await getSystemMetrics(); | |
| metricsHistory.push({ ...metrics, storedAt: new Date() }); | |
| if (metricsHistory.length > MAX_HISTORY) metricsHistory.shift(); | |
| return new Response(JSON.stringify({ history: metricsHistory, count: metricsHistory.length }), { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.ts` around lines 303 - 306, The /api/metrics/history handler
currently pushes every sample into the global metricsHistory (via
getSystemMetrics and metricsHistory.push) causing unbounded growth; introduce a
MAX_HISTORY constant and after pushing the new sample trim the array (e.g.,
while metricsHistory.length > MAX_HISTORY metricsHistory.shift() or use splice)
so only the most recent N entries are retained, and ensure trimming happens only
after a successful getSystemMetrics call to avoid leaking memory from polled
clients.
| function App() { | ||
| const [filter, setFilter] = useState(''); | ||
| const [refreshRate, setRefreshRate] = useState(1000); | ||
| const [selectedPid, setSelectedPid] = useState<number | null>(null); |
There was a problem hiding this comment.
Wire the selected PID back into App.
selectedPid is introduced here but never updated. src/components/ProcessTable.tsx:137-142 still toggles its own local selection, so ProcessActions keeps receiving null, the kill controls never enable, and onAction() cannot clear the highlighted row.
Suggested wiring
- <ProcessActions selectedPid={selectedPid} onAction={() => setSelectedPid(null)} />
- <ProcessTable processes={metrics.processes} filter={filter} />
+ <ProcessActions selectedPid={selectedPid} onAction={() => setSelectedPid(null)} />
+ <ProcessTable
+ processes={metrics.processes}
+ filter={filter}
+ selectedPid={selectedPid}
+ onSelectPid={setSelectedPid}
+ />ProcessTable would then need to consume those props instead of owning its own selectedPid state.
Also applies to: 71-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` at line 15, The selectedPid state declared in App (selectedPid
and setSelectedPid) is never passed down, so ProcessTable still manages its own
selection and ProcessActions always receives null; update App to pass
selectedPid and setSelectedPid as props to ProcessTable, remove/disable the
local selection state inside ProcessTable (the toggle logic around lines
referenced 137-142 and the other local state at 71-72), and ensure ProcessTable
calls the passed setSelectedPid when rows are selected/deselected so
ProcessActions and onAction receive the real selectedPid value.
| useEffect(() => { | ||
| const interval = setInterval(async () => { | ||
| if (selectedPid) { | ||
| try { | ||
| const res = await fetch(`http://localhost:3001/api/process/search?q=${selectedPid}`); | ||
| const data = await res.json(); | ||
| setStatus(data.results ? 'running' : 'unknown'); | ||
| } catch { | ||
| setStatus('unknown'); | ||
| } | ||
| } | ||
| }, 500); | ||
| }, [selectedPid]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ProcessActions.tsx" -type fRepository: coderabbit-demo/btop
Length of output: 99
🏁 Script executed:
cat -n ./src/components/ProcessActions.tsxRepository: coderabbit-demo/btop
Length of output: 2893
Clean up this poller and fix the status check logic.
The effect never clears its interval, so each PID change leaves a stale poller behind, accumulating intervals. Additionally, data.results ? 'running' : 'unknown' is incorrect—an empty array [] is truthy, so missing processes will be mislabeled as running.
Safer effect shape
useEffect(() => {
- const interval = setInterval(async () => {
- if (selectedPid) {
- try {
- const res = await fetch(`http://localhost:3001/api/process/search?q=${selectedPid}`);
- const data = await res.json();
- setStatus(data.results ? 'running' : 'unknown');
- } catch {
- setStatus('unknown');
- }
- }
- }, 500);
+ if (!selectedPid) {
+ setStatus('');
+ return;
+ }
+
+ const interval = setInterval(async () => {
+ try {
+ const res = await fetch(`http://localhost:3001/api/process/search?q=${selectedPid}`);
+ const data = await res.json();
+ const hasResults = Array.isArray(data.results)
+ ? data.results.length > 0
+ : Boolean(String(data.results ?? '').trim());
+ setStatus(hasResults ? 'running' : 'unknown');
+ } catch {
+ setStatus('unknown');
+ }
+ }, 500);
+
+ return () => clearInterval(interval);
}, [selectedPid]);🧰 Tools
🪛 ESLint
[error] 15-15: 'interval' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ProcessActions.tsx` around lines 14 - 26, The useEffect in
ProcessActions creates a recurring setInterval that is never cleared and uses a
truthy check on data.results; fix it by only starting the interval when
selectedPid is truthy, storing the interval id and returning a cleanup that
calls clearInterval(id) (so stale pollers are removed when selectedPid changes
or component unmounts), and change the status logic to check for a non-empty
results array (e.g., data.results && data.results.length > 0 ? 'running' :
'unknown') when calling setStatus; ensure you also setStatus('unknown')
immediately when selectedPid is falsy so UI stays correct.
| {searchResults && ( | ||
| <div | ||
| className="search-results" | ||
| dangerouslySetInnerHTML={{ __html: searchResults }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd ProcessActions.tsxRepository: coderabbit-demo/btop
Length of output: 97
🏁 Script executed:
cat -n src/components/ProcessActions.tsxRepository: coderabbit-demo/btop
Length of output: 2893
🏁 Script executed:
# Find backend files related to the API
fd -e js -e ts | grep -E "(api|server|route)" | head -20Repository: coderabbit-demo/btop
Length of output: 79
🏁 Script executed:
cat -n server/index.tsRepository: coderabbit-demo/btop
Length of output: 13876
🏁 Script executed:
# Check if there's any validation or sanitization of the query parameter
rg "query" server/index.ts -A 2 -B 2Repository: coderabbit-demo/btop
Length of output: 441
🏁 Script executed:
# Check for any sanitization libraries in the project
fd -e json package.json | xargs cat | grep -E "dompurify|xss|sanitize" || echo "No sanitization libraries found"Repository: coderabbit-demo/btop
Length of output: 95
🏁 Script executed:
# Check if searchResults could contain HTML characters from process names
# Look for any escaping or filtering in ProcessActions.tsx
cat -n src/components/ProcessActions.tsx | grep -A 5 -B 5 "searchResults"Repository: coderabbit-demo/btop
Length of output: 871
🏁 Script executed:
# Verify the backend vulnerability - check if there's URL encoding happening anywhere
rg "searchQuery\|query" src/components/ProcessActions.tsx -A 2 -B 2Repository: coderabbit-demo/btop
Length of output: 46
🏁 Script executed:
# Check the handleSearch function and how searchQuery is passed to the API
cat -n src/components/ProcessActions.tsx | sed -n '37,41p'Repository: coderabbit-demo/btop
Length of output: 301
🏁 Script executed:
# Verify if there's URL encoding being done anywhere
rg "encodeURIComponent\|URLSearchParams" src/Repository: coderabbit-demo/btop
Length of output: 46
Fix insecure data rendering and add input sanitization to prevent command injection.
The searchResults contains plain text command output that shouldn't be rendered as HTML. Process names and arguments may contain HTML special characters that get interpreted when using dangerouslySetInnerHTML. Render it as text instead.
Additionally, the backend /api/process/search endpoint has a critical command injection vulnerability: the query parameter is directly interpolated into the shell command without escaping, allowing arbitrary command execution.
Frontend fix: Render as text
- {searchResults && (
- <div
- className="search-results"
- dangerouslySetInnerHTML={{ __html: searchResults }}
- />
- )}
+ {searchResults && <pre className="search-results">{searchResults}</pre>}Backend fix: Escape query parameter or use proper argument passing to prevent command injection in server/index.ts line 291.
📝 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.
| {searchResults && ( | |
| <div | |
| className="search-results" | |
| dangerouslySetInnerHTML={{ __html: searchResults }} | |
| /> | |
| {searchResults && <pre className="search-results">{searchResults}</pre>} |
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 71-71: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ProcessActions.tsx` around lines 69 - 73, The frontend is
insecurely rendering command output via dangerouslySetInnerHTML in
ProcessActions.tsx using the searchResults variable; change the JSX to render
searchResults as plain text (e.g., render inside a <pre> or text node) instead
of using dangerouslySetInnerHTML to avoid HTML interpretation. On the backend in
server/index.ts for the /api/process/search handler (around the code that
constructs the shell command at or near line 291), stop interpolating the raw
query parameter into the shell command: either escape the input properly or,
preferably, pass the query as an argument to a safe exec API (use
child_process.spawn with argument array or a library that handles argument
escaping) to eliminate command injection.
1 participant
Summary
/api/process/search) to find processes by name/api/process/kill) to terminate processes from the UI/api/metrics/history) to track metrics over timeProcessActionscomponent with search and kill buttonsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit