tailscalectl/TailscaleWidget.qml

@@ -16,6 +16,7 @@ PluginComponent {
     property var peers: []
     property string _copyText: ""
     property int _copyIndex: 0
+    property bool _pendingToggle: false  // transient one-shot for post-action verification (to be removed in first-principles refactor)
 
     layerNamespacePlugin: "tailscalectl"
     popoutWidth: 360
@@ -33,11 +34,11 @@ PluginComponent {
 
         onExited: (code, status) => {
             if (code !== 0) {
-                var action = root.isConnected ? "disconnect" : "connect";
-                var detail = toggleProcess.stderr.text.trim();
-                ToastService.showError("tailscalectl", TailscaleLib.formatError(action, detail));
+                var action = root.isConnected ? "disconnect" : "connect"
+                var detail = toggleProcess.stderr.text.trim()
+                ToastService.showError("tailscalectl", TailscaleLib.formatError(action, detail))
             }
-            root.refreshStatus();
+            statusCheck.running = true
         }
     }
 
@@ -65,10 +66,10 @@ PluginComponent {
 
         onExited: (code, status) => {
             if (code !== 0) {
-                var detail = exitNodeProcess.stderr.text.trim();
-                ToastService.showError("tailscalectl", TailscaleLib.formatError("set", detail));
+                var detail = exitNodeProcess.stderr.text.trim()
+                ToastService.showError("tailscalectl", TailscaleLib.formatError("set", detail))
             }
-            root.refreshStatus();
+            statusCheck.running = true
         }
     }
 
@@ -94,22 +95,19 @@ PluginComponent {
                 ToastService.showError("tailscalectl", TailscaleLib.formatError("status"))
             }
 
-            const cmd = TailscaleLib.commandForPendingAction(statusCheck._pendingAction, root.isConnected);
-            if (cmd) {
-                toggleProcess.command = cmd;
-                toggleProcess.running = true;
+            // Post-action verification: if a toggle was requested, use the fresh state we just received.
+            if (root._pendingToggle) {
+                root._pendingToggle = false
+                toggleProcess.command = TailscaleLib.buildToggleCommand(root.isConnected)
+                toggleProcess.running = true
             }
-            statusCheck._pendingAction = null;
         }
     }
 
     function toggleTailscale() {
-        statusCheck._pendingAction = "toggle";
-        statusCheck.running = true;
-    }
-
-    function refreshStatus() {
-        statusCheck.running = true;
+        // Post-action verification: force a fresh status check, then act on the real ground truth.
+        root._pendingToggle = true
+        statusCheck.running = true
     }
 
     function setExitNode(hostname) {

tailscalectl/lib.js

@@ -1,15 +1,15 @@
 function parsePeers(peerMap) {
-    if (!peerMap) { return []; }
+    if (!peerMap) return []
     return Object.keys(peerMap).map(function (key) {
-        if (!Object.prototype.hasOwnProperty.call(peerMap, key)) { return null; }
-        var p = peerMap[key];
+        if (!Object.prototype.hasOwnProperty.call(peerMap, key)) return null
+        var p = peerMap[key]
         return {
             hostname: p.HostName || key,
             ip: (p.TailscaleIPs && p.TailscaleIPs.length) ? p.TailscaleIPs[0] : "",
             online: p.Online || false,
             exitNode: p.ExitNodeOption || false
-        };
-    }).filter(function (peer) { return peer !== null; });
+        }
+    }).filter(function (peer) { return peer !== null })
 }
 
 function makeExitNodeCommand(hostname) {
@@ -23,15 +23,15 @@ function makeExitNodeCommand(hostname) {
 }
 
 function findActiveExitNode(peerMap) {
-    if (!peerMap) { return ""; }
+    if (!peerMap) return ""
     for (const key in peerMap) {
-        if (!Object.prototype.hasOwnProperty.call(peerMap, key)) { continue; }
-        const p = peerMap[key];
+        if (!Object.prototype.hasOwnProperty.call(peerMap, key)) continue
+        const p = peerMap[key]
         if (p.ExitNode) {
-            return p.HostName || key;
+            return p.HostName || key
         }
     }
-    return "";
+    return ""
 }
 
 const clipboardTools = [
@@ -52,51 +52,53 @@ function getStrings() {
         disconnected: "Disconnected",
         exitNodePrefix: "Exit node: ",
         none: "None",
-        copied: function (text) { return "Copied " + text + " to clipboard"; },
+        copied: function (text) { return "Copied " + text + " to clipboard" },
         clearExitNode: "×",
         setExitNode: "↗",
         invalidExitNodeHostname: "Invalid exit node hostname"
-    };
+    }
 }
 
 // Light UI predicates — keep the view thin.
 function shouldShowClearExitNode(currentExitNode) {
-    return currentExitNode !== "";
+    return currentExitNode !== ""
 }
 
 function isActiveExitNode(currentExitNode, hostname) {
-    return currentExitNode === hostname;
+    return currentExitNode === hostname
 }
 
 // Security: validate hostnames coming from tailscale status JSON.
 // Fail closed on obviously malicious input.
 function isValidExitNodeHostname(hostname) {
-    if (typeof hostname !== "string") { return false; }
-    if (hostname === "") { return true; }
+    if (typeof hostname !== "string") return false;
+    if (hostname === "") return true; // clear command
+    // Permissive enough for real Tailscale hostnames while rejecting
+    // classic shell metacharacters and control characters.
     return /^[a-zA-Z0-9]([a-zA-Z0-9-_.]{0,62}[a-zA-Z0-9])?$/.test(hostname);
 }
 
 function parseStatusResult(jsonText) {
     try {
-        const data = JSON.parse(jsonText);
+        const data = JSON.parse(jsonText)
         return {
             isConnected: data.BackendState === "Running",
             tailscaleIP: (data.Self && data.Self.TailscaleIPs && data.Self.TailscaleIPs[0]) || "",
             currentExitNode: findActiveExitNode(data.Peer || {}),
             peers: parsePeers(data.Peer || {})
-        };
+        }
     } catch (e) {
-        return { isConnected: false, tailscaleIP: "", currentExitNode: "", peers: [] };
+        return { isConnected: false, tailscaleIP: "", currentExitNode: "", peers: [] }
     }
 }
 
 function buildToggleCommand(isConnected) {
-    return isConnected ? ["tailscale", "down"] : ["tailscale", "up"];
+    return isConnected ? ["tailscale", "down"] : ["tailscale", "up"]
 }
 
 // Single source of truth for the status command used for on-demand and post-action verification.
 function getStatusCommand() {
-    return ["tailscale", "status", "--json"];
+    return ["tailscale", "status", "--json"]
 }
 
 function errorMessage(cmd) {
@@ -123,17 +125,7 @@ function formatError(action, detail) {
     return base;
 }
 
-const PendingAction = Object.freeze({
-    TOGGLE: "toggle"
-});
-
-function commandForPendingAction(pending, freshIsConnected) {
-    if (pending === PendingAction.TOGGLE) {
-        return buildToggleCommand(freshIsConnected);
-    }
-    return null;
-}
-
+// CommonJS export for Node.js tests (ignored by QML)
 if (typeof module !== "undefined" && module.exports) {
-    module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, getClipboardCommands, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode, PendingAction, commandForPendingAction };
+    module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, getClipboardCommands, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode }
 }

