From 68f2d67c56f283a750371c65fcd536af71ad2313 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:26:17 +0800 Subject: [PATCH] fix(paths): preserve native selections through analysis and removal --- src/application/Uninstall.ts | 27 ++++--- .../JavaScriptArtifactReconstruction.ts | 6 +- src/artifacts/SafeOutputTree.ts | 7 +- .../extraction/ArtifactExtraction.ts | 6 +- .../setup/selectedClientHomePaths.test.ts | 64 ++++++++++++++++- .../wakaruCommandConfiguration.test.ts | 6 +- tests/boundary/mcp/electronMcp.test.ts | 72 +++++++++++++++++++ .../artifactProvider.extraction.test.ts | 8 +-- 8 files changed, 171 insertions(+), 25 deletions(-) diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index c76169bbe7..5e10a9e2e0 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -1,3 +1,4 @@ +import { appendCallerPath } from "../filesystem/callerPath.js"; import { isClientConfigurationCleanupFailure, retryClientConfigurationCleanup, @@ -18,7 +19,7 @@ import { ConfigurationError } from "../domain/configurationErrors.js"; import { copyFile, lstat, realpath, rm, rmdir } from "node:fs/promises"; import { constants as fsConstants } from "node:fs"; import { homeDirectoryFromEnvironment } from "../config/homeDirectory.js"; -import { basename, dirname, join } from "node:path"; +import { basename, dirname } from "node:path"; import { defaultKunaCacheDirectory } from "../config/kunaCacheDirectory.js"; import { linuxHopperReleaseCacheDirectory } from "./LinuxHopperCacheDirectory.js"; @@ -1046,7 +1047,7 @@ const purgeSelectedData = async ( fileSystem: UninstallFileSystem, ): Promise => { const items = [ - ...(await purgeManagedData(join(home, ".rea"), fileSystem, [ + ...(await purgeManagedData(appendCallerPath(home, ".rea"), fileSystem, [ "cache", "state", ])), @@ -1097,7 +1098,11 @@ const purgeManagedData = async ( } catch (cause: unknown) { return names.map((name) => isMissing(cause) - ? item(name, "skipped", `${join(root, name)} does not exist.`) + ? item( + name, + "skipped", + `${appendCallerPath(root, name)} does not exist.`, + ) : item( name, "failed", @@ -1107,7 +1112,9 @@ const purgeManagedData = async ( } const items: UninstallItem[] = []; for (const name of names) - items.push(await removeManagedPath(join(root, name), name, fileSystem)); + items.push( + await removeManagedPath(appendCallerPath(root, name), name, fileSystem), + ); return items; }; @@ -1210,7 +1217,7 @@ const inspectManagedSkill = async ( if (root.isDirectory?.() !== true) return item(name, "retained", "Skill path is not a directory."); - const manifestPath = join(path, "SKILL.md"); + const manifestPath = appendCallerPath(path, "SKILL.md"); try { const manifest = await fileSystem.stat(manifestPath); if (manifest.isSymbolicLink()) @@ -1315,7 +1322,9 @@ const removeEmptyManagedDirectories = async ( ): Promise => { if (fileSystem.removeEmptyDirectory === undefined) return undefined; for (const directory of [ - ...MANAGED_SKILL_DIRECTORIES.map((relative) => join(root, relative)), + ...MANAGED_SKILL_DIRECTORIES.map((relative) => + appendCallerPath(root, relative), + ), root, ]) { try { @@ -1347,7 +1356,7 @@ const removeManagedSkillFile = async ( const segments = relativePath.split(/[\\/]/u); let parent = root; for (const segment of segments.slice(0, -1)) { - parent = join(parent, segment); + parent = appendCallerPath(parent, segment); try { const stats = await fileSystem.stat(parent); if (stats.isSymbolicLink() || stats.isDirectory?.() !== true) @@ -1361,7 +1370,7 @@ const removeManagedSkillFile = async ( ? item( name, "skipped", - `${join(parent, segments.at(-1) ?? "")} does not exist.`, + `${appendCallerPath(parent, segments.at(-1) ?? "")} does not exist.`, ) : item( name, @@ -1371,7 +1380,7 @@ const removeManagedSkillFile = async ( } } - const target = join(root, relativePath); + const target = appendCallerPath(root, relativePath); let stats: ManagedPathStats; try { stats = await fileSystem.stat(target); diff --git a/src/application/javascript/JavaScriptArtifactReconstruction.ts b/src/application/javascript/JavaScriptArtifactReconstruction.ts index 88677d6e3c..e16e67b568 100644 --- a/src/application/javascript/JavaScriptArtifactReconstruction.ts +++ b/src/application/javascript/JavaScriptArtifactReconstruction.ts @@ -8,7 +8,7 @@ import { buildJavaScriptRouteInventorySteps, type JavaScriptRouteInventory, } from "../../domain/javascript/javascriptRouteInventory.js"; -import { access, lstat, realpath } from "node:fs/promises"; +import { access, realpath, stat } from "node:fs/promises"; import { setImmediate } from "node:timers/promises"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; @@ -443,8 +443,10 @@ const resolveSelectedInput = async ( inputPath: string, ): Promise<{ readonly path: string; readonly metadata: Stats }> => { try { + // Native lookup must admit the selected traversal first: Darwin realpath + // can collapse file/.. even though that path cannot be opened. + const metadata = await stat(inputPath); const path = await realpath(inputPath); - const metadata = await lstat(path); // Directory enumeration needs read access; resolving its entries also needs search access. await access( path, diff --git a/src/artifacts/SafeOutputTree.ts b/src/artifacts/SafeOutputTree.ts index 19f152e956..401f25a71a 100644 --- a/src/artifacts/SafeOutputTree.ts +++ b/src/artifacts/SafeOutputTree.ts @@ -100,7 +100,7 @@ export class SafeOutputTree { options: { readonly acceptExistingEmpty?: boolean } = {}, ): Promise { const { requested, canonicalOutput } = - await this.#resolveDestination(outputRoot); + await this.resolveDestination(outputRoot); let created = true; await mkdir(canonicalOutput, { mode: 0o700 }).catch((cause: unknown) => { if (!isAlreadyExists(cause)) throw cause; @@ -147,7 +147,7 @@ export class SafeOutputTree { /** Read-only early admission; create() still rechecks and acquires exclusively. */ static async checkDestination(outputRoot: string): Promise { - const { canonicalOutput } = await this.#resolveDestination(outputRoot); + const { canonicalOutput } = await this.resolveDestination(outputRoot); const existing = await lstat(canonicalOutput).catch((cause: unknown) => { if (cause instanceof Error && "code" in cause && cause.code === "ENOENT") return undefined; @@ -183,7 +183,8 @@ export class SafeOutputTree { }); } - static async #resolveDestination(outputRoot: string) { + /** Resolve an admissible parent without acquiring output; create rechecks it. */ + static async resolveDestination(outputRoot: string) { if (!isAbsolute(outputRoot)) throw new SafeOutputTreeDestinationRefusal( "Extraction output root must be absolute", diff --git a/src/artifacts/extraction/ArtifactExtraction.ts b/src/artifacts/extraction/ArtifactExtraction.ts index ec3d2dc5a4..48e110e79d 100644 --- a/src/artifacts/extraction/ArtifactExtraction.ts +++ b/src/artifacts/extraction/ArtifactExtraction.ts @@ -9,7 +9,6 @@ import type { } from "../ArtifactResourceScope.js"; import { lstat, realpath } from "node:fs/promises"; import { dirname } from "node:path"; -import { absoluteCallerPath } from "../../filesystem/callerPath.js"; import { isPathWithinRoot } from "../../domain/localPath.js"; import { SafeOutputTreeDestinationRefusal } from "../SafeOutputTreeAdmission.js"; @@ -149,9 +148,10 @@ export const materializeArtifactInventoryInScope = async ( if (zipSource === undefined) { const canonicalSource = await realpath(sourcePath); if ((await lstat(canonicalSource)).isDirectory()) { - const parent = await realpath( - dirname(absoluteCallerPath(input.outputRoot)), + const { canonicalOutput } = await SafeOutputTree.resolveDestination( + input.outputRoot, ); + const parent = dirname(canonicalOutput); if (isPathWithinRoot(canonicalSource, parent)) throw new SafeOutputTreeDestinationRefusal( `Extraction output parent ${parent} is inside source directory ${canonicalSource}. Choose a temporary directory outside the source.`, diff --git a/tests/acceptance/setup/selectedClientHomePaths.test.ts b/tests/acceptance/setup/selectedClientHomePaths.test.ts index e71af9e6a3..46bc514aaf 100644 --- a/tests/acceptance/setup/selectedClientHomePaths.test.ts +++ b/tests/acceptance/setup/selectedClientHomePaths.test.ts @@ -10,6 +10,7 @@ import { dirname, join } from "node:path"; import { expect } from "vitest"; +import { MANAGED_SKILL_FILES } from "../../../src/application/SetupSkill.js"; import { parseClientConfiguration } from "../../../src/application/ClientConfigurationDocument.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; @@ -111,7 +112,7 @@ cliTest.skipIf(process.platform === "win32")( ); cliTest.skipIf(process.platform === "win32")( - "installs shared skills in the native HOME selection with backups and repeat setup", + "uses the native HOME selection for shared skill setup, readback and uninstall", async ({ cli }) => { const root = await createTestTempDirectory("rea-selected-skill-home-"); const selected = join(root, "selected"); @@ -136,7 +137,11 @@ cliTest.skipIf(process.platform === "win32")( cli.run({ arguments: [...arguments_, "--json"], cwd: root, - environment: { HOME: home, USERPROFILE: home }, + environment: { + HOME: home, + USERPROFILE: home, + XDG_CACHE_HOME: `${home}/.cache`, + }, }); const configured = await run([ "setup", @@ -200,5 +205,60 @@ cliTest.skipIf(process.platform === "win32")( name.startsWith(".rea-skill-"), ), ).toEqual([]); + const cacheRelative = + process.platform === "darwin" + ? "Library/Caches/rea/kuna" + : process.platform === "linux" + ? ".cache/rea/kuna" + : undefined; + const cacheDirectories = [ + ".rea/cache", + ".rea/state", + ...(cacheRelative === undefined ? [] : [cacheRelative]), + ]; + for (const relative of cacheDirectories) { + await mkdir(join(selected, relative), { recursive: true }); + await mkdir(join(root, relative), { recursive: true }); + await writeFile(join(selected, relative, "marker"), "selected cache"); + await writeFile(join(root, relative, "marker"), "unselected cache"); + } + const legacyUserNote = join(selected, ".rea/analyst-note.txt"); + await writeFile(legacyUserNote, "legacy user note\n"); + const userNote = join(skill, "analyst-note.txt"); + await writeFile(userNote, "selected user note\n"); + for (let attempt = 0; attempt < 2; attempt += 1) { + const uninstalled = await run(["uninstall", "--purge-data"]); + expect( + uninstalled.exitCode, + uninstalled.stdout + uninstalled.stderr, + ).toBe(0); + expect(uninstalled.json).toMatchObject({ status: "complete" }); + for (const relative of cacheDirectories) { + await expect(access(join(selected, relative))).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(await readFile(join(root, relative, "marker"), "utf8")).toBe( + "unselected cache", + ); + } + expect(await readFile(legacyUserNote, "utf8")).toBe("legacy user note\n"); + for (const relativePath of MANAGED_SKILL_FILES) + await expect(access(join(skill, relativePath))).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(await readFile(userNote, "utf8")).toBe("selected user note\n"); + expect(await readFile(join(skill, "SKILL.md.rea.backup"), "utf8")).toBe( + original, + ); + expect( + await readFile(`${join(skill, reference)}.rea.backup`, "utf8"), + ).toBe(oldReference); + expect(await readFile(join(lexicalSkill, "SKILL.md"), "utf8")).toBe( + sentinel, + ); + expect(await readFile(join(lexicalSkill, reference), "utf8")).toBe( + sentinelReference, + ); + } }, ); diff --git a/tests/boundary/javascript-recovery/wakaruCommandConfiguration.test.ts b/tests/boundary/javascript-recovery/wakaruCommandConfiguration.test.ts index ce9845be5d..6d2b482d8c 100644 --- a/tests/boundary/javascript-recovery/wakaruCommandConfiguration.test.ts +++ b/tests/boundary/javascript-recovery/wakaruCommandConfiguration.test.ts @@ -1,4 +1,4 @@ -import { mkdir, symlink, writeFile } from "node:fs/promises"; +import { mkdir, realpath, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it } from "vitest"; import { resolveWakaruCommand } from "../../../src/javascript/recovery/WakaruCommand.js"; @@ -43,5 +43,7 @@ linuxX64("allows a symlink to a regular Wakaru resource limiter", async () => { REA_WAKARU_COMMAND: command, REA_JAVASCRIPT_PRLIMIT_COMMAND: limiter, }), - ).resolves.toMatchObject({ limiter }); + ).resolves.toMatchObject({ + limiter: { invocationPath: limiter, resolvedPath: await realpath(target) }, + }); }); diff --git a/tests/boundary/mcp/electronMcp.test.ts b/tests/boundary/mcp/electronMcp.test.ts index aa503be737..c9f416fe86 100644 --- a/tests/boundary/mcp/electronMcp.test.ts +++ b/tests/boundary/mcp/electronMcp.test.ts @@ -25,6 +25,7 @@ import { } from "../../fixtures/fakeCdpBrowser.js"; import { writeElectronBoundaryFixture } from "../../fixtures/electronBoundaryApplication.js"; import { createElectronActiveObservationFixtureResult } from "../../../src/domain/javascript/electronActiveObservation.fixture.js"; +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { AsarArtifactReader } from "../../../src/artifacts/AsarArtifactReader.js"; import { toolAvailabilityInventory } from "../../../src/contracts/toolOutputSchemaPrimitives.js"; @@ -659,6 +660,77 @@ const evidenceFor = (value: unknown) => { return parseEvidence(value); }; +it.skipIf(process.platform === "win32")( + "admits the selected application source before canonicalizing its path through MCP", + async () => { + const root = await realpath( + await createTestTempDirectory("rea-application-source-admission-"), + ); + await mkdir(join(root, "app")); + await mkdir(join(root, "selected/app"), { recursive: true }); + await mkdir(join(root, "selected/child")); + await writeFile(join(root, "file"), "not a directory"); + await writeFile(join(root, "app/package.json"), '{"name":"sibling"}'); + await writeFile( + join(root, "selected/app/package.json"), + '{"name":"selected"}', + ); + await symlink("selected/child", join(root, "link")); + const session = createTestBinarySession(() => { + throw new Error( + "Static application analysis must not start a deep provider", + ); + }); + const server = createServer({ kind: "session", session }); + const client = new Client({ + name: "application-source-admission", + version: "1", + }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + resources.push(client, server, session); + await server.connect(serverTransport); + await client.connect(clientTransport); + const initial = await client.callTool({ + name: "analyze_javascript_application", + arguments: { input_path: join(root, "app") }, + }); + expect(initial.isError, JSON.stringify(initial)).not.toBe(true); + const initialEvidence = parseEvidence(initial.structuredContent); + const refused = await client.callTool({ + name: "analyze_javascript_application", + arguments: { input_path: `${root}/file/../app` }, + }); + expect(refused.isError).toBe(true); + expect(parseMcpToolError(refused)).toMatchObject({ + error: { + code: "invalid_request", + details: { issues: [{ path: ["input_path"] }] }, + }, + }); + const selected = `${root}/link/../app`; + const admitted = await client.callTool({ + name: "analyze_javascript_application", + arguments: { input_path: selected }, + }); + expect(admitted.isError, JSON.stringify(admitted)).not.toBe(true); + const evidence = parseEvidence(admitted.structuredContent); + const result = javascriptApplicationAnalysisResultSchema.parse( + evidence.normalized_result, + ); + expect(evidence.parameters.input_path).toBe(selected); + expect(evidence.subject?.local_path).toBe(join(root, "selected/app")); + expect(result.input_path).toBe(join(root, "selected/app")); + expect(result.root_artifact_sha256).not.toBe( + javascriptApplicationAnalysisResultSchema.parse( + initialEvidence.normalized_result, + ).root_artifact_sha256, + ); + expect(initialEvidence.subject?.local_path).toBe(join(root, "app")); + await client.ping(); + }, +); + const activePathAdmissionClient = async ( capture: ElectronActiveObservationPort["capture"], ): Promise => { diff --git a/tests/boundary/providers/native/artifactProvider.extraction.test.ts b/tests/boundary/providers/native/artifactProvider.extraction.test.ts index 52471b3318..81d1dd9462 100644 --- a/tests/boundary/providers/native/artifactProvider.extraction.test.ts +++ b/tests/boundary/providers/native/artifactProvider.extraction.test.ts @@ -15,9 +15,9 @@ import type { BinaryTarget } from "../../../../src/domain/binaryTargetTypes.js"; import { withArtifactClient } from "../../../support/artifactClientScope.js"; describe("artifact extraction", () => { - it.skipIf(process.platform === "win32")( - "refuses a regular-file parent before acquiring output and permits a corrected request", - async () => { + it.skipIf(process.platform === "win32").each(["file/..", "missing"])( + "refuses an invalid parent (%s) before acquiring output and permits a corrected request", + async (parent) => { const root = await createTestTempDirectory("rea-extract-invalid-parent-"); const source = join(root, "source"); await mkdir(source); @@ -27,7 +27,7 @@ describe("artifact extraction", () => { await withArtifactClient(target(source, "directory"), async (client) => { const refused = await client.execute("extract_artifact", { - output_root: `${root}/file/../out`, + output_root: `${root}/${parent}/out`, }); expect(refused).toMatchObject({ ok: false,