fix: a mount with two names was still outside the containment check

.github/workflows/ci.yml

@@ -29,7 +29,7 @@ jobs:
   # The job names share the `test on …` stem so that one search finds all three: the required status
   # checks in a ruleset are picked by name from a flat list, and names without a common stem mean one
   # search per check. Keep the stem when adding a job.
-  test:
+  linux:
     timeout-minutes: 15
     name: test on Linux
     runs-on: ubuntu-latest
@@ -60,7 +60,7 @@ jobs:
       # typecheck + build + unit + the end-to-end push against a local git-http-backend.
       - run: corepack yarn check
 
-  native-windows:
+  windows:
     timeout-minutes: 15
     name: test on Windows (refused)
     runs-on: windows-latest

src/commands/stage.ts

@@ -21,7 +21,7 @@ import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
 
-import { fail, insideMountedPath, parseArgs, say, warn, writeSecretFile } from './support.ts';
+import { canonicalPath, fail, insideMountedPath, parseArgs, say, warn, writeSecretFile } from './support.ts';
 
 /** The record `stage` leaves behind, so the running code is identifiable afterwards. */
 export const STAGED_RECORD = 'STAGED.json';
@@ -69,7 +69,15 @@ export function performStage(input: StageInput): StageResult {
         'private key in reach, so code there is code the agent can rewrite — stage outside every mount.',
     );
   }
