From 8f4e77e72f4b0dabd2ef9a40076e99cb9a60f9e5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 18:06:39 +0000 Subject: [PATCH] test: tighten channel auth and network coverage --- src/line/accounts.test.ts | 172 +++++++++++++++++++--------- src/telegram/network-errors.test.ts | 135 +++++++++------------- 2 files changed, 172 insertions(+), 135 deletions(-) diff --git a/src/line/accounts.test.ts b/src/line/accounts.test.ts index 06433f6f8e7..9a15afc6cd9 100644 --- a/src/line/accounts.test.ts +++ b/src/line/accounts.test.ts @@ -12,6 +12,15 @@ import { describe("LINE accounts", () => { const originalEnv = { ...process.env }; + const tempDirs: string[] = []; + + const createSecretFile = (fileName: string, contents: string) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-line-account-")); + tempDirs.push(dir); + const filePath = path.join(dir, fileName); + fs.writeFileSync(filePath, contents, "utf8"); + return filePath; + }; beforeEach(() => { process.env = { ...originalEnv }; @@ -21,6 +30,9 @@ describe("LINE accounts", () => { afterEach(() => { process.env = originalEnv; + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } }); describe("resolveLineAccount", () => { @@ -101,8 +113,47 @@ describe("LINE accounts", () => { expect(account.tokenSource).toBe("none"); }); + it("resolves default account credentials from files", () => { + const cfg: OpenClawConfig = { + channels: { + line: { + tokenFile: createSecretFile("token.txt", "file-token\n"), + secretFile: createSecretFile("secret.txt", "file-secret\n"), + }, + }, + }; + + const account = resolveLineAccount({ cfg }); + + expect(account.channelAccessToken).toBe("file-token"); + expect(account.channelSecret).toBe("file-secret"); + expect(account.tokenSource).toBe("file"); + }); + + it("resolves named account credentials from account-level files", () => { + const cfg: OpenClawConfig = { + channels: { + line: { + accounts: { + business: { + tokenFile: createSecretFile("business-token.txt", "business-file-token\n"), + secretFile: createSecretFile("business-secret.txt", "business-file-secret\n"), + }, + }, + }, + }, + }; + + const account = resolveLineAccount({ cfg, accountId: "business" }); + + expect(account.channelAccessToken).toBe("business-file-token"); + expect(account.channelSecret).toBe("business-file-secret"); + expect(account.tokenSource).toBe("file"); + }); + it.runIf(process.platform !== "win32")("rejects symlinked token and secret files", () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-line-account-")); + tempDirs.push(dir); const tokenFile = path.join(dir, "token.txt"); const tokenLink = path.join(dir, "token-link.txt"); const secretFile = path.join(dir, "secret.txt"); @@ -125,74 +176,83 @@ describe("LINE accounts", () => { expect(account.channelAccessToken).toBe(""); expect(account.channelSecret).toBe(""); expect(account.tokenSource).toBe("none"); - fs.rmSync(dir, { recursive: true, force: true }); }); }); describe("resolveDefaultLineAccountId", () => { - it("prefers channels.line.defaultAccount when configured", () => { - const cfg: OpenClawConfig = { - channels: { - line: { - defaultAccount: "business", - accounts: { - business: { enabled: true }, - support: { enabled: true }, + it.each([ + { + name: "prefers channels.line.defaultAccount when configured", + cfg: { + channels: { + line: { + defaultAccount: "business", + accounts: { + business: { enabled: true }, + support: { enabled: true }, + }, }, }, - }, - }; - - const id = resolveDefaultLineAccountId(cfg); - expect(id).toBe("business"); - }); - - it("normalizes channels.line.defaultAccount before lookup", () => { - const cfg: OpenClawConfig = { - channels: { - line: { - defaultAccount: "Business Ops", - accounts: { - "business-ops": { enabled: true }, + } satisfies OpenClawConfig, + expected: "business", + }, + { + name: "normalizes channels.line.defaultAccount before lookup", + cfg: { + channels: { + line: { + defaultAccount: "Business Ops", + accounts: { + "business-ops": { enabled: true }, + }, }, }, - }, - }; - - const id = resolveDefaultLineAccountId(cfg); - expect(id).toBe("business-ops"); - }); - - it("returns first named account when default not configured", () => { - const cfg: OpenClawConfig = { - channels: { - line: { - accounts: { - business: { enabled: true }, + } satisfies OpenClawConfig, + expected: "business-ops", + }, + { + name: "returns first named account when default not configured", + cfg: { + channels: { + line: { + accounts: { + business: { enabled: true }, + }, }, }, - }, - }; - - const id = resolveDefaultLineAccountId(cfg); - - expect(id).toBe("business"); - }); - - it("falls back when channels.line.defaultAccount is missing", () => { - const cfg: OpenClawConfig = { - channels: { - line: { - defaultAccount: "missing", - accounts: { - business: { enabled: true }, + } satisfies OpenClawConfig, + expected: "business", + }, + { + name: "falls back when channels.line.defaultAccount is missing", + cfg: { + channels: { + line: { + defaultAccount: "missing", + accounts: { + business: { enabled: true }, + }, }, }, - }, - }; - - const id = resolveDefaultLineAccountId(cfg); - expect(id).toBe("business"); + } satisfies OpenClawConfig, + expected: "business", + }, + { + name: "prefers the default account when base credentials are configured", + cfg: { + channels: { + line: { + channelAccessToken: "base-token", + accounts: { + business: { enabled: true }, + }, + }, + }, + } satisfies OpenClawConfig, + expected: DEFAULT_ACCOUNT_ID, + }, + ])("$name", ({ cfg, expected }) => { + expect(resolveDefaultLineAccountId(cfg)).toBe(expected); }); }); diff --git a/src/telegram/network-errors.test.ts b/src/telegram/network-errors.test.ts index 56106a292b8..f8437aa2a2f 100644 --- a/src/telegram/network-errors.test.ts +++ b/src/telegram/network-errors.test.ts @@ -9,6 +9,11 @@ import { tagTelegramNetworkError, } from "./network-errors.js"; +const errorWithCode = (message: string, code: string) => + Object.assign(new Error(message), { code }); +const errorWithTelegramCode = (message: string, error_code: number) => + Object.assign(new Error(message), { error_code }); + describe("isRecoverableTelegramNetworkError", () => { it("tracks Telegram polling origin separately from generic network matching", () => { const slackDnsError = Object.assign( @@ -32,16 +37,12 @@ describe("isRecoverableTelegramNetworkError", () => { expect(isTelegramPollingNetworkError(slackDnsError)).toBe(true); }); - it("detects recoverable error codes", () => { - const err = Object.assign(new Error("timeout"), { code: "ETIMEDOUT" }); - expect(isRecoverableTelegramNetworkError(err)).toBe(true); - }); - - it("detects additional recoverable error codes", () => { - const aborted = Object.assign(new Error("aborted"), { code: "ECONNABORTED" }); - const network = Object.assign(new Error("network"), { code: "ERR_NETWORK" }); - expect(isRecoverableTelegramNetworkError(aborted)).toBe(true); - expect(isRecoverableTelegramNetworkError(network)).toBe(true); + it.each([ + ["ETIMEDOUT", "timeout"], + ["ECONNABORTED", "aborted"], + ["ERR_NETWORK", "network"], + ])("detects recoverable error code %s", (code, message) => { + expect(isRecoverableTelegramNetworkError(errorWithCode(message, code))).toBe(true); }); it("detects AbortError names", () => { @@ -69,6 +70,19 @@ describe("isRecoverableTelegramNetworkError", () => { expect(isRecoverableTelegramNetworkError(err, { context: "polling" })).toBe(true); }); + it("honors allowMessageMatch=false for broad snippet matches", () => { + expect( + isRecoverableTelegramNetworkError(new Error("Undici: socket failure"), { + allowMessageMatch: false, + }), + ).toBe(false); + expect( + isRecoverableTelegramNetworkError(new Error("TypeError: fetch failed"), { + allowMessageMatch: false, + }), + ).toBe(true); + }); + it("skips broad message matches for send context", () => { const networkRequestErr = new Error("Network request for 'sendMessage' failed!"); expect(isRecoverableTelegramNetworkError(networkRequestErr, { context: "send" })).toBe(false); @@ -97,6 +111,14 @@ describe("isRecoverableTelegramNetworkError", () => { expect(isRecoverableTelegramNetworkError(err)).toBe(true); }); + it("normalizes blank tagged origins to null and finds nested tags", () => { + const inner = new Error("inner"); + tagTelegramNetworkError(inner, { method: " ", url: " " }); + const outer = Object.assign(new Error("outer"), { cause: inner }); + expect(getTelegramNetworkErrorOrigin(outer)).toEqual({ method: null, url: null }); + expect(isTelegramPollingNetworkError(outer)).toBe(false); + }); + // Grammy HttpError tests (issue #3815) // Grammy wraps fetch errors in .error property, not .cause describe("Grammy HttpError", () => { @@ -138,49 +160,18 @@ describe("isRecoverableTelegramNetworkError", () => { }); describe("isSafeToRetrySendError", () => { - it("allows retry for ECONNREFUSED (pre-connect, message not sent)", () => { - const err = Object.assign(new Error("connect ECONNREFUSED"), { code: "ECONNREFUSED" }); - expect(isSafeToRetrySendError(err)).toBe(true); - }); - - it("allows retry for ENOTFOUND (DNS failure, message not sent)", () => { - const err = Object.assign(new Error("getaddrinfo ENOTFOUND"), { code: "ENOTFOUND" }); - expect(isSafeToRetrySendError(err)).toBe(true); - }); - - it("allows retry for EAI_AGAIN (transient DNS, message not sent)", () => { - const err = Object.assign(new Error("getaddrinfo EAI_AGAIN"), { code: "EAI_AGAIN" }); - expect(isSafeToRetrySendError(err)).toBe(true); - }); - - it("allows retry for ENETUNREACH (no route to host, message not sent)", () => { - const err = Object.assign(new Error("connect ENETUNREACH"), { code: "ENETUNREACH" }); - expect(isSafeToRetrySendError(err)).toBe(true); - }); - - it("allows retry for EHOSTUNREACH (host unreachable, message not sent)", () => { - const err = Object.assign(new Error("connect EHOSTUNREACH"), { code: "EHOSTUNREACH" }); - expect(isSafeToRetrySendError(err)).toBe(true); - }); - - it("does NOT allow retry for ECONNRESET (message may already be delivered)", () => { - const err = Object.assign(new Error("read ECONNRESET"), { code: "ECONNRESET" }); - expect(isSafeToRetrySendError(err)).toBe(false); - }); - - it("does NOT allow retry for ETIMEDOUT (message may already be delivered)", () => { - const err = Object.assign(new Error("connect ETIMEDOUT"), { code: "ETIMEDOUT" }); - expect(isSafeToRetrySendError(err)).toBe(false); - }); - - it("does NOT allow retry for EPIPE (connection broken mid-transfer, message may be delivered)", () => { - const err = Object.assign(new Error("write EPIPE"), { code: "EPIPE" }); - expect(isSafeToRetrySendError(err)).toBe(false); - }); - - it("does NOT allow retry for UND_ERR_CONNECT_TIMEOUT (ambiguous timing)", () => { - const err = Object.assign(new Error("connect timeout"), { code: "UND_ERR_CONNECT_TIMEOUT" }); - expect(isSafeToRetrySendError(err)).toBe(false); + it.each([ + ["ECONNREFUSED", "connect ECONNREFUSED", true], + ["ENOTFOUND", "getaddrinfo ENOTFOUND", true], + ["EAI_AGAIN", "getaddrinfo EAI_AGAIN", true], + ["ENETUNREACH", "connect ENETUNREACH", true], + ["EHOSTUNREACH", "connect EHOSTUNREACH", true], + ["ECONNRESET", "read ECONNRESET", false], + ["ETIMEDOUT", "connect ETIMEDOUT", false], + ["EPIPE", "write EPIPE", false], + ["UND_ERR_CONNECT_TIMEOUT", "connect timeout", false], + ])("returns %s => %s", (code, message, expected) => { + expect(isSafeToRetrySendError(errorWithCode(message, code))).toBe(expected); }); it("does NOT allow retry for non-network errors", () => { @@ -196,19 +187,12 @@ describe("isSafeToRetrySendError", () => { }); describe("isTelegramServerError", () => { - it("returns true for error_code 500", () => { - const err = Object.assign(new Error("Internal Server Error"), { error_code: 500 }); - expect(isTelegramServerError(err)).toBe(true); - }); - - it("returns true for error_code 502", () => { - const err = Object.assign(new Error("Bad Gateway"), { error_code: 502 }); - expect(isTelegramServerError(err)).toBe(true); - }); - - it("returns false for error_code 403", () => { - const err = Object.assign(new Error("Forbidden"), { error_code: 403 }); - expect(isTelegramServerError(err)).toBe(false); + it.each([ + ["Internal Server Error", 500, true], + ["Bad Gateway", 502, true], + ["Forbidden", 403, false], + ])("returns %s for error_code %s", (message, errorCode, expected) => { + expect(isTelegramServerError(errorWithTelegramCode(message, errorCode))).toBe(expected); }); it("returns false for plain Error", () => { @@ -217,19 +201,12 @@ describe("isTelegramServerError", () => { }); describe("isTelegramClientRejection", () => { - it("returns true for error_code 400", () => { - const err = Object.assign(new Error("Bad Request"), { error_code: 400 }); - expect(isTelegramClientRejection(err)).toBe(true); - }); - - it("returns true for error_code 403", () => { - const err = Object.assign(new Error("Forbidden"), { error_code: 403 }); - expect(isTelegramClientRejection(err)).toBe(true); - }); - - it("returns false for error_code 502", () => { - const err = Object.assign(new Error("Bad Gateway"), { error_code: 502 }); - expect(isTelegramClientRejection(err)).toBe(false); + it.each([ + ["Bad Request", 400, true], + ["Forbidden", 403, true], + ["Bad Gateway", 502, false], + ])("returns %s for error_code %s", (message, errorCode, expected) => { + expect(isTelegramClientRejection(errorWithTelegramCode(message, errorCode))).toBe(expected); }); it("returns false for plain Error", () => {