trevato/forge

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

More
Forks1g fMapg aRuns12g rEventsFlows

MCP `ssh_keys`: list, add and remove the caller's own SSH keys, with the same rules as the API #4814

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/4795-mcp-ssh-keys-list-add-and-remove-the-cal → main 10 commits fork 4 threads · 4 unresolved

  • Merged. scout/4795-mcp-ssh-keys-list-add-and-remove-the-cal was merged into main as e7748df.

Review

Sign in to review this change.
7 files changed +165 −14
docs/MCP.md +1
@@ -102,6 +102,7 @@ for a tunnel and a forge that sleeps.
102102 | `review` | `{ repo, number, verdict, reason? }`: approve a pull (`approve`) or request changes (`changes`, with a `reason`), as the page does; a writer's or the crew's, and never your own pull | yes |
103103 | `threads` | `{ repo, number, anchor?, resolved? }`: a pull's review threads with their comments; with `anchor` (from the list) resolve it (`resolved` true, the default) or reopen it (false), by the API's rules: a writer or someone who commented in the thread | yes |
104104 | `usage` | `{ repo, days? }`: what the crew's runs spent — tokens, cost, wall time — by subscription, agent, issue, and each account's 5-hour and weekly standing; a writer's | |
105 | `ssh_keys` | `{ action?, title?, key?, id? }`: list your own SSH keys (default), add one (`key` is an authorized_keys line, `title` optional) or remove one by `id`; the API's rules and errors, add and remove need an `rw` token | yes |
105106 | `boards` | `{ slug?, panel?, from?, to?, step? }` (person admins): without `slug`, lists the forge's boards (slug, title, range, refresh, panels); with `slug`, that board; with `panel` (place from 0, up to 23), that panel's data over the `from`/`to` window and `step` | |
106107 | `board_write` | `{ slug, board }` (person admins): write a whole board under a slug, made or replaced: `{ title, range, refresh, panels: [{ title, chart, query, w, h }] }` | yes |
107108 | `board_delete` | `{ slug }` (person admins): delete a board by slug; its history keeps it | yes |
src/core/keys.ts +13
@@ -244,3 +244,16 @@ export function touchKey(db: Database, keyId: string): void {
244244 keyId,
245245 );
246246 }
247
248 /** What the API and MCP answer for a key: one shape for both. */
249 export function publicKey(key: UserKey) {
250 return {
251 id: key.id,
252 name: key.name,
253 algo: key.algo,
254 fingerprint: key.fingerprint,
255 key: key.key,
256 createdAt: key.createdAt,
257 lastUsedAt: key.lastUsedAt,
258 };
259 }
src/http/api.ts +1 −13
@@ -211,7 +211,7 @@ import {
211211 type Issue,
212212 type IssueState,
213213 } from "../core/issues.ts";
214 import { listKeys, type UserKey } from "../core/keys.ts";
214 import { listKeys, publicKey } from "../core/keys.ts";
215215 import {
216216 getOrg,
217217 listOrgsForUser,
@@ -526,18 +526,6 @@ function respondFleet(error: FleetError): Response {
526526 );
527527 }
528528
529 function publicKey(key: UserKey) {
530 return {
531 id: key.id,
532 name: key.name,
533 algo: key.algo,
534 fingerprint: key.fingerprint,
535 key: key.key,
536 createdAt: key.createdAt,
537 lastUsedAt: key.lastUsedAt,
538 };
539 }
540
541529 function withLabels<T extends Issue>(ctx: Ctx, issue: T) {
542530 return {
543531 ...readerMarked(ctx, issue.repoId, [issue])[0]!,
src/http/mcp.ts +40
@@ -193,6 +193,8 @@ import { plannerDeclines } from "../crew/autonomy.ts";
193193 import { checkStale } from "../checks/stale.ts";
194194 import { checkSummary, listActiveRuns } from "../core/runs.ts";
195195 import { scopeAllows } from "../core/tokens.ts";
196 import { addSshKey, deleteSshKey } from "../actions/users.ts";
197 import { listKeys, publicKey } from "../core/keys.ts";
196198 import { lastOwnerSays } from "../core/orgs.ts";
197199 import { LAST_WAY_IN, parentIssuerOf, unlinkParent } from "../core/federation.ts";
198200 import { getUserByName, getUsersByIds, type User } from "../core/users.ts";
@@ -318,6 +320,13 @@ const bad = (error: string, message: string): Answer => ({
318320 message,
319321 });
320322
323 const keyWords = (error: string): string =>
324 ({
325 invalid_key: "not a public key in authorized_keys form",
326 key_exists: "that key is already registered",
327 not_found: "no such key of yours",
328 })[error] ?? error;
329
321330 interface Tool {
322331 readonly name: string;
323332 readonly description: string;
@@ -4728,6 +4737,37 @@ const TOOLS: readonly Tool[] = [
47284737 });
47294738 },
47304739 },
4740 {
4741 name: "ssh_keys",
4742 description: "Your SSH keys; add: key, remove: id.",
4743 inputSchema: {
4744 type: "object",
4745 properties: {
4746 action: { type: "string", enum: ["list", "add", "remove"] },
4747 title: { type: "string" },
4748 key: { type: "string" },
4749 id: { type: "string" },
4750 },
4751 },
4752 writes: true,
4753 readsWhen: (args) => (args["action"] ?? "list") === "list",
4754 run(ctx, user, args) {
4755 const action = args["action"] ?? "list";
4756 if (action === "list") return good(listKeys(ctx.app.db, user.id).map(publicKey));
4757 if (action === "add") {
4758 if (typeof args["key"] !== "string")
4759 return bad("invalid_key", "key is an authorized_keys line");
4760 const title = typeof args["title"] === "string" ? args["title"] : "";
4761 const added = addSshKey(ctx.app, user, { name: title, key: args["key"] });
4762 return added.ok ? good(publicKey(added.value)) : bad(added.error, keyWords(added.error));
4763 }
4764 if (action === "remove") {
4765 const removed = deleteSshKey(ctx.app, user, typeof args["id"] === "string" ? args["id"] : "");
4766 return removed.ok ? good({ removed: args["id"] }) : bad(removed.error, keyWords(removed.error));
4767 }
4768 return bad("invalid_action", "action is list, add or remove");
4769 },
4770 },
47314771 {
47324772 name: "email_verify",
47334773 description: "Send yourself a new verification link.",
test/mcp-size.test.ts +2 −1
@@ -89,7 +89,8 @@ test("tools/list stays under 40 KB, with 3 KB of headroom (37,000 bytes)", async
8989 for (const t of tools) walk(t.name, t.inputSchema, "");
9090 expect(long, `property descriptions over ${PROPERTY_MAX} bytes`).toEqual([]);
9191 expect(bytes, "the argument descriptions were cut by 1,000 bytes").toBeLessThanOrEqual(
92 36_700,
92 // 36,700 before `ssh_keys` (#4795), which costs about 210 bytes.
93 36_950,
9394 );
9495 // readOnlyHint defaults to false; spelling the default out on every write
9596 // tool cost 2 KB, so only a read tool carries annotations.
test/mcp-ssh-keys.test.ts added +107
@@ -0,0 +1,107 @@
1 /**
2 * The `ssh_keys` tool is `/api/v1/user/keys` over MCP: the same actions, the
3 * same scope, and only the caller's own keys.
4 */
5 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
6 import { mkdtemp, rm } from "node:fs/promises";
7 import { tmpdir } from "node:os";
8 import { join } from "node:path";
9 import { createApp, type App } from "../src/app.ts";
10 import { createToken } from "../src/core/tokens.ts";
11 import { createUser } from "../src/core/users.ts";
12 import { serve, type Servers } from "../src/http/server.ts";
13
14 const KEY_A =
15 "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl a@laptop";
16 const KEY_B =
17 "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIJd2AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA b@box";
18
19 describe("MCP ssh_keys", () => {
20 let dataDir: string;
21 let app: App;
22 let servers: Servers;
23 let aliceRw: string;
24 let aliceRo: string;
25 let bobRw: string;
26 let id = 1;
27
28 const tool = async (token: string, args: unknown) => {
29 const res = await fetch(`${servers.url}/mcp`, {
30 method: "POST",
31 headers: {
32 "content-type": "application/json",
33 accept: "application/json, text/event-stream",
34 authorization: `Bearer ${token}`,
35 },
36 body: JSON.stringify({
37 jsonrpc: "2.0",
38 id: id++,
39 method: "tools/call",
40 params: { name: "ssh_keys", arguments: args },
41 }),
42 });
43 const body: any = await res.json();
44 const text: string = body.result.content[0].text;
45 return body.result.isError === true
46 ? { error: text }
47 : { value: JSON.parse(text) };
48 };
49
50 beforeAll(async () => {
51 dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-keys-"));
52 app = createApp({ dataDir, port: 0 });
53 servers = await serve(app);
54 const mk = async (username: string) => {
55 const u = await createUser(app.db, {
56 origin: "seed",
57 trusted: true,
58 username,
59 email: `${username}@forge.local`,
60 password: "a-good-passphrase",
61 });
62 if (!u.ok) throw new Error(u.error);
63 return u.value;
64 };
65 const alice = await mk("alice");
66 const bob = await mk("bob");
67 aliceRw = createToken(app.db, alice.id, "rw", "rw").token;
68 aliceRo = createToken(app.db, alice.id, "ro", "ro").token;
69 bobRw = createToken(app.db, bob.id, "rw", "rw").token;
70 }, 30_000);
71
72 afterAll(async () => {
73 servers.stop();
74 app.close();
75 await rm(dataDir, { recursive: true, force: true });
76 });
77
78 test("a person adds, lists and removes a key; bad and duplicate keys are refused", async () => {
79 const added = await tool(aliceRw, { action: "add", title: "laptop", key: KEY_A });
80 expect(added.value.name).toBe("laptop");
81 expect(added.value.fingerprint).toStartWith("SHA256:");
82 const listed = await tool(aliceRw, { action: "list" });
83 expect(listed.value.map((k: any) => k.id)).toEqual([added.value.id]);
84 expect((await tool(aliceRw, { action: "add", key: KEY_A })).error).toStartWith("key_exists");
85 expect((await tool(aliceRw, { action: "add", key: "nonsense" })).error).toStartWith("invalid_key");
86 expect((await tool(aliceRw, { action: "remove", id: added.value.id })).value).toEqual({
87 removed: added.value.id,
88 });
89 expect((await tool(aliceRw, {})).value).toEqual([]);
90 });
91
92 test("a read-only token lists but is refused add and remove", async () => {
93 const added = await tool(aliceRw, { action: "add", title: "x", key: KEY_B });
94 expect(added.value.id).toBeDefined();
95 expect(Array.isArray((await tool(aliceRo, { action: "list" })).value)).toBe(true);
96 expect((await tool(aliceRo, { action: "add", key: KEY_A })).error).toStartWith("forbidden");
97 expect((await tool(aliceRo, { action: "remove", id: added.value.id })).error).toStartWith("forbidden");
98 expect((await tool(aliceRw, { action: "list" })).value).toHaveLength(1);
99 });
100
101 test("one person cannot remove another's key", async () => {
102 const [mine] = (await tool(aliceRw, { action: "list" })).value;
103 expect((await tool(bobRw, { action: "remove", id: mine.id })).error).toStartWith("not_found");
104 expect((await tool(bobRw, { action: "list" })).value).toEqual([]);
105 expect((await tool(aliceRw, { action: "list" })).value).toHaveLength(1);
106 });
107 });
test/mcp.test.ts +1
@@ -274,6 +274,7 @@ describe("the forge over MCP", () => {
274274 "cloud_account",
275275 "quotas",
276276 "signup",
277 "ssh_keys",
277278 "email_verify",
278279 "parent_unlink",
279280 "boards",

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs