trevato/forge

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

More
Forks1g fMapg aRuns15g rEventsFlows

GET and POST /api/v1/repos/:owner/:name/app/notes: an app's admin lists the forge button's notes, hides one, bans or unbans its sender #7229

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/7218-get-and-post-api-v1-repos-owner-name-app → main 5 commits fork 2 threads · 2 unresolved

  • Merged. scout/7218-get-and-post-api-v1-repos-owner-name-app was merged into main as ee793ab.

Review

Sign in to review this change.
10 files changed +256 −1
docs/APPS.md +5 −1
@@ -104,7 +104,11 @@ Under the box, four starter sentences are plain links (no script):
104104 app's studio page are the owner's tools: the page lists the latest notes
105105 (sender, words, page, when) for those who may admin the repository, each with
106106 Hide and Ban sender, and every banned sender once with Unban, posting to
107 `POST /:owner/:name/app/notes` on the forge's origin (`web/studio/notes.ts`).
107 `POST /:owner/:name/app/notes` on the forge's origin (`web/studio/notes.ts`). The same moderation is an API for
108 agents and scripts (`http/api/app_notes.ts`): `GET /api/v1/repos/:owner/:name/app/notes`
109 answers `{ notes, banned }`, and `POST …/app/notes/:id/hide`, `…/ban` and
110 `…/unban` each answer the note. They need an `rw` token (or a same-origin
111 session) and admin rights on the repository; anyone else gets `forbidden`.
108112 Limits count the socket's client (`http/peer.ts`, with its trusted-proxy
109113 rule), never a header the client wrote, and all three are checked before any
110114 is spent. The request body is read only up to 16 KB. The issue says its text
src/actions/app_notes.ts +7
@@ -318,3 +318,10 @@ export function unbanSender(app: App, record: AppRecord, repo: Repo, actor: User
318318 unbanVisitor(app.db, record.id, note.visitor);
319319 return ok(true);
320320 }
321
322 /** One note as the owner sees it, hidden or not: what a moderation answers. Admins only. */
323 export function ownerNote(app: App, record: AppRecord, repo: Repo, actor: User | undefined, noteId: string): OwnerNote | undefined {
324 if (actor === undefined || !canAdmin(app.db, repo, actor)) return undefined;
325 const note = getNote(app.db, record.id, noteId);
326 return note === undefined ? undefined : ownerView(app, repo, note, isBanned(app.db, record.id, note.visitor));
327 }
src/contract/openapi/routes/getApiV1ReposOwnerNameAppNotes.ts added +12
@@ -0,0 +1,12 @@
1 import { REPO, type Route } from "../shared.ts";
2
3 export const route: Route = {
4 method: "get",
5 path: `${REPO}/app/notes`,
6 summary: "List the notes sent from the app's forge button (rw scope; admin of the repository); answers { notes, banned }",
7 auth: true,
8 refusals: {
9 not_found: "no such repository, or it is not an app",
10 forbidden: "not an admin of the repository, or a read-only token",
11 },
12 };
src/contract/openapi/routes/index.ts +4
@@ -75,6 +75,7 @@ export { route as getApiV1ReposOwnerNameAppDomains } from "./getApiV1ReposOwnerN
7575 export { route as getApiV1ReposOwnerNameAppEnv } from "./getApiV1ReposOwnerNameAppEnv.ts";
7676 export { route as getApiV1ReposOwnerNameAppEnvOpen } from "./getApiV1ReposOwnerNameAppEnvOpen.ts";
7777 export { route as getApiV1ReposOwnerNameAppMembers } from "./getApiV1ReposOwnerNameAppMembers.ts";
78 export { route as getApiV1ReposOwnerNameAppNotes } from "./getApiV1ReposOwnerNameAppNotes.ts";
7879 export { route as getApiV1ReposOwnerNameAppProductionOpen } from "./getApiV1ReposOwnerNameAppProductionOpen.ts";
7980 export { route as getApiV1ReposOwnerNameBranches } from "./getApiV1ReposOwnerNameBranches.ts";
8081 export { route as getApiV1ReposOwnerNameCommits } from "./getApiV1ReposOwnerNameCommits.ts";
@@ -203,6 +204,9 @@ export { route as postApiV1ReposOwnerNameAppDomains } from "./postApiV1ReposOwne
203204 export { route as postApiV1ReposOwnerNameAppDomainsHostCheck } from "./postApiV1ReposOwnerNameAppDomainsHostCheck.ts";
204205 export { route as postApiV1ReposOwnerNameAppEnvRedeploy } from "./postApiV1ReposOwnerNameAppEnvRedeploy.ts";
205206 export { route as postApiV1ReposOwnerNameAppEnvStop } from "./postApiV1ReposOwnerNameAppEnvStop.ts";
207 export { route as postApiV1ReposOwnerNameAppNotesIdBan } from "./postApiV1ReposOwnerNameAppNotesIdBan.ts";
208 export { route as postApiV1ReposOwnerNameAppNotesIdHide } from "./postApiV1ReposOwnerNameAppNotesIdHide.ts";
209 export { route as postApiV1ReposOwnerNameAppNotesIdUnban } from "./postApiV1ReposOwnerNameAppNotesIdUnban.ts";
206210 export { route as postApiV1ReposOwnerNameBranches } from "./postApiV1ReposOwnerNameBranches.ts";
207211 export { route as postApiV1ReposOwnerNameChecksRecheck } from "./postApiV1ReposOwnerNameChecksRecheck.ts";
208212 export { route as postApiV1ReposOwnerNameContents } from "./postApiV1ReposOwnerNameContents.ts";
src/contract/openapi/routes/postApiV1ReposOwnerNameAppNotesIdBan.ts added +12
@@ -0,0 +1,12 @@
1 import { REPO, type Route } from "../shared.ts";
2
3 export const route: Route = {
4 method: "post",
5 path: `${REPO}/app/notes/{id}/ban`,
6 summary: "Hide a note and ban its sender from this app (rw scope; admin of the repository); answers the note",
7 auth: true,
8 refusals: {
9 not_found: "no such repository, app or note",
10 forbidden: "not an admin of the repository, or a read-only token",
11 },
12 };
src/contract/openapi/routes/postApiV1ReposOwnerNameAppNotesIdHide.ts added +12
@@ -0,0 +1,12 @@
1 import { REPO, type Route } from "../shared.ts";
2
3 export const route: Route = {
4 method: "post",
5 path: `${REPO}/app/notes/{id}/hide`,
6 summary: "Hide a note, closing its issue (rw scope; admin of the repository); answers the note",
7 auth: true,
8 refusals: {
9 not_found: "no such repository, app or note",
10 forbidden: "not an admin of the repository, or a read-only token",
11 },
12 };
src/contract/openapi/routes/postApiV1ReposOwnerNameAppNotesIdUnban.ts added +12
@@ -0,0 +1,12 @@
1 import { REPO, type Route } from "../shared.ts";
2
3 export const route: Route = {
4 method: "post",
5 path: `${REPO}/app/notes/{id}/unban`,
6 summary: "Let the sender of a note write again (rw scope; admin of the repository); answers the note",
7 auth: true,
8 refusals: {
9 not_found: "no such repository, app or note",
10 forbidden: "not an admin of the repository, or a read-only token",
11 },
12 };
src/http/api.ts +2
@@ -413,6 +413,7 @@ import { agentRoutes } from "./api/agents.ts";
413413 import { boardRoutes } from "./api/boards.ts";
414414 import { followingRoutes } from "./api/following.ts";
415415 import { labelsRoutes } from "./api/labels.ts";
416 import { appNoteRoutes } from "./api/app_notes.ts";
416417 import { reportRoutes } from "./api/reports.ts";
417418 import { rollbackRoutes } from "./api/rollback.ts";
418419 import { takeBackRoutes } from "./api/take_back.ts";
@@ -2684,6 +2685,7 @@ export function apiRoutes(router: Router<Handler>): void {
26842685 });
26852686
26862687 reportRoutes(router);
2688 appNoteRoutes(router);
26872689 rollbackRoutes(router);
26882690 takeBackRoutes(router);
26892691
src/http/api/app_notes.ts added +56
@@ -0,0 +1,56 @@
1 /**
sentinelcrew

This door also needs a line in docs/APPS.md (around lines 103-107, where the owner's tools are listed) giving the four routes, the rw scope and the admin requirement.
2 * Moderating what visitors send from an app's forge button, over the API:
3 * `GET …/app/notes` lists the notes and the banned senders, and
4 * `POST …/app/notes/:id/hide`, `…/ban` and `…/unban` act on one note. Admins
5 * of the repository only; the rules live in `actions/app_notes.ts`.
6 */
7
8 import { moderateNote, ownerNote, ownerNotes, unbanSender } from "../../actions/app_notes.ts";
9 import { getAppByRepo } from "../../core/apps.ts";
10 import { loadRepo } from "../api.ts";
11 import type { Ctx, Handler } from "../context.ts";
12 import { apiError, forbidden, json, notFound, requireScope } from "../respond.ts";
13 import type { Router } from "../router.ts";
14
15 /** The repository and its app, or the refusal. */
16 function target(ctx: Ctx) {
17 const denied = requireScope(ctx, "rw");
18 if (denied) return denied;
19 const repo = loadRepo(ctx);
20 if (!repo) return notFound();
21 const record = getAppByRepo(ctx.app.db, repo.id);
22 if (record === undefined) return notFound();
23 return { repo, record };
24 }
25
26 export function appNoteRoutes(router: Router<Handler>): void {
27 router.add("GET", "/api/v1/repos/:owner/:name/app/notes", (ctx) => {
28 const t = target(ctx);
29 if (t instanceof Response) return t;
30 const seen = ownerNotes(ctx.app, t.record, t.repo, ctx.auth.user);
31 return seen === undefined ? forbidden() : json(seen);
32 });
33
34 router.add("POST", "/api/v1/repos/:owner/:name/app/notes/:id/hide", moderation("hide"));
35 router.add("POST", "/api/v1/repos/:owner/:name/app/notes/:id/ban", moderation("ban"));
36 router.add("POST", "/api/v1/repos/:owner/:name/app/notes/:id/unban", moderation("unban"));
37 }
38
39 function moderation(action: "hide" | "ban" | "unban"): Handler {
40 return async (ctx) => {
41 const t = target(ctx);
42 if (t instanceof Response) return t;
43 const id = ctx.params.id ?? "";
44 const done =
45 action === "unban"
46 ? unbanSender(ctx.app, t.record, t.repo, ctx.auth.user, id)
47 : await moderateNote(ctx.app, t.record, t.repo, ctx.auth.user, id, action);
48 if (!done.ok) {
49 if (done.error === "forbidden") return forbidden();
50 if (done.error === "not_found") return notFound();
51 return apiError("unavailable", 503, "that could not be done just now");
52 }
53 const note = ownerNote(ctx.app, t.record, t.repo, ctx.auth.user, id);
54 return note === undefined ? notFound() : json(note);
55 };
56 }
test/api-app-notes.test.ts added +134
@@ -0,0 +1,134 @@
1 /**
2 * Moderating the forge button's notes over the API: a note sent through the
3 * button's endpoint is listed, hidden, banned and unbanned by an admin of the
4 * repository; a writer who is not an admin, a read-only token and a stranger
5 * are refused.
6 */
7
8 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
9 import { mkdtemp, rm } from "node:fs/promises";
10 import { tmpdir } from "node:os";
11 import { join } from "node:path";
12 import { createApp, type App } from "../src/app.ts";
13 import { newApp } from "../src/actions/apps.ts";
14 import { AppsRuntime } from "../src/apps/runtime.ts";
15 import { notesOfApp } from "../src/core/app_notes.ts";
16 import { getAppByRepo } from "../src/core/apps.ts";
17 import { setMember } from "../src/core/roles.ts";
18 import { addTeamMember, createOrg, createTeam } from "../src/core/orgs.ts";
19 import { getRepo } from "../src/core/repos.ts";
20 import { createToken } from "../src/core/tokens.ts";
21 import { createUser, type User } from "../src/core/users.ts";
22 import { serve, type Servers } from "../src/http/server.ts";
23 import { quietLogs } from "./helpers/globals.ts";
24
25 quietLogs();
26
27 const DOMAIN = "apps.test";
28 const BASE = "/api/v1/repos/acme/notes/app/notes";
29
30 describe("the app's notes over the API", () => {
31 let dataDir: string;
32 let app: App;
33 let servers: Servers;
34 let runtime: AppsRuntime;
35 const tokens: Record<string, string> = {};
36
37 beforeAll(async () => {
38 dataDir = await mkdtemp(join(tmpdir(), "forge-api-app-notes-"));
39 app = createApp({ dataDir, port: 0, host: "forge.test" });
40 servers = await serve(app);
41 runtime = new AppsRuntime(app, { domain: DOMAIN, scheme: "http", publicPort: servers.port, forgeUrl: servers.url, concurrency: 1 });
42 await runtime.start();
43 const make = async (username: string, first = false): Promise<User> => {
44 const made = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase", adminIfFirst: first });
45 if (!made.ok) throw new Error(made.error);
46 return made.value;
47 };
48 const siteAdmin = await make("siteadmin", true);
49 const founder = await make("founder");
50 const writer = await make("writer");
51 const stranger = await make("stranger");
52 setMember(app.db, founder.id, true, siteAdmin.id, new Date());
53 const org = createOrg(app.db, "acme", founder.id);
54 if (!org.ok) throw new Error(org.error);
55 const team = createTeam(app.db, org.value.id, "Writers", "write");
56 if (!team.ok) throw new Error(team.error);
57 addTeamMember(app.db, team.value, writer.id);
58 const made = await newApp(app, founder, { name: "notes", owner: "acme", visibility: "public" });
59 if (!made.ok) throw new Error(String(made.error));
60 tokens.founder = createToken(app.db, founder.id, "rw", "rw").token;
61 tokens.readonly = createToken(app.db, founder.id, "ro", "ro").token;
62 tokens.writer = createToken(app.db, writer.id, "rw", "rw").token;
63 tokens.stranger = createToken(app.db, stranger.id, "rw", "rw").token;
64 }, 120_000);
65
66 afterAll(async () => {
67 await runtime.stop();
68 servers.stop();
69 app.close();
70 await rm(dataDir, { recursive: true, force: true });
71 });
72
73 const api = (who: string, path: string, method = "GET") =>
74 fetch(`${servers.url}${path}`, { method, headers: { authorization: `Bearer ${tokens[who]}` } });
75 /** A note through the forge button's endpoint, from a fresh anonymous visitor. */
76 let sends = 0;
77 const send = async (text: string) => {
78 const host = `notes.${DOMAIN}:${servers.port}`;
79 const res = await fetch(`${servers.url}/__forge/note`, {
80 method: "POST",
sentinelcrew outdated

Math.random() in x-forwarded-for: loopback is a trusted proxy in tests, so this random address really becomes the rate-limit source, and the run is nondeterministic. Use a fixed address per send (a counter), or drop the header, because 4 sends stay under PER_SOURCE=6.
81 headers: { host, origin: `http://${host}`, "content-type": "application/json", "x-forwarded-for": `10.7.0.${++sends}` },
82 body: JSON.stringify({ text, page: `http://${host}/list`, viewport: "390x844" }),
83 });
84 expect(res.status).toBe(201);
85 };
86 const listed = async () => (await (await api("founder", BASE)).json()) as { notes: { id: string; text: string; banned: boolean }[]; banned: { id: string; banned: boolean }[] };
87
88 test("an admin lists, hides, bans and unbans", async () => {
89 await send("First note");
90 await send("Second note");
91 await send("Third note");
92 const first = await listed();
93 expect(first.notes.map((n) => n.text).sort()).toEqual(["First note", "Second note", "Third note"]);
94 expect(first.banned).toEqual([]);
95
96 const [a, b] = [first.notes.find((n) => n.text === "First note")!, first.notes.find((n) => n.text === "Second note")!];
97 const hid = await api("founder", `${BASE}/${a.id}/hide`, "POST");
98 expect(hid.status).toBe(200);
99 expect(((await hid.json()) as { id: string }).id).toBe(a.id);
100 expect((await listed()).notes.map((n) => n.id)).not.toContain(a.id);
101
102 const ban = await api("founder", `${BASE}/${b.id}/ban`, "POST");
103 expect(ban.status).toBe(200);
104 expect(((await ban.json()) as { banned: boolean }).banned).toBe(true);
105 const after = await listed();
106 expect(after.notes.map((n) => n.id)).not.toContain(b.id);
107 expect(after.banned.map((n) => n.id)).toEqual([b.id]);
108
109 const unban = await api("founder", `${BASE}/${b.id}/unban`, "POST");
110 expect(unban.status).toBe(200);
111 expect(((await unban.json()) as { banned: boolean }).banned).toBe(false);
112 expect((await listed()).banned).toEqual([]);
113 const repo = getRepo(app.db, "acme", "notes")!;
114 const record = getAppByRepo(app.db, repo.id)!;
115 expect(notesOfApp(app.db, record.id)).toHaveLength(1);
116 });
117
118 test("a writer who is not an admin, a stranger and a read-only token are refused", async () => {
119 await send("Another note");
120 const id = (await listed()).notes[0]!.id;
121 for (const who of ["writer", "stranger", "readonly"]) {
122 expect((await api(who, BASE)).status).toBe(403);
123 for (const action of ["hide", "ban", "unban"]) expect((await api(who, `${BASE}/${id}/${action}`, "POST")).status).toBe(403);
124 }
125 expect((await listed()).notes.map((n) => n.id)).toContain(id);
126 expect((await fetch(`${servers.url}${BASE}`)).status).toBe(401);
127 });
128
129 test("an unknown note or a repository that is not an app is not found", async () => {
130 expect((await api("founder", `${BASE}/nope/hide`, "POST")).status).toBe(404);
131 expect((await api("founder", `${BASE}/nope/unban`, "POST")).status).toBe(404);
132 expect((await api("founder", "/api/v1/repos/acme/missing/app/notes")).status).toBe(404);
133 });
134 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs