Keep experiment destinations outside the source checkout

Author: mchwang

docs/implementation/read-only-review.md

@@ -46,3 +46,5 @@ Review round 3 identified demo fixture config escapes and inherited Git environm
 Review round 4 had no inline findings but raised two summary concerns. Both were reproduced: planting accepted a non-ASCII path under case-insensitive identity even though the viewer refuses it; accepted cards retained an unplanned scope label. Planting now applies the same path guard, and accepted cards explicitly say “Accepted outside plan.” The underlying provenance remains intact. Unit/browser regressions cover both fixes; no concerns were declined.
 
 Review round 5 found a symlinked-ancestor escape and raw-spelling collisions in planting guards. Both regression cases failed before the fix. Demo creation/reuse now checks every ancestor (canonicalizing only the OS temporary-directory prefix); planting compares canonical identities for declared files and every path in the base/commit trees before cloning. The summary's status-styling concern had no additional concrete example; accepted cards already use the explicit label added in round 4.
+
+Review round 6 reproduced destination-inside-source mutation; planting now resolves the existing destination ancestor and rejects canonical source descendants before creating files, including `.git` and symlink aliases. The Git-environment finding was declined: `gitRaw` already passes `{cwd, env, ...}` to `execFileSync`. A new end-to-end test with inherited `GIT_DIR`/`GIT_WORK_TREE` passed before any production change. The summary's snapshot-race claim supplied no concrete interleaving; the service already checks revision/snapshot/review version before publishing a view and writes use atomic store CAS, covered by concurrent-write tests.

scripts/plant.ts

@@ -1,7 +1,7 @@
 import { isolatedGitEnvironment } from './git-environment.ts';
 import { execFileSync } from 'node:child_process';
-import { existsSync, mkdirSync, writeFileSync, appendFileSync, lstatSync, readFileSync } from 'node:fs';
-import { resolve, join, dirname } from 'node:path';
+import { existsSync, mkdirSync, writeFileSync, appendFileSync, lstatSync, readFileSync, realpathSync } from 'node:fs';
+import { resolve, join, dirname, basename, relative, isAbsolute } from 'node:path';
 import { randomInt, randomUUID } from 'node:crypto';
 import { pathToFileURL } from 'node:url';
 import { Store } from '../runner/store.ts';
@@ -15,6 +15,11 @@ export interface PlantInput { declaredText: string; undeclaredText: string; unde
 export function plant(config: ReviewConfig, destination: string, input: PlantInput): string {
   const root=resolve(destination);
   if(existsSync(root))throw new Error('Experiment destination must not exist.');
+  let parent=root; const missing:string[]=[];
+  while(!lstatSync(parent,{throwIfNoEntry:false})) { missing.unshift(basename(parent)); parent=dirname(parent); }
+  const canonicalDestination=resolve(realpathSync(parent),...missing);
+  const sourceRelative=relative(realpathSync(config.repository),canonicalDestination);
+  if(sourceRelative===''||(!sourceRelative.startsWith('..'+(process.platform==='win32'?'\\':'/'))&&sourceRelative!=='..'&&!isAbsolute(sourceRelative))) throw new Error('Experiment destination must be outside the source repository.');
   if((!isRepoPath(input.undeclaredPath)||input.undeclaredPath.includes('/'))||!input.declaredText?.trim()||!input.undeclaredText?.trim()||input.declaredText.length>4000||input.undeclaredText.length>4000)throw new Error('Invalid plant input.');
   const pathKey=(path:string)=>{
     if(!config.pathIdentity.caseSensitive && /[^\x20-\x7e]/.test(path)) throw new Error('Non-ASCII case-insensitive paths require a filesystem-specific identity adapter.');

test/plant.test.ts

@@ -1,5 +1,5 @@
-import { it,expect,afterEach } from 'vitest';
-import { mkdtempSync,readFileSync,rmSync } from 'node:fs';
+import { it,expect,afterEach,vi } from 'vitest';
+import { mkdtempSync,readFileSync,rmSync,symlinkSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { execFileSync } from 'node:child_process';
@@ -25,3 +25,18 @@ it('rejects canonical declared and existing path collisions before creating a cl
  const config=createDemo(join(root,'source'));config.pathIdentity.caseSensitive=false;
  for (const path of ['RETRY.TS','RUN.SH']) expect(()=>plant(config,join(root,'experiment'),{declaredText:'// extra',undeclaredText:'diagnostic',undeclaredPath:path})).toThrow(/outside every declared file|already exists/);
 },15000);
+
+it('rejects experiment destinations inside the source, including symlink aliases', () => {
+ const root=mkdtempSync(join(tmpdir(),'codeboost-plant-destination-'));roots.push(root);
+ const config=createDemo(join(root,'source'));symlinkSync(config.repository,join(root,'alias'));
+ for(const destination of [join(config.repository,'experiment'),join(config.repository,'.git','experiment'),join(root,'alias','nested','experiment')]) {
+   expect(()=>plant(config,destination,{declaredText:'// extra',undeclaredText:'diagnostic',undeclaredPath:'extra.txt'})).toThrow(/outside the source repository/);
+ }
+},15000);
+it('passes its isolated environment to every planting Git process', () => {
+ const root=mkdtempSync(join(tmpdir(),'codeboost-plant-env-'));roots.push(root);
+ const config=createDemo(join(root,'source'));
+ vi.stubEnv('GIT_DIR',join(root,'outside.git'));vi.stubEnv('GIT_WORK_TREE',join(root,'outside'));
+ try {expect(plant(config,join(root,'experiment'),{declaredText:'// extra',undeclaredText:'diagnostic',undeclaredPath:'extra.txt'})).toBe(join(root,'experiment','review.json'));}
+ finally {vi.unstubAllEnvs();}
+},15000);