Merge pull request #2021 from Automattic/fix/2015-readonly-mount-cycle

Fix readonly mount symlink cycles

Author: chubes4

packages/runtime-playground/src/mount-materialization.ts

@@ -104,40 +104,11 @@ export async function stageReadonlyPlaygroundMounts(mounts: MountSpec[]): Promis
       const source = join(root, `${index}-${basename(mount.source) || "mount"}`)
       const sourceRoot = await realpath(mount.source)
       const diagnostics: MaterializationDiagnostic[] = []
-      await cp(mount.source, source, {
-        recursive: mount.type !== "file",
-        dereference: true,
-        async filter(entry) {
-          if (!(await lstat(entry)).isSymbolicLink()) {
-            return true
-          }
-          let reason: "dangling-target" | "source-escape"
-          try {
-            const target = await realpath(entry)
-            const relativeTarget = relative(sourceRoot, target)
-            if (relativeTarget !== ".." && !relativeTarget.startsWith(`..${sep}`) && !isAbsolute(relativeTarget)) {
-              return true
-            }
-            reason = "source-escape"
-          } catch {
-            reason = "dangling-target"
-          }
-          const path = relative(mount.source, entry).replace(/\\/g, "/") || "."
-          diagnostics.push({
-            code: "readonly-mount-symlink-skipped",
-            message: `Skipped ${reason} symlink in readonly mount: ${path}`,
-            severity: "warning",
-            phase: "playground-readonly-mount-staging",
-            metadata: {
-              mountIndex: index,
-              mountTarget: mount.target,
-              path,
-              reason,
-            },
-          })
-          return false
-        },
-      })
+      if (mount.type === "file") {
+        await cp(mount.source, source, { dereference: true })
+      } else {
+        await stageReadonlyDirectory(mount, index, sourceRoot, source, diagnostics)
+      }
       diagnostics.sort((left, right) => String(left.metadata?.path) < String(right.metadata?.path) ? -1 : String(left.metadata?.path) > String(right.metadata?.path) ? 1 : 0)
       return { mount: { ...mount, source }, diagnostics }
     }))
@@ -166,6 +137,74 @@ export async function stageReadonlyPlaygroundMounts(mounts: MountSpec[]): Promis
   }
 }
 
