diff --git a/.env.example b/.env.example index be7bbf3..2d1643f 100644 --- a/.env.example +++ b/.env.example @@ -38,6 +38,8 @@ SANDBOX_NETWORK_POLICY=allow-list # Deadline for opening a Render investigation Sandbox: restore, boot, verify.sh, and reading its manifest. # SANDBOX_PROVISION_TIMEOUT_MS=300000 # REPOS=render-oss/cli,render-oss/sdk +# Origins callbacks may be POSTed to; unset turns callbacks off. http://localhost: works locally. +# CALLBACK_ALLOWED_ORIGINS=https://hooks.example.com # ── Snapshot refresh cron ──────────────────────────────────────────────── GITHUB_TOKEN= diff --git a/README.md b/README.md index 296ff41..738266d 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,8 @@ A reader can follow each link to the exact lines and decide for themselves. - The model API key arrives as a one-time file that the runner deletes on read. The runner's Node binary is execute-only and signal-triggered debugging is disabled, protecting its memory from the agent's commands. The snapshot build and every restored Sandbox must verify both protections, or the build fails and the Sandbox gets no work. - HTTPS egress defaults to the model provider's API host and can be replaced with an exact host and leftmost-wildcard allow-list. DNS still resolves arbitrary names. - Git tokens exist only in the builder Sandbox while it clones. They are scrubbed and verified absent before the snapshot is taken. +- Callbacks are off until you list the allowed origins in the workflow's `CALLBACK_ALLOWED_ORIGINS`. Delivery never follows redirects. +- Callbacks are off until you list the allowed origins in the workflow's `CALLBACK_ALLOWED_ORIGINS`. Delivery never follows redirects. - Anyone who holds `API_TOKEN` can read the configured repositories through the researcher, so share it like a read token for those repositories. - The API signs question IDs with a separate, server-only `QUESTION_ID_SECRET`. The Blueprint generates it; keep it stable across API replicas and restarts, and never share it with callers or the workflow. See [question-ID rotation and migration](docs/1_EXTENSION.md#question-id-signing-and-rotation). diff --git a/docs/0_QUICKSTART.md b/docs/0_QUICKSTART.md index ef6d83d..709b39e 100644 --- a/docs/0_QUICKSTART.md +++ b/docs/0_QUICKSTART.md @@ -270,6 +270,7 @@ All services emit one JSON object per line. The most useful events are: | `turn_finished` | Workflow | Final outcome, model, prompt version, commits, usage, cost, and duration | | `investigation_failed` | Workflow | The agent ran but did not submit an accepted result | | `workflow_misconfigured` | Workflow | Model or Sandbox configuration is invalid | +| `deliver_skipped` | Workflow | The callback's origin isn't in `CALLBACK_ALLOWED_ORIGINS` | | `deliver_failed` | Workflow | Callback retries were exhausted | | `snapshot_refresh` | Cron | Snapshot build result, build commit, repositories, and duration | diff --git a/docs/1_EXTENSION.md b/docs/1_EXTENSION.md index 62196c5..7b692c7 100644 --- a/docs/1_EXTENSION.md +++ b/docs/1_EXTENSION.md @@ -272,8 +272,17 @@ questions. Legacy signatures are not accepted. ### Receive callbacks Polling suits scripts and short-lived callers. A long-lived integration, such -as a chat bot or support desk, should pass a callback instead. When one is -supplied, the `deliver` subtask sends: +as a chat bot or support desk, should pass a callback instead. + +Callbacks are off until you set `CALLBACK_ALLOWED_ORIGINS` on the workflow +service to a comma-separated list of origins, such as +`https://hooks.example.com`. Delivery runs on the workflow host, outside the +Sandbox's egress rules, so it only POSTs to an exact listed origin (any path) +and never follows redirects. A callback to any other origin is skipped and +logged as `deliver_skipped`; the reply is still available by polling. In local +development, `http://localhost:` origins are also accepted. + +For an approved callback, the `deliver` subtask sends: - `X-Researcher-Timestamp`: Unix seconds. - `X-Researcher-Signature`: @@ -362,6 +371,7 @@ string is treated as unset. | `SANDBOX_NETWORK_POLICY` | `allow-list` | `allow-list`, `allow-all`, or `deny-all` | | `SANDBOX_ALLOWED_DOMAINS` | Model host | Comma-separated HTTPS egress hosts; supports leftmost wildcards | | `SANDBOX_TIMEOUT_SECONDS` | `1800` | Provider-side termination backstop | +| `CALLBACK_ALLOWED_ORIGINS` | None (callbacks off) | Comma-separated origins callbacks may be delivered to | | `SNAPSHOT_NAME` | `config.ts` | Snapshot-name override | | `REPOS` | None | Repository-list override | diff --git a/docs/2_ARCHITECTURE.md b/docs/2_ARCHITECTURE.md index 811634b..33eb8e6 100644 --- a/docs/2_ARCHITECTURE.md +++ b/docs/2_ARCHITECTURE.md @@ -149,7 +149,8 @@ answered, so a chain of follow-ups only ever reads the latest run. `turn_finished` outcome. - `investigate`: retryable infrastructure boundary; opens and runs a fresh Sandbox. -- `deliver`: independently retryable signed callback POST. +- `deliver`: independently retryable signed callback POST, only to an origin + listed in `CALLBACK_ALLOWED_ORIGINS`, with redirects refused. The parent does not retry. Infrastructure failures thrown by `investigate` retry on a fresh Sandbox; model and submission failures become user-safe diff --git a/src/callback.ts b/src/callback.ts index c0dd97d..7ec4ee7 100644 --- a/src/callback.ts +++ b/src/callback.ts @@ -20,6 +20,28 @@ export function signCallback( return { signature: `v1=${sig}`, timestamp: ts }; } +const LOOPBACK = new Set(["localhost", "127.0.0.1", "[::1]"]); + +/** + * CALLBACK_ALLOWED_ORIGINS as normalized origins; unset or blank disables callbacks. Delivery runs on the + * workflow host, outside Sandbox egress rules, so only operator-approved origins are reachable. + */ +export function callbackOrigins(env: NodeJS.ProcessEnv = process.env): string[] { + const raw = env.CALLBACK_ALLOWED_ORIGINS?.trim(); + if (!raw) return []; + return raw.split(",").map((entry) => { + const url = URL.parse(entry.trim()); + const localHttp = url?.protocol === "http:" && LOOPBACK.has(url.hostname) && env.RENDER !== "true"; + if (!url || (url.protocol !== "https:" && !localHttp) || url.username || url.password || url.pathname !== "/" || url.search || url.hash) { + throw new Error(`CALLBACK_ALLOWED_ORIGINS: ${JSON.stringify(entry.trim())} must be an https:// origin with no path`); + } + return url.origin; + }); +} + +export const isApprovedCallback = (url: string, origins: readonly string[]): boolean => + origins.includes(URL.parse(url)?.origin ?? ""); + const CALLBACK_MAX_SKEW_SECONDS = 5 * 60; export function verifyCallback(opts: { diff --git a/src/tasks.ts b/src/tasks.ts index c02664d..1264db8 100644 --- a/src/tasks.ts +++ b/src/tasks.ts @@ -12,7 +12,7 @@ import type { RepoManifest } from "./manifest.ts"; import { RUNNER_EXIT, RunnerInput, type RunnerEvent, type Usage } from "./runner/protocol.ts"; import { SandboxMisconfiguredError, SnapshotUnavailableError, type OpenSandbox, type RunInvestigationResult, type Sandbox } from "./sandbox/index.ts"; import { getBuiltinModel } from "@earendil-works/pi-ai/providers/all"; -import { signCallback } from "./callback.ts"; +import { callbackOrigins, isApprovedCallback, signCallback } from "./callback.ts"; import type { Log } from "./log.ts"; import type { Config } from "./workflow.ts"; @@ -210,7 +210,9 @@ export async function answerQuestion( reply = { kind: "failure", traceId, reason: failureReason(investigateResult.code, config.agentTimeoutMs) }; } - if (input.callback) { + if (input.callback && !isApprovedCallback(input.callback.url, config.callbackOrigins)) { + log.warn("callback origin not approved", { event: "deliver_skipped", trace_id: traceId, origin: URL.parse(input.callback.url)?.origin }); + } else if (input.callback) { const body = JSON.stringify({ trace_id: traceId, reply, @@ -240,8 +242,9 @@ export async function answerQuestion( return { trace_id: traceId, outcome, reply }; } -/** POST the callback body with a signed request. Throws on non-2xx or network error. */ -export async function deliverCallback(input: DeliverInput): Promise { +/** POST the callback body with a signed request. Throws on an unapproved origin, a redirect, non-2xx, or network error. */ +export async function deliverCallback(input: DeliverInput, origins: readonly string[] = callbackOrigins()): Promise { + if (!isApprovedCallback(input.callback.url, origins)) throw new Error("callback origin is not in CALLBACK_ALLOWED_ORIGINS"); const { signature, timestamp } = signCallback(input.callback.secret, input.body); const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 10_000); @@ -254,6 +257,7 @@ export async function deliverCallback(input: DeliverInput): Promise { "X-Researcher-Signature": signature, }, body: input.body, + redirect: "error", signal: controller.signal, }); if (!res.ok) throw new Error(`deliver got ${res.status}`); diff --git a/src/workflow.ts b/src/workflow.ts index 78a89da..f41324f 100644 --- a/src/workflow.ts +++ b/src/workflow.ts @@ -7,6 +7,7 @@ import { task, type TaskContext } from "@renderinc/sdk/workflows"; import { z } from "zod"; import { answerQuestion, deliverCallback, investigateTask, type InvestigateInput, type InvestigateResult, type DeliverInput } from "./tasks.ts"; import { createLog } from "./log.ts"; +import { callbackOrigins } from "./callback.ts"; const MINUTE = 60_000; @@ -78,6 +79,7 @@ const Env = z modelApiKey: providerApiKey(e.MODEL_PROVIDER, env), agentTimeoutMs: e.AGENT_TIMEOUT_MS, sandboxProvisionTimeoutMs: e.SANDBOX_PROVISION_TIMEOUT_MS, + callbackOrigins: callbackOrigins(env), }; }); export type Config = z.output; diff --git a/test/tasks.test.ts b/test/tasks.test.ts index 606e05b..480cc89 100644 --- a/test/tasks.test.ts +++ b/test/tasks.test.ts @@ -1,6 +1,6 @@ import { createServer } from "node:http"; import { describe, expect, it } from "vitest"; -import { verifyCallback } from "../src/callback.ts"; +import { callbackOrigins, isApprovedCallback, verifyCallback } from "../src/callback.ts"; import type { Evidence, InvestigationResult } from "../src/investigation.ts"; import { createLog } from "../src/log.ts"; import type { AnswerQuestionInput } from "../src/question.ts"; @@ -60,7 +60,7 @@ const CALLBACK = { url: "https://example.com/hook", secret: "a".repeat(32) }; function harness(env: Record = {}) { const sandboxes = new FakeSandboxes(); const logs: Record[] = []; - const config = loadConfig(env); + const config = loadConfig({ CALLBACK_ALLOWED_ORIGINS: "https://example.com", ...env }); const log = createLog({}, (line: string) => void logs.push(JSON.parse(line))); const deliveries: DeliverInput[] = []; const h = { @@ -104,6 +104,17 @@ describe("answerQuestion", () => { expect(h.sandboxes.closed).toEqual(["sbx-1"]); }); + it("skips delivery to an unapproved callback origin but still returns the reply for polling", async () => { + const h = harness({ CALLBACK_ALLOWED_ORIGINS: "https://receiver.example" }); + h.sandboxes.script(success()); + + const result = await h.ask({ ...QUESTION, callback: CALLBACK }); + + expect(result.outcome).toBe("answer"); + expect(h.deliveries).toEqual([]); + expect(h.logs).toContainEqual(expect.objectContaining({ event: "deliver_skipped", origin: "https://example.com" })); + }); + it("returns a clarification when the agent needs one", async () => { const h = harness(); h.sandboxes.script(success(clarification)); @@ -294,8 +305,8 @@ describe("deliverCallback", () => { const base = `http://localhost:${(server.address() as { port: number }).port}`; const body = JSON.stringify({ trace_id: "t1", reply: { kind: "failure", traceId: "t1", reason: "test" } }); try { - await deliverCallback({ callback: { url: `${base}/hook`, secret }, body }); - await expect(deliverCallback({ callback: { url: `${base}/fail`, secret }, body })).rejects.toThrow("500"); + await deliverCallback({ callback: { url: `${base}/hook`, secret }, body }, [base]); + await expect(deliverCallback({ callback: { url: `${base}/fail`, secret }, body }, [base])).rejects.toThrow("500"); } finally { server.close(); } @@ -310,6 +321,66 @@ describe("deliverCallback", () => { timestamp: headers["x-researcher-timestamp"] as string, })).toBe(true); }); + + it("refuses an unapproved origin without sending anything", async () => { + let received = 0; + const server = createServer((_req, res) => { received++; res.end(); }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const url = `http://127.0.0.1:${(server.address() as { port: number }).port}/hook`; + try { + await expect(deliverCallback({ callback: { url, secret: "a".repeat(32) }, body: "{}" }, ["https://receiver.example"])) + .rejects.toThrow("CALLBACK_ALLOWED_ORIGINS"); + expect(received).toBe(0); + } finally { + server.close(); + } + }); + + it("rejects a redirect without forwarding the signed body", async () => { + let forwarded = 0; + const target = createServer((_req, res) => { forwarded++; res.end(); }); + await new Promise((resolve) => target.listen(0, "127.0.0.1", resolve)); + const redirector = createServer((_req, res) => { + res.writeHead(307, { location: `http://127.0.0.1:${(target.address() as { port: number }).port}/capture` }); + res.end(); + }); + await new Promise((resolve) => redirector.listen(0, "127.0.0.1", resolve)); + const origin = `http://127.0.0.1:${(redirector.address() as { port: number }).port}`; + try { + await expect(deliverCallback({ callback: { url: `${origin}/hook`, secret: "a".repeat(32) }, body: "{}" }, [origin])).rejects.toThrow(); + expect(forwarded).toBe(0); + } finally { + target.close(); + redirector.close(); + } + }); +}); + +describe("callbackOrigins", () => { + it("is empty when unset or blank, which disables callbacks", () => { + expect(callbackOrigins({})).toEqual([]); + expect(callbackOrigins({ CALLBACK_ALLOWED_ORIGINS: " " })).toEqual([]); + }); + + it("normalizes origins and matches every path on an exact origin only", () => { + const origins = callbackOrigins({ CALLBACK_ALLOWED_ORIGINS: "https://Hooks.example:443/, https://events.example:8443" }); + expect(origins).toEqual(["https://hooks.example", "https://events.example:8443"]); + expect(isApprovedCallback("https://hooks.example/a/b?c=1", origins)).toBe(true); + for (const url of ["https://sub.hooks.example/", "https://hooks.example:8443/", "http://hooks.example/", "https://10.0.0.1/", "not a url"]) { + expect(isApprovedCallback(url, origins), url).toBe(false); + } + }); + + it.each(["https://hooks.example/path", "https://hooks.example?q=1", "https://user:pw@hooks.example", "http://hooks.example", "ftp://hooks.example", "nope"])( + "rejects %s", (origin) => { + expect(() => callbackOrigins({ CALLBACK_ALLOWED_ORIGINS: origin })).toThrow("CALLBACK_ALLOWED_ORIGINS"); + }, + ); + + it("allows loopback HTTP only off Render", () => { + expect(callbackOrigins({ CALLBACK_ALLOWED_ORIGINS: "http://localhost:4000" })).toEqual(["http://localhost:4000"]); + expect(() => callbackOrigins({ CALLBACK_ALLOWED_ORIGINS: "http://localhost:4000", RENDER: "true" })).toThrow(); + }); }); describe("task timeouts", () => {