From f75954cb9b0cb45d79fbd83cc8e4c32d50dc17b7 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Fri, 21 Aug 2026 11:51:38 +0330 Subject: [PATCH 1/3] 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/3] 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, From 0dd295f91ea5d831152a48e1384022a8a058c163 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Fri, 21 Aug 2026 12:16:47 +0330 Subject: [PATCH 3/3] feat(diag): show applied and pending firewall rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three sources, because they answer three different questions and are not interchangeable. What dezhban recorded installing. internal/applied writes the exact ruleset text handed to the backend, timestamped, beside state.json at 0644 like the state file — so the unprivileged menubar app can read it. Recorded by wrapping the runner's Backend rather than by calling Save at each Apply: the run loop applies from nineteen places, and a record only as complete as the last person to remember it is worse than none. The wrapper adds no goroutine and no writer, so the single-writer invariant is untouched, and it records only after a successful Apply — a failed one leaves the previous ruleset live, and describing rules that were never installed is the one thing a reader of this file must be able to rely on not happening. Unblock and Cleanup clear it, so a stale ruleset can never be read as the live posture. What the kernel holds. FirewallBackend gains InstalledRules, implemented for pf, nft and WFP, each scoped to dezhban's own anchor/table/group so it can never become a way to dump unrelated firewall state. It is a read: it does not go through Apply and does not touch the single-writer rule. It needs root, which is why nothing calls it on a tick. pf and nft additionally flag the loaded-but-not- evaluated cases their IsBlocked already checks for. What each posture would apply, which print-rules already rendered purely. A record with no kernel rules is reported and never repaired — the run loop's verify tick already owns that, and a second repairer would be a second writer. Neither surface diffs the two texts: the kernel renders its own normalised form of what was loaded, so a byte comparison would report drift on every healthy host. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 17 ++ cmd/dezhban/main.go | 151 ++++++++++++++- docs/concepts/modes.md | 33 ++++ docs/contribute/testing.md | 29 +++ docs/usage/cli.md | 23 ++- gui/macos/Sources/DezhbanCore/Rulesets.swift | 133 +++++++++++++ gui/macos/Sources/DezhbanMenu/AppState.swift | 50 +++++ .../Sources/DezhbanMenu/DezhbanCLI.swift | 29 +++ .../Sources/DezhbanMenu/DiagnosticsView.swift | 178 ++++++++++++++++++ .../DezhbanCoreTests/RulesetsTests.swift | 77 ++++++++ internal/applied/applied.go | 112 +++++++++++ internal/applied/applied_test.go | 92 +++++++++ internal/firewall/backend.go | 13 ++ internal/firewall/nft_linux.go | 25 +++ internal/firewall/pf_darwin.go | 36 ++++ internal/firewall/render_darwin.go | 5 + internal/firewall/render_linux.go | 5 + internal/firewall/render_windows.go | 5 + internal/firewall/wfp_windows.go | 28 +++ internal/runner/recording.go | 101 ++++++++++ internal/runner/recording_test.go | 131 +++++++++++++ internal/runner/runner.go | 12 ++ 22 files changed, 1281 insertions(+), 4 deletions(-) create mode 100644 gui/macos/Sources/DezhbanCore/Rulesets.swift create mode 100644 gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift create mode 100644 internal/applied/applied.go create mode 100644 internal/applied/applied_test.go create mode 100644 internal/runner/recording.go create mode 100644 internal/runner/recording_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 074af2e..5d35d5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,23 @@ current as you land changes. ### Added +- **The firewall rules are visible in Diagnostics.** Three things, because they + answer three different questions: what dezhban **recorded installing** (and + when), what the **kernel actually holds** (read back on demand, needs your + password), and what **each posture would apply** — guard, full block, switch + window — rendered without applying anything. Each carries a plain-language + caption saying what that posture does to your traffic. When dezhban recorded + applying rules and the firewall holds none, the pane says so; it does not offer + to repair, because the running daemon's own verification tick already does + that and a second repairer would be a second writer. +- **`dezhban print-rules --applied` and `--installed`**, the CLI half of the + above. `--applied` reads a record dezhban now writes on every successful apply + (a 0644 file beside the state file — no root, same on every platform). + `--installed` asks the firewall itself, scoped to dezhban's own + anchor/table/group and needing root for that reason; it installs nothing and + repairs nothing. `--json` on either for machine output. The two texts will not + match byte for byte on a healthy host — the firewall renders its own + normalised form — so neither surface diffs them. - **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 diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index b608a77..b27eefe 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -25,6 +25,7 @@ import ( "syscall" "time" + "github.com/behnam-rk/dezhban/internal/applied" "github.com/behnam-rk/dezhban/internal/armed" "github.com/behnam-rk/dezhban/internal/command" "github.com/behnam-rk/dezhban/internal/config" @@ -67,7 +68,7 @@ Commands: status Show version, config, and current state validate Load and validate a config file (no root, no side effects) monitor Live read-only view: IP, country, tunnel state, endpoints, verdict - print-rules Print the firewall ruleset a block/guard would apply, without applying it + print-rules Print the firewall ruleset a block/guard would apply (--applied: what is applied now) doctor Diagnose VPN guard config (tunnels, endpoints, lockout risks) panic Force-remove dezhban's rules even if nothing is running install Register dezhban as a boot-persistent OS service @@ -795,6 +796,7 @@ func assembleOptions(cfg *config.Config, cfgPath string, log *slog.Logger, ov ru PollCommand: pollCommand, Publish: publish, BlockedCountries: cfg.BlockedCountries, + AppliedRulesPath: applied.Path(stateDir()), ReloadConfig: reload, WriteConfig: writeConfigKeysAt, AllowConfigOps: cfg.Control.AllowConfigOps, @@ -1810,8 +1812,23 @@ func cmdPrintRules(args []string) int { fs := flag.NewFlagSet("print-rules", flag.ExitOnError) cfgPath := fs.String("config", "", "path to config file (JSON)") mode := fs.String("mode", "guard", "policy to render: guard, fullblock, or switch") + appliedOnly := fs.Bool("applied", false, "print the ruleset dezhban last applied, instead of rendering one") + installed := fs.Bool("installed", false, "read dezhban's rules back out of the kernel (needs root)") + asJSON := fs.Bool("json", false, "machine-readable output (with --applied or --installed)") _ = fs.Parse(args) + if *appliedOnly && *installed { + fmt.Fprintln(os.Stderr, "--applied and --installed are two different sources; pick one.") + fmt.Fprintln(os.Stderr, "--applied is what dezhban recorded installing; --installed is what the kernel holds now.") + return 2 + } + if *appliedOnly { + return printAppliedRules(*asJSON) + } + if *installed { + return printInstalledRules(*asJSON) + } + cfg, err := loadConfig(*cfgPath) if err != nil { fmt.Fprintln(os.Stderr, "config error:", err) @@ -1831,6 +1848,138 @@ func cmdPrintRules(args []string) int { return 0 } +// printAppliedRules prints what the daemon recorded applying, as opposed to what +// a posture WOULD apply (which the rest of print-rules renders, purely). +// +// This is dezhban's own account, not a reading of the kernel: it is what the run +// loop handed the backend, timestamped, and it is the half that works +// unprivileged and identically on every platform. The label says so, because +// "the current rules" would be a claim this cannot make. +// +// Nothing recorded is an ordinary answer, not a failure — a daemon in standby +// has applied nothing, and neither has one that was never started. It exits 0 +// and says so, so a caller can tell that apart from an error. +func printAppliedRules(asJSON bool) int { + path := applied.Path(stateDir()) + rec, ok, err := applied.Load(path) + if err != nil { + fmt.Fprintln(os.Stderr, "could not read the applied-ruleset record:", err) + return 1 + } + if asJSON { + if !ok { + fmt.Println("null") + return 0 + } + out, err := json.MarshalIndent(rec, "", " ") + if err != nil { + fmt.Fprintln(os.Stderr, "encode failed:", err) + return 1 + } + fmt.Println(string(out)) + return 0 + } + if !ok { + fmt.Fprintf(os.Stderr, "no ruleset recorded at %s.\n", path) + fmt.Fprintln(os.Stderr, "dezhban records one on every apply; in standby it has applied nothing.") + return 0 + } + fmt.Fprintf(os.Stderr, "# %s ruleset dezhban applied at %s (mode %s)\n", + rec.Backend, rec.At.Local().Format(time.RFC3339), rec.Mode) + fmt.Fprintln(os.Stderr, "# This is what dezhban installed, not a reading of the kernel.") + fmt.Print(rec.Rules) + return 0 +} + +// installedRules is the machine shape of a kernel readback, paired with the +// record of what dezhban believes it applied so a consumer does not have to +// fetch and correlate the two itself. `Drift` is the finding. +type installedRules struct { + // Installed is the rule text read out of the kernel, empty when dezhban has + // no rules loaded. + Installed string `json:"installed"` + // Loaded is false when dezhban has no rules in the kernel at all — an + // ordinary answer (standby, nothing running), never an error. + Loaded bool `json:"loaded"` + // Applied is what the daemon recorded installing, absent when nothing was + // recorded. + Applied *applied.Record `json:"applied,omitempty"` + // Drift is true when dezhban has a record of what it applied and the kernel + // disagrees about whether rules are loaded at all. It deliberately does NOT + // diff the two texts: `pfctl -s rules` renders a normalised form of what was + // loaded, so a byte comparison would report drift on every healthy host. The + // text is shown to a human for that reason. + Drift bool `json:"drift"` + // Backend names the syntax of Installed. + Backend string `json:"backend"` +} + +// printInstalledRules reads dezhban's rules back out of the kernel — the other +// half of the picture from --applied, which is only dezhban's own account. +// +// A READ: it installs nothing and changes nothing, so it does not touch the +// single-writer rule that governs Apply. It does need root, which is why it is +// on demand rather than on a tick — and why nothing in the daemon calls it. +// Repairing a discrepancy is not this command's job either: the run loop's +// verify tick already owns that, and a second repairer would be a second writer. +func printInstalledRules(asJSON bool) int { + rec, hasRecord, recErr := applied.Load(applied.Path(stateDir())) + if recErr != nil { + fmt.Fprintln(os.Stderr, "note: could not read the applied-ruleset record:", recErr) + } + backend, err := firewall.New() + if err != nil { + fmt.Fprintln(os.Stderr, "firewall backend unavailable:", err) + return 1 + } + text, loaded, err := backend.InstalledRules() + if err != nil { + fmt.Fprintln(os.Stderr, "could not read the installed rules:", err) + if !privilege.IsPrivileged() { + fmt.Fprintln(os.Stderr, "reading the firewall back needs root — try: sudo dezhban print-rules --installed") + } + return 1 + } + + out := installedRules{ + Installed: text, + Loaded: loaded, + Backend: firewall.RulesetKind, + Drift: hasRecord && !loaded, + } + if hasRecord { + out.Applied = &rec + } + if asJSON { + data, err := json.MarshalIndent(out, "", " ") + if err != nil { + fmt.Fprintln(os.Stderr, "encode failed:", err) + return 1 + } + fmt.Println(string(data)) + return 0 + } + + if out.Drift { + fmt.Fprintf(os.Stderr, "WARNING: dezhban recorded applying a %q ruleset at %s,\n", + rec.Mode, rec.At.Local().Format(time.RFC3339)) + fmt.Fprintln(os.Stderr, "but the kernel holds no dezhban rules. Something removed them.") + fmt.Fprintln(os.Stderr, "dezhban's own verification re-applies on its next tick; `dezhban status` will say.") + return 0 + } + if !loaded { + fmt.Fprintln(os.Stderr, "no dezhban rules are loaded (standby, or nothing running).") + return 0 + } + fmt.Fprintf(os.Stderr, "# %s rules currently loaded, read from the kernel\n", out.Backend) + if hasRecord { + fmt.Fprintf(os.Stderr, "# dezhban applied a %q ruleset at %s\n", + rec.Mode, rec.At.Local().Format(time.RFC3339)) + } + fmt.Print(text) + return 0 +} + // checkStatus classifies one doctorReport check for a machine consumer (the // macOS Diagnostics pane) without it having to parse Summary/Details prose. type checkStatus string diff --git a/docs/concepts/modes.md b/docs/concepts/modes.md index f434e66..3e6237d 100644 --- a/docs/concepts/modes.md +++ b/docs/concepts/modes.md @@ -444,3 +444,36 @@ dezhban print-rules --mode switch --config > Note these previews are static config, not the runtime posture: a config with > no tunnel previews as a full block here, while the running daemon idles > rule-free in STANDBY until a tunnel is actually observed up. + +## What is enforcing right now + +The previews above answer "what would this posture do?". Two other flags answer +"what is happening?", and they are deliberately different sources: + +```sh +dezhban print-rules --applied # what dezhban recorded installing, and when +sudo dezhban print-rules --installed # what the firewall itself holds +``` + +`--applied` reads a record the daemon writes on every successful apply, beside +the state file. It is dezhban's **own account** — the exact text it handed the +firewall, with the tunnel interfaces and endpoint addresses resolved at that +moment, which is why it can be more accurate than re-rendering the config after +the fact. It needs no root and works the same on every platform. It says nothing +about the kernel, and its label says so. + +`--installed` asks the firewall. It is scoped to dezhban's own +anchor/table/group, never a dump of unrelated firewall state, and it is a **read** +— it installs nothing and repairs nothing. It needs root, which is why nothing +runs it on a timer. + +When dezhban has a record of applying rules and the firewall holds none, +`--installed` reports it. That is the case something outside dezhban flushed the +firewall, and it is reported rather than repaired: the run loop's own +verification tick already re-applies missing rules, and a second repairer would +be a second writer. + +The two texts will **not** match byte for byte on a healthy host — the firewall +renders its own normalised form of what was loaded — so neither surface diffs +them. The macOS app shows all three (applied, in the kernel, and the per-posture +previews) in Diagnostics › Firewall rules. diff --git a/docs/contribute/testing.md b/docs/contribute/testing.md index 6524da1..81813a5 100644 --- a/docs/contribute/testing.md +++ b/docs/contribute/testing.md @@ -1202,6 +1202,35 @@ end up typing a password. `dezhban doctor` prints in a terminal. - [ ] CLI missing → the guided "dezhban CLI not found" state, not a blank list. +### Firewall rules (Diagnostics) + +- [ ] **Applied appears without a password.** With the guard up, open + Diagnostics: "Applied by dezhban — Guard" shows a timestamp and the pf + ruleset, with no prompt. Compare it against + `dezhban print-rules --applied` in a terminal — same text. +- [ ] **It tracks the posture.** Drive a block with `--simulate-country IR`; the + applied row becomes "Full block" and the timestamp moves. Open a switch + window; it becomes "Switch window". +- [ ] **Teardown clears it.** `sudo dezhban stop` (or `panic`), then re-open + Diagnostics: the row reads "no ruleset recorded yet". A stale ruleset shown + as live over an open network is the failure this must never have. +- [ ] **The kernel readback asks for a password and only reads.** "Read from the + kernel…" prompts once and shows `pfctl -a dezhban -s rules` output. Confirm + nothing changed: `dezhban status` and the posture are identical before and + after, and running it with the guard DOWN reports "no dezhban rules are + loaded" rather than an error. +- [ ] **Drift is reported, not repaired.** With the guard up, flush the anchor by + hand (`sudo pfctl -a dezhban -F rules`), then "Read from the kernel…": the + pane must warn that dezhban applied rules the firewall no longer holds, and + must offer **no** repair button. Then confirm the daemon's own verify tick + re-applies them within `vpn.advanced.verifyInterval` and the log says so. +- [ ] **The previews cost nothing and need no root.** As an unprivileged user + with dezhban stopped, expand each of Guard / Full block / Switch window: + each renders, and each matches `dezhban print-rules --mode `. +- [ ] **Only what is opened is fetched.** Visiting Diagnostics with every + disclosure collapsed must spawn no `print-rules` subprocess (watch with + `sudo fs_usage -w -f exec | grep dezhban`, or Activity Monitor). + ### Help pane The pane's whole reason for existing is that it works while the guard has cut diff --git a/docs/usage/cli.md b/docs/usage/cli.md index e4c76ac..bf7d07b 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -9,7 +9,7 @@ Commands: unblock Remove dezhban's firewall rules (root) status Show version, config, service, and block state (--json for tooling) validate Load + validate a config file (no root, no effects) - print-rules Print the ruleset a block/guard would apply, without applying it + print-rules Print the ruleset a block/guard would apply (--applied/--installed: what is live) doctor Diagnose VPN guard config (tunnels, endpoints, lockout risks) monitor Live read-only view: IP, country, tunnel state, endpoints, verdict panic Force-remove dezhban's rules even with no daemon (root) @@ -51,7 +51,7 @@ daemon** over its control socket and need no password at all — provided | Command | Needs a password? | |---|---| | `block`, `unblock`, `switch`, `pause`, `resume` | **No** — the running daemon performs them (see [config.md](config.md#control-block)). Only if no daemon is listening do they fall back — `block`/`unblock` act on the firewall directly; `switch`/`pause`/`resume` write the root-owned command file, which itself needs a running daemon to consume it. Either way, root. | -| `status`, `validate`, `print-rules`, `doctor`, `monitor`, `detect-vpn` | **No** — read-only, no root, no firewall effects. | +| `status`, `validate`, `print-rules`, `doctor`, `monitor`, `detect-vpn` | **No** — read-only, no root, no firewall effects. The one exception is `print-rules --installed`, which reads the firewall itself and therefore needs root; it still installs and changes nothing. | | `install`, `uninstall`, `start`, `stop`, `restart` | Yes — a daemon can't install, start, or stop itself. Rare (install-time). | | `panic` | Yes — deliberately independent of the daemon, so the lockout escape hatch works when nothing else does. | | `run` | Yes — it *is* the daemon. | @@ -239,6 +239,8 @@ Inspect and validate before you risk a block — none of these touch the firewal ```sh dezhban validate --config # parse + validate, summarize dezhban print-rules --mode guard --config # exact ruleset, not applied +dezhban print-rules --applied # what dezhban recorded installing +sudo dezhban print-rules --installed # what the firewall itself holds dezhban doctor --config # tunnels, subnets, endpoint sanity dezhban doctor --discover --config # macOS: find the VPN's real server IP dezhban doctor --json --config # the same checks as structured JSON @@ -246,7 +248,22 @@ dezhban monitor --config # live: IP, country, tunne ``` `monitor` streams the live state the decision rests on; add `--once` for a single -snapshot. `print-rules --mode` takes `guard`, `fullblock`, or `switch`. `doctor +snapshot. `print-rules --mode` takes `guard`, `fullblock`, or `switch`, and +renders purely — no root, no firewall effects. + +`--applied` and `--installed` answer the other question, "what is enforcing right +now?", from two deliberately different sources. `--applied` reads a record +dezhban writes on every successful apply (a 0644 file beside the state file, so +the menubar app can read it without root): the exact text handed to the firewall, +timestamped, with the interfaces and endpoints resolved at that moment. +`--installed` asks the firewall — scoped to dezhban's own anchor/table/group, +never a dump of unrelated state — and needs root for that reason. It is a read: +it installs nothing and repairs nothing. When dezhban recorded applying rules and +the firewall holds none, `--installed` says so; repairing that is the running +daemon's verification tick's job, not this command's. Add `--json` to either for +machine output. The two texts will not match byte for byte on a healthy host, so +neither surface diffs them — see +[modes.md](../concepts/modes.md#what-is-enforcing-right-now). `doctor --json` prints the identical findings `doctor` reports in prose — `{checks: [{name, status, summary, details, fixes}], ok}` — for a consumer (the macOS app's Diagnostics pane) that needs to render them itself rather than parse diff --git a/gui/macos/Sources/DezhbanCore/Rulesets.swift b/gui/macos/Sources/DezhbanCore/Rulesets.swift new file mode 100644 index 0000000..220adf7 --- /dev/null +++ b/gui/macos/Sources/DezhbanCore/Rulesets.swift @@ -0,0 +1,133 @@ +import Foundation + +/// The firewall rules dezhban recorded applying — `print-rules --applied --json`, +/// mirroring Go's `applied.Record`. +/// +/// This is dezhban's own account of what it installed, not a reading of the +/// kernel, and every surface showing it must say so. The distinction is not +/// pedantry: something outside dezhban can flush a firewall, and a pane that +/// called this "the current rules" would go on claiming the guard was enforcing +/// over a wide-open network. +public struct AppliedRuleset: Codable, Hashable { + public let mode: String + public let at: Date + public let rules: String + /// The mechanism the text is written for — "pf", "nft", "wfp" — so a reader + /// does not have to infer a syntax from the platform. + public let backend: String + + public init(mode: String, at: Date, rules: String, backend: String) { + self.mode = mode + self.at = at + self.rules = rules + self.backend = backend + } + + /// Go writes RFC 3339 with fractional seconds; `.iso8601` alone rejects + /// those, which would turn a perfectly good record into "no rules recorded". + public static func decode(_ data: Data) -> AppliedRuleset? { + for strategy in [rfc3339Fractional, rfc3339] { + let d = JSONDecoder() + d.dateDecodingStrategy = .formatted(strategy) + if let v = try? d.decode(AppliedRuleset.self, from: data) { return v } + } + return nil + } + + private static let rfc3339Fractional: DateFormatter = formatter("yyyy-MM-dd'T'HH:mm:ss.SSSSSSZZZZZ") + private static let rfc3339: DateFormatter = formatter("yyyy-MM-dd'T'HH:mm:ssZZZZZ") + + private static func formatter(_ format: String) -> DateFormatter { + let f = DateFormatter() + f.locale = Locale(identifier: "en_US_POSIX") + f.dateFormat = format + f.timeZone = TimeZone(secondsFromGMT: 0) + return f + } +} + +/// What the kernel actually holds — `print-rules --installed --json`. +/// +/// The privileged half of the picture, taken on demand rather than on a tick. +/// It is a READ: it installs nothing, so it does not touch the rule that only +/// the run loop may apply. +public struct InstalledRuleset: Hashable { + public let installed: String + /// False when dezhban has no rules in the kernel at all. An ordinary + /// answer — standby, or nothing running — never an error. + public let loaded: Bool + public let applied: AppliedRuleset? + /// True when dezhban has a record of applying rules and the kernel holds + /// none. Deliberately NOT a text diff: the kernel renders a normalised form + /// of what was loaded, so comparing bytes would report drift on every + /// healthy host. The texts are shown side by side for a person to read. + public let drift: Bool + public let backend: String + + public init(installed: String, loaded: Bool, applied: AppliedRuleset?, + drift: Bool, backend: String) { + self.installed = installed + self.loaded = loaded + self.applied = applied + self.drift = drift + self.backend = backend + } + + /// Decoded by hand rather than through Codable so the nested `applied` + /// record can reuse AppliedRuleset's two-format date handling. + public static func decode(_ data: Data) -> InstalledRuleset? { + guard let obj = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { + return nil + } + var nested: AppliedRuleset? + if let sub = obj["applied"], + let subData = try? JSONSerialization.data(withJSONObject: sub) { + nested = AppliedRuleset.decode(subData) + } + return InstalledRuleset( + installed: obj["installed"] as? String ?? "", + loaded: obj["loaded"] as? Bool ?? false, + applied: nested, + drift: obj["drift"] as? Bool ?? false, + backend: obj["backend"] as? String ?? "") + } +} + +/// The postures whose rulesets can be previewed without applying anything. +/// +/// These are the stable `print-rules --mode` identifiers, which CLAUDE.md pins +/// as part of the CLI contract — they are not display strings and must not be +/// renamed to read better. +public enum RulesetPreview: String, CaseIterable, Identifiable, Sendable { + case guardMode = "guard" + case fullBlock = "fullblock" + case switchWindow = "switch" + + public var id: String { rawValue } + + public var label: String { + switch self { + case .guardMode: return "Guard" + case .fullBlock: return "Full block" + case .switchWindow: return "Switch window" + } + } + + /// What this posture does to traffic, in one line — the caption beside the + /// rules, because a ruleset is not self-explanatory to the person most + /// likely to be reading it. + public var detail: String { + switch self { + case .guardMode: + return "The standing posture: only the VPN tunnel and the handshake to its server may leave. " + + "Everything else is dropped, so a tunnel drop cuts traffic with no leak window." + case .fullBlock: + return "What happens when the VPN's exit lands in a blocked country: the tunnel's own pass is " + + "removed too, so no traffic reaches that exit — but the handshake to the server stays " + + "open, so the VPN can still move." + case .switchWindow: + return "The bounded window you open deliberately to connect a new VPN. It closes early on a " + + "confirmed good exit, and always at its deadline." + } + } +} diff --git a/gui/macos/Sources/DezhbanMenu/AppState.swift b/gui/macos/Sources/DezhbanMenu/AppState.swift index f18f831..ed921ef 100644 --- a/gui/macos/Sources/DezhbanMenu/AppState.swift +++ b/gui/macos/Sources/DezhbanMenu/AppState.swift @@ -164,6 +164,15 @@ final class AppState: ObservableObject { @Published var doctorReport: DoctorReport? @Published var doctorError: String? @Published var doctorRunning = false + + /// The rules dezhban recorded applying, and the rules the kernel actually + /// holds. Two separate reads: the first is unprivileged and refreshed with + /// the rest of the pane, the second costs a password and only happens when + /// asked for. + @Published var appliedRules: AppliedRuleset? + @Published var installedRules: InstalledRuleset? + @Published var installedRulesError: String? + @Published var installedRulesRunning = false /// The sidebar's yellow dot: the last doctor report has something a person /// should look at. A dedicated Bool (not derived in the cell) so the /// sidebar can subscribe with removeDuplicates() and never reload at 1 Hz. @@ -327,6 +336,47 @@ final class AppState: ObservableObject { } } + /// Reads what dezhban recorded applying. Unprivileged and cheap — the record + /// is a small file beside state.json — so it refreshes with the rest of the + /// Diagnostics pane rather than on demand. + func refreshAppliedRules() { + guard cliFound else { return } + DispatchQueue.global(qos: .userInitiated).async { [weak self] in + let rules = DezhbanCLI.readAppliedRules() + DispatchQueue.main.async { self?.appliedRules = rules } + } + } + + /// Reads dezhban's rules back out of the kernel. Costs an admin prompt, so + /// it is never automatic. + /// + /// A READ — it installs nothing, changes nothing, and does not go through + /// `Backend.Apply`, so it leaves the run loop's single-writer rule alone. + /// There is deliberately no repair here either: the run loop's verification + /// tick already re-applies rules that go missing, and a second repairer + /// would be a second writer. + func readInstalledRules() { + guard !installedRulesRunning, cliFound else { return } + installedRulesRunning = true + installedRulesError = nil + DispatchQueue.global(qos: .userInitiated).async { [weak self] in + let r = DezhbanCLI.runPrivileged(["print-rules", "--installed", "--json"]) + let decoded = r.ok ? r.output.data(using: .utf8).flatMap(InstalledRuleset.decode) : nil + DispatchQueue.main.async { + guard let self else { return } + self.installedRulesRunning = false + if let decoded { + self.installedRules = decoded + } else { + self.installedRules = nil + self.installedRulesError = r.output.isEmpty + ? "No output from `dezhban print-rules --installed`." + : r.output + } + } + } + } + /// The background-trigger form: runs doctor only when the last report is /// older than maxAge (or absent). The staleness gate is load-bearing — /// callers include the essential-class edge into warning/blocked, and a diff --git a/gui/macos/Sources/DezhbanMenu/DezhbanCLI.swift b/gui/macos/Sources/DezhbanMenu/DezhbanCLI.swift index 2f7fa80..0bb6bde 100644 --- a/gui/macos/Sources/DezhbanMenu/DezhbanCLI.swift +++ b/gui/macos/Sources/DezhbanMenu/DezhbanCLI.swift @@ -272,6 +272,35 @@ enum DezhbanCLI { return ProfilesInfo.decode(data) } + /// Reads what dezhban recorded applying, via `print-rules --applied --json`. + /// Unprivileged: the record is a 0644 file beside state.json, written so the + /// menubar app can read it without root. + /// + /// nil covers both "nothing recorded" (the CLI prints `null`) and a CLI too + /// old to know the flag. The pane says "nothing recorded yet" either way, + /// which is true in both cases — it must never claim rules that are not + /// there. + static func readAppliedRules() -> AppliedRuleset? { + guard let bin = binaryPath() else { return nil } + let r = exec(bin, ["print-rules", "--applied", "--json"]) + guard r.status == 0, let data = r.out.data(using: .utf8) else { return nil } + return AppliedRuleset.decode(data) + } + + /// Renders what one posture WOULD apply, via `print-rules --mode `. + /// Pure, unprivileged, and with no firewall effects — the same guarantee the + /// command carries in a terminal. + /// + /// stdout only (`exec`, not `.run`): autodetect writes a timestamped line to + /// stderr on every call, and folding that into the rules would make the text + /// differ from run to run for no reason. + static func renderRules(mode: RulesetPreview) -> String? { + guard let bin = binaryPath() else { return nil } + let r = exec(bin, ["print-rules", "--mode", mode.rawValue, "--config", resolvedConfigPath()]) + guard r.status == 0, !r.out.isEmpty else { return nil } + return r.out + } + /// Reads all three presets and which (if any) matches the current config, /// via `config preset list --json`. static func readPresets() -> [PresetSummary]? { diff --git a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift index 52d06ab..b150960 100644 --- a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift +++ b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift @@ -23,6 +23,7 @@ struct DiagnosticsView: View { // refresh when what is there has gone stale. state.runDoctorIfStale(maxAge: 15 * 60) state.refreshVPNInventoryIfStale() + state.refreshAppliedRules() } } @@ -46,6 +47,7 @@ struct DiagnosticsView: View { private func run() { state.runDoctor(discover: discover) state.refreshVPNInventoryIfStale(maxAge: 0) + state.refreshAppliedRules() } @ViewBuilder @@ -77,6 +79,7 @@ struct DiagnosticsView: View { } } vpnInventorySection + firewallRulesSection if let report = state.doctorReport { Section { Label(report.ok ? "No lockout risk found" : "Found something to fix", @@ -100,6 +103,137 @@ struct DiagnosticsView: View { } } + // MARK: - firewall rules + + /// What the guard is doing to your traffic, in three parts, because they + /// answer three different questions and are not interchangeable: + /// + /// - **Applied** — what dezhban recorded installing, and when. Its own + /// account: cheap, unprivileged, and identical on every platform. + /// - **In the kernel** — what is actually loaded, read back on demand. + /// Costs a password, so it is never automatic. This is the half that can + /// see something outside dezhban having flushed the firewall. + /// - **Would apply** — the ruleset of each posture, rendered without + /// applying anything (`print-rules --mode`). The safe way to find out + /// what FULL BLOCK does before you are in it. + /// + /// The labels say which is which. "The current rules" would be a claim only + /// the middle one can make. + @ViewBuilder + private var firewallRulesSection: some View { + Section("Firewall rules") { + appliedRow + installedRow + previewRows + } + } + + @ViewBuilder + private var appliedRow: some View { + if let a = state.appliedRules { + rulesDisclosure( + title: "Applied by dezhban — \(postureLabel(a.mode))", + caption: "What dezhban installed at \(Self.stamp.string(from: a.at)), in \(a.backend) syntax. " + + "This is dezhban's own record, not a reading of the firewall.", + rules: a.rules) + } else { + Label("No ruleset recorded yet — dezhban writes one every time it applies rules. " + + "In standby it has applied none.", + systemImage: "doc.text") + .font(.callout) + .foregroundStyle(.secondary) + } + } + + @ViewBuilder + private var installedRow: some View { + VStack(alignment: .leading, spacing: 6) { + HStack(spacing: 10) { + Button("Read from the kernel…") { state.readInstalledRules() } + .disabled(state.installedRulesRunning || !state.cliFound) + .help("Asks the firewall itself what dezhban rules it holds. Needs your password. " + + "It only reads — nothing is installed, changed, or repaired.") + if state.installedRulesRunning { ProgressView().controlSize(.small) } + } + if let error = state.installedRulesError { + Label(error, systemImage: "exclamationmark.triangle.fill") + .font(.callout) + .foregroundStyle(.orange) + .textSelection(.enabled) + } + if let i = state.installedRules { + if i.drift { + // The finding, stated plainly. No repair button: the run + // loop's verification tick already re-applies rules that go + // missing, and a second repairer would be a second writer of + // the firewall. + Label("dezhban applied rules, but the firewall holds none. Something removed them. " + + "dezhban's own verification re-applies on its next check — this pane only reports.", + systemImage: "exclamationmark.triangle.fill") + .font(.callout) + .foregroundStyle(.orange) + } else if !i.loaded { + Label("No dezhban rules are loaded. That is expected in standby, or with dezhban stopped.", + systemImage: "info.circle") + .font(.callout) + .foregroundStyle(.secondary) + } else { + rulesDisclosure( + title: "In the kernel now", + caption: "Read back from the firewall, in \(i.backend) syntax. It will not match the " + + "applied text byte for byte — the firewall renders its own normalised form.", + rules: i.installed) + } + } + } + } + + @ViewBuilder + private var previewRows: some View { + ForEach(RulesetPreview.allCases) { mode in + rulesDisclosure( + title: "Would apply — \(mode.label)", + caption: mode.detail, + rules: nil, + load: { DezhbanCLI.renderRules(mode: mode) }) + } + } + + /// One collapsed ruleset. `rules` is text already in hand; `load` fetches it + /// the first time it is opened instead — the three previews each cost a + /// subprocess, and rendering all of them on every visit to this pane would + /// be three processes nobody asked for. + @ViewBuilder + private func rulesDisclosure(title: String, caption: String, + rules: String?, + load: (() -> String?)? = nil) -> some View { + DisclosureGroup { + RulesetBody(rules: rules, load: load) + } label: { + VStack(alignment: .leading, spacing: 2) { + Text(title).font(.callout.weight(.medium)) + Text(caption) + .font(.caption) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + } + } + } + + /// The posture strings are stable CLI identifiers, not display text, so they + /// are mapped rather than shown raw. An unknown one is shown as-is: a + /// daemon newer than this app is not a reason to hide what it said. + private func postureLabel(_ mode: String) -> String { + RulesetPreview(rawValue: mode)?.label ?? mode + } + + private static let stamp: DateFormatter = { + let f = DateFormatter() + f.dateStyle = .none + f.timeStyle = .medium + return f + }() + /// The VPN inventory (`detect-vpn --json`): which tunnels and VPN apps /// detection can see, and which one is connected now. Hidden entirely when /// the CLI is too old for the subcommand — degrade by omission, never a @@ -256,3 +390,47 @@ struct DiagnosticsView: View { } } + +/// The body of one ruleset disclosure: monospaced, selectable, and scrollable in +/// its own right so a long ruleset cannot stretch the pane. +/// +/// It exists as a view rather than a `@ViewBuilder` function so `load` can run +/// once, on first appearance, and hold its result. The three posture previews +/// each cost a `print-rules` subprocess; rendering them eagerly would spawn +/// three processes on every visit to Diagnostics for text nobody may open. +private struct RulesetBody: View { + let rules: String? + let load: (() -> String?)? + + @State private var loaded: String? + @State private var failed = false + + var body: some View { + Group { + if let text = rules ?? loaded { + ScrollView([.horizontal, .vertical]) { + Text(text) + .font(.system(.caption, design: .monospaced)) + .textSelection(.enabled) + .frame(maxWidth: .infinity, alignment: .leading) + } + .frame(maxHeight: 260) + } else if failed { + Text("Couldn't render this ruleset. `dezhban print-rules` needs a config it can read.") + .font(.caption) + .foregroundStyle(.secondary) + } else { + ProgressView().controlSize(.small) + } + } + .onAppear { + guard rules == nil, loaded == nil, let load else { return } + DispatchQueue.global(qos: .userInitiated).async { + let text = load() + DispatchQueue.main.async { + if let text { loaded = text } else { failed = true } + } + } + } + } +} diff --git a/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift b/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift new file mode 100644 index 0000000..8a27d6d --- /dev/null +++ b/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift @@ -0,0 +1,77 @@ +import Foundation +import Testing +@testable import DezhbanCore + +/// The producer side — what gets recorded, and when — is pinned by Go's +/// internal/applied and internal/runner tests. This is the consumer side: that +/// the app decodes what `print-rules --applied/--installed --json` emits. +struct RulesetsTests { + /// Go's encoding/json writes time.Time as RFC 3339 with fractional seconds. + /// Foundation's `.iso8601` strategy rejects those outright, which would turn + /// a perfectly good record into "no rules recorded" — a pane claiming the + /// guard had installed nothing while it was enforcing. + @Test func decodesGosFractionalTimestamps() throws { + let json = """ + {"version":1,"mode":"guard","at":"2026-08-21T14:02:11.123456+02:00", + "rules":"block drop out all\\n","backend":"pf"} + """ + let a = try #require(AppliedRuleset.decode(Data(json.utf8))) + #expect(a.mode == "guard") + #expect(a.backend == "pf") + #expect(a.rules == "block drop out all\n") + } + + /// Whole seconds, no fraction — what Go emits when the instant happens to + /// land on one. Both forms have to decode or the pane works only sometimes. + @Test func decodesWholeSecondTimestamps() throws { + let json = """ + {"version":1,"mode":"fullblock","at":"2026-08-21T14:02:11Z","rules":"x\\n","backend":"nft"} + """ + let a = try #require(AppliedRuleset.decode(Data(json.utf8))) + #expect(a.mode == "fullblock") + #expect(a.backend == "nft") + } + + /// `null` is what the CLI prints when nothing has been recorded — an + /// ordinary state, not a parse failure, and the caller shows "nothing + /// recorded yet" for both. + @Test func nullIsNotARecord() { + #expect(AppliedRuleset.decode(Data("null".utf8)) == nil) + } + + @Test func decodesAnInstalledReadbackWithItsNestedRecord() throws { + let json = """ + {"installed":"block drop out all\\n","loaded":true, + "applied":{"version":1,"mode":"guard","at":"2026-08-21T14:02:11.5Z", + "rules":"block drop out all\\n","backend":"pf"}, + "drift":false,"backend":"pf"} + """ + let i = try #require(InstalledRuleset.decode(Data(json.utf8))) + #expect(i.loaded) + #expect(!i.drift) + #expect(i.applied?.mode == "guard") + } + + /// Rules recorded, none in the kernel: the finding this readback exists to + /// surface. It must survive decoding intact — a drift flag lost in transit + /// is a tampering report nobody sees. + @Test func driftSurvivesDecoding() throws { + let json = """ + {"installed":"","loaded":false,"drift":true,"backend":"pf"} + """ + let i = try #require(InstalledRuleset.decode(Data(json.utf8))) + #expect(i.drift) + #expect(!i.loaded) + #expect(i.applied == nil) + } + + /// The preview modes are the stable `print-rules --mode` identifiers named + /// in CLAUDE.md. Renaming one to read better breaks the CLI contract. + @Test func previewModesAreTheStableCLIIdentifiers() { + #expect(RulesetPreview.allCases.map(\.rawValue) == ["guard", "fullblock", "switch"]) + for mode in RulesetPreview.allCases { + #expect(!mode.label.isEmpty) + #expect(!mode.detail.isEmpty) + } + } +} diff --git a/internal/applied/applied.go b/internal/applied/applied.go new file mode 100644 index 0000000..c528d44 --- /dev/null +++ b/internal/applied/applied.go @@ -0,0 +1,112 @@ +// Package applied records the firewall ruleset the daemon last installed, so a +// diagnostic surface can show what is actually being enforced rather than +// asking the reader to re-derive it. +// +// `dezhban print-rules --mode guard|fullblock|switch` already renders what each +// posture WOULD apply — pure, root-free, and available at any time. What was +// missing is the other half: which of those is live right now, rendered from the +// policy that was actually handed to the backend, including the tunnel +// interfaces and endpoint addresses resolved at that moment. Those change while +// the daemon runs, so re-rendering after the fact can quietly disagree with what +// the kernel holds. +// +// This is dezhban's own account of what it did, not a reading of the kernel. It +// is the cheap half of the picture and works identically on every platform; the +// GUI pairs it with an on-demand privileged readback, and a disagreement between +// the two is itself the finding. Deliberately NOT a substitute for the run +// loop's verify tick, which is what notices and repairs rules going missing. +// +// The record lives beside the state file (see cmd/dezhban.defaultStatePath), +// same convention as internal/learned and internal/armed: daemon-owned, +// machine-derived, never the user's config, and safe to discard — a missing or +// corrupt file just means "nothing recorded yet". Every write is a whole-file +// atomic replace, so a reader never sees a torn file. Mode 0644 like state.json: +// the unprivileged menubar app has to be able to read it, and it holds nothing +// `print-rules` would not print for free. +package applied + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/behnam-rk/dezhban/internal/atomicfile" + "github.com/behnam-rk/dezhban/internal/state" +) + +// version is the on-disk schema version. Bump on an incompatible change. +const version = 1 + +// FileName is the record's name within the state directory. +const FileName = "applied-rules.json" + +// Record is the whole applied-rules.json document. +type Record struct { + Version int `json:"version"` + // Mode is the posture string the ruleset installs — the same stable + // identifier print-rules --mode takes ("guard", "fullblock", "switch"). + Mode string `json:"mode"` + // At is when the apply succeeded. A reader shows it verbatim: "what dezhban + // applied at 14:02:11" is an honest label in a way "the current rules" is + // not, because nothing here observes the kernel. + At time.Time `json:"at"` + // Rules is the exact text handed to the backend. + Rules string `json:"rules"` + // Backend names the mechanism the text is written for ("pf", "nft", "wfp"), + // so a reader does not have to infer a syntax from the platform it happens + // to be running on. + Backend string `json:"backend"` +} + +// Path returns the record's path within the given state directory. +func Path(stateDir string) string { return filepath.Join(stateDir, FileName) } + +// Save writes the record atomically. Errors are the caller's to log and +// swallow: this is a diagnostic aid, and failing to record what was applied +// must never be a reason not to apply it. +func Save(path string, r Record) error { + r.Version = version + data, err := json.MarshalIndent(r, "", " ") + if err != nil { + return fmt.Errorf("encode %s: %w", FileName, err) + } + if dir := filepath.Dir(path); dir != "" && dir != "." { + if err := os.MkdirAll(dir, state.DirMode); err != nil { + return fmt.Errorf("create %s: %w", dir, err) + } + } + return atomicfile.Write(path, append(data, '\n'), 0o644) +} + +// Load reads the record. A missing file is (Record{}, false, nil) — "nothing +// recorded yet" is an ordinary state, not an error, and the surfaces that read +// this must say so rather than reporting a failure. +func Load(path string) (Record, bool, error) { + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return Record{}, false, nil + } + if err != nil { + return Record{}, false, fmt.Errorf("read %s: %w", path, err) + } + var r Record + if err := json.Unmarshal(data, &r); err != nil { + // Same call as learned.json and armed.json: a corrupt record is + // discarded, never fatal. It describes the past, and the daemon's + // enforcement does not depend on it. + return Record{}, false, fmt.Errorf("parse %s: %w", path, err) + } + return r, true, nil +} + +// Remove deletes the record. Called when rules are torn down, so a stale +// ruleset cannot be read as current after an Unblock or Cleanup. A missing file +// is success. +func Remove(path string) error { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} diff --git a/internal/applied/applied_test.go b/internal/applied/applied_test.go new file mode 100644 index 0000000..d1a0792 --- /dev/null +++ b/internal/applied/applied_test.go @@ -0,0 +1,92 @@ +package applied + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +func TestSaveLoadRoundTrip(t *testing.T) { + path := Path(t.TempDir()) + want := Record{ + Mode: "guard", + At: time.Date(2026, 8, 21, 14, 2, 11, 0, time.UTC), + Rules: "pass out quick on utun4 all\nblock drop out all\n", + Backend: "pf", + } + if err := Save(path, want); err != nil { + t.Fatalf("Save: %v", err) + } + got, ok, err := Load(path) + if err != nil || !ok { + t.Fatalf("Load: ok=%v err=%v", ok, err) + } + if got.Mode != want.Mode || got.Rules != want.Rules || got.Backend != want.Backend { + t.Errorf("round trip lost data: %+v", got) + } + if !got.At.Equal(want.At) { + t.Errorf("At = %v, want %v", got.At, want.At) + } + if got.Version != version { + t.Errorf("Version = %d, want %d", got.Version, version) + } +} + +// The GUI runs unprivileged and has to be able to read this, exactly like +// state.json. 0600 would make the pane useless to the surface it exists for. +func TestRecordIsWorldReadable(t *testing.T) { + path := Path(t.TempDir()) + if err := Save(path, Record{Mode: "guard"}); err != nil { + t.Fatalf("Save: %v", err) + } + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if fi.Mode().Perm() != 0o644 { + t.Errorf("mode = %v, want 0644", fi.Mode().Perm()) + } +} + +// "Nothing recorded yet" is an ordinary state — a daemon in standby has applied +// nothing — and must not read as a failure to the surfaces that show it. +func TestMissingFileIsNotAnError(t *testing.T) { + _, ok, err := Load(filepath.Join(t.TempDir(), "nope.json")) + if ok || err != nil { + t.Errorf("ok=%v err=%v, want false/nil", ok, err) + } +} + +// A stale ruleset read as current after teardown would say the guard is +// enforcing when nothing is. +func TestRemoveClearsTheRecordAndIsIdempotent(t *testing.T) { + path := Path(t.TempDir()) + if err := Save(path, Record{Mode: "guard"}); err != nil { + t.Fatal(err) + } + for i := 0; i < 2; i++ { + if err := Remove(path); err != nil { + t.Fatalf("Remove #%d: %v", i, err) + } + } + if _, ok, _ := Load(path); ok { + t.Error("record survived Remove") + } +} + +// Corrupt is discarded, never fatal: it describes the past, and enforcement +// does not depend on it. Same call as learned.json and armed.json. +func TestCorruptRecordIsDiscardedNotFatal(t *testing.T) { + path := Path(t.TempDir()) + if err := os.WriteFile(path, []byte("{not json"), 0o644); err != nil { + t.Fatal(err) + } + _, ok, err := Load(path) + if ok { + t.Error("a corrupt record was reported as usable") + } + if err == nil { + t.Error("a corrupt record should still be reported to the caller to log") + } +} diff --git a/internal/firewall/backend.go b/internal/firewall/backend.go index 51b2880..e4ef027 100644 --- a/internal/firewall/backend.go +++ b/internal/firewall/backend.go @@ -134,4 +134,17 @@ type FirewallBackend interface { // Cleanup is an always-safe, best-effort teardown for shutdown/panic. It // never returns fatally; failures are the caller's to log. Cleanup() error + // InstalledRules reads dezhban's rules back OUT of the kernel, as text, for + // a diagnostic surface to compare against what the daemon recorded applying + // (internal/applied). Scoped to dezhban's own tag/anchor/table like every + // other operation here — it must never dump unrelated firewall state. + // + // It is a READ. It installs nothing and changes nothing, so it does not + // belong to the single-writer rule that governs Apply: any goroutine, and + // any process, may call it. It does generally need root, which is why it is + // on demand rather than on a tick. + // + // The bool is false when dezhban has no rules loaded at all — an ordinary + // answer (standby, or nothing running), not an error. + InstalledRules() (string, bool, error) } diff --git a/internal/firewall/nft_linux.go b/internal/firewall/nft_linux.go index 37d9f12..e5feb4b 100644 --- a/internal/firewall/nft_linux.go +++ b/internal/firewall/nft_linux.go @@ -121,6 +121,31 @@ func (b *nftBackend) IsBlocked() (bool, error) { return outputChainPolicyIsDrop(out), nil } +// InstalledRules renders dezhban's own table back out of the kernel. +// +// Scoped to `inet dezhban` by listTable, so it reports our table and nothing +// else — it can never become a way to dump a user's unrelated nftables +// configuration. A read: it installs nothing, needs no lock here, and is safe +// from any goroutine or process. It does need root/CAP_NET_ADMIN, which is why +// nothing calls it on a tick. +// +// A table with an output chain whose policy has drifted off drop is loaded but +// not enforcing — the same gap IsBlocked checks — so the text says so, because +// whoever is reading it has to be able to see that. +func (b *nftBackend) InstalledRules() (string, bool, error) { + out, exists, err := b.listTable() + if err != nil || !exists { + return "", false, err + } + var sb strings.Builder + if !outputChainPolicyIsDrop(out) { + sb.WriteString("# WARNING: the output chain's policy is no longer drop —\n") + sb.WriteString("# this table is loaded but is not cutting anything.\n") + } + sb.WriteString(out) + return sb.String(), true, nil +} + // outputChainPolicyIsDrop reports whether nft's rendered `list table` output // still shows the output chain's hook policy as drop. Split out from // IsBlocked so it can be exercised in tests against captured `nft list table` diff --git a/internal/firewall/pf_darwin.go b/internal/firewall/pf_darwin.go index 1af410d..899de8e 100644 --- a/internal/firewall/pf_darwin.go +++ b/internal/firewall/pf_darwin.go @@ -202,6 +202,42 @@ func (b *pfBackend) IsBlocked() (bool, error) { return mainRulesetReferencesAnchor(main), nil } +// InstalledRules reads dezhban's anchor back out of the kernel. +// +// Scoped to `-a dezhban` exactly like every other operation here: it reports our +// own rules and nothing else, so it can never become a way to dump a user's +// unrelated pf configuration. The anchor reference line from the main ruleset is +// prepended when present, because a loaded anchor that the main ruleset does not +// reference is not being evaluated at all — the same gap IsBlocked checks for, +// and the reader of this text has to be able to see it. +// +// A read, not a write: it takes no lock in this package and is safe from any +// goroutine or process. It does need root, which is why nothing calls it on a +// tick. +func (b *pfBackend) InstalledRules() (string, bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), pfctlTimeout) + defer cancel() + + rules, err := pfctlCtx(ctx, "", "-a", anchorName, "-s", "rules") + if err != nil { + return "", false, fmt.Errorf("read the dezhban anchor: %w", err) + } + if strings.TrimSpace(rules) == "" { + return "", false, nil + } + var b0 strings.Builder + if main, err := pfctlCtx(ctx, "", "-s", "rules"); err == nil { + if mainRulesetReferencesAnchor(main) { + b0.WriteString("# main ruleset references the dezhban anchor\n") + } else { + b0.WriteString("# WARNING: the main ruleset does NOT reference the dezhban anchor —\n") + b0.WriteString("# these rules are loaded but pf never descends into them.\n") + } + } + b0.WriteString(rules) + return b0.String(), true, nil +} + // mainRulesetReferencesAnchor reports whether pfctl's rendered main ruleset // still contains our anchor reference. Split out from IsBlocked so it can be // exercised in tests against captured `pfctl -s rules` output without diff --git a/internal/firewall/render_darwin.go b/internal/firewall/render_darwin.go index 0a7a61e..af3d8ed 100644 --- a/internal/firewall/render_darwin.go +++ b/internal/firewall/render_darwin.go @@ -11,3 +11,8 @@ package firewall func RenderRules(p Policy) (string, error) { return renderRuleset(p), nil } + +// RulesetKind names the mechanism RenderRules writes for, so a surface showing +// the text does not have to infer a syntax from the platform it happens to be +// running on. Here: the pf ruleset `pfctl -a dezhban -f -` loads. +const RulesetKind = "pf" diff --git a/internal/firewall/render_linux.go b/internal/firewall/render_linux.go index b729e31..e708c56 100644 --- a/internal/firewall/render_linux.go +++ b/internal/firewall/render_linux.go @@ -9,3 +9,8 @@ package firewall func RenderRules(p Policy) (string, error) { return renderNftRuleset(p), nil } + +// RulesetKind names the mechanism RenderRules writes for, so a surface showing +// the text does not have to infer a syntax from the platform it happens to be +// running on. Here: the nftables ruleset `nft -f -` loads. +const RulesetKind = "nft" diff --git a/internal/firewall/render_windows.go b/internal/firewall/render_windows.go index 662524e..59d094c 100644 --- a/internal/firewall/render_windows.go +++ b/internal/firewall/render_windows.go @@ -9,3 +9,8 @@ package firewall func RenderRules(p Policy) (string, error) { return renderBlockScript(p), nil } + +// RulesetKind names the mechanism RenderRules writes for, so a surface showing +// the text does not have to infer a syntax from the platform it happens to be +// running on. Here: the PowerShell that installs the WFP rules. +const RulesetKind = "wfp" diff --git a/internal/firewall/wfp_windows.go b/internal/firewall/wfp_windows.go index c1a18bc..e681e78 100644 --- a/internal/firewall/wfp_windows.go +++ b/internal/firewall/wfp_windows.go @@ -204,6 +204,34 @@ func (b *wfpBackend) IsBlocked() (bool, error) { return true, nil } +// InstalledRules renders dezhban's own firewall rules back out of Windows, plus +// each profile's default outbound action — which is where the actual blocking +// lives on this platform (see the Model note above renderBlockScript), so a list +// of allow rules without it would be a misleading half of the picture. +// +// Scoped to `-Group dezhban`, exactly like Remove-NetFirewallRule, so it reports +// our rules and nothing else. A read: it changes nothing and is safe from any +// goroutine or process. It does need an elevated shell, which is why nothing +// calls it on a tick. +func (b *wfpBackend) InstalledRules() (string, bool, error) { + script := strings.Join([]string{ + "$g = Get-NetFirewallRule -Group " + groupName + " -ErrorAction SilentlyContinue", + "if ($null -eq $g) { 'NONE'; exit 0 }", + "'# default outbound action per profile'", + "Get-NetFirewallProfile | Select-Object Name,DefaultOutboundAction | Format-Table -AutoSize | Out-String", + "'# dezhban rules'", + "$g | Select-Object DisplayName,Direction,Action,Enabled | Format-Table -AutoSize | Out-String", + }, "\n") + out, err := powershell(script) + if err != nil { + return "", false, fmt.Errorf("read the dezhban firewall group: %w", err) + } + if strings.TrimSpace(out) == "NONE" { + return "", false, nil + } + return out, true, nil +} + // queryBlockedAndDefaults combines the group-existence check and the // per-profile DefaultOutboundAction query into a single PowerShell // invocation. IsBlocked is called synchronously from the run loop's verifyC diff --git a/internal/runner/recording.go b/internal/runner/recording.go new file mode 100644 index 0000000..27350e9 --- /dev/null +++ b/internal/runner/recording.go @@ -0,0 +1,101 @@ +package runner + +import ( + "io" + "log/slog" + "time" + + "github.com/behnam-rk/dezhban/internal/applied" + "github.com/behnam-rk/dezhban/internal/firewall" +) + +// recordingBackend records what was applied, then gets out of the way. +// +// A decorator rather than a `applied.Save` beside each `Backend.Apply`: the run +// loop applies from nineteen places, and a record that is only as complete as +// the last person to remember it is worse than none — a surface would show a +// stale posture with no way to tell. Wrapping makes a new call site recorded by +// construction. +// +// It preserves the single-writer invariant exactly, because it adds no writer: +// every method is called from the run-loop goroutine, by the same code that +// called the wrapped backend before. That also means the fields below need no +// locking, and nothing here may be moved onto another goroutine. The write is +// an atomic replace of a small file — bounded work, on the goroutine that owns +// window expiry and geo ticks, which is why it must stay that shape. +// +// Every failure to record is logged and swallowed. This is a diagnostic aid; +// failing to write down what was applied must never become a reason not to +// apply it, and must never turn a successful enforcement into a returned error. +type recordingBackend struct { + // Embedded so the wrapper stays exactly as narrow as the interface the run + // loop uses. Widening Backend to carry a diagnostic read would put a method + // on the enforcement seam that enforcement never calls. + Backend + path string + log *slog.Logger + // now is injected so a test can assert the recorded timestamp instead of + // asserting that some time passed. + now func() time.Time +} + +// newRecordingBackend wraps b when path is non-empty; otherwise it returns b +// unchanged, so a caller with no state directory (tests, Windows service +// harnesses) is unaffected. +func newRecordingBackend(b Backend, path string, log *slog.Logger) Backend { + if path == "" || b == nil { + return b + } + if log == nil { + // Run does not default a nil Log, and every method here logs on the + // failure path. A diagnostic aid must not be the thing that panics the + // daemon on the one day the disk is full. + log = slog.New(slog.NewTextHandler(io.Discard, nil)) + } + return &recordingBackend{Backend: b, path: path, log: log, now: time.Now} +} + +func (r *recordingBackend) Apply(p firewall.Policy) error { + // Record only what actually landed. A failed Apply leaves the previous + // ruleset live, so overwriting the record first would describe rules that + // were never installed — the one thing a surface reading this must be able + // to rely on not happening. + if err := r.Backend.Apply(p); err != nil { + return err + } + rules, err := firewall.RenderRules(p) + if err != nil { + r.log.Warn("could not render the applied ruleset for the diagnostics record", "err", err) + return nil + } + rec := applied.Record{ + Mode: p.Mode.String(), + At: r.now(), + Rules: rules, + Backend: firewall.RulesetKind, + } + if err := applied.Save(r.path, rec); err != nil { + r.log.Warn("could not record the applied ruleset", "err", err, "path", r.path) + } + return nil +} + +func (r *recordingBackend) Unblock() error { + err := r.Backend.Unblock() + // Clear even when Unblock failed: the rules are in an unknown state, and a + // record that confidently names the old posture is worse than none. + r.clear() + return err +} + +func (r *recordingBackend) Cleanup() error { + err := r.Backend.Cleanup() + r.clear() + return err +} + +func (r *recordingBackend) clear() { + if err := applied.Remove(r.path); err != nil { + r.log.Warn("could not clear the applied-ruleset record", "err", err, "path", r.path) + } +} diff --git a/internal/runner/recording_test.go b/internal/runner/recording_test.go new file mode 100644 index 0000000..a4db03a --- /dev/null +++ b/internal/runner/recording_test.go @@ -0,0 +1,131 @@ +package runner + +import ( + "errors" + "net/netip" + "testing" + "time" + + "github.com/behnam-rk/dezhban/internal/applied" + "github.com/behnam-rk/dezhban/internal/firewall" +) + +func recordingAt(t *testing.T, at time.Time) (Backend, *fakeBackend, string) { + t.Helper() + inner := &fakeBackend{} + path := applied.Path(t.TempDir()) + b := newRecordingBackend(inner, path, discardLog()) + b.(*recordingBackend).now = func() time.Time { return at } + return b, inner, path +} + +func guardPolicy() firewall.Policy { + return firewall.Policy{ + Mode: firewall.ModeGuard, + TunnelIfaces: []string{"utun4"}, + VPNEndpoints: []netip.Addr{netip.MustParseAddr("203.0.113.7")}, + } +} + +func TestRecordingBackendRecordsWhatItApplied(t *testing.T) { + at := time.Date(2026, 8, 21, 14, 2, 11, 0, time.UTC) + b, inner, path := recordingAt(t, at) + + if err := b.Apply(guardPolicy()); err != nil { + t.Fatalf("Apply: %v", err) + } + if len(inner.policies) != 1 { + t.Fatalf("the wrapped backend saw %d applies, want 1", len(inner.policies)) + } + + rec, ok, err := applied.Load(path) + if err != nil || !ok { + t.Fatalf("Load: ok=%v err=%v", ok, err) + } + if rec.Mode != "guard" { + t.Errorf("Mode = %q, want \"guard\"", rec.Mode) + } + if !rec.At.Equal(at) { + t.Errorf("At = %v, want %v", rec.At, at) + } + if rec.Backend != firewall.RulesetKind { + t.Errorf("Backend = %q, want %q", rec.Backend, firewall.RulesetKind) + } + // The recorded text must be what this policy renders, not a re-render of + // some later state: the resolved endpoint has to be in it. + want, err := firewall.RenderRules(guardPolicy()) + if err != nil { + t.Fatal(err) + } + if rec.Rules != want { + t.Errorf("recorded rules differ from RenderRules for the same policy") + } +} + +// A failed Apply leaves the PREVIOUS ruleset live. Recording the attempt would +// describe rules that were never installed — the one thing a reader of this +// file has to be able to rely on not happening. +func TestAFailedApplyRecordsNothing(t *testing.T) { + b, inner, path := recordingAt(t, time.Unix(0, 0)) + if err := b.Apply(guardPolicy()); err != nil { + t.Fatal(err) + } + first, _, _ := applied.Load(path) + + inner.applyErr = errors.New("pfctl exploded") + fullBlock := firewall.Policy{Mode: firewall.ModeFullBlock} + if err := b.Apply(fullBlock); err == nil { + t.Fatal("Apply returned nil for a failing backend") + } + + after, ok, _ := applied.Load(path) + if !ok { + t.Fatal("the previous record was destroyed by a failed apply") + } + if after.Mode != first.Mode || after.Rules != first.Rules { + t.Errorf("a failed apply overwrote the record: %q", after.Mode) + } +} + +// After teardown there are no rules. A record left behind would be read as the +// live posture — a surface saying "guard is enforcing" over an open network. +func TestUnblockAndCleanupClearTheRecord(t *testing.T) { + for _, tc := range []struct { + name string + call func(Backend) error + }{ + {"unblock", func(b Backend) error { return b.Unblock() }}, + {"cleanup", func(b Backend) error { return b.Cleanup() }}, + } { + t.Run(tc.name, func(t *testing.T) { + b, _, path := recordingAt(t, time.Unix(0, 0)) + if err := b.Apply(guardPolicy()); err != nil { + t.Fatal(err) + } + if err := tc.call(b); err != nil { + t.Fatal(err) + } + if _, ok, _ := applied.Load(path); ok { + t.Error("the record survived teardown") + } + }) + } +} + +// An empty path is "recording off" and must hand back the backend untouched, so +// a caller with no state directory pays nothing and behaves identically. +func TestNoPathMeansNoWrapper(t *testing.T) { + inner := &fakeBackend{} + if got := newRecordingBackend(inner, "", discardLog()); got != Backend(inner) { + t.Error("an empty path still wrapped the backend") + } +} + +// Run does not default a nil Log, and every failure path here logs. A +// diagnostic aid must not be what panics the daemon. +func TestANilLoggerDoesNotPanic(t *testing.T) { + b := newRecordingBackend(&fakeBackend{}, applied.Path(t.TempDir()), nil) + if err := b.Apply(guardPolicy()); err != nil { + t.Fatalf("Apply: %v", err) + } +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index ed17604..1537dc8 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -375,6 +375,12 @@ type Options struct { // BlockedCountries is copied verbatim into each published snapshot so an // observer can show what the daemon is configured to block. Informational only. BlockedCountries []string + // AppliedRulesPath, when non-empty, is where the ruleset text of each + // successful Apply is recorded (internal/applied) for the Diagnostics pane. + // Run wraps Backend to do it, so every Apply is covered including ones added + // later. Purely diagnostic and best-effort: a failed write is logged and the + // enforcement stands. Empty → nothing is recorded. + AppliedRulesPath string // ReloadC delivers replacement settings to the running loop, so a config // edit takes effect without a restart. Nil (the default) means reloading is @@ -607,6 +613,12 @@ func (o Options) pendingFlip(standby, windowOpen bool) *state.PendingFlip { // the daemon — that is the invariant that keeps the operator from being locked // out of their own network. func Run(ctx context.Context, o Options) error { + // Wrap BEFORE anything can apply — including the deferred Cleanup below, + // which has to clear the record rather than leave a ruleset on disk that a + // reader would take for live. Adds no goroutine and no writer: every call + // still comes from this loop. + o.Backend = newRecordingBackend(o.Backend, o.AppliedRulesPath, o.Log) + defer func() { if err := o.Backend.Cleanup(); err != nil { o.Log.Warn("cleanup failed; rules may persist (run `dezhban panic`)", "err", err)