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(