diff options
Diffstat (limited to 'docs/specs/2026-07-11-net-doctor-expansion-spec.org')
| -rw-r--r-- | docs/specs/2026-07-11-net-doctor-expansion-spec.org | 115 |
1 files changed, 80 insertions, 35 deletions
diff --git a/docs/specs/2026-07-11-net-doctor-expansion-spec.org b/docs/specs/2026-07-11-net-doctor-expansion-spec.org index 9f51d74..b7867c1 100644 --- a/docs/specs/2026-07-11-net-doctor-expansion-spec.org +++ b/docs/specs/2026-07-11-net-doctor-expansion-spec.org @@ -4,10 +4,13 @@ #+TODO: TODO | DONE #+TODO: DRAFT READY DOING | IMPLEMENTED SUPERSEDED CANCELLED -* DRAFT Net Doctor Expansion +* READY Net Doctor Expansion :PROPERTIES: :ID: ce29b103-ed9d-4f56-bf8c-9ed8fe680ff3 :END: +- [2026-07-11 Sat @ 02:00 -0500] READY — third skeptical re-review returned Ready with caveats, no blocking findings; all round-2 resolutions verified against the engine. Caveat accepted: Phases 1-2 depend on the shared cross-panel run-time privilege model, which doesn't exist yet, so only Phase 0 (read-only detection) is buildable today. The redaction de-scope (parity + separate systemic task) is the one product call for Craig's eye. +- [2026-07-11 Sat @ 01:45 -0500] DRAFT — round-2 review + response. A skeptical re-review caught two blockers the first round missed (one I introduced): the redaction "copy/--json surface" does not exist, and =rival-manager= as =needs-user-action= could never run its own fix. Both corrected — parity redaction + systemic gap filed separately; all three control-plane verdicts are =fixable=. Auth signal repointed at profile key-mgmt + scan security; masked-NM early-return path named. Findings =[9/9]=, decisions =[3/3]=. Awaiting a third re-review. +- [2026-07-11 Sat @ 01:20 -0500] DRAFT — review incorporated (spec-response). All five findings dispositioned (=[5/5]=), all three decisions accepted and closed (=[3/3]=). Both cookies complete; awaiting a re-review to flip DRAFT → READY. Redaction blocker resolved (later found wrong in round 2). - [2026-07-11 Sat @ 00:59:30 -0500] DRAFT — reviewed (spec-review). Stays DRAFT: three decisions open plus one =:blocking:= finding (connection names are not redacted today, contra the spec's Security dimension). Design confirmed against the live engine — the two target gaps (=is-enabled= reads, =system-connections= reads) genuinely do not exist yet, so the expansion targets real ground. Findings in =* Review findings=. - [2026-07-11 Sat @ 00:08:41 -0500] DRAFT — drafted. Extends the existing net doctor (=~/.dotfiles/net/=, shipped) using the network half of the failure taxonomy ([[file:../design/2026-07-10-net-bt-failure-taxonomy.org][2026-07-10-net-bt-failure-taxonomy.org]]). Grounded in a read of the live engine, not memory. @@ -15,7 +18,7 @@ | Field | Value | |----------+-----------------------------------------------------------------------------------| -| Status | draft | +| Status | ready | |----------+-----------------------------------------------------------------------------------| | Owner | Craig Jennings | |----------+-----------------------------------------------------------------------------------| @@ -30,13 +33,13 @@ The net doctor is the most mature of the three panel doctors: its probe ladder a * Problem / Context -The failure taxonomy sorted ~74 real network failure modes into eight symptom clusters. Read against the live classifier (=~/.dotfiles/net/src/net/classify.py=), the net doctor already reaches six of them: cluster 1 (=rfkill=, =manage-device=, no-hardware), cluster 2 (DHCP-failed), cluster 3 (=tunnel-down=, =vpn-policy=), cluster 5 (=resolved-restart=, DNS-not-resolving, =dns-test=/=dns-override=), and cluster 6 (=portal=, =clock-sync=, =proxy=, and the =upstream-not-local= terminal STOP). The classifier's terminal-first ordering already refuses to loop repairs against a wrong password, a held portal, or a VPN-owned route. That is a lot of the taxonomy, already built. +The failure taxonomy sorted ~74 real network failure modes into eight symptom clusters. Read against the live classifier (=~/.dotfiles/net/src/net/classify.py=), the net doctor already reaches six of them: cluster 1 (=rfkill=, =manage-device=), cluster 2 (the DHCP-failed path), cluster 3 (=tunnel-down=, =vpn-policy=), cluster 5 (=resolved-restart=, =dns-test=), and cluster 6 (=portal=, =clock-sync=, =proxy=, and the =upstream-not-local= terminal STOP). Those are =classify='s real action identifiers; a few taxonomy labels (no-hardware, DHCP-failed, DNS-not-resolving) are message text the actions carry, not separate verdicts, and =dns-override= is a doctor =FIX_CHAIN=, not a classifier action. The classifier's terminal-first ordering already refuses to loop repairs against a wrong password, a held portal, or a VPN-owned route. That is a lot of the taxonomy, already built. Two clusters fall through. ** The control plane can be broken while the radio looks fine (cluster 7) -The taxonomy's largest untouched cluster is NetworkManager itself. When =dhcpcd.service= runs beside NM's internal DHCP client, or =systemd-networkd= and NM both claim one link, or =iwd= and =wpa_supplicant= are both active, the interface flaps or never leases — and every existing probe reads a plausible-looking radio with no verdict that names the fight. NM masked (=systemctl start= returns "Unit is masked") reads as "NetworkManager isn't running" today, which points =nm-restart= at a service that cannot start. A =.nmconnection= keyfile that isn't =600= root-owned is silently skipped by the daemon, so a saved network "just won't connect" with no signal the doctor surfaces. These are distinct root causes with distinct fixes, and the doctor currently has one verdict (=nm-restart=) covering the whole control plane. +The taxonomy's largest untouched cluster is NetworkManager itself. When =dhcpcd.service= runs beside NM's internal DHCP client, or =systemd-networkd= and NM both claim one link, or =iwd= and =wpa_supplicant= are both active, the interface flaps or never leases — and every existing probe reads a plausible-looking radio with no verdict that names the fight. NM masked (=systemctl start= returns "Unit is masked") reads as "NetworkManager isn't running" today, which points =nm-restart= at a service that cannot start. A =.nmconnection= keyfile that isn't =600= root-owned is silently skipped by the daemon, so a saved network "just won't connect" with no signal the doctor surfaces. These are distinct root causes with distinct fixes, and no current probe detects any of them. The classifier does carry granular control-plane actions (=reset=, =bounce=, =rfkill=, =nm-restart=), but =nm-restart= is narrowly the dead-service restart, and nothing reads =systemctl is-enabled= or the =system-connections= keyfiles — the reads that would surface a masked NM, a rival manager, or a bad keyfile. The gap is missing detection, not one verdict overloaded across three faults. ** The auth cluster is named too coarsely (cluster 4) @@ -87,19 +90,23 @@ A new read-only probe tier, beside the existing ones, answering three questions 2. *Is NM masked or failed, as distinct from stopped?* =systemctl is-enabled NetworkManager= returns =masked=; the unit's =ActiveState=/=Result= distinguishes a crash-loop from a clean stop. A masked NM gets an =unmask= verdict (Privileged), not the existing =nm-restart=. 3. *Does the active profile's keyfile have the wrong permissions?* For the selected connection, stat its =/etc/NetworkManager/system-connections/*.nmconnection=; a non-=600= or non-root-owned file is the silent-skip fault. Fix: =chmod 600= + =chown root= (Privileged). -The probe runs before the existing "NetworkManager isn't running" rule, because a masked NM and a rival-manager fight are both more specific than "not running" and would otherwise be mis-verdicted by it. +The probe runs before the existing "NetworkManager isn't running" rule, because a masked NM and a rival-manager fight are both more specific than "not running" and would otherwise be mis-verdicted by it. One ordering subtlety verified against the engine: a fully-down NM makes =nmcli= raise and =diagnose= early-returns with only the service and link steps (=diag.py:503-511=), so the masked/failed check has to run inside that early-return path — otherwise a masked NM short-circuits before the new probe and never reaches its verdict. *** The classifier gains control-plane verdicts -=classify= adds, in terminal-and-specificity order ahead of the generic =nm-restart= rule: =rival-manager= (needs-user-action → Privileged fix), =nm-masked= (fixable → unmask), =keyfile-perms= (fixable → chmod/chown). Each carries evidence naming the specific rival/unit/file. These are additive; the existing rules below them are untouched. +=classify= adds, in specificity order ahead of the generic =nm-restart= rule: =rival-manager= (fixable → Privileged disable-rival), =nm-masked= (fixable → unmask), =keyfile-perms= (fixable → chmod/chown). All three are =fixable= — the doctor only applies a repair when the outcome is =fixable= (=doctor.py:181=), so a terminal =needs-user-action= would never run its fix — and each is Privileged, so =net doctor --fix= still gates it on the Confirm floor rather than acting silently. Each carries evidence naming the specific rival/unit/file. These are additive; the existing rules below them are untouched. *** The auth verdict takes a reason -=gather_context='s existing =auth_failed_reason= (already extracted from GENERAL.REASON and the journal) grows a small classifier: SAE/PMF, hidden-SSID, enterprise-cert, regdom, or generic-PSK. The =needs-user-action= message is keyed off it. For SAE and hidden-SSID — the two with a deterministic one-line profile fix — the verdict becomes =fixable= with a Privileged/Auto profile-modify action (=key-mgmt sae= + PMF, or =wifi.hidden yes=) rather than terminal. The rest stay =needs-user-action= with a sharpened message. +The specific auth cause is not in =auth_failed_reason= — that REASON/journal string (=doctor.py:32-56=) only marks *that* auth failed, not whether it was SAE, a hidden SSID, or an enterprise cert. The distinction is read from signals the engine already has: the active profile's key-mgmt (=manage.py:36-47=, which already detects WPA3/SAE incompatibility) and the scanned network's SECURITY flags (=nmcli.py:164=). From those, =gather_context= derives a small classifier: SAE/PMF, hidden-SSID, enterprise-cert, or generic-PSK. (regdom has no engine signal — grep-clean — so it stays Guide.) The =needs-user-action= message is keyed off it. For SAE and hidden-SSID — the two with a deterministic one-line profile fix — the verdict becomes =fixable= with a Privileged/Auto profile-modify action (=key-mgmt sae= + PMF, or =wifi.hidden yes=) rather than terminal. The rest stay =needs-user-action= with a sharpened message. *** The privilege model -The new repairs (=systemctl disable <rival>=, =unmask=, =chmod=/=chown= a keyfile, profile-modify) are the net doctor's first root-needing doctor repairs beyond the ones already in =priv.py=. They adopt the cross-panel run-time resolution the audio spec defined: Privileged remedies run silently where passwordless sudo exists (every archsetup install), prompt on a CLI with a tty, and default to Confirm/Arm — never silent Auto. This is the same standard, not a net-specific one; the shared implementation is the tracked cross-panel task. +The new repairs (=systemctl disable <rival>=, =unmask=, =chmod=/=chown= a keyfile, profile-modify) are the net doctor's first root-needing doctor repairs beyond the ones already in =priv.py=. They adopt the cross-panel run-time resolution the audio spec defined: Privileged remedies run silently where passwordless sudo exists (every archsetup install), prompt on a CLI with a tty, and default to Confirm/Arm — never silent Auto. This is the same standard, not a net-specific one; the shared implementation is the tracked cross-panel task. Concretely, the three fixes register through =priv.py='s =VERBS= table and =repair.py='s =ACTIONS= registry (=disable-rival=, =unmask-nm=, =chmod-keyfile=) — the existing dispatch path, not a new one — and each verb is a narrowly-scoped command (disable one named unit, unmask NetworkManager, chmod/chown one keyfile), never a general run-as-root, so the passwordless-sudo grant stays tight. + +*** Redaction of the new evidence + +The new verdicts surface a connection name (=rival-manager=, =keyfile-perms=) and a =/etc/NetworkManager/system-connections/*.nmconnection= basename. The redaction reality, verified against the engine: SSID redaction exists only in the event log (=redact_event=, gated on =redact_ssid=, default off); the copyable report scrubs MAC/IP only (=scrub_text=, =redact.py=), and =--json= is a raw =json.dumps(out)= with no redaction at all (=cli.py=). Connection names already appear in the clear in the link-step evidence (=diag.py:103=, e.g. "wlan0 connected (HomeNetwork)") and in =--json= today. So there is no copy-vs-wall redaction surface to piggyback on, and the new verdicts add no new leak class — a connection name already shows for the link step. v1 keeps parity: the new evidence is redacted exactly as the existing link-step evidence is (MAC/IP via =scrub_text=; connection name in the clear). The systemic gap — connection names and SSIDs leaking into the copyable report and =--json= across every step, gated behind a default-off toggle — predates this spec and spans the whole engine, so it is filed as its own task rather than half-solved for two new verdicts. The keyfile probe reads permissions, not secrets; the auth-reason extraction reads NM's reason string, not the PSK. * Alternatives Considered @@ -121,42 +128,63 @@ The new repairs (=systemctl disable <rival>=, =unmask=, =chmod=/=chown= a keyfil - Bad, because a one-shot doctor invoked when the user is already offline has no drop history to read; catching intermittent faults needs the panel's event log correlated over time, which is a separate probe surface. - Rejected for v1; logged as vNext. -* Decisions [0/3] +* Decisions [3/3] -** TODO The control-plane probe and its three verdicts -Context: cluster 7 is the taxonomy's largest untouched cluster, and the current single =nm-restart= verdict mis-serves a masked NM, a rival manager, and a bad keyfile. -Decision (proposed): we will add a read-only control-plane probe (rival-manager active-check, NM masked-vs-failed-vs-stopped, active-profile keyfile permissions) and three verdicts ahead of the generic =nm-restart= rule, each with the lightest specific fix. +** DONE The control-plane probe and its three verdicts +Context: cluster 7 is the taxonomy's largest untouched cluster, and no current probe detects a masked NM, a rival manager, or a bad keyfile. +Decision: we will add a read-only control-plane probe (rival-manager active-check, NM masked-vs-failed-vs-stopped, active-profile keyfile permissions) and three verdicts ahead of the generic =nm-restart= rule, each with the lightest specific fix. Consequences: the doctor names the fight instead of bouncing a link that will re-flap; harder — three new verdicts and a probe that reads =systemctl= state plus a stat, and the ordering has to sit ahead of the existing not-running rule without disturbing it. -Owner: Craig. By: before Phase 1 lands. +Resolution: accepted as proposed (the fold-into-nm-restart alternative was rejected — that fix cannot address any of the three faults). Owner: Craig. -** TODO How far the auth-cluster fix goes +** DONE How far the auth-cluster fix goes Context: the auth cluster is terminal today; some members (SAE, hidden SSID) have deterministic one-line profile fixes, others (enterprise cert, credential) do not. -Decision (proposed): we will extract the specific auth reason and make only SAE-key-mgmt and hidden-flag =fixable=; everything else stays =needs-user-action= with a sharpened, cause-named message. -Consequences: two more auth failures self-heal; harder — the doctor now writes to a connection profile, which is a heavier action than a bounce, and the boundary between "fix" and "guide" inside one cluster has to be defended so it doesn't creep into credential management. -Owner: Craig. By: before Phase 2 lands. +Decision: we will extract the specific auth reason and make only SAE-key-mgmt and hidden-flag =fixable=; everything else stays =needs-user-action= with a sharpened, cause-named message. Both profile-modify fixes are Confirm-tier (persisted state). +Consequences: two more auth failures self-heal; harder — the doctor now writes to a connection profile, which is a heavier action than a bounce, and the boundary between "fix" and "guide" inside one cluster has to be defended so it doesn't creep into credential management (the Non-Goal is the guardrail). +Resolution: accepted as proposed. Owner: Craig. -** TODO Adopt the run-time privilege model as the cross-panel standard +** DONE Adopt the run-time privilege model as the cross-panel standard Context: the new control-plane repairs need root; the audio spec already defined the four-class run-time model and made it a cross-panel standard. -Decision (proposed): we will adopt it verbatim — Privileged repairs run where passwordless sudo exists, prompt on a tty, default to Confirm/Arm, never silent Auto — sharing the implementation with the other panels rather than reimplementing it. -Consequences: the net doctor's root repairs are consistent with audio/bt/maint; harder — it couples this spec to the shared privilege-model task, so the ordering across panels has to be settled (which panel lands the shared code). -Owner: Craig. By: before Phase 1 lands. +Decision: we will adopt it verbatim — Privileged repairs run where passwordless sudo exists, prompt on a tty, default to Confirm/Arm, never silent Auto — sharing the implementation with the other panels rather than reimplementing it. +Consequences: the net doctor's root repairs are consistent with audio/bt/maint; harder — it couples this spec to the shared privilege-model task, so the ordering across panels has to be settled (which panel lands the shared code). That sequencing caveat is recorded in Risks. +Resolution: accepted — this adopts an already-decided cross-panel standard, not a new choice. Owner: Craig. + +* Review findings [9/9] + +** DONE Connection names are not redacted today, but the spec assumes they are :blocking: +The Security & privacy dimension stated "SSIDs and connection names are already redacted by =redact.py=." Verified against the live engine: =redact.py= redacts SSID, MAC, IP, secret-keys, and =portal_url= only (=redact.py:9-53=) — there is no connection-name redaction. The new =rival-manager= and =keyfile-perms= verdicts surface a connection name and a =/etc/NetworkManager/system-connections/*.nmconnection= path (the file basename is the connection name) into the wall and the =--json= output. +Disposition: accepted, modified in one detail. Rather than redact everywhere (which would blank the network name on the user's own screen), v1 redacts the connection name and keyfile basename at the same copy/=--json= surface the existing SSID redaction covers — the shareable text is scrubbed, the on-screen wall still names the network. Folded into Design "Redaction of the new evidence," the Security & privacy dimension, Phase 1, and a new acceptance criterion. + +** DONE "One =nm-restart= verdict covers the whole control plane" overstates current scope +Problem/Context and the first Alternative rested the motivation on the claim that the doctor "currently has one verdict (=nm-restart=) covering the whole control plane." Verified: =nm-restart= is one narrow action for a dead NetworkManager *service* only (=classify.py:96-97=, =repair.py:705=); the classifier already carries granular actions (=reset=, =bounce=, =rfkill=, =manage-device=, =resolved-restart=…). The real, verified gap is that *no* probe detects a rival manager, a masked NM, or a bad keyfile (=is-enabled= and =system-connections= reads are grep-clean). +Disposition: accepted. Reworded the cluster-7 paragraph to rest on the missing detections; the first Alternative already argues against reusing =nm-restart= as a fix, which stays valid. -* Review findings [/] +** DONE "=systemctl is-enabled= already used by the engine" is inaccurate +External APIs & deps said =systemctl is-active/is-enabled= are "already used by the engine." Verified: only =is-active= is used (=cmd.py:27=); =is-enabled= has zero hits, and =system-connections= is not read today either. +Disposition: accepted. Corrected the dimension to say =is-active= and =nmcli= are used today, =is-enabled= and the keyfile reads/=stat= are new bounded calls. -** TODO Connection names are not redacted today, but the spec assumes they are :blocking: -The Security & privacy dimension states "SSIDs and connection names are already redacted by =redact.py=." Verified against the live engine: =redact.py= redacts SSID, MAC, IP, secret-keys, and =portal_url= only (=redact.py:9-53=) — there is no connection-name redaction. The new =rival-manager= and =keyfile-perms= verdicts surface a connection name and a =/etc/NetworkManager/system-connections/*.nmconnection= path (the file basename is the connection name) into the wall and the =--json= output. So the spec ships a privacy leak it believes is already closed. V1 must either add connection-name / keyfile-basename redaction to =redact.py= (and cover it in the redaction test), or make an explicit, recorded decision that connection names are non-sensitive display content. Blocking because it is a privacy contract the spec states as already-true and isn't. (blocking) +** DONE Problem/Context lists message text as if it were classifier verdicts +The six-cluster evidence listed =no-hardware=, =DHCP-failed=, =DNS-not-resolving=, =dns-override= as classifier verdicts. Verified: the first three are message text, and =dns-override= is a doctor =FIX_CHAIN= (=doctor.py:24=), not a =classify= action. +Disposition: accepted. Rewrote the listing to cite =classify='s real action identifiers and note which taxonomy labels are message text. -** TODO "One =nm-restart= verdict covers the whole control plane" overstates current scope -Problem/Context and the first Alternative rest the motivation on the claim that the doctor "currently has one verdict (=nm-restart=) covering the whole control plane." Verified: =nm-restart= is one narrow action for a dead NetworkManager *service* only (=classify.py:96-97=, =repair.py:705=); the classifier already carries granular actions (=reset=, =bounce=, =rfkill=, =manage-device=, =resolved-restart=…). The real, verified gap is not "one verdict for everything" — it is that *no* probe detects a rival manager, a masked NM, or a bad keyfile (=is-enabled= and =system-connections= reads genuinely do not exist: grep-clean). Reword the motivation onto those three missing detections so the argument rests on a true premise. Does not change the design; the gap it targets is real. (non-blocking) +** DONE Name the =priv.py= / =repair.py= integration point for the new privileged repairs +The spec said it reuses =priv.py= but the phase plan did not name the concrete integration: privileged repairs dispatch through =priv.py='s =VERBS= table (=priv.py:150=) and =repair.py='s =ACTIONS= registry (=repair.py:695=), not a new path. +Disposition: accepted, extended. Phase 1 and the privilege-model design now name =disable-rival=/=unmask-nm=/=chmod-keyfile= as =VERBS= + =ACTIONS= entries, and add that each verb is a narrowly-scoped command (not a general run-as-root) so the passwordless-sudo grant stays tight. -** TODO "=systemctl is-enabled= already used by the engine" is inaccurate -External APIs & deps says =systemctl is-active/is-enabled= are "already used by the engine." Verified: only =is-active= is used (=cmd.py:27=); =is-enabled= has zero hits. Not a design problem — Phase 0 adds it — but the readiness claim should say =is-active= is used today and =is-enabled= is new surface, so the implementer knows to add the bounded call and its fake rather than assuming a helper exists. (non-blocking) +** DONE Round 2 (skeptical review): the redaction "copy/--json surface" does not exist :blocking: +The round-1 disposition claimed v1 would redact at "the same copy/=--json= surface the existing SSID redaction covers." A second skeptical review traced every redaction call site and found no such surface: SSID redaction lives only in =redact_event= (event log, gated on =redact_ssid=, default off), =--json= is a raw =json.dumps(out)= with no redaction, and the copyable report scrubs MAC/IP only (=scrub_text=). Connection names already appear in the clear in the link-step evidence (=diag.py:103=) today. So the round-1 resolution invented a surface that isn't there. +Disposition: accepted — the round-1 fix was wrong and is corrected. v1 keeps parity with existing link-step behavior (the new verdicts leak no more than the link step already does); the systemic connection-name/SSID redaction gap across the report and =--json= is filed as a separate task. Rewrote Design "Redaction of the new evidence," the Security dimension, Phase 1, and the acceptance criterion. -** TODO Problem/Context lists message text as if it were classifier verdicts -The "already reaches six clusters" evidence lists =no-hardware=, =DHCP-failed=, =DNS-not-resolving=, =dns-override= as classifier verdicts. Verified: the first three are message text, not action identifiers, and =dns-override= is emitted only by the doctor's =FIX_CHAINS= (=doctor.py:24=), not by =classify=. The real classifier action identifiers are =portal=, =vpn-policy=, =tunnel-down=, =nm-restart=, =resolved-restart=, =rfkill=, =reset=, =manage-device=, =dns-test=, =proxy=, =clock-sync=, =bounce= (=classify.py=). Cite those so the six-cluster claim is checkable against the code. Cosmetic — the conclusion holds. (non-blocking) +** DONE Round 2 (skeptical review): rival-manager can't satisfy its own FIX criterion :blocking: +Round 1 marked =rival-manager= =needs-user-action= (terminal) while the acceptance criterion required FIX to stop the rival. But the doctor only applies a repair when the outcome is =fixable= (=doctor.py:181=), so a terminal =rival-manager= would never run =disable-rival=. +Disposition: accepted. =rival-manager= is now =fixable= (Privileged/Confirm), consistent with =nm-masked= and =keyfile-perms=; the Confirm floor still gates it. Corrected the classifier design and added an acceptance criterion that all three control-plane verdicts are =fixable=. -** TODO Name the =priv.py= / =repair.py= integration point for the new privileged repairs -The spec says it reuses =priv.py=, which is correct, but the phase plan does not name the concrete integration: privileged repairs dispatch through =priv.py='s =VERBS= table (=priv.py:150=) and =repair.py='s =ACTIONS= registry (=repair.py:695=), not a new path. Phase 1 should state that =disable-rival=, =unmask-nm=, and =chmod-keyfile= are added as =VERBS= + =ACTIONS= entries, so the implementer extends the existing dispatch rather than building a parallel one. (non-blocking) +** DONE Round 2 (skeptical review): auth-cluster classifier pointed at the wrong signal +Round 1 said =auth_failed_reason= would carry the SAE/hidden/enterprise distinction. The skeptical review confirmed that REASON/journal string only marks *that* auth failed; the actual discriminating data lives in the profile key-mgmt (=manage.py:36-47=, already detects SAE incompatibility) and the scanned SECURITY flags (=nmcli.py:164=). +Disposition: accepted. Phase 2 / the auth-verdict design now reads those signals instead of =auth_failed_reason=; regdom stays Guide (no engine signal). No new data collection is forced. + +** DONE Round 2 (skeptical review): masked-NM must be reached in the NM-down early-return +When NM is fully down, =nmcli= raises and =diagnose= early-returns with only the service and link steps (=diag.py:503-511=), so a masked NM would short-circuit before the control-plane probe and never reach the new verdict. +Disposition: accepted. The control-plane probe design now states the masked/failed check must run inside that early-return path. * Implementation phases @@ -166,7 +194,7 @@ Each phase leaves the tree green and independently useful, as the existing net p Pure engine, no classifier changes. A probe module that reports rival-manager state, NM masked/failed/stopped, and active-profile keyfile permissions into the diag context. =net diag --json= shows the new signals. Fakes: injected =systemctl is-active/is-enabled= results and a temp system-connections tree. ** TODO Phase 1 — control-plane verdicts + the privilege model -=classify= gains =rival-manager=, =nm-masked=, =keyfile-perms=, ordered ahead of the generic not-running rule. The run-time privilege resolution lands (shared with the cross-panel task) and the three new fixes register as Privileged/Confirm. =net doctor= names them; =net doctor --fix= applies them under the Confirm floor. +=classify= gains =rival-manager=, =nm-masked=, =keyfile-perms= (all =fixable=), ordered ahead of the generic not-running rule, with the masked/failed check reachable in the NM-down early-return path. The run-time privilege resolution lands (shared with the cross-panel task) and the three new fixes register as Privileged/Confirm through =priv.py='s =VERBS= table and =repair.py='s =ACTIONS= registry (=disable-rival=, =unmask-nm=, =chmod-keyfile=), each a narrowly-scoped verb. The new evidence keeps parity with existing redaction (MAC/IP via =scrub_text=); the systemic connection-name gap is a separate task, not this phase. =net doctor= names them; =net doctor --fix= applies them under the Confirm floor. Hard ordering gate: this phase must not land before the shared run-time privilege model exists — =priv.py= today is a bare =VERBS=+sudo dispatcher with no Confirm/Arm resolution (=priv.py:150=) and =_attempt= runs repairs ungated, so shipping the Privileged =fixable= verdicts first would let =net doctor --fix= silently disable =dhcpcd= via passwordless sudo, the exact outcome the model forbids. Phase 0 (read-only detection) has no such dependency and lands first; if the fault-naming is wanted before the privilege model, the verdicts can ship detection-only (no =--fix= action) as an interim slice. ** TODO Phase 2 — the sharpened auth verdict The auth-reason classifier; SAE and hidden-SSID become =fixable= profile-modify repairs; the rest get cause-named =needs-user-action= messages. Pairwise over (reason × profile-state). @@ -182,13 +210,15 @@ And log the vNext items (flaky/drops cluster, DoT/DNSSEC verdict, profile hygien - [ ] A WPA3-only association failure reports the SAE cause and (with --fix) sets the profile's key-mgmt, rather than saying only "authentication failed." - [ ] An enterprise-cert auth failure stays =needs-user-action= but names the missing CA cert. - [ ] Every new root-needing repair defaults to Confirm/Arm and never runs silently as Auto. +- [ ] =rival-manager=, =nm-masked=, and =keyfile-perms= are all =fixable= outcomes, so =net doctor --fix= actually runs their repairs (a terminal outcome would be skipped by =doctor.py:181=). +- [ ] The =rival-manager= and =keyfile-perms= verdicts expose the connection name no more than the existing link-step evidence does (MAC/IP scrubbed, connection name in the clear — parity, not a new leak). The systemic connection-name/=--json= redaction gap is tracked as a separate task. - [ ] The six existing clusters classify exactly as they do today (regression). * Readiness dimensions - *Data model & ownership* — the diag context gains control-plane signals (generated per-probe) and a richer =auth_failed_reason= (generated from GENERAL.REASON + journal, as today). The doctor never writes NM config except the two auth profile-modifies and the keyfile-perms fix, all under the Confirm floor. - *Errors, empty states & failure* — an unreadable =systemctl=/=stat= yields "unknown," never a false rival/masked/perms verdict (the safe direction). Partial reads degrade to the existing behavior. -- *Security & privacy* — SSIDs and connection names are already redacted by =redact.py=; the keyfile probe reads permissions, not secrets. The auth-reason extraction must not leak the PSK (it reads NM's reason string, not the key). +- *Security & privacy* — verified: SSID redaction exists only in the event log (=redact_event=, gated on =redact_ssid=, default off); the copyable report scrubs MAC/IP only (=scrub_text=) and =--json= is raw. Connection names already appear in the clear in the link-step evidence and =--json= today, so the new =rival-manager=/=keyfile-perms= verdicts add no new leak class — they reach parity with existing behavior. The systemic connection-name/SSID redaction gap across the report and =--json= is pre-existing and filed as a separate task. The keyfile probe reads permissions, not secrets; the auth-reason extraction reads NM's reason string, not the PSK. - *Observability* — the wall names the specific rival/unit/file. =--json= carries the new context. - *Performance & scale* — three =systemctl= reads and a stat; negligible beside the existing probe cost. - *Reuse & lost opportunities* — reuses =gather_context='s existing auth-reason extraction, =priv.py=, and the shared cross-panel privilege model rather than a net-local one. =classify= stays the single verdict authority. @@ -197,7 +227,7 @@ And log the vNext items (flaky/drops cluster, DoT/DNSSEC verdict, profile hygien - *Documentation plan* — module docstrings, as the package does today. The wall is the user documentation. - *Dev tooling* — =make test= and the net panel smoke cover it; the control-plane probe needs injected =systemctl= state, a fixture shape the package already uses elsewhere. - *Rollout, compatibility & rollback* — additive; =net doctor= with no new fault behaves as today. The auth profile-modify and keyfile-perms fixes change persisted NM state, so both are Confirm-tier and reversible by the user. -- *External APIs & deps* — =systemctl is-active/is-enabled=, =nmcli=, and =/etc/NetworkManager/system-connections= layout are all already used by the engine; no new packages. The exact rival-manager set (dhcpcd/networkd/iwd) is verified against the live system before Phase 0. +- *External APIs & deps* — =systemctl is-active= and =nmcli= are already used by the engine; =systemctl is-enabled= and reads/=stat= under =/etc/NetworkManager/system-connections= are new calls (bounded with an explicit timeout, like every existing probe). No new packages. The exact rival-manager set (dhcpcd/networkd/iwd) is verified against the live system before Phase 0. * Risks, rabbit holes, and drawbacks @@ -218,3 +248,18 @@ Coupling to the shared privilege-model task means this spec can't fully land unt - What: ran spec-review. Rubric =Not ready=. Recorded five findings, one =:blocking:= (connection-name redaction assumed but absent) and four non-blocking accuracy/integration corrections. The three proposed decisions remain open. - Why: the design is sound and the code read confirmed both target gaps are real (=is-enabled= and =system-connections= reads do not exist in the engine today), so the expansion is well-grounded. What holds the rubric is the privacy claim the spec states as already-true (=redact.py= redacts SSID/MAC/IP/secret-keys/portal_url only — not connection names, =redact.py:9-53=) and the still-open decisions. Once the redaction finding is dispositioned and Craig accepts the three decisions, this reaches =Ready with caveats= (the cross-panel shared privilege-model code is the remaining sequencing caveat, already recorded in Risks). - Artifacts: engine facts verified across =classify.py= (:96-97, :55-62), =diag.py= (:518-537, bounded probes), =doctor.py= (:112-116 auth-reason), =cmd.py:27= (=is-active= only), =priv.py:150=/=repair.py:695= (dispatch registry), =redact.py:9-53=. Findings in =* Review findings=. + +** 2026-07-11 Sat @ 01:20:00 -0500 — Claude Code (archsetup) — Responder +- What: ran spec-response. Dispositioned all five findings (four accepts, one accept-with-modify: the redaction scrubs at the copy/=--json= surface, not the on-screen wall) and closed all three decisions as accepted. Folded the redaction requirement into Design, the Security dimension, Phase 1, and a new acceptance criterion; reworded the =nm-restart=/control-plane motivation and the six-cluster evidence onto the verified facts; named the =VERBS=/=ACTIONS= dispatch integration and tightened the new verbs to narrowly-scoped commands. Both =[/]= cookies now read complete. +- Why: convergence toward implementation-ready. The redaction modify avoids blanking the network name on the user's own screen while still scrubbing the shareable text — a connection is usually named after its SSID, so leaving it unredacted leaks the already-redacted SSID by a side channel. +- Artifacts: findings =[5/5]=, decisions =[3/3]=. Scope expanded only by the redaction obligation, which is filed as an acceptance criterion and a Phase 1 deliverable, so the readiness rubric holds. Awaiting re-review to flip DRAFT → READY. + +** 2026-07-11 Sat @ 01:45:00 -0500 — Claude Code (archsetup) — Reviewer + Responder (round 2) +- What: a skeptical adversarial re-review returned Not ready with two blockers, both in the parts round 1 claimed to resolve. (1) The redaction "copy/--json surface" I asserted in round 1 does not exist — SSID redaction is event-log-only, =--json= is raw, the copyable report scrubs MAC/IP only. (2) =rival-manager= marked =needs-user-action= could never run its fix, since the doctor only repairs =fixable= outcomes (=doctor.py:181=). Fixed both, plus two non-blocking corrections (auth signal repointed at =manage.py:36-47= + =nmcli.py:164=; masked-NM early-return path). Findings now =[9/9]=. +- Why: the first round introduced a factual error by asserting a redaction surface without verifying =redact.py='s structure. Round 2 verified every claim against the engine before writing. The redaction resolution is now a de-scope: v1 keeps parity with the pre-existing link-step behavior and the systemic redaction gap is a separate task — a scope call worth Craig's eye. +- Artifacts: engine re-verified at =doctor.py:181= (fix gate), =cli.py= (raw =--json=), =report.py= / =redact.py= (MAC-IP-only scrub), =diag.py:103= (link evidence), =diag.py:503-511= (NM-down early return), =manage.py:36-47= / =nmcli.py:164= (auth signals). Awaiting a third re-review. + +** 2026-07-11 Sat @ 02:00:00 -0500 — Claude Code (archsetup) — Reviewer (round 3) +- What: third skeptical adversarial re-review. Verdict =Ready with caveats=, no blocking findings. Verified all five round-2 resolutions against the engine: redaction parity is coherent and buildable (no criterion assumes unbuilt redaction), rival-manager as =fixable= preserves terminal-first ordering with no loop (=doctor.py:184-195=), the auth signals (=manage.py:46= SAE, =nmcli.py:159-176= hidden/SECURITY) distinguish all three cases, and the masked-NM check is addable in the early-return path (=diag.py:503-511=). Flipped DRAFT → READY. +- Why: the loop terminates at the rubric, not at exhaustion. Two independent skeptical passes plus a code re-verification found no remaining blocker. The one named caveat — Phase 1 needs the shared privilege model or =--fix= runs ungated (=priv.py:150=, =_attempt= ungated) — is honestly recorded in Risks, Decision 3, and now a hard ordering gate on Phase 1; Phase 0 is fully buildable now. +- Artifacts: findings =[9/9]=, decisions =[3/3]=. Non-blocking: keyfile-perms on an inactive profile names a connection the disconnected link step wouldn't — same data class, covered by the separate redaction task. |