-  if (to === repo || to.startsWith(repo + path.sep) || repo.startsWith(to + path.sep)) {
+  // Canonical, for the same reason the mount check is: a `--to` that is a symlink into the repository
+  // *is* the repository, and comparing the spellings would miss it.
+  const realRepo = canonicalPath(repo);
+  const realTo = canonicalPath(to);
+  if (
+    realTo === realRepo ||
+    realTo.startsWith(realRepo + path.sep) ||
+    realRepo.startsWith(realTo + path.sep)
+  ) {
     throw new Error(`refusing to stage ${repo} into ${to}: the repository and the target overlap`);
   }
 

src/commands/support.ts

@@ -111,6 +111,43 @@ export function isInsideContainer(): boolean {
   return fs.existsSync('/home/app/Workspaces') && !fs.existsSync('/volume1');
 }
 
+/**
+ * A path with the symlinks in its existing part resolved.
+ *
+ * `realpathSync` throws for a path that does not exist yet, and `stage` is asked to write to one, so the
+ * nearest existing ancestor is resolved and the rest is appended. Comparing the raw spellings is what let
+ * `/var/services/homes/u/Workspaces` and `/volume1/homes/u/Workspaces` — one directory with two names —
+ * look like different places, which is exactly the bypass the mount and overlap checks exist to close.
+ *
+ * @param target - the path to canonicalize.
+ * @returns the real path, or the resolved path when nothing on it exists.
+ * @throws {Error} when the path cannot be resolved for a reason other than being absent, because a
+ *   lexical answer there could call a path inside a mount outside it.
+ */
+export function canonicalPath(target: string): string {
+  const absolute = path.resolve(target);
+  let existing = absolute;
+  const missing: string[] = [];
+  for (;;) {
+    try {
+      const real = fs.realpathSync(existing);
+      return missing.length === 0 ? real : path.join(real, ...missing.toReversed());
+    } catch (error) {
+      // Only a path that is not there is walked up. Anything else — EACCES, ELOOP — means the real path
+      // cannot be known, and a lexical fallback could miss a symlinked component and pass a path the
+      // check exists to refuse, so it propagates and the caller fails closed.
+      const code = (error as NodeJS.ErrnoException).code;
+      if (code !== 'ENOENT' && code !== 'ENOTDIR') throw error;
+      const parent = path.dirname(existing);
+      // Unreachable while the root exists and realpaths; kept so a filesystem that refuses even `/`
+      // cannot spin here forever.
+      if (parent === existing) return absolute;
+      missing.push(path.basename(existing));
+      existing = parent;
+    }
+  }
+}
+
 /**
  * The container's mounts under the user's home, as seen from the host.
  *
@@ -126,14 +163,19 @@ export function mountedPaths(home: string = os.homedir()): string[] {
 /**
  * Whether a path is inside one of the container's mounts.
  *
+ * Both sides are canonicalized before they are compared: `$HOME` on the host may be one name for a
+ * directory the container sees under another (`/var/services/homes/u` and `/volume1/homes/u`), and a
+ * lexical prefix check calls the same directory two different places.
+ *
  * @param target - the path to check.
  * @param home - the home directory to resolve against.
  * @returns the mount it is inside, or null.
  */
 export function insideMountedPath(target: string, home: string = os.homedir()): string | null {
-  const resolved = path.resolve(target);
+  const resolved = canonicalPath(target);
   for (const mounted of mountedPaths(home)) {
-    if (resolved === mounted || resolved.startsWith(`${mounted}${path.sep}`)) return mounted;
+    const real = canonicalPath(mounted);
+    if (resolved === real || resolved.startsWith(`${real}${path.sep}`)) return mounted;
   }
   return null;
 }

test/unit/commands.test.ts

@@ -105,6 +105,54 @@ describe('mounted path detection', () => {
     assert.equal(insideMountedPath('/volume1/docker/git-cred-broker', home), null);
     assert.equal(insideMountedPath('/home/app/Workspaces-evil', home), null, 'must not prefix-match a sibling');
   });
+
+  it('resolves the aliases of one directory instead of trusting the spelling', () => {
+    // Measured on the host this was found on: `$HOME` is /var/services/homes/u while the container sees the
+    // same directory as /volume1/homes/u, so a lexical prefix check called a path inside the mount outside
+    // it — and `stage` wrote the broker's code into a directory the agent can rewrite.
+    const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-mount-alias-'));
+    const real = path.join(root, 'real');
+    fs.mkdirSync(path.join(real, 'Workspaces'), { recursive: true });
+    const alias = path.join(root, 'alias');
+    // 'junction' so the case also runs on Windows, where a plain symlink needs a privilege; the type is
+    // ignored on the platforms that have no junctions.
+    fs.symlinkSync(real, alias, 'junction');
+    try {
+      const mounted = path.join(alias, 'Workspaces');
+      assert.equal(insideMountedPath(path.join(real, 'Workspaces', 'x'), alias), mounted, 'real target, aliased home');
+      assert.equal(insideMountedPath(path.join(real, 'Workspaces'), alias), mounted, 'and the mount itself');
+      assert.equal(insideMountedPath(path.join(real, 'elsewhere'), alias), null, 'a sibling is still outside');
+      assert.equal(
+        insideMountedPath(path.join(alias, 'Workspaces', 'y'), real),
+        path.join(real, 'Workspaces'),
+        'and the reverse spelling',
+      );
+    } finally {
+      fs.rmSync(root, { recursive: true, force: true });
+    }
+  });
+
+  it('canonicalizes a mount that does not exist yet, which is the path stage is asked to create', () => {
+    // `stage --to` is normally a directory that is not there yet, so the mount's own path may not exist
+    // either. The nearest existing ancestor has to be resolved on both sides or the spellings differ.
+    const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gcb-mount-missing-'));
+    const real = path.join(root, 'real');
+    fs.mkdirSync(real, { recursive: true });
+    const alias = path.join(root, 'alias');
+    fs.symlinkSync(real, alias, 'junction');
+    try {
+      const mounted = path.join(alias, 'Workspaces');
+      assert.equal(insideMountedPath(path.join(real, 'Workspaces', 'new', 'deep'), alias), mounted);
+      assert.equal(insideMountedPath(path.join(real, 'Workspaces'), alias), mounted);
+      assert.equal(
+        insideMountedPath(path.join(real, 'Workspaces-evil'), alias),
+        null,
+        'a sibling is not inside, with the mount missing too',
+      );
+    } finally {
+      fs.rmSync(root, { recursive: true, force: true });
+    }
+  });
 });
 
 describe('setup', () => {

test/unit/stage.test.ts

@@ -91,6 +91,23 @@ describe('stage', () => {
     );
   });
 
+  it('refuses a mount whose path is spelled through an alias', () => {
+    // The host spells $HOME /var/services/homes/u; the container sees the same directory as
+    // /volume1/homes/u. A lexical check let a target under the second name through, which staged the
+    // broker's code into a directory the agent can rewrite — the one thing this command exists to stop.
+    const repo = makeRepo('alias');
+    const real = path.join(scratch, 'alias-real');
+    fs.mkdirSync(path.join(real, 'Workspaces'), { recursive: true });
+    const alias = path.join(scratch, 'alias-link');
+    fs.symlinkSync(real, alias, 'junction');
+
+    assert.throws(
+      () => performStage({ repo, to: path.join(real, 'Workspaces', 'app'), ref: 'HEAD', home: alias }),
+      /mounted into the container/,
+    );
+    assert.equal(fs.existsSync(path.join(real, 'Workspaces', 'app')), false, 'and nothing was written');
+  });
+
   it('refuses to clear the deployment directory that holds the key', () => {
     const repo = makeRepo('deployment');
     const to = path.join(scratch, 'deployment-dir');
@@ -120,4 +137,24 @@ describe('stage', () => {
       /overlap/,
     );
   });
+
+  it('refuses a target that is a symlink into the repository', () => {
+    // The overlap check compares canonical paths for the same reason the mount check does: a `--to` that
+    // is a symlink to the repository *is* the repository, and the spellings differ.
+    const repo = makeRepo('overlap-link');
+
+    const link = path.join(scratch, 'link-to-repo');
+    fs.symlinkSync(repo, link, 'junction');
+    assert.throws(
+      () => performStage({ repo, to: link, ref: 'HEAD', home: scratch }),
+      /repository and the target overlap/,
+    );
+
+    const sub = path.join(scratch, 'link-into-repo');
+    fs.symlinkSync(path.join(repo, 'src'), sub, 'junction');
+    assert.throws(
+      () => performStage({ repo, to: sub, ref: 'HEAD', home: scratch }),
+      /repository and the target overlap/,
+    );
+  });
 });