From 9167eaf09d1eef4ec73462e0f2a2b1d3be21c4cf Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Fri, 21 Aug 2026 12:16:47 +0330 Subject: [PATCH 1/8] 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 c34dd5b..060693b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,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, this app's preferences and saved window state), then opens 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 84d23ad..6ff6df6 100644 --- a/docs/contribute/testing.md +++ b/docs/contribute/testing.md @@ -1362,6 +1362,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 7bdbebb..654dcc0 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 07424cc..76e7892 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. @@ -354,6 +363,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 1a41ae4..14e6501 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) From 48340c2a029b3a576e1b60efea51ff9c5d547293 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Sun, 6 Sep 2026 09:31:03 +0330 Subject: [PATCH 2/8] fix(print-rules): refuse the flag combinations it cannot honour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found reading the diff before review, not by the reviewer. print-rules now carries two kinds of flag: one describing a ruleset to RENDER (--mode) and two selecting a live ruleset to REPORT (--applied, --installed). Only the --applied/--installed pair was refused. The other two combinations were accepted and half-discarded: print-rules --applied --mode fullblock # --mode ignored, exit 0 print-rules --json # --json ignored, prints text Both are the shape this project calls its worst bug — a flag accepted and then quietly dropped — and the second is the more misleading, since a caller parsing that output gets firewall syntax where it asked for JSON. Each is now refused with exit 2 naming the flag to drop, matching the existing --applied/--installed refusal. The check is on what the user TYPED, via fs.Visit, not on flag values: --mode defaults to "guard", so testing its value would reject every plain --applied run. TestPrintRulesAppliedIsFineWithoutMode pins that, and TestPrintRulesRefusesFlagsItCannotHonour covers all four refusals — the two new cases return 0 and 1 on the unfixed code. Also: CLAUDE.md said the privileged set was "exactly" a list that did not include this, and listed print-rules among the commands needing no root. `--installed` reads the kernel back and does need it. Corrected using the same "X but not X --sub" idiom the paragraph already uses for `setup` and `vpn list`, and the refusals are documented in cli.md. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 9 ++++--- cmd/dezhban/main.go | 17 ++++++++++++ cmd/dezhban/print_rules_flags_test.go | 38 +++++++++++++++++++++++++++ docs/usage/cli.md | 9 ++++++- 4 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 cmd/dezhban/print_rules_flags_test.go diff --git a/CLAUDE.md b/CLAUDE.md index a2feaed..7016571 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,13 +76,16 @@ The **privileged set** — requires root/admin — is exactly: `run`, `block`, `pause`, `resume`, `vpn add`/`remove`/`promote`/`forget`/`import` (but not `vpn list`/`show`), `setup` (but not `setup --questions`, which asks nothing and only reports what the wizard would ask), `config set`/`edit`/`preset apply`, `token enroll`/`forget` (but not -`token status`), and `upgrade download`/`upgrade apply` (macOS only — `download`'s staging directory is root-owned so a local user -can't swap the verified `.pkg` before `apply` installs it). `switch`, `pause`, +`token status`), `upgrade download`/`upgrade apply` (macOS only — `download`'s staging directory is root-owned so a local user +can't swap the verified `.pkg` before `apply` installs it), and `print-rules --installed` +(but not the rest of `print-rules`, which stays pure rendering — `--installed` reads the +kernel back through `FirewallBackend.InstalledRules`, which needs root; it is still a +READ and installs nothing). `switch`, `pause`, and `resume` are usually passwordless in practice: they ask the running daemon over its control socket first (gated by `control.allowSwitchOps`/ `control.allowPauseOps` respectively) and only fall back to the root-owned command file when no daemon answers. Everything else — `status`, `detect-vpn`, -`validate`, `print-rules`, `doctor`, `monitor`, `vpn list`/`show`, +`validate`, `print-rules` (except `--installed`, above), `doctor`, `monitor`, `vpn list`/`show`, `config show`/`path`/`schema`/`preset list`/`preset show`/`preset diff`, `token status`, `completion`, `upgrade check`, `version`, `help` — is read-only: no root, no firewall effects. Full reference: diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index b27eefe..d022181 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -1817,11 +1817,28 @@ func cmdPrintRules(args []string) int { asJSON := fs.Bool("json", false, "machine-readable output (with --applied or --installed)") _ = fs.Parse(args) + // Which flags the user actually typed, as opposed to their defaults. --mode + // has a non-empty default, so its value alone cannot tell the two apart, and + // a flag that is accepted and then quietly discarded is the shape of bug + // this tool least wants to ship. + typed := map[string]bool{} + fs.Visit(func(f *flag.Flag) { typed[f.Name] = true }) + 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 || *installed) && typed["mode"] { + fmt.Fprintln(os.Stderr, "--mode renders a posture that is not in force; --applied and --installed report the one that is.") + fmt.Fprintln(os.Stderr, "Drop --mode to report the live ruleset, or drop --applied/--installed to render a hypothetical one.") + return 2 + } + if typed["json"] && !*appliedOnly && !*installed { + fmt.Fprintln(os.Stderr, "--json needs --applied or --installed; a rendered ruleset is firewall syntax, not JSON.") + fmt.Fprintln(os.Stderr, "Use 'print-rules --mode ' for the text, or add --applied/--installed for a JSON document.") + return 2 + } if *appliedOnly { return printAppliedRules(*asJSON) } diff --git a/cmd/dezhban/print_rules_flags_test.go b/cmd/dezhban/print_rules_flags_test.go new file mode 100644 index 0000000..69a3e2d --- /dev/null +++ b/cmd/dezhban/print_rules_flags_test.go @@ -0,0 +1,38 @@ +package main + +import "testing" + +// print-rules carries two kinds of flag: ones describing a ruleset to RENDER +// (--mode) and ones selecting a live ruleset to REPORT (--applied, --installed). +// Mixing them, or asking for --json where the output is firewall syntax, is a +// request that cannot be honoured — and accepting it while quietly ignoring half +// of it is the failure this project treats as the worst kind. +func TestPrintRulesRefusesFlagsItCannotHonour(t *testing.T) { + for _, tc := range []struct { + name string + args []string + want int + }{ + {"two sources at once", []string{"--applied", "--installed"}, 2}, + {"mode with applied", []string{"--applied", "--mode", "fullblock"}, 2}, + {"mode with installed", []string{"--installed", "--mode", "guard"}, 2}, + {"json with no source", []string{"--json"}, 2}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := cmdPrintRules(tc.args); got != tc.want { + t.Errorf("cmdPrintRules(%v) = %d, want %d", tc.args, got, tc.want) + } + }) + } +} + +// The refusal must be about what the user TYPED, not about a flag's default. +// --mode defaults to "guard", so testing its value instead of whether it was +// given would reject every plain --applied run. +func TestPrintRulesAppliedIsFineWithoutMode(t *testing.T) { + // No record exists under the test's state dir, which is the documented + // "nothing recorded" answer: exit 0, not a refusal and not a failure. + if got := cmdPrintRules([]string{"--applied"}); got != 0 { + t.Errorf("cmdPrintRules(--applied) = %d, want 0", got) + } +} diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 654dcc0..244fec1 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -263,7 +263,14 @@ 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 +[modes.md](../concepts/modes.md#what-is-enforcing-right-now). + +The three selectors are mutually exclusive and saying so is an error rather than +a silent preference: `--applied` with `--installed` is refused (they are two +sources, not two views of one), so is `--mode` with either (it renders a posture +that is *not* in force, which is the opposite question), and so is `--json` +without one of them (a rendered ruleset is firewall syntax, not JSON). Each exits +2 and says which flag to drop. `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 From 508b690392f0ce7c7aaeab4cf1bb08518db47912 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Sun, 6 Sep 2026 09:46:33 +0330 Subject: [PATCH 3/8] fix(diag): the applied record outlived the rules it described MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1 of the review loop, from two independent reviewers (GitHub Copilot on the PR and a read-only local agent) that agreed on the headline finding. **`panic` and `unblock` never cleared the record.** Both tear rules down through a raw `firewall.New()` backend, and only the runner's decorator knew how to clear `applied-rules.json`. So after `sudo dezhban panic` the record survived, and `print-rules --applied` and the Diagnostics pane both went on reporting "guard applied at 14:02" over a network that command had just thrown wide open. `panic` is the worst place for this: it is deliberately independent of the running service, so the deferred Cleanup that normally clears the record never runs, and it is the moment an operator is asking precisely whether the rules are gone. Both paths now clear it, and clear it even when the teardown reported an error — the rules are then in an unknown state, and a record that confidently names the old posture is worse than none. **`block` recorded nothing.** The mirror of the same gap: rules installed by hand were absent from a diagnostic that claimed to show what dezhban had applied. It understates rather than overstates, but a record that is only truthful when the service happened to be enforcing is not one an operator can use. Also from the same round: - Windows reported "no dezhban rules are loaded" for a host that is fully cut. `Remove-NetFirewallRule -Group dezhban` takes away only the allow rules, so a profile whose `DefaultOutboundAction` is still `Block` is enforcing with no group present — and the readback returned before ever emitting the profile table. The defaults are now read first and unconditionally, and the CLI prints the text even when no group is loaded. The "no rules" answer is also found as its own line rather than by matching the whole output, since `-ErrorAction SilentlyContinue` leaves warnings on the success stream — incidental text made "no rules" read as "rules loaded" and displayed the noise as the kernel's ruleset. - pf dropped its anchor-reference verdict silently when the main-ruleset read failed, so "could not check" rendered identically to "checked, fine" — for the exact non-enforcing state that check exists to expose. It now says so, and gets its own timeout instead of the remainder of the anchor read's. - `--installed --json` printed a stderr note on a corrupt record. The app captures stdout and stderr together, so that prepended prose to the document and turned a good privileged readback into an error in the pane. Human output only now. - A corrupt record made `--applied` exit 1, contradicting internal/applied's own "discarded, never fatal" contract. It is now reported on stderr and treated as absence, which is what the contract says and what the test name already claimed. - `--config` was accepted and discarded on `--applied`/`--installed` — the same shape 48340c2 refused for `--mode`, one flag over. - The kernel readback is a snapshot nothing refreshes, shown under "In the kernel now". Read during GUARD, it kept describing the firewall after FULL BLOCK engaged. It is now titled with the time it was read, cleared when the pane refreshes or closes, and its caption says it is a snapshot. Timestamps also carry their date: a record from three days ago rendered as a bare "14:02:11" reads as today. - Two on-host checks asserted things a correct build fails: that `panic` cleared the record (it did not, until now), and that a collapsed Diagnostics pane spawns no `print-rules` — it always fetches the cheap `--applied --json` row. Both reworded, and the teardown check now names all three routes separately. - CLAUDE.md: 48340c2 put `print-rules --installed` in the privileged set, but that set auto-re-execs under sudo via requireRoot and this deliberately does not — nor does it need elevation on Windows. Now described as what it is, outside the set. Tests: teardown-clears and block-records (both fail on the unfixed code), the corrupt-record exit status, the `--config` refusal, the Windows marker scan against captured output, and the nanosecond timestamp form `time.Now()` actually emits — the fixtures only covered six digits. The record path is now injectable so these do not read the developer's own live ruleset off /var/db/dezhban. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 17 ++- cmd/dezhban/main.go | 106 ++++++++++++++++-- cmd/dezhban/print_rules_flags_test.go | 84 +++++++++++++- docs/contribute/testing.md | 29 ++++- gui/macos/Sources/DezhbanMenu/AppState.swift | 18 +++ .../Sources/DezhbanMenu/DiagnosticsView.swift | 24 +++- .../DezhbanCoreTests/RulesetsTests.swift | 16 +++ internal/firewall/pf_darwin.go | 24 ++-- internal/firewall/wfp_windows.go | 34 +++++- internal/firewall/wfp_windows_test.go | 25 +++++ 10 files changed, 336 insertions(+), 41 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7016571..22dbffc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,22 +76,27 @@ The **privileged set** — requires root/admin — is exactly: `run`, `block`, `pause`, `resume`, `vpn add`/`remove`/`promote`/`forget`/`import` (but not `vpn list`/`show`), `setup` (but not `setup --questions`, which asks nothing and only reports what the wizard would ask), `config set`/`edit`/`preset apply`, `token enroll`/`forget` (but not -`token status`), `upgrade download`/`upgrade apply` (macOS only — `download`'s staging directory is root-owned so a local user -can't swap the verified `.pkg` before `apply` installs it), and `print-rules --installed` -(but not the rest of `print-rules`, which stays pure rendering — `--installed` reads the -kernel back through `FirewallBackend.InstalledRules`, which needs root; it is still a -READ and installs nothing). `switch`, `pause`, +`token status`), and `upgrade download`/`upgrade apply` (macOS only — `download`'s staging directory is root-owned so a local user +can't swap the verified `.pkg` before `apply` installs it). `switch`, `pause`, and `resume` are usually passwordless in practice: they ask the running daemon over its control socket first (gated by `control.allowSwitchOps`/ `control.allowPauseOps` respectively) and only fall back to the root-owned command file when no daemon answers. Everything else — `status`, `detect-vpn`, -`validate`, `print-rules` (except `--installed`, above), `doctor`, `monitor`, `vpn list`/`show`, +`validate`, `print-rules`, `doctor`, `monitor`, `vpn list`/`show`, `config show`/`path`/`schema`/`preset list`/`preset show`/`preset diff`, `token status`, `completion`, `upgrade check`, `version`, `help` — is read-only: no root, no firewall effects. Full reference: [docs/usage/cli.md](docs/usage/cli.md); the upgrade design in full: [docs/usage/upgrade.md](docs/usage/upgrade.md). +`print-rules --installed` is the one read-only command that still needs root on +unix, because it asks the kernel for dezhban's own rules +(`FirewallBackend.InstalledRules`) rather than rendering them. It is deliberately +NOT in the privileged set: that set auto-re-execs under sudo via `requireRoot`, +and silently elevating a diagnostic read is not something a read should do — it +prints the `sudo` hint and exits instead. It installs and changes nothing, and on +Windows it needs no elevation at all. + ## Rules that must not be broken The design depends on these invariants (rationale in diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index d022181..22593b9 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -1047,10 +1047,12 @@ func cmdBlock(args []string) int { switch { case *force: - if err := fw.Apply(forceBlockPolicy()); err != nil { + forced := forceBlockPolicy() + if err := fw.Apply(forced); err != nil { log.Error("forced block failed", "err", err) return 1 } + recordAppliedBestEffort(forced) log.Info("network force-blocked: all egress cut except loopback — no geo-provider pass, no automatic recovery; restore with `dezhban unblock` or `dezhban panic`") default: // `--guard` installs the always-on interface guard (tunnel stays open, @@ -1065,6 +1067,7 @@ func cmdBlock(args []string) int { log.Error("block failed", "err", err) return 1 } + recordAppliedBestEffort(d.Policy) if *guard { log.Info("vpn guard active", "tunnels", d.Tunnels, "endpoints", len(d.Endpoints)) } else { @@ -1209,6 +1212,59 @@ func resolveProviderAddrs(cfg *config.Config, log *slog.Logger) []netip.Addr { return hosts } +// appliedPath resolves the applied-rules record. Indirected through a variable +// so a test can point it at a temp directory: stateDir() is a hardcoded absolute +// path, so without this a unit test reads — and prints — the developer's own +// live ruleset, and fails outright on a host whose record happens to be corrupt. +var appliedPath = func() string { return applied.Path(stateDir()) } + +// recordAppliedBestEffort notes a ruleset this command installed directly, +// keeping `print-rules --applied` and the Diagnostics pane true for the paths +// that never go near the run loop. +// +// The daemon records through internal/runner's decorator, which covers every +// Apply the loop makes. `block` bypasses the loop entirely — it is root, with no +// daemon or deliberately around one — so without this the record would say +// "nothing applied" while rules this command installed were enforcing. That +// understates rather than overstates, which is why it is the less urgent half of +// the pair below, but a diagnostic that is only right when the daemon did it is +// not one an operator can use. +// +// Best-effort by the same rule as everywhere else this record is touched: +// failing to write down what happened must never fail the thing that happened. +func recordAppliedBestEffort(p firewall.Policy) { + rules, err := firewall.RenderRules(p) + if err != nil { + fmt.Fprintln(os.Stderr, "warning — could not render the applied ruleset for diagnostics:", err) + return + } + rec := applied.Record{Mode: p.Mode.String(), At: time.Now(), Rules: rules, Backend: firewall.RulesetKind} + if err := applied.Save(appliedPath(), rec); err != nil { + fmt.Fprintln(os.Stderr, "warning — could not record the applied ruleset:", err) + } +} + +// clearAppliedRecordBestEffort drops the record after this command tore rules +// down, so nothing can read a ruleset that is no longer installed as the live +// posture. +// +// This is the dangerous direction, and it is why `panic` needs it most. `panic` +// is deliberately daemon-independent — the escape hatch for a crashed daemon +// that left a block in place — so the run loop's deferred Cleanup, which is what +// normally clears this, never runs. Leaving the record behind meant +// `print-rules --applied` and the pane both reporting "guard applied at 14:02" +// over a network this command had just thrown wide open, at the one moment an +// operator is asking whether the rules are really gone. +// +// Cleared even when the teardown reported an error, exactly as the runner's +// decorator does: the rules are then in an unknown state, and a record that +// confidently names the old posture is worse than no record at all. +func clearAppliedRecordBestEffort(what string) { + if err := applied.Remove(appliedPath()); err != nil { + fmt.Fprintf(os.Stderr, "%s: warning — could not clear the applied-ruleset record: %v\n", what, err) + } +} + func cmdUnblock(args []string) int { fs := flag.NewFlagSet("unblock", flag.ExitOnError) cfgPath := fs.String("config", "", "path to config file (JSON)") @@ -1237,8 +1293,12 @@ func cmdUnblock(args []string) int { fmt.Fprintln(os.Stderr, "firewall backend unavailable:", err) return 1 } - if err := fw.Unblock(); err != nil { - fmt.Fprintln(os.Stderr, "unblock failed:", err) + unblockErr := fw.Unblock() + // Before the error return: a failed Unblock leaves the rules in an unknown + // state, and the record must not go on naming the old posture. + clearAppliedRecordBestEffort("unblock") + if unblockErr != nil { + fmt.Fprintln(os.Stderr, "unblock failed:", unblockErr) return 1 } // This path runs as root with no daemon involved (or bypassing one via @@ -1288,8 +1348,13 @@ func cmdPanic(args []string) int { } // Cleanup is best-effort and idempotent: it restores any saved prior state // (e.g. pf) and removes dezhban's rules whether or not a daemon owns them. - if err := fw.Cleanup(); err != nil { - fmt.Fprintln(os.Stderr, "panic: teardown reported an error (rules may persist):", err) + cleanupErr := fw.Cleanup() + // Cleared whatever Cleanup reported, and before the error return: this + // command is the escape hatch for a daemon that is not running, so nothing + // else will ever clear it. + clearAppliedRecordBestEffort("panic") + if cleanupErr != nil { + fmt.Fprintln(os.Stderr, "panic: teardown reported an error (rules may persist):", cleanupErr) return 1 } fmt.Println("dezhban: panic teardown complete — all dezhban rules removed, connectivity restored") @@ -1829,6 +1894,11 @@ func cmdPrintRules(args []string) int { fmt.Fprintln(os.Stderr, "--applied is what dezhban recorded installing; --installed is what the kernel holds now.") return 2 } + if (*appliedOnly || *installed) && typed["config"] { + fmt.Fprintln(os.Stderr, "--config does not reach --applied/--installed: both read dezhban's own state directory,") + fmt.Fprintln(os.Stderr, "which is a fixed path, not something the config file moves. Drop --config.") + return 2 + } if (*appliedOnly || *installed) && typed["mode"] { fmt.Fprintln(os.Stderr, "--mode renders a posture that is not in force; --applied and --installed report the one that is.") fmt.Fprintln(os.Stderr, "Drop --mode to report the live ruleset, or drop --applied/--installed to render a hypothetical one.") @@ -1877,11 +1947,16 @@ func cmdPrintRules(args []string) int { // 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()) + path := appliedPath() rec, ok, err := applied.Load(path) if err != nil { - fmt.Fprintln(os.Stderr, "could not read the applied-ruleset record:", err) - return 1 + // internal/applied's contract is that a corrupt record is discarded and + // never fatal — it describes the past, and enforcement does not depend + // on it. Reporting it as a failure instead of as "nothing recorded" + // contradicted that, and made an unreadable file indistinguishable from + // a broken command. Said out loud on stderr, then treated as absence. + fmt.Fprintln(os.Stderr, "warning — the applied-ruleset record is unreadable, treating it as absent:", err) + ok = false } if asJSON { if !ok { @@ -1940,8 +2015,12 @@ type installedRules struct { // 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 { + rec, hasRecord, recErr := applied.Load(appliedPath()) + if recErr != nil && !asJSON { + // Human output only. The macOS app runs `--installed --json` through a + // privileged helper that captures stdout and stderr TOGETHER, so a note + // printed here would prepend prose to the document, fail the decode, and + // turn a successful privileged readback into an error in the pane. fmt.Fprintln(os.Stderr, "note: could not read the applied-ruleset record:", recErr) } backend, err := firewall.New() @@ -1986,6 +2065,13 @@ func printInstalledRules(asJSON bool) int { } if !loaded { fmt.Fprintln(os.Stderr, "no dezhban rules are loaded (standby, or nothing running).") + // Still print whatever the backend returned. On Windows the blocking + // lives in each profile's DefaultOutboundAction rather than in the rule + // group, so a host with no group can still be fully cut — and printing + // nothing here would describe that lockout as standby. + if strings.TrimSpace(text) != "" { + fmt.Print(text) + } return 0 } fmt.Fprintf(os.Stderr, "# %s rules currently loaded, read from the kernel\n", out.Backend) diff --git a/cmd/dezhban/print_rules_flags_test.go b/cmd/dezhban/print_rules_flags_test.go index 69a3e2d..1f86f42 100644 --- a/cmd/dezhban/print_rules_flags_test.go +++ b/cmd/dezhban/print_rules_flags_test.go @@ -1,6 +1,13 @@ package main -import "testing" +import ( + "os" + "path/filepath" + "testing" + + "github.com/behnam-rk/dezhban/internal/applied" + "github.com/behnam-rk/dezhban/internal/firewall" +) // print-rules carries two kinds of flag: ones describing a ruleset to RENDER // (--mode) and ones selecting a live ruleset to REPORT (--applied, --installed). @@ -17,6 +24,7 @@ func TestPrintRulesRefusesFlagsItCannotHonour(t *testing.T) { {"mode with applied", []string{"--applied", "--mode", "fullblock"}, 2}, {"mode with installed", []string{"--installed", "--mode", "guard"}, 2}, {"json with no source", []string{"--json"}, 2}, + {"config with applied", []string{"--applied", "--config", "/tmp/x.json"}, 2}, } { t.Run(tc.name, func(t *testing.T) { if got := cmdPrintRules(tc.args); got != tc.want { @@ -30,9 +38,79 @@ func TestPrintRulesRefusesFlagsItCannotHonour(t *testing.T) { // --mode defaults to "guard", so testing its value instead of whether it was // given would reject every plain --applied run. func TestPrintRulesAppliedIsFineWithoutMode(t *testing.T) { - // No record exists under the test's state dir, which is the documented - // "nothing recorded" answer: exit 0, not a refusal and not a failure. + withTempAppliedRecord(t) if got := cmdPrintRules([]string{"--applied"}); got != 0 { t.Errorf("cmdPrintRules(--applied) = %d, want 0", got) } } + +// withTempAppliedRecord points the record at a temp dir for the duration of one +// test. stateDir() is a hardcoded absolute path, so without this these tests +// read the developer's own live ruleset off the host. +func withTempAppliedRecord(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), applied.FileName) + prev := appliedPath + appliedPath = func() string { return path } + t.Cleanup(func() { appliedPath = prev }) + return path +} + +// The record must not outlive the rules it describes. `panic` and `unblock` tear +// the firewall down without any daemon involved, so nothing else will ever clear +// it — and a surviving record is read as the live posture, which is a pane +// saying "guard is enforcing" over a network that command just threw open. +func TestTearingDownClearsTheAppliedRecord(t *testing.T) { + path := withTempAppliedRecord(t) + + recordAppliedBestEffort(firewall.Policy{Mode: firewall.ModeFullBlock}) + if _, ok, err := applied.Load(path); err != nil || !ok { + t.Fatalf("record not written: ok=%v err=%v", ok, err) + } + + clearAppliedRecordBestEffort("panic") + _, ok, err := applied.Load(path) + if err != nil { + t.Fatalf("load after clear: %v", err) + } + if ok { + t.Error("the record survived teardown; --applied would report a posture that is gone") + } + // Teardown runs on failure paths too, so clearing has to be idempotent. + clearAppliedRecordBestEffort("panic") +} + +// What `block` installs directly is recorded too, or the diagnostic is only +// truthful when the daemon happened to be the one enforcing. +func TestABlockAppliedByHandIsRecorded(t *testing.T) { + path := withTempAppliedRecord(t) + + recordAppliedBestEffort(firewall.Policy{Mode: firewall.ModeFullBlock}) + rec, ok, err := applied.Load(path) + if err != nil || !ok { + t.Fatalf("record not written: ok=%v err=%v", ok, err) + } + if rec.Mode != firewall.ModeFullBlock.String() { + t.Errorf("mode = %q, want %q", rec.Mode, firewall.ModeFullBlock.String()) + } + if rec.Rules == "" { + t.Error("no ruleset text recorded") + } + if rec.Backend != firewall.RulesetKind { + t.Errorf("backend = %q, want %q", rec.Backend, firewall.RulesetKind) + } + _ = os.Remove(path) +} + +// internal/applied's contract is that a corrupt record is discarded, never +// fatal. --applied has to agree: an unreadable file is "nothing recorded", +// which exits 0, not a command failure. +func TestACorruptRecordReadsAsNothingRecorded(t *testing.T) { + path := withTempAppliedRecord(t) + if err := os.WriteFile(path, []byte("{ this is not json"), 0o644); err != nil { + t.Fatal(err) + } + if got := cmdPrintRules([]string{"--applied"}); got != 0 { + t.Errorf("cmdPrintRules(--applied) on a corrupt record = %d, want 0", got) + } +} diff --git a/docs/contribute/testing.md b/docs/contribute/testing.md index 6ff6df6..9bdae1d 100644 --- a/docs/contribute/testing.md +++ b/docs/contribute/testing.md @@ -1371,9 +1371,18 @@ end up typing a password. - [ ] **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. +- [ ] **Teardown clears it — by every route.** Check all three separately: + `sudo dezhban stop` (the daemon's own Cleanup), `sudo dezhban panic`, and + `sudo dezhban unblock --force`. After each, re-open Diagnostics and run + `dezhban print-rules --applied`: both must read "no ruleset recorded yet". + `panic` is the one that matters most and the one that was broken — it is + deliberately daemon-independent, so nothing else will ever clear the + record. A stale ruleset shown as live over an open network is the failure + this must never have. +- [ ] **A block applied by hand is recorded.** `sudo dezhban block --guard` + with no daemon running, then `dezhban print-rules --applied`: it shows that + ruleset, not "nothing recorded". The record must not be truthful only when + the daemon happened to be the one enforcing. - [ ] **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 @@ -1387,9 +1396,17 @@ end up typing a password. - [ ] **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). +- [ ] **Only what is opened is rendered.** Visiting Diagnostics with every + disclosure collapsed must spawn no `print-rules --mode` subprocess (watch + with `sudo fs_usage -w -f exec | grep dezhban`, or Activity Monitor). One + `print-rules --applied --json` is expected on every visit — that is the + cheap unprivileged record the "Applied by dezhban" row is made of, and it + is fetched whether or not anything is expanded. +- [ ] **The kernel readback does not outlive its posture.** Read from the + kernel with the guard up, then force FULL BLOCK (`--simulate-country IR`). + The kernel row must not still be presenting the guard ruleset: it is + titled with the time it was read, and pressing **Run diagnostics** clears + it rather than leaving a stale snapshot beside fresh rows. ### Help pane diff --git a/gui/macos/Sources/DezhbanMenu/AppState.swift b/gui/macos/Sources/DezhbanMenu/AppState.swift index 76e7892..5a0879d 100644 --- a/gui/macos/Sources/DezhbanMenu/AppState.swift +++ b/gui/macos/Sources/DezhbanMenu/AppState.swift @@ -171,8 +171,24 @@ final class AppState: ObservableObject { /// asked for. @Published var appliedRules: AppliedRuleset? @Published var installedRules: InstalledRuleset? + /// When the kernel readback above was captured. A readback is a SNAPSHOT, + /// not a subscription: nothing re-reads it when the posture changes, so + /// showing it under a bare "In the kernel now" would let a guard ruleset + /// read during GUARD go on describing the firewall after FULL BLOCK + /// engaged — the "the current rules" claim Rulesets.swift says no surface + /// may make. The pane labels it with this instead. + @Published var installedRulesAt: Date? @Published var installedRulesError: String? @Published var installedRulesRunning = false + + /// Drops a kernel readback, so a stale snapshot cannot outlive the posture + /// it described. Called when the pane re-runs its diagnostics and when it + /// goes away; re-reading costs a password, so it is never done automatically. + func clearInstalledRules() { + installedRules = nil + installedRulesAt = nil + installedRulesError = nil + } /// 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. @@ -394,8 +410,10 @@ final class AppState: ObservableObject { self.installedRulesRunning = false if let decoded { self.installedRules = decoded + self.installedRulesAt = Date() } else { self.installedRules = nil + self.installedRulesAt = nil self.installedRulesError = r.output.isEmpty ? "No output from `dezhban print-rules --installed`." : r.output diff --git a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift index b150960..babe450 100644 --- a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift +++ b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift @@ -17,6 +17,7 @@ struct DiagnosticsView: View { Divider() content } + .onDisappear { state.clearInstalledRules() } .onAppear { // The report and the inventory live on AppState (they feed the // sidebar badge and survive navigation); this pane only asks for a @@ -48,6 +49,11 @@ struct DiagnosticsView: View { state.runDoctor(discover: discover) state.refreshVPNInventoryIfStale(maxAge: 0) state.refreshAppliedRules() + // The kernel readback is a snapshot and re-reading it costs a password, + // so a refresh drops it rather than silently renewing it. Keeping it + // would leave the previous posture's rules under a heading that says + // "now", beside freshly-read rows. + state.clearInstalledRules() } @ViewBuilder @@ -178,10 +184,16 @@ struct DiagnosticsView: View { .font(.callout) .foregroundStyle(.secondary) } else { + // Titled by WHEN it was read, never "now": this is a + // snapshot nothing refreshes, and the posture can change + // underneath it. 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.", + title: state.installedRulesAt.map { "In the kernel, read at \(Self.stamp.string(from: $0))" } + ?? "In the kernel, as read", + caption: "Read back from the firewall, in \(i.backend) syntax. It is a snapshot from when " + + "you pressed the button, not a live view — read it again after the posture changes. " + + "It will not match the applied text byte for byte: the firewall renders its own " + + "normalised form.", rules: i.installed) } } @@ -227,9 +239,13 @@ struct DiagnosticsView: View { RulesetPreview(rawValue: mode)?.label ?? mode } + /// Times shown beside a ruleset carry their DATE as well. A record survives + /// restarts, so one applied three days ago rendered as a bare "14:02:11" + /// reads as today — precisely the wrong impression for the row that tells + /// someone what is enforcing. private static let stamp: DateFormatter = { let f = DateFormatter() - f.dateStyle = .none + f.dateStyle = .short f.timeStyle = .medium return f }() diff --git a/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift b/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift index 8a27d6d..1f99aaf 100644 --- a/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift +++ b/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift @@ -21,6 +21,22 @@ struct RulesetsTests { #expect(a.rules == "block drop out all\n") } + /// Nanosecond precision — NINE fractional digits, which is what `time.Now()` + /// actually produces on Linux and macOS, and therefore the form the record + /// almost always carries in the field. The six-digit case above is the one + /// that gets written by hand in a fixture; this is the one that ships. + @Test func decodesNanosecondTimestamps() throws { + let json = """ + {"version":1,"mode":"fullblock","at":"2026-08-21T14:02:11.123456789+02:00", + "rules":"block drop out all\\n","backend":"pf"} + """ + let a = try #require(AppliedRuleset.decode(Data(json.utf8))) + #expect(a.mode == "fullblock") + // A timestamp that silently decoded to the epoch would render as 1970 + // beside a live ruleset, so pin that it actually parsed. + #expect(a.at.timeIntervalSince1970 > 1_700_000_000) + } + /// 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 { diff --git a/internal/firewall/pf_darwin.go b/internal/firewall/pf_darwin.go index 899de8e..a2239ee 100644 --- a/internal/firewall/pf_darwin.go +++ b/internal/firewall/pf_darwin.go @@ -226,13 +226,23 @@ func (b *pfBackend) InstalledRules() (string, bool, error) { 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") - } + // Its own timeout, not the remainder of the one the anchor read just spent: + // sharing the budget meant a slow first call could leave nothing for this + // one, and the verdict below is the whole point of reading the main ruleset. + mctx, mcancel := context.WithTimeout(context.Background(), pfctlTimeout) + defer mcancel() + switch main, err := pfctlCtx(mctx, "", "-s", "rules"); { + case err != nil: + // Never silently. A loaded anchor that pf does not descend into is + // exactly the non-enforcing state this readback exists to expose, so + // "could not check" must not render identically to "checked, fine". + b0.WriteString("# WARNING: could not read the main ruleset, so whether pf descends into\n") + b0.WriteString("# the dezhban anchor is UNKNOWN — these rules may be loaded but inert.\n") + case mainRulesetReferencesAnchor(main): + b0.WriteString("# main ruleset references the dezhban anchor\n") + default: + 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 diff --git a/internal/firewall/wfp_windows.go b/internal/firewall/wfp_windows.go index e681e78..ffb1065 100644 --- a/internal/firewall/wfp_windows.go +++ b/internal/firewall/wfp_windows.go @@ -213,12 +213,26 @@ func (b *wfpBackend) IsBlocked() (bool, error) { // 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. +// noRulesMarker is emitted on its own line when dezhban's rule group is absent. +// A marker line rather than a bare whole-output word because -ErrorAction +// SilentlyContinue does not suppress warnings on the success stream — the same +// reason parseProfileQuery scans for its answer instead of comparing the whole +// string. Incidental leading text would otherwise make "no rules" read as +// "rules loaded", with the noise shown to the user as the kernel's ruleset. +const noRulesMarker = "# NO-DEZHBAN-RULES" + func (b *wfpBackend) InstalledRules() (string, bool, error) { + // The profile defaults come FIRST and unconditionally, before the group is + // even looked up. On Windows that default is where the blocking actually + // lives: Remove-NetFirewallRule -Group dezhban takes away only the allow + // rules, so a host whose group was removed by hand while + // DefaultOutboundAction is still Block is fully cut — and reporting that as + // "no dezhban rules are loaded" would describe a total lockout as standby. 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", + "$g = Get-NetFirewallRule -Group " + groupName + " -ErrorAction SilentlyContinue", + "if ($null -eq $g) { '" + noRulesMarker + "'; exit 0 }", "'# dezhban rules'", "$g | Select-Object DisplayName,Direction,Action,Enabled | Format-Table -AutoSize | Out-String", }, "\n") @@ -226,10 +240,20 @@ func (b *wfpBackend) InstalledRules() (string, bool, error) { if err != nil { return "", false, fmt.Errorf("read the dezhban firewall group: %w", err) } - if strings.TrimSpace(out) == "NONE" { - return "", false, nil + // Text is returned either way: with no group there is still a profile table + // worth reading, and it is the half that says whether egress is cut. + return out, !hasNoRulesMarker(out), nil +} + +// hasNoRulesMarker reports whether the script said dezhban's group is absent. +// Split out so it can be exercised against captured output on any platform. +func hasNoRulesMarker(out string) bool { + for _, line := range strings.Split(out, "\n") { + if strings.TrimSpace(line) == noRulesMarker { + return true + } } - return out, true, nil + return false } // queryBlockedAndDefaults combines the group-existence check and the diff --git a/internal/firewall/wfp_windows_test.go b/internal/firewall/wfp_windows_test.go index f2c2775..d72b27c 100644 --- a/internal/firewall/wfp_windows_test.go +++ b/internal/firewall/wfp_windows_test.go @@ -362,3 +362,28 @@ func TestRenderBlockScriptTunnelScopedProviders(t *testing.T) { t.Errorf("a tunnel group cannot be expressed in WFP — the provider pass must be omitted, not emitted unscoped:\n%s", grp) } } + +// -ErrorAction SilentlyContinue does not suppress warnings on the success +// stream, so the "no rules" answer has to be found as its own line rather than +// by comparing the whole output. Incidental leading text would otherwise make +// "no dezhban rules" read as "rules loaded", and the noise itself would be +// shown to the user as what the kernel holds. +func TestNoRulesMarkerIsFoundAsALine(t *testing.T) { + for _, tc := range []struct { + name string + out string + want bool + }{ + {"marker alone", noRulesMarker, true}, + {"marker after a profile table", "# default outbound action per profile\nName Action\n---- ------\n" + noRulesMarker + "\n", true}, + {"marker behind a stray warning", "WARNING: something chatty\n" + noRulesMarker + "\n", true}, + {"rules present", "# dezhban rules\nDisplayName Direction\n----------- ---------\ndezhban-out Outbound\n", false}, + {"marker only as a substring", "# NO-DEZHBAN-RULES-BUT-ACTUALLY-SOME here\n", false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := hasNoRulesMarker(tc.out); got != tc.want { + t.Errorf("hasNoRulesMarker(%q) = %v, want %v", tc.out, got, tc.want) + } + }) + } +} From 28cb2a06a8acced031a0f3cb5935b43e78e6310d Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Sun, 6 Sep 2026 10:40:02 +0330 Subject: [PATCH 4/8] fix(diag): drift discarded the text that proves a Windows lockout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2. The headline finding is the loop's own: round 1 taught the no-rules branch to print the readback text, because on Windows the blocking lives in each profile's DefaultOutboundAction rather than in the rule group — and left the DRIFT branch, eight lines above, still throwing it away. That is the branch taken whenever a record exists, which is exactly when someone is asking. A host whose group was removed while its profile default is still Block is fully cut, and was told "the kernel holds no dezhban rules" with the profile table that proves egress is cut discarded. Both branches print it now. From the branch itself: - pf checked two of the three things IsBlocked checks. An anchor that is loaded and referenced while pf is switched off entirely (`pfctl -d`) filters nothing, and rendered as a clean readback with no warning. The status probe is now there, in the same shape as the anchor-reference verdict, and says so. - The whole Firewall-rules section sat inside the pane's `doctorReport != nil || vpnInventory != nil` gate, so the applied record and the "Read from the kernel…" button were invisible on a host where `doctor --json` cannot run — the state someone is most likely diagnosing — and on every first open until the async doctor returned. None of the three rows needs doctor. This is the same bug the comment right above that gate describes for the VPN inventory, one section over. - The applied row put the posture in its title and the time only in its caption, so a pane held open across GUARD → FULL BLOCK kept reading "Applied by dezhban — Guard". The time is now in the title, where the claim is made. That read is unprivileged and cheap, which is why it gets a timestamp rather than the clearing the kernel row got. - `--config` was refused but not documented; the help line named `--applied` but not `--installed`; and all three completion scripts offered neither. The wiring, not just the helpers, is now tested. Both fixes this loop made could be deleted with the whole suite still green: TestEveryDirectFirewallPathKeepsTheRecordHonest walks main.go's AST and fails when cmdBlock stops recording or cmdPanic/cmdUnblock stop clearing (an AST guard because all three need root and a real firewall — same technique, and same reason, as TestNoTestInPackageMainIsParallel), and TestRunWiresTheRecordingBackend drives Run end to end and fails with "recorded nothing — the backend was never wrapped" when the decorator is unwired. Both were confirmed against the unfixed code. Co-Authored-By: Claude Opus 5 --- cmd/dezhban/applied_wiring_test.go | 62 ++++++++++++++++++ cmd/dezhban/completion.go | 6 +- cmd/dezhban/main.go | 12 +++- docs/usage/cli.md | 12 ++-- .../Sources/DezhbanMenu/DiagnosticsView.swift | 18 +++++- internal/firewall/pf_darwin.go | 11 ++++ internal/runner/recording_test.go | 63 +++++++++++++++++++ 7 files changed, 174 insertions(+), 10 deletions(-) create mode 100644 cmd/dezhban/applied_wiring_test.go diff --git a/cmd/dezhban/applied_wiring_test.go b/cmd/dezhban/applied_wiring_test.go new file mode 100644 index 0000000..4a673ec --- /dev/null +++ b/cmd/dezhban/applied_wiring_test.go @@ -0,0 +1,62 @@ +package main + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" +) + +// The three commands that change the firewall WITHOUT the run loop must each +// keep the applied record honest. internal/runner's decorator covers every +// Apply the loop makes; these bypass it — `panic` most importantly, since it is +// deliberately independent of the running service, so nothing else will ever +// clear the record it leaves behind. +// +// An AST guard rather than a behavioural test because all three demand root and +// a real firewall: the helpers themselves are covered in +// print_rules_flags_test.go, but deleting a CALL to one left the whole suite +// green, which made the fix that added them unprotected. Same technique, and +// same reason, as TestNoTestInPackageMainIsParallel. +func TestEveryDirectFirewallPathKeepsTheRecordHonest(t *testing.T) { + want := map[string]string{ + "cmdBlock": "recordAppliedBestEffort", + "cmdUnblock": "clearAppliedRecordBestEffort", + "cmdPanic": "clearAppliedRecordBestEffort", + } + + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "main.go", nil, 0) + if err != nil { + t.Fatalf("parse main.go: %v", err) + } + + found := map[string]bool{} + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv != nil { + continue + } + callee, watched := want[fn.Name.Name] + if !watched { + continue + } + found[fn.Name.Name] = true + calls := false + ast.Inspect(fn, func(n ast.Node) bool { + if id, ok := n.(*ast.Ident); ok && id.Name == callee { + calls = true + } + return !calls + }) + if !calls { + t.Errorf("%s does not call %s — it changes the firewall directly, so the "+ + "applied record would describe a posture that is not in force", fn.Name.Name, callee) + } + } + for name := range want { + if !found[name] { + t.Errorf("%s not found in main.go — this guard would pass vacuously", name) + } + } +} diff --git a/cmd/dezhban/completion.go b/cmd/dezhban/completion.go index d5aa4cb..4dc9d1a 100644 --- a/cmd/dezhban/completion.go +++ b/cmd/dezhban/completion.go @@ -61,7 +61,7 @@ _dezhban() { return fi case "$cur" in - -*) COMPREPLY=( $(compgen -W "--config --mode --force --guard --dry-run --once --json --discover --simulate-country --verbose -v --no-sudo --no-daemon" -- "$cur") ) ;; + -*) COMPREPLY=( $(compgen -W "--config --mode --applied --installed --force --guard --dry-run --once --json --discover --simulate-country --verbose -v --no-sudo --no-daemon" -- "$cur") ) ;; esac } complete -F _dezhban dezhban @@ -83,7 +83,7 @@ _dezhban() { config) compadd -- path show get set reset edit; return ;; token) compadd -- status enroll forget; return ;; esac - compadd -- --config --mode --force --guard --dry-run --once --json --discover --simulate-country --verbose --no-sudo --no-daemon + compadd -- --config --mode --applied --installed --force --guard --dry-run --once --json --discover --simulate-country --verbose --no-sudo --no-daemon } compdef _dezhban dezhban ` @@ -95,6 +95,8 @@ complete -c dezhban -n '__fish_use_subcommand' -a '` + completionCommands + `' # flag values complete -c dezhban -l mode -x -a 'guard fullblock switch' complete -c dezhban -l config -r +complete -c dezhban -l applied +complete -c dezhban -l installed complete -c dezhban -n '__fish_seen_subcommand_from completion' -a 'bash zsh fish' complete -c dezhban -n '__fish_seen_subcommand_from config' -a 'path show get set reset edit' complete -c dezhban -n '__fish_seen_subcommand_from token' -a 'status enroll forget' diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index 22593b9..0de8e2d 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -68,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 (--applied: what is applied now) + print-rules Print the ruleset a block/guard would apply (--applied/--installed: what is live) 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 @@ -2061,6 +2061,16 @@ func printInstalledRules(asJSON bool) int { 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.") + // Print the text here too, for the same reason the no-rules branch below + // does: on Windows the blocking lives in each profile's + // DefaultOutboundAction rather than in the rule group, so a host whose + // group was removed while the default is still Block is FULLY CUT — and + // this is the branch taken whenever a record exists, which is exactly + // when someone is asking. Dropping the profile table here described that + // lockout as "nothing is loaded". + if strings.TrimSpace(text) != "" { + fmt.Print(text) + } return 0 } if !loaded { diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 244fec1..6d082f3 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -265,12 +265,14 @@ 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). -The three selectors are mutually exclusive and saying so is an error rather than -a silent preference: `--applied` with `--installed` is refused (they are two +The selectors are mutually exclusive and saying so is an error rather than a +silent preference: `--applied` with `--installed` is refused (they are two sources, not two views of one), so is `--mode` with either (it renders a posture -that is *not* in force, which is the opposite question), and so is `--json` -without one of them (a rendered ruleset is firewall syntax, not JSON). Each exits -2 and says which flag to drop. `doctor +that is *not* in force, which is the opposite question), so is `--json` without +one of them (a rendered ruleset is firewall syntax, not JSON), and so is +`--config` with either — both read dezhban's own state directory, which is a +fixed path the config file does not move. Each exits 2 and says which flag to +drop. `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/DezhbanMenu/DiagnosticsView.swift b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift index babe450..67e8e7e 100644 --- a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift +++ b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift @@ -65,7 +65,16 @@ struct DiagnosticsView: View { // async doctor run returned; after a doctor failure with nothing // retained; and permanently on a host where `doctor --json` cannot run // at all. refreshVPNInventoryIfStale fetched it and nothing showed it. - if state.doctorReport != nil || state.vpnInventory != nil { + // + // The firewall-rules section needs NEITHER of them: the applied record is + // its own unprivileged read, the kernel button is a button, and the + // previews render from config. Leaving it inside the gate hid it exactly + // where it is most wanted — on a host where `doctor --json` cannot run, + // which is the state someone is most likely diagnosing — and on every + // first open until the async doctor returned. Same bug the paragraph + // above describes for the inventory, one section over, so the gate now + // opens for anything the List can show. + if state.doctorReport != nil || state.vpnInventory != nil || state.cliFound { List { if let error = state.doctorError { Section { @@ -137,8 +146,13 @@ struct DiagnosticsView: View { @ViewBuilder private var appliedRow: some View { if let a = state.appliedRules { + // The posture is in the title and the time is only in the caption, + // so a pane held open across GUARD → FULL BLOCK kept reading + // "Applied by dezhban — Guard". This read is unprivileged and cheap, + // unlike the kernel one, so the honest fix is to say WHEN, in the + // title, where the claim is made. rulesDisclosure( - title: "Applied by dezhban — \(postureLabel(a.mode))", + title: "Applied by dezhban — \(postureLabel(a.mode)), at \(Self.stamp.string(from: a.at))", 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) diff --git a/internal/firewall/pf_darwin.go b/internal/firewall/pf_darwin.go index a2239ee..82427fb 100644 --- a/internal/firewall/pf_darwin.go +++ b/internal/firewall/pf_darwin.go @@ -226,6 +226,17 @@ func (b *pfBackend) InstalledRules() (string, bool, error) { return "", false, nil } var b0 strings.Builder + // pf being switched off entirely (`pfctl -d`) is the third way a loaded + // anchor enforces nothing, alongside an empty anchor and a main ruleset that + // does not reference it. IsBlocked checks all three; a readback that checked + // only two would render a disabled firewall as a healthy one. + if info, err := pfctlCtx(ctx, "", "-s", "info"); err != nil { + b0.WriteString("# WARNING: could not read pf's status, so whether pf is enabled at all\n") + b0.WriteString("# is UNKNOWN — these rules may be loaded but inert.\n") + } else if !strings.Contains(info, "Status: Enabled") { + b0.WriteString("# WARNING: pf is DISABLED — these rules are loaded but nothing is\n") + b0.WriteString("# being filtered. Re-enable with `sudo pfctl -e`.\n") + } // Its own timeout, not the remainder of the one the anchor read just spent: // sharing the budget meant a slow first call could leave nothing for this // one, and the verdict below is the whole point of reading the main ruleset. diff --git a/internal/runner/recording_test.go b/internal/runner/recording_test.go index a4db03a..64aeec1 100644 --- a/internal/runner/recording_test.go +++ b/internal/runner/recording_test.go @@ -1,13 +1,17 @@ package runner import ( + "context" "errors" "net/netip" + "path/filepath" "testing" "time" "github.com/behnam-rk/dezhban/internal/applied" "github.com/behnam-rk/dezhban/internal/firewall" + "github.com/behnam-rk/dezhban/internal/netdetect" + "github.com/behnam-rk/dezhban/internal/state" ) func recordingAt(t *testing.T, at time.Time) (Backend, *fakeBackend, string) { @@ -129,3 +133,62 @@ func TestANilLoggerDoesNotPanic(t *testing.T) { t.Fatalf("Apply: %v", err) } } + +// The decorator's own behaviour is covered above, but Run WIRING it is what +// makes any of that reach a real daemon. Deleting the newRecordingBackend call +// from Run left every test in this file green while nothing was ever recorded, +// so this drives the loop end to end: a Run given an AppliedRulesPath must leave +// a record behind after it applies. +func TestRunWiresTheRecordingBackend(t *testing.T) { + path := filepath.Join(t.TempDir(), applied.FileName) + + be := &fakeBackend{} + mon := &countingMonitor{cc: "US"} // allowed exit → healthy GUARD + tun := &scriptedWatcher{} + o := recoveryOpts(be, mon, tun.watcher()) + o.AppliedRulesPath = path + + snaps := make(chan state.Snapshot, 64) + o.Publish = func(s state.Snapshot) { + select { + case snaps <- s: + default: + } + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan error, 1) + go func() { done <- Run(ctx, o) }() + + tun.send(t, netdetect.TunnelState{Up: true, Names: []string{"utun4"}, Detail: "connected"}) + if !waitFor(t, snaps, func(s state.Snapshot) bool { return s.Posture == "guard" }) { + t.Fatal("never reached healthy GUARD, so nothing was applied to record") + } + + // The apply and the record both happen on the run loop, in that order, so + // a snapshot showing the posture means the write has been attempted. + var rec applied.Record + for i := 0; i < 200; i++ { + r, ok, err := applied.Load(path) + if err == nil && ok { + rec = r + break + } + time.Sleep(10 * time.Millisecond) + } + if rec.Rules == "" { + t.Fatal("Run applied a guard ruleset but recorded nothing — the backend was never wrapped") + } + if rec.Mode != firewall.ModeGuard.String() { + t.Errorf("recorded mode = %q, want %q", rec.Mode, firewall.ModeGuard.String()) + } + + cancel() + <-done + // Run's deferred Cleanup tears the rules down, so the record must not + // outlive it — a stale record reads as the live posture. + if _, ok, err := applied.Load(path); err == nil && ok { + t.Error("the record survived Run's shutdown Cleanup") + } +} From 11d42c3182e8a0962af22b694dbf3a8debd19331 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Sun, 6 Sep 2026 13:39:47 +0330 Subject: [PATCH 5/8] fix(diag): a failed record left the previous posture advertised MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3, from the hosted reviewer's second pass. Two of the four findings are the loop's own work. **A failed save left a stale record, in both writers.** `atomicfile.Write` leaves the old file in place when the replacement fails, so a successful Apply whose record could not be written left the PREVIOUS posture on disk being read as current — the exact failure the decorator exists to prevent, arriving by the one path that looked like it merely lost information. "Nothing recorded" is an ordinary answer; a confidently wrong posture is not. Both `internal/runner/recording.go` and the CLI helper this loop added now drop the old record when the write fails, the same shape as `writeAppliedAction`'s failure path in wfp_windows.go, which already carried this reasoning. **An in-flight privileged read could repopulate a cleared snapshot.** The kernel readback sits behind a password prompt, so pressing Run diagnostics or leaving the pane while that prompt is open let the completion restore exactly the snapshot the clear had invalidated — under a heading naming when it was read. Each read now captures a generation that every clear bumps, and a completion whose generation no longer matches is discarded. **`{}` decoded as "no rules loaded, no drift".** Every field of an installed-rules readback was optional-with-default, so any JSON object at all became a confident standby message instead of taking the error path. False reassurance about whether a kill switch is enforcing is the one thing this surface must never produce. `loaded`, `drift` and `backend` are required now; `installed` stays defaulted, since it is legitimately empty when nothing is loaded. **"No dezhban rules are loaded" asserted more than it knew.** On Windows the blocking lives in each profile's DefaultOutboundAction, so an absent rule group does not mean traffic is flowing. Both the CLI and the pane now scope that claim to dezhban's own rules and show what the firewall actually reported alongside it, rather than describing a possible lockout as standby. Tests: TestAFailedSaveDropsTheStaleRecord (the save is injected, because every way a real write fails either also breaks the removal under test or cannot be provoked in a unit test — the struct already injects `now` for the same reason), and aMalformedReadbackDoesNotDecodeAsStandby. Both confirmed against the unfixed code. Co-Authored-By: Claude Opus 5 --- cmd/dezhban/main.go | 20 +++++++++++- gui/macos/Sources/DezhbanCore/Rulesets.swift | 20 ++++++++++-- gui/macos/Sources/DezhbanMenu/AppState.swift | 15 +++++++++ .../Sources/DezhbanMenu/DiagnosticsView.swift | 14 +++++++- .../DezhbanCoreTests/RulesetsTests.swift | 14 ++++++++ internal/runner/recording.go | 18 +++++++++-- internal/runner/recording_test.go | 32 +++++++++++++++++++ 7 files changed, 126 insertions(+), 7 deletions(-) diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index 0de8e2d..c6ed0d7 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -1241,6 +1241,13 @@ func recordAppliedBestEffort(p firewall.Policy) { rec := applied.Record{Mode: p.Mode.String(), At: time.Now(), Rules: rules, Backend: firewall.RulesetKind} if err := applied.Save(appliedPath(), rec); err != nil { fmt.Fprintln(os.Stderr, "warning — could not record the applied ruleset:", err) + // atomicfile.Write leaves the old file in place when the replacement + // fails, so the record would go on naming the posture BEFORE this one. + // Dropping it is the safe direction: "nothing recorded" is an ordinary + // answer, a confidently wrong posture is not. + if rmErr := applied.Remove(appliedPath()); rmErr != nil { + fmt.Fprintln(os.Stderr, "warning — could not clear the stale applied-ruleset record:", rmErr) + } } } @@ -2074,7 +2081,18 @@ func printInstalledRules(asJSON bool) int { return 0 } if !loaded { - fmt.Fprintln(os.Stderr, "no dezhban rules are loaded (standby, or nothing running).") + // "dezhban's rules" is the honest scope of this claim, not "the + // firewall". On Windows the blocking lives in each profile's + // DefaultOutboundAction, so the group being absent does NOT mean egress + // is open — and asserting standby over a readback that says otherwise + // would be the misreport this section exists to avoid. + if strings.TrimSpace(text) != "" { + fmt.Fprintln(os.Stderr, "dezhban has no rules of its own loaded. That is expected in standby,") + fmt.Fprintln(os.Stderr, "or with dezhban stopped — but read what the firewall reported below") + fmt.Fprintln(os.Stderr, "before concluding that your traffic is flowing freely.") + } else { + fmt.Fprintln(os.Stderr, "no dezhban rules are loaded (standby, or nothing running).") + } // Still print whatever the backend returned. On Windows the blocking // lives in each profile's DefaultOutboundAction rather than in the rule // group, so a host with no group can still be fully cut — and printing diff --git a/gui/macos/Sources/DezhbanCore/Rulesets.swift b/gui/macos/Sources/DezhbanCore/Rulesets.swift index 220adf7..45569f8 100644 --- a/gui/macos/Sources/DezhbanCore/Rulesets.swift +++ b/gui/macos/Sources/DezhbanCore/Rulesets.swift @@ -84,12 +84,26 @@ public struct InstalledRuleset: Hashable { let subData = try? JSONSerialization.data(withJSONObject: sub) { nested = AppliedRuleset.decode(subData) } + // The fields the CLI ALWAYS emits are required, not defaulted. Defaulting + // them made every JSON object decode — `{}` included — as a confident + // "no rules loaded, no drift", so a malformed or version-skewed response + // reached the pane as a benign standby message instead of taking the + // error path. False reassurance about whether a kill switch is enforcing + // is the one thing this surface must never produce; failing to decode is + // recoverable, and the pane already says so. + // + // `installed` stays optional-with-default on purpose: it is legitimately + // absent-or-empty when nothing is loaded. + guard let loaded = obj["loaded"] as? Bool, + let drift = obj["drift"] as? Bool, + let backend = obj["backend"] as? String + else { return nil } return InstalledRuleset( installed: obj["installed"] as? String ?? "", - loaded: obj["loaded"] as? Bool ?? false, + loaded: loaded, applied: nested, - drift: obj["drift"] as? Bool ?? false, - backend: obj["backend"] as? String ?? "") + drift: drift, + backend: backend) } } diff --git a/gui/macos/Sources/DezhbanMenu/AppState.swift b/gui/macos/Sources/DezhbanMenu/AppState.swift index 5a0879d..e099d30 100644 --- a/gui/macos/Sources/DezhbanMenu/AppState.swift +++ b/gui/macos/Sources/DezhbanMenu/AppState.swift @@ -188,7 +188,17 @@ final class AppState: ObservableObject { installedRules = nil installedRulesAt = nil installedRulesError = nil + // Invalidate any read still in flight. The privileged call sits behind a + // password prompt, so it can easily outlive the clear that was meant to + // discard it — pressing Run diagnostics, or leaving the pane, while the + // prompt is open — and its completion would then repopulate exactly the + // snapshot clearing existed to throw away. + installedRulesGeneration &+= 1 } + + /// Bumped by every clear, captured by every read, compared on completion. + /// A read whose generation no longer matches is discarded rather than shown. + private var installedRulesGeneration = 0 /// 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. @@ -402,12 +412,17 @@ final class AppState: ObservableObject { guard !installedRulesRunning, cliFound else { return } installedRulesRunning = true installedRulesError = nil + let generation = installedRulesGeneration 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 + // Someone cleared while this was behind the password prompt. + // Publishing now would restore the very snapshot that clear + // invalidated, under a heading naming the time it was read. + guard generation == self.installedRulesGeneration else { return } if let decoded { self.installedRules = decoded self.installedRulesAt = Date() diff --git a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift index 67e8e7e..f5e9516 100644 --- a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift +++ b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift @@ -193,10 +193,22 @@ struct DiagnosticsView: View { .font(.callout) .foregroundStyle(.orange) } else if !i.loaded { - Label("No dezhban rules are loaded. That is expected in standby, or with dezhban stopped.", + // Scoped to dezhban's OWN rules. On Windows the blocking + // lives in each profile's default outbound action, so their + // absence does not mean egress is open — and the readback + // below says which. + Label("dezhban has no rules of its own loaded — expected in standby, or with dezhban stopped.", systemImage: "info.circle") .font(.callout) .foregroundStyle(.secondary) + if !i.installed.isEmpty { + rulesDisclosure( + title: state.installedRulesAt.map { "What the firewall reported, read at \(Self.stamp.string(from: $0))" } + ?? "What the firewall reported", + caption: "dezhban's own rules are absent, but this is what the firewall said when asked. " + + "On Windows the outbound default is where the blocking lives.", + rules: i.installed) + } } else { // Titled by WHEN it was read, never "now": this is a // snapshot nothing refreshes, and the posture can change diff --git a/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift b/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift index 1f99aaf..8db44a6 100644 --- a/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift +++ b/gui/macos/Tests/DezhbanCoreTests/RulesetsTests.swift @@ -90,4 +90,18 @@ struct RulesetsTests { #expect(!mode.detail.isEmpty) } } + + /// A malformed or version-skewed response must take the ERROR path, not + /// decode into a confident "nothing is loaded, no drift". False reassurance + /// about whether a kill switch is enforcing is the one thing this surface + /// must never produce. + @Test func aMalformedReadbackDoesNotDecodeAsStandby() { + #expect(InstalledRuleset.decode(Data("{}".utf8)) == nil) + // Missing `drift` alone is enough: it is the finding the pane renders. + let noDrift = #"{"installed":"","loaded":false,"backend":"pf"}"# + #expect(InstalledRuleset.decode(Data(noDrift.utf8)) == nil) + // A complete document still decodes. + let good = #"{"installed":"","loaded":false,"drift":false,"backend":"pf"}"# + #expect(InstalledRuleset.decode(Data(good.utf8)) != nil) + } } diff --git a/internal/runner/recording.go b/internal/runner/recording.go index 27350e9..5d3fa27 100644 --- a/internal/runner/recording.go +++ b/internal/runner/recording.go @@ -37,6 +37,11 @@ type recordingBackend struct { // now is injected so a test can assert the recorded timestamp instead of // asserting that some time passed. now func() time.Time + // save is injected for the same reason: the failure path below must drop a + // stale record, and the ways a real write fails (a full disk) are not ones + // a test can arrange without also breaking the removal that the fix depends + // on. Defaults to applied.Save. + save func(string, applied.Record) error } // newRecordingBackend wraps b when path is non-empty; otherwise it returns b @@ -52,7 +57,7 @@ func newRecordingBackend(b Backend, path string, log *slog.Logger) Backend { // 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} + return &recordingBackend{Backend: b, path: path, log: log, now: time.Now, save: applied.Save} } func (r *recordingBackend) Apply(p firewall.Policy) error { @@ -74,8 +79,17 @@ func (r *recordingBackend) Apply(p firewall.Policy) error { Rules: rules, Backend: firewall.RulesetKind, } - if err := applied.Save(r.path, rec); err != nil { + if err := r.save(r.path, rec); err != nil { r.log.Warn("could not record the applied ruleset", "err", err, "path", r.path) + // The PREVIOUS record is still on disk — atomicfile.Write leaves the + // old file intact when the replacement fails — and it now names a + // posture that is no longer the one installed. That is the stale record + // this whole decorator exists to prevent, arriving by the one path that + // looked like it only lost information. Absent is the designed + // fallback ("nothing recorded yet"); wrong is not. + // Same reasoning, and the same shape, as writeAppliedAction's failure + // path in internal/firewall/wfp_windows.go. + r.clear() } return nil } diff --git a/internal/runner/recording_test.go b/internal/runner/recording_test.go index 64aeec1..f776c68 100644 --- a/internal/runner/recording_test.go +++ b/internal/runner/recording_test.go @@ -192,3 +192,35 @@ func TestRunWiresTheRecordingBackend(t *testing.T) { t.Error("the record survived Run's shutdown Cleanup") } } + +// atomicfile.Write leaves the OLD file in place when the replacement fails, so +// a failed record after a SUCCESSFUL apply would leave the previous posture +// advertised as current — the stale record this decorator exists to prevent, +// arriving by the one path that looks like it merely loses information. +// "Nothing recorded" is an ordinary answer; a confidently wrong posture is not. +func TestAFailedSaveDropsTheStaleRecord(t *testing.T) { + path := filepath.Join(t.TempDir(), applied.FileName) + inner := &fakeBackend{} + be := newRecordingBackend(inner, path, discardLog()).(*recordingBackend) + + // A first, healthy apply leaves a guard record behind. + if err := be.Apply(firewall.Policy{Mode: firewall.ModeGuard, TunnelIfaces: []string{"utun4"}}); err != nil { + t.Fatalf("first apply: %v", err) + } + if _, ok, _ := applied.Load(path); !ok { + t.Fatal("first apply recorded nothing") + } + + // The next write fails while the apply itself still succeeds. Injected + // rather than arranged on disk: the ways a real save fails either also + // break the removal (a read-only directory) or cannot be provoked here (a + // full disk), and it is the removal that is under test. + be.save = func(string, applied.Record) error { return errors.New("no space left on device") } + + if err := be.Apply(firewall.Policy{Mode: firewall.ModeFullBlock}); err != nil { + t.Fatalf("apply must still succeed when only the record fails: %v", err) + } + if rec, ok, _ := applied.Load(path); ok { + t.Errorf("a %q record survived a failed save; it names the posture BEFORE the one applied", rec.Mode) + } +} From da4f3cf447ce8041039f8c3712b382f64d6362a2 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Sun, 6 Sep 2026 13:44:04 +0330 Subject: [PATCH 6/8] fix(diag): a loaded firewall that filters nothing read as healthy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3, second pass. One branch defect and three regressions this loop introduced itself. **Loaded is not enforcing, and only the ruleset text said so.** pf switched off with `pfctl -d`, an anchor the main ruleset no longer references, an nft chain whose policy drifted off drop — in every one of those the rules are all present and nothing is filtered. The backends already detected each case, but encoded it ONLY as a `# WARNING:` line inside the returned text: a JSON consumer saw `{"loaded":true, "drift":false}` and concluded healthy, and the pane rendered a collapsed disclosure with nothing visibly wrong, so an operator had to expand it and read pf syntax to discover the kill switch was not switching anything off. That is the state where every other signal reads healthy, which is exactly why it needed to be the loudest. `firewall.Warnings` makes those lines a contract rather than a formatting choice, `--json` gains `enforcing` and `warnings`, the CLI prints them above the ruleset, and the pane gets an orange row that needs no expanding. The Swift side defaults both fields, so an older CLI degrades to the previous behaviour rather than failing to decode. The three the loop caused: - pf's new status probe reused the context the anchor read had already spent from, while the main-ruleset read beside it allocated a fresh one with a comment explaining why sharing is wrong. Same hazard, opposite treatment, in one function — and a slow anchor read would have made a healthy host print "could not read pf's status … may be loaded but inert", the readback crying wolf about the exact condition it exists to report. Every probe now gets its own full budget, and the comment says why this differs from IsBlocked. - The AST guard under-bit where it claimed to bite: `cmdBlock` records from two branches, and deleting either one alone left it green — the deletion class its own doc comment named. It counts call sites now, and its failure message distinguishes "you moved this into a helper, update the guard" from "you removed it", so a refactor is not reported as a security regression. - Widening the pane's visibility gate to include `cliFound` made the outer `else` unreachable, silently deleting the "No results yet — run diagnostics" prompt, which is the pane's only call to action on a healthy host's first open. It lives inside the List now, beside the firewall rows. TestWarningsAreFoundAndFolded covers the two-line warnings the backends actually emit; the strengthened guard fails with "cmdBlock calls recordAppliedBestEffort 1 time(s), want 2" when one site is removed. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 ++- cmd/dezhban/applied_wiring_test.go | 58 +++++++++++-------- cmd/dezhban/main.go | 21 +++++++ docs/contribute/testing.md | 8 +++ docs/usage/cli.md | 9 ++- gui/macos/Sources/DezhbanCore/Rulesets.swift | 23 +++++++- .../Sources/DezhbanMenu/DiagnosticsView.swift | 34 ++++++++++- internal/firewall/pf_darwin.go | 15 +++-- internal/firewall/warnings.go | 46 +++++++++++++++ internal/firewall/warnings_test.go | 38 ++++++++++++ 10 files changed, 225 insertions(+), 34 deletions(-) create mode 100644 internal/firewall/warnings.go create mode 100644 internal/firewall/warnings_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 060693b..df17caf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,12 @@ current as you land changes. 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. + that and a second repairer would be a second writer. The readback also reports + **loaded is not enforcing**: pf switched off, an anchor the main ruleset no + longer references, or an nft chain whose policy drifted off `drop` all leave + dezhban's rules present and filtering nothing. That gets its own row, and its + own `enforcing` field in `--json`, because it is the state where every other + signal reads healthy. - **`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). diff --git a/cmd/dezhban/applied_wiring_test.go b/cmd/dezhban/applied_wiring_test.go index 4a673ec..9a3204c 100644 --- a/cmd/dezhban/applied_wiring_test.go +++ b/cmd/dezhban/applied_wiring_test.go @@ -19,10 +19,18 @@ import ( // green, which made the fix that added them unprotected. Same technique, and // same reason, as TestNoTestInPackageMainIsParallel. func TestEveryDirectFirewallPathKeepsTheRecordHonest(t *testing.T) { - want := map[string]string{ - "cmdBlock": "recordAppliedBestEffort", - "cmdUnblock": "clearAppliedRecordBestEffort", - "cmdPanic": "clearAppliedRecordBestEffort", + // The COUNT matters, not just presence: cmdBlock applies from two branches + // (--force and the default plan), and asserting "calls it at all" stayed + // green when either one alone lost its call — the exact deletion this guard + // claims to catch. + want := []struct { + fn string + callee string + calls int + }{ + {"cmdBlock", "recordAppliedBestEffort", 2}, + {"cmdUnblock", "clearAppliedRecordBestEffort", 1}, + {"cmdPanic", "clearAppliedRecordBestEffort", 1}, } fset := token.NewFileSet() @@ -31,32 +39,36 @@ func TestEveryDirectFirewallPathKeepsTheRecordHonest(t *testing.T) { t.Fatalf("parse main.go: %v", err) } - found := map[string]bool{} + fns := map[string]*ast.FuncDecl{} for _, decl := range f.Decls { - fn, ok := decl.(*ast.FuncDecl) - if !ok || fn.Recv != nil { - continue + if fn, ok := decl.(*ast.FuncDecl); ok && fn.Recv == nil { + fns[fn.Name.Name] = fn } - callee, watched := want[fn.Name.Name] - if !watched { + } + + for _, w := range want { + fn, ok := fns[w.fn] + if !ok { + t.Errorf("%s not found in main.go — this guard would pass vacuously", w.fn) continue } - found[fn.Name.Name] = true - calls := false + got := 0 ast.Inspect(fn, func(n ast.Node) bool { - if id, ok := n.(*ast.Ident); ok && id.Name == callee { - calls = true + call, ok := n.(*ast.CallExpr) + if !ok { + return true } - return !calls + if id, ok := call.Fun.(*ast.Ident); ok && id.Name == w.callee { + got++ + } + return true }) - if !calls { - t.Errorf("%s does not call %s — it changes the firewall directly, so the "+ - "applied record would describe a posture that is not in force", fn.Name.Name, callee) - } - } - for name := range want { - if !found[name] { - t.Errorf("%s not found in main.go — this guard would pass vacuously", name) + if got != w.calls { + t.Errorf("%s calls %s %d time(s), want %d.\n"+ + "If you MOVED the call into a helper this guard is simply out of date — update it.\n"+ + "If you REMOVED it, that path changes the firewall without keeping the applied\n"+ + "record honest, and a surface will report a posture that is not in force.", + w.fn, w.callee, got, w.calls) } } } diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index c6ed0d7..e1f1687 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -2011,6 +2011,16 @@ type installedRules struct { Drift bool `json:"drift"` // Backend names the syntax of Installed. Backend string `json:"backend"` + // Warnings are the reasons the loaded rules are not actually filtering — + // pf switched off, an anchor the main ruleset no longer references, an nft + // chain whose policy drifted off drop. Empty on a healthy host. + Warnings []string `json:"warnings,omitempty"` + // Enforcing is the question a reader actually has, and it is NOT `loaded`: + // a firewall can hold every rule dezhban installed and filter none of them. + // Carried as its own field because the backends already know the answer, + // and leaving it discoverable only inside the ruleset text meant a JSON + // consumer saw {"loaded":true,"drift":false} and concluded healthy. + Enforcing bool `json:"enforcing"` } // printInstalledRules reads dezhban's rules back out of the kernel — the other @@ -2044,11 +2054,14 @@ func printInstalledRules(asJSON bool) int { return 1 } + warnings := firewall.Warnings(text) out := installedRules{ Installed: text, Loaded: loaded, Backend: firewall.RulesetKind, Drift: hasRecord && !loaded, + Warnings: warnings, + Enforcing: loaded && len(warnings) == 0, } if hasRecord { out.Applied = &rec @@ -2102,6 +2115,14 @@ func printInstalledRules(asJSON bool) int { } return 0 } + // Loaded but inert is its own answer, and the loudest one here: the rules + // are all present, so every other signal reads healthy. + for _, w := range warnings { + fmt.Fprintln(os.Stderr, "WARNING:", w) + } + if len(warnings) > 0 { + fmt.Fprintln(os.Stderr, "dezhban's rules are loaded but are NOT filtering. See above.") + } 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", diff --git a/docs/contribute/testing.md b/docs/contribute/testing.md index 9bdae1d..d74d389 100644 --- a/docs/contribute/testing.md +++ b/docs/contribute/testing.md @@ -1388,6 +1388,14 @@ end up typing a password. 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. +- [ ] **Loaded but not filtering is reported as loudly as missing.** With the + guard up, disable pf itself (`sudo pfctl -d`) — the rules stay loaded — then + "Read from the kernel…": the pane must show an orange **"dezhban's rules are + loaded but are NOT filtering"** row without expanding anything, and + `sudo dezhban print-rules --installed --json` must report + `"enforcing": false` with a `warnings` entry. Re-enable with `sudo pfctl -e`. + This is the state where every other signal reads healthy, so a warning + buried inside the ruleset text would never be found. - [ ] **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 diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 6d082f3..d292070 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -261,8 +261,13 @@ 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 +machine output, whose `enforcing` field is the one to read: **loaded is not +enforcing**. A firewall can hold every rule dezhban installed and filter none of +them — pf switched off with `pfctl -d`, an anchor the main ruleset stopped +referencing, an nft chain whose policy drifted off `drop` — and in that state +every other signal looks healthy, because the rules really are all there. +`enforcing` is false and `warnings` says why. 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). The selectors are mutually exclusive and saying so is an error rather than a diff --git a/gui/macos/Sources/DezhbanCore/Rulesets.swift b/gui/macos/Sources/DezhbanCore/Rulesets.swift index 45569f8..4848505 100644 --- a/gui/macos/Sources/DezhbanCore/Rulesets.swift +++ b/gui/macos/Sources/DezhbanCore/Rulesets.swift @@ -63,14 +63,26 @@ public struct InstalledRuleset: Hashable { /// healthy host. The texts are shown side by side for a person to read. public let drift: Bool public let backend: String + /// Why the loaded rules are not filtering — pf switched off, an anchor the + /// main ruleset no longer references, an nft chain whose policy drifted off + /// drop. Empty on a healthy host. + public let warnings: [String] + /// The question a reader actually has, and it is NOT `loaded`: a firewall + /// can hold every rule dezhban installed and filter none of them. Left + /// inside the ruleset text, that state rendered as a collapsed disclosure + /// with nothing visibly wrong. + public let enforcing: Bool public init(installed: String, loaded: Bool, applied: AppliedRuleset?, - drift: Bool, backend: String) { + drift: Bool, backend: String, + warnings: [String] = [], enforcing: Bool = true) { self.installed = installed self.loaded = loaded self.applied = applied self.drift = drift self.backend = backend + self.warnings = warnings + self.enforcing = enforcing } /// Decoded by hand rather than through Codable so the nested `applied` @@ -98,12 +110,19 @@ public struct InstalledRuleset: Hashable { let drift = obj["drift"] as? Bool, let backend = obj["backend"] as? String else { return nil } + // Both default rather than being required: a CLI predating them emits + // neither, and an older CLI must degrade to the previous behaviour + // rather than fail to decode. `enforcing` defaults to `loaded` there, + // which is exactly what the pane assumed before this existed. + let warnings = obj["warnings"] as? [String] ?? [] return InstalledRuleset( installed: obj["installed"] as? String ?? "", loaded: loaded, applied: nested, drift: drift, - backend: backend) + backend: backend, + warnings: warnings, + enforcing: obj["enforcing"] as? Bool ?? loaded) } } diff --git a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift index f5e9516..d00e89a 100644 --- a/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift +++ b/gui/macos/Sources/DezhbanMenu/DiagnosticsView.swift @@ -93,6 +93,7 @@ struct DiagnosticsView: View { .textSelection(.enabled) } } + noReportYetRow vpnInventorySection firewallRulesSection if let report = state.doctorReport { @@ -108,8 +109,6 @@ struct DiagnosticsView: View { } } .listStyle(.inset) - } else if let error = state.doctorError { - guided(symbol: "exclamationmark.triangle", title: "Couldn't run diagnostics", message: error) } else if !state.cliFound { guided(symbol: "questionmark.circle", title: "dezhban CLI not found", message: "Install the dezhban command-line tool, then run diagnostics again.") @@ -118,6 +117,23 @@ struct DiagnosticsView: View { } } + /// The prompt that used to live in the outer `else`. Widening the section's + /// gate to include `cliFound` made that branch unreachable — the first + /// condition is true whenever `cliFound` is — which silently deleted the + /// pane's only call to action on a healthy host's first open. It belongs + /// inside the List now, beside the firewall rows that are there already. + @ViewBuilder + private var noReportYetRow: some View { + if state.doctorReport == nil && !state.doctorRunning { + Section { + Label("Run diagnostics to see tunnels, endpoints, and lockout risks.", + systemImage: "stethoscope") + .font(.callout) + .foregroundStyle(.secondary) + } + } + } + // MARK: - firewall rules /// What the guard is doing to your traffic, in three parts, because they @@ -210,6 +226,20 @@ struct DiagnosticsView: View { rules: i.installed) } } else { + // Loaded but NOT filtering is the loudest thing this pane + // can learn, and every other signal reads healthy when it + // happens — the rules are all present. It gets its own + // visible row rather than living inside a collapsed + // ruleset, where an operator would have to expand pf syntax + // and read it to find out nothing is being filtered. + if !i.enforcing { + Label("dezhban's rules are loaded but are NOT filtering.\n" + + i.warnings.joined(separator: "\n"), + systemImage: "exclamationmark.octagon.fill") + .font(.callout) + .foregroundStyle(.orange) + .textSelection(.enabled) + } // Titled by WHEN it was read, never "now": this is a // snapshot nothing refreshes, and the posture can change // underneath it. diff --git a/internal/firewall/pf_darwin.go b/internal/firewall/pf_darwin.go index 82427fb..9922328 100644 --- a/internal/firewall/pf_darwin.go +++ b/internal/firewall/pf_darwin.go @@ -230,16 +230,23 @@ func (b *pfBackend) InstalledRules() (string, bool, error) { // anchor enforces nothing, alongside an empty anchor and a main ruleset that // does not reference it. IsBlocked checks all three; a readback that checked // only two would render a disabled firewall as a healthy one. - if info, err := pfctlCtx(ctx, "", "-s", "info"); err != nil { + ictx, icancel := context.WithTimeout(context.Background(), pfctlTimeout) + defer icancel() + if info, err := pfctlCtx(ictx, "", "-s", "info"); err != nil { b0.WriteString("# WARNING: could not read pf's status, so whether pf is enabled at all\n") b0.WriteString("# is UNKNOWN — these rules may be loaded but inert.\n") } else if !strings.Contains(info, "Status: Enabled") { b0.WriteString("# WARNING: pf is DISABLED — these rules are loaded but nothing is\n") b0.WriteString("# being filtered. Re-enable with `sudo pfctl -e`.\n") } - // Its own timeout, not the remainder of the one the anchor read just spent: - // sharing the budget meant a slow first call could leave nothing for this - // one, and the verdict below is the whole point of reading the main ruleset. + // Its own timeout, like the status probe above and for the same reason: + // every probe here gets a full pfctlTimeout rather than the remainder of a + // shared budget. Sharing one meant a slow earlier call could leave nothing + // for a later probe, which then fails on deadline and prints a WARNING on a + // perfectly healthy host — the readback crying wolf about the exact + // condition it exists to report. IsBlocked shares one budget because it + // returns a single bool and a timeout there is simply an error; this + // returns text a person reads, so a false warning is worse than a slow read. mctx, mcancel := context.WithTimeout(context.Background(), pfctlTimeout) defer mcancel() switch main, err := pfctlCtx(mctx, "", "-s", "rules"); { diff --git a/internal/firewall/warnings.go b/internal/firewall/warnings.go new file mode 100644 index 0000000..95108d9 --- /dev/null +++ b/internal/firewall/warnings.go @@ -0,0 +1,46 @@ +package firewall + +import "strings" + +// WarningPrefix marks a line InstalledRules prepends to a readback when the +// rules are LOADED but not actually filtering — pf switched off, an anchor the +// main ruleset no longer references, an nft output chain whose policy drifted +// off drop. +// +// It is a contract, not a formatting choice. "Loaded" and "enforcing" are +// different questions, and the bool InstalledRules returns answers only the +// first: a firewall can hold every rule dezhban installed and filter nothing. +// Leaving that discoverable only by reading pf syntax inside a collapsed pane +// is how a non-enforcing kill switch reads as healthy, so the warning has to be +// something a caller can find without understanding the ruleset it is wrapped in. +const WarningPrefix = "# WARNING:" + +// Warnings returns the warning lines in a readback, in order, with the prefix +// and surrounding whitespace stripped. Empty when the readback carries none — +// which is the healthy case, and the only one in which "loaded" means +// "enforcing". +// +// Warnings are emitted as consecutive lines, so a continuation line (one that +// follows a warning and is itself a comment) is folded into the warning above +// it rather than dropped: the second half of "pf is DISABLED — these rules are +// loaded but nothing is / being filtered" is the half that says what it means. +func Warnings(readback string) []string { + var out []string + inWarning := false + for _, line := range strings.Split(readback, "\n") { + trimmed := strings.TrimSpace(line) + switch { + case strings.HasPrefix(trimmed, WarningPrefix): + out = append(out, strings.TrimSpace(strings.TrimPrefix(trimmed, WarningPrefix))) + inWarning = true + case inWarning && strings.HasPrefix(trimmed, "#"): + cont := strings.TrimSpace(strings.TrimPrefix(trimmed, "#")) + if cont != "" && len(out) > 0 { + out[len(out)-1] += " " + cont + } + default: + inWarning = false + } + } + return out +} diff --git a/internal/firewall/warnings_test.go b/internal/firewall/warnings_test.go new file mode 100644 index 0000000..e2222b2 --- /dev/null +++ b/internal/firewall/warnings_test.go @@ -0,0 +1,38 @@ +package firewall + +import ( + "reflect" + "testing" +) + +// "Loaded" and "enforcing" are different questions. A readback that carries a +// warning describes a firewall holding dezhban's rules and filtering nothing, +// and a caller has to be able to find that without parsing the ruleset. +func TestWarningsAreFoundAndFolded(t *testing.T) { + for _, tc := range []struct { + name string + in string + want []string + }{ + {"healthy readback carries none", "# main ruleset references the dezhban anchor\nblock drop out all\n", nil}, + {"empty", "", nil}, + { + name: "a two-line warning folds into one", + in: "# WARNING: pf is DISABLED — these rules are loaded but nothing is\n" + + "# being filtered. Re-enable with `sudo pfctl -e`.\n" + + "block drop out all\n", + want: []string{"pf is DISABLED — these rules are loaded but nothing is being filtered. Re-enable with `sudo pfctl -e`."}, + }, + { + name: "rules after a warning do not extend it", + in: "# WARNING: something\nblock drop out all\n# an ordinary comment\n", + want: []string{"something"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if got := Warnings(tc.in); !reflect.DeepEqual(got, tc.want) { + t.Errorf("Warnings() = %#v, want %#v", got, tc.want) + } + }) + } +} From ddb969dd4f1e22ba8291bfaa3622c486678da5e6 Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Sun, 6 Sep 2026 13:50:53 +0330 Subject: [PATCH 7/8] fix(diag): a warning absorbed the comment printed after it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmation round, and both findings are the loop's own from the previous commit. Neither changes what is enforced. **One warning is now exactly one line.** `Warnings()` folded a warning's continuation lines into it and could not tell a continuation from the ORDINARY comment that follows one. pf emits "# main ruleset references the dezhban anchor" directly after a disabled-pf warning, so the fold produced a single entry reading "…nothing is being filtered. Re-enable with `sudo pfctl -e`. main ruleset references the dezhban anchor" — a sentence that contradicts itself, shown in the pane's orange row and read as warnings[0] by any JSON consumer, in exactly the scenario the on-host check tells a tester to reproduce. The heuristic is gone rather than tuned: the backends emit each warning on one line and Warnings() just collects them. Long lines wrap; a rule that guesses which comments belong together does not. TestEveryEmittedWarningIsSelfContained pins the pf pair that broke it. **`enforcing` is only as good as the warnings its backend emits**, and WFP emits none. On Windows a host with dezhban's group present and a profile whose DefaultOutboundAction is no longer Block reports enforcing while filtering nothing. Deciding that properly means comparing the live defaults against appliedActionPath() the way IsBlocked does, and that is not code to write blind on a platform this repo does not yet test (see the matrix note in ci.yml). So it is named where it lives — a TODO in wfp_windows.go, a comment on the field, and a caveat in cli.md — rather than left as a promise the field does not keep there. Co-Authored-By: Claude Opus 5 --- cmd/dezhban/main.go | 5 ++++ docs/usage/cli.md | 5 +++- internal/firewall/nft_linux.go | 3 +- internal/firewall/pf_darwin.go | 12 +++----- internal/firewall/warnings.go | 26 +++++++---------- internal/firewall/warnings_test.go | 46 ++++++++++++++++++++++++------ internal/firewall/wfp_windows.go | 9 ++++++ 7 files changed, 72 insertions(+), 34 deletions(-) diff --git a/cmd/dezhban/main.go b/cmd/dezhban/main.go index e1f1687..79d8000 100644 --- a/cmd/dezhban/main.go +++ b/cmd/dezhban/main.go @@ -2020,6 +2020,11 @@ type installedRules struct { // Carried as its own field because the backends already know the answer, // and leaving it discoverable only inside the ruleset text meant a JSON // consumer saw {"loaded":true,"drift":false} and concluded healthy. + // + // It is only as good as the warnings its backend emits. pf and nft report + // every way their rules can be loaded-but-inert; WFP reports none yet, so + // on Windows this currently degrades to `loaded`. Documented in cli.md + // rather than quietly overstated. Enforcing bool `json:"enforcing"` } diff --git a/docs/usage/cli.md b/docs/usage/cli.md index d292070..4e514c2 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -266,7 +266,10 @@ enforcing**. A firewall can hold every rule dezhban installed and filter none of them — pf switched off with `pfctl -d`, an anchor the main ruleset stopped referencing, an nft chain whose policy drifted off `drop` — and in that state every other signal looks healthy, because the rules really are all there. -`enforcing` is false and `warnings` says why. The two texts will not match byte +`enforcing` is false and `warnings` says why. It is only as good as the warnings +its backend emits: pf and nft report every way their rules can be loaded and +inert, **Windows does not yet**, so `enforcing` there currently means no more +than `loaded`. 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). diff --git a/internal/firewall/nft_linux.go b/internal/firewall/nft_linux.go index e5feb4b..5f60475 100644 --- a/internal/firewall/nft_linux.go +++ b/internal/firewall/nft_linux.go @@ -139,8 +139,7 @@ func (b *nftBackend) InstalledRules() (string, bool, error) { } 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("# WARNING: the output chain's policy is no longer drop — this table is loaded but is not cutting anything.\n") } sb.WriteString(out) return sb.String(), true, nil diff --git a/internal/firewall/pf_darwin.go b/internal/firewall/pf_darwin.go index 9922328..926a796 100644 --- a/internal/firewall/pf_darwin.go +++ b/internal/firewall/pf_darwin.go @@ -233,11 +233,9 @@ func (b *pfBackend) InstalledRules() (string, bool, error) { ictx, icancel := context.WithTimeout(context.Background(), pfctlTimeout) defer icancel() if info, err := pfctlCtx(ictx, "", "-s", "info"); err != nil { - b0.WriteString("# WARNING: could not read pf's status, so whether pf is enabled at all\n") - b0.WriteString("# is UNKNOWN — these rules may be loaded but inert.\n") + b0.WriteString("# WARNING: could not read pf's status, so whether pf is enabled at all is UNKNOWN — these rules may be loaded but inert.\n") } else if !strings.Contains(info, "Status: Enabled") { - b0.WriteString("# WARNING: pf is DISABLED — these rules are loaded but nothing is\n") - b0.WriteString("# being filtered. Re-enable with `sudo pfctl -e`.\n") + b0.WriteString("# WARNING: pf is DISABLED — these rules are loaded but nothing is being filtered. Re-enable with `sudo pfctl -e`.\n") } // Its own timeout, like the status probe above and for the same reason: // every probe here gets a full pfctlTimeout rather than the remainder of a @@ -254,13 +252,11 @@ func (b *pfBackend) InstalledRules() (string, bool, error) { // Never silently. A loaded anchor that pf does not descend into is // exactly the non-enforcing state this readback exists to expose, so // "could not check" must not render identically to "checked, fine". - b0.WriteString("# WARNING: could not read the main ruleset, so whether pf descends into\n") - b0.WriteString("# the dezhban anchor is UNKNOWN — these rules may be loaded but inert.\n") + b0.WriteString("# WARNING: could not read the main ruleset, so whether pf descends into the dezhban anchor is UNKNOWN — these rules may be loaded but inert.\n") case mainRulesetReferencesAnchor(main): b0.WriteString("# main ruleset references the dezhban anchor\n") default: - 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("# WARNING: the main ruleset does NOT reference the dezhban anchor — these rules are loaded but pf never descends into them.\n") } b0.WriteString(rules) return b0.String(), true, nil diff --git a/internal/firewall/warnings.go b/internal/firewall/warnings.go index 95108d9..cfac6a3 100644 --- a/internal/firewall/warnings.go +++ b/internal/firewall/warnings.go @@ -20,26 +20,22 @@ const WarningPrefix = "# WARNING:" // which is the healthy case, and the only one in which "loaded" means // "enforcing". // -// Warnings are emitted as consecutive lines, so a continuation line (one that -// follows a warning and is itself a comment) is folded into the warning above -// it rather than dropped: the second half of "pf is DISABLED — these rules are -// loaded but nothing is / being filtered" is the half that says what it means. +// One warning is exactly one line, which is a rule the backends keep rather +// than something inferred here. An earlier version folded a warning's +// continuation lines into it, and could not tell a continuation from the +// ORDINARY comment that follows: pf emits "# main ruleset references the +// dezhban anchor" right after a disabled-pf warning, so the fold produced a +// single entry reading "…nothing is being filtered. Re-enable with `sudo pfctl +// -e`. main ruleset references the dezhban anchor" — a self-contradicting +// sentence, in the pane's orange row, in exactly the scenario the on-host check +// tells a tester to reproduce. Long lines wrap; a heuristic that guesses which +// comments belong together does not. func Warnings(readback string) []string { var out []string - inWarning := false for _, line := range strings.Split(readback, "\n") { trimmed := strings.TrimSpace(line) - switch { - case strings.HasPrefix(trimmed, WarningPrefix): + if strings.HasPrefix(trimmed, WarningPrefix) { out = append(out, strings.TrimSpace(strings.TrimPrefix(trimmed, WarningPrefix))) - inWarning = true - case inWarning && strings.HasPrefix(trimmed, "#"): - cont := strings.TrimSpace(strings.TrimPrefix(trimmed, "#")) - if cont != "" && len(out) > 0 { - out[len(out)-1] += " " + cont - } - default: - inWarning = false } } return out diff --git a/internal/firewall/warnings_test.go b/internal/firewall/warnings_test.go index e2222b2..bc44e46 100644 --- a/internal/firewall/warnings_test.go +++ b/internal/firewall/warnings_test.go @@ -2,13 +2,14 @@ package firewall import ( "reflect" + "strings" "testing" ) // "Loaded" and "enforcing" are different questions. A readback that carries a // warning describes a firewall holding dezhban's rules and filtering nothing, // and a caller has to be able to find that without parsing the ruleset. -func TestWarningsAreFoundAndFolded(t *testing.T) { +func TestWarningsAreFoundOnePerLine(t *testing.T) { for _, tc := range []struct { name string in string @@ -17,16 +18,26 @@ func TestWarningsAreFoundAndFolded(t *testing.T) { {"healthy readback carries none", "# main ruleset references the dezhban anchor\nblock drop out all\n", nil}, {"empty", "", nil}, { - name: "a two-line warning folds into one", - in: "# WARNING: pf is DISABLED — these rules are loaded but nothing is\n" + - "# being filtered. Re-enable with `sudo pfctl -e`.\n" + + name: "a warning is one entry", + in: "# WARNING: pf is DISABLED — these rules are loaded but nothing is being filtered.\n" + "block drop out all\n", - want: []string{"pf is DISABLED — these rules are loaded but nothing is being filtered. Re-enable with `sudo pfctl -e`."}, + want: []string{"pf is DISABLED — these rules are loaded but nothing is being filtered."}, }, { - name: "rules after a warning do not extend it", - in: "# WARNING: something\nblock drop out all\n# an ordinary comment\n", - want: []string{"something"}, + // The regression this shape exists to prevent: pf prints its + // anchor-reference line directly after a disabled-pf warning, and a + // fold that guessed at continuations merged the two into one + // self-contradicting sentence. + name: "an ordinary comment after a warning stays separate", + in: "# WARNING: pf is DISABLED — nothing is being filtered.\n" + + "# main ruleset references the dezhban anchor\n" + + "block drop out all\n", + want: []string{"pf is DISABLED — nothing is being filtered."}, + }, + { + name: "two warnings are two entries", + in: "# WARNING: first thing\n# WARNING: second thing\nblock drop out all\n", + want: []string{"first thing", "second thing"}, }, } { t.Run(tc.name, func(t *testing.T) { @@ -36,3 +47,22 @@ func TestWarningsAreFoundAndFolded(t *testing.T) { }) } } + +// Every warning the backends actually emit must survive Warnings() intact and +// alone. A warning that arrives merged with its neighbour is the pane's orange +// row and a JSON consumer's warnings[0], so its text is a contract. +func TestEveryEmittedWarningIsSelfContained(t *testing.T) { + // The exact pair pf produces with `pfctl -d` and the anchor still + // referenced — the scenario docs/contribute/testing.md tells a tester to + // reproduce. + readback := "# WARNING: pf is DISABLED — these rules are loaded but nothing is being filtered. Re-enable with `sudo pfctl -e`.\n" + + "# main ruleset references the dezhban anchor\n" + + "block drop out all\n" + got := Warnings(readback) + if len(got) != 1 { + t.Fatalf("Warnings() = %#v, want exactly one", got) + } + if strings.Contains(got[0], "references the dezhban anchor") { + t.Errorf("the warning absorbed the comment after it: %q", got[0]) + } +} diff --git a/internal/firewall/wfp_windows.go b/internal/firewall/wfp_windows.go index ffb1065..c43fec3 100644 --- a/internal/firewall/wfp_windows.go +++ b/internal/firewall/wfp_windows.go @@ -221,6 +221,15 @@ func (b *wfpBackend) IsBlocked() (bool, error) { // "rules loaded", with the noise shown to the user as the kernel's ruleset. const noRulesMarker = "# NO-DEZHBAN-RULES" +// TODO(windows): this emits no firewall.WarningPrefix line, ever, so +// `enforcing` degrades to `loaded` on Windows — a host with dezhban's group +// present and a profile whose DefaultOutboundAction is no longer Block reports +// enforcing while filtering nothing. Deciding that properly means comparing the +// live defaults against appliedActionPath(), the way IsBlocked does, and it is +// not written blind: Windows is an unfinished target here (see the matrix note +// in .github/workflows/ci.yml) and this cannot be exercised on the machines +// that build it. docs/usage/cli.md carries the caveat so the field is not read +// as a promise it does not keep on this platform. func (b *wfpBackend) InstalledRules() (string, bool, error) { // The profile defaults come FIRST and unconditionally, before the group is // even looked up. On Windows that default is where the blocking actually From 0a8abec54a7d21965a8db8309d1d6f18691a644e Mon Sep 17 00:00:00 2001 From: Behnam RK Date: Sun, 6 Sep 2026 13:58:33 +0330 Subject: [PATCH 8/8] docs: this branch's changelog entries belonged in Unreleased MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rebase onto post-release main put them inside `## [0.12.0]`. That section is already tagged and published, and it does not contain any of this work — merging it would have made main's changelog claim a shipped release included a feature that was not in it, and left the next release with an empty [Unreleased] despite a whole feature landing. Both entries move to [Unreleased] under their own `### Added`. The [0.12.0] section is now byte-identical to main's again. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df17caf..65251e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,6 @@ current as you land changes. ## [Unreleased] -## [0.12.0] - 2026-09-06 - ### Added - **The firewall rules are visible in Diagnostics.** Three things, because they @@ -38,6 +36,11 @@ current as you land changes. 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. + +## [0.12.0] - 2026-09-06 + +### 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, this app's preferences and saved window state), then opens