diff --git a/README.md b/README.md index 65a4396..88cab13 100644 --- a/README.md +++ b/README.md @@ -71,11 +71,12 @@ A repository adds one step and a `roach.json` file: run: pnpm test ``` -`roach.json` is at the root of the repository. It has the `rules` of the -run, and can have `allow` (`RunConfig` in `src/service.ts`): +`roach.json` is at the root of the repository. It has the `allow` origins +and the `rules` of the run (`RunConfig` in `src/service.ts`): ```json { + "allow": ["https://ai-gateway.vercel.sh"], "rules": [ { "name": "model", @@ -108,13 +109,16 @@ A run takes `RunConfig` (`src/service.ts`): `tenant`, `mode`, `rules`, recordings and fails a miss with HTTP 412, so nothing goes live. `record` sends every request live and records it again. `off` records and replays nothing. -- `allow`: the only origins that the proxy sends requests to. It refuses all - other origins with HTTP 403. The upstream host always comes from this - list, not from the client. +- `allow`: the only origins that the proxy sends requests to, 1 to 32 of + them. It refuses all other origins with HTTP 403. The upstream host + always comes from this list, not from the request. The service sends + requests only to public addresses, so an origin that resolves to a + private, loopback, or link-local address also gets HTTP 403. - `rules`: the traffic to record. `name` uses letters, digits, `_`, `.`, and `-`. `match` has `method`, `url` (a prefix), and `headers`. `keyHeaders` adds request headers to the key. `values` - names the values that change from run to run. + names the values that change from run to run. A run can have 64 rules, + and a rule can have 32 values. - Credentials: the proxy never writes the `secrets` of the service config or of the run. Use `secrets` of the run for the credentials of a CI job that a response can show but no request header has, such as a token that @@ -183,6 +187,14 @@ it, and the provider refuses a changed thinking block. Keep the patterns narrow. A broad pattern makes requests that differ in a real way use the same recording. +The service takes any pattern that compiles in 1 s, up to 2048 +characters. A run without the token must use the same patterns as the run +that recorded, or its keys do not match. All runs share one process, so +the key of each request has a time limit: 1 s, and 0.5 s for each MiB of +the body. The built-in patterns take less than 0.1 s for each MiB. A +request whose key takes longer fails with HTTP 400, and other runs go on. +Avoid nested quantifiers, such as `(a+)+`. + ## Misses The proxy adds each request without a recording to `stats().misses`, with @@ -214,10 +226,16 @@ first miss of each test, which is the one to fix. stats. A proxied request names its run in `Proxy-Authorization`, as `Basic base64(:)`. The proxy URL of the run has them, so `HTTPS_PROXY` sends them. A request without them gets HTTP 407. -- **Value patterns.** A pattern runs for every run in the process, so a - run can only use the patterns in `VALUE_PATTERNS` and the ones in - `valuePatterns` of the config. Check each one for slow backtracking - before you add it. +- **Config of a run.** Each run sends its own `allow` and `rules`, with + their value patterns. A tenant that calls a new origin or uses a new + pattern changes only its own config. The service needs no change and no + restart. Two checks keep one run from harming the others: + - The proxy sends requests only to public addresses (`addresses.ts`). + It checks the address of each connection after DNS, so a host name + cannot point a run at the metadata server, at the VPC, or at the VM. + Only tests and local development set `privateNetworks`. + - The key of a request has a time limit (`request-key.ts`), so a slow + pattern fails only its own request. - **Sentry.** With `sentryDsn`, each read and write of a recording sends the metric `roach.recording`. Its attributes are `tenant`, `rule`, `key`, `run` (the `name` of the run), and `result`: `replayed`, `missed`, or @@ -261,9 +279,8 @@ The `Image` workflow builds the image on each pull request and pushes it on 1. Make the package `ghcr.io/getsentry/roach` public once, after the first push to `main`, so the VM can pull it without credentials. 2. `deploy/gcp/roach.tfvars` has the values of the Sentry deployment: the - GCP project `roach-511216`, the domain, `allow`, and `value_patterns`. - `allow` and `value_patterns` must cover the rules of every tenant. Another - deployment changes these values. + GCP project `roach-511216` and the domain. Another deployment changes + these values. It has no origins or patterns: runs send them. 3. The Terraform state is in the private bucket that `backend "gcs"` in `versions.tf` names. The state holds the CA key and the write token, so the bucket must stay private. Make the bucket before the first @@ -296,10 +313,8 @@ To deploy a new image, restart the VM: To add a project, add its repository to the repositories of the `ROACH_TOKEN` organization secret, in the GitHub settings of the -organization. Its CI jobs set `tenant` to `GITHUB_REPOSITORY`. When the -project calls origins or uses value patterns that are not in -`roach.tfvars` yet, add them and apply again. Terraform writes a new config version, and the -VM reads it at its next start. +organization. Its CI jobs set `tenant` to `GITHUB_REPOSITORY`, and send +their own origins and patterns. The service needs no change. To change the write token, run `terraform apply -var-file=roach.tfvars -replace=random_password.write_token`, @@ -376,6 +391,7 @@ The proxy is in `src/`. Tests are in `tests/`. The production setup is in - `types.ts`: rules, modes, and the results of a run. - `server.ts`: sockets, HTTPS interception, the allow list, and the control API. +- `addresses.ts`: the public addresses that the proxy can send requests to. - `service.ts`: the shared service: tenants, runs, access, and metrics. - `recorder.ts`: modes, sessions, replay, and misses. - `request-key.ts`: the key of a request. diff --git a/deploy/gcp/main.tf b/deploy/gcp/main.tf index e51ae74..6ea2804 100644 --- a/deploy/gcp/main.tf +++ b/deploy/gcp/main.tf @@ -84,9 +84,7 @@ resource "google_secret_manager_secret_version" "config" { port = local.port publicUrl = "https://${var.domain}" bucket = google_storage_bucket.recordings.name - allow = var.allow writeTokenHash = sha256(random_password.write_token.result) - valuePatterns = var.value_patterns ca = { cert = tls_self_signed_cert.ca.cert_pem key = tls_private_key.ca.private_key_pem diff --git a/deploy/gcp/roach.tfvars b/deploy/gcp/roach.tfvars index 7a6aa47..8a60a8d 100644 --- a/deploy/gcp/roach.tfvars +++ b/deploy/gcp/roach.tfvars @@ -7,21 +7,7 @@ project = "roach-511216" # The record is in getsentry/ops (getsentry/ops#24232). domain = "roach-proxy.getsentry.net" -# The origins of Junior's evals (packages/junior-evals/src/recording-rules.ts). -allow = [ - "https://ai-gateway.vercel.sh", - "https://vercel.com", - "https://api.vercel.com", - "https://oidc.vercel.com", - "https://docs.slack.dev", -] - -# Junior's own value patterns, in addition to the built-in ones. Each one must -# be equal to its source in packages/junior-evals/src/recording-rules.ts, or -# the service refuses the run. -value_patterns = [ - "(? + Object.assign( + new Error( + `Roach does not send requests to ${host}, because ${address} is not a public address`, + ), + { status: 403, code: "ERR_ROACH_NOT_PUBLIC" }, + ); + +/** + * A DNS lookup for `http.request()` that fails when the host has an + * address that is not public. It checks all addresses of the host, so the + * choice of the connection does not matter. + */ +export const publicLookup: LookupFunction = (hostname, options, callback) => { + dnsLookup(hostname, { ...options, all: true }, (error, addresses) => { + if (error) { + callback(error, "", 0); + return; + } + const blocked = addresses.find(({ address }) => !isPublicAddress(address)); + if (blocked) { + callback(notPublicError(hostname, blocked.address), "", 0); + return; + } + if (options.all) { + // The type has only the single-address form of the callback. + (callback as unknown as (e: null, list: LookupAddress[]) => void)( + null, + addresses, + ); + return; + } + callback(null, addresses[0]!.address, addresses[0]!.family); + }); +}; diff --git a/src/certificates.ts b/src/certificates.ts index f10a5cc..1356bc4 100644 --- a/src/certificates.ts +++ b/src/certificates.ts @@ -3,7 +3,9 @@ * * The proxy intercepts HTTPS. It signs one certificate for each host when a * client first connects to that host, and signs it again before it - * expires. Clients must trust the authority certificate. The service loads + * expires. Runs name their own hosts, so the authority keeps the + * certificates of at most `MAX_HOSTS` hosts, and keeps no files for them. + * Clients must trust the authority certificate. The service loads * a fixed one, so that clients can trust it across restarts. */ import { execFile } from "node:child_process"; @@ -44,6 +46,8 @@ export interface CertificateAuthorityPem { const HOST_DAYS = 7; /** Sign a host again when its certificate is older than this. */ const RESIGN_MS = (HOST_DAYS - 1) * 24 * 60 * 60 * 1000; +/** The hosts with a certificate in memory. More hosts drop the oldest one. */ +const MAX_HOSTS = 1000; /** * Create a certificate authority in a new temporary directory. Without @@ -109,10 +113,11 @@ export async function createCertificateAuthority( "-out", cert, ]); - return tls.createSecureContext({ - cert: await readFile(cert, "utf8"), - key: hostKeyPem, - }); + const certPem = await readFile(cert, "utf8"); + await Promise.all( + [csr, extensions, cert].map((file) => rm(file, { force: true })), + ); + return tls.createSecureContext({ cert: certPem, key: hostKeyPem }); }; return { @@ -126,7 +131,12 @@ export async function createCertificateAuthority( fresh.context.catch(() => { if (contexts.get(host) === fresh) contexts.delete(host); }); + contexts.delete(host); contexts.set(host, fresh); + // A map keeps the order of insertion, so the first key is the oldest. + if (contexts.size > MAX_HOSTS) { + contexts.delete(contexts.keys().next().value!); + } entry = fresh; } return entry.context; diff --git a/src/request-key.ts b/src/request-key.ts index 6106296..eefa7fb 100644 --- a/src/request-key.ts +++ b/src/request-key.ts @@ -5,6 +5,10 @@ * and the body. A JSON body has sorted object keys, so key order does not * matter. Each changing value of the rule is `<>` in the key * (`values.ts`). + * + * The patterns of `values` come from the client, and all runs share one + * process. A slow pattern would block every run, so a key has a time limit + * (`KEY_TIME_LIMIT`). */ import { createHash } from "node:crypto"; import type { IncomingHttpHeaders } from "node:http"; @@ -15,11 +19,27 @@ import { type KnownValues, type RequestValues, type ValuePatterns, + withTimeLimit, } from "./values.ts"; /** Change this to make every recording a miss. */ const KEY_VERSION = "http-v1"; +/** + * The time that the key of one request can take: 1 s, and 0.5 s for each + * MiB of the body. The built-in patterns take less than 0.1 s for each + * MiB. A key that takes longer fails its request, and other runs go on. + */ +const KEY_TIME_LIMIT = { baseMs: 1000, msPerMiB: 500 }; + +/** The parts of a request that make its key. */ +interface RequestToKey { + method: string; + url: string; + headers: IncomingHttpHeaders; + body: string; +} + /** A request as the key sees it. */ export interface KeyedRequest { /** The name of the recording file, without `.json`. */ @@ -100,16 +120,24 @@ const sha256 = (text: string) => createHash("sha256").update(text).digest("hex"); /** * The key and the changing values of a request. `known` holds - * the values of earlier requests in the same session (`values.ts`). + * the values of earlier requests in the same session (`values.ts`). It + * throws an error with HTTP status 400 when it takes too long. */ export function keyRequest( rule: RecordingRule, - request: { - method: string; - url: string; - headers: IncomingHttpHeaders; - body: string; - }, + request: RequestToKey, + known?: KnownValues, +): KeyedRequest { + const mib = Buffer.byteLength(request.body) / (1024 * 1024); + return withTimeLimit( + Math.ceil(KEY_TIME_LIMIT.baseMs + KEY_TIME_LIMIT.msPerMiB * mib), + () => keyRequestNow(rule, request, known), + ); +} + +function keyRequestNow( + rule: RecordingRule, + request: RequestToKey, known?: KnownValues, ): KeyedRequest { const headers = Object.fromEntries( diff --git a/src/server.ts b/src/server.ts index c46b12b..7a8747c 100644 --- a/src/server.ts +++ b/src/server.ts @@ -6,9 +6,11 @@ * rule matches to the recorder (`recorder.ts`). Other requests go live * without a change, and their responses stream. * - * It sends requests only to the `allow` origins. It refuses other origins - * with HTTP 403 and sends nothing. The upstream host always comes from - * `allow`, never from the client. Only the path comes from the client. + * It sends requests only to the `allow` origins of the run. It refuses + * other origins with HTTP 403 and sends nothing. The upstream host always + * comes from `allow`, never from the request. Only the path comes from the + * request. It also refuses an origin whose address is not public + * (`addresses.ts`), unless the service config has `privateNetworks`. * * Each response has an `x-roach` header: `replayed`, `live`, * `missed` (`replay` mode had no recording), or `passthrough` (no rule @@ -19,6 +21,7 @@ import http from "node:http"; import https from "node:https"; import { isIP, type Socket } from "node:net"; import tls from "node:tls"; +import { isPublicAddress, notPublicError, publicLookup } from "./addresses.ts"; import type { CertificateAuthority } from "./certificates.ts"; import type { ProxyRequest, Recorder } from "./recorder.ts"; @@ -122,11 +125,13 @@ export function sameToken( /** * Send a request to an allowed origin. The host, port, and scheme come from - * the configuration. Only the path comes from the client. + * the configuration. Only the path comes from the client. Without + * `privateNetworks`, only public addresses get the request. */ function sendUpstream( origin: URL, request: ProxyRequest, + privateNetworks: boolean, extraHeaders: http.OutgoingHttpHeaders = {}, ): Promise { const headers: http.OutgoingHttpHeaders = {}; @@ -143,12 +148,19 @@ function sendUpstream( const client = origin.protocol === "https:" ? https : http; // A URL keeps the brackets of an IPv6 host. A request option has none. const hostname = origin.hostname.replace(/^\[|\]$/g, ""); + // Node does not call `lookup` for an IP address, so check it here. + if (!privateNetworks && isIP(hostname) && !isPublicAddress(hostname)) { + return Promise.reject(notPublicError(hostname, hostname)); + } return new Promise((resolve, reject) => { const upstream = client.request( { protocol: origin.protocol, hostname, port: origin.port || undefined, + // The check is on the address of the connection. A kept-alive + // connection had the check when it opened. + lookup: privateNetworks ? undefined : publicLookup, servername: isIP(hostname) ? undefined : hostname, path: `${request.url.pathname}${request.url.search}`, method: request.method, @@ -163,7 +175,7 @@ function sendUpstream( /** Proxy one request to an allowed origin, through the recorder. */ async function proxyRequest( - { recorder, origins, replayOnly }: ProxyTarget, + { recorder, origins, replayOnly, privateNetworks = false }: ProxyTarget, incoming: http.IncomingMessage, outgoing: http.ServerResponse, tunnelOrigin: string | undefined, @@ -196,7 +208,7 @@ async function proxyRequest( } if (!rule) { recorder.countPassthrough(origin.origin, request.headers); - const upstream = await sendUpstream(origin, request); + const upstream = await sendUpstream(origin, request, privateNetworks); outgoing.writeHead(upstream.statusCode ?? 502, upstream.statusMessage, { ...upstream.headers, "x-roach": "passthrough", @@ -208,7 +220,7 @@ async function proxyRequest( const response = await recorder.respond(rule, request, async () => { // Ask for a plain body, so the recording is readable. - const upstream = await sendUpstream(origin, request, { + const upstream = await sendUpstream(origin, request, privateNetworks, { "accept-encoding": "identity", }); return { @@ -287,6 +299,11 @@ export interface ProxyTarget { * service sets this for a run without the write token. */ replayOnly?: boolean; + /** + * Send requests to addresses that are not public too, such as + * `127.0.0.1`. Only for tests and local development. + */ + privateNetworks?: boolean; /** Set when the target stops. Its tunnels then refuse new requests. */ closed?: boolean; } diff --git a/src/service.ts b/src/service.ts index 415289d..46547c2 100644 --- a/src/service.ts +++ b/src/service.ts @@ -16,6 +16,13 @@ * - The service gives each run its own id and token. A proxied request * names its run in `Proxy-Authorization`, as `Basic base64(:)`. * The proxy URL of the run has them, so `HTTPS_PROXY` sends them. + * - A run sends its own config: its origins (`allow`), rules, and value + * patterns. So a tenant changes what it records without a change to the + * service. Two checks keep that safe for the other tenants. The proxy + * sends requests only to public addresses (`addresses.ts`), so a run + * cannot reach the metadata server or the VPC. The key of a request has + * a time limit (`request-key.ts`), so a slow pattern fails only its own + * request. * - The recordings of a tenant are under `/`, in a directory or a * GCS bucket (`gcs.ts`). Tenants never share recordings, because a key * does not include credentials. @@ -46,7 +53,7 @@ import { } from "./server.ts"; import { RULE_NAME, type RecordingStore } from "./store.ts"; import type { RecordingMode, RecordingRule, RecordingStats } from "./types.ts"; -import { VALUE_PATTERNS } from "./values.ts"; +import { invalidValuePatterns } from "./values.ts"; /** The configuration of the service. The operator owns it. */ export interface RoachServiceConfig { @@ -63,21 +70,19 @@ export interface RoachServiceConfig { directory?: string; /** A GCS bucket for the recordings. Set this or `directory`. */ bucket?: string; - /** The only origins that runs can reach. A run can allow fewer. */ - allow: string[]; /** * The SHA-256 of the write token, in hex. All tenants share the token. * Give it only to CI jobs that you trust. */ writeTokenHash: string; - /** - * More regular expression sources that rules can use in `values`, in - * addition to `VALUE_PATTERNS`. A slow pattern blocks all runs, so check - * each one for catastrophic backtracking before you add it. - */ - valuePatterns?: string[]; /** Credentials to redact in all recordings. */ secrets?: string[]; + /** + * Let runs reach addresses that are not public, such as `127.0.0.1` and + * the metadata server. Only for tests and local development. Default: + * `false`. + */ + privateNetworks?: boolean; /** * The certificate authority that signs intercepted hosts. Clients trust * it, so a deployed service must keep it across restarts. Default: a new @@ -97,8 +102,12 @@ export interface RunConfig { tenant: string; mode: RecordingMode; rules: RecordingRule[]; - /** Default: all origins of the service. */ - allow?: string[]; + /** + * The only origins that the run can reach, such as + * `https://ai-gateway.vercel.sh`. The service sends requests only to + * their public addresses. + */ + allow: string[]; /** * The name of the run in Sentry, such as * `${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}`. Default: the run id. @@ -150,6 +159,15 @@ const RUN_TIMEOUT_MS = 6 * 60 * 60 * 1000; */ const MAX_PUBLIC_RUNS = 100; const MAX_RUN_NAME = 128; +/** + * Limits of the config of a run. Anyone can start a run, so they keep the + * work of a run small: the service makes a certificate for each host that + * a run tunnels to, and runs each value pattern on each request. + */ +const MAX_RUN_ORIGINS = 32; +const MAX_RULES = 64; +const MAX_RULE_VALUES = 32; +const MAX_PATTERN_LENGTH = 2048; /** Enough for the API keys and OIDC tokens of a CI job. */ const MAX_RUN_SECRETS = 32; const MAX_SECRET_LENGTH = 16 * 1024; @@ -184,11 +202,7 @@ function proxyCredentials( } /** Check the run config of a client. Returns the error, or `undefined`. */ -function invalidRunConfig( - config: Partial, - serviceOrigins: Map, - valuePatterns: Set, -): string | undefined { +function invalidRunConfig(config: Partial): string | undefined { if (!config.mode || !MODES.has(config.mode)) { return "mode must be auto, off, record, or replay"; } @@ -209,7 +223,9 @@ function invalidRunConfig( return `secrets must be a list of at most ${MAX_RUN_SECRETS} strings of at most ${MAX_SECRET_LENGTH} characters`; } } - if (!Array.isArray(config.rules)) return "rules must be a list"; + if (!Array.isArray(config.rules) || config.rules.length > MAX_RULES) { + return `rules must be a list of at most ${MAX_RULES} rules`; + } const names = new Set(); for (const rule of config.rules) { if (typeof rule?.name !== "string" || !RULE_NAME.test(rule.name)) { @@ -220,29 +236,41 @@ function invalidRunConfig( if (typeof rule.match !== "object" || rule.match === null) { return `rule ${rule.name} must have match`; } - // A pattern runs on the requests of all runs in one process, so a - // client can only use patterns that the operator knows to be fast. + // A slow pattern fails only its own requests (`request-key.ts`), so + // the service takes any pattern that compiles. if (rule.values !== undefined) { if (typeof rule.values !== "object" || rule.values === null) { return `values of rule ${rule.name} must be an object`; } - for (const [name, source] of Object.entries(rule.values)) { - if (typeof source !== "string" || !valuePatterns.has(source)) { - return `value ${name} of rule ${rule.name} is not a pattern that the service allows`; + const values = Object.entries(rule.values); + if (values.length > MAX_RULE_VALUES) { + return `rule ${rule.name} can have at most ${MAX_RULE_VALUES} values`; + } + for (const [name, source] of values) { + if (typeof source !== "string" || source.length > MAX_PATTERN_LENGTH) { + return `value ${name} of rule ${rule.name} must be a pattern of at most ${MAX_PATTERN_LENGTH} characters`; } } } } - if (config.allow !== undefined) { - if (!Array.isArray(config.allow)) return "allow must be a list"; - for (const origin of config.allow) { - // The client sends JSON, so check the type at run time. - if ( - typeof origin !== "string" || - !serviceOrigins.has(origin.replace(/\/$/, "")) - ) { - return `the service does not allow ${origin}`; - } + const patternError = invalidValuePatterns( + config.rules.flatMap((rule) => (rule.values ? [rule.values] : [])), + ); + if (patternError) return patternError; + if ( + !Array.isArray(config.allow) || + config.allow.length === 0 || + config.allow.length > MAX_RUN_ORIGINS + ) { + return `allow must be a list of 1 to ${MAX_RUN_ORIGINS} origins`; + } + for (const origin of config.allow) { + // The client sends JSON, so check the type at run time. + if (typeof origin !== "string") return "allow must be a list of origins"; + try { + parseOrigins([origin.replace(/\/$/, "")]); + } catch { + return `${origin} is not an http or https origin`; } } return undefined; @@ -297,13 +325,8 @@ export async function startRoachService( if (config.sentryDsn) { Sentry.init({ dsn: config.sentryDsn, tracesSampleRate: 0 }); } - const serviceOrigins = parseOrigins(config.allow); const runs = new Map(); const authority = await createCertificateAuthority(config.ca); - const valuePatterns = new Set([ - ...Object.values(VALUE_PATTERNS), - ...(config.valuePatterns ?? []), - ]); const storeOf = (tenant: string): RecordingStore => config.bucket ? createGcsStore(config.bucket, `${tenant}/`) @@ -320,7 +343,6 @@ export async function startRoachService( const createRun = (runConfig: RunConfig, canWrite: boolean): RemoteRun => { const id = randomBytes(8).toString("hex"); const token = randomBytes(24).toString("hex"); - const allow = runConfig.allow ?? config.allow; const store = runStore( storeOf(runConfig.tenant), { tenant: runConfig.tenant, run: runConfig.name ?? id }, @@ -330,9 +352,12 @@ export async function startRoachService( id, token, started: Date.now(), - origins: parseOrigins(allow.map((origin) => origin.replace(/\/$/, ""))), + origins: parseOrigins( + runConfig.allow.map((origin) => origin.replace(/\/$/, "")), + ), // A public run must not be an open proxy to the allowed origins. replayOnly: !canWrite, + privateNetworks: config.privateNetworks === true, // Each run learns only the credentials of its own requests. recorder: createRecorder( { @@ -387,7 +412,7 @@ export async function startRoachService( sendJson(outgoing, 400, { error: "tenant must be owner/repo" }); return; } - const error = invalidRunConfig(runConfig, serviceOrigins, valuePatterns); + const error = invalidRunConfig(runConfig); if (error) { sendJson(outgoing, 400, { error }); return; diff --git a/src/values.ts b/src/values.ts index 2613430..c2a64ee 100644 --- a/src/values.ts +++ b/src/values.ts @@ -16,6 +16,7 @@ * A value that the response makes itself, such as a date that the model * calculates, stays as it was recorded. */ +import vm from "node:vm"; /** * Lookbehind: the value does not continue a word or number. The proxy @@ -102,6 +103,44 @@ function compile(patterns: ValuePatterns): CompiledValues { return result; } +/** + * Run `fn` with a time limit. A slow pattern cannot stop itself, but the + * `timeout` of `vm` stops any code that runs too long, also code outside + * the context that the script calls. A timeout throws an error with HTTP + * status 400. + */ +export function withTimeLimit(ms: number, fn: () => T): T { + try { + return vm.runInContext("fn()", vm.createContext({ fn }), { + timeout: ms, + }) as T; + } catch (error) { + if ((error as { code?: string }).code === "ERR_SCRIPT_EXECUTION_TIMEOUT") { + throw Object.assign( + new Error(`the value patterns took over ${ms} ms. Make them simpler.`), + { status: 400 }, + ); + } + throw error; + } +} + +/** + * Check that the patterns of each rule compile. Returns the error, or + * `undefined`. A pattern can be slow to compile, so all of them together + * have 1 s. + */ +export function invalidValuePatterns( + list: ValuePatterns[], +): string | undefined { + try { + withTimeLimit(1000, () => list.forEach((patterns) => compile(patterns))); + return undefined; + } catch (error) { + return (error as Error).message; + } +} + /** Values that an earlier request of the same session had, by value. */ export type KnownValues = ReadonlyMap; diff --git a/tests/deployed.test.ts b/tests/deployed.test.ts index 1ba5b1a..9215ef3 100644 --- a/tests/deployed.test.ts +++ b/tests/deployed.test.ts @@ -93,7 +93,10 @@ async function ciJob(job: { summary: string; }> { const workspace = await mkdtemp(path.join(directory, "job-")); - const config = { rules: [{ name: "model", match: { method: "POST" } }] }; + const config = { + allow: [upstreamOrigin], + rules: [{ name: "model", match: { method: "POST" } }], + }; await writeFile(path.join(workspace, "roach.json"), JSON.stringify(config)); await writeFile( path.join(workspace, "job.mjs"), @@ -273,7 +276,9 @@ beforeAll(async () => { port: servicePort, publicUrl: frontUrl, bucket: BUCKET, - allow: [upstreamOrigin], + // The upstream is on 127.0.0.2 here. A deployed service has no such + // option, so runs reach only public addresses. + privateNetworks: true, writeTokenHash: createHash("sha256").update(TOKEN).digest("hex"), ca: { cert: caCert, key: await readFile(caKeyFile, "utf8") }, sentryDsn: `http://public@127.0.0.1:${sentryPort}/1`, diff --git a/tests/recording.test.ts b/tests/recording.test.ts index c085243..8e6c1c6 100644 --- a/tests/recording.test.ts +++ b/tests/recording.test.ts @@ -43,6 +43,7 @@ async function start(mode: RecordingMode): Promise { { tenant: TENANT, mode, + allow: [origin], rules: [ { name: "model", @@ -119,7 +120,7 @@ beforeEach(async () => { directory = await mkdtemp(path.join(tmpdir(), "roach-")); service = await startRoachService({ directory, - allow: [origin], + privateNetworks: true, writeTokenHash: createHash("sha256").update(TOKEN).digest("hex"), }); }); diff --git a/tests/service.test.ts b/tests/service.test.ts index 8bbb8e5..bce2c27 100644 --- a/tests/service.test.ts +++ b/tests/service.test.ts @@ -39,6 +39,7 @@ async function startRun( { tenant: "acme/alpha", mode, + allow: [origin], rules: [ { name: "model", match: { method: "POST", url: `${origin}/v1/` } }, ], @@ -103,7 +104,8 @@ beforeEach(async () => { directory = await mkdtemp(path.join(tmpdir(), "roach-service-")); service = await startRoachService({ directory, - allow: [origin], + // The upstream of the tests is on 127.0.0.1. + privateNetworks: true, writeTokenHash: hash("write-token"), secrets: ["service-secret-value-123"], }); @@ -221,19 +223,38 @@ describe("roach service", () => { ); } await expect( - startRun("write-token", "auto", { allow: ["https://example.com"] }), - ).rejects.toThrow(/does not allow/); + startRun("write-token", "auto", { allow: ["file:///etc/passwd"] }), + ).rejects.toThrow(/not an http or https origin/); + await expect( + startRun("write-token", "auto", { allow: [] }), + ).rejects.toThrow(/allow must be a list/); await expect( startRun("write-token", "auto", { rules: [{ name: "../escape", match: {} }], }), ).rejects.toThrow(/rule name/); - // A pattern runs for all runs, so only known patterns are allowed. await expect( startRun("write-token", "auto", { - rules: [{ name: "model", match: {}, values: { slow: "(a+)+$" } }], + rules: [{ name: "model", match: {}, values: { broken: "(" } }], }), - ).rejects.toThrow(/not a pattern that the service allows/); + ).rejects.toThrow(/Invalid regular expression/); + // Patterns can also be slow to compile. They have a time limit too. + // Without it, these take over a minute. Each pattern is different, + // so V8 cannot reuse an earlier one. + await expect( + startRun("write-token", "auto", { + rules: Array.from({ length: 64 }, (_, rule) => ({ + name: `model${rule}`, + match: {}, + values: Object.fromEntries( + Array.from({ length: 32 }, (__, value) => [ + `v${value}`, + `(?:(?:(?:(?:a?){30}){30}){${30 + rule}}){${30 + value}}`, + ]), + ), + })), + }), + ).rejects.toThrow(/value patterns took over/); await expect( startRun("write-token", "auto", { secrets: [42 as unknown as string] }), ).rejects.toThrow(/secrets must be a list/); @@ -285,6 +306,66 @@ describe("roach service", () => { expect(liveRequests).toBe(1); }); + it("takes origins and patterns from the run, and keeps the service safe", async () => { + // A run can use any pattern, also one that backtracks a lot. Its key + // has a time limit, so only its own request fails. + const slow = await startRun("write-token", "auto", { + rules: [ + { + name: "model", + match: { method: "POST", url: `${origin}/v1/` }, + values: { slow: "(a+)+$" }, + }, + ], + }); + const started = Date.now(); + const stuck = await send(slow.url, { prompt: `${"a".repeat(40)}!` }); + expect(stuck.status).toBe(400); + expect(stuck.body).toMatch(/value patterns took over/); + expect(Date.now() - started).toBeLessThan(5000); + // The process goes on for other runs. + const other = await startRun("write-token", "auto"); + expect(await send(other.url, { prompt: "hi" })).toMatchObject({ + status: 200, + source: "live", + }); + expect(liveRequests).toBe(1); + + // A deployed service sends requests only to public addresses. A run + // names its own origins, so it must not reach the metadata server or + // the network of the service, by IP address or by host name. + const port = new URL(origin).port; + const guarded = await startRoachService({ + directory, + writeTokenHash: hash("write-token"), + }); + try { + for (const target of [origin, `http://localhost:${port}`]) { + const run = await startRemoteRun( + { url: guarded.url, token: "write-token" }, + { + tenant: "acme/alpha", + mode: "auto", + allow: [target], + rules: [{ name: "model", match: { url: `${target}/v1/` } }], + }, + ); + runs.push(run); + const agent = new ProxyAgent({ uri: run.url, proxyTunnel: true }); + agents.push(agent); + // One request that a rule records, and one that goes through. + for (const url of [`${target}/v1/messages`, `${target}/other`]) { + const response = await request(url, { dispatcher: agent }); + expect(response.statusCode).toBe(403); + expect(await response.body.text()).toMatch(/not a public address/); + } + } + } finally { + await guarded.close(); + } + expect(liveRequests).toBe(1); + }); + it("redacts request credentials and the secrets of the service and the run", async () => { // A run secret is not in a credential header, so only the run config // tells the service about it.