fix(node): discover the browser's libraries instead of hardcoding them - #20
fix(node): discover the browser's libraries instead of hardcoding them#20dviejokfs wants to merge 5 commits into
Conversation
record_runtime_libraries filtered ldd output with /=> \//, which only
matches lines where the loader resolved the library to a path. A missing
one looks like
libatomic.so.1 => not found
with no path, so it was silently dropped: the recorded list came out
short and the runtime image was built without the package, with no error
anywhere. The helper exists precisely to avoid hardcoding package names,
and this is the case where it quietly stopped doing that.
Resolve those with apt-file, which maps a filename to the package
providing it without needing the file present. The query is anchored to
the multiarch library directory deliberately — a bare basename search for
libatomic.so.1 also matches lib32atomic1 and the -cross packages, and
libnss3.so matches firefox-esr and thunderbird, so an unanchored head -1
installs something wildly wrong. A library nothing provides now fails the
build with an explanation rather than producing an image that dies with
a loader error.
apt-file and its ~90MB index are only fetched when something is actually
missing.
Refs #14
Five problems a security review found in the previous commit. A soname reaches this from a binary the app produced, and it was interpolated raw into an extended regex. `|` has the lowest precedence, so DT_NEEDED of `x|usr/sbin/sshd` escaped the path anchor entirely, resolved to openssh-server, and had it installed as root in the runtime image with the build exiting 0. Validate the soname first. The anchor only covered /usr/lib/<triplet>/. On Debian the essential libraries are still recorded unmerged, so libz.so.1, libc.so.6, libgcc_s.so.1 and libtinfo.so.6 all resolved to nothing and failed the build — the opposite of the point. It also missed non-gnu triplets like arm-linux-gnueabihf. `for lib in $missing` was unquoted, so a soname of `*` expanded against the build directory. `set -f`. `head -1` was locale-dependent: the same source picked libavcodec-extra59 under LC_ALL=C and libavcodec59 under en_US.UTF-8. Pin the collation, and stop guessing — 24 sonames in bookworm have several providers, and for libc++.so.1 the first-sorted answer is the oldest ABI. Fail with the candidates and tell the user to pick one. The test pinned the vulnerable pattern as a literal and would not have caught any of this; it now asserts the guarantees instead.
set -f at the top of the script disabled globbing everywhere, including the line whose argument is documented as a glob. No current caller passes one — cobol and crystal name a single binary — but the PHP provider's own copy globs an extension directory, and a caller that inspects several binaries would have silently got no results. Turn expansion off only around the loop over sonames, which is the one place a crafted DT_NEEDED could expand against the build directory.
CHROMIUM_RUNTIME was fifteen bookworm package names maintained by hand, and it was wrong in both directions. Measured against Chrome for Testing 151: ldd finds seven libraries the list omits, which apt happened to pull in transitively, and the list carries libcups2, libpango-1.0-0 and libcairo2 which the headless shell does not link. The names are also bookworm's — libasound2 and libatk1.0-0 were renamed by the t64 transition, so a trixie base image would have failed at apt-get. Point ldd at the browser the install step actually downloaded and install what it says. That is exact per binary, per version and per Debian release, and it costs nothing when no browser is present. Fonts stay declared, and they are the whole residue: a browser opens them through fontconfig rather than linking them, so nothing about the binary reveals they are needed, and without them Chromium draws text as empty boxes rather than failing in a way anyone would notice. Everything else Chromium needs turned out to be a DT_NEEDED entry — the dlopen residue I expected to find is not there. Closes #15
📓 Changelog previewThis is what your commits will add to the generated ## [Unreleased]
### Fixed
- Resolve libraries ldd reports as missing instead of dropping them
- Validate sonames and widen the library anchor
- Keep pathname expansion on for the ldd argument
- **node:** Discover the browser's libraries instead of hardcoding them
- **node:** Find the browser instead of assuming npm's layout |
The glob was written for npm's hoisted node_modules. pnpm does not hoist playwright-core — the browsers land under node_modules/.pnpm/playwright-core@1.62.1/node_modules/... — so it matched nothing, and matching nothing was silent: ldd printed nothing, the recorded list came out empty, the runtime stage skipped its install on [ -s ], and the build exited 0. The image shipped with no browser libraries and failed on first launch. Against the hardcoded list this was a straight regression, and it hit every pnpm project. It also missed chrome-headless-shell for both tools, which lives in chrome-headless-shell-linux64 rather than chrome-linux64 — and that is the binary Playwright launches by default. Bounded only by luck: the shell's NEEDED set is currently a subset of the full browser's. Use find, collect both binaries, and fail the build when the search comes up empty rather than shipping an image that dies later.
|
An audit of this stack found two blockers, both reproduced and both now fixed in The one that mattered most: pnpm shipped an image with no browser librariesThe glob was written for npm's hoisted Matching nothing was silent: It also missed Now uses Verified end to end on the case that was broken — a pnpm Playwright app: Also fixed, on #19
Accepted, not fixed
Also fixed a precedence bug the audit found in my own test — 235 workspace tests, conformance green on |
1 participant
Closes #15. Stacked on #19 — it uses the discovery mechanism that PR fixes, so it should land after it. The diff below is only this commit.
What changes
CHROMIUM_RUNTIMEwas fifteen bookworm package names maintained by hand. It goes away. The install step pointslddat the browser it just downloaded, and the runtime image installs whatever that reports.Why the hardcoded list had to go
It was wrong in both directions, measured against Chrome for Testing 151:
lddfinds, list omitslibatspi2.0-0 libdbus-1-3 libexpat1 libglib2.0-0 libx11-6 libxcb1 libxext6libcups2 libpango-1.0-0 libcairo2It worked only because apt pulled the first group in transitively. And the names are bookworm's —
libasound2andlibatk1.0-0were renamed by thet64transition, so a trixie base image fails atapt-get install, which is #15.Discovery is exact per binary, per version and per Debian release, and costs nothing when there is no browser.
The one thing that stays declared
Fonts, and they are the entire residue:
Row A is the full
chromebinary — the one modern Puppeteer launches by default — where those three areDT_NEEDED. My earlier measurement usedchrome-headless-shelland I wrongly concluded they were unnecessary; they are found byldd, I was inspecting the wrong artefact. So thedlopenresidue I went looking for does not exist for Chromium: everything except fonts is discoverable.Fonts are not. A browser opens them through fontconfig, so nothing about the binary reveals them, and without them Chromium draws text as empty boxes rather than failing in a way anyone would notice — row C changes the output without changing the pass/fail.
Test plan
./scripts/conformance.sh playwright-app→ 8/8, includingserves expected content, which is rendered by Chromium inside the container and read back out of the DOM. Passing it with zero hardcoded library names is the whole claim of this PR.cobol-app,crystal-server(same helper) andnode-expressall passfonts-liberationand notlibnss3; the install step inspects the browser; an app with no browser gets neithercargo test --workspace→ 233 passed; fmt and clippy cleanReviewer notes
playwright-core/.local-browsers/*/chrome-linux*/chrome*and.cache/puppeteer/*/*/chrome-linux*/chrome*encode where each tool puts its browser. If either changes,lddmatches nothing, the recorded list is empty, and the image ships without libraries — failing at run time, not build time. Theplaywright-appconformance job catches that for Playwright on every CI run; Puppeteer has no equivalent example, so that gap is real and worth a follow-up.apt-file's ~90MB index once, in the install layer, because the browser's libraries are genuinely absent from the build image. It is cached with that layer and skipped entirely for every app without a browser.