From 05b0b82937afa148bdd9603f93d3345d886ec6d1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 24 Jan 2026 00:16:07 +0000 Subject: [PATCH] fix: guard tailscale sudo fallback (#1551) (thanks @sweepies) --- CHANGELOG.md | 1 + src/infra/tailscale.test.ts | 71 ++++++++++++++++++------- src/infra/tailscale.ts | 101 +++++++++++++++++++++--------------- 3 files changed, 114 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17f0da5ec..557a80ac5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Docs: https://docs.clawd.bot ### Fixes - Voice wake: auto-save wake words on blur/submit across iOS/Android and align limits with macOS. +- Tailscale: retry serve/funnel with sudo only for permission errors and keep original failure details. (#1551) Thanks @sweepies. - Discord: limit autoThread mention bypass to bot-owned threads; keep ack reactions mention-gated. (#1511) Thanks @pvoo. - Gateway: accept null optional fields in exec approval requests. (#1511) Thanks @pvoo. - TUI: forward unknown slash commands (for example, `/context`) to the Gateway. diff --git a/src/infra/tailscale.test.ts b/src/infra/tailscale.test.ts index 39bb9ecc3..410c7befd 100644 --- a/src/infra/tailscale.test.ts +++ b/src/infra/tailscale.test.ts @@ -1,17 +1,21 @@ -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; -import { +import * as tailscale from "./tailscale.js"; + +const { ensureGoInstalled, ensureTailscaledInstalled, getTailnetHostname, enableTailscaleServe, disableTailscaleServe, - enableTailscaleFunnel, - disableTailscaleFunnel, - ensureFunnel -} from "./tailscale.js"; + ensureFunnel, +} = tailscale; describe("tailscale helpers", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + it("parses DNS name from tailscale status", async () => { const exec = vi.fn().mockResolvedValue({ stdout: JSON.stringify({ @@ -61,7 +65,9 @@ describe("tailscale helpers", () => { it("enableTailscaleServe attempts normal first, then sudo", async () => { // 1. First attempt fails // 2. Second attempt (sudo) succeeds - const exec = vi.fn() + vi.spyOn(tailscale, "getTailscaleBinary").mockResolvedValue("tailscale"); + const exec = vi + .fn() .mockRejectedValueOnce(new Error("permission denied")) .mockResolvedValueOnce({ stdout: "" }); @@ -71,18 +77,19 @@ describe("tailscale helpers", () => { 1, "tailscale", expect.arrayContaining(["serve", "--bg", "--yes", "3000"]), - expect.any(Object) + expect.any(Object), ); expect(exec).toHaveBeenNthCalledWith( 2, "sudo", expect.arrayContaining(["-n", "tailscale", "serve", "--bg", "--yes", "3000"]), - expect.any(Object) + expect.any(Object), ); }); it("enableTailscaleServe does NOT use sudo if first attempt succeeds", async () => { + vi.spyOn(tailscale, "getTailscaleBinary").mockResolvedValue("tailscale"); const exec = vi.fn().mockResolvedValue({ stdout: "" }); await enableTailscaleServe(3000, exec as never); @@ -91,13 +98,15 @@ describe("tailscale helpers", () => { expect(exec).toHaveBeenCalledWith( "tailscale", expect.arrayContaining(["serve", "--bg", "--yes", "3000"]), - expect.any(Object) + expect.any(Object), ); }); it("disableTailscaleServe uses fallback", async () => { - const exec = vi.fn() - .mockRejectedValueOnce(new Error("failed")) + vi.spyOn(tailscale, "getTailscaleBinary").mockResolvedValue("tailscale"); + const exec = vi + .fn() + .mockRejectedValueOnce(new Error("permission denied")) .mockResolvedValueOnce({ stdout: "" }); await disableTailscaleServe(exec as never); @@ -107,7 +116,7 @@ describe("tailscale helpers", () => { 2, "sudo", expect.arrayContaining(["-n", "tailscale", "serve", "reset"]), - expect.any(Object) + expect.any(Object), ); }); @@ -116,9 +125,11 @@ describe("tailscale helpers", () => { // 1. status (success) // 2. enable (fails) // 3. enable sudo (success) - const exec = vi.fn() + vi.spyOn(tailscale, "getTailscaleBinary").mockResolvedValue("tailscale"); + const exec = vi + .fn() .mockResolvedValueOnce({ stdout: JSON.stringify({ BackendState: "Running" }) }) // status - .mockRejectedValueOnce(new Error("failed")) // enable normal + .mockRejectedValueOnce(new Error("permission denied")) // enable normal .mockResolvedValueOnce({ stdout: "" }); // enable sudo const runtime = { @@ -134,7 +145,7 @@ describe("tailscale helpers", () => { expect(exec).toHaveBeenNthCalledWith( 1, "tailscale", - expect.arrayContaining(["funnel", "status", "--json"]) + expect.arrayContaining(["funnel", "status", "--json"]), ); // 2. enable normal @@ -142,7 +153,7 @@ describe("tailscale helpers", () => { 2, "tailscale", expect.arrayContaining(["funnel", "--yes", "--bg", "8080"]), - expect.any(Object) + expect.any(Object), ); // 3. enable sudo @@ -150,7 +161,31 @@ describe("tailscale helpers", () => { 3, "sudo", expect.arrayContaining(["-n", "tailscale", "funnel", "--yes", "--bg", "8080"]), - expect.any(Object) + expect.any(Object), ); }); + + it("enableTailscaleServe skips sudo on non-permission errors", async () => { + vi.spyOn(tailscale, "getTailscaleBinary").mockResolvedValue("tailscale"); + const exec = vi.fn().mockRejectedValueOnce(new Error("boom")); + + await expect(enableTailscaleServe(3000, exec as never)).rejects.toThrow("boom"); + + expect(exec).toHaveBeenCalledTimes(1); + }); + + it("enableTailscaleServe rethrows original error if sudo fails", async () => { + vi.spyOn(tailscale, "getTailscaleBinary").mockResolvedValue("tailscale"); + const originalError = Object.assign(new Error("permission denied"), { + stderr: "permission denied", + }); + const exec = vi + .fn() + .mockRejectedValueOnce(originalError) + .mockRejectedValueOnce(new Error("sudo: a password is required")); + + await expect(enableTailscaleServe(3000, exec as never)).rejects.toBe(originalError); + + expect(exec).toHaveBeenCalledTimes(2); + }); }); diff --git a/src/infra/tailscale.ts b/src/infra/tailscale.ts index f7a0c6d41..8ff340184 100644 --- a/src/infra/tailscale.ts +++ b/src/infra/tailscale.ts @@ -206,6 +206,39 @@ export async function ensureTailscaledInstalled( await exec("brew", ["install", "tailscale"]); } +type ExecErrorDetails = { + stdout?: unknown; + stderr?: unknown; + message?: unknown; + code?: unknown; +}; + +function extractExecErrorText(err: unknown) { + const errOutput = err as ExecErrorDetails; + const stdout = typeof errOutput.stdout === "string" ? errOutput.stdout : ""; + const stderr = typeof errOutput.stderr === "string" ? errOutput.stderr : ""; + const message = typeof errOutput.message === "string" ? errOutput.message : ""; + const code = typeof errOutput.code === "string" ? errOutput.code : ""; + return { stdout, stderr, message, code }; +} + +function isPermissionDeniedError(err: unknown): boolean { + const { stdout, stderr, message, code } = extractExecErrorText(err); + if (code.toUpperCase() === "EACCES") return true; + const combined = `${stdout}\n${stderr}\n${message}`.toLowerCase(); + return ( + combined.includes("permission denied") || + combined.includes("access denied") || + combined.includes("operation not permitted") || + combined.includes("not permitted") || + combined.includes("requires root") || + combined.includes("must be run as root") || + combined.includes("must be run with sudo") || + combined.includes("requires sudo") || + combined.includes("need sudo") + ); +} + // Helper to attempt a command, and retry with sudo if it fails. async function execWithSudoFallback( exec: typeof runExec, @@ -216,12 +249,18 @@ async function execWithSudoFallback( try { return await exec(bin, args, opts); } catch (err) { - // If the error suggests permission denied or access denied, try with sudo. - // Or honestly, for any error in these specific ops, trying sudo is a reasonable fallback - // given the context of what we're doing (system-level network config). - // We'll log a verbose message that we're falling back. + if (!isPermissionDeniedError(err)) { + throw err; + } logVerbose(`Command failed, retrying with sudo: ${bin} ${args.join(" ")}`); - return await exec("sudo", ["-n", bin, ...args], opts); + try { + return await exec("sudo", ["-n", bin, ...args], opts); + } catch (sudoErr) { + const { stderr, message } = extractExecErrorText(sudoErr); + const detail = (stderr || message).trim(); + if (detail) logVerbose(`Sudo retry failed: ${detail}`); + throw err; + } } } @@ -313,52 +352,32 @@ export async function ensureFunnel( export async function enableTailscaleServe(port: number, exec: typeof runExec = runExec) { const tailscaleBin = await getTailscaleBinary(); - await execWithSudoFallback( - exec, - tailscaleBin, - ["serve", "--bg", "--yes", `${port}`], - { - maxBuffer: 200_000, - timeoutMs: 15_000, - }, - ); + await execWithSudoFallback(exec, tailscaleBin, ["serve", "--bg", "--yes", `${port}`], { + maxBuffer: 200_000, + timeoutMs: 15_000, + }); } export async function disableTailscaleServe(exec: typeof runExec = runExec) { const tailscaleBin = await getTailscaleBinary(); - await execWithSudoFallback( - exec, - tailscaleBin, - ["serve", "reset"], - { - maxBuffer: 200_000, - timeoutMs: 15_000, - }, - ); + await execWithSudoFallback(exec, tailscaleBin, ["serve", "reset"], { + maxBuffer: 200_000, + timeoutMs: 15_000, + }); } export async function enableTailscaleFunnel(port: number, exec: typeof runExec = runExec) { const tailscaleBin = await getTailscaleBinary(); - await execWithSudoFallback( - exec, - tailscaleBin, - ["funnel", "--bg", "--yes", `${port}`], - { - maxBuffer: 200_000, - timeoutMs: 15_000, - }, - ); + await execWithSudoFallback(exec, tailscaleBin, ["funnel", "--bg", "--yes", `${port}`], { + maxBuffer: 200_000, + timeoutMs: 15_000, + }); } export async function disableTailscaleFunnel(exec: typeof runExec = runExec) { const tailscaleBin = await getTailscaleBinary(); - await execWithSudoFallback( - exec, - tailscaleBin, - ["funnel", "reset"], - { - maxBuffer: 200_000, - timeoutMs: 15_000, - }, - ); + await execWithSudoFallback(exec, tailscaleBin, ["funnel", "reset"], { + maxBuffer: 200_000, + timeoutMs: 15_000, + }); }