+async function stageReadonlyDirectory(mount: MountSpec, mountIndex: number, sourceRoot: string, destination: string, diagnostics: MaterializationDiagnostic[]): Promise<void> {
+  const visit = async (directory: string, stagedDirectory: string, relativeDirectory: string, ancestors: ReadonlySet<string>): Promise<void> => {
+    await mkdir(stagedDirectory, { recursive: true })
+    const entries = await readdir(directory, { withFileTypes: true })
+    entries.sort((left, right) => left.name.localeCompare(right.name))
+
+    for (const entry of entries) {
+      const path = join(directory, entry.name)
+      const stagedPath = join(stagedDirectory, entry.name)
+      const relativePath = relativeDirectory ? `${relativeDirectory}/${entry.name}` : entry.name
+      const entryStat = await lstat(path)
+      if (!entryStat.isSymbolicLink()) {
+        if (entryStat.isDirectory()) {
+          const target = await realpath(path)
+          await visit(target, stagedPath, relativePath, new Set([...ancestors, target]))
+        } else {
+          await cp(path, stagedPath)
+        }
+        continue
+      }
+
+      let target: string
+      try {
+        target = await realpath(path)
+      } catch {
+        addReadonlySymlinkDiagnostic(diagnostics, mount, mountIndex, relativePath, "dangling-target")
+        continue
+      }
+      if (!pathIsWithinRoot(sourceRoot, target)) {
+        addReadonlySymlinkDiagnostic(diagnostics, mount, mountIndex, relativePath, "source-escape")
+        continue
+      }
+      const targetStat = await stat(target)
+      if (!targetStat.isDirectory()) {
+        await cp(target, stagedPath)
+        continue
+      }
+      if (ancestors.has(target)) {
+        addReadonlySymlinkDiagnostic(diagnostics, mount, mountIndex, relativePath, "directory-cycle")
+        continue
+      }
+      await visit(target, stagedPath, relativePath, new Set([...ancestors, target]))
+    }
+  }
+
+  await visit(sourceRoot, destination, "", new Set([sourceRoot]))
+}
+
+function pathIsWithinRoot(root: string, path: string): boolean {
+  const relativePath = relative(root, path)
+  return relativePath !== ".." && !relativePath.startsWith(`..${sep}`) && !isAbsolute(relativePath)
+}
+
+function addReadonlySymlinkDiagnostic(diagnostics: MaterializationDiagnostic[], mount: MountSpec, mountIndex: number, path: string, reason: "dangling-target" | "source-escape" | "directory-cycle"): void {
+  diagnostics.push({
+    code: "readonly-mount-symlink-skipped",
+    message: `Skipped ${reason} symlink in readonly mount: ${path}`,
+    severity: "warning",
+    phase: "playground-readonly-mount-staging",
+    metadata: {
+      mountIndex,
+      mountTarget: mount.target,
+      path,
+      reason,
+    },
+  })
+}
+
 function readonlyMountStagingPhaseResult(mounts: number, diagnostics: MaterializationDiagnostic[]): MaterializationPhaseResult {
   return materializationPhaseResult({
     phase: "playground-readonly-mount-staging",

tests/playground-readonly-mounts.test.ts

@@ -78,6 +78,12 @@ try {
   await symlink("../host-secret.php", join(pluginSource, "host-secret.php"))
   await symlink("host-secret.php", join(pluginSource, "host-secret-chain.php"))
   await symlink("../tracked-symlink-plugin-private/secret.php", join(pluginSource, "prefix-secret.php"))
+  await mkdir(join(pluginSource, "package-a", "vendor"), { recursive: true })
+  await mkdir(join(pluginSource, "package-b", "vendor"), { recursive: true })
+  await writeFile(join(pluginSource, "package-a", "package.php"), "<?php return 'package-a';\n")
+  await writeFile(join(pluginSource, "package-b", "package.php"), "<?php return 'package-b';\n")
+  await symlink("../../package-b", join(pluginSource, "package-a", "vendor", "package-b"))
+  await symlink("../../package-a", join(pluginSource, "package-b", "vendor", "package-a"))
   await execFileAsync("git", ["init", "--quiet"], { cwd: pluginSource })
   await execFileAsync("git", ["add", "."], { cwd: pluginSource })
   const trackedSymlinks = (await execFileAsync("git", ["ls-files", "-s", "--", "*.php", "*.sh"], { cwd: pluginSource })).stdout.trim().split("\n").filter((entry) => entry.startsWith("120000 ")).map((entry) => entry.split("\t")[1]).sort()
@@ -92,6 +98,9 @@ try {
   assert.equal(await readFile(join(stagedPlugin, "includes", "runtime.php"), "utf8"), "<?php return 'runtime';\n", "nested runtime files remain available")
   assert.equal(await readFile(join(stagedPlugin, "runtime-link.php"), "utf8"), "<?php return 'runtime';\n", "a contained symlink is dereferenced into the staged mount")
   assert.equal(await readFile(join(stagedPlugin, "runtime-chain.php"), "utf8"), "<?php return 'runtime';\n", "a contained symlink chain is dereferenced into the staged mount")
+  assert.equal(await readFile(join(stagedPlugin, "package-a", "vendor", "package-b", "package.php"), "utf8"), "<?php return 'package-b';\n", "a contained directory symlink is dereferenced into the staged mount")
+  await assert.rejects(access(join(stagedPlugin, "package-a", "vendor", "package-b", "vendor", "package-a")), /ENOENT/, "a directory cycle stops at the repeated real directory")
+  await assert.rejects(access(join(stagedPlugin, "package-b", "vendor", "package-a", "vendor", "package-b")), /ENOENT/, "both cycle directions remain bounded")
   await assert.rejects(access(join(stagedPlugin, "build.sh")), /ENOENT/, "a tracked dangling symlink is not materialized")
   await assert.rejects(access(join(stagedPlugin, "build-chain.sh")), /ENOENT/, "a chained dangling symlink is not materialized")
   await assert.rejects(access(join(stagedPlugin, "host-secret.php")), /ENOENT/, "a symlink escaping the source cannot expose a host file")
@@ -102,13 +111,15 @@ try {
     { code: "readonly-mount-symlink-skipped", metadata: { mountIndex: 0, mountTarget: "/wordpress/wp-content/plugins/tracked-symlink-plugin", path: "build.sh", reason: "dangling-target" } },
     { code: "readonly-mount-symlink-skipped", metadata: { mountIndex: 0, mountTarget: "/wordpress/wp-content/plugins/tracked-symlink-plugin", path: "host-secret-chain.php", reason: "source-escape" } },
     { code: "readonly-mount-symlink-skipped", metadata: { mountIndex: 0, mountTarget: "/wordpress/wp-content/plugins/tracked-symlink-plugin", path: "host-secret.php", reason: "source-escape" } },
+    { code: "readonly-mount-symlink-skipped", metadata: { mountIndex: 0, mountTarget: "/wordpress/wp-content/plugins/tracked-symlink-plugin", path: "package-a/vendor/package-b/vendor/package-a", reason: "directory-cycle" } },
+    { code: "readonly-mount-symlink-skipped", metadata: { mountIndex: 0, mountTarget: "/wordpress/wp-content/plugins/tracked-symlink-plugin", path: "package-b/vendor/package-a/vendor/package-b", reason: "directory-cycle" } },
     { code: "readonly-mount-symlink-skipped", metadata: { mountIndex: 0, mountTarget: "/wordpress/wp-content/plugins/tracked-symlink-plugin", path: "prefix-secret.php", reason: "source-escape" } },
   ], "skipped symlinks produce deterministic structured diagnostics")
   const serializedDiagnostics = JSON.stringify(staging.diagnostics)
   assert.equal(serializedDiagnostics.includes(root), false, "diagnostics do not expose absolute host paths")
   assert.equal(serializedDiagnostics.includes("../../../.github/build.sh"), false, "diagnostics do not expose dangling link targets")
   assert.equal(serializedDiagnostics.includes("../tracked-symlink-plugin-private/secret.php"), false, "diagnostics do not expose escaping link targets")
-  assert.deepEqual(staging.phaseResult.metadata, { mounts: 1, skipped: 5, diagnostics: staging.diagnostics }, "staging evidence includes the skip diagnostics")
+  assert.deepEqual(staging.phaseResult.metadata, { mounts: 1, skipped: 7, diagnostics: staging.diagnostics }, "staging evidence includes the skip diagnostics")
   await staging[Symbol.asyncDispose]()
   assert.deepEqual(await readonlyStagingDirectories(), stagingDirectoriesBefore, "successful staging cleanup removes its temporary root")
 
@@ -126,7 +137,7 @@ try {
   const mountMaterialization = startupProgress.find((event) => event.phase === "preview:materializing-mounts")?.detail?.materialization
   assert.deepEqual((mountMaterialization as { metadata?: Record<string, unknown> })?.metadata, {
     mounts: 3,
-    skipped: 5,
+    skipped: 7,
     diagnostics: staging.diagnostics.map((diagnostic) => ({
       ...diagnostic,
       metadata: { ...diagnostic.metadata, mountIndex: 3 },