From 91dda0a5357c21ea43bfb4975f6c45f99bc8ea51 Mon Sep 17 00:00:00 2001 From: Darren Paxton Date: Fri, 2 Oct 2026 11:32:27 -0400 Subject: [PATCH] fix(builder): skip runtime resolution for a forced-native build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RUNTIME=podman in .env breaks every containerized extension build, regardless of extension — reported with a full root-cause trace and a live rebuild confirming both the failure and the fix. build-extension.sh re-invokes itself inside the builder container to do the actual dotnet/npm work. docker-compose.yml's builder service sets DUPLO_BUILD_NATIVE=1 in its environment for exactly this re-entrant call — nesting a container runtime inside the build container isn't supported, and builder_mode() already has a documented branch for it. But builder_dispatch() called runtime_resolve unconditionally, before ever checking DUPLO_BUILD_NATIVE. Since the whole repo (.env included) is bind-mounted into the container at /work, the inner invocation's _envv lookup read RUNTIME=podman straight off the mounted file, runtime_requested() returned non-empty, and the fallback treated that as an explicit pin and fataled — before builder_mode()'s want_native=1 short-circuit ever got evaluated. RUNTIME=docker, the default, usually masks this: docker is typically also on the host's PATH, so the same resolve call that should be skipped happens to succeed by accident. Same latent bug either way. Fix: guard the runtime_resolve call with the already-computed want_native flag, so a forced-native build — which the container always is — skips runtime resolution entirely instead of racing it. No behavior change on the host-side (outer) dispatch, where want_native is unset by default and the existing resolve-or-die logic still runs. Verified both ways: reproduced the exact reported error in isolation against the unpatched code (same message, same exit 1), confirmed the fix resolves it, and confirmed the host-side path — a developer who names a runtime they genuinely don't have — still fatals correctly rather than silently going native. Also verified end-to-end: rebuilding extensions/soc2-posture-extension with RUNTIME=podman set succeeds after the fix, fails identically before it. Tests: 5, new file (none existed for _builder.sh before). Confirmed RED against the unpatched code — exactly the forced-native/unresolvable-RUNTIME case fails, nothing else — then GREEN with the fix. Co-Authored-By: Claude Sonnet 5 --- scripts/_builder.sh | 24 +++++++----- tests/test-builder-dispatch.sh | 68 ++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 9 deletions(-) create mode 100755 tests/test-builder-dispatch.sh diff --git a/scripts/_builder.sh b/scripts/_builder.sh index aae8ea1..944ce5d 100644 --- a/scripts/_builder.sh +++ b/scripts/_builder.sh @@ -438,15 +438,21 @@ builder_dispatch() { # value we actually resolved, which is not necessarily the one in the environment. local want_native; want_native="$(builder_clean_value "${DUPLO_BUILD_NATIVE:-$(_envv DUPLO_BUILD_NATIVE)}")" - # Resolve the CLI before probing. Failure is only fatal when a runtime was NAMED: the caller said - # which one to use, and quietly building with a different one — or dropping to a native build they - # did not ask for — is not what they asked for. Auto-detect finding nothing stays silent, because - # builder_mode's native fallback below still builds on a machine with the full toolchain, and that is - # what keeps a runtime-less CI runner working. The re-run is to print the specific reason, which was - # suppressed on the first call. - runtime_resolve 2>/dev/null || { - if [ -n "$(runtime_requested)" ]; then runtime_resolve; exit 1; fi - } + # Resolve the CLI before probing — but only when native wasn't already forced. want_native=1 outranks + # everything in builder_mode below, so a runtime that fails to resolve at this point (e.g. RUNTIME=podman + # read back out of the bind-mounted .env while running *inside* the builder container, which has no + # reason to carry a podman binary) must not be fatal: nothing downstream is going to use it. + # + # Failure is only fatal when a runtime was NAMED: the caller said which one to use, and quietly building + # with a different one — or dropping to a native build they did not ask for — is not what they asked + # for. Auto-detect finding nothing stays silent, because builder_mode's native fallback below still + # builds on a machine with the full toolchain, and that is what keeps a runtime-less CI runner working. + # The re-run is to print the specific reason, which was suppressed on the first call. + if [ "$want_native" != 1 ]; then + runtime_resolve 2>/dev/null || { + if [ -n "$(runtime_requested)" ]; then runtime_resolve; exit 1; fi + } + fi local mode; mode="$(builder_mode \ "$want_native" \ diff --git a/tests/test-builder-dispatch.sh b/tests/test-builder-dispatch.sh new file mode 100755 index 0000000..85626ca --- /dev/null +++ b/tests/test-builder-dispatch.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env bash +# Checks builder_dispatch's runtime-resolution guard: a forced-native build (DUPLO_BUILD_NATIVE=1, which +# is exactly what the builder container's own compose environment sets — see docker-compose.yml's +# `builder` service) must never fail because of a $RUNTIME value it has no intention of using. +set -uo pipefail +cd "$(dirname "$0")/.." || exit 1 +PASS=0; FAIL=0 +t() { printf ' %s … ' "$1"; } +ok() { echo "ok"; PASS=$((PASS+1)); } +bad() { echo "FAIL: $1"; FAIL=$((FAIL+1)); } + +echo "builder_dispatch native-skip guard:" + +# dispatch_rc +# +# Returns (as its own exit status — never captured via $(), since the code under test calls `exit` +# directly on its failure path, which would terminate a command-substitution subshell before any +# echo-the-exit-code line inside it ever ran) whatever builder_dispatch itself exits with. +# +# `command -v podman` is shadowed to simulate the builder container, which has no reason to carry a +# podman binary even when the bind-mounted .env the re-entrant build reads says RUNTIME=podman. +dispatch_rc() { + ( RV="$1"; NATIVE="$2"; HASPODMAN="$3" + export DUPLO_ENV_FILE=/dev/null + [ -n "$RV" ] && export RUNTIME="$RV" || unset RUNTIME + [ -n "$NATIVE" ] && export DUPLO_BUILD_NATIVE="$NATIVE" || unset DUPLO_BUILD_NATIVE + command() { + if [ "${1-}" = -v ] && [ "${2-}" = podman ] && [ "$HASPODMAN" != 1 ]; then return 1; fi + builtin command "$@" + } + . ./scripts/_runtime.sh + . ./scripts/_builder.sh + builder_probe_in_container() { echo "$NATIVE"; } + builder_probe_toolchain() { echo 1; } + builder_probe_runtime() { echo 0; } + builder_dispatch some-script.sh some-dir + ) >/dev/null 2>&1 +} + +t "a forced-native build is unaffected by a \$RUNTIME it cannot resolve" +# This is the exact failure reported from a real build: the builder container's own compose environment +# sets DUPLO_BUILD_NATIVE=1, but the whole repo — .env included — is bind-mounted into it, so the +# re-entrant build.sh invocation reads RUNTIME=podman straight off the host's real .env even though the +# builder image has no podman binary and never needed one. +dispatch_rc podman 1 0; RC=$? +[ "$RC" = 0 ] && ok || bad "forced-native build still fataled on an unresolvable \$RUNTIME (rc=$RC)" + +t "a forced-native build is unaffected when nothing is on PATH at all" +dispatch_rc "" 1 0; RC=$? +[ "$RC" = 0 ] && ok || bad "rc=$RC" + +t "a HOST build (not forced-native) still fatals when the named runtime cannot be resolved" +# The guard must be scoped to the forced-native case only — a developer on their own machine who pinned +# RUNTIME=podman without installing it still needs the real error, not a silent native fallback they +# never asked for. +dispatch_rc podman "" 0; RC=$? +[ "$RC" != 0 ] && ok || bad "a host build with a genuinely broken \$RUNTIME exited 0 — the guard is too broad" + +t "a HOST build with no \$RUNTIME pinned still resolves normally (auto-detect, not skipped)" +dispatch_rc "" "" 1; RC=$? +[ "$RC" = 0 ] && ok || bad "rc=$RC (auto-detect should have found podman and proceeded)" + +t "bash -n on the touched files" +if bash -n scripts/_builder.sh 2>/dev/null; then ok; else bad "syntax error"; fi + +echo +echo "$PASS passed, $FAIL failed" +[ "$FAIL" -eq 0 ]