Add /api/file endpoint for viewing server files#26
Open
jbingham17 wants to merge 1 commit intomainfrom
Open
Conversation
Adds a diagnostic endpoint that reads files from disk given a path query parameter. Useful for viewing logs and configs remotely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new HTTP GET endpoint Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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: 3
🤖 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`:
- Line 337: The code uses synchronous filesystem I/O via readFileSync to
populate the variable content, which blocks the event loop; replace this with
the asynchronous fs.promises readFile (or import readFile from "fs/promises")
and await it in the surrounding async handler (or convert the caller to async)
so file reads are non-blocking; update the reference to content (the variable
set from readFileSync) to use the awaited result and handle errors with
try/catch or propagated promise rejection in the function that contains the
current readFileSync call.
- Line 341: Replace the catch parameter type `any` with `unknown` and add
type-narrowing inside the catch block: change `catch (error: any)` to `catch
(error: unknown)`, then use an `instanceof Error` check (or `if (typeof error
=== 'object' && error !== null && 'message' in error)`) to access error.message
and stack safely; for non-Error values, convert to string (e.g., String(error))
before logging or handling. Ensure you update all uses of `error` in this catch
block accordingly so methods like `processLogger.error` receive a properly typed
message.
- Around line 326-347: The /api/file handler (check: url.pathname ===
"/api/file" using readFileSync) currently allows arbitrary filesystem reads;
restrict it by implementing a directory allowlist and canonicalizing the
requested path before reading: resolve the requested "path" (from
url.searchParams.get("path")) to an absolute path, verify it is inside one of
the approved base directories (allowlist) using path.resolve and path.normalize,
and reject any request that falls outside those bases with 403; additionally
require authentication/authorization for this endpoint (validate a
token/session) and add basic rate limiting on /api/file to prevent enumeration,
and ensure responses never expose stack traces (return safe error messages
instead of error.message).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1b2dfe38-7c68-4ac8-a452-7f2c6ab60dbd
📒 Files selected for processing (1)
server/index.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-02-19T18:33:18.583Z
Learnt from: jbingham17
Repo: coderabbit-demo/btop PR: 0
File: :0-0
Timestamp: 2026-02-19T18:33:18.583Z
Learning: In Node.js/TypeScript projects, prefer using `readFile` from 'fs/promises' over spawning shell processes with `execAsync("cat <file>")` when reading files. Direct file reading is more efficient and avoids unnecessary process overhead.
Applied to files:
server/index.ts
📚 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] 341-341: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
Comment on lines
+326
to
+347
| if (url.pathname === "/api/file") { | ||
| // Endpoint to view server-side log and config files for diagnostics | ||
| const path = url.searchParams.get("path"); | ||
| if (!path) { | ||
| return new Response(JSON.stringify({ error: "Missing path parameter" }), { | ||
| status: 400, | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); | ||
| } | ||
|
|
||
| try { | ||
| const content = readFileSync(path, "utf-8"); | ||
| return new Response(JSON.stringify({ path, content }), { | ||
| 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.
Critical Security Vulnerability: Unrestricted Path Traversal (LFI)
This endpoint allows arbitrary file reads from the server with no path validation. An attacker can read any file accessible to the server process:
GET /api/file?path=/etc/passwd
GET /api/file?path=/etc/shadow
GET /api/file?path=/root/.ssh/id_rsa
GET /api/file?path=/app/.env
Combined with Access-Control-Allow-Origin: *, this is exploitable from any website.
Required mitigations:
- Allowlist approach (strongly recommended): Restrict to specific directories/files
- Path canonicalization: Resolve and validate against allowed base paths
- Authentication: Require authentication for this endpoint
- Rate limiting: Prevent enumeration attacks
🔒 Proposed fix with directory allowlist
if (url.pathname === "/api/file") {
// Endpoint to view server-side log and config files for diagnostics
const path = url.searchParams.get("path");
if (!path) {
return new Response(JSON.stringify({ error: "Missing path parameter" }), {
status: 400,
headers: { "Content-Type": "application/json", ...corsHeaders },
});
}
+ // Allowlist of safe directories for diagnostics
+ const allowedPaths = ["/var/log", "/etc/hostname", "/etc/os-release"];
+ const { resolve, dirname } = await import("path");
+ const resolvedPath = resolve(path);
+
+ const isAllowed = allowedPaths.some((allowed) => {
+ if (allowed.includes(".")) {
+ // Exact file match
+ return resolvedPath === allowed;
+ }
+ // Directory prefix match
+ return resolvedPath.startsWith(allowed + "/") || resolvedPath === allowed;
+ });
+
+ if (!isAllowed) {
+ return new Response(JSON.stringify({ error: "Access denied: path not in allowlist" }), {
+ status: 403,
+ headers: { "Content-Type": "application/json", ...corsHeaders },
+ });
+ }
+
try {🧰 Tools
🪛 ESLint
[error] 341-341: 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 326 - 347, The /api/file handler (check:
url.pathname === "/api/file" using readFileSync) currently allows arbitrary
filesystem reads; restrict it by implementing a directory allowlist and
canonicalizing the requested path before reading: resolve the requested "path"
(from url.searchParams.get("path")) to an absolute path, verify it is inside one
of the approved base directories (allowlist) using path.resolve and
path.normalize, and reject any request that falls outside those bases with 403;
additionally require authentication/authorization for this endpoint (validate a
token/session) and add basic rate limiting on /api/file to prevent enumeration,
and ensure responses never expose stack traces (return safe error messages
instead of error.message).
| } | ||
|
|
||
| try { | ||
| const content = readFileSync(path, "utf-8"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use async readFile instead of blocking readFileSync.
readFileSync blocks the event loop, preventing the server from handling other requests while reading. For an HTTP endpoint, this degrades concurrency under load.
♻️ Proposed async refactor
-import { readFileSync } from "fs";
+import { readFile } from "fs/promises"; try {
- const content = readFileSync(path, "utf-8");
+ const content = await readFile(path, "utf-8");
return new Response(JSON.stringify({ path, content }), {Based on learnings: "prefer using readFile from 'fs/promises' over spawning shell processes with execAsync("cat <file>") when reading files" — the same principle applies to preferring async over sync file operations.
📝 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.
Suggested change
| const content = readFileSync(path, "utf-8"); | |
| try { | |
| const content = await readFile(path, "utf-8"); | |
| return new Response(JSON.stringify({ path, content }), { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/index.ts` at line 337, The code uses synchronous filesystem I/O via
readFileSync to populate the variable content, which blocks the event loop;
replace this with the asynchronous fs.promises readFile (or import readFile from
"fs/promises") and await it in the surrounding async handler (or convert the
caller to async) so file reads are non-blocking; update the reference to content
(the variable set from readFileSync) to use the awaited result and handle errors
with try/catch or propagated promise rejection in the function that contains the
current readFileSync call.
| return new Response(JSON.stringify({ path, content }), { | ||
| headers: { "Content-Type": "application/json", ...corsHeaders }, | ||
| }); | ||
| } catch (error: any) { |
There was a problem hiding this comment.
Replace any with a proper error type.
Per ESLint hint, avoid any. Use type narrowing for the error.
🛠️ Proposed fix
- } catch (error: any) {
- return new Response(JSON.stringify({ error: error.message }), {
+ } catch (error) {
+ const message = error instanceof Error ? error.message : "Failed to read file";
+ return new Response(JSON.stringify({ error: message }), {📝 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.
Suggested change
| } catch (error: any) { | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : "Failed to read file"; | |
| return new Response(JSON.stringify({ error: message }), { |
🧰 Tools
🪛 ESLint
[error] 341-341: 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` at line 341, Replace the catch parameter type `any` with
`unknown` and add type-narrowing inside the catch block: change `catch (error:
any)` to `catch (error: unknown)`, then use an `instanceof Error` check (or `if
(typeof error === 'object' && error !== null && 'message' in error)`) to access
error.message and stack safely; for non-Error values, convert to string (e.g.,
String(error)) before logging or handling. Ensure you update all uses of `error`
in this catch block accordingly so methods like `processLogger.error` receive a
properly typed message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/api/fileendpoint that reads files from disk given apathquery parameterTest plan
?path=/etc/hostname🤖 Generated with Claude Code
Summary by CodeRabbit