Stabilize paired reviewer recovery and owner follow-up
This commit is contained in:
@@ -25,7 +25,10 @@ import {
|
||||
} from './room-role-context.js';
|
||||
import {
|
||||
assertReadonlyWorkspaceRepoConnectivity,
|
||||
buildClaudeReadonlySandboxSettings,
|
||||
buildReviewerGitGuardEnv,
|
||||
getClaudeReadonlySandboxMode,
|
||||
isClaudeReadonlyReviewerRuntime,
|
||||
isReviewerMutatingShellCommand,
|
||||
isReviewerRuntime,
|
||||
} from './reviewer-runtime.js';
|
||||
@@ -545,6 +548,8 @@ async function runQuery(
|
||||
containerInput: ContainerInput,
|
||||
sdkEnv: Record<string, string | undefined>,
|
||||
reviewerRuntime: boolean,
|
||||
claudeReadonlyReviewerRuntime: boolean,
|
||||
claudeReadonlySandboxMode: 'strict' | 'best-effort' | null,
|
||||
): Promise<{
|
||||
newSessionId?: string;
|
||||
closedDuringQuery: boolean;
|
||||
@@ -622,8 +627,20 @@ async function runQuery(
|
||||
if (thinking) log(`Thinking config: ${JSON.stringify(thinking)}`);
|
||||
if (effort) log(`Effort: ${effort}`);
|
||||
if (reviewerRuntime) log('Reviewer runtime restrictions enabled');
|
||||
if (claudeReadonlyReviewerRuntime) {
|
||||
log(
|
||||
`Claude host reviewer read-only sandbox enabled (${claudeReadonlySandboxMode || 'unknown'})`,
|
||||
);
|
||||
}
|
||||
if (claudeReadonlySandboxMode === 'best-effort') {
|
||||
log(
|
||||
'Claude host reviewer sandbox capability unavailable on this host, using best-effort read-only mode',
|
||||
);
|
||||
}
|
||||
|
||||
const allowedTools = reviewerRuntime
|
||||
const readonlyReviewerRuntime =
|
||||
reviewerRuntime || claudeReadonlyReviewerRuntime;
|
||||
const allowedTools = readonlyReviewerRuntime
|
||||
? [
|
||||
'Bash',
|
||||
'Read', 'Glob', 'Grep',
|
||||
@@ -644,6 +661,18 @@ async function runQuery(
|
||||
'mcp__ejclaw__*'
|
||||
];
|
||||
|
||||
const readonlyProtectedPaths = [effectiveCwd, ...extraDirs].filter(
|
||||
(value): value is string => Boolean(value),
|
||||
);
|
||||
const claudeReadonlySandboxSettings =
|
||||
claudeReadonlyReviewerRuntime && claudeReadonlySandboxMode
|
||||
? buildClaudeReadonlySandboxSettings(
|
||||
readonlyProtectedPaths,
|
||||
undefined,
|
||||
claudeReadonlySandboxMode,
|
||||
)
|
||||
: undefined;
|
||||
|
||||
for await (const message of query({
|
||||
prompt: stream,
|
||||
options: {
|
||||
@@ -657,6 +686,11 @@ async function runQuery(
|
||||
env: sdkEnv,
|
||||
permissionMode: 'bypassPermissions',
|
||||
allowDangerouslySkipPermissions: true,
|
||||
...(claudeReadonlySandboxSettings
|
||||
? {
|
||||
sandbox: claudeReadonlySandboxSettings,
|
||||
}
|
||||
: {}),
|
||||
settingSources: ['project', 'user'],
|
||||
abortController: agentAbortController,
|
||||
mcpServers: {
|
||||
@@ -684,7 +718,7 @@ async function runQuery(
|
||||
PreToolUse: [
|
||||
{
|
||||
matcher: 'Bash',
|
||||
hooks: reviewerRuntime
|
||||
hooks: readonlyReviewerRuntime
|
||||
? [createReviewerBashGuardHook(), createSanitizeBashHook()]
|
||||
: [createSanitizeBashHook()],
|
||||
},
|
||||
@@ -950,9 +984,19 @@ async function main(): Promise<void> {
|
||||
const reviewerRuntime =
|
||||
process.env.EJCLAW_REVIEWER_RUNTIME === '1' ||
|
||||
isReviewerRuntime(containerInput.roomRoleContext);
|
||||
const claudeReadonlyReviewerRuntime =
|
||||
isClaudeReadonlyReviewerRuntime(containerInput.roomRoleContext);
|
||||
const claudeReadonlySandboxMode = claudeReadonlyReviewerRuntime
|
||||
? getClaudeReadonlySandboxMode()
|
||||
: null;
|
||||
const readonlyRuntime =
|
||||
reviewerRuntime || process.env.EJCLAW_ARBITER_RUNTIME === '1';
|
||||
const guardedSdkEnv = buildReviewerGitGuardEnv(sdkEnv, reviewerRuntime);
|
||||
reviewerRuntime ||
|
||||
claudeReadonlyReviewerRuntime ||
|
||||
process.env.EJCLAW_ARBITER_RUNTIME === '1';
|
||||
const guardedSdkEnv = buildReviewerGitGuardEnv(
|
||||
sdkEnv,
|
||||
reviewerRuntime || claudeReadonlyReviewerRuntime,
|
||||
);
|
||||
assertReadonlyWorkspaceRepoConnectivity(guardedSdkEnv, readonlyRuntime);
|
||||
|
||||
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
@@ -1092,6 +1136,8 @@ async function main(): Promise<void> {
|
||||
containerInput,
|
||||
guardedSdkEnv,
|
||||
reviewerRuntime,
|
||||
claudeReadonlyReviewerRuntime,
|
||||
claudeReadonlySandboxMode,
|
||||
);
|
||||
if (queryResult.newSessionId) {
|
||||
sessionId = queryResult.newSessionId;
|
||||
|
||||
@@ -5,6 +5,8 @@ import path from 'path';
|
||||
|
||||
import type { RoomRoleContext } from './room-role-context.js';
|
||||
|
||||
export type ClaudeReadonlySandboxMode = 'strict' | 'best-effort';
|
||||
|
||||
const BLOCKED_GIT_SUBCOMMANDS = new Set([
|
||||
'add',
|
||||
'am',
|
||||
@@ -40,6 +42,89 @@ export function isReviewerRuntime(
|
||||
return roomRoleContext?.role === 'reviewer';
|
||||
}
|
||||
|
||||
export function isClaudeReadonlyReviewerRuntime(
|
||||
roomRoleContext?: RoomRoleContext,
|
||||
): boolean {
|
||||
return (
|
||||
process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE === '1' &&
|
||||
process.env.EJCLAW_CLAUDE_REVIEWER_READONLY === '1' &&
|
||||
roomRoleContext?.role === 'reviewer'
|
||||
);
|
||||
}
|
||||
|
||||
let cachedLinuxBubblewrapReadonlyCapability: boolean | undefined;
|
||||
|
||||
function commandExistsForRuntime(command: string): boolean {
|
||||
try {
|
||||
execFileSync('bash', ['-lc', `command -v ${command}`], {
|
||||
stdio: ['ignore', 'ignore', 'ignore'],
|
||||
});
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function canUseLinuxBubblewrapReadonlySandbox(): boolean {
|
||||
if (cachedLinuxBubblewrapReadonlyCapability != null) {
|
||||
return cachedLinuxBubblewrapReadonlyCapability;
|
||||
}
|
||||
if (os.platform() !== 'linux') {
|
||||
cachedLinuxBubblewrapReadonlyCapability = false;
|
||||
return false;
|
||||
}
|
||||
if (!commandExistsForRuntime('bwrap')) {
|
||||
cachedLinuxBubblewrapReadonlyCapability = false;
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
execFileSync('bwrap', ['--ro-bind', '/', '/', '/bin/true'], {
|
||||
stdio: ['ignore', 'ignore', 'ignore'],
|
||||
});
|
||||
cachedLinuxBubblewrapReadonlyCapability = true;
|
||||
} catch {
|
||||
cachedLinuxBubblewrapReadonlyCapability = false;
|
||||
}
|
||||
|
||||
return cachedLinuxBubblewrapReadonlyCapability;
|
||||
}
|
||||
|
||||
export function getClaudeReadonlySandboxMode(
|
||||
platform: NodeJS.Platform = os.platform(),
|
||||
linuxCapabilityProbe: () => boolean = canUseLinuxBubblewrapReadonlySandbox,
|
||||
): ClaudeReadonlySandboxMode {
|
||||
if (platform === 'linux') {
|
||||
return linuxCapabilityProbe() ? 'strict' : 'best-effort';
|
||||
}
|
||||
return 'best-effort';
|
||||
}
|
||||
|
||||
export function buildClaudeReadonlySandboxSettings(
|
||||
protectedPaths: string[],
|
||||
platform: NodeJS.Platform = os.platform(),
|
||||
sandboxMode: ClaudeReadonlySandboxMode = getClaudeReadonlySandboxMode(
|
||||
platform,
|
||||
),
|
||||
) {
|
||||
const normalizedPaths = [...new Set(
|
||||
protectedPaths
|
||||
.filter((value): value is string => Boolean(value))
|
||||
.map((value) => path.resolve(value)),
|
||||
)];
|
||||
|
||||
return {
|
||||
enabled: true,
|
||||
failIfUnavailable: sandboxMode === 'strict',
|
||||
autoAllowBashIfSandboxed: true,
|
||||
allowUnsandboxedCommands: false,
|
||||
filesystem:
|
||||
normalizedPaths.length > 0
|
||||
? { denyWrite: normalizedPaths }
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
function resolveGitBinary(baseEnv: NodeJS.ProcessEnv): string {
|
||||
return execFileSync('bash', ['-lc', 'command -v git'], {
|
||||
encoding: 'utf-8',
|
||||
|
||||
@@ -3,15 +3,44 @@ import fs from 'fs';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
assertReadonlyWorkspaceRepoConnectivity,
|
||||
buildClaudeReadonlySandboxSettings,
|
||||
buildReviewerGitGuardEnv,
|
||||
getClaudeReadonlySandboxMode,
|
||||
isClaudeReadonlyReviewerRuntime,
|
||||
isReviewerMutatingShellCommand,
|
||||
isReviewerRuntime,
|
||||
} from '../src/reviewer-runtime.js';
|
||||
|
||||
const ORIGINAL_CLAUDE_REVIEWER_READONLY =
|
||||
process.env.EJCLAW_CLAUDE_REVIEWER_READONLY;
|
||||
const ORIGINAL_UNSAFE_HOST_PAIRED_MODE =
|
||||
process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE;
|
||||
|
||||
afterEach(() => {
|
||||
if (ORIGINAL_CLAUDE_REVIEWER_READONLY == null) {
|
||||
delete process.env.EJCLAW_CLAUDE_REVIEWER_READONLY;
|
||||
} else {
|
||||
process.env.EJCLAW_CLAUDE_REVIEWER_READONLY =
|
||||
ORIGINAL_CLAUDE_REVIEWER_READONLY;
|
||||
}
|
||||
|
||||
if (ORIGINAL_UNSAFE_HOST_PAIRED_MODE == null) {
|
||||
delete process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE;
|
||||
} else {
|
||||
process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE =
|
||||
ORIGINAL_UNSAFE_HOST_PAIRED_MODE;
|
||||
}
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
delete process.env.EJCLAW_CLAUDE_REVIEWER_READONLY;
|
||||
delete process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE;
|
||||
});
|
||||
|
||||
function createTempRepo(prefix: string): string {
|
||||
const cwd = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
|
||||
execFileSync('git', ['init'], {
|
||||
@@ -56,6 +85,76 @@ describe('claude reviewer runtime guard', () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('detects claude host reviewer read-only mode from env + role', () => {
|
||||
process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE = '1';
|
||||
process.env.EJCLAW_CLAUDE_REVIEWER_READONLY = '1';
|
||||
|
||||
expect(
|
||||
isClaudeReadonlyReviewerRuntime({
|
||||
serviceId: 'claude',
|
||||
role: 'reviewer',
|
||||
ownerServiceId: 'codex-main',
|
||||
reviewerServiceId: 'claude',
|
||||
failoverOwner: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isClaudeReadonlyReviewerRuntime({
|
||||
serviceId: 'claude',
|
||||
role: 'owner',
|
||||
ownerServiceId: 'codex-main',
|
||||
reviewerServiceId: 'claude',
|
||||
failoverOwner: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('builds a read-only sandbox with normalized protected paths', () => {
|
||||
const sandbox = buildClaudeReadonlySandboxSettings([
|
||||
'/repo/work',
|
||||
'/repo/work',
|
||||
'/repo/owner/../owner',
|
||||
], 'linux', 'strict');
|
||||
|
||||
expect(sandbox).toMatchObject({
|
||||
enabled: true,
|
||||
failIfUnavailable: true,
|
||||
autoAllowBashIfSandboxed: true,
|
||||
allowUnsandboxedCommands: false,
|
||||
filesystem: {
|
||||
denyWrite: ['/repo/work', '/repo/owner'],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('uses best-effort sandbox mode when linux capability probe fails', () => {
|
||||
expect(getClaudeReadonlySandboxMode('linux', () => false)).toBe(
|
||||
'best-effort',
|
||||
);
|
||||
|
||||
const sandbox = buildClaudeReadonlySandboxSettings(
|
||||
['/repo/work'],
|
||||
'linux',
|
||||
'best-effort',
|
||||
);
|
||||
|
||||
expect(sandbox.failIfUnavailable).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps non-linux reviewers in best-effort sandbox mode', () => {
|
||||
expect(getClaudeReadonlySandboxMode('darwin', () => true)).toBe(
|
||||
'best-effort',
|
||||
);
|
||||
|
||||
const sandbox = buildClaudeReadonlySandboxSettings(
|
||||
['/repo/work'],
|
||||
'darwin',
|
||||
'best-effort',
|
||||
);
|
||||
|
||||
expect(sandbox.failIfUnavailable).toBe(false);
|
||||
});
|
||||
|
||||
it('flags mutating shell commands', () => {
|
||||
expect(isReviewerMutatingShellCommand('git commit -m "x"')).toBe(false);
|
||||
expect(isReviewerMutatingShellCommand('git -c color.ui=false commit -m "x"')).toBe(
|
||||
|
||||
Reference in New Issue
Block a user