From 76859d13eb44076a07bb997c4ff29535e12fde50 Mon Sep 17 00:00:00 2001 From: Jacob Prall Date: Sat, 31 Oct 2026 15:33:50 -0700 Subject: [PATCH] Limit Sandbox egress to an HTTPS allow-list Task Sandboxes reach github.com, the agents' model APIs, and the repository's configured egress hosts. The snapshot builder reaches github.com, deb.debian.org, and the repository's hosts; install-tools.sh switches the image's Debian sources to HTTPS so apt works under the list. A repository can opt out with egress: "allow-all". Co-authored-by: Cursor --- AGENTS.md | 8 +++--- README.md | 6 +++-- app/config/index.ts | 23 +++++++++++++++- app/harness/pi.ts | 6 +++++ app/harness/types.ts | 5 ++++ app/sandbox/egress.ts | 42 +++++++++++++++++++++++++++++ app/sandbox/sandbox.ts | 13 +++++---- app/sandbox/snapshot.ts | 14 +++++----- app/sandbox/verify.ts | 10 +------ docs/architecture.md | 22 ++++++++++++--- docs/configuration.md | 45 +++++++++++++++++++++++++++++++ docs/runbooks/extend.md | 5 +++- docs/runbooks/setup.md | 8 ++++-- factory.config.ts | 7 +++++ sandbox/install-tools.sh | 4 +++ tests/agents-catalog.test.ts | 4 +++ tests/config.test.ts | 27 +++++++++++++++++++ tests/harness-pi.test.ts | 11 ++++++++ tests/helpers/scripted-harness.ts | 1 + tests/sandbox-render.test.ts | 29 ++++++++++++++++++-- 20 files changed, 256 insertions(+), 34 deletions(-) create mode 100644 app/sandbox/egress.ts create mode 100644 tests/config.test.ts diff --git a/AGENTS.md b/AGENTS.md index 8575657a..2dac06a5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,7 +90,8 @@ strict output schemas. ## Cross-cutting changes - Configuration key: update `FactoryConfig` in `factory.config.ts` and - `docs/configuration.md`. + `docs/configuration.md`. Check what the type system can't in + `validateConfig` (`app/config/index.ts`). - Environment variable: update `.env.example`, `SERVICE_ENV` and, for a secret, `ENV_GROUPS` in `scripts/lib/requirements.ts`, and `docs/configuration.md`. Secrets belong in an env group, not a @@ -99,8 +100,9 @@ strict output schemas. - App permission, event, or label: update `scripts/lib/requirements.ts`, setup behavior, and doctor tests. - Agent harness: declare the env var of its key as the adapter's - `credential`; setup and `doctor` derive the key list. It must give the - model only the factory's tools; add it to `HARNESSES` in + `credential` and the hosts it calls as its `egress`; setup and `doctor` + derive the key list, and Sandbox allow-lists include every harness's + hosts. It must give the model only the factory's tools; add it to `HARNESSES` in `tests/helpers/runner.ts` if it can run the conformance suite. - `sandbox/`: refresh every repository snapshot after deployment. Changes to `app/runner/`, `app/harness/`, or agents reach Sandboxes with the Workflows diff --git a/README.md b/README.md index 7cd42419..0f028e0f 100644 --- a/README.md +++ b/README.md @@ -86,9 +86,11 @@ tests, authority rules, deploys, and snapshot refreshes. submodules, oversized changes, and branches with human commits. - GitHub must reproduce the candidate tree SHA before the factory publishes it. +- Sandboxes reach only an HTTPS allow-list: GitHub, the agents' model APIs, + and the hosts each repository's install and verify commands need. -Sandboxes use allow-all egress, and the model key exists in runner memory -during a call. Read the [trust boundaries](docs/architecture.md#trust-boundaries) +Allowed hosts can still carry data out, and the model key exists in runner +memory during a call. Read the [trust boundaries](docs/architecture.md#trust-boundaries) and [residual risks](docs/architecture.md#residual-risks) before using private code or untrusted pull requests. diff --git a/app/config/index.ts b/app/config/index.ts index 0d6a3b4c..7e456823 100644 --- a/app/config/index.ts +++ b/app/config/index.ts @@ -1,7 +1,28 @@ -import config, { type RepoConfig } from "../../factory.config.js"; +import config, { type FactoryConfig, type RepoConfig } from "../../factory.config.js"; export { config, type RepoConfig }; +/** A lowercase hostname with at least one dot, as the Sandbox egress rules take it; `*.` only as the leftmost label. */ +const EGRESS_HOST = /^(\*\.)?([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.)+[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$/; + +/** Throws naming the first host that isn't a bare hostname: no scheme, port, path, or uppercase. */ +export function assertEgressHosts(hosts: readonly string[], where: string): void { + for (const host of hosts) { + if (!EGRESS_HOST.test(host)) { + throw new Error(`${where}: egress host ${JSON.stringify(host)} must be a lowercase hostname such as registry.npmjs.org or *.example.com`); + } + } +} + +/** Checks what the type system can't. Runs when this module loads, so an invalid config stops every service at startup. */ +export function validateConfig(c: FactoryConfig): void { + for (const [fullName, repo] of Object.entries(c.repos)) { + if (repo.egress !== undefined && repo.egress !== "allow-all") assertEgressHosts(repo.egress, `factory.config.ts repos["${fullName}"]`); + } +} + +validateConfig(config); + /** One repository a run acts on, resolved from the webhook that started it. */ export interface RepoContext { owner: string; diff --git a/app/harness/pi.ts b/app/harness/pi.ts index 8e65c005..77852e1a 100644 --- a/app/harness/pi.ts +++ b/app/harness/pi.ts @@ -23,6 +23,11 @@ export interface PiOptions { models?: () => MutableModels; /** The env var holding the key. Default: the provider's, from PI_PROVIDER_KEY_ENV; required for other providers. */ credential?: string; + /** + * The hosts model requests go to. Default: the host of `model.baseUrl`. Set it when `models` resolves the model + * to a different base URL, such as a proxy. + */ + egress?: string[]; } /** Runs an agent on Pi (`@earendil-works/pi-agent-core`) with the factory's tools. */ @@ -37,6 +42,7 @@ export function pi(opts: PiOptions): Harness { name: "pi", label: `${provider}/${id}`, credential, + egress: opts.egress ?? [new URL(opts.model.baseUrl).hostname], driver: piDriver({ model: opts.model, thinkingLevel, models: opts.models }), }; } diff --git a/app/harness/types.ts b/app/harness/types.ts index bf77df5d..9855741c 100644 --- a/app/harness/types.ts +++ b/app/harness/types.ts @@ -12,4 +12,9 @@ export interface Harness extends HarnessRuntime { * for each run; setup and doctor manage it. */ readonly credential: string; + /** + * The HTTPS hosts the harness calls from the sandbox, such as its model provider's API. Sandboxes block every + * destination not on their allow-list, so a host missing here makes every model request fail. + */ + readonly egress: readonly string[]; } diff --git a/app/sandbox/egress.ts b/app/sandbox/egress.ts new file mode 100644 index 00000000..78f2cc62 --- /dev/null +++ b/app/sandbox/egress.ts @@ -0,0 +1,42 @@ +import type { SandboxNetworkPolicy } from "@renderinc/sdk/experimental"; +import { AGENTS } from "../agents/registry.js"; +import { assertEgressHosts, type RepoContext } from "../config/index.js"; + +/** Clone and fetch reach GitHub over HTTPS. */ +const GITHUB_HOST = "github.com"; +/** install-tools.sh installs tools the base image lacks from Debian's mirror, switched to HTTPS. */ +const DEBIAN_MIRROR = "deb.debian.org"; + +/** Every registered agent's model hosts: a sandbox opens before its task picks agents, and one sandbox can run several. */ +const MODEL_HOSTS = AGENTS.flatMap((agent) => agent.harness.egress); + +/** + * The policy a task sandbox is created with. Render fixes it at creation: a restored snapshot doesn't carry one, + * and a running sandbox's policy can't change. Under `allow-list` only HTTPS to these hosts gets out. + */ +export function taskEgress(repo: RepoContext): SandboxNetworkPolicy { + return egressPolicy(repo, [GITHUB_HOST, ...MODEL_HOSTS]); +} + +/** The snapshot builder's policy: it never calls a model, but it installs base-image tools. */ +export function builderEgress(repo: RepoContext): SandboxNetworkPolicy { + return egressPolicy(repo, [GITHUB_HOST, DEBIAN_MIRROR]); +} + +/** Appended to install failures: a blocked host fails as a reset connection, which reads like a network flake. */ +export function egressHint(repo: RepoContext, policy: SandboxNetworkPolicy): string { + if (policy.type !== "allow-list") return ""; + return ( + `\nSandbox egress is limited to ${policy.rules.map((rule) => rule.domain).join(", ")}. ` + + `If install needs another host, add it to egress for "${repo.fullName}" in factory.config.ts.` + ); +} + +function egressPolicy(repo: RepoContext, base: readonly string[]): SandboxNetworkPolicy { + const extra = repo.config.egress ?? []; + if (extra === "allow-all") return { type: "allow-all" }; + // The API rejects a domain listed twice. + const domains = [...new Set([...base, ...extra])]; + assertEgressHosts(domains, `sandbox egress for ${repo.fullName}`); + return { type: "allow-list", rules: domains.map((domain) => ({ domain })) }; +} diff --git a/app/sandbox/sandbox.ts b/app/sandbox/sandbox.ts index e5f208cb..d2ec6c67 100644 --- a/app/sandbox/sandbox.ts +++ b/app/sandbox/sandbox.ts @@ -1,8 +1,10 @@ import { randomUUID } from "node:crypto"; import { SandboxSnapshotNotFoundError, SandboxSnapshotNotReadyError } from "@renderinc/sdk"; +import type { SandboxNetworkPolicy } from "@renderinc/sdk/experimental"; import { z } from "zod"; import { config, type RepoContext } from "../config/index.js"; import { type SandboxClient, sandboxClient } from "./client.js"; +import { egressHint, taskEgress } from "./egress.js"; import { errorMessage, log, SAFE_REF } from "./shell.js"; import { asRunner, @@ -19,7 +21,7 @@ import { snapshotName, } from "./snapshot.js"; import { APP_DIR, type OpenedSandbox, REPO_DIR, RUN_AS, type Sandbox, type SandboxRef, SCRATCH_DIR, type SnapshotManifest } from "./types.js"; -import { ALLOW_ALL_EGRESS, verifySandbox } from "./verify.js"; +import { verifySandbox } from "./verify.js"; const SnapshotManifestSchema = z.object({ repo: z.string(), @@ -66,7 +68,8 @@ export interface OpenSandboxDeps extends SnapshotDeps { export async function openSandbox(repo: RepoContext, ref: SandboxRef, deps: OpenSandboxDeps = {}): Promise { checkRef(ref); const client = deps.client ?? sandboxClient(); - const id = await createFromSnapshot(client, repo, deps); + const egress = taskEgress(repo); + const id = await createFromSnapshot(client, repo, egress, deps); try { await client.waitRunning(id); await ensureRunnerPayload(client, id); @@ -82,7 +85,7 @@ export async function openSandbox(repo: RepoContext, ref: SandboxRef, deps: Open const lockfileHash = await computeLockfileHash(client, id); if (lockfileHash !== manifest.lockfileHash || install !== manifest.install) { log("info", "reinstalling dependencies", { sandbox: id, repo: repo.fullName, headSha }); - await runInstall(client, id, install); + await runInstall(client, id, install, egressHint(repo, egress)); } } return { ...connectSandbox(id, client), headSha, manifest }; @@ -120,14 +123,14 @@ function isSnapshotUnavailable(err: unknown): boolean { return err instanceof SandboxSnapshotNotFoundError || err instanceof SandboxSnapshotNotReadyError; } -async function createFromSnapshot(client: SandboxClient, repo: RepoContext, deps: OpenSandboxDeps): Promise { +async function createFromSnapshot(client: SandboxClient, repo: RepoContext, egress: SandboxNetworkPolicy, deps: OpenSandboxDeps): Promise { const name = snapshotName(repo); const create = async () => ( await client.call(() => client.api.create({ snapshotName: name, - networkPolicy: ALLOW_ALL_EGRESS, + networkPolicy: egress, timeoutSeconds: config.sandbox.timeoutSeconds, }), ) diff --git a/app/sandbox/snapshot.ts b/app/sandbox/snapshot.ts index 725267d9..df0a2edb 100644 --- a/app/sandbox/snapshot.ts +++ b/app/sandbox/snapshot.ts @@ -12,9 +12,10 @@ import type { RepoContext } from "../config/index.js"; import { githubToken } from "../github/auth.js"; import { bundleRunner } from "../runner/bundle.js"; import { type SandboxApi, type SandboxClient, TRANSIENT } from "./client.js"; +import { builderEgress, egressHint } from "./egress.js"; import { errorMessage, log, SAFE_REF, shq } from "./shell.js"; import { APP_DIR, REPO_DIR, RUN_AS, SANDBOX_ROOT, SCRATCH_DIR, type SnapshotManifest } from "./types.js"; -import { ALLOW_ALL_EGRESS, verifySandbox } from "./verify.js"; +import { verifySandbox } from "./verify.js"; /** The one named snapshot per repository: `factory--`, restricted to [A-Za-z0-9_.-]. */ export function snapshotName(repo: Pick): string { @@ -78,7 +79,7 @@ export function credentialFile(token: string): string { // ── Build ─────────────────────────────────────────────────────────────── // -// Each step is an exec or upload in an allow-all builder sandbox, ordered by trust: +// Each step is an exec or upload in a builder sandbox limited to builderEgress, ordered by trust: // // 1. Upload the scripts and runner payload; install tools; create the runner user; self-test the runner // 2. Upload a read-only token, clone at `sha`, scrub the token and verify none remains @@ -97,7 +98,8 @@ export async function buildSnapshot(client: SandboxClient, repo: RepoContext, sh const started = Date.now(); log("info", "snapshot build started", { repo: repo.fullName, sha, snapshot: name }); - const { id } = await client.call(() => client.api.create({ timeoutSeconds: BUILDER_TIMEOUT_SECONDS, networkPolicy: ALLOW_ALL_EGRESS })); + const egress = builderEgress(repo); + const { id } = await client.call(() => client.api.create({ timeoutSeconds: BUILDER_TIMEOUT_SECONDS, networkPolicy: egress })); try { await client.waitRunning(id); @@ -145,7 +147,7 @@ export async function buildSnapshot(client: SandboxClient, repo: RepoContext, sh await client.check(id, `chown -R ${RUN_AS}: ${REPO_DIR}`, prepare); const lockfileHash = await computeLockfileHash(client, id); const install = repo.config.install?.trim() || null; - if (install) await runInstall(client, id, install); + if (install) await runInstall(client, id, install, egressHint(repo, egress)); // ── 4. Manifest, lockdown, verify, snapshot ────────────────────── const manifest: SnapshotManifest = { @@ -222,10 +224,10 @@ export async function computeLockfileHash(client: SandboxClient, id: string): Pr * Runs the repository's install command as the runner user, then fails if it changed any tracked file: the * checkout is what agents edit and publish from, so a dirty tree would leak into every PR. */ -export async function runInstall(client: SandboxClient, id: string, install: string): Promise { +export async function runInstall(client: SandboxClient, id: string, install: string, failureHint = ""): Promise { const res = await client.exec(id, asRunner("bash", "-c", install), { timeoutMs: client.t.prepareTimeoutMs }); if (res.exitCode !== 0) { - throw new Error(`install (${install}) exited ${res.exitCode}: ${(res.stderr || res.stdout).trim().slice(-2000)}`); + throw new Error(`install (${install}) exited ${res.exitCode}: ${(res.stderr || res.stdout).trim().slice(-2000)}${failureHint}`); } const dirty = ( await client.check(id, asRunner("git", "status", "--porcelain", "--untracked-files=no"), { diff --git a/app/sandbox/verify.ts b/app/sandbox/verify.ts index be7dca64..ef97c941 100644 --- a/app/sandbox/verify.ts +++ b/app/sandbox/verify.ts @@ -1,16 +1,8 @@ import { z } from "zod"; -import type { SandboxApi, SandboxClient } from "./client.js"; +import type { SandboxClient } from "./client.js"; import { errorMessage } from "./shell.js"; import { APP_DIR } from "./types.js"; -type NetworkPolicy = NonNullable[0]>["networkPolicy"]>; - -/** - * Allow-all egress for every sandbox, in the shape the API accepts, `{ type }`. SDK 1.2.0's types still describe - * the older `{ default, allowedDomains }` shape. - */ -export const ALLOW_ALL_EGRESS = { type: "allow-all" } as unknown as NetworkPolicy; - const SandboxInfo = z.object({ runAs: z.string().min(1), uid: z.number().int(), diff --git a/docs/architecture.md b/docs/architecture.md index 63112b76..2778891c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -199,10 +199,18 @@ Clone and fetch credentials are read-only and temporary. Runtime files are root-owned and read-only. Agents and repository commands run as an unprivileged user. +Every Sandbox is created with an HTTPS egress allow-list built by the +Workflows host from trusted configuration. Task Sandboxes reach `github.com`, +the agents' model APIs, and the repository's configured `egress` hosts. The +snapshot builder reaches `github.com`, Debian's package mirror, and the +repository's hosts. Render enforces the list outside the Sandbox and fixes it +at creation, so nothing inside the Sandbox can widen it. See +[Sandbox egress](configuration.md#sandbox-egress). + The model key enters the Sandbox during an agent call. The runner receives it through a root-only path and stdin, removes provider keys from tool -environments, and runs under a non-dumpable Node binary. These controls reduce -exposure; they do not make untrusted code safe. +environments, and runs under a non-dumpable Node binary. These controls and +the allow-list reduce exposure; they do not make untrusted code safe. ## Security invariants @@ -220,12 +228,20 @@ Changes must preserve these properties: 9. Implement the specification as it existed when approved. 10. Give every Sandbox a provider timeout and `finally` cleanup. Record a GitHub task's Sandbox so cancellation can terminate it. +11. Create every Sandbox with an egress allow-list built from trusted + configuration. Only a repository's configured `egress: "allow-all"` lifts it. Tests pin these invariants and the tool authority of every built-in agent. ## Residual risks -- Sandboxes use allow-all egress. +- Allowed hosts remain exfiltration paths. Code in a Sandbox can send data to + an attacker's own GitHub account or model API account through `github.com` + or the model API, and to anything a wildcard rule admits. Task Sandboxes + keep `github.com` for their whole lifetime because the policy can't change + after the fetch. +- DNS resolution is not restricted by the allow-list. +- A repository configured with `egress: "allow-all"` has unrestricted egress. - The model key exists in runner memory during a call. - Prompt injection can influence specifications, code, and reviews. - Anyone with `minimumPermission` can start model spend. diff --git a/docs/configuration.md b/docs/configuration.md index 2f3e786f..f4c19fc4 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -16,11 +16,13 @@ export default defineFactoryConfig({ baseBranch: "main", install: "npm ci", verify: ["npm run build", "npm test"], + egress: ["registry.npmjs.org"], }, "acme/api": { baseBranch: "main", install: "go mod download", verify: ["go test ./..."], + egress: ["proxy.golang.org", "sum.golang.org"], }, }, labels: { @@ -52,11 +54,48 @@ from any other repository, even if the App is installed there. | `baseBranch` | Yes | Branch used for Triage, implementation, and snapshots | | `install` | No | Dependency command run after clone credentials are removed | | `verify` | Yes | Commands required before publishing and during PR review | +| `egress` | No | Extra HTTPS hosts this repository's Sandboxes may reach, or `"allow-all"` | Use deterministic install commands such as `npm ci`. Snapshot creation fails if installation changes a tracked file. Use `verify: []` only when you intentionally want no command-based verification. +### Sandbox egress + +Sandboxes reach the network through an HTTPS allow-list that Render enforces. +Every other destination fails as a reset connection. The factory builds each +list: + +| Sandbox | Allowed hosts | +|---|---| +| Task Sandbox | `github.com`, every agent harness's model API, and the repository's `egress` | +| Snapshot builder | `github.com`, `deb.debian.org`, and the repository's `egress` | + +List in `egress` every host the repository's `install` and `verify` commands +download from. Task Sandboxes need the list too: they reinstall when the +lockfiles change. Common hosts: + +| Ecosystem | Hosts | +|---|---| +| npm | `registry.npmjs.org` | +| Go modules | `proxy.golang.org`, `sum.golang.org` | +| Go toolchain download | `go.dev`, `dl.google.com` | +| PyPI | `pypi.org`, `files.pythonhosted.org` | + +A rule matches one exact hostname. Use `*.example.com` to admit its +subdomains; a wildcard may only be the leftmost label. Only HTTPS is admitted. +An invalid host stops both services at startup. An install that fails under +the allow-list names the allowed hosts in its error. + +Set `egress: "allow-all"` to lift the restriction for one repository. Prefer +listing hosts: the allow-list is what stops dependency scripts and +prompt-injected agents from sending code or the model key to arbitrary +destinations. + +Render fixes a Sandbox's policy when it is created, so an `egress` change +applies to the next Sandbox after a Workflows deploy. No snapshot refresh is +needed. + After adding a repository: 1. commit and deploy the configuration; @@ -124,6 +163,12 @@ needs no other configuration change. The `pi` adapter supports Anthropic 3. deploy `factory-workflows`; and 4. run `npm run doctor -- --service workflows`. +Each harness also declares its `egress`: the hosts its model requests go to. +Every task Sandbox's allow-list includes the hosts of all registered agents. +The `pi` adapter uses the host of the model's `baseUrl`, so switching +providers needs no egress change. Pass `egress` to `pi()` when a custom +`models` registry serves the model from another host, such as a proxy. + ### Tool support Every harness gives the model the factory's own tools, so an agent's tools and diff --git a/docs/runbooks/extend.md b/docs/runbooks/extend.md index 6ad0fcad..f4cd36c6 100644 --- a/docs/runbooks/extend.md +++ b/docs/runbooks/extend.md @@ -142,7 +142,10 @@ driver that only translates the library. configuration and returns a `Harness` (`app/harness/types.ts`): - `name` and `label` name the library and model before a run; - `credential` is the env var on the Workflows service that holds its key; - the host sends its value into the Sandbox; and + the host sends its value into the Sandbox; + - `egress` lists the HTTPS hosts the library calls from the Sandbox, such + as its model API. Sandboxes block every host not on their allow-list, so + derive it from the library's configuration when you can; and - `driver` is a `Driver` (`app/runner/core/driver.ts`). 2. Write the driver in `app/runner//`. `open(ctx)` resolves the model and reports the library version; the session's `run()` converses with the diff --git a/docs/runbooks/setup.md b/docs/runbooks/setup.md index 073d7308..d506897b 100644 --- a/docs/runbooks/setup.md +++ b/docs/runbooks/setup.md @@ -74,6 +74,7 @@ repos: { baseBranch: "main", install: "npm ci", verify: ["npm run build", "npm test"], + egress: ["registry.npmjs.org"], }, }, ``` @@ -84,8 +85,11 @@ Requirements: - name an existing base branch; - confirm Issues is enabled under the target repository's Settings > General > Features (ask an admin to enable it if necessary); - use an install command that is deterministic and leaves tracked files - unchanged; and -- list every command required before publishing. + unchanged; +- list every command required before publishing; and +- list in `egress` every host the install and verify commands download from, + such as the package registry. Sandboxes block every other destination. See + [Sandbox egress](../configuration.md#sandbox-egress). Use `verify: []` only when the user explicitly wants no command-based verification. Keep the default labels, permission, limits, and Sandbox timeout diff --git a/factory.config.ts b/factory.config.ts index 2444fbf8..db28014c 100644 --- a/factory.config.ts +++ b/factory.config.ts @@ -5,6 +5,12 @@ export interface RepoConfig { install?: string; /** Commands that must pass before a PR opens and that PR Reviewer runs. */ verify: string[]; + /** + * HTTPS hosts this repository's sandboxes may reach beyond github.com and the agents' model APIs: package + * registries and anything `install` or `verify` downloads. Exact hostnames; `*.` only as the leftmost label. + * Every other destination is blocked. "allow-all" lifts the restriction for this repository. + */ + egress?: string[] | "allow-all"; } export interface FactoryConfig { @@ -39,6 +45,7 @@ export default defineFactoryConfig({ baseBranch: "main", install: "npm ci", verify: ["npm run build", "npm test"], + egress: ["registry.npmjs.org"], }, }, labels: { diff --git a/sandbox/install-tools.sh b/sandbox/install-tools.sh index 420ba487..12e95fe0 100755 --- a/sandbox/install-tools.sh +++ b/sandbox/install-tools.sh @@ -20,6 +20,10 @@ have useradd || pkgs="$pkgs passwd" if [ -n "$pkgs" ]; then log "apt-get install$pkgs" export DEBIAN_FRONTEND=noninteractive + # The image's Debian sources use plain HTTP, and the sandbox egress allow-list admits only HTTPS. + for src in /etc/apt/sources.list /etc/apt/sources.list.d/*; do + if [ -f "$src" ]; then sed -i 's|http://deb\.debian\.org/|https://deb.debian.org/|g' "$src"; fi + done apt-get update -qq >&2 # shellcheck disable=SC2086 # intentional word splitting of the package list apt-get install -y -qq --no-install-recommends ca-certificates $pkgs >&2 diff --git a/tests/agents-catalog.test.ts b/tests/agents-catalog.test.ts index 0cbc203c..87d9abf6 100644 --- a/tests/agents-catalog.test.ts +++ b/tests/agents-catalog.test.ts @@ -33,6 +33,10 @@ describe("agent authority", () => { expect(agents.every((agent) => agent.tools.length > 0)).toBe(true); }); + it("lets every agent's sandbox reach only its model provider's API beyond the factory's own hosts", () => { + for (const agent of agents) expect(agent.harness.egress, agent.id).toEqual(["api.anthropic.com"]); + }); + it("rejects unknown fields in every object an agent can submit, so a misspelled field is named in the error", () => { const looseObjects = (schema: unknown, path: string): string[] => { if (typeof schema !== "object" || schema === null) return []; diff --git a/tests/config.test.ts b/tests/config.test.ts new file mode 100644 index 00000000..13652511 --- /dev/null +++ b/tests/config.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { validateConfig } from "../app/config/index.js"; +import { testConfig } from "./fixtures/config.js"; + +const withEgress = (egress: string[] | "allow-all") => ({ + ...testConfig, + repos: { "acme/web": { baseBranch: "main", verify: [], egress } }, +}); + +describe("egress configuration", () => { + it("accepts hostnames, a leftmost wildcard, and allow-all", () => { + expect(() => validateConfig(withEgress(["registry.npmjs.org", "*.pkg.dev", "proxy.golang.org"]))).not.toThrow(); + expect(() => validateConfig(withEgress("allow-all"))).not.toThrow(); + }); + + it.each([ + "https://registry.npmjs.org", + "registry.npmjs.org/", + "registry.npmjs.org:443", + "Registry.npmjs.org", + "localhost", + "api.*.example.com", + "", + ])("rejects %j, naming the repository", (host) => { + expect(() => validateConfig(withEgress([host]))).toThrow(`factory.config.ts repos["acme/web"]: egress host ${JSON.stringify(host)}`); + }); +}); diff --git a/tests/harness-pi.test.ts b/tests/harness-pi.test.ts index a54cbdeb..a9773d28 100644 --- a/tests/harness-pi.test.ts +++ b/tests/harness-pi.test.ts @@ -1,5 +1,7 @@ import { fauxAssistantMessage, fauxText, fauxToolCall } from "@earendil-works/pi-ai"; +import { getBuiltinModel } from "@earendil-works/pi-ai/providers/all"; import { describe, expect, it } from "vitest"; +import { pi } from "../app/harness/index.js"; import { fauxPi } from "./helpers/pi-scripted.js"; import { GOOD, makeInput, makeWorkspace, parseEvents, runContract, SUBMIT, terminal, toolResults } from "./helpers/runner.js"; @@ -10,6 +12,15 @@ const calls = (...list: [string, Record][]) => ); const overloaded = () => fauxAssistantMessage([], { stopReason: "error", errorMessage: "overloaded_error: Overloaded" }); +describe("pi harness egress", () => { + const model = getBuiltinModel("anthropic", "claude-sonnet-5"); + + it("defaults to the model's API host, and takes an explicit list for a model served elsewhere", () => { + expect(pi({ model }).egress).toEqual(["api.anthropic.com"]); + expect(pi({ model, egress: ["llm-proxy.example.com"] }).egress).toEqual(["llm-proxy.example.com"]); + }); +}); + describe("pi driver", { timeout: 30_000 }, () => { it("ends with no_submission when the model stops without submitting, without asking it again", async () => { const { harness, stub } = fauxPi([fauxAssistantMessage([fauxText("All done.")]), calls([SUBMIT, GOOD])]); diff --git a/tests/helpers/scripted-harness.ts b/tests/helpers/scripted-harness.ts index 4a0bd3f4..a70e716e 100644 --- a/tests/helpers/scripted-harness.ts +++ b/tests/helpers/scripted-harness.ts @@ -16,6 +16,7 @@ export function scripted(turns: readonly TestTurn[]): Harness { name: "scripted", label: "scripted", credential: "SCRIPTED_API_KEY", + egress: [], driver: { open: (ctx) => ({ version: "1.0.0", model: "scripted", run: () => play(ctx, turns) }) }, }; } diff --git a/tests/sandbox-render.test.ts b/tests/sandbox-render.test.ts index bc405a40..1a8e5a4b 100644 --- a/tests/sandbox-render.test.ts +++ b/tests/sandbox-render.test.ts @@ -35,6 +35,8 @@ const HEAD_SHA = "2".repeat(40); const HASH = "a".repeat(64); const NAME = "factory-acme-web"; +const allowList = (...domains: string[]) => ({ type: "allow-list", rules: domains.map((domain) => ({ domain })) }); + const repo = (overrides: Partial = {}): RepoContext => ({ owner: "acme", name: "web", @@ -193,7 +195,7 @@ describe("buildSnapshot", () => { const creates = fake.calls.filter((c) => c.op === "create"); expect(creates).toHaveLength(1); - expect(creates[0]?.input).toEqual({ timeoutSeconds: 3_600, networkPolicy: { type: "allow-all" } }); + expect(creates[0]?.input).toEqual({ timeoutSeconds: 3_600, networkPolicy: allowList("github.com", "deb.debian.org") }); const tokenUpload = fake.index((c) => c.op === "upload" && c.path === "/tmp/factory-stage/creds/github.com"); const order = [ @@ -275,6 +277,20 @@ describe("buildSnapshot", () => { expect(env.data).toContain("FACTORY_NODE=/usr/bin/node\n"); }); + it("adds the repository's egress hosts to the builder's allow-list, once each", async () => { + await buildSnapshot(client(), repo({ egress: ["registry.npmjs.org", "github.com"] }), BASE_SHA, { readToken }); + expect(fake.inputs("create")).toEqual([ + { timeoutSeconds: 3_600, networkPolicy: allowList("github.com", "deb.debian.org", "registry.npmjs.org") }, + ]); + }); + + it("names the allow-list when install fails, since a blocked host looks like a network flake", async () => { + fake.overrides = (_id, command) => (command.includes("'npm ci'") ? [out("npm error ECONNRESET\n", "stderr"), exit(1)] : []); + await expect(buildSnapshot(client(), repo(), BASE_SHA, { readToken })).rejects.toThrow( + 'Sandbox egress is limited to github.com, deb.debian.org. If install needs another host, add it to egress for "acme/web" in factory.config.ts.', + ); + }); + it("never installs when the scrub fails", async () => { fake.overrides = (_id, command) => command.includes("scrub-credentials.sh") ? [out("scrub-credentials: token for github.com found in x\n", "stderr"), exit(1)] : []; @@ -294,7 +310,7 @@ describe("openSandbox", () => { const creates = fake.calls.filter((c) => c.op === "create"); expect(creates.map((c) => c.input)).toEqual([ - { snapshotName: NAME, networkPolicy: { type: "allow-all" }, timeoutSeconds: config.sandbox.timeoutSeconds }, + { snapshotName: NAME, networkPolicy: allowList("github.com", "api.anthropic.com"), timeoutSeconds: config.sandbox.timeoutSeconds }, ]); expect(fake.commands().filter((c) => c.includes("verify.sh"))).toEqual(["bash /opt/factory/app/verify.sh"]); @@ -322,6 +338,15 @@ describe("openSandbox", () => { expect(fake.calls.some((c) => c.op === "terminate")).toBe(false); }); + it("allows the repository's egress hosts, and lifts the limit only for a repository configured allow-all", async () => { + await openSandbox(repo({ egress: ["proxy.golang.org", "*.example.com"] }), { kind: "branch", name: "main" }, deps()); + await openSandbox(repo({ egress: "allow-all" }), { kind: "branch", name: "main" }, deps()); + expect(fake.inputs("create").map((input) => (input as { networkPolicy: unknown }).networkPolicy)).toEqual([ + allowList("github.com", "api.anthropic.com", "proxy.golang.org", "*.example.com"), + { type: "allow-all" }, + ]); + }); + it("builds the snapshot inline exactly once when it is missing, then retries", async () => { let missing = true; fake.createError = (input) => (input?.snapshotName && missing ? new SandboxSnapshotNotFoundError("not found", 404) : undefined);