Merge branch 'feature_sanitize_shell_commands' into vibes

Reviewed-on: #36

AGENTS.md

@@ -1,4 +1,16 @@
 ## Documentation for agents
 
+You **MUST** read each of these documents before contributing to this repository:
+- Git workflow rules: `docs/agents/git-workflow.md`
 - Repo interaction rules: `docs/agents/repo-instructions.md`
 - Domain model & context rules: `docs/agents/domain.md`
+
+## Critical Coding & Version Control Conduct
+
+- **NEVER** commit directly to `master` or `testing without explicit human instruction.
+- **NEVER** merge into `master` or `testing` without explict human instruction.
+- **ALWAYS** create new branches when working on non-trivial coding tasks.
+- **ALWAYS** commit self-contained logical unit of work.
+
+## Issues, PRs, and Comments Conduct
+- **ALWAYS** end your written contributions with `Written by AI agent working for @jtmorris. Model: <MODEL NAME>.`. Replace `<MODEL NAME>` with the LLM model, version, and, if relevant, number of parameters. For example: `Claude Sonnet 4.7`, `Grok 4.3`, `Qwen 3.6 27B`.

docs/agents/git-workflow.md

@@ -0,0 +1,106 @@
+# Git Workflow — Branching Strategy and Rules
+
+## Branch Hierarchy
+
+```
+feature_*  →  vibes  →  testing  →  master
+ (work)     (agent)   (beta)     (stable)
+```
+
+### `master`
+- **Purpose**: Stable, tested, production-ready code. What end-users run.
+- **Who commits**: Human only. Agents **must not** commit or merge here without explicit human instruction.
+- **Requirements**: All tests pass. Code reviewed and approved by human.
+
+### `testing`
+- **Purpose**: Beta-quality code. May contain bugs. Not guaranteed safe.
+- **Who commits**: Human only. Agents **must not** commit or merge here without explicit human instruction.
+- **Requirements**: Human has reviewed `vibes` and approved promotion.
+
+### `vibes`
+- **Purpose**: Agent integration branch. Completed features, bug fixes, and tasks land here for human review.
+- **Who commits**: Agents merge completed feature branches here. Human reviews and either approves (promotes to `testing`) or rejects (sends agent back to feature branch).
+- **Requirements**: Feature branch's tests pass. Feature branch work is complete per task scope.
+
+### `feature_<slug>`
+- **Purpose**: Isolated work branches for a single feature, bug fix, or task.
+- **Who commits**: Agents and humans.
+- **Naming**: `feature_` prefix followed by a short, git-safe slug describing the task (e.g., `feature_add_toast_notifications`, `fix_exit_node_lookup`).
+- **Branch point**: Usually off `master`. If chaining tasks or building on unmerged work, branch off `vibes` or `testing` as appropriate.
+
+## Commit Discipline
+
+Agents must commit frequently, after each self-contained logical unit of work. **Do not batch all changes into a single commit.**
+
+- Commit after each passing test, completed function, or logical change.
+- Each commit should be small, focused, and meaningful on its own.
+- Every commit must be traceable, reversible, and not break existing tests.
+- Granular commits enable easy bisecting, clear history, and precise rollbacks.
+
+## Commit Messages
+
+Use conventional commit format:
+
+```
+<type>: <short description>
+
+<optional body explaining why>
+```
+
+Types: `feat`, `fix`, `refactor`, `docs`, `style`, `test`, `chore`, `perf`, `ci`, `build`.
+
+Messages must explain **what** changed and **why**. Avoid vague messages like "fix stuff" or "update code".
+
+## Rebasing & Syncing
+
+Before merging a feature branch into `vibes`:
+
+1. Rebase the feature branch onto the latest state of its base branch (`master`, `vibes`, or `testing`).
+2. Resolve any conflicts during rebase.
+3. Verify tests still pass after rebase.
+
+This keeps history linear and avoids unnecessary merge commits.
+
+## Workflow
+
+### Starting Work
+1. Create a new `feature_<slug>` branch from the appropriate base branch (default: `master`).
+2. Push the feature branch to remote.
+3. Implement the feature, bug fix, or task.
+4. Commit frequently (see Commit Discipline above).
+5. Run tests after each commit. Ensure project-wide testing strategy passes.
+
+### Completing Work (Agent)
+1. Rebase feature branch onto latest base branch.
+2. Merge the feature branch into `vibes` (preserve individual commits, do not squash).
+3. Push `vibes` to remote.
+4. Notify human that work is on `vibes` for review. **Stop and wait for human feedback.**
+
+### Human Review
+1. Human reviews `vibes` branch.
+2. **If approved**: Human merges `vibes` → `testing`.
+3. **If rejected**: Human communicates what needs fixing. Agent returns to the feature branch (or creates a new one) to address issues.
+
+### Promoting to Stable
+1. Human validates `testing` branch is ready.
+2. Human merges `testing` → `master`.
+
+## Agent Guardrails
+
+- **NEVER** commit directly to `master` or `testing` without explicit human instruction.
+- **NEVER** merge into `master` or `testing` without explicit human instruction.
+- Always work in a `feature_<slug>` branch.
+- Merge completed work to `vibes`, then stop and wait for human review.
+- Preserve individual commits when merging — do not squash.
+- Rebase feature branches before merging to `vibes` — do not create merge commits.
+
+## Branch Management
+
+- `feature_*` branches may be deleted after merging to `vibes` if no longer needed.
+- If work is rejected from `vibes`, keep the feature branch for iterative fixes.
+- `vibes` should be kept in a mergeable state — do not let it accumulate broken code.
+- Push feature branches to remote for backup and visibility.
+
+## Quick Spikes & Exploration
+
+For quick local exploration or solo spikes, use `.scratch/<feature-slug>/` as documented in `docs/agents/repo-instructions.md`. When spike work becomes real, move it to a proper `feature_*` branch.

tailscalectl/TailscaleWidget.qml

@@ -62,11 +62,11 @@ PluginComponent {
     Process {
         id: detectClipboard
 
-        command: ["sh", "-c", "which dms >/dev/null 2>&1 && echo 'dms cl copy' || which wl-copy >/dev/null 2>&1 && echo 'wl-copy' || which clipmanctl >/dev/null 2>&1 && echo 'clipmanctl copy' || echo none"]
+        command: ["sh", "-c", "if which dms >/dev/null 2>&1; then echo 'dms cl copy'; elif which wl-copy >/dev/null 2>&1; then echo 'wl-copy'; elif which clipmanctl >/dev/null 2>&1; then echo 'clipmanctl copy'; elif which xclip >/dev/null 2>&1; then echo 'xclip -selection clipboard'; elif which xsel >/dev/null 2>&1; then echo 'xsel --clipboard --input'; else echo none; fi"]
 
         stdout: StdioCollector {
             onStreamFinished: {
-                root.clipboardCmd = this.text.trim()
+                root.clipboardCmd = TailscaleLib.parseClipboardDetection(this.text)
             }
         }
 
@@ -82,19 +82,11 @@ PluginComponent {
 
         stdout: StdioCollector {
             onStreamFinished: {
-                try {
-                    const data = JSON.parse(this.text)
-                    root.isConnected = data.BackendState === "Running"
-                    root.tailscaleIP = (data.Self?.TailscaleIPs?.[0]) || ""
-                    root.currentExitNode = TailscaleLib.findActiveExitNode(data.Peer || {})
-                    root.peers = TailscaleLib.parsePeers(data.Peer || {})
-                } catch (e) {
-                    root.isConnected = false
-                    root.tailscaleIP = ""
-                    root.currentExitNode = ""
-                    root.peers = []
-                    ToastService.showError("tailscalectl", "Failed to parse Tailscale status")
-                }
+                const state = TailscaleLib.parseStatusResult(this.text)
+                root.isConnected = state.isConnected
+                root.tailscaleIP = state.tailscaleIP
+                root.currentExitNode = state.currentExitNode
+                root.peers = state.peers
             }
         }
 
@@ -119,11 +111,7 @@ PluginComponent {
             ToastService.showError("tailscalectl", "Tailscale not available")
             return
         }
-        if (root.isConnected) {
-            toggleProcess.command = ["tailscale", "down"]
-        } else {
-            toggleProcess.command = ["tailscale", "up"]
-        }
+        toggleProcess.command = TailscaleLib.buildToggleCommand(root.isConnected)
         toggleProcess.running = true
     }
 
@@ -141,7 +129,12 @@ PluginComponent {
             ToastService.showError("tailscalectl", "No clipboard tool found")
             return
         }
-        copyProcess.command = ["sh", "-c", "printf '%s' '" + text.replace(/'/g, "'\\''") + "' | " + root.clipboardCmd]
+        var cmd = TailscaleLib.buildCopyCommand(text, root.clipboardCmd)
+        if (!cmd) {
+            ToastService.showError("tailscalectl", "Invalid clipboard command")
+            return
+        }
+        copyProcess.command = cmd
         copyProcess.running = true
         ToastService.showInfo("Copied " + text + " to clipboard")
     }

tailscalectl/lib.js

@@ -26,6 +26,41 @@ function findActiveExitNode(peerMap) {
     return ""
 }
 
+var safeClipboardCmds = ["dms cl copy", "wl-copy", "clipmanctl copy", "xclip -selection clipboard", "xsel --clipboard --input", "none"]
+
+function validateClipboardCmd(cmd) {
+    return typeof cmd === "string" && safeClipboardCmds.includes(cmd)
+}
+
+function buildCopyCommand(text, clipboardCmd) {
+    if (!validateClipboardCmd(clipboardCmd)) return null
+    var escaped = text.replace(/'/g, "'\\''")
+    return ["sh", "-c", "printf '%s' '" + escaped + "' | " + clipboardCmd]
+}
+
+function parseStatusResult(jsonText) {
+    try {
+        const data = JSON.parse(jsonText)
+        return {
+            isConnected: data.BackendState === "Running",
+            tailscaleIP: (data.Self && data.Self.TailscaleIPs && data.Self.TailscaleIPs[0]) || "",
+            currentExitNode: findActiveExitNode(data.Peer || {}),
+            peers: parsePeers(data.Peer || {})
+        }
+    } catch (e) {
+        return { isConnected: false, tailscaleIP: "", currentExitNode: "", peers: [] }
+    }
+}
+
+function buildToggleCommand(isConnected) {
+    return isConnected ? ["tailscale", "down"] : ["tailscale", "up"]
+}
+
+function parseClipboardDetection(stdout) {
+    var trimmed = typeof stdout === "string" ? stdout.trim() : ""
+    return validateClipboardCmd(trimmed) ? trimmed : "none"
+}
+
 function errorMessage(cmd) {
     var messages = {
         "up": "Failed to connect to Tailscale",
@@ -40,5 +75,5 @@ function errorMessage(cmd) {
 
 // CommonJS export for Node.js tests (ignored by QML)
 if (typeof module !== "undefined" && module.exports) {
-    module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage }
+    module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, validateClipboardCmd, buildCopyCommand, parseClipboardDetection, buildToggleCommand, parseStatusResult }
 }

test/lib.test.js

@@ -1,7 +1,7 @@
 import { test } from "node:test"
 import assert from "node:assert"
 import lib from "../tailscalectl/lib.js"
-const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage } = lib
+const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, validateClipboardCmd, buildCopyCommand, parseClipboardDetection, buildToggleCommand, parseStatusResult } = lib
 
 test("parsePeers extracts exitNode from ExitNodeOption", () => {
     const peerMap = {
@@ -74,3 +74,134 @@ test("errorMessage returns generic message for unknown command", () => {
     const msg = errorMessage("unknown", 1)
     assert.strictEqual(msg, "Tailscale command failed")
 })
+
+// --- validateClipboardCmd ---
+
+test("validateClipboardCmd accepts whitelisted values", () => {
+    assert.strictEqual(validateClipboardCmd("dms cl copy"), true)
+    assert.strictEqual(validateClipboardCmd("wl-copy"), true)
+    assert.strictEqual(validateClipboardCmd("clipmanctl copy"), true)
+    assert.strictEqual(validateClipboardCmd("xclip -selection clipboard"), true)
+    assert.strictEqual(validateClipboardCmd("xsel --clipboard --input"), true)
+    assert.strictEqual(validateClipboardCmd("none"), true)
+})
+
+test("validateClipboardCmd rejects malicious inputs", () => {
+    assert.strictEqual(validateClipboardCmd("rm -rf /"), false)
+    assert.strictEqual(validateClipboardCmd("echo hi; malicious"), false)
+    assert.strictEqual(validateClipboardCmd("$(whoami)"), false)
+    assert.strictEqual(validateClipboardCmd("wl-copy || rm -rf /"), false)
+})
+
+test("validateClipboardCmd rejects empty and falsy inputs", () => {
+    assert.strictEqual(validateClipboardCmd(""), false)
+    assert.strictEqual(validateClipboardCmd(null), false)
+    assert.strictEqual(validateClipboardCmd(undefined), false)
+})
+
+// --- buildCopyCommand ---
+
+test("buildCopyCommand returns safe command for whitelisted clipboard tool", () => {
+    const cmd = buildCopyCommand("hello", "wl-copy")
+    assert.ok(Array.isArray(cmd))
+    assert.strictEqual(cmd[0], "sh")
+    assert.strictEqual(cmd[1], "-c")
+})
+
+test("buildCopyCommand returns null for invalid clipboard command", () => {
+    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))
+})
+
+// --- parseClipboardDetection ---
+
+test("parseClipboardDetection passes through expected output strings", () => {
+    assert.strictEqual(parseClipboardDetection("dms cl copy"), "dms cl copy")
+    assert.strictEqual(parseClipboardDetection("wl-copy"), "wl-copy")
+    assert.strictEqual(parseClipboardDetection("clipmanctl copy"), "clipmanctl copy")
+    assert.strictEqual(parseClipboardDetection("none"), "none")
+})
+
+test("parseClipboardDetection trims extra whitespace", () => {
+    assert.strictEqual(parseClipboardDetection("  wl-copy  \n"), "wl-copy")
+})
+
+test("parseClipboardDetection falls back to none for multi-line output", () => {
+    assert.strictEqual(parseClipboardDetection("wl-copy\nrm -rf /"), "none")
+})
+
+test("parseClipboardDetection falls back to empty output", () => {
+    assert.strictEqual(parseClipboardDetection(""), "none")
+    assert.strictEqual(parseClipboardDetection("   "), "none")
+})
+
+test("parseClipboardDetection falls back to none for unexpected strings", () => {
+    assert.strictEqual(parseClipboardDetection("pbpaste"), "none")
+    assert.strictEqual(parseClipboardDetection("custom-tool"), "none")
+})
+
+// --- buildToggleCommand ---
+
+test("buildToggleCommand returns down command when connected", () => {
+    assert.deepStrictEqual(buildToggleCommand(true), ["tailscale", "down"])
+})
+
+test("buildToggleCommand returns up command when disconnected", () => {
+    assert.deepStrictEqual(buildToggleCommand(false), ["tailscale", "up"])
+})
+
+test("buildToggleCommand treats null and undefined as disconnected", () => {
+    assert.deepStrictEqual(buildToggleCommand(null), ["tailscale", "up"])
+    assert.deepStrictEqual(buildToggleCommand(undefined), ["tailscale", "up"])
+})
+
+// --- parseStatusResult ---
+
+test("parseStatusResult produces correct state from valid JSON", () => {
+    const json = JSON.stringify({
+        BackendState: "Running",
+        Self: { TailscaleIPs: ["100.64.0.5"] },
+        Peer: {
+            "key-1": { HostName: "router", TailscaleIPs: ["100.64.0.1"], Online: true, ExitNode: true, ExitNodeOption: true }
+        }
+    })
+    const state = parseStatusResult(json)
+    assert.strictEqual(state.isConnected, true)
+    assert.strictEqual(state.tailscaleIP, "100.64.0.5")
+    assert.strictEqual(state.currentExitNode, "router")
+    assert.strictEqual(state.peers.length, 1)
+})
+
+test("parseStatusResult returns safe defaults for invalid JSON", () => {
+    const state = parseStatusResult("not json at all")
+    assert.strictEqual(state.isConnected, false)
+    assert.strictEqual(state.tailscaleIP, "")
+    assert.strictEqual(state.currentExitNode, "")
+    assert.strictEqual(state.peers.length, 0)
+})
+
+test("parseStatusResult handles missing Self gracefully", () => {
+    const json = JSON.stringify({ BackendState: "Running", Peer: {} })
+    const state = parseStatusResult(json)
+    assert.strictEqual(state.isConnected, true)
+    assert.strictEqual(state.tailscaleIP, "")
+})
+
+test("parseStatusResult handles missing and empty Peer gracefully", () => {
+    const json = JSON.stringify({ BackendState: "Running", Self: { TailscaleIPs: ["100.64.0.5"] } })
+    const state = parseStatusResult(json)
+    assert.strictEqual(state.peers.length, 0)
+    assert.strictEqual(state.currentExitNode, "")
+})
+
+test("parseStatusResult sets isConnected false for non-Running BackendState", () => {
+    const json = JSON.stringify({ BackendState: "NeedsLogin", Self: { TailscaleIPs: ["100.64.0.5"] }, Peer: {} })
+    const state = parseStatusResult(json)
+    assert.strictEqual(state.isConnected, false)
+})