Skip to content

feat(diag): recent problems and a redacted export bundle - #55

Open
Behnam-RK wants to merge 4 commits into
mainfrom
feat/problems-and-export
Open

feat(diag): recent problems and a redacted export bundle#55
Behnam-RK wants to merge 4 commits into
mainfrom
feat/problems-and-export

Conversation

@Behnam-RK

Copy link
Copy Markdown
Owner

Stacked on #53#52#51#50#49.

"I want to collect errors somehow" — two surfaces over the same idea. dezhban already knows what went wrong; until now finding out meant knowing where a root-owned log lived and reading slog output by eye.

Recent problems

Diagnostics gains a Recent problems section: warn-and-worse records from dezhban's own log, newest first, with the evidence dezhban logged beside each one.

Three states, kept deliberately distinct — "nothing logged as a warning or an error" is shown in green as the good answer it is, and never collapsed into "couldn't read the log". Collapsing them would make a healthy host look like a broken reader.

Backed by a new dezhban logs (--level, --since, --limit, --json). No root: the log is 0644 by design, the same call state.json makes.

Parsing lives in Go

internal/logread parses slog's text format back into records. That format is written in this repo, so a second parser in Swift would be a second thing to get wrong about quoting — msg="rules missing, re-applied" repairs=2 splits into a truncated message and two garbage attrs under a naive strings.Split — and it could not be tested against the writer.

Two properties it holds to:

  • The rotated archives are read too, oldest first. The interesting failure is often the one that pushed the file over its rotation threshold.
  • Nothing is silently dropped. A line the parser cannot make sense of comes back with its raw text intact; an unrecognised level sorts as INFO rather than being filtered out by a warn-and-above query. A level this build does not know is not evidence the record is unimportant.

Attrs are carried as ordered pairs all the way to the SwiftUI row — the order dezhban wrote them in reads as a sentence, and a dictionary anywhere on that path would shuffle it.

The bundle

dezhban report (and Diagnostics → Export…) writes one zip: config, state.json, learned.json, armed.json, the ruleset dezhban last applied, doctor's findings, what each posture would apply, and recent log records — plus a README explaining each.

A file that is missing is noted inside the bundle rather than failing the collection. A host that never ran dezhban has no state, and that must not be the reason you cannot collect a report.

Nothing is sent anywhere. It is a local file; sharing it is the operator's decision. Not a gap to close later — this is a tool whose job is that traffic does not leave the machine, and CLAUDE.md already denies dezhban upgrade its own firewall pass on the same reasoning.

Redaction, and why it is shaped this way

internal/redact replaces addresses and hostnames with stable placeholders, not [redacted]. The same address is the same token everywhere it appears, so "the rules pass ip-1 but the endpoint is ip-2" survives as a finding — flattening everything to one token would hide exactly the class of bug a bundle is collected to diagnose.

Three decisions worth reviewing:

  • Structural addresses are kept. Loopback, unspecified, private, link-local and multicast. They identify nobody — every install has them — and redacting them turns pass on lo0 to 127.0.0.1 into pass on lo0 to ip-4, which hides that the rule is loopback.
  • Hostnames use an ALLOW-list, not a deny-list. That direction is the whole safety property: an unanticipated name is redacted. A deny-list would leak every hostname nobody thought of, which is precisely the VPN provider this exists to hide. The allow-list is dezhban's own shipped geo providers (identical on every install, and which one answered is a real diagnostic) plus file-ish suffixes.
  • Disabled is a true pass-through through the same code path. No second, less-tested route for the unredacted case to drift down.

The bundle's README legend reports counts ("23 distinct IP addresses → ip-1 … ip-23"), never originals — it ships inside the bundle, so listing them would undo the whole exercise.

The opt-out is loud in three places: the checkbox label says what the bundle will contain rather than describing the mechanism, the CLI warns on stderr, and the bundle's own README says NONE under Redaction.

Verification

  • go build, go vet (plus GOOS=linux and GOOS=windows), go test — pass.
  • internal/logread: quoted messages with spaces stay one message, escaped quotes survive, list attrs both forms, unparseable lines are kept, unknown levels are not dropped, level filtering, --limit keeps the most recent, archives read oldest-first, missing log is empty not an error, --since.
  • internal/redact: a public address does not survive; the same address always gets the same placeholder; structural addresses and prefix lengths are kept; an unknown hostname is redacted while shipped providers and filenames are not; version strings are left alone; disabled is a pass-through; the legend never contains the originals; and a real pf ruleset loses every identifier while keeping utun4, lo0, port 51820 and block drop out all.
  • swift test — 210 tests; LogRecordsTests covers Go's fractional timestamps, attr ordering, and that two byte-identical records from a retry loop stay distinct rows.
  • Ran dezhban report against this machine's real config and grepped the unpacked bundle for identifiers. Only 10.0.0.0, 169.254.0.0, 172.16.0.0, 192.168.0.0, 224.0.0.0 and 239.0.0.0 survived — the deliberately-kept ranges. No real endpoint, provider hostname, or exit IP appeared.

