diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bb4705..074af2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,33 @@ 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 +- **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. @@ -34,6 +59,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/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/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..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**. @@ -997,6 +1017,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/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/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/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/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/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/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/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, 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