diff --git a/scripts/orchestrate-codex-worker.sh b/scripts/orchestrate-codex-worker.sh index a93caa7b2..a22703b9e 100755 --- a/scripts/orchestrate-codex-worker.sh +++ b/scripts/orchestrate-codex-worker.sh @@ -65,8 +65,24 @@ esac # Contain the task file to the current worktree so a malicious launcher cannot # point the worker at /etc/passwd or a sibling checkout. -task_real="$(realpath -m "$task_file" 2>/dev/null || readlink -f "$task_file" 2>/dev/null || printf '%s' "$task_file")" -work_real="$(pwd -P 2>/dev/null || pwd)" +canonical_path() { + local candidate="$1" + if command -v cygpath >/dev/null 2>&1; then + # Native Node callers supply C:\ paths (sometimes 8.3 names), whereas + # MSYS pwd returns /c/ paths. Expand and normalize both before comparison. + candidate="$(cygpath -a -m -l "$candidate")" || return 1 + candidate="$(cygpath -a -u "$candidate")" || return 1 + fi + # Only existing paths are valid. Resolve symlinks and never fall back to + # an uncanonicalized string if the platform cannot resolve the path. + realpath "$candidate" 2>/dev/null || readlink -f "$candidate" 2>/dev/null +} + +if ! task_real="$(canonical_path "$task_file")" || ! work_real="$(canonical_path "$(pwd -P)")"; then + echo "[ECC worker] Refusing to run: cannot resolve task file or worktree" >&2 + write_status "failed" "- Error: cannot resolve task file or worktree" + exit 1 +fi case "$task_real" in "$work_real"/*) ;; *) diff --git a/tests/hooks/context-prompt-suggestions.test.js b/tests/hooks/context-prompt-suggestions.test.js index 1b51bdf83..b6c95bef3 100644 --- a/tests/hooks/context-prompt-suggestions.test.js +++ b/tests/hooks/context-prompt-suggestions.test.js @@ -116,7 +116,9 @@ test('loading advice matches the saved mode', () => { }); test('shell metacharacters in a store path remain data rather than executable advice', () => { - const root = '/tmp/store-$(touch SHOULD_NOT_RUN)-`echo injected`-"quoted"'; + // Use a native canonical absolute root and filename characters legal on + // Windows while still exercising shell substitutions and command separators. + const root = path.join(path.resolve(os.tmpdir()), "store-$(touch SHOULD_NOT_RUN)-`echo injected`-'quote'-semi;and&"); const routingModule = require('../../scripts/lib/context-routing-index'); const originals = { readBinding: routingModule.readBinding, readRoutingIndex: routingModule.readRoutingIndex, suggestContext: routingModule.suggestContext }; diff --git a/tests/scripts/orchestrate-codex-worker.test.js b/tests/scripts/orchestrate-codex-worker.test.js index 48a3ca6e0..a02a9df6f 100644 --- a/tests/scripts/orchestrate-codex-worker.test.js +++ b/tests/scripts/orchestrate-codex-worker.test.js @@ -24,18 +24,18 @@ function test(desc, fn) { } } -function runWorker({ policy, exitCode = 0 } = {}) { +function runWorker({ policy, exitCode = 0, outsideTask = false, relativeTask = false } = {}) { const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-orch-worker-')); const binDir = path.join(tempRoot, 'bin'); const captureFile = path.join(tempRoot, 'args.json'); - const taskFile = path.join(tempRoot, 'task.md'); + const taskFile = outsideTask ? `${tempRoot}-outside.md` : path.join(tempRoot, 'task.md'); const handoffFile = path.join(tempRoot, 'handoff.md'); const statusFile = path.join(tempRoot, 'status.md'); try { fs.mkdirSync(binDir); fs.writeFileSync(taskFile, 'Make a focused change.'); - fs.writeFileSync(path.join(binDir, 'codex'), `#!${process.execPath} + fs.writeFileSync(path.join(binDir, 'codex'), `#!/usr/bin/env node const fs = require('fs'); const args = process.argv.slice(2); fs.writeFileSync(process.env.ECC_TEST_CAPTURE, JSON.stringify(args)); @@ -57,7 +57,7 @@ process.exit(Number(process.env.ECC_TEST_EXIT)); }; if (policy === undefined) delete env.ECC_CODEX_APPROVAL_POLICY; else env.ECC_CODEX_APPROVAL_POLICY = policy; - const result = spawnSync('bash', [SCRIPT, taskFile, handoffFile, statusFile], { + const result = spawnSync('bash', [SCRIPT, relativeTask ? 'task.md' : taskFile, handoffFile, statusFile], { cwd: tempRoot, env, encoding: 'utf8', timeout: 10000 }); return { @@ -68,6 +68,7 @@ process.exit(Number(process.env.ECC_TEST_EXIT)); }; } finally { fs.rmSync(tempRoot, { recursive: true, force: true }); + if (outsideTask) fs.rmSync(taskFile, { force: true }); } } @@ -81,6 +82,20 @@ test('starts a worker with the current CLI grammar and configured model', () => assert.ok(result.workerStatus.includes('- State: completed')); }); +test('accepts a relative task file resolved inside the worktree', () => { + const result = runWorker({ relativeTask: true }); + assert.strictEqual(result.status, 0, result.stderr); + assert.ok(result.args); + assert.ok(result.workerStatus.includes('- State: completed')); +}); + +test('rejects a readable sibling task before invoking Codex', () => { + const result = runWorker({ outsideTask: true }); + assert.notStrictEqual(result.status, 0); + assert.strictEqual(result.args, null); + assert.ok(result.workerStatus.includes('task file outside worktree')); +}); + test('forwards the explicit supported approval policy', () => { const result = runWorker({ policy: 'on-request' }); assert.strictEqual(result.status, 0, result.stderr);