Members can read a private repository's pull request forks
A pull request's fork of a private repository was readable only by whoever opened it, so nobody else in the workspace could see its changes or check it out. It is now readable by everyone who can read the repository it came from. Pushing to it stays with its author. - runner: model routing moved to its own module with tests, including one that sends a request built from the settings to a stand-in gateway - runner: the agent's closing summary is written as a pull request description
7 files+147−340/7 viewed
| 133 | 133 | .await | |
| 134 | 134 | } | |
| 135 | 135 | ||
| 136 | − | /// Resolves a repo the viewer may read; private repos look missing. | |
| 137 | − | async fn readable(&self, path: &RepoPath, viewer: &Viewer) -> Result<Option<Repo>> { | |
| 136 | + | /// Whether the viewer may read `repo`. A pull request's fork of a | |
| 137 | + | /// private repository can be read by everyone who can read that | |
| 138 | + | /// repository, so its members can review and check out the change, as | |
| 139 | + | /// well as by whoever opened the pull request. | |
| 140 | + | async fn may_read(&self, repo: &Repo, viewer: &Viewer) -> Result<bool> { | |
| 141 | + | if can_read(repo, viewer) { | |
| 142 | + | return Ok(true); | |
| 143 | + | } | |
| 144 | + | let Some(source_id) = &repo.fork_of else { | |
| 145 | + | return Ok(false); | |
| 146 | + | }; | |
| 138 | 147 | Ok(self | |
| 139 | 148 | .registry | |
| 140 | − | .by_path(path) | |
| 149 | + | .by_id(source_id) | |
| 141 | 150 | .await? | |
| 142 | − | .filter(|repo| can_read(repo, viewer))) | |
| 151 | + | .is_some_and(|source| can_read(&source, viewer))) | |
| 143 | 152 | } | |
| 144 | 153 | ||
| 154 | + | /// `repo`, if there is one and the viewer may read it. | |
| 155 | + | async fn visible(&self, repo: Option<Repo>, viewer: &Viewer) -> Result<Option<Repo>> { | |
| 156 | + | Ok(match repo { | |
| 157 | + | Some(repo) if self.may_read(&repo, viewer).await? => Some(repo), | |
| 158 | + | _ => None, | |
| 159 | + | }) | |
| 160 | + | } | |
| 161 | + | ||
| 162 | + | /// Resolves a repo the viewer may read; private repos look missing. | |
| 163 | + | async fn readable(&self, path: &RepoPath, viewer: &Viewer) -> Result<Option<Repo>> { | |
| 164 | + | self.visible(self.registry.by_path(path).await?, viewer) | |
| 165 | + | .await | |
| 166 | + | } | |
| 167 | + | ||
| 145 | 168 | async fn get(&self, a: GetArgs) -> Result<Outcome<Repo>> { | |
| 146 | 169 | Ok(self | |
| 147 | 170 | .readable(&a.path, &a.viewer) | |
| 151 | 174 | ||
| 152 | 175 | async fn get_by_id(&self, a: GetByIdArgs) -> Result<Outcome<Repo>> { | |
| 153 | 176 | Ok(self | |
| 154 | − | .registry | |
| 155 | − | .by_id(&a.id) | |
| 177 | + | .visible(self.registry.by_id(&a.id).await?, &a.viewer) | |
| 156 | 178 | .await? | |
| 157 | − | .filter(|repo| can_read(repo, &a.viewer)) | |
| 158 | 179 | .map_or_else(not_found, Outcome::Ok)) | |
| 159 | 180 | } | |
| 160 | 181 | ||
| 416 | 437 | let allowed = if write { | |
| 417 | 438 | can_write(&repo, &a.viewer) | |
| 418 | 439 | } else { | |
| 419 | − | can_read(&repo, &a.viewer) | |
| 440 | + | self.may_read(&repo, &a.viewer).await? | |
| 420 | 441 | }; | |
| 421 | 442 | if !allowed { | |
| 422 | 443 | return Ok(denied()); | |
| 556 | 577 | ||
| 557 | 578 | async fn compare(&self, a: CompareArgs) -> Result<Outcome<Comparison>> { | |
| 558 | 579 | let Some(repo) = self | |
| 559 | − | .registry | |
| 560 | − | .by_id(&a.repo_id) | |
| 580 | + | .visible(self.registry.by_id(&a.repo_id).await?, &a.viewer) | |
| 561 | 581 | .await? | |
| 562 | − | .filter(|repo| can_read(repo, &a.viewer)) | |
| 563 | 582 | else { | |
| 564 | 583 | return Ok(not_found()); | |
| 565 | 584 | }; |
| 40 | 40 | format!("{}--{}", repo.namespace, repo.name) | |
| 41 | 41 | } | |
| 42 | 42 | ||
| 43 | + | /// Whether the viewer may read `repo`, going by the repository alone. A | |
| 44 | + | /// private pull request fork is also readable by whoever can read the | |
| 45 | + | /// repository it came from, which `Repos::may_read` checks. | |
| 43 | 46 | pub fn can_read(repo: &Repo, viewer: &Viewer) -> bool { | |
| 44 | 47 | !repo.is_private || can_write(repo, viewer) | |
| 45 | 48 | } | |
| 46 | 49 | ||
| 47 | − | /// A repository belongs to its workspace, so any member may write to it. An | |
| 50 | + | /// A repository belongs to its workspace, so any member may write to it. A | |
| 48 | 51 | /// pull request's fork belongs to whoever opened the pull request. | |
| 49 | 52 | pub fn can_write(repo: &Repo, viewer: &Viewer) -> bool { | |
| 50 | 53 | viewer.as_ref().is_some_and(|user| { |
| 5 | 5 | "type": "module", | |
| 6 | 6 | "license": "MIT", | |
| 7 | 7 | "scripts": { | |
| 8 | + | "test": "node --test src/*.test.ts", | |
| 8 | 9 | "typecheck": "wrangler types --include-env=false && tsc -p tsconfig.json", | |
| 9 | 10 | "deploy": "wrangler deploy" | |
| 10 | 11 | }, |
| 18 | 18 | workClient, | |
| 19 | 19 | } from "@g1t/contracts"; | |
| 20 | 20 | ||
| 21 | + | import { type ConfiguredModel, modelEnv } from "./model-env"; | |
| 22 | + | ||
| 21 | 23 | export interface RunnerEnv { | |
| 22 | 24 | SANDBOX: DurableObjectNamespace<AttemptSandbox>; | |
| 23 | 25 | IDENTITY: ServiceBinding; | |
| 78 | 80 | // harmlessly. | |
| 79 | 81 | const run = await this.ctx.storage.get<Run>("run"); | |
| 80 | 82 | if (run) await workClient(this.env.WORK).closePull(run.actor, run.repo, run.number); | |
| 81 | − | } | |
| 82 | − | } | |
| 83 | − | ||
| 84 | − | type ConfiguredModel = AgentModel & { model: string }; | |
| 85 | − | ||
| 86 | − | /** Where the sandbox sends model requests, and what it sends with them. */ | |
| 87 | − | function modelEnv(env: RunnerEnv, model: ConfiguredModel): Record<string, string> { | |
| 88 | − | const vars: Record<string, string> = { | |
| 89 | − | ANTHROPIC_API_KEY: env.ANTHROPIC_API_KEY!, | |
| 90 | − | ANTHROPIC_MODEL: model.model, | |
| 91 | − | // Recorded at the top of the session, so anyone can see what ran. | |
| 92 | − | AGENT_MODEL_NAME: `${model.modelName} (${model.label})`, | |
| 93 | − | }; | |
| 94 | − | if (env.AI_GATEWAY_ID) { | |
| 95 | − | vars.ANTHROPIC_BASE_URL = `https://gateway.ai.cloudflare.com/v1/${env.CLOUDFLARE_ACCOUNT_ID}/${env.AI_GATEWAY_ID}/anthropic`; | |
| 96 | − | if (env.AI_GATEWAY_TOKEN) { | |
| 97 | − | vars.AI_GATEWAY_TOKEN = env.AI_GATEWAY_TOKEN; | |
| 98 | − | vars.ANTHROPIC_CUSTOM_HEADERS = `cf-aig-authorization: Bearer ${env.AI_GATEWAY_TOKEN}`; | |
| 99 | − | } | |
| 100 | 83 | } | |
| 101 | − | return vars; | |
| 102 | 84 | } | |
| 103 | 85 | ||
| 104 | 86 | function buildPrompt(issue: Issue, instructions: string): string { | |
| 114 | 96 | } | |
| 115 | 97 | if (instructions) parts.push(instructions); | |
| 116 | 98 | parts.push( | |
| 117 | − | "Make the change and keep it focused on the issue. Commit your work with a clear message. Do not push; that is done for you. Finish with a short summary of what you changed and why.", | |
| 99 | + | "Make the change and keep it focused on the issue. Commit your work with a clear message. Do not push; that is done for you. Finish with a short summary of what you changed and why. It becomes the description of your pull request, so write it for a reviewer and leave out whether anything was committed or pushed.", | |
| 118 | 100 | ); | |
| 119 | 101 | return parts.filter(Boolean).join("\n\n"); | |
| 120 | 102 | } |
| 1 | + | import assert from "node:assert/strict"; | |
| 2 | + | import { createServer } from "node:http"; | |
| 3 | + | import { test } from "node:test"; | |
| 4 | + | ||
| 5 | + | import { type ConfiguredModel, modelEnv } from "./model-env.ts"; | |
| 6 | + | ||
| 7 | + | const model: ConfiguredModel = { | |
| 8 | + | id: "balanced", | |
| 9 | + | label: "Balanced", | |
| 10 | + | description: "", | |
| 11 | + | modelName: "Claude Sonnet 5.5", | |
| 12 | + | model: "claude-sonnet-5-5", | |
| 13 | + | }; | |
| 14 | + | const direct = { ANTHROPIC_API_KEY: "sk-test", AI_GATEWAY_ID: "", CLOUDFLARE_ACCOUNT_ID: "acct" }; | |
| 15 | + | ||
| 16 | + | test("without a gateway, requests go to the provider directly", () => { | |
| 17 | + | const vars = modelEnv(direct, model); | |
| 18 | + | assert.equal(vars.ANTHROPIC_BASE_URL, undefined); | |
| 19 | + | assert.equal(vars.ANTHROPIC_MODEL, "claude-sonnet-5-5"); | |
| 20 | + | assert.equal(vars.AGENT_MODEL_NAME, "Claude Sonnet 5.5 (Balanced)"); | |
| 21 | + | }); | |
| 22 | + | ||
| 23 | + | test("with a gateway, requests go through it", () => { | |
| 24 | + | const vars = modelEnv({ ...direct, AI_GATEWAY_ID: "g1t" }, model); | |
| 25 | + | assert.equal(vars.ANTHROPIC_BASE_URL, "https://gateway.ai.cloudflare.com/v1/acct/g1t/anthropic"); | |
| 26 | + | assert.equal(vars.ANTHROPIC_CUSTOM_HEADERS, undefined); | |
| 27 | + | }); | |
| 28 | + | ||
| 29 | + | test("an authenticated gateway is sent its token", () => { | |
| 30 | + | const vars = modelEnv({ ...direct, AI_GATEWAY_ID: "g1t", AI_GATEWAY_TOKEN: "tok" }, model); | |
| 31 | + | assert.equal(vars.ANTHROPIC_CUSTOM_HEADERS, "cf-aig-authorization: Bearer tok"); | |
| 32 | + | }); | |
| 33 | + | ||
| 34 | + | // A stand-in for the gateway: what a sandbox's agent sends, given these | |
| 35 | + | // variables, arrives at the gateway's path with both credentials. | |
| 36 | + | test("a request built from these variables reaches a gateway as expected", async () => { | |
| 37 | + | const seen: { url?: string; key?: string; gateway?: string; model?: string } = {}; | |
| 38 | + | const server = createServer((request, response) => { | |
| 39 | + | let body = ""; | |
| 40 | + | request.on("data", (chunk) => (body += chunk)); | |
| 41 | + | request.on("end", () => { | |
| 42 | + | seen.url = request.url; | |
| 43 | + | seen.key = request.headers["x-api-key"] as string; | |
| 44 | + | seen.gateway = request.headers["cf-aig-authorization"] as string; | |
| 45 | + | seen.model = JSON.parse(body).model; | |
| 46 | + | response.setHeader("content-type", "application/json"); | |
| 47 | + | response.end(JSON.stringify({ type: "message", content: [] })); | |
| 48 | + | }); | |
| 49 | + | }); | |
| 50 | + | await new Promise<void>((resolve) => server.listen(0, resolve)); | |
| 51 | + | const { port } = server.address() as { port: number }; | |
| 52 | + | ||
| 53 | + | const vars = modelEnv({ ...direct, AI_GATEWAY_ID: "g1t", AI_GATEWAY_TOKEN: "tok" }, model); | |
| 54 | + | // Same path as the real gateway, on the stand-in's address. | |
| 55 | + | const base = vars.ANTHROPIC_BASE_URL.replace("https://gateway.ai.cloudflare.com", `http://localhost:${port}`); | |
| 56 | + | const [name, value] = vars.ANTHROPIC_CUSTOM_HEADERS.split(": "); | |
| 57 | + | await fetch(`${base}/v1/messages`, { | |
| 58 | + | method: "POST", | |
| 59 | + | headers: { "x-api-key": vars.ANTHROPIC_API_KEY, [name]: value, "content-type": "application/json" }, | |
| 60 | + | body: JSON.stringify({ model: vars.ANTHROPIC_MODEL, max_tokens: 1, messages: [] }), | |
| 61 | + | }); | |
| 62 | + | server.close(); | |
| 63 | + | ||
| 64 | + | assert.deepEqual(seen, { | |
| 65 | + | url: "/v1/acct/g1t/anthropic/v1/messages", | |
| 66 | + | key: "sk-test", | |
| 67 | + | gateway: "Bearer tok", | |
| 68 | + | model: "claude-sonnet-5-5", | |
| 69 | + | }); | |
| 70 | + | }); |
| 1 | + | import type { AgentModel } from "@g1t/contracts"; | |
| 2 | + | ||
| 3 | + | /** A model as configured: what people see, and what is sent to the provider. */ | |
| 4 | + | export type ConfiguredModel = AgentModel & { model: string }; | |
| 5 | + | ||
| 6 | + | /** The settings that decide where model requests go. */ | |
| 7 | + | export type ModelRouting = { | |
| 8 | + | ANTHROPIC_API_KEY?: string; | |
| 9 | + | /** A Cloudflare AI Gateway id; empty sends requests to the provider directly. */ | |
| 10 | + | AI_GATEWAY_ID: string; | |
| 11 | + | CLOUDFLARE_ACCOUNT_ID: string; | |
| 12 | + | /** Needed only if the gateway requires authentication. */ | |
| 13 | + | AI_GATEWAY_TOKEN?: string; | |
| 14 | + | }; | |
| 15 | + | ||
| 16 | + | /** Where the sandbox sends model requests, and what it sends with them. */ | |
| 17 | + | export function modelEnv(env: ModelRouting, model: ConfiguredModel): Record<string, string> { | |
| 18 | + | const vars: Record<string, string> = { | |
| 19 | + | ANTHROPIC_API_KEY: env.ANTHROPIC_API_KEY!, | |
| 20 | + | ANTHROPIC_MODEL: model.model, | |
| 21 | + | // Recorded at the top of the session, so anyone can see what ran. | |
| 22 | + | AGENT_MODEL_NAME: `${model.modelName} (${model.label})`, | |
| 23 | + | }; | |
| 24 | + | if (env.AI_GATEWAY_ID) { | |
| 25 | + | vars.ANTHROPIC_BASE_URL = `https://gateway.ai.cloudflare.com/v1/${env.CLOUDFLARE_ACCOUNT_ID}/${env.AI_GATEWAY_ID}/anthropic`; | |
| 26 | + | if (env.AI_GATEWAY_TOKEN) { | |
| 27 | + | vars.AI_GATEWAY_TOKEN = env.AI_GATEWAY_TOKEN; | |
| 28 | + | vars.ANTHROPIC_CUSTOM_HEADERS = `cf-aig-authorization: Bearer ${env.AI_GATEWAY_TOKEN}`; | |
| 29 | + | } | |
| 30 | + | } | |
| 31 | + | return vars; | |
| 32 | + | } |
| 1 | 1 | { | |
| 2 | 2 | "extends": "../../tsconfig.base.json", | |
| 3 | − | "include": ["src/**/*", "worker-configuration.d.ts"] | |
| 3 | + | "include": [ | |
| 4 | + | "src/**/*", | |
| 5 | + | "worker-configuration.d.ts" | |
| 6 | + | ], | |
| 7 | + | "exclude": [ | |
| 8 | + | "src/**/*.test.ts" | |
| 9 | + | ] | |
| 4 | 10 | } |