feat(pr-daemon): hand review sessions their forge token
A sandboxed review session can't read ~/.env.claude, so $GITEA_TOKEN was never set and reviewers improvised — one of them fell back to `git credential fill`, another skipped posting. A forge can now name a second, write-capable variable in reviewTokenEnv. The daemon passes its value into the session as $GITEA_TOKEN ($GH_TOKEN on GitHub), through the generated Claude settings file or the codex profile's shell_environment_policy, both now written 0600. tokenEnv stays read-only and unchanged. Nothing is injected when reviewTokenEnv is unset.
This commit is contained in:
@@ -238,14 +238,25 @@ Config from `bin/reviewer-config.example.json` to `~/.config/reviewer/config.jso
|
|||||||
Secrets in `~/.config/reviewer/env`, never here:
|
Secrets in `~/.config/reviewer/env`, never here:
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
REVIEWER_GITEA_TOKEN=... # read-only
|
REVIEWER_GITEA_TOKEN=... # read-only
|
||||||
REVIEWER_GITHUB_TOKEN=... # read-only
|
REVIEWER_GITHUB_TOKEN=... # read-only
|
||||||
REVIEWER_GITEA_SECRET=... # webhook HMAC
|
REVIEWER_GITEA_REVIEW_TOKEN=... # optional, write:issue — handed to review sessions
|
||||||
|
REVIEWER_GITEA_SECRET=... # webhook HMAC
|
||||||
REVIEWER_GITHUB_SECRET=...
|
REVIEWER_GITHUB_SECRET=...
|
||||||
```
|
```
|
||||||
|
|
||||||
The daemon's tokens are read-only — it never writes to a forge, which is also
|
The daemon's own tokens are read-only — it never writes to a forge, which is
|
||||||
why it doesn't mark notifications read.
|
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
|
Claude review sessions need `bubblewrap` and `socat` on the box, or the sandbox
|
||||||
cannot start and the session refuses to run (`failIfUnavailable`). That is
|
cannot start and the session refuses to run (`failIfUnavailable`). That is
|
||||||
|
|||||||
@@ -14,6 +14,7 @@
|
|||||||
"gitea": {
|
"gitea": {
|
||||||
"api": "https://git.example.com/api/v1",
|
"api": "https://git.example.com/api/v1",
|
||||||
"tokenEnv": "REVIEWER_GITEA_TOKEN",
|
"tokenEnv": "REVIEWER_GITEA_TOKEN",
|
||||||
|
"reviewTokenEnv": "REVIEWER_GITEA_REVIEW_TOKEN",
|
||||||
"webhookSecretEnv": "REVIEWER_GITEA_SECRET",
|
"webhookSecretEnv": "REVIEWER_GITEA_SECRET",
|
||||||
"self": ["you", "you-bot"]
|
"self": ["you", "you-bot"]
|
||||||
},
|
},
|
||||||
|
|||||||
+42
-10
@@ -38,7 +38,15 @@ type Config = {
|
|||||||
pathRoots?: string[]; // scanned one level deep to find clones by origin URL
|
pathRoots?: string[]; // scanned one level deep to find clones by origin URL
|
||||||
reviewers?: Reviewer[]; // rotation pool for review sessions
|
reviewers?: Reviewer[]; // rotation pool for review sessions
|
||||||
ledger?: string; // append-only record of which reviewer got which PR
|
ledger?: string; // append-only record of which reviewer got which PR
|
||||||
forges: Record<string, { api: string; tokenEnv: string; self: string | string[]; webhookSecretEnv?: string }>;
|
forges: Record<string, {
|
||||||
|
api: string;
|
||||||
|
tokenEnv: string;
|
||||||
|
self: string | string[];
|
||||||
|
webhookSecretEnv?: string;
|
||||||
|
// Write-capable token handed to review sessions so they can post findings.
|
||||||
|
// Separate from tokenEnv, which is read-only and stays that way.
|
||||||
|
reviewTokenEnv?: string;
|
||||||
|
}>;
|
||||||
repos: RepoConfig[];
|
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
|
// 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
|
// 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).
|
// 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, string>): string {
|
||||||
const wt = gitWorktrees(mainRepo);
|
const wt = gitWorktrees(mainRepo);
|
||||||
const settings = {
|
const settings = {
|
||||||
|
env,
|
||||||
permissions: {
|
permissions: {
|
||||||
defaultMode: "dontAsk",
|
defaultMode: "dontAsk",
|
||||||
allow: ["Read(//**)"],
|
allow: ["Read(//**)"],
|
||||||
@@ -541,8 +550,9 @@ function writeClaudeSettings(mainRepo: string): string {
|
|||||||
},
|
},
|
||||||
};
|
};
|
||||||
const path = join(REVIEW_SETTINGS_DIR, `${slug(mainRepo)}.json`);
|
const path = join(REVIEW_SETTINGS_DIR, `${slug(mainRepo)}.json`);
|
||||||
mkdirSync(REVIEW_SETTINGS_DIR, { recursive: true });
|
mkdirSync(REVIEW_SETTINGS_DIR, { recursive: true, mode: 0o700 });
|
||||||
writeFileSync(path, JSON.stringify(settings, null, 2));
|
// 0600: this file now carries the session's forge token.
|
||||||
|
writeFileSync(path, JSON.stringify(settings, null, 2), { mode: 0o600 });
|
||||||
return path;
|
return path;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -552,22 +562,43 @@ function writeClaudeSettings(mainRepo: string): string {
|
|||||||
// question without the prompt and without the trust. It goes in a profile
|
// 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
|
// file because the key is a quoted path, and -c would lose the quotes on the
|
||||||
// way through aoe's argument string.
|
// way through aoe's argument string.
|
||||||
function writeCodexProfile(mainRepo: string): string {
|
function writeCodexProfile(mainRepo: string, env: Record<string, string>): string {
|
||||||
const name = `review-${slug(mainRepo)}`;
|
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 });
|
mkdirSync(CODEX_HOME, { recursive: true });
|
||||||
writeFileSync(join(CODEX_HOME, `${name}.config.toml`),
|
writeFileSync(join(CODEX_HOME, `${name}.config.toml`),
|
||||||
`# generated by reviewer-poll.ts -- PR review session for ${mainRepo}\n` +
|
`# generated by reviewer-poll.ts -- PR review session for ${mainRepo}\n` +
|
||||||
`[projects."${mainRepo}"]\ntrust_level = "untrusted"\n\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;
|
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<string, string> {
|
||||||
|
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
|
// Every arg here has to survive being space-joined into one --extra-args
|
||||||
// string, so no quotes and no brackets: paths only.
|
// string, so no quotes and no brackets: paths only.
|
||||||
function sandboxArgs(tool: string, mainRepo: string): string[] {
|
function sandboxArgs(tool: string, mainRepo: string, env: Record<string, string>): string[] {
|
||||||
if (tool === "claude") return ["--settings", writeClaudeSettings(mainRepo)];
|
if (tool === "claude") return ["--settings", writeClaudeSettings(mainRepo, env)];
|
||||||
if (tool === "codex") {
|
if (tool === "codex") {
|
||||||
return ["--profile", writeCodexProfile(mainRepo),
|
return ["--profile", writeCodexProfile(mainRepo, env),
|
||||||
"--sandbox", "workspace-write", "--ask-for-approval", "never",
|
"--sandbox", "workspace-write", "--ask-for-approval", "never",
|
||||||
"--add-dir", gitWorktrees(mainRepo)];
|
"--add-dir", gitWorktrees(mainRepo)];
|
||||||
}
|
}
|
||||||
@@ -609,7 +640,8 @@ async function createReview(pr: Pr, authorTool?: string): Promise<void> {
|
|||||||
await git(pr.cfg.path!, ["fetch", "origin", `+refs/pull/${pr.number}/head:${local}`]);
|
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),
|
const args = ["-p", profile, "add", pr.cfg.path!, "--title", t, "--group", group(pr),
|
||||||
"--worktree", local, "--cmd", reviewer.tool];
|
"--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(" "));
|
if (extra.length) args.push("--extra-args", extra.join(" "));
|
||||||
await aoe(args);
|
await aoe(args);
|
||||||
clearYolo(profile, t);
|
clearYolo(profile, t);
|
||||||
|
|||||||
@@ -137,6 +137,12 @@ GitHub uses `gh`. Gitea uses plain REST against
|
|||||||
`Authorization: token` header — never in a URL, never in a commit
|
`Authorization: token` header — never in a URL, never in a commit
|
||||||
message. `source ~/.env.claude` if the token isn't in the environment.
|
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=<forge host>\n\n' | git credential fill`.
|
||||||
|
|
||||||
`$GITEA_TOKEN` is the name, and the only one. `~/.config/reviewer/config.json`
|
`$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
|
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
|
read-only token, it is loaded into the daemon process and nothing else, and in
|
||||||
|
|||||||
Reference in New Issue
Block a user