From c1db54ab5c9fd8f51d71ef767d612bd262de6f45 Mon Sep 17 00:00:00 2001 From: vybe Date: Fri, 22 May 2026 21:35:07 +0000 Subject: [PATCH 1/3] refactor(clipboard): replace object map + 4 helpers with simple array + getClipboardCommands; QML becomes dumb index driver - lib.js: single clipboardTools array + getClipboardCommands(text) returning argv lists - Delete validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, safeClipboardTools, old clipboardTools object - Remove noClipboardTool / invalidClipboardCommand from getStrings (dead) - TailscaleWidget.qml: remove _copyCurrentTool + _executeCopy; add _copyIndex + _runNextCopy - copyProcess.onExited and copyToClipboard now purely index-driven, no tool names - Update tests: remove 4 old clipboard test blocks + dead string assertions; add focused getClipboardCommands tests - All 34 tests pass This is the minimal honest design given Process is a QML type. --- tailscalectl/TailscaleWidget.qml | 23 ++++----- tailscalectl/lib.js | 44 +++++------------- test/lib.test.js | 80 ++++++++------------------------ 3 files changed, 39 insertions(+), 108 deletions(-) diff --git a/tailscalectl/TailscaleWidget.qml b/tailscalectl/TailscaleWidget.qml index eb052ba..1a25c30 100644 --- a/tailscalectl/TailscaleWidget.qml +++ b/tailscalectl/TailscaleWidget.qml @@ -15,7 +15,7 @@ PluginComponent { property string currentExitNode: "" property var peers: [] property string _copyText: "" - property string _copyCurrentTool: "" + property int _copyIndex: 0 property bool _pendingToggle: false // transient one-shot for post-action verification (to be removed in first-principles refactor) layerNamespacePlugin: "tailscalectl" @@ -50,13 +50,9 @@ 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 { - var detail = copyProcess.stderr.text.trim() - ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard", detail)) + root._copyIndex += 1 + root._runNextCopy() } } } @@ -126,18 +122,17 @@ PluginComponent { function copyToClipboard(text) { root._copyText = text - // Simple two-tool fallback per first-principles plan: dms first, then wl-copy. - root._copyCurrentTool = "dms" - root._executeCopy() + root._copyIndex = 0 + root._runNextCopy() } - function _executeCopy() { - var cmd = TailscaleLib.buildCopyCommand(root._copyText, root._copyCurrentTool) - if (!cmd) { + function _runNextCopy() { + const cmds = TailscaleLib.getClipboardCommands(root._copyText) + if (root._copyIndex >= cmds.length) { ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard")) return } - copyProcess.command = cmd + copyProcess.command = cmds[root._copyIndex] copyProcess.running = true } diff --git a/tailscalectl/lib.js b/tailscalectl/lib.js index 7e6f0ff..6d28fe8 100644 --- a/tailscalectl/lib.js +++ b/tailscalectl/lib.js @@ -34,34 +34,15 @@ function findActiveExitNode(peerMap) { return "" } -var safeClipboardTools = ["dms", "wl-copy"] +const clipboardTools = [ + { argv: ["dms", "cl", "copy"] }, + { argv: ["wl-copy"] } +]; - // Direct argv form — no sh -c, no escaping, no future injection surface. - // clipmanctl intentionally dropped (niche optional history manager, not needed on DMS + Wayland). - 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 getClipboardCommands(text) { + return clipboardTools.map(function (tool) { + return tool.argv.concat([text]); + }); } function getStrings() { @@ -72,8 +53,6 @@ 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" @@ -89,9 +68,8 @@ function isActiveExitNode(currentExitNode, hostname) { return currentExitNode === hostname } -// Security: validate hostnames coming from tailscale status JSON before we -// concatenate them into a command argument. Direct argv is already used -// (see #49), but we still fail closed on obviously malicious input. +// 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; // clear command @@ -149,5 +127,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, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } + module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, getClipboardCommands, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } } diff --git a/test/lib.test.js b/test/lib.test.js index 8a5ff9f..ecddc98 100644 --- a/test/lib.test.js +++ b/test/lib.test.js @@ -1,13 +1,13 @@ import { test } from "node:test" import assert from "node:assert" import lib from "../tailscalectl/lib.js" -const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } = lib +const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, getClipboardCommands, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } = lib /* * 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. + * getStrings, shouldShowClearExitNode, isActiveExitNode, and getClipboardCommands + * are exercised here via node:test. * * The four Process objects, 5-second Timer, onExited handlers, copy fallback * state machine, and overall widget behavior in TailscaleWidget.qml have no @@ -88,67 +88,27 @@ test("errorMessage returns generic message for unknown command", () => { assert.strictEqual(msg, "Tailscale command failed") }) -// --- validateClipboardTool --- +// --- getClipboardCommands (simple array, argv only, per #48 first-principles) --- -test("validateClipboardTool accepts whitelisted tool names", () => { - assert.strictEqual(validateClipboardTool("dms"), true) - assert.strictEqual(validateClipboardTool("wl-copy"), true) - // clipmanctl intentionally dropped (see #49) +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"]) }) -test("validateClipboardTool rejects malicious inputs", () => { - assert.strictEqual(validateClipboardTool("rm -rf /"), false) - assert.strictEqual(validateClipboardTool("echo hi; malicious"), false) - assert.strictEqual(validateClipboardTool("$(whoami)"), false) - assert.strictEqual(validateClipboardTool("wl-copy || rm -rf /"), false) +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")) }) -test("validateClipboardTool rejects empty and falsy inputs", () => { - assert.strictEqual(validateClipboardTool(""), false) - assert.strictEqual(validateClipboardTool(null), false) - assert.strictEqual(validateClipboardTool(undefined), false) -}) - -// --- 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) +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])) }) // --- getStrings (for #34 i18n prep) --- @@ -160,8 +120,6 @@ 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", () => { From ca595f19df9ae2cf13185cf516d608d28fe59402 Mon Sep 17 00:00:00 2001 From: vybe Date: Fri, 22 May 2026 21:45:13 +0000 Subject: [PATCH 2/3] test: update coverage comment to reflect current reality - Remove all historical references (#48, first-principles, newly extracted, etc.) - Remove mentions of deleted components (5s Timer, old copy state machine) - Make the comment strictly about what the tests actually cover and the real shortcoming (no automated QML coverage) - Per request: code comments exist to explain the code, not preserve history --- test/lib.test.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test/lib.test.js b/test/lib.test.js index ecddc98..8d3c0f0 100644 --- a/test/lib.test.js +++ b/test/lib.test.js @@ -4,16 +4,13 @@ import lib from "../tailscalectl/lib.js" const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, isValidExitNodeHostname, getClipboardCommands, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } = lib /* - * Test coverage note for #48: - * All pure functions exported from lib.js (including the newly extracted - * getStrings, shouldShowClearExitNode, isActiveExitNode, and getClipboardCommands - * are exercised here via node:test. + * Unit tests for the pure functions exported from lib.js. * - * The four Process objects, 5-second Timer, onExited handlers, copy fallback - * state machine, and overall widget behavior in TailscaleWidget.qml have no - * automated test coverage. There is no QML test runner in this project. - * Those paths are verified manually (see verification checklist in the - * feature branch commits). + * All functions in lib.js are exercised via Node's built-in test runner. + * + * TailscaleWidget.qml has no automated test coverage. The four Process + * objects, their onExited handlers, and all widget UI behavior must be + * verified manually. */ test("parsePeers extracts exitNode from ExitNodeOption", () => { From 77dd84c279f5f2fd32e6ce3a93685358e728c45c Mon Sep 17 00:00:00 2001 From: vybe Date: Fri, 22 May 2026 21:47:41 +0000 Subject: [PATCH 3/3] Code formatting cleanup. --- tailscalectl/lib.js | 4 ++-- test/lib.test.js | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tailscalectl/lib.js b/tailscalectl/lib.js index 6d28fe8..2879eac 100644 --- a/tailscalectl/lib.js +++ b/tailscalectl/lib.js @@ -59,8 +59,8 @@ function getStrings() { } } - // Light UI predicates — keep the view thin. - function shouldShowClearExitNode(currentExitNode) { +// Light UI predicates — keep the view thin. +function shouldShowClearExitNode(currentExitNode) { return currentExitNode !== "" } diff --git a/test/lib.test.js b/test/lib.test.js index 8d3c0f0..c5ab20b 100644 --- a/test/lib.test.js +++ b/test/lib.test.js @@ -85,7 +85,7 @@ test("errorMessage returns generic message for unknown command", () => { assert.strictEqual(msg, "Tailscale command failed") }) -// --- getClipboardCommands (simple array, argv only, per #48 first-principles) --- +// --- getClipboardCommands --- test("getClipboardCommands returns ordered argv arrays with text appended", () => { const cmds = getClipboardCommands("1.2.3.4") @@ -108,7 +108,7 @@ test("getClipboardCommands is deterministic and open for future tools", () => { assert.ok(Array.isArray(cmds[1])) }) -// --- getStrings (for #34 i18n prep) --- +// --- getStrings --- test("getStrings returns canonical UI strings for the widget", () => { const s = getStrings() @@ -196,7 +196,7 @@ test("parseStatusResult sets isConnected false for non-Running BackendState", () assert.strictEqual(state.isConnected, false) }) -// --- formatError (central error + detail formatting per first-principles plan) --- +// --- formatError (central error + detail formatting) --- test("formatError returns base message without detail", () => { assert.strictEqual(formatError("status"), "Failed to read Tailscale status") @@ -216,7 +216,7 @@ test("formatError handles empty or falsy detail gracefully", () => { assert.strictEqual(formatError("connect", null), "Failed to connect to Tailscale") }) -// --- isValidExitNodeHostname + makeExitNodeCommand safety (balanced paranoia, direct-argv defense-in-depth) --- +// --- isValidExitNodeHostname + makeExitNodeCommand safety --- test("isValidExitNodeHostname accepts empty string (clear)", () => { assert.strictEqual(isValidExitNodeHostname(""), true) @@ -244,7 +244,7 @@ test("makeExitNodeCommand still produces correct argv for valid input", () => { assert.deepStrictEqual(makeExitNodeCommand("gluetun-sjc"), ["tailscale", "set", "--exit-node=gluetun-sjc"]) }) -// --- getStatusCommand (new pure helper for low-duty-cycle architecture) --- +// --- getStatusCommand --- test("getStatusCommand returns the canonical tailscale status --json argv", () => { const cmd = getStatusCommand()