🤖 fix: pinned files outside the checkout store real folder paths and read at the filesystem root

src/node/services/artifactRuntimeStore.test.ts

@@ -11,6 +11,7 @@ import {
   parseArtifactListOutput,
   readArtifactBytesOnRuntime,
   readArtifactOnRuntime,
+  resolveRealDirOnRuntime,
   writeArtifactOnRuntime,
 } from "./artifactRuntimeStore";
 import {
@@ -269,6 +270,31 @@ describe("artifactRuntimeStore", () => {
       });
     });
 
+    test("a pinned read can use the filesystem root as its folder", async () => {
+      // A pinned file directly under `/` is read from `/`: containment must not require `//`.
+      const checkout = path.join(tempDir, "checkout");
+      await fs.mkdir(checkout);
+      await fs.writeFile(path.join(checkout, "a.txt"), "rooted");
+      const options = { allowHidden: true, requireArtifactsBasename: false };
+      const relPath = path.join(checkout, "a.txt").slice(1);
+      expect(
+        await readArtifactBytesOnRuntime(runtime, "/", relPath, CAP, undefined, options)
+      ).toMatchObject({ status: "ok", bytes: Buffer.from("rooted") });
+      expect(await readArtifactFromDir("/", relPath, CAP, options)).toMatchObject({
+        success: true,
+        data: { status: "ok", content: "rooted" },
+      });
+    });
+
+    test("resolves a folder's real path on the runtime", async () => {
+      const real = path.join(tempDir, "real dir");
+      await fs.mkdir(real);
+      await fs.symlink(real, path.join(tempDir, "link"));
+      expect(await resolveRealDirOnRuntime(runtime, path.join(tempDir, "link"))).toBe(real);
+      expect(await resolveRealDirOnRuntime(runtime, real)).toBe(real);
+      expect(await resolveRealDirOnRuntime(runtime, path.join(tempDir, "missing"))).toBeNull();
+    });
+
     test("reads a pinned checkout file only when the artifacts name check is off", async () => {
       const checkout = path.join(tempDir, "checkout");
       await fs.mkdir(path.join(checkout, ".config"), { recursive: true });

src/node/services/artifactRuntimeStore.ts

@@ -31,6 +31,7 @@
  */
 
 const LIST_MAGIC = "XUMARTIFACTS1";
+const REAL_DIR_MAGIC = "XUMREALDIR1";
 const READ_MAGIC = "XUMREAD1";
 const WRITE_MAGIC = "XUMWRITE1";
 const EXEC_TIMEOUT_SECONDS = 30;
@@ -152,6 +153,38 @@
   }
 }
 
