From f75954cb9b0cb45d79fbd83cc8e4c32d50dc17b7 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Fri, 21 Aug 2026 11:51:38 +0330 Subject: [PATCH 1/2] feat: complete uninstall and purge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit uninstall.sh removed only root-owned state, so everything belonging to the logged-in user survived it: the preference domain, the login-keychain control token, and the login-item registration. That is also why the setup wizard stopped appearing after a reinstall — FirstRunDecision.offer reads dezhban.firstRunCompleted out of a preference domain no uninstaller ever cleared, so a machine with an empty /etc/dezhban still answered "already done". The missing feature and the reported bug are the same defect from two ends. Root cannot do this work: a login keychain item's ACL is bound to the user's session and a login item is registered per user. So the app owns the per-user half. Settings gains Remove Dezhban…, which clears the keychain token and its capability probe, both login-item registrations, the saved window state, and the preference domains (current and the dead com.dezhban.DezhbanMenu), then opens Terminal on the root uninstaller and quits. Terminal rather than an in-app sheet because uninstall.sh quits Dezhban and deletes its bundle partway through: a progress sheet dies mid-teardown and cannot distinguish a finished uninstall from one that stopped after panic removed the rules. A terminal window outlives the app and shows the rule teardown, which for a kill switch is the step you most want to watch succeed. Preference domains are cleared last, immediately before terminate, because AppKit writes defaults as the app winds down; uninstall.sh repeats the deletion for $SUDO_USER for the same reason, and names the two per-user items it cannot reach rather than failing at them silently. Refs docs/adr/0015-complete-purge-semantics.md Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 21 +++ docs/adr/0015-complete-purge-semantics.md | 115 +++++++++++++++ docs/adr/README.md | 1 + docs/contribute/testing.md | 30 ++++ docs/usage/troubleshooting.md | 56 ++++++++ .../Sources/DezhbanMenu/AppActions.swift | 45 ++++++ .../Sources/DezhbanMenu/ControlToken.swift | 16 +++ gui/macos/Sources/DezhbanMenu/LoginItem.swift | 7 +- gui/macos/Sources/DezhbanMenu/Purge.swift | 133 ++++++++++++++++++ .../Sources/DezhbanMenu/SettingsView.swift | 76 ++++++++++ gui/macos/build-app.sh | 6 +- packaging/macos/uninstall.sh | 29 ++++ 12 files changed, 533 insertions(+), 2 deletions(-) create mode 100644 docs/adr/0015-complete-purge-semantics.md create mode 100644 gui/macos/Sources/DezhbanMenu/Purge.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bb4705..e823d22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,18 @@ current as you land changes. ## [Unreleased] +### Added + +- **Settings → Remove Dezhban…** — the complete uninstall, from the app. It + removes what only your own login session can reach (the Touch ID key in the + login keychain, the "open at login" registration, this app's preferences and + saved window state), then opens Terminal running the root uninstaller and + quits — so you watch the firewall-rule teardown happen instead of trusting a + dialog that is about to be deleted. "Keep my dezhban configuration in + /etc/dezhban" maps to the uninstaller's existing `KEEP_CONFIG=1`. Other user + accounts are deliberately untouched + ([ADR-0015](docs/adr/0015-complete-purge-semantics.md)). + ### Changed - **Contextual help lands on the key you asked about.** The **?** beside a @@ -34,6 +46,15 @@ current as you land changes. ### Fixed +- **The setup wizard appears again after a reinstall.** `uninstall.sh` removed + only root-owned state, so the app's `dezhban.firstRunCompleted` preference + outlived every uninstall — a machine with an empty `/etc/dezhban` and no VPN + still answered "the wizard has been completed", and stayed silent. The + uninstaller now clears the invoking user's preference domains (including the + dead `com.dezhban.DezhbanMenu` one from a superseded bundle identifier), and + the app's own Remove Dezhban… clears everything else this account holds. If + you are hitting this today, Settings → Run Setup Again… is the route that + needs no uninstall at all. - **"Open minimized" now actually decides whether the window opens.** The app used to infer a login launch from `NSApplication.launchIsDefaultUserInfoKey`, which reported wrong in both directions — the window appeared at login with diff --git a/docs/adr/0015-complete-purge-semantics.md b/docs/adr/0015-complete-purge-semantics.md new file mode 100644 index 0000000..9aefe19 --- /dev/null +++ b/docs/adr/0015-complete-purge-semantics.md @@ -0,0 +1,115 @@ +# ADR-0015: What a complete purge removes, and what it deliberately does not + +**Date**: 2026-08-21 +**Status**: accepted +**Deciders**: Behnam RK + +## Context + +`packaging/macos/uninstall.sh` runs as root and removes what root owns: the +binary, `Dezhban.app`, `/etc/dezhban`, `/var/db/dezhban`, the launchd plist, the +pkg receipts. Everything belonging to the logged-in user survived it — the app's +preference domain, the control token in the login keychain, and the login-item +registration. + +That was not merely untidy. `FirstRunDecision.offer` is +`!isComplete && !vpnKnown`, where `isComplete` reads +`dezhban.firstRunCompleted` from the app's preference domain. Because uninstall +never cleared that domain, a machine that had been uninstalled and reinstalled — +with an empty `/etc/dezhban` and no VPN configured — still answered "the wizard +has been completed", so the setup flow never appeared. The reported bug ("setup +didn't auto-start on first run") and the missing feature ("I want a complete +uninstall") are the same defect from two ends. On the machine where this was +diagnosed, a second preference domain from a superseded bundle identifier +(`com.dezhban.DezhbanMenu`) was also still present. + +Root cannot do this work. A login keychain item's ACL is bound to the user's +session, and a login item is registered per user through `SMAppService`. Both +require the user's own process. + +## Decision + +Purge is split by who can perform it, and the app owns the half root cannot. + +**Dezhban.app** gains Settings → **Remove Dezhban…**. It removes, in this +account's session: the control token and its capability-probe item (via +`ControlToken.purge()`), the login-item registration (both the ADR-0014 agent +and any surviving `mainApp` registration), the saved-window-state directories, +and the preference domains — current and legacy. It then opens **Terminal.app** +running `sudo sh /usr/local/share/dezhban/uninstall.sh` and quits. + +**`uninstall.sh`** additionally deletes both preference domains for `$SUDO_USER` +on a best-effort basis, and prints the two per-user items it cannot do +(keychain, login item) with the commands that finish the job. + +Explicitly **not** removed: other user accounts' state, and notification +authorization. + +## Alternatives considered + +### Alternative 1: keep the first-run gate, make the config the only truth + +Drop `isComplete` from `FirstRunDecision.offer`, so the wizard is offered +whenever no VPN is known. + +- **Pros**: fixes the reported bug in one line, with no purge work at all. +- **Cons**: makes a modal wizard appear on every launch for anyone who + deliberately runs without a configured VPN, and leaves the keychain token, + login item and preference domains behind on every uninstall regardless. +- **Why not**: it treats the symptom. Settings already has "Run Setup Again…" + as the manual route, so the gate is not the thing standing between a user and + the wizard — a stale flag no uninstaller ever cleared is. + +### Alternative 2: `uninstall.sh` loops over `/Users` and does everything + +- **Pros**: one code path; works for a CLI-only install; reaches every account. +- **Cons**: `sudo -u` into an account that is not logged in cannot unlock its + login keychain, so the keychain step fails or prompts unpredictably; and a + script that deletes other people's data because root ran it is a scope no + uninstaller should claim. +- **Why not**: it cannot actually do the keychain half, which is the half that + needed a session in the first place. + +### Alternative 3: a progress sheet in the app instead of Terminal + +- **Pros**: feels native; no window the user has to close afterwards. +- **Cons**: `uninstall.sh` quits Dezhban and deletes its bundle partway + through, so the sheet dies mid-teardown. The user could not distinguish a + finished uninstall from one that stopped after `panic` removed the rules. +- **Why not**: a kill switch's removal has one step you must be able to watch + succeed — the rule teardown — and this hides exactly that one. + +## Consequences + +### Positive + +- A reinstall genuinely looks like a fresh machine, so the first-run wizard + appears when it should. +- The keychain token and login item no longer outlive the app they belong to. + A surviving login item makes macOS keep trying to launch a deleted bundle. +- The teardown is visible. The user watches `panic` remove the rules in a window + that outlives the app. + +### Negative + +- Removal now spans two surfaces, and the app must run at least once for the + per-user half to happen. Someone who deletes `Dezhban.app` in Finder and then + runs `uninstall.sh` gets the preference domains (via `$SUDO_USER`) but keeps + the keychain item and login item. The script prints both commands rather than + leaving that silent. +- Terminal.app must exist and be scriptable. When it is not, the app says so and + prints the command instead of quitting into a half-removed install. + +### Risks + +- **The confirmation is destructive and irreversible.** It is a critical-style + alert that names every category removed, Cancel is the default button, and the + return key cannot trigger removal. "Keep my dezhban configuration in + /etc/dezhban" maps to the script's existing `KEEP_CONFIG=1`. +- **Preference deletion could be undone by the app's own shutdown**, since + AppKit writes defaults as it winds down. Mitigated by clearing the domains + last, immediately before `NSApp.terminate`, and by the script repeating the + deletion for `$SUDO_USER`. +- **Scope creep.** This record exists so a later change cannot quietly widen + purge to other accounts or to data dezhban did not create. Widening it needs a + new ADR. diff --git a/docs/adr/README.md b/docs/adr/README.md index 2d76a1b..4e29fd3 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -26,6 +26,7 @@ New records use [template.md](template.md) and take the next free number. | [0012](0012-app-checked-biometrics-on-unsigned-builds.md) | App-checked biometrics on unsigned builds, rather than no biometrics | accepted, implemented | | [0013](0013-geo-provider-pass-opt-out.md) | The geo-provider pass gets an opt-out, not a redesign | accepted, implemented | | [0014](0014-login-item-launch-marker.md) | The login item carries an explicit launch marker | accepted, implemented | +| [0015](0015-complete-purge-semantics.md) | What a complete purge removes, and what it deliberately does not | accepted, implemented | > **0006 is the one to read first if you are touching the geo lookup.** It records why > the obvious implementation silently defeats the exit-country check, and it exists diff --git a/docs/contribute/testing.md b/docs/contribute/testing.md index 180a221..3e71f0e 100644 --- a/docs/contribute/testing.md +++ b/docs/contribute/testing.md @@ -997,6 +997,36 @@ task gui:build && open dist/Dezhban.app - [ ] **Staleness.** Kill the daemon → the icon goes gray after the 90 s staleness window, and Overview switches to the guided "Stopped" state. +### Remove Dezhban (complete purge) + +Destructive and one-way — run these on a machine you are willing to reinstall +on. See [ADR-0015](../adr/0015-complete-purge-semantics.md). + +- [ ] **Cancel is the default.** Settings → Remove Dezhban…, press Return: the + sheet dismisses with nothing removed. The guard is still enforcing. +- [ ] **The per-user half actually goes.** Enroll Touch ID and enable "open at + login" first, then remove. After the app quits: + `defaults read com.behnam-rk.dezhban.app` fails with "domain does not + exist"; `security find-generic-password -s sh.dezhban.menu` reports + `SecKeychainSearchCopyNext: The specified item could not be found`; + Dezhban no longer appears in System Settings › General › Login Items. +- [ ] **The teardown is visible.** Terminal opens, `sudo` prompts, and the + transcript shows `panic` removing the rules BEFORE anything is deleted. + Confirm the network works throughout — a half-removed kill switch that + leaves a block-all rule loaded is the one outcome this must never produce. +- [ ] **KEEP_CONFIG.** With the checkbox ticked, `/etc/dezhban/dezhban.json` + survives; without it, `/etc/dezhban` is gone. +- [ ] **Reinstall looks fresh.** Reinstall, launch the app: the first-run wizard + opens. This is the bug the purge exists to fix. +- [ ] **Terminal unavailable degrades honestly.** Rename + `/usr/local/share/dezhban/uninstall.sh` and remove: the app reports that + dezhban is still installed and still enforcing, prints the command, and + does **not** quit. +- [ ] **The script's own per-user pass.** On a second account, run + `sudo sh /usr/local/share/dezhban/uninstall.sh` directly: that account's + preference domains are deleted, and the output names the keychain item and + login item it did not touch. A third account's settings are untouched. + ### Actions - [ ] **The action row explains itself before the click.** Titles are short diff --git a/docs/usage/troubleshooting.md b/docs/usage/troubleshooting.md index ed121f8..89c4815 100644 --- a/docs/usage/troubleshooting.md +++ b/docs/usage/troubleshooting.md @@ -510,3 +510,59 @@ dezhban validate --config # prints the precise validation error ``` See [config.md](config.md) for every field and its constraints. + +## I reinstalled, but the setup wizard never appears + +The app offers the first-run wizard only when it has never been completed on +this account *and* dezhban knows no VPN server. The first half is a flag in the +app's own preferences, and it used to outlive every uninstall — so a machine +with an empty `/etc/dezhban` could still answer "already done" and stay silent. + +Check both halves: + +```sh +defaults read com.behnam-rk.dezhban.app dezhban.firstRunCompleted # 1 means "already done" +dezhban config show # look at vpn.endpoints / profiles +``` + +Two ways out. **Settings → Run Setup Again…** walks the same questions with your +current settings filled in — that is the intended route, and it does not require +uninstalling anything. Or clear the flag: + +```sh +defaults delete com.behnam-rk.dezhban.app dezhban.firstRunCompleted +``` + +A Mac that ran an early build may also still carry a `com.dezhban.DezhbanMenu` +preference domain from a superseded bundle identifier. It is inert, and +**Settings → Remove Dezhban…** clears it. + +## Removing dezhban completely + +**Settings → Remove Dezhban…** in the app is the complete route. It removes what +only your own login session can reach — the Touch ID key in your login keychain, +the "open at login" registration, and this app's preferences — then opens +Terminal running the root uninstaller and quits, so you watch the firewall-rule +teardown happen rather than trusting a dialog that is about to be deleted. Tick +"Keep my dezhban configuration in /etc/dezhban" to keep your config. + +Without the app, the root half alone is: + +```sh +sudo sh /usr/local/share/dezhban/uninstall.sh # everything +sudo KEEP_CONFIG=1 sh /usr/local/share/dezhban/uninstall.sh # keep /etc/dezhban +``` + +It removes every firewall rule first (`panic`), so it can never leave you cut +off. It also clears the invoking user's app preferences, but it cannot reach the +keychain or the login item — it prints these: + +```sh +security delete-generic-password -s sh.dezhban.menu +``` + +and untick Dezhban in **System Settings › General › Login Items**. Other user +accounts keep their own app settings either way, and notification permission is +removed in **System Settings › Notifications**. See +[ADR-0015](../adr/0015-complete-purge-semantics.md) for why the work is split +this way. diff --git a/gui/macos/Sources/DezhbanMenu/AppActions.swift b/gui/macos/Sources/DezhbanMenu/AppActions.swift index 6cd1fd7..8a52dac 100644 --- a/gui/macos/Sources/DezhbanMenu/AppActions.swift +++ b/gui/macos/Sources/DezhbanMenu/AppActions.swift @@ -98,6 +98,51 @@ enum AppActions { [["panic"], ["stop"], ["uninstall"]] } + /// Where the installer leaves the uninstaller matching the installed + /// version. Both `scripts/install.sh` and the `.pkg` put it here. + static let uninstallerPath = "/usr/local/share/dezhban/uninstall.sh" + + /// The exact command that removes everything root owns. Shown to the user + /// verbatim when Terminal cannot be opened, so it has to be copy-pasteable + /// as printed. + static func uninstallerCommand(keepConfig: Bool) -> String { + let prefix = keepConfig ? "sudo KEEP_CONFIG=1 sh " : "sudo sh " + return prefix + uninstallerPath + } + + /// Opens Terminal.app running the root uninstaller. Reports whether Terminal + /// actually took the command. + /// + /// Terminal rather than an in-app privileged sequence, because the script + /// quits this app and deletes its bundle partway through — see + /// `SettingsView.uninstallEverything`. The user types their password into + /// `sudo` in a window they own, and watches the `panic` teardown land. + /// + /// The command is assembled from constants only, never from user input, so + /// there is nothing here to quote-escape. Keep it that way: this string is + /// executed as root. + @discardableResult + static func openUninstallerInTerminal(keepConfig: Bool) -> Bool { + guard FileManager.default.fileExists(atPath: uninstallerPath) else { + return false + } + let command = uninstallerCommand(keepConfig: keepConfig) + let script = """ + tell application "Terminal" + activate + do script "\(command)" + end tell + """ + guard let apple = NSAppleScript(source: script) else { return false } + var err: NSDictionary? + apple.executeAndReturnError(&err) + if let err = err { + NSLog("DezhbanMenu: could not start the uninstaller in Terminal: %@", err) + return false + } + return true + } + /// download then apply, under ONE admin prompt — same reasoning as /// installCommands: the prompt is the expensive thing, and these two /// steps are meaningless run apart (apply has nothing staged without diff --git a/gui/macos/Sources/DezhbanMenu/ControlToken.swift b/gui/macos/Sources/DezhbanMenu/ControlToken.swift index dbcee66..ae6c22e 100644 --- a/gui/macos/Sources/DezhbanMenu/ControlToken.swift +++ b/gui/macos/Sources/DezhbanMenu/ControlToken.swift @@ -451,6 +451,22 @@ enum ControlToken { /// docs/adr/0012-app-checked-biometrics-on-unsigned-builds.md, which records /// the measurements and names "modernising this back to `SecItemDelete`" as /// the regression to watch for. + /// Removes every keychain item this app owns: the token and the capability + /// probe. Reports whether anything was actually there. + /// + /// Lives here rather than in `Purge` because the account names are private + /// to this type, and deliberately so — the probe account exists precisely so + /// a probe can never collide with a real token, and a second place naming it + /// would be a second place to get that wrong. Uses `remove`, never + /// `SecItemDelete`, for the ACL reason documented on it. + @discardableResult + static func purge() -> Bool { + let hadToken = remove(account: account) + let hadProbe = remove(account: probeAccount) + clearOrphaned() + return hadToken || hadProbe + } + @discardableResult static func remove(account: String = ControlToken.account) -> Bool { // Look the item up with the modern API — `kSecReturnRef` hands back a diff --git a/gui/macos/Sources/DezhbanMenu/LoginItem.swift b/gui/macos/Sources/DezhbanMenu/LoginItem.swift index 5407d18..6cf6dcd 100644 --- a/gui/macos/Sources/DezhbanMenu/LoginItem.swift +++ b/gui/macos/Sources/DezhbanMenu/LoginItem.swift @@ -22,7 +22,12 @@ enum LoginItem { /// installs it under; launchd rejects a mismatch, and SMAppService reports /// it only as a `.notFound` status. build-app.sh greps this file for the /// label it installs, so the three cannot drift apart silently. - private static let plistName = "com.behnam-rk.dezhban.app.login.plist" + /// + /// Internal rather than private: `Purge` unregisters the same agent, and a + /// second copy of this string is a second thing to forget to rename. The + /// build-time check matches on `static let plistName = …`, without the access + /// modifier, so it holds either way. + static let plistName = "com.behnam-rk.dezhban.app.login.plist" /// Whether the one-shot move off `SMAppService.mainApp` has been attempted. /// diff --git a/gui/macos/Sources/DezhbanMenu/Purge.swift b/gui/macos/Sources/DezhbanMenu/Purge.swift new file mode 100644 index 0000000..e2e0ec8 --- /dev/null +++ b/gui/macos/Sources/DezhbanMenu/Purge.swift @@ -0,0 +1,133 @@ +import AppKit +import Foundation +import ServiceManagement + +/// Removing the per-user half of a dezhban install — the half +/// `packaging/macos/uninstall.sh` cannot reach. +/// +/// The uninstaller runs as root and removes root-owned things: the binary, the +/// app bundle, `/etc/dezhban`, `/var/db/dezhban`, the launchd plist, the pkg +/// receipts. Everything belonging to the logged-in user survived it: the +/// preference domain, the login-keychain token, the login-item registration. +/// A user's login keychain is not usefully reachable from a root script, and a +/// login item is registered per user, so this work has to happen in the user's +/// own session — which is exactly what this app is. +/// +/// The consequence of it never having happened: `dezhban.firstRunCompleted` +/// outlived every reinstall, so `FirstRunDecision.offer` refused to show the +/// setup wizard on what was, from the daemon's side, a completely fresh +/// machine. Fixing the purge is what fixes that. +/// +/// What this deliberately does NOT touch is recorded in +/// docs/adr/0015-complete-purge-semantics.md: other user accounts (this app +/// speaks only for the account running it) and notification authorization +/// (macOS owns it; there is no API to revoke it). +enum Purge { + /// The app's preference domain before the bundle identifier was settled. + /// Still on disk on any Mac that ran an early build — a purge that leaves + /// it behind is not a purge. + static let legacyBundleID = "com.dezhban.DezhbanMenu" + + /// One thing removed, and whether it was there to remove. Reported rather + /// than logged so the caller can tell the user what actually happened — + /// "nothing to remove" and "failed to remove" must not look alike. + struct Step { + let what: String + let removed: Bool + let error: String? + } + + /// Removes everything this account holds. Returns one Step per item, in the + /// order performed. + /// + /// Ordering matters in one place: the preference domains go LAST. AppKit + /// writes window frames and other defaults as the app winds down, so a + /// domain cleared early would simply be recreated before the process exits. + /// The root uninstaller repeats the domain deletion for `$SUDO_USER` for + /// the same reason — belt and braces, because this is the step whose + /// survival caused the original bug. + @discardableResult + static func perUser() -> [Step] { + var steps: [Step] = [] + steps.append(removeKeychainToken()) + steps.append(removeLoginItem()) + steps.append(contentsOf: removeSavedState()) + steps.append(contentsOf: removePreferenceDomains()) + return steps + } + + // MARK: - keychain + + /// The control token, plus the capability probe item that enrollment writes. + /// Routed through `ControlToken` rather than `SecItemDelete` here, because + /// that type owns both the account names and the reason a plain + /// `SecItemDelete` is refused with `-25244` across code identities. + private static func removeKeychainToken() -> Step { + Step(what: "control token (login keychain)", + removed: ControlToken.purge(), + error: nil) + } + + // MARK: - login item + + /// Unregisters both the LaunchAgent and any surviving `mainApp` + /// registration from before ADR-0014. Either may legitimately be absent, so + /// "not registered" is a success, not an error. + private static func removeLoginItem() -> Step { + var failures: [String] = [] + var removed = false + for (name, svc) in [("login agent", SMAppService.agent(plistName: LoginItem.plistName)), + ("legacy login item", SMAppService.mainApp)] { + guard svc.status == .enabled else { continue } + do { + try svc.unregister() + removed = true + } catch { + failures.append("\(name): \(error.localizedDescription)") + } + } + return Step(what: "start at login", + removed: removed, + error: failures.isEmpty ? nil : failures.joined(separator: "; ")) + } + + // MARK: - on-disk per-user state + + private static func removeSavedState() -> [Step] { + bundleIDs.map { id in + let url = FileManager.default.homeDirectoryForCurrentUser + .appendingPathComponent("Library/Saved Application State/\(id).savedState") + return removeItem(at: url, what: "saved window state (\(id))") + } + } + + private static func removePreferenceDomains() -> [Step] { + bundleIDs.map { id in + let defaults = UserDefaults.standard + let had = defaults.persistentDomain(forName: id) != nil + defaults.removePersistentDomain(forName: id) + // Deprecated, and correct here: the process is about to exit, and + // the point is that the deletion reaches disk before it does. + defaults.synchronize() + return Step(what: "preferences (\(id))", removed: had, error: nil) + } + } + + /// This app's identifier and the one it used to have. `Bundle.main` is nil + /// only for a bare SwiftPM binary, where there is no domain to clear. + private static var bundleIDs: [String] { + [Bundle.main.bundleIdentifier, legacyBundleID].compactMap { $0 } + } + + private static func removeItem(at url: URL, what: String) -> Step { + guard FileManager.default.fileExists(atPath: url.path) else { + return Step(what: what, removed: false, error: nil) + } + do { + try FileManager.default.removeItem(at: url) + return Step(what: what, removed: true, error: nil) + } catch { + return Step(what: what, removed: false, error: error.localizedDescription) + } + } +} diff --git a/gui/macos/Sources/DezhbanMenu/SettingsView.swift b/gui/macos/Sources/DezhbanMenu/SettingsView.swift index 2c24977..7c86e38 100644 --- a/gui/macos/Sources/DezhbanMenu/SettingsView.swift +++ b/gui/macos/Sources/DezhbanMenu/SettingsView.swift @@ -342,6 +342,20 @@ struct SettingsView: View { Text("Some advanced options (control socket, geo providers, allowlist) live only in the config file.") .foregroundStyle(.secondary) } + Section { + Button("Remove Dezhban…", action: uninstallEverything) + .disabled(!state.cliFound) + if !state.cliFound { + Text("The dezhban command-line tool isn’t installed, so there is nothing for the " + + "uninstaller to remove. You can still delete Dezhban.app from Applications.") + .font(.callout) + .foregroundStyle(.secondary) + } + } header: { + sectionHeader("Remove Dezhban", + "Takes the guard down, removes every firewall rule, and deletes " + + "everything Dezhban installed on this Mac.") + } } .formStyle(.grouped) @@ -388,6 +402,68 @@ struct SettingsView: View { .padding(PaneMetrics.footerPadding) } + // MARK: - remove everything + + /// The complete removal: this account's own state first, in user context, + /// then the root uninstaller in a Terminal window, then quit. + /// + /// Handed to Terminal rather than run in-app for a reason the script makes + /// unavoidable: `uninstall.sh` quits Dezhban and deletes the bundle partway + /// through, so this app cannot survive to report its own result. A progress + /// sheet would die mid-teardown and leave the user unable to tell a finished + /// uninstall from one that stopped after `panic` removed the rules. A + /// terminal window outlives the app and shows every step, including that + /// teardown — which for a kill switch is the step you most want to see + /// succeed. + private func uninstallEverything() { + let alert = NSAlert() + alert.alertStyle = .critical + alert.messageText = "Remove Dezhban from this Mac?" + alert.informativeText = """ + All dezhban firewall rules are removed first, so nothing is left blocking your network. + + This then deletes the dezhban service, the command-line tool, Dezhban.app, its learned VPN state, your Touch ID key, this app's settings, and its "open at login" registration. + + It does not touch other user accounts on this Mac. Notification permission is removed in System Settings › Notifications. + + This cannot be undone. + """ + let keepConfig = NSButton(checkboxWithTitle: "Keep my dezhban configuration in /etc/dezhban", + target: nil, action: nil) + keepConfig.state = .off + alert.accessoryView = keepConfig + alert.addButton(withTitle: "Remove Dezhban") + alert.addButton(withTitle: "Cancel") + // Cancel is the default: the return key must not be able to uninstall a + // kill switch, and the destructive button should cost a deliberate click. + alert.buttons.first?.keyEquivalent = "" + alert.buttons.last?.keyEquivalent = "\r" + guard alert.runModal() == .alertFirstButtonReturn else { return } + + // The per-user half, in this account's own session. Root cannot do it, + // and it is the half whose survival made every reinstall look like an + // already-configured machine to the first-run wizard. + let steps = Purge.perUser() + for step in steps where step.error != nil { + NSLog("DezhbanMenu: purge could not remove %@: %@", step.what, step.error ?? "") + } + + guard AppActions.openUninstallerInTerminal(keepConfig: keepConfig.state == .on) else { + // Terminal never opened, so nothing root-owned was removed. Say so + // rather than quitting into a half-removed install: the per-user + // state above is gone, but the guard is still enforcing. + let failed = NSAlert() + failed.alertStyle = .warning + failed.messageText = "Could not open Terminal" + failed.informativeText = "Dezhban is still installed and still enforcing. " + + "Finish removing it by running this in a terminal:\n\n" + + AppActions.uninstallerCommand(keepConfig: keepConfig.state == .on) + failed.runModal() + return + } + NSApp.terminate(nil) + } + // MARK: - explicit restart /// Restart is a lifecycle action about the running daemon, not a settings diff --git a/gui/macos/build-app.sh b/gui/macos/build-app.sh index f0be54a..30be1c6 100755 --- a/gui/macos/build-app.sh +++ b/gui/macos/build-app.sh @@ -123,7 +123,11 @@ fi # check for "…app.login"; and a bare substring search for the label also matched # LoginItem's dispatch-queue label, "…app.loginitem", which contains it — so the # check could not fail for that file no matter what plistName said. -swift_decl="private static let plistName = \"$AGENT_LABEL.plist\"" +# No access modifier in the pattern: `plistName` is internal so `Purge` can +# unregister the same agent without a second copy of the string, and this check has +# to keep holding across that. A fixed-string match on the tail is satisfied by +# `private static let …` and `static let …` alike. +swift_decl="static let plistName = \"$AGENT_LABEL.plist\"" if ! grep -qF "$swift_decl" "$HERE/Sources/DezhbanMenu/LoginItem.swift"; then echo "build-app.sh: LoginItem.swift does not declare plistName as '$AGENT_LABEL.plist' — SMAppService would name a plist that does not exist, reported only as the .notFound status nobody reads" >&2 exit 1 diff --git a/packaging/macos/uninstall.sh b/packaging/macos/uninstall.sh index c7ba454..78e9222 100755 --- a/packaging/macos/uninstall.sh +++ b/packaging/macos/uninstall.sh @@ -375,6 +375,35 @@ else rm -rf "$CONFIG_DIR" fi +# Per-user state, best effort, for the account that invoked sudo. +# +# This is the half a root script cannot properly reach, and the half whose +# survival used to make a reinstall look like an already-configured machine to +# the app's first-run wizard: the preference domain outlived every uninstall, so +# `dezhban.firstRunCompleted` stayed set while /etc/dezhban was empty. +# +# The app's own "Remove Dezhban…" does this — plus the login-keychain token and +# the login-item registration, which genuinely require the user's session and +# are NOT attempted here. Repeating the defaults deletion is deliberate: the app +# quits after clearing them, and AppKit can rewrite window frames on the way out. +# +# Only $SUDO_USER, never a loop over /Users: this script speaks for the person +# running it. Other accounts are named below rather than touched. +if [ -n "${SUDO_USER:-}" ] && [ "$SUDO_USER" != root ]; then + echo "removing $SUDO_USER's app preferences ..." + for domain in com.behnam-rk.dezhban.app com.dezhban.DezhbanMenu; do + sudo -u "$SUDO_USER" defaults delete "$domain" >/dev/null 2>&1 || true + done + echo "note: $SUDO_USER's Touch ID key and \"open at login\" registration are not removed here." >&2 + echo " Both live in that account's own session — use Dezhban.app's" >&2 + echo " Settings › Remove Dezhban before uninstalling, or remove them by hand:" >&2 + echo " security delete-generic-password -s sh.dezhban.menu" >&2 + echo " and untick Dezhban in System Settings › General › Login Items." >&2 +else + echo "note: no invoking user to clean up after (running as root directly)." >&2 +fi +echo "note: other user accounts on this Mac keep their own dezhban app settings." >&2 + # Forget the receipts, or macOS still believes dezhban is installed (and a later # install of an older version would be refused as a downgrade). pkgutil --forget com.behnam-rk.dezhban.cli >/dev/null 2>&1 || true From 496be741a26694655e6e6058c6661ba25f0ede1f Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Fri, 21 Aug 2026 11:59:53 +0330 Subject: [PATCH 2/2] feat(setup): two-step wizard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Countries, then one "Use automatic VPN detection?" tickbox with the manual fields — tunnel interfaces, self-hosted config files, endpoints — hanging off it. The opening "Configure your VPN now?" question is gone, and both wizards read the same question set, so the CLI changes with the app. Dropping configureVPN removes the thing that stood between a re-run and a working config, so two guards replace it. autoMode's default is seeded from the config: a config with pinned vpn.tunnelInterfaces starts on manual, because Apply clears TunnelInterfaces under AutoMode on purpose and a wizard defaulting to automatic would silently unpin them. Input.Endpoints becomes *[]string, nil when the question was never shown. On macOS the endpoint question is gated behind "not automatic", so a re-run that leaves detection on reaches Apply with no answer — writing that as an empty list would delete the user's server. Same convention, and the same reasoning, as the existing AutoDiscover *bool. The Swift side already had this property via shouldAsk. Off macOS the question is ungated instead: there is no live discovery, so an endpoint is required whichever mode is chosen. The CLI now asks a group in waves. A huh form binds every field before any is answered, so a question gated on another question in the same group would be decided by that question's seeded default. Rather than split the shared question set to suit one renderer, the terminal asks the ungated questions, re-evaluates, and asks whatever that opened up — the app shows step 2 as one screen because it re-evaluates gates as answers change. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 13 ++ cmd/dezhban/setup.go | 61 ++++++-- docs/contribute/testing.md | 26 +++- docs/usage/cli.md | 42 +++-- .../Sources/DezhbanCore/SetupQuestions.swift | 17 ++- .../SetupQuestionsTests.swift | 118 ++++++++++----- internal/setup/answers.go | 62 +++++--- internal/setup/questions.go | 58 ++++--- internal/setup/setup_test.go | 143 ++++++++++++++---- 9 files changed, 396 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e823d22..074af2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,19 @@ current as you land changes. ### Changed +- **The setup wizard is two steps.** Blocked countries, then one "Use automatic + VPN detection?" tickbox with the manual fields — tunnel interfaces, self-hosted + config files, endpoints — revealed underneath it when you untick it. The + opening "Configure your VPN now?" question is gone; a run always writes the + VPN keys, so instead the detection answer is **seeded from your config** and a + config with pinned `vpn.tunnelInterfaces` starts on manual, meaning a re-run + clicked straight through preserves your pins. A question that is not asked + still writes no key, so leaving automatic detection on does not blank + endpoints you set by hand. Off macOS, where there is no live discovery, the + endpoint question is asked whichever mode you pick. Both wizards read the same + question set, so `dezhban setup` changes with the app — in a terminal step 2 + appears as two consecutive prompts, since a form cannot react to an answer + given inside itself. - **Contextual help lands on the key you asked about.** The **?** beside a setting used to open one of four section anchors shared by every key in that section; it now scrolls to that key's own row in the configuration reference. diff --git a/cmd/dezhban/setup.go b/cmd/dezhban/setup.go index 7704b97..2af3bb5 100644 --- a/cmd/dezhban/setup.go +++ b/cmd/dezhban/setup.go @@ -66,19 +66,37 @@ func cmdSetup(args []string) int { // Asked a screenful at a time, in Group order, so a gate can be evaluated // against answers already given — which is exactly what makes the VPN // branch a branch. + // + // Within a group, in waves. A huh form binds every field before any of them + // is answered, so a question gated on another question in the SAME group + // would be decided by that question's seeded default rather than by what + // the user just typed. The macOS app has no such problem — it re-evaluates + // gates as answers change and shows the whole step at once, which is what + // makes step 2 a single screen there — so rather than splitting the shared + // question set to suit one renderer, this one asks the ungated questions, + // re-evaluates, and asks whatever that opened up. for _, group := range groupsOf(qs) { - var fields []huh.Field - for _, q := range qs { - if q.Group != group || !answers.ShouldAsk(q) { - continue + asked := map[string]bool{} + for { + var fields []huh.Field + for _, q := range qs { + if q.Group != group || asked[q.ID] || !answers.ShouldAsk(q) { + continue + } + // Defer anything still waiting on an unanswered question in + // this same group; the next wave picks it up. + if q.Gated() && !asked[q.RequiresID] && gateIsInGroup(qs, q, group) { + continue + } + asked[q.ID] = true + fields = append(fields, field(q, answers)) + } + if len(fields) == 0 { + break + } + if err := runForm(huh.NewForm(huh.NewGroup(fields...))); err != nil { + return formExit(err) } - fields = append(fields, field(q, answers)) - } - if len(fields) == 0 { - continue - } - if err := runForm(huh.NewForm(huh.NewGroup(fields...))); err != nil { - return formExit(err) } } @@ -86,7 +104,7 @@ func cmdSetup(args []string) int { // reported but doesn't abort the wizard). Reading files is the caller's job, // not internal/setup's. var profiles []config.Profile - if answers.Bool("configureVPN") { + { for _, f := range setup.SplitList(answers.Text("profileFiles")) { eps, format, ierr := vpnimport.Extract(f) if ierr != nil { @@ -112,7 +130,7 @@ func cmdSetup(args []string) int { } // --- lockout guard: warn if an endpoint sits inside a tunnel subnet --- - if answers.Bool("configureVPN") { + { if warn := setup.EndpointLockoutWarning(cfg); warn != "" { var proceed bool fmt.Fprintln(os.Stderr, warn) @@ -176,9 +194,7 @@ func cmdSetup(args []string) int { } else { fmt.Println("later, enable it with: sudo dezhban install && sudo dezhban start") } - if answers.Bool("configureVPN") { - fmt.Println("to connect a brand-new VPN whose server isn't known yet: dezhban switch, then connect it.") - } + fmt.Println("to connect a brand-new VPN whose server isn't known yet: dezhban switch, then connect it.") return 0 } @@ -304,3 +320,16 @@ func isInteractive() bool { func isTerminal(f *os.File) bool { return term.IsTerminal(f.Fd()) } + +// gateIsInGroup reports whether the question q depends on lives in the same +// group — the case the wave loop above has to defer, because a huh form cannot +// react to an answer given inside itself. A gate pointing at an EARLIER group is +// already decided by the time this group runs and needs no deferral. +func gateIsInGroup(qs []setup.Question, q setup.Question, group int) bool { + for _, other := range qs { + if other.ID == q.RequiresID { + return other.Group == group + } + } + return false +} diff --git a/docs/contribute/testing.md b/docs/contribute/testing.md index 3e71f0e..6524da1 100644 --- a/docs/contribute/testing.md +++ b/docs/contribute/testing.md @@ -716,18 +716,38 @@ macOS only, privileged (`dezhban upgrade download`/`apply`). See forms are bound to the same answers. - [ ] Re-running it **without** naming profile files keeps the profiles already imported (`dezhban vpn list`). +- [ ] **Two steps.** Step 1 is countries. Step 2 opens with "Use automatic VPN + detection?"; leaving it ticked ends the wizard there, unticking it asks for + tunnel interfaces, self-hosted config files, and endpoints. In a terminal + that second half arrives as a follow-up prompt, not the same screen. +- [ ] **A re-run on a pinned config keeps its pins.** With + `vpn.tunnelInterfaces` set, re-run and press Enter through everything: + automatic detection must arrive **already unticked**, and + `dezhban config show` must still list the same interfaces. This is what + replaced the old "Configure your VPN now?" escape, so it is the check that + matters most in this section. +- [ ] **A re-run under automatic detection keeps configured endpoints.** With + `vpn.endpoints` set and no pinned interfaces, re-run, leave automatic + detection ticked, finish: the endpoints are unchanged. The question was + never asked, so nothing may have been written. +- [ ] **Off macOS the endpoint question always appears.** On Linux or Windows, + leave automatic detection ticked — you must still be asked for an endpoint, + because there is no live discovery to find one. - [ ] `dezhban setup --questions --json` runs with no TTY, no root, and no config file present, and lists the same questions the wizard asks. ### First-run wizard (macOS app) -- [ ] With no VPN configured and `defaults delete com.dezhban.menu +- [ ] With no VPN configured and `defaults delete com.behnam-rk.dezhban.app dezhban.firstRunCompleted`, launching the app opens the window **and** the wizard. With a VPN already configured from the CLI, it does not — the questions were already answered. - [ ] The questions, their order, and the gating match `dezhban setup` run in a - terminal on the same host. Declining "Configure your VPN now?" skips the - whole VPN branch in both. + terminal on the same host. Unticking "Use automatic VPN detection?" reveals + the same three manual fields in both — in the app they appear **on the same + screen**, without paging forward. +- [ ] **Two steps, labelled as two.** The step counter reads "Step 1 of 2" and + "Step 2 of 2"; unticking automatic detection must not add a third. - [ ] Saving writes through one `config set` (one password prompt, or none with a token enrolled) and the values land in `dezhban config show`. Choosing automatic detection leaves `vpn.tunnelInterfaces` **empty**. diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 43838b5..e4c76ac 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -331,15 +331,39 @@ succeeds and says so; the new values are read the next time it starts. validation, and ruleset preview as `detect-vpn`/`validate`/`print-rules`. Writes to the system path need root (hence `sudo`); a permission error prints a `sudo` hint. -The wizard asks only what has no safe default: blocked countries (plus a -free-text field for other codes), whether to configure the VPN now, automatic -vs. manual detection, and — when configuring — tunnel interfaces (manual mode -only), self-hosted config files to import, and endpoints. Everything it used -to also ask (poll interval, log level, provider quorum, physical DNS, -auto-discovery) ships with a sane default and lives in the app's Settings or -`config set`; a wizard run leaves those keys untouched, so re-running setup -never clobbers a tuned value. The one silent defaulting decision it kept: a -brand-new macOS config gets live endpoint discovery turned on. +The wizard is **two steps**, and asks only what has no safe default: + +1. **Blocked countries** — a checklist of the common ones, plus a free-text + field for any other ISO codes. +2. **Automatic VPN detection?** — on by default. Leave it on and dezhban finds + the tunnel and, on macOS, learns the server address itself. Untick it and the + manual fields appear: tunnel interfaces, self-hosted config files to import, + and endpoints. + +Everything it used to also ask (poll interval, log level, provider quorum, +physical DNS, auto-discovery) ships with a sane default and lives in the app's +Settings or `config set`; a wizard run leaves those keys untouched, so +re-running setup never clobbers a tuned value. **A question that is not asked +writes no key** — so leaving automatic detection on does not blank the endpoints +of someone who set them by hand. The exception is off macOS, where there is no +live discovery: the endpoint question is asked whichever detection mode you +pick, because without it the config cannot enforce. + +Two consequences of there being no "configure your VPN now?" question, which +this wizard used to open with. A run always writes the VPN keys, so the +automatic-detection answer is **seeded from your config**: a config with pinned +`vpn.tunnelInterfaces` starts on manual, and pressing Enter through the wizard +preserves the pins rather than clearing them. And choosing automatic detection +deliberately clears those pins, because a leftover pin is precisely what stops +autodetection from happening. + +The one silent defaulting decision it kept: a brand-new macOS config gets live +endpoint discovery turned on. + +In a terminal, step 2 is shown as two consecutive prompts rather than one +screen — the form library binds every field before any is answered, so a +question that appears only when you untick another has to come after it. The +macOS app re-evaluates as you type and shows step 2 as a single screen. `setup --questions` is the exception: it prints what the wizard *would* ask — each question, what it writes, its seeded answer, and which earlier answer diff --git a/gui/macos/Sources/DezhbanCore/SetupQuestions.swift b/gui/macos/Sources/DezhbanCore/SetupQuestions.swift index e183bd4..d578029 100644 --- a/gui/macos/Sources/DezhbanCore/SetupQuestions.swift +++ b/gui/macos/Sources/DezhbanCore/SetupQuestions.swift @@ -141,14 +141,20 @@ public struct SetupAnswers { /// The `key=value` pairs one batched `config set` should write. /// - /// Three rules are not derivable from a question's `key` alone, and they + /// Two rules are not derivable from a question's `key` alone, and they /// mirror Go's `setup.Apply` exactly: /// - the free-text country codes fold into `blockedCountries`; - /// - answering "no" to the VPN branch writes none of its keys, so a VPN - /// somebody already configured is left alone; /// - choosing automatic detection CLEARS pinned interfaces rather than /// skipping the key, because a leftover pin is what makes autodetect not /// happen. + /// + /// The third rule is `shouldAsk` itself, and it is load-bearing now that + /// there is no "configure your VPN now?" question to skip the branch: a key + /// whose question was never shown must not be written. On macOS the + /// endpoint question is gated behind "not automatic", so a re-run that + /// leaves automatic detection on produces no `vpn.endpoints=` pair at all — + /// which is what keeps it from blanking a configured server. Go's `Apply` + /// achieves the same with a nil `Input.Endpoints`. public func configPairs(for questions: [SetupQuestion]) -> [String] { var pairs: [String] = [] for q in questions where shouldAsk(q) && !q.key.isEmpty { @@ -162,14 +168,15 @@ public struct SetupAnswers { pairs.append("\(q.key)=\(self[q.id])") } } - if bool("configureVPN") && bool("autoMode") { + if bool("autoMode") { pairs.append("vpn.tunnelInterfaces=") } return pairs } /// The VPN config files to import, which are not a config key at all — they - /// become profiles through `dezhban vpn import`. + /// become profiles through `dezhban vpn import`. Empty under automatic + /// detection, where the question is not asked. public var profileFiles: [String] { list("profileFiles") } } diff --git a/gui/macos/Tests/DezhbanCoreTests/SetupQuestionsTests.swift b/gui/macos/Tests/DezhbanCoreTests/SetupQuestionsTests.swift index 99f1ada..2633d7b 100644 --- a/gui/macos/Tests/DezhbanCoreTests/SetupQuestionsTests.swift +++ b/gui/macos/Tests/DezhbanCoreTests/SetupQuestionsTests.swift @@ -19,16 +19,15 @@ struct SetupQuestionsTests { "options":[{"label":"Iran (IR)","value":"IR"},{"label":"Russia (RU)","value":"RU"}], "selected":["IR"],"group":1}, {"id":"otherCountries","kind":"list","title":"Other country codes","default":"AQ","group":1}, - {"id":"configureVPN","kind":"bool","title":"Configure your VPN now?","default":"true","group":1}, {"id":"autoMode","kind":"bool","title":"Use automatic VPN detection? (recommended)", - "default":"true","group":2,"requiresId":"configureVPN","requiresValue":"true"}, + "default":"true","group":2}, {"id":"tunnels","key":"vpn.tunnelInterfaces","kind":"multiselect","title":"Tunnel interface(s)", "options":[{"label":"utun4","value":"utun4"},{"label":"utun7","value":"utun7"}], - "selected":["utun4"],"group":3,"requiresId":"autoMode","requiresValue":"false"}, - {"id":"profileFiles","kind":"list","title":"Self-hosted VPN config files","group":4, - "requiresId":"configureVPN","requiresValue":"true"}, + "selected":["utun4"],"group":2,"requiresId":"autoMode","requiresValue":"false"}, + {"id":"profileFiles","kind":"list","title":"Self-hosted VPN config files","group":2, + "requiresId":"autoMode","requiresValue":"false"}, {"id":"endpoints","key":"vpn.endpoints","kind":"list","title":"VPN endpoint(s)", - "default":"203.0.113.7","group":4,"requiresId":"configureVPN","requiresValue":"true"} + "default":"203.0.113.7","group":2,"requiresId":"autoMode","requiresValue":"false"} ] """ @@ -38,13 +37,16 @@ struct SetupQuestionsTests { @Test func decodesTheDaemonsQuestions() throws { let qs = try Self.questions() - #expect(qs.count == 8) + #expect(qs.count == 7) let countries = try #require(qs.first { $0.id == "blockedCountries" }) #expect(countries.selected == ["IR"]) #expect(countries.options.map(\.value) == ["IR", "RU"]) // Absent `key` decodes as "no config key", not as a decode failure. #expect(try #require(qs.first { $0.id == "otherCountries" }).key.isEmpty) - #expect(try #require(qs.first { $0.id == "autoMode" }).isGated) + // autoMode is the gate now, not a gated question — everything manual + // hangs off it, and it hangs off nothing. + #expect(!(try #require(qs.first { $0.id == "autoMode" }).isGated)) + #expect(try #require(qs.first { $0.id == "endpoints" }).isGated) #expect(!(try #require(qs.first { $0.id == "pollInterval" }).isGated)) } @@ -52,23 +54,46 @@ struct SetupQuestionsTests { let a = SetupAnswers(questions: try Self.questions()) #expect(a["pollInterval"] == "15s") #expect(a.list("blockedCountries") == ["IR"]) - #expect(a.bool("configureVPN")) + #expect(a.bool("autoMode")) + } + + /// Two steps, matching Go's TestTheWizardIsTwoGroups. The app renders one + /// group per step, so a third group is a third screen. + @Test func theWizardIsTwoSteps() throws { + #expect(Set(try Self.questions().map(\.group)) == [1, 2]) } + /// Step 2 is one automatic-detection tickbox with every manual field hanging + /// off it — the reveal-in-place the app renders, and the same gate the CLI + /// evaluates. @Test func gatingMatchesTheCLI() throws { let qs = try Self.questions() var a = SetupAnswers(questions: qs) - a["configureVPN"] = "false" - for q in qs where q.requiresID == "configureVPN" { - #expect(!a.shouldAsk(q), "\(q.id) should be hidden when the VPN branch is declined") + a["autoMode"] = "true" + for q in qs where q.requiresID == "autoMode" { + #expect(!a.shouldAsk(q), "\(q.id) should be hidden under automatic detection") } - a["configureVPN"] = "true" - a["autoMode"] = "true" - #expect(!a.shouldAsk(try #require(qs.first { $0.id == "tunnels" }))) a["autoMode"] = "false" - #expect(a.shouldAsk(try #require(qs.first { $0.id == "tunnels" }))) + for id in ["tunnels", "profileFiles", "endpoints"] { + #expect(a.shouldAsk(try #require(qs.first { $0.id == id })), + "\(id) should be asked once automatic detection is unticked") + } + } + + /// The rule that replaced "configure your VPN now?": a question that was + /// never shown writes no key. Under automatic detection on macOS the + /// endpoint question is hidden, so a re-run must produce no + /// `vpn.endpoints=` pair — writing an empty one would delete a configured + /// server. Mirrors Go's TestAnUnaskedEndpointListTouchesNoEndpoint. + @Test func anUnaskedQuestionWritesNoKey() throws { + let qs = try Self.questions() + var a = SetupAnswers(questions: qs) + a["autoMode"] = "true" + + let pairs = a.configPairs(for: qs) + #expect(!pairs.contains { $0.hasPrefix("vpn.endpoints=") }) } /// The free-text codes fold into the same key as the checkboxes — they are @@ -91,7 +116,6 @@ struct SetupQuestionsTests { @Test func automaticDetectionClearsPinnedInterfaces() throws { let qs = try Self.questions() var a = SetupAnswers(questions: qs) - a["configureVPN"] = "true" a["autoMode"] = "true" let pairs = a.configPairs(for: qs) @@ -101,7 +125,6 @@ struct SetupQuestionsTests { @Test func pinningWritesTheChosenInterfaces() throws { let qs = try Self.questions() var a = SetupAnswers(questions: qs) - a["configureVPN"] = "true" a["autoMode"] = "false" a["tunnels"] = "utun4,utun7" @@ -111,17 +134,26 @@ struct SetupQuestionsTests { "a pinned config must not also be cleared") } - /// Declining the VPN branch writes none of its keys, so a VPN somebody - /// already configured is left alone — the same rule as Go's setup.Apply. - @Test func decliningTheVPNBranchWritesNoVPNKey() throws { - let qs = try Self.questions() - var a = SetupAnswers(questions: qs) - a["configureVPN"] = "false" + /// A wizard seeded with `autoMode: false` — which the daemon does whenever + /// vpn.tunnelInterfaces is pinned — and clicked straight through must write + /// those pins back, not clear them. This is the consumer side of Go's + /// TestAutoModeSeedsFalseWhenInterfacesArePinned: the app renders whatever + /// default arrives, so the guard only holds if seeding drives it. + @Test func aSeededManualModeReWritesItsPins() throws { + let qs = try Self.questions().map { q -> SetupQuestion in + guard q.id == "autoMode" else { return q } + return SetupQuestion(questionID: q.questionID, key: q.key, kind: q.kind, + title: q.title, description: q.description, + options: q.options, defaultValue: "false", + selected: q.selected, group: q.group, + requiresID: q.requiresID, requiresValue: q.requiresValue) + } + let a = SetupAnswers(questions: qs) + #expect(!a.bool("autoMode"), "the seeded default must drive the answer") let pairs = a.configPairs(for: qs) - #expect(!pairs.contains { $0.hasPrefix("vpn.") }) - // The answers that were given still apply. - #expect(pairs.contains { $0.hasPrefix("pollInterval=") }) + #expect(pairs.contains("vpn.tunnelInterfaces=utun4")) + #expect(!pairs.contains("vpn.tunnelInterfaces=")) } /// Profile files are not a config key: they become profiles through @@ -147,8 +179,7 @@ struct SetupQuestionsTests { // MARK: - the shrunk wizard /// The daemon's question list after the 2026-08 shrink: blocked countries, - /// configure-VPN?, auto-vs-manual, and the gated VPN details — nothing - /// else. Everything above must keep working with this list, because the + /// auto-vs-manual and the gated VPN details — nothing else. Everything above must keep working with this list, because the /// view renders whatever arrives, and the id-keyed special cases /// (blockedCountries+otherCountries fold, autoMode's tunnel clearing) must /// hold with the surrounding questions gone. @@ -158,16 +189,15 @@ struct SetupQuestionsTests { "options":[{"label":"Iran (IR)","value":"IR"},{"label":"Russia (RU)","value":"RU"}], "selected":["IR"],"group":1}, {"id":"otherCountries","kind":"list","title":"Other country codes","default":"AQ","group":1}, - {"id":"configureVPN","kind":"bool","title":"Configure your VPN now?","default":"true","group":1}, {"id":"autoMode","kind":"bool","title":"Use automatic VPN detection? (recommended)", - "default":"true","group":2,"requiresId":"configureVPN","requiresValue":"true"}, + "default":"true","group":2}, {"id":"tunnels","key":"vpn.tunnelInterfaces","kind":"multiselect","title":"Tunnel interface(s)", - "options":[{"label":"utun4","value":"utun4"}],"selected":[],"group":3, + "options":[{"label":"utun4","value":"utun4"}],"selected":[],"group":2, "requiresId":"autoMode","requiresValue":"false"}, - {"id":"profileFiles","kind":"list","title":"Self-hosted VPN config files","group":4, - "requiresId":"configureVPN","requiresValue":"true"}, - {"id":"endpoints","key":"vpn.endpoints","kind":"list","title":"VPN endpoint(s)","group":4, - "requiresId":"configureVPN","requiresValue":"true"} + {"id":"profileFiles","kind":"list","title":"Self-hosted VPN config files","group":2, + "requiresId":"autoMode","requiresValue":"false"}, + {"id":"endpoints","key":"vpn.endpoints","kind":"list","title":"VPN endpoint(s)","group":2, + "requiresId":"autoMode","requiresValue":"false"} ] """ @@ -177,10 +207,10 @@ struct SetupQuestionsTests { @Test func shrunkListDecodesAndGates() throws { let qs = try Self.shrunkQuestions() - #expect(qs.count == 7) + #expect(qs.count == 6) var a = SetupAnswers(questions: qs) - // Default flow: configure yes, automatic yes — the tunnel question is - // never asked. + // Default flow: automatic detection on — the tunnel question is never + // asked. let tunnels = try #require(qs.first { $0.id == "tunnels" }) #expect(!a.shouldAsk(tunnels)) a["autoMode"] = "false" @@ -205,9 +235,15 @@ struct SetupQuestionsTests { let qs = try Self.shrunkQuestions() var a = SetupAnswers(questions: qs) a["endpoints"] = "203.0.113.7" - let pairs = a.configPairs(for: qs) - // configureVPN + autoMode both default true → pinned interfaces cleared. + var pairs = a.configPairs(for: qs) + // autoMode defaults true → pinned interfaces cleared, and the endpoint + // question is not asked, so its answer is not written even though one + // was set. #expect(pairs.contains("vpn.tunnelInterfaces=")) + #expect(!pairs.contains { $0.hasPrefix("vpn.endpoints=") }) + + a["autoMode"] = "false" + pairs = a.configPairs(for: qs) #expect(pairs.contains("vpn.endpoints=203.0.113.7")) } } diff --git a/internal/setup/answers.go b/internal/setup/answers.go index bfecefd..383fc10 100644 --- a/internal/setup/answers.go +++ b/internal/setup/answers.go @@ -143,13 +143,20 @@ func (a *Answers) ShouldAsk(q Question) bool { // Input is the collected answers, in the shape the config wants them. type Input struct { - Hysteresis string - Countries []string - ConfigureVPN bool + Hysteresis string + Countries []string // AutoMode is automatic tunnel detection: no pinned interface names. - AutoMode bool - Tunnels, Endpoints []string - Profiles []config.Profile + AutoMode bool + Tunnels []string + // Endpoints is nil when the wizard never asked — on macOS the question is + // gated behind "not automatic", because live discovery learns the server + // address there. Nil rather than empty for the same reason AutoDiscover is + // a pointer: Apply must leave an unasked key ALONE, and an empty slice is + // indistinguishable from "asked, and cleared on purpose". Writing it + // unconditionally would blank the endpoints of anyone who re-ran setup and + // left automatic detection on. + Endpoints *[]string + Profiles []config.Profile // AutoDiscover is nil when nothing answered it — the wizard no longer asks; // a surface that still collects an explicit answer can set it. Nil rather // than false because Apply must leave an unanswered key ALONE: writing @@ -178,26 +185,44 @@ func (a *Answers) Input(hysteresis string, profiles []config.Profile) Input { // The no-tunnels-detected form of the question is free text. tunnels = SplitList(a.Text("tunnels")) } + var endpoints *[]string + if a.wasAsked("endpoints") { + eps := SplitList(a.Text("endpoints")) + endpoints = &eps + } return Input{ - Hysteresis: hysteresis, - Countries: countries, - ConfigureVPN: a.Bool("configureVPN"), - AutoMode: a.Bool("autoMode"), - Tunnels: tunnels, - Endpoints: SplitList(a.Text("endpoints")), - Profiles: profiles, + Hysteresis: hysteresis, + Countries: countries, + AutoMode: a.Bool("autoMode"), + Tunnels: tunnels, + Endpoints: endpoints, + Profiles: profiles, // Nil unless a surface asked the (no-longer-offered) question anyway; // nil leaves the configured value untouched in Apply. AutoDiscover: a.OptionalBool("autoDiscover"), } } +// wasAsked reports whether the question with this id exists and its gate was +// satisfied by the answers collected — i.e. whether the user actually saw it. +// The question set is retained by NewAnswers precisely so this does not have to +// be re-derived by every caller. +func (a *Answers) wasAsked(id string) bool { + for _, q := range a.asked { + if q.ID == id { + return a.ShouldAsk(q) + } + } + return false +} + // Apply writes collected answers onto cfg. Validation happens after, by the // caller: this only assembles. // // A question the user never reached leaves its part of the config alone. That -// is why the VPN keys are written only when ConfigureVPN is true — answering -// "no" must not blank out a tunnel someone configured earlier. +// is why Endpoints is a pointer: on macOS the question is gated behind "not +// automatic", and an unasked endpoint list must not blank out a server someone +// configured earlier. // Keys the wizard no longer asks about (pollInterval, logLevel, // providerQuorum, vpn.allowPhysicalDNS) are deliberately not assigned at all: // unasked means untouched, so re-running setup can never clobber a value tuned @@ -208,9 +233,6 @@ func Apply(cfg *config.Config, in Input) { } cfg.BlockedCountries = in.Countries // config.Normalize upper-cases and de-dupes on save - if !in.ConfigureVPN { - return - } if in.AutoMode { // Automatic detection: no pinned interface names (Normalize implies // autodetect), plus live discovery where supported. @@ -218,7 +240,9 @@ func Apply(cfg *config.Config, in Input) { } else { cfg.VPN.TunnelInterfaces = in.Tunnels } - cfg.VPN.Endpoints = in.Endpoints + if in.Endpoints != nil { + cfg.VPN.Endpoints = *in.Endpoints + } cfg.VPN.Profiles = mergeProfiles(cfg.VPN.Profiles, in.Profiles) switch { case in.AutoDiscover != nil: diff --git a/internal/setup/questions.go b/internal/setup/questions.go index a3a9ad7..f7987fe 100644 --- a/internal/setup/questions.go +++ b/internal/setup/questions.go @@ -145,12 +145,38 @@ func Questions(opts Options) []Question { endpointDesc = "Server IP(s)/hostname(s), comma-separated. Required on this platform (no live discovery)." } + // Automatic detection is the recommendation, but never at the cost of + // silently unpinning interfaces someone chose on purpose: a config with + // pinned vpn.tunnelInterfaces seeds this to false, so clicking straight + // through a re-run preserves them. This is load-bearing now that there is + // no "configure your VPN?" question to skip the whole branch — + // TestAutoModeSeedsFalseWhenInterfacesArePinned pins it. + autoModeDefault := "true" + if len(cfg.VPN.TunnelInterfaces) > 0 { + autoModeDefault = "false" + } + + // Endpoints are gated behind "not automatic" on macOS, where live discovery + // learns the server address. Everywhere else there is no discovery, so the + // endpoint is required whichever detection mode is chosen and the question + // is ungated — a Linux host that picked automatic detection and was never + // asked for a server would end up with a config that cannot enforce. + endpointsRequires, endpointsRequiresValue := "autoMode", "false" + if !macOS { + endpointsRequires, endpointsRequiresValue = "", "" + } + // The wizard asks only what has no safe default: what to block, and how to // find the VPN. Everything it used to also ask (poll interval, log level, // provider quorum, physical DNS, auto-discovery) ships with a sane default, // lives in Settings/`config set`, and — critically — is left UNTOUCHED by a // wizard run, so re-running setup never clobbers a tuned value // (TestAnUnaskedQuestionLeavesItsKeyAlone pins this). + // + // Two groups, which is two steps: what to block, then how to find the VPN. + // Everything in group 2 hangs off the one automatic-detection question, so + // unticking it reveals the manual fields in place rather than paging to + // another screen. return []Question{ { ID: "blockedCountries", Key: "blockedCountries", Kind: KindMultiSelect, Group: 1, @@ -165,37 +191,29 @@ func Questions(opts Options) []Question { Description: "Comma-separated ISO codes not listed above (optional).", Default: strings.Join(extra, ","), }, - { - ID: "configureVPN", Kind: KindBool, Group: 1, - Title: "Configure your VPN now?", - Description: "dezhban only enforces once it knows your VPN's tunnel and server. " + - "Say no and it starts in standby — fully open, nothing blocked — until you " + - "run 'dezhban setup' again or edit the config.", - Default: "true", - }, { ID: "autoMode", Kind: KindBool, Group: 2, Title: "Use automatic VPN detection? (recommended)", Description: "dezhban finds your tunnel and, on macOS, learns the server address " + - "itself — works with any VPN and survives redials.", - Default: "true", - RequiresID: "configureVPN", - RequiresValue: "true", + "itself — works with any VPN and survives redials. Untick it to name your " + + "tunnel and server yourself.", + Default: autoModeDefault, }, tunnelQuestion(opts.DetectedTunnels, cfg.VPN.TunnelInterfaces), { - ID: "profileFiles", Kind: KindList, Group: 4, + ID: "profileFiles", Kind: KindList, Group: 2, Title: "Self-hosted VPN config files", Description: "Comma-separated paths to WireGuard/.conf, OpenVPN/.ovpn, or V2Ray " + "JSON to import as profiles (optional).", - RequiresID: "configureVPN", RequiresValue: "true", + RequiresID: "autoMode", RequiresValue: "false", }, { - ID: "endpoints", Key: "vpn.endpoints", Kind: KindList, Group: 4, - Title: "VPN endpoint(s)", - Description: endpointDesc, - Default: strings.Join(cfg.VPN.Endpoints, ","), - RequiresID: "configureVPN", RequiresValue: "true", + ID: "endpoints", Key: "vpn.endpoints", Kind: KindList, Group: 2, + Title: "VPN endpoint(s)", + Description: endpointDesc, + Default: strings.Join(cfg.VPN.Endpoints, ","), + RequiresID: endpointsRequires, + RequiresValue: endpointsRequiresValue, }, } } @@ -204,7 +222,7 @@ func Questions(opts Options) []Question { // none were — the same split the CLI's tunnelSelector used to make on its own. func tunnelQuestion(detected, configured []string) Question { q := Question{ - ID: "tunnels", Key: "vpn.tunnelInterfaces", Group: 3, + ID: "tunnels", Key: "vpn.tunnelInterfaces", Group: 2, Title: "Tunnel interface(s)", RequiresID: "autoMode", RequiresValue: "false", } diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index d2f1f4b..c225adf 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -15,9 +15,9 @@ func TestApplyAutoMode(t *testing.T) { cfg := config.Default() Apply(&cfg, Input{ Hysteresis: "3", - ConfigureVPN: true, AutoMode: true, + AutoMode: true, Tunnels: []string{"utun9"}, // must be ignored in auto mode - Endpoints: []string{"vpn.example.com"}, + Endpoints: eps("vpn.example.com"), Profiles: []config.Profile{{Name: "home", Endpoints: []string{"203.0.113.7"}}}, AutoDiscover: boolPtr(true), }) @@ -93,26 +93,33 @@ func TestAnUnaskedQuestionLeavesItsKeyAlone(t *testing.T) { func TestApplyAdvancedPin(t *testing.T) { cfg := config.Default() Apply(&cfg, Input{ - Hysteresis: "3", - ConfigureVPN: true, AutoMode: false, - Tunnels: []string{"utun4"}, - Endpoints: []string{"203.0.113.7"}, + Hysteresis: "3", + AutoMode: false, + Tunnels: []string{"utun4"}, + Endpoints: eps("203.0.113.7"), }) if len(cfg.VPN.TunnelInterfaces) != 1 || cfg.VPN.TunnelInterfaces[0] != "utun4" { t.Errorf("advanced mode should pin utun4, got %v", cfg.VPN.TunnelInterfaces) } } -// Answering "no" to "configure your VPN now?" must leave a VPN somebody already -// set up completely alone — the wizard is also how people change their -// blocked-country list. -func TestDecliningTheVPNBranchTouchesNoVPNKey(t *testing.T) { +// An UNASKED endpoint question must leave a configured server alone. +// +// This replaced the "configure your VPN now?" question as the thing standing +// between a re-run and someone's working config. On macOS the endpoint question +// is gated behind "not automatic", so a user who re-runs setup to change their +// blocked-country list — and leaves automatic detection on, as recommended — +// reaches Apply with no endpoint answer at all. Writing that as an empty list +// would delete their server. +func TestAnUnaskedEndpointListTouchesNoEndpoint(t *testing.T) { cfg := config.Default() cfg.VPN.TunnelInterfaces = []string{"utun4"} cfg.VPN.Endpoints = []string{"203.0.113.7"} cfg.VPN.AllowPhysicalDNS = true - Apply(&cfg, Input{Countries: []string{"IR", "SY"}, ConfigureVPN: false}) + // Endpoints nil is what Input produces when the question was never shown. + Apply(&cfg, Input{Countries: []string{"IR", "SY"}, AutoMode: false, + Tunnels: []string{"utun4"}}) if !reflect.DeepEqual(cfg.VPN.TunnelInterfaces, []string{"utun4"}) { t.Errorf("tunnels changed: %v", cfg.VPN.TunnelInterfaces) @@ -139,13 +146,13 @@ func TestImportedProfilesAddToTheSavedOnes(t *testing.T) { } // A run that imported nothing keeps both. - Apply(&cfg, Input{ConfigureVPN: true, AutoMode: true}) + Apply(&cfg, Input{AutoMode: true}) if len(cfg.VPN.Profiles) != 2 { t.Fatalf("a run importing nothing must keep saved profiles, got %+v", cfg.VPN.Profiles) } // A run that re-imports one replaces that one and keeps the other. - Apply(&cfg, Input{ConfigureVPN: true, AutoMode: true, + Apply(&cfg, Input{AutoMode: true, Profiles: []config.Profile{{Name: "work", Endpoints: []string{"192.0.2.5"}}}}) if len(cfg.VPN.Profiles) != 2 { t.Fatalf("re-importing a profile must not drop the others, got %+v", cfg.VPN.Profiles) @@ -194,21 +201,21 @@ func TestQuestionsSeedFromTheConfig(t *testing.T) { func TestAutoDiscoverDefaultsOnlyForANewMacConfig(t *testing.T) { fresh := config.Default() fresh.VPN.AutoDiscoverEndpoints = false // Default() has it on; force the observable flip - Apply(&fresh, Input{ConfigureVPN: true, AutoMode: true, MacOS: true, ConfigExisted: false}) + Apply(&fresh, Input{AutoMode: true, MacOS: true, ConfigExisted: false}) if !fresh.VPN.AutoDiscoverEndpoints { t.Error("a brand-new macOS config should get discovery on") } existing := config.Default() existing.VPN.AutoDiscoverEndpoints = false - Apply(&existing, Input{ConfigureVPN: true, AutoMode: true, MacOS: true, ConfigExisted: true}) + Apply(&existing, Input{AutoMode: true, MacOS: true, ConfigExisted: true}) if existing.VPN.AutoDiscoverEndpoints { t.Error("an existing config's explicit false must be preserved") } linux := config.Default() linux.VPN.AutoDiscoverEndpoints = false - Apply(&linux, Input{ConfigureVPN: true, AutoMode: true, MacOS: false, ConfigExisted: false}) + Apply(&linux, Input{AutoMode: true, MacOS: false, ConfigExisted: false}) if linux.VPN.AutoDiscoverEndpoints { t.Error("discovery is macOS-only; a new Linux config must not have it defaulted on") } @@ -217,7 +224,7 @@ func TestAutoDiscoverDefaultsOnlyForANewMacConfig(t *testing.T) { answered := config.Default() answered.VPN.AutoDiscoverEndpoints = true off := false - Apply(&answered, Input{ConfigureVPN: true, AutoMode: true, MacOS: true, ConfigExisted: false, AutoDiscover: &off}) + Apply(&answered, Input{AutoMode: true, MacOS: true, ConfigExisted: false, AutoDiscover: &off}) if answered.VPN.AutoDiscoverEndpoints { t.Error("an explicit false answer must win over the new-config default") } @@ -248,32 +255,107 @@ func TestTunnelQuestionFollowsDetection(t *testing.T) { // --- gating --- -func TestGatingHidesTheWholeVPNBranch(t *testing.T) { +// Automatic detection is the one gate left, and everything manual hangs off it. +func TestAutomaticDetectionGatesEveryManualField(t *testing.T) { qs := Questions(Options{GOOS: "darwin"}) a := NewAnswers(qs) - a.Set("configureVPN", "false") + a.Set("autoMode", "true") for _, q := range qs { - if q.RequiresID == "configureVPN" && a.ShouldAsk(q) { - t.Errorf("%s should not be asked when the VPN branch was declined", q.ID) + if q.RequiresID == "autoMode" && a.ShouldAsk(q) { + t.Errorf("%s should not be asked under automatic detection", q.ID) } } + for _, id := range []string{"tunnels", "endpoints", "profileFiles"} { + if !gatedOnAutoMode(qs, id) { + t.Errorf("%s is not gated on autoMode; on macOS it must be", id) + } + } + + a.Set("autoMode", "false") + for _, id := range []string{"tunnels", "endpoints", "profileFiles"} { + if !asked(qs, a, id) { + t.Errorf("declining automatic detection must ask %s", id) + } + } +} - a.Set("configureVPN", "true") +// Off macOS there is no live discovery, so the endpoint is required whichever +// detection mode is chosen. Gating it would let a Linux host finish the wizard +// with a config that cannot enforce. +func TestEndpointsAreUngatedWhereThereIsNoDiscovery(t *testing.T) { + qs := Questions(Options{GOOS: "linux"}) + a := NewAnswers(qs) a.Set("autoMode", "true") + if !asked(qs, a, "endpoints") { + t.Error("endpoints must be asked under automatic detection off macOS") + } + if gatedOnAutoMode(qs, "endpoints") { + t.Error("endpoints is gated on autoMode off macOS") + } +} + +// Two steps, which is the whole shape of the wizard: what to block, then how to +// find the VPN. A third group would mean a third screen in the app. +func TestTheWizardIsTwoGroups(t *testing.T) { + for _, goos := range []string{"darwin", "linux", "windows"} { + groups := map[int]bool{} + for _, q := range Questions(Options{GOOS: goos}) { + groups[q.Group] = true + } + if len(groups) != 2 || !groups[1] || !groups[2] { + t.Errorf("%s: groups = %v, want exactly {1, 2}", goos, groups) + } + } +} + +// The guard that replaced "configure your VPN now?". Without it, a re-run on a +// config with pinned interfaces would default to automatic detection, and +// clicking straight through would silently unpin them — Apply clears +// TunnelInterfaces under AutoMode on purpose. +func TestAutoModeSeedsFalseWhenInterfacesArePinned(t *testing.T) { + pinned := config.Default() + pinned.VPN.TunnelInterfaces = []string{"utun4"} + if got := defaultOf(Questions(Options{Config: &pinned, GOOS: "darwin"}), "autoMode"); got != "false" { + t.Errorf("autoMode default with pinned interfaces = %q, want \"false\"", got) + } + + fresh := config.Default() + fresh.VPN.TunnelInterfaces = nil + if got := defaultOf(Questions(Options{Config: &fresh, GOOS: "darwin"}), "autoMode"); got != "true" { + t.Errorf("autoMode default with no pinned interfaces = %q, want \"true\"", got) + } +} + +func gatedOnAutoMode(qs []Question, id string) bool { for _, q := range qs { - if q.ID == "tunnels" && a.ShouldAsk(q) { - t.Error("automatic detection must not ask which interface to pin") + if q.ID == id { + return q.RequiresID == "autoMode" && q.RequiresValue == "false" } } - a.Set("autoMode", "false") + return false +} + +func asked(qs []Question, a *Answers, id string) bool { + for _, q := range qs { + if q.ID == id { + return a.ShouldAsk(q) + } + } + return false +} + +func defaultOf(qs []Question, id string) string { for _, q := range qs { - if q.ID == "tunnels" && !a.ShouldAsk(q) { - t.Error("declining automatic detection must ask which interface to pin") + if q.ID == id { + return q.Default } } + return "" } +func eps(v ...string) *[]string { return &v } + // Walking the wizard and pressing Enter on every question must land on the // config you started with. Anything else means a default is stated in one place // and applied differently in another — the drift Phase M exists to prevent, @@ -289,10 +371,9 @@ func TestAnsweringNothingChangesNothing(t *testing.T) { qs := Questions(Options{Config: &cfg, GOOS: "darwin", DetectedTunnels: []string{"utun4"}}) a := NewAnswers(qs) - // The one answer with no config to seed it: the VPN branch is offered, and - // its own sub-answers are seeded, so accepting them must be a no-op too. - a.Set("configureVPN", "true") - a.Set("autoMode", "false") + // Nothing is Set here on purpose. autoMode seeds itself to false from the + // pinned interfaces above, which is exactly the guard being tested: pressing + // Enter through the whole wizard must not unpin them. after := cfg // Hysteresis has no question; the wizard carries the current value through,