-
Notifications
You must be signed in to change notification settings - Fork 968
feat(desktop): support parent directory navigation and external file paths #2057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e3113d6
5de239b
9067750
362aea6
a8f1f75
e65c0f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -905,6 +905,60 @@ export const createFilesystemRouter = () => { | |||||||||||||||||
| return { copied, errors }; | ||||||||||||||||||
| }), | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Read a file by absolute path with size cap and binary detection. | ||||||||||||||||||
| * Used for viewing files outside the current worktree. | ||||||||||||||||||
| */ | ||||||||||||||||||
| readFile: publicProcedure | ||||||||||||||||||
| .input( | ||||||||||||||||||
| z.object({ | ||||||||||||||||||
| filePath: z.string(), | ||||||||||||||||||
| }), | ||||||||||||||||||
| ) | ||||||||||||||||||
| .query( | ||||||||||||||||||
| async ({ | ||||||||||||||||||
| input, | ||||||||||||||||||
| }): Promise< | ||||||||||||||||||
| | { | ||||||||||||||||||
| ok: true; | ||||||||||||||||||
| content: string; | ||||||||||||||||||
| truncated: boolean; | ||||||||||||||||||
| byteLength: number; | ||||||||||||||||||
| } | ||||||||||||||||||
| | { | ||||||||||||||||||
| ok: false; | ||||||||||||||||||
| reason: "not-found" | "too-large" | "binary"; | ||||||||||||||||||
| } | ||||||||||||||||||
| > => { | ||||||||||||||||||
| const MAX_FILE_SIZE = 2 * 1024 * 1024; | ||||||||||||||||||
| try { | ||||||||||||||||||
| const stats = await fs.stat(input.filePath); | ||||||||||||||||||
| if (stats.size > MAX_FILE_SIZE) { | ||||||||||||||||||
| return { ok: false, reason: "too-large" }; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const buffer = await fs.readFile(input.filePath); | ||||||||||||||||||
|
|
||||||||||||||||||
| // Binary detection (same as readWorkingFile) | ||||||||||||||||||
| const checkLength = Math.min(buffer.length, BINARY_CHECK_SIZE); | ||||||||||||||||||
| for (let i = 0; i < checkLength; i++) { | ||||||||||||||||||
| if (buffer[i] === 0) { | ||||||||||||||||||
| return { ok: false, reason: "binary" }; | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+943
to
+947
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Duplicate binary detection logic — reuse the existing (Based on your team's feedback about avoiding duplicating business logic in multiple components.) Prompt for AI agents
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return { | ||||||||||||||||||
| ok: true, | ||||||||||||||||||
| content: buffer.toString("utf-8"), | ||||||||||||||||||
| truncated: false, | ||||||||||||||||||
| byteLength: buffer.length, | ||||||||||||||||||
| }; | ||||||||||||||||||
| } catch { | ||||||||||||||||||
| return { ok: false, reason: "not-found" }; | ||||||||||||||||||
|
Comment on lines
+956
to
+957
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Catch-all block maps every error to (Based on your team's feedback about avoiding empty catch blocks that hide failures.) Prompt for AI agents
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
| }, | ||||||||||||||||||
| ), | ||||||||||||||||||
|
Comment on lines
+912
to
+960
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This procedure accepts any renderer-supplied path and reads it directly, bypassing the worktree/path validation model used by secure file reads ( 🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| exists: publicProcedure | ||||||||||||||||||
| .input(z.object({ path: z.string() })) | ||||||||||||||||||
| .query(async ({ input }) => { | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,5 @@ | ||||||
| import { isAbsolute, normalize, relative, resolve } from "pathe"; | ||||||
|
|
||||||
| export function getWorkspaceToolFilePath({ | ||||||
| toolName, | ||||||
| args, | ||||||
|
|
@@ -26,39 +28,74 @@ export function normalizeWorkspaceFilePath({ | |||||
| filePath: string; | ||||||
| workspaceRoot?: string; | ||||||
| }): string | null { | ||||||
| let normalizedPath = filePath.trim(); | ||||||
| let normalizedPath = stripFileUri(filePath.trim()); | ||||||
| if (!normalizedPath) return null; | ||||||
|
|
||||||
| if (normalizedPath.startsWith("file://")) { | ||||||
| const rawPath = normalizedPath.slice(7); | ||||||
| try { | ||||||
| normalizedPath = decodeURIComponent(rawPath); | ||||||
| } catch { | ||||||
| normalizedPath = rawPath; | ||||||
| normalizedPath = normalize(normalizedPath); | ||||||
|
|
||||||
| if (workspaceRoot) { | ||||||
| const root = normalize(workspaceRoot); | ||||||
| if (normalizedPath === root) return null; | ||||||
| if (isAbsolute(normalizedPath)) { | ||||||
| const rel = relative(root, normalizedPath); | ||||||
| // relative() returns a path starting with ".." if outside the root | ||||||
| if (rel.startsWith("..")) return null; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The Prompt for AI agents
Suggested change
|
||||||
| normalizedPath = rel; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| normalizedPath = normalizedPath.replaceAll("\\", "/"); | ||||||
| if (!normalizedPath || normalizedPath === ".") return null; | ||||||
| if (isAbsolute(normalizedPath)) return null; | ||||||
|
|
||||||
| const normalizedRoot = workspaceRoot | ||||||
| ? workspaceRoot.replaceAll("\\", "/").replace(/\/+$/, "") | ||||||
| : ""; | ||||||
| return normalizedPath; | ||||||
| } | ||||||
|
|
||||||
| if (normalizedRoot) { | ||||||
| if (normalizedPath === normalizedRoot) return null; | ||||||
| if (normalizedPath.startsWith(`${normalizedRoot}/`)) { | ||||||
| normalizedPath = normalizedPath.slice(normalizedRoot.length + 1); | ||||||
| } | ||||||
| /** | ||||||
| * Resolves a file path to an absolute path given a workspace root. | ||||||
| * Handles file:// URIs, relative paths, and already-absolute paths. | ||||||
| * Returns null if the path is empty or resolves to the workspace root itself. | ||||||
| */ | ||||||
| export function resolveToAbsolutePath({ | ||||||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||||||
| filePath, | ||||||
| workspaceRoot, | ||||||
| }: { | ||||||
| filePath: string; | ||||||
| workspaceRoot?: string; | ||||||
| }): string | null { | ||||||
| const normalizedPath = stripFileUri(filePath.trim()); | ||||||
| if (!normalizedPath) return null; | ||||||
|
|
||||||
| // Remote URL — pass through | ||||||
| if ( | ||||||
| normalizedPath.startsWith("https://") || | ||||||
| normalizedPath.startsWith("http://") | ||||||
| ) { | ||||||
| return normalizedPath; | ||||||
| } | ||||||
|
|
||||||
| while (normalizedPath.startsWith("./")) { | ||||||
| normalizedPath = normalizedPath.slice(2); | ||||||
| // Already absolute — normalize and return | ||||||
| if (isAbsolute(normalizedPath)) { | ||||||
| return normalize(normalizedPath); | ||||||
| } | ||||||
|
|
||||||
| if (!normalizedPath || normalizedPath === ".") return null; | ||||||
| if (normalizedPath.startsWith("/")) return null; | ||||||
| // Relative path — resolve against workspace root | ||||||
| if (!workspaceRoot) return null; | ||||||
|
|
||||||
| return normalizedPath; | ||||||
| const resolved = resolve(workspaceRoot, normalizedPath); | ||||||
| // Don't return the workspace root itself | ||||||
| if (resolved === normalize(workspaceRoot)) return null; | ||||||
|
|
||||||
| return resolved; | ||||||
| } | ||||||
|
|
||||||
| function stripFileUri(path: string): string { | ||||||
| if (!path.startsWith("file://")) return path; | ||||||
| const rawPath = path.slice(7); | ||||||
| try { | ||||||
| return decodeURIComponent(rawPath); | ||||||
| } catch { | ||||||
| return rawPath; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| function toStringValue(value: unknown): string | null { | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reject non-regular files before
fs.readFile().The size check alone is not a safe guard here. FIFOs, device nodes, and other special files can report small or zero sizes, and
fs.readFile()can then block or blow past the 2 MB policy. Bail out unlessstats.isFile()is true.🔒 Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents