feat(cert): add --remove-cert flag and Remove CA button for clean-slate revocation

[dazzling-no-more]

Summary

• Adds mhrv-rs --remove-cert (CLI) and a Remove CA button in the desktop UI for a verified clean-slate CA revocation: clears the OS trust store (macOS login+system keychains, Linux anchor dirs, Windows user+machine Trusted Root), best-effort NSS cleanup (Firefox profiles + Chrome/Chromium on Linux), and deletes the on-disk ca/ directory. config.json and the Apps Script deployment are never touched, so users don't have to redeploy Code.gs.
• Safety first: is_ca_trusted_by_name() verification runs before file deletion and before NSS mutation. A failed OS removal returns RemovalIncomplete, preserves ca/, and leaves browser state alone — retries are idempotent. RemovalOutcome::{Clean, NssIncomplete} lets the UI/CLI print accurate "OS CA removed, browser cleanup partial" status instead of silent false success.
• sudo-safe on Unix: reconcile_sudo_environment() detects geteuid() == 0 + SUDO_USER at each binary's main() entry and re-roots HOME to the invoking user — so data dir / Firefox profiles / macOS login keychain target the real user rather than root.

⚠️ Testing status

Only Windows has been smoke-tested end-to-end (Install → Check → Remove → Check round-trip via both CLI and UI, plus the mutex-on-flags exit-2 behavior). The macOS and Linux paths are built from the existing install-side patterns and covered by unit tests for all the pure logic, but the platform-specific security delete-certificate / update-ca-certificates / trust extract-compat code paths have not been executed on real hardware in this branch. A reviewer on macOS and a reviewer on at least one Linux distro (ideally one Debian-family and one RHEL-family) walking through the test plan below before merge would be valuable.

What changed

• src/cert_installer.rs — remove_ca + per-platform helpers, RemovalOutcome, NssReport, reconcile_sudo_environment, marker-gated Firefox enterprise_roots pref (user-authored lines preserved), idempotent NSS delete that distinguishes "cert not found" from DB-locked/corrupt errors (regression guard for SEC_ERROR_LOCKED_DATABASE)
• src/main.rs — --remove-cert flag, mutually exclusive with --install-cert, calls reconcile_sudo_environment() at startup
• src/bin/ui.rs — Remove CA button, Cmd::RemoveCa handler, shared cert_op_in_progress gate covering both Install and Remove, active-proxy guard for Remove (the CA keypair is live in memory while the proxy runs)
• README.md — English + Persian docs for the new flag, sudo behavior note, correct CN (MasterHttpRelayVPN) for manual cleanup paths, upgrade note about the pre-marker enterprise_roots cosmetic orphan

Tests

29 new unit tests covering the pure logic:

• Firefox user.js marker-block install/strip roundtrips and idempotency (bare lines respected as user-owned)
• getent passwd home-dir parsing (Debian format + malformed inputs + macOS fallback semantics)
• NssReport::is_clean() state rules
• NSS stderr classification (standard "could not find cert", alt wording, locked DB, corrupt DB, permission denied, empty stderr)

Side-effecting paths (security, certutil, update-ca-certificates) are covered by manual E2E per platform since the codebase doesn't yet have a command-runner abstraction.

Test plan

Windows — ✅ smoke-tested locally

• cargo test --lib — 101/101 passes locally
• UI: Install CA → Check CA → Remove CA → Check CA round-trip; verify log shows file=missing trust_store=not trusted after Remove
• CLI: mhrv-rs --install-cert then mhrv-rs --remove-cert; verify %APPDATA%\mhrv-rs\ca\ gone
• CLI: mhrv-rs --install-cert --remove-cert returns exit 2 with --install-cert and --remove-cert cannot be combined

Still to test

