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..2879eac 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,16 +53,14 @@ 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. - function shouldShowClearExitNode(currentExitNode) { +// Light UI predicates — keep the view thin. +function shouldShowClearExitNode(currentExitNode) { return currentExitNode !== "" } @@ -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..c5ab20b 100644 --- a/test/lib.test.js +++ b/test/lib.test.js @@ -1,19 +1,16 @@ 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. + * 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", () => { @@ -88,70 +85,30 @@ test("errorMessage returns generic message for unknown command", () => { assert.strictEqual(msg, "Tailscale command failed") }) -// --- validateClipboardTool --- +// --- getClipboardCommands --- -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) +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])) }) -// --- 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) --- +// --- getStrings --- test("getStrings returns canonical UI strings for the widget", () => { const s = getStrings() @@ -160,8 +117,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", () => { @@ -241,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") @@ -261,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) @@ -289,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()