diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c82b2b138f..fb010932e70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(security): prevent workspace PATH injection via service env and trash helpers. (#73264) Thanks @pgondhi987. - Active Memory: allow `allowedChatTypes` to include explicit portal/webchat sessions and classify `agent:...:explicit:...` session keys before opaque session ids can shadow the chat type. Fixes #65775. (#66285) Thanks @Lidang-Jiang. - Active Memory: allow the hidden recall sub-agent to use both `memory_recall` and the legacy `memory_search`/`memory_get` memory tool contract, so bundled `memory-lancedb` recall works without breaking the default `memory-core` path. Fixes #73502. (#73584) Thanks @Takhoffman. - fix(device-pairing): validate callerScopes against resolved token scopes on repair [AI]. (#72925) Thanks @pgondhi987. diff --git a/extensions/browser/src/browser/trash.test.ts b/extensions/browser/src/browser/trash.test.ts index 81532d225d3..6335794dd9f 100644 --- a/extensions/browser/src/browser/trash.test.ts +++ b/extensions/browser/src/browser/trash.test.ts @@ -8,36 +8,197 @@ vi.mock("../process/exec.js", () => ({ runExec, })); +function mockTrashContainer(...suffixes: string[]) { + let call = 0; + return vi.spyOn(fs, "mkdtempSync").mockImplementation((prefix) => { + const suffix = suffixes[call] ?? "secure"; + call += 1; + return `${prefix}${suffix}`; + }); +} + describe("browser trash", () => { beforeEach(() => { vi.restoreAllMocks(); runExec.mockReset(); vi.spyOn(Date, "now").mockReturnValue(123); vi.spyOn(os, "homedir").mockReturnValue("/home/test"); + vi.spyOn(os, "tmpdir").mockReturnValue("/tmp"); + vi.spyOn(fs, "lstatSync").mockReturnValue({ + isDirectory: () => true, + isSymbolicLink: () => false, + } as fs.Stats); + vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => String(candidate)); }); - it("returns the target path when trash exits successfully", async () => { + it("moves paths to a reserved user trash container without invoking a PATH-resolved command", async () => { const { movePathToTrash } = await import("./trash.js"); - runExec.mockResolvedValue(undefined); - const mkdirSync = vi.spyOn(fs, "mkdirSync"); - const renameSync = vi.spyOn(fs, "renameSync"); - - await expect(movePathToTrash("/tmp/demo")).resolves.toBe("/tmp/demo"); - expect(runExec).toHaveBeenCalledWith("trash", ["/tmp/demo"], { timeoutMs: 10_000 }); - expect(mkdirSync).not.toHaveBeenCalled(); - expect(renameSync).not.toHaveBeenCalled(); - }); - - it("falls back to rename when trash exits non-zero", async () => { - const { movePathToTrash } = await import("./trash.js"); - runExec.mockRejectedValue(new Error("permission denied")); const mkdirSync = vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); - const existsSync = vi.spyOn(fs, "existsSync").mockReturnValue(false); + const mkdtempSync = mockTrashContainer("secure"); + const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined); + const cpSync = vi.spyOn(fs, "cpSync"); + const rmSync = vi.spyOn(fs, "rmSync"); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-secure/demo", + ); + expect(runExec).not.toHaveBeenCalled(); + expect(mkdirSync).toHaveBeenCalledWith("/home/test/.Trash", { + recursive: true, + mode: 0o700, + }); + expect(mkdtempSync).toHaveBeenCalledWith("/home/test/.Trash/demo-123-"); + expect(renameSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo"); + expect(cpSync).not.toHaveBeenCalled(); + expect(rmSync).not.toHaveBeenCalled(); + }); + + it("uses the resolved trash directory for reserved destinations", async () => { + const { movePathToTrash } = await import("./trash.js"); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => { + const value = String(candidate); + if (value === "/home/test") { + return "/real/home/test"; + } + if (value === "/home/test/.Trash") { + return "/real/home/test/.Trash"; + } + return value; + }); + const mkdtempSync = mockTrashContainer("secure"); const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined); - await expect(movePathToTrash("/tmp/demo")).resolves.toBe("/home/test/.Trash/demo-123"); - expect(mkdirSync).toHaveBeenCalledWith("/home/test/.Trash", { recursive: true }); - expect(existsSync).toHaveBeenCalledWith("/home/test/.Trash/demo-123"); - expect(renameSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123"); + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/real/home/test/.Trash/demo-123-secure/demo", + ); + expect(mkdtempSync).toHaveBeenCalledWith("/real/home/test/.Trash/demo-123-"); + expect(renameSync).toHaveBeenCalledWith( + "/tmp/demo", + "/real/home/test/.Trash/demo-123-secure/demo", + ); + }); + + it("refuses to trash filesystem roots", async () => { + const { movePathToTrash } = await import("./trash.js"); + + await expect(movePathToTrash("/")).rejects.toThrow("Refusing to trash root path"); + }); + + it("refuses to trash paths outside allowed roots", async () => { + const { movePathToTrash } = await import("./trash.js"); + + await expect(movePathToTrash("/etc/openclaw-demo")).rejects.toThrow( + "Refusing to trash path outside allowed roots", + ); + }); + + it("refuses to use a symlinked trash directory", async () => { + const { movePathToTrash } = await import("./trash.js"); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + vi.spyOn(fs, "lstatSync").mockReturnValue({ + isDirectory: () => true, + isSymbolicLink: () => true, + } as fs.Stats); + + await expect(movePathToTrash("/tmp/demo")).rejects.toThrow( + "Refusing to use non-directory/symlink trash directory", + ); + }); + + it("falls back to copy and remove when rename crosses filesystems", async () => { + const { movePathToTrash } = await import("./trash.js"); + const exdev = Object.assign(new Error("cross-device"), { code: "EXDEV" }); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + mockTrashContainer("secure"); + vi.spyOn(fs, "renameSync").mockImplementation(() => { + throw exdev; + }); + const cpSync = vi.spyOn(fs, "cpSync").mockImplementation(() => undefined); + const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-secure/demo", + ); + expect(cpSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo", { + recursive: true, + force: false, + errorOnExist: true, + }); + expect(rmSync).toHaveBeenCalledWith("/tmp/demo", { recursive: true, force: false }); + }); + + it("retries copy fallback when the copy destination is created concurrently", async () => { + const { movePathToTrash } = await import("./trash.js"); + const exdev = Object.assign(new Error("cross-device"), { code: "EXDEV" }); + const copyCollision = Object.assign(new Error("copy exists"), { + code: "ERR_FS_CP_EEXIST", + }); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + mockTrashContainer("first", "second"); + vi.spyOn(fs, "renameSync").mockImplementation(() => { + throw exdev; + }); + const cpSync = vi + .spyOn(fs, "cpSync") + .mockImplementationOnce(() => { + throw copyCollision; + }) + .mockImplementation(() => undefined); + const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-second/demo", + ); + expect(cpSync).toHaveBeenNthCalledWith( + 1, + "/tmp/demo", + "/home/test/.Trash/demo-123-first/demo", + { + recursive: true, + force: false, + errorOnExist: true, + }, + ); + expect(cpSync).toHaveBeenNthCalledWith( + 2, + "/tmp/demo", + "/home/test/.Trash/demo-123-second/demo", + { + recursive: true, + force: false, + errorOnExist: true, + }, + ); + expect(rmSync).toHaveBeenCalledTimes(1); + expect(Date.now).toHaveBeenCalledTimes(1); + }); + + it("retries with the same timestamp when the destination is created concurrently", async () => { + const { movePathToTrash } = await import("./trash.js"); + const collision = Object.assign(new Error("exists"), { code: "EEXIST" }); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + mockTrashContainer("first", "second"); + const renameSync = vi + .spyOn(fs, "renameSync") + .mockImplementationOnce(() => { + throw collision; + }) + .mockImplementation(() => undefined); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-second/demo", + ); + expect(renameSync).toHaveBeenNthCalledWith( + 1, + "/tmp/demo", + "/home/test/.Trash/demo-123-first/demo", + ); + expect(renameSync).toHaveBeenNthCalledWith( + 2, + "/tmp/demo", + "/home/test/.Trash/demo-123-second/demo", + ); + expect(Date.now).toHaveBeenCalledTimes(1); }); }); diff --git a/extensions/browser/src/browser/trash.ts b/extensions/browser/src/browser/trash.ts index c0b1d6094d6..a76def77c24 100644 --- a/extensions/browser/src/browser/trash.ts +++ b/extensions/browser/src/browser/trash.ts @@ -1,22 +1,141 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { generateSecureToken } from "../infra/secure-random.js"; -import { runExec } from "../process/exec.js"; -export async function movePathToTrash(targetPath: string): Promise { - try { - await runExec("trash", [targetPath], { timeoutMs: 10_000 }); - return targetPath; - } catch { - const trashDir = path.join(os.homedir(), ".Trash"); - fs.mkdirSync(trashDir, { recursive: true }); - const base = path.basename(targetPath); - let dest = path.join(trashDir, `${base}-${Date.now()}`); - if (fs.existsSync(dest)) { - dest = path.join(trashDir, `${base}-${Date.now()}-${generateSecureToken(6)}`); +const TRASH_DESTINATION_COLLISION_CODES = new Set(["EEXIST", "ENOTEMPTY", "ERR_FS_CP_EEXIST"]); +const TRASH_DESTINATION_RETRY_LIMIT = 4; + +function getFsErrorCode(error: unknown): string | undefined { + if (!error || typeof error !== "object" || !("code" in error)) { + return undefined; + } + const code = (error as NodeJS.ErrnoException).code; + return typeof code === "string" ? code : undefined; +} + +function isTrashDestinationCollision(error: unknown): boolean { + const code = getFsErrorCode(error); + return Boolean(code && TRASH_DESTINATION_COLLISION_CODES.has(code)); +} + +function isSameOrChildPath(candidate: string, parent: string): boolean { + return candidate === parent || candidate.startsWith(`${parent}${path.sep}`); +} + +function resolveAllowedTrashRoots(): string[] { + const roots = [os.homedir(), os.tmpdir()].map((root) => { + try { + return path.resolve(fs.realpathSync.native(root)); + } catch { + return path.resolve(root); } - fs.renameSync(targetPath, dest); - return dest; + }); + return [...new Set(roots)]; +} + +function assertAllowedTrashTarget(targetPath: string): void { + let resolvedTargetPath = path.resolve(targetPath); + try { + resolvedTargetPath = path.resolve(fs.realpathSync.native(targetPath)); + } catch { + // The subsequent move will surface missing or inaccessible targets. + } + const isAllowed = resolveAllowedTrashRoots().some( + (root) => resolvedTargetPath !== root && isSameOrChildPath(resolvedTargetPath, root), + ); + if (!isAllowed) { + throw new Error(`Refusing to trash path outside allowed roots: ${targetPath}`); } } + +function resolveTrashDir(): string { + const homeDir = os.homedir(); + const trashDir = path.join(homeDir, ".Trash"); + fs.mkdirSync(trashDir, { recursive: true, mode: 0o700 }); + const trashDirStat = fs.lstatSync(trashDir); + if (!trashDirStat.isDirectory() || trashDirStat.isSymbolicLink()) { + throw new Error(`Refusing to use non-directory/symlink trash directory: ${trashDir}`); + } + const realHome = path.resolve(fs.realpathSync.native(homeDir)); + const resolvedTrashDir = path.resolve(fs.realpathSync.native(trashDir)); + if (resolvedTrashDir === realHome || !isSameOrChildPath(resolvedTrashDir, realHome)) { + throw new Error(`Trash directory escaped home directory: ${trashDir}`); + } + return resolvedTrashDir; +} + +function trashBaseName(targetPath: string): string { + const resolvedTargetPath = path.resolve(targetPath); + if (resolvedTargetPath === path.parse(resolvedTargetPath).root) { + throw new Error(`Refusing to trash root path: ${targetPath}`); + } + const base = path.basename(resolvedTargetPath).replace(/[\\/]+/g, ""); + if (!base) { + throw new Error(`Unable to derive safe trash basename for: ${targetPath}`); + } + return base; +} + +function resolveContainedPath(root: string, leaf: string): string { + const resolvedRoot = path.resolve(root); + const resolvedPath = path.resolve(resolvedRoot, leaf); + if (!isSameOrChildPath(resolvedPath, resolvedRoot) || resolvedPath === resolvedRoot) { + throw new Error(`Trash destination escaped trash directory: ${resolvedPath}`); + } + return resolvedPath; +} + +function reserveTrashDestination(trashDir: string, base: string, timestamp: number): string { + const containerPrefix = resolveContainedPath(trashDir, `${base}-${timestamp}-`); + const container = fs.mkdtempSync(containerPrefix); + const resolvedContainer = path.resolve(container); + const resolvedTrashDir = path.resolve(trashDir); + if ( + resolvedContainer === resolvedTrashDir || + !isSameOrChildPath(resolvedContainer, resolvedTrashDir) + ) { + throw new Error(`Trash destination escaped trash directory: ${container}`); + } + return resolveContainedPath(container, base); +} + +function movePathToDestination(targetPath: string, dest: string): boolean { + try { + fs.renameSync(targetPath, dest); + return true; + } catch (error) { + if (getFsErrorCode(error) !== "EXDEV") { + if (isTrashDestinationCollision(error)) { + return false; + } + throw error; + } + } + + try { + fs.cpSync(targetPath, dest, { recursive: true, force: false, errorOnExist: true }); + fs.rmSync(targetPath, { recursive: true, force: false }); + return true; + } catch (error) { + if (isTrashDestinationCollision(error)) { + return false; + } + throw error; + } +} + +export async function movePathToTrash(targetPath: string): Promise { + // Avoid resolving external trash helpers through the service PATH during cleanup. + const base = trashBaseName(targetPath); + assertAllowedTrashTarget(targetPath); + const trashDir = resolveTrashDir(); + const timestamp = Date.now(); + for (let attempt = 0; attempt < TRASH_DESTINATION_RETRY_LIMIT; attempt += 1) { + const dest = reserveTrashDestination(trashDir, base, timestamp); + if (movePathToDestination(targetPath, dest)) { + return dest; + } + } + + throw new Error(`Unable to choose a unique trash destination for ${targetPath}`); +} diff --git a/src/commands/daemon-install-helpers.test.ts b/src/commands/daemon-install-helpers.test.ts index 3eaaacef213..104b32ddcbc 100644 --- a/src/commands/daemon-install-helpers.test.ts +++ b/src/commands/daemon-install-helpers.test.ts @@ -556,7 +556,17 @@ describe("buildGatewayInstallPlan — dotenv merge", () => { port: 3000, runtime: "node", existingEnvironment: { - PATH: ".:/tmp/evil:/custom/go/bin:/usr/bin", + PATH: [ + ".", + "/tmp/evil", + "/proc/self/cwd/evil-bin", + "/proc/thread-self/cwd/evil-bin", + "/proc/12345/cwd/evil-bin", + "/proc/self/root/evil-bin", + `${process.cwd()}/evil-bin`, + "/custom/go/bin", + "/usr/bin", + ].join(path.delimiter), GOBIN: "/Users/test/.local/gopath/bin", BLOGWATCHER_HOME: "/Users/test/.blogwatcher", NODE_OPTIONS: "--require /tmp/evil.js", @@ -573,6 +583,68 @@ describe("buildGatewayInstallPlan — dotenv merge", () => { expect(plan.environment.OPENCLAW_SERVICE_MARKER).toBeUndefined(); }); + it("drops existing PATH entries that resolve through symlinks into temp dirs", async () => { + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: "/from-service", + OPENCLAW_PORT: "3000", + PATH: "/managed/bin:/usr/bin", + TMPDIR: "/tmp", + }, + }); + const realpathNative = vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => { + const value = String(candidate); + if (value === "/opt/safe/bin") { + return "/tmp/evil/bin"; + } + if (value === "/opt/safe") { + return "/tmp/evil"; + } + if (value === "/opt/safe/missing-bin") { + throw Object.assign(new Error("missing"), { code: "ENOENT" }); + } + return value; + }); + + try { + const plan = await buildGatewayInstallPlan({ + env: { HOME: tmpDir }, + port: 3000, + runtime: "node", + existingEnvironment: { + PATH: "/opt/safe/bin:/opt/safe/missing-bin:/custom/go/bin:/usr/bin", + }, + }); + + expect(plan.environment.PATH).toBe("/managed/bin:/usr/bin:/custom/go/bin"); + } finally { + realpathNative.mockRestore(); + } + }); + + it("drops workspace-derived PATH entries even when HOME equals the install cwd", async () => { + const cwd = process.cwd(); + mockNodeGatewayPlanFixture({ + serviceEnvironment: { + HOME: cwd, + OPENCLAW_PORT: "3000", + PATH: "/managed/bin:/usr/bin", + TMPDIR: "/tmp", + }, + }); + + const plan = await buildGatewayInstallPlan({ + env: { HOME: cwd }, + port: 3000, + runtime: "node", + existingEnvironment: { + PATH: `${cwd}/evil-bin:/custom/go/bin:/usr/bin`, + }, + }); + + expect(plan.environment.PATH).toBe("/managed/bin:/usr/bin:/custom/go/bin"); + }); + it("drops keys that were previously tracked as managed service env", async () => { mockNodeGatewayPlanFixture({ serviceEnvironment: { diff --git a/src/commands/daemon-install-helpers.ts b/src/commands/daemon-install-helpers.ts index 0a8311513d6..5da786c9323 100644 --- a/src/commands/daemon-install-helpers.ts +++ b/src/commands/daemon-install-helpers.ts @@ -1,3 +1,4 @@ +import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import type { AuthProfileStore } from "../agents/auth-profiles/types.js"; @@ -179,13 +180,63 @@ function mergeServicePath( .map((value) => value?.trim()) .filter((value): value is string => Boolean(value)) .map((value) => path.resolve(value)); - const shouldPreservePathSegment = (segment: string) => { - if (!path.isAbsolute(segment)) { - return false; + const realTmpDirs = normalizedTmpDirs.map((tmpRoot) => { + try { + return path.normalize(fs.realpathSync.native(tmpRoot)); + } catch { + return tmpRoot; } + }); + const isSameOrChildPath = (candidate: string, parent: string) => + candidate === parent || candidate.startsWith(`${parent}${path.sep}`); + const isUnsafeProcPath = (candidate: string) => + candidate === `${path.sep}proc` || candidate.startsWith(`${path.sep}proc${path.sep}`); + const realpathExistingPath = (candidate: string): string | undefined => { + const parts: string[] = []; + let current = candidate; + while (current && current !== path.dirname(current)) { + try { + const realCurrent = path.normalize(fs.realpathSync.native(current)); + return path.normalize(path.join(realCurrent, ...parts.toReversed())); + } catch { + parts.push(path.basename(current)); + current = path.dirname(current); + } + } + try { + return path.normalize(path.join(fs.realpathSync.native(current), ...parts.toReversed())); + } catch { + return undefined; + } + }; + const normalizePreservedPathSegment = (segment: string): string | undefined => { + if (!path.isAbsolute(segment)) { + return undefined; + } + const normalized = path.normalize(segment); + if (isUnsafeProcPath(normalized)) { + return undefined; + } + const cwd = path.resolve(process.cwd()); + if (isSameOrChildPath(normalized, cwd)) { + return undefined; + } + try { + const realSegment = realpathExistingPath(normalized); + const realCwd = path.normalize(fs.realpathSync.native(cwd)); + if (realSegment && isSameOrChildPath(realSegment, realCwd)) { + return undefined; + } + } catch { + // Legacy PATH entries may no longer exist; keep filtering best-effort. + } + return normalized; + }; + const shouldPreserveNormalizedPathSegment = (segment: string) => { const resolved = path.resolve(segment); - return !normalizedTmpDirs.some( - (tmpRoot) => resolved === tmpRoot || resolved.startsWith(`${tmpRoot}${path.sep}`), + const realResolved = realpathExistingPath(resolved) ?? resolved; + return ![...normalizedTmpDirs, ...realTmpDirs].some( + (tmpRoot) => isSameOrChildPath(resolved, tmpRoot) || isSameOrChildPath(realResolved, tmpRoot), ); }; const addPath = (value: string | undefined, options?: { preserve?: boolean }) => { @@ -194,14 +245,15 @@ function mergeServicePath( } for (const segment of value.split(path.delimiter)) { const trimmed = segment.trim(); - if (options?.preserve && !shouldPreservePathSegment(trimmed)) { + const candidate = options?.preserve ? normalizePreservedPathSegment(trimmed) : trimmed; + if (options?.preserve && (!candidate || !shouldPreserveNormalizedPathSegment(candidate))) { continue; } - if (!trimmed || seen.has(trimmed)) { + if (!candidate || seen.has(candidate)) { continue; } - seen.add(trimmed); - segments.push(trimmed); + seen.add(candidate); + segments.push(candidate); } }; addPath(nextPath); diff --git a/src/daemon/service-env.test.ts b/src/daemon/service-env.test.ts index a8cb98c798b..aecbb329b98 100644 --- a/src/daemon/service-env.test.ts +++ b/src/daemon/service-env.test.ts @@ -1,6 +1,7 @@ +import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { resolveGatewayStateDir } from "./paths.js"; import { buildMinimalServicePath, @@ -254,6 +255,79 @@ describe("getMinimalServicePathParts - Linux user directories", () => { expect(result).toContain("/opt/fnm/current/bin"); }); + it("excludes env-configured bin roots derived from the install workspace", () => { + const result = getMinimalServicePathPartsFromEnv({ + platform: "linux", + cwd: "/home/testuser/workspace", + env: { + HOME: "/home/testuser", + PNPM_HOME: "/home/testuser/workspace/evil-pnpm-home", + NPM_CONFIG_PREFIX: "/proc/thread-self/cwd/evil-npm-prefix", + BUN_INSTALL: "/proc/12345/cwd/evil-bun", + VOLTA_HOME: "/opt/volta", + ASDF_DATA_DIR: "relative-asdf", + NIX_PROFILES: "/nix/var/nix/profiles/default /home/testuser/workspace/evil-nix-profile", + }, + existsSync: noneExist, + }); + + expect(result).not.toContain("/home/testuser/workspace/evil-pnpm-home"); + expect(result).not.toContain("/proc/thread-self/cwd/evil-npm-prefix/bin"); + expect(result).not.toContain("/proc/12345/cwd/evil-bun/bin"); + expect(result).not.toContain("relative-asdf/shims"); + expect(result).not.toContain("/home/testuser/workspace/evil-nix-profile/bin"); + expect(result).toContain("/opt/volta/bin"); + expect(result).toContain("/nix/var/nix/profiles/default/bin"); + }); + + it("excludes env-configured bin roots whose existing parent resolves into the workspace", () => { + const realpathNative = vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => { + const value = String(candidate); + if (value === "/tmp/workspace-link") { + return "/home/testuser/workspace"; + } + if (value === "/home/testuser/workspace" || value === "/home/testuser") { + return value; + } + throw Object.assign(new Error("missing"), { code: "ENOENT" }); + }); + + try { + const result = getMinimalServicePathPartsFromEnv({ + platform: "linux", + cwd: "/home/testuser/workspace", + env: { + HOME: "/home/testuser", + PNPM_HOME: "/tmp/workspace-link/missing-pnpm-home", + VOLTA_HOME: "/opt/volta", + }, + existsSync: noneExist, + }); + + expect(result).not.toContain("/tmp/workspace-link/missing-pnpm-home"); + expect(result).toContain("/opt/volta/bin"); + } finally { + realpathNative.mockRestore(); + } + }); + + it("keeps env-configured user toolchain roots when the install cwd is HOME", () => { + const result = getMinimalServicePathPartsFromEnv({ + platform: "linux", + cwd: "/home/testuser", + env: { + HOME: "/home/testuser", + PNPM_HOME: "/home/testuser/.local/share/pnpm", + FNM_DIR: "/home/testuser/.local/share/fnm", + }, + existsSync: noneExist, + }); + + expect(result).toContain("/home/testuser/.local/share/pnpm"); + expect(result).toContain("/home/testuser/.local/share/fnm/aliases/default/bin"); + expect(result).toContain("/home/testuser/.local/share/fnm/current/bin"); + }); + it("emits only existing hard-coded version-manager fallbacks", () => { const exists = (candidate: string) => candidate === "/home/testuser/.volta/bin" || diff --git a/src/daemon/service-env.ts b/src/daemon/service-env.ts index 1dcbe9005eb..d87cd6db344 100644 --- a/src/daemon/service-env.ts +++ b/src/daemon/service-env.ts @@ -29,6 +29,7 @@ export type MinimalServicePathOptions = { platform?: NodeJS.Platform; extraDirs?: string[]; home?: string; + cwd?: string; env?: Record; existsSync?: (candidate: string) => boolean; }; @@ -66,10 +67,86 @@ function readServiceProxyEnvironment( return proxyUrl ? { OPENCLAW_PROXY_URL: proxyUrl } : {}; } -function addNonEmptyDir(dirs: string[], dir: string | undefined): void { - if (dir) { - dirs.push(dir); +function normalizeServicePathDir(dir: string | undefined): string | undefined { + const trimmed = dir?.trim(); + // Service PATH snapshots are only emitted for macOS/Linux; keep POSIX semantics + // even when tests or helper callers run on Windows. + if (!trimmed || !path.posix.isAbsolute(trimmed)) { + return undefined; } + return path.posix.normalize(trimmed); +} + +function realpathServicePathDir(dir: string): string | undefined { + try { + return path.posix.normalize(fs.realpathSync.native(dir)); + } catch { + return undefined; + } +} + +function realpathExistingServicePathDir(dir: string): string | undefined { + const parts: string[] = []; + let current = dir; + while (current && current !== path.posix.dirname(current)) { + const realCurrent = realpathServicePathDir(current); + if (realCurrent) { + return path.posix.normalize(path.posix.join(realCurrent, ...parts.toReversed())); + } + parts.push(path.posix.basename(current)); + current = path.posix.dirname(current); + } + const realRoot = realpathServicePathDir(current); + return realRoot + ? path.posix.normalize(path.posix.join(realRoot, ...parts.toReversed())) + : undefined; +} + +function isSameOrChildPath(candidate: string, parent: string): boolean { + return candidate === parent || candidate.startsWith(`${parent}/`); +} + +function isUnsafeProcPath(candidate: string): boolean { + return candidate === "/proc" || candidate.startsWith("/proc/"); +} + +function isWorkspaceDerivedPath( + dir: string, + options: Pick, +): boolean { + // Install-time workspace env vars must not become durable service PATH entries. + if (isUnsafeProcPath(dir)) { + return true; + } + const cwd = normalizeServicePathDir(options.cwd ?? process.cwd()); + if (!cwd) { + return false; + } + const home = normalizeServicePathDir(options.home); + if (home && cwd === home) { + return false; + } + if (isSameOrChildPath(dir, cwd)) { + return true; + } + const realDir = realpathExistingServicePathDir(dir); + const realCwd = realpathServicePathDir(cwd); + const realHome = home ? realpathServicePathDir(home) : undefined; + return Boolean( + realDir && realCwd && realHome !== realCwd && isSameOrChildPath(realDir, realCwd), + ); +} + +function addEnvConfiguredBinDir( + dirs: string[], + dir: string | undefined, + options: Pick, +): void { + const normalized = normalizeServicePathDir(dir); + if (!normalized || isWorkspaceDerivedPath(normalized, options)) { + return; + } + dirs.push(normalized); } function appendSubdir(base: string | undefined, subdir: string): string | undefined { @@ -105,12 +182,13 @@ function addCommonUserBinDirs( function addCommonEnvConfiguredBinDirs( dirs: string[], env: Record | undefined, + options: Pick, ): void { - addNonEmptyDir(dirs, env?.PNPM_HOME); - addNonEmptyDir(dirs, appendSubdir(env?.NPM_CONFIG_PREFIX, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.BUN_INSTALL, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.VOLTA_HOME, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.ASDF_DATA_DIR, "shims")); + addEnvConfiguredBinDir(dirs, env?.PNPM_HOME, options); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.NPM_CONFIG_PREFIX, "bin"), options); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.BUN_INSTALL, "bin"), options); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.VOLTA_HOME, "bin"), options); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.ASDF_DATA_DIR, "shims"), options); } // Nix shell precedence: rightmost profile in NIX_PROFILES = highest priority. @@ -119,11 +197,12 @@ function addNixProfileBinDirs( dirs: string[], home: string, env: Record | undefined, + options: Pick, ): void { const nixProfiles = env?.NIX_PROFILES?.trim(); if (nixProfiles) { for (const profile of nixProfiles.split(/\s+/).toReversed()) { - addNonEmptyDir(dirs, appendSubdir(profile, "bin")); + addEnvConfiguredBinDir(dirs, appendSubdir(profile, "bin"), options); } } else { dirs.push(`${home}/.nix-profile/bin`); @@ -152,29 +231,31 @@ export function resolveDarwinUserBinDirs( home: string | undefined, env?: Record, existsSync: (candidate: string) => boolean = fs.existsSync, + options: Pick = {}, ): string[] { if (!home) { return []; } const dirs: string[] = []; + const pathOptions = { ...options, home }; // Env-configured bin roots (override defaults when present). // Note: FNM_DIR on macOS defaults to ~/Library/Application Support/fnm // Note: PNPM_HOME on macOS defaults to ~/Library/pnpm - addCommonEnvConfiguredBinDirs(dirs, env); + addCommonEnvConfiguredBinDirs(dirs, env, pathOptions); // nvm: no stable default path, relies on env or user's shell config // User must set NVM_DIR and source nvm.sh for it to work - addNonEmptyDir(dirs, env?.NVM_DIR); + addEnvConfiguredBinDir(dirs, env?.NVM_DIR, pathOptions); // fnm: use aliases/default (not current) - addNonEmptyDir(dirs, appendSubdir(env?.FNM_DIR, "aliases/default/bin")); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.FNM_DIR, "aliases/default/bin"), pathOptions); // pnpm: binary is directly in PNPM_HOME (not in bin subdirectory) // Common user bin directories addCommonUserBinDirs(dirs, home, existsSync); // Nix Home Manager (cross-platform) - addNixProfileBinDirs(dirs, home, env); + addNixProfileBinDirs(dirs, home, env, pathOptions); // Node version managers - macOS specific paths // nvm: no stable default path, depends on user's shell configuration @@ -196,24 +277,26 @@ export function resolveLinuxUserBinDirs( home: string | undefined, env?: Record, existsSync: (candidate: string) => boolean = fs.existsSync, + options: Pick = {}, ): string[] { if (!home) { return []; } const dirs: string[] = []; + const pathOptions = { ...options, home }; // Env-configured bin roots (override defaults when present). - addCommonEnvConfiguredBinDirs(dirs, env); - addNonEmptyDir(dirs, appendSubdir(env?.NVM_DIR, "current/bin")); - addNonEmptyDir(dirs, appendSubdir(env?.FNM_DIR, "aliases/default/bin")); - addNonEmptyDir(dirs, appendSubdir(env?.FNM_DIR, "current/bin")); + addCommonEnvConfiguredBinDirs(dirs, env, pathOptions); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.NVM_DIR, "current/bin"), pathOptions); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.FNM_DIR, "aliases/default/bin"), pathOptions); + addEnvConfiguredBinDir(dirs, appendSubdir(env?.FNM_DIR, "current/bin"), pathOptions); // Common user bin directories addCommonUserBinDirs(dirs, home, existsSync); // Nix Home Manager (cross-platform) - addNixProfileBinDirs(dirs, home, env); + addNixProfileBinDirs(dirs, home, env, pathOptions); // Node version managers addExistingDir(dirs, `${home}/.nvm/current/bin`, existsSync); // nvm with current symlink @@ -240,9 +323,9 @@ export function getMinimalServicePathParts(options: MinimalServicePathOptions = const existsSync = options.existsSync ?? fs.existsSync; const userDirs = platform === "linux" - ? resolveLinuxUserBinDirs(options.home, options.env, existsSync) + ? resolveLinuxUserBinDirs(options.home, options.env, existsSync, options) : platform === "darwin" - ? resolveDarwinUserBinDirs(options.home, options.env, existsSync) + ? resolveDarwinUserBinDirs(options.home, options.env, existsSync, options) : []; const add = (dir: string) => { diff --git a/src/plugin-sdk/browser-maintenance.test.ts b/src/plugin-sdk/browser-maintenance.test.ts index 86e63d9d839..b19b25d4965 100644 --- a/src/plugin-sdk/browser-maintenance.test.ts +++ b/src/plugin-sdk/browser-maintenance.test.ts @@ -1,3 +1,5 @@ +import fs from "node:fs"; +import os from "node:os"; import { beforeEach, describe, expect, it, vi } from "vitest"; const closeTrackedBrowserTabsForSessionsImpl = vi.hoisted(() => vi.fn()); @@ -12,11 +14,29 @@ vi.mock("../process/exec.js", () => ({ runExec, })); +function mockTrashContainer(...suffixes: string[]) { + let call = 0; + return vi.spyOn(fs, "mkdtempSync").mockImplementation((prefix) => { + const suffix = suffixes[call] ?? "secure"; + call += 1; + return `${prefix}${suffix}`; + }); +} + describe("browser maintenance", () => { beforeEach(() => { + vi.restoreAllMocks(); closeTrackedBrowserTabsForSessionsImpl.mockReset(); loadBundledPluginPublicSurfaceModuleSync.mockReset(); runExec.mockReset(); + vi.spyOn(Date, "now").mockReturnValue(123); + vi.spyOn(os, "homedir").mockReturnValue("/home/test"); + vi.spyOn(os, "tmpdir").mockReturnValue("/tmp"); + vi.spyOn(fs, "lstatSync").mockReturnValue({ + isDirectory: () => true, + isSymbolicLink: () => false, + } as fs.Stats); + vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => String(candidate)); loadBundledPluginPublicSurfaceModuleSync.mockReturnValue({ closeTrackedBrowserTabsForSessions: closeTrackedBrowserTabsForSessionsImpl, }); @@ -46,17 +66,180 @@ describe("browser maintenance", () => { }); }); - it("uses the local trash command before falling back", async () => { - runExec.mockResolvedValue({ - stdout: "", - stderr: "", - code: 0, - signal: null, - }); + it("moves paths to a reserved user trash container without invoking a PATH-resolved command", async () => { + const mkdirSync = vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + const mkdtempSync = mockTrashContainer("secure"); + const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined); + const cpSync = vi.spyOn(fs, "cpSync"); + const rmSync = vi.spyOn(fs, "rmSync"); const { movePathToTrash } = await import("./browser-maintenance.js"); - await expect(movePathToTrash("/tmp/demo")).resolves.toBe("/tmp/demo"); - expect(runExec).toHaveBeenCalledWith("trash", ["/tmp/demo"], { timeoutMs: 10_000 }); + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-secure/demo", + ); + expect(runExec).not.toHaveBeenCalled(); + expect(mkdirSync).toHaveBeenCalledWith("/home/test/.Trash", { + recursive: true, + mode: 0o700, + }); + expect(mkdtempSync).toHaveBeenCalledWith("/home/test/.Trash/demo-123-"); + expect(renameSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo"); + expect(cpSync).not.toHaveBeenCalled(); + expect(rmSync).not.toHaveBeenCalled(); + }); + + it("uses the resolved trash directory for reserved destinations", async () => { + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => { + const value = String(candidate); + if (value === "/home/test") { + return "/real/home/test"; + } + if (value === "/home/test/.Trash") { + return "/real/home/test/.Trash"; + } + return value; + }); + const mkdtempSync = mockTrashContainer("secure"); + const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined); + + const { movePathToTrash } = await import("./browser-maintenance.js"); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/real/home/test/.Trash/demo-123-secure/demo", + ); + expect(mkdtempSync).toHaveBeenCalledWith("/real/home/test/.Trash/demo-123-"); + expect(renameSync).toHaveBeenCalledWith( + "/tmp/demo", + "/real/home/test/.Trash/demo-123-secure/demo", + ); + }); + + it("refuses to trash filesystem roots", async () => { + const { movePathToTrash } = await import("./browser-maintenance.js"); + + await expect(movePathToTrash("/")).rejects.toThrow("Refusing to trash root path"); + }); + + it("refuses to trash paths outside allowed roots", async () => { + const { movePathToTrash } = await import("./browser-maintenance.js"); + + await expect(movePathToTrash("/etc/openclaw-demo")).rejects.toThrow( + "Refusing to trash path outside allowed roots", + ); + }); + + it("refuses to use a symlinked trash directory", async () => { + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + vi.spyOn(fs, "lstatSync").mockReturnValue({ + isDirectory: () => true, + isSymbolicLink: () => true, + } as fs.Stats); + + const { movePathToTrash } = await import("./browser-maintenance.js"); + + await expect(movePathToTrash("/tmp/demo")).rejects.toThrow( + "Refusing to use non-directory/symlink trash directory", + ); + }); + + it("falls back to copy and remove when rename crosses filesystems", async () => { + const exdev = Object.assign(new Error("cross-device"), { code: "EXDEV" }); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + mockTrashContainer("secure"); + vi.spyOn(fs, "renameSync").mockImplementation(() => { + throw exdev; + }); + const cpSync = vi.spyOn(fs, "cpSync").mockImplementation(() => undefined); + const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined); + + const { movePathToTrash } = await import("./browser-maintenance.js"); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-secure/demo", + ); + expect(cpSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo", { + recursive: true, + force: false, + errorOnExist: true, + }); + expect(rmSync).toHaveBeenCalledWith("/tmp/demo", { recursive: true, force: false }); + }); + + it("retries copy fallback when the copy destination is created concurrently", async () => { + const exdev = Object.assign(new Error("cross-device"), { code: "EXDEV" }); + const copyCollision = Object.assign(new Error("copy exists"), { + code: "ERR_FS_CP_EEXIST", + }); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + mockTrashContainer("first", "second"); + vi.spyOn(fs, "renameSync").mockImplementation(() => { + throw exdev; + }); + const cpSync = vi + .spyOn(fs, "cpSync") + .mockImplementationOnce(() => { + throw copyCollision; + }) + .mockImplementation(() => undefined); + const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined); + + const { movePathToTrash } = await import("./browser-maintenance.js"); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-second/demo", + ); + expect(cpSync).toHaveBeenNthCalledWith( + 1, + "/tmp/demo", + "/home/test/.Trash/demo-123-first/demo", + { + recursive: true, + force: false, + errorOnExist: true, + }, + ); + expect(cpSync).toHaveBeenNthCalledWith( + 2, + "/tmp/demo", + "/home/test/.Trash/demo-123-second/demo", + { + recursive: true, + force: false, + errorOnExist: true, + }, + ); + expect(rmSync).toHaveBeenCalledTimes(1); + expect(Date.now).toHaveBeenCalledTimes(1); + }); + + it("retries with the same timestamp when the destination is created concurrently", async () => { + const collision = Object.assign(new Error("exists"), { code: "EEXIST" }); + vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined); + mockTrashContainer("first", "second"); + const renameSync = vi + .spyOn(fs, "renameSync") + .mockImplementationOnce(() => { + throw collision; + }) + .mockImplementation(() => undefined); + + const { movePathToTrash } = await import("./browser-maintenance.js"); + + await expect(movePathToTrash("/tmp/demo")).resolves.toBe( + "/home/test/.Trash/demo-123-second/demo", + ); + expect(renameSync).toHaveBeenNthCalledWith( + 1, + "/tmp/demo", + "/home/test/.Trash/demo-123-first/demo", + ); + expect(renameSync).toHaveBeenNthCalledWith( + 2, + "/tmp/demo", + "/home/test/.Trash/demo-123-second/demo", + ); + expect(Date.now).toHaveBeenCalledTimes(1); }); }); diff --git a/src/plugin-sdk/browser-maintenance.ts b/src/plugin-sdk/browser-maintenance.ts index 06bdd490269..04a869f387a 100644 --- a/src/plugin-sdk/browser-maintenance.ts +++ b/src/plugin-sdk/browser-maintenance.ts @@ -12,12 +12,10 @@ type CloseTrackedBrowserTabsParams = { type BrowserMaintenanceSurface = { closeTrackedBrowserTabsForSessions: (params: CloseTrackedBrowserTabsParams) => Promise; }; -type SecureRandomRuntime = typeof import("../infra/secure-random.js"); -type ExecRuntime = typeof import("../process/exec.js"); let cachedBrowserMaintenanceSurface: BrowserMaintenanceSurface | undefined; -let secureRandomRuntimePromise: Promise | undefined; -let execRuntimePromise: Promise | undefined; +const TRASH_DESTINATION_COLLISION_CODES = new Set(["EEXIST", "ENOTEMPTY", "ERR_FS_CP_EEXIST"]); +const TRASH_DESTINATION_RETRY_LIMIT = 4; function hasRequestedSessionKeys(sessionKeys: Array): boolean { return sessionKeys.some((key) => Boolean(key?.trim())); @@ -32,14 +30,123 @@ function loadBrowserMaintenanceSurface(): BrowserMaintenanceSurface { return cachedBrowserMaintenanceSurface; } -function loadSecureRandomRuntime(): Promise { - secureRandomRuntimePromise ??= import("../infra/secure-random.js"); - return secureRandomRuntimePromise; +function getFsErrorCode(error: unknown): string | undefined { + if (!error || typeof error !== "object" || !("code" in error)) { + return undefined; + } + const code = (error as NodeJS.ErrnoException).code; + return typeof code === "string" ? code : undefined; } -function loadExecRuntime(): Promise { - execRuntimePromise ??= import("../process/exec.js"); - return execRuntimePromise; +function isTrashDestinationCollision(error: unknown): boolean { + const code = getFsErrorCode(error); + return Boolean(code && TRASH_DESTINATION_COLLISION_CODES.has(code)); +} + +function isSameOrChildPath(candidate: string, parent: string): boolean { + return candidate === parent || candidate.startsWith(`${parent}${path.sep}`); +} + +function resolveAllowedTrashRoots(): string[] { + const roots = [os.homedir(), os.tmpdir()].map((root) => { + try { + return path.resolve(fs.realpathSync.native(root)); + } catch { + return path.resolve(root); + } + }); + return [...new Set(roots)]; +} + +function assertAllowedTrashTarget(targetPath: string): void { + let resolvedTargetPath = path.resolve(targetPath); + try { + resolvedTargetPath = path.resolve(fs.realpathSync.native(targetPath)); + } catch { + // The subsequent move will surface missing or inaccessible targets. + } + const isAllowed = resolveAllowedTrashRoots().some( + (root) => resolvedTargetPath !== root && isSameOrChildPath(resolvedTargetPath, root), + ); + if (!isAllowed) { + throw new Error(`Refusing to trash path outside allowed roots: ${targetPath}`); + } +} + +function resolveTrashDir(): string { + const homeDir = os.homedir(); + const trashDir = path.join(homeDir, ".Trash"); + fs.mkdirSync(trashDir, { recursive: true, mode: 0o700 }); + const trashDirStat = fs.lstatSync(trashDir); + if (!trashDirStat.isDirectory() || trashDirStat.isSymbolicLink()) { + throw new Error(`Refusing to use non-directory/symlink trash directory: ${trashDir}`); + } + const realHome = path.resolve(fs.realpathSync.native(homeDir)); + const resolvedTrashDir = path.resolve(fs.realpathSync.native(trashDir)); + if (resolvedTrashDir === realHome || !isSameOrChildPath(resolvedTrashDir, realHome)) { + throw new Error(`Trash directory escaped home directory: ${trashDir}`); + } + return resolvedTrashDir; +} + +function trashBaseName(targetPath: string): string { + const resolvedTargetPath = path.resolve(targetPath); + if (resolvedTargetPath === path.parse(resolvedTargetPath).root) { + throw new Error(`Refusing to trash root path: ${targetPath}`); + } + const base = path.basename(resolvedTargetPath).replace(/[\\/]+/g, ""); + if (!base) { + throw new Error(`Unable to derive safe trash basename for: ${targetPath}`); + } + return base; +} + +function resolveContainedPath(root: string, leaf: string): string { + const resolvedRoot = path.resolve(root); + const resolvedPath = path.resolve(resolvedRoot, leaf); + if (!isSameOrChildPath(resolvedPath, resolvedRoot) || resolvedPath === resolvedRoot) { + throw new Error(`Trash destination escaped trash directory: ${resolvedPath}`); + } + return resolvedPath; +} + +function reserveTrashDestination(trashDir: string, base: string, timestamp: number): string { + const containerPrefix = resolveContainedPath(trashDir, `${base}-${timestamp}-`); + const container = fs.mkdtempSync(containerPrefix); + const resolvedContainer = path.resolve(container); + const resolvedTrashDir = path.resolve(trashDir); + if ( + resolvedContainer === resolvedTrashDir || + !isSameOrChildPath(resolvedContainer, resolvedTrashDir) + ) { + throw new Error(`Trash destination escaped trash directory: ${container}`); + } + return resolveContainedPath(container, base); +} + +function movePathToDestination(targetPath: string, dest: string): boolean { + try { + fs.renameSync(targetPath, dest); + return true; + } catch (error) { + if (getFsErrorCode(error) !== "EXDEV") { + if (isTrashDestinationCollision(error)) { + return false; + } + throw error; + } + } + + try { + fs.cpSync(targetPath, dest, { recursive: true, force: false, errorOnExist: true }); + fs.rmSync(targetPath, { recursive: true, force: false }); + return true; + } catch (error) { + if (isTrashDestinationCollision(error)) { + return false; + } + throw error; + } } export async function closeTrackedBrowserTabsForSessions( @@ -60,22 +167,17 @@ export async function closeTrackedBrowserTabsForSessions( } export async function movePathToTrash(targetPath: string): Promise { - const [{ generateSecureToken }, { runExec }] = await Promise.all([ - loadSecureRandomRuntime(), - loadExecRuntime(), - ]); - try { - await runExec("trash", [targetPath], { timeoutMs: 10_000 }); - return targetPath; - } catch { - const trashDir = path.join(os.homedir(), ".Trash"); - fs.mkdirSync(trashDir, { recursive: true }); - const base = path.basename(targetPath); - let dest = path.join(trashDir, `${base}-${Date.now()}`); - if (fs.existsSync(dest)) { - dest = path.join(trashDir, `${base}-${Date.now()}-${generateSecureToken(6)}`); + // Avoid resolving external trash helpers through the service PATH during cleanup. + const base = trashBaseName(targetPath); + assertAllowedTrashTarget(targetPath); + const trashDir = resolveTrashDir(); + const timestamp = Date.now(); + for (let attempt = 0; attempt < TRASH_DESTINATION_RETRY_LIMIT; attempt += 1) { + const dest = reserveTrashDestination(trashDir, base, timestamp); + if (movePathToDestination(targetPath, dest)) { + return dest; } - fs.renameSync(targetPath, dest); - return dest; } + + throw new Error(`Unable to choose a unique trash destination for ${targetPath}`); }