Merge remote-tracking branch 'origin/main' into codex/lane-g1-planning-screen

Author: mchwang

git/clone.ts

@@ -1,7 +1,7 @@
 import { execFileSync } from 'node:child_process';
 import { randomUUID } from 'node:crypto';
 import { lstatSync, mkdtempSync, opendirSync, realpathSync, rmSync } from 'node:fs';
-import { isAbsolute, join, relative, resolve } from 'node:path';
+import { isAbsolute, join, relative, resolve, sep } from 'node:path';
 import type { TaskClone } from '../agents/contract.ts';
 import { runInProcessGroup, type ProcessGroup } from '../agents/process-group.ts';
 import { runTrackedProcess, type ProcessGroupLifecycle } from '../agents/tracked-docker.ts';
@@ -46,6 +46,19 @@ export interface CloneOptions {
   readonly timeoutMs?: number;
 }
 
+interface PathSemantics {
+  readonly sep: string;
+  isAbsolute(path: string): boolean;
+  relative(from: string, to: string): string;
+}
+
+/** Whether `candidate` is `base` itself or one of its descendants under the supplied platform's path rules. */
+export function pathIsWithin(base: string, candidate: string,
+  paths: PathSemantics = { sep, isAbsolute, relative }): boolean {
+  const rel = paths.relative(base, candidate);
+  return rel === '' || (!paths.isAbsolute(rel) && rel !== '..' && !rel.startsWith(`..${paths.sep}`));
+}
+
 /**
  * The clone, as a sequence of Git calls. Both variants run these same steps; only how each call runs differs. Every
  * failure after the clone directory exists, an abort included, removes it before the error leaves.
@@ -62,19 +75,15 @@ function* cloneSteps(options: CloneOptions): Generator<CloneStep, TaskClone, Git
     return value;
   };
   const source = realpathSync(options.source), parent = realpathSync(options.parent);
-  const within = (base: string, path: string) => {
-    const rel = relative(base, path);
-    return rel === '' || (!isAbsolute(rel) && rel !== '..' && !rel.startsWith('../'));
-  };
-  if (within(source, parent)) throw new Error('Task storage must be outside the source repository.');
+  if (pathIsWithin(source, parent)) throw new Error('Task storage must be outside the source repository.');
   function* run(cwd: string, ...args: string[]): Generator<CloneStep, string, GitOutcome | undefined> {
     const outcome = yield { cwd, args, timeoutMs: remaining() };
     if (!outcome || outcome.status !== 0) throw outcome?.error ?? new Error(`git ${args[0] ?? ''} failed.`);
     remaining();
     return outcome.stdout.trim();
   }
   const common = realpathSync(resolve(source, yield* run(source, 'rev-parse', '--git-common-dir')));
-  if (within(common, parent)) throw new Error('Task storage must be outside source metadata.');
+  if (pathIsWithin(common, parent)) throw new Error('Task storage must be outside source metadata.');
   function* audit(metadata: string, independent: boolean): Generator<CloneStep, void, GitOutcome | undefined> {
     for (const name of ['shallow', 'info/grafts', 'objects/info/alternates', 'objects/info/http-alternates']) {
       if (lstatSync(join(metadata, name), { throwIfNoEntry: false })) throw new Error(`Unsupported Git storage: ${name}`);

test/agent-clone.test.ts

@@ -2,9 +2,9 @@ import { execFileSync } from 'node:child_process';
 import { fixtureGit } from './fixtures/git.ts';
 import { mkdtempSync, mkdirSync, readFileSync, readdirSync, rmSync, lstatSync, symlinkSync, writeFileSync, renameSync, opendirSync } from 'node:fs';
 import { tmpdir } from 'node:os';
-import { join } from 'node:path';
+import { join, win32 } from 'node:path';
 import { afterEach, describe, expect, it, vi } from 'vitest';
-import { createTaskClone } from '../git/clone.ts';
+import { createTaskClone, pathIsWithin } from '../git/clone.ts';
 
 vi.mock('node:fs', async importOriginal => {
   const actual = await importOriginal<typeof import('node:fs')>();
@@ -26,6 +26,22 @@ function fixture() {
   return { source, parent, head: git(source, 'rev-parse', 'HEAD'), taskId: 'task-1' };
 }
 afterEach(() => { vi.restoreAllMocks(); vi.resetAllMocks(); for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); });
+describe('path containment', () => {
+  it.each([
+    ['same directory despite case', 'C:\\repo', 'c:\\REPO', true],
+    ['nested directory', 'C:\\repo', 'C:\\repo\\tasks', true],
+    ['similarly named sibling', 'C:\\repo', 'C:\\repository', false],
+    ['same-drive sibling', 'C:\\repo', 'C:\\tasks', false],
+    ['parent directory', 'C:\\repo\\source', 'C:\\repo', false],
+    ['different drive', 'C:\\repo', 'D:\\tasks', false],
+    ['nested UNC directory', '\\\\server\\share\\repo', '\\\\server\\share\\repo\\tasks', true],
+    ['same-share UNC sibling', '\\\\server\\share\\repo', '\\\\server\\share\\tasks', false],
+    ['different UNC share', '\\\\server\\share\\repo', '\\\\server\\other\\tasks', false],
+    ['different UNC server', '\\\\server\\share\\repo', '\\\\other\\share\\tasks', false],
+  ])('uses Windows boundaries for a %s', (_name, base, candidate, expected) => {
+    expect(pathIsWithin(base, candidate, win32)).toBe(expected);
+  });
+});
 describe('isolated staging clone', () => {
   it('copies objects, ignores dirty source changes, and has no origin or shared metadata', () => {
     const input = fixture();
@@ -66,9 +82,9 @@ describe('isolated staging clone', () => {
     git(input.source, 'update-ref', `refs/replace/${input.head}`, input.head);
     expect(() => createTaskClone(input)).toThrow('Replacement');
   });
-  it('rejects nested storage, including paths through symlinks', () => {
+  it('rejects nested storage after resolving a directory alias, including a Windows junction', () => {
     const input = fixture(), nested = join(input.source, 'tasks'), alias = join(input.parent, 'alias');
-    mkdirSync(nested); symlinkSync(nested, alias);
+    mkdirSync(nested); symlinkSync(nested, alias, process.platform === 'win32' ? 'junction' : 'dir');
     expect(() => createTaskClone({ ...input, parent: alias })).toThrow('outside');
   });
   it('requires a full existing commit and finite time budget', () => {