diff --git a/admz/api/routes/devices.py b/admz/api/routes/devices.py index 6883dcc..9be3632 100644 --- a/admz/api/routes/devices.py +++ b/admz/api/routes/devices.py @@ -849,7 +849,8 @@ async def reveal_fleet_setting(key: str, request: Request): # -------------------------------------------------------------------------- class RecoveryRequest(BaseModel): - # Only 'reprovision' is queued (runs when the device returns factory-default); + # Only 'reprovision' is queued (raises a setup notice when the device returns + # factory-defaulted; the name is historical, nothing is provisioned); # 'remove' is immediate via DELETE /devices/{id}, so it isn't queued here. intent: str = "reprovision" username: str = "root" @@ -862,10 +863,12 @@ async def queue_recovery( req: RecoveryRequest, registry: DeviceRegistry = Depends(get_registry), ): - """Queue a pre-authorized recovery for a factory-defaulted device: when it - next reports needsetup (now, or after a future factory reset) the health - sweep re-provisions it (creates the admin account with a generated - password — never the fleet default: #185, FR-CRED-007). Authenticated + audited.""" + """Queue a follow-up for a factory-defaulted device: when it next reports + needsetup (now, or after a future factory reset) the health sweep raises a + ``setup`` notice in the Console so a person onboards it. It never provisions + unattended — that would write the fleet root password to whatever answers + (ADR-0068). ``username`` is accepted for old callers and ignored. + Authenticated + audited.""" from admz.audit import record_event from admz.auth import get_current_principal from admz.authz import require_authenticated_principal @@ -888,9 +891,9 @@ async def queue_recovery( spec = { "trigger_kind": "detection", "action_type": "reprovision", "device_id": device_id, "event": "on_needs_setup", - "action_params": {"username": req.username}, + "action_params": {}, "description": ( - f"Re-provision {device_id} when it returns factory-defaulted" + f"Raise a setup notice when {device_id} returns factory-defaulted" ), } return gate_task_write("create_task", device_id, spec, @@ -898,17 +901,17 @@ async def queue_recovery( pid = pending_actions.create( device_id=device_id, - action={"action": "reprovision", "username": req.username}, + action={"action": "reprovision"}, trigger=TRIGGER_NEEDS_SETUP, approved_by=str(principal), - description=f"Re-provision {device_id} when it returns factory-defaulted", + description=f"Raise a setup notice when {device_id} returns factory-defaulted", ) record_event(principal, "device.queue_recovery", resource=f"device:{device_id}", details={"intent": "reprovision", "pending_id": pid}) return { "success": True, "queued": True, "pending_id": pid, - "message": ("Re-provision queued — runs on the next health check, or " - "after a future factory reset."), + "message": ("Queued — when the device reports factory-defaulted, a " + "Console notice asks for it to be onboarded."), } diff --git a/admz/api/templates/device_detail.html b/admz/api/templates/device_detail.html index 668fb5e..d1b5904 100644 --- a/admz/api/templates/device_detail.html +++ b/admz/api/templates/device_detail.html @@ -275,8 +275,8 @@

No pending actions.

- Re-provision this device (create an admin with a generated password) when it next reports factory-defaulted. - + When this device next reports factory-defaulted, raise a notice in the Console so you can set it up. ADMZ never sets a device up unattended. +
@@ -776,7 +776,7 @@

}); const data = await r.json(); const ok = r.ok && data && data.success; - recMsg(ok ? (data.message || 'Queued — will re-provision when it returns factory-defaulted.') + recMsg(ok ? (data.message || 'Queued — a Console notice will ask you to set it up when it returns factory-defaulted.') : ('Error: ' + ((data && data.detail) || 'failed')), ok); if (ok) loadPending(); } catch (e) { recMsg('Request failed.', false); } @@ -801,15 +801,24 @@

const d = await r.json(); const me = (d.devices || []).find(function (x) { return x.device_id === DEVICE_ID; }); if (me && me.status === 'needs_setup') { + // ADR-0068: setting it up is attended, behind one approval — 'root' from + // the fleet root password, then ADMZ's own 'admz' account. The button + // runs the same onboarding as the credential banner below and follows + // its approval link. const b = document.getElementById('recovery-banner'); if (!b) return; b.innerHTML = '
This device is ' - + 'factory-defaulted (needs setup). Queue a re-provision below, or ' + + 'border-bottom:1px solid var(--border);display:flex;gap:10px;align-items:center;flex-wrap:wrap">' + + ' ' + + 'This device is factory-defaulted (needs setup). Setting it up gives ' + + 'root the fleet root password and creates ADMZ\'s own ' + + 'admz account, after you approve. Or ' + '' - + 'decommission it.
'; + + 'decommission it.' + + '' + + ''; if (window.lucide) lucide.createIcons(); + wireRunOnboarding(); } else if (me && (me.status === 'no_credentials' || me.status === 'auth_failed')) { // ADR-0064 slice B: the durable way back in. The chat's capture card // lasts five minutes and the session ten; this banner lasts as long as @@ -845,6 +854,13 @@

+ '' + ''; if (window.lucide) lucide.createIcons(); + wireRunOnboarding(); + } + } catch (e) { /* non-fatal */ } +})(); +// One onboarding handler for both banners above: it follows a capture or +// approval link when onboarding ends in one. +function wireRunOnboarding() { const btn = document.getElementById('run-onboarding-btn'); if (btn) btn.addEventListener('click', async () => { btn.disabled = true; @@ -861,9 +877,7 @@

} catch (e) { out.textContent = 'onboarding request failed'; } finally { btn.disabled = false; } }); - } - } catch (e) { /* non-fatal */ } -})(); +} loadPending(); {% endblock %} diff --git a/admz/api/templates/tasks.html b/admz/api/templates/tasks.html index 608847d..1154848 100644 --- a/admz/api/templates/tasks.html +++ b/admz/api/templates/tasks.html @@ -279,7 +279,7 @@

New event detection

drift_audit: { label: 'Check drift', hint: 'Re-reads live config and records drift vs. the blessed baseline, raising a Console notice when drift appears. Read-only on devices.', icon: 'radar' }, notify: { label: 'Notify (raise a Console notice)', hint: 'Raises a notice in the Console’s Needs attention strip.', icon: 'bell-ring' }, survey: { label: 'Survey (contribute to the API atlas)', hint: 'Bundles anonymized capability data for the axis-api-atlas project.', icon: 'upload-cloud' }, - reprovision: { label: 'Re-provision', hint: 'Creates the admin with a generated password when the device returns factory-defaulted.', icon: 'wrench' }, + reprovision: { label: 'Setup notice', hint: 'Raises a Console notice to set the device up when it returns factory-defaulted. Never sets it up unattended.', icon: 'bell-ring' }, }; const EVENT_LABELS = { on_needs_setup: 'when factory-defaulted', on_online: 'when back online' }; diff --git a/admz/chatbot/system_prompt.py b/admz/chatbot/system_prompt.py index f21fc96..16d74f2 100644 --- a/admz/chatbot/system_prompt.py +++ b/admz/chatbot/system_prompt.py @@ -267,7 +267,7 @@ is factory-defaulted (`needsetup`) and ADMZ's stored credentials no longer work — its health shows **"Needs setup"**, not "auth failed". - When the user asks to factory-reset a device, briefly ASK what should - happen afterward (re-provision when it returns / remove / leave it). + happen afterward (set it up again when it returns / remove / leave it). - To ACTUALLY run the reset, gate it like any dangerous op: call `query_catalog` then **`execute_operation`** with the factory-default operation. That tool call is what produces the REAL confirmation — @@ -279,18 +279,25 @@ `blocked: True` `execute_operation` just returned. Do NOT say "I have queued a factory reset" and show a link — if you haven't called `execute_operation`, there is no reset and no link; call it. -- "Queue" applies ONLY to the post-reset RECOVERY, never the reset. If - they chose re-provision, ALSO call `queue_device_recovery(device_id)` - to pre-authorize it (the health monitor re-provisions the device when - it returns `needsetup`). That's a separate step from gating the reset; - the chat doesn't wait on the reboot. Tell them it's queued + that - re-provision needs the health monitor enabled. -- This also works for a device that is ALREADY "Needs setup" (e.g. the - user reset it earlier): offer `queue_device_recovery` to recover it, - or `delete_device` to decommission it. +- "Queue" applies ONLY to the post-reset follow-up, never the reset. If + they chose to set it up again, ALSO call + `queue_device_recovery(device_id)`. It does NOT set the device up: when + the device returns `needsetup`, the health monitor raises a Console + notice ("Factory-reset — onboard it to set it up"). Say exactly that — + the chat doesn't wait on the reboot, they will see a notice, and it + needs the health monitor enabled. ADMZ never sets a device up + unattended, because that writes the fleet root password (ADR-0068). +- When a `[console]` note says the user opened a notice that a device came + back factory-defaulted, or the user asks to set up a device that is + "Needs setup", call `onboard_device(device_id)`. For a factory-defaulted + device it asks for approval, then sets `root` to the fleet root password + and creates ADMZ's own `admz` account. +- For a device ALREADY "Needs setup", don't queue anything: offer + `onboard_device` now, or `delete_device` to decommission it. - Use `list_device_recovery` to report what's queued and - `cancel_device_recovery(pending_id)` to undo a queued recovery. -- Re-provision passwords are generated per device and are NEVER shown. + `cancel_device_recovery(pending_id)` to undo a queued follow-up. +- No password is ever shown: `admz`'s is generated, and `root`'s is the + fleet root password the operator set. # Tool argument hygiene diff --git a/admz/mcp/server.py b/admz/mcp/server.py index 776c3c7..1ffdf9d 100644 --- a/admz/mcp/server.py +++ b/admz/mcp/server.py @@ -4971,19 +4971,20 @@ def _queue_device_recovery(self, arguments: Dict[str, Any]) -> Dict[str, Any]: device_id = arguments.get("device_id") intent = (arguments.get("intent") or "reprovision").strip() - username = arguments.get("username", "root") + # `username` used to name the admin to create on re-provision. Nothing + # is provisioned unattended any more (ADR-0068), so it is ignored. if not device_id: return {"success": False, "error": "device_id is required"} - # Queuing is a pre-authorization for a destructive provision — the - # anonymous principal may not arm it (mirrors the REST gate). + # Queuing arms standing behaviour — the anonymous principal may not arm + # it (mirrors the REST gate). if getattr(self.principal, "is_anonymous", False): return { "success": False, "error": "PermissionDenied", "message": ( - "Queuing a recovery pre-authorizes a future re-provision and " - "requires an authenticated principal (not the anonymous " - "default). Authenticate to the web/REST surface first." + "Queuing a recovery arms a standing task and requires an " + "authenticated principal (not the anonymous default). " + "Authenticate to the web/REST surface first." ), } if not self.registry.device_exists(device_id): @@ -5001,17 +5002,19 @@ def _queue_device_recovery(self, arguments: Dict[str, Any]) -> Dict[str, Any]: "action_type": "reprovision", "device_id": device_id, "event": "on_needs_setup", - "action_params": {"username": username}, + "action_params": {}, "description": ( - f"Re-provision {device_id} when it returns factory-defaulted" + f"Raise a setup notice when {device_id} returns factory-defaulted" ), } env = gate_task_write( "create_task", device_id, spec, describe_create(spec)) env["message"] = ( - f"{env.get('message', '')} The re-provision is armed only when " - "the user approves the confirmation card; it then fires on the " - "next health check once the device reports factory-defaulted." + f"{env.get('message', '')} It is armed only when the user " + "approves the confirmation card. When the device next reports " + "factory-defaulted, the health check raises a Console notice asking " + "for it to be onboarded — it does not set the device up by itself " + "(ADR-0068)." ).strip() return env diff --git a/admz/mcp/tools/recovery.py b/admz/mcp/tools/recovery.py index 3218f4b..2198e58 100644 --- a/admz/mcp/tools/recovery.py +++ b/admz/mcp/tools/recovery.py @@ -1,9 +1,10 @@ -"""MCP Tool definitions: deferred device recovery (factory-defaulted → re-provision). +"""MCP Tool definitions: deferred device recovery (factory-defaulted → setup notice). -These let the chatbot queue a *pre-authorized* follow-up that fires when a device -next reports factory-defaulted (needsetup) — the trigger-based counterpart to the -time-based snapshot schedules. The health-monitor sweep is the evaluator; the -actual re-provision runs only because the operator authorized it here, up front. +These let the chatbot queue a follow-up that fires when a device next reports +factory-defaulted (needsetup) — the trigger-based counterpart to the time-based +snapshot schedules. The health-monitor sweep is the evaluator. Since ADR-0068 the +follow-up never provisions unattended: it raises a Console notice so a person +onboards the device, behind the usual approval. """ from typing import List @@ -14,16 +15,18 @@ Tool( name="queue_device_recovery", description=( - "Pre-authorize a recovery that runs automatically when a device next " - "reports factory-defaulted (needsetup). Use this AFTER a factory reset " - "(or for a device already showing 'Needs setup') so the chat doesn't " - "have to wait ~1-2 min for the reboot: the queued action fires on the " - "next health check once the device comes back. Currently supports " - "intent='reprovision' — re-creates the admin account with a generated " - "per-device password (never the fleet default, and never shown). The device must be a " - "registered device_id. This is a deliberate authorization: only queue " - "it when the operator has asked to recover the device. Returns a " - "pending_id you can later cancel. Requires an authenticated principal." + "Queue a follow-up for when a device next reports factory-defaulted " + "(needsetup). Use it AFTER a factory reset, so the chat doesn't have to " + "wait ~1-2 min for the reboot. When the device comes back, the next health " + "check raises a Console notice ('Factory-reset — onboard it to set it " + "up'); reviewing that notice brings the device back to the chat to be " + "onboarded with onboard_device, behind the usual approval. It does NOT " + "set the device up by itself: since ADR-0068 ADMZ never writes the fleet " + "root password unattended. For a device ALREADY showing 'Needs setup', " + "don't queue anything — call onboard_device now. The only intent is " + "'reprovision' (the name is historical). The device must be a " + "registered device_id. Returns a pending_id you can later cancel. " + "Requires an authenticated principal." ), inputSchema={ "type": "object", @@ -37,15 +40,10 @@ }, "intent": { "type": "string", - "description": "Recovery to run when it returns. Only 'reprovision' for now.", + "description": "What to do when it returns. Only 'reprovision', which raises a setup notice.", "enum": ["reprovision"], "default": "reprovision", }, - "username": { - "type": "string", - "description": "Admin username to create on re-provision (default 'root').", - "default": "root", - }, }, "required": ["device_id"], }, diff --git a/admz/mcp/tools/tasks.py b/admz/mcp/tools/tasks.py index 5f1190e..e35cc1f 100644 --- a/admz/mcp/tools/tasks.py +++ b/admz/mcp/tools/tasks.py @@ -1,8 +1,8 @@ """MCP Tool definitions: unified Tasks view (ADR-0037). A single read across both task kinds — time-based **schedules** (snapshot / -drift_audit / survey) and trigger-based **detection** tasks (e.g. re-provision -when a device returns factory-defaulted). Creating/managing tasks still uses the +drift_audit / survey) and trigger-based **detection** tasks (e.g. a setup +notice when a device returns factory-defaulted). Creating/managing tasks still uses the per-kind tools (``create_snapshot_schedule`` …, ``queue_device_recovery`` …), which now share the same underlying store. """ @@ -18,7 +18,7 @@ "List ALL automated tasks in one view: time-based schedules " "(recurring snapshot / drift_audit / survey jobs) AND trigger-based " "detection tasks (one-shot, fire when a device's state matches — e.g. " - "re-provision when it returns factory-defaulted). Use this for " + "a setup notice when it returns factory-defaulted). Use this for " "questions like 'what's scheduled or queued?'. Optionally filter by " "device_id (tasks targeting that device) or kind ('schedule' or " "'detection'). Read-only." diff --git a/admz/notices/notes.py b/admz/notices/notes.py index a5342bd..7239e31 100644 --- a/admz/notices/notes.py +++ b/admz/notices/notes.py @@ -14,7 +14,7 @@ import time from typing import Iterable, List, Optional -from admz.notices.store import KIND_DRIFT, Notice +from admz.notices.store import KIND_DRIFT, KIND_SETUP, Notice PREFIX = "[console] The user opened" CLOSING = "Nothing has been changed." @@ -72,6 +72,16 @@ def describe(notice: Notice, now: Optional[float] = None) -> str: f"configuration drift on device {device} — {' and '.join(parts)}; " f"{seen}, last confirmed {_ago(confirmed, now)} by {source}" ) + if notice.kind == KIND_SETUP: + # Says what onboarding will do, because onboarding is the one action + # this notice exists to prompt — and it is gated, so saying it here + # commits nothing. + return ( + f"device {device} came back factory-defaulted — {source} noticed it " + f"{_ago(notice.created_at, now)}. ADMZ has no working credential for " + f"it until it is onboarded; onboarding sets 'root' to the fleet root " + f"password and creates ADMZ's own 'admz' account, behind one approval" + ) where = f"on device {device}" if device else "fleet-wide" return ( f"an event detection ({_ident(notice.task_id) or 'no task id'}) fired " diff --git a/admz/notices/producers.py b/admz/notices/producers.py index e26131e..fdc6979 100644 --- a/admz/notices/producers.py +++ b/admz/notices/producers.py @@ -10,7 +10,9 @@ Provenance rides a context variable: the ``drift_audit`` handler wraps its sweep in :func:`notice_provenance`, and anything else reads as a manual ``check_drift``. The ``notify`` task action raises an ``event`` notice, keyed -per task and device, so a detection that fires ten times bumps one row. +per task and device, so a detection that fires ten times bumps one row. A +queued re-provision raises a ``setup`` notice — one per device — when the +device comes back factory-defaulted, and a successful onboarding closes it. Rows carry identifiers, counts and class names only (see the store). """ @@ -23,7 +25,7 @@ from dataclasses import dataclass from typing import Any, Dict, Iterator, Optional -from admz.notices.store import KIND_DRIFT, KIND_EVENT, SEVERITIES, Notice +from admz.notices.store import KIND_DRIFT, KIND_EVENT, KIND_SETUP, SEVERITIES, Notice logger = logging.getLogger(__name__) @@ -31,6 +33,7 @@ SOURCE_DRIFT_AUDIT = "drift_audit" SOURCE_BACKFILL = "backfill" SOURCE_NOTIFY = "notify" +SOURCE_RECOVERY = "recovery" #: How a note or the prompt names a source. SOURCE_LABELS = { @@ -38,8 +41,12 @@ SOURCE_DRIFT_AUDIT: "the scheduled drift audit", SOURCE_BACKFILL: "the drift cache at startup", SOURCE_NOTIFY: "an event detection", + SOURCE_RECOVERY: "a queued recovery", } +#: What a setup notice is closed with once the device has working credentials. +RESOLUTION_ONBOARDED = "onboarded" + TRANSITIONS_THAT_RAISE = ("appeared", "changed") @@ -196,6 +203,37 @@ def event_notice(task: Any, message: str) -> Notice: ) +def setup_subject(device_id: str) -> str: + return f"setup:{device_id}" + + +def setup_notice(device_id: str, task_id: str = "") -> Notice: + """Raise (or bump) the notice that a device is factory-defaulted and + waiting to be onboarded. + + This is what a queued re-provision does now. Provisioning writes the fleet + root password, and ADR-0068 forbids doing that unattended — against + whatever answers at the device's address hours after the approval — so the + task hands the moment to a person instead. One live notice per device, + whichever task noticed; onboarding closes it (:func:`resolve_setup`). + """ + return _store().raise_notice( + kind=KIND_SETUP, + subject_key=setup_subject(device_id), + title="Factory-reset — onboard it to set it up", + summary={"action": "onboard"}, + device_id=device_id, + severity="medium", + source=SOURCE_RECOVERY, + task_id=task_id or "", + ) + + +def resolve_setup(device_id: str, by: str = "") -> Optional[Notice]: + """Close the device's setup notice: it has working credentials now.""" + return _store().resolve(setup_subject(device_id), RESOLUTION_ONBOARDED, by=by) + + def backfill_drift_notices(registry: Any) -> int: """Raise a notice for drift that predates notices. diff --git a/admz/notices/store.py b/admz/notices/store.py index fa393ed..58ff8ec 100644 --- a/admz/notices/store.py +++ b/admz/notices/store.py @@ -36,7 +36,10 @@ class names and timestamps — never a parameter value, and never a KIND_DRIFT = "drift" KIND_EVENT = "event" -KINDS = (KIND_DRIFT, KIND_EVENT) +#: A device came back factory-defaulted and is waiting to be onboarded — what a +#: queued re-provision now raises instead of provisioning unattended (ADR-0068). +KIND_SETUP = "setup" +KINDS = (KIND_DRIFT, KIND_EVENT, KIND_SETUP) STATUS_OPEN = "open" STATUS_SNOOZED = "snoozed" diff --git a/admz/onboarding.py b/admz/onboarding.py index 6796af1..db06a50 100644 --- a/admz/onboarding.py +++ b/admz/onboarding.py @@ -157,11 +157,25 @@ def _with_survey(result: Dict[str, Any]) -> Dict[str, Any]: gated/failed exits, and never as a side effect of a gate firing. For ``already_credentialed`` (which fires on every re-onboard of a healthy device) the queue is first-sight only: a device with capability rows has - been surveyed or audited already.""" + been surveyed or audited already. + + Every success exit also closes the device's ``setup`` notice (ADR-0071), + the one a queued recovery raises when the device comes back + factory-defaulted.""" status = result.get("status") device_id = result.get("device_id") or "" if not device_id: return result + if status in (PROVISIONED, OWN_ACCOUNT_CREATED, ALREADY_CREDENTIALED): + # The device has working credentials, so it no longer needs setting up: + # close the setup notice a queued recovery raised for it, if there is + # one. A notice is attention, never a blocker, so a failure is logged. + try: + from admz.notices.producers import resolve_setup + + resolve_setup(device_id, by="onboarding") + except Exception: # noqa: BLE001 + logger.debug("setup notice not closed for %s", device_id, exc_info=True) from admz.device_capabilities import capability_store, enqueue_capability_survey # ENTRY_CREDENTIALS_SAVED is deliberately absent (ADR-0068): it is no longer diff --git a/admz/tasks/handlers.py b/admz/tasks/handlers.py index 5442037..47d04b2 100644 --- a/admz/tasks/handlers.py +++ b/admz/tasks/handlers.py @@ -364,69 +364,48 @@ async def _run_acs_action(task: Task, ctx: TaskContext) -> Dict[str, Any]: @register_task_handler("reprovision") async def _run_reprovision(task: Task, ctx: TaskContext) -> Dict[str, Any]: - """Deferred re-provision of a factory-defaulted device — ADR-0037's - ``reprovision`` detection task, which since ADR-0068 is refused rather than - performed (below). Moved from recovery_actions.py; now reads deps from - ``ctx`` instead of a startup closure. + """A queued recovery fired: the device came back factory-defaulted. - ``attended=False`` (ADR-0068) — **this handler no longer provisions.** It - reports a refusal instead, and that is the point rather than a regression. + ADR-0037's ``reprovision`` detection task. **It never provisions.** It + raises a ``setup`` notice in the Console (ADR-0071), so a person onboards + the device; the notice's review asks the chat to do exactly that, behind + the usual approval, and a successful onboarding closes it. - Since ADR-0068 provisioning writes the fleet **break-glass** root password - — one value, shared across every device ADMZ provisions, known to the + Why not provision here: since ADR-0068 provisioning writes the fleet root + password — one value, shared by every device ADMZ provisions, known to the operator by design. This handler fires unattended, on the health sweep's - schedule, up to 24h after an operator approved the task, against whatever - host answers at the device's registered address *at that later moment*. The + schedule, up to 24h after the task was approved, against whatever host + answers at the device's registered address *at that later moment*. The trigger (``needsetup=yes``) is itself an unauthenticated device response and - nothing on this path re-verifies the peer (#185; #326 for the residual). So - a spoofed peer — a reassigned DHCP lease, ARP spoofing, the port a - decommissioned camera vacated — would walk away with a credential valid on - every device ADMZ has provisioned. That is a fleet-wide credential handed to - whoever answered, which is categorically worse than the per-device generated - password this path used to write. - - The previous answer was ``allow_fleet_default=False``: opt out of the shared - secret and generate per device. ADR-0068 removes that flag, because the root - password now has its own setting — and removing it *without* this refusal - would have made this handler silently START sending the shared value, which - is the exposure the ADR exists to prevent. The refusal is therefore - structural, not a comment. - - **Deferring to an attended flow is what this file already recommended** as - the honest fix for #185/#326, alongside real peer identity (unverified as - buildable today). This is that deferral. The visible cost: a factory- - defaulted device that appears while nobody is watching stays - ``needs_setup`` until an operator onboards it, where before it would have - been provisioned with a password only ADMZ held. + nothing here re-verifies the peer (#185; #326). A spoofed peer — a + reassigned DHCP lease, ARP spoofing, the port a decommissioned camera + vacated — would walk away with a credential valid on every device ADMZ + provisioned. + + From ADR-0068 until 2026-09-23 this handler called + ``provision_factory_default(attended=False)``, which refused, so every fired + task failed while the chat still promised a re-provision. Handing the + moment to a person is the attended flow ADR-0068 asked for. + + A store failure is a failure (#455), never a silent success. """ - from admz.provisioning import provision_factory_default + from admz.notices.producers import setup_notice device_id = task.device_id or (task.device_ids or [""])[0] - if ctx.registry is None: - raise RuntimeError("reprovision: task context has no registry") - info = ctx.registry.get_device_info(device_id) - host = info.get("host") or info.get("ip_address") - if not host: - raise ValueError(f"device {device_id} has no host to provision") - result = await provision_factory_default( - ctx.catalog, ctx.executors, ctx.registry, - device_id=device_id, host=host, - username=(task.action_params or {}).get("username", "root"), - attended=False, - ) - if not result.get("success"): - raise RuntimeError(result.get("error") or "provision failed") - logger.info("deferred reprovision succeeded for %s", device_id) - # Carry the SOURCE forward, never the password (GH #326). `provisioning` - # already distinguishes provided / fleet_default / generated, and this - # handler was dropping it on the floor — so the audit row for a fired - # reprovision could not say which mode produced the credential it just - # created. That is the forensic question #326's phantom-provision gap makes - # worth answering: an operator looking at a suspect provision needs to know - # what was set, and the alternative is inferring it from the code path that - # was live at the time. + if not device_id: + raise ValueError("reprovision: the task names no device") + try: + notice = setup_notice(device_id, task_id=str(getattr(task, "id", "") or "")) + except Exception as exc: # noqa: BLE001 — reported, not swallowed + logger.warning("reprovision: could not raise a setup notice for %s: %s", + device_id, exc) + return {"success": False, "error": f"notice not raised: {exc}", + "summary": f"setup notice failed: {exc}"} + logger.info("deferred recovery for %s raised setup notice #%s", + device_id, notice.id) return { "success": True, - "summary": f"re-provisioned {device_id}", - "password_source": result.get("password_source"), + "notice_id": notice.id, + "summary": (f"{device_id} is factory-defaulted: raised notice " + f"#{notice.id} to onboard it"), } diff --git a/docs/ACCEPTANCE.md b/docs/ACCEPTANCE.md index 1044afa..cb57b25 100644 --- a/docs/ACCEPTANCE.md +++ b/docs/ACCEPTANCE.md @@ -305,7 +305,8 @@ deliberately does not perform: - [`tests/e2e/MANUAL_password_tests.md`](../tests/e2e/MANUAL_password_tests.md) — real device-password rotation. - [`tests/e2e/MANUAL_recovery_tests.md`](../tests/e2e/MANUAL_recovery_tests.md) - — factory-defaulted device detection and re-provision. + — factory-defaulted device detection and setup (a queued recovery raises + a setup notice; the device is set up through the chat, behind approval). > Both predate #180 and name `:4242` in their prerequisites. **Read that as > production and substitute staging**, or you are running the exact procedure diff --git a/docs/MCP_TOOLS_REFERENCE.md b/docs/MCP_TOOLS_REFERENCE.md index d9324a1..f47aa40 100644 --- a/docs/MCP_TOOLS_REFERENCE.md +++ b/docs/MCP_TOOLS_REFERENCE.md @@ -193,29 +193,32 @@ offline period followed by a healthy response. from the catalog) ### `queue_device_recovery` -Pre-authorize a **trigger-based** recovery (the counterpart to the -time-based snapshot schedules). When the device next reports -factory-defaulted (`needsetup`), the health-monitor sweep fires the queued -`reprovision` task — so a factory reset from chat doesn't block on the -~1–2 min reboot. - -> **Since ADR-0068 (2026-09-14) the fired task does not provision.** -> Provisioning now writes the fleet root password, one value shared by every -> device ADMZ provisions, and an unattended write to whatever answers at the -> device's address could hand it to a spoofed peer. So the handler calls -> `provision_factory_default(attended=False)`, which refuses -> (`unattended_not_permitted`) and writes nothing; the task is marked `failed` -> and a `deferred_action_failed` audit row is written. The device stays -> `needs_setup` until it is onboarded attended — `onboard_device` or -> `provision_device`, with an operator approving the write — which sets `root` -> to the fleet root password, creates ADMZ's own `admz` account with a -> generated password, and stores only `admz`. -> -> _Corrected 2026-09-16: this entry said the sweep re-provisions the device -> with a generated per-device password. The tool's own description and the -> chat prompt still say so._ -- **Args:** `device_id` (required); `intent` (only `reprovision` for now); - `username` (default `root`) +Queue a **trigger-based** follow-up for a factory reset (the counterpart to +the time-based snapshot schedules), so a reset from chat doesn't block on the +~1–2 min reboot. When the device next reports factory-defaulted +(`needsetup`), the health-monitor sweep fires the queued task, and the task +raises a **`setup` notice** in the Console: *"Factory-reset — onboard it to set +it up"*. Reviewing that notice brings the device back to the chat. There, +`onboard_device` asks for approval, then sets `root` to the fleet root +password, creates ADMZ's own `admz` account and stores only `admz`. A successful +onboarding closes the notice. + +**It never sets the device up by itself.** Provisioning writes the fleet root +password, one value shared by every device ADMZ provisions (ADR-0068). This +task fires unattended against whatever answers at the device's address, which a +spoofed peer could be, so it hands the moment to a person instead. +- **History:** + - From ADR-0068 (2026-09-14), the fired task called + `provision_factory_default(attended=False)`, which refused. Every fired task + therefore failed, while this entry, the tool description and the chat + prompt still promised a re-provision. + - 2026-09-16: this entry was corrected to describe the refusal. + - 2026-09-23: the task raises the notice instead. +- For a device **already** in `needs_setup`, don't queue anything: call + `onboard_device` now. +- **Args:** `device_id` (required); `intent` (only `reprovision` — the name is + historical). `username` was removed; the REST route still accepts it and + ignores it. - **Returns:** `{success, queued, pending_id, device_id, trigger, message}` - Requires an authenticated principal (anonymous may not arm it) and the health monitor to be enabled (it is the evaluator). The pending action @@ -237,7 +240,7 @@ Cancel a still-pending deferred recovery by id. ### `list_tasks` Unified view of ALL automated tasks (ADR-0037): time-based **schedules** (recurring snapshot / drift_audit / survey) AND trigger-based **detection** tasks -(one-shot — e.g. re-provision when a device returns factory-defaulted). Use for +(one-shot — e.g. a setup notice when a device returns factory-defaulted). Use for "what's scheduled or queued?". Read-only. Creating/managing still uses the per-kind tools (`create_snapshot_schedule` …, `queue_device_recovery` …), which share the same store. diff --git a/docs/specification/decisions/0068-root-is-a-break-glass-credential-admz-sets-and-never-stores.md b/docs/specification/decisions/0068-root-is-a-break-glass-credential-admz-sets-and-never-stores.md index 3220976..ada402c 100644 --- a/docs/specification/decisions/0068-root-is-a-break-glass-credential-admz-sets-and-never-stores.md +++ b/docs/specification/decisions/0068-root-is-a-break-glass-credential-admz-sets-and-never-stores.md @@ -160,6 +160,8 @@ That is a real trade, not a neutral one, and it is accepted deliberately. **One mitigation is required rather than optional.** The unattended `reprovision` detection handler (`tasks/handlers.py:388`) fires up to 24 hours later against whatever answers on that IP, with no human present, and is ungated by ADR-0059 design. It must **not** write the break-glass password — it defers to an attended flow, which `handlers.py:370-377` already names as the honest fix. An attended provision is a human looking at one device; the deferred handler is the case #326 describes, where *"the registry can end up believing it holds a working credential it never actually set."* +> **As built, 2026-09-23.** For nine days the "defer" was only a refusal. The handler called `provision_factory_default(attended=False)`, every fired task failed, and the chat still offered "queue a re-provision". The attended flow now exists: a fired task raises a `setup` notice in the Console, its review sends the chat to `onboard_device` behind the usual approval, and a successful onboarding closes it ([ADR-0071's amendment](0071-a-task-raises-a-notice-the-console-delivers-it.md#amendment-2026-09-23--a-third-kind-setup)). The handler never reaches provisioning. + **#165 gets worse, and should be re-read.** `pwdgrp.cgi:add-user` is `risk_level: normal` → confirmation level `none` → ungated, while `remove-user` is `service-affecting`. This ADR adds a second add-user write per device and a root rotation. The gate that matters is onboarding's, not the operation's, so this is not a new hole — but the asymmetry #165 describes now covers more writes. **#296 is partly answered and should be reconciled, not closed by this.** Its part 2 asks for shared-vs-per-device as explicit first-class modes; `fleet_root_password` *is* the shared mode arriving for real, for `root` only, with `admz` permanently per-device. Its part 1 (encrypt `default_password` at rest) is already done. diff --git a/docs/specification/decisions/0071-a-task-raises-a-notice-the-console-delivers-it.md b/docs/specification/decisions/0071-a-task-raises-a-notice-the-console-delivers-it.md index 4d17a8d..85182ec 100644 --- a/docs/specification/decisions/0071-a-task-raises-a-notice-the-console-delivers-it.md +++ b/docs/specification/decisions/0071-a-task-raises-a-notice-the-console-delivers-it.md @@ -142,6 +142,17 @@ One PR (ADR-0070's PR 3); the two MCP tools ride ADR-0070's tool module. - One table, one router, one prompt slot, one strip; no new principal, no new gate, no change to the note mechanism ADR-0066 built. - The strip can briefly disagree with reality after a successful revert until the next check; stated above rather than hidden. +## Amendment 2026-09-23 — a third kind: `setup` + +A queued recovery (`queue_device_recovery`, ADR-0037's `reprovision` detection task) now raises a notice instead of provisioning. + +From ADR-0068 until this date it called `provision_factory_default(attended=False)`. That refused, because provisioning writes the fleet root password and must not do so unattended against whatever answers at the device's address. So every fired task failed, while the chat still offered "queue a re-provision" after a factory reset. ADR-0068 asked for this path to defer to an attended flow. A notice is that flow, built from this ADR's parts: + +- **Kind `setup`, subject `setup:`.** There is one live notice per device, whichever task noticed. Its title is *"Factory-reset — onboard it to set it up"*, its source is `recovery` ("a queued recovery"), and its severity is medium. +- **The review note says what onboarding will do**: the device came back factory-defaulted, ADMZ has no working credential for it, and onboarding sets `root` to the fleet root password and creates `admz` behind one approval. It carries identifiers only, like every note (§4). The prompt tells the model to answer it with `onboard_device`. +- **Any successful onboarding closes it** (`onboarded`): `provisioned`, `admz_account_created` or `already_credentialed`, through the same success hook that queues the capability survey. A gated or failed onboarding leaves it open. +- **The handler never reaches provisioning.** A test pins that on the parsed code, not the text. + ## What would falsify this If operators dismiss rather than review, the strip is noise and the fix is the trigger policy — quieter cadence, `notify_console=false` on the hourly audit — not a chattier assistant. If `occurrences` climbs into the hundreds on a device, the signature is too sensitive for a notice and the producer needs a debounce. If the `continuation_in_flight` refusal is never hit, the claim check is ceremony and the trailing-row predicate alone was enough. diff --git a/docs/specification/requirements/credential-storage.md b/docs/specification/requirements/credential-storage.md index ab5bddd..b9e3b4c 100644 --- a/docs/specification/requirements/credential-storage.md +++ b/docs/specification/requirements/credential-storage.md @@ -72,8 +72,10 @@ shipped 2026-09-14): - Only `admz` is stored. A root credential is never stored per device, so the generated password is never the device's only credential. - With no fleet root password set, provisioning refuses - (`root_password_not_configured`) and writes nothing. The unattended - `reprovision` handler always refuses (`unattended_not_permitted`). + (`root_password_not_configured`) and writes nothing. Nothing provisions + unattended. A queued recovery (`reprovision`) raises a `setup` notice for a + person to onboard the device (since 2026-09-23). `attended=False` still + refuses (`unattended_not_permitted`) for any caller that tries. - Returns a structured outcome; no password is ever echoed in the response. `provision_device(device_id | host)` runs the same onboarding resolution as diff --git a/docs/specification/requirements/fleet-health.md b/docs/specification/requirements/fleet-health.md index 38d1d8c..bb8ee63 100644 --- a/docs/specification/requirements/fleet-health.md +++ b/docs/specification/requirements/fleet-health.md @@ -74,7 +74,9 @@ The sweep **classifies and never resolves**: it does not re-run onboarding, try entry credentials, or open capture sessions (NFR-HLT-002; ADR-0034's one gate) — the one exception being a *pre-authorised* detection task (ADR-0037's `reprovision` on `on_needs_setup`), which is an approval deferred to a trigger, -not a sweep decision. Leaving the state is the operator's — capture from the device page or +not a sweep decision. Since 2026-09-23 even that one resolves nothing: it raises +a `setup` notice for a person to onboard the device (ADR-0068 forbids an +unattended write of the fleet root password). Leaving the state is the operator's — capture from the device page or the chat card — or a deliberate `onboard_device` re-run. The seven-hour trace that forced this: an A1210 registered without credentials read `online` on every surface until a baseline capture happened to need a password. diff --git a/docs/specification/user-stories/device-recovery.md b/docs/specification/user-stories/device-recovery.md index fa82104..76070d4 100644 --- a/docs/specification/user-stories/device-recovery.md +++ b/docs/specification/user-stories/device-recovery.md @@ -66,5 +66,5 @@ when the device is actually up but rejecting credentials. card yet — a long reboot relies on the assistant re-invoking with the `baseline_bootid`. The job-store + live-card design is open in #49 as v2. - 📋 **No automatic post-reboot reconciliation.** Recovery reports the device - is back; acting on a `needsetup` device (re-provisioning) is a separate, + is back; acting on a `needsetup` device (setting it up) is a separate, explicit follow-up. diff --git a/tests/e2e/MANUAL_recovery_tests.md b/tests/e2e/MANUAL_recovery_tests.md index fe6e4ec..08d86df 100644 --- a/tests/e2e/MANUAL_recovery_tests.md +++ b/tests/e2e/MANUAL_recovery_tests.md @@ -2,7 +2,9 @@ Covers the parts of the `needs_setup` / deferred-recovery feature (PR #70/#71) that the automated E2E suite can't safely or fully assert: the **UI rendering** -and the **real destructive re-provision** (which mutates a device). +and the **real factory reset → setup** (which mutates a device). Since +2026-09-23 a queued recovery never provisions by itself: it raises a `setup` +notice, and the device is set up through the chat, behind an approval. The automated coverage lives in `test_22_recovery_rest.py` (REST lifecycle + gates, no cost) and `test_23_recovery_chat.py` (chat intent-capture, Gemini cost). @@ -19,19 +21,19 @@ defaulted device in inventory (the lab Q3538-SLVE `B8A44F661A2F` @ 192.168.1.238 | A2 | The roster status counts / filters | A `needs_setup` group exists; the device is in it | | A3 | Open the device detail page → drift card | Says *"factory-defaulted (needs provisioning) — recover or decommission"*, not a wall of "removed" fields | | **B. Recovery card UI (safe — queue + cancel only)** | | | -| B1 | On a `needs_setup` device detail page | Amber banner: *"This device is factory-defaulted… Queue a re-provision below, or decommission it."* | -| B2 | Click **Queue re-provision** | Success message; the pending list shows *"⏳ reprovision · …expires in 24h"* with a **Cancel** button | +| B1 | On a `needs_setup` device detail page | Amber banner: *"This device is factory-defaulted (needs setup). Setting it up gives root the fleet root password and creates ADMZ's own admz account, after you approve. Or decommission it."* with a **Set it up** button | +| B2 | Click **Queue setup notice** | Success message; the pending list shows *"⏳ reprovision · …expires in 24h"* with a **Cancel** button | | B3 | Reload the page | The pending action is still listed (persisted, cross-process) | | B4 | Click **Cancel** on the pending action | It disappears from the list | | B5 | The **decommission** link in the banner | Triggers the existing Delete-device flow (its own confirm) — *don't confirm unless you intend to delete* | | **C. Chat intent-capture (Gemini cost)** | | | -| C1 | Chat: "I want to factory reset device ``" | Agent ASKS the follow-up (re-provision / remove / leave) before proceeding; gates the reset itself | -| C2 | Chat: "Queue a re-provision for `` when it comes back" | Agent calls `queue_device_recovery`, confirms it's armed (then cancel it: "cancel that recovery") | +| C1 | Chat: "I want to factory reset device ``" | Agent ASKS the follow-up (set it up again / remove / leave) before proceeding; gates the reset itself | +| C2 | Chat: "Set `` up again when it comes back from a reset" | Agent calls `queue_device_recovery` and says a Console notice will ask you to set it up — **not** that it will re-provision by itself. Then cancel it: "cancel that recovery" | | C3 | Chat: "What recovery actions are pending?" | Agent calls `list_device_recovery`, reports the queued one | -| **D. Real re-provision (DESTRUCTIVE — needs go-ahead + the live device)** | | | -| D1 | With the Q3538 in `needs_setup`, click **Queue re-provision** (or chat-queue it) | Pending action armed | -| D2 | Wait for the next health sweep (~60s) with the monitor enabled | Sweep fires the pending action **once**; audit log shows it attributed to the approver + "deferred-trigger" | -| D3 | Re-check the device health | Flips `needs_setup` → `online`; an admin account now exists (created with a generated per-device password — never displayed) | +| **D. Real factory reset → setup (DESTRUCTIVE — needs go-ahead + the live device)** | | | +| D1 | Factory-reset the Q3538, then click **Queue setup notice** (or chat-queue it) | Pending action armed | +| D2 | Wait for the next health sweep (~60s) with the monitor enabled | Sweep fires the pending action **once**; the Console strip shows *"Factory-reset — onboard it to set it up"*; the audit row names the notice (`notice_id`) and is attributed to the approver + "deferred-trigger". Nothing was written to the device | +| D3 | Click **Review in chat** on that notice, then approve the card the chat raises | The chat calls `onboard_device`; after approval the device has **two** accounts — `root` with the fleet root password (log in with it by hand to check) and `admz` — and flips to `online`; the notice closes | | D4 | Re-run the queue (fire-once check) | The fired action is gone; it does **not** fire again | | D5 | Audit/event log | No device password anywhere in the payloads | diff --git a/tests/e2e/test_23_recovery_chat.py b/tests/e2e/test_23_recovery_chat.py index 0429dce..82cc23b 100644 --- a/tests/e2e/test_23_recovery_chat.py +++ b/tests/e2e/test_23_recovery_chat.py @@ -20,6 +20,7 @@ "recover", "recovery", "decommission", "remove", "leave it", "afterward", "afterwards", "after the reset", "once it", "comes back", "when it returns", "when it comes back", "needs setup", "factory", "queue", + "set it up", "notice", ) diff --git a/tests/test_fleet_health.py b/tests/test_fleet_health.py index 5639323..80df1a6 100644 --- a/tests/test_fleet_health.py +++ b/tests/test_fleet_health.py @@ -1200,27 +1200,25 @@ class TestDeferredActionAuditRecordsPasswordSource: it from whichever code path happened to be live at the time. """ - def test_the_handler_carries_the_source_forward(self, monkeypatch): + def test_the_handler_names_its_notice_for_the_audit_row(self): + """Since 2026-09-23 a fired recovery raises a setup notice rather than + provisioning, so what the audit row can attribute is the notice it + raised — `notice_id`, already on the allow-list below.""" import asyncio from types import SimpleNamespace + from admz.fleet.health import _AUDITABLE_OUTCOME_KEYS from admz.tasks import handlers as h - async def fake_provision(*a, **kw): - return {"success": True, "password_source": "generated"} - - import admz.provisioning as prov - monkeypatch.setattr(prov, "provision_factory_default", fake_provision) - - task = SimpleNamespace(device_id="cam-01", device_ids=None, - action_params={}, action_type="reprovision") - ctx = SimpleNamespace( - registry=SimpleNamespace( - get_device_info=lambda d: {"host": "192.0.2.1"}), - catalog=None, executors={}) - out = asyncio.run( - h.get_task_handler("reprovision")(task, ctx)) - assert out["password_source"] == "generated" + task = SimpleNamespace(id="t-audit", device_id="cam-audit-01", + device_ids=None, action_params={}, + action_type="reprovision") + out = asyncio.run(h.get_task_handler("reprovision")( + task, SimpleNamespace(registry=None, catalog=None, executors={}))) + assert out["success"] is True + assert isinstance(out["notice_id"], int) + assert "notice_id" in _AUDITABLE_OUTCOME_KEYS + assert "password_source" not in out def test_the_audit_row_carries_it(self): """The other half — the handler's return used to be thrown away.""" @@ -1268,24 +1266,34 @@ def test_the_source_values_are_modes_not_secrets(self): "the fleet default_password is an entry credential; no provisioning " "mode may name it as something written to a device (FR-CRED-007)") - def test_the_deferred_handler_no_longer_emits_a_source_at_all(self): + def test_the_deferred_handler_never_reaches_provisioning(self): """ADR-0068's mitigation, pinned where #326's guard lives. This class exists because a fired reprovision created a credential and the audit row could not say which mode produced it. Under ADR-0068 the - unattended handler does not provision at all — it refuses, because the - value it would now write is the FLEET-WIDE break-glass root password and - the peer is unverified (#185/#326). So the forensic question this class - asks is answered a stronger way: there is no unattended provision to - attribute. The allow-list above stays as the guard for any handler that - starts returning an outcome again. + unattended handler must not provision at all: the value it would write + is the FLEET-WIDE root password and the peer is unverified (#185/#326). + So there is no unattended provision to attribute. + + Checked on the parsed code, not the text: the docstring names + ``provision_factory_default`` to explain why it is not called, and a + substring test would pass on that alone. """ + import ast import inspect + import textwrap from admz.tasks import handlers - src = inspect.getsource(handlers._run_reprovision) - assert "attended=False" in src, ( - "the unattended reprovision handler must declare itself unattended; " - "without it provision_factory_default writes the shared break-glass " - "root password to whatever answered") + tree = ast.parse(textwrap.dedent(inspect.getsource(handlers._run_reprovision))) + called = { + (n.func.id if isinstance(n.func, ast.Name) else getattr(n.func, "attr", "")) + for n in ast.walk(tree) if isinstance(n, ast.Call) + } + imported = {a.name for n in ast.walk(tree) + if isinstance(n, ast.ImportFrom) for a in n.names} + assert "setup_notice" in called, "the handler no longer raises the notice" + assert not ({"provision_factory_default", "write_root_account", + "adopt_with_admz_account"} & (called | imported)), ( + "the unattended recovery reaches provisioning again; it would write " + "the shared fleet root password to whatever answered") diff --git a/tests/test_provisioning.py b/tests/test_provisioning.py index 35a7847..ad97121 100644 --- a/tests/test_provisioning.py +++ b/tests/test_provisioning.py @@ -239,79 +239,83 @@ async def execute(self, op, device, creds, params): class TestReprovisionHandler: - @pytest.mark.asyncio - async def test_handler_provisions(self, monkeypatch): - from admz.fleet.pending_actions import execute_pending_action - from admz.recovery_actions import register_recovery_handlers + """A queued recovery that fires raises a ``setup`` notice and never + provisions (ADR-0068; changed 2026-09-23). + + Provisioning writes the fleet root password, and this handler fires + unattended against whatever answers at the device's address hours after + the approval (#185/#326). From ADR-0068 until 2026-09-23 it called + ``provision_factory_default(attended=False)``, which refused, so every fired + task failed while the chat still promised a re-provision. It now hands the + moment to a person: a Console notice that leads to an attended onboarding. + """ + + @staticmethod + def _live_setup(device_id): + from admz.notices.producers import setup_subject + from admz.notices.store import notices_store + return notices_store.get_live(setup_subject(device_id)) - called = {} + @pytest.mark.asyncio + async def test_it_raises_a_setup_notice_and_never_provisions(self, monkeypatch): + from admz.tasks.handlers import execute_task_action + from admz.tasks.store import Task - async def fake_provision(catalog, executors, registry, *, device_id, host, - username="root", attended=True): - called["args"] = (device_id, host, username) - called["attended"] = attended - return {"success": True} + async def must_not_run(*a, **k): + raise AssertionError("the unattended handler reached provisioning") monkeypatch.setattr( - "admz.provisioning.provision_factory_default", fake_provision - ) + "admz.provisioning.provision_factory_default", must_not_run) - class _Ctx: - registry = _Registry() - catalog = object() - executors = {} + out = await execute_task_action(Task( + id="t-setup-1", device_id="cam-setup-1", action_type="reprovision")) - register_recovery_handlers(_Ctx()) - await execute_pending_action({"action": "reprovision"}, "cam-1") - assert called["args"] == ("cam-1", "1.2.3.4", "root") - # ADR-0068 (was GH #185's `allow_fleet_default=False`): the unattended - # handler must declare itself unattended at the call site. Removing that - # declaration would make it silently start writing the fleet break-glass - # root password to whatever answered. - assert called["attended"] is False + assert out["success"] is True + notice = self._live_setup("cam-setup-1") + assert notice is not None and out["notice_id"] == notice.id + assert (notice.kind, notice.task_id) == ("setup", "t-setup-1") @pytest.mark.asyncio - async def test_handler_raises_on_provision_failure(self, monkeypatch): - from admz.fleet.pending_actions import execute_pending_action - from admz.recovery_actions import register_recovery_handlers + async def test_a_second_firing_bumps_the_same_notice(self): + from admz.tasks.handlers import execute_task_action + from admz.tasks.store import Task - async def fake_provision(*a, **k): - return {"success": False, "error": "device rejected user"} + first = await execute_task_action(Task( + id="t-a", device_id="cam-setup-2", action_type="reprovision")) + second = await execute_task_action(Task( + id="t-b", device_id="cam-setup-2", action_type="reprovision")) + assert first["notice_id"] == second["notice_id"] + assert self._live_setup("cam-setup-2").occurrences == 2 - monkeypatch.setattr( - "admz.provisioning.provision_factory_default", fake_provision - ) + @pytest.mark.asyncio + async def test_a_notice_store_failure_is_reported(self, monkeypatch): + """#455: a handler that cannot do its job says so, rather than + reporting a success the task row would then show as done.""" + from admz.tasks.handlers import execute_task_action + from admz.tasks.store import Task - class _Ctx: - registry = _Registry() - catalog = object() - executors = {} + def broken(*a, **k): + raise RuntimeError("no such table") - register_recovery_handlers(_Ctx()) - with pytest.raises(RuntimeError): - await execute_pending_action({"action": "reprovision"}, "cam-1") + monkeypatch.setattr("admz.notices.producers.setup_notice", broken) + out = await execute_task_action(Task( + id="t-c", device_id="cam-setup-3", action_type="reprovision")) + assert out["success"] is False + assert "notice not raised" in out["error"] @pytest.mark.asyncio async def test_unattended_reprovision_never_sends_a_SHARED_secret( self, monkeypatch ): - """GH #185/#326, end to end through the REAL provision_factory_default - (not mocked) — unlike the two tests above, which stub it out entirely - and so cannot see what actually reaches the (fake) device. - - Deliberately an outcome test, not an implementation test: it names no - keyword argument. If a future change "simplifies" the interactive and - unattended call sites back into one path, this goes red on the fact that - matters — a fleet-wide credential left the process bound for an address - ADMZ cannot verify. - - **Inverted by ADR-0068.** It used to assert the handler *generated* a - per-device password instead of sending the shared one. Now provisioning - writes the shared break-glass password by design, so the only safe - unattended behaviour is not to provision at all: the handler must fail - loudly and touch nothing. A spoofed peer on a reassigned DHCP lease - would otherwise walk away with a credential valid on every device ADMZ - has provisioned. + """GH #185/#326, end to end through the real wiring and the REAL + provision_factory_default (not mocked): with a fleet root password + configured, nothing leaves the process for the device — the queued + recovery raises a notice and touches nothing. + + Deliberately an outcome test, not an implementation test. If a future + change routes this handler back into provisioning, this goes red on the + fact that matters: a fleet-wide credential bound for an address ADMZ + cannot verify. """ from admz.fleet.pending_actions import execute_pending_action from admz.recovery_actions import register_recovery_handlers @@ -328,15 +332,39 @@ class _Ctx: executors = {"vapix": execr} register_recovery_handlers(_Ctx()) - with pytest.raises(RuntimeError, match="attended"): - await execute_pending_action({"action": "reprovision"}, "cam-1") + await execute_pending_action({"action": "reprovision"}, "cam-setup-4") assert execr.sent == [], ( - "the unattended reprovision handler contacted the device; under " - "ADR-0068 the next thing it would send is the fleet-wide " - "break-glass root password (GH #185/#326)") + "the unattended recovery contacted the device; under ADR-0068 the " + "next thing it would send is the fleet-wide root password " + "(GH #185/#326)") assert BREAK_GLASS_SECRET not in repr(execr.sent) assert reg.accounts == {} + assert self._live_setup("cam-setup-4") is not None + + @pytest.mark.asyncio + async def test_onboarding_closes_the_notice(self): + """The notice asks for an onboarding; a successful one answers it.""" + from admz.notices.producers import RESOLUTION_ONBOARDED, setup_notice + from admz.notices.store import notices_store + from admz.onboarding import PROVISIONED, _with_survey + + notice = setup_notice("cam-setup-5") + _with_survey({"status": PROVISIONED, "device_id": "cam-setup-5"}) + + assert self._live_setup("cam-setup-5") is None + closed = notices_store.get(notice.id) + assert (closed.status, closed.resolution) == ("handled", RESOLUTION_ONBOARDED) + + @pytest.mark.asyncio + async def test_a_gated_or_failed_onboarding_leaves_it_open(self): + """Control: only a success exit closes it.""" + from admz.notices.producers import setup_notice + from admz.onboarding import APPROVAL_REQUIRED, _with_survey + + setup_notice("cam-setup-6") + _with_survey({"status": APPROVAL_REQUIRED, "device_id": "cam-setup-6"}) + assert self._live_setup("cam-setup-6") is not None class TestAdoptWithAdmzAccount: diff --git a/tests/test_setup_notices.py b/tests/test_setup_notices.py new file mode 100644 index 0000000..64f0a1d --- /dev/null +++ b/tests/test_setup_notices.py @@ -0,0 +1,81 @@ +"""A factory-reset device the operator asked to recover raises a setup notice. + +Since ADR-0068 ADMZ never provisions unattended: provisioning writes the fleet +root password, and a queued recovery fires hours later against whatever +answers at the device's address. Until 2026-09-23 the queued task still tried, +was refused, and failed, while the chat promised a re-provision. It now raises a +``setup`` notice, the notice's review asks the chat to onboard the device (behind +the usual approval), and a successful onboarding closes it. + +These pin the words the model and the operator read, which are the other half +of the change. +""" + +from pathlib import Path + +import admz + + +def _src(rel): + return (Path(admz.__file__).parent / rel).read_text(encoding="utf-8") + + +class TestTheNotice: + def test_setup_is_a_notice_kind(self): + from admz.notices.store import KIND_SETUP, KINDS + assert KIND_SETUP == "setup" and KIND_SETUP in KINDS + + def test_one_live_notice_per_device(self): + from admz.notices.producers import setup_notice + a = setup_notice("cam-sn-1", task_id="t1") + b = setup_notice("cam-sn-1", task_id="t2") + assert a.id == b.id and b.occurrences == 2 + + def test_the_review_note_says_what_onboarding_will_do(self): + from admz.notices.notes import CLOSING, review_note + from admz.notices.producers import setup_notice + + note = review_note([setup_notice("cam-sn-2")]) + assert "device cam-sn-2 came back factory-defaulted" in note + assert "a queued recovery noticed it" in note + assert "fleet root password" in note and "'admz'" in note + assert "behind one approval" in note + assert note.endswith(CLOSING) + assert "\n" not in note + + def test_the_title_the_strip_shows(self): + from admz.notices.producers import setup_notice + assert setup_notice("cam-sn-3").title == "Factory-reset — onboard it to set it up" + + +class TestTheWordsTheModelReads: + def test_the_tool_no_longer_promises_a_provision(self): + from admz.mcp.tools.recovery import TOOLS + + tool = next(t for t in TOOLS if t.name == "queue_device_recovery") + assert "Console notice" in tool.description + assert "does NOT set the device up" in tool.description + assert "onboard_device" in tool.description + assert "generated" not in tool.description, ( + "the tool still promises a generated-password re-provision") + assert "username" not in tool.input_schema["properties"] + + def test_the_prompt_sends_a_needs_setup_device_to_onboarding(self): + src = _src("chatbot/system_prompt.py") + assert "Re-provision passwords are generated" not in src + assert "call `onboard_device(device_id)`" in src + assert "don't queue anything" in src + + +class TestTheWordsTheOperatorReads: + def test_the_device_page_offers_to_set_it_up(self): + page = _src("api/templates/device_detail.html") + assert "Queue re-provision" not in page + assert "Queue setup notice" in page + assert ">Set it up" in page + assert "create an admin with a generated password" not in page + + def test_the_tasks_page_labels_the_action(self): + page = _src("api/templates/tasks.html") + assert "reprovision: { label: 'Setup notice'" in page + assert "Creates the admin with a generated password" not in page diff --git a/tests/test_survey_provisioning_gate.py b/tests/test_survey_provisioning_gate.py index b539d18..60a4565 100644 --- a/tests/test_survey_provisioning_gate.py +++ b/tests/test_survey_provisioning_gate.py @@ -289,15 +289,26 @@ class TestTheLegitimateCallersAreUntouched: """The measured claim behind gating the entry points rather than the provisioning step. If any of these became gated, the design is wrong.""" - def test_the_scheduled_reprovision_task_reaches_provisioning_directly(self): - """The decisive one. Nothing can approve a widget on the scheduler's - behalf, so a gate here would not delay the write — it would fail it.""" + def test_the_scheduled_recovery_task_no_longer_provisions_at_all(self): + """This used to pin the opposite: that the scheduled reprovision reached + provisioning directly, because nothing can approve a widget on the + scheduler's behalf. ADR-0068 moved the line. An unattended write of the + fleet root password is refused outright, so since 2026-09-23 the task + raises a setup notice and a person onboards the device through the gated + path. It must neither provision nor raise a gate nobody can answer. + Checked on the parsed code: the docstring names the functions it avoids. + """ + import ast import inspect + import textwrap from admz.tasks import handlers - src = inspect.getsource(handlers._run_reprovision) - assert "provision_factory_default" in src - assert "gate_scan_write" not in src and "blocked" not in src + tree = ast.parse(textwrap.dedent(inspect.getsource(handlers._run_reprovision))) + names = {getattr(n.func, "id", getattr(n.func, "attr", "")) + for n in ast.walk(tree) if isinstance(n, ast.Call)} + assert "provision_factory_default" not in names + assert "gate_scan_write" not in names + assert "setup_notice" in names def test_the_rest_single_device_onboard_is_ungated(self): """The operator typed this device's address; intent is explicit and