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.
This commit is contained in:
parent
e695db6650
commit
6e4e5d51ec
3 changed files with 39 additions and 108 deletions
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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 }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue