Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
"tree-kill": "^1.2.2",
"trpc-electron": "^0.1.2",
"tw-animate-css": "^1.4.0",
"use-resize-observer": "^9.1.0",
"zod": "^4.3.5",
"zustand": "^5.0.8"
},
Expand Down
286 changes: 286 additions & 0 deletions apps/desktop/src/lib/trpc/routers/filesystem/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,286 @@
import fs from "node:fs/promises";
import path from "node:path";
import { shell } from "electron";
import type { DirectoryEntry } from "shared/file-tree-types";
import { z } from "zod";
import { publicProcedure, router } from "../..";

export const createFilesystemRouter = () => {
return router({
readDirectory: publicProcedure
.input(
z.object({
dirPath: z.string(),
rootPath: z.string(),
includeHidden: z.boolean().default(false),
}),
)
Comment on lines +10 to +17
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

wc -l apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 121


🏁 Script executed:

cat -n apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 9179


🏁 Script executed:

# Search for path validation utilities in the codebase
find apps/desktop -type f -name "*.ts" -o -name "*.tsx" | head -20 | xargs grep -l "resolvePath\|normalizePath\|validatePath" 2>/dev/null | head -10

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Look for imports and path-related utilities in the filesystem directory
ls -la apps/desktop/src/lib/trpc/routers/filesystem/

Repository: superset-sh/superset

Length of output: 235


🏁 Script executed:

# Check if there's a shared utility for path validation
find apps/desktop/src/lib -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "path.*boundary\|validatePath\|normalizePath" 2>/dev/null

Repository: superset-sh/superset

Length of output: 127


🏁 Script executed:

# Search for TRPCError usage patterns in this file and similar routers
rg "TRPCError" apps/desktop/src/lib/trpc/routers/ -A 2 -B 1 --max-count=15

Repository: superset-sh/superset

Length of output: 8337


🏁 Script executed:

cat -n apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.ts

Repository: superset-sh/superset

Length of output: 18362


🏁 Script executed:

# Check how other routers validate paths
rg "path.*resolve\|normalized\|FORBIDDEN" apps/desktop/src/lib/trpc/routers/ --max-count=20

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Check the path-validation module referenced in secure-fs
cat -n apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts | head -100

Repository: superset-sh/superset

Length of output: 3624


🏁 Script executed:

# Verify how filesystem router is being called - check if rootPath is meant to be a worktree path
rg "readDirectory\|filesystem\." apps/desktop/src --max-count=15 -A 2 -B 2

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Check if there's middleware or context that validates rootPath
rg "createFilesystemRouter\|filesystem" apps/desktop/src --max-count=20

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Check the path-validation module more completely to understand resolvePathInWorktree
cat -n apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts | head -200

Repository: superset-sh/superset

Length of output: 6563


🏁 Script executed:

# Verify that filesystem router is indeed exposed without guards
rg "createFilesystemRouter" apps/desktop -B 3 -A 3

Repository: superset-sh/superset

Length of output: 1840


🏁 Script executed:

# Check what middleware or context is available for context around these procedures
cat -n apps/desktop/src/lib/trpc/index.ts | head -50

Repository: superset-sh/superset

Length of output: 1642


Implement path validation and proper TRPC error handling throughout.

These procedures accept raw filesystem paths without boundary checks. A malicious renderer could traverse outside the allowed root using ../ sequences or symlinks. Additionally, the entire router lacks TRPCError usage and silently swallows errors:

  • readDirectory (line 48), exists (line 259), stat (line 282) return empty/null values on error instead of throwing
  • createFile, createDirectory, rename throw generic Error instead of TRPCError
  • delete, move, copy collect errors in arrays without throwing

Use the existing secureFs utilities from apps/desktop/src/lib/trpc/routers/changes/security/secure-fs.ts which provide assertRegisteredWorktree() and assertRealpathInWorktree() to validate paths against worktree boundaries. Throw TRPCError with appropriate codes: NOT_FOUND for missing files, BAD_REQUEST for invalid input (e.g., absolute paths), INTERNAL_SERVER_ERROR for unexpected failures.

🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 10 - 17,
The router accepts raw filesystem paths and swallows or throws generic errors;
update readDirectory, exists, stat, createFile, createDirectory, rename, delete,
move, and copy to validate paths and use TRPCError: import and call
secureFs.assertRegisteredWorktree() and secureFs.assertRealpathInWorktree() at
the start of each procedure to prevent ../ or symlink escapes and reject
absolute paths, convert silent empty/null returns in readDirectory/exists/stat
into TRPCError with NOT_FOUND or BAD_REQUEST as appropriate, replace generic
Error throws in createFile/createDirectory/rename with TRPCError (BAD_REQUEST
for invalid input, INTERNAL_SERVER_ERROR for unexpected failures), and when
delete/move/copy collect errors convert those into a TRPCError containing
details (use INTERNAL_SERVER_ERROR or NOT_FOUND per case); ensure TRPCError is
used consistently with clear messages and proper codes across the referenced
functions.

.query(async ({ input }): Promise<DirectoryEntry[]> => {
const { dirPath, rootPath, includeHidden } = input;

try {
const entries = await fs.readdir(dirPath, { withFileTypes: true });

return entries
.filter((entry) => includeHidden || !entry.name.startsWith("."))
.map((entry) => {
const fullPath = path.join(dirPath, entry.name);
const relativePath = path.relative(rootPath, fullPath);
return {
id: relativePath,
name: entry.name,
path: fullPath,
relativePath,
isDirectory: entry.isDirectory(),
};
})
.sort((a, b) => {
if (a.isDirectory !== b.isDirectory) {
return a.isDirectory ? -1 : 1;
}
return a.name.localeCompare(b.name);
});
} catch (error) {
console.error("[filesystem/readDirectory] Failed:", {
dirPath,
error,
});
return [];
}
Comment on lines +18 to +49
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, verify the file exists and check its content
ls -la apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 166


🏁 Script executed:

# Check the full context of the readDirectory function
head -60 apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 1661


🏁 Script executed:

# Search for other error handling patterns in tRPC routes to ensure consistency
rg "throw new TRPCError" apps/desktop/src/lib/trpc/ -A 2 -B 2

Repository: superset-sh/superset

Length of output: 5451


🏁 Script executed:

# Check if there are other filesystem operations that handle permissions
cat -n apps/desktop/src/lib/trpc/routers/filesystem/index.ts | tail -100

Repository: superset-sh/superset

Length of output: 3226


🏁 Script executed:

# Look for error code handling patterns in the codebase
rg "error.code|error instanceof" apps/desktop/src/lib/trpc/routers/ -B 2 -A 2 | head -50

Repository: superset-sh/superset

Length of output: 4401


Don't swallow readDirectory errors by returning an empty list.

Failing reads should surface as TRPCError instead. Use NOT_FOUND for missing directories and FORBIDDEN for permission errors, matching the error context.

🔧 Suggested approach
+import { TRPCError } from "@trpc/server";
...
} catch (error) {
  console.error("[filesystem/readDirectory] Failed:", { dirPath, error });
+ const errorMessage = error instanceof Error ? error.message : String(error);
+ if (errorMessage.includes("EACCES")) {
+   throw new TRPCError({
+     code: "FORBIDDEN",
+     message: `Permission denied: ${dirPath}`,
+   });
+ }
+ throw new TRPCError({
+   code: "NOT_FOUND",
+   message: `Directory not found or inaccessible: ${dirPath}`,
+ });
- return [];
}

Per coding guidelines: never swallow errors silently. Per tRPC guidelines: use TRPCError with NOT_FOUND when a resource doesn't exist and FORBIDDEN when lacking permission.

🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 18 - 49,
The current .query handler for reading directories swallows filesystem errors
and returns an empty array; instead, catch the error and throw a TRPCError from
'@trpc/server' with appropriate codes: if error.code === 'ENOENT' throw new
TRPCError({ code: 'NOT_FOUND', message: `Directory not found: ${dirPath}`,
cause: error }); if error.code === 'EACCES' or other permission errors throw new
TRPCError({ code: 'FORBIDDEN', message: `Permission denied: ${dirPath}`, cause:
error }); for other errors throw a TRPCError({ code: 'INTERNAL_SERVER_ERROR',
message: 'Failed to read directory', cause: error }); ensure you import
TRPCError and remove the fallback return [] in the readDirectory/.query handler
so failures surface to the client.

}),

createFile: publicProcedure
.input(
z.object({
dirPath: z.string(),
fileName: z.string(),
content: z.string().default(""),
}),
)
.mutation(async ({ input }) => {
const filePath = path.join(input.dirPath, input.fileName);

try {
await fs.access(filePath);
throw new Error(`File already exists: ${input.fileName}`);
} catch (error) {
if (
error instanceof Error &&
error.message.includes("already exists")
) {
throw error;
}
}

await fs.writeFile(filePath, input.content, "utf-8");
return { path: filePath };
}),

createDirectory: publicProcedure
.input(
z.object({
parentPath: z.string(),
dirName: z.string(),
}),
)
.mutation(async ({ input }) => {
const dirPath = path.join(input.parentPath, input.dirName);

try {
await fs.access(dirPath);
throw new Error(`Directory already exists: ${input.dirName}`);
} catch (error) {
if (
error instanceof Error &&
error.message.includes("already exists")
) {
throw error;
}
}

await fs.mkdir(dirPath, { recursive: true });
return { path: dirPath };
}),

rename: publicProcedure
.input(
z.object({
oldPath: z.string(),
newName: z.string(),
}),
)
.mutation(async ({ input }) => {
const newPath = path.join(path.dirname(input.oldPath), input.newName);

try {
await fs.access(newPath);
throw new Error(`Target already exists: ${input.newName}`);
} catch (error) {
if (
error instanceof Error &&
error.message.includes("already exists")
) {
throw error;
}
}

await fs.rename(input.oldPath, newPath);
return { oldPath: input.oldPath, newPath };
}),
Comment on lines +52 to +129
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Confirm file path and check for other similar patterns
head -5 apps/desktop/src/lib/trpc/routers/filesystem/index.ts
wc -l apps/desktop/src/lib/trpc/routers/filesystem/index.ts
grep -n "fs.access" apps/desktop/src/lib/trpc/routers/filesystem/index.ts
grep -n "TRPCError" apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 526


🏁 Script executed:

# Check what fs module is being used and if TRPCError is imported
head -30 apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 1012


🏁 Script executed:

# Look for similar catch patterns with message checking
rg -A 5 "error.message.includes" apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 336


🏁 Script executed:

# Check the other fs.access calls at lines 178, 221, 251 to see if same pattern repeats
sed -n '170,230p' apps/desktop/src/lib/trpc/routers/filesystem/index.ts | cat -n
sed -n '245,260p' apps/desktop/src/lib/trpc/routers/filesystem/index.ts | cat -n

Repository: superset-sh/superset

Length of output: 2646


Fix error handling in fs.access() calls to prevent masking permission and I/O errors.

The fs.access() pattern in createFile, createDirectory, and rename silently swallows non-ENOENT errors (permission denied, I/O errors, etc.) by filtering catch blocks on message content. This masks real failures and violates error handling guidelines.

Update all three procedures to:

  1. Explicitly handle ENOENT (expected—file/directory doesn't exist)
  2. Throw TRPCError with code BAD_REQUEST when target already exists
  3. Log and throw TRPCError with code INTERNAL_SERVER_ERROR for unexpected access failures
Example fix
+import { TRPCError } from "@trpc/server";
...
try {
  await fs.access(filePath);
- throw new Error(`File already exists: ${input.fileName}`);
+ throw new TRPCError({
+   code: "BAD_REQUEST",
+   message: `File already exists: ${input.fileName}`,
+ });
} catch (error) {
- if (error instanceof Error && error.message.includes("already exists")) {
-   throw error;
- }
+ if (error instanceof TRPCError) throw error;
+ const err = error as NodeJS.ErrnoException;
+ if (err?.code !== "ENOENT") {
+   console.error("[filesystem/createFile] access failed:", { filePath, error });
+   throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" });
+ }
}
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 52 - 129,
The fs.access() catch blocks in createFile, createDirectory, and rename
currently swallow non-ENOENT errors; change them to explicitly check error.code
=== "ENOENT" (treat as non-existent) and for the "already exists" case detect
access success and throw a TRPCError with code BAD_REQUEST (including the
name/path); for any other access errors log the error and throw a TRPCError with
code INTERNAL_SERVER_ERROR. Update the access-handling logic inside createFile,
createDirectory, and rename (where fs.access is called) to distinguish ENOENT vs
other errors and to throw TRPCError instead of generic Error for consistency.


delete: publicProcedure
.input(
z.object({
paths: z.array(z.string()),
permanent: z.boolean().default(false),
}),
)
.mutation(async ({ input }) => {
const deleted: string[] = [];
const errors: { path: string; error: string }[] = [];

for (const filePath of input.paths) {
try {
if (input.permanent) {
await fs.rm(filePath, { recursive: true, force: true });
} else {
await shell.trashItem(filePath);
}
deleted.push(filePath);
} catch (error) {
errors.push({
path: filePath,
error: error instanceof Error ? error.message : String(error),
});
}
}

return { deleted, errors };
}),

move: publicProcedure
.input(
z.object({
sourcePaths: z.array(z.string()),
destinationDir: z.string(),
}),
)
.mutation(async ({ input }) => {
const moved: { from: string; to: string }[] = [];
const errors: { path: string; error: string }[] = [];

for (const sourcePath of input.sourcePaths) {
try {
const fileName = path.basename(sourcePath);
const destPath = path.join(input.destinationDir, fileName);

try {
await fs.access(destPath);
throw new Error(`Target already exists: ${fileName}`);
} catch (accessError) {
if (
accessError instanceof Error &&
accessError.message.includes("already exists")
) {
throw accessError;
}
}

await fs.rename(sourcePath, destPath);
moved.push({ from: sourcePath, to: destPath });
} catch (error) {
errors.push({
path: sourcePath,
error: error instanceof Error ? error.message : String(error),
});
}
}

return { moved, errors };
}),

copy: publicProcedure
.input(
z.object({
sourcePaths: z.array(z.string()),
destinationDir: z.string(),
}),
)
.mutation(async ({ input }) => {
const copied: { from: string; to: string }[] = [];
const errors: { path: string; error: string }[] = [];

for (const sourcePath of input.sourcePaths) {
try {
const fileName = path.basename(sourcePath);
let destPath = path.join(input.destinationDir, fileName);

let counter = 1;
while (true) {
try {
await fs.access(destPath);
const ext = path.extname(fileName);
const base = path.basename(fileName, ext);
destPath = path.join(
input.destinationDir,
`${base} (${counter})${ext}`,
);
counter++;
} catch {
break;
}
}

await fs.cp(sourcePath, destPath, { recursive: true });
copied.push({ from: sourcePath, to: destPath });
} catch (error) {
errors.push({
path: sourcePath,
error: error instanceof Error ? error.message : String(error),
});
}
}

return { copied, errors };
}),

exists: publicProcedure
.input(z.object({ path: z.string() }))
.query(async ({ input }) => {
try {
await fs.access(input.path);
const stats = await fs.stat(input.path);
return {
exists: true,
isDirectory: stats.isDirectory(),
isFile: stats.isFile(),
};
} catch {
return { exists: false, isDirectory: false, isFile: false };
}
}),

stat: publicProcedure
.input(z.object({ path: z.string() }))
.query(async ({ input }) => {
try {
const stats = await fs.stat(input.path);
return {
size: stats.size,
isDirectory: stats.isDirectory(),
isFile: stats.isFile(),
isSymbolicLink: stats.isSymbolicLink(),
createdAt: stats.birthtime.toISOString(),
modifiedAt: stats.mtime.toISOString(),
accessedAt: stats.atime.toISOString(),
};
} catch (error) {
console.error("[filesystem/stat] Failed:", {
path: input.path,
error,
});
return null;
}
Comment on lines +247 to +283
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n apps/desktop/src/lib/trpc/routers/filesystem/index.ts | sed -n '240,290p'

Repository: superset-sh/superset

Length of output: 1464


🏁 Script executed:

head -40 apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 1263


🏁 Script executed:

grep -n "TRPCError" apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

find apps/desktop/src/lib/trpc/routers -name "*.ts" -o -name "*.tsx" | head -5

Repository: superset-sh/superset

Length of output: 345


🏁 Script executed:

grep -l "TRPCError" apps/desktop/src/lib/trpc/routers/*.ts apps/desktop/src/lib/trpc/routers/*/*.ts 2>/dev/null | head -3

Repository: superset-sh/superset

Length of output: 222


🏁 Script executed:

head -20 apps/desktop/src/lib/trpc/routers/external/index.ts

Repository: superset-sh/superset

Length of output: 561


🏁 Script executed:

grep -A 5 "throw new TRPCError" apps/desktop/src/lib/trpc/routers/external/index.ts | head -20

Repository: superset-sh/superset

Length of output: 172


🏁 Script executed:

grep -B 5 -A 3 "NOT_FOUND\|NOT_IMPLEMENTED\|FORBIDDEN" apps/desktop/src/lib/trpc/routers/*/index.ts apps/desktop/src/lib/trpc/routers/*/*/index.ts 2>/dev/null | head -40

Repository: superset-sh/superset

Length of output: 1508


🏁 Script executed:

wc -l apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 121


🏁 Script executed:

grep -B 3 -A 8 "catch" apps/desktop/src/lib/trpc/routers/filesystem/index.ts

Repository: superset-sh/superset

Length of output: 2902


🏁 Script executed:

grep -n "\.query\|\.mutation" apps/desktop/src/lib/trpc/routers/filesystem/index.ts | head -20

Repository: superset-sh/superset

Length of output: 438


Avoid silently swallowing filesystem errors in exists and stat.

The exists procedure uses a bare catch block that returns false for all errors, including permission denied and IO failures. The stat procedure logs the error but returns null, both of which violate the error handling guidelines.

For exists, differentiate ENOENT (file not found—return false) from other errors (throw TRPCError with INTERNAL_SERVER_ERROR). For stat, throw TRPCError with NOT_FOUND instead of returning null.

🔧 Suggested pattern
+import { TRPCError } from "@trpc/server";
...
} catch (error) {
- return { exists: false, isDirectory: false, isFile: false };
+ if ((error as NodeJS.ErrnoException)?.code === "ENOENT") {
+   return { exists: false, isDirectory: false, isFile: false };
+ }
+ console.error("[filesystem/exists] Failed:", { path: input.path, error });
+ throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" });
}
} catch (error) {
  console.error("[filesystem/stat] Failed:", {
    path: input.path,
    error,
  });
- return null;
+ throw new TRPCError({
+   code: "NOT_FOUND",
+   message: `Path not found: ${input.path}`,
+ });
}
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 247 -
283, The exists and stat procedures currently swallow filesystem errors; update
exists (the publicProcedure named exists that calls fs.access and fs.stat) to
catch errors, check error.code === 'ENOENT' and return { exists: false, ... }
only for that case, but rethrow other errors as a TRPCError with code
'INTERNAL_SERVER_ERROR' and include the original error message; update stat (the
publicProcedure named stat that calls fs.stat) to stop returning null on failure
and instead throw a TRPCError with code 'NOT_FOUND' (include path and original
error in the message or meta) so callers receive proper error semantics.

}),
});
};
2 changes: 2 additions & 0 deletions apps/desktop/src/lib/trpc/routers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { createCacheRouter } from "./cache";
import { createChangesRouter } from "./changes";
import { createConfigRouter } from "./config";
import { createExternalRouter } from "./external";
import { createFilesystemRouter } from "./filesystem";
import { createHotkeysRouter } from "./hotkeys";
import { createMenuRouter } from "./menu";
import { createNotificationsRouter } from "./notifications";
Expand All @@ -30,6 +31,7 @@ export const createAppRouter = (getWindow: () => BrowserWindow | null) => {
workspaces: createWorkspacesRouter(),
terminal: createTerminalRouter(),
changes: createChangesRouter(),
filesystem: createFilesystemRouter(),
notifications: createNotificationsRouter(),
ports: createPortsRouter(),
menu: createMenuRouter(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { ChangeCategory, ChangedFile } from "shared/changes-types";
import {
getStatusColor,
getStatusIndicator,
} from "../../../Sidebar/ChangesView/utils";
} from "../../../RightSidebar/ChangesView/utils";
import { createFileKey, useScrollContext } from "../../context";
import { DiffViewer } from "../DiffViewer";
import { FileDiffHeader } from "./components/FileDiffHeader";
Expand Down
Loading