feat(nordvpn-client): implement milestone M1 diagnostics and classification

skills/nordvpn-client/scripts/nordvpn-client.test.js

@@ -11,22 +11,38 @@ function loadInternals() {
 module.exports = {
   buildMacTailscaleState:
     typeof buildMacTailscaleState === "function" ? buildMacTailscaleState : undefined,
+  buildMacDnsState:
+    typeof buildMacDnsState === "function" ? buildMacDnsState : undefined,
   buildWireguardConfig:
     typeof buildWireguardConfig === "function" ? buildWireguardConfig : undefined,
   buildLookupResult:
     typeof buildLookupResult === "function" ? buildLookupResult : undefined,
   cleanupMacWireguardState:
     typeof cleanupMacWireguardState === "function" ? cleanupMacWireguardState : undefined,
+  cleanupMacWireguardAndDnsState:
+    typeof cleanupMacWireguardAndDnsState === "function" ? cleanupMacWireguardAndDnsState : undefined,
+  collectMacWireguardDiagnostics:
+    typeof collectMacWireguardDiagnostics === "function" ? collectMacWireguardDiagnostics : undefined,
+  inspectMacWireguardHelperSecurity:
+    typeof inspectMacWireguardHelperSecurity === "function" ? inspectMacWireguardHelperSecurity : undefined,
   getMacTailscalePath:
     typeof getMacTailscalePath === "function" ? getMacTailscalePath : undefined,
+  isBenignMacWireguardAbsentError:
+    typeof isBenignMacWireguardAbsentError === "function" ? isBenignMacWireguardAbsentError : undefined,
   isMacTailscaleActive:
     typeof isMacTailscaleActive === "function" ? isMacTailscaleActive : undefined,
   normalizeSuccessfulConnectState:
     typeof normalizeSuccessfulConnectState === "function" ? normalizeSuccessfulConnectState : undefined,
   normalizeStatusState:
     typeof normalizeStatusState === "function" ? normalizeStatusState : undefined,
+  shouldManageMacDnsService:
+    typeof shouldManageMacDnsService === "function" ? shouldManageMacDnsService : undefined,
   sanitizeOutputPayload:
     typeof sanitizeOutputPayload === "function" ? sanitizeOutputPayload : undefined,
+  shouldFinalizeMacWireguardConnect:
+    typeof shouldFinalizeMacWireguardConnect === "function" ? shouldFinalizeMacWireguardConnect : undefined,
+  shouldResumeMacTailscale:
+    typeof shouldResumeMacTailscale === "function" ? shouldResumeMacTailscale : undefined,
   shouldAttemptMacWireguardDisconnect:
     typeof shouldAttemptMacWireguardDisconnect === "function" ? shouldAttemptMacWireguardDisconnect : undefined,
   detectMacWireguardActiveFromIfconfig:
@@ -78,7 +94,7 @@ test("buildLookupResult supports lookup all=true mode", () => {
   assert.equal(JSON.stringify(buildLookupResult("104.26.9.44", { all: false })), JSON.stringify(["104.26.9.44", 4]));
 });
 
-test("buildWireguardConfig includes NordVPN DNS for the vanilla macOS config path", () => {
+test("buildWireguardConfig omits DNS and relies on post-connect networksetup on macOS", () => {
   const { buildWireguardConfig } = loadInternals();
   assert.equal(typeof buildWireguardConfig, "function");
 
@@ -91,10 +107,41 @@ test("buildWireguardConfig includes NordVPN DNS for the vanilla macOS config pat
     "PRIVATEKEY"
   );
 
-  assert.equal(config.includes("DNS = 103.86.96.100, 103.86.99.100"), true);
+  assert.equal(config.includes("DNS = 103.86.96.100, 103.86.99.100"), false);
   assert.equal(config.includes("AllowedIPs = 0.0.0.0/0"), true);
 });
 
+test("shouldManageMacDnsService keeps active physical services and excludes virtual ones", () => {
+  const { shouldManageMacDnsService } = loadInternals();
+  assert.equal(typeof shouldManageMacDnsService, "function");
+
+  assert.equal(shouldManageMacDnsService("Wi-Fi"), true);
+  assert.equal(shouldManageMacDnsService("USB 10/100/1000 LAN"), true);
+  assert.equal(shouldManageMacDnsService("Tailscale"), false);
+  assert.equal(shouldManageMacDnsService("Thunderbolt Bridge"), false);
+  assert.equal(shouldManageMacDnsService("Acme VPN"), false);
+});
+
+test("buildMacDnsState records DNS and search domains per service", () => {
+  const { buildMacDnsState } = loadInternals();
+  assert.equal(typeof buildMacDnsState, "function");
+
+  assert.equal(
+    JSON.stringify(
+      buildMacDnsState([
+        { name: "Wi-Fi", dnsServers: ["1.1.1.1"], searchDomains: [] },
+        { name: "USB 10/100/1000 LAN", dnsServers: [], searchDomains: ["lan"] },
+      ])
+    ),
+    JSON.stringify({
+      services: [
+        { name: "Wi-Fi", dnsServers: ["1.1.1.1"], searchDomains: [] },
+        { name: "USB 10/100/1000 LAN", dnsServers: [], searchDomains: ["lan"] },
+      ],
+    })
+  );
+});
+
 test("getMacTailscalePath falls back to /opt/homebrew/bin/tailscale when PATH lookup is missing", () => {
   const { getMacTailscalePath } = loadInternals();
   assert.equal(typeof getMacTailscalePath, "function");
@@ -118,6 +165,16 @@ test("buildMacTailscaleState records whether tailscale was active", () => {
   );
 });
 
+test("shouldResumeMacTailscale only resumes when previously active and not already running", () => {
+  const { shouldResumeMacTailscale } = loadInternals();
+  assert.equal(typeof shouldResumeMacTailscale, "function");
+
+  assert.equal(shouldResumeMacTailscale({ tailscaleWasActive: true }, false), true);
+  assert.equal(shouldResumeMacTailscale({ tailscaleWasActive: true }, true), false);
+  assert.equal(shouldResumeMacTailscale({ tailscaleWasActive: false }, false), false);
+  assert.equal(shouldResumeMacTailscale(null, false), false);
+});
+
 test("cleanupMacWireguardState removes stale config and last-connection files", () => {
   const { cleanupMacWireguardState } = loadInternals();
   assert.equal(typeof cleanupMacWireguardState, "function");
@@ -138,6 +195,104 @@ test("cleanupMacWireguardState removes stale config and last-connection files",
   assert.equal(fs.existsSync(lastConnectionPath), false);
 });
 
+test("cleanupMacWireguardAndDnsState removes stale config, DNS snapshot, and last-connection files", () => {
+  const { cleanupMacWireguardAndDnsState } = loadInternals();
+  assert.equal(typeof cleanupMacWireguardAndDnsState, "function");
+
+  const tmpDir = fs.mkdtempSync(path.join(fs.mkdtempSync("/tmp/nordvpn-client-test-"), "state-"));
+  const configPath = path.join(tmpDir, "nordvpnctl.conf");
+  const lastConnectionPath = path.join(tmpDir, "last-connection.json");
+  const dnsStatePath = path.join(tmpDir, "dns.json");
+  fs.writeFileSync(configPath, "wireguard-config");
+  fs.writeFileSync(lastConnectionPath, "{\"country\":\"Germany\"}");
+  fs.writeFileSync(dnsStatePath, "{\"services\":[]}");
+
+  const result = cleanupMacWireguardAndDnsState({
+    configPath,
+    lastConnectionPath,
+    dnsStatePath,
+  });
+
+  assert.equal(result.cleaned, true);
+  assert.equal(fs.existsSync(configPath), false);
+  assert.equal(fs.existsSync(lastConnectionPath), false);
+  assert.equal(fs.existsSync(dnsStatePath), false);
+});
+
+test("inspectMacWireguardHelperSecurity rejects a user-owned helper path", () => {
+  const { inspectMacWireguardHelperSecurity } = loadInternals();
+  assert.equal(typeof inspectMacWireguardHelperSecurity, "function");
+
+  const result = inspectMacWireguardHelperSecurity("/tmp/nordvpn-wireguard-helper.sh", {
+    fileExists: () => true,
+    statSync: () => ({
+      uid: 501,
+      gid: 20,
+      mode: 0o100755,
+    }),
+  });
+
+  assert.equal(result.exists, true);
+  assert.equal(result.hardened, false);
+  assert.match(result.reason, /root-owned/i);
+});
+
+test("inspectMacWireguardHelperSecurity accepts a root-owned non-writable helper path", () => {
+  const { inspectMacWireguardHelperSecurity } = loadInternals();
+  assert.equal(typeof inspectMacWireguardHelperSecurity, "function");
+
+  const result = inspectMacWireguardHelperSecurity("/tmp/nordvpn-wireguard-helper.sh", {
+    fileExists: () => true,
+    statSync: () => ({
+      uid: 0,
+      gid: 0,
+      mode: 0o100755,
+    }),
+  });
+
+  assert.equal(result.exists, true);
+  assert.equal(result.hardened, true);
+  assert.equal(result.reason, "");
+});
+
+test("collectMacWireguardDiagnostics captures bounded wg/ifconfig/route/process output", async () => {
+  const { collectMacWireguardDiagnostics } = loadInternals();
+  assert.equal(typeof collectMacWireguardDiagnostics, "function");
+
+  const seen = [];
+  const result = await collectMacWireguardDiagnostics({
+    interfaceName: "nordvpnctl",
+    runExec: async (command, args) => {
+      seen.push(`${command} ${args.join(" ")}`);
+      if (command === "/opt/homebrew/bin/wg") {
+        return { ok: true, stdout: "interface: nordvpnctl\npeer: abc123", stderr: "", error: "" };
+      }
+      if (command === "ifconfig") {
+        return { ok: true, stdout: "utun8: flags=8051\n\tinet 10.5.0.2 --> 10.5.0.2", stderr: "", error: "" };
+      }
+      if (command === "route") {
+        return { ok: true, stdout: "default 10.5.0.2 UGSc", stderr: "", error: "" };
+      }
+      if (command === "pgrep") {
+        return { ok: true, stdout: "1234 wireguard-go utun\n5678 wg-quick up nordvpnctl", stderr: "", error: "" };
+      }
+      throw new Error(`unexpected command: ${command}`);
+    },
+  });
+
+  assert.deepEqual(seen, [
+    "/opt/homebrew/bin/wg show nordvpnctl",
+    "ifconfig nordvpnctl",
+    "route -n get default",
+    "pgrep -fl wireguard-go|wg-quick|nordvpnctl",
+  ]);
+  assert.equal(result.interfaceName, "nordvpnctl");
+  assert.equal(result.wgShow.includes("peer: abc123"), true);
+  assert.equal(result.ifconfig.includes("10.5.0.2"), true);
+  assert.equal(result.routes.includes("default 10.5.0.2"), true);
+  assert.equal(result.processes.includes("wireguard-go"), true);
+});
+
 test("shouldAttemptMacWireguardDisconnect does not trust active=false when residual state exists", () => {
   const { shouldAttemptMacWireguardDisconnect } = loadInternals();
   assert.equal(typeof shouldAttemptMacWireguardDisconnect, "function");
@@ -173,6 +328,15 @@ test("shouldAttemptMacWireguardDisconnect does not trust active=false when resid
   );
 });
 
+test("isBenignMacWireguardAbsentError recognizes stale-interface teardown errors", () => {
+  const { isBenignMacWireguardAbsentError } = loadInternals();
+  assert.equal(typeof isBenignMacWireguardAbsentError, "function");
+
+  assert.equal(isBenignMacWireguardAbsentError("wg-quick: `nordvpnctl' is not a WireGuard interface"), true);
+  assert.equal(isBenignMacWireguardAbsentError("Unable to access interface: No such file or directory"), true);
+  assert.equal(isBenignMacWireguardAbsentError("permission denied"), false);
+});
+
 test("normalizeSuccessfulConnectState marks the connect snapshot active after verified macOS wireguard connect", () => {
   const { normalizeSuccessfulConnectState } = loadInternals();
   assert.equal(typeof normalizeSuccessfulConnectState, "function");
@@ -206,6 +370,16 @@ test("normalizeSuccessfulConnectState marks the connect snapshot active after ve
   assert.equal(state.wireguard.endpoint, "de1227.nordvpn.com:51820");
 });
 
+test("shouldFinalizeMacWireguardConnect requires a verified wireguard connect", () => {
+  const { shouldFinalizeMacWireguardConnect } = loadInternals();
+  assert.equal(typeof shouldFinalizeMacWireguardConnect, "function");
+
+  assert.equal(shouldFinalizeMacWireguardConnect({ backend: "wireguard" }, { ok: true }), true);
+  assert.equal(shouldFinalizeMacWireguardConnect({ backend: "wireguard" }, { ok: false }), false);
+  assert.equal(shouldFinalizeMacWireguardConnect({ backend: "cli" }, { ok: true }), false);
+  assert.equal(shouldFinalizeMacWireguardConnect(null, { ok: true }), false);
+});
+
 test("normalizeStatusState marks macOS wireguard connected when public IP matches the last successful target", () => {
   const { normalizeStatusState } = loadInternals();
   assert.equal(typeof normalizeStatusState, "function");