fix(security): close the TOCTOU in the symlink-safe status write (Codex review)
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 <noreply@anthropic.com>feat/v1-foundation
parent
a6176341e2
commit
3c25f097fa
|
|
@ -42,6 +42,9 @@ yarn-error.log
|
||||||
# host-side update transcript (written at the repo root — NOT under ./run — so a compromised
|
# 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)
|
# container cannot symlink-redirect the root updater's tee; see docker/restart-sentinel/watch.sh)
|
||||||
/update.log
|
/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
|
# internal / AI-collaboration docs — kept on the dev box, never committed, mirrored or shipped
|
||||||
/CLAUDE.md
|
/CLAUDE.md
|
||||||
|
|
|
||||||
|
|
@ -35,16 +35,17 @@ COMPOSE_FILE="${CLUSEV_DIR}/docker-compose.prod.yml"
|
||||||
TAG="clusev-restart"
|
TAG="clusev-restart"
|
||||||
log() { printf '%s %s: %s\n' "$(date -Is)" "$TAG" "$*"; }
|
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
|
# run_write_json PAYLOAD — write PAYLOAD to run/update-phase.json SYMLINK-SAFELY as root. ./run is
|
||||||
# directory the app container can write). ./run is container-writable, so a compromised container
|
# the container-writable bind mount, so a compromised container could pre-plant update-phase.json as
|
||||||
# could pre-plant update-phase.json as a symlink to an arbitrary host path; a plain `> file` would
|
# a symlink to an arbitrary host path; a plain `> file` would follow it and let this root write escape
|
||||||
# follow it and let this root write escape ./run. We write a temp in ./run and rename(2) over the
|
# ./run. We build the temp at the REPO ROOT — same filesystem as ./run but NOT mounted into any
|
||||||
# target: rename replaces the path itself (link included) with our regular file and NEVER follows the
|
# container, so the container cannot unlink/replace it mid-write (no TOCTOU) — then rename(2) it over
|
||||||
# link. Best-effort — a write failure must never change the update's outcome.
|
# 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() {
|
run_write_json() {
|
||||||
local d="${CLUSEV_DIR}/run" tmp
|
local d="${CLUSEV_DIR}/run" tmp
|
||||||
mkdir -p "$d" 2>/dev/null || true
|
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; }
|
printf '%s\n' "$1" > "$tmp" 2>/dev/null || { rm -f "$tmp" 2>/dev/null; return 0; }
|
||||||
chmod 644 "$tmp" 2>/dev/null || true
|
chmod 644 "$tmp" 2>/dev/null || true
|
||||||
mv -f "$tmp" "${d}/update-phase.json" 2>/dev/null || rm -f "$tmp" 2>/dev/null
|
mv -f "$tmp" "${d}/update-phase.json" 2>/dev/null || rm -f "$tmp" 2>/dev/null
|
||||||
|
|
|
||||||
12
install.sh
12
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`.
|
# a write failure must NEVER abort the update, hence the guards + `return 0`.
|
||||||
STAGE_FILE="run/update-phase.json"
|
STAGE_FILE="run/update-phase.json"
|
||||||
set_stage() {
|
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
|
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
|
tmp="$(mktemp ./.phase.XXXXXX 2>/dev/null)" || return 0
|
||||||
chmod 644 "$STAGE_FILE" 2>/dev/null || true
|
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
|
return 0
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
10
update.sh
10
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,
|
# 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.
|
# and so a stale stage from a previous update is reset.
|
||||||
set_stage() {
|
set_stage() {
|
||||||
# Symlink-safe write: ./run is the container-writable bind mount, so update-phase.json could be
|
# Symlink-safe write (mirror of run_write_json in the restart-sentinel): ./run is the
|
||||||
# pre-planted as a symlink; write a temp in ./run and rename(2) over the target so this root write
|
# container-writable bind mount, so update-phase.json could be pre-planted as a symlink. Build the
|
||||||
# replaces the link itself and never follows it (mirror of run_write_json in the restart-sentinel).
|
# 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
|
local d="$REPO_DIR/run" tmp
|
||||||
mkdir -p "$d" 2>/dev/null || true
|
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; }
|
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
|
chmod 644 "$tmp" 2>/dev/null || true
|
||||||
mv -f "$tmp" "${d}/update-phase.json" 2>/dev/null || rm -f "$tmp" 2>/dev/null
|
mv -f "$tmp" "${d}/update-phase.json" 2>/dev/null || rm -f "$tmp" 2>/dev/null
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue