From bd1b2c74ae29af310f6ef6cef1a8b305811249bb Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:27:44 +0800 Subject: [PATCH] fix(browser): resolve companions from the selected manifest directory Resolve the filesystem-selected manifest parent once before deriving script paths and reuse the shared containment rule. Preserve selected manifest spelling and capture metadata while retaining source integrity, symlink refusal and cleanup. Cover differing and identical lexical siblings through both artifact consumers and public CLI/MCP parity. --- src/browser/assets/LocalWebScriptArtifacts.ts | 17 ++- tests/acceptance/webModuleCli.test.ts | 16 ++- tests/acceptance/webSourceLocationCli.test.ts | 19 ++- .../filesystem/webScriptManifestPaths.test.ts | 115 +++++++++++++++++- 4 files changed, 147 insertions(+), 20 deletions(-) diff --git a/src/browser/assets/LocalWebScriptArtifacts.ts b/src/browser/assets/LocalWebScriptArtifacts.ts index 13391bff87..18a699bd5c 100644 --- a/src/browser/assets/LocalWebScriptArtifacts.ts +++ b/src/browser/assets/LocalWebScriptArtifacts.ts @@ -1,5 +1,5 @@ import { lstat, realpath } from "node:fs/promises"; -import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { dirname, join } from "node:path"; import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { readStableArtifact } from "../../artifacts/readStableArtifact.js"; @@ -7,6 +7,7 @@ import { normalizeArtifactPath } from "../../artifacts/ArtifactPaths.js"; import type { WebScriptArtifactPort } from "../../application/WebScriptArtifactPort.js"; import type { ExecutionOptions } from "../../application/AnalysisProvider.js"; import { AnalysisCapabilityUnavailableError } from "../../domain/analysisErrorCore.js"; +import { isPathWithinRoot } from "../../domain/localPath.js"; import { err, ok } from "../../domain/result.js"; import type { SelectedWebScriptInput } from "../../domain/webScriptArtifacts.js"; import { webScriptExportManifestSchema } from "../../domain/webScriptExport.js"; @@ -89,10 +90,12 @@ export class LocalWebScriptArtifacts implements WebScriptArtifactPort { "path", `Windows source path cannot contain backslashes as literal name characters: ${JSON.stringify(portable)}`, ); - const root = resolve(dirname(input.manifest_path)); + const selectedParent = dirname(input.manifest_path); + callerSelected = false; + targetPath = selectedParent; + const root = await realpath(selectedParent); const sourcePath = join(root, "files", portable); targetPath = sourcePath; - callerSelected = false; await assertContained(root, sourcePath); const sourceBytes = await readStableArtifact( sourcePath, @@ -145,14 +148,8 @@ export class LocalWebScriptArtifacts implements WebScriptArtifactPort { } const assertContained = async (root: string, path: string): Promise => { - const canonicalRoot = await realpath(root); const canonicalFile = await realpath(path); - const fromRoot = relative(canonicalRoot, canonicalFile); - if ( - fromRoot === ".." || - fromRoot.startsWith(`..${sep}`) || - isAbsolute(fromRoot) - ) + if (!isPathWithinRoot(root, canonicalFile)) throw new ArtifactReaderFailure( "path", `Selected source escapes its manifest directory: ${path}`, diff --git a/tests/acceptance/webModuleCli.test.ts b/tests/acceptance/webModuleCli.test.ts index 93789a9110..1cfe3a1cb9 100644 --- a/tests/acceptance/webModuleCli.test.ts +++ b/tests/acceptance/webModuleCli.test.ts @@ -1,5 +1,5 @@ import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; -import { mkdir, writeFile } from "node:fs/promises"; +import { mkdir, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, onTestFinished } from "vitest"; import { createWebModuleTraceService } from "../../src/composition/webModules.js"; @@ -24,8 +24,14 @@ cliTest( join(root, "files", "modules", "main.js"), artifacts.source, ); - const manifestPath = join(root, "manifest.json").replaceAll("\\", "/"); + let manifestPath = join(root, "manifest.json").replaceAll("\\", "/"); await writeFile(manifestPath, JSON.stringify(manifest)); + if (process.platform !== "win32") { + await mkdir(join(root, "lexical")); + await mkdir(join(root, "nested")); + await symlink(join(root, "nested"), join(root, "lexical", "link")); + manifestPath = `${root}/lexical/link/../manifest.json`; + } const environment = { REA_LOG_LEVEL: "silent", REA_BROWSER_EXECUTABLE: "relative-missing-browser", @@ -45,7 +51,11 @@ cliTest( expect(cliResponse.json).toMatchObject({ normalized_result: { engine: null, - source: { script_index: 0 }, + manifest: { path: manifestPath, reported_output_directory: root }, + source: { + script_index: 0, + path: join(root, "files", "modules", "main.js"), + }, imports: [ { resolution: { state: "unknown", reason: "computed-specifier" } }, ], diff --git a/tests/acceptance/webSourceLocationCli.test.ts b/tests/acceptance/webSourceLocationCli.test.ts index 313410c2ba..f8ed61eff1 100644 --- a/tests/acceptance/webSourceLocationCli.test.ts +++ b/tests/acceptance/webSourceLocationCli.test.ts @@ -7,7 +7,7 @@ import { getDefaultEnvironment, } from "@modelcontextprotocol/client/stdio"; import { fileURLToPath } from "node:url"; -import { mkdir, writeFile } from "node:fs/promises"; +import { mkdir, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { webSourceLocationFixture } from "../fixtures/webSourceLocation.js"; import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; @@ -20,9 +20,15 @@ cliTest( const fixture = webSourceLocationFixture(); await mkdir(join(root, "files", "modules"), { recursive: true }); await writeFile(join(root, "files", "modules", "main.js"), fixture.source); - const manifest = join(root, "manifest.json").replaceAll("\\", "/"); + let manifest = join(root, "manifest.json").replaceAll("\\", "/"); const map = join(root, "app.js.map"); await writeFile(manifest, JSON.stringify(fixture.manifest)); + if (process.platform !== "win32") { + await mkdir(join(root, "lexical")); + await mkdir(join(root, "nested")); + await symlink(join(root, "nested"), join(root, "lexical", "link")); + manifest = `${root}/lexical/link/../manifest.json`; + } await writeFile(map, fixture.sourceMap.text); const response = await cli.run({ arguments: [ @@ -45,7 +51,14 @@ cliTest( expect(response.exitCode, response.stdout + response.stderr).toBe(0); expect(response.json).toMatchObject({ normalized_result: { - source: { script_index: 0 }, + manifest: { + path: manifest, + reported_output_directory: fixture.manifest.output_directory, + }, + source: { + script_index: 0, + path: join(root, "files", "modules", "main.js"), + }, source_map: { association: "caller-selected" }, generated_offset: 0, execution: "unknown", diff --git a/tests/boundary/filesystem/webScriptManifestPaths.test.ts b/tests/boundary/filesystem/webScriptManifestPaths.test.ts index 762ecd397e..286e6ae0fe 100644 --- a/tests/boundary/filesystem/webScriptManifestPaths.test.ts +++ b/tests/boundary/filesystem/webScriptManifestPaths.test.ts @@ -1,10 +1,18 @@ -import { mkdir, writeFile } from "node:fs/promises"; -import { isAbsolute, join, relative, sep } from "node:path"; -import { expect, it } from "vitest"; +import { createHash } from "node:crypto"; +import { mkdir, symlink, writeFile } from "node:fs/promises"; +import { dirname, isAbsolute, join, relative, sep } from "node:path"; +import { expect, it, onTestFinished } from "vitest"; +import { exportWebScripts } from "../../../src/application/WebScriptExportService.js"; +import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { LocalWebScriptArtifacts } from "../../../src/browser/assets/LocalWebScriptArtifacts.js"; +import { LocalWebSourceLocationArtifacts } from "../../../src/browser/assets/WebSourceLocationArtifacts.js"; +import { LocalWebModuleArtifacts } from "../../../src/browser/modules/WebModuleArtifacts.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { webScriptExportResultSchema } from "../../../src/domain/webScriptExport.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { webModuleArtifactsFixture } from "../../fixtures/webModuleTrace.js"; +import { scriptScenarioFixture } from "../../fixtures/webScriptCapture.js"; +import { webSourceLocationFixture } from "../../fixtures/webSourceLocation.js"; const materialize = async () => { const root = join( @@ -21,6 +29,105 @@ const materialize = async () => { return { root, artifact, sourcePath, manifestPath }; }; +it.skipIf(process.platform === "win32").each([ + { consumer: "module", sentinel: "different" }, + { consumer: "module", sentinel: "identical" }, + { consumer: "source-location", sentinel: "different" }, + { consumer: "source-location", sentinel: "identical" }, +] as const)( + "loads the POSIX symlink-parent companion through $consumer with $sentinel lexical sibling bytes", + async ({ consumer, sentinel }) => { + const base = await createTestTempDirectory("rea-manifest-physical-parent-"); + const source = Buffer.from('export const selected = "physical export";\n'); + const sourceUrl = "https://app.test/modules/main.js"; + const capturePath = join(base, "capture.json"); + await writeFile( + capturePath, + JSON.stringify( + scriptScenarioFixture([{ url: sourceUrl, bytes: source }]), + ), + ); + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close()); + const exported = await exportWebScripts( + { + capture_path: capturePath, + output_directory: join(base, "physical-export"), + }, + resources, + ); + if (!exported.ok) throw exported.error; + const report = webScriptExportResultSchema.parse( + exported.value.normalized_result, + ); + const selected = report.scripts[0]; + if (selected?.content.state !== "exported") + throw new Error("Expected the real export to retain its selected source"); + + const physicalSourcePath = join( + report.output_directory, + "files", + selected.content.relative_path, + ); + const lexicalSourcePath = join( + base, + "files", + selected.content.relative_path, + ); + await mkdir(dirname(lexicalSourcePath), { recursive: true }); + await writeFile( + lexicalSourcePath, + sentinel === "identical" + ? source + : Buffer.from('export const selected = "lexical sibling";\n'), + ); + const nested = join(report.output_directory, "nested"); + await mkdir(nested); + await symlink(nested, join(base, "link")); + const selectedManifestPath = `${base}/link/../manifest.json`; + const sourceMap = webSourceLocationFixture().sourceMap; + const mapPath = join(base, "selected.map"); + await writeFile(mapPath, sourceMap.text); + + const artifacts = + consumer === "module" + ? new LocalWebModuleArtifacts() + : new LocalWebSourceLocationArtifacts(); + onTestFinished(() => artifacts.close()); + const input = { manifest_path: selectedManifestPath, script_index: 0 }; + const result = + artifacts instanceof LocalWebModuleArtifacts + ? await artifacts.load(input) + : await artifacts.load({ + ...input, + source_map: { path: mapPath, url: sourceMap.url }, + generated_position: { line: 1, column: 0 }, + }); + if (!result.ok) throw result.error; + + expect(result.value.source).toBe(source.toString("utf8")); + expect(result.value.sourceFile).toEqual({ + path: physicalSourcePath, + sha256: createHash("sha256").update(source).digest("hex"), + bytes: source.length, + }); + expect(result.value.sourceFile.path).not.toBe(lexicalSourcePath); + expect(result.value.manifestFile).toEqual({ + ...report.manifest, + path: selectedManifestPath, + }); + expect(result.value.manifest.output_directory).toBe( + report.output_directory, + ); + expect(result.value.manifest.scripts[0]?.url).toBe(sourceUrl); + if (consumer === "source-location") + expect(result.value).toHaveProperty("sourceMap", { + ...sourceMap, + file: { ...sourceMap.file, path: mapPath }, + }); + }, +); + it("reads equivalent local manifest spellings without changing reported identity", async () => { const fixture = await materialize(); const paths = new Set([ @@ -58,7 +165,7 @@ it("reads equivalent local manifest spellings without changing reported identity } }); -it("still rejects changed source bytes after normalizing the selected manifest path", async () => { +it("still rejects changed source bytes after resolving the selected manifest's physical directory", async () => { const fixture = await materialize(); await writeFile(fixture.sourcePath, "tampered source"); const response = await new LocalWebScriptArtifacts(