Public: anyone can read this; the crew can change it.
MCP `secrets`: list names, set and remove the forge's sealed secrets, never reading a value back #4802
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4792-mcp-secrets-list-names-set-and-remove-th → main 4 commits fork 2 threads · 2 unresolved
4 files
changed +183
−0
docs/MCP.md +1
| @@ -39,6 +39,7 @@ for a tunnel and a forge that sleeps. | ||
| 39 | 39 | | `pulls` | `{ repo, state?, limit? }`: a repository's pulls, newest first, one brief line each: `number`, `title`, `author`, `head`, `check` (state), `approvals`, `changesRequested`, `mergeable` (null once closed); `state` open (default) or closed, `limit` 1-50 (default 20); no bodies, no diffs | no | |
| 40 | 40 | | `read_file` | a file (utf-8, or base64 for binary) or a directory listing at a ref, as GET …/contents | no | |
| 41 | 41 | | `seal` | `{ repo, number, sealed }`: seal a security report (and the pull that fixes it) so only writers and the crew see it, or unseal it for every reader; a writer's, recorded | yes | |
| 42 | | `secrets` | `{ scope, ref?, action, name?, value? }` (forge admin): `list` the names and timestamps of the forge's sealed secrets, `set` or `remove` one; `scope` is forge, org or repo, `ref` the org or `owner/name`; a value is never read back | yes | | |
| 42 | 43 | | `now` | what the crew is doing right now: who, on what, since when, at what cost; a planner that declined the direction and waits for you comes first (`run: null`, its words in `says`) | | |
| 43 | 44 | | `stop` | stop one run by its id from `now` | yes | |
| 44 | 45 | | `health` | how this forge is: the version and commit it runs, draining or not, the rollout and its incidents | | |
src/http/mcp.ts +40
| @@ -30,6 +30,7 @@ import { ISSUE_TEAMS_SAY, nameTeams } from "../actions/issue_teams.ts"; | ||
| 30 | 30 | import { teamsOfIssue } from "../core/issue_teams.ts"; |
| 31 | 31 | import { changeTeam, planTeam, readTeams, TEAM_SAYS } from "../actions/teams.ts"; |
| 32 | 32 | import { readScorecards } from "../actions/scorecard.ts"; |
| 33 | import { listSecrets, putSecret, removeSecret } from "../actions/secrets.ts"; | |
| 33 | 34 | import { addAccount, allowWrite, auditAccount, removeAccount, renameAccount, reverify, rotateAccount } from "../actions/cloud.ts"; |
| 34 | 35 | import { usesByVendor, type VendorUse } from "../cloud/declared.ts"; |
| 35 | 36 | import { cloudOverview } from "../cloud/overview.ts"; |
| @@ -1221,6 +1222,45 @@ const TOOLS: readonly Tool[] = [ | ||
| 1221 | 1222 | }); |
| 1222 | 1223 | }, |
| 1223 | 1224 | }, |
| 1225 | { | |
| 1226 | name: "secrets", | |
| 1227 | description: "Admin: list|set|remove sealed secrets; values are write-only.", | |
| 1228 | inputSchema: { | |
| 1229 | type: "object", | |
| 1230 | properties: { | |
| 1231 | scope: { type: "string" }, | |
| 1232 | ref: { type: "string" }, | |
| 1233 | action: { type: "string" }, | |
| 1234 | name: { type: "string" }, | |
| 1235 | value: { type: "string" }, | |
| 1236 | }, | |
| 1237 | }, | |
| 1238 | writes: true, | |
| 1239 | adminOnly: true, | |
| 1240 | run(ctx, user, args) { | |
| 1241 | // The API asks for an admin token on all three routes; so does this. | |
| 1242 | if (!scopeAllows(ctx.auth.scope ?? "ro", "admin")) | |
| 1243 | return bad("forbidden", "secrets need an admin token"); | |
| 1244 | const str = (v: unknown): string => (typeof v === "string" ? v : ""); | |
| 1245 | const scope = str(args["scope"]); | |
| 1246 | const ref = str(args["ref"]); | |
| 1247 | const name = str(args["name"]); | |
| 1248 | const action = args["action"]; | |
| 1249 | if (action === "list") { | |
| 1250 | const listed = listSecrets(ctx.app, user, scope, ref); | |
| 1251 | return listed.ok ? good({ items: listed.value }) : bad(listed.error, listed.error); | |
| 1252 | } | |
| 1253 | if (action === "set") { | |
| 1254 | const set = putSecret(ctx.app, user, scope, ref, name, str(args["value"])); | |
| 1255 | return set.ok ? good(set.value) : bad(set.error, set.error); | |
| 1256 | } | |
| 1257 | if (action === "remove") { | |
| 1258 | const gone = removeSecret(ctx.app, user, scope, ref, name); | |
| 1259 | return gone.ok ? good({ removed: name }) : bad(gone.error, gone.error); | |
| 1260 | } | |
| 1261 | return bad("invalid", "action is list, set or remove"); | |
| 1262 | }, | |
| 1263 | }, | |
| 1224 | 1264 | { |
| 1225 | 1265 | name: "issue_teams", |
| 1226 | 1266 | description: |
test/mcp-secrets.test.ts added +141
| @@ -0,0 +1,141 @@ | ||
| 1 | /** | |
| 2 | * The `secrets` tool is /api/v1/secrets over MCP: the same actions, admins | |
| 3 | * only, and no answer ever carries a value. | |
| 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 { createRepo } from "../src/actions/repos.ts"; | |
| 11 | import { createOrg } from "../src/core/orgs.ts"; | |
| 12 | import { createToken } from "../src/core/tokens.ts"; | |
| 13 | import { createUser } from "../src/core/users.ts"; | |
| 14 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 15 | import { promote } from "./helpers/members.ts"; | |
| 16 | ||
| 17 | describe("MCP secrets", () => { | |
| 18 | let dataDir: string; | |
| 19 | let app: App; | |
| 20 | let servers: Servers; | |
| 21 | let admin: string; | |
| 22 | let adminRw: string; | |
| 23 | let other: string; | |
| 24 | let id = 1; | |
| 25 | const VALUE = "s3cr3t-value-that-must-not-leak"; | |
| 26 | ||
| 27 | const rpc = async (token: string, method: string, params?: unknown) => { | |
| 28 | const res = await fetch(`${servers.url}/mcp`, { | |
| 29 | method: "POST", | |
| 30 | headers: { | |
| 31 | "content-type": "application/json", | |
| 32 | accept: "application/json, text/event-stream", | |
| 33 | authorization: `Bearer ${token}`, | |
| 34 | }, | |
| 35 | body: JSON.stringify({ jsonrpc: "2.0", id: id++, method, params }), | |
| 36 | }); | |
| 37 | return (await res.json()) as any; | |
| 38 | }; | |
| 39 | const tool = async (token: string, args: unknown) => { | |
| 40 | const body = await rpc(token, "tools/call", { name: "secrets", arguments: args }); | |
| 41 | const text: string = body.result.content[0].text; | |
| 42 | return { text, isError: body.result.isError === true }; | |
| 43 | }; | |
| 44 | ||
| 45 | beforeAll(async () => { | |
| 46 | dataDir = await mkdtemp(join(tmpdir(), "forge-mcp-secrets-")); | |
| 47 | app = createApp({ dataDir, port: 0 }); | |
| 48 | servers = await serve(app); | |
| 49 | const a = await createUser(app.db, { origin: "seed", trusted: true, | |
| 50 | username: "alice", | |
| 51 | email: "[email protected]", | |
| 52 | password: "a-good-passphrase", | |
| 53 | adminIfFirst: true, | |
| 54 | }); | |
| 55 | if (!a.ok) throw new Error(a.error); | |
| 56 | const b = await createUser(app.db, { origin: "seed", trusted: true, | |
| 57 | username: "bob", | |
| 58 | email: "[email protected]", | |
| 59 | password: "a-good-passphrase", | |
| 60 | }); | |
| 61 | if (!b.ok) throw new Error(b.error); | |
| 62 | promote(app.db, b.value); | |
outdated
With a |
||
| 63 | admin = createToken(app.db, a.value.id, "admin", "admin").token; | |
| 64 | adminRw = createToken(app.db, a.value.id, "rw", "rw").token; | |
| 65 | other = createToken(app.db, b.value.id, "rw", "rw").token; | |
| 66 | const org = createOrg(app.db, "acme", a.value.id); | |
| 67 | if (!org.ok) throw new Error(org.error); | |
| 68 | const made = await createRepo(app, a.value, { name: "game", visibility: "public" }); | |
| 69 | if (!made.ok) throw new Error(String(made.error)); | |
| 70 | }, 30_000); | |
| 71 | ||
| 72 | afterAll(async () => { | |
| 73 | servers.stop(); | |
| 74 | app.close(); | |
| 75 | await rm(dataDir, { recursive: true, force: true }); | |
| 76 | }); | |
| 77 | ||
| 78 | for (const [scope, ref] of [ | |
| 79 | ["forge", undefined], | |
| 80 | ["org", "acme"], | |
| 81 | ["repo", "alice/game"], | |
| 82 | ] as const) { | |
| 83 | test(`an admin sets, lists and removes at the ${scope} scope, and no answer holds the value`, async () => { | |
| 84 | const base = { scope, ref }; | |
| 85 | const set = await tool(admin, { ...base, action: "set", name: "API_KEY", value: VALUE }); | |
| 86 | expect(set.isError).toBe(false); | |
| 87 | const list = await tool(admin, { ...base, action: "list" }); | |
| 88 | expect(JSON.parse(list.text).items.map((s: any) => s.name)).toEqual(["API_KEY"]); | |
| 89 | const gone = await tool(admin, { ...base, action: "remove", name: "API_KEY" }); | |
| 90 | expect(gone.isError).toBe(false); | |
| 91 | const after = await tool(admin, { ...base, action: "list" }); | |
| 92 | expect(JSON.parse(after.text).items).toEqual([]); | |
| 93 | const missing = await tool(admin, { ...base, action: "remove", name: "API_KEY" }); | |
| 94 | expect(missing.isError).toBe(true); | |
| 95 | for (const a of [set, list, gone, after, missing]) expect(a.text).not.toContain(VALUE); | |
| 96 | }); | |
| 97 | } | |
| 98 | ||
| 99 | test("a bad value or scope is refused with the API's error, never echoing the value", async () => { | |
| 100 | const scope = await tool(admin, { scope: "moon", action: "set", name: "K", value: VALUE }); | |
| 101 | expect(scope.isError).toBe(true); | |
| 102 | expect(scope.text).toContain("invalid_scope"); | |
| 103 | expect(scope.text).not.toContain(VALUE); | |
| 104 | const name = await tool(admin, { scope: "forge", action: "set", name: "bad name", value: VALUE }); | |
| 105 | expect(name.text).toContain("invalid_name"); | |
| 106 | expect(name.text).not.toContain(VALUE); | |
| 107 | const action = await tool(admin, { scope: "forge", action: "peek" }); | |
| 108 | expect(action.isError).toBe(true); | |
| 109 | }); | |
| 110 | ||
| 111 | test("an admin's rw token is refused on list, set and remove, and nothing changes", async () => { | |
| 112 | await tool(admin, { scope: "forge", action: "set", name: "KEPT", value: VALUE }); | |
| 113 | for (const args of [ | |
| 114 | { scope: "forge", action: "list" }, | |
| 115 | { scope: "forge", action: "set", name: "KEPT", value: "overwritten" }, | |
| 116 | { scope: "forge", action: "remove", name: "KEPT" }, | |
| 117 | ]) { | |
| 118 | const r = await tool(adminRw, args); | |
| 119 | expect(r.isError).toBe(true); | |
| 120 | expect(r.text).toContain("forbidden"); | |
| 121 | } | |
| 122 | const list = await tool(admin, { scope: "forge", action: "list" }); | |
| 123 | expect(JSON.parse(list.text).items.map((s: any) => s.name)).toEqual(["KEPT"]); | |
| 124 | await tool(admin, { scope: "forge", action: "remove", name: "KEPT" }); | |
| 125 | }); | |
| 126 | ||
| 127 | test("a non-admin is refused and not listed the tool", async () => { | |
| 128 | const listed = await rpc(other, "tools/list"); | |
| 129 | expect(listed.result.tools.map((t: any) => t.name)).not.toContain("secrets"); | |
| 130 | const seen = await rpc(admin, "tools/list"); | |
| 131 | expect(seen.result.tools.map((t: any) => t.name)).toContain("secrets"); | |
| 132 | const call = await rpc(other, "tools/call", { | |
| 133 | name: "secrets", | |
| 134 | arguments: { scope: "forge", action: "set", name: "API_KEY", value: VALUE }, | |
| 135 | }); | |
| 136 | expect(call.error ?? call.result.isError).toBeTruthy(); | |
| 137 | expect(JSON.stringify(call)).not.toContain(VALUE); | |
| 138 | const after = await tool(admin, { scope: "forge", action: "list" }); | |
| 139 | expect(JSON.parse(after.text).items).toEqual([]); | |
| 140 | }); | |
| 141 | }); |
test/mcp.test.ts +1
| @@ -192,6 +192,7 @@ describe("the forge over MCP", () => { | ||
| 192 | 192 | "proposal_tokens", |
| 193 | 193 | "asks_undecide", |
| 194 | 194 | "seal", |
| 195 | "secrets", | |
| 195 | 196 | "issue_teams", |
| 196 | 197 | "now", |
| 197 | 198 | "stop", |
new above
offline
High:
writes: trueonly makes the dispatcher check for anrwtoken, but/api/v1/secrets/*requiresadmin(requireScope(ctx, "admin")). Addif (!scopeAllows(ctx.auth.scope ?? "ro", "admin")) return bad("forbidden", …)at the top ofrun, as at mcp.ts:1450, so this tool can't do more than the API allows.