fix(web): require clone dir strictly under build_vms on remove (W6)
remove_vm() deletes vmx.parent, so containment must check the clone dir, not just the vmx: a crafted vmx_id of build_vms/x.vmx passed the old check and would rmtree the entire clone base. Now the parent must be strictly inside build_vms.
This commit is contained in:
@@ -24,7 +24,7 @@ Stato passi (spunta man mano):
|
||||
- [x] **W3** — `operations/audit.py`: append-only JSONL + lettura. Config `[web]` in `config.py`.
|
||||
- [x] **W4** — App web core: `web/app.py` factory + `commands/web.py` (`ci web serve`). Dashboard **read-only** (VM, cache, disco, runner). Ancora **senza auth**, bind localhost.
|
||||
- [x] **W5** — Auth: Gitea OIDC (Authlib) + sessione firmata + allowlist utenti (fail-closed) + secret solo da keyring. Tutte le route dati protette (`/healthz`,`/static`,`/login`,`/auth/callback` aperte). Hardening: `ci web serve` rifiuta bind non-loopback quando `[web.oidc]` assente (open mode solo su localhost).
|
||||
- [ ] **W6** — Azioni mutanti + safety: remove VM, cleanup orfane, purge cache, backup. Confirm gate + CSRF + audit + dry-run. HTMX partials.
|
||||
- [x] **W6** — Azioni mutanti + safety: remove VM, cleanup orfane, purge cache, backup. Confirm gate 2-step (preview dry-run → execute) + CSRF sincronizzatore + audit (preview+execute) + path-containment (clone dir **strettamente** sotto build_vms) + 409 job-race guard. `SessionMiddleware` sempre attivo (secret effimero in open mode) per CSRF. HTMX partials.
|
||||
- [ ] **W7** — UI dark Gitea-style (asset vendorizzati) + installer ripetibile (`setup-web-linux.sh`, unit systemd, `docs/WEB-UI.md`) + validazione finale (test, coverage, mypy, ruff, smoke manuale).
|
||||
|
||||
**Definition of done globale**: `ci web serve` gira come `ci-web.service`, login via Gitea, dashboard dark mostra stato live, le 4 azioni (kill VM / cleanup orfane / purge cache / backup) funzionano con conferma + audit, suite test verde con coverage ≥ 90 % e `mypy --strict` pulito.
|
||||
|
||||
@@ -197,7 +197,12 @@ async def vms_remove(
|
||||
config: Config = request.app.state.config
|
||||
|
||||
vmx = _decode_vmx_id(vmx_id)
|
||||
if vmx is None or not _is_contained(vmx, config.paths.build_vms):
|
||||
# remove_vm() deletes vmx.parent (the clone dir), so the *clone dir* — not
|
||||
# merely the vmx — must be STRICTLY inside build_vms. Checking vmx.parent
|
||||
# rejects a crafted ``build_vms/x.vmx`` whose parent IS build_vms itself,
|
||||
# which would otherwise rmtree the entire clone base. build_vms being an
|
||||
# ancestor of vmx.parent also implies vmx is contained.
|
||||
if vmx is None or not _is_contained(vmx.parent, config.paths.build_vms):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="vmx path is not contained in config.paths.build_vms",
|
||||
|
||||
@@ -356,6 +356,38 @@ def test_vm_remove_outside_build_vms_is_400_and_does_not_delete(tmp_path: Path)
|
||||
assert not any(e.result == "ok" for e in entries)
|
||||
|
||||
|
||||
def test_vm_remove_vmx_directly_in_build_vms_is_400_not_wiping_base(tmp_path: Path) -> None:
|
||||
"""A vmx whose parent IS build_vms would rmtree the whole clone base -- reject.
|
||||
|
||||
remove_vm() deletes vmx.parent; for ``build_vms/x.vmx`` that parent is
|
||||
build_vms itself. The containment check must reject it so a crafted
|
||||
vmx_id can't wipe every clone at once.
|
||||
"""
|
||||
config = _make_config(tmp_path)
|
||||
build_vms = config.paths.build_vms
|
||||
build_vms.mkdir(parents=True, exist_ok=True)
|
||||
survivor = build_vms / "Clone_keep"
|
||||
survivor.mkdir()
|
||||
(survivor / "Clone_keep.vmx").write_text("keep", encoding="utf-8")
|
||||
|
||||
danger_vmx = build_vms / "x.vmx" # parent == build_vms itself
|
||||
danger_vmx.write_text("x", encoding="utf-8")
|
||||
|
||||
fake_backend = _FakeBackend([])
|
||||
app = create_app(config)
|
||||
app.dependency_overrides[get_backend] = lambda: fake_backend
|
||||
client = TestClient(app)
|
||||
csrf = _get_csrf(client)
|
||||
vmx_id = routes_module.encode_vmx_id(danger_vmx)
|
||||
|
||||
resp = client.post(f"/vms/{vmx_id}/remove", data={"csrf": csrf, "stage": "execute"})
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert build_vms.exists() # clone base NOT wiped
|
||||
assert survivor.exists() # sibling clone survived
|
||||
assert fake_backend.delete_calls == []
|
||||
|
||||
|
||||
def test_vm_remove_malformed_vmx_id_is_400(tmp_path: Path) -> None:
|
||||
config = _make_config(tmp_path)
|
||||
app = create_app(config)
|
||||
|
||||
Reference in New Issue
Block a user