• Linux + sudo: confirm log line Detected sudo invocation (SUDO_USER=…): re-rooting HOME to … and that the cert is removed from the real user's Firefox user.js / ~/.pki/nssdb, not root's
• Linux refresh failure: simulate broken update-ca-certificates (e.g. move it aside) and confirm ca/ survives + RemovalIncomplete is reported
• Linux Debian-family + RHEL-family: verify the correct anchor-dir/refresh-cmd pair fires for each
• macOS login-keychain-only install: run mhrv-rs --remove-cert as normal user, confirm no sudo prompt (system-keychain probe avoids escalation when the cert isn't there)
• macOS system-keychain install: verify sudo escalation works when the cert IS in the system keychain
• Any platform UI concurrency: click Start, immediately click Remove CA — button disabled, handler rejects with "proxy is running or starting"
• Any platform UI serialization: click Install CA then Remove CA back-to-back — confirm cert_op_in_progress gate prevents the race

Compatibility with Mode::Full (#94)

Full mode doesn't use the MITM CA, so Remove CA is harmless there:

• apps_script / google_only users: unchanged, works as described.
• apps_script → full migrators: Remove CA is the recommended cleanup step after switching.
• full-from-day-one users: no trust-store entry → verification passes → ca/ deleted if present → no-op in practice.

[therealaleph]

Reviewed the diff and ran locally on macOS host — cargo check clean, cargo test --quiet = 101/101 passes (up from 75 on main, so ~26 net new). The code itself is well-structured and the quality signals (bilingual docs update, changelog entry, 29-test test plan, candid "only Windows tested" disclosure in the PR body) are genuinely impressive for a first contribution here.

That said, I can't auto-merge this one for two concrete reasons:

1. macOS + Linux E2E paths are self-declared as untested. You called this out in the PR body, which I appreciate. But security delete-certificate on macOS keychains, update-ca-certificates / trust extract-compat on Linux distros, and the certutil NSS operations against Firefox profiles are the kind of code paths where a bug means a reviewer ends up with an orphan root CA they have to manually clean with Keychain Access / /etc/ssl/certs/. I don't want to ship an untested cert-removal flow to users who might not be comfortable with manual keychain cleanup when it fails.

2. You're a new contributor to this repo (first PR). Combined with 1377 additions in a security-adjacent module, the conservative thing is to leave this open for explicit maintainer sign-off rather than auto-merging on my "cargo tests pass" alone. That's a rule rather than a reflection on the code quality.

What would unblock merge:

Ideally three smoke tests from three separate reviewers (one macOS, one Debian/Ubuntu-family Linux, one Fedora/RHEL-family Linux) walking through your ## Test plan checklist. If that's too high a bar, at minimum one macOS reviewer and one Linux reviewer.

I'll flag this in the repo for maintainer attention. If nobody steps up within a few days, I can run the macOS path myself on a disposable VM rather than my host machine — but that's slower than someone who can do it on their actual device.

Two small review notes on the diff itself (not blockers):

1. reconcile_sudo_environment() re-rooting HOME via SUDO_USER is the right approach, but I'd want a test that specifically covers the "no SUDO_USER set but euid==0" case (real root login, not sudo). The current behavior should be "don't re-root, leave HOME as /root" and that's not explicitly asserted in the suite as far as I read.

2. The pre-marker enterprise_roots cosmetic orphan note in your README upgrade section is helpful — thanks for surfacing that. Might be worth an actual warning print in remove_ca when it detects a pre-marker line (just so the user knows their Firefox user.js has an orphan pref, not that anything's broken).

Thanks again for the depth — this is the kind of PR I'd love to see more of.

---

_{[reply via Anthropic Claude | reviewed by @therealaleph]}

[dazzling-no-more]

Thanks for the careful review, the detailed context on the merge policy is appreciated, and the code-quality compliment means a lot.

Quick note on the first-contributor point: this is actually not my first PR to this repo. I also contributed the google_only bootstrap mode a while back (the direct SNI-rewrite path that lets users reach script.google.com to deploy Code.gs before they have an Apps Script relay).

On why this feature matters: the MITM CA this app installs has its private key on the user's disk, and the OS trusts it for every HTTPS site. That's a non-trivial capability to leave lying around, if the key is ever exposed (lost laptop, leaked backup, a machine sold or handed down, etc.), anyone holding it can mint certificates the browser silently accepts as legitimate for any domain. And on the OS side, a stale trusted root is effectively a standing MITM authorization until someone notices it in the cert store. Without a clean-slate uninstall path, users who try mhrv-rs and move on, or switch to Full Tunnel Mode and no longer need a local MITM, end up with that capability sitting on disk and in their trust store indefinitely. So I think having a one-command clean removal is worth some review rigor, which I fully agree with.

On the two small notes, I've pushed a follow-up commit addressing both:

1. reconcile_sudo_home decision logic extracted into a pure should_reconcile_for(euid, sudo_user) helper with four branch tests covering every case in the matrix, including the one you called out explicitly: euid == 0 && SUDO_USER unset (real root login, not sudo). That branch now has an explicit assert_eq!(should_reconcile_for(0, None), None) so the "leave HOME as /root" invariant is pinned down.

2. Pre-marker orphan warning disable_firefox_enterprise_roots now logs an info-level hint when a profile's user.js has a bare security.enterprise_roots.enabled = true without our marker above it. The log line explains it's cosmetic (Firefox falls back to its built-in root store once the CA leaves the OS trust store) and suggests manual removal if the user wants a clean file. New has_bare_enterprise_roots pure helper with four tests.

Test count: 109/109 passing.

On the platform-coverage bar completely reasonable, and no pressure on the VM offer. Take it when it's convenient. If any macOS or Linux reviewer sees this thread and wants to walk the ## Test plan checklist (the Install → Check → Remove → Check round-trip, the sudo HOME re-rooting verification, and the Linux refresh-failure retry case are the ones I'd most want another pair of eyes on), that would unblock merge faster than you carving out VM time.

Thanks again.

[therealaleph]

Heads-up: we just rewrote git history on main for a privacy-related cleanup (a few old commits had a contributor's real email/name leaked via git config user.email on their local machine; rewritten to the canonical noreply form). Force-push to main + all version tags landed a few minutes ago.

This PR's branch is based on the pre-rewrite SHAs, so you'll need to rebase before it can merge cleanly. Easiest path:

git fetch origin
git checkout feature/delete_certificate
git rebase origin/main
# resolve any conflicts (your changes don't touch any rewritten files,
# so this should be conflict-free — just SHA pointers updating)
git push --force-with-lease

If --force-with-lease complains, your local clone is out of sync because every SHA changed; git fetch origin && git reset --hard <your-fork>/feature/delete_certificate first to re-anchor, then rebase.

Functionally nothing about your PR has changed and the review is still where we left it — still awaiting macOS + Linux smoke tests from reviewers. Sorry for the disruption.

---

_{[reply via Anthropic Claude | reviewed by @therealaleph]}

[therealaleph]

Hey @dazzling-no-more — thanks for the cert-removal work. The feature itself looks solid (the RemovalOutcome::{Clean, NssIncomplete}, the reconcile_sudo_environment() for sudo-aware HOME re-rooting, and the marker-gated Firefox enterprise_roots pref are all the right shapes), but the branch can't merge as-is.

The diff is showing +16,403 / -1,210 across 95 files because the fork point predates a lot of work that's already in main:

• android/ directory in full (merged via earlier Android PRs)
• tunnel-node/ workspace (merged in v1.5.0)
• src/android_jni.rs, src/tunnel_client.rs, src/update_check.rs (all already in main)
• docs/changelog/v1.1.0.md through v1.2.13.md (all already in main)
• assets/apps_script/Code.gs, CodeFull.gs (already in main)
• pre-built artifacts under releases/

So GitHub is showing this as adding all of that again, not because you wrote it twice but because git thinks your branch lacks those commits.

Could you do a clean rebase onto current main and force-push? Concretely:

git fetch origin
git rebase origin/main
# resolve conflicts — most should auto-resolve since you'd be re-applying just your cert-removal commits on top
git push --force-with-lease

If the rebase gets ugly because the cert-installer conflicts are nontrivial, the cleanest path is probably:

git checkout main && git pull
git checkout -b fix/remove-cert-rebased
# cherry-pick just your cert-installer + main.rs + ui.rs + README cert commits onto fresh main

Once the diff is just the --remove-cert feature (probably ~1500 LOC across src/cert_installer.rs, src/main.rs, src/bin/ui.rs, README.md), happy to actually review the trust-store removal logic per platform. The unit-test coverage you described (29 new tests for the pure logic) is a great start.

The Windows smoke test is enough to start — I'll do macOS, and we can ask for a Linux pair from someone with a Debian + RHEL box once the diff is reviewable.

---

_{[reply via Anthropic Claude | reviewed by @therealaleph]}

[dazzling-no-more]

@therealaleph rebase is done and it can be merged if you managed to test it on macos.

[therealaleph]

Merged in v1.7.1 — thanks for the rebase and the careful work here.

Code review notes that landed me on "merge":

• reconcile_sudo_environment() is correctly gated. The geteuid() == 0 && SUDO_USER set non-empty non-root triple-check rules out every "untrusted SUDO_USER" attack I could think of. EUID gate is the load-bearing part — appreciate that you split should_reconcile_for into a pure function specifically so it could be unit-tested.
• is_ca_trusted_by_name() being path-independent in remove_ca is the right call. A pre-deleted ca.crt would otherwise mask a still-trusted stale root; you got that right. The "verify before mutate, then mutate, then re-verify is implicit via idempotent retry" sequencing is meticulous.
• No changes to install_ca / install_macos / install_linux / install_windows — pure addition. The blast radius for existing users on the install path is zero.

macOS verified at merge. I ran mhrv-rs --remove-cert on real hardware (Apple Silicon). Output:

INFO Removing CA certificate on macos...
INFO Removed CA from login keychain.
WARN NSS certutil not found — cannot automatically remove CA from Firefox/Chrome NSS stores.
INFO Deleted CA files at /Users/.../Application Support/mhrv-rs/ca
INFO OS CA removed. NSS cleanup skipped — NSS certutil not found.
INFO A fresh CA will be generated next time the proxy starts — run --install-cert then to re-trust it.
exit=0

• Login keychain delete: works ✅
• NSS-missing graceful fallback: works ✅
• File cleanup: works ✅
• RemovalOutcome::NssIncomplete reasoning surfaced clearly to user ✅

So that's macOS off the "still to test" list. Linux paths still rely on the unit-test coverage and your structural similarity to the install path; if any user reports a problem we'll have logs to triage from. The mutex-on-flags exit-2 behavior also verified on macOS.

Two minor follow-ups I'd take in a future PR (no urgency):

1. The "system keychain only requires sudo when the cert is there" probe. The login-keychain-only smoke I just did didn't escalate — perfect. But the system-keychain case wasn't exercised. If you ever do another machine with an --install-cert to system store, a smoke of --remove-cert to confirm the sudo prompt would close that test box.

2. A Linux Debian-family + RHEL-family pair test. Whoever has a Fedora/Alma/Rocky box around — running the install/remove cycle would land both anchor-dir paths. I don't have that stack right now.

Closing #121 as released. The umbrella was the cleanest way to track this; thanks for the structured PR description making the merge call straightforward.

---

_{[reply via Anthropic Claude | reviewed by @therealaleph]}