trevato/forge

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

More
Forks1g fMapg aRuns18g rEventsFlows

The terminal lists a repository's webhooks, adds one and removes one, never showing a secret, as MCP `hooks` does #7282

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/7243-the-terminal-lists-a-repository-s-webhoo → main 4 commits fork 3 threads · 3 unresolved

  • Merged. scout/7243-the-terminal-lists-a-repository-s-webhoo was merged into main as 0157dab.

Review

Sign in to review this change.
3 files changed +368 −0
src/tui/screens/hooks.ts added +150
@@ -0,0 +1,150 @@
1 /**
2 * A repository's webhooks in the terminal, an admin's: each hook's url,
3 * kinds and last delivery, never its secret. `a` asks for url, kinds and a
4 * secret (typed masked, 16–200 characters) and adds it through
5 * `createWebhook`; `d` removes the selected one after a confirm through
6 * `deleteWebhook`. The words of a refusal are MCP `hooks`'s.
7 */
8
9 import { createWebhook, deleteWebhook } from "../../actions/repos.ts";
10 import { canAdmin, type Repo } from "../../core/repos.ts";
11 import { listWebhooks, recentDeliveries, type Webhook } from "../../core/webhooks.ts";
12 import type { Ctx, Screen } from "../app/app.ts";
13 import type { Command } from "../app/palette.ts";
14 import { binding } from "../app/keys.ts";
15 import type { Session } from "../session.ts";
16 import { style } from "../term/style.ts";
17 import { truncate } from "../term/width.ts";
18 import { fill, len, split } from "../ui/layout.ts";
19 import { emptyState } from "../ui/widgets.ts";
20 import { ListState, confirmScreen, drawList, later, promptScreen, type ListRow } from "./common.ts";
21
22 const LIMITS = { url: 2048, kinds: 200, secret: 200 } as const;
23 const NOT_ADMIN = "no such app or repository, or you do not administer it";
24 const SHORT_SECRET = "secret is required: 16 to 200 characters, the key deliveries are signed with";
25
26 /** The command that opens the screen: offered to an admin of the repository only. */
27 export function hooksCommands(f: Session, ctx: Ctx, repo: Repo | undefined): Command[] {
28 if (repo === undefined || f.user === undefined || !canAdmin(f.db, repo, f.user)) return [];
29 return [
30 {
31 id: "webhooks",
32 title: `${repo.ownerName}/${repo.name} — Webhooks`,
33 hint: "list, add and remove webhooks",
34 run: () => ctx.push(hooksScreen(f, ctx, repo)),
35 },
36 ];
37 }
38
39 const clean = (s: string): string => s.replace(/[\p{Cc}\p{Cf}\u2028\u2029]/gu, "").trim();
40
41 function lastDelivery(f: Session, hook: Webhook): string {
42 const d = recentDeliveries(f.db, hook.id, 1)[0];
43 if (d === undefined) return "no delivery yet";
44 const how = d.lastStatus !== null ? ` ${d.lastStatus}` : d.lastError !== null ? ` ${clean(d.lastError)}` : "";
45 return `last ${d.kind}: ${d.state}${how}`;
46 }
47
48 export function hooksScreen(f: Session, ctx: Ctx, repo: Repo): Screen {
49 const state = new ListState();
50 const allowed = (): boolean => f.user !== undefined && canAdmin(f.db, repo, f.user);
51 const hooks = (): Webhook[] => (allowed() ? listWebhooks(f.db, repo.id) : []);
52 let note = "";
53
54 const add = (url: string, kinds: string, secret: string): string | undefined => {
55 if (secret.length < 16 || secret.length > LIMITS.secret) return SHORT_SECRET;
56 // The core's kinds are space-separated; a comma is accepted and turned into one.
57 const spec = kinds.split(/[\s,]+/).filter((k) => k !== "").join(" ");
58 const made = createWebhook(f.app, f.user, repo, { url, kinds: spec || "*", secret });
59 if (!made.ok) return made.error === "invalid_url" ? "a webhook url is https://…" : NOT_ADMIN;
60 note = `added ${clean(made.value.webhook.url)}`;
61 ctx.invalidate();
62 return undefined;
63 };
64
junipercrew outdated

Over-200 secret is refused here but untested; add a test.
65 const ask = (): void => {
junipercrew outdated

Core expects space-separated kinds; the prompt label at line ~77 says comma-separated and the value is passed through raw. Normalize here (split /[\s,]+/, join ' ') or hooks never match.
66 if (!allowed()) return;
67 ctx.push(
68 promptScreen(ctx, screen, {
69 title: "Add a webhook",
70 label: "url (https://…)",
71 onSubmit: (url) => {
72 if (url === "" || url.length > LIMITS.url) return `url is text of at most ${LIMITS.url} characters`;
73 return later(ctx, () =>
74 promptScreen(ctx, screen, {
75 title: "Add a webhook",
76 label: "kinds (e.g. push issue.*, empty for all)",
77 onSubmit: (kinds) => {
78 if (kinds.length > LIMITS.kinds) return `kinds is text of at most ${LIMITS.kinds} characters`;
79 return later(ctx, () =>
80 promptScreen(ctx, screen, {
81 title: "Add a webhook",
82 label: "secret (16–200 characters)",
83 mask: true,
84 onSubmit: (secret) => add(url, kinds, secret),
85 }),
86 );
87 },
88 }),
89 );
90 },
91 }),
92 );
93 };
94
95 const remove = (hook: Webhook): void =>
96 ctx.push(
97 confirmScreen(ctx, screen, {
98 title: "Remove webhook",
99 body: [truncate(clean(hook.url), 50), "Deliveries to it stop."],
100 actions: [{ label: "Cancel", variant: "ghost" }, { label: "Remove", variant: "danger" }],
101 onChoose: (i) => {
102 if (i !== 1) return;
103 const done = deleteWebhook(f.app, f.user, repo, hook.id);
104 note = done.ok ? `removed ${clean(hook.url)}` : "no such webhook here";
105 ctx.invalidate();
106 },
107 }),
108 );
109
110 const screen: Screen = {
111 title: "Webhooks",
112 keys: [
113 binding(["j", "k"], "move"),
114 binding(["a"], "add a webhook"),
115 binding(["d"], "remove the selected one"),
116 ],
117 render(area, s) {
118 const c = ctx.theme.c;
119 const [head, body] = split(area, "v", [len(1), fill(1)]);
120 const text = note !== "" ? note : `${repo.ownerName}/${repo.name} · webhooks (secrets are never shown)`;
121 s.text(head!.x, head!.y, truncate(text, head!.w), style(c.mutedFg, c.bg), head!.x + head!.w);
122 const all = hooks();
123 if (all.length === 0) {
124 emptyState(s, body!, c, {
125 title: allowed() ? "No webhooks" : "Not available",
126 body: allowed() ? "Press a to add one." : NOT_ADMIN,
127 });
128 return;
129 }
130 const rows: ListRow[] = all.map((h) => ({
131 title: clean(h.url),
132 secondary: `kinds ${clean(h.kinds)}`,
133 meta: lastDelivery(f, h),
134 }));
135 drawList(s, body!, c, state, rows);
136 },
137 key(e) {
138 if (state.key(e, hooks().length)) return true;
139 if (e.ctrl || e.alt || !allowed()) return false;
140 if (e.key === "a") ask();
141 else if (e.key === "d") {
142 const hook = hooks()[state.index];
143 if (hook === undefined) return false;
144 remove(hook);
145 } else return false;
146 return true;
147 },
148 };
149 return screen;
150 }
src/tui/screens/workspace.ts +2
@@ -99,6 +99,7 @@ import { canSeeAnswers } from "../../web/studio/answers.ts";
9999 import { answersScreen } from "./answers.ts";
100100 import { commitsScreen } from "./commits.ts";
101101 import { deploysScreen } from "./deploys.ts";
102 import { hooksCommands } from "./hooks.ts";
102103 import { runsScreen } from "./runs.ts";
103104 import {
104105 Filter,
@@ -1288,6 +1289,7 @@ export function workspaceScreen(
12881289 ...reportsCommands(f, ctx, repo),
12891290 ...noteCommands(f, ctx, () => screen, repo),
12901291 ...yoursCommands(f, ctx, repo),
1292 ...hooksCommands(f, ctx, repo),
12911293 ...visionCommands(f, ctx, repo),
12921294 ...appEnvCommands(f, ctx, repo),
12931295 ],
test/tui-hooks.test.ts added +216
@@ -0,0 +1,216 @@
1 /**
2 * A repository's webhooks in the terminal (#7243): an admin lists, adds and
3 * removes; a short secret is refused in MCP `hooks`'s words; whoever does not
4 * admin the repository is never offered the screen; no frame shows a secret.
5 */
6 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
7 import { mkdtemp, rm } from "node:fs/promises";
8 import { tmpdir } from "node:os";
9 import { join } from "node:path";
10 import { createRepo } from "../src/actions/repos.ts";
11 import { createApp, type App } from "../src/app.ts";
12 import { canAdmin, canWrite, type Repo } from "../src/core/repos.ts";
13 import { createOrg, createTeam, addTeamMember } from "../src/core/orgs.ts";
14 import { createUser, type User } from "../src/core/users.ts";
15 import { enqueueForEvent, listWebhooks } from "../src/core/webhooks.ts";
16 import { createForgeApp } from "../src/tui/forge.ts";
17 import { hooksCommands } from "../src/tui/screens/hooks.ts";
18 import { repoScreen } from "../src/tui/screens/repo.ts";
19 import { local } from "../src/tui/screens/where.ts";
20 import { headlessTerminal } from "../src/tui/term/terminal.ts";
21 import { quietLogs } from "./helpers/globals.ts";
22 import { settleApp } from "./helpers/tui.ts";
23 import { proveAll } from "./helpers/verified.ts";
24
25 quietLogs();
26
27 let dir: string;
28 let app: App;
29 let alice: User;
30 let bob: User;
31 let repo: Repo;
32
33 /** Open Webhooks from the palette once its row has drawn. */
34 async function palette(h: ReturnType<typeof open>): Promise<void> {
35 await h.feed(":", ..."Webhooks");
36 for (let i = 0; i < 100 && !h.text().includes("list, add and remove webhooks"); i++)
37 await new Promise<void>((r) => setTimeout(r, 10)), await h.feed();
38 await h.feed("enter");
39 }
40
41 const SECRET = "s3cret-signing-key-0123456789";
42
43 const KEYS: Record<string, string> = { enter: "\r", right: "\x1b[C" };
44
45 function open(user: User, at: Repo = repo) {
46 const terminal = headlessTerminal(120, 40);
47 const { app: tui, session } = createForgeApp(
48 app,
49 { sshUsername: user.username, remoteAddress: "127.0.0.1", fingerprint: "SHA256:test", user },
50 { terminal, exitHooks: false },
51 );
52 tui.start();
53 tui.ctx.push(repoScreen(session, tui.ctx, local(session, at)));
54 const frames: string[] = [];
55 const text = (): string => {
56 const t = tui.screen.toText();
57 frames.push(t);
58 return t;
59 };
60 return {
61 session,
62 ctx: tui.ctx,
63 frames,
64 text,
65 feed: async (...keys: string[]) => {
66 for (const k of keys) {
67 terminal.feed(KEYS[k] ?? k);
68 await settleApp(tui);
69 }
70 },
71 stop: () => tui.stop?.(),
72 };
73 }
74
75 beforeAll(async () => {
76 dir = await mkdtemp(join(tmpdir(), "forge-tui-hooks-"));
77 app = createApp({ dataDir: dir, port: 0 });
78 const mk = async (username: string, first = false) => {
79 const r = await createUser(app.db, {
80 origin: "seed",
81 trusted: true,
82 username,
83 email: `${username}@forge.local`,
84 password: "a-good-passphrase",
85 adminIfFirst: first,
86 });
87 if (!r.ok) throw new Error(r.error);
88 return r.value;
89 };
90 alice = await mk("alice", true);
91 bob = await mk("bob");
92 proveAll(app);
93 const made = await createRepo(app, alice, { name: "lobby", visibility: "public" });
94 if (!made.ok) throw new Error(String(made.error));
95 repo = made.value;
96 });
97 afterAll(async () => {
98 app.db.close();
99 await rm(dir, { recursive: true, force: true });
100 });
101
102 describe("webhooks in the terminal", () => {
103 test("an admin lists, adds and removes a hook; no frame shows the secret", async () => {
104 const h = open(alice);
105 await palette(h);
106 expect(h.text()).toContain("No webhooks");
107
108 await h.feed("a", ..."https://example.com/hook", "enter");
109 await h.feed(..."push", "enter");
110 await h.feed(...SECRET, "enter");
111 const hooks = listWebhooks(app.db, repo.id);
112 expect(hooks.length).toBe(1);
113 expect(hooks[0]!.url).toBe("https://example.com/hook");
114 expect(hooks[0]!.kinds).toBe("push");
115 expect(h.text()).toContain("https://example.com/hook");
116 expect(h.text()).toContain("kinds push");
117 expect(h.text()).toContain("no delivery yet");
118
119 await h.feed("d");
120 expect(h.text()).toContain("Remove webhook");
121 await h.feed("right", "enter");
122 expect(listWebhooks(app.db, repo.id).length).toBe(0);
123 expect(h.text()).toContain("No webhooks");
124 expect(h.frames.join("\n")).not.toContain(SECRET);
125 h.stop();
126 });
127
128 test("a short secret is refused in MCP's words and nothing is added", async () => {
129 const h = open(alice);
130 await palette(h);
131 await h.feed("a", ..."https://example.com/x", "enter", "enter");
132 await h.feed(..."tooshort", "enter");
133 expect(h.text()).toContain("secret is required: 16 to 200 characters");
134 expect(listWebhooks(app.db, repo.id).length).toBe(0);
135 expect(h.frames.join("\n")).not.toContain("tooshort");
136 h.stop();
137 });
138
139 test("a refused url says so and nothing is added", async () => {
140 const h = open(alice);
141 await palette(h);
142 await h.feed("a", ..."http://example.com/x", "enter", "enter");
143 await h.feed(...SECRET, "enter");
144 expect(h.text()).toContain("a webhook url is https://…");
145 expect(listWebhooks(app.db, repo.id).length).toBe(0);
146 h.stop();
147 });
junipercrew outdated

Use a writer who is not an admin here (or add a second case): that is the gate that can actually go wrong.
148
149 test("someone who does not admin the repository is never offered it", async () => {
150 const h = open(bob);
151 await h.feed(":", ..."Webhooks");
152 for (let i = 0; i < 100 && h.text().includes("searching…"); i++)
153 await new Promise<void>((r) => setTimeout(r, 10)), await h.feed();
154 expect(h.text()).not.toContain("list, add and remove webhooks");
155 h.stop();
156 });
157
158 test("comma- or space-separated kinds are stored space-separated", async () => {
159 const h = open(alice);
160 await palette(h);
161 await h.feed("a", ..."https://example.com/two", "enter", ..."push,issue.*", "enter", ...SECRET, "enter");
162 const hook = listWebhooks(app.db, repo.id).find((x) => x.url.endsWith("/two"))!;
163 expect(hook.kinds).toBe("push issue.*");
164 expect(h.text()).toContain("kinds push issue.*");
165 h.stop();
166 });
167
168 test("a url, kinds or secret over its length is refused and nothing is added", async () => {
169 const before = listWebhooks(app.db, repo.id).length;
170 let h = open(alice);
171 await palette(h);
172 await h.feed("a", "https://example.com/" + "x".repeat(2040), "enter");
173 expect(h.text()).toContain("url is text of at most 2048 characters");
174 h.stop();
175 h = open(alice);
176 await palette(h);
177 await h.feed("a", ..."https://example.com/k", "enter", "k".repeat(201), "enter");
178 expect(h.text()).toContain("kinds is text of at most 200 characters");
179 h.stop();
180 h = open(alice);
181 await palette(h);
182 await h.feed("a", ..."https://example.com/s", "enter", "enter", "s".repeat(201), "enter");
183 expect(h.text()).toContain("secret is required: 16 to 200 characters");
184 expect(listWebhooks(app.db, repo.id).length).toBe(before);
185 expect(h.frames.join("\n")).not.toContain("sssssssssss");
186 h.stop();
187 });
188
189 test("a delivery row shows as the last delivery", async () => {
190 const event = app.bus.append({ kind: "push", repoId: repo.id, actorId: alice.id, payload: {} });
191 enqueueForEvent(app.db, event);
192 const h = open(alice);
193 await palette(h);
194 expect(h.text()).toContain("last push: pending");
195 h.stop();
196 });
197
198 test("a writer who is not an admin is never offered it and cannot reach it", async () => {
199 const org = createOrg(app.db, "acme", alice.id);
200 if (!org.ok) throw new Error(org.error);
201 const team = createTeam(app.db, org.value.id, "devs", "write");
202 if (!team.ok) throw new Error(team.error);
203 addTeamMember(app.db, team.value, bob.id);
204 const made = await createRepo(app, alice, { owner: "acme", name: "shop", visibility: "public" });
205 if (!made.ok) throw new Error(String(made.error));
206 expect(canWrite(app.db, made.value, bob)).toBe(true);
207 expect(canAdmin(app.db, made.value, bob)).toBe(false);
208 const h = open(bob, made.value);
209 await h.feed(":", ..."Webhooks");
210 for (let i = 0; i < 100 && h.text().includes("searching…"); i++)
211 await new Promise<void>((r) => setTimeout(r, 10)), await h.feed();
212 expect(h.text()).not.toContain("list, add and remove webhooks");
213 expect(hooksCommands(h.session, h.ctx, made.value)).toEqual([]);
214 h.stop();
215 });
216 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs