feat(#728): allowlist dashboard-confirmed data-dir moves to the stack data root#734
Merged
Merged
Conversation
… data root #719 made the four *_DATA_DIR moves confirm-gated, so a dashboard operator who types APPLY can now relocate a service's data dir. The destination was still checked only by assert_safe_dir — a BLOCKLIST that refuses the catastrophic roots but passes any other absolute path. At host-shell trust that is proportionate; at dashboard trust it lets a confirmed move target another user's home or another service's volume and have pithead mkdir/chown -R it and bind-mount it into a recreated container — a destination trust-escalation. control_approval_gate runs ONLY for dashboard commits (the host `apply` path never calls it), so it is exactly where the control-only rule belongs. For a control-channel move, narrow the destination from a blocklist to an ALLOWLIST: permit only a path under the stack's own data root ($PWD/data) or a parent the stack already keeps data in (each live *_DATA_DIR's parent — covers a #455 co-located data root). Anything else is refused even with the APPLY token and stays host-CLI only. The host `./pithead apply` path keeps the wider blocklist. Tier-1 stack tests: a control-confirmed move under the data root applies with APPLY; a move to an arbitrary non-blocklisted-but-non-allowed path is refused even with APPLY; the same path from the host shell still applies. Docs: SECURITY.md, docs/dashboard.md, CHANGELOG. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…wildcard Security review flagged the skip-condition `DYNAMIC_*` as broader than the values that actually resolve to an in-root stack default. resolve_default defaults exactly "" | auto | DYNAMIC_DATA | DYNAMIC_HOST | DYNAMIC_ID and passes anything else (e.g. DYNAMIC_FOO) through literally. Mirror that exact set so the allowlist skip can never swallow a non-sentinel, and pin the assert_safe_dir- runs-first ordering that keeps `..`/relative dests from reaching this block. Not exploitable before this change (a DYNAMIC_-prefixed string can't be absolute, and assert_safe_dir refuses it in the dry-run re-derivation first) — tightens a latent defense-in-depth mismatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #728. #719 made the four
*_DATA_DIRmoves confirm-gated, so a dashboard operator who typesAPPLYcan now relocate a service's data dir. The destination was still checked only byassert_safe_dir— a blocklist that refuses the catastrophic roots (/,$HOME, bare mounts) but passes any other absolute path. At host-shell trust that is proportionate; at dashboard trust it lets a confirmed move target another user's home or another service's volume and have pitheadmkdir/chown -Rit and bind-mount it into a recreated container — a destination trust-escalation.Control-vs-host split
control_approval_gateruns only for dashboard commits — it is called solely fromcontrol_commit(the control runner's commit path); the host./pithead applypath never calls it and keeps rendering throughparse_and_validate_config→assert_safe_dir. So the gate itself is the control-channel signal; no new env var or flag is needed. The allowlist lives inside the gate, scoping it to control-originated moves automatically.The allowlist
For a control-channel move, the destination is narrowed from a blocklist to an allowlist: permit only a path under
$PWD/data, the install dir'sdata/), or*_DATA_DIR's parent from.env(covers an operator-established, co-located data root, Standardize the deploy-box layout: upgrade maintains a symlink; dashboard data joins the shared data root #455).Anything else is refused even with the
APPLYtoken and stays host-CLI only. Only explicit absolute paths are checked;auto/empty resolves to a stack default that is under a data root by construction.assert_safe_dirstill runs at apply time.Tests (tier-1 stack,
tests/stack/run.sh)$C/data/monero-v2) applies withAPPLY.$SANDBOX/other-service-vol/monero) is refused even withAPPLY, cleared from staged, config.json untouched.Docs
SECURITY.md,docs/dashboard.md,CHANGELOG.md.Note
This is a security change to the control gate — a security-reviewer pass is warranted.
🤖 Generated with Claude Code