From 3c25f097fabd0b36f5e02981cd1262915702fa3d Mon Sep 17 00:00:00 2001 From: boban Date: Sat, 4 Jul 2026 10:33:16 +0200 Subject: [PATCH] fix(security): close the TOCTOU in the symlink-safe status write (Codex review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first hardening pass created the update-phase.json temp with mktemp INSIDE ./run (the container-writable bind mount) and reopened it by pathname (`> "$tmp"`). A container racing the root writer could unlink that temp and drop a symlink in its place before the reopen, redirecting the root write outside ./run. All THREE status-writers now build the temp at the repo ROOT — same filesystem as ./run (so the follow-up mv -f is an atomic, symlink-safe rename(2)) but NOT mounted into any container, so it cannot be tampered mid-write: watch.sh run_write_json, update.sh set_stage, AND install.sh set_stage (reachable as root directly and via update.sh -> exec install.sh). The transient /.phase.* temps are gitignored. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 3 +++ docker/restart-sentinel/watch.sh | 15 ++++++++------- install.sh | 12 ++++++++++-- update.sh | 10 ++++++---- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index 0920a14..12ef133 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,9 @@ yarn-error.log # host-side update transcript (written at the repo root — NOT under ./run — so a compromised # container cannot symlink-redirect the root updater's tee; see docker/restart-sentinel/watch.sh) /update.log +# transient status temp files: created at the repo root then atomically renamed into ./run (same fs, +# container-inaccessible) so the root write can't be symlink-redirected — normally gone in an instant +/.phase.* # internal / AI-collaboration docs — kept on the dev box, never committed, mirrored or shipped /CLAUDE.md diff --git a/docker/restart-sentinel/watch.sh b/docker/restart-sentinel/watch.sh index c3130c7..4de6faf 100755 --- a/docker/restart-sentinel/watch.sh +++ b/docker/restart-sentinel/watch.sh @@ -35,16 +35,17 @@ COMPOSE_FILE="${CLUSEV_DIR}/docker-compose.prod.yml" TAG="clusev-restart" log() { printf '%s %s: %s\n' "$(date -Is)" "$TAG" "$*"; } -# run_write_json PAYLOAD — write PAYLOAD to run/update-phase.json SYMLINK-SAFELY (as root, into a -# directory the app container can write). ./run is container-writable, so a compromised container -# could pre-plant update-phase.json as a symlink to an arbitrary host path; a plain `> file` would -# follow it and let this root write escape ./run. We write a temp in ./run and rename(2) over the -# target: rename replaces the path itself (link included) with our regular file and NEVER follows the -# link. Best-effort — a write failure must never change the update's outcome. +# run_write_json PAYLOAD — write PAYLOAD to run/update-phase.json SYMLINK-SAFELY as root. ./run is +# the container-writable bind mount, so a compromised container could pre-plant update-phase.json as +# a symlink to an arbitrary host path; a plain `> file` would follow it and let this root write escape +# ./run. We build the temp at the REPO ROOT — same filesystem as ./run but NOT mounted into any +# container, so the container cannot unlink/replace it mid-write (no TOCTOU) — then rename(2) it over +# the target: an atomic same-fs rename replaces the path itself (link included) with our regular file +# and NEVER follows the link. Best-effort — a write failure must never change the update's outcome. run_write_json() { local d="${CLUSEV_DIR}/run" tmp mkdir -p "$d" 2>/dev/null || true - tmp="$(mktemp "${d}/.phase.XXXXXX" 2>/dev/null)" || return 0 + tmp="$(mktemp "${CLUSEV_DIR}/.phase.XXXXXX" 2>/dev/null)" || return 0 printf '%s\n' "$1" > "$tmp" 2>/dev/null || { rm -f "$tmp" 2>/dev/null; return 0; } chmod 644 "$tmp" 2>/dev/null || true mv -f "$tmp" "${d}/update-phase.json" 2>/dev/null || rm -f "$tmp" 2>/dev/null diff --git a/install.sh b/install.sh index 1ab2c12..edbefb8 100755 --- a/install.sh +++ b/install.sh @@ -30,9 +30,17 @@ phase() { printf '\n\033[1m[%s] %s\033[0m\n' "$1" "$2"; } # a write failure must NEVER abort the update, hence the guards + `return 0`. STAGE_FILE="run/update-phase.json" set_stage() { + # Symlink-safe write (see docker/restart-sentinel/watch.sh run_write_json): run/ is the + # container-writable bind mount, so update-phase.json could be pre-planted as a symlink and a plain + # `> file` would let this root write (install.sh runs as root, incl. via update.sh) follow it out of + # run/. Build the temp at the repo root (cwd — same fs as run/, not mounted into any container) and + # rename(2) it over the target: the atomic same-fs rename replaces the link itself, never follows it. + local tmp mkdir -p run 2>/dev/null || true - printf '{"stage":"%s","at":%s}\n' "$1" "$(date +%s 2>/dev/null || echo 0)" > "$STAGE_FILE" 2>/dev/null || true - chmod 644 "$STAGE_FILE" 2>/dev/null || true + tmp="$(mktemp ./.phase.XXXXXX 2>/dev/null)" || return 0 + printf '{"stage":"%s","at":%s}\n' "$1" "$(date +%s 2>/dev/null || echo 0)" > "$tmp" 2>/dev/null || { rm -f "$tmp" 2>/dev/null; return 0; } + chmod 644 "$tmp" 2>/dev/null || true + mv -f "$tmp" "$STAGE_FILE" 2>/dev/null || rm -f "$tmp" 2>/dev/null return 0 } diff --git a/update.sh b/update.sh index c8fa800..543cac5 100755 --- a/update.sh +++ b/update.sh @@ -32,12 +32,14 @@ export GIT_HTTP_LOW_SPEED_TIME="${GIT_HTTP_LOW_SPEED_TIME:-30}" # Written first as "fetch" so the screen leaves the time-guess behind the moment the pull begins, # and so a stale stage from a previous update is reset. set_stage() { - # Symlink-safe write: ./run is the container-writable bind mount, so update-phase.json could be - # pre-planted as a symlink; write a temp in ./run and rename(2) over the target so this root write - # replaces the link itself and never follows it (mirror of run_write_json in the restart-sentinel). + # Symlink-safe write (mirror of run_write_json in the restart-sentinel): ./run is the + # container-writable bind mount, so update-phase.json could be pre-planted as a symlink. Build the + # temp at the REPO ROOT — same filesystem as ./run but not mounted into any container, so it can't + # be tampered mid-write (no TOCTOU) — then rename(2) it over the target: an atomic same-fs rename + # replaces the link itself with our regular file and never follows it. local d="$REPO_DIR/run" tmp mkdir -p "$d" 2>/dev/null || true - tmp="$(mktemp "${d}/.phase.XXXXXX" 2>/dev/null)" || return 0 + tmp="$(mktemp "${REPO_DIR}/.phase.XXXXXX" 2>/dev/null)" || return 0 printf '{"stage":"%s","at":%s}\n' "$1" "$(date +%s 2>/dev/null || echo 0)" > "$tmp" 2>/dev/null || { rm -f "$tmp" 2>/dev/null; return 0; } chmod 644 "$tmp" 2>/dev/null || true mv -f "$tmp" "${d}/update-phase.json" 2>/dev/null || rm -f "$tmp" 2>/dev/null