From fea905384040b290bc093cc339fd0e8cacc2ecbd Mon Sep 17 00:00:00 2001 From: t Date: Fri, 31 Jul 2026 10:09:29 +0800 Subject: [PATCH 1/2] =?UTF-8?q?=E7=B5=B1=E4=B8=80=E8=A7=A3=E9=99=A4?= =?UTF-8?q?=E6=A8=A1=E5=9E=8B=20runner=20=E6=AC=8A=E9=99=90=E9=99=90?= =?UTF-8?q?=E5=88=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/review.yml | 8 - .github/workflows/weekly-eval.yml | 1 - AGENTS.md | 6 +- README.md | 57 +++---- README.zh-TW.md | 34 ++-- action.yml | 15 +- scripts/deploy-ubuntu.test.mjs | 10 ++ src/shared/AGENTS.md | 6 +- src/shared/codex-ephemeral-home.test.ts | 5 - src/shared/codex-runners.test.ts | 215 +++--------------------- src/shared/codex.test.ts | 39 +++++ src/shared/codex.ts | 48 ++---- 12 files changed, 143 insertions(+), 301 deletions(-) diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index a113727..2fee725 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -221,8 +221,6 @@ jobs: NEEDLEFISH_MODEL_INPUT: ${{ inputs.model || github.event.inputs.model }} NEEDLEFISH_TIMEOUT_MS_INPUT: ${{ inputs.timeout_ms || github.event.inputs.timeout_ms }} CODEX_REASONING_EFFORT: ${{ inputs.codex_reasoning_effort || github.event.inputs.codex_reasoning_effort }} - # Deliberate on this disposable runner; the read-only toolset is probe-verified. - NEEDLEFISH_ALLOW_PI_RUNNER: "1" # ubuntu-needlefish routes pi through the local CLIProxyAPI (ccp); # credentials live in the proxy, provider registered in ~/.pi/agent/models.json. PI_PROVIDER: cliproxy @@ -234,12 +232,6 @@ jobs: echo "Missing $HOME/.local/bin/needlefish. Run scripts/deploy-ubuntu.sh on the self-hosted runner." >&2 exit 1 fi - # Grok 4.5 emits valid review JSON only in its unsandboxed mode; - # opt into that mode for the explicitly selected Grok lane only. - unset NEEDLEFISH_ALLOW_GROK_UNSANDBOXED - if [ "$NEEDLEFISH_RUNNER_INPUT" = "grok" ]; then - export NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1 - fi args=(--github --pr "${{ inputs.pr_number || github.event.inputs.pr_number || github.event.pull_request.number }}") if [ -z "$NEEDLEFISH_MODEL_INPUT" ]; then case "$NEEDLEFISH_RUNNER_INPUT" in diff --git a/.github/workflows/weekly-eval.yml b/.github/workflows/weekly-eval.yml index d2a28db..95b7c08 100644 --- a/.github/workflows/weekly-eval.yml +++ b/.github/workflows/weekly-eval.yml @@ -24,7 +24,6 @@ jobs: fetch-depth: 0 - name: Run full eval env: - NEEDLEFISH_ALLOW_PI_RUNNER: "1" # pi routes through the local CLIProxyAPI (ccp) on ubuntu-needlefish. PI_PROVIDER: cliproxy PI_BIN: /home/termtek/.npm-global/bin/pi diff --git a/AGENTS.md b/AGENTS.md index ccf46d3..cb5bda2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,7 +8,7 @@ Frank owns this. Keep replies and docs terse unless formal prose is requested. ## OVERVIEW -Needlefish is a strict TypeScript PR review CLI. It collects local or GitHub PR diffs, sends them through read-only model runners, prunes weak findings with a critic pass, and derives the final verdict deterministically from validated findings. +Needlefish is a strict TypeScript PR review CLI. It collects local or GitHub PR diffs, sends them through isolated model runners, prunes weak findings with a critic pass, and derives the final verdict deterministically from validated findings. ## STRUCTURE @@ -36,7 +36,7 @@ needlefish/ | Verdict rules | `src/core/verdict.ts` | Deterministic; do not let model prose decide pass/fail. | | Local review | `src/adapters/local.ts` | Writes `~/.cache/needlefish//last-review.json`. | | GitHub review | `src/adapters/github.ts` | Posts COMMENT review plus `Needlefish` check-run. | -| Runner invocation | `src/shared/codex.ts`, `src/shared/runner-process.ts` | Timeout, retry, sandbox, and runner env behavior live here. | +| Runner invocation | `src/shared/codex.ts`, `src/shared/runner-process.ts` | Timeout, retry, target isolation, and runner env behavior live here. | | Git/PR bundle shape | `src/shared/repo.ts`, `src/shared/schema.ts` | `agentsMd` is read from target repo root only. | | Prompt behavior | `prompts/*.md` | Must remain read-only and output JSON contracts exactly. | | Tests | `src/**/*.test.ts`, `scripts/test.mjs` | Node test runner, no Jest/Vitest. | @@ -77,7 +77,7 @@ needlefish/ - Do not implement `--fix` or multi-repo config unless explicitly requested. - `--recheck` forces a full re-review (bypassing the same-head dedupe in GitHub mode); it is never incremental verification. - Do not substitute global `AGENTS.md`, `~/.codex/*`, or CLI-injected files as target repo review policy. Only bundle `agentsMd` counts. -- Do not add new runner permissions that can mutate target repos; current model runners are read-only/sandboxed by design. +- Do not weaken the throwaway clone, token stripping, fixed `HEAD`, or post-run mutation checks that isolate unrestricted model runners from the original target repo. ## EVAL DISCIPLINE diff --git a/README.md b/README.md index 841a1c6..5380564 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ [繁體中文](README.zh-TW.md) -> Strict, local, read-only PR review that acts like a senior engineer — it +> Strict, local PR review that acts like a senior engineer — it > flags only real defects and stays silent on everything else.