The redaction check is also in docs/contribute/testing.md as an on-host step, because it is the one that matters: a redactor that misses a field is worse than none, since it advertises a safety it did not deliver.

🤖 Generated with Claude Code

Behnam-RK and others added 4 commits August 22, 2026 10:39
uninstall.sh removed only root-owned state, so everything belonging to the
logged-in user survived it: the preference domain, the login-keychain control
token, and the login-item registration. That is also why the setup wizard stopped
appearing after a reinstall — FirstRunDecision.offer reads
dezhban.firstRunCompleted out of a preference domain no uninstaller ever cleared,
so a machine with an empty /etc/dezhban still answered "already done". The
missing feature and the reported bug are the same defect from two ends.

Root cannot do this work: a login keychain item's ACL is bound to the user's
session and a login item is registered per user. So the app owns the per-user
half. Settings gains Remove Dezhban…, which clears the keychain token and its
capability probe, both login-item registrations, the saved window state, and the
preference domains (current and the dead com.dezhban.DezhbanMenu), then opens
Terminal on the root uninstaller and quits.

Terminal rather than an in-app sheet because uninstall.sh quits Dezhban and
deletes its bundle partway through: a progress sheet dies mid-teardown and cannot
distinguish a finished uninstall from one that stopped after panic removed the
rules. A terminal window outlives the app and shows the rule teardown, which for
a kill switch is the step you most want to watch succeed.

Preference domains are cleared last, immediately before terminate, because AppKit
writes defaults as the app winds down; uninstall.sh repeats the deletion for
$SUDO_USER for the same reason, and names the two per-user items it cannot reach
rather than failing at them silently.

Refs docs/adr/0015-complete-purge-semantics.md

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Countries, then one "Use automatic VPN detection?" tickbox with the manual
fields — tunnel interfaces, self-hosted config files, endpoints — hanging off
it. The opening "Configure your VPN now?" question is gone, and both wizards
read the same question set, so the CLI changes with the app.

Dropping configureVPN removes the thing that stood between a re-run and a
working config, so two guards replace it.

autoMode's default is seeded from the config: a config with pinned
vpn.tunnelInterfaces starts on manual, because Apply clears TunnelInterfaces
under AutoMode on purpose and a wizard defaulting to automatic would silently
unpin them.

Input.Endpoints becomes *[]string, nil when the question was never shown. On
macOS the endpoint question is gated behind "not automatic", so a re-run that
leaves detection on reaches Apply with no answer — writing that as an empty list
would delete the user's server. Same convention, and the same reasoning, as the
existing AutoDiscover *bool. The Swift side already had this property via
shouldAsk. Off macOS the question is ungated instead: there is no live
discovery, so an endpoint is required whichever mode is chosen.

The CLI now asks a group in waves. A huh form binds every field before any is
answered, so a question gated on another question in the same group would be
decided by that question's seeded default. Rather than split the shared question
set to suit one renderer, the terminal asks the ungated questions, re-evaluates,
and asks whatever that opened up — the app shows step 2 as one screen because it
re-evaluates gates as answers change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Two surfaces over the same idea: dezhban already knows what went wrong, and
until now finding out meant knowing where a root-owned log lived and reading
slog output by eye.

internal/logread parses the daemon's own text-format log back into records.
Parsing belongs in Go because that is where the format is written — a second
parser in Swift would be a second thing to get wrong about slog's quoting, and
could not be tested against the writer. It reads the rotated archives too: the
interesting failure is often the one that pushed the file over its rotation
threshold. A line it cannot parse is kept, not dropped; an unrecognised level
sorts as INFO rather than being filtered away.

internal/redact replaces network identifiers with STABLE placeholders, so the
same server is the same token everywhere and "the rules pass ip-1 but the
endpoint is ip-2" survives as a finding. Loopback, private, link-local and
multicast addresses are kept — they identify nobody, and hiding them makes a
ruleset unreadable. Hostname handling is an ALLOW-list, which is the whole
safety property: an unanticipated name is redacted rather than leaked, where a
deny-list would leak exactly the VPN provider this exists to hide. Disabled is a
true pass-through through the same code path, so the full-fidelity case cannot
drift down a less-tested route.

`dezhban report` writes one zip and stops there. Nothing is transmitted, for the
reason CLAUDE.md already denies `upgrade` its own firewall pass. A missing input
is noted inside the bundle rather than failing the collection — a host that
never ran dezhban has no state, and that must not be why a report cannot be
collected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Behnam-RK
Behnam-RK force-pushed the feat/problems-and-export branch from 28e19f8 to 0115bc5 Compare August 22, 2026 07:11
Base automatically changed from feat/diagnostics-firewall-rules to main September 6, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant