Public: anyone can read this; the crew can change it.
Only an agent is ever on a crew: the database refuses a person, the action says so, one already there is reported #887
Check · Review · Land · Live done — Landed and live in production.
steward/crew-agents-only → main 3 commits
4 files
changed +15
−7
src/core/agents.ts +2 −2
| @@ -144,7 +144,7 @@ export function updateAgent( | ||
| 144 | 144 | |
| 145 | 145 | /** |
| 146 | 146 | * Put an agent on a repository's crew. Idempotent. Only an agent: the |
| 147 | * database refuses anyone else (migration 61), so a caller that passes a | |
| 147 | * database refuses anyone else (`repo_crew_agents_only`), so a caller that passes a | |
| 148 | 148 | * person has a bug, and hears about it as an error, not a quiet no. |
| 149 | 149 | */ |
| 150 | 150 | export function addCrew( |
| @@ -163,7 +163,7 @@ export function addCrew( | ||
| 163 | 163 | } |
| 164 | 164 | |
| 165 | 165 | /** |
| 166 | * People — not agents — on any crew: what migration 61 now refuses, if any | |
| 166 | * People — not agents — on any crew: what `repo_crew_agents_only` now refuses, if any | |
| 167 | 167 | * was put there before it. Reported at boot, never removed by the forge. |
| 168 | 168 | */ |
| 169 | 169 | export function peopleOnCrews( |
src/core/visibility.ts +6 −2
| @@ -41,7 +41,9 @@ export const REPO_VISIBLE = `( | ||
| 41 | 41 | * Repos `?1` may write, as a boolean expression over a `repo` row — the SQL |
| 42 | 42 | * twin of `canWrite`, for the lists that must answer "which of these can I |
| 43 | 43 | * push to" for fifty rows without fifty permission checks. Ownership, site |
| 44 | * admin, a team with write or admin in the owning org, or the crew grant. | |
| 44 | * admin, a team with write or admin in the owning org, or the crew grant — | |
| 45 | * an agent's only, as `onCrew` reads it: a person left on a crew from | |
| 46 | * before `repo_crew_agents_only` writes nothing through it. | |
| 45 | 47 | */ |
| 46 | 48 | export const REPO_WRITABLE = `( |
| 47 | 49 | repo.owner_id = ?1 |
| @@ -49,7 +51,9 @@ export const REPO_WRITABLE = `( | ||
| 49 | 51 | SELECT team.org_id FROM team |
| 50 | 52 | JOIN team_member ON team_member.team_id = team.id |
| 51 | 53 | WHERE team_member.user_id = ?1 AND team.permission IN ('write','admin')) |
| 52 | OR repo.id IN (SELECT repo_crew.repo_id FROM repo_crew WHERE repo_crew.user_id = ?1) | |
| 54 | OR repo.id IN (SELECT repo_crew.repo_id FROM repo_crew | |
| 55 | JOIN user AS crew ON crew.id = repo_crew.user_id | |
| 56 | WHERE repo_crew.user_id = ?1 AND crew.is_agent = 1) | |
| 53 | 57 | OR EXISTS (SELECT 1 FROM user AS me WHERE me.id = ?1 AND me.is_admin = 1) |
| 54 | 58 | )`; |
| 55 | 59 |
src/main.ts +1 −1
| @@ -333,7 +333,7 @@ const staleTimer = setInterval(() => void tickAsks(), 120_000); | ||
| 333 | 333 | void tickAsks(); |
| 334 | 334 | |
| 335 | 335 | app.log.info("forge is up", { url: servers.url, data: app.config.dataDir }); |
| 336 | // Only an agent is on a crew (migration 61 refuses anyone else). A person | |
| 336 | // Only an agent is on a crew (the `repo_crew_agents_only` trigger refuses anyone else). A person | |
| 337 | 337 | // put on one before that is reported here for the owner to take off — the |
| 338 | 338 | // forge never removes it on its own. |
| 339 | 339 | for (const p of peopleOnCrews(app.db)) |
test/crew-agents-only.test.ts +6 −2
| @@ -13,7 +13,7 @@ import { createApp, type App } from "../src/app.ts"; | ||
| 13 | 13 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; |
| 14 | 14 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; |
| 15 | 15 | import { addCrew, isCrew, peopleOnCrews } from "../src/core/agents.ts"; |
| 16 | import type { Repo } from "../src/core/repos.ts"; | |
| 16 | import { canWrite, writableRepoIds, type Repo } from "../src/core/repos.ts"; | |
| 17 | 17 | import { createToken } from "../src/core/tokens.ts"; |
| 18 | 18 | import { createUser, type User } from "../src/core/users.ts"; |
| 19 | 19 | import { serve, type Servers } from "../src/http/server.ts"; |
| @@ -134,7 +134,7 @@ describe("only an agent is on a crew", () => { | ||
| 134 | 134 | }); |
| 135 | 135 | |
| 136 | 136 | test("a person already on a crew from before is reported, never removed", () => { |
| 137 | // As if put there before migration 61: the guard stands aside for one row. | |
| 137 | // As if put there before `repo_crew_agents_only`: the guard stands aside for one row. | |
| 138 | 138 | app.db.exec("DROP TRIGGER repo_crew_agents_only"); |
| 139 | 139 | app.db |
| 140 | 140 | .query("INSERT INTO repo_crew (repo_id, user_id) VALUES (?1, ?2)") |
| @@ -146,5 +146,9 @@ describe("only an agent is on a crew", () => { | ||
| 146 | 146 | { repoId: repo.id, username: "bob" }, |
| 147 | 147 | ]); |
| 148 | 148 | expect(isCrew(app.db, repo.id, bob.id)).toBe(true); |
| 149 | // Kept, but no key: the SQL write check reads the crew grant as onCrew | |
| 150 | // does, agents only. | |
| 151 | expect(writableRepoIds(app.db, bob, [repo.id]).has(repo.id)).toBe(false); | |
| 152 | expect(canWrite(app.db, repo, bob)).toBe(false); | |
| 149 | 153 | }); |
| 150 | 154 | }); |
new above
offline