fix(10-03): quote the Dockerfile heredoc so comments cannot execute
`cat > "$WORK_DIR/Dockerfile.rootfs" <<DOCKERFILE` was unquoted, so the build shell performed command substitution on the Dockerfile body. Any backtick in a Dockerfile COMMENT was executed on the build host and its output spliced into the generated file. Six comments did this. One of them ran `systemctl start archipelago-fips.service` against the build machine on every ISO build; the others were harmless only by accident of being command-not-found. Fixes the class, not the six instances. The delimiter is now quoted, so the body is emitted verbatim and a future backticked comment is inert. Verified the boundary by line range first: the other backticked comments in this file (:264, :809, :1188, :1289, :1506, :1605, :3597, :3651) are ordinary shell comments outside any unquoted heredoc and were never at risk — they are untouched. The body needs exactly four build-time values and they are all package names (LINUX_IMAGE_PKG, GRUB_EFI_PKG, GRUB_EFI_SIGNED_PKG, GRUB_PC_PKG), on four consecutive lines. So quoting was practical: the heredoc is split into DOCKERFILE_HEAD and DOCKERFILE_TAIL, both quoted, with a single explicit printf interpolating those four names between them. Escapes that existed only because the heredoc was unquoted are undone in the same pass: six trailing `\\` become `\` (Docker line continuations) and four `\$` become `$` (RUN arguments reach the shell verbatim — Docker does not substitute variables in RUN). Verified by rendering the generated Dockerfile before and after with the same inputs and diffing them normalised (continuations joined, whitespace collapsed). Both are 190 normalised lines and the ONLY differences are the six comments regaining their text — every instruction is byte-identical. Before: "# the archipelago backend calls" / after: "# the archipelago backend calls `systemctl start archipelago-fips.service`". Test: case 7 asserts every heredoc writing Dockerfile.rootfs has a quoted delimiter, and when one is not, reports which body lines would execute. The assertion is on the delimiter, not on backticks — with quoting a backticked comment is legal and six of them are back in the body on purpose, so flagging backticks would flag a non-bug and fail on the very comments this restored. This bug is invisible to `bash -n`; an instance of it introduced earlier in this plan hung a syntactically-clean build for two minutes before being caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2efab5f219
commit
d9b3a7d5e0
@@ -268,7 +268,23 @@ if [ ! -f "$ROOTFS_TAR" ] || [ "${1:-}" == "--rebuild" ] || [ "$(cat "$ROOTFS_ST
|
||||
echo " Using Docker to create Debian root filesystem..."
|
||||
|
||||
# Create a Dockerfile for building the rootfs
|
||||
cat > "$WORK_DIR/Dockerfile.rootfs" <<DOCKERFILE
|
||||
# The Dockerfile body is written with a QUOTED heredoc delimiter.
|
||||
#
|
||||
# It used to be unquoted, which meant the build shell performed command
|
||||
# substitution on the body: any backtick in a Dockerfile COMMENT was executed
|
||||
# on the build host and its output spliced into the Dockerfile. Six comments
|
||||
# did that, and one of them ran "systemctl start archipelago-fips.service"
|
||||
# against the build machine on every ISO build. The comment text was also
|
||||
# silently deleted from the generated Dockerfile.
|
||||
#
|
||||
# bash -n cannot see this class of bug — the script is syntactically perfect
|
||||
# either way — so quoting the delimiter is the fix, not vigilance about
|
||||
# backticks. tests/first-boot-secrets/run-tests.sh case 7 fails if the
|
||||
# delimiter is ever unquoted again, or if a substitution appears in the body.
|
||||
#
|
||||
# The whole body needs exactly four build-time values, all package names, and
|
||||
# they are interpolated explicitly by the printf between the two halves.
|
||||
cat > "$WORK_DIR/Dockerfile.rootfs" <<'DOCKERFILE_HEAD'
|
||||
# ─── Stage 1: Build the FIPS mesh daemon .deb at a pinned tag ────────────
|
||||
#
|
||||
# FIPS (github.com/jmcorgan/fips) is a fast Nostr-keyed mesh routing
|
||||
@@ -288,14 +304,14 @@ ENV DEBIAN_FRONTEND=noninteractive
|
||||
# - libnftnl-dev, libmnl-dev, clang, libclang-dev: rustables →
|
||||
# bindgen (the gateway feature enables rustables for nftables
|
||||
# integration). bindgen panics without libclang.so.
|
||||
RUN apt-get update && apt-get install -y --no-install-recommends \\
|
||||
git ca-certificates build-essential pkg-config dpkg-dev \\
|
||||
libdbus-1-dev libssl-dev \\
|
||||
clang libclang-dev libnftnl-dev libmnl-dev \\
|
||||
RUN apt-get update && apt-get install -y --no-install-recommends \
|
||||
git ca-certificates build-essential pkg-config dpkg-dev \
|
||||
libdbus-1-dev libssl-dev \
|
||||
clang libclang-dev libnftnl-dev libmnl-dev \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
RUN cargo install --locked cargo-deb
|
||||
ARG FIPS_VERSION=v0.4.1
|
||||
RUN git clone --depth 1 --branch "\$FIPS_VERSION" \\
|
||||
RUN git clone --depth 1 --branch "$FIPS_VERSION" \
|
||||
https://github.com/jmcorgan/fips.git /src/fips
|
||||
WORKDIR /src/fips
|
||||
# fips-gateway is gated behind the `gateway` Cargo feature (depends on
|
||||
@@ -328,10 +344,15 @@ RUN echo "deb http://deb.debian.org/debian trixie main non-free-firmware" > /etc
|
||||
|
||||
# Install all packages we need including nginx, podman, tor, and openssl (for self-signed certs)
|
||||
RUN apt-get update && apt-get -y full-upgrade && apt-get install -y --no-install-recommends \
|
||||
${LINUX_IMAGE_PKG} \
|
||||
${GRUB_EFI_PKG} \
|
||||
${GRUB_EFI_SIGNED_PKG} \
|
||||
${GRUB_PC_PKG} \
|
||||
DOCKERFILE_HEAD
|
||||
|
||||
# The ONLY build-time interpolation in the entire Dockerfile: the kernel and
|
||||
# GRUB package names, which vary by architecture and Debian suite.
|
||||
printf ' %s \\\n' \
|
||||
"$LINUX_IMAGE_PKG" "$GRUB_EFI_PKG" "$GRUB_EFI_SIGNED_PKG" "$GRUB_PC_PKG" \
|
||||
>> "$WORK_DIR/Dockerfile.rootfs"
|
||||
|
||||
cat >> "$WORK_DIR/Dockerfile.rootfs" <<'DOCKERFILE_TAIL'
|
||||
systemd \
|
||||
systemd-sysv \
|
||||
dbus \
|
||||
@@ -438,7 +459,7 @@ RUN useradd -m -s /bin/bash -G sudo,dialout,audio archipelago && \
|
||||
echo "root:archipelago" | chpasswd && \
|
||||
echo "archipelago ALL=(ALL) NOPASSWD:ALL" > /etc/sudoers.d/archipelago
|
||||
# Verify password hash was set (not locked)
|
||||
RUN grep -q "^archipelago:\$" /etc/shadow && echo "Password set OK" || echo "WARNING: password may not be set"
|
||||
RUN grep -q "^archipelago:$" /etc/shadow && echo "Password set OK" || echo "WARNING: password may not be set"
|
||||
|
||||
# Set hostname
|
||||
RUN echo "archipelago" > /etc/hostname
|
||||
@@ -482,8 +503,8 @@ RUN mkdir -p /etc/archipelago/ssl
|
||||
# anyone edits that package list.
|
||||
RUN set -e; \
|
||||
for bin in /usr/bin/openssl /usr/bin/ssh-keygen; do \
|
||||
if [ ! -x "\$bin" ]; then \
|
||||
echo "FATAL: \$bin missing or not executable in the rootfs." >&2; \
|
||||
if [ ! -x "$bin" ]; then \
|
||||
echo "FATAL: $bin missing or not executable in the rootfs." >&2; \
|
||||
echo "first-boot-secrets.sh cannot generate per-device SSH host keys" >&2; \
|
||||
echo "or the TLS keypair without it, and that failure is permanent." >&2; \
|
||||
echo "Restore openssl / openssh-server in the package list above." >&2; \
|
||||
@@ -572,7 +593,7 @@ RUN systemctl enable archipelago-fips.service || true
|
||||
# (env file doesn't exist until onboarding) so we mask it outright.
|
||||
# `systemctl mask` alone doesn't stick because the real .service file is
|
||||
# already in place — explicit rm + /dev/null symlink is what sticks.
|
||||
RUN rm -f /etc/systemd/system/nostr-vpn.service && \\
|
||||
RUN rm -f /etc/systemd/system/nostr-vpn.service && \
|
||||
ln -sf /dev/null /etc/systemd/system/nostr-vpn.service
|
||||
|
||||
# Remove policy-rc.d so services can start on first boot
|
||||
@@ -640,7 +661,7 @@ RUN rm -f /etc/ssh/ssh_host_* && \
|
||||
{ [ -L /var/lib/dbus/machine-id ] || rm -f /var/lib/dbus/machine-id ; } && \
|
||||
mkdir -p /opt/archipelago && \
|
||||
printf 'F-03 identity strip: this rootfs was built with the identity-strip layer.\nRemoved:\n /etc/ssh/ssh_host_*\n /etc/archipelago/ssl/archipelago.key\n /etc/archipelago/ssl/archipelago.crt\nTruncated:\n /etc/machine-id\nRecreated per device by archipelago-first-boot-secrets.service on first boot.\n' > /opt/archipelago/rootfs-identity-stripped
|
||||
DOCKERFILE
|
||||
DOCKERFILE_TAIL
|
||||
|
||||
# Copy nginx snippets for HTTPS (PWA, app proxies)
|
||||
if [ -d "$SCRIPT_DIR/../configs/snippets" ]; then
|
||||
|
||||
@@ -364,6 +364,56 @@ else
|
||||
echo " generator heredoc spans lines $SS_START-$SS_END of $BUILDER"
|
||||
fi
|
||||
|
||||
# ── Case 7: the Dockerfile heredoc delimiter must be quoted ──────────────
|
||||
# Lives in this harness rather than a sibling because it guards the same file
|
||||
# and the same failure mode the rest of these cases exist for: a build-side
|
||||
# defect that is invisible to `bash -n` and only shows up as damage on a build
|
||||
# host. Splitting it into its own runner would mean two commands to remember
|
||||
# and one of them getting skipped.
|
||||
#
|
||||
# The bug: `cat > ... <<DOCKERFILE` (unquoted) makes the build shell perform
|
||||
# command substitution on the Dockerfile body, so a backtick inside a COMMENT
|
||||
# is executed on the build host and its output spliced into the Dockerfile.
|
||||
# Six comments did exactly that, and one of them ran `systemctl start
|
||||
# archipelago-fips.service` against the build machine on every ISO build. The
|
||||
# comment text was silently deleted from the generated Dockerfile too.
|
||||
#
|
||||
# The assertion is on the DELIMITER, not on backticks. With a quoted delimiter
|
||||
# a backticked comment is inert and perfectly legal — six of them are back in
|
||||
# the body on purpose. Flagging backticks would be flagging a non-bug, and
|
||||
# would fail on the very comments this fix restored. Quoting is the fix;
|
||||
# vigilance about backticks is not.
|
||||
c7=""
|
||||
DF_HEREDOCS=$(grep -nE 'cat >>? "\$WORK_DIR/Dockerfile\.rootfs" <<' "$BUILDER" || true)
|
||||
if [ -z "$DF_HEREDOCS" ]; then
|
||||
c7="$c7 no-dockerfile-heredoc-found"
|
||||
else
|
||||
while IFS= read -r hd; do
|
||||
[ -z "$hd" ] && continue
|
||||
ln=${hd%%:*}
|
||||
delim=$(printf '%s' "$hd" | sed -E 's/.*<<-?[[:space:]]*//')
|
||||
case "$delim" in
|
||||
\'*\'|\"*\")
|
||||
: ;; # quoted — the body is emitted verbatim, nothing executes
|
||||
*)
|
||||
c7="$c7 UNQUOTED-DELIMITER-at-line-$ln"
|
||||
# Only meaningful when unquoted: report what would actually run.
|
||||
bare=$(printf '%s' "$delim" | tr -d "\"'")
|
||||
endln=$(awk -v s="$ln" -v d="$bare" 'NR>s && $0==d { print NR; exit }' "$BUILDER")
|
||||
if [ -n "$endln" ]; then
|
||||
subs=$(awk -v s="$ln" -v e="$endln" 'NR>s && NR<e && (/`/ || /\$\(/) { print NR }' "$BUILDER" | tr '\n' ',')
|
||||
[ -n "$subs" ] && c7="$c7 would-execute-at-lines:${subs%,}"
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
done <<< "$DF_HEREDOCS"
|
||||
fi
|
||||
if [ -z "$c7" ]; then
|
||||
ok "Dockerfile heredoc delimiters are quoted — a backticked comment cannot execute"
|
||||
else
|
||||
bad "Dockerfile heredoc quoting ->$c7"
|
||||
fi
|
||||
|
||||
# ── Summary ───────────────────────────────────────────────────────────────
|
||||
echo
|
||||
echo "──────── first-boot-secrets summary ────────"
|
||||
|
||||
Reference in New Issue
Block a user