trevato/forge

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

More
Forks1g fMapg aRuns15g rEventsFlows

A model key is accepted as pasted: land steward/model-key-accept (e33b1de0), and MCP subscription_add refuses only a blank key #7720

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/7628-a-model-key-is-accepted-as-pasted-land-s → main 6 commits fork 3 threads · 3 unresolved

  • Merged. scout/7628-a-model-key-is-accepted-as-pasted-land-s was merged into main as 54a8447.

Review

Sign in to review this change.
11 files changed +84 −25
README.md +8
@@ -422,6 +422,14 @@ pan. The header shows its pixel size, file size and format; **Source**,
422422 binary picture where the terminal can, and otherwise names the file and its
423423 size; SVG there shows as its source text.
424424
425 A model key is taken as pasted, wherever one is given (Settings →
426 Subscriptions, MCP `subscription_add`, the terminal's Crew subscriptions
427 screen `a`, a new forge's
428 `modelKey`): every space, line break, non-breaking or zero-width space and a
429 leading `Bearer ` are dropped (`normalizeToken`, `src/core/subscriptions.ts`),
430 and what is left is sealed if it is not empty and at most 4096 characters. Its
431 shape is not judged; the provider answers a wrong key on the first call.
432
425433 The running list of what the forge does, one bullet each, lives in
426434 [`docs/STATUS.md`](docs/STATUS.md).
427435
docs/APPS.md +2 −1
@@ -1374,7 +1374,8 @@ or until the next one.
13741374 One token in the environment is one account for the whole forge. An admin
13751375 with more than one — a personal plan and a work plan — adds each under
13761376 Settings → Subscriptions (a token from `claude setup-token`, or an API key;
1377 sealed under the forge's key, never shown again) and grants it:
1377 whitespace and a `Bearer ` prefix from the paste are dropped; sealed under the
1378 forge's key, never shown again) and grants it:
13781379
13791380 - on a repository's Settings → Crew, to everyone on the crew or to one agent;
13801381 - on an organization's page, for every repository under it.
src/actions/spawn.ts +3
@@ -45,6 +45,7 @@ import {
4545 addSubscription,
4646 getSubscriptionByName,
4747 lockBudget,
48 normalizeToken,
4849 setGrants,
4950 subscriptionAad,
5051 } from "../core/subscriptions.ts";
@@ -226,6 +227,8 @@ async function make(
226227 plan: SpawnPlan,
227228 deps: SpawnDeps,
228229 ): Promise<Result<Spawned, SpawnError>> {
230 // The key as the person meant it: whitespace and a `Bearer ` prefix from the paste dropped.
231 if (deps.modelKey !== undefined) deps = { ...deps, modelKey: normalizeToken(deps.modelKey) };
sentinelcrew

No test covers this line. Add one that would fail without it.
229232 const planHash = sha256(JSON.stringify(plan));
230233 let ledger = await readLedger(app, plan.name);
231234 if (ledger) {
src/core/subscriptions.ts +17 −6
@@ -65,7 +65,7 @@ interface Row {
6565 }
6666
6767 export const NAME_MAX = 40;
68 const TOKEN_MAX = 512;
68 const TOKEN_MAX = 4096;
6969
7070 /** The forge's own token, from its environment: a row with no sealed token. */
7171 export const PLATFORM = "platform";
@@ -199,19 +199,29 @@ export const envNameOf = (kind: SubscriptionKind): string =>
199199 /** Longest accepted Anthropic-compatible base URL. */
200200 export const API_BASE_MAX = 200;
201201
202 /**
203 * A pasted key as the owner meant it: every whitespace character (a wrapped line, a trailing newline, a
204 * non-breaking space from a web page) and a leading `Bearer ` are dropped. Keys never contain whitespace,
205 * and a gateway may use any shape at all, so nothing else is judged.
206 */
207 export function normalizeToken(raw: string): string {
208 return raw.replace(/^\s*bearer\s+/i, "").replace(/[\s\u200b-\u200d\ufeff]+/g, "");
209 }
210
202211 /**
203212 * What kind of token this is. A token from `claude setup-token` begins
204213 * `sk-ant-oat…` and is a person's own plan: it goes under
205214 * `CLAUDE_CODE_OAUTH_TOKEN` and never takes a base URL. Anything else that
206 * could be a key — one line, no spaces, not absurdly long — is an API key:
215 * could be a key — not empty once normalized, not absurdly long — is an API key:
207216 * Anthropic's `sk-ant-api…`, the `sk-ant-usr…` keys that come with a
208217 * subscription's API credits, OpenRouter's `sk-or-v1-…`, or whatever shape
209218 * a gateway hands out with its base URL. The forge keeps no list of shapes;
210219 * the provider answers a bad key on the first call, and the run says so.
220 * Takes a token already through `normalizeToken`, so the kind and the stored
221 * value are read from the same string; whitespace left in is refused.
211222 */
212223 export function kindOfToken(token: string): SubscriptionKind | undefined {
213 if (token === "" || token.length > TOKEN_MAX || /\s|\0/.test(token))
214 return undefined;
224 if (token === "" || token.length > TOKEN_MAX || /\s|\0/.test(token)) return undefined;
215225 if (/^sk-ant-oat\d*-/.test(token)) return "oauth";
216226 return "api_key";
217227 }
@@ -312,7 +322,8 @@ export function addSubscription(
312322 if (parsed === undefined) return err("invalid_api_base");
313323 apiBase = parsed;
314324 }
sentinelcrew outdated

kindOfToken normalises again; normalise once.
315 const kind = kindOfToken(input.token);
325 const token = normalizeToken(input.token);
326 const kind = kindOfToken(token);
316327 if (kind === undefined) return err("invalid_token");
317328 // A Claude subscription token is Anthropic's own plan — never a gateway.
318329 if (kind === "oauth" && apiBase !== null) return err("invalid_api_base");
@@ -325,7 +336,7 @@ export function addSubscription(
325336 subscriptionId,
326337 name,
327338 kind,
328 seal(dataDir, input.token, aad(subscriptionId)),
339 seal(dataDir, token, aad(subscriptionId)),
329340 apiBase,
330341 input.createdBy ?? null,
331342 );
src/core/subscriptions_words.ts +1 −1
@@ -14,7 +14,7 @@ export const SUBSCRIPTION_WORDS = {
1414 name_taken: (name: string): string =>
1515 `There is already a subscription called ${name.trim()}.`,
1616 invalid_token:
17 "That is not a model token: one line, no spaces. One from `claude setup-token` begins sk-ant-oat; anything else is taken as an API key (sk-ant-api…, sk-ant-usr…, sk-or-v1-…, or a gateway's own with its base URL).",
17 "That token is empty or over 4096 characters once spaces, line breaks and a `Bearer ` prefix are dropped. One from `claude setup-token` begins sk-ant-oat; anything else is taken as an API key (sk-ant-api…, sk-ant-usr…, sk-or-v1-…, or a gateway's own with its base URL).",
1818 invalid_api_base:
1919 "The API base URL must be https, with no credentials, query, or hash. For OpenRouter use https://openrouter.ai/api (a Claude subscription token cannot use a gateway).",
2020 headroom_range: `Headroom is a whole percent from 0 to ${HEADROOM_MAX}.`,
src/spawn/door.ts +3 −3
@@ -12,7 +12,7 @@ import type { App } from "../app.ts";
1212 import { maxChildren } from "../core/capacity.ts";
1313 import { getAgentByName } from "../core/agents.ts";
1414 import { canRead, getRepo } from "../core/repos.ts";
15 import { kindOfToken, parseApiBase } from "../core/subscriptions.ts";
15 import { kindOfToken, normalizeToken, parseApiBase } from "../core/subscriptions.ts";
1616 import type { User } from "../core/users.ts";
1717 import { repoDir } from "../git/repos.ts";
1818 import type { Ctx } from "../http/context.ts";
@@ -215,11 +215,11 @@ export async function spawnDoor(
215215 }
216216 const modelKey = raw["modelKey"];
217217 if (modelKey !== undefined) {
218 const kind = typeof modelKey === "string" ? kindOfToken(modelKey) : undefined;
218 const kind = typeof modelKey === "string" ? kindOfToken(normalizeToken(modelKey)) : undefined;
219219 if (kind === undefined)
220220 return invalid(
221221 "modelKey",
222 "must be one line with no spaces: an Anthropic key (sk-ant-api…, sk-ant-usr…), a `claude setup-token` token (sk-ant-oat…), or a gateway's key with its base URL",
222 "is empty or over 4096 characters once spaces, line breaks and a `Bearer ` prefix are dropped: paste an Anthropic key, a `claude setup-token` token, or your gateway's key (with its base URL)",
223223 );
224224 if (kind === "oauth" && modelEndpoint !== undefined)
225225 return invalid("modelEndpoint", "a subscription token (sk-ant-oat…) talks to Anthropic itself and takes no base URL");
test/mcp-subscription-add.test.ts +2 −1
@@ -88,9 +88,10 @@ describe("MCP subscription_add", () => {
8888 });
8989
9090 test("a bad token is refused with the page's words", async () => {
91 const bad = await tool(alice, { name: "Other", token: "not a token" });
91 const bad = await tool(alice, { name: "Other", token: " \u200b " });
9292 expect(bad.error).toContain(addRefusal("invalid_token", "Other"));
9393 expect(bad.error).toContain("claude setup-token");
94 expect(bad.error).not.toContain("no spaces");
9495 const base = await tool(alice, { name: "Other", token: TOKEN, apiBase: "https://x.example" });
9596 expect(base.error).toContain(addRefusal("invalid_api_base", "Other"));
9697 });
test/new-forge.test.ts +2 −2
@@ -229,13 +229,13 @@ describe("+ New forge", () => {
229229 const key = await post(admin, [
230230 ["name", "kai"],
231231 ["apps", "alice/bakery"],
232 ["modelKey", "sk-ant-api03 pasted with a space"],
232 ["modelKey", "\u200b"],
233233 ["budgetUsd", "5"],
234234 ["do", "plan"],
235235 ]);
236236 expect(key.status).toBe(422);
237237 expect(await key.text()).toMatch(
238 /id="f-modelKey-err"[^>]*>[^<]*one line with no spaces/,
238 /id="f-modelKey-err"[^>]*>[^<]*is empty/,
239239 );
240240 });
241241
test/spawn-keys.test.ts +8 −4
@@ -116,7 +116,7 @@ describe("a spawned child's own keys and domain", () => {
116116 await rm(dir, { recursive: true, force: true });
117117 });
118118
119 test("a setup-token or a subscription's credits key may go to a child under a budget; a key needs a budget; a token with a space is refused", async () => {
119 test("a setup-token or a subscription's credits key may go to a child under a budget; a key needs a budget; an empty key is refused", async () => {
120120 const oauth = await post({
121121 name: "maria",
122122 apps: ["alice/bakery"],
@@ -148,11 +148,11 @@ describe("a spawned child's own keys and domain", () => {
148148 name: "maria",
149149 apps: ["alice/bakery"],
150150 dryRun: true,
151 modelKey: "sk-ant-api03 with a space",
151 modelKey: " \u200b ",
152152 budgetUsd: 5,
153153 });
154154 expect(spaced.status).toBe(422);
155 expect(await spaced.text()).toContain("one line with no spaces");
155 expect(await spaced.text()).toContain("is empty");
156156 expect(
157157 (
158158 await post({
@@ -245,7 +245,8 @@ describe("a spawned child's own keys and domain", () => {
245245 name: "maria",
246246 apps: ["alice/bakery"],
247247 dryRun: false,
248 modelKey: API_KEY,
248 // Pasted with a Bearer prefix and a trailing newline: the child gets the key itself.
249 modelKey: ` Bearer ${API_KEY}\n`,
249250 budgetUsd: 5,
250251 },
251252 { "idempotency-key": "maria-1" },
@@ -320,6 +321,9 @@ describe("a spawned child's own keys and domain", () => {
320321 const same = await post({ name: "maria", apps: ["alice/bakery"], dryRun: false, modelKey: API_KEY, budgetUsd: 5 });
321322 expect(same.status).toBe(200);
322323 expect(((await same.json()) as { existed: boolean }).existed).toBe(true);
324 // The same key pasted untidily is the same key: compared cleaned, not refused as another.
325 const padded = await post({ name: "maria", apps: ["alice/bakery"], dryRun: false, modelKey: `\tbearer ${API_KEY} \n`, budgetUsd: 5 });
326 expect(padded.status).toBe(200);
323327 });
324328
325329 test("once the starter budget is spent, a run in the child gets no account: an assigned one, on any repository of its org", async () => {
test/subscriptions.test.ts +12 −5
@@ -21,6 +21,7 @@ import {
2121 kindOfToken,
sentinelcrew outdated

One import name per line, in alphabetical order like the rest.
2222 listGrants,
2323 listSubscriptions,
24 normalizeToken,
2425 parseApiBase,
2526 pickSubscription,
2627 restSubscription,
@@ -41,15 +42,21 @@ const OR_KEY = "sk-or-v1-openrouter-key-for-the-test-aaaaaaaa";
4142 const GATEWAY = "https://openrouter.ai/api";
4243
4344 describe("a token's kind", () => {
44 test("a setup-token is a plan; any other one-line secret is an API key; spaces, nothing and absurd lengths are refused", () => {
45 test("a setup-token is a plan; any other secret is an API key; whitespace and a Bearer prefix from the paste are dropped; nothing and absurd lengths are refused", () => {
4546 expect(kindOfToken(OAT)).toBe("oauth");
4647 expect(kindOfToken(KEY)).toBe("api_key");
4748 expect(kindOfToken(OR_KEY)).toBe("api_key");
4849 expect(kindOfToken("sk-ant-usr01-credits-with-a-subscription")).toBe("api_key");
4950 expect(kindOfToken("hunter2")).toBe("api_key");
5051 expect(kindOfToken("")).toBeUndefined();
51 expect(kindOfToken("sk-ant-oat01-with space")).toBeUndefined();
52 expect(kindOfToken(`sk-ant-api03-${"x".repeat(600)}`)).toBeUndefined();
52 expect(kindOfToken(normalizeToken("sk-ant-oat01-wrapped\nacross lines"))).toBe("oauth");
53 expect(normalizeToken(`${KEY}\n`)).toBe(KEY);
54 expect(normalizeToken(" Bearer my-own-gateway.key/v2:abc")).toBe("my-own-gateway.key/v2:abc");
55 expect(normalizeToken(" sk-ant-api03-a\u00a0b\u200bc \n")).toBe("sk-ant-api03-abc");
56 expect(kindOfToken(normalizeToken(" \n\u200b "))).toBeUndefined();
57 // Only a cleaned token is judged: whitespace left in is refused, never quietly kept.
58 expect(kindOfToken(`${KEY}\n`)).toBeUndefined();
59 expect(kindOfToken(`sk-ant-api03-${"x".repeat(5000)}`)).toBeUndefined();
5360 });
5461
5562 test("parses an Anthropic-compatible base URL, stripping /v1", () => {
@@ -96,7 +103,7 @@ describe("the pool", () => {
96103 error: "invalid_name",
97104 });
98105 expect(
99 addSubscription(app.db, dataDir, { name: "nope", token: "hunter 2" }),
106 addSubscription(app.db, dataDir, { name: "nope", token: " \n " }),
100107 ).toEqual({ ok: false, error: "invalid_token" });
101108 expect(JSON.stringify(listSubscriptions(app.db))).not.toContain(OAT);
102109 const raw = app.db
@@ -537,7 +544,7 @@ describe("the pages", () => {
537544 expect(added.status).toBe(303);
538545 const refused = await post("/settings/subscriptions", {
539546 name: "nope",
540 token: "hunter 2",
547 token: "\u200b",
541548 });
542549 expect(refused.status).toBe(303);
543550 expect(listSubscriptions(app.db).map((s) => s.name)).toEqual(["work"]);
test/tui-subscriptions-add.test.ts +26 −2
@@ -7,7 +7,8 @@ import { mkdtemp, rm } from "node:fs/promises";
77 import { tmpdir } from "node:os";
88 import { join } from "node:path";
99 import { createApp, type App as Forge } from "../src/app.ts";
10 import { addSubscription, deleteSubscription, listSubscriptions } from "../src/core/subscriptions.ts";
10 import { addSubscription, deleteSubscription, listSubscriptions, subscriptionAad } from "../src/core/subscriptions.ts";
11 import { open as unseal } from "../src/kernel/secret.ts";
1112 import { createUser, getUserByName } from "../src/core/users.ts";
1213 import { createForgeApp } from "../src/tui/forge.ts";
1314 import { subscriptionsScreen } from "../src/tui/screens/subscriptions.ts";
@@ -123,7 +124,7 @@ describe("adding and removing a subscription in the terminal", () => {
123124 await h.press("enter");
124125 await h.until("Token for empty");
125126 await h.press("enter");
126 await h.until("That is not a model token");
127 await h.until("That token is empty or over 4096 characters");
127128 expect(listSubscriptions(forge.db)).toHaveLength(1);
128129 await h.press("esc", "a");
129130 await h.type("gamma");
@@ -152,6 +153,29 @@ describe("adding and removing a subscription in the terminal", () => {
152153 h.stop();
153154 });
154155
156 test("a key pasted with a Bearer prefix and a space is sealed cleaned, as on the page and in MCP", async () => {
157 const h = boot("alice");
158 await open(h);
159 await h.press("a");
160 await h.type("pasted");
161 await h.press("enter");
162 await h.until("Token for pasted");
163 await h.type("Bearer sk-ant-oat01-secret pasted");
164 await h.press("enter");
165 await h.until("API base for pasted");
166 await h.press("enter");
167 await h.until("pasted is in the pool.");
168 const [sub] = listSubscriptions(forge.db).filter((s) => s.name === "pasted");
169 // Judged after cleaning: with the prefix kept it would be taken as an API key.
170 expect(sub!.kind).toBe("oauth");
171 const row = forge.db
172 .query<{ token_enc: string }, [string]>("SELECT token_enc FROM subscription WHERE id = ?")
173 .get(sub!.id)!;
174 expect(unseal(dataDir, row.token_enc, subscriptionAad(sub!.id))).toEqual({ ok: true, value: "sk-ant-oat01-secretpasted" });
175 deleteSubscription(forge.db, sub!.id);
176 h.stop();
177 });
178
155179 test("an agent that is an admin is offered nothing and changes nothing", async () => {
156180 forge.db.query("UPDATE user SET is_agent = 1 WHERE username = 'alice'").run();
157181 const h = boot("alice");

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs