Skip to content

feat(installer): leftover-data guard — never silently adopt an earlier install's data (#376) - #384

Merged
shujaatTracebloc merged 13 commits into
developfrom
feat/376-installer-leftover-guard
Jul 24, 2026
Merged

feat(installer): leftover-data guard — never silently adopt an earlier install's data (#376)#384
shujaatTracebloc merged 13 commits into
developfrom
feat/376-installer-leftover-guard

Conversation

@saadqbal

@saadqbalsaadqbal commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Closes#376. Implements the installer leftover-guard from RFC-0003 (cli#366, D3 — §4). Related: #367 (node-local storage), design: backend#1151.

Problem

A new install (one that creates a fresh cluster) did mkdir -p on its data dirs and adopted whatever already existed under HOST_DATA_DIR — data left by an earlier install (different layout, older version, custom dir) was silently picked up, so a "fresh" install wasn't guaranteed fresh.

What this does

Adds guard_leftover_data (scripts/lib/cluster.sh), called from create_clusteronly when creating a NEW cluster — an existing cluster is an in-place reuse/upgrade whose data stays by design (§3.3), so routine re-runs are unaffected. When it finds real client data it stops and forces a choice:

  • Interactive:reuse / wipe / new dir / abort (abort is the default).
  • Non-interactive:--reuse-data / --wipe-data / --data-dir=PATH (or TB_LEFTOVER_ACTION). No terminal + no choice → fail-safe abort, never adopt.
  • Bypass:TRACEBLOC_SKIP_LEFTOVER_GUARD=1.

Detection (_leftover_data_dirs) covers both on-disk layouts — flat ($HOST_DATA_DIR/{mysql,data}) and per-release ($HOST_DATA_DIR/<rel>/…) — and keys on real data (MySQL/dataset files); empty dirs, values.yaml, and install logs are ignored.

Safety

  • Scoped to HOST_DATA_DIR only. HOST_DATASET_DIR may be a shared network mount, so it is never scanned or wiped (a test asserts this).
  • Wipe only removes paths under the already-validated HOST_DATA_DIR (validate_config guarantees it's under $HOME and not a system path).
  • Doubles as the node-local migration prompt (Implement RFC-0003 local storage: node-local (Option C, single-node) + drop chmod 777 #367): existing ~/.tracebloc data is never silently stranded.

Tests / checks

  • New scripts/tests/leftover-guard.bats — 13 tests (detection both layouts, reuse/wipe/new-dir, interactive + non-interactive, fail-safe abort, HOST_DATASET_DIR preserved).
  • Full bats suite: 380/380 pass.
  • shellcheck --severity=error (CI-equivalent) and check-style.sh: clean.
  • manifest.sha256 regenerated (--check passes).

Docs

README "Reinstalling on a machine that still holds data" note + --help/env-var docs.

🤖 Generated with Claude Code


Note

Medium Risk
Changes first-run install behavior and can delete local MySQL/dataset dirs on wipe; mitigated by explicit choices, non-interactive abort by default, and extensive safety checks/tests.

Overview
New installs that create a fresh k3d cluster now run a leftover-data guard before provisioning: if HOST_DATA_DIR already has real MySQL/dataset files (flat or per-release layout), the installer stops instead of silently adopting it. Users choose reuse, wipe, another directory, or abort (interactive default is abort); non-interactive runs need --reuse-data, --wipe-data, or --data-dir=PATH, otherwise the install fails closed. Existing clusters are unchanged (in-place reuse/upgrades skip the guard).

The guard is scoped to HOST_DATA_DIR only—HOST_DATASET_DIR is never scanned or wiped—with symlink-safe detection, pipefail-safe non-empty checks, and wipe paths constrained under $HOME. validate_config now rejects empty HOST_DATA_DIR, expands leading ~, and blocks using $HOME itself as the data dir.

CLI/docs: new flags and env vars on install-k8s.sh, README reinstall note, --help/golden updates, refreshed manifest.sha256, plus leftover-guard.bats and HOST_DATA_DIR validation tests in common.bats.

Reviewed by Cursor Bugbot for commit e3f477b. Bugbot is set up for automated code reviews on this repo. Configure here.

A new install (one that creates a fresh cluster) used to mkdir -p its data
dirs and silently adopt whatever was already under HOST_DATA_DIR — data
from an earlier install (different layout, older version, custom dir) got
picked up, so a "fresh" install was not guaranteed fresh.
Add guard_leftover_data (cluster.sh), called from create_cluster only when
creating a NEW cluster (an existing cluster is an in-place reuse/upgrade
whose data stays by design, §3.3). It detects real client data — MySQL or
dataset files — across both the flat and per-release layouts, and forces a
choice instead of adopting it:
- interactive: reuse / wipe / new dir / abort (abort is the default)
- non-interactive: --reuse-data / --wipe-data / --data-dir=PATH
(or TB_LEFTOVER_ACTION); no terminal + no choice => fail-safe abort
- TRACEBLOC_SKIP_LEFTOVER_GUARD=1 bypasses entirely
Scoped to HOST_DATA_DIR only — HOST_DATASET_DIR may be a shared network
mount, so it is never scanned or wiped. Also doubles as the node-local
migration prompt (#367): existing ~/.tracebloc data is never silently
stranded.
Implements RFC-0003 D3 (cli#366, §4). Adds scripts/tests/leftover-guard.bats
(13 tests), README + --help/env docs, and regenerates manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbalsaadqbal self-assigned this Jul 24, 2026
Comment threadscripts/lib/cluster.sh
Comment threadscripts/lib/cluster.sh
Comment threadscripts/lib/cluster.sh
…elp lines
The copy-catalog byte-exact golden captures install-k8s.sh --help output; the
new --reuse-data/--wipe-data/--data-dir usage + leftover-data section drifted
it. Regenerated via TB_UPDATE_GOLDEN=1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/lib/cluster.sh
saadqbaland others added 3 commits July 24, 2026 16:58
…put (Bugbot #384)
- _leftover_data_dirs: a find|head|grep pipeline SIGPIPEs find once its
output exceeds the pipe buffer (a real multi-table MySQL dir); under the
installer's set -o pipefail that read as "empty" and the guard silently
adopted real leftovers — the exact failure it exists to prevent. Read the
first path from a process substitution instead (find's status stays out of
the check, still short-circuits). Regression test seeds 3000 files + pipefail.
- Interactive reply/newdir reads now go through _read_sanitized
(_strip_paste_garbage + whitespace trim), so pasted CSI escapes can't enter
HOST_DATA_DIR and a spaces-only path no longer slips past the non-empty check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…fixes)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…384)
_wipe_leftover_data only warned on rm -rf failure and the guard still
returned success + logged "wiped"; create_cluster then adopted whatever
survived — common when container MySQL dirs are root-owned and the host
user can't remove them — silently breaking the "wipe means gone" fail-safe.
Now _wipe_leftover_data verifies each target is actually gone (not just rm's
exit code) and returns non-zero if any survived; the guard's wipe branch
errors out instead of proceeding. Regression test: an unremovable dir makes
wipe abort (exit 1) with the survivor left in place, not adopted.
Regenerates manifest.sha256 for the cluster.sh change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@LukasWodkaLukasWodka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the guard logic end-to-end — this is careful, well-tested work (default = abort, fail-closed on no-terminal, 18 bats cases, R8 manifest regenerated, HOST_DATASET_DIR correctly excluded from the wipe scope). One thing to confirm before merge, since it's destructive-capable:

Verify the HOST_DATA_DIR non-empty invariant guarding the wipe. The wipe loop removes $d only when it matches the "$HOST_DATA_DIR"/* case pattern. If HOST_DATA_DIR were ever empty/unset at the point guard_leftover_data runs, that pattern collapses to /* and the safety guard is defeated. The code comment states validate_config guarantees it is under $HOME — so the safety hinges entirely on guard_leftover_data always being called after validate_config has enforced a non-empty, under-$HOME HOST_DATA_DIR.

Could you confirm (or add a belt-and-suspenders assertion at the top of the wipe branch)?

[[ -n "$HOST_DATA_DIR" && "$HOST_DATA_DIR" == "$HOME"/* ]] || error "refusing to wipe: HOST_DATA_DIR unset/invalid"

That makes the wipe safe even if the call ordering ever changes. Everything else looks good to me.

(Claude-assisted review, run by @LukasWodka to keep the queue moving — flagging for a human eye on the wipe-safety invariant given this touches customer data.)

…ipe (Lukas review #384)
Belt-and-suspenders for the destructive path: _wipe_leftover_data now
asserts HOST_DATA_DIR is a non-empty path strictly under $HOME before any
rm. Today _leftover_data_dirs already returns empty when it's unset (so the
wipe never runs), and validate_config enforces the invariant — but placing
the check in the destructive function guards the rm even if a future refactor
changes call ordering (an empty HOST_DATA_DIR would collapse the
"$HOST_DATA_DIR"/* scope pattern to /*). Two tests: empty + outside-$HOME
both refuse.
Regenerates manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbal

Copy link
Copy Markdown
ContributorAuthor

Added in c8b9f94. Put the assertion inside _wipe_leftover_data (rather than the wipe branch) so it guards the rm for any caller, not just the current path:

[[ -n"${HOST_DATA_DIR:-}"&&"$HOST_DATA_DIR"=="$HOME"/* ]] \
|| error "Refusing to wipe: HOST_DATA_DIR is unset or not under \$HOME (got '${HOST_DATA_DIR:-}')."

For completeness on the invariant you asked about: guard_leftover_data is only ever called from create_cluster, which runs after validate_config (main() → validate_config → … → create_cluster), and _leftover_data_dirs already returns nothing when HOST_DATA_DIR is unset — so the wipe can't run today with an empty value. The assertion is the belt-and-suspenders that keeps it safe if that ordering ever changes. Two tests cover it (empty + outside-$HOME both refuse). Suite is now 20 bats cases.

🤖 Addressed by Claude Code

Comment threadscripts/lib/cluster.sh
…#384)
_leftover_data_dirs used find ... 2>/dev/null, so a root/container-owned
mysql/data dir the host user can't list produced no paths AND swallowed the
Permission-denied error — looking like a clean slate, letting create_cluster
adopt the leftovers (the exact fail-open the guard exists to prevent, and the
same ownership case the wipe path already treats as fatal).
Now capture find's stderr: a file found OR any find error => leftover. Keeps
the pipefail-safe read < <(find) (no find|head pipe). Regression test: a
chmod 000 mysql dir is detected, not skipped. Regenerates manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/lib/cluster.sh Outdated
…th (Bugbot #384)
The previous fix redirected find's stderr to a temp file; if mktemp failed,
stderr went to /dev/null and the unreadable-dir check never fired -> a
root/container-owned leftover dir looked empty and got adopted. Remove the
temp file entirely:
- explicit [[ -r && -x ]] check fails closed on an unlistable top dir (the
common container-owned case),
- a temp-file-free $(find … 2>&1 >/dev/null) fallback (only when no top-level
file was read) catches an unreadable subdir too.
Keeps the pipefail-safe read < <(find). Added a subdir-unreadable regression
test. Regenerates manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/lib/cluster.sh
Comment threadscripts/lib/cluster.sh
Comment threadscripts/install-k8s.sh
…gbot #384)
Three findings on the new-dir/wipe surface:
- Symlink traversal (Med): detection walked $HOST_DATA_DIR/*/ incl. dir
symlinks, so a planted 'evil -> /etc' gave …/evil/mysql and wipe's rm -rf
followed it outside scope. Detection now skips symlinked candidates
(-L check on both the per-release glob and each candidate); wipe refuses to
rm a symlink as a backstop.
- Tilde paths (Med): --data-dir=~/foo / new-dir '~/foo' were read literally,
so validate_config turned them into $HOME/~/foo and failed. Expand a
leading ~ / ~/ at the validate_config choke point.
- Empty data-dir (Low): --data-dir= set HOST_DATA_DIR='' which resolved to a
surprise dir; validate_config now fails closed on empty.
Tests: symlink-not-a-candidate, wipe-refuses-symlink, tilde-expands,
empty-fails-closed. Regenerates manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/lib/cluster.sh
`guard_leftover_data` decided interactivity with `[[ -r "$TB_TTY" ]]`, but
/dev/tty is world-readable even with no controlling terminal (CI, curl|bash).
Those runs took the interactive branch, `read` failed immediately, and the
guard aborted with the generic abort text instead of the non-interactive
guidance that lists --reuse-data / --wipe-data.
Add `_tty_usable` — the same `{ : <"$TB_TTY"; }` openability probe assess.sh
already uses — and gate both the reuse/wipe/new-dir prompt and the new-dir
read on it.
Regression test: readable-but-unopenable /dev/tty -> non-interactive guidance
(skips in an interactive shell). Regenerates manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/lib/common.sh
… (Bugbot #384)
validate_config allowed HOST_DATA_DIR to equal $HOME, and the new tilde
expansion maps a bare `~` to $HOME — so a common new-dir / --data-dir input
would select the whole home directory. The installer would then chmod 777
home-level logs/mysql dirs, bind-mount all of $HOME into the cluster, and
leftover detection would treat any existing ~/data or ~/mysql as install data.
Require HOST_DATA_DIR to be strictly UNDER $HOME (never $HOME itself); this
also closes the same footgun via HOST_DATA_DIR=$HOME / --data-dir=$HOME.
Tests: `== $HOME` rejected, bare `~` rejected. Regenerates manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1c7c38c. Configure here.

Comment threadscripts/lib/cluster.sh
shujaat hasanand others added 2 commits July 24, 2026 15:35
)
The per-release scan walked every subdir of HOST_DATA_DIR, including the
flat-layout `mysql`/`data` dirs. A real MySQL datadir always contains a nested
`mysql` system schema, so detection reported both $HOST_DATA_DIR/mysql and
$HOST_DATA_DIR/mysql/mysql as leftover roots — mislabeling layout, confusing
the prompt, and doubling wipe targets.
Skip the flat data dirs in the per-release walk; they are already candidates.
Test: a datadir with a nested `mysql` schema reports exactly one root.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@shujaatTracebloc
shujaatTracebloc merged commit 5f6b016 into developJul 24, 2026
31 checks passed
@shujaatTracebloc
shujaatTracebloc deleted the feat/376-installer-leftover-guard branch July 24, 2026 13:42

@shujaatTraceblocshujaatTracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — reviewed end-to-end, and Bugbot's re-review on e3f477b now concludes success with all 12 findings resolved.

Verified independently (not just trusting fix replies):

  • Full leftover-guard suite green locally, shellcheck --severity=error clean, manifest.sha256 --check clean, branch current with develop (manifest conflict resolved by regenerating).
  • The design is sound: a new-cluster-only guard that fails closed everywhere — no-TTY, unreadable/root-owned dirs, wipe survivors, symlink-escape, and now == $HOME all abort rather than silently adopt.

Findings, all confirmed genuinely fixed:

  • Detection: pipefail-safe multi-file scan; unreadable dirs fail closed (no temp-file fail-open path); symlinked release dirs skipped; flat datadir's nested mysql schema no longer double-counted.
  • Wipe: verifies removal (fails closed on survivors); refuses symlink-escape; asserts HOST_DATA_DIR non-empty & under $HOME.
  • Input: paste/CSI sanitized + trimmed; TTY probed for openability (not just -r) so no-TTY runs get the actionable non-interactive guidance; ~/~/ expanded; empty --data-dir rejected; HOST_DATA_DIR == $HOME rejected.

Every fix carries a regression test. LGTM. 🚢

@LukasWodka

Copy link
Copy Markdown
Contributor

Functional review \u2014 passed\n\nBasis: the behavioural suites that ran on this PR at merge against real environments, not mocks:\n\n- on multiple Ubuntu releases + \u2014 a real install exercised end-to-end on real distros, behind a TLS-inspecting proxy\n\nI could not reach the dev API (no dev credentials), so rather than rubber-stamp I used the strongest evidence available: these suites exercise the actual behaviour this change alters, on real infrastructure. Advancing \u2192 .\n\nIf the functional reviewer wants a manual pass on dev in addition, please move it back and say so.

LukasWodka added a commit that referenced this pull request Jul 27, 2026
…#441)
The guard advised HOST_DATA_DIR=/local/path, but validate_config
requires the data dir under $HOME (Bugbot #384 hardening) — so the
exact audience this guard exists for (network home) was pointed at a
fix that fails validation on re-run. The copy now states the real
constraint and the two workable paths (local-home user, or the explicit
TRACEBLOC_ALLOW_NETWORK_FS=1 override with its risk), and notes that
datasets may stay on network storage via HOST_DATASET_DIR. Test asserts
the impossible advice stays gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 27, 2026
#441)
* fix(installer): run the network-FS guard before the log dir is created (#432)
setup_log_file mkdirs HOST_DATA_DIR and tees the whole session's output
onto it BEFORE run_preflight fires — so on the exact machine the
network-FS guard was built for (NFS home + sudo + root_squash), the
unguarded mkdir failed with a bare error, or the log dir landed
squashed/nobody-owned, before the friendly named failure could print.
New early_data_dir_guard runs right after validate_config and before
setup_log_file: same filesystem classification (extracted as the shared
_pf_is_network_fstype), console-only, silent on local/undetermined
filesystems, defers to the full check's warning under
TRACEBLOC_ALLOW_NETWORK_FS. The call is declare -F-guarded so a stale
bootstrap without the new helper proceeds as before.
Closes#432
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(installer): early NFS guard names a followable remediation (Bugbot #441)
The guard advised HOST_DATA_DIR=/local/path, but validate_config
requires the data dir under $HOME (Bugbot #384 hardening) — so the
exact audience this guard exists for (network home) was pointed at a
fix that fails validation on re-run. The copy now states the real
constraint and the two workable paths (local-home user, or the explicit
TRACEBLOC_ALLOW_NETWORK_FS=1 override with its risk), and notes that
datasets may stay on network storage via HOST_DATASET_DIR. Test asserts
the impossible advice stays gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(installer): early NFS guard skips an existing data dir (Bugbot #441 r2)
The early guard's job is protecting the pre-log mkdir; an existing data
dir has no at-risk mkdir, and a healthy machine's re-run must keep
reaching the assess hand-off exactly as it did when the network-FS check
lived only in run_preflight. Existing dir -> silent pass; the full
preflight guard still classifies network storage for real (re)installs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@saadqbal@LukasWodka@shujaatTracebloc