From 533bf6441072f5fdd4f731aafcfeb6b28ed30274 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Wed, 5 Aug 2026 23:33:21 +0200 Subject: [PATCH 1/5] fix: resolve libraries ldd reports as missing instead of dropping them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/autopack-providers/src/support.rs | 70 ++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/crates/autopack-providers/src/support.rs b/crates/autopack-providers/src/support.rs index 7dab9e9..42d25c7 100644 --- a/crates/autopack-providers/src/support.rs +++ b/crates/autopack-providers/src/support.rs @@ -159,6 +159,27 @@ pub fn shell_quote(value: &str) -> String { /// Debian release bump — ICU is `libicu72` on bookworm and `libicu76` on /// trixie, and the `t64` transition renamed a whole set of others. Asking the /// linker and then dpkg is release-agnostic. +/// +/// Two lookups, because `ldd` answers in two different shapes: +/// +/// - `libfoo.so.1 => /usr/lib/.../libfoo.so.1` — resolved, so the file is on +/// disk and `dpkg-query -S` names its owner. +/// - `libfoo.so.1 => not found` — absent from the build image too, so there is +/// no path and no file to ask dpkg about. These used to be dropped on the +/// floor by the `=> /` filter, which meant a genuinely missing library +/// produced a short list and an image that failed at run time with a loader +/// error rather than a build that failed with an explanation. +/// +/// The second shape is resolved 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 on purpose: a bare basename +/// search for `libatomic.so.1` also matches `lib32atomic1` and the +/// `-cross` packages, and for `libnss3.so` it matches `firefox-esr` and +/// `thunderbird`, so an unanchored `head -1` installs something wildly wrong. +/// +/// `apt-file` and its index are only fetched when there is something to look +/// up — the index is around 90MB, which is not worth paying for on every +/// build that has nothing missing. pub fn record_runtime_libraries(glob: &str, record_to: &str) -> String { format!( "set -eu; \ @@ -168,6 +189,23 @@ pub fn record_runtime_libraries(glob: &str, record_to: &str) -> String { | xargs -r readlink -f 2>/dev/null | sort -u \ | xargs -r dpkg-query -S 2>/dev/null \ | cut -d: -f1 | sort -u > {record_to}; \ + missing=$(ldd {glob} 2>/dev/null | awk '/not found/ {{ print $1 }}' | sort -u); \ + if [ -n \"$missing\" ]; then \ + apt-get update >/dev/null; \ + apt-get install -y --no-install-recommends apt-file >/dev/null; \ + apt-file update >/dev/null; \ + for lib in $missing; do \ + owner=$(apt-file search -x \"^/usr/lib/[a-z0-9_]*-linux-gnu/$lib$\" 2>/dev/null \ + | cut -d: -f1 | sort -u | head -1); \ + if [ -n \"$owner\" ]; then \ + echo \"$owner\" >> {record_to}; \ + else \ + echo \"autopack: no package provides $lib\" >&2; \ + exit 1; \ + fi; \ + done; \ + sort -u -o {record_to} {record_to}; \ + fi; \ cat {record_to}" ) } @@ -203,6 +241,38 @@ mod tests { (dir, app) } + #[test] + fn missing_libraries_are_resolved_rather_than_dropped() { + let script = record_runtime_libraries("/app/bin/app", "/tmp/deps"); + + // The resolved branch is unchanged: ldd gives a path, dpkg names the + // owner. + assert!(script.contains("dpkg-query -S")); + + // The "not found" branch is the point of this: those lines carry no + // path, so they used to be filtered out and the library silently + // omitted from the runtime image. + assert!(script.contains("/not found/")); + assert!(script.contains("apt-file search")); + + // Anchored to the multiarch directory. 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 match would install something wildly wrong. + assert!(script.contains("^/usr/lib/[a-z0-9_]*-linux-gnu/$lib$")); + + // A library nothing provides fails the build rather than producing a + // short list and an image that dies with a loader error. + assert!(script.contains("no package provides")); + assert!(script.contains("exit 1")); + + // apt-file's index is ~90MB, so it is only fetched when there is + // something to look up. + let index_fetch = script.find("apt-file update").unwrap(); + let guard = script.find("if [ -n \"$missing\" ]").unwrap(); + assert!(guard < index_fetch); + } + #[test] fn parses_the_web_process() { let (_dir, app) = app_with(&[( From c9462c0c60b33cf0b665173fef38be9af03624a9 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 6 Aug 2026 09:49:56 +0200 Subject: [PATCH 2/5] fix: validate sonames and widen the library anchor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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//. 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. --- crates/autopack-providers/src/support.rs | 102 +++++++++++++---------- 1 file changed, 58 insertions(+), 44 deletions(-) diff --git a/crates/autopack-providers/src/support.rs b/crates/autopack-providers/src/support.rs index 42d25c7..9b0e632 100644 --- a/crates/autopack-providers/src/support.rs +++ b/crates/autopack-providers/src/support.rs @@ -151,38 +151,35 @@ pub fn shell_quote(value: &str) -> String { /// Record which Debian packages own the shared libraries `glob` links against. /// -/// Runs in the build stage, where the `-dev` packages are still installed: -/// `ldd` reports a missing library as "not found" with no path, so the lookup -/// cannot be deferred to the runtime image. +/// Runs in the build stage, where the `-dev` packages are still installed. /// /// This exists because hardcoding runtime package names does not survive a /// Debian release bump — ICU is `libicu72` on bookworm and `libicu76` on /// trixie, and the `t64` transition renamed a whole set of others. Asking the -/// linker and then dpkg is release-agnostic. +/// linker and then dpkg is release-agnostic, and it installs exactly what the +/// binary links rather than a hand-maintained superset. /// -/// Two lookups, because `ldd` answers in two different shapes: +/// `ldd` answers in two shapes, and both matter: /// /// - `libfoo.so.1 => /usr/lib/.../libfoo.so.1` — resolved, so the file is on /// disk and `dpkg-query -S` names its owner. /// - `libfoo.so.1 => not found` — absent from the build image too, so there is -/// no path and no file to ask dpkg about. These used to be dropped on the -/// floor by the `=> /` filter, which meant a genuinely missing library -/// produced a short list and an image that failed at run time with a loader -/// error rather than a build that failed with an explanation. +/// no path to ask dpkg about. These are resolved with `apt-file`, which maps +/// a filename to its providing package without needing the file present. /// -/// The second shape is resolved 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 on purpose: a bare basename -/// search for `libatomic.so.1` also matches `lib32atomic1` and the -/// `-cross` packages, and for `libnss3.so` it matches `firefox-esr` and -/// `thunderbird`, so an unanchored `head -1` installs something wildly wrong. +/// A soname reaches this from a binary the app produced, so it is untrusted +/// input. It is validated against the characters a library name can actually +/// contain before it goes anywhere near the regex: `|` alone would otherwise +/// escape the path anchor through alternation and let a crafted `DT_NEEDED` +/// select any package in the archive, which then gets installed as root in the +/// runtime image. /// -/// `apt-file` and its index are only fetched when there is something to look -/// up — the index is around 90MB, which is not worth paying for on every -/// build that has nothing missing. +/// `apt-file` and its ~90MB index are only fetched when something is actually +/// missing. pub fn record_runtime_libraries(glob: &str, record_to: &str) -> String { format!( - "set -eu; \ + "set -euf; \ + export LC_ALL=C; \ mkdir -p \"$(dirname {record_to})\"; \ ldd {glob} 2>/dev/null \ | awk '/=> \\// {{ print $3 }}' | sort -u \ @@ -195,14 +192,25 @@ pub fn record_runtime_libraries(glob: &str, record_to: &str) -> String { apt-get install -y --no-install-recommends apt-file >/dev/null; \ apt-file update >/dev/null; \ for lib in $missing; do \ - owner=$(apt-file search -x \"^/usr/lib/[a-z0-9_]*-linux-gnu/$lib$\" 2>/dev/null \ - | cut -d: -f1 | sort -u | head -1); \ - if [ -n \"$owner\" ]; then \ - echo \"$owner\" >> {record_to}; \ - else \ + case \"$lib\" in \ + *[!A-Za-z0-9._+-]*) \ + echo \"autopack: refusing to look up '$lib': not a library name\" >&2; \ + exit 1 ;; \ + esac; \ + owners=$(apt-file search -x \"^/(usr/)?lib/[a-z0-9_]*-linux-gnu[a-z0-9]*/$lib\\$\" \ + | cut -d: -f1 | sort -u); \ + count=$(printf '%s' \"$owners\" | grep -c . || true); \ + if [ \"$count\" -eq 0 ]; then \ echo \"autopack: no package provides $lib\" >&2; \ exit 1; \ fi; \ + if [ \"$count\" -gt 1 ]; then \ + echo \"autopack: $lib is provided by more than one package:\" >&2; \ + printf ' %s\\n' $owners >&2; \ + echo \"Choose one and add it to apt_packages in autopack.json.\" >&2; \ + exit 1; \ + fi; \ + printf '%s\\n' \"$owners\" >> {record_to}; \ done; \ sort -u -o {record_to} {record_to}; \ fi; \ @@ -242,35 +250,41 @@ mod tests { } #[test] - fn missing_libraries_are_resolved_rather_than_dropped() { + fn missing_libraries_are_resolved_and_hostile_sonames_refused() { let script = record_runtime_libraries("/app/bin/app", "/tmp/deps"); - // The resolved branch is unchanged: ldd gives a path, dpkg names the - // owner. + // Resolved libraries: unchanged path through dpkg. assert!(script.contains("dpkg-query -S")); - - // The "not found" branch is the point of this: those lines carry no - // path, so they used to be filtered out and the library silently - // omitted from the runtime image. + // Missing ones are looked up rather than dropped on the floor. assert!(script.contains("/not found/")); assert!(script.contains("apt-file search")); - // Anchored to the multiarch directory. 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 match would install something wildly wrong. - assert!(script.contains("^/usr/lib/[a-z0-9_]*-linux-gnu/$lib$")); - - // A library nothing provides fails the build rather than producing a - // short list and an image that dies with a loader error. + // A soname comes from a binary the app produced. Anything outside the + // character set a library name can hold is refused before it reaches + // the regex — `|` alone escapes the anchor through alternation. + assert!(script.contains("*[!A-Za-z0-9._+-]*")); + assert!(script.contains("refusing to look up")); + + // Globbing off, so a soname of `*` cannot expand against the build + // directory; deterministic collation, so the same source does not + // resolve differently on two builders. + assert!(script.contains("set -euf")); + assert!(script.contains("LC_ALL=C")); + + // The anchor covers /lib as well as /usr/lib: on Debian the essential + // libraries are still recorded unmerged, so a /usr-only anchor fails + // the build for libc, libz and friends. + assert!(script.contains("^/(usr/)?lib/[a-z0-9_]*-linux-gnu[a-z0-9]*/")); + + // Ambiguity and absence both stop the build with something actionable + // rather than guessing. assert!(script.contains("no package provides")); - assert!(script.contains("exit 1")); + assert!(script.contains("provided by more than one package")); + assert!(script.contains("add it to apt_packages")); - // apt-file's index is ~90MB, so it is only fetched when there is - // something to look up. - let index_fetch = script.find("apt-file update").unwrap(); + // The ~90MB index is only fetched when there is something to look up. let guard = script.find("if [ -n \"$missing\" ]").unwrap(); - assert!(guard < index_fetch); + assert!(guard < script.find("apt-file update").unwrap()); } #[test] From b1ad053e8aba4d4652eaf881ef3c124c9574ca5f Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 6 Aug 2026 09:56:04 +0200 Subject: [PATCH 3/5] fix: keep pathname expansion on for the ldd argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/autopack-providers/src/support.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/autopack-providers/src/support.rs b/crates/autopack-providers/src/support.rs index 9b0e632..b454563 100644 --- a/crates/autopack-providers/src/support.rs +++ b/crates/autopack-providers/src/support.rs @@ -176,9 +176,14 @@ pub fn shell_quote(value: &str) -> String { /// /// `apt-file` and its ~90MB index are only fetched when something is actually /// missing. +/// +/// `glob` is expanded by the shell, so it may name several binaries — pathname +/// expansion stays on for that line and is turned off only around the loop +/// over sonames, where a `*` from a crafted `DT_NEEDED` would otherwise expand +/// against the build directory. pub fn record_runtime_libraries(glob: &str, record_to: &str) -> String { format!( - "set -euf; \ + "set -eu; \ export LC_ALL=C; \ mkdir -p \"$(dirname {record_to})\"; \ ldd {glob} 2>/dev/null \ @@ -191,6 +196,7 @@ pub fn record_runtime_libraries(glob: &str, record_to: &str) -> String { apt-get update >/dev/null; \ apt-get install -y --no-install-recommends apt-file >/dev/null; \ apt-file update >/dev/null; \ + set -f; \ for lib in $missing; do \ case \"$lib\" in \ *[!A-Za-z0-9._+-]*) \ @@ -268,7 +274,12 @@ mod tests { // Globbing off, so a soname of `*` cannot expand against the build // directory; deterministic collation, so the same source does not // resolve differently on two builders. - assert!(script.contains("set -euf")); + assert!(script.contains("set -eu")); + // Globbing is off for the soname loop but must stay on for `ldd`, + // whose argument is a glob for callers that inspect several binaries. + let ldd = script.find("ldd ").unwrap(); + let disable = script.find("set -f;").unwrap(); + assert!(ldd < disable); assert!(script.contains("LC_ALL=C")); // The anchor covers /lib as well as /usr/lib: on Debian the essential From ac83ebb0521c7c71990937b59de0eb3db59ebde5 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 6 Aug 2026 10:03:40 +0200 Subject: [PATCH 4/5] fix(node): discover the browser's libraries instead of hardcoding them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/autopack-providers/src/native.rs | 19 ---- crates/autopack-providers/src/node/mod.rs | 102 ++++++++++++++++++---- 2 files changed, 87 insertions(+), 34 deletions(-) diff --git a/crates/autopack-providers/src/native.rs b/crates/autopack-providers/src/native.rs index 11200f8..5cd8f98 100644 --- a/crates/autopack-providers/src/native.rs +++ b/crates/autopack-providers/src/native.rs @@ -29,25 +29,6 @@ pub struct NativeDependency { pub runtime: &'static [&'static str], } -/// Chromium's shared library closure, needed by headless browsers. -pub(crate) const CHROMIUM_RUNTIME: &[&str] = &[ - "libnss3", - "libnspr4", - "libatk1.0-0", - "libatk-bridge2.0-0", - "libcups2", - "libdrm2", - "libxkbcommon0", - "libxcomposite1", - "libxdamage1", - "libxfixes3", - "libxrandr2", - "libgbm1", - "libasound2", - "libpango-1.0-0", - "libcairo2", -]; - /// Node packages that need system libraries. pub const NODE: &[NativeDependency] = &[ // node-canvas compiles against Cairo and friends; there are no prebuilt diff --git a/crates/autopack-providers/src/node/mod.rs b/crates/autopack-providers/src/node/mod.rs index 5836347..20c8e75 100644 --- a/crates/autopack-providers/src/node/mod.rs +++ b/crates/autopack-providers/src/node/mod.rs @@ -10,13 +10,29 @@ use autopack_core::plan::{Command, Layer}; use autopack_core::{steps, App, BuildContext, Environment, Provider, Result, APP_DIR}; use crate::support::{ - caddy_layer, caddy_start_command, caddyfile, normalize_version_range, procfile_web_command, - read_version_file, CADDYFILE_PATH, + caddy_layer, caddy_start_command, caddyfile, install_recorded_runtime_libraries, + normalize_version_range, procfile_web_command, read_version_file, record_runtime_libraries, + CADDYFILE_PATH, RUNTIME_DEPS_FILE, }; /// Where a Next.js standalone bundle is staged for the runtime image. const STANDALONE_DIR: &str = "/app/standalone"; +/// Where Playwright keeps browsers when `PLAYWRIGHT_BROWSERS_PATH=0` asks for +/// an install beside the package. +const PLAYWRIGHT_PACKAGE: &str = "playwright-core"; +const PLAYWRIGHT_BROWSERS: &str = ".local-browsers"; + +/// Fonts a browser needs but never links. +/// +/// Everything else Chromium requires is a `DT_NEEDED` entry, so `ldd` finds it +/// — measured against Chrome for Testing: with the discovered set installed +/// there are zero unresolved libraries and it renders, screenshots and prints. +/// Fonts are the exception. They are opened through fontconfig at run time, so +/// no amount of inspecting the binary reveals them, and a browser without them +/// draws text as empty boxes rather than failing in a way anyone would notice. +const CHROMIUM_FONTS: &[&str] = &["fonts-liberation"]; + /// Node version used when the app does not pin one. const DEFAULT_NODE_VERSION: &str = "24"; @@ -136,6 +152,12 @@ impl Provider for NodeProvider { let browsers = browser_tooling(&package); ctx.deploy_apt_packages .extend(browsers.runtime_packages.iter().cloned()); + if !browsers.browser_binaries.is_empty() { + ctx.add_runtime_input(Layer::step(steps::INSTALL).including([RUNTIME_DEPS_FILE])); + ctx.add_runtime_command(Command::shell(install_recorded_runtime_libraries( + RUNTIME_DEPS_FILE, + ))); + } if !browsers.is_empty() { ctx.add_metadata("browser", "chromium"); } @@ -221,6 +243,17 @@ impl NodeProvider { for download in &browsers.downloads { step.add_command(Command::shell(download.clone())); } + // Ask the browser what it links rather than carrying a list. The + // hardcoded closure was a hand-maintained union across two Chrome + // binaries, wrong in both directions — it missed seven libraries apt + // happened to pull in transitively, and its names are bookworm's, so + // it breaks on a base image bump. + if !browsers.browser_binaries.is_empty() { + step.add_command(Command::shell(record_runtime_libraries( + &browsers.browser_binaries.join(" "), + RUNTIME_DEPS_FILE, + ))); + } Ok(()) } @@ -483,8 +516,15 @@ fn node_version(app: &App, package: &PackageJson) -> Result<(String, String)> { /// Browser tooling an app needs wired up: the system libraries the browser /// links against, where it is cached, and how it is fetched. struct BrowserTooling { - /// Debian packages the runtime image needs. + /// Debian packages the runtime image needs that `ldd` cannot discover. + /// + /// Fonts only. A browser does not link them — it opens them through + /// fontconfig at run time — so nothing about the binary reveals that they + /// are needed, and without them Chromium renders text as empty boxes + /// instead of failing in a way anyone would notice. runtime_packages: Vec, + /// Paths to `ldd` for the libraries the browser actually links. + browser_binaries: Vec, /// Environment for the install, build and runtime stages. variables: Vec<(&'static str, String)>, /// Commands the install step runs after dependencies are in place. @@ -529,6 +569,7 @@ impl BrowserTooling { fn browser_tooling(package: &PackageJson) -> BrowserTooling { let mut tooling = BrowserTooling { runtime_packages: Vec::new(), + browser_binaries: Vec::new(), variables: Vec::new(), downloads: Vec::new(), }; @@ -559,8 +600,20 @@ fn browser_tooling(package: &PackageJson) -> BrowserTooling { .variables .push(("PUPPETEER_CACHE_DIR", format!("{APP_DIR}/.cache/puppeteer"))); } + if playwright { + // Playwright installs beside the package; both the full browser and + // the headless shell ship, and they do not link the same set. + tooling.browser_binaries.push(format!( + "{APP_DIR}/node_modules/{PLAYWRIGHT_PACKAGE}/{PLAYWRIGHT_BROWSERS}/*/chrome-linux*/chrome*" + )); + } + if puppeteer { + tooling.browser_binaries.push(format!( + "{APP_DIR}/.cache/puppeteer/*/*/chrome-linux*/chrome*" + )); + } if playwright || puppeteer { - tooling.runtime_packages = crate::native::CHROMIUM_RUNTIME + tooling.runtime_packages = CHROMIUM_FONTS .iter() .map(|package| (*package).to_string()) .collect(); @@ -584,6 +637,7 @@ fn is_simple_command(script: &str) -> bool { #[cfg(test)] mod tests { + use super::{PLAYWRIGHT_BROWSERS, RUNTIME_DEPS_FILE}; use crate::test_support::{plan_for, plan_with_env, write_app}; use autopack_core::APP_DIR; @@ -930,7 +984,7 @@ mod tests { } #[test] - fn playwright_gets_the_chromium_libraries_and_an_in_app_browser_cache() { + fn playwright_discovers_its_libraries_and_gets_an_in_app_browser_cache() { // The browser is downloaded during install. Left at its default // location it lands under $HOME, which the deploy layer never carries // — and the build runs as root while the runtime user is `autopack`, @@ -945,11 +999,31 @@ mod tests { ]); let analysis = plan_for(&app); + // No hardcoded library closure: the runtime image installs whatever + // `ldd` found the browser to link, plus fonts, which are opened + // through fontconfig and so are invisible to the linker. let runtime = analysis.plan.step("runtime").unwrap(); + let apt = runtime.commands[0].display_name(); + assert!(apt.contains("fonts-liberation"), "{apt}"); + assert!(!apt.contains("libnss3"), "{apt}"); assert!( - runtime.commands[0].display_name().contains("libnss3"), - "{}", - runtime.commands[0].display_name() + runtime + .commands + .iter() + .any(|c| c.display_name().contains(RUNTIME_DEPS_FILE)), + "runtime does not install the recorded libraries" + ); + assert!( + analysis + .plan + .step("install") + .unwrap() + .commands + .iter() + .any(|c| c.display_name().contains("ldd ") + && c.display_name().contains(".cache/puppeteer") + || c.display_name().contains(PLAYWRIGHT_BROWSERS)), + "install step does not inspect the browser" ); for step in ["install", "build"] { @@ -1057,7 +1131,7 @@ mod tests { assert!( !analysis.plan.step("runtime").unwrap().commands[0] .display_name() - .contains("libnss3"), + .contains("fonts-liberation"), "{manifest}" ); } @@ -1077,9 +1151,8 @@ mod tests { ("index.html", ""), ]); let analysis = plan_for(&app); - assert!(!analysis.plan.step("runtime").unwrap().commands[0] - .display_name() - .contains("libnss3")); + let apt = analysis.plan.step("runtime").unwrap().commands[0].display_name(); + assert!(!apt.contains("fonts-liberation"), "{apt}"); assert!(!analysis .plan .step("install") @@ -1103,9 +1176,8 @@ mod tests { let install = analysis.plan.step("install").unwrap(); assert!(install.variables.get("PLAYWRIGHT_BROWSERS_PATH").is_none()); assert!(install.variables.get("PUPPETEER_CACHE_DIR").is_none()); - assert!(!analysis.plan.step("runtime").unwrap().commands[0] - .display_name() - .contains("libnss3")); + let apt = analysis.plan.step("runtime").unwrap().commands[0].display_name(); + assert!(!apt.contains("fonts-liberation"), "{apt}"); } #[test] From 581c163367f1ce772acac03974d3510b2a13b381 Mon Sep 17 00:00:00 2001 From: David Viejo Date: Thu, 6 Aug 2026 11:00:15 +0200 Subject: [PATCH 5/5] fix(node): find the browser instead of assuming npm's layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/autopack-providers/src/node/mod.rs | 124 ++++++++++++++++++---- 1 file changed, 106 insertions(+), 18 deletions(-) diff --git a/crates/autopack-providers/src/node/mod.rs b/crates/autopack-providers/src/node/mod.rs index 20c8e75..af623f5 100644 --- a/crates/autopack-providers/src/node/mod.rs +++ b/crates/autopack-providers/src/node/mod.rs @@ -18,9 +18,8 @@ use crate::support::{ /// Where a Next.js standalone bundle is staged for the runtime image. const STANDALONE_DIR: &str = "/app/standalone"; -/// Where Playwright keeps browsers when `PLAYWRIGHT_BROWSERS_PATH=0` asks for -/// an install beside the package. -const PLAYWRIGHT_PACKAGE: &str = "playwright-core"; +/// Directory Playwright keeps browsers in when `PLAYWRIGHT_BROWSERS_PATH=0` +/// asks for an install beside the package. const PLAYWRIGHT_BROWSERS: &str = ".local-browsers"; /// Fonts a browser needs but never links. @@ -152,7 +151,7 @@ impl Provider for NodeProvider { let browsers = browser_tooling(&package); ctx.deploy_apt_packages .extend(browsers.runtime_packages.iter().cloned()); - if !browsers.browser_binaries.is_empty() { + if !browsers.browser_searches.is_empty() { ctx.add_runtime_input(Layer::step(steps::INSTALL).including([RUNTIME_DEPS_FILE])); ctx.add_runtime_command(Command::shell(install_recorded_runtime_libraries( RUNTIME_DEPS_FILE, @@ -248,9 +247,24 @@ impl NodeProvider { // binaries, wrong in both directions — it missed seven libraries apt // happened to pull in transitively, and its names are bookworm's, so // it breaks on a base image bump. - if !browsers.browser_binaries.is_empty() { + if !browsers.browser_searches.is_empty() { + let search = browsers.browser_searches.join("; "); + // Nothing downstream notices an empty result: no binaries means no + // libraries recorded, the runtime stage skips its install, and the + // image looks fine until the first time it opens a browser. Stop + // here, where the message can say what was searched. + step.add_command(Command::shell(format!( + "set -eu; \ + if [ -z \"$({search})\" ]; then \ + echo 'autopack: the install step downloaded no browser' >&2; \ + echo 'Searched: {search}' >&2; \ + echo 'If the download was skipped (PUPPETEER_SKIP_DOWNLOAD, \ +PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD), either unset it or drop the dependency.' >&2; \ + exit 1; \ + fi" + ))); step.add_command(Command::shell(record_runtime_libraries( - &browsers.browser_binaries.join(" "), + &format!("$({search})"), RUNTIME_DEPS_FILE, ))); } @@ -523,8 +537,8 @@ struct BrowserTooling { /// are needed, and without them Chromium renders text as empty boxes /// instead of failing in a way anyone would notice. runtime_packages: Vec, - /// Paths to `ldd` for the libraries the browser actually links. - browser_binaries: Vec, + /// Shell expressions that list the browser binaries to inspect. + browser_searches: Vec, /// Environment for the install, build and runtime stages. variables: Vec<(&'static str, String)>, /// Commands the install step runs after dependencies are in place. @@ -569,7 +583,7 @@ impl BrowserTooling { fn browser_tooling(package: &PackageJson) -> BrowserTooling { let mut tooling = BrowserTooling { runtime_packages: Vec::new(), - browser_binaries: Vec::new(), + browser_searches: Vec::new(), variables: Vec::new(), downloads: Vec::new(), }; @@ -600,16 +614,27 @@ fn browser_tooling(package: &PackageJson) -> BrowserTooling { .variables .push(("PUPPETEER_CACHE_DIR", format!("{APP_DIR}/.cache/puppeteer"))); } + // `find` rather than a positional glob. pnpm does not hoist + // `playwright-core` to the top of node_modules — the browsers land under + // `.pnpm/playwright-core@1.62.1/node_modules/...` — so a glob written for + // npm's layout matches nothing there, and matching nothing is silent: the + // recorded list comes out empty and the image ships with no browser + // libraries at all. + // + // Both binaries are collected. Playwright launches the headless shell by + // default and Puppeteer the full browser, they sit in differently named + // directories, and their link sets are not identical. if playwright { - // Playwright installs beside the package; both the full browser and - // the headless shell ship, and they do not link the same set. - tooling.browser_binaries.push(format!( - "{APP_DIR}/node_modules/{PLAYWRIGHT_PACKAGE}/{PLAYWRIGHT_BROWSERS}/*/chrome-linux*/chrome*" + tooling.browser_searches.push(format!( + "find {APP_DIR}/node_modules -path '*/{PLAYWRIGHT_BROWSERS}/*' -type f \\ + \\( -name chrome -o -name chrome-headless-shell -o -name headless_shell \\) \\ + 2>/dev/null" )); } if puppeteer { - tooling.browser_binaries.push(format!( - "{APP_DIR}/.cache/puppeteer/*/*/chrome-linux*/chrome*" + tooling.browser_searches.push(format!( + "find {APP_DIR}/.cache/puppeteer -type f \\ + \\( -name chrome -o -name chrome-headless-shell \\) 2>/dev/null" )); } if playwright || puppeteer { @@ -1020,9 +1045,10 @@ mod tests { .unwrap() .commands .iter() - .any(|c| c.display_name().contains("ldd ") - && c.display_name().contains(".cache/puppeteer") - || c.display_name().contains(PLAYWRIGHT_BROWSERS)), + .any(|c| { + c.display_name().contains("ldd ") + && c.display_name().contains(PLAYWRIGHT_BROWSERS) + }), "install step does not inspect the browser" ); @@ -1162,6 +1188,68 @@ mod tests { .any(|c| c.display_name().contains("playwright install"))); } + #[test] + fn a_missing_browser_fails_the_build_rather_than_the_container() { + // A search that finds nothing is silent: no libraries are recorded, + // the runtime stage skips its install, and the image dies the first + // time it opens a browser. pnpm hits exactly this — it does not hoist + // playwright-core, so a glob written for npm's layout finds nothing. + let (_dir, app) = write_app(&[ + ( + "package.json", + r#"{"dependencies":{"puppeteer":"^24.0.0"},"scripts":{"start":"node s.js"}}"#, + ), + ("package-lock.json", "{}"), + ("s.js", ""), + ]); + let analysis = plan_for(&app); + let install = analysis.plan.step("install").unwrap(); + let names: Vec<_> = install.commands.iter().map(|c| c.display_name()).collect(); + + let guard = names + .iter() + .position(|n| n.contains("downloaded no browser")) + .expect("no guard against a missing browser"); + assert!(names[guard].contains("exit 1")); + assert!(names[guard].contains("SKIP_DOWNLOAD")); + + // The guard has to run before the inspection it protects. + let record = names.iter().position(|n| n.contains("ldd ")).unwrap(); + assert!(guard < record, "{names:?}"); + } + + #[test] + fn browser_discovery_does_not_assume_a_hoisted_layout() { + // pnpm puts the browsers under + // node_modules/.pnpm/playwright-core@1.62.1/node_modules/... — a + // positional glob written for npm finds nothing there, and finding + // nothing produced an image with no browser libraries and a build + // that exited 0. + let (_dir, app) = write_app(&[ + ( + "package.json", + r#"{"dependencies":{"playwright":"^1.62.1"},"scripts":{"start":"node s.js"}}"#, + ), + ("pnpm-lock.yaml", ""), + ("s.js", ""), + ]); + let analysis = plan_for(&app); + let record = analysis + .plan + .step("install") + .unwrap() + .commands + .iter() + .map(|c| c.display_name()) + .find(|n| n.contains("ldd ")) + .expect("no discovery command"); + assert!(record.contains("find "), "{record}"); + // Both binaries: Playwright launches the headless shell by default, + // Puppeteer the full browser, and they do not link the same set. + assert!(record.contains("-name chrome-headless-shell"), "{record}"); + assert!(record.contains("-name chrome "), "{record}"); + } + #[test] fn an_app_without_a_browser_gets_no_browser_environment() { let (_dir, app) = write_app(&[