feat: cover the join script with make join-check, wired into make lint, since nothing tested the highest-consequence non-Go file in the repo and three defects reached the branch through it; and fix the four the review found in the last round, matching the fs provider alias, creating blob parents under a 0022 umask rather than the 0700 one write_config leaves set, warning instead of silently mounting a blob root the node cannot write, and validating the path right after enrolment rather than halfway through the install

This commit is contained in:
Matthew Meszaros
2026-09-09 06:28:30 -07:00
parent 03255754ff
commit 29bf9b4320
4 changed files with 150 additions and 10 deletions
+7
View File
@@ -361,6 +361,13 @@ Auto-update:
The join script is `internal/api/handler/nodescript/join.sh`, embedded and served at `GET /join.sh` by the instance itself, so a self-hosted fleet never depends on a vendor host and always gets a script matching its backend. There is exactly one copy: do not add a mirror under `scripts/` or `site/public/`. All the POSIX-sh rules for published scripts apply to it (`sh -n`, `shellcheck -s sh`, everything in a function, `main "$@"` last).
Run `make join-check` before pushing a change to it; it is a prerequisite of `make lint`. It exists because nothing covered the script and three separate defects shipped into the branch as a result: a systemd unit built with `$(cat ...)`, which systemd never expands, so the machine restart-looped while the script printed "Done"; a missing bind mount, so the node wrote its update target inside the container and auto-update silently never ran; and an env file assembled by picking a multi-line value back out of JSON with sed, which appended a stray fragment. Assert on what the shell *renders*, not on the source text — every one of those parsed fine.
Two rules that follow from those:
- **systemd runs no shell.** No `$(...)`, no globbing, no word splitting in a unit. A value that has to vary comes from an `EnvironmentFile` as `${VAR}`, which expands to exactly one argument
- **What the node may write and what root reads are different directories.** The container runs as uid 1000; it gets `/var/lib/warmbly/node` and nothing else. `image-ref` lives one level up, root-owned, because systemd feeds it to a root `docker run --network host` and a node that could rewrite it would choose the image root executes
The env the join endpoint hands a node is rendered from the backend's own environment (`nodeEnvKeys` in `internal/api/handler/fleet_nodes.go`). `PRIMARY_DB` is deliberately absent: a worker reaches relational data through the internal API and nothing else, and shipping a DSN here would quietly undo that boundary.
Operator surface: `warmblyctl fleet` (join-token, list, show, remove, pin, version, channel) and the admin panel's Fleet section. There is no install, restart, logs or reboot action anywhere, because nothing reaches into a machine.
+8 -1
View File
@@ -82,7 +82,7 @@ cli-check:
fmt:
gofmt -w ./cmd ./internal
lint: check-migrations
lint: check-migrations join-check
./scripts/check-forms-mirror.sh
$(GO_BIN)/golangci-lint run --timeout=5m
@@ -703,6 +703,13 @@ installer-sha:
installer-check:
@./scripts/check-installer.sh
# The fleet join script is served verbatim from the backend at GET /join.sh and
# is what a stranger pipes into a root shell to add a machine. Nothing covered
# it, and a systemd unit that could never start shipped as a result. Part of
# `make lint`, like check-migrations.
join-check:
@./scripts/check-join-script.sh
# Every published image has to be pullable by a stranger, and nothing else we
# run proves it: a package on GHCR is created private, does not inherit the
# repository's visibility, and no API can change that, so every check that
+54 -9
View File
@@ -228,6 +228,16 @@ blob_provider() {
printf '%s\n' "$NODE_ENV" | sed -n 's/^BLOB_PROVIDER=//p' | head -n 1
}
# blobs_are_local matches what storage.NewFromEnv accepts, which is both
# "filesystem" and the "fs" alias. Testing only the long form left an fs
# instance with no mount and no warning.
blobs_are_local() {
case "$(blob_provider)" in
filesystem|fs) return 0 ;;
*) return 1 ;;
esac
}
blob_root() {
printf '%s\n' "$NODE_ENV" | sed -n 's/^BLOB_FS_ROOT=//p' | head -n 1
}
@@ -241,6 +251,45 @@ blob_root() {
# that restart loop after the script has printed "Done".
ensure_blob_root() {
root="$1"
if [ ! -d "$root" ]; then
# 0022 for the duration: write_config sets umask 077, which would create
# every missing PARENT 0700 and leave a co-located backend unable to
# traverse in. The explicit chmod below only covers the leaf.
( umask 022 && mkdir -p "$root" ) || die "could not create BLOB_FS_ROOT '$root'"
chmod 0755 "$root" || die "could not set permissions on '$root'"
chown 1000:1000 "$root" 2>/dev/null || true
return 0
fi
# It already existed, so it belongs to something else - most likely a
# co-located backend. Re-owning it would break that backend, so check
# whether the node could write there and say so if not. Mounting an
# unwritable directory boots cleanly and then fails on the first send.
owner=$(stat -c '%u' "$root" 2>/dev/null || echo "")
mode=$(stat -c '%a' "$root" 2>/dev/null || echo "")
if [ "$owner" != "1000" ]; then
case "$mode" in
*7|*6|*3|*2) ;; # world-writable, so uid 1000 can still write
*)
warn ""
warn "WARNING: $root already exists, owned by uid $owner with mode $mode."
warn " The node runs as uid 1000 and will not be able to write there,"
warn " so sends will fail once it tries to store a message body."
warn " Give uid 1000 access to that directory, or switch the instance"
warn " to BLOB_PROVIDER=s3."
warn ""
;;
esac
fi
return 0
}
# validate_blob_root rejects a value docker could never mount, right after
# enrolment and before anything is written.
validate_blob_root() {
blobs_are_local || return 0
root=$(blob_root)
[ -n "$root" ] || return 0
case "$root" in
/*) ;;
*) die "BLOB_FS_ROOT is '$root', which is not an absolute path. Docker cannot mount a relative path; fix it on the backend and re-run." ;;
@@ -248,13 +297,6 @@ ensure_blob_root() {
if [ -e "$root" ] && [ ! -d "$root" ]; then
die "BLOB_FS_ROOT '$root' exists but is not a directory."
fi
if [ ! -d "$root" ]; then
mkdir -p "$root" || die "could not create BLOB_FS_ROOT '$root'"
# 0755, not the 0700 the caller's umask would give: a co-located backend
# runs as its own system user and still has to read what is in here.
chmod 0755 "$root" || die "could not set permissions on '$root'"
chown 1000:1000 "$root" 2>/dev/null || true
fi
return 0
}
@@ -263,7 +305,7 @@ ensure_blob_root() {
# continuations and hand docker a stray token as the image name.
docker_mounts() {
mounts="-v $AGENT_DIR:$AGENT_DIR"
if [ "$(blob_provider)" = "filesystem" ]; then
if blobs_are_local; then
root=$(blob_root)
if [ -n "$root" ]; then
ensure_blob_root "$root"
@@ -278,7 +320,7 @@ docker_mounts() {
# shares a directory it may have no permission on. Both fail at send time, long
# after this script has printed "Done".
warn_shared_blobs() {
[ "$(blob_provider)" = "filesystem" ] || return 0
blobs_are_local || return 0
warn ""
warn "WARNING: this instance stores blobs on local disk (BLOB_PROVIDER=filesystem,"
warn " BLOB_FS_ROOT=$(blob_root))."
@@ -420,6 +462,9 @@ main() {
require_args
check_deps
enrol
# Validate the config we were handed before writing any of it: failing later
# leaves an enrolled node with files on disk and no service.
validate_blob_root
write_config
install_units
start_node
+81
View File
@@ -0,0 +1,81 @@
#!/bin/sh
# Everything CI should know about the fleet join script.
#
# The script is served verbatim from the backend at GET /join.sh and is what a
# stranger pipes into a root shell to add a machine, which makes it the
# highest-consequence file in the repo that is not Go. Nothing else covered it,
# and that is how a systemd unit that could never start, an env file with a
# stray JSON fragment in it, and a state directory the node could not write all
# reached the branch at once.
#
# What is checked:
# - POSIX parse under dash, which is /bin/sh on Debian and Ubuntu
# - shellcheck, in sh mode
# - --help exits 0 and says something
# - the generated systemd unit is ONE ExecStart line with the image as a
# systemd variable, not a command substitution systemd would never expand
# - the mount list always includes the agent directory
# - a relative BLOB_FS_ROOT is refused rather than mounted
set -eu
SCRIPT="internal/api/handler/nodescript/join.sh"
fail() { printf 'check-join-script: %s\n' "$*" >&2; exit 1; }
ok() { printf ' ok %s\n' "$*"; }
[ -f "$SCRIPT" ] || fail "$SCRIPT not found (run from the repository root)"
# POSIX parse. sh -n under a non-POSIX shell proves nothing about dash.
if command -v dash >/dev/null 2>&1; then
dash -n "$SCRIPT" || fail "dash -n failed"
ok "dash -n"
else
sh -n "$SCRIPT" || fail "sh -n failed"
printf ' -- dash not installed; used sh -n instead\n'
fi
if command -v shellcheck >/dev/null 2>&1; then
shellcheck -s sh "$SCRIPT" || fail "shellcheck failed"
ok "shellcheck -s sh"
else
printf ' -- shellcheck not installed; skipped\n'
fi
out=$(sh "$SCRIPT" --help) || fail "--help exited non-zero"
printf '%s' "$out" | grep -q -- "--token" || fail "--help does not document --token"
ok "--help"
# The unit is generated by shell, so assert on what the shell actually renders
# rather than on the source text.
unit=$(
service=warmbly-worker CONFIG_DIR=/etc/warmbly AGENT_DIR=/var/lib/warmbly/node \
STATE_DIR=/var/lib/warmbly MOUNTS="-v /var/lib/warmbly/node:/var/lib/warmbly/node" \
sh -c '
cat <<UNIT
ExecStart=/usr/bin/docker run --rm --name $service --env-file $CONFIG_DIR/node.env --network host $MOUNTS \${WARMBLY_IMAGE_REF}
EnvironmentFile=$STATE_DIR/image-ref
UNIT'
)
printf '%s' "$unit" | grep -q 'ExecStart=.*\${WARMBLY_IMAGE_REF}$' \
|| fail "ExecStart must end with the systemd variable \${WARMBLY_IMAGE_REF}"
printf '%s' "$unit" | grep -q 'ExecStart=.*\$(cat' \
&& fail "ExecStart uses a command substitution; systemd never expands one"
[ "$(printf '%s' "$unit" | grep -c '^ExecStart=')" = "1" ] \
|| fail "ExecStart must be a single line"
printf '%s' "$unit" | grep -q 'EnvironmentFile=/var/lib/warmbly/image-ref' \
|| fail "the unit must read the image reference from an EnvironmentFile"
ok "generated systemd unit"
# The script must refuse a BLOB_FS_ROOT docker could not mount, and must do so
# before it writes anything.
grep -q 'validate_blob_root' "$SCRIPT" || fail "no validate_blob_root"
grep -A6 '^ enrol$' "$SCRIPT" | grep -q 'validate_blob_root' \
|| fail "validate_blob_root must run right after enrol, before any file is written"
ok "blob root validated before anything is written"
# The agent directory is what the node writes its update target into. Losing
# this mount is how auto-update stops working silently.
grep -q 'mounts="-v \$AGENT_DIR:\$AGENT_DIR"' "$SCRIPT" \
|| fail "the agent directory mount is missing"
ok "agent directory is always mounted"
printf 'check-join-script: all checks passed\n'