Public: anyone can read this; the crew can change it.
A test pins what contributing refuses, and that a refusal leaves no fork, branch or commit behind #6996
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6977-a-test-pins-what-contributing-refuses-an → main 1 commit fork 2 threads · 2 unresolved
2 files
changed +262
−0
src/actions/contribute.ts +7
| @@ -382,6 +382,13 @@ export async function contribute( | ||
| 382 | 382 | |
| 383 | 383 | const base = (input.base ?? "").trim() || upstream.value.defaultBranch; |
| 384 | 384 | if (!isSafeRef(base)) return err("unknown_base"); |
| 385 | // A base the upstream does not have is refused here, before a fork, a | |
| 386 | // branch or a commit exists. The default branch is the origin's own word. | |
| 387 | if (base !== upstream.value.defaultBranch) { | |
| 388 | const listed = await provider.value.listBranches(upstream.value.fullName); | |
| 389 | if (!listed.ok) return err("origin_unreachable"); | |
| 390 | if (!listed.value.some((b) => b.name === base)) return err("unknown_base"); | |
| 391 | } | |
| 385 | 392 | |
| 386 | 393 | return upstream.value.permissions.push |
| 387 | 394 | ? await contributeDirect(app, user.value, origin, upstream.value, { |
test/contribute-refusals.test.ts added +255
| @@ -0,0 +1,255 @@ | ||
| 1 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 2 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { createApp, type App } from "../src/app.ts"; | |
| 6 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 7 | import { | |
| 8 | mirrorAndWait, | |
| 9 | newFakeState, | |
| 10 | serveFakeForge, | |
| 11 | type FakeForge, | |
| 12 | } from "./fake-origin.ts"; | |
| 13 | import { proveAll } from "./helpers/verified.ts"; | |
| 14 | import { promote } from "./helpers/members.ts"; | |
| 15 | import { tokenFor } from "./helpers/tokens.ts"; | |
| 16 | ||
| 17 | /** | |
| 18 | * `contribute` refuses up front so nothing is half-made (#6977): every | |
| 19 | * refusal below leaves the origin without a new fork, branch or commit and | |
| 20 | * the forge without a new repository or pull. And the one failure that comes | |
| 21 | * after the commit landed — opening the pull — carries the commit. | |
| 22 | */ | |
| 23 | ||
| 24 | const ME = "me"; | |
| 25 | const OWNER = "octo"; | |
| 26 | const PASSWORD = "refusals-account-pass"; | |
| 27 | ||
| 28 | const read = async <T>(response: Response): Promise<T> => | |
| 29 | (await response.json()) as T; | |
| 30 | ||
| 31 | describe("contribute refusals", () => { | |
| 32 | let dataDir = ""; | |
| 33 | let originDataDir = ""; | |
| 34 | let app: App; | |
| 35 | let servers: Servers; | |
| 36 | let originApp: App; | |
| 37 | let originServers: Servers; | |
| 38 | let fake: FakeForge; | |
| 39 | let aliceToken = ""; | |
| 40 | let bobToken = ""; | |
| 41 | let carolToken = ""; | |
| 42 | let originToken = ""; | |
| 43 | let mirrorPath = ""; | |
| 44 | ||
| 45 | const mkAccount = async (server: Servers, username: string) => { | |
| 46 | await fetch(`${server.url}/api/v1/users`, { | |
| 47 | method: "POST", | |
| 48 | headers: { "content-type": "application/json" }, | |
| 49 | body: JSON.stringify({ | |
| 50 | username, | |
| 51 | email: `${username}@f.dev`, | |
| 52 | password: PASSWORD, | |
| 53 | }), | |
| 54 | }); | |
| 55 | return tokenFor(server === originServers ? originApp : app, username, "admin"); | |
| 56 | }; | |
| 57 | ||
| 58 | const contribute = ( | |
| 59 | body: Record<string, unknown>, | |
| 60 | token: string | null = aliceToken, | |
| 61 | ): Promise<Response> => | |
| 62 | fetch(`${servers.url}/api/v1/repos/${mirrorPath}/contribute`, { | |
| 63 | method: "POST", | |
| 64 | headers: { | |
| 65 | "content-type": "application/json", | |
| 66 | ...(token === null ? {} : { authorization: `Bearer ${token}` }), | |
| 67 | }, | |
| 68 | body: JSON.stringify(body), | |
| 69 | }); | |
| 70 | ||
| 71 | const attempt = { | |
| 72 | branch: "fix-typo", | |
| 73 | message: "fix", | |
| 74 | files: [{ path: "a.txt", content: "one, fixed\n" }], | |
| 75 | openPull: { title: "fix" }, | |
| 76 | }; | |
| 77 | ||
| 78 | const count = (sql: string): number => | |
| 79 | app.db.query<{ n: number }, []>(sql).get()?.n ?? -1; | |
| 80 | ||
| 81 | /** Everything a half-made contribution would have left behind. */ | |
| 82 | const world = async () => ({ | |
| 83 | fakeRepos: fake.state.repos.map((r) => r.fullName).sort(), | |
| 84 | fakeBranches: [...fake.state.branches.entries()], | |
| 85 | fakeCommits: [...fake.state.commits.entries()], | |
| 86 | fakeWrites: fake.state.requests.filter((r) => r.method !== "GET").length, | |
| 87 | forkBranches: await ( | |
| 88 | await fetch(`${originServers.url}/api/v1/repos/${ME}/gears/branches`) | |
| 89 | ).text(), | |
| 90 | repos: count("SELECT COUNT(*) AS n FROM repo"), | |
| 91 | pulls: count("SELECT COUNT(*) AS n FROM pull"), | |
| 92 | }); | |
| 93 | ||
| 94 | const refused = async ( | |
| 95 | response: Response, | |
| 96 | status: number, | |
| 97 | code: string, | |
| 98 | before: Awaited<ReturnType<typeof world>>, | |
| 99 | ) => { | |
| 100 | expect(response.status).toBe(status); | |
| 101 | expect((await read<{ error: string }>(response)).error).toBe(code); | |
| 102 | expect(await world()).toEqual(before); | |
| 103 | }; | |
| 104 | ||
| 105 | beforeAll(async () => { | |
| 106 | dataDir = await mkdtemp(join(tmpdir(), "forge-cr-")); | |
| 107 | originDataDir = await mkdtemp(join(tmpdir(), "forge-cr-up-")); | |
| 108 | ||
| 109 | originApp = createApp({ dataDir: originDataDir, port: 0 }); | |
| 110 | originServers = await serve(originApp); | |
| 111 | await mkAccount(originServers, OWNER); | |
| 112 | originToken = await mkAccount(originServers, ME); | |
| 113 | proveAll(originApp); | |
| 114 | promote(originApp.db); | |
| 115 | ||
| 116 | const as = (user: string) => | |
| 117 | `Basic ${Buffer.from(`${user}:${PASSWORD}`).toString("base64")}`; | |
| 118 | const up = (path: string, user: string, body: unknown) => | |
| 119 | fetch(`${originServers.url}${path}`, { | |
| 120 | method: "POST", | |
| 121 | headers: { "content-type": "application/json", authorization: as(user) }, | |
| 122 | body: JSON.stringify(body), | |
| 123 | }); | |
| 124 | await up("/api/v1/repos", OWNER, { name: "gears", visibility: "public" }); | |
| 125 | await up(`/api/v1/repos/${OWNER}/gears/contents`, OWNER, { | |
| 126 | branch: "main", | |
| 127 | message: "first", | |
| 128 | files: [{ path: "a.txt", content: "one\n" }], | |
| 129 | }); | |
| 130 | const forked = await up(`/api/v1/repos/${OWNER}/gears/forks`, ME, {}); | |
| 131 | expect(forked.status).toBe(201); | |
| 132 | ||
| 133 | const state = newFakeState("forgejo", ME, originToken); | |
| 134 | state.repos.push({ | |
| 135 | id: 1, | |
| 136 | fullName: `${OWNER}/gears`, | |
| 137 | cloneUrl: `${originServers.url}/${OWNER}/gears.git`, | |
| 138 | defaultBranch: "main", | |
| 139 | push: false, | |
| 140 | admin: false, | |
| 141 | }); | |
| 142 | fake = serveFakeForge(state); | |
| 143 | ||
| 144 | app = createApp({ dataDir, port: 0 }); | |
| 145 | servers = await serve(app); | |
| 146 | aliceToken = await mkAccount(servers, "alice"); | |
| 147 | bobToken = await mkAccount(servers, "bob"); | |
| 148 | proveAll(app); | |
| 149 | promote(app.db); | |
| 150 | // Made after everyone else was proven: this one never is. | |
| 151 | carolToken = await mkAccount(servers, "carol"); | |
| 152 | ||
| 153 | const connected = await fetch(`${servers.url}/api/v1/user/origins`, { | |
| 154 | method: "POST", | |
| 155 | headers: { | |
| 156 | "content-type": "application/json", | |
| 157 | authorization: `Bearer ${aliceToken}`, | |
Pin one status for signed-out. Accepting 401 or 404 lets the refusal change unnoticed. |
||
| 158 | }, | |
| 159 | body: JSON.stringify({ host: fake.url, token: originToken }), | |
| 160 | }); | |
| 161 | expect(connected.status).toBe(201); | |
| 162 | const originId = (await read<{ id: string }>(connected)).id; | |
| 163 | const job = await mirrorAndWait( | |
| 164 | await fetch(`${servers.url}/api/v1/user/origins/${originId}/mirror`, { | |
| 165 | method: "POST", | |
| 166 | headers: { | |
| 167 | "content-type": "application/json", | |
| 168 | authorization: `Bearer ${aliceToken}`, | |
| 169 | }, | |
| 170 | body: JSON.stringify({ fullName: `${OWNER}/gears` }), | |
| 171 | }), | |
| 172 | ); | |
| 173 | expect(job?.stage).toBe("done"); | |
| 174 | mirrorPath = `${job!.repo!.ownerName}/${job!.repo!.name}`; | |
| 175 | }); | |
| 176 | ||
| 177 | afterAll(async () => { | |
| 178 | fake.stop(); | |
| 179 | servers.stop(); | |
| 180 | originServers.stop(); | |
| 181 | app.close(); | |
| 182 | originApp.close(); | |
| 183 | await rm(dataDir, { recursive: true, force: true }); | |
| 184 | await rm(originDataDir, { recursive: true, force: true }); | |
| 185 | }); | |
| 186 | ||
| 187 | test("signed out is refused", async () => { | |
| 188 | const before = await world(); | |
| 189 | const response = await contribute(attempt, null); | |
| 190 | expect([401, 404]).toContain(response.status); | |
| 191 | expect(await world()).toEqual(before); | |
| 192 | }); | |
| 193 | ||
| 194 | test("an unverified account is `unverified`", async () => { | |
| 195 | const before = await world(); | |
| 196 | await refused(await contribute(attempt, carolToken), 403, "unverified", before); | |
| 197 | }); | |
| 198 | ||
| 199 | test("an origin that is not the caller's is `not_found`", async () => { | |
| 200 | const before = await world(); | |
| 201 | const response = await contribute(attempt, bobToken); | |
| 202 | expect(response.status).toBe(404); | |
| 203 | expect(await world()).toEqual(before); | |
| 204 | }); | |
| 205 | ||
| 206 | test.each(["refs/heads/x", "a..b", "x/../y"])( | |
| 207 | "branch %p is `invalid_branch`", | |
| 208 | async (branch) => { | |
| 209 | const before = await world(); | |
| 210 | await refused( | |
| 211 | await contribute({ ...attempt, branch }), | |
| 212 | 422, | |
| 213 | "invalid_branch", | |
| 214 | before, | |
| 215 | ); | |
| 216 | }, | |
| 217 | ); | |
| 218 | ||
| 219 | test("a base the upstream does not have is `unknown_base`", async () => { | |
| 220 | const before = await world(); | |
| 221 | await refused( | |
| 222 | await contribute({ ...attempt, base: "no-such-branch" }), | |
| 223 | 422, | |
| 224 | "unknown_base", | |
| 225 | before, | |
| 226 | ); | |
| 227 | await refused( | |
| 228 | await contribute({ ...attempt, base: "a..b" }), | |
| 229 | 422, | |
| 230 | "unknown_base", | |
| 231 | before, | |
| 232 | ); | |
| 233 | }); | |
| 234 | ||
| 235 | test("a pull that cannot open after the commit landed carries the commit", async () => { | |
| 236 | fake.state.refuseWrite = "no pulls today"; | |
| 237 | const response = await contribute(attempt); | |
| 238 | fake.state.refuseWrite = ""; | |
| 239 | expect(response.status).toBe(422); | |
| 240 | const failed = await read<{ | |
| 241 | error: string; | |
| 242 | commit: { commitOid: string }; | |
| 243 | }>(response); | |
| 244 | expect(failed.error).toBe("pull_failed"); | |
| 245 | expect(failed.commit.commitOid).toMatch(/^[0-9a-f]{40}$/); | |
| 246 | ||
| 247 | // The commit is not a claim: it is on the fork at the origin. | |
| 248 | const branches = await read<{ items: { name: string; oid: string }[] }>( | |
| 249 | await fetch(`${originServers.url}/api/v1/repos/${ME}/gears/branches`), | |
| 250 | ); | |
| 251 | expect(branches.items.find((b) => b.name === "fix-typo")?.oid).toBe( | |
| 252 | failed.commit.commitOid, | |
| 253 | ); | |
| 254 | }); | |
| 255 | }); |
new above
offline
The origin_unreachable branch is not covered by any test. Make the fake origin fail the branches call and assert the code and an unchanged world.