admin
d96e5bddd0
handleDiskFormat's customer-confirmed branch formatted the mutable req.Device path; the durable id only bound the confirmation, never the mkfs target. A /dev reassignment between inspect and mkfs could wipe the wrong physical disk. Now mirrors signedjobs.WipeExecutor: resolve confirmed durable id -> current device, re-derive + require exact match, re-inspect (still data-bearing), then format THAT device. Any refusal -> 409, no mkfs. New antiRetargetResolve helper (injected deps, unit-tested: mismatch/gone/blank/empty all refuse). Injectable reresolveWipe seam on Server (defaults to real storage funcs). BRANCH ONLY — pending supervised review/deploy (see AGENT-001-FIX-NOTES.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
53 lines
4.1 KiB
Markdown
53 lines
4.1 KiB
Markdown
# AGENT-001 fix — inline customer-confirmed wipe: anti-retarget re-resolution
|
|
|
|
**Branch:** `fix/agent-001-wipe-durable-reresolve` (off `main` @ d17b5ab) — **PENDING REVIEW, NOT deployed.**
|
|
**Status:** branch-only. No agent build, no deploy, no golden rebake, no re-provision. That is reserved for a supervised session (this fix touches the destructive storage-wipe path).
|
|
|
|
## The bug
|
|
|
|
`internal/localapi/disks.go` `handleDiskFormat`, the data-bearing **customer-confirmed** branch
|
|
(`dec.Allowed`), inspected and gate-bound the device by its durable id but then called
|
|
`s.disks.Format(ctx, req.Device, …)` on the **caller-supplied mutable `/dev` path**. The durable id
|
|
only bound the *confirmation*; it was never re-resolved to the device just before `mkfs`. If a USB
|
|
re-enumeration reassigned `/dev/sdbN` to a **different physical disk** between inspection and `mkfs`
|
|
(the classify→mkfs TOCTOU), the wipe hit the wrong drive — data loss on an unintended disk.
|
|
|
|
The signed-jobs path (`internal/signedjobs/wipe.go` `WipeExecutor.Execute`) already does this
|
|
correctly: resolve durable→device, re-derive the device's durable id and require an exact match,
|
|
re-inspect, then format the *re-resolved* device. The inline path did not.
|
|
|
|
## What changed
|
|
|
|
- **New `internal/localapi/wipe_reresolve.go`:**
|
|
- `antiRetargetResolve(durableID, resolve, derive, inspect)` — pure, injected-deps helper mirroring
|
|
`WipeExecutor`: refuse empty durable id; resolve→current device; re-derive and require exact
|
|
match (a `/dev` node now pointing at a different disk derives a different id → refuse); re-inspect
|
|
and require still-`DataBearing()`. Returns the **re-resolved device** to format.
|
|
- `(*Server).reresolveDurableForWipe` wires it with the real `storage.ResolveDurableDevice` /
|
|
`storage.DeviceDurableID` / `s.disks.InspectDevice`.
|
|
- **`internal/localapi/disks.go`:** the `dec.Allowed` branch now calls `s.reresolveWipe(ctx, deviceDurable)`
|
|
and formats the **returned** device (never `req.Device`). On any refusal it returns `409 Conflict`
|
|
with the reason and does **not** call `mkfs`. The response/log now reference the re-resolved device.
|
|
- **`internal/localapi/server.go`:** new injectable field `reresolveWipe`, defaulted in `NewServer` to
|
|
`s.reresolveDurableForWipe`. Production behaviour unchanged (same real functions); the seam exists so
|
|
the inline path is unit-testable without real `/dev/disk/by-*`.
|
|
- **Tests:** `wipe_reresolve_test.go` covers `antiRetargetResolve` directly — happy path, empty durable,
|
|
no-longer-resolves, **durable-id mismatch (the core retarget case)**, no-longer-data-bearing,
|
|
re-inspect error. The existing integration test `TestFormat_DataBearing_UserDataConfirmed_Formats`
|
|
stubs `reresolveWipe` (the test helper sets it). Full `go build/vet/test ./...` green at the branch HEAD.
|
|
|
|
## Why it can't break legitimate wipes
|
|
|
|
`storage.DeviceDurableID` produces `byid:`/`byuuid:` ids and `storage.ResolveDurableDevice` accepts
|
|
exactly those — they round-trip. A normal user-data drive resolves back to itself, re-derives the same
|
|
id, and inspects data-bearing → format proceeds. Only three new refusals occur, all correct:
|
|
durable id gone, durable-id mismatch (retarget), or no-longer-data-bearing.
|
|
|
|
## Review checklist for the supervised merge+deploy+golden-rebake session
|
|
|
|
1. Confirm `antiRetargetResolve` refuses on mismatch/gone/blank/empty (run `go test ./internal/localapi/ -run AntiRetarget -v`).
|
|
2. Sanity-check `DeviceDurableID`↔`ResolveDurableDevice` scheme round-trip on the real demo USB (`felhom-usb`) so a genuine customer wipe still resolves+matches (read-only `DeviceDurableID` on the live device; do NOT wipe).
|
|
3. Confirm the `409 Conflict` refusal surfaces sensibly in the controller UI (controller `agentapi` maps the format response — verify it doesn't treat 409 as a hard error that hides the reason).
|
|
4. Bump agent version + CHANGELOG, build, golden-rebake, re-provision per the agent deploy runbook.
|
|
5. Live-validate one happy customer-confirmed wipe on a demo scratch drive (re-resolve matches → mkfs) and confirm the audit log line shows the re-resolved device == the confirmed durable id.
|