Repository navigation
Commit 1fee13d
fix(service-print): bind the local placement's browser-server control socket to 127.0.0.1 (#24155)
Fixes #23968
Clause-②: no
## What
The print service's local placement (`OS_PRINT_CHROMIUM_PATH` /
`chromiumPath`) starts a playwright-core browser server for every render
and every boot probe, then connects to it. Its `chromium.launchServer`
call passed no `host`, so the control socket's bind rested on
playwright-core's default (`localhost` in the pinned 1.63.0). It now
passes `host: '127.0.0.1'`, the hardening PR #23811's ACCEPT set as its
condition.
- `packages/services/service-print/src/chromium-driver.ts`
- `launchServer({ executablePath, headless: true, timeout, host:
LOCAL_CONTROL_HOST })`, where `LOCAL_CONTROL_HOST = '127.0.0.1'` is a
module-private constant (not exported).
- The `ChromiumLauncher` type declares `host: string` on
`launchServer`'s options. It is required, so dropping the option from
the call is also a type error (measured below: `TS2345`).
- The header comment's **local** bullet now says the control socket is
bound to IPv4 loopback by this package and why, and that a host without
that address cannot open it.
- One `patch` changeset for `@objectstack/service-print`.
Not touched: the `port`, the remote placement, the plugin's options and
environment variables, the exports, the `playwright-core` version.
## Premise, checked on `origin/main` `747c8e9c48` before any edit
- `chromium-driver.ts:220` called `launchServer({ executablePath: …,
headless: true, timeout: timeoutMs })` with no `host`, and the launcher
type at `:105` had no `host` field.
- `playwright-core` is pinned exact at `1.63.0`. In the installed copy,
`lib/coreBundle.js:10175-10177` reads `listen(port = 0, hostname,
defaultPath) { … hostname ??= "localhost"`, and `:57206-57208` shows
`launchServer` calling `server.listen(options.port, options.host)`.
- `:10181-10193`: `wsEndpoint()` is built from `server.address()`, the
bound socket's own address, so it names where the socket really listens.
- `:8978-8986` (`computeAllowedHosts`): an explicit `127.0.0.1` keeps
the Host and Origin allow-list on, with the same loopback set
`localhost` got.
## Pins
1. **Test Core** (`chromium-driver.unit.test.ts`, no browser).
`stagedLauncher` now also returns its `launchServer` mock. A new test
runs the boot probe and then a render, and asserts the `host` option of
each `launchServer` call equals `['127.0.0.1', '127.0.0.1']`.
2. **Real Chromium** (`chromium-driver.test.ts`, inside the existing
`describe.skipIf(!CHROMIUM)` block). A local-placement test drives the
shipped driver through a launcher that wraps the real playwright-core
`chromium` and records each started server's `wsEndpoint()`. It asserts
that endpoint's hostname is `127.0.0.1`. Where no Chromium is installed
it skips with the file's existing warning, as before.
- **Chromium was present in this run**: `/opt/pw-browsers/chromium`,
found through `PLAYWRIGHT_BROWSERS_PATH`. The test ran and passed.
### Ablation
The fix was committed first. Each mutation went through
`scripts/ablation-replace.mjs`, which checks the anchor count on disk,
inside a trap that restores from `HEAD`. Each restore was proven by
blob: after restore, the blob was `84907ccbc2e2`, equal to the `HEAD`
blob, and `git diff HEAD` was empty.
- **A: drop the option.** The anchor `host: LOCAL_CONTROL_HOST,` went
from 1 hit to 0 on disk, blob `84907ccbc2e2` to `f79108c782c5`.
- The unit pin went **red**: `expected [ undefined, undefined ] to
deeply equal [ '127.0.0.1', '127.0.0.1' ]`.
- The real-Chromium pin stayed **green**. That was expected: this host's
`/etc/hosts` maps `localhost` to `127.0.0.1` only, so the library
default lands on the same address. The real pin checks where the socket
is bound, and the unit pin is the one that holds the explicit option.
- `tsc` (same mutation, separate run): `error TS2345: Argument of type
'{ executablePath: string; headless: true; timeout: number; }' is not
assignable to parameter of type '{ executablePath: string; headless:
boolean; timeout: number; host: string; }'`.
- **B: rebind to another loopback address.** `'127.0.0.1'` became
`'127.0.0.2'`, to show the real pin reads the bound address. The
real-Chromium pin went **red**: `expected '127.0.0.2' to be
'127.0.0.1'`.
## A host with no IPv4 loopback (card item 5)
**Measured, by a stand-in.** The kernel answers `EADDRNOTAVAIL` when a
bind address is not assigned to any local interface. A host with no IPv4
loopback gives that answer for `127.0.0.1`. I used an address this host
does not have, `192.0.2.1` (TEST-NET-1), in place of the missing
`127.0.0.1`, with the real Chromium. No network configuration was
changed.
- `chromium.launchServer({ …, host: '192.0.2.1' })` rejects with `listen
EADDRNOTAVAIL: address not available 192.0.2.1`. It does not fall back
to any other address.
- The shipped driver's probe, the path `PrintServicePlugin.start()`
takes, throws `Chromium could not be reached at the local executable
/opt/pw-browsers/chromium: listen EADDRNOTAVAIL: address not available
192.0.2.1`.
- The rest is read from code, not measured: `start()` logs that reason
as `printing is off until this is fixed — …`
(`print-service-plugin.ts:94-97`) and `status()` answers `unavailable`.
A print is refused with `SERVICE_UNAVAILABLE` / `503` once its re-probe
fails (`print-service.ts:250-276`).
- **A limit, measured.** playwright-core launches Chromium before it
binds the socket (`coreBundle.js:57186-57207`), and a failed bind does
not close that browser.
- 1.5 s after the rejection, 6 processes were still descended from the
measuring node process, and the process did not exit on its own.
- `timeout` stopped it at 120 s. playwright's own SIGTERM handler then
cleaned up the browsers, and no orphan was left.
- In a long-running server on such a host, each failed open would
therefore leave one Chromium running: one for the boot probe, and one
for each print's re-probe.
- The same was true before this change on any host where binding
`localhost` fails. This PR does not change that library behaviour.
**Stated limits, not measured here:**
- A real host without `127.0.0.1`. I tried to set one up by deleting the
address inside a private network namespace, and this session's
permission classifier refused that step.
- A host whose `localhost` resolves to `::1` first. This kernel has no
IPv6: binding `::1` answers `EAFNOSUPPORT`.
- Node's `listen` binds the first address a name resolves to, so the old
default would have bound `[::1]` on such a host. The socket moves to
`127.0.0.1` there.
- The driver still connects, because it dials whatever `wsEndpoint()`
names.
- How rare such a host is. Linux assigns `127.0.0.1/8` to a loopback
device when it comes up (`net/ipv4/devinet.c`; a fresh network namespace
here bound `127.0.0.1` too). So a host lacks it only when someone
removed it.
## Tests (at the final head `8ba6a17be0`)
- `pnpm --filter @objectstack/service-print build && … typecheck && …
exec vitest run --reporter=verbose` exited 0.
- `check-dts-emitted: @objectstack/service-print - 2/2 declared
declaration file(s) present`.
- 6 test files, 81 tests passed, including both new pins.
- `node -e "require('…/service-print/dist/index.cjs')"` loads, with 7
exports.
## Gates (local, at `8ba6a17be0`)
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 63 commands. I ran all of them
sequentially and recorded each exit code before any pipe.
- 62 exited 0.
- `pnpm check:dual-build-cjs-loads` exited 3, `PREREQUISITE NOT MET`: it
reads every package's `dist/`, and only this package's closure is built
here. That makes it **NOT MEASURED** locally. `Lint & Repo Gates` runs
it in CI. The `require()` of this package's CJS build above is the
direct reading for the one package touched.
- `--ran` reconciliation: `63 derived famil(ies) accounted for — 62 run,
1 NOT-MEASURED (1 DERIVED from a recorded exit 3)`, 0 unrun.
- The 4 roster gates whose rosters sit under these paths were also run,
each with exit 0: `check-changeset-fixed`, `check:authz-resolver`,
`check:error-code-casing`, `check:filter-alias-parity`.
- CI's check-runs are not in yet as this PR opens; the os-dev report on
the card carries them.
## Acceptance notes
- **playwright-core leaves a launched Chromium running when its bind
fails** (measured above). This is library behaviour, it already existed
before this PR, and it is reachable only on a host where the bind fails.
No public-door reach was measured on a real deployment, so it is noted
here and not filed. Carrier: none.
- **"Module-private".** The claim called `ChromiumLauncher`
module-private. Strictly, the module exports it as a type (`export type
{ ChromiumLauncher, … }`, used by the two test files). It is not
re-exported from `src/index.ts`, and it is absent from `dist/index.d.ts`
and `dist/index.d.cts` (0 hits; the control, `PrintServicePlugin`, has
5). So making `host` required changes no published type, and `Clause-②:
no` holds.
---
_Generated by [Claude
Code](https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 135e19e commit 1fee13d
4 files changed
Lines changed: 72 additions & 9 deletions
File tree
- .changeset
- packages/services/service-print/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
307 | 307 | | |
308 | 308 | | |
309 | 309 | | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
310 | 335 | | |
311 | 336 | | |
312 | 337 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
16 | 23 | | |
17 | 24 | | |
18 | 25 | | |
| |||
102 | 109 | | |
103 | 110 | | |
104 | 111 | | |
105 | | - | |
| 112 | + | |
| 113 | + | |
106 | 114 | | |
107 | 115 | | |
108 | 116 | | |
109 | 117 | | |
110 | 118 | | |
111 | 119 | | |
112 | 120 | | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
113 | 124 | | |
114 | 125 | | |
115 | 126 | | |
| |||
217 | 228 | | |
218 | 229 | | |
219 | 230 | | |
220 | | - | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
221 | 237 | | |
222 | 238 | | |
223 | 239 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
| 71 | + | |
71 | 72 | | |
72 | | - | |
| 73 | + | |
73 | 74 | | |
74 | 75 | | |
75 | 76 | | |
76 | | - | |
| 77 | + | |
77 | 78 | | |
78 | 79 | | |
79 | 80 | | |
| |||
86 | 87 | | |
87 | 88 | | |
88 | 89 | | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
89 | 100 | | |
90 | 101 | | |
91 | 102 | | |
| |||
0 commit comments