diff --git a/src/__tests__/path_utils.test.ts b/src/__tests__/path_utils.test.ts new file mode 100644 index 0000000..85dca5e --- /dev/null +++ b/src/__tests__/path_utils.test.ts @@ -0,0 +1,227 @@ +import { safeJoin } from "@/ipc/utils/path_utils"; +import { describe, it, expect } from "vitest"; +import path from "node:path"; +import os from "node:os"; + +describe("safeJoin", () => { + const testBaseDir = "/app/workspace"; + const testBaseDirWindows = "C:\\app\\workspace"; + + describe("safe paths", () => { + it("should join simple relative paths", () => { + const result = safeJoin(testBaseDir, "src", "components", "Button.tsx"); + expect(result).toBe( + path.join(testBaseDir, "src", "components", "Button.tsx"), + ); + }); + + it("should handle single file names", () => { + const result = safeJoin(testBaseDir, "package.json"); + expect(result).toBe(path.join(testBaseDir, "package.json")); + }); + + it("should handle nested directories", () => { + const result = safeJoin(testBaseDir, "src/pages/home/index.tsx"); + expect(result).toBe(path.join(testBaseDir, "src/pages/home/index.tsx")); + }); + + it("should handle paths with dots in filename", () => { + const result = safeJoin(testBaseDir, "config.test.js"); + expect(result).toBe(path.join(testBaseDir, "config.test.js")); + }); + + it("should handle empty path segments", () => { + const result = safeJoin(testBaseDir, "", "src", "", "file.ts"); + expect(result).toBe(path.join(testBaseDir, "", "src", "", "file.ts")); + }); + + it("should handle multiple path segments", () => { + const result = safeJoin(testBaseDir, "a", "b", "c", "d", "file.txt"); + expect(result).toBe( + path.join(testBaseDir, "a", "b", "c", "d", "file.txt"), + ); + }); + + it("should work with actual temp directory", () => { + const tempDir = os.tmpdir(); + const result = safeJoin(tempDir, "test", "file.txt"); + expect(result).toBe(path.join(tempDir, "test", "file.txt")); + }); + + it("should handle Windows-style relative paths with backslashes", () => { + const result = safeJoin(testBaseDir, "src\\components\\Button.tsx"); + expect(result).toBe( + path.join(testBaseDir, "src\\components\\Button.tsx"), + ); + }); + + it("should handle mixed forward/backslashes in relative paths", () => { + const result = safeJoin(testBaseDir, "src/components\\ui/button.tsx"); + expect(result).toBe( + path.join(testBaseDir, "src/components\\ui/button.tsx"), + ); + }); + + it("should handle Windows-style nested directories", () => { + const result = safeJoin( + testBaseDir, + "pages\\home\\components\\index.tsx", + ); + expect(result).toBe( + path.join(testBaseDir, "pages\\home\\components\\index.tsx"), + ); + }); + + it("should handle relative paths starting with dot and backslash", () => { + const result = safeJoin(testBaseDir, ".\\src\\file.txt"); + expect(result).toBe(path.join(testBaseDir, ".\\src\\file.txt")); + }); + }); + + describe("unsafe paths - directory traversal", () => { + it("should throw on simple parent directory traversal", () => { + expect(() => safeJoin(testBaseDir, "../outside.txt")).toThrow( + /would escape the base directory/, + ); + }); + + it("should throw on multiple parent directory traversals", () => { + expect(() => safeJoin(testBaseDir, "../../etc/passwd")).toThrow( + /would escape the base directory/, + ); + }); + + it("should throw on complex traversal paths", () => { + expect(() => safeJoin(testBaseDir, "src/../../../etc/passwd")).toThrow( + /would escape the base directory/, + ); + }); + + it("should throw on mixed traversal with valid components", () => { + expect(() => + safeJoin( + testBaseDir, + "src", + "components", + "..", + "..", + "..", + "outside.txt", + ), + ).toThrow(/would escape the base directory/); + }); + + it("should throw on absolute Unix paths", () => { + expect(() => safeJoin(testBaseDir, "/etc/passwd")).toThrow( + /would escape the base directory/, + ); + }); + + it("should throw on absolute Windows paths", () => { + expect(() => + safeJoin(testBaseDir, "C:\\Windows\\System32\\config"), + ).toThrow(/would escape the base directory/); + }); + + it("should throw on Windows UNC paths", () => { + expect(() => + safeJoin(testBaseDir, "\\\\server\\share\\file.txt"), + ).toThrow(/would escape the base directory/); + }); + + it("should throw on home directory shortcuts", () => { + expect(() => safeJoin(testBaseDir, "~/secrets.txt")).toThrow( + /would escape the base directory/, + ); + }); + }); + + describe("edge cases", () => { + it("should handle Windows-style base paths", () => { + const result = safeJoin(testBaseDirWindows, "src", "file.txt"); + expect(result).toBe(path.join(testBaseDirWindows, "src", "file.txt")); + }); + + it("should throw on Windows traversal from Unix base", () => { + expect(() => safeJoin(testBaseDir, "..\\..\\file.txt")).toThrow( + /would escape the base directory/, + ); + }); + + it("should handle current directory references safely", () => { + const result = safeJoin(testBaseDir, "./src/file.txt"); + expect(result).toBe(path.join(testBaseDir, "./src/file.txt")); + }); + + it("should handle nested current directory references", () => { + const result = safeJoin(testBaseDir, "src/./components/./Button.tsx"); + expect(result).toBe( + path.join(testBaseDir, "src/./components/./Button.tsx"), + ); + }); + + it("should throw when current dir plus traversal escapes", () => { + expect(() => safeJoin(testBaseDir, "./../../outside.txt")).toThrow( + /would escape the base directory/, + ); + }); + + it("should handle very long paths safely", () => { + const longPath = Array(50).fill("subdir").join("/") + "/file.txt"; + const result = safeJoin(testBaseDir, longPath); + expect(result).toBe(path.join(testBaseDir, longPath)); + }); + + it("should allow Windows-style paths that look like drive letters but aren't", () => { + // These look like they could be problematic but are actually safe relative paths + const result1 = safeJoin(testBaseDir, "C_drive\\file.txt"); + expect(result1).toBe(path.join(testBaseDir, "C_drive\\file.txt")); + + const result2 = safeJoin(testBaseDir, "src\\C-file.txt"); + expect(result2).toBe(path.join(testBaseDir, "src\\C-file.txt")); + }); + + it("should handle Windows paths with multiple backslashes (not UNC)", () => { + // Single backslashes in the middle are fine - it's only \\ at the start that's UNC + const result = safeJoin(testBaseDir, "src\\\\components\\\\Button.tsx"); + expect(result).toBe( + path.join(testBaseDir, "src\\\\components\\\\Button.tsx"), + ); + }); + + it("should provide descriptive error messages", () => { + expect(() => safeJoin("/base", "../outside.txt")).toThrow( + 'Unsafe path: joining "../outside.txt" with base "/base" would escape the base directory', + ); + }); + + it("should provide descriptive error for multiple segments", () => { + expect(() => safeJoin("/base", "src", "..", "..", "outside.txt")).toThrow( + 'Unsafe path: joining "src, .., .., outside.txt" with base "/base" would escape the base directory', + ); + }); + }); + + describe("boundary conditions", () => { + it("should allow paths at the exact boundary", () => { + const result = safeJoin(testBaseDir, "."); + expect(result).toBe(path.join(testBaseDir, ".")); + }); + + it("should handle paths that approach but don't cross boundary", () => { + const result = safeJoin(testBaseDir, "deep/nested/../file.txt"); + expect(result).toBe(path.join(testBaseDir, "deep/nested/../file.txt")); + }); + + it("should handle root directory as base", () => { + const result = safeJoin("/", "tmp/file.txt"); + expect(result).toBe(path.join("/", "tmp/file.txt")); + }); + + it("should throw when trying to escape root", () => { + expect(() => safeJoin("/tmp", "../etc/passwd")).toThrow( + /would escape the base directory/, + ); + }); + }); +}); diff --git a/src/ipc/processors/response_processor.ts b/src/ipc/processors/response_processor.ts index 4e89b71..c739905 100644 --- a/src/ipc/processors/response_processor.ts +++ b/src/ipc/processors/response_processor.ts @@ -5,6 +5,7 @@ import fs from "node:fs"; import { getDyadAppPath } from "../../paths/paths"; import path from "node:path"; import git from "isomorphic-git"; +import { safeJoin } from "../utils/path_utils"; import log from "electron-log"; import { executeAddDependency } from "./executeAddDependency"; @@ -296,11 +297,11 @@ export async function processFullResponseActions( } writtenFiles.push("package.json"); const pnpmFilename = "pnpm-lock.yaml"; - if (fs.existsSync(path.join(appPath, pnpmFilename))) { + if (fs.existsSync(safeJoin(appPath, pnpmFilename))) { writtenFiles.push(pnpmFilename); } const packageLockFilename = "package-lock.json"; - if (fs.existsSync(path.join(appPath, packageLockFilename))) { + if (fs.existsSync(safeJoin(appPath, packageLockFilename))) { writtenFiles.push(packageLockFilename); } } @@ -319,7 +320,7 @@ export async function processFullResponseActions( // Process all file deletions for (const filePath of dyadDeletePaths) { - const fullFilePath = path.join(appPath, filePath); + const fullFilePath = safeJoin(appPath, filePath); // Delete the file if it exists if (fs.existsSync(fullFilePath)) { @@ -362,8 +363,8 @@ export async function processFullResponseActions( // Process all file renames for (const tag of dyadRenameTags) { - const fromPath = path.join(appPath, tag.from); - const toPath = path.join(appPath, tag.to); + const fromPath = safeJoin(appPath, tag.from); + const toPath = safeJoin(appPath, tag.to); // Ensure target directory exists const dirPath = path.dirname(toPath); @@ -427,7 +428,7 @@ export async function processFullResponseActions( for (const tag of dyadWriteTags) { const filePath = tag.path; const content = tag.content; - const fullFilePath = path.join(appPath, filePath); + const fullFilePath = safeJoin(appPath, filePath); // Ensure directory exists const dirPath = path.dirname(fullFilePath); diff --git a/src/ipc/utils/path_utils.ts b/src/ipc/utils/path_utils.ts new file mode 100644 index 0000000..6c9fd19 --- /dev/null +++ b/src/ipc/utils/path_utils.ts @@ -0,0 +1,60 @@ +import path from "node:path"; + +/** + * Safely joins paths while ensuring the result stays within the base directory. + * This prevents directory traversal attacks where malicious paths like "../../etc/passwd" + * could be used to access files outside the intended directory. + * + * @param basePath The base directory that should contain the result + * @param ...paths Path segments to join with the base path + * @returns The joined path if it's within the base directory + * @throws Error if the resulting path would be outside the base directory + */ +export function safeJoin(basePath: string, ...paths: string[]): string { + // Check if any of the path segments are absolute paths (which would be unsafe) + for (const pathSegment of paths) { + if (path.isAbsolute(pathSegment)) { + throw new Error( + `Unsafe path: joining "${paths.join(", ")}" with base "${basePath}" would escape the base directory`, + ); + } + // Also check for home directory shortcuts which are effectively absolute + if (pathSegment.startsWith("~/")) { + throw new Error( + `Unsafe path: joining "${paths.join(", ")}" with base "${basePath}" would escape the base directory`, + ); + } + // Check for Windows-style absolute paths (C:\, D:\, etc.) + if (/^[A-Za-z]:[/\\]/.test(pathSegment)) { + throw new Error( + `Unsafe path: joining "${paths.join(", ")}" with base "${basePath}" would escape the base directory`, + ); + } + // Check for UNC paths (\\server\share) + if (pathSegment.startsWith("\\\\")) { + throw new Error( + `Unsafe path: joining "${paths.join(", ")}" with base "${basePath}" would escape the base directory`, + ); + } + } + + // Join all the paths + const joinedPath = path.join(basePath, ...paths); + + // Resolve both paths to absolute paths to handle any ".." components + const resolvedBasePath = path.resolve(basePath); + const resolvedJoinedPath = path.resolve(joinedPath); + + // Check if the resolved joined path starts with the base path + // Use path.relative to ensure we're doing a proper path comparison + const relativePath = path.relative(resolvedBasePath, resolvedJoinedPath); + + // If relativePath starts with ".." or is absolute, then resolvedJoinedPath is outside basePath + if (relativePath.startsWith("..") || path.isAbsolute(relativePath)) { + throw new Error( + `Unsafe path: joining "${paths.join(", ")}" with base "${basePath}" would escape the base directory`, + ); + } + + return joinedPath; +}