+/**
+ * Real path of a folder on the runtime (symlinks resolved), or null when it is missing or not a
+ * folder. Pins outside the checkout store it: the read script refuses a symlinked root folder.
+ */
+export async function resolveRealDirOnRuntime(
+  runtime: Runtime,
+  dir: string,
+  abortSignal?: AbortSignal
+): Promise<string | null> {
+  const result = await execBuffered(
+    runtime,
+    `cd -P -- "$XUM_DIR" 2>/dev/null || exit 0\nprintf '${REAL_DIR_MAGIC}\\0'\npwd -P\n`,
+    {
+      cwd: "/",
+      pathEnv: { XUM_DIR: dir },
+      timeout: EXEC_TIMEOUT_SECONDS,
+      abortSignal,
+      maxOutputBytes: READ_OUTPUT_SLACK_BYTES,
+    }
+  );
+  if (result.exitCode !== 0) {
+    throw new Error(
+      `Resolving a folder failed (exit ${result.exitCode}): ${result.stderr.trim().slice(0, 500)}`
+    );
+  }
+  // A login banner can come first; pwd ends its line with one newline.
+  const start = result.stdout.indexOf(`${REAL_DIR_MAGIC}\0`);
+  if (start < 0) return null;
+  const real = result.stdout.slice(start + REAL_DIR_MAGIC.length + 1).replace(/\n$/, "");
+  return real.startsWith("/") ? real : null;
+}
+
 export async function listArtifactsOnRuntime(
   runtime: Runtime,
   artifactsDir: string,
@@ -186,8 +219,8 @@
   maxBytes: number,
   options?: Pick<ArtifactReadOptions, "requireArtifactsBasename">
 ): string {
   assert(segments.length > 0, "segments must not be empty");
   assert(Number.isInteger(maxBytes) && maxBytes > 0, "maxBytes must be a positive integer");
   const dirSegments = segments.slice(0, -1).map((segment) => shescape.quote(segment));
   const leaf = shescape.quote(segments[segments.length - 1]);
   // The root's real path must be its real parent plus its own name (or `artifacts`).
@@ -198,28 +231,36 @@
 if [ -L "$d" ] || [ ! -d "$d" ]; then xum_missing; fi
 cd -- "$d" 2>/dev/null || xum_missing
 xum_root=$(pwd -P) || xum_missing
-xum_parent=$(cd -P -- "$` +
+xum_up=$` +
     "{d%/*}" +
+    String.raw`
+xum_parent=$(cd -P -- "$` +
+    "{xum_up:-/}" +
     String.raw`" 2>/dev/null && pwd -P) || xum_missing
-if [ "$xum_root" != "$xum_parent/${rootName}" ]; then xum_missing; fi
+if [ "$xum_root" != "$` +
+    "{xum_parent%/}" +
+    String.raw`/${rootName}" ]; then xum_missing; fi
+xum_pre=$` +
+    "{xum_root%/}" +
+    String.raw`/
 for xum_seg in ${dirSegments.join(" ")}; do
   if [ -L "$xum_seg" ] || [ ! -d "$xum_seg" ]; then xum_missing; fi
   cd -- "$xum_seg" 2>/dev/null || xum_missing
 done
-case $(pwd -P) in "$xum_root"/*|"$xum_root") ;; *) xum_missing ;; esac
+case $(pwd -P) in "$xum_pre"*|"$xum_root") ;; *) xum_missing ;; esac
 xum_leaf=${leaf}
 if [ -L "$xum_leaf" ] || [ ! -f "$xum_leaf" ]; then xum_missing; fi
 ${STAT_PRELUDE}
 xum_send() {
   if xum_fd=$(readlink "/proc/$$/fd/3" 2>/dev/null); then
-    case $xum_fd in "$xum_root"/*) ;; *) xum_missing ;; esac
+    case $xum_fd in "$xum_pre"*) ;; *) xum_missing ;; esac
   fi
   if [ -L "$xum_leaf" ]; then xum_missing; fi
   xum_m=$(xum_meta "./$xum_leaf") || xum_missing
   set -- $xum_m
   if [ "$1" -gt ${maxBytes} ]; then printf '${READ_MAGIC}\0too_large\0%s\0%s\0' "$1" "$2"; exit 0; fi
   printf '${READ_MAGIC}\0ok\0%s\0%s\0' "$1" "$2"
   head -c ${maxBytes + 1} <&3
   exit 0
 }
 xum_send 3< "./$xum_leaf" || xum_missing

src/node/services/artifactStore.ts

@@ -78,6 +78,11 @@
   return path.join(scratchDir, ARTIFACTS_DIR_NAME);
 }
 
+/** `dir` plus one separator: a pinned file's folder can be the filesystem root (`/`). */
+function dirPrefix(dir: string): string {
+  return dir.endsWith(path.sep) ? dir : dir + path.sep;
+}
+
 function isMissing(error: unknown): boolean {
   const code = (error as NodeJS.ErrnoException | undefined)?.code;
   return code === "ENOENT" || code === "ENOTDIR";
@@ -343,7 +348,7 @@
   maxBytes: number,
   options?: ArtifactReadOptions
 ): Promise<ArtifactBytesOutcome> {
   assert(Number.isInteger(maxBytes) && maxBytes > 0, "maxBytes must be a positive integer");
   const segments = parseArtifactRelativePath(relPath, options);
   if (typeof segments === "string") return { status: "invalid", error: segments };
 
@@ -365,7 +370,7 @@
     throw error;
   }
   // Symlinked parent folders could still point elsewhere: confine by real path.
-  if (!realTarget.startsWith(realDir + path.sep)) return notFound;
+  if (!realTarget.startsWith(dirPrefix(realDir))) return notFound;
 
   let handle: fs.FileHandle;
   try {
@@ -403,8 +408,8 @@
     // Read at most maxBytes + 1 so a file that grew after fstat is still caught.
     const chunks: Buffer[] = [];
     let total = 0;
     while (total <= maxBytes) {
       const chunk = Buffer.alloc(Math.min(READ_CHUNK_BYTES, maxBytes + 1 - total));
       const { bytesRead } = await handle.read(chunk, 0, chunk.length, total);
       if (bytesRead === 0) break;
       chunks.push(chunk.subarray(0, bytesRead));
@@ -610,7 +615,7 @@
     fdPath = undefined;
   }
   if (fdPath !== undefined) {
-    return fdPath === realTarget && fdPath.startsWith(realDir + path.sep);
+    return fdPath === realTarget && fdPath.startsWith(dirPrefix(realDir));
   }
   if (requireDescriptorPath) return false;
   try {

src/node/services/pinnedArtifactFiles.test.ts

@@ -2,6 +2,8 @@ import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:
 import * as fs from "fs/promises";
 import * as os from "os";
 import * as path from "path";
+import { LocalRuntime } from "@/node/runtime/LocalRuntime";
+import * as runtimeHelpers from "@/node/runtime/runtimeHelpers";
 import { acquireCrossProcessLock } from "@/node/utils/main/crossProcessLock";
 import * as artifactStoreModule from "./artifactStore";
 import type { ArtifactsContext } from "./artifactsOperations";
@@ -24,6 +26,8 @@ describe("pinned workspace files", () => {
   let projects: Array<{ projectPath: string; projectName: string }> | undefined;
   let subProjectPath: string | undefined;
   let runtimeConfig: Record<string, unknown>;
+  let workspaceName: string;
+  let projectPath: string;
 
   beforeEach(async () => {
     tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "pinned-files-"));
@@ -34,6 +38,8 @@ describe("pinned workspace files", () => {
     projects = undefined;
     subProjectPath = undefined;
     runtimeConfig = { type: "worktree", srcBaseDir: tempDir };
+    workspaceName = "checkout";
+    projectPath = tempDir;
     context = {
       config: { sessionsDir: path.join(tempDir, "sessions") },
       workspaceService: {
@@ -42,8 +48,8 @@ describe("pinned workspace files", () => {
             workspaceId === "ws"
               ? {
                   id: "ws",
-                  name: "checkout",
-                  projectPath: tempDir,
+                  name: workspaceName,
+                  projectPath,
                   projectName: "project",
                   namedWorkspacePath: checkout,
                   runtimeConfig,
@@ -100,7 +106,7 @@ describe("pinned workspace files", () => {
   test("a file outside the checkout pins by its absolute path and reads live", async () => {
     // File tool cards offer "Open as artifact" for any path the agent read, such as a plan or a
     // skill file under the home folder.
-    const outside = path.join(tempDir, "notes", "plan.md");
+    const outside = path.join(await fs.realpath(tempDir), "notes", "plan.md");
     await fs.mkdir(path.dirname(outside), { recursive: true });
     await fs.writeFile(outside, "# Plan");
     expect(await pinFile(context, { workspaceId: "ws", path: outside })).toEqual({
@@ -135,6 +141,94 @@ describe("pinned workspace files", () => {
     });
   });
 
+  test("one file reached through a symlinked folder is one pin", async () => {
+    const real = await fs.realpath(tempDir);
+    await fs.mkdir(path.join(real, "notes"));
+    await fs.writeFile(path.join(real, "notes", "plan.md"), "# Plan");
+    await fs.symlink(path.join(real, "notes"), path.join(real, "notes-link"));
+    await fs.mkdir(path.join(checkout, "docs"));
+    await fs.writeFile(path.join(checkout, "docs", "x.md"), "x");
+    await fs.symlink(path.join(checkout, "docs"), path.join(real, "docs-link"));
+
+    // An alias of an outside folder stores the real path, so both spellings are one pin.
+    for (const alias of ["notes", "notes-link"]) {
+      expect(
+        await pinFile(context, { workspaceId: "ws", path: path.join(real, alias, "plan.md") })
+      ).toEqual({ success: true, data: { path: path.join(real, "notes", "plan.md") } });
+    }
+    // An alias that leads into the checkout is a checkout pin.
+    expect(
+      await pinFile(context, { workspaceId: "ws", path: path.join(real, "docs-link", "x.md") })
+    ).toEqual({ success: true, data: { path: "docs/x.md" } });
+    const listed = await listPinnedFiles(context, { workspaceId: "ws" });
+    expect(listed.success && listed.data.available && listed.data.files.map((f) => f.path)).toEqual(
+      [path.join(real, "notes", "plan.md"), "docs/x.md"]
+    );
+  });
+
+  test("an SSH checkout pins and reads an outside file in a symlinked folder", async () => {
+    // A LocalRuntime stands in for the SSH host: pins and reads run the real runtime scripts.
+    const real = await fs.realpath(tempDir);
+    runtimeConfig = { type: "ssh", host: "example", srcBaseDir: real };
+    // An in-place workspace (name equals project path) roots the checkout at the project path.
+    projectPath = workspaceName = path.join(real, "checkout");
+    const create = spyOn(runtimeHelpers, "createRuntimeForWorkspace").mockReturnValue(
+      new LocalRuntime(real)
+    );
+    try {
+      await fs.mkdir(path.join(real, "notes"));
+      await fs.writeFile(path.join(real, "notes", "plan.md"), "# Plan");
+      await fs.symlink(path.join(real, "notes"), path.join(real, "notes-link"));
+      const stored = path.join(real, "notes", "plan.md");
+      expect(
+        await pinFile(context, {
+          workspaceId: "ws",
+          path: path.join(real, "notes-link", "plan.md"),
+        })
+      ).toEqual({ success: true, data: { path: stored } });
+      expect(await readPinnedFile(context, { workspaceId: "ws", path: stored })).toMatchObject({
+        success: true,
+        data: { status: "ok", content: "# Plan" },
+      });
+      expect(create).toHaveBeenCalled();
+    } finally {
+      create.mockRestore();
+    }
+  });
+
+  test("re-pinning an alias stored before real folder paths keeps the one entry", async () => {
+    const real = await fs.realpath(tempDir);
+    await fs.mkdir(path.join(real, "notes"));
+    await fs.writeFile(path.join(real, "notes", "plan.md"), "# Plan");
+    await fs.symlink(path.join(real, "notes"), path.join(real, "notes-link"));
+    const alias = path.join(real, "notes-link", "plan.md");
+    // An older build stored the alias as given.
+    await addPinnedPath(path.join(tempDir, "sessions", "ws"), alias);
+    for (const request of [alias, path.join(real, "notes", "plan.md")]) {
+      expect(await pinFile(context, { workspaceId: "ws", path: request })).toEqual({
+        success: true,
+        data: { path: alias },
+      });
+    }
+    const listed = await listPinnedFiles(context, { workspaceId: "ws" });
+    expect(listed.success && listed.data.available && listed.data.files.map((f) => f.path)).toEqual(
+      [alias]
+    );
+  });
+
+  test("a drive-letter entry on a POSIX host is invalid, not a path below the backend's folder", async () => {
+    if (process.platform === "win32") return;
+    // Only a hand edit of the store can add one. Read as a folder `C:` it would resolve against
+    // the backend's working folder.
+    const sessionDir = path.join(tempDir, "sessions", "ws");
+    await addPinnedPath(sessionDir, "C:/x.md");
+    const read = await readPinnedFile(context, { workspaceId: "ws", path: "C:/x.md" });
+    expect(read.success).toBe(false);
+    expect(read).not.toHaveProperty("missing");
+    const listed = await listPinnedFiles(context, { workspaceId: "ws" });
+    expect(listed.success && listed.data.available && listed.data.files).toEqual([]);
+  });
+
   test("refuses outside files that are missing, folders or symlinks", async () => {
     const notes = path.join(tempDir, "notes");
     await fs.mkdir(notes, { recursive: true });