test/lib.test.js

@@ -1,7 +1,7 @@
-import { test } from "node:test";
-import assert from "node:assert";
-import lib from "../tailscalectl/lib.js";
-const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, getClipboardCommands, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode, PendingAction, commandForPendingAction } = lib;
+import { test } from "node:test"
+import assert from "node:assert"
+import lib from "../tailscalectl/lib.js"
+const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, getClipboardCommands, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } = lib
 
 /*
  * Unit tests for the pure functions exported from lib.js.
@@ -27,87 +27,91 @@ test("parsePeers extracts exitNode from ExitNodeOption", () => {
             Online: true,
             ExitNodeOption: false
         }
-    };
+    }
 
-    const peers = parsePeers(peerMap);
+    const peers = parsePeers(peerMap)
 
-    assert.strictEqual(peers[0].exitNode, true);
-    assert.strictEqual(peers[1].exitNode, false);
-});
+    assert.strictEqual(peers[0].exitNode, true)
+    assert.strictEqual(peers[1].exitNode, false)
+})
 
 test("makeExitNodeCommand returns tailscale set command for hostname", () => {
-    const cmd = makeExitNodeCommand("router");
-    assert.deepStrictEqual(cmd, ["tailscale", "set", "--exit-node=router"]);
-});
+    const cmd = makeExitNodeCommand("router")
+    assert.deepStrictEqual(cmd, ["tailscale", "set", "--exit-node=router"])
+})
 
 test("makeExitNodeCommand with empty string clears exit node", () => {
-    const cmd = makeExitNodeCommand("");
-    assert.deepStrictEqual(cmd, ["tailscale", "set", "--exit-node="]);
-});
+    const cmd = makeExitNodeCommand("")
+    assert.deepStrictEqual(cmd, ["tailscale", "set", "--exit-node="])
+})
 
 test("findActiveExitNode returns hostname of peer with ExitNode=true", () => {
     const peerMap = {
         "peer-1": { HostName: "gluetun-sjc", ExitNode: true, ExitNodeOption: true },
         "peer-2": { HostName: "gluetun-den", ExitNode: false, ExitNodeOption: true }
-    };
-    assert.strictEqual(findActiveExitNode(peerMap), "gluetun-sjc");
-});
+    }
+    assert.strictEqual(findActiveExitNode(peerMap), "gluetun-sjc")
+})
 
 test("findActiveExitNode returns empty string when no exit node", () => {
     const peerMap = {
         "peer-1": { HostName: "laptop", ExitNode: false, ExitNodeOption: false }
-    };
-    assert.strictEqual(findActiveExitNode(peerMap), "");
-});
+    }
+    assert.strictEqual(findActiveExitNode(peerMap), "")
+})
 
 test("errorMessage returns user-friendly message for tailscale up failure", () => {
-    const msg = errorMessage("up", 1);
-    assert.strictEqual(msg, "Failed to connect to Tailscale");
+    const msg = errorMessage("up", 1)
+    assert.strictEqual(msg, "Failed to connect to Tailscale")
 })
 
 test("errorMessage returns user-friendly message for tailscale down failure", () => {
-    const msg = errorMessage("down", 1);
-    assert.strictEqual(msg, "Failed to disconnect from Tailscale");
-});
+    const msg = errorMessage("down", 1)
+    assert.strictEqual(msg, "Failed to disconnect from Tailscale")
+})
 
 test("errorMessage returns user-friendly message for tailscale set failure", () => {
-    const msg = errorMessage("set", 1);
-    assert.strictEqual(msg, "Failed to set exit node");
-});
+    const msg = errorMessage("set", 1)
+    assert.strictEqual(msg, "Failed to set exit node")
+})
 
 test("errorMessage returns user-friendly message for tailscale status failure", () => {
-    const msg = errorMessage("status", 1);
-    assert.strictEqual(msg, "Failed to read Tailscale status");
-});
+    const msg = errorMessage("status", 1)
+    assert.strictEqual(msg, "Failed to read Tailscale status")
+})
 
 test("errorMessage returns generic message for unknown command", () => {
-    const msg = errorMessage("unknown", 1);
-    assert.strictEqual(msg, "Tailscale command failed");
-});
+    const msg = errorMessage("unknown", 1)
+    assert.strictEqual(msg, "Tailscale command failed")
+})
+
+// --- getClipboardCommands ---
 
 test("getClipboardCommands returns ordered argv arrays with text appended", () => {
-    const cmds = getClipboardCommands("1.2.3.4");
-    assert.ok(Array.isArray(cmds));
-    assert.strictEqual(cmds.length, 2);
-    assert.deepStrictEqual(cmds[0], ["dms", "cl", "copy", "1.2.3.4"]);
-    assert.deepStrictEqual(cmds[1], ["wl-copy", "1.2.3.4"]);
-});
+    const cmds = getClipboardCommands("1.2.3.4")
+    assert.ok(Array.isArray(cmds))
+    assert.strictEqual(cmds.length, 2)
+    assert.deepStrictEqual(cmds[0], ["dms", "cl", "copy", "1.2.3.4"])
+    assert.deepStrictEqual(cmds[1], ["wl-copy", "1.2.3.4"])
+})
 
 test("getClipboardCommands handles text with special characters safely (direct argv)", () => {
-    const cmds = getClipboardCommands("it's a 'test' with \"quotes\" and\nnewlines");
-    assert.ok(Array.isArray(cmds));
-    assert.strictEqual(cmds.length, 2);
-    assert.ok(cmds[0].includes("it's a 'test' with \"quotes\" and\nnewlines"));
-});
+    const cmds = getClipboardCommands("it's a 'test' with \"quotes\" and\nnewlines")
+    assert.ok(Array.isArray(cmds))
+    assert.strictEqual(cmds.length, 2)
+    assert.ok(cmds[0].includes("it's a 'test' with \"quotes\" and\nnewlines"))
+})
 
 test("getClipboardCommands is deterministic and open for future tools", () => {
-    const cmds = getClipboardCommands("foo");
-    assert.ok(Array.isArray(cmds[0]));
-    assert.ok(Array.isArray(cmds[1]));
-});
+    const cmds = getClipboardCommands("foo")
+    assert.ok(Array.isArray(cmds[0]))
+    assert.ok(Array.isArray(cmds[1]))
+})
+
+// --- getStrings ---
 
 test("getStrings returns canonical UI strings for the widget", () => {
-    const s = getStrings();
+    const s = getStrings()
     assert.ok(s.header)
     assert.ok(s.connected)
     assert.ok(s.disconnected)
@@ -147,17 +151,7 @@ test("buildToggleCommand treats null and undefined as disconnected", () => {
     assert.deepStrictEqual(buildToggleCommand(undefined), ["tailscale", "up"])
 })
 
-test("commandForPendingAction returns toggle command when pending is TOGGLE and passes through buildToggleCommand logic", () => {
-    assert.deepStrictEqual(commandForPendingAction(PendingAction.TOGGLE, true), ["tailscale", "down"]);
-    assert.deepStrictEqual(commandForPendingAction(PendingAction.TOGGLE, false), ["tailscale", "up"]);
-    assert.deepStrictEqual(commandForPendingAction(PendingAction.TOGGLE, null), ["tailscale", "up"]);
-});
-
-test("commandForPendingAction returns null for no pending action or unknown pending value", () => {
-    assert.strictEqual(commandForPendingAction(null, true), null);
-    assert.strictEqual(commandForPendingAction(undefined, false), null);
-    assert.strictEqual(commandForPendingAction("something-else", true), null);
-});
+// --- parseStatusResult ---
 
 test("parseStatusResult produces correct state from valid JSON", () => {
     const json = JSON.stringify({