diff --git a/AGENTS.md b/AGENTS.md index 204ff62a..c79ec1f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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. diff --git a/Makefile b/Makefile index d929e642..8583bab8 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/internal/api/handler/nodescript/join.sh b/internal/api/handler/nodescript/join.sh index e6335b48..ff802b28 100755 --- a/internal/api/handler/nodescript/join.sh +++ b/internal/api/handler/nodescript/join.sh @@ -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 diff --git a/scripts/check-join-script.sh b/scripts/check-join-script.sh new file mode 100755 index 00000000..a321ecb4 --- /dev/null +++ b/scripts/check-join-script.sh @@ -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 <