From 88ff1e4814285601dfa30a92566cb932ef8c474d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:01:51 -0400 Subject: [PATCH 1/2] fix: refuse the private-hosts switch in production (#185) AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS was read as a plain equality with no environment gate, and .env.example shipped it live, so a deployment inherited it by copying the file. That is the ordinary path rather than an unusual one. It lifts more than browsing: app.ts hands the same value to createAgentRoutes, and target.ts:257 is an early return { allowed: true } rather than a relaxation of one rule, so with it on a signed-in person can register a Bot at a link-local address. Now mirrors the KEY_ENCRYPTION_KEY placeholder at config.ts:228: throws under NODE_ENV=production naming the variable and what to do, warns everywhere else and still allows it, so the local workflow is unchanged. The example line is commented out, since copying the file is how a deployment ends up with it. Dockerfile set the switch and NODE_ENV=production, so the all-in-one image would have refused to start. The comment justified it as the server reaching its own browser, but that path is checkComputerAddress, which never consults this switch and decides on the protocol and the metadata floor alone. The switch only governed where a Bot may browse, which on a default bridge network is the host's LAN. Removed, and the comment now says what it actually controls. The cloud metadata floor is unchanged and still refuses 169.254.169.254 and its spellings with the switch on. The flag-absent default is still false, now with a production test guarding it. --- .env.example | 7 +- CHANGELOG.md | 23 +++++++ Dockerfile | 11 +++- README.md | 2 +- server/src/config.ts | 33 +++++++++- server/tests/config.test.ts | 123 +++++++++++++++++++++++++++++++++++- 6 files changed, 190 insertions(+), 9 deletions(-) diff --git a/.env.example b/.env.example index 51e1e246..65362425 100644 --- a/.env.example +++ b/.env.example @@ -158,8 +158,11 @@ AGENT_COMPUTER_URL=http://localhost:4100 # without this value and refuses every request that does not present it. Use a long random value; # `scripts/start.sh` sets a development one for you. COMPUTER_TOKEN= -# Local only. Lets a Bot browse this machine's own services; never set this in a deployment. -AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true +# Local only, and commented out because this file gets copied. Uncommenting it lets a Bot browse this +# machine's own services, which is what a laptop wants and what a deployment must never have: it +# lifts the private-address floor in browsing and in the endpoint a Bot may be registered against. +# The server refuses to start with it set under NODE_ENV=production. +# AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # # What a Bot may do on its computer, as one JSON object. Absent uses the built-in default, which # permits the acting tools and forbids nothing, and records every action either way. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0f9206..22c5d6ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,29 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Upgrading + +**A deployment that sets `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` with `NODE_ENV=production` no +longer starts.** Remove the line and it starts again. Nothing else needs changing, and a deployment +that never set it is unaffected. + +The switch lets a Bot reach addresses inside the deployment's own network — `10.0.0.5`, +`192.168.1.1`, `127.0.0.1:5432`, a link-local address — and it does that in two places, not one: +browsing, and the endpoint a Bot may be registered against. It exists for a laptop, where the +services a Bot is asked to look at are the ones running beside it. + +The reason this is a refusal rather than a warning is how a deployment came to have it. `.env.example` +shipped the line on, and copying that file is the ordinary way an environment gets filled in, so the +path to a hosted deployment reaching its own network was not forgetting to set something, it was +inheriting something. It now ships commented out, which means a laptop that wants the old behaviour +uncomments it and everything else arrives without it. Under any other `NODE_ENV` the switch works +exactly as before, with a warning at boot saying it does not travel. + +The cloud metadata addresses — `169.254.169.254`, `metadata.google.internal`, and the IPv6 and +NAT64 spellings of them — were refused whatever this switch said, before and after. That floor has +not moved. What changed is that it is no longer the only thing left standing in a production +deployment that copied the example. + ## 0.0.4 ### A click citing a ref this deployment cannot resolve is refused diff --git a/Dockerfile b/Dockerfile index 105ff461..674ce392 100644 --- a/Dockerfile +++ b/Dockerfile @@ -173,10 +173,15 @@ RUN apt-get update && apt-get install -y --no-install-recommends sudo \ ENV WORKSPACE_DIR=/workspace ENV PROFILES_DIR=/profiles -# The browser is on loopback inside this container and reachable from nowhere else, which is why the -# private-host allowance is on: the server is browsing to its own sibling process, not the internet. +# The browser is on loopback inside this container and reachable from nowhere else. +# +# No private-host allowance is set for it, and none is needed. Reaching the sibling process goes +# through `checkComputerAddress`, which decides on the protocol and the metadata floor and never +# consults `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS`. That switch governs where a *Bot* may browse and +# which endpoint one may be registered against, so setting it here bought nothing for this address +# and let a Bot reach whatever this container's network reaches, which on a default bridge is the +# host's LAN. ENV AGENT_COMPUTER_URL=http://127.0.0.1:4100 -ENV AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true # NOTHING THAT MATTERS RUNS AS ROOT. # diff --git a/README.md b/README.md index f028234b..c09c2c74 100644 --- a/README.md +++ b/README.md @@ -291,7 +291,7 @@ provider's discovery document listed in `TRUSTED_ORIGINS`, not only the issuer. - `agent-computer` drives a browser holding real logins. `docker-compose.yml` binds it to loopback; leave it there. - Store credentials through `/admin/credentials`, which encrypts them. Do not put credential values in tenant YAML or in committed files. -- `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` lets a Bot reach services on this machine. Unset it if you would rather it could not. +- `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` lets a Bot reach services on this machine. It ships commented out in `.env.example`, is for a laptop only, and a deployment running with `NODE_ENV=production` refuses to start while it is set. ## Development diff --git a/server/src/config.ts b/server/src/config.ts index e9e7339d..553e668f 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -480,6 +480,36 @@ function runtimeCapabilities(environment: Environment): RuntimeCapabilities { }; } +/** + * Whether a Bot may reach addresses inside this deployment's own network. + * + * Off unless asked for, and the asking is only allowed on a laptop. The switch exists so that a + * local deployment can browse the services running beside it; what it turns off is not one rule but + * the whole private-address floor, in navigation and in the endpoint a Bot may be registered + * against, so with it on a signed-in person can point a Bot at a link-local address. + * + * Refused in production for the reason the example encryption key is: the way a deployment ends up + * with it is not forgetting to set something, it is copying `.env.example`, which shipped it on. The + * cloud metadata addresses are refused underneath this either way — see `computer/target.ts` — but + * that floor is the last one, not the only one worth keeping. + */ +function privateHostsAllowed(environment: Environment): boolean { + if (optional(environment, "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS") !== "true") { + return false; + } + + if (environment.NODE_ENV === "production") { + throw new Error( + "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true is for local development only: it lets a Bot reach this deployment's own network. Remove it from this deployment's environment.", + ); + } + console.warn( + "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true lets a Bot reach this machine's own services. Fine locally, and for local development only. Remove it before deploying.", + ); + + return true; +} + function computerConfig(environment: Environment): ComputerConfig | undefined { const supervisorAddress = optional(environment, "COMPUTER_SUPERVISOR_URL"); const sharedAddress = optional(environment, "AGENT_COMPUTER_URL"); @@ -494,8 +524,7 @@ function computerConfig(environment: Environment): ComputerConfig | undefined { */ const computerToken = optional(environment, "COMPUTER_TOKEN"); - const allowPrivateHosts = - optional(environment, "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS") === "true"; + const allowPrivateHosts = privateHostsAllowed(environment); const policy = actionPolicy(environment); const supervisorUrl = url(environment, "COMPUTER_SUPERVISOR_URL"); diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 9deec2fa..32adf8ad 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { describe, expect, spyOn, test } from "bun:test"; import { configuredAuthProviders, loadConfig } from "../src/config"; // Intelligence is part of the MINIMUM contract, so it belongs in the base environment every other @@ -27,6 +28,19 @@ const baseEnvironment = { * The provider tests need the opposite starting point, or "Microsoft is configured" cannot be told * apart from "Microsoft and the Google that was already there". */ +/** + * A deployment that is actually deployed. + * + * `baseEnvironment` carries the example encryption key, which is refused under + * `NODE_ENV=production` — so a production case built on it fails on the key before it reaches + * whatever it meant to test. A real key here keeps each production test about its own subject. + */ +const productionEnvironment = { + ...baseEnvironment, + NODE_ENV: "production", + KEY_ENCRYPTION_KEY: "b3BlbmJvdC1wcm9kdWN0aW9uLXRlc3Qta2V5LTMyMzI=", +}; + const { GOOGLE_OAUTH_CLIENT_ID: _googleId, GOOGLE_OAUTH_CLIENT_SECRET: _googleSecret, @@ -418,6 +432,113 @@ describe("deployment configuration", () => { expect(loadConfig(baseEnvironment).computer).toBeUndefined(); }); + // `.env.example` used to ship AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true, and copying that file is the + // ordinary way a deployment gets its environment. So the way a hosted deployment ends up reaching + // its own network is not forgetting to set something, it is inheriting something. Refused in + // production for the same reason the example encryption key is: convenient locally, and an opening + // anywhere else. + test("refuses to start when a production deployment allows private hosts", () => { + expect(() => + loadConfig({ + ...productionEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true", + }), + ).toThrow("AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS"); + }); + + // The refusal has to name the way out, because the person reading it at boot is looking at a file + // they copied and does not necessarily know which line is the problem. + test("says to remove the line, and that it is local only", () => { + const attempt = () => + loadConfig({ + ...productionEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true", + }); + + expect(attempt).toThrow("local development only"); + expect(attempt).toThrow("Remove it"); + }); + + // The half of the matrix that was always right and has to stay right: absent means off, including + // in the environment where the new refusal lives. + test("starts in production when nothing asked for private hosts", () => { + const config = loadConfig({ + ...productionEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + COMPUTER_TOKEN: "computer-token", + }); + + expect(config.computer?.allowPrivateHosts).toBe(false); + }); + + // The local workflow is the reason the flag exists, so outside production it still does exactly + // what it did. Warned about, because a laptop is where a deployment is configured and the warning + // is the only chance to say this line does not travel. + test.each(["development", undefined])( + "warns and still allows private hosts under NODE_ENV=%p", + (nodeEnv) => { + const consoleWarn = spyOn(console, "warn").mockImplementation(() => {}); + + try { + const config = loadConfig({ + ...baseEnvironment, + ...(nodeEnv ? { NODE_ENV: nodeEnv } : {}), + AGENT_COMPUTER_URL: "http://localhost:4100", + AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true", + }); + + expect(config.computer?.allowPrivateHosts).toBe(true); + // Searched rather than indexed: `baseEnvironment` carries the example encryption key, which + // warns on its own account first. + const warning = consoleWarn.mock.calls + .map(([first]) => String(first)) + .find((line) => line.includes("AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS")); + + expect(warning).toBeDefined(); + expect(warning).toContain("local development only"); + expect(warning).toContain("Remove it before deploying"); + } finally { + consoleWarn.mockRestore(); + } + }, + ); + + // The refusal above only helps a deployment that reads it. The reason there was anything to refuse + // is that the file everybody copies arrived with the switch on, so the file is worth asserting + // about directly: a live line here is the regression, whatever the code does afterwards. + test("the shipped example does not turn private hosts on", () => { + const example = readFileSync( + new URL("../../.env.example", import.meta.url), + "utf8", + ); + + // Commented-out mentions are wanted — that is how the switch stays discoverable for a laptop. + const live = example + .split("\n") + .filter((line) => + /^\s*AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS\s*=/.test(line), + ); + + expect(live).toEqual([]); + }); + + // Anything that is not the exact opt-in is not an opt-in, so it is not the thing being refused + // either. A deployment that wrote something else has private hosts off and starts. + test.each(["false", "1", "yes", ""])( + "starts in production on AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=%p", + (value) => { + const config = loadConfig({ + ...productionEnvironment, + AGENT_COMPUTER_URL: "http://localhost:4100", + AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: value, + }); + + expect(config.computer?.allowPrivateHosts).toBe(false); + }, + ); + test.each([ ["Docker", "COMPUTER_SUPERVISOR_URL"], ["shared", "AGENT_COMPUTER_URL"], From b9501c16fa828a45b87aeda986ea23c042e3427c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:44:35 -0400 Subject: [PATCH 2/2] fix: compare NODE_ENV the way the switch beside it is read (#185) Review of #198 turned up three things. NODE_ENV was compared raw while AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS goes through optional(), which trims. A trailing space out of an env file walked past the refusal while the switch beside it still counted as set. Now read through the same helper, with a test. The Upgrading entry only covered the deployment that fails to start. The one-container image set the switch and NODE_ENV=production, so it really did run with private-host browsing on: a Bot could browse a private address there and a coworker could be registered at one, and both stop. The image bakes NODE_ENV=production, so there is no override from inside it. Named in the entry rather than left to be found. README, docs/configuration.md and docs/architecture.md each describe the switch and none mentioned the refusal. The architecture bullet sits three lines under the KEY_ENCRYPTION_KEY one that already says it, which is the most visible place the parity was missing. --- CHANGELOG.md | 8 ++++++++ README.md | 2 +- docs/architecture.md | 2 +- docs/configuration.md | 2 +- server/src/config.ts | 4 +++- server/tests/config.test.ts | 14 ++++++++++++++ 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22c5d6ea..482f9e54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,14 @@ inheriting something. It now ships commented out, which means a laptop that want uncomments it and everything else arrives without it. Under any other `NODE_ENV` the switch works exactly as before, with a warning at boot saying it does not travel. +**The one-container image shipped with the switch on, and no longer does.** It set both +`AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` and `NODE_ENV=production`, so the image really did run +with private-host browsing enabled. Two things that worked there stop: a Bot browsing a private +address such as an intranet page, and registering a coworker at a private endpoint like +`http://10.0.0.20:8000/ag-ui`. Because the image bakes in `NODE_ENV=production`, there is no +override — a deployment that needs either of those wants the compose setup or its own image rather +than the all-in-one. The image continues to start, and everything else in it is unchanged. + The cloud metadata addresses — `169.254.169.254`, `metadata.google.internal`, and the IPv6 and NAT64 spellings of them — were refused whatever this switch said, before and after. That floor has not moved. What changed is that it is no longer the only thing left standing in a production diff --git a/README.md b/README.md index c09c2c74..c57b4af8 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ Settings worth knowing: | `COMPUTER_SANDBOX` | Set to `on` for Chromium's own sandbox, where the host permits it. | | `EMBEDDED_POSTGRES` | Set to `on` for a database inside the deployment container. | | `AGENT_COMPUTER_POLICY` | JSON action policy. Malformed JSON stops server startup. | -| `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Lets a Bot reach this machine's own services. | +| `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Lets a Bot reach this machine's own services. Local only, and refused under `NODE_ENV=production`. | | `TENANT_PACKAGE_DIR` | Directory containing tenant YAML. Defaults to `../examples/fintech`. | | `DEPLOYMENT_ID` | Names this deployment when two share one Intelligence project. | diff --git a/docs/architecture.md b/docs/architecture.md index f3a1d9c3..6751e95f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -182,5 +182,5 @@ Connector credentials are stored through the credential vault and referenced by - `KEY_ENCRYPTION_KEY` must be a base64-encoded 32-byte value. The example key is refused with `NODE_ENV=production`. - Credential plaintext is encrypted at rest, never returned by APIs, and redacted from audit events. - Browser navigation allows `http` and `https`; cloud metadata addresses are refused under every configuration. -- `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` is for local development only. +- `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` is for local development only, and a deployment running with `NODE_ENV=production` refuses to start while it is set. - Computer tokens and supervisor tokens must be long random values outside local development. diff --git a/docs/configuration.md b/docs/configuration.md index a617f6fc..4a54392d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -178,7 +178,7 @@ where `` is `google`, `microsoft` or `okta`. | `COMPUTER_BROWSER_IDLE_MS` | How long an untouched browser is kept. 30 minutes by default; `0` keeps them resident. | | `COMPUTER_SUPERVISOR_URL` | Supervisor URL for per-Bot computers. If absent, Bots share `AGENT_COMPUTER_URL`. | | `SUPERVISOR_TOKEN` | Bearer token required by the supervisor. | -| `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Local-only private-host browsing when `true`. Cloud metadata addresses are still refused. | +| `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Local-only private-host browsing when `true`. A deployment running with `NODE_ENV=production` refuses to start while it is set. Cloud metadata addresses are refused either way. | | `AGENT_COMPUTER_POLICY` | JSON action policy: `{"mode":"enforce","deny":[...],"allow":[...]}`. | | `COMPUTER_RUNTIME` | Set to `runsc` to run supervised computers under gVisor. | | `COMPUTER_SANDBOX` | Set to `on` to enable Chromium's own sandbox where the host permits user namespaces. Which way it went is printed at start-up. | diff --git a/server/src/config.ts b/server/src/config.ts index 553e668f..ef04eafe 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -498,7 +498,9 @@ function privateHostsAllowed(environment: Environment): boolean { return false; } - if (environment.NODE_ENV === "production") { + // Through `optional`, so the comparison trims. Read raw, `NODE_ENV="production "` out of an env + // file would slip past a gate that the switch beside it, which does trim, would still trip. + if (optional(environment, "NODE_ENV") === "production") { throw new Error( "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true is for local development only: it lets a Bot reach this deployment's own network. Remove it from this deployment's environment.", ); diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 32adf8ad..d41e45dc 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -447,6 +447,20 @@ describe("deployment configuration", () => { ).toThrow("AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS"); }); + // Both sides of the comparison come out of the same env file, and the switch is read through + // `optional`, which trims. Comparing NODE_ENV raw would mean a trailing space typed into that file + // slipped past the refusal while the switch beside it still counted as set. + test("refuses a production deployment whose NODE_ENV carries whitespace", () => { + expect(() => + loadConfig({ + ...productionEnvironment, + NODE_ENV: "production ", + AGENT_COMPUTER_URL: "http://localhost:4100", + AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true", + }), + ).toThrow("AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS"); + }); + // The refusal has to name the way out, because the person reading it at boot is looking at a file // they copied and does not necessarily know which line is the problem. test("says to remove the line, and that it is local only", () => {