diff --git a/docs/nordvpn-client.md b/docs/nordvpn-client.md index 340ea7c..57691bd 100644 --- a/docs/nordvpn-client.md +++ b/docs/nordvpn-client.md @@ -78,9 +78,10 @@ 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 `connect`, the skill snapshots current DNS/search-domain settings on eligible physical services and then applies NordVPN DNS only after that stable gate, one last liveness check, and a post-DNS system-hostname-resolution 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. +- if persistence, exit verification, or post-DNS hostname resolution fails, the skill rolls back before treating the connect as successful and resumes Tailscale if it stopped it. +- when the skill intentionally stops Tailscale for a VPN session, it writes a short-lived suppression marker so host watchdogs do not immediately run `tailscale up` and fight the VPN route change. - 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. @@ -248,7 +249,9 @@ For deeper troubleshooting, use: node skills/nordvpn-client/scripts/nordvpn-client.js status --debug ``` -`--debug` keeps the internal local paths and other low-level metadata in the JSON output. +`--debug` keeps the internal local paths, helper-hardening diagnostics, and other low-level metadata in the JSON output. + +If you also run local watchdogs such as `healthwatch.sh`, they should honor the NordVPN Tailscale suppression marker at `~/.nordvpn-client/tailscale-suppressed` and skip automatic `tailscale up` while the marker is fresh or the NordVPN WireGuard tunnel is active. ## Troubleshooting diff --git a/skills/nordvpn-client/SKILL.md b/skills/nordvpn-client/SKILL.md index 9c02a64..510b403 100644 --- a/skills/nordvpn-client/SKILL.md +++ b/skills/nordvpn-client/SKILL.md @@ -44,9 +44,10 @@ node scripts/nordvpn-client.js status --debug - 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 + - NordVPN DNS is applied only after the tunnel remains up, the final liveness check still shows the requested exit, and system hostname resolution still works afterward - `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 + - the skill writes a short-lived Tailscale suppression marker during VPN connect so host watchdogs do not immediately re-run `tailscale up` - `NordVPN.app` may remain installed but is only the manual fallback ## Credentials @@ -96,7 +97,7 @@ Operational note: ## Output Rules -- normal JSON output redacts local path metadata +- normal JSON output redacts local path metadata and helper-hardening diagnostics - use `--debug` only when deeper troubleshooting requires internal local paths and helper/config metadata ## Troubleshooting Cues diff --git a/skills/nordvpn-client/scripts/nordvpn-client.js b/skills/nordvpn-client/scripts/nordvpn-client.js index 1005700..bc809e9 100644 --- a/skills/nordvpn-client/scripts/nordvpn-client.js +++ b/skills/nordvpn-client/scripts/nordvpn-client.js @@ -16,6 +16,7 @@ const AUTH_CACHE_PATH = path.join(STATE_DIR, "auth.json"); const LAST_CONNECTION_PATH = path.join(STATE_DIR, "last-connection.json"); const MAC_DNS_STATE_PATH = path.join(STATE_DIR, "dns.json"); const MAC_TAILSCALE_STATE_PATH = path.join(STATE_DIR, "tailscale.json"); +const MAC_TAILSCALE_SUPPRESS_PATH = path.join(STATE_DIR, "tailscale-suppressed"); const OPERATION_LOCK_PATH = path.join(STATE_DIR, "operation.lock"); const OPENCLAW_NORDVPN_CREDENTIALS_DIR = path.join( os.homedir(), @@ -42,7 +43,10 @@ const NORDVPN_MAC_DNS_SERVERS = ["103.86.96.100", "103.86.99.100"]; const CONNECT_PERSISTENCE_ATTEMPTS = 6; const CONNECT_PERSISTENCE_DELAY_MS = 2000; const CONNECT_TOTAL_TIMEOUT_MS = 90000; -const REDACTED_PATH_KEYS = new Set(["cliPath", "appPath", "configPath", "helperPath", "tokenSource"]); +const POST_DNS_RESOLUTION_HOSTNAMES = ["www.google.com", "api.openai.com", "docs.openclaw.ai"]; +const POST_DNS_RESOLUTION_TIMEOUT_MS = 4000; +const POST_DNS_SETTLE_DELAY_MS = 1500; +const REDACTED_PATH_KEYS = new Set(["cliPath", "appPath", "configPath", "helperPath", "tokenSource", "helperSecurity"]); function sanitizeOutputPayload(payload) { if (Array.isArray(payload)) { @@ -353,6 +357,19 @@ function buildMacTailscaleState(tailscaleWasActive) { return { tailscaleWasActive: Boolean(tailscaleWasActive) }; } +function markMacTailscaleRecoverySuppressed() { + ensureDir(STATE_DIR); + writeTextFile(MAC_TAILSCALE_SUPPRESS_PATH, `${new Date().toISOString()}\n`, 0o600); +} + +function clearMacTailscaleRecoverySuppressed() { + try { + fs.unlinkSync(MAC_TAILSCALE_SUPPRESS_PATH); + } catch { + // Ignore cleanup errors. + } +} + function inspectMacWireguardHelperSecurity(helperPath, deps = {}) { const fileExistsFn = deps.fileExists || fileExists; const statSyncFn = deps.statSync || fs.statSync; @@ -647,12 +664,15 @@ async function getMacTailscaleStatus() { async function stopMacTailscaleIfActive() { const status = await getMacTailscaleStatus(); if (!status.active) { + clearMacTailscaleRecoverySuppressed(); writeJsonFile(MAC_TAILSCALE_STATE_PATH, buildMacTailscaleState(false)); return { tailscaleWasActive: false }; } + markMacTailscaleRecoverySuppressed(); const tailscale = getMacTailscalePath(); const result = await runExec(tailscale, ["down"]); if (!result.ok) { + clearMacTailscaleRecoverySuppressed(); throw new Error((result.stderr || result.stdout || result.error).trim() || "tailscale down failed"); } writeJsonFile(MAC_TAILSCALE_STATE_PATH, buildMacTailscaleState(true)); @@ -668,6 +688,7 @@ async function resumeMacTailscaleIfNeeded() { currentStatus = null; } if (!shouldResumeMacTailscale(state, currentStatus && currentStatus.active)) { + clearMacTailscaleRecoverySuppressed(); try { fs.unlinkSync(MAC_TAILSCALE_STATE_PATH); } catch { @@ -680,6 +701,7 @@ async function resumeMacTailscaleIfNeeded() { if (!result.ok) { throw new Error((result.stderr || result.stdout || result.error).trim() || "tailscale up failed"); } + clearMacTailscaleRecoverySuppressed(); try { fs.unlinkSync(MAC_TAILSCALE_STATE_PATH); } catch { @@ -712,6 +734,52 @@ async function resolveHostnameWithFallback(hostname, options = {}) { return ""; } +async function verifySystemHostnameResolution(hostnames = POST_DNS_RESOLUTION_HOSTNAMES, options = {}) { + const lookup = + options.lookup || + ((hostname) => + dns.promises.lookup(hostname, { + family: 4, + })); + const timeoutMs = Number.isFinite(options.timeoutMs) && options.timeoutMs > 0 ? options.timeoutMs : POST_DNS_RESOLUTION_TIMEOUT_MS; + const settleDelayMs = + Number.isFinite(options.settleDelayMs) && options.settleDelayMs >= 0 ? options.settleDelayMs : POST_DNS_SETTLE_DELAY_MS; + const errors = []; + + if (settleDelayMs > 0) { + await sleep(settleDelayMs); + } + + for (const hostname of hostnames) { + try { + const result = await Promise.race([ + Promise.resolve().then(() => lookup(hostname)), + sleep(timeoutMs).then(() => { + throw new Error(`timeout after ${timeoutMs}ms`); + }), + ]); + const address = Array.isArray(result) ? result[0] && result[0].address : result && result.address; + if (address) { + return { + ok: true, + hostname, + address, + }; + } + errors.push(`${hostname}: no address returned`); + } catch (error) { + errors.push(`${hostname}: ${error.message || String(error)}`); + } + } + + return { + ok: false, + hostname: "", + address: "", + error: errors.join("; "), + }; +} + function buildLookupResult(address, options = {}) { if (options && options.all) { return [{ address, family: 4 }]; @@ -1147,9 +1215,7 @@ function buildStateSummary(installProbe, ipInfo) { connectMode = "wireguard"; recommendedAction = installProbe.tokenAvailable ? installProbe.wireguard.sudoReady - ? installProbe.wireguard.helperSecurity && !installProbe.wireguard.helperSecurity.hardened - ? `WireGuard tooling is available, but the helper at ${MAC_WG_HELPER_PATH} is not hardened yet: ${installProbe.wireguard.helperSecurity.reason}` - : "Use token-based WireGuard automation on macOS." + ? "Use token-based WireGuard automation on macOS." : `WireGuard tooling and token are available, but connect/disconnect require non-interactive sudo for ${MAC_WG_HELPER_PATH}. Allow that helper in sudoers, then rerun login/connect.` : `Set NORDVPN_TOKEN, NORDVPN_TOKEN_FILE, or place the token at ${DEFAULT_TOKEN_FILE} for automated macOS NordLynx/WireGuard connects.`; } else if (installProbe.platform === "darwin" && installProbe.appInstalled) { @@ -1955,6 +2021,27 @@ async function main() { } verified = liveness; await applyMacNordDns(); + const dnsResolution = await verifySystemHostnameResolution(); + if (!dnsResolution.ok) { + const rollback = await disconnectNordvpn(await probeInstallation(platform)); + lock.release(); + emitJson( + { + action, + requestedTarget: target, + connectResult, + persistence, + verified: false, + verification: verified.ipInfo, + dnsResolution, + rollback, + error: `Connected but system DNS resolution failed after DNS finalization: ${dnsResolution.error}`, + state: buildStateSummary(await probeInstallation(platform), await getPublicIpInfo()), + }, + 1, + true + ); + } writeJsonFile(LAST_CONNECTION_PATH, { backend: "wireguard", interfaceName: MAC_WG_INTERFACE, diff --git a/skills/nordvpn-client/scripts/nordvpn-client.test.js b/skills/nordvpn-client/scripts/nordvpn-client.test.js index 7fbcfd0..8b6c1f4 100644 --- a/skills/nordvpn-client/scripts/nordvpn-client.test.js +++ b/skills/nordvpn-client/scripts/nordvpn-client.test.js @@ -11,6 +11,10 @@ function loadInternals() { module.exports = { buildMacTailscaleState: typeof buildMacTailscaleState === "function" ? buildMacTailscaleState : undefined, + markMacTailscaleRecoverySuppressed: + typeof markMacTailscaleRecoverySuppressed === "function" ? markMacTailscaleRecoverySuppressed : undefined, + clearMacTailscaleRecoverySuppressed: + typeof clearMacTailscaleRecoverySuppressed === "function" ? clearMacTailscaleRecoverySuppressed : undefined, buildMacDnsState: typeof buildMacDnsState === "function" ? buildMacDnsState : undefined, buildWireguardConfig: @@ -57,6 +61,8 @@ module.exports = { typeof detectMacWireguardActiveFromIfconfig === "function" ? detectMacWireguardActiveFromIfconfig : undefined, resolveHostnameWithFallback: typeof resolveHostnameWithFallback === "function" ? resolveHostnameWithFallback : undefined, + verifySystemHostnameResolution: + typeof verifySystemHostnameResolution === "function" ? verifySystemHostnameResolution : undefined, verifyConnectionWithRetry: typeof verifyConnectionWithRetry === "function" ? verifyConnectionWithRetry : undefined, };`; @@ -203,6 +209,19 @@ test("buildMacTailscaleState records whether tailscale was active", () => { ); }); +test("tailscale recovery suppression marker can be created and cleared", () => { + const { markMacTailscaleRecoverySuppressed, clearMacTailscaleRecoverySuppressed } = loadInternals(); + assert.equal(typeof markMacTailscaleRecoverySuppressed, "function"); + assert.equal(typeof clearMacTailscaleRecoverySuppressed, "function"); + + const markerPath = path.join(process.env.HOME || "", ".nordvpn-client", "tailscale-suppressed"); + clearMacTailscaleRecoverySuppressed(); + markMacTailscaleRecoverySuppressed(); + assert.equal(fs.existsSync(markerPath), true); + clearMacTailscaleRecoverySuppressed(); + assert.equal(fs.existsSync(markerPath), false); +}); + test("shouldResumeMacTailscale only resumes when previously active and not already running", () => { const { shouldResumeMacTailscale } = loadInternals(); assert.equal(typeof shouldResumeMacTailscale, "function"); @@ -542,6 +561,10 @@ test("sanitizeOutputPayload redacts local path metadata from normal JSON output" wireguard: { configPath: "/Users/stefano/.nordvpn-client/wireguard/nordvpnctl.conf", helperPath: "/Users/stefano/.openclaw/workspace/skills/nordvpn-client/scripts/nordvpn-wireguard-helper.sh", + helperSecurity: { + hardened: false, + reason: "Helper must be root-owned before privileged actions are trusted.", + }, authCache: { tokenSource: "default:/Users/stefano/.openclaw/workspace/.clawdbot/credentials/nordvpn/token.txt", }, @@ -553,6 +576,7 @@ test("sanitizeOutputPayload redacts local path metadata from normal JSON output" assert.equal(sanitized.appPath, null); assert.equal(sanitized.wireguard.configPath, null); assert.equal(sanitized.wireguard.helperPath, null); + assert.equal(sanitized.wireguard.helperSecurity, null); assert.equal(sanitized.wireguard.authCache.tokenSource, null); assert.equal(sanitized.wireguard.endpoint, "jp454.nordvpn.com:51820"); }); @@ -606,3 +630,42 @@ test("resolveHostnameWithFallback uses fallback resolvers when system lookup fai assert.equal(address, "104.26.9.44"); assert.deepEqual(calls, ["1.1.1.1:ipapi.co", "8.8.8.8:ipapi.co"]); }); + +test("verifySystemHostnameResolution succeeds when any system lookup resolves", async () => { + const { verifySystemHostnameResolution } = loadInternals(); + assert.equal(typeof verifySystemHostnameResolution, "function"); + + const calls = []; + const result = await verifySystemHostnameResolution(["www.google.com", "api.openai.com"], { + timeoutMs: 5, + lookup: async (hostname) => { + calls.push(hostname); + if (hostname === "www.google.com") { + throw new Error("ENOTFOUND"); + } + return { address: "104.18.33.45", family: 4 }; + }, + }); + + assert.equal(result.ok, true); + assert.equal(result.hostname, "api.openai.com"); + assert.equal(result.address, "104.18.33.45"); + assert.deepEqual(calls, ["www.google.com", "api.openai.com"]); +}); + +test("verifySystemHostnameResolution fails when all hostnames fail system lookup", async () => { + const { verifySystemHostnameResolution } = loadInternals(); + assert.equal(typeof verifySystemHostnameResolution, "function"); + + const result = await verifySystemHostnameResolution(["www.google.com", "api.openai.com"], { + timeoutMs: 5, + lookup: async (hostname) => { + throw new Error(`${hostname}: timeout`); + }, + }); + + assert.equal(result.ok, false); + assert.equal(result.hostname, ""); + assert.match(result.error, /www\.google\.com/); + assert.match(result.error, /api\.openai\.com/); +});