diff --git a/src/application/ClientConfigPath.ts b/src/application/ClientConfigPath.ts index 67e634b01b..52c1091bb8 100644 --- a/src/application/ClientConfigPath.ts +++ b/src/application/ClientConfigPath.ts @@ -1,5 +1,6 @@ import { lstat, realpath } from "node:fs/promises"; -import { basename, dirname, join, resolve } from "node:path"; +import { basename, dirname, join } from "node:path"; +import { absoluteCallerPath } from "../filesystem/callerPath.js"; interface ClientConfigPathStats { readonly uid?: number; @@ -70,9 +71,9 @@ const resolveOwnedConfigSymlink = async ( const canonicalRegularConfigPath = async ( requestedPath: string, fileSystem: ClientConfigPathFileSystem, -): Promise => { +): Promise => { if (fileSystem.realpath === undefined) return requestedPath; - let ancestor = resolve(requestedPath); + let ancestor = absoluteCallerPath(requestedPath); const missing: string[] = []; while (true) { try { @@ -80,6 +81,8 @@ const canonicalRegularConfigPath = async ( } catch (cause: unknown) { if (!isAbsentPath(cause) || dirname(ancestor) === ancestor) return requestedPath; + // An absent directory before '..' cannot establish the selected parent. + if (basename(ancestor) === "..") return undefined; missing.unshift(basename(ancestor)); ancestor = dirname(ancestor); } diff --git a/src/application/ClientConfigurationPublication.ts b/src/application/ClientConfigurationPublication.ts index a2af20999d..1fb69059d9 100644 --- a/src/application/ClientConfigurationPublication.ts +++ b/src/application/ClientConfigurationPublication.ts @@ -4,6 +4,7 @@ import { basename, dirname, join, resolve, sep } from "node:path"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; import { fileHandleMatchesBytes } from "../filesystem/fileHandleMatchesBytes.js"; +import { absoluteCallerPath } from "../filesystem/callerPath.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { OwnedStagedOutput } from "../filesystem/OwnedStagedOutput.js"; import { @@ -290,10 +291,10 @@ const isParentAliasRetarget = async ( ); } if (requestedStats.isSymbolicLink()) return false; - const requestedParent = dirname(resolve(requestedPath)); + const requestedParent = dirname(absoluteCallerPath(requestedPath)); try { const canonicalParent = await realpath(requestedParent); - return resolve(canonicalParent) !== requestedParent; + return canonicalParent !== requestedParent; } catch (cause: unknown) { void cause; return false; diff --git a/tests/boundary/filesystem/clientConfigurationTransactions.test.ts b/tests/boundary/filesystem/clientConfigurationTransactions.test.ts index f00eaf6fb2..6802427ada 100644 --- a/tests/boundary/filesystem/clientConfigurationTransactions.test.ts +++ b/tests/boundary/filesystem/clientConfigurationTransactions.test.ts @@ -2,9 +2,11 @@ import { chmod, copyFile, lstat, + mkdir, readFile, realpath, rm, + symlink, writeFile, } from "node:fs/promises"; import { constants as fsConstants } from "node:fs"; @@ -25,6 +27,43 @@ import { import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { ClientConfigurationPublication } from "../../../src/application/ClientConfigurationPublication.js"; +it + .skipIf(process.platform === "win32") + .each(["direct", "parent", "lexical-parent"] as const)( + "keeps the selected file after a parent alias retarget: %s", + async (variant) => { + const parentTraversal = variant !== "direct"; + const root = await createTestTempDirectory("rea-config-retarget-"); + const first = join(root, "first"); + const second = variant === "lexical-parent" ? root : join(root, "second"); + for (const directory of [first, second]) { + await mkdir(join(directory, "child"), { recursive: true }); + await writeFile(join(directory, "config.json"), "original"); + } + const alias = join(root, "link"); + await symlink(parentTraversal ? join(first, "child") : first, alias); + const requestedPath = `${alias}/${parentTraversal ? "../" : ""}config.json`; + const selected = join(first, "config.json"); + const state = await lstat(selected); + await rm(alias); + await symlink(parentTraversal ? join(second, "child") : second, alias); + const publication = await ClientConfigurationPublication.write( + selected, + "updated", + "original", + undefined, + { requestedPath, state }, + ); + await publication.verify(async () => { + await publication.assertCurrent(); + expect(await readFile(selected, "utf8")).toBe("updated"); + expect(await readFile(join(second, "config.json"), "utf8")).toBe( + "original", + ); + }); + }, +); + const failedVerificationFileSystem = ( failure: "verification" | "recovery", ): UninstallFileSystem => { diff --git a/tests/boundary/filesystem/setupConfig.test.ts b/tests/boundary/filesystem/setupConfig.test.ts index ea21eec1b3..94117bc95f 100644 --- a/tests/boundary/filesystem/setupConfig.test.ts +++ b/tests/boundary/filesystem/setupConfig.test.ts @@ -121,6 +121,42 @@ it.skipIf(process.platform === "win32")( }, ); +it.skipIf(process.platform === "win32").each([false, true])( + "preserves filesystem parent traversal when the configuration exists=%s", + async (exists) => { + const root = await createTestTempDirectory("rea-config-parent-traversal-"); + const selectedDirectory = join(root, "selected"); + await mkdir(join(selectedDirectory, "child"), { recursive: true }); + await symlink(join(selectedDirectory, "child"), join(root, "link"), "dir"); + const selected = join(selectedDirectory, "config.json"); + const unrelated = join(root, "config.json"); + const original = '{"theme":"keep"}\n'; + await writeFile(unrelated, original); + if (exists) await writeFile(selected, original); + const requested = `${root}/link/../config.json`; + expect(await resolveClientConfigTransactionPath(requested)).toBe(selected); + expect( + await configureClientConfiguration({ + name: "cursor", + format: "json", + configPath: requested, + }), + ).toMatchObject({ status: "configured" }); + expect(JSON.parse(await readFile(selected, "utf8"))).toMatchObject({ + mcpServers: { rea: { command: "npx" } }, + }); + expect(await readFile(unrelated, "utf8")).toBe(original); + await expect(lstat(`${unrelated}.rea.backup`)).rejects.toMatchObject({ + code: "ENOENT", + }); + if (exists) + expect(await readFile(`${selected}.rea.backup`, "utf8")).toBe(original); + expect( + await resolveClientConfigTransactionPath(`${root}/absent/../config.json`), + ).toBeUndefined(); + }, +); + describe("JSON client configuration transaction: writing configuration through safe paths", () => { it.skipIf(typeof process.getuid !== "function")( "rejects a symlink target not owned by the current user",