Skip to content

Commit bbdb7f2

Browse files
Keep the private-hosts switch out of production (#198)
* 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. * 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.
1 parent 6826e11 commit bbdb7f2

8 files changed

Lines changed: 217 additions & 12 deletions

File tree

.env.example

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,11 @@ AGENT_COMPUTER_URL=http://localhost:4100
158158
# without this value and refuses every request that does not present it. Use a long random value;
159159
# `scripts/start.sh` sets a development one for you.
160160
COMPUTER_TOKEN=
161-
# Local only. Lets a Bot browse this machine's own services; never set this in a deployment.
162-
AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true
161+
# Local only, and commented out because this file gets copied. Uncommenting it lets a Bot browse this
162+
# machine's own services, which is what a laptop wants and what a deployment must never have: it
163+
# lifts the private-address floor in browsing and in the endpoint a Bot may be registered against.
164+
# The server refuses to start with it set under NODE_ENV=production.
165+
# AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true
163166
#
164167
# What a Bot may do on its computer, as one JSON object. Absent uses the built-in default, which
165168
# permits the acting tools and forbids nothing, and records every action either way.

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,37 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
88

99
## Unreleased
1010

11+
### Upgrading
12+
13+
**A deployment that sets `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` with `NODE_ENV=production` no
14+
longer starts.** Remove the line and it starts again. Nothing else needs changing, and a deployment
15+
that never set it is unaffected.
16+
17+
The switch lets a Bot reach addresses inside the deployment's own network — `10.0.0.5`,
18+
`192.168.1.1`, `127.0.0.1:5432`, a link-local address — and it does that in two places, not one:
19+
browsing, and the endpoint a Bot may be registered against. It exists for a laptop, where the
20+
services a Bot is asked to look at are the ones running beside it.
21+
22+
The reason this is a refusal rather than a warning is how a deployment came to have it. `.env.example`
23+
shipped the line on, and copying that file is the ordinary way an environment gets filled in, so the
24+
path to a hosted deployment reaching its own network was not forgetting to set something, it was
25+
inheriting something. It now ships commented out, which means a laptop that wants the old behaviour
26+
uncomments it and everything else arrives without it. Under any other `NODE_ENV` the switch works
27+
exactly as before, with a warning at boot saying it does not travel.
28+
29+
**The one-container image shipped with the switch on, and no longer does.** It set both
30+
`AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` and `NODE_ENV=production`, so the image really did run
31+
with private-host browsing enabled. Two things that worked there stop: a Bot browsing a private
32+
address such as an intranet page, and registering a coworker at a private endpoint like
33+
`http://10.0.0.20:8000/ag-ui`. Because the image bakes in `NODE_ENV=production`, there is no
34+
override — a deployment that needs either of those wants the compose setup or its own image rather
35+
than the all-in-one. The image continues to start, and everything else in it is unchanged.
36+
37+
The cloud metadata addresses — `169.254.169.254`, `metadata.google.internal`, and the IPv6 and
38+
NAT64 spellings of them — were refused whatever this switch said, before and after. That floor has
39+
not moved. What changed is that it is no longer the only thing left standing in a production
40+
deployment that copied the example.
41+
1142
## 0.0.4
1243

1344
### A click citing a ref this deployment cannot resolve is refused

Dockerfile

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,15 @@ RUN apt-get update && apt-get install -y --no-install-recommends sudo \
173173
ENV WORKSPACE_DIR=/workspace
174174
ENV PROFILES_DIR=/profiles
175175

176-
# The browser is on loopback inside this container and reachable from nowhere else, which is why the
177-
# private-host allowance is on: the server is browsing to its own sibling process, not the internet.
176+
# The browser is on loopback inside this container and reachable from nowhere else.
177+
#
178+
# No private-host allowance is set for it, and none is needed. Reaching the sibling process goes
179+
# through `checkComputerAddress`, which decides on the protocol and the metadata floor and never
180+
# consults `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS`. That switch governs where a *Bot* may browse and
181+
# which endpoint one may be registered against, so setting it here bought nothing for this address
182+
# and let a Bot reach whatever this container's network reaches, which on a default bridge is the
183+
# host's LAN.
178184
ENV AGENT_COMPUTER_URL=http://127.0.0.1:4100
179-
ENV AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true
180185

181186
# NOTHING THAT MATTERS RUNS AS ROOT.
182187
#

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ Settings worth knowing:
202202
| `COMPUTER_SANDBOX` | Set to `on` for Chromium's own sandbox, where the host permits it. |
203203
| `EMBEDDED_POSTGRES` | Set to `on` for a database inside the deployment container. |
204204
| `AGENT_COMPUTER_POLICY` | JSON action policy. Malformed JSON stops server startup. |
205-
| `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Lets a Bot reach this machine's own services. |
205+
| `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Lets a Bot reach this machine's own services. Local only, and refused under `NODE_ENV=production`. |
206206
| `TENANT_PACKAGE_DIR` | Directory containing tenant YAML. Defaults to `../examples/fintech`. |
207207
| `DEPLOYMENT_ID` | Names this deployment when two share one Intelligence project. |
208208

@@ -291,7 +291,7 @@ provider's discovery document listed in `TRUSTED_ORIGINS`, not only the issuer.
291291

292292
- `agent-computer` drives a browser holding real logins. `docker-compose.yml` binds it to loopback; leave it there.
293293
- Store credentials through `/admin/credentials`, which encrypts them. Do not put credential values in tenant YAML or in committed files.
294-
- `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` lets a Bot reach services on this machine. Unset it if you would rather it could not.
294+
- `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.
295295

296296
## Development
297297

docs/architecture.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,5 +182,5 @@ Connector credentials are stored through the credential vault and referenced by
182182
- `KEY_ENCRYPTION_KEY` must be a base64-encoded 32-byte value. The example key is refused with `NODE_ENV=production`.
183183
- Credential plaintext is encrypted at rest, never returned by APIs, and redacted from audit events.
184184
- Browser navigation allows `http` and `https`; cloud metadata addresses are refused under every configuration.
185-
- `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true` is for local development only.
185+
- `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.
186186
- Computer tokens and supervisor tokens must be long random values outside local development.

docs/configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ where `<provider>` is `google`, `microsoft` or `okta`.
178178
| `COMPUTER_BROWSER_IDLE_MS` | How long an untouched browser is kept. 30 minutes by default; `0` keeps them resident. |
179179
| `COMPUTER_SUPERVISOR_URL` | Supervisor URL for per-Bot computers. If absent, Bots share `AGENT_COMPUTER_URL`. |
180180
| `SUPERVISOR_TOKEN` | Bearer token required by the supervisor. |
181-
| `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Local-only private-host browsing when `true`. Cloud metadata addresses are still refused. |
181+
| `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. |
182182
| `AGENT_COMPUTER_POLICY` | JSON action policy: `{"mode":"enforce","deny":[...],"allow":[...]}`. |
183183
| `COMPUTER_RUNTIME` | Set to `runsc` to run supervised computers under gVisor. |
184184
| `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. |

server/src/config.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,38 @@ function runtimeCapabilities(environment: Environment): RuntimeCapabilities {
480480
};
481481
}
482482

483+
/**
484+
* Whether a Bot may reach addresses inside this deployment's own network.
485+
*
486+
* Off unless asked for, and the asking is only allowed on a laptop. The switch exists so that a
487+
* local deployment can browse the services running beside it; what it turns off is not one rule but
488+
* the whole private-address floor, in navigation and in the endpoint a Bot may be registered
489+
* against, so with it on a signed-in person can point a Bot at a link-local address.
490+
*
491+
* Refused in production for the reason the example encryption key is: the way a deployment ends up
492+
* with it is not forgetting to set something, it is copying `.env.example`, which shipped it on. The
493+
* cloud metadata addresses are refused underneath this either way — see `computer/target.ts` — but
494+
* that floor is the last one, not the only one worth keeping.
495+
*/
496+
function privateHostsAllowed(environment: Environment): boolean {
497+
if (optional(environment, "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS") !== "true") {
498+
return false;
499+
}
500+
501+
// Through `optional`, so the comparison trims. Read raw, `NODE_ENV="production "` out of an env
502+
// file would slip past a gate that the switch beside it, which does trim, would still trip.
503+
if (optional(environment, "NODE_ENV") === "production") {
504+
throw new Error(
505+
"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.",
506+
);
507+
}
508+
console.warn(
509+
"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.",
510+
);
511+
512+
return true;
513+
}
514+
483515
function computerConfig(environment: Environment): ComputerConfig | undefined {
484516
const supervisorAddress = optional(environment, "COMPUTER_SUPERVISOR_URL");
485517
const sharedAddress = optional(environment, "AGENT_COMPUTER_URL");
@@ -494,8 +526,7 @@ function computerConfig(environment: Environment): ComputerConfig | undefined {
494526
*/
495527
const computerToken = optional(environment, "COMPUTER_TOKEN");
496528

497-
const allowPrivateHosts =
498-
optional(environment, "AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS") === "true";
529+
const allowPrivateHosts = privateHostsAllowed(environment);
499530
const policy = actionPolicy(environment);
500531

501532
const supervisorUrl = url(environment, "COMPUTER_SUPERVISOR_URL");

server/tests/config.test.ts

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { describe, expect, test } from "bun:test";
1+
import { readFileSync } from "node:fs";
2+
import { describe, expect, spyOn, test } from "bun:test";
23
import { configuredAuthProviders, loadConfig } from "../src/config";
34

45
// Intelligence is part of the MINIMUM contract, so it belongs in the base environment every other
@@ -27,6 +28,19 @@ const baseEnvironment = {
2728
* The provider tests need the opposite starting point, or "Microsoft is configured" cannot be told
2829
* apart from "Microsoft and the Google that was already there".
2930
*/
31+
/**
32+
* A deployment that is actually deployed.
33+
*
34+
* `baseEnvironment` carries the example encryption key, which is refused under
35+
* `NODE_ENV=production` — so a production case built on it fails on the key before it reaches
36+
* whatever it meant to test. A real key here keeps each production test about its own subject.
37+
*/
38+
const productionEnvironment = {
39+
...baseEnvironment,
40+
NODE_ENV: "production",
41+
KEY_ENCRYPTION_KEY: "b3BlbmJvdC1wcm9kdWN0aW9uLXRlc3Qta2V5LTMyMzI=",
42+
};
43+
3044
const {
3145
GOOGLE_OAUTH_CLIENT_ID: _googleId,
3246
GOOGLE_OAUTH_CLIENT_SECRET: _googleSecret,
@@ -418,6 +432,127 @@ describe("deployment configuration", () => {
418432
expect(loadConfig(baseEnvironment).computer).toBeUndefined();
419433
});
420434

435+
// `.env.example` used to ship AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=true, and copying that file is the
436+
// ordinary way a deployment gets its environment. So the way a hosted deployment ends up reaching
437+
// its own network is not forgetting to set something, it is inheriting something. Refused in
438+
// production for the same reason the example encryption key is: convenient locally, and an opening
439+
// anywhere else.
440+
test("refuses to start when a production deployment allows private hosts", () => {
441+
expect(() =>
442+
loadConfig({
443+
...productionEnvironment,
444+
AGENT_COMPUTER_URL: "http://localhost:4100",
445+
AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true",
446+
}),
447+
).toThrow("AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS");
448+
});
449+
450+
// Both sides of the comparison come out of the same env file, and the switch is read through
451+
// `optional`, which trims. Comparing NODE_ENV raw would mean a trailing space typed into that file
452+
// slipped past the refusal while the switch beside it still counted as set.
453+
test("refuses a production deployment whose NODE_ENV carries whitespace", () => {
454+
expect(() =>
455+
loadConfig({
456+
...productionEnvironment,
457+
NODE_ENV: "production ",
458+
AGENT_COMPUTER_URL: "http://localhost:4100",
459+
AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true",
460+
}),
461+
).toThrow("AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS");
462+
});
463+
464+
// The refusal has to name the way out, because the person reading it at boot is looking at a file
465+
// they copied and does not necessarily know which line is the problem.
466+
test("says to remove the line, and that it is local only", () => {
467+
const attempt = () =>
468+
loadConfig({
469+
...productionEnvironment,
470+
AGENT_COMPUTER_URL: "http://localhost:4100",
471+
AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true",
472+
});
473+
474+
expect(attempt).toThrow("local development only");
475+
expect(attempt).toThrow("Remove it");
476+
});
477+
478+
// The half of the matrix that was always right and has to stay right: absent means off, including
479+
// in the environment where the new refusal lives.
480+
test("starts in production when nothing asked for private hosts", () => {
481+
const config = loadConfig({
482+
...productionEnvironment,
483+
AGENT_COMPUTER_URL: "http://localhost:4100",
484+
COMPUTER_TOKEN: "computer-token",
485+
});
486+
487+
expect(config.computer?.allowPrivateHosts).toBe(false);
488+
});
489+
490+
// The local workflow is the reason the flag exists, so outside production it still does exactly
491+
// what it did. Warned about, because a laptop is where a deployment is configured and the warning
492+
// is the only chance to say this line does not travel.
493+
test.each(["development", undefined])(
494+
"warns and still allows private hosts under NODE_ENV=%p",
495+
(nodeEnv) => {
496+
const consoleWarn = spyOn(console, "warn").mockImplementation(() => {});
497+
498+
try {
499+
const config = loadConfig({
500+
...baseEnvironment,
501+
...(nodeEnv ? { NODE_ENV: nodeEnv } : {}),
502+
AGENT_COMPUTER_URL: "http://localhost:4100",
503+
AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: "true",
504+
});
505+
506+
expect(config.computer?.allowPrivateHosts).toBe(true);
507+
// Searched rather than indexed: `baseEnvironment` carries the example encryption key, which
508+
// warns on its own account first.
509+
const warning = consoleWarn.mock.calls
510+
.map(([first]) => String(first))
511+
.find((line) => line.includes("AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS"));
512+
513+
expect(warning).toBeDefined();
514+
expect(warning).toContain("local development only");
515+
expect(warning).toContain("Remove it before deploying");
516+
} finally {
517+
consoleWarn.mockRestore();
518+
}
519+
},
520+
);
521+
522+
// The refusal above only helps a deployment that reads it. The reason there was anything to refuse
523+
// is that the file everybody copies arrived with the switch on, so the file is worth asserting
524+
// about directly: a live line here is the regression, whatever the code does afterwards.
525+
test("the shipped example does not turn private hosts on", () => {
526+
const example = readFileSync(
527+
new URL("../../.env.example", import.meta.url),
528+
"utf8",
529+
);
530+
531+
// Commented-out mentions are wanted — that is how the switch stays discoverable for a laptop.
532+
const live = example
533+
.split("\n")
534+
.filter((line) =>
535+
/^\s*AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS\s*=/.test(line),
536+
);
537+
538+
expect(live).toEqual([]);
539+
});
540+
541+
// Anything that is not the exact opt-in is not an opt-in, so it is not the thing being refused
542+
// either. A deployment that wrote something else has private hosts off and starts.
543+
test.each(["false", "1", "yes", ""])(
544+
"starts in production on AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS=%p",
545+
(value) => {
546+
const config = loadConfig({
547+
...productionEnvironment,
548+
AGENT_COMPUTER_URL: "http://localhost:4100",
549+
AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS: value,
550+
});
551+
552+
expect(config.computer?.allowPrivateHosts).toBe(false);
553+
},
554+
);
555+
421556
test.each([
422557
["Docker", "COMPUTER_SUPERVISOR_URL"],
423558
["shared", "AGENT_COMPUTER_URL"],

0 commit comments

Comments
 (0)