From 04802166a6295f8c5bfa18b5cc6e41ee9287657d Mon Sep 17 00:00:00 2001 From: Eyejoker Date: Sat, 4 Apr 2026 22:01:08 +0900 Subject: [PATCH] Fix reviewer canonical repo mounts --- runners/agent-runner/src/index.ts | 6 + runners/agent-runner/src/reviewer-runtime.ts | 60 ++++++++ .../test/reviewer-runtime.test.ts | 55 +++++++ runners/codex-runner/src/index.ts | 6 + runners/codex-runner/src/reviewer-runtime.ts | 60 ++++++++ .../test/reviewer-runtime.test.ts | 55 +++++++ src/container-runner.test.ts | 55 +++++++ src/container-runner.ts | 144 +++++++++++++----- 8 files changed, 404 insertions(+), 37 deletions(-) diff --git a/runners/agent-runner/src/index.ts b/runners/agent-runner/src/index.ts index 6c469fc..17e4efe 100644 --- a/runners/agent-runner/src/index.ts +++ b/runners/agent-runner/src/index.ts @@ -24,6 +24,7 @@ import { type RoomRoleContext, } from './room-role-context.js'; import { + assertReadonlyWorkspaceRepoConnectivity, buildReviewerGitGuardEnv, isReviewerMutatingShellCommand, isReviewerRuntime, @@ -948,7 +949,12 @@ async function main(): Promise { const reviewerRuntime = process.env.EJCLAW_REVIEWER_RUNTIME === '1' || isReviewerRuntime(containerInput.roomRoleContext); + const readonlyRuntime = + reviewerRuntime || + process.env.EJCLAW_ARBITER_RUNTIME === '1' || + containerInput.roomRoleContext?.role === 'arbiter'; const guardedSdkEnv = buildReviewerGitGuardEnv(sdkEnv, reviewerRuntime); + assertReadonlyWorkspaceRepoConnectivity(guardedSdkEnv, readonlyRuntime); const __dirname = path.dirname(fileURLToPath(import.meta.url)); const mcpServerPath = path.join(__dirname, 'ipc-mcp-stdio.js'); diff --git a/runners/agent-runner/src/reviewer-runtime.ts b/runners/agent-runner/src/reviewer-runtime.ts index 2d313e5..1d0856d 100644 --- a/runners/agent-runner/src/reviewer-runtime.ts +++ b/runners/agent-runner/src/reviewer-runtime.ts @@ -190,6 +190,66 @@ exec "$real_git" "$@" }; } +function readGitOutput( + args: string[], + baseEnv: NodeJS.ProcessEnv, + cwd: string, +): string { + return execFileSync('git', args, { + cwd, + env: baseEnv, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); +} + +export function assertReadonlyWorkspaceRepoConnectivity( + baseEnv: NodeJS.ProcessEnv, + enabled: boolean, +): void { + if (!enabled) { + return; + } + + const protectedWorkDir = baseEnv.EJCLAW_WORK_DIR || ''; + if (!protectedWorkDir) { + return; + } + + let originUrl = ''; + try { + originUrl = readGitOutput( + ['config', '--get', 'remote.origin.url'], + baseEnv, + protectedWorkDir, + ); + } catch { + return; + } + + if (!path.isAbsolute(originUrl) || !fs.existsSync(originUrl)) { + throw new Error( + `EJClaw readonly runtime cannot access local git origin path: ${originUrl || '(missing)'}`, + ); + } + + try { + readGitOutput(['rev-parse', '--git-dir'], baseEnv, originUrl); + } catch { + throw new Error( + `EJClaw readonly runtime origin path is not mounted as a git repository: ${originUrl}`, + ); + } + + try { + readGitOutput(['ls-remote', 'origin', 'HEAD'], baseEnv, protectedWorkDir); + } catch { + throw new Error( + `EJClaw readonly runtime cannot resolve local git origin from ${protectedWorkDir}. Check canonical repo mount for ${originUrl}.`, + ); + } +} + export function isReviewerMutatingShellCommand(command: string): boolean { const normalized = command.trim(); return ( diff --git a/runners/agent-runner/test/reviewer-runtime.test.ts b/runners/agent-runner/test/reviewer-runtime.test.ts index ba3677b..65e3807 100644 --- a/runners/agent-runner/test/reviewer-runtime.test.ts +++ b/runners/agent-runner/test/reviewer-runtime.test.ts @@ -6,6 +6,7 @@ import path from 'path'; import { describe, expect, it } from 'vitest'; import { + assertReadonlyWorkspaceRepoConnectivity, buildReviewerGitGuardEnv, isReviewerMutatingShellCommand, isReviewerRuntime, @@ -28,6 +29,17 @@ function createTempRepo(prefix: string): string { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'], }); + fs.writeFileSync(path.join(cwd, 'README.md'), 'seed\n'); + execFileSync('git', ['add', 'README.md'], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + execFileSync('git', ['commit', '-m', 'seed'], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); return cwd; } @@ -135,4 +147,47 @@ describe('claude reviewer runtime guard', () => { ); } }); + + it('accepts a mounted local origin path that resolves as a git repo', () => { + const originDir = createTempRepo('ejclaw-reviewer-origin-'); + const cwd = createTempRepo('ejclaw-reviewer-workspace-'); + execFileSync('git', ['remote', 'add', 'origin', originDir], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + const env = buildReviewerGitGuardEnv( + { + PATH: process.env.PATH, + EJCLAW_WORK_DIR: cwd, + }, + true, + ); + + expect(() => assertReadonlyWorkspaceRepoConnectivity(env, true)).not.toThrow(); + }); + + it('fails fast when the local origin path is not mounted as a git repo', () => { + const cwd = createTempRepo('ejclaw-reviewer-workspace-'); + const missingOriginDir = path.join( + os.tmpdir(), + `ejclaw-reviewer-missing-origin-${Date.now()}`, + ); + execFileSync('git', ['remote', 'add', 'origin', missingOriginDir], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + const env = buildReviewerGitGuardEnv( + { + PATH: process.env.PATH, + EJCLAW_WORK_DIR: cwd, + }, + true, + ); + + expect(() => assertReadonlyWorkspaceRepoConnectivity(env, true)).toThrow( + `EJClaw readonly runtime cannot access local git origin path: ${missingOriginDir}`, + ); + }); }); diff --git a/runners/codex-runner/src/index.ts b/runners/codex-runner/src/index.ts index 8159712..037a2d8 100644 --- a/runners/codex-runner/src/index.ts +++ b/runners/codex-runner/src/index.ts @@ -24,6 +24,7 @@ import { type RoomRoleContext, } from './room-role-context.js'; import { + assertReadonlyWorkspaceRepoConnectivity, buildReviewerGitGuardEnv, isReviewerRuntime, } from './reviewer-runtime.js'; @@ -405,7 +406,12 @@ async function runAppServerSession( const reviewerRuntime = process.env.EJCLAW_REVIEWER_RUNTIME === '1' || isReviewerRuntime(containerInput.roomRoleContext); + const readonlyRuntime = + reviewerRuntime || + process.env.EJCLAW_ARBITER_RUNTIME === '1' || + containerInput.roomRoleContext?.role === 'arbiter'; const clientEnv = buildReviewerGitGuardEnv(process.env, reviewerRuntime); + assertReadonlyWorkspaceRepoConnectivity(clientEnv, readonlyRuntime); const client = new CodexAppServerClient({ cwd: EFFECTIVE_CWD, env: clientEnv, diff --git a/runners/codex-runner/src/reviewer-runtime.ts b/runners/codex-runner/src/reviewer-runtime.ts index 338e565..a77cdcd 100644 --- a/runners/codex-runner/src/reviewer-runtime.ts +++ b/runners/codex-runner/src/reviewer-runtime.ts @@ -186,3 +186,63 @@ exec "$real_git" "$@" PATH: `${wrapperDir}:${baseEnv.PATH || ''}`, }; } + +function readGitOutput( + args: string[], + baseEnv: NodeJS.ProcessEnv, + cwd: string, +): string { + return execFileSync('git', args, { + cwd, + env: baseEnv, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); +} + +export function assertReadonlyWorkspaceRepoConnectivity( + baseEnv: NodeJS.ProcessEnv, + enabled: boolean, +): void { + if (!enabled) { + return; + } + + const protectedWorkDir = baseEnv.EJCLAW_WORK_DIR || ''; + if (!protectedWorkDir) { + return; + } + + let originUrl = ''; + try { + originUrl = readGitOutput( + ['config', '--get', 'remote.origin.url'], + baseEnv, + protectedWorkDir, + ); + } catch { + return; + } + + if (!path.isAbsolute(originUrl) || !fs.existsSync(originUrl)) { + throw new Error( + `EJClaw readonly runtime cannot access local git origin path: ${originUrl || '(missing)'}`, + ); + } + + try { + readGitOutput(['rev-parse', '--git-dir'], baseEnv, originUrl); + } catch { + throw new Error( + `EJClaw readonly runtime origin path is not mounted as a git repository: ${originUrl}`, + ); + } + + try { + readGitOutput(['ls-remote', 'origin', 'HEAD'], baseEnv, protectedWorkDir); + } catch { + throw new Error( + `EJClaw readonly runtime cannot resolve local git origin from ${protectedWorkDir}. Check canonical repo mount for ${originUrl}.`, + ); + } +} diff --git a/runners/codex-runner/test/reviewer-runtime.test.ts b/runners/codex-runner/test/reviewer-runtime.test.ts index eb629ab..b2a542c 100644 --- a/runners/codex-runner/test/reviewer-runtime.test.ts +++ b/runners/codex-runner/test/reviewer-runtime.test.ts @@ -6,6 +6,7 @@ import path from 'path'; import { describe, expect, it } from 'vitest'; import { + assertReadonlyWorkspaceRepoConnectivity, buildReviewerGitGuardEnv, isReviewerRuntime, } from '../src/reviewer-runtime.js'; @@ -27,6 +28,17 @@ function createTempRepo(prefix: string): string { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'], }); + fs.writeFileSync(path.join(cwd, 'README.md'), 'seed\n'); + execFileSync('git', ['add', 'README.md'], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + execFileSync('git', ['commit', '-m', 'seed'], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); return cwd; } @@ -122,4 +134,47 @@ describe('codex reviewer runtime guard', () => { ); } }); + + it('accepts a mounted local origin path that resolves as a git repo', () => { + const originDir = createTempRepo('ejclaw-reviewer-origin-'); + const cwd = createTempRepo('ejclaw-reviewer-workspace-'); + execFileSync('git', ['remote', 'add', 'origin', originDir], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + const env = buildReviewerGitGuardEnv( + { + PATH: process.env.PATH, + EJCLAW_WORK_DIR: cwd, + }, + true, + ); + + expect(() => assertReadonlyWorkspaceRepoConnectivity(env, true)).not.toThrow(); + }); + + it('fails fast when the local origin path is not mounted as a git repo', () => { + const cwd = createTempRepo('ejclaw-reviewer-workspace-'); + const missingOriginDir = path.join( + os.tmpdir(), + `ejclaw-reviewer-missing-origin-${Date.now()}`, + ); + execFileSync('git', ['remote', 'add', 'origin', missingOriginDir], { + cwd, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + const env = buildReviewerGitGuardEnv( + { + PATH: process.env.PATH, + EJCLAW_WORK_DIR: cwd, + }, + true, + ); + + expect(() => assertReadonlyWorkspaceRepoConnectivity(env, true)).toThrow( + `EJClaw readonly runtime cannot access local git origin path: ${missingOriginDir}`, + ); + }); }); diff --git a/src/container-runner.test.ts b/src/container-runner.test.ts index 4796fde..b587bf5 100644 --- a/src/container-runner.test.ts +++ b/src/container-runner.test.ts @@ -1,3 +1,4 @@ +import { execFileSync } from 'child_process'; import fs from 'fs'; import path from 'path'; @@ -76,6 +77,25 @@ function makeGroup(): RegisteredGroup { }; } +function initGitRepo(repoDir: string): void { + fs.mkdirSync(repoDir, { recursive: true }); + execFileSync('git', ['init'], { + cwd: repoDir, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + execFileSync('git', ['config', 'user.email', 'test@example.com'], { + cwd: repoDir, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + execFileSync('git', ['config', 'user.name', 'EJClaw Test'], { + cwd: repoDir, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); +} + describe('container-runner path compatibility', () => { const previousCodeXHome = process.env.CODEX_HOME; const previousOauthToken = process.env.CLAUDE_CODE_OAUTH_TOKEN; @@ -143,6 +163,41 @@ describe('container-runner path compatibility', () => { ); }); + it('mounts a local absolute origin target and shadows its .env', () => { + const group = makeGroup(); + const canonicalRepoDir = path.join(TEST_ROOT, 'canonical'); + const ownerWorkspaceDir = path.join(TEST_ROOT, 'workspace', 'owner'); + initGitRepo(canonicalRepoDir); + initGitRepo(ownerWorkspaceDir); + execFileSync('git', ['remote', 'add', 'origin', canonicalRepoDir], { + cwd: ownerWorkspaceDir, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + fs.writeFileSync(path.join(canonicalRepoDir, '.env'), 'CANONICAL=1\n'); + fs.mkdirSync(path.join(TEST_GROUPS_DIR, group.folder), { recursive: true }); + fs.mkdirSync(path.join(TEST_DATA_DIR, 'ipc', group.folder), { + recursive: true, + }); + + const mounts = buildReviewerMounts(group, ownerWorkspaceDir); + + expect(mounts).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + hostPath: canonicalRepoDir, + containerPath: canonicalRepoDir, + readonly: true, + }), + expect.objectContaining({ + hostPath: '/dev/null', + containerPath: path.join(canonicalRepoDir, '.env'), + readonly: true, + }), + ]), + ); + }); + it('keeps dynamic workdir and role env on docker exec instead of baking them into container creation', () => { const createArgs = buildCreateArgs([], 'ejclaw-reviewer-brain'); const createText = createArgs.join(' '); diff --git a/src/container-runner.ts b/src/container-runner.ts index 41c9afa..d3cd7dd 100644 --- a/src/container-runner.ts +++ b/src/container-runner.ts @@ -85,6 +85,82 @@ function pushMountOnce(mounts: VolumeMount[], mount: VolumeMount): void { } } +function normalizeLocalGitPath(remoteUrl: string): string | null { + if (!remoteUrl) { + return null; + } + if (path.isAbsolute(remoteUrl)) { + return path.resolve(remoteUrl); + } + if (remoteUrl.startsWith('file://')) { + try { + const parsed = new URL(remoteUrl); + if (parsed.protocol === 'file:') { + return path.resolve(parsed.pathname); + } + } catch { + return null; + } + } + return null; +} + +function resolveLocalOriginTarget(workspaceDir: string): string | null { + try { + const remoteUrl = execFileSync( + 'git', + ['config', '--get', 'remote.origin.url'], + { + cwd: workspaceDir, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 5000, + }, + ).trim(); + const localPath = normalizeLocalGitPath(remoteUrl); + if (!localPath || !fs.existsSync(localPath)) { + return null; + } + return localPath; + } catch { + return null; + } +} + +function pushWorktreeGitMetadataMounts( + mounts: VolumeMount[], + repoDir: string, +): void { + const dotGitPath = path.join(repoDir, '.git'); + try { + const stat = fs.statSync(dotGitPath); + if (!stat.isFile()) { + return; + } + const content = fs.readFileSync(dotGitPath, 'utf-8').trim(); + const match = content.match(/^gitdir:\s*(.+)$/); + if (!match) { + return; + } + const worktreeGitDir = path.resolve(repoDir, match[1]); + const parentGitDir = path.resolve(worktreeGitDir, '..', '..'); + if (!fs.existsSync(parentGitDir)) { + return; + } + pushMountOnce(mounts, { + hostPath: parentGitDir, + containerPath: parentGitDir, + readonly: true, + }); + logger.debug( + { parentGitDir, worktreeGitDir, repoDir }, + 'Mounting parent .git for worktree resolution', + ); + } catch { + // Not a git repo or .git missing — skip + } +} + // ── pnpm store detection ───────────────────────────────────────── function detectPnpmStorePath(workspaceDir: string): string | null { @@ -282,6 +358,18 @@ export function buildReviewerMounts( containerPath: PRIMARY_PROJECT_MOUNT, readonly: true, }); + const canonicalRepoDir = resolveLocalOriginTarget(ownerWorkspaceDir); + if ( + canonicalRepoDir && + canonicalRepoDir !== ownerWorkspaceDir && + canonicalRepoDir !== PRIMARY_PROJECT_MOUNT + ) { + pushMountOnce(mounts, { + hostPath: canonicalRepoDir, + containerPath: canonicalRepoDir, + readonly: true, + }); + } // Compatibility mount: expose the owner workspace at the same absolute path // inside the container so owner-authored absolute paths still resolve. if (ownerWorkspaceDir !== PRIMARY_PROJECT_MOUNT) { @@ -295,32 +383,9 @@ export function buildReviewerMounts( // Git worktree support: worktree's .git file references the parent repo's // .git directory via absolute path. Mount the parent .git at the same host // path so git commands resolve inside the container. - const dotGitPath = path.join(ownerWorkspaceDir, '.git'); - try { - const stat = fs.statSync(dotGitPath); - if (stat.isFile()) { - // Worktree: .git is a file containing "gitdir: /path/to/.git/worktrees/name" - const content = fs.readFileSync(dotGitPath, 'utf-8').trim(); - const match = content.match(/^gitdir:\s*(.+)$/); - if (match) { - const worktreeGitDir = path.resolve(ownerWorkspaceDir, match[1]); - // worktreeGitDir = /parent/.git/worktrees/name → parent .git = ../../ - const parentGitDir = path.resolve(worktreeGitDir, '..', '..'); - if (fs.existsSync(parentGitDir)) { - pushMountOnce(mounts, { - hostPath: parentGitDir, - containerPath: parentGitDir, - readonly: true, - }); - logger.debug( - { parentGitDir, worktreeGitDir }, - 'Mounting parent .git for worktree resolution', - ); - } - } - } - } catch { - // Not a git repo or .git missing — skip + pushWorktreeGitMetadataMounts(mounts, ownerWorkspaceDir); + if (canonicalRepoDir) { + pushWorktreeGitMetadataMounts(mounts, canonicalRepoDir); } // pnpm global store: mount at the same host path so hardlinks resolve. @@ -336,18 +401,23 @@ export function buildReviewerMounts( // Shadow .env so reviewer cannot read secrets from mounted project const envFile = path.join(ownerWorkspaceDir, '.env'); + const shadowPaths = new Set(); if (fs.existsSync(envFile)) { - const shadowPaths = new Set([ - path.join(PRIMARY_PROJECT_MOUNT, '.env'), - path.join(ownerWorkspaceDir, '.env'), - ]); - for (const shadowPath of shadowPaths) { - pushMountOnce(mounts, { - hostPath: '/dev/null', - containerPath: shadowPath, - readonly: true, - }); - } + shadowPaths.add(path.join(PRIMARY_PROJECT_MOUNT, '.env')); + shadowPaths.add(path.join(ownerWorkspaceDir, '.env')); + } + const canonicalEnvFile = canonicalRepoDir + ? path.join(canonicalRepoDir, '.env') + : null; + if (canonicalEnvFile && fs.existsSync(canonicalEnvFile)) { + shadowPaths.add(canonicalEnvFile); + } + for (const shadowPath of shadowPaths) { + pushMountOnce(mounts, { + hostPath: '/dev/null', + containerPath: shadowPath, + readonly: true, + }); } // Attachments directory: read-only (Discord file uploads downloaded here)