@@ -26,9 +26,8 @@ duplicate behavior — never style. - **Deterministic verdicts.** The `pass` / `needs_human` / `changes_requested` verdict is derived from the surviving findings by fixed rules, never freehanded by the model. -- **Sandboxed, read-only runners.** Reviews run read-only by default; non-Codex - runners execute in a throwaway clean clone and are checked for tampering after - every model call. +- **Isolated review targets.** Reviews run against a throwaway clean clone and + are checked for tampering after every model call. - **Guarded evals.** Every prompt/pipeline change is measured against an 84-scenario harness with active anti-cheat guards before it ships (see [Benchmarks](#benchmarks)). @@ -311,10 +310,8 @@ gh workflow run review.yml -R frankekn/needlefish --ref main \ -f pr_number="$PR_NUMBER" -f runner=grok -f model=grok-4.5 ``` -The Grok 4.5 lane deliberately opts out of Grok's process-level plan mode so -the CLI returns valid review JSON. Use it only on a self-hosted runner you -control; the workflow sets `NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1` only when -`runner=grok` is selected. +All production model runners execute without their own process-level permission +restrictions. Use them only on a self-hosted runner you control. Because the caller pins `@main`, fixes to needlefish's `review.yml` propagate to every target repo automatically. The runner must have needlefish deployed at @@ -330,9 +327,9 @@ SHA so review jobs can fail before spending model tokens when a runner is stale. ```bash ssh termtek@ubuntu 'sh -s' < scripts/deploy-ubuntu.sh ``` - For a fleet, dispatch the same release SHA to all six selected runners and - verify each runner reports the same installed metadata before trusting the - fleet. + The current production fleet uses one shared x64 installation plus two + separate ARM installations. Deploy the same release SHA to all three + installations and verify their installed metadata before trusting the fleet. 3. Ensure the runner has `gh` and the selected model CLI on `PATH`. 4. On that runner, auth the selected CLI once. For Codex: ```bash @@ -439,32 +436,24 @@ When neither `--runner` nor `NEEDLEFISH_RUNNER` is set and none of `codex`, `claude`, or `opencode` can be found, Needlefish exits with install commands for those three CLIs instead of a stack trace. -Codex runs with `--ignore-user-config -c model_reasoning_effort="" -s -read-only`. `medium` is the default; set `CODEX_REASONING_EFFORT=high` to -restore the old default, or `xhigh` for the highest-effort mode. Claude Code runs with -`--permission-mode plan`, `--safe-mode`, and no session persistence. grok runs -with `--permission-mode plan` by default. The self-hosted GitHub Grok 4.5 lane -sets `NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1` for valid JSON output, which omits -that restraint and is why it is limited to an explicitly selected runner. -opencode -runs with `--pure` and never uses `--dangerously-skip-permissions`, but as of -this writing opencode's headless `run` mode has **no** documented read-only or -permission flag — a live probe confirmed it executes shell/tool calls with no -gate at all in that mode. Because of that gap, the `opencode` runner refuses to -start unless `NEEDLEFISH_ALLOW_OPENCODE_RUNNER=1` is set explicitly; set it only -if you've separately sandboxed the environment opencode runs in. If opencode's -CLI later ships a real permission/sandbox flag, this opt-in gate should replace -the flag instead of the env-var check. pi runs with `--no-session --mode text ---provider openai-codex --thinking `; pi advertises a `--tools -read,grep,find,ls` read-only mode, but that restriction is unverified under -headless invocation (same class as opencode), so the `pi` runner refuses to -start unless `NEEDLEFISH_ALLOW_PI_RUNNER=1` is set explicitly. ACP runs a +Codex runs with `--ignore-user-config --ignore-rules +--dangerously-bypass-approvals-and-sandbox` so its inspection commands are not +blocked by execpolicy rules, approval prompts, or the host sandbox. Needlefish +still runs it inside a throwaway clean clone, strips GitHub tokens, fixes the +expected `HEAD`, and rejects any worktree mutation. `medium` is the default; set +`CODEX_REASONING_EFFORT=high` to restore the old default, or `xhigh` for the +highest-effort mode. Claude Code runs with +`--dangerously-skip-permissions`, `--safe-mode`, and no session persistence. +Grok runs with `--always-approve --permission-mode bypassPermissions --no-plan +--sandbox off`. opencode runs with `--auto` in headless mode and an inline +`permission: "allow"` override for its global and build-agent permissions. pi +runs with `--no-session --mode text --provider openai-codex --thinking ` +and its default full toolset. ACP runs a JSON-RPC 2.0 Agent Client Protocol process over stdio from `NEEDLEFISH_ACP_BIN`; Needlefish sends `session/cancel` on timeout, then applies the same process-group kill path as the CLI runners. Closed PRs are skipped before diffing or model -invocation. Non-Codex runners execute inside a throwaway clean clone at the -review head commit; -needlefish checks that sandbox with +invocation. All CLI runners execute inside a throwaway clean clone at the review +head commit; needlefish checks that clone with `git status --porcelain --untracked-files=all --ignored=matching` and verifies `HEAD` did not move after each successful model call. diff --git a/README.zh-TW.md b/README.zh-TW.md index e207b18..52d2dba 100644 --- a/README.zh-TW.md +++ b/README.zh-TW.md @@ -23,8 +23,8 @@ Needlefish 會在 merge 前檢查 diff，只回報真正的缺陷：錯誤、回 修的就捨棄。沒有風格挑剔，沒有雜訊。 - **確定性 verdict。** `pass`／`needs_human`／`changes_requested` 由保留下來的 finding 依固定規則推導，不由模型自由決定。 -- **Sandboxed、唯讀 runner。** 審查預設唯讀；非 Codex runner 會在 throwaway - clean clone 中執行，並在每次模型呼叫後檢查是否遭竄改。 +- **隔離的審查目標。** 審查會在 throwaway clean clone 中執行，並在每次模型 + 呼叫後檢查是否遭竄改。 - **有防護的 evals。** 每次 prompt／pipeline 變更上線前，都會用 84 個情境的 harness（啟用 anti-cheat guards）量測（見 [Benchmarks](#benchmarks)）。 @@ -272,15 +272,16 @@ gh workflow run review.yml -R frankekn/needlefish --ref main \ -f pr_number="$PR_NUMBER" -f runner=grok -f model=grok-4.5 ``` -Grok 4.5 lane 會刻意停用 Grok 的 process-level plan mode，才能輸出有效 -JSON。workflow 只在明確選擇 `runner=grok` 時設定 -`NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1`，因此只能在你控制的 runner 上使用。 +所有 production model runner 都不套用各 runner 自己的 process-level 權限限制， +因此只能在你控制的 self-hosted runner 上使用。 1. 在目標 repo 註冊 self-hosted runner，並限制在自己控制的機器。 2. 在 runner 部署 Needlefish；`main` 的 push 會觸發 `needlefish-deploy`： ```bash ssh termtek@ubuntu 'sh -s' < scripts/deploy-ubuntu.sh ``` + 目前 production fleet 是一份共用 x64 安裝加兩份獨立 ARM 安裝。三份安裝都要 + 部署相同 release SHA，並確認 installed metadata 一致。 3. 確認 `gh` 與選定的模型 CLI 位於 `PATH`。 4. 以 runner service account 登入 CLI。Codex 例如： ```bash @@ -348,15 +349,20 @@ Fork PR 預設不會收到 secrets，workflow 會跳過它們；不要在不了 | Codex reasoning effort | `CODEX_REASONING_EFFORT` | `medium`（reusable workflow：`gpt-5.6-terra` 時為 `high`） | | timeout | `NEEDLEFISH_TIMEOUT_MS` | `600000` | -Codex 使用 `--ignore-user-config` 與唯讀 sandbox；Claude 使用 plan mode； -Grok 預設使用 `--permission-mode plan`，但 self-hosted Grok 4.5 lane 會 -明確停用它以取得有效 JSON。opencode 與 pi 的 headless 唯讀能力未完全驗證， -因此必須分別設定 `NEEDLEFISH_ALLOW_OPENCODE_RUNNER=1` 或 -`NEEDLEFISH_ALLOW_PI_RUNNER=1`。ACP 透過 `NEEDLEFISH_ACP_BIN` 使用 JSON-RPC -2.0 stdio process，timeout 時會先送 `session/cancel` 再終止 process group。 - -非 Codex runner 會在 review head 的 throwaway clean clone 中執行；每次成功 -呼叫後都會確認 sandbox 沒有未提交變更且 `HEAD` 沒有移動。 +Codex 使用 `--ignore-user-config --ignore-rules +--dangerously-bypass-approvals-and-sandbox`，避免檢查命令遭 execpolicy rule、 +approval prompt 或 host sandbox 阻擋。Needlefish 仍會把它放在 throwaway clean +clone 內執行、移除 GitHub token、固定預期 `HEAD`，並拒絕任何 worktree 變更。 +Claude 使用 `--dangerously-skip-permissions`；Grok 使用 `--always-approve +--permission-mode bypassPermissions --no-plan --sandbox off`；opencode 使用 +`--auto` headless mode，並以 inline `permission: "allow"` 覆寫 global 與 +build-agent 權限；pi 使用預設完整 toolset。這些 production runner 都不需要額外的 +unsandboxed opt-in。 +ACP 透過 `NEEDLEFISH_ACP_BIN` 使用 JSON-RPC 2.0 stdio process，timeout 時會先送 +`session/cancel` 再終止 process group。 + +所有 CLI runner 都會在 review head 的 throwaway clean clone 中執行；每次成功 +呼叫後都會確認 clone 沒有未提交變更且 `HEAD` 沒有移動。 ### Runner subprocess 環境 diff --git a/action.yml b/action.yml index e32a19d..c3ac466 100644 --- a/action.yml +++ b/action.yml @@ -1,6 +1,6 @@ name: Needlefish PR Review description: >- - Strict PR review agent. Reviews the pull request diff with a read-only model + Strict PR review agent. Reviews the pull request diff with an isolated model runner and posts a COMMENT review plus a Needlefish check-run. inputs: pr_number: @@ -47,16 +47,6 @@ runs: corepack enable corepack prepare "$(node -p 'require("./package.json").packageManager')" --activate pnpm install --frozen-lockfile - - name: Enable unprivileged user namespaces (Linux hosted runners) - shell: bash - run: | - # Ubuntu 23.10+ AppArmor blocks unprivileged user namespaces, which - # breaks the codex CLI sandbox (bwrap) on GitHub-hosted VMs and - # degrades reviews to diff-only. Best-effort: a locked-down runner - # without sudo just keeps the degraded mode. - if [ "$(uname -s)" = "Linux" ] && sudo -n true 2>/dev/null; then - sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 || true - fi - name: Install model runner CLI shell: bash env: @@ -107,9 +97,6 @@ runs: echo "No PR number: pass pr_number or trigger on pull_request events." >&2 exit 1 fi - # CI runs on a disposable VM, so the pi runner's unverified-sandbox gate - # (src/shared/codex.ts) is deliberately satisfied here. - if [ "$RUNNER_INPUT" = "pi" ]; then export NEEDLEFISH_ALLOW_PI_RUNNER=1; fi args=(--github --pr "$pr" --repo "$TARGET_REPO_PATH") if [ -n "$RUNNER_INPUT" ]; then args+=(--runner "$RUNNER_INPUT"); fi if [ -n "$MODEL_INPUT" ]; then args+=(--model "$MODEL_INPUT"); fi diff --git a/scripts/deploy-ubuntu.test.mjs b/scripts/deploy-ubuntu.test.mjs index 885d9ce..b14551d 100644 --- a/scripts/deploy-ubuntu.test.mjs +++ b/scripts/deploy-ubuntu.test.mjs @@ -7,3 +7,13 @@ test("deploy does not install an optional model runner", async () => { assert.doesNotMatch(script, /npm install -g @mariozechner\/pi/); }); + +test("hosted action does not require runner permission opt-ins", async () => { + const action = await readFile("action.yml", "utf8"); + + assert.doesNotMatch( + action, + /NEEDLEFISH_ALLOW_(?:OPENCODE_RUNNER|PI_RUNNER|GROK_UNSANDBOXED)/, + ); + assert.doesNotMatch(action, /apparmor_restrict_unprivileged_userns/); +}); diff --git a/src/shared/AGENTS.md b/src/shared/AGENTS.md index 76c4d34..4f10dc7 100644 --- a/src/shared/AGENTS.md +++ b/src/shared/AGENTS.md @@ -2,7 +2,7 @@ ## OVERVIEW -`src/shared/` is the high-blast-radius layer: git/gh subprocesses, model runner sandboxing, JSON normalization, schema contracts, rendering, and file classification. +`src/shared/` is the high-blast-radius layer: git/gh subprocesses, model runner isolation, JSON normalization, schema contracts, rendering, and file classification. ## WHERE TO LOOK @@ -19,13 +19,13 @@ - Treat subprocess output and model/GitHub JSON as untrusted. - Use typed unions from `runner.ts` and `schema.ts`; do not widen to strings. -- Preserve runner safety checks: clean sandbox, fixed `HEAD`, stripped GitHub tokens, read-only runner modes. +- Preserve target isolation checks: clean throwaway clone, fixed `HEAD`, stripped GitHub tokens, and post-run dirty-worktree rejection. - Keep process timeout behavior durable. A child that traps `SIGTERM` must not hang the review path. - For test repos, use temp dirs and the fixture helpers in `codex-runner-test-fixtures.ts`. ## ANTI-PATTERNS -- Do not add a new runner without sandbox, timeout, token-stripping, and dirty-worktree tests. +- Do not add a new runner without target isolation, timeout, token-stripping, and dirty-worktree tests. - Do not read target policy from anywhere except target root `AGENTS.md` or `git show :AGENTS.md`. - Do not swallow `spawn`/buffer/timeout failures into a passing review. - Do not weaken schema normalization to accept malformed findings. diff --git a/src/shared/codex-ephemeral-home.test.ts b/src/shared/codex-ephemeral-home.test.ts index 263785e..5479be6 100644 --- a/src/shared/codex-ephemeral-home.test.ts +++ b/src/shared/codex-ephemeral-home.test.ts @@ -490,7 +490,6 @@ test("runCodex ephemeral HOME: opencode accepts explicitly passed provider API k const fakeHome = mkdtempSync(path.join(os.tmpdir(), "needlefish-fakehome-")); const previous = { bin: process.env.OPENCODE_BIN, - allowOpenCode: process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER, ephemeral: process.env.NEEDLEFISH_EPHEMERAL_HOME, retry: process.env.NEEDLEFISH_NO_RETRY, home: process.env.HOME, @@ -504,7 +503,6 @@ test("runCodex ephemeral HOME: opencode accepts explicitly passed provider API k t.after(() => { for (const [name, value] of Object.entries({ OPENCODE_BIN: previous.bin, - NEEDLEFISH_ALLOW_OPENCODE_RUNNER: previous.allowOpenCode, NEEDLEFISH_EPHEMERAL_HOME: previous.ephemeral, NEEDLEFISH_NO_RETRY: previous.retry, HOME: previous.home, @@ -532,7 +530,6 @@ test("runCodex ephemeral HOME: opencode accepts explicitly passed provider API k ); chmodSync(bin, 0o755); process.env.OPENCODE_BIN = bin; - process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = "1"; process.env.NEEDLEFISH_EPHEMERAL_HOME = "1"; process.env.NEEDLEFISH_NO_RETRY = "1"; process.env.HOME = fakeHome; @@ -616,7 +613,6 @@ test("runCodex ephemeral HOME: opencode stages custom XDG auth roots into dispos const previous = Object.fromEntries( [ "OPENCODE_BIN", - "NEEDLEFISH_ALLOW_OPENCODE_RUNNER", "NEEDLEFISH_EPHEMERAL_HOME", "NEEDLEFISH_NO_RETRY", "HOME", @@ -651,7 +647,6 @@ test("runCodex ephemeral HOME: opencode stages custom XDG auth roots into dispos ); chmodSync(bin, 0o755); process.env.OPENCODE_BIN = bin; - process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = "1"; process.env.NEEDLEFISH_EPHEMERAL_HOME = "1"; process.env.NEEDLEFISH_NO_RETRY = "1"; process.env.HOME = fakeHome; diff --git a/src/shared/codex-runners.test.ts b/src/shared/codex-runners.test.ts index cd6332b..2c8fad6 100644 --- a/src/shared/codex-runners.test.ts +++ b/src/shared/codex-runners.test.ts @@ -19,7 +19,7 @@ import { readStringArray, } from "./codex-runner-test-fixtures"; -test("runCodex invokes claude in non-interactive plan mode", async (t) => { +test("runCodex invokes claude without permission restrictions", async (t) => { const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); const repo = initRepo(tmp); const targetHeadSha = headSha(repo); @@ -64,12 +64,11 @@ test("runCodex invokes claude in non-interactive plan mode", async (t) => { const args = readStringArray(argsPath); assert.equal(output, '{"ok":true}'); - assert.deepEqual(args.slice(0, 7), [ + assert.deepEqual(args.slice(0, 6), [ "--print", "--output-format", "text", - "--permission-mode", - "plan", + "--dangerously-skip-permissions", "--safe-mode", "--no-session-persistence", ]); @@ -85,19 +84,16 @@ test("runCodex extracts opencode json text output", async (t) => { const argsPath = path.join(tmp, "args.json"); const inputPath = path.join(tmp, "prompt-copy.txt"); const stdinPath = path.join(tmp, "stdin.txt"); + const configPath = path.join(tmp, "config.txt"); const previous = { bin: process.env.OPENCODE_BIN, runner: process.env.NEEDLEFISH_RUNNER, - allowOpenCode: process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER, }; t.after(() => { if (previous.bin === undefined) delete process.env.OPENCODE_BIN; else process.env.OPENCODE_BIN = previous.bin; if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowOpenCode === undefined) - delete process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER; - else process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = previous.allowOpenCode; rmSync(tmp, { recursive: true, force: true }); }); writeFileSync( @@ -107,6 +103,7 @@ test("runCodex extracts opencode json text output", async (t) => { "const fs = require('node:fs');", "const args = process.argv.slice(2);", `fs.writeFileSync(${JSON.stringify(argsPath)}, JSON.stringify(args));`, + `fs.writeFileSync(${JSON.stringify(configPath)}, process.env.OPENCODE_CONFIG_CONTENT || '');`, `fs.writeFileSync(${JSON.stringify(stdinPath)}, fs.readFileSync(0, 'utf8'));`, "const promptFile = args[args.indexOf('--file') + 1];", `fs.writeFileSync(${JSON.stringify(inputPath)}, fs.readFileSync(promptFile, 'utf8'));`, @@ -117,7 +114,6 @@ test("runCodex extracts opencode json text output", async (t) => { chmodSync(bin, 0o755); process.env.OPENCODE_BIN = bin; process.env.NEEDLEFISH_RUNNER = "opencode"; - process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = "1"; const output = await runCodex("prompt", { repoPath: repo, @@ -127,15 +123,17 @@ test("runCodex extracts opencode json text output", async (t) => { const args = readStringArray(argsPath); assert.equal(output, '{"ok":true}'); - assert.deepEqual(args.slice(0, 5), [ + assert.equal(args.includes("--auto"), true); + assert.deepEqual(args.slice(0, 6), [ "run", "--format", "json", "--pure", + "--auto", "--dir", ]); - assert.notEqual(args[5], repo); - assert.equal(args[6], "--file"); + assert.notEqual(args[6], repo); + assert.equal(args[7], "--file"); assert.equal( args.at(-1), "Use the attached prompt file as your complete instruction.", @@ -143,6 +141,10 @@ test("runCodex extracts opencode json text output", async (t) => { assert.equal(args.includes("prompt"), false); assert.equal(readFileSync(inputPath, "utf8"), "prompt"); assert.equal(readFileSync(stdinPath, "utf8"), ""); + assert.deepEqual(JSON.parse(readFileSync(configPath, "utf8")), { + permission: "allow", + agent: { build: { permission: "allow" } }, + }); }); test("runCodex rejects non-codex runners that dirty the target repo", async (t) => { @@ -315,16 +317,12 @@ test("runCodex reports opencode exit errors before parsing stdout", async (t) => const previous = { bin: process.env.OPENCODE_BIN, runner: process.env.NEEDLEFISH_RUNNER, - allowOpenCode: process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER, }; t.after(() => { if (previous.bin === undefined) delete process.env.OPENCODE_BIN; else process.env.OPENCODE_BIN = previous.bin; if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowOpenCode === undefined) - delete process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER; - else process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = previous.allowOpenCode; rmSync(tmp, { recursive: true, force: true }); }); writeFileSync( @@ -339,7 +337,6 @@ test("runCodex reports opencode exit errors before parsing stdout", async (t) => chmodSync(bin, 0o755); process.env.OPENCODE_BIN = bin; process.env.NEEDLEFISH_RUNNER = "opencode"; - process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = "1"; let caught: unknown; try { @@ -366,7 +363,7 @@ test("runCodex reports opencode exit errors before parsing stdout", async (t) => assert.equal(Object.keys(err).includes("rawOutput"), false); }); -test("runCodex passes grok plan permission mode when unset or set to 0", async () => { +test("runCodex omits grok permission restrictions by default", async () => { const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); const repo = initRepo(tmp); const bin = path.join(tmp, "grok-bin.js"); @@ -374,7 +371,6 @@ test("runCodex passes grok plan permission mode when unset or set to 0", async ( const previous = { bin: process.env.GROK_BIN, runner: process.env.NEEDLEFISH_RUNNER, - allowUnsandboxed: process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED, }; try { writeFileSync( @@ -389,61 +385,6 @@ test("runCodex passes grok plan permission mode when unset or set to 0", async ( chmodSync(bin, 0o755); process.env.GROK_BIN = bin; process.env.NEEDLEFISH_RUNNER = "grok"; - for (const allowUnsandboxed of [undefined, "0"] as const) { - if (allowUnsandboxed === undefined) - delete process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED; - else process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED = allowUnsandboxed; - - const output = await runCodex("prompt", { - repoPath: repo, - targetHeadSha: headSha(repo), - timeoutMs: 1000, - }); - const args = readStringArray(argsPath); - - assert.equal(output, '{"ok":true}'); - const flagIndex = args.indexOf("--permission-mode"); - assert.notEqual(flagIndex, -1); - assert.equal(args[flagIndex + 1], "plan"); - } - } finally { - if (previous.bin === undefined) delete process.env.GROK_BIN; - else process.env.GROK_BIN = previous.bin; - if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; - else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowUnsandboxed === undefined) - delete process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED; - else - process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED = previous.allowUnsandboxed; - rmSync(tmp, { recursive: true, force: true }); - } -}); - -test("runCodex omits grok permission mode when unsandboxed env is set", async () => { - const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); - const repo = initRepo(tmp); - const bin = path.join(tmp, "grok-bin.js"); - const argsPath = path.join(tmp, "args.json"); - const previous = { - bin: process.env.GROK_BIN, - runner: process.env.NEEDLEFISH_RUNNER, - allowUnsandboxed: process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED, - }; - try { - writeFileSync( - bin, - [ - "#!/usr/bin/env node", - "const fs = require('node:fs');", - `fs.writeFileSync(${JSON.stringify(argsPath)}, JSON.stringify(process.argv.slice(2)));`, - "process.stdout.write('{\"ok\":true}');", - ].join("\n"), - ); - chmodSync(bin, 0o755); - process.env.GROK_BIN = bin; - process.env.NEEDLEFISH_RUNNER = "grok"; - process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED = "1"; - const output = await runCodex("prompt", { repoPath: repo, targetHeadSha: headSha(repo), @@ -452,7 +393,13 @@ test("runCodex omits grok permission mode when unsandboxed env is set", async () const args = readStringArray(argsPath); assert.equal(output, '{"ok":true}'); - assert.equal(args.includes("--permission-mode"), false); + assert.equal(args.includes("--always-approve"), true); + assert.equal( + args[args.indexOf("--permission-mode") + 1], + "bypassPermissions", + ); + assert.equal(args.includes("--no-plan"), true); + assert.equal(args[args.indexOf("--sandbox") + 1], "off"); assert.ok(args.includes("--output-format")); assert.equal(args[args.indexOf("--output-format") + 1], "plain"); assert.ok(args.includes("--prompt-file")); @@ -461,60 +408,11 @@ test("runCodex omits grok permission mode when unsandboxed env is set", async () else process.env.GROK_BIN = previous.bin; if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowUnsandboxed === undefined) - delete process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED; - else - process.env.NEEDLEFISH_ALLOW_GROK_UNSANDBOXED = previous.allowUnsandboxed; - rmSync(tmp, { recursive: true, force: true }); - } -}); - -test("runCodex refuses the opencode runner without explicit opt-in", async (t) => { - const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); - const repo = initRepo(tmp); - const bin = path.join(tmp, "opencode-bin.js"); - const previous = { - bin: process.env.OPENCODE_BIN, - runner: process.env.NEEDLEFISH_RUNNER, - allowOpenCode: process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER, - }; - t.after(() => { - if (previous.bin === undefined) delete process.env.OPENCODE_BIN; - else process.env.OPENCODE_BIN = previous.bin; - if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; - else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowOpenCode === undefined) - delete process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER; - else process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = previous.allowOpenCode; rmSync(tmp, { recursive: true, force: true }); - }); - writeFileSync( - bin, - ["#!/usr/bin/env node", "process.stdout.write('{\"ok\":true}');"].join( - "\n", - ), - ); - chmodSync(bin, 0o755); - process.env.OPENCODE_BIN = bin; - process.env.NEEDLEFISH_RUNNER = "opencode"; - for (const allowOpenCode of [undefined, "0"] as const) { - if (allowOpenCode === undefined) - delete process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER; - else process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = allowOpenCode; - - await assert.rejects( - () => - runCodex("prompt", { - repoPath: repo, - targetHeadSha: headSha(repo), - timeoutMs: 1000, - }), - /NEEDLEFISH_ALLOW_OPENCODE_RUNNER/, - ); } }); -test("runCodex allows opencode when explicitly opted in", async (t) => { +test("runCodex invokes opencode without an opt-in gate", async (t) => { const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); const repo = initRepo(tmp); const bin = path.join(tmp, "opencode-bin.js"); @@ -524,16 +422,12 @@ test("runCodex allows opencode when explicitly opted in", async (t) => { const previous = { bin: process.env.OPENCODE_BIN, runner: process.env.NEEDLEFISH_RUNNER, - allowOpenCode: process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER, }; t.after(() => { if (previous.bin === undefined) delete process.env.OPENCODE_BIN; else process.env.OPENCODE_BIN = previous.bin; if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowOpenCode === undefined) - delete process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER; - else process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = previous.allowOpenCode; rmSync(tmp, { recursive: true, force: true }); }); writeFileSync( @@ -553,7 +447,6 @@ test("runCodex allows opencode when explicitly opted in", async (t) => { chmodSync(bin, 0o755); process.env.OPENCODE_BIN = bin; process.env.NEEDLEFISH_RUNNER = "opencode"; - process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = "1"; const output = await runCodex("prompt", { repoPath: repo, @@ -563,11 +456,13 @@ test("runCodex allows opencode when explicitly opted in", async (t) => { const args = readStringArray(argsPath); assert.equal(output, '{"ok":true}'); - assert.deepEqual(args.slice(0, 5), [ + assert.equal(args.includes("--auto"), true); + assert.deepEqual(args.slice(0, 6), [ "run", "--format", "json", "--pure", + "--auto", "--dir", ]); assert.equal(readFileSync(inputPath, "utf8"), "prompt"); @@ -583,7 +478,6 @@ test("runCodex invokes pi with default provider/model/thinking flags and the pro const previous = { bin: process.env.PI_BIN, runner: process.env.NEEDLEFISH_RUNNER, - allowPi: process.env.NEEDLEFISH_ALLOW_PI_RUNNER, }; try { writeFileSync( @@ -599,7 +493,6 @@ test("runCodex invokes pi with default provider/model/thinking flags and the pro chmodSync(bin, 0o755); process.env.PI_BIN = bin; process.env.NEEDLEFISH_RUNNER = "pi"; - process.env.NEEDLEFISH_ALLOW_PI_RUNNER = "1"; const output = await runCodex("prompt", { repoPath: repo, @@ -620,8 +513,6 @@ test("runCodex invokes pi with default provider/model/thinking flags and the pro "gpt-5.6-sol", "--thinking", "medium", - "--tools", - "read,grep,find,ls", ]); assert.equal(readFileSync(stdinPath, "utf8"), "prompt"); } finally { @@ -629,55 +520,6 @@ test("runCodex invokes pi with default provider/model/thinking flags and the pro else process.env.PI_BIN = previous.bin; if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowPi === undefined) - delete process.env.NEEDLEFISH_ALLOW_PI_RUNNER; - else process.env.NEEDLEFISH_ALLOW_PI_RUNNER = previous.allowPi; - rmSync(tmp, { recursive: true, force: true }); - } -}); - -test("runCodex refuses the pi runner without explicit opt-in", async () => { - const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); - const repo = initRepo(tmp); - const bin = path.join(tmp, "pi-bin.js"); - const previous = { - bin: process.env.PI_BIN, - runner: process.env.NEEDLEFISH_RUNNER, - allowPi: process.env.NEEDLEFISH_ALLOW_PI_RUNNER, - }; - try { - writeFileSync( - bin, - ["#!/usr/bin/env node", "process.stdout.write('{\"ok\":true}');"].join( - "\n", - ), - ); - chmodSync(bin, 0o755); - process.env.PI_BIN = bin; - process.env.NEEDLEFISH_RUNNER = "pi"; - for (const allowPi of [undefined, "0"] as const) { - if (allowPi === undefined) - delete process.env.NEEDLEFISH_ALLOW_PI_RUNNER; - else process.env.NEEDLEFISH_ALLOW_PI_RUNNER = allowPi; - - await assert.rejects( - () => - runCodex("prompt", { - repoPath: repo, - targetHeadSha: headSha(repo), - timeoutMs: 1000, - }), - /NEEDLEFISH_ALLOW_PI_RUNNER/, - ); - } - } finally { - if (previous.bin === undefined) delete process.env.PI_BIN; - else process.env.PI_BIN = previous.bin; - if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; - else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowPi === undefined) - delete process.env.NEEDLEFISH_ALLOW_PI_RUNNER; - else process.env.NEEDLEFISH_ALLOW_PI_RUNNER = previous.allowPi; rmSync(tmp, { recursive: true, force: true }); } }); @@ -687,12 +529,10 @@ test("runCodex rejects an invalid pi thinking effort", async () => { const repo = initRepo(tmp); const previous = { runner: process.env.NEEDLEFISH_RUNNER, - allowPi: process.env.NEEDLEFISH_ALLOW_PI_RUNNER, noRetry: process.env.NEEDLEFISH_NO_RETRY, }; try { process.env.NEEDLEFISH_RUNNER = "pi"; - process.env.NEEDLEFISH_ALLOW_PI_RUNNER = "1"; process.env.NEEDLEFISH_NO_RETRY = "1"; await assert.rejects( @@ -708,9 +548,6 @@ test("runCodex rejects an invalid pi thinking effort", async () => { } finally { if (previous.runner === undefined) delete process.env.NEEDLEFISH_RUNNER; else process.env.NEEDLEFISH_RUNNER = previous.runner; - if (previous.allowPi === undefined) - delete process.env.NEEDLEFISH_ALLOW_PI_RUNNER; - else process.env.NEEDLEFISH_ALLOW_PI_RUNNER = previous.allowPi; if (previous.noRetry === undefined) delete process.env.NEEDLEFISH_NO_RETRY; else process.env.NEEDLEFISH_NO_RETRY = previous.noRetry; rmSync(tmp, { recursive: true, force: true }); diff --git a/src/shared/codex.test.ts b/src/shared/codex.test.ts index 9f152f2..d49411f 100644 --- a/src/shared/codex.test.ts +++ b/src/shared/codex.test.ts @@ -20,6 +20,45 @@ test("extractJson rejects output without a JSON object", () => { assert.throws(() => extractJson(text), /no JSON object found/); }); +test("runCodex invokes Codex without approval or sandbox restrictions", async (t) => { + const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); + const repo = initRepo(tmp); + const bin = path.join(tmp, "codex-bin.js"); + const argsPath = path.join(tmp, "args.json"); + const previous = process.env.CODEX_BIN; + t.after(() => { + if (previous === undefined) delete process.env.CODEX_BIN; + else process.env.CODEX_BIN = previous; + rmSync(tmp, { recursive: true, force: true }); + }); + writeFileSync( + bin, + [ + "#!/usr/bin/env node", + "const fs = require('node:fs');", + `fs.writeFileSync(${JSON.stringify(argsPath)}, JSON.stringify(process.argv.slice(2)));`, + "const out = process.argv[process.argv.indexOf('--output-last-message') + 1];", + "fs.writeFileSync(out, '{\"ok\":true}');", + ].join("\n") + ); + chmodSync(bin, 0o755); + process.env.CODEX_BIN = bin; + + await runCodex("prompt", { + repoPath: repo, + runner: "codex", + targetHeadSha: headSha(repo), + timeoutMs: 1000, + }); + + const args = JSON.parse(readFileSync(argsPath, "utf8")) as string[]; + assert.equal(args.includes("--dangerously-bypass-approvals-and-sandbox"), true); + assert.equal(args.includes("--ignore-rules"), true); + assert.equal(args.includes("-s"), false); + assert.equal(args.includes("--sandbox"), false); + assert.equal(args.some((arg) => arg.startsWith("--sandbox=")), false); +}); + test("runCodex retry backoff yields the event loop", async (t) => { const tmp = mkdtempSync(path.join(os.tmpdir(), "needlefish-test-")); const repo = initRepo(tmp); diff --git a/src/shared/codex.ts b/src/shared/codex.ts index 0b5eb2b..83af5ea 100644 --- a/src/shared/codex.ts +++ b/src/shared/codex.ts @@ -442,23 +442,6 @@ export async function runCodex( opts: CodexOptions, ): Promise { const runner = resolveRunner(opts); - if (runner === "opencode" && !envFlagOn("NEEDLEFISH_ALLOW_OPENCODE_RUNNER")) { - throw new Error( - "The opencode runner has no verified process-level sandbox restraint in headless mode " + - "(it executes tool calls with no permission gate) and must be explicitly enabled via " + - "NEEDLEFISH_ALLOW_OPENCODE_RUNNER=1.", - ); - } - // Fail closed (single live probe is not a guarantee): on 2026-07-10 pi with - // `--tools read,grep,find,ls` reported "no write, shell, bash, or edit tool is - // available" and created no file when instructed to; keep the env gate anyway. - if (runner === "pi" && !envFlagOn("NEEDLEFISH_ALLOW_PI_RUNNER")) { - throw new Error( - "The pi runner has no verified process-level sandbox restraint in headless mode " + - "(it executes tool calls with no permission gate) and must be explicitly enabled via " + - "NEEDLEFISH_ALLOW_PI_RUNNER=1.", - ); - } const maxAttempts = envFlagOn("NEEDLEFISH_NO_RETRY") ? 1 : 2; const startedAt = Date.now(); let attempts = 0; @@ -721,8 +704,8 @@ async function runCodexCli( "--ignore-user-config", "-c", `model_reasoning_effort="${reasoningEffort}"`, - "-s", - "read-only", + "--dangerously-bypass-approvals-and-sandbox", + "--ignore-rules", "--skip-git-repo-check", "--output-last-message", lastMsg, @@ -761,8 +744,7 @@ async function runClaude(invocation: RunnerInvocation): Promise { "--print", "--output-format", "text", - "--permission-mode", - "plan", + "--dangerously-skip-permissions", "--safe-mode", "--no-session-persistence", ]; @@ -784,6 +766,10 @@ async function runClaude(invocation: RunnerInvocation): Promise { async function runOpenCode( invocation: RunnerInvocation, ): Promise { + const unrestrictedConfig = JSON.stringify({ + permission: "allow", + agent: { build: { permission: "allow" } }, + }); const promptPath = path.join(invocation.tmp, "prompt.md"); writeFileSync(promptPath, invocation.prompt, { mode: 0o600 }); const args = [ @@ -791,6 +777,7 @@ async function runOpenCode( "--format", "json", "--pure", + "--auto", "--dir", invocation.repoPath, ]; @@ -806,7 +793,10 @@ async function runOpenCode( stdin: "", repoPath: invocation.repoPath, timeoutMs: invocation.timeoutMs, - env: invocation.env, + env: { + ...invocation.env, + OPENCODE_CONFIG_CONTENT: unrestrictedConfig, + }, }); return { res, out: res.stdout ?? "" }; } @@ -821,13 +811,13 @@ async function runGrok(invocation: RunnerInvocation): Promise { promptPath, "--output-format", "plain", + "--always-approve", + "--permission-mode", + "bypassPermissions", + "--no-plan", + "--sandbox", + "off", ]; - // Fail closed in plan mode: on 2026-07-09 grok-4.5 produced 0/8 valid review - // JSON in plan mode, but it was write-restrained. The env opt-in unlocks the - // working unsandboxed mode; grok CLI --sandbox read-only and --disallowed-tools - // were verified ineffective at preventing writes that day. - if (!envFlagOn("NEEDLEFISH_ALLOW_GROK_UNSANDBOXED")) - args.push("--permission-mode", "plan"); if (invocation.reasoningEffort) args.push("--reasoning-effort", invocation.reasoningEffort); const res = await spawnRunnerProcess({ @@ -881,8 +871,6 @@ async function runPi(invocation: RunnerInvocation): Promise { invocation.model ?? "gpt-5.6-sol", "--thinking", thinking, - "--tools", - "read,grep,find,ls", ]; // Prompt goes on stdin, not argv: a review bundle can exceed OS ARG_MAX as a // positional arg. Verified 2026-07-10: `pi -p --no-session --mode text` with From 719445a9a42bc82f1f3553ed941d14626f0056fd Mon Sep 17 00:00:00 2001 From: t Date: Fri, 31 Jul 2026 10:15:11 +0800 Subject: [PATCH 2/2] =?UTF-8?q?=E8=A8=98=E9=8C=84=20runner=20=E4=B8=BB?= =?UTF-8?q?=E6=A9=9F=E4=BF=A1=E4=BB=BB=E9=82=8A=E7=95=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index cb5bda2..a472cab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,7 @@ needlefish/ - TypeScript is strict/no-emit with `moduleResolution: "bundler"`. - Use `unknown` at JSON/model/GitHub boundaries, then validate or narrow. - All NEEDLEFISH_* boolean flags go through `envFlagOn` in `src/shared/env.ts`; only `"1"` is on. +- Production model CLIs intentionally run without process-level permission or sandbox restrictions on trusted self-hosted runners. Do not restore those restrictions; preserve the throwaway target clone and post-run integrity checks instead. - Keep tests beside the code path as `src/**/*.test.ts`. - Use Node built-ins (`node:test`, `assert/strict`, `spawnSync`, temp dirs) before adding dependencies. - Stub external CLIs in tests with temp scripts and env vars.