tailscalectl/TailscaleWidget.qml

@@ -15,7 +15,7 @@ PluginComponent {
     property string currentExitNode: ""
     property var peers: []
     property string _copyText: ""
-    property int _copyIndex: 0
+    property string _copyCurrentTool: ""
     property bool _pendingToggle: false  // transient one-shot for post-action verification (to be removed in first-principles refactor)
 
     layerNamespacePlugin: "tailscalectl"
@@ -50,9 +50,13 @@ PluginComponent {
         onExited: (code, status) => {
             if (code === 0) {
                 ToastService.showInfo(TailscaleLib.getStrings().copied(root._copyText))
+            } else if (root._copyCurrentTool === "dms") {
+                // One simple fallback attempt: try wl-copy.
+                root._copyCurrentTool = "wl-copy"
+                root._executeCopy()
             } else {
-                root._copyIndex += 1
+                var detail = copyProcess.stderr.text.trim()
-                root._runNextCopy()
+                ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard", detail))
             }
         }
     }
@@ -122,17 +126,18 @@ PluginComponent {
 
     function copyToClipboard(text) {
         root._copyText = text
-        root._copyIndex = 0
+        // Simple two-tool fallback per first-principles plan: dms first, then wl-copy.
-        root._runNextCopy()
+        root._copyCurrentTool = "dms"
+        root._executeCopy()
     }
 
-    function _runNextCopy() {
+    function _executeCopy() {
-        const cmds = TailscaleLib.getClipboardCommands(root._copyText)
+        var cmd = TailscaleLib.buildCopyCommand(root._copyText, root._copyCurrentTool)
-        if (root._copyIndex >= cmds.length) {
+        if (!cmd) {
             ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard"))
             return
         }
-        copyProcess.command = cmds[root._copyIndex]
+        copyProcess.command = cmd
         copyProcess.running = true
     }
 

tailscalectl/lib.js

@@ -34,15 +34,34 @@ function findActiveExitNode(peerMap) {
     return ""
 }
 
-const clipboardTools = [
+var safeClipboardTools = ["dms", "wl-copy"]
-    { argv: ["dms", "cl", "copy"] },
-    { argv: ["wl-copy"] }
-];
 
-function getClipboardCommands(text) {
+    // Direct argv form — no sh -c, no escaping, no future injection surface.
-    return clipboardTools.map(function (tool) {
+    // clipmanctl intentionally dropped (niche optional history manager, not needed on DMS + Wayland).
-        return tool.argv.concat([text]);
+    var clipboardTools = {
-    });
+    "dms":     { argv: ["dms", "cl", "copy"] },
+    "wl-copy": { argv: ["wl-copy"] }
+}
+
+function validateClipboardTool(tool) {
+    return typeof tool === "string" && safeClipboardTools.includes(tool)
+}
+
+function buildCopyCommand(text, tool) {
+    const entry = clipboardTools[tool]
+    if (!entry) return null
+    // Pure argv — the Process will pass the string verbatim. No shell involved.
+    return [...entry.argv, text]
+}
+
+function nextClipboardTool(currentTool) {
+    var idx = safeClipboardTools.indexOf(currentTool)
+    if (idx < 0) return safeClipboardTools[0]
+    return safeClipboardTools[(idx + 1) % safeClipboardTools.length]
+}
+
+function allClipboardTools() {
+    return safeClipboardTools.slice()
 }
 
 function getStrings() {
@@ -53,14 +72,16 @@ function getStrings() {
         exitNodePrefix: "Exit node: ",
         none: "None",
         copied: function (text) { return "Copied " + text + " to clipboard" },
+        noClipboardTool: "No clipboard tool found",
+        invalidClipboardCommand: "Invalid clipboard command",
         clearExitNode: "×",
         setExitNode: "↗",
         invalidExitNodeHostname: "Invalid exit node hostname"
     }
 }
 
-// Light UI predicates — keep the view thin.
+    // Light UI predicates — keep the view thin.
-function shouldShowClearExitNode(currentExitNode) {
+    function shouldShowClearExitNode(currentExitNode) {
     return currentExitNode !== ""
 }
 
@@ -68,8 +89,9 @@ function isActiveExitNode(currentExitNode, hostname) {
     return currentExitNode === hostname
 }
 
-// Security: validate hostnames coming from tailscale status JSON.
+// Security: validate hostnames coming from tailscale status JSON before we
-// Fail closed on obviously malicious input.
+// concatenate them into a command argument. Direct argv is already used
+// (see #49), but we still fail closed on obviously malicious input.
 function isValidExitNodeHostname(hostname) {
     if (typeof hostname !== "string") return false;
     if (hostname === "") return true; // clear command
@@ -127,5 +149,5 @@ function formatError(action, detail) {
 
 // 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 }
+    module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode }
 }

test/lib.test.js

@@ -1,16 +1,19 @@
 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
+const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } = lib
 
 /*
- * Unit tests for the pure functions exported from lib.js.
+ * Test coverage note for #48:
+ * All pure functions exported from lib.js (including the newly extracted
+ * getStrings, shouldShowClearExitNode, isActiveExitNode, and the clipboard
+ * helpers) are exercised here via node:test.
  *
- * All functions in lib.js are exercised via Node's built-in test runner.
+ * The four Process objects, 5-second Timer, onExited handlers, copy fallback
- *
+ * state machine, and overall widget behavior in TailscaleWidget.qml have no
- * TailscaleWidget.qml has no automated test coverage. The four Process
+ * automated test coverage. There is no QML test runner in this project.
- * objects, their onExited handlers, and all widget UI behavior must be
+ * Those paths are verified manually (see verification checklist in the
- * verified manually.
+ * feature branch commits).
  */
 
 test("parsePeers extracts exitNode from ExitNodeOption", () => {
@@ -85,30 +88,70 @@ test("errorMessage returns generic message for unknown command", () => {
     assert.strictEqual(msg, "Tailscale command failed")
 })
 
-// --- getClipboardCommands ---
+// --- validateClipboardTool ---
 
-test("getClipboardCommands returns ordered argv arrays with text appended", () => {
+test("validateClipboardTool accepts whitelisted tool names", () => {
-    const cmds = getClipboardCommands("1.2.3.4")
+    assert.strictEqual(validateClipboardTool("dms"), true)
-    assert.ok(Array.isArray(cmds))
+    assert.strictEqual(validateClipboardTool("wl-copy"), true)
-    assert.strictEqual(cmds.length, 2)
+    // clipmanctl intentionally dropped (see #49)
-    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)", () => {
+test("validateClipboardTool rejects malicious inputs", () => {
-    const cmds = getClipboardCommands("it's a 'test' with \"quotes\" and\nnewlines")
+    assert.strictEqual(validateClipboardTool("rm -rf /"), false)
-    assert.ok(Array.isArray(cmds))
+    assert.strictEqual(validateClipboardTool("echo hi; malicious"), false)
-    assert.strictEqual(cmds.length, 2)
+    assert.strictEqual(validateClipboardTool("$(whoami)"), false)
-    assert.ok(cmds[0].includes("it's a 'test' with \"quotes\" and\nnewlines"))
+    assert.strictEqual(validateClipboardTool("wl-copy || rm -rf /"), false)
 })
 
-test("getClipboardCommands is deterministic and open for future tools", () => {
+test("validateClipboardTool rejects empty and falsy inputs", () => {
-    const cmds = getClipboardCommands("foo")
+    assert.strictEqual(validateClipboardTool(""), false)
-    assert.ok(Array.isArray(cmds[0]))
+    assert.strictEqual(validateClipboardTool(null), false)
-    assert.ok(Array.isArray(cmds[1]))
+    assert.strictEqual(validateClipboardTool(undefined), false)
 })
 
-// --- getStrings ---
+// --- buildCopyCommand ---
+
+test("buildCopyCommand returns safe command for whitelisted clipboard tool", () => {
+    const cmd = buildCopyCommand("hello", "wl-copy")
+    assert.ok(Array.isArray(cmd))
+    assert.deepStrictEqual(cmd, ["wl-copy", "hello"])  // direct argv, no sh -c (see #49)
+})
+
+test("buildCopyCommand returns null for invalid clipboard tool", () => {
+    assert.strictEqual(buildCopyCommand("hello", "rm -rf /"), null)
+    assert.strictEqual(buildCopyCommand("hello", ""), null)
+    assert.strictEqual(buildCopyCommand("hello", null), null)
+})
+
+test("buildCopyCommand safely handles text with special characters", () => {
+    const cmd = buildCopyCommand("it's a 'test' with \"quotes\" and\nnewlines", "wl-copy")
+    assert.ok(Array.isArray(cmd))
+})
+
+// --- nextClipboardTool ---
+
+test("nextClipboardTool cycles through tools", () => {
+    assert.strictEqual(nextClipboardTool("dms"), "wl-copy")
+    assert.strictEqual(nextClipboardTool("wl-copy"), "dms")  // clipmanctl dropped (#49)
+})
+
+test("nextClipboardTool falls back to first tool for unknown input", () => {
+    assert.strictEqual(nextClipboardTool(""), "dms")
+    assert.strictEqual(nextClipboardTool("unknown"), "dms")
+})
+
+// --- allClipboardTools ---
+
+test("allClipboardTools returns the list of safe tools", () => {
+    const tools = allClipboardTools()
+    assert.ok(Array.isArray(tools))
+    assert.strictEqual(tools.length, 2)
+    assert.ok(tools.includes("dms"))
+    assert.ok(tools.includes("wl-copy"))
+    // clipmanctl intentionally removed (#49)
+})
+
+// --- getStrings (for #34 i18n prep) ---
 
 test("getStrings returns canonical UI strings for the widget", () => {
     const s = getStrings()
@@ -117,6 +160,8 @@ test("getStrings returns canonical UI strings for the widget", () => {
     assert.ok(s.disconnected)
     assert.ok(s.exitNodePrefix)
     assert.ok(s.copied)
+    assert.ok(s.noClipboardTool)
+    assert.ok(s.invalidClipboardCommand)
 })
 
 test("getStrings.copied interpolates the text", () => {
@@ -196,7 +241,7 @@ test("parseStatusResult sets isConnected false for non-Running BackendState", ()
     assert.strictEqual(state.isConnected, false)
 })
 
-// --- formatError (central error + detail formatting) ---
+// --- formatError (central error + detail formatting per first-principles plan) ---
 
 test("formatError returns base message without detail", () => {
     assert.strictEqual(formatError("status"), "Failed to read Tailscale status")
@@ -216,7 +261,7 @@ test("formatError handles empty or falsy detail gracefully", () => {
     assert.strictEqual(formatError("connect", null), "Failed to connect to Tailscale")
 })
 
-// --- isValidExitNodeHostname + makeExitNodeCommand safety ---
+// --- isValidExitNodeHostname + makeExitNodeCommand safety (balanced paranoia, direct-argv defense-in-depth) ---
 
 test("isValidExitNodeHostname accepts empty string (clear)", () => {
     assert.strictEqual(isValidExitNodeHostname(""), true)
@@ -244,7 +289,7 @@ test("makeExitNodeCommand still produces correct argv for valid input", () => {
     assert.deepStrictEqual(makeExitNodeCommand("gluetun-sjc"), ["tailscale", "set", "--exit-node=gluetun-sjc"])
 })
 
-// --- getStatusCommand ---
+// --- getStatusCommand (new pure helper for low-duty-cycle architecture) ---
 
 test("getStatusCommand returns the canonical tailscale status --json argv", () => {
     const cmd = getStatusCommand()