From db8bd27e42dd3d8d0dde563d5cba392a928f23e8 Mon Sep 17 00:00:00 2001 From: Vijit Singh Date: Thu, 4 Jun 2026 09:00:30 -0500 Subject: [PATCH] pithead: validate dashboard.host + resolve reset-dashboard targets from .env (#130, #139) #130: dashboard.host is rendered verbatim into the Caddyfile site address but was never validated. A value with a space, newline, or `{`/`}` would break the Caddyfile (or inject directives). Add an is_valid_host check in parse_and_validate_config (hostname/IP chars only), mirroring the stratum_bind validation. #139: reset-dashboard wiped the dirs from a fresh config.json parse, so editing a *.data_dir before reset (without an apply) could `rm -rf` a directory the running stack never used. Resolve the delete targets from .env (the live deployment) instead, assert_safe_dir them, and refuse to run rather than guess if .env doesn't name them. Tests: is_valid_host unit; bad-dashboard.host rejected; reset targets the .env dir (not a config-only path) and refuses without .env dirs. Suite 111 passed, shellcheck clean. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 7 +++++++ pithead | 39 +++++++++++++++++++++++++++++------- tests/stack/run.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c082320..0193076 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,10 @@ per the process in [`docs/releasing.md`](docs/releasing.md). a space check" fallback was previously unreachable under `set -e`) (#127). - `pithead doctor` now exits non-zero when a critical check FAILS, so it can be used as a cron/CI/monitoring health gate (it previously always exited 0); warnings alone still exit 0 (#127). +- `pithead reset-dashboard` now resolves the data directories it wipes from `.env` (the live + deployment) instead of re-reading `config.json` — editing a `*.data_dir` in `config.json` + before resetting (without an `apply`) can no longer wipe a directory the stack never used. It + also refuses to run rather than guess if `.env` doesn't name them (#139). ### Security @@ -90,3 +94,6 @@ per the process in [`docs/releasing.md`](docs/releasing.md). - All externally-pulled base/runtime images are now pinned by immutable `@sha256` digest (caddy, docker-socket-proxy, the Tari node, and the `ubuntu`/`python`/`alpine` build bases), so a re-pushed tag or a registry MITM can't silently change the running image (#135). +- `dashboard.host` is now validated (hostname/IP characters only) before it's rendered into the + Caddyfile, so a value containing whitespace, a newline, or `{`/`}` can no longer break the + Caddyfile or inject reverse-proxy directives — mirroring the `stratum_bind` validation (#130). diff --git a/pithead b/pithead index 6541f83..59ad23b 100755 --- a/pithead +++ b/pithead @@ -430,20 +430,31 @@ reset_dashboard() { fi log "Resetting dashboard and p2pool..." - parse_and_validate_config + # Resolve what to delete from .env (the LIVE deployment's data dirs), NOT a fresh config.json + # parse — otherwise editing a *.data_dir in config.json before `reset-dashboard` (without an + # `apply`) would wipe a directory the running stack never used, possibly one the user just + # pointed at. Refuse to guess if .env doesn't name them, and re-check they're safe to rm (#139). + local dashboard_dir p2pool_dir + dashboard_dir=$(env_get DASHBOARD_DATA_DIR) + p2pool_dir=$(env_get P2POOL_DATA_DIR) + if [ -z "$dashboard_dir" ] || [ -z "$p2pool_dir" ]; then + error "Could not read DASHBOARD_DATA_DIR / P2POOL_DATA_DIR from .env — refusing to guess what to delete. Run '$0 setup' or '$0 apply' first." + fi + assert_safe_dir "$dashboard_dir" + assert_safe_dir "$p2pool_dir" log "Stopping dashboard and p2pool containers..." docker compose rm -s -f -v dashboard p2pool log "Removing data directories..." - [ -d "$DASHBOARD_DIR" ] && sudo rm -rf "$DASHBOARD_DIR" - [ -d "$P2POOL_DIR" ] && sudo rm -rf "$P2POOL_DIR" + [ -d "$dashboard_dir" ] && sudo rm -rf "$dashboard_dir" + [ -d "$p2pool_dir" ] && sudo rm -rf "$p2pool_dir" log "Recreating data directories..." - mkdir -p "$DASHBOARD_DIR" "$P2POOL_DIR" - sudo chown -R "$REAL_USER":"$REAL_USER" "$P2POOL_DIR" - mkdir -p "$P2POOL_DIR/stats" - sudo chmod -R 755 "$P2POOL_DIR/stats" + mkdir -p "$dashboard_dir" "$p2pool_dir" + sudo chown -R "$REAL_USER":"$REAL_USER" "$p2pool_dir" + mkdir -p "$p2pool_dir/stats" + sudo chmod -R 755 "$p2pool_dir/stats" log "Bringing services back up..." docker compose up -d dashboard p2pool @@ -766,6 +777,14 @@ is_ipv4() { return 0 } +# True if $1 is a plausible hostname or IP literal — i.e. only the characters a host can contain +# (letters, digits, dot, hyphen, colon for IPv6). Used to validate dashboard.host before it's +# rendered into the Caddyfile site address: anything with whitespace, a newline, or Caddy-significant +# characters ({ } /) could break or inject into the generated Caddyfile. +is_valid_host() { + [[ "$1" =~ ^[A-Za-z0-9.:_-]+$ ]] +} + # Best-effort detection of the host's IANA timezone (Linux + macOS), used as the dashboard # default when dashboard.timezone is "auto"/unset. Falls back to Etc/UTC if it can't tell. detect_host_timezone() { @@ -1097,6 +1116,12 @@ parse_and_validate_config() { # "auto"/empty/DYNAMIC_HOST here means "decide later" (preserved value, prompt, or hostname) DASHBOARD_HOST=$(resolve_default "$(jq -r '.dashboard.host // empty' "$CONFIG_FILE")" "") + # A non-"auto" host is rendered verbatim into the Caddyfile site address (generate_caddyfile), + # so reject anything that isn't a bare hostname/IP — a space, newline, or `{`/`}` would break + # the Caddyfile (or inject directives). Mirrors the stratum_bind validation above (#130). + if [ -n "$DASHBOARD_HOST" ] && ! is_valid_host "$DASHBOARD_HOST"; then + error "dashboard.host must be a hostname or IP address — only letters, digits, dots, hyphens, and colons (got \"$DASHBOARD_HOST\")." + fi # Ensure a strict true/false string is returned, defaulting to true DASHBOARD_SECURE=$(jq -r 'if .dashboard.secure != null then .dashboard.secure | tostring else "true" end' "$CONFIG_FILE") # Timezone for the dashboard's timestamps/charts. "auto"/empty -> the host's timezone diff --git a/tests/stack/run.sh b/tests/stack/run.sh index ce9d67c..88216ae 100755 --- a/tests/stack/run.sh +++ b/tests/stack/run.sh @@ -90,6 +90,15 @@ PATH="$BOOT/bin:$PATH" FAKE_BOOT=service run_sourced "$SANDBOX" docker_boot_enab PATH="$BOOT/bin:$PATH" FAKE_BOOT=socket run_sourced "$SANDBOX" docker_boot_enabled; assert_rc "docker.socket enabled -> 0" "$?" "0" PATH="$BOOT/bin:$PATH" FAKE_BOOT=none run_sourced "$SANDBOX" docker_boot_enabled; assert_rc "neither enabled -> 1" "$?" "1" +echo "== unit: is_valid_host (#130) ==" +run_sourced "$SANDBOX" is_valid_host "box.lan" >/dev/null 2>&1; assert_rc "accepts hostname" "$?" "0" +run_sourced "$SANDBOX" is_valid_host "192.168.1.10" >/dev/null 2>&1; assert_rc "accepts IPv4" "$?" "0" +run_sourced "$SANDBOX" is_valid_host "fe80::1" >/dev/null 2>&1; assert_rc "accepts IPv6" "$?" "0" +run_sourced "$SANDBOX" is_valid_host "bad host" >/dev/null 2>&1; assert_rc "rejects space" "$?" "1" +run_sourced "$SANDBOX" is_valid_host 'evil{block}' >/dev/null 2>&1; assert_rc "rejects braces" "$?" "1" +run_sourced "$SANDBOX" is_valid_host "a/b" >/dev/null 2>&1; assert_rc "rejects slash" "$?" "1" +run_sourced "$SANDBOX" is_valid_host "" >/dev/null 2>&1; assert_rc "rejects empty" "$?" "1" + echo "== unit: describe_change ==" assert_contains "prune is DEST" "$(run_sourced "$SANDBOX" describe_change MONERO_PRUNE 1 0)" "DEST" assert_contains "rpc lan is DEST" "$(run_sourced "$SANDBOX" describe_change MONERO_RPC_BIND 127.0.0.1 0.0.0.0)" "DEST" @@ -234,6 +243,13 @@ out="$(cd "$V" && PATH="$V/bin:$PATH" ./pithead apply -y 2>&1)"; rc=$? assert_rc "invalid stratum_bind rejected" "$rc" "1" assert_contains "invalid stratum_bind message" "$out" "p2pool.stratum_bind" +# A dashboard.host with Caddyfile-breaking characters (space/braces) must be rejected before render. +seed_env +printf '{ "monero": {"mode":"local","wallet_address":"%s","node_username":"u","node_password":"p"}, "tari":{"wallet_address":"T"}, "p2pool":{"pool":"main"}, "dashboard":{"secure":true,"host":"bad host{x}"} }\n' "$WALLET" > "$V/config.json" +out="$(cd "$V" && PATH="$V/bin:$PATH" ./pithead apply -y 2>&1)"; rc=$? +assert_rc "invalid dashboard.host rejected" "$rc" "1" +assert_contains "invalid dashboard.host message" "$out" "dashboard.host" + echo "== black-box: apply preserves secrets + propagates ==" seed_env printf '{ "monero": {"mode":"local","wallet_address":"%s","node_username":"u","node_password":"p"}, "tari":{"wallet_address":"T"}, "p2pool":{"pool":"mini"}, "dashboard":{"secure":false,"host":"box.lan"} }\n' "$WALLET" > "$V/config.json" @@ -386,6 +402,40 @@ assert_contains "doctor runs to the summary" "$out" "Diagnostics summar assert_contains "doctor flags the unreachable daemon" "$out" "Docker daemon is not reachable" assert_rc "doctor exits 1 on a critical FAIL" "$rc" "1" +echo "== black-box: reset-dashboard targets .env dirs, not config.json (#139) ==" +# reset-dashboard must wipe the LIVE deployment's data dirs (from .env), not a path the user may +# have edited into config.json without applying. docker = noop; sudo only LOGS (never executes the +# rm), so we can assert what it would have targeted without deleting anything. +R="$SANDBOX/reset"; mkdir -p "$R/bin" "$R/envdir/dashboard" "$R/envdir/p2pool"; cp "$STACK" "$R/pithead" +printf '#!/usr/bin/env bash\nexit 0\n' > "$R/bin/docker" +cat > "$R/bin/sudo" <<'EOF' +#!/usr/bin/env bash +echo "[sudo] $*" >> "${SUDO_LOG:-/dev/null}" +exit 0 +EOF +chmod +x "$R/bin/docker" "$R/bin/sudo" +cat > "$R/.env" < "$R/config.json" +SUDO_LOG="$R/sudo.log"; : > "$SUDO_LOG" +out="$(cd "$R" && SUDO_LOG="$SUDO_LOG" PATH="$R/bin:$PATH" ./pithead reset-dashboard -y 2>&1)"; rc=$? +assert_rc "reset-dashboard succeeds" "$rc" "0" +sudo_calls="$(cat "$SUDO_LOG")" +assert_contains "reset rm targets the .env dashboard dir" "$sudo_calls" "rm -rf $R/envdir/dashboard" +case "$sudo_calls" in *CONFIGONLY*) bad "reset must ignore the config-only data_dir" "$sudo_calls" ;; *) ok "reset ignores the config-only data_dir" ;; esac + +echo "== black-box: reset-dashboard refuses to guess without .env dirs (#139) ==" +printf 'DEPLOYMENT_COMPLETED=true\nCOMPOSE_PROFILES=local_node\nHOST_IP=box.lan\n' > "$R/.env" +out="$(cd "$R" && SUDO_LOG=/dev/null PATH="$R/bin:$PATH" ./pithead reset-dashboard -y 2>&1)"; rc=$? +assert_rc "reset refuses with no data dirs in .env" "$rc" "1" +assert_contains "reset refuse message" "$out" "refusing to guess" + # --------------------------------------------------------------------------- echo "" printf 'pithead tests: \033[1;32m%d passed\033[0m, ' "$PASS"