From b3a59b5b457cc0e04a6f00d7d66b15410a6bb856 Mon Sep 17 00:00:00 2001 From: Stefano Fiorini Date: Mon, 30 Mar 2026 12:08:25 -0500 Subject: [PATCH] fix(nordvpn-client): validate live utun persistence before dns pinning --- docs/nordvpn-client.md | 12 ++++++++++- skills/nordvpn-client/SKILL.md | 11 ++++++++++ .../nordvpn-client/scripts/nordvpn-client.js | 8 +++---- .../scripts/nordvpn-wireguard-helper.sh | 21 +++++++++++++------ 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/docs/nordvpn-client.md b/docs/nordvpn-client.md index 5ae1cdb..340ea7c 100644 --- a/docs/nordvpn-client.md +++ b/docs/nordvpn-client.md @@ -69,13 +69,18 @@ Current macOS backend: - NordLynx/WireGuard - `wireguard-go` - `wireguard-tools` -- NordVPN DNS in the generated WireGuard config: +- explicit macOS DNS management on eligible physical services: - `103.86.96.100` - `103.86.99.100` Important behavior: - `NordVPN.app` may remain installed, but the automated backend does not reuse app login state. +- the generated WireGuard config intentionally stays free of `DNS = ...` so `wg-quick` does not rewrite every macOS network service behind the skill’s back. +- during `connect`, the skill first proves the tunnel is stable with a bounded persistence gate that reuses the allowed helper `probe` action and a verified public exit. +- during `connect`, the skill snapshots current DNS/search-domain settings on eligible physical services and then applies NordVPN DNS only after that stable gate and one last liveness check succeed. +- during `disconnect`, or after a failed/stale teardown, the skill restores the saved DNS/search-domain snapshot. +- if persistence or exit verification fails, the skill rolls back before DNS is pinned and resumes Tailscale if it stopped it. - The skill automatically suspends Tailscale before connect if Tailscale is active. - The skill resumes Tailscale after disconnect, or after a failed connect, if it stopped it. - The Homebrew NordVPN app does not need to be uninstalled. @@ -144,6 +149,8 @@ Add this exact rule: stefano ALL=(root) NOPASSWD: /Users/stefano/.openclaw/workspace/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh probe, /Users/stefano/.openclaw/workspace/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh up, /Users/stefano/.openclaw/workspace/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh down ``` +Do not add extra helper actions just for persistence checks unless you are also updating host sudoers. The current implementation intentionally rides the persistence check on `probe` so the existing `probe/up/down` rule remains sufficient. + If you run the repo copy directly instead of the installed OpenClaw skill, adjust the helper path accordingly. ## Common Flows @@ -188,7 +195,9 @@ Expected macOS behavior: - stop Tailscale if active - select a NordVPN server for the target - bring up the WireGuard tunnel +- prove persistence of the live `utun*` runtime via the helper `probe` path - verify the public exit location +- run one final liveness check before applying NordVPN DNS - return JSON describing the chosen server and final verified location ### Verify @@ -209,6 +218,7 @@ Expected macOS behavior: - attempt `wg-quick down` whenever there is active or residual NordVPN WireGuard state - remove stale local NordVPN state files after teardown +- restore automatic DNS when the saved DNS snapshot is obviously just NordVPN-pinned leftovers - resume Tailscale if the skill had suspended it ## Output Model diff --git a/skills/nordvpn-client/SKILL.md b/skills/nordvpn-client/SKILL.md index c96832f..9c02a64 100644 --- a/skills/nordvpn-client/SKILL.md +++ b/skills/nordvpn-client/SKILL.md @@ -41,6 +41,11 @@ node scripts/nordvpn-client.js status --debug - use NordLynx/WireGuard through `wireguard-go` and `wireguard-tools` - `install` bootstraps them with Homebrew - `login` validates the token for the WireGuard backend + - the generated WireGuard config stays free of `DNS = ...` + - `connect` now requires a bounded persistence gate plus a verified exit before success is declared + - the skill snapshots and applies NordVPN DNS only to eligible physical services while connected + - NordVPN DNS is applied only after the tunnel remains up and the final liveness check still shows the requested exit + - `disconnect` restores the saved DNS/search-domain state even if the tunnel state is stale - Tailscale is suspended before connect and resumed after disconnect or failed connect - `NordVPN.app` may remain installed but is only the manual fallback @@ -75,6 +80,10 @@ Exact `visudo` rule for the installed OpenClaw skill: stefano ALL=(root) NOPASSWD: /Users/stefano/.openclaw/workspace/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh probe, /Users/stefano/.openclaw/workspace/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh up, /Users/stefano/.openclaw/workspace/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh down ``` +Operational note: + +- the persistence gate reuses the already-allowed `probe` action to confirm the live `utun*` WireGuard runtime and does not require extra sudoers actions beyond `probe`, `up`, and `down` + ## Agent Guidance - run `status` first when the machine state is unclear @@ -83,6 +92,7 @@ stefano ALL=(root) NOPASSWD: /Users/stefano/.openclaw/workspace/skills/nordvpn-c - use `connect` before location-sensitive skills such as `web-automation` - use `verify` after connect when you need an explicit location check - use `disconnect` after the follow-up task +- if `connect` fails its persistence or final verification gate, treat that as a safe rollback, not a partial success ## Output Rules @@ -98,6 +108,7 @@ stefano ALL=(root) NOPASSWD: /Users/stefano/.openclaw/workspace/skills/nordvpn-c - connect succeeds but final state looks inconsistent: - rely on the verified public IP/location first - then inspect `status --debug` +- `verified: true` but `persistence.stable: false` should not happen anymore; if it does, the skill should roll back instead of pinning DNS - disconnect should leave: - normal public IP restored - no active WireGuard state diff --git a/skills/nordvpn-client/scripts/nordvpn-client.js b/skills/nordvpn-client/scripts/nordvpn-client.js index 32ef433..1005700 100644 --- a/skills/nordvpn-client/scripts/nordvpn-client.js +++ b/skills/nordvpn-client/scripts/nordvpn-client.js @@ -781,6 +781,7 @@ function parseMacWireguardHelperStatus(output) { return { active: ["1", "true", "yes", "on"].includes(`${parsed.active || ""}`.toLowerCase()), interfaceName: parsed.interfaceName || MAC_WG_INTERFACE, + wireguardInterface: parsed.wireguardInterface || null, configPath: parsed.configPath || null, raw: `${output || ""}`.trim(), }; @@ -788,7 +789,7 @@ function parseMacWireguardHelperStatus(output) { async function getMacWireguardHelperStatus(installProbe, options = {}) { const runSudoWireguardFn = options.runSudoWireguard || runSudoWireguard; - const result = await runSudoWireguardFn(installProbe, "status"); + const result = await runSudoWireguardFn(installProbe, "probe"); const parsed = parseMacWireguardHelperStatus(result.stdout || result.stderr || ""); return { ...parsed, @@ -1012,8 +1013,7 @@ async function probeMacWireguard() { const helperPath = fileExists(MAC_WG_HELPER_PATH) ? MAC_WG_HELPER_PATH : null; const helperSecurity = inspectMacWireguardHelperSecurity(helperPath); const sudoProbe = helperPath ? await runExec("sudo", ["-n", helperPath, "probe"]) : { ok: false }; - const helperStatus = - helperPath && sudoProbe.ok ? parseMacWireguardHelperStatus((await runExec("sudo", ["-n", helperPath, "status"])).stdout) : null; + const helperStatus = helperPath && sudoProbe.ok ? parseMacWireguardHelperStatus(sudoProbe.stdout || sudoProbe.stderr || "") : null; let active = false; let showRaw = ""; let endpoint = ""; @@ -1668,7 +1668,6 @@ async function disconnectNordvpn(installProbe) { if (!down.ok) { const message = (down.stderr || down.stdout || down.error).trim(); if (isBenignMacWireguardAbsentError(message)) { - await runSudoWireguard(installProbe, "cleanup"); const dnsState = await restoreMacDnsIfNeeded(); const cleaned = cleanupMacWireguardAndDnsState(); const tailscale = await resumeMacTailscaleIfNeeded(); @@ -1683,7 +1682,6 @@ async function disconnectNordvpn(installProbe) { } throw new Error(message || "wg-quick down failed"); } - await runSudoWireguard(installProbe, "cleanup"); const dnsState = await restoreMacDnsIfNeeded(); const cleaned = cleanupMacWireguardAndDnsState(); const tailscale = await resumeMacTailscaleIfNeeded(); diff --git a/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh b/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh index 62f76a3..1c1b879 100755 --- a/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh +++ b/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh @@ -18,21 +18,30 @@ WG_INTERFACE="nordvpnctl" PATH="/opt/homebrew/bin:/usr/bin:/bin:/usr/sbin:/sbin" export PATH -if [ "$ACTION" = "probe" ]; then +if [ "$ACTION" = "probe" ] || [ "$ACTION" = "status" ]; then test -x "$WG_QUICK" - exit 0 -fi - -if [ "$ACTION" = "status" ]; then ACTIVE=0 - if [ -x "$WG" ] && "$WG" show "$WG_INTERFACE" >/dev/null 2>&1; then + RUNTIME_INTERFACE="" + if [ -x "$WG" ]; then + RUNTIME_INTERFACE=$("$WG" show interfaces 2>/dev/null | awk 'NF { print $1; exit }') + fi + if [ -n "$RUNTIME_INTERFACE" ]; then + ACTIVE=1 + elif [ -x "$WG" ] && "$WG" show "$WG_INTERFACE" >/dev/null 2>&1; then ACTIVE=1 elif /sbin/ifconfig "$WG_INTERFACE" >/dev/null 2>&1; then ACTIVE=1 + elif pgrep -f "wg-quick up $WG_CONFIG" >/dev/null 2>&1; then + ACTIVE=1 + elif pgrep -f "wireguard-go utun" >/dev/null 2>&1; then + ACTIVE=1 fi echo "active=$ACTIVE" echo "interfaceName=$WG_INTERFACE" + if [ -n "$RUNTIME_INTERFACE" ]; then + echo "wireguardInterface=$RUNTIME_INTERFACE" + fi if [ -f "$WG_CONFIG" ]; then echo "configPath=$WG_CONFIG" fi