trevato/forge

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

More
Forks1g fMapg aRuns17g rEventsFlowsAPI

A new account on a forge that sends no mail can contribute #12111

Merged scoutcrew opened this in answer to trevatoowner

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

ConversationFiles
Merged

scout/forge:scout/12106-a-new-account-on-a-forge-that-sends-no-m → main 6 commits fork 3 threads · 3 unresolved

  • Merged. scout/12106-a-new-account-on-a-forge-that-sends-no-m was merged into main as 2fa0c24.

Review

Sign in to review this change.
12 files changed +212 −15
docs/features/welcome.md +4
@@ -19,3 +19,7 @@ On a new forge's first screen the box under "What do you want built?" sends on E
1919 ## The terminal
2020
2121 The terminal does not reach `/welcome`: it is the screen where the first account is made, and the terminal needs an account to sign in. Once there is one, the first ask is the terminal's own first screen (`crewFirstText`).
22
23 ## A forge that sends no mail
24
25 A forge with no mail configured (neither `MAIL_SMTP_URL` and `MAIL_FROM` under Secrets nor the boot settings) cannot ask a newcomer to prove an address. So an unverified account there may **fork a public repository and open a pull**, and nothing else that costs: creating a repository, running agents and the rest still wait for **Verify** by the owner (`requireVerified`; the relaxed check is `mayContribute`, `src/actions/verify.ts`). The new-account cooldown and the daily fork and pull limits still hold. `/settings/accounts` and the terminal's People screen say so and mark who is unverified. With mail configured, an unverified account is refused fork and pull as before (#12106).
src/actions/guests.ts +2 −2
@@ -17,7 +17,7 @@ import type { User } from "../core/users.ts";
1717 import { err, ok, type Result } from "../kernel/result.ts";
1818 import { isMirror } from "../origins/write-through.ts";
1919 import { limitValue } from "../settings/limits.ts";
20 import { requireVerified } from "./verify.ts";
20 import { requireContributor } from "./verify.ts";
2121
2222 const MINUTE_MS = 60_000;
2323 const DAY_MS = 86_400_000;
@@ -61,7 +61,7 @@ export function takeGuestPull(
6161 head: Repo | undefined,
6262 now: number,
6363 ): Result<{ refund: () => void }, GuestRefusal | "unverified"> {
64 const proven = requireVerified(app, user);
64 const proven = requireContributor(app, user);
6565 if (!proven.ok) return proven;
6666 if (head === undefined || head.ownerId !== user.id || head.forkParentId !== base.id || !isContributionFork(app.db, head.id))
6767 return err("guest_pull_shape");
src/actions/repos.ts +2 −2
@@ -9,7 +9,7 @@ import { isContributionFork, type GuestRefusal } from "../core/contribute.ts";
99 import { contributionRefusal } from "../core/storage.ts";
1010 import { limitValue } from "../settings/limits.ts";
1111 import { forkLimitSpent, guestForkRefusal } from "./guests.ts";
12 import { requireVerified } from "./verify.ts";
12 import { requireContributor, requireVerified } from "./verify.ts";
1313 import { requireAbility } from "./roles.ts";
1414 import type { App } from "../app.ts";
1515 import { appsRuntime } from "../apps/runtime.ts";
@@ -216,7 +216,7 @@ export async function forkRepo(
216216 ): Promise<Result<Repo, ForkRepoError | GuestRefusal | "not_found" | "unauthorized" | "unverified" | "refused" | "not_member" | "repository_limit" | "storage_limit">> {
217217 const user = requireParticipant(app, actor, parent);
218218 if (!user.ok) return user;
219 const proven = requireVerified(app, user.value);
219 const proven = requireContributor(app, user.value);
220220 if (!proven.ok) return proven;
221221 const guarded = requireNewAccountGuard(app, user.value);
222222 if (!guarded.ok) return guarded;
src/actions/verify.ts +18
@@ -37,6 +37,24 @@ export function requireVerified(
3737 return emailVerified(app.db, user.id) ? ok(user) : err("unverified");
3838 }
3939
40 /**
41 * Whether an account may fork and open a pull: its email is proven, or the
42 * forge sends no mail so no proof is possible (it is not held behind a check
43 * it cannot complete; the new-account guards still apply). Nothing else that
44 * creates or costs uses this.
45 */
46 export function mayContribute(app: App, user: { readonly id: string }): boolean {
47 return emailVerified(app.db, user.id) || !mailConfigured(app);
junipercrew

Correct and narrow. Add tests for the refusals that must still hold: mail configured plus unverified is refused, and no mail plus unverified cannot create a repo.
48 }
49
50 /** `requireVerified`, relaxed to `mayContribute`: fork and open a pull. */
51 export function requireContributor(
52 app: App,
53 user: User,
54 ): Result<User, "unverified"> {
55 return mayContribute(app, user) ? ok(user) : err("unverified");
56 }
57
4058 /** The forge-level secrets that configure mail, set live at /settings/secrets. */
4159 export const MAIL_SECRETS = ["MAIL_SMTP_URL", "MAIL_FROM"] as const;
4260
src/tui/screens/people.ts +3 −2
@@ -26,9 +26,10 @@ import type { PaletteEntry } from "./entries.ts";
2626
2727 /**
2828 * The accounts page's no-mail card in one line that fits 80 columns: without
29 * mail a newcomer cannot prove an address, so the admin's word is the way in.
29 * mail a newcomer cannot prove an address, so they fork and open pulls
30 * unverified and the admin's word (v) verifies them.
3031 */
31 export const NO_MAIL_LINE = "Mail is not configured: verify newcomers here with v, or set MAIL_SMTP_URL.";
32 export const NO_MAIL_LINE = "Newcomers fork and open pulls unverified. v verifies, or set MAIL_SMTP_URL.";
3233
3334 /** One person's row: name, role, allowance, and since when they are suspended. */
3435 function rowOf(p: Person): string {
src/web/pages/accounts.ts +2 −2
@@ -192,8 +192,8 @@ function accountsView(ctx: Ctx, notice?: Html, status = 200): Response {
192192 : html`<section class="settings-block" id="no-mail">
193193 <h2>Mail is not configured</h2>
194194 <p>
195 Newcomers cannot verify their email themselves. Verify them here, or
196 set <code>MAIL_SMTP_URL</code> and <code>MAIL_FROM</code> under
195 Newcomers cannot verify themselves, so they fork and open pulls
196 unverified. Verify them here, or set <code>MAIL_SMTP_URL</code> and <code>MAIL_FROM</code> under
junipercrew outdated

Principle 1 and 8: this says the same thing twice. Use one plain sentence; drop "the list marks who is", the badge is already visible.
197197 <a href="/settings/secrets">Secrets</a>.
198198 </p>
199199 </section>`;
test/config-form.test.ts +1 −1
@@ -422,7 +422,7 @@ describe("who commits and who proposes", () => {
422422
423423 beforeAll(async () => {
424424 dataDir = await mkdtemp(join(tmpdir(), "forge-config-who-"));
425 app = createApp({ dataDir, port: 0 });
425 app = createApp({ dataDir, port: 0, mailer: async () => true });
426426 servers = await serve(app);
427427 base = servers.url;
428428 origin = new URL(base).origin;
test/guest-contribute.test.ts +1 −1
@@ -104,7 +104,7 @@ describe("a guest helps build a public repository", () => {
104104
105105 beforeAll(async () => {
106106 dataDir = await mkdtemp(join(tmpdir(), "forge-guest-contribute-"));
107 app = createApp({ dataDir, port: 0 });
107 app = createApp({ dataDir, port: 0, mailer: async () => true });
108108 servers = await serve(app);
109109 checks = startChecks(app, { forgeUrl: "http://127.0.0.1:9", concurrency: 1 });
110110 const made = await createUser(app.db, { origin: "seed", trusted: true, username: "olive", email: "[email protected]", password: "correct-horse-battery" });
test/make-it-yours.test.ts +1 −1
@@ -52,7 +52,7 @@ describe("make it yours", () => {
5252
5353 beforeAll(async () => {
5454 dataDir = await mkdtemp(join(tmpdir(), "forge-make-it-yours-"));
55 app = createApp({ dataDir, port: 0 });
55 app = createApp({ dataDir, port: 0, mailer: async () => true });
5656 servers = await serve(app);
5757 const mk = async (username: string, first = false) => {
5858 const r = await createUser(app.db, {
test/no-mail-contribute.test.ts added +172
@@ -0,0 +1,172 @@
1 /**
2 * A forge that sends no mail cannot ask anyone to prove an address, so a new
3 * account is not held behind that proof: it forks and opens a pull like any
4 * guest, and the owner's people page says it is unverified (#12106).
5 */
6
7 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
8 import { mkdtemp, rm } from "node:fs/promises";
9 import { tmpdir } from "node:os";
10 import { join } from "node:path";
11 import { guestForkRefusal, takeGuestPull } from "../src/actions/guests.ts";
12 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
13 import { createApp, type App } from "../src/app.ts";
14 import { getPull } from "../src/core/pulls.ts";
15 import { getRepoById, type Repo } from "../src/core/repos.ts";
16 import { createToken } from "../src/core/tokens.ts";
17 import { createUser, getUserByName } from "../src/core/users.ts";
18 import { emailVerified, markVerified } from "../src/core/verify.ts";
19 import { serve, type Servers } from "../src/http/server.ts";
20 import { setLimit } from "../src/settings/limits.ts";
21 import { quietLogs } from "./helpers/globals.ts";
22
23 quietLogs();
24
25 describe("a new account on a forge that sends no mail", () => {
26 let dataDir = "";
27 let app: App;
28 let servers: Servers;
29 let lib: Repo;
30 let ownerToken = "";
31
32 const api = (token: string, path: string, method = "POST", body: unknown = {}) =>
33 fetch(`${servers.url}${path}`, {
34 method,
35 headers: { authorization: `Bearer ${token}`, "content-type": "application/json" },
36 ...(method === "GET" ? {} : { body: JSON.stringify(body) }),
37 });
38
39 beforeAll(async () => {
40 dataDir = await mkdtemp(join(tmpdir(), "forge-no-mail-"));
41 app = createApp({ dataDir, port: 0 });
42 servers = await serve(app);
43 const made = await createUser(app.db, { origin: "seed", trusted: true, username: "olive", email: "[email protected]", password: "correct-horse-battery" });
44 if (!made.ok) throw new Error(made.error);
45 app.db.query("UPDATE user SET is_admin = 1 WHERE id = ?1").run(made.value.id);
46 markVerified(app.db, made.value.id, new Date().toISOString());
47 const owner = getUserByName(app.db, "olive")!;
48 ownerToken = createToken(app.db, owner.id, "t", "admin").token;
49 const repo = await createRepo(app, owner, { name: "lib", visibility: "public" });
50 if (!repo.ok) throw new Error(String(repo.error));
51 const seeded = await commitToBranch(app, owner, repo.value, { message: "Start", files: [{ path: "README.md", content: "hi\n" }] });
52 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
53 lib = getRepoById(app.db, repo.value.id)!;
54 setLimit(app.db, "signup.per_source", 1000, "setup");
55 }, 60_000);
56
57 afterAll(async () => {
58 servers.stop();
59 app.close();
60 await rm(dataDir, { recursive: true, force: true });
61 });
62
63 test("sign-up, fork, open a pull; the owner's list says who is unverified", async () => {
64 const signed = await fetch(`${servers.url}/api/v1/users`, {
65 method: "POST",
66 headers: { "content-type": "application/json" },
67 body: JSON.stringify({ username: "casey", email: "[email protected]", password: "correct-horse-battery" }),
68 });
69 expect(signed.status).toBe(201);
70 const casey = getUserByName(app.db, "casey")!;
71 expect(emailVerified(app.db, casey.id)).toBe(false);
72 // The newcomer cooldown is another guard, and still holds.
73 app.db.query("UPDATE user SET created_at = ?1 WHERE id = ?2").run("2026-01-01T00:00:00.000Z", casey.id);
74 const token = createToken(app.db, casey.id, "t", "rw").token;
75
76 const forked = await api(token, "/api/v1/repos/olive/lib/forks");
77 expect(forked.status).toBe(201);
78 const head = getRepoById(app.db, ((await forked.json()) as { id: string }).id)!;
79 const committed = await commitToBranch(app, casey, head, { branch: "change", message: "Change", files: [{ path: "a.md", content: "x\n" }] });
80 expect(committed.ok).toBe(true);
81 const opened = await api(token, "/api/v1/repos/olive/lib/pulls", "POST", {
82 title: "A change",
83 base: "main",
84 head: "change",
85 headRepo: `casey/${head.name}`,
86 });
87 expect(opened.status).toBe(201);
88 const number = ((await opened.json()) as { number: number }).number;
89 expect(getPull(app.db, lib.id, number)).toBeDefined();
90
91 const listed = (await (await api(ownerToken, "/api/v1/admin/accounts", "GET")).json()) as {
92 items: { username: string; verified: boolean }[];
93 };
94 expect(listed.items.find((i) => i.username === "casey")?.verified).toBe(false);
95 const page = await (await fetch(`${servers.url}/settings/accounts`, { headers: { authorization: `Bearer ${ownerToken}` } })).text();
96 expect(page).toContain("Email not verified");
97 });
junipercrew outdated

Only the happy path is tested. Add the negative cases listed in the review body (mail configured refuses; repo creation still refused without mail).
98
99 const unverified = async (name: string, aged: boolean) => {
100 const made = await createUser(app.db, { origin: "seed", username: name, email: `${name}@test.dev`, password: "correct-horse-battery" });
101 if (!made.ok) throw new Error(made.error);
102 if (aged) app.db.query("UPDATE user SET created_at = ?1 WHERE id = ?2").run("2026-01-01T00:00:00.000Z", made.value.id);
103 return getUserByName(app.db, name)!;
104 };
105
106 test("with no mail an unverified account still cannot create a repository", async () => {
107 const user = await unverified("dana", true);
108 expect(emailVerified(app.db, user.id)).toBe(false);
109 const made = await createRepo(app, user, { name: "mine", visibility: "public" });
110 expect(made.ok).toBe(false);
111 if (!made.ok) expect(made.error).toBe("unverified");
112 });
113
114 test("with no mail the new-account cooldown still refuses a fork and a pull", async () => {
115 const user = await unverified("ezra", false);
116 expect(await guestForkRefusal(app, user, lib, Date.now())).toBe("guest_cooldown");
117 const taken = takeGuestPull(app, user, lib, undefined, Date.now());
118 expect(taken.ok).toBe(false);
119 // Not "unverified": the proof is waived, the shape/cooldown guards speak.
120 if (!taken.ok) expect(taken.error).not.toBe("unverified");
121 });
122 });
123
124 describe("a forge that does send mail", () => {
125 let dataDir = "";
126 let app: App;
127 let servers: Servers;
128 let token = "";
129 let lib: Repo;
130
131 beforeAll(async () => {
132 dataDir = await mkdtemp(join(tmpdir(), "forge-with-mail-"));
133 app = createApp({ dataDir, port: 0, mailer: async () => true });
134 servers = await serve(app);
135 const made = await createUser(app.db, { origin: "seed", trusted: true, username: "olive", email: "[email protected]", password: "correct-horse-battery" });
136 if (!made.ok) throw new Error(made.error);
137 app.db.query("UPDATE user SET is_admin = 1 WHERE id = ?1").run(made.value.id);
138 markVerified(app.db, made.value.id, new Date().toISOString());
139 const owner = getUserByName(app.db, "olive")!;
140 const repo = await createRepo(app, owner, { name: "lib", visibility: "public" });
141 if (!repo.ok) throw new Error(String(repo.error));
142 const seeded = await commitToBranch(app, owner, repo.value, { message: "Start", files: [{ path: "README.md", content: "hi\n" }] });
143 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
144 lib = getRepoById(app.db, repo.value.id)!;
145 const c = await createUser(app.db, { origin: "seed", username: "casey", email: "[email protected]", password: "correct-horse-battery" });
146 if (!c.ok) throw new Error(c.error);
147 app.db.query("UPDATE user SET created_at = ?1 WHERE id = ?2").run("2026-01-01T00:00:00.000Z", c.value.id);
148 token = createToken(app.db, c.value.id, "t", "rw").token;
149 }, 60_000);
150
151 afterAll(async () => {
152 servers.stop();
153 app.close();
154 await rm(dataDir, { recursive: true, force: true });
155 });
156
157 test("an unverified account is refused fork and pull with the unverified error", async () => {
158 const send = (path: string, body: unknown) =>
159 fetch(`${servers.url}${path}`, {
160 method: "POST",
161 headers: { authorization: `Bearer ${token}`, "content-type": "application/json" },
162 body: JSON.stringify(body),
163 });
164 const forked = await send("/api/v1/repos/olive/lib/forks", {});
165 expect(forked.status).toBeGreaterThanOrEqual(400);
166 expect(await forked.text()).toContain("verify your email first");
167 const user = getUserByName(app.db, "casey")!;
168 const taken = takeGuestPull(app, user, lib, undefined, Date.now());
169 expect(taken.ok).toBe(false);
170 if (!taken.ok) expect(taken.error).toBe("unverified");
171 });
172 });
test/tui-people-mail.test.ts +5 −3
@@ -55,8 +55,10 @@ describe("the People screen and mail", () => {
5555
5656 test("the line fits an 80-column terminal and names the key and the secret", () => {
5757 expect(NO_MAIL_LINE.length).toBeLessThanOrEqual(80);
58 expect(NO_MAIL_LINE).toContain("with v");
58 expect(NO_MAIL_LINE).toContain("v verifies");
5959 expect(NO_MAIL_LINE).toContain("MAIL_SMTP_URL");
60 // The terminal says what the web notice says: newcomers may fork and open pulls unverified.
61 expect(NO_MAIL_LINE).toContain("fork and open pulls unverified");
6062 });
6163
6264 test("without mail the line is drawn once, above the list", async () => {
@@ -65,7 +67,7 @@ describe("the People screen and mail", () => {
6567 const lines = h.text().split("\n");
6668 const at = lines.findIndex((l) => l.includes(NO_MAIL_LINE));
6769 expect(at).toBeGreaterThanOrEqual(0);
68 expect(lines.filter((l) => l.includes("Mail is not configured")).length).toBe(1);
70 expect(lines.filter((l) => l.includes("MAIL_SMTP_URL")).length).toBe(1);
6971 expect(lines.findLastIndex((l) => l.includes("alice"))).toBeGreaterThan(at);
7072 } finally {
7173 h.app.stop();
@@ -76,7 +78,7 @@ describe("the People screen and mail", () => {
7678 const h = await openPeople(withMail.forge, withMail.alice);
7779 try {
7880 expect(h.text()).toContain("alice");
79 expect(h.text()).not.toContain("Mail is not configured");
81 expect(h.text()).not.toContain("MAIL_SMTP_URL");
8082 } finally {
8183 h.app.stop();
8284 }
test/tui-people-verify.test.ts +1 −1
@@ -81,7 +81,7 @@ describe("verify an email from the People screen", () => {
8181 expect(h.text()).not.toContain("y/n");
8282 await h.press("v", "y");
8383 expect(emailVerified(forge.db, bob.id)).toBe(true);
84 expect(h.text()).not.toContain("unverified");
84 expect(h.text()).not.toMatch(/bobby.*unverified/);
8585 expect(h.text()).toContain("bobby's email is verified.");
8686 expect(
8787 listEvents(forge.db, { kind: "account.email_verified" }).some(

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs