trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns16g rEventsFlows

A pull that comes from another forge runs nothing with secrets until a person says so #8994

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/8964-a-pull-that-comes-from-another-forge-run → main 7 commits fork

  • Merged. scout/8964-a-pull-that-comes-from-another-forge-run was merged into main as eb71574.

Review

Sign in to review this change.
17 files changed +545 −18
docs/features/guest-pulls.md added +29
@@ -0,0 +1,29 @@
1 # Guest pulls
2
3 A pull whose author lives on another forge — brought in from the origin, or
4 whose head lives in a fork there — is a **guest** (`src/origins/guest.ts`).
5
6 Until a person who can write the repository approves it once:
7
8 - its preview starts with none of the app's variables and no forge token
9 (`AppsRuntime.secretsFor` in `src/apps/runtime.ts`);
10 - the crew does not send it to review (`src/crew/review.ts`).
11
12 Its check needs nothing here: a check never carries the repository's
13 secrets, a guest's or anyone's (`src/checks/service.ts` builds its
14 environment from the forge's own names only).
15
16 The pull page says it is a guest to everyone; writers see **Approve this
17 guest**, a same-origin form posting to `/:owner/:name/pulls/:number/approve-guest`.
18 In the terminal the pull screen says the same, and a writer presses `G`.
19 Both call `approveGuestPull` (`src/actions/pulls.ts`), which announces
20 `pull.guest_approved`: open pages hear it live, and the preview starts again
21 holding the app's variables.
22
23 **The approval is per pull, not per head.** It is kept in `guest_approval`
24 and stays when the guest pushes again: every later head of that pull runs
25 with the app's variables too. What still asks per head is whether the code
26 may run here at all (`src/core/pull_trust.ts`, "Approve to run here"), so a
27 new head from a fork on the origin waits for that approval before anything
28 runs. Approve a guest when you trust its author with the preview's
29 variables, not only its current diff.
src/actions/pulls.ts +24
@@ -60,6 +60,7 @@ import {
6060 type ThreadState,
6161 } from "../core/reviews.ts";
6262 import { isProposed } from "../core/pull_trust.ts";
63 import { approveGuest } from "../origins/guest.ts";
6364 import { grantRun } from "../core/run_grant.ts";
6465 import { commitIdentity } from "../core/commit_identity.ts";
6566 import { getUserById, getUserByName, type User } from "../core/users.ts";
@@ -893,3 +894,26 @@ export const approveToRun = (
893894 isProposed(app.db, pull)
894895 ? letRun(app, actor, repo, pull)
895896 : approveHere(app, actor, repo, pull);
897
898 /**
899 * The one door every surface calls to approve a guest from another forge
900 * (`origins/guest.ts`): the page's form and the terminal's `G`. Announced, so
901 * the preview restarts holding the app's variables and every open view of
902 * the pull hears it.
903 */
904 export function approveGuestPull(
905 app: App,
906 actor: User | undefined,
907 repo: Repo,
908 pull: Pull,
909 ): Result<"approved" | "already", "forbidden" | "not_guest"> {
910 const done = approveGuest(app.db, repo, pull, actor);
911 if (done.ok && done.value === "approved")
912 app.bus.append({
913 kind: "pull.guest_approved",
914 repoId: repo.id,
915 actorId: actor?.id,
916 payload: { number: pull.number },
917 });
918 return done;
919 }
src/apps/runtime.ts +30 −15
@@ -36,6 +36,7 @@ import { requeueAbandonedChecks } from "../checks/service.ts";
3636 import { checkInstallCache } from "../checks/install.ts";
3737 import { getPull, listPulls, type Pull } from "../core/pulls.ts";
3838 import { pullMayRun } from "../core/pull_trust.ts";
39 import { guestWaits } from "../origins/guest.ts";
3940 import { canEnterApp, getRepoById, type Repo } from "../core/repos.ts";
4041 import { runningLimitSentence } from "../settings/quotas.ts";
4142 import type { User } from "../core/users.ts";
@@ -908,7 +909,9 @@ export class AppsRuntime {
908909 return;
909910 }
910911 // An approval is what lets a reader's pull run: deploy its head now.
912 // A guest's approval restarts its preview with the app's variables.
911913 case "pull.run_granted":
914 case "pull.guest_approved":
912915 case "pull.reviewed": {
913916 // A writer asking for changes may withdraw what let the preview run:
914917 // if the code it serves may no longer run, it stops now, not at its
@@ -943,7 +946,9 @@ export class AppsRuntime {
943946 repo,
944947 previewEnv(pull.number),
945948 head,
946 "approved",
949 // The same head may be serving already: start it again so it
950 // comes up holding the variables it ran without.
951 event.kind === "pull.guest_approved" ? "redeploy" : "approved",
947952 event.actorId ?? undefined,
948953 );
949954 return;
@@ -1395,13 +1400,7 @@ export class AppsRuntime {
13951400 ssh: this.sshFor(state.app, state.env),
13961401 // Read at start, never cached: a change is a redeploy, and the
13971402 // process that comes up is the one that sees it.
1398 vars: resolveAppEnv(
1399 this.app.db,
1400 this.app.config.dataDir,
1401 state.app.id,
1402 state.env,
1403 ),
1404 forgeToken: token.token,
1403 ...this.secretsFor(repo, state.app.id, state.env, token.token),
14051404 log: write,
14061405 warm,
14071406 cell: this.cellFor(state.app, state.env),
@@ -1633,6 +1632,28 @@ export class AppsRuntime {
16331632 };
16341633 }
16351634
1635 /**
1636 * What a process for `env` starts holding: the operator's variables, read
1637 * at start, and the forge token. A preview of a guest pull nobody has
1638 * approved yet (`origins/guest.ts`) holds neither.
1639 */
1640 secretsFor(
1641 repo: Repo,
1642 appId: string,
1643 env: EnvName,
1644 forgeToken: string,
1645 ): { vars: Record<string, string>; forgeToken: string | undefined } {
1646 const number = envPull(env);
1647 const pull =
1648 number === undefined ? undefined : getPull(this.app.db, repo.id, number);
1649 if (pull !== undefined && guestWaits(this.app.db, pull))
1650 return { vars: {}, forgeToken: undefined };
1651 return {
1652 vars: resolveAppEnv(this.app.db, this.app.config.dataDir, appId, env),
1653 forgeToken,
1654 };
1655 }
1656
16361657 /**
16371658 * An environment asleep is started again, warm, from the checkout it
16381659 * kept; one start serves every request that arrives while it happens.
@@ -1712,13 +1733,7 @@ export class AppsRuntime {
17121733 forgeUrl: this.options.forgeUrl,
17131734 identityKey: identityKey(this.app.config.dataDir, record.id),
17141735 ssh: this.sshFor(record, env),
1715 vars: resolveAppEnv(
1716 this.app.db,
1717 this.app.config.dataDir,
1718 record.id,
1719 env,
1720 ),
1721 forgeToken: token.token,
1736 ...this.secretsFor(repo, record.id, env, token.token),
17221737 log: write,
17231738 signal: new AbortController().signal,
17241739 warm: true,
src/core/event_audience.ts +1
@@ -99,6 +99,7 @@ const READER_KINDS: ReadonlySet<string> = new Set([
9999 "pull.reviewed",
100100 "pull.thread_resolved",
101101 "pull.run_granted",
102 "pull.guest_approved",
102103 "team.changed",
103104 "forge.severed",
104105 "forge.follow_mode",
src/core/migration-pins/20261104090000-guest-approval.pin added +1
@@ -0,0 +1,1 @@
1 65487f97300c8574b55f4251b1bff0aa07ada44fa66dd2d62dbc9605e41c7823
src/core/migrations/20261104090000-guest-approval.ts added +10
@@ -0,0 +1,10 @@
1 // A pull from another forge is a guest (#8964): its check and preview run
2 // without the repository's secrets until a person who can write approves it
3 // once. One row per approved pull, keyed by the pull's issue id. Additive.
4 export default `
5 CREATE TABLE guest_approval (
6 pull_id TEXT PRIMARY KEY,
7 approved_by TEXT NOT NULL,
8 approved_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now'))
9 );
10 `;
src/crew/review.ts +4
@@ -29,6 +29,7 @@ import { reviewSummariesFor, reviewSummary } from "../core/reviews.ts";
2929 import { checksForHead } from "../core/runs.ts";
3030 import { getTeam } from "../core/teams.ts";
3131 import { createLog } from "../kernel/log.ts";
32 import { guestWaits } from "../origins/guest.ts";
3233 import { reviewerPaused } from "./reviewer_paused.ts";
3334 import { queueAgentRun } from "./dispatch.ts";
3435 import { type Handover, handoverOf, handoversOf } from "./guard.ts";
@@ -451,6 +452,9 @@ export async function sendToReview(
451452 const reviewers = reviewersOf(app.db, repo.id);
452453 // A paused reviewer reads like an empty seat: the pull waits for a person, and says so.
453454 if (reviewers.length === 0 || reviewerPaused(repo)) return "none";
455 // A guest from another forge waits for a person's word before the crew
456 // touches it (`origins/guest.ts`).
457 if (guestWaits(app.db, pull)) return "held";
454458 // The reviewer's slots bound every route, the teams' too. The most (PILLAR_MAX) is no cap: the sweep already bounds what it sends.
455459 if (repo.reviewerSlots < PILLAR_MAX && reviewsInFlight(app.db, repo.id) >= repo.reviewerSlots) return "wait";
456460 if (teamsOfPull(app.db, pull).length > 0) {
src/design/activity.ts +1
@@ -135,6 +135,7 @@ const WORDS: Record<string, string> = {
135135 "pull.commented": "commented on a pull request",
136136 "pull.reviewed": "reviewed a pull request",
137137 "pull.run_granted": "let a pull request's head run",
138 "pull.guest_approved": "approved a guest pull request",
138139 "pull.labeled": "labelled a pull request",
139140 "pull.held": "held a pull request",
140141 "pull.released": "released a pull request",
src/http/server.ts +6
@@ -335,6 +335,7 @@ import {
335335 pullThreadFragment,
336336 threadResolveSubmit,
337337 } from "../web/pages/pull.ts";
338 import { pullApproveGuestSubmit } from "../web/pages/pull-guest.ts";
338339 import { conflictsFinish, conflictsPage, proposeSubmit } from "../web/pages/conflicts.ts";
339340 import {
340341 appDisable,
@@ -1132,6 +1133,11 @@ export function buildRouter(): Router<Handler> {
11321133 "/:owner/:name/pulls/:number/approve-here",
11331134 pullApproveHereSubmit,
11341135 );
1136 router.add(
1137 "POST",
1138 "/:owner/:name/pulls/:number/approve-guest",
1139 pullApproveGuestSubmit,
1140 );
11351141 router.add(
11361142 "POST",
11371143 "/:owner/:name/pulls/:number/threads/:anchor/resolve",
src/origins/guest.ts added +67
@@ -0,0 +1,67 @@
1 /**
2 * A guest pull: one whose author lives on another forge — it was brought in
3 * from the origin (its issue names an `origin_author`) or its head lives in
4 * a fork there (`originHeadRepo`). Its check and preview run without the
5 * repository's secrets, and the crew does not review it, until a person —
6 * never an agent — who can write the repository approves it once. The
7 * approval is for the pull, not a head: a later push by the same author
8 * runs with the secrets too. Whether a head's code may run at all is still
9 * `core/pull_trust.ts`'s question, asked per head. Pure data.
10 */
11
12 import type { Database } from "bun:sqlite";
13 import { getIssueById } from "../core/issues.ts";
14 import type { Pull } from "../core/pulls.ts";
15 import { canWrite, type Repo } from "../core/repos.ts";
16 import type { User } from "../core/users.ts";
17 import { err, ok, type Result } from "../kernel/result.ts";
18
19 /** Whether `pull`'s author lives on another forge. */
20 export function isGuestPull(db: Database, pull: Pull): boolean {
21 if (pull.originHeadRepo !== null) return true;
22 return (getIssueById(db, pull.id)?.originAuthor ?? null) !== null;
23 }
24
25 /** Who approved the guest `pull`, and when; undefined until someone has. */
26 export function guestApproval(
27 db: Database,
28 pullId: string,
29 ): { approvedBy: string; approvedAt: string } | undefined {
30 const row = db
31 .query<{ approved_by: string; approved_at: string }, [string]>(
32 "SELECT approved_by, approved_at FROM guest_approval WHERE pull_id = ?1",
33 )
34 .get(pullId);
35 return row === null
36 ? undefined
37 : { approvedBy: row.approved_by, approvedAt: row.approved_at };
38 }
39
40 /** A guest nobody has approved yet: no secrets, no crew. */
41 export function guestWaits(db: Database, pull: Pull): boolean {
42 return isGuestPull(db, pull) && guestApproval(db, pull.id) === undefined;
43 }
44
45 /** Whether `user` may approve a guest on `repo`: a person who can write it. */
46 export const mayApproveGuest = (
47 db: Database,
48 repo: Repo,
49 user: User | undefined,
50 ): boolean => user !== undefined && user.kind === "user" && canWrite(db, repo, user);
51
52 /** Approve the guest `pull`, once. A second approval keeps the first. */
53 export function approveGuest(
54 db: Database,
55 repo: Repo,
56 pull: Pull,
57 user: User | undefined,
58 ): Result<"approved" | "already", "forbidden" | "not_guest"> {
59 if (!mayApproveGuest(db, repo, user) || user === undefined) return err("forbidden");
60 if (!isGuestPull(db, pull)) return err("not_guest");
61 const changed = db
62 .query(
63 "INSERT INTO guest_approval (pull_id, approved_by) VALUES (?1, ?2) ON CONFLICT (pull_id) DO NOTHING",
64 )
65 .run(pull.id, user.id).changes;
66 return ok(changed === 0 ? "already" : "approved");
67 }
src/tui/screens/pull.ts +48 −1
@@ -33,6 +33,7 @@ import {
3333 } from "../../actions/issues.ts";
3434 import {
3535 addReview,
36 approveGuestPull,
3637 approveToRun as letItRun,
3738 mergePull,
3839 pullThreads,
@@ -58,6 +59,7 @@ import { getAppByRepo } from "../../core/apps.ts";
5859 import { teamVerdicts, type TeamVerdict } from "../../core/issue_teams.ts";
5960 import { getIssue, type Issue } from "../../core/issues.ts";
6061 import { isProposed, pullMayRun } from "../../core/pull_trust.ts";
62 import { guestWaits, mayApproveGuest } from "../../origins/guest.ts";
6163 import { runningProgress, stepsLine } from "../../checks/progress.ts";
6264 import { askCheck } from "../../checks/again.ts";
6365 import { checkStale, checksHold } from "../../checks/stale.ts";
@@ -216,6 +218,9 @@ export function teamLine(v: TeamVerdict): string {
216218 /** How often a pull's screen re-reads while its check is queued or running. */
217219 const CHECK_TICK_MS = 5000;
218220
221 /** A waiting guest's line on its screen; the page says it in full (`web/pages/pull-guest.ts`). */
222 export const GUEST_NOTE = "a guest from another forge: no app variables, no crew, until a writer approves";
223
219224 export function pullScreen(
220225 f: Session,
221226 ctx: Ctx,
@@ -875,6 +880,43 @@ export function pullScreen(
875880 });
876881 };
877882
883 /** Whether this open pull is a guest from another forge nobody has approved yet. */
884 const guestWaiting = (): boolean => {
885 if (repo === undefined) return false;
886 const row = getPull(f.db, repo.id, number);
887 return (
888 row !== undefined &&
889 row.state === "open" &&
890 row.mergedAt === null &&
891 guestWaits(f.db, row)
892 );
893 };
894
895 /** `G` on offer: a waiting guest, and a person who can write here. The page's gate. */
896 const mayApproveThisGuest = (): boolean =>
897 repo !== undefined && guestWaiting() && mayApproveGuest(f.db, repo, f.user);
898
899 /** `G` — approve this guest once; `approveGuestPull` asks again who may. */
900 const approveThisGuest = (): void => {
901 if (repo === undefined) return;
902 const row = getPull(f.db, repo.id, number);
903 if (row === undefined) return;
904 const done = approveGuestPull(f.app, f.user, repo, row);
905 if (!done.ok) {
906 ctx.toast(
907 done.error === "forbidden"
908 ? "only a person with write access can approve a guest"
909 : "this pull is from this forge: it is no guest",
910 "danger",
911 );
912 return;
913 }
914 runAsync(ctx, async () => {
915 await reload();
916 ctx.toast("approved as a guest: its preview starts again with the app's variables", "success");
917 });
918 };
919
878920 /** `v` — the conflict blocks, read; `pullConflicts` holds the writer gate. */
879921 const seeConflicts = (): void => {
880922 if (repo === undefined) return;
@@ -964,6 +1006,7 @@ export function pullScreen(
9641006 ),
9651007 ]
9661008 : []),
1009 ...(mayApproveThisGuest() ? [binding(["G"], "approve this guest")] : []),
9671010 binding(["a"], "assign"),
9681011 ...(mayHold() && (hold !== undefined || detail?.issue.state === "open")
9691012 ? [binding(["h"], hold === undefined ? "hold" : "let it go")]
@@ -1055,7 +1098,7 @@ export function pullScreen(
10551098 x1,
10561099 head!.y + 1,
10571100 truncate(
1058 ` ${facts?.mergedAt != null ? "merged" : issue.state} · opened by ${who(issue.authorName, issue.originAuthor)} ${ago(issue.createdAt)}${assignedText(issueRow, ctx.icons)}${facts?.proposal ? ` · ${proposedByText(facts.proposal)}` : ""}${hold === undefined ? "" : ` · held by ${hold.heldByName}: ${hold.why}`}${step === null ? "" : ` · step: ${stepLine(step)}`}${next === null ? "" : ` · next: ${next.who}, ${next.why}`}`,
1101 ` ${facts?.mergedAt != null ? "merged" : issue.state} · opened by ${who(issue.authorName, issue.originAuthor)} ${ago(issue.createdAt)}${guestWaiting() ? ` · ${GUEST_NOTE}` : ""}${assignedText(issueRow, ctx.icons)}${facts?.proposal ? ` · ${proposedByText(facts.proposal)}` : ""}${hold === undefined ? "" : ` · held by ${hold.heldByName}: ${hold.why}`}${step === null ? "" : ` · step: ${stepLine(step)}`}${next === null ? "" : ` · next: ${next.who}, ${next.why}`}`,
10591102 Math.max(0, edge - x1),
10601103 ),
10611104 style(c.mutedFg, c.bg),
@@ -1183,6 +1226,10 @@ export function pullScreen(
11831226 review();
11841227 return true;
11851228 }
1229 if (e.key === "G" && mayApproveThisGuest()) {
1230 approveThisGuest();
1231 return true;
1232 }
11861233 if (e.key === "A" && mayApproveHere()) {
11871234 approveToRun();
11881235 return true;
src/web/live.ts +1 −1
@@ -299,7 +299,7 @@ export const liveClientJs = /* js */ `
299299 'push', 'repo.created', 'repo.forked',
300300 'issue.opened', 'issue.closed', 'issue.reopened', 'issue.commented', 'issue.labeled', 'issue.teams', 'issue.edited',
301301 'pull.opened', 'pull.closed', 'pull.reopened', 'pull.merged', 'pull.commented', 'pull.reviewed',
302 'pull.held', 'pull.released', 'pull.edited', 'pull.run_granted',
302 'pull.held', 'pull.released', 'pull.edited', 'pull.run_granted', 'pull.guest_approved',
303303 'branch.created', 'branch.deleted',
304304 'label.created', 'label.updated', 'label.deleted',
305305 'run.queued', 'run.started', 'run.finished', 'app.serving',
src/web/pages/pull-guest.ts added +68
@@ -0,0 +1,68 @@
1 /**
2 * A guest pull on its page (`origins/guest.ts`): everyone sees that it came
3 * from another forge and its preview runs without the app's variables; a
4 * person who can write the repository sees the one action that lifts that. The POST is
5 * read through `readForm` (the `rw` floor and the same-origin check), and
6 * `approveGuestPull` asks again who may.
7 */
8
9 import type { Ctx } from "../../http/context.ts";
10 import type { Pull } from "../../core/pulls.ts";
11 import type { Repo } from "../../core/repos.ts";
12 import { approveGuestPull } from "../../actions/pulls.ts";
13 import {
14 guestApproval,
15 isGuestPull,
16 mayApproveGuest,
17 } from "../../origins/guest.ts";
18 import { repoBase, signInFirst } from "../chrome.ts";
19 import { readForm } from "../form.ts";
20 import { html, type Html } from "../html.ts";
21 import { flashTo } from "../layout.ts";
22 import { pullContext } from "../studio/change.ts";
23 import { btn, card, icon } from "../ui.ts";
24
25 export const GUEST_WAITS =
26 "This pull comes from another forge, so it is a guest: its preview runs without the app's variables or a forge token, and the crew leaves it alone, until a person who can write here approves it.";
27 export const GUEST_APPROVED = "Approved as a guest: its preview starts again with the app's variables, and the crew takes it up.";
28
29 /** The guest notice; empty for a pull from this forge, or one already approved. */
30 export function guestCard(ctx: Ctx, repo: Repo, pull: Pull, action: string): Html {
31 if (pull.state !== "open" || pull.mergedAt !== null) return html``;
32 if (!isGuestPull(ctx.app.db, pull)) return html``;
33 if (guestApproval(ctx.app.db, pull.id) !== undefined) return html``;
34 const writer = mayApproveGuest(ctx.app.db, repo, ctx.auth.user);
35 return card(
36 html`<p>${GUEST_WAITS}</p>
37 ${writer
38 ? html`<form method="post" action="${action}">
39 ${btn("Approve this guest", { size: "sm", icon: "check" })}
40 </form>`
41 : null}`,
42 {
43 extra: "mb",
44 head: html`${icon("globe")}
45 <h2>A guest</h2>`,
46 },
47 );
48 }
49
50 /** POST /:owner/:name/pulls/:number/approve-guest — a writer's word, once. */
51 export async function pullApproveGuestSubmit(ctx: Ctx): Promise<Response> {
52 const found = pullContext(ctx);
53 if (found instanceof Response) return found;
54 if (!ctx.auth.user) return signInFirst(ctx);
55 const form = await readForm(ctx, "rw");
56 if (form instanceof Response) return form;
57 const dest = `${repoBase(found.repo)}/pulls/${found.pull.number}`;
58 const done = approveGuestPull(ctx.app, ctx.auth.user, found.repo, found.pull);
59 if (!done.ok)
60 return flashTo(ctx, dest, {
61 tone: "danger",
62 message:
63 done.error === "forbidden"
64 ? "Only a person with write access can approve a guest."
65 : "This pull is from this forge: it is no guest.",
66 });
67 return flashTo(ctx, dest, { message: GUEST_APPROVED });
68 }
src/web/pages/pull.ts +3 −1
@@ -112,6 +112,7 @@ import { specFindings } from "./pull-spec.ts";
112112 import { pullPreviewFindings } from "../../core/preview_findings.ts";
113113 import { crewActivityCard } from "./runs.ts";
114114 import { holdCard } from "../studio/hold.ts";
115 import { guestCard } from "./pull-guest.ts";
115116 import {
116117 changeHead,
117118 changeTabs,
@@ -1012,6 +1013,7 @@ export async function pullFilesPage(ctx: Ctx): Promise<Response> {
10121013 ${mergeBox(ctx, repo, pull, status, mayMerge, by, check, isApp, tier, stale)}
10131014 <div class="split">
10141015 <div>
1016 ${guestCard(ctx, repo, pull, `${thread}/approve-guest`)}
10151017 ${approveHereCard(ctx, repo, pull, status.headOid, `${thread}/approve-here`)}
10161018 ${reviewCard(ctx, repo, pull, `${thread}/review-state`)}
10171019 </div>
@@ -1232,7 +1234,7 @@ export function approveHereCard(
12321234 <p>
12331235 ${proposed
12341236 ? "A child forge proposed this head, so its check and preview wait for you. Letting it run is not a review and does not count toward one; it runs this exact code here, holding the repository owner's app token. A new push waits again."
1235 : "This head is from a fork on the origin, so its check and preview wait for you. Approving lets them run this exact code here, holding the repository owner's app token. A new push waits again."}
1237 : "This head is from a fork on the origin, so its check and preview wait for you. Approving lets them run this exact code here; it holds the repository's secrets only once it is approved as a guest too. A new push waits again."}
12361238 </p>
12371239 ${btn(proposed ? "Let it run here" : "Approve to run here", { size: "sm", icon: "check" })}
12381240 </form>`,
src/web/pages/thread.ts +5
@@ -170,6 +170,11 @@ const EVENT_LOOK: Record<string, { icon: IconName; tone: Tone; verb: string }> =
170170 tone: "default",
171171 verb: "let this head run",
172172 },
173 "pull.guest_approved": {
174 icon: "check",
175 tone: "default",
176 verb: "approved this guest",
177 },
173178 "pull.edited": { icon: "pencil", tone: "default", verb: "edited this" },
174179 "pull.released": {
175180 icon: "check",
src/web/studio/change.ts +2
@@ -65,6 +65,7 @@ import {
6565 import { previewCard } from "../pages/apps.ts";
6666 import { lookCard } from "../pages/looks.ts";
6767 import { approveHereCard, pullChecksLineFor, teamsCard } from "../pages/pull.ts";
68 import { guestCard } from "../pages/pull-guest.ts";
6869 import { teamVerdicts } from "../../core/issue_teams.ts";
6970 import { liveRegionsJs } from "../client.ts";
7071 import { pullState } from "../pages/pulls.ts";
@@ -676,6 +677,7 @@ async function changeConversation(
676677 spent,
677678 "gate",
678679 () => html`${held ? holdCard(ctx, repo, pull, { act: true }) : null}
680 ${guestCard(ctx, repo, pull, `${thread}/approve-guest`)}
679681 ${approveHereCard(ctx, repo, pull, headOid, `${thread}/approve-here`)}
680682 ${teamsCard(teamVerdicts(db, { ...pull, headOid }))}
681683 ${when(wall, (w) => wallCard(w, writer ? thread : undefined))}`,
test/guest-pull.test.ts added +245
@@ -0,0 +1,245 @@
1 /**
2 * A pull from another forge is a guest (#8964): its preview starts without
3 * the app's variables or a forge token, and the crew leaves it alone, until a
4 * person who can write the repository approves it once — on the page through
5 * a same-origin form, or in the terminal with `G`. The approval is per pull:
6 * a later push by the guest keeps it.
7 */
8
9 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
10 import { mkdtemp, rm } from "node:fs/promises";
11 import { tmpdir } from "node:os";
12 import { join } from "node:path";
13 import { openPull } from "../src/actions/pulls.ts";
14 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
15 import { createApp, type App } from "../src/app.ts";
16 import { AppsRuntime } from "../src/apps/runtime.ts";
17 import { previewEnv } from "../src/apps/data.ts";
18 import { listEvents } from "../src/core/events.ts";
19 import { createForgeApp } from "../src/tui/forge.ts";
20 import { GUEST_NOTE, pullScreen } from "../src/tui/screens/pull.ts";
21 import { local } from "../src/tui/screens/where.ts";
22 import { headlessTerminal } from "../src/tui/term/terminal.ts";
23 import { settleApp } from "./helpers/tui.ts";
24 import { setAppEnv } from "../src/core/app_env.ts";
25 import { createApp as createAppRecord } from "../src/core/apps.ts";
26 import { getPull, type Pull } from "../src/core/pulls.ts";
27 import type { Repo } from "../src/core/repos.ts";
28 import { createSession } from "../src/core/sessions.ts";
29 import { createUser, getUserByName, type User } from "../src/core/users.ts";
30 import { forgeHttp, type ForgeHttp } from "../src/http/server.ts";
31 import { approveGuest, guestWaits, isGuestPull } from "../src/origins/guest.ts";
32 import { sendToReview } from "../src/crew/review.ts";
33 import { GUEST_WAITS } from "../src/web/pages/pull-guest.ts";
34 import { quietLogs } from "./helpers/globals.ts";
35 import { promote } from "./helpers/members.ts";
36 import { seatAs } from "./helpers/seat.ts";
37 import { addToCrew, createAgent } from "../src/actions/crew.ts";
38
39 quietLogs();
40
41 describe("a pull from another forge is a guest until a writer approves it", () => {
42 let dataDir: string;
43 let app: App;
44 let http: ForgeHttp;
45 let alice: User;
46 let carol: User;
47 let lib: Repo;
48 let appId: string;
49 let runtime: AppsRuntime;
50 let n = 0;
51 const url = "http://forge.test";
52
53 async function pull(mark: "head" | "author" | "none"): Promise<Pull> {
54 const branch = `g${++n}`;
55 const committed = await commitToBranch(app, alice, lib, {
56 branch,
57 message: "From elsewhere",
58 files: [{ path: `${branch}.md`, content: "hi\n" }],
59 });
60 if (!committed.ok) throw new Error(JSON.stringify(committed.error));
61 const opened = await openPull(app, alice, lib, {
62 title: "Theirs",
63 baseBranch: "main",
64 headBranch: branch,
65 });
66 if (!opened.ok) throw new Error(String(opened.error));
67 if (mark === "head")
68 app.db.run("UPDATE pull SET origin_head_repo = 'octo/lib' WHERE issue_id = ?1", [opened.value.id]);
69 if (mark === "author")
70 app.db.run("UPDATE issue SET origin_author = 'octocat' WHERE id = ?1", [opened.value.id]);
71 return getPull(app.db, lib.id, opened.value.number)!;
72 }
73
74 beforeAll(async () => {
75 dataDir = await mkdtemp(join(tmpdir(), "forge-guest-pull-"));
76 app = createApp({ dataDir, port: 0, host: "forge.test" });
77 http = forgeHttp(app);
78 const people: User[] = [];
79 for (const [username, first] of [["alice", true], ["carol", false]] as const) {
80 const made = await createUser(app.db, {
81 origin: "seed",
82 trusted: true,
83 username,
84 email: `${username}@forge.local`,
85 password: "a-good-passphrase",
86 adminIfFirst: first,
87 });
88 if (!made.ok) throw new Error(made.error);
89 people.push(made.value);
90 }
91 [alice, carol] = people as [User, User];
92 promote(app.db, carol);
93 const made = await createRepo(app, alice, { name: "lib", visibility: "public" });
94 if (!made.ok) throw new Error(String(made.error));
95 lib = made.value;
96 const seeded = await commitToBranch(app, alice, lib, {
97 message: "Start",
98 files: [{ path: "README.md", content: "lib\n" }],
99 });
100 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
101 const record = createAppRecord(app.db, { repoId: lib.id, slug: "lib", access: "public" });
102 if (!record.ok) throw new Error(record.error);
103 appId = record.value.id;
104 // Never started: only asked what a process it would start holds.
105 runtime = new AppsRuntime(app, { domain: "apps.test", scheme: "http", forgeUrl: url, concurrency: 1 });
106 const set = setAppEnv(app.db, dataDir, record.value.id, "API_KEY", "s3cret", "preview");
107 if (!set.ok) throw new Error(set.error);
108 }, 60_000);
109
110 afterAll(async () => {
111 http.close();
112 app.close();
113 await rm(dataDir, { recursive: true, force: true });
114 });
115
116 const held = (pull: Pull) => runtime.secretsFor(lib, appId, previewEnv(pull.number), "tok");
117
118 test("a waiting guest's preview starts with no variables and no forge token; after approval it has both", async () => {
119 const guest = await pull("head");
120 expect(isGuestPull(app.db, guest)).toBe(true);
121 expect(held(guest)).toEqual({ vars: {}, forgeToken: undefined });
122
123 // A reader's word does not count; a writer's does, once.
124 expect(approveGuest(app.db, lib, guest, carol)).toEqual({ ok: false, error: "forbidden" });
125 expect(held(guest)).toEqual({ vars: {}, forgeToken: undefined });
126 expect(approveGuest(app.db, lib, guest, alice)).toEqual({ ok: true, value: "approved" });
127 expect(approveGuest(app.db, lib, guest, alice)).toEqual({ ok: true, value: "already" });
128 expect(held(guest)).toEqual({ vars: { API_KEY: "s3cret" }, forgeToken: "tok" });
129 });
130
131 test("the approval is per pull: a later push by the guest keeps it", async () => {
132 const guest = await pull("head");
133 expect(approveGuest(app.db, lib, guest, alice).ok).toBe(true);
134 const pushed = await commitToBranch(app, alice, lib, {
135 branch: guest.headBranch,
136 message: "And more",
137 files: [{ path: "more.md", content: "more\n" }],
138 });
139 if (!pushed.ok) throw new Error(JSON.stringify(pushed.error));
140 const after = getPull(app.db, lib.id, guest.number)!;
141 expect(guestWaits(app.db, after)).toBe(false);
142 expect(held(after).vars).toEqual({ API_KEY: "s3cret" });
143 });
144
145 test("a pull brought in from the origin is a guest too; one from here is not", async () => {
146 const brought = await pull("author");
147 expect(guestWaits(app.db, brought)).toBe(true);
148 expect(held(brought).vars).toEqual({});
149 const own = await pull("none");
150 expect(isGuestPull(app.db, own)).toBe(false);
151 expect(held(own)).toEqual({ vars: { API_KEY: "s3cret" }, forgeToken: "tok" });
152 expect(approveGuest(app.db, lib, own, alice)).toEqual({ ok: false, error: "not_guest" });
153 });
154
155 test("the crew does not send a waiting guest to review, though a reviewer is seated", async () => {
156 const agent = await createAgent(app, alice, { username: "sentinel" });
157 if (!agent.ok) throw new Error(String(agent.error));
158 expect(addToCrew(app, alice, lib, "sentinel").ok).toBe(true);
159 expect(seatAs(app.db, lib.id, getUserByName(app.db, "sentinel")!.id, "reviewer")).toBe(true);
160 const guest = await pull("head");
161 expect(await sendToReview(app, lib, guest)).toBe("held");
162 });
163
164 test("the page says it is a guest; only a writer is offered the approval, same-origin", async () => {
165 const guest = await pull("head");
166 const as = (user: User) => `forge_session=${createSession(app.db, user.id).token}`;
167 const page = (cookie: string) =>
168 http
169 .handle(new Request(`${url}/alice/lib/pulls/${guest.number}`, { headers: { cookie } }))
170 .then((r) => r.text());
171 const words = GUEST_WAITS.slice(0, 40);
172 const reader = await page(as(carol));
173 expect(reader).toContain(words);
174 expect(reader).not.toContain("approve-guest");
175 const writer = await page(as(alice));
176 expect(writer).toContain(words);
177 expect(writer).toContain("approve-guest");
178
179 const post = (cookie: string, origin: string) =>
180 http.handle(
181 new Request(`${url}/alice/lib/pulls/${guest.number}/approve-guest`, {
182 method: "POST",
183 headers: { cookie, origin, "content-type": "application/x-www-form-urlencoded" },
184 body: "",
185 }),
186 );
187 await post(as(alice), "http://evil.test");
188 expect(guestWaits(app.db, guest)).toBe(true);
189 await post(as(carol), url);
190 expect(guestWaits(app.db, guest)).toBe(true);
191 await post(as(alice), url);
192 expect(guestWaits(app.db, guest)).toBe(false);
193 // Announced, so an open page of the pull and the preview hear it.
194 expect(
195 listEvents(app.db, { repoId: lib.id, kind: "pull.guest_approved" }).some(
196 (e) => e.payload["number"] === guest.number,
197 ),
198 ).toBe(true);
199 expect(await page(as(alice))).not.toContain(words);
200 }, 30_000);
201
202 test("the terminal says it is a guest; only a writer gets G, and G approves it", async () => {
203 const guest = await pull("head");
204 const open = async (user: User) => {
205 const { app: tui, session } = createForgeApp(
206 app,
207 { sshUsername: user.username, remoteAddress: "127.0.0.1", fingerprint: `SHA256:${user.username}`, user },
208 { terminal: headlessTerminal(200, 44), exitHooks: false },
209 );
210 tui.start();
211 tui.ctx.push(pullScreen(session, tui.ctx, local(session, lib), guest.number));
212 const end = Date.now() + 20_000;
213 while (!tui.screen.toText().includes("a guest from another forge")) {
214 if (Date.now() > end) throw new Error(tui.screen.toText());
215 await settleApp(tui);
216 await Bun.sleep(20);
217 }
218 return tui;
219 };
220 const reader = await open(carol);
221 try {
222 expect(reader.screen.toText()).toContain(GUEST_NOTE.slice(0, 40));
223 expect(reader.screen.toText()).not.toContain("approve this guest");
224 reader.dispatch({ type: "key", key: "G", ctrl: false, alt: false, shift: true });
225 await settleApp(reader);
226 expect(guestWaits(app.db, guest)).toBe(true);
227 } finally {
228 reader.stop();
229 }
230 const writer = await open(alice);
231 try {
232 expect(writer.screen.toText()).toContain("approve this guest");
233 writer.dispatch({ type: "key", key: "G", ctrl: false, alt: false, shift: true });
234 await settleApp(writer);
235 expect(guestWaits(app.db, guest)).toBe(false);
236 expect(
237 listEvents(app.db, { repoId: lib.id, kind: "pull.guest_approved" }).some(
238 (e) => e.payload["number"] === guest.number,
239 ),
240 ).toBe(true);
241 } finally {
242 writer.stop();
243 }
244 }, 60_000);
245 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs