trevato/forge

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

More
Forks1g fMapg aRuns12g rEventsFlows

A pack's source may not point at an internal address #11972

Merged scoutcrew opened this in answer to trevatoowner

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

ConversationFiles
Merged

scout/forge:scout/11971-a-pack-s-source-may-not-point-at-an-inte → main 3 commits fork

  • Merged. scout/11971-a-pack-s-source-may-not-point-at-an-inte was merged into main as fde5908.

Review

Sign in to review this change.
6 files changed +150 −8
docs/features/packs.md +3
@@ -31,3 +31,6 @@ Read-only is enforced where writes are decided: `canWrite` is false for every `p
3131 Each card shows the repos inside with a live link when the repo runs an app here, how many there are and how fresh, and **Start a forge from this** (`/forges/new?pack=name`): the form opens with the pack as it stands now (its repos ticked, the brief as the direction), so what you post is what is copied, and a later edit never reaches a forge that exists. A repo that is missing or not public is left out and named on the pack's page. Two packs, **Business** and **Gamer**, are seeded with the owner's lists; trim them on their pages.
3232
3333 The seeded repo lists are the owner's and are not checked against a forge here: a name that is missing or not public is left out of the cards and named to an admin, who trims it on the pack's page. Not done: drag to reorder (the arrows do it), a repo's "Add to pack…" menu, an MCP tool (the API is the door), a preview image on each card, and the crew brief is carried as the child's direction, not as a per-agent brief. The older `pack.json` packs still list under "Deploy on your own machine".
34
35 ## A source may not point inside (#11971)
36 Before a pack's source is cloned, `src/packs/source_guard.ts` resolves its host and refuses it when any answer is loopback, private, link-local, unique-local, CGNAT/tailnet or unspecified (the list in `src/kernel/outbound.ts`, the forge's one). Git then dials the address that was judged and follows no redirect. Only port 443 is allowed, unless the forge itself is the source. `parsePack` drops an internal address literal at once; the port rule is the guard's alone, because only it knows the forge's own address. Every deploy path runs the guard on the address the pack names, even when a test clones from elsewhere.
scripts/pack.ts +16 −2
@@ -16,6 +16,7 @@ import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
1616 import { tmpdir } from "node:os";
1717 import { join } from "node:path";
1818 import { ForgeApi } from "../src/cli/api.ts";
19 import { guardSource, type SourceGuardOptions } from "../src/packs/source_guard.ts";
1920 import { deployPack, type Hands } from "../src/packs/deploy.ts";
2021 import { parsePack, type Pack } from "../src/packs/pack.ts";
2122 import { finishWords, OWNER, prepareOwner } from "../src/packs/up.ts";
@@ -84,6 +85,8 @@ async function git(args: string[], env: Record<string, string>, cwd?: string): P
8485 export interface HandsOptions {
8586 /** Where to clone a pack's source from, when not the address the pack names (a test's local repository). */
8687 readonly cloneFrom?: (source: string) => string;
88 /** How a source's host is judged (resolver, exempt addresses): a test's seam. The forge's own address is always let through. */
89 readonly guard?: SourceGuardOptions;
8790 }
8891
8992 async function gitOut(args: string[], env: Record<string, string>, cwd: string): Promise<string | null> {
@@ -128,7 +131,13 @@ export function realHands(url: string, token: string, options: HandsOptions = {}
128131 try {
129132 const bare = join(dir, "src.git");
130133 // Source history is fetched without hooks or templates and pushed as plain branches and tags.
131 const cloned = await git(["clone", "--bare", "--quiet", ...(branch === undefined ? [] : ["--branch", branch]), options.cloneFrom?.(source) ?? cloneUrl(source), bare], cloneEnv());
134 const from = options.cloneFrom?.(source) ?? cloneUrl(source);
135 // The address the pack names is judged and pinned before any connection, whatever is cloned
136 // in its place: no internal host, no redirect, port 443 (#11971). A test's local clone keeps
137 // git's plain environment, since the pinned one allows only http and https.
138 const guarded = await guardSource(cloneUrl(source), cloneEnv(), { forge: url, ...options.guard });
139 if (!guarded.ok) return { ok: false, why: guarded.error };
140 const cloned = await git(["clone", "--bare", "--quiet", ...(branch === undefined ? [] : ["--branch", branch]), from, bare], options.cloneFrom === undefined ? guarded.value : cloneEnv());
132141 if (cloned !== null) return { ok: false, why: cloned };
133142 const pushed = await git(
134143 ["push", "--quiet", "--force", `${url}/${owner}/${name}.git`, "refs/heads/*:refs/heads/*", "refs/tags/*:refs/tags/*"],
@@ -201,6 +210,8 @@ export interface UpOptions {
201210 readonly dataDir: string;
202211 /** Where to clone a pack's source from (a test's local repository). */
203212 readonly cloneFrom?: (source: string) => string;
213 /** How a source's host is judged: a test's seam, passed to the hands. */
214 readonly guard?: SourceGuardOptions;
204215 /** Where the words go; the console by default. */
205216 readonly say?: (line: string) => void;
206217 /** Aborting stops the forge and ends `up`; Ctrl-C and SIGTERM do the same. */
@@ -238,7 +249,10 @@ export async function up(pack: Pack, options: UpOptions): Promise<number> {
238249 await child.exited;
239250 return 1;
240251 }
241 const steps = await deployPack(pack, realHands(url, prepared.token, options.cloneFrom === undefined ? {} : { cloneFrom: options.cloneFrom }));
252 const steps = await deployPack(pack, realHands(url, prepared.token, {
253 ...(options.cloneFrom === undefined ? {} : { cloneFrom: options.cloneFrom }),
254 ...(options.guard === undefined ? {} : { guard: options.guard }),
255 }));
242256 for (const s of steps) say(`${mark(s)} ${s.what}${s.note ? ` — ${s.note}` : ""}`);
243257 if (prepared.passphrase !== undefined) say(`\nSign in as "${OWNER}" with the passphrase kept in ${prepared.passphraseFile}`);
244258 for (const line of finishWords(url, prepared.credential)) say(line);
src/packs/pack.ts +6
@@ -7,6 +7,8 @@
77 * like one is refused, never ignored, so a published pack cannot ask for them.
88 */
99
10 import { isIP } from "node:net";
11 import { isRefusedAddress } from "../kernel/outbound.ts";
1012 import { err, ok, type Result } from "../kernel/result.ts";
1113 import { CREW_ROLES, type CrewRole } from "../core/agents.ts";
1214
@@ -67,6 +69,10 @@ export function sourceOf(v: unknown): string | undefined {
6769 const url = raw === undefined ? null : URL.parse(raw);
6870 if (url === null || url.protocol !== "https:" || url.username !== "" || url.password !== "") return undefined;
6971 if (url.search !== "" || url.hash !== "") return undefined;
72 // An internal address literal is refused at once; a name and a port are judged when it is
73 // cloned (`source_guard.ts`), which alone knows the forge's own address.
74 const literal = url.hostname.replace(/^\[|\]$/g, "");
75 if ((isIP(literal) !== 0 && isRefusedAddress(literal))) return undefined;
7076 const parts = url.pathname.split("/").filter((s) => s !== "");
7177 if (parts.length !== 2) return undefined;
7278 const repo = parts[1]!.replace(/\.git$/, "");
src/packs/source_guard.ts added +49
@@ -0,0 +1,49 @@
1 /**
2 * Where a pack's source may point (#11971). Before anything is cloned for a
3 * pack, its host is resolved and judged by the forge's one address list
4 * (`kernel/outbound.ts`: loopback, RFC 1918, link-local, unique-local,
5 * CGNAT/tailnet, unspecified, multicast); git is then told to dial the address
6 * that passed and to follow no redirect, so a second lookup or a 302 cannot
7 * reach a refused one. Only port 443 is allowed, unless the forge itself is
8 * the source. Pure but for the lookup, which is handed in.
9 */
10
11 import { err, ok, type Result } from "../kernel/result.ts";
12 import { vetHost, type OutboundOptions } from "../kernel/outbound.ts";
13 import { pinFor, remoteGitEnv } from "../git/remote-env.ts";
14
15 export interface SourceGuardOptions extends OutboundOptions {
16 /** The forge's own address: a source on this host and port is the forge itself, let through. */
17 readonly forge?: string;
18 /** Permit any port. For tests only. */
19 readonly anyPort?: boolean;
20 }
21
22 const hostOf = (url: URL): string => `${url.hostname}:${url.port !== "" ? url.port : url.protocol === "https:" ? "443" : "80"}`;
23
24 /** The one sentence a refused source gets. */
25 export const refusal = (host: string, why: string): string =>
26 `the source host ${host.slice(0, 100)} ${why}, so the forge will not clone it.`;
27
28 /**
29 * The git environment pinned to the vetted address of `address`'s host, or
30 * the sentence naming the host and why it is refused. `base` is git's
31 * environment; the pin and the no-redirect rule are added to it.
32 */
33 export async function guardSource(
34 address: string,
35 base: Record<string, string>,
36 options: SourceGuardOptions = {},
37 ): Promise<Result<Record<string, string>, string>> {
38 const url = URL.parse(address);
39 if (url === null) return err(`the source ${address.slice(0, 100)} is not an address, so the forge will not clone it.`);
40 const forge = options.forge === undefined ? null : URL.parse(options.forge);
41 if (forge !== null && hostOf(forge) === hostOf(url)) return ok(remoteGitEnv(base));
42 if (url.protocol !== "https:" && options.plain !== true) return err(`the source ${address.slice(0, 100)} is not an https address, so the forge will not clone it.`);
43 const bare = url.hostname.replace(/^\[|\]$/g, "");
44 if (options.anyPort !== true && hostOf(url) !== `${url.hostname}:443`) return err(refusal(bare, "is on a port other than 443"));
45 const vetted = await vetHost(bare, options);
46 if (!vetted.ok)
47 return err(refusal(bare, vetted.error === "refused" ? "is, or resolves to, an internal address" : "could not be resolved"));
48 return ok(remoteGitEnv(base, pinFor(address, vetted.value[0]!)));
49 }
test/packs-hands.test.ts +71 −2
@@ -19,9 +19,12 @@ import { createForgeApp, forgeCommands } from "../src/tui/forge.ts";
1919 import { packLines, packsScreen } from "../src/tui/screens/packs.ts";
2020 import { headlessTerminal } from "../src/tui/term/terminal.ts";
2121 import { cloneEnv, cloneUrl, pushEnv, realHands } from "../scripts/pack.ts";
22 import { guardSource } from "../src/packs/source_guard.ts";
2223 import { quietLogs } from "./helpers/globals.ts";
2324 import { settleApp } from "./helpers/tui.ts";
2425
26 const LOOP = { allow: ["127.0.0.1"], anyPort: true, plain: true };
27
2528 quietLogs();
2629
2730 describe("git credentials", () => {
@@ -45,7 +48,7 @@ describe("git credentials", () => {
4548 },
4649 });
4750 try {
48 const r = await realHands("http://127.0.0.1:1", "secret-token").carryHistory(`http://127.0.0.1:${stranger.port}/x.git`, "o", "n", undefined);
51 const r = await realHands("http://127.0.0.1:1", "secret-token", { guard: LOOP }).carryHistory(`http://127.0.0.1:${stranger.port}/x.git`, "o", "n", undefined);
4952 expect(r.ok).toBe(false);
5053 expect(seen.length).toBeGreaterThan(0);
5154 expect(seen.every((h) => h === null)).toBe(true);
@@ -72,7 +75,7 @@ describe("clone URL", () => {
7275 },
7376 });
7477 try {
75 const r = await realHands("http://127.0.0.1:1", "t").carryHistory(`http://127.0.0.1:${stranger.port}/owner/name`, "o", "n", undefined);
78 const r = await realHands("http://127.0.0.1:1", "t", { guard: LOOP }).carryHistory(`http://127.0.0.1:${stranger.port}/owner/name`, "o", "n", undefined);
7679 expect(r.ok).toBe(false);
7780 expect(seen.some((p) => p.startsWith("/owner/name.git/"))).toBe(true);
7881 expect(seen.some((p) => p.startsWith("/owner/name/") && !p.startsWith("/owner/name.git/"))).toBe(false);
@@ -370,3 +373,69 @@ describe("forge-repo sources", () => {
370373 expect(r.ok && r.value.apps[0]!.source).toBe("https://github.com/a/b");
371374 });
372375 });
376
377 describe("a pack source may not point at an internal address (#11971)", () => {
378 const via = (map: Record<string, string[]>) => ({ resolve: async (h: string) => map[h] ?? ["93.184.216.34"] });
379 const refused = async (source: string, guard?: object) => {
380 const r = await realHands("https://forge.example", "t", guard === undefined ? {} : { guard }).carryHistory(source, "o", "n", undefined);
381 expect(r.ok).toBe(false);
382 return r.ok ? "" : r.why;
383 };
384
385 test("literal internal addresses are refused before any connection, naming the host", async () => {
386 for (const host of ["127.0.0.1", "10.1.2.3", "192.168.1.5", "100.84.0.9", "[::1]", "169.254.169.254"]) {
387 const why = await refused(`https://${host}/a/b`);
388 expect(why).toContain(host.replace(/[[\]]/g, ""));
389 expect(why).toContain("internal address");
390 }
391 });
392
393 test("a public name that resolves to a private address is refused", async () => {
394 const why = await refused("https://sneaky.example/a/b", via({ "sneaky.example": ["93.184.216.34", "10.0.0.7"] }));
395 expect(why).toContain("sneaky.example");
396 });
397
398 test("a port other than 443 is refused", async () => {
399 expect(await refused("https://example.com:8443/a/b", via({}))).toContain("443");
400 });
401
402 test("a public host that redirects to an internal one is not followed", async () => {
403 let target = 0;
404 const inner = Bun.serve({ port: 0, fetch: () => (target++, new Response("no", { status: 404 })) });
405 const outer = Bun.serve({ port: 0, fetch: () => new Response(null, { status: 302, headers: { location: `http://127.0.0.1:${inner.port}/x.git/info/refs` } }) });
406 try {
407 const why = await refused(`http://127.0.0.1:${outer.port}/a/b`, LOOP);
408 expect(why).toMatch(/redirect|unable|fatal|error/i);
409 expect(target).toBe(0);
410 } finally {
411 await outer.stop(true);
412 await inner.stop(true);
413 }
414 });
415
416 test("the pack format refuses an internal literal and a port at parse time", () => {
417 for (const source of ["https://127.0.0.1/a/b", "https://10.0.0.1/a/b", "https://[::1]/a/b"]) {
418 const pack = { pack: 1, name: "p", title: "P", version: "1.0.0", description: "d", apps: [{ name: "a", description: "d", source }] };
419 expect(parsePack(JSON.stringify(pack)).ok).toBe(false);
420 }
421 });
422
423 test("a source on a port is left to the guard, which lets the forge's own port through", async () => {
424 const pack = { pack: 1, name: "p", title: "P", version: "1.0.0", description: "d", apps: [{ name: "a", description: "d", source: "https://forge.example:8443/a/b" }] };
425 expect(parsePack(JSON.stringify(pack)).ok).toBe(true);
426 expect((await guardSource("https://forge.example:8443/a/b.git", {}, { forge: "https://forge.example:8443", ...via({}) })).ok).toBe(true);
427 expect((await guardSource("https://other.example:8443/a/b.git", {}, { forge: "https://forge.example:8443", ...via({}) })).ok).toBe(false);
428 });
429
430 test("a clone seam does not skip the guard", async () => {
431 const r = await realHands("https://forge.example", "t", { cloneFrom: () => "/nonexistent" }).carryHistory("https://127.0.0.1/a/b", "o", "n", undefined);
432 expect(r.ok).toBe(false);
433 expect(r.ok ? "" : r.why).toContain("internal address");
434 });
435
436 test("a public https source passes the guard and is pinned to the address that was judged", async () => {
437 const r = await guardSource("https://example.com/a/b.git", {}, via({}));
438 expect(r.ok).toBe(true);
439 if (r.ok) expect(Object.values(r.value)).toContain("example.com:443:93.184.216.34");
440 });
441 });
test/packs-up.test.ts +5 −4
@@ -45,6 +45,7 @@ let app: App;
4545 let servers: Servers;
4646 let token: string;
4747 let passphrase: string | undefined;
48 const PUBLIC = { resolve: async () => ["93.184.216.34"] };
4849 const fake = new Map<string, string>();
4950
5051 beforeAll(async () => {
@@ -104,7 +105,7 @@ describe("the one command, against a fake source", () => {
104105 for (const name of ["games", "business"]) {
105106 test(`${name}: every app arrives with history, followed; the crew is configured; no secret or variable travels`, async () => {
106107 const pack = fixturePacks().find((p) => p.name === name)!;
107 const steps = await deployPack(pack, realHands(servers.url, token, { cloneFrom: (s) => fake.get(s)! }));
108 const steps = await deployPack(pack, realHands(servers.url, token, { cloneFrom: (s) => fake.get(s)!, guard: PUBLIC }));
108109 expect(steps.filter((s) => s.state !== "done")).toEqual([]);
109110 for (const a of pack.apps) {
110111 const repo = getRepo(app.db, OWNER, a.name)!;
@@ -131,7 +132,7 @@ describe("the one command, against a fake source", () => {
131132
132133 test("running it again keeps what exists", async () => {
133134 const pack = fixturePacks()[0]!;
134 const steps = await deployPack(pack, realHands(servers.url, token, { cloneFrom: (s) => fake.get(s)! }));
135 const steps = await deployPack(pack, realHands(servers.url, token, { cloneFrom: (s) => fake.get(s)!, guard: PUBLIC }));
135136 expect(steps.map((s) => `${s.what} ${s.state} ${s.note}`)).toEqual(steps.map((s) => `${s.what} kept already here`));
136137 });
137138
@@ -160,7 +161,7 @@ describe("the one command, against a fake source", () => {
160161
161162 test("a partial deploy is finished by running it again: a kept app that was never followed is followed", async () => {
162163 const pack = fixturePacks()[0]!;
163 const hands = realHands(servers.url, token, { cloneFrom: (s) => fake.get(s)! });
164 const hands = realHands(servers.url, token, { cloneFrom: (s) => fake.get(s)!, guard: PUBLIC });
164165 // First run: the follow of the first app fails after the app and its history are in.
165166 const one = { ...pack, apps: [{ ...pack.apps[0]!, name: "partial-app" }], crew: [] };
166167 let fail = true;
@@ -235,7 +236,7 @@ describe("up() itself", () => {
235236 for (const n of ["ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN", "FORGE_CLAUDE_CONFIG_DIR"]) delete env[n];
236237 try {
237238 const pack = fixturePacks()[0]!;
238 const running = up(pack, { port, dataDir: d, cloneFrom: (s) => fake.get(s)!, say: (l) => lines.push(l), signal: stop.signal, env });
239 const running = up(pack, { port, dataDir: d, cloneFrom: (s) => fake.get(s)!, guard: PUBLIC, say: (l) => lines.push(l), signal: stop.signal, env });
239240 let ended: number | undefined;
240241 void running.then((c) => (ended = c));
241242 for (let i = 0; i < 700 && ended === undefined && !lines.some((l) => l.startsWith("Your forge is serving")); i++) await Bun.sleep(100);

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs