diff --git a/src/infra/install-package-dir.test.ts b/src/infra/install-package-dir.test.ts index 9dcd532cdc4..cc0abac1b3c 100644 --- a/src/infra/install-package-dir.test.ts +++ b/src/infra/install-package-dir.test.ts @@ -11,6 +11,58 @@ async function listMatchingDirs(root: string, prefix: string): Promise .map((entry) => entry.name); } +function normalizeDarwinTmpPath(filePath: string): string { + return process.platform === "darwin" && filePath.startsWith("/private/var/") + ? filePath.slice("/private".length) + : filePath; +} + +async function rebindInstallBasePath(params: { + installBaseDir: string; + preservedDir: string; + outsideTarget: string; +}): Promise { + await fs.rename(params.installBaseDir, params.preservedDir); + await fs.symlink( + params.outsideTarget, + params.installBaseDir, + process.platform === "win32" ? "junction" : undefined, + ); +} + +async function withInstallBaseReboundOnRealpathCall(params: { + installBaseDir: string; + preservedDir: string; + outsideTarget: string; + rebindAtCall: number; + run: () => Promise; +}): Promise { + const installBasePath = normalizeDarwinTmpPath(path.resolve(params.installBaseDir)); + const realRealpath = fs.realpath.bind(fs); + let installBaseRealpathCalls = 0; + const realpathSpy = vi + .spyOn(fs, "realpath") + .mockImplementation(async (...args: Parameters) => { + const filePath = normalizeDarwinTmpPath(path.resolve(String(args[0]))); + if (filePath === installBasePath) { + installBaseRealpathCalls += 1; + if (installBaseRealpathCalls === params.rebindAtCall) { + await rebindInstallBasePath({ + installBaseDir: params.installBaseDir, + preservedDir: params.preservedDir, + outsideTarget: params.outsideTarget, + }); + } + } + return await realRealpath(...args); + }); + try { + return await params.run(); + } finally { + realpathSpy.mockRestore(); + } +} + describe("installPackageDir", () => { let fixtureRoot = ""; @@ -103,4 +155,97 @@ describe("installPackageDir", () => { const backupRoot = path.join(installBaseDir, ".openclaw-install-backups"); await expect(fs.readdir(backupRoot)).resolves.toHaveLength(0); }); + + it("aborts without outside writes when the install base is rebound before publish", async () => { + fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-install-package-dir-")); + const sourceDir = path.join(fixtureRoot, "source"); + const installBaseDir = path.join(fixtureRoot, "plugins"); + const preservedInstallRoot = path.join(fixtureRoot, "plugins-preserved"); + const outsideInstallRoot = path.join(fixtureRoot, "outside-plugins"); + const targetDir = path.join(installBaseDir, "demo"); + await fs.mkdir(sourceDir, { recursive: true }); + await fs.mkdir(installBaseDir, { recursive: true }); + await fs.mkdir(outsideInstallRoot, { recursive: true }); + await fs.writeFile(path.join(sourceDir, "marker.txt"), "new"); + + const warnings: string[] = []; + await withInstallBaseReboundOnRealpathCall({ + installBaseDir, + preservedDir: preservedInstallRoot, + outsideTarget: outsideInstallRoot, + rebindAtCall: 3, + run: async () => { + await expect( + installPackageDir({ + sourceDir, + targetDir, + mode: "install", + timeoutMs: 1_000, + copyErrorPrefix: "failed to copy plugin", + hasDeps: false, + depsLogMessage: "Installing deps…", + logger: { warn: (message) => warnings.push(message) }, + }), + ).resolves.toEqual({ + ok: false, + error: "failed to copy plugin: Error: install base directory changed during install", + }); + }, + }); + + await expect( + fs.stat(path.join(outsideInstallRoot, "demo", "marker.txt")), + ).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(warnings).toContain( + "Install base directory changed during install; aborting staged publish.", + ); + }); + + it("warns and leaves the backup in place when the install base changes before backup cleanup", async () => { + fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-install-package-dir-")); + const sourceDir = path.join(fixtureRoot, "source"); + const installBaseDir = path.join(fixtureRoot, "plugins"); + const preservedInstallRoot = path.join(fixtureRoot, "plugins-preserved"); + const outsideInstallRoot = path.join(fixtureRoot, "outside-plugins"); + const targetDir = path.join(installBaseDir, "demo"); + await fs.mkdir(sourceDir, { recursive: true }); + await fs.mkdir(installBaseDir, { recursive: true }); + await fs.mkdir(outsideInstallRoot, { recursive: true }); + await fs.mkdir(path.join(installBaseDir, "demo"), { recursive: true }); + await fs.writeFile(path.join(installBaseDir, "demo", "marker.txt"), "old"); + await fs.writeFile(path.join(sourceDir, "marker.txt"), "new"); + + const warnings: string[] = []; + const result = await withInstallBaseReboundOnRealpathCall({ + installBaseDir, + preservedDir: preservedInstallRoot, + outsideTarget: outsideInstallRoot, + rebindAtCall: 7, + run: async () => + await installPackageDir({ + sourceDir, + targetDir, + mode: "update", + timeoutMs: 1_000, + copyErrorPrefix: "failed to copy plugin", + hasDeps: false, + depsLogMessage: "Installing deps…", + logger: { warn: (message) => warnings.push(message) }, + }), + }); + + expect(result).toEqual({ ok: true }); + expect(warnings).toContain( + "Install base directory changed before backup cleanup; leaving backup in place.", + ); + await expect( + fs.stat(path.join(outsideInstallRoot, "demo", "marker.txt")), + ).rejects.toMatchObject({ + code: "ENOENT", + }); + const backupRoot = path.join(preservedInstallRoot, ".openclaw-install-backups"); + await expect(fs.readdir(backupRoot)).resolves.toHaveLength(1); + }); }); diff --git a/src/infra/install-package-dir.ts b/src/infra/install-package-dir.ts index d2364558f5c..17878599160 100644 --- a/src/infra/install-package-dir.ts +++ b/src/infra/install-package-dir.ts @@ -4,6 +4,12 @@ import { runCommandWithTimeout } from "../process/exec.js"; import { fileExists } from "./archive.js"; import { assertCanonicalPathWithinBase } from "./install-safe-path.js"; +const INSTALL_BASE_CHANGED_ERROR_MESSAGE = "install base directory changed during install"; +const INSTALL_BASE_CHANGED_ABORT_WARNING = + "Install base directory changed during install; aborting staged publish."; +const INSTALL_BASE_CHANGED_BACKUP_WARNING = + "Install base directory changed before backup cleanup; leaving backup in place."; + function isObjectRecord(value: unknown): value is Record { return Boolean(value) && typeof value === "object" && !Array.isArray(value); } @@ -68,26 +74,54 @@ function isRelativePathInsideBase(relativePath: string): boolean { ); } +function isInstallBaseChangedError(error: unknown): boolean { + return error instanceof Error && error.message === INSTALL_BASE_CHANGED_ERROR_MESSAGE; +} + async function assertInstallBaseStable(params: { installBaseDir: string; expectedRealPath: string; }): Promise { const baseLstat = await fs.lstat(params.installBaseDir); if (!baseLstat.isDirectory() || baseLstat.isSymbolicLink()) { - throw new Error("install base directory changed during install"); + throw new Error(INSTALL_BASE_CHANGED_ERROR_MESSAGE); } const currentRealPath = await fs.realpath(params.installBaseDir); if (currentRealPath !== params.expectedRealPath) { - throw new Error("install base directory changed during install"); + throw new Error(INSTALL_BASE_CHANGED_ERROR_MESSAGE); } } +async function cleanupInstallTempDir(dirPath: string | null): Promise { + if (!dirPath) { + return; + } + await fs.rm(dirPath, { recursive: true, force: true }).catch(() => undefined); +} + +async function resolveInstallPublishTarget(params: { + installBaseDir: string; + targetDir: string; +}): Promise<{ installBaseRealPath: string; canonicalTargetDir: string }> { + const installBaseResolved = path.resolve(params.installBaseDir); + const targetResolved = path.resolve(params.targetDir); + const targetRelativePath = path.relative(installBaseResolved, targetResolved); + if (!isRelativePathInsideBase(targetRelativePath)) { + throw new Error("invalid install target path"); + } + const installBaseRealPath = await fs.realpath(params.installBaseDir); + return { + installBaseRealPath, + canonicalTargetDir: path.join(installBaseRealPath, targetRelativePath), + }; +} + export async function installPackageDir(params: { sourceDir: string; targetDir: string; mode: "install" | "update"; timeoutMs: number; - logger?: { info?: (message: string) => void }; + logger?: { info?: (message: string) => void; warn?: (message: string) => void }; copyErrorPrefix: string; hasDeps: boolean; depsLogMessage: string; @@ -100,22 +134,29 @@ export async function installPackageDir(params: { installBaseDir, candidatePaths: [params.targetDir], }); - const installBaseResolved = path.resolve(installBaseDir); - const targetResolved = path.resolve(params.targetDir); - const targetRelativePath = path.relative(installBaseResolved, targetResolved); - if (!isRelativePathInsideBase(targetRelativePath)) { - return { ok: false, error: `${params.copyErrorPrefix}: Error: invalid install target path` }; + let installBaseRealPath: string; + let canonicalTargetDir: string; + try { + ({ installBaseRealPath, canonicalTargetDir } = await resolveInstallPublishTarget({ + installBaseDir, + targetDir: params.targetDir, + })); + } catch (err) { + return { ok: false, error: `${params.copyErrorPrefix}: ${String(err)}` }; } - const installBaseRealPath = await fs.realpath(installBaseDir); - const canonicalTargetDir = path.join(installBaseRealPath, targetRelativePath); let stageDir: string | null = null; let backupDir: string | null = null; - const fail = async (error: string) => { - await restoreBackup(); - if (stageDir) { - await fs.rm(stageDir, { recursive: true, force: true }).catch(() => undefined); - stageDir = null; + const fail = async (error: string, cause?: unknown) => { + const installBaseChanged = isInstallBaseChangedError(cause); + if (installBaseChanged) { + params.logger?.warn?.(INSTALL_BASE_CHANGED_ABORT_WARNING); + } else { + await restoreBackup(); + if (stageDir) { + await cleanupInstallTempDir(stageDir); + stageDir = null; + } } return { ok: false as const, error }; }; @@ -135,13 +176,13 @@ export async function installPackageDir(params: { stageDir = await fs.mkdtemp(path.join(installBaseRealPath, ".openclaw-install-stage-")); await fs.cp(params.sourceDir, stageDir, { recursive: true }); } catch (err) { - return await fail(`${params.copyErrorPrefix}: ${String(err)}`); + return await fail(`${params.copyErrorPrefix}: ${String(err)}`, err); } try { await params.afterCopy?.(stageDir); } catch (err) { - return await fail(`post-copy validation failed: ${String(err)}`); + return await fail(`post-copy validation failed: ${String(err)}`, err); } if (params.hasDeps) { @@ -174,7 +215,7 @@ export async function installPackageDir(params: { }); await fs.rename(canonicalTargetDir, backupDir); } catch (err) { - return await fail(`${params.copyErrorPrefix}: ${String(err)}`); + return await fail(`${params.copyErrorPrefix}: ${String(err)}`, err); } } @@ -186,14 +227,27 @@ export async function installPackageDir(params: { await fs.rename(stageDir, canonicalTargetDir); stageDir = null; } catch (err) { - return await fail(`${params.copyErrorPrefix}: ${String(err)}`); + return await fail(`${params.copyErrorPrefix}: ${String(err)}`, err); } + if (backupDir) { + try { + await assertInstallBaseStable({ + installBaseDir, + expectedRealPath: installBaseRealPath, + }); + } catch (err) { + if (isInstallBaseChangedError(err)) { + params.logger?.warn?.(INSTALL_BASE_CHANGED_BACKUP_WARNING); + } + backupDir = null; + } + } if (backupDir) { await fs.rm(backupDir, { recursive: true, force: true }).catch(() => undefined); } if (stageDir) { - await fs.rm(stageDir, { recursive: true, force: true }).catch(() => undefined); + await cleanupInstallTempDir(stageDir); } return { ok: true }; @@ -204,7 +258,7 @@ export async function installPackageDirWithManifestDeps(params: { targetDir: string; mode: "install" | "update"; timeoutMs: number; - logger?: { info?: (message: string) => void }; + logger?: { info?: (message: string) => void; warn?: (message: string) => void }; copyErrorPrefix: string; depsLogMessage: string; manifestDependencies?: Record;