diff --git a/README.md b/README.md index e115da6..cad44d4 100644 --- a/README.md +++ b/README.md @@ -238,14 +238,25 @@ Config from `bin/reviewer-config.example.json` to `~/.config/reviewer/config.jso Secrets in `~/.config/reviewer/env`, never here: ```sh -REVIEWER_GITEA_TOKEN=... # read-only -REVIEWER_GITHUB_TOKEN=... # read-only -REVIEWER_GITEA_SECRET=... # webhook HMAC +REVIEWER_GITEA_TOKEN=... # read-only +REVIEWER_GITHUB_TOKEN=... # read-only +REVIEWER_GITEA_REVIEW_TOKEN=... # optional, write:issue — handed to review sessions +REVIEWER_GITEA_SECRET=... # webhook HMAC REVIEWER_GITHUB_SECRET=... ``` -The daemon's tokens are read-only — it never writes to a forge, which is also -why it doesn't mark notifications read. +The daemon's own tokens are read-only — it never writes to a forge, which is +also why it doesn't mark notifications read. + +A review session is a different case: it has to post its findings, and the +sandbox denies it `~/.env.claude`, where `$GITEA_TOKEN` normally comes from. +Name a write-capable variable in a forge's `reviewTokenEnv` and the daemon +passes its value into the session as `$GITEA_TOKEN` (`$GH_TOKEN` on GitHub) — +through the generated Claude settings file (`env`) or codex profile +(`shell_environment_policy.set`), both written 0600. Leave `reviewTokenEnv` +out and nothing is injected; the session falls back to the forge's credential +helper, which is what it did before. Scope it to commenting: on Gitea that is +`write:issue`, and nothing else. Claude review sessions need `bubblewrap` and `socat` on the box, or the sandbox cannot start and the session refuses to run (`failIfUnavailable`). That is diff --git a/bin/reviewer-config.example.json b/bin/reviewer-config.example.json index d006123..7c70299 100644 --- a/bin/reviewer-config.example.json +++ b/bin/reviewer-config.example.json @@ -14,6 +14,7 @@ "gitea": { "api": "https://git.example.com/api/v1", "tokenEnv": "REVIEWER_GITEA_TOKEN", + "reviewTokenEnv": "REVIEWER_GITEA_REVIEW_TOKEN", "webhookSecretEnv": "REVIEWER_GITEA_SECRET", "self": ["you", "you-bot"] }, diff --git a/bin/reviewer-poll.ts b/bin/reviewer-poll.ts index a95efb8..07f2588 100644 --- a/bin/reviewer-poll.ts +++ b/bin/reviewer-poll.ts @@ -38,7 +38,15 @@ type Config = { pathRoots?: string[]; // scanned one level deep to find clones by origin URL reviewers?: Reviewer[]; // rotation pool for review sessions ledger?: string; // append-only record of which reviewer got which PR - forges: Record; + forges: Record; repos: RepoConfig[]; }; @@ -520,9 +528,10 @@ const gitWorktrees = (mainRepo: string) => join(mainRepo, ".git/worktrees"); // tools are denied outright, and the seen and findings files are written with // a shell redirect instead (Claude Code treats .git as a protected path that // no allow rule opens, so an Edit rule there would be dead config). -function writeClaudeSettings(mainRepo: string): string { +function writeClaudeSettings(mainRepo: string, env: Record): string { const wt = gitWorktrees(mainRepo); const settings = { + env, permissions: { defaultMode: "dontAsk", allow: ["Read(//**)"], @@ -541,8 +550,9 @@ function writeClaudeSettings(mainRepo: string): string { }, }; const path = join(REVIEW_SETTINGS_DIR, `${slug(mainRepo)}.json`); - mkdirSync(REVIEW_SETTINGS_DIR, { recursive: true }); - writeFileSync(path, JSON.stringify(settings, null, 2)); + mkdirSync(REVIEW_SETTINGS_DIR, { recursive: true, mode: 0o700 }); + // 0600: this file now carries the session's forge token. + writeFileSync(path, JSON.stringify(settings, null, 2), { mode: 0o600 }); return path; } @@ -552,22 +562,43 @@ function writeClaudeSettings(mainRepo: string): string { // question without the prompt and without the trust. It goes in a profile // file because the key is a quoted path, and -c would lose the quotes on the // way through aoe's argument string. -function writeCodexProfile(mainRepo: string): string { +function writeCodexProfile(mainRepo: string, env: Record): string { const name = `review-${slug(mainRepo)}`; + // `set` is applied after codex's default excludes, which drop every variable + // whose name looks like a credential -- so a token named here survives. + const injected = Object.entries(env) + .map(([k, v]) => `${k} = ${JSON.stringify(v)}`) + .join(", "); mkdirSync(CODEX_HOME, { recursive: true }); writeFileSync(join(CODEX_HOME, `${name}.config.toml`), `# generated by reviewer-poll.ts -- PR review session for ${mainRepo}\n` + `[projects."${mainRepo}"]\ntrust_level = "untrusted"\n\n` + - `[sandbox_workspace_write]\nnetwork_access = true\n`); + `[sandbox_workspace_write]\nnetwork_access = true\n` + + (injected ? `\n[shell_environment_policy]\nset = { ${injected} }\n` : ""), + { mode: 0o600 }); return name; } +// The token a review session posts findings with, under the name the skills +// already look for. ~/.env.claude, where that name normally comes from, is on +// the sandbox deny list, so a session that is not handed one has none. +function reviewToken(forge: string): Record { + const name = config.forges[forge]?.reviewTokenEnv; + if (!name) return {}; + const value = process.env[name]; + if (!value) { + log(`${name} unset: review sessions on ${forge} get no injected token`); + return {}; + } + return { [forge === "github" ? "GH_TOKEN" : "GITEA_TOKEN"]: value }; +} + // Every arg here has to survive being space-joined into one --extra-args // string, so no quotes and no brackets: paths only. -function sandboxArgs(tool: string, mainRepo: string): string[] { - if (tool === "claude") return ["--settings", writeClaudeSettings(mainRepo)]; +function sandboxArgs(tool: string, mainRepo: string, env: Record): string[] { + if (tool === "claude") return ["--settings", writeClaudeSettings(mainRepo, env)]; if (tool === "codex") { - return ["--profile", writeCodexProfile(mainRepo), + return ["--profile", writeCodexProfile(mainRepo, env), "--sandbox", "workspace-write", "--ask-for-approval", "never", "--add-dir", gitWorktrees(mainRepo)]; } @@ -609,7 +640,8 @@ async function createReview(pr: Pr, authorTool?: string): Promise { await git(pr.cfg.path!, ["fetch", "origin", `+refs/pull/${pr.number}/head:${local}`]); const args = ["-p", profile, "add", pr.cfg.path!, "--title", t, "--group", group(pr), "--worktree", local, "--cmd", reviewer.tool]; - const extra = [...sandboxArgs(reviewer.tool, pr.cfg.path!), ...(reviewer.args ?? [])]; + const extra = [...sandboxArgs(reviewer.tool, pr.cfg.path!, reviewToken(pr.forge)), + ...(reviewer.args ?? [])]; if (extra.length) args.push("--extra-args", extra.join(" ")); await aoe(args); clearYolo(profile, t); diff --git a/skills/pr-common/COMMON.md b/skills/pr-common/COMMON.md index 68d2a00..e5052c9 100644 --- a/skills/pr-common/COMMON.md +++ b/skills/pr-common/COMMON.md @@ -137,6 +137,12 @@ GitHub uses `gh`. Gitea uses plain REST against `Authorization: token` header — never in a URL, never in a commit message. `source ~/.env.claude` if the token isn't in the environment. +A review session can't do that: reviews run sandboxed with every +credential file on the deny list. The daemon puts the token in the +environment there instead. If `$GITEA_TOKEN` is empty anyway, the one +fallback is the host's git credential helper — `printf +'protocol=https\nhost=\n\n' | git credential fill`. + `$GITEA_TOKEN` is the name, and the only one. `~/.config/reviewer/config.json` names a different variable in its `tokenEnv` field: that is the daemon's own read-only token, it is loaded into the daemon process and nothing else, and in