Coverage gate, smart installer, and sudo-less CLI path - #38
Merged
Conversation
Three-part hardening pass: - Test quality + coverage gate: a checked-in .testcoverage.yml enforced by `task test:cover` and CI, poll-based tests replacing time.Sleep, t.Parallel across the pure packages, an in-process CLI harness, and blockPlan/ parseOverrides extracted so privileged commands have a testable pure core. - Smart installer: scripts/install.sh gains a TTY-gated interactive menu (state-aware: fresh/upgrade/reinstall), [n/N] progress, and a full uninstall flow. `dezhban upgrade can-activate` closes a real gap where the installer's upgrade path could restart a running daemon through FULL BLOCK, violating ADR-0007 — no override, matching `sudo dezhban restart` as the deliberate escape hatch. Piped installs (`curl | sudo bash`) are unchanged. - Sudo-less CLI path: docs/usage/passwordless.md walks through pointing control.group at the host's existing admin group so day-to-day ops (block/unblock/switch/pause/resume/hold) need no password once configured; `dezhban doctor` gained a control check; the page is bundled into the macOS app's Help pane; and sudo is trimmed from read-only/socket-routed command examples across the user-facing docs (frozen docs, contributor checklists, and curl | sudo bash lines are untouched by design). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Behnam-RK
added a commit
that referenced
this pull request
Jul 28, 2026
- install.sh: gate interactivity on `-t 0` (stdin), not `-t 1` (stdout). A piped `curl | sudo bash` still has the terminal as its STDOUT, so the old test made the advertised one-liner install stop at a menu — the exact opposite of what the script header, install.md, and the changelog all promise. stdin is the only stream that distinguishes the two invocations. - install.sh: install the new CLI BEFORE asking `upgrade can-activate`, and ask the newly-installed binary. `can-activate` ships for the first time in this same change, so asking the previously-installed binary meant every upgrade from an older release answered "unknown subcommand", refused the gate, and silently never restarted a running daemon. Installing first is safe and already documented as such (the old daemon keeps its inode) — this also makes upgrade.md's description of the sequence literally true. - passwordless.md: `control.group` is NOT empty by default on macOS, where it is "admin". Said the opposite, on a page bundled into the macOS app's own Help pane, and contradicted cli.md in the same PR. - harness_test.go: drain the captured stdout/stderr pipes from goroutines started before run(), not with ReadAll after it returns. A pipe holds only a fixed kernel buffer, so any command printing more than that would have hung the test forever. Also closes the read ends. - ci.yml / Taskfile.yml: pin go-test-coverage to v2.19.0 instead of @latest — the gate decides whether CI goes red, so upstream must not be able to change that verdict with no commit here. Install unconditionally, so the pin isn't silently defeated by a stale binary already on PATH. - CHANGELOG: the sudo trim touched neither README.md nor docs/concepts/; say what it actually changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- install.sh: gate interactivity on `-t 0` (stdin), not `-t 1` (stdout). A piped `curl | sudo bash` still has the terminal as its STDOUT, so the old test made the advertised one-liner install stop at a menu — the exact opposite of what the script header, install.md, and the changelog all promise. stdin is the only stream that distinguishes the two invocations. - install.sh: install the new CLI BEFORE asking `upgrade can-activate`, and ask the newly-installed binary. `can-activate` ships for the first time in this same change, so asking the previously-installed binary meant every upgrade from an older release answered "unknown subcommand", refused the gate, and silently never restarted a running daemon. Installing first is safe and already documented as such (the old daemon keeps its inode) — this also makes upgrade.md's description of the sequence literally true. - passwordless.md: `control.group` is NOT empty by default on macOS, where it is "admin". Said the opposite, on a page bundled into the macOS app's own Help pane, and contradicted cli.md in the same PR. - harness_test.go: drain the captured stdout/stderr pipes from goroutines started before run(), not with ReadAll after it returns. A pipe holds only a fixed kernel buffer, so any command printing more than that would have hung the test forever. Also closes the read ends. - ci.yml / Taskfile.yml: pin go-test-coverage to v2.19.0 instead of @latest — the gate decides whether CI goes red, so upstream must not be able to change that verdict with no commit here. Install unconditionally, so the pin isn't silently defeated by a stale binary already on PATH. - CHANGELOG: the sudo trim touched neither README.md nor docs/concepts/; say what it actually changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Behnam-RK
force-pushed
the
feat/coverage-installer-passwordless
branch
from
July 29, 2026 07:59
bff11eb to
17b25ef
Compare
doctor's new control: check violated doctorCheck.Fixes' own contract
("commands or actions ... never prose about them — the GUI badges each
one") by putting "see docs/usage/passwordless.md" in Fixes on three
branches, and dropped sudo from `config set control.enabled true` — a
command this PR's own policy says must keep it. The doc pointer moves to
Details, the group branches get a real runnable `config set control.group`
line chosen by GOOS, and the forbidden branch carries prose with no Fix at
all, since add-to-group is usermod on Linux and dseditgroup on macOS.
A table-driven case now checks every branch, not just the exercised ones.
Also: runDoctor probed the socket even with control.enabled=false, where
buildControlCheck discards the result — controlReachable and controlStatus
already short-circuit the same way.
docs/usage/passwordless.md's "what still needs root" omitted config
set/edit/preset apply — the very command the page opens with — so a reader
would read its own `sudo` as a typo.
CI ran the full suite three times on ubuntu (plain, race, coverage). The
profile is now produced by the matrix-wide run and only GATED on ubuntu;
-race stays the one uninstrumented run. .testcoverage.yml now records that
its floors are LINUX numbers, since internal/firewall and internal/netdetect
are build-tagged and a Mac measures different files than the gate does.
Test-comment corrections: config.Default() does carry geo providers (the
claim that it doesn't is contradicted by TestCmdMonitorNoProviders in the
same file), and runCLI's os.Stdout swap forbids t.Parallel() package-wide,
not just in harness_test.go. scriptedWatcher.send now fails by name instead
of silently giving up at its deadline. Installer prints "next steps:" after
the setup wizard rather than above it, and says why SHA256SUMS bypasses dl().
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
install.sh's `start` after an upgrade stop was unguarded: under `set -e` a failure aborted the script silently, after the stop had already torn down every firewall rule — a completed run left the host unprotected with no message and no reason to suspect it. It now dies with a warning naming the exposure and the fix. This one predates the PR (it is on main too), hence the CHANGELOG entry. The optional setup wizard was unguarded the same way, so cancelling it swallowed the "next steps" footer and the uninstall hint; a cancelled wizard now also restores the `dezhban setup` line to that footer. The installer decides whether to restart a daemon by grepping `status --json` for a string owned by internal/svc — a cross-language contract with no compiler behind it, whose silent failure mode is telling the user "the service was not running" while their old build keeps running. Extracted svc.StatusInstalledRunning and pinned both sides with a test (verified it fails when either side drifts). Nits: recovery_test.go's send now uses pollUntil, the helper this same PR added to the same package, instead of a hand-rolled deadline loop; cli.md's link label said docs/upgrade.md for docs/usage/upgrade.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Enforce the "no t.Parallel() in package main" invariant instead of only documenting it above runCLI. It is the one rule in that package a compiler cannot keep, and this PR made it easier to break both ways: it added a new test file there and it wrote the unit-test policy that now asks for t.Parallel() BY DEFAULT. A violation races two goroutines on the global os.Stdout/os.Stderr and silently redirects one test's output into another's pipe — nondeterministic, and it lands on whoever runs the suite next rather than on the author. TestNoTestInPackageMainIsParallel walks each test file's AST (not a text scan, which would flag the files that merely describe the rule) and names file:line. Verified to fail on an injected violation. - Name that exception in docs/contribute/testing.md's t.Parallel() bullet. The policy already exempts "process-global state" in the abstract; the one package where that actually bites should say so, especially since — unlike t.Setenv, whose own panic is the tell — nothing here fails on its own. - countingHandler now holds an *atomic.Int64 rather than a *int64 driven by atomic.AddInt64/LoadInt64, matching countingMonitor and scriptedWatcher in the same package (both added by this PR). The typed form is the one that cannot be read non-atomically by accident. Also avoided parser.ParseDir in the new guard: it is deprecated in favour of golang.org/x/tools/go/packages, and a test does not get to add a dependency. Verified: build + vet (darwin/linux/windows), go test ./... 755 passed in 26 packages, same under -race, coverage gate PASS (total 60.2%). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three-part hardening pass, negotiated and implemented end to end:
.testcoverage.ymlenforced bytask test:coverand CI, shaped to real measured coverage (95% default is unrealistic for OS-boundary packages likefirewall/svc/netdetect, so those get documented, justified floors instead). Replaced six blindtime.Sleepwaits with bounded polling, addedt.Parallel()across the pure packages, added an in-processcmd/dezhban/harness_test.gocovering the CLI's read-only surface (previously untested), and extractedblockPlan/parseOverridesso privileged commands have a testable pure core.scripts/install.shgains a TTY-gated interactive menu (state-aware: fresh/upgrade/reinstall),[n/N]progress, and a full uninstall flow (typed confirmation, keep-config default yes). Piped installs (curl | sudo bash) are byte-for-byte unchanged. Closes a real gap: the installer's upgrade path could previously restart a running daemon through FULL BLOCK, violating ADR-0007 —dezhban upgrade can-activategates it now, with no override, matchingsudo dezhban restartas the deliberate escape hatch.docs/usage/passwordless.mdwalks through pointingcontrol.groupat the host's existing admin group (sudo/wheel/admin) so day-to-day ops (block/unblock/switch/pause/resume/hold) need no password once configured — grants no new authority, since anyone who can alreadysudois already a member.dezhban doctorgained acontrol:check, the page is bundled into the macOS app's Help pane, andsudois trimmed from read-only/socket-routed command examples across the user-facing docs (ADRs, contributor checklists, and everycurl | sudo bashline are untouched by design —config set/edit/preset applyalso deliberately keepsudo, since that path needs an enrolled control token, not just group membership).Test plan
go build ./.../go vet ./.../GOOS=windows go vet ./...go test ./... -race -count=1— 746 passedtask test:cover— package (60%) and total (55%) thresholds satisfiedgo test ./internal/help ./internal/vocab ./internal/config— bundled-page constraints, glossary lint, config-doc drift all greenshellcheck -S warning scripts/install.sh— cleandocs/adr/*,releasing.md,development.md) untouched, everycurl | sudo bashline intact, genuinely-root command examples preserveddocs/contribute/testing.md's checklist; not run in this environment, requires root and a real VPN🤖 Generated with Claude Code