merge: clipboard simplification into vibes
- Simple array + getClipboardCommands(text) as single source of truth - Removed all old clipboard object map, validate/next/all helpers, and QML-side tool knowledge - QML now uses dumb index-driven retry via _runNextCopy - Updated test coverage comment to be current and non-historical - All tests pass Feature branch: feature_clipboard_simple_array
This commit is contained in:
commit
909b935f72
3 changed files with 49 additions and 121 deletions
|
|
@ -15,7 +15,7 @@ PluginComponent {
|
||||||
property string currentExitNode: ""
|
property string currentExitNode: ""
|
||||||
property var peers: []
|
property var peers: []
|
||||||
property string _copyText: ""
|
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)
|
property bool _pendingToggle: false // transient one-shot for post-action verification (to be removed in first-principles refactor)
|
||||||
|
|
||||||
layerNamespacePlugin: "tailscalectl"
|
layerNamespacePlugin: "tailscalectl"
|
||||||
|
|
@ -50,13 +50,9 @@ PluginComponent {
|
||||||
onExited: (code, status) => {
|
onExited: (code, status) => {
|
||||||
if (code === 0) {
|
if (code === 0) {
|
||||||
ToastService.showInfo(TailscaleLib.getStrings().copied(root._copyText))
|
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 {
|
} else {
|
||||||
var detail = copyProcess.stderr.text.trim()
|
root._copyIndex += 1
|
||||||
ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard", detail))
|
root._runNextCopy()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -126,18 +122,17 @@ PluginComponent {
|
||||||
|
|
||||||
function copyToClipboard(text) {
|
function copyToClipboard(text) {
|
||||||
root._copyText = text
|
root._copyText = text
|
||||||
// Simple two-tool fallback per first-principles plan: dms first, then wl-copy.
|
root._copyIndex = 0
|
||||||
root._copyCurrentTool = "dms"
|
root._runNextCopy()
|
||||||
root._executeCopy()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function _executeCopy() {
|
function _runNextCopy() {
|
||||||
var cmd = TailscaleLib.buildCopyCommand(root._copyText, root._copyCurrentTool)
|
const cmds = TailscaleLib.getClipboardCommands(root._copyText)
|
||||||
if (!cmd) {
|
if (root._copyIndex >= cmds.length) {
|
||||||
ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard"))
|
ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard"))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
copyProcess.command = cmd
|
copyProcess.command = cmds[root._copyIndex]
|
||||||
copyProcess.running = true
|
copyProcess.running = true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -34,34 +34,15 @@ function findActiveExitNode(peerMap) {
|
||||||
return ""
|
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.
|
function getClipboardCommands(text) {
|
||||||
// clipmanctl intentionally dropped (niche optional history manager, not needed on DMS + Wayland).
|
return clipboardTools.map(function (tool) {
|
||||||
var clipboardTools = {
|
return tool.argv.concat([text]);
|
||||||
"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() {
|
function getStrings() {
|
||||||
|
|
@ -72,16 +53,14 @@ function getStrings() {
|
||||||
exitNodePrefix: "Exit node: ",
|
exitNodePrefix: "Exit node: ",
|
||||||
none: "None",
|
none: "None",
|
||||||
copied: function (text) { return "Copied " + text + " to clipboard" },
|
copied: function (text) { return "Copied " + text + " to clipboard" },
|
||||||
noClipboardTool: "No clipboard tool found",
|
|
||||||
invalidClipboardCommand: "Invalid clipboard command",
|
|
||||||
clearExitNode: "×",
|
clearExitNode: "×",
|
||||||
setExitNode: "↗",
|
setExitNode: "↗",
|
||||||
invalidExitNodeHostname: "Invalid exit node hostname"
|
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 !== ""
|
return currentExitNode !== ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -89,9 +68,8 @@ function isActiveExitNode(currentExitNode, hostname) {
|
||||||
return currentExitNode === hostname
|
return currentExitNode === hostname
|
||||||
}
|
}
|
||||||
|
|
||||||
// Security: validate hostnames coming from tailscale status JSON before we
|
// Security: validate hostnames coming from tailscale status JSON.
|
||||||
// concatenate them into a command argument. Direct argv is already used
|
// Fail closed on obviously malicious input.
|
||||||
// (see #49), but we still fail closed on obviously malicious input.
|
|
||||||
function isValidExitNodeHostname(hostname) {
|
function isValidExitNodeHostname(hostname) {
|
||||||
if (typeof hostname !== "string") return false;
|
if (typeof hostname !== "string") return false;
|
||||||
if (hostname === "") return true; // clear command
|
if (hostname === "") return true; // clear command
|
||||||
|
|
@ -149,5 +127,5 @@ function formatError(action, detail) {
|
||||||
|
|
||||||
// CommonJS export for Node.js tests (ignored by QML)
|
// CommonJS export for Node.js tests (ignored by QML)
|
||||||
if (typeof module !== "undefined" && module.exports) {
|
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,19 +1,16 @@
|
||||||
import { test } from "node:test"
|
import { test } from "node:test"
|
||||||
import assert from "node:assert"
|
import assert from "node:assert"
|
||||||
import lib from "../tailscalectl/lib.js"
|
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:
|
* Unit tests for the pure functions exported from lib.js.
|
||||||
* All pure functions exported from lib.js (including the newly extracted
|
|
||||||
* getStrings, shouldShowClearExitNode, isActiveExitNode, and the clipboard
|
|
||||||
* helpers) are exercised here via node:test.
|
|
||||||
*
|
*
|
||||||
* The four Process objects, 5-second Timer, onExited handlers, copy fallback
|
* All functions in lib.js are exercised via Node's built-in test runner.
|
||||||
* state machine, and overall widget behavior in TailscaleWidget.qml have no
|
*
|
||||||
* automated test coverage. There is no QML test runner in this project.
|
* TailscaleWidget.qml has no automated test coverage. The four Process
|
||||||
* Those paths are verified manually (see verification checklist in the
|
* objects, their onExited handlers, and all widget UI behavior must be
|
||||||
* feature branch commits).
|
* verified manually.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
test("parsePeers extracts exitNode from ExitNodeOption", () => {
|
test("parsePeers extracts exitNode from ExitNodeOption", () => {
|
||||||
|
|
@ -88,70 +85,30 @@ test("errorMessage returns generic message for unknown command", () => {
|
||||||
assert.strictEqual(msg, "Tailscale command failed")
|
assert.strictEqual(msg, "Tailscale command failed")
|
||||||
})
|
})
|
||||||
|
|
||||||
// --- validateClipboardTool ---
|
// --- getClipboardCommands ---
|
||||||
|
|
||||||
test("validateClipboardTool accepts whitelisted tool names", () => {
|
test("getClipboardCommands returns ordered argv arrays with text appended", () => {
|
||||||
assert.strictEqual(validateClipboardTool("dms"), true)
|
const cmds = getClipboardCommands("1.2.3.4")
|
||||||
assert.strictEqual(validateClipboardTool("wl-copy"), true)
|
assert.ok(Array.isArray(cmds))
|
||||||
// clipmanctl intentionally dropped (see #49)
|
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", () => {
|
test("getClipboardCommands handles text with special characters safely (direct argv)", () => {
|
||||||
assert.strictEqual(validateClipboardTool("rm -rf /"), false)
|
const cmds = getClipboardCommands("it's a 'test' with \"quotes\" and\nnewlines")
|
||||||
assert.strictEqual(validateClipboardTool("echo hi; malicious"), false)
|
assert.ok(Array.isArray(cmds))
|
||||||
assert.strictEqual(validateClipboardTool("$(whoami)"), false)
|
assert.strictEqual(cmds.length, 2)
|
||||||
assert.strictEqual(validateClipboardTool("wl-copy || rm -rf /"), false)
|
assert.ok(cmds[0].includes("it's a 'test' with \"quotes\" and\nnewlines"))
|
||||||
})
|
})
|
||||||
|
|
||||||
test("validateClipboardTool rejects empty and falsy inputs", () => {
|
test("getClipboardCommands is deterministic and open for future tools", () => {
|
||||||
assert.strictEqual(validateClipboardTool(""), false)
|
const cmds = getClipboardCommands("foo")
|
||||||
assert.strictEqual(validateClipboardTool(null), false)
|
assert.ok(Array.isArray(cmds[0]))
|
||||||
assert.strictEqual(validateClipboardTool(undefined), false)
|
assert.ok(Array.isArray(cmds[1]))
|
||||||
})
|
})
|
||||||
|
|
||||||
// --- buildCopyCommand ---
|
// --- getStrings ---
|
||||||
|
|
||||||
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", () => {
|
test("getStrings returns canonical UI strings for the widget", () => {
|
||||||
const s = getStrings()
|
const s = getStrings()
|
||||||
|
|
@ -160,8 +117,6 @@ test("getStrings returns canonical UI strings for the widget", () => {
|
||||||
assert.ok(s.disconnected)
|
assert.ok(s.disconnected)
|
||||||
assert.ok(s.exitNodePrefix)
|
assert.ok(s.exitNodePrefix)
|
||||||
assert.ok(s.copied)
|
assert.ok(s.copied)
|
||||||
assert.ok(s.noClipboardTool)
|
|
||||||
assert.ok(s.invalidClipboardCommand)
|
|
||||||
})
|
})
|
||||||
|
|
||||||
test("getStrings.copied interpolates the text", () => {
|
test("getStrings.copied interpolates the text", () => {
|
||||||
|
|
@ -241,7 +196,7 @@ test("parseStatusResult sets isConnected false for non-Running BackendState", ()
|
||||||
assert.strictEqual(state.isConnected, false)
|
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", () => {
|
test("formatError returns base message without detail", () => {
|
||||||
assert.strictEqual(formatError("status"), "Failed to read Tailscale status")
|
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")
|
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)", () => {
|
test("isValidExitNodeHostname accepts empty string (clear)", () => {
|
||||||
assert.strictEqual(isValidExitNodeHostname(""), true)
|
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"])
|
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", () => {
|
test("getStatusCommand returns the canonical tailscale status --json argv", () => {
|
||||||
const cmd = getStatusCommand()
|
const cmd = getStatusCommand()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue