From b109fa53eab415c359466df22cfa5b45b2b47896 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 07:37:11 +0000 Subject: [PATCH] refactor(core): dedupe gateway runtime and config tests --- src/commands/doctor-state-integrity.test.ts | 78 ++++----- src/config/config.hooks-module-paths.test.ts | 103 +++++++----- src/config/config.identity-defaults.test.ts | 50 +++--- src/config/sessions/sessions.test.ts | 91 ++++++---- src/daemon/runtime-paths.test.ts | 58 +++---- src/gateway/auth.test.ts | 106 ++++++------ src/gateway/client.test.ts | 62 +++---- src/gateway/openai-http.e2e.test.ts | 71 ++++---- src/gateway/server-runtime-config.test.ts | 11 ++ src/gateway/startup-auth.test.ts | 84 +++++----- src/gateway/tools-invoke-http.test.ts | 23 +-- src/hooks/install.test.ts | 28 ++-- src/infra/npm-pack-install.test.ts | 158 ++++++++++-------- src/infra/retry.test.ts | 76 ++++----- src/infra/system-run-command.test.ts | 33 ++-- src/infra/tailscale.test.ts | 54 ++++-- ...handled-rejections.fatal-detection.test.ts | 39 +++-- src/infra/watch-node.test.ts | 39 +++-- src/line/auto-reply-delivery.test.ts | 55 ++++-- src/line/webhook-node.test.ts | 41 +++-- 20 files changed, 699 insertions(+), 561 deletions(-) diff --git a/src/commands/doctor-state-integrity.test.ts b/src/commands/doctor-state-integrity.test.ts index 907a7d71a51..a72eb2cce99 100644 --- a/src/commands/doctor-state-integrity.test.ts +++ b/src/commands/doctor-state-integrity.test.ts @@ -46,6 +46,25 @@ function setupSessionState(cfg: OpenClawConfig, env: NodeJS.ProcessEnv, homeDir: fs.mkdirSync(path.dirname(storePath), { recursive: true }); } +function stateIntegrityText(): string { + return vi + .mocked(note) + .mock.calls.filter((call) => call[1] === "State integrity") + .map((call) => String(call[0])) + .join("\n"); +} + +const OAUTH_PROMPT_MATCHER = expect.objectContaining({ + message: expect.stringContaining("Create OAuth dir at"), +}); + +async function runStateIntegrity(cfg: OpenClawConfig) { + setupSessionState(cfg, process.env, process.env.HOME ?? ""); + const confirmSkipInNonInteractive = vi.fn(async () => false); + await noteStateIntegrity(cfg, { confirmSkipInNonInteractive }); + return confirmSkipInNonInteractive; +} + describe("doctor state integrity oauth dir checks", () => { let envSnapshot: EnvSnapshot; let tempHome = ""; @@ -68,23 +87,11 @@ describe("doctor state integrity oauth dir checks", () => { it("does not prompt for oauth dir when no whatsapp/pairing config is active", async () => { const cfg: OpenClawConfig = {}; - setupSessionState(cfg, process.env, tempHome); - const confirmSkipInNonInteractive = vi.fn(async () => false); - - await noteStateIntegrity(cfg, { confirmSkipInNonInteractive }); - - expect(confirmSkipInNonInteractive).not.toHaveBeenCalledWith( - expect.objectContaining({ - message: expect.stringContaining("Create OAuth dir at"), - }), - ); - const stateIntegrityText = vi - .mocked(note) - .mock.calls.filter((call) => call[1] === "State integrity") - .map((call) => String(call[0])) - .join("\n"); - expect(stateIntegrityText).toContain("OAuth dir not present"); - expect(stateIntegrityText).not.toContain("CRITICAL: OAuth dir missing"); + const confirmSkipInNonInteractive = await runStateIntegrity(cfg); + expect(confirmSkipInNonInteractive).not.toHaveBeenCalledWith(OAUTH_PROMPT_MATCHER); + const text = stateIntegrityText(); + expect(text).toContain("OAuth dir not present"); + expect(text).not.toContain("CRITICAL: OAuth dir missing"); }); it("prompts for oauth dir when whatsapp is configured", async () => { @@ -93,22 +100,9 @@ describe("doctor state integrity oauth dir checks", () => { whatsapp: {}, }, }; - setupSessionState(cfg, process.env, tempHome); - const confirmSkipInNonInteractive = vi.fn(async () => false); - - await noteStateIntegrity(cfg, { confirmSkipInNonInteractive }); - - expect(confirmSkipInNonInteractive).toHaveBeenCalledWith( - expect.objectContaining({ - message: expect.stringContaining("Create OAuth dir at"), - }), - ); - const stateIntegrityText = vi - .mocked(note) - .mock.calls.filter((call) => call[1] === "State integrity") - .map((call) => String(call[0])) - .join("\n"); - expect(stateIntegrityText).toContain("CRITICAL: OAuth dir missing"); + const confirmSkipInNonInteractive = await runStateIntegrity(cfg); + expect(confirmSkipInNonInteractive).toHaveBeenCalledWith(OAUTH_PROMPT_MATCHER); + expect(stateIntegrityText()).toContain("CRITICAL: OAuth dir missing"); }); it("prompts for oauth dir when a channel dmPolicy is pairing", async () => { @@ -119,15 +113,15 @@ describe("doctor state integrity oauth dir checks", () => { }, }, }; - setupSessionState(cfg, process.env, tempHome); - const confirmSkipInNonInteractive = vi.fn(async () => false); + const confirmSkipInNonInteractive = await runStateIntegrity(cfg); + expect(confirmSkipInNonInteractive).toHaveBeenCalledWith(OAUTH_PROMPT_MATCHER); + }); - await noteStateIntegrity(cfg, { confirmSkipInNonInteractive }); - - expect(confirmSkipInNonInteractive).toHaveBeenCalledWith( - expect.objectContaining({ - message: expect.stringContaining("Create OAuth dir at"), - }), - ); + it("prompts for oauth dir when OPENCLAW_OAUTH_DIR is explicitly configured", async () => { + process.env.OPENCLAW_OAUTH_DIR = path.join(tempHome, ".oauth"); + const cfg: OpenClawConfig = {}; + const confirmSkipInNonInteractive = await runStateIntegrity(cfg); + expect(confirmSkipInNonInteractive).toHaveBeenCalledWith(OAUTH_PROMPT_MATCHER); + expect(stateIntegrityText()).toContain("CRITICAL: OAuth dir missing"); }); }); diff --git a/src/config/config.hooks-module-paths.test.ts b/src/config/config.hooks-module-paths.test.ts index 57d949d7219..8ff4cb554ad 100644 --- a/src/config/config.hooks-module-paths.test.ts +++ b/src/config/config.hooks-module-paths.test.ts @@ -2,57 +2,78 @@ import { describe, expect, it } from "vitest"; import { validateConfigObjectWithPlugins } from "./config.js"; describe("config hooks module paths", () => { - it("rejects absolute hooks.mappings[].transform.module", () => { - const res = validateConfigObjectWithPlugins({ - agents: { list: [{ id: "pi" }] }, - hooks: { - mappings: [ - { - match: { path: "custom" }, - action: "agent", - transform: { module: "/tmp/transform.mjs" }, - }, - ], - }, - }); + const expectRejectedIssuePath = (config: Record, expectedPath: string) => { + const res = validateConfigObjectWithPlugins(config); expect(res.ok).toBe(false); - if (!res.ok) { - expect(res.issues.some((iss) => iss.path === "hooks.mappings.0.transform.module")).toBe(true); + if (res.ok) { + throw new Error("expected validation failure"); } + expect(res.issues.some((iss) => iss.path === expectedPath)).toBe(true); + }; + + it("rejects absolute hooks.mappings[].transform.module", () => { + expectRejectedIssuePath( + { + agents: { list: [{ id: "pi" }] }, + hooks: { + mappings: [ + { + match: { path: "custom" }, + action: "agent", + transform: { module: "/tmp/transform.mjs" }, + }, + ], + }, + }, + "hooks.mappings.0.transform.module", + ); }); it("rejects escaping hooks.mappings[].transform.module", () => { - const res = validateConfigObjectWithPlugins({ - agents: { list: [{ id: "pi" }] }, - hooks: { - mappings: [ - { - match: { path: "custom" }, - action: "agent", - transform: { module: "../escape.mjs" }, - }, - ], + expectRejectedIssuePath( + { + agents: { list: [{ id: "pi" }] }, + hooks: { + mappings: [ + { + match: { path: "custom" }, + action: "agent", + transform: { module: "../escape.mjs" }, + }, + ], + }, }, - }); - expect(res.ok).toBe(false); - if (!res.ok) { - expect(res.issues.some((iss) => iss.path === "hooks.mappings.0.transform.module")).toBe(true); - } + "hooks.mappings.0.transform.module", + ); }); it("rejects absolute hooks.internal.handlers[].module", () => { - const res = validateConfigObjectWithPlugins({ - agents: { list: [{ id: "pi" }] }, - hooks: { - internal: { - enabled: true, - handlers: [{ event: "command:new", module: "/tmp/handler.mjs" }], + expectRejectedIssuePath( + { + agents: { list: [{ id: "pi" }] }, + hooks: { + internal: { + enabled: true, + handlers: [{ event: "command:new", module: "/tmp/handler.mjs" }], + }, }, }, - }); - expect(res.ok).toBe(false); - if (!res.ok) { - expect(res.issues.some((iss) => iss.path === "hooks.internal.handlers.0.module")).toBe(true); - } + "hooks.internal.handlers.0.module", + ); + }); + + it("rejects escaping hooks.internal.handlers[].module", () => { + expectRejectedIssuePath( + { + agents: { list: [{ id: "pi" }] }, + hooks: { + internal: { + enabled: true, + handlers: [{ event: "command:new", module: "../handler.mjs" }], + }, + }, + }, + "hooks.internal.handlers.0.module", + ); }); }); diff --git a/src/config/config.identity-defaults.test.ts b/src/config/config.identity-defaults.test.ts index 6c3d15f9bed..5421a8dad57 100644 --- a/src/config/config.identity-defaults.test.ts +++ b/src/config/config.identity-defaults.test.ts @@ -6,6 +6,24 @@ import { loadConfig } from "./config.js"; import { withTempHome } from "./home-env.test-harness.js"; describe("config identity defaults", () => { + const defaultIdentity = { + name: "Samantha", + theme: "helpful sloth", + emoji: "🦥", + }; + + const configWithDefaultIdentity = (messages: Record) => ({ + agents: { + list: [ + { + id: "main", + identity: defaultIdentity, + }, + ], + }, + messages, + }); + const writeAndLoadConfig = async (home: string, config: Record) => { const configDir = path.join(home, ".openclaw"); await fs.mkdir(configDir, { recursive: true }); @@ -19,21 +37,7 @@ describe("config identity defaults", () => { it("does not derive mention defaults and only sets ackReactionScope when identity is present", async () => { await withTempHome("openclaw-config-identity-", async (home) => { - const cfg = await writeAndLoadConfig(home, { - agents: { - list: [ - { - id: "main", - identity: { - name: "Samantha", - theme: "helpful sloth", - emoji: "🦥", - }, - }, - ], - }, - messages: {}, - }); + const cfg = await writeAndLoadConfig(home, configWithDefaultIdentity({})); expect(cfg.messages?.responsePrefix).toBeUndefined(); expect(cfg.messages?.groupChat?.mentionPatterns).toBeUndefined(); @@ -152,21 +156,7 @@ describe("config identity defaults", () => { it("respects empty responsePrefix to disable identity defaults", async () => { await withTempHome("openclaw-config-identity-", async (home) => { - const cfg = await writeAndLoadConfig(home, { - agents: { - list: [ - { - id: "main", - identity: { - name: "Samantha", - theme: "helpful sloth", - emoji: "🦥", - }, - }, - ], - }, - messages: { responsePrefix: "" }, - }); + const cfg = await writeAndLoadConfig(home, configWithDefaultIdentity({ responsePrefix: "" })); expect(cfg.messages?.responsePrefix).toBe(""); }); diff --git a/src/config/sessions/sessions.test.ts b/src/config/sessions/sessions.test.ts index 8924a3f1054..e5b9a72d735 100644 --- a/src/config/sessions/sessions.test.ts +++ b/src/config/sessions/sessions.test.ts @@ -19,6 +19,28 @@ import { resolveSessionResetPolicy } from "./reset.js"; import { appendAssistantMessageToSessionTranscript } from "./transcript.js"; import type { SessionEntry } from "./types.js"; +function useTempSessionsFixture(prefix: string) { + let tempDir = ""; + let storePath = ""; + let sessionsDir = ""; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + sessionsDir = path.join(tempDir, "agents", "main", "sessions"); + fs.mkdirSync(sessionsDir, { recursive: true }); + storePath = path.join(sessionsDir, "sessions.json"); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + return { + storePath: () => storePath, + sessionsDir: () => sessionsDir, + }; +} + describe("session path safety", () => { it("rejects unsafe session IDs", () => { const unsafeSessionIds = ["../etc/passwd", "a/b", "a\\b", "/abs"]; @@ -148,20 +170,7 @@ describe("session store lock (Promise chain mutex)", () => { }); describe("appendAssistantMessageToSessionTranscript", () => { - let tempDir: string; - let storePath: string; - let sessionsDir: string; - - beforeEach(() => { - tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "transcript-test-")); - sessionsDir = path.join(tempDir, "agents", "main", "sessions"); - fs.mkdirSync(sessionsDir, { recursive: true }); - storePath = path.join(sessionsDir, "sessions.json"); - }); - - afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); - }); + const fixture = useTempSessionsFixture("transcript-test-"); it("creates transcript file and appends message for valid session", async () => { const sessionId = "test-session-id"; @@ -173,12 +182,12 @@ describe("appendAssistantMessageToSessionTranscript", () => { channel: "discord", }, }; - fs.writeFileSync(storePath, JSON.stringify(store), "utf-8"); + fs.writeFileSync(fixture.storePath(), JSON.stringify(store), "utf-8"); const result = await appendAssistantMessageToSessionTranscript({ sessionKey, text: "Hello from delivery mirror!", - storePath, + storePath: fixture.storePath(), }); expect(result.ok).toBe(true); @@ -206,20 +215,7 @@ describe("appendAssistantMessageToSessionTranscript", () => { }); describe("resolveAndPersistSessionFile", () => { - let tempDir: string; - let storePath: string; - let sessionsDir: string; - - beforeEach(() => { - tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "session-file-test-")); - sessionsDir = path.join(tempDir, "agents", "main", "sessions"); - fs.mkdirSync(sessionsDir, { recursive: true }); - storePath = path.join(sessionsDir, "sessions.json"); - }); - - afterEach(() => { - fs.rmSync(tempDir, { recursive: true, force: true }); - }); + const fixture = useTempSessionsFixture("session-file-test-"); it("persists fallback topic transcript paths for sessions without sessionFile", async () => { const sessionId = "topic-session-id"; @@ -230,22 +226,47 @@ describe("resolveAndPersistSessionFile", () => { updatedAt: Date.now(), }, }; - fs.writeFileSync(storePath, JSON.stringify(store), "utf-8"); - const sessionStore = loadSessionStore(storePath, { skipCache: true }); - const fallbackSessionFile = resolveSessionTranscriptPathInDir(sessionId, sessionsDir, 456); + fs.writeFileSync(fixture.storePath(), JSON.stringify(store), "utf-8"); + const sessionStore = loadSessionStore(fixture.storePath(), { skipCache: true }); + const fallbackSessionFile = resolveSessionTranscriptPathInDir( + sessionId, + fixture.sessionsDir(), + 456, + ); const result = await resolveAndPersistSessionFile({ sessionId, sessionKey, sessionStore, - storePath, + storePath: fixture.storePath(), sessionEntry: sessionStore[sessionKey], fallbackSessionFile, }); expect(result.sessionFile).toBe(fallbackSessionFile); - const saved = loadSessionStore(storePath, { skipCache: true }); + const saved = loadSessionStore(fixture.storePath(), { skipCache: true }); + expect(saved[sessionKey]?.sessionFile).toBe(fallbackSessionFile); + }); + + it("creates and persists entry when session is not yet present", async () => { + const sessionId = "new-session-id"; + const sessionKey = "agent:main:telegram:group:123"; + fs.writeFileSync(fixture.storePath(), JSON.stringify({}), "utf-8"); + const sessionStore = loadSessionStore(fixture.storePath(), { skipCache: true }); + const fallbackSessionFile = resolveSessionTranscriptPathInDir(sessionId, fixture.sessionsDir()); + + const result = await resolveAndPersistSessionFile({ + sessionId, + sessionKey, + sessionStore, + storePath: fixture.storePath(), + fallbackSessionFile, + }); + + expect(result.sessionFile).toBe(fallbackSessionFile); + expect(result.sessionEntry.sessionId).toBe(sessionId); + const saved = loadSessionStore(fixture.storePath(), { skipCache: true }); expect(saved[sessionKey]?.sessionFile).toBe(fallbackSessionFile); }); }); diff --git a/src/daemon/runtime-paths.test.ts b/src/daemon/runtime-paths.test.ts index 677bfad30ba..cd76d2da016 100644 --- a/src/daemon/runtime-paths.test.ts +++ b/src/daemon/runtime-paths.test.ts @@ -19,17 +19,21 @@ afterEach(() => { vi.resetAllMocks(); }); +function mockNodePathPresent(nodePath: string) { + fsMocks.access.mockImplementation(async (target: string) => { + if (target === nodePath) { + return; + } + throw new Error("missing"); + }); +} + describe("resolvePreferredNodePath", () => { const darwinNode = "/opt/homebrew/bin/node"; const fnmNode = "/Users/test/.fnm/node-versions/v24.11.1/installation/bin/node"; it("prefers execPath (version manager node) over system node", async () => { - fsMocks.access.mockImplementation(async (target: string) => { - if (target === darwinNode) { - return; - } - throw new Error("missing"); - }); + mockNodePathPresent(darwinNode); const execFile = vi.fn().mockResolvedValue({ stdout: "24.11.1\n", stderr: "" }); @@ -46,12 +50,7 @@ describe("resolvePreferredNodePath", () => { }); it("falls back to system node when execPath version is unsupported", async () => { - fsMocks.access.mockImplementation(async (target: string) => { - if (target === darwinNode) { - return; - } - throw new Error("missing"); - }); + mockNodePathPresent(darwinNode); const execFile = vi .fn() @@ -71,12 +70,7 @@ describe("resolvePreferredNodePath", () => { }); it("ignores execPath when it is not node", async () => { - fsMocks.access.mockImplementation(async (target: string) => { - if (target === darwinNode) { - return; - } - throw new Error("missing"); - }); + mockNodePathPresent(darwinNode); const execFile = vi.fn().mockResolvedValue({ stdout: "22.12.0\n", stderr: "" }); @@ -96,12 +90,7 @@ describe("resolvePreferredNodePath", () => { }); it("uses system node when it meets the minimum version", async () => { - fsMocks.access.mockImplementation(async (target: string) => { - if (target === darwinNode) { - return; - } - throw new Error("missing"); - }); + mockNodePathPresent(darwinNode); // Node 22.12.0+ is the minimum required version const execFile = vi.fn().mockResolvedValue({ stdout: "22.12.0\n", stderr: "" }); @@ -119,12 +108,7 @@ describe("resolvePreferredNodePath", () => { }); it("skips system node when it is too old", async () => { - fsMocks.access.mockImplementation(async (target: string) => { - if (target === darwinNode) { - return; - } - throw new Error("missing"); - }); + mockNodePathPresent(darwinNode); // Node 22.11.x is below minimum 22.12.0 const execFile = vi.fn().mockResolvedValue({ stdout: "22.11.0\n", stderr: "" }); @@ -162,12 +146,7 @@ describe("resolveSystemNodeInfo", () => { const darwinNode = "/opt/homebrew/bin/node"; it("returns supported info when version is new enough", async () => { - fsMocks.access.mockImplementation(async (target: string) => { - if (target === darwinNode) { - return; - } - throw new Error("missing"); - }); + mockNodePathPresent(darwinNode); // Node 22.12.0+ is the minimum required version const execFile = vi.fn().mockResolvedValue({ stdout: "22.12.0\n", stderr: "" }); @@ -185,6 +164,13 @@ describe("resolveSystemNodeInfo", () => { }); }); + it("returns undefined when system node is missing", async () => { + fsMocks.access.mockRejectedValue(new Error("missing")); + const execFile = vi.fn(); + const result = await resolveSystemNodeInfo({ env: {}, platform: "darwin", execFile }); + expect(result).toBeNull(); + }); + it("renders a warning when system node is too old", () => { const warning = renderSystemNodeWarning( { diff --git a/src/gateway/auth.test.ts b/src/gateway/auth.test.ts index f6525d502a5..b8376085ba1 100644 --- a/src/gateway/auth.test.ts +++ b/src/gateway/auth.test.ts @@ -46,6 +46,46 @@ function createTailscaleWhois() { } describe("gateway auth", () => { + async function expectTokenMismatchWithLimiter(params: { + reqHeaders: Record; + allowRealIpFallback?: boolean; + }) { + const limiter = createLimiterSpy(); + const res = await authorizeGatewayConnect({ + auth: { mode: "token", token: "secret", allowTailscale: false }, + connectAuth: { token: "wrong" }, + req: { + socket: { remoteAddress: "127.0.0.1" }, + headers: params.reqHeaders, + } as never, + trustedProxies: ["127.0.0.1"], + ...(params.allowRealIpFallback ? { allowRealIpFallback: true } : {}), + rateLimiter: limiter, + }); + expect(res.ok).toBe(false); + expect(res.reason).toBe("token_mismatch"); + return limiter; + } + + async function expectTailscaleHeaderAuthResult(params: { + authorize: typeof authorizeHttpGatewayConnect | typeof authorizeWsControlUiGatewayConnect; + expected: { ok: false; reason: string } | { ok: true; method: string; user: string }; + }) { + const res = await params.authorize({ + auth: { mode: "token", token: "secret", allowTailscale: true }, + connectAuth: null, + tailscaleWhois: createTailscaleWhois(), + req: createTailscaleForwardedReq(), + }); + expect(res.ok).toBe(params.expected.ok); + if (!params.expected.ok) { + expect(res.reason).toBe(params.expected.reason); + return; + } + expect(res.method).toBe(params.expected.method); + expect(res.user).toBe(params.expected.user); + } + it("resolves token/password from OPENCLAW gateway env vars", () => { expect( resolveGatewayAuth({ @@ -238,82 +278,40 @@ describe("gateway auth", () => { }); it("keeps tailscale header auth disabled on HTTP auth wrapper", async () => { - const res = await authorizeHttpGatewayConnect({ - auth: { mode: "token", token: "secret", allowTailscale: true }, - connectAuth: null, - tailscaleWhois: createTailscaleWhois(), - req: createTailscaleForwardedReq(), + await expectTailscaleHeaderAuthResult({ + authorize: authorizeHttpGatewayConnect, + expected: { ok: false, reason: "token_missing" }, }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("token_missing"); }); it("enables tailscale header auth on ws control-ui auth wrapper", async () => { - const res = await authorizeWsControlUiGatewayConnect({ - auth: { mode: "token", token: "secret", allowTailscale: true }, - connectAuth: null, - tailscaleWhois: createTailscaleWhois(), - req: createTailscaleForwardedReq(), + await expectTailscaleHeaderAuthResult({ + authorize: authorizeWsControlUiGatewayConnect, + expected: { ok: true, method: "tailscale", user: "peter" }, }); - expect(res.ok).toBe(true); - expect(res.method).toBe("tailscale"); - expect(res.user).toBe("peter"); }); it("uses proxy-aware request client IP by default for rate-limit checks", async () => { - const limiter = createLimiterSpy(); - const res = await authorizeGatewayConnect({ - auth: { mode: "token", token: "secret", allowTailscale: false }, - connectAuth: { token: "wrong" }, - req: { - socket: { remoteAddress: "127.0.0.1" }, - headers: { "x-forwarded-for": "203.0.113.10" }, - } as never, - trustedProxies: ["127.0.0.1"], - rateLimiter: limiter, + const limiter = await expectTokenMismatchWithLimiter({ + reqHeaders: { "x-forwarded-for": "203.0.113.10" }, }); - - expect(res.ok).toBe(false); - expect(res.reason).toBe("token_mismatch"); expect(limiter.check).toHaveBeenCalledWith("203.0.113.10", "shared-secret"); expect(limiter.recordFailure).toHaveBeenCalledWith("203.0.113.10", "shared-secret"); }); it("ignores X-Real-IP fallback by default for rate-limit checks", async () => { - const limiter = createLimiterSpy(); - const res = await authorizeGatewayConnect({ - auth: { mode: "token", token: "secret", allowTailscale: false }, - connectAuth: { token: "wrong" }, - req: { - socket: { remoteAddress: "127.0.0.1" }, - headers: { "x-real-ip": "203.0.113.77" }, - } as never, - trustedProxies: ["127.0.0.1"], - rateLimiter: limiter, + const limiter = await expectTokenMismatchWithLimiter({ + reqHeaders: { "x-real-ip": "203.0.113.77" }, }); - - expect(res.ok).toBe(false); - expect(res.reason).toBe("token_mismatch"); expect(limiter.check).toHaveBeenCalledWith("127.0.0.1", "shared-secret"); expect(limiter.recordFailure).toHaveBeenCalledWith("127.0.0.1", "shared-secret"); }); it("uses X-Real-IP when fallback is explicitly enabled", async () => { - const limiter = createLimiterSpy(); - const res = await authorizeGatewayConnect({ - auth: { mode: "token", token: "secret", allowTailscale: false }, - connectAuth: { token: "wrong" }, - req: { - socket: { remoteAddress: "127.0.0.1" }, - headers: { "x-real-ip": "203.0.113.77" }, - } as never, - trustedProxies: ["127.0.0.1"], + const limiter = await expectTokenMismatchWithLimiter({ + reqHeaders: { "x-real-ip": "203.0.113.77" }, allowRealIpFallback: true, - rateLimiter: limiter, }); - - expect(res.ok).toBe(false); - expect(res.reason).toBe("token_mismatch"); expect(limiter.check).toHaveBeenCalledWith("203.0.113.77", "shared-secret"); expect(limiter.recordFailure).toHaveBeenCalledWith("203.0.113.77", "shared-secret"); }); diff --git a/src/gateway/client.test.ts b/src/gateway/client.test.ts index b86d66bd9ba..bdb18f5aded 100644 --- a/src/gateway/client.test.ts +++ b/src/gateway/client.test.ts @@ -95,6 +95,22 @@ function getLatestWs(): MockWebSocket { return ws; } +function createClientWithIdentity( + deviceId: string, + onClose: (code: number, reason: string) => void, +) { + const identity: DeviceIdentity = { + deviceId, + privateKeyPem: "private-key", + publicKeyPem: "public-key", + }; + return new GatewayClient({ + url: "ws://127.0.0.1:18789", + deviceIdentity: identity, + onClose, + }); +} + describe("GatewayClient security checks", () => { beforeEach(() => { wsInstances.length = 0; @@ -177,16 +193,7 @@ describe("GatewayClient close handling", () => { it("clears stale token on device token mismatch close", () => { const onClose = vi.fn(); - const identity: DeviceIdentity = { - deviceId: "dev-1", - privateKeyPem: "private-key", - publicKeyPem: "public-key", - }; - const client = new GatewayClient({ - url: "ws://127.0.0.1:18789", - deviceIdentity: identity, - onClose, - }); + const client = createClientWithIdentity("dev-1", onClose); client.start(); getLatestWs().emitClose( @@ -208,16 +215,7 @@ describe("GatewayClient close handling", () => { throw new Error("disk unavailable"); }); const onClose = vi.fn(); - const identity: DeviceIdentity = { - deviceId: "dev-2", - privateKeyPem: "private-key", - publicKeyPem: "public-key", - }; - const client = new GatewayClient({ - url: "ws://127.0.0.1:18789", - deviceIdentity: identity, - onClose, - }); + const client = createClientWithIdentity("dev-2", onClose); client.start(); expect(() => { @@ -235,16 +233,7 @@ describe("GatewayClient close handling", () => { it("does not break close flow when pairing clear rejects", async () => { clearDevicePairingMock.mockRejectedValue(new Error("pairing store unavailable")); const onClose = vi.fn(); - const identity: DeviceIdentity = { - deviceId: "dev-3", - privateKeyPem: "private-key", - publicKeyPem: "public-key", - }; - const client = new GatewayClient({ - url: "ws://127.0.0.1:18789", - deviceIdentity: identity, - onClose, - }); + const client = createClientWithIdentity("dev-3", onClose); client.start(); expect(() => { @@ -258,4 +247,17 @@ describe("GatewayClient close handling", () => { expect(onClose).toHaveBeenCalledWith(1008, "unauthorized: device token mismatch"); client.stop(); }); + + it("does not clear auth state for non-mismatch close reasons", () => { + const onClose = vi.fn(); + const client = createClientWithIdentity("dev-4", onClose); + + client.start(); + getLatestWs().emitClose(1008, "unauthorized: signature invalid"); + + expect(clearDeviceAuthTokenMock).not.toHaveBeenCalled(); + expect(clearDevicePairingMock).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledWith(1008, "unauthorized: signature invalid"); + client.stop(); + }); }); diff --git a/src/gateway/openai-http.e2e.test.ts b/src/gateway/openai-http.e2e.test.ts index 62662b0d029..2169bf0e92b 100644 --- a/src/gateway/openai-http.e2e.test.ts +++ b/src/gateway/openai-http.e2e.test.ts @@ -58,6 +58,22 @@ async function postChatCompletions(port: number, body: unknown, headers?: Record return res; } +async function expectChatCompletionsDisabled( + start: (port: number) => Promise<{ close: (opts?: { reason?: string }) => Promise }>, +) { + const port = await getFreePort(); + const server = await start(port); + try { + const res = await postChatCompletions(port, { + model: "openclaw", + messages: [{ role: "user", content: "hi" }], + }); + expect(res.status).toBe(404); + } finally { + await server.close({ reason: "test done" }); + } +} + function parseSseDataLines(text: string): string[] { return text .split("\n") @@ -68,35 +84,12 @@ function parseSseDataLines(text: string): string[] { describe("OpenAI-compatible HTTP API (e2e)", () => { it("rejects when disabled (default + config)", { timeout: 120_000 }, async () => { - { - const port = await getFreePort(); - const server = await startServerWithDefaultConfig(port); - try { - const res = await postChatCompletions(port, { - model: "openclaw", - messages: [{ role: "user", content: "hi" }], - }); - expect(res.status).toBe(404); - } finally { - await server.close({ reason: "test done" }); - } - } - - { - const port = await getFreePort(); - const server = await startServer(port, { + await expectChatCompletionsDisabled(startServerWithDefaultConfig); + await expectChatCompletionsDisabled((port) => + startServer(port, { openAiChatCompletionsEnabled: false, - }); - try { - const res = await postChatCompletions(port, { - model: "openclaw", - messages: [{ role: "user", content: "hi" }], - }); - expect(res.status).toBe(404); - } finally { - await server.close({ reason: "test done" }); - } - } + }), + ); }); it("handles request validation and routing", async () => { @@ -133,6 +126,15 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { expect(message).toContain(line); } }; + const getFirstAgentCall = () => + (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0] as + | { + sessionKey?: string; + message?: string; + extraSystemPrompt?: string; + } + | undefined; + const getFirstAgentMessage = () => getFirstAgentCall()?.message ?? ""; try { { @@ -252,8 +254,7 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { }); expect(res.status).toBe(200); - const opts = (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0]; - const message = (opts as { message?: string } | undefined)?.message ?? ""; + const message = getFirstAgentMessage(); expectMessageContext(message, { history: ["User: Hello, who are you?", "Assistant: I am Claude."], current: ["User: What did I just ask you?"], @@ -272,8 +273,7 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { }); expect(res.status).toBe(200); - const opts = (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0]; - const message = (opts as { message?: string } | undefined)?.message ?? ""; + const message = getFirstAgentMessage(); expect(message).not.toContain(HISTORY_CONTEXT_MARKER); expect(message).not.toContain(CURRENT_MESSAGE_MARKER); expect(message).toBe("Hello"); @@ -291,9 +291,7 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { }); expect(res.status).toBe(200); - const opts = (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0]; - const extraSystemPrompt = - (opts as { extraSystemPrompt?: string } | undefined)?.extraSystemPrompt ?? ""; + const extraSystemPrompt = getFirstAgentCall()?.extraSystemPrompt ?? ""; expect(extraSystemPrompt).toBe("You are a helpful assistant."); await res.text(); } @@ -311,8 +309,7 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { }); expect(res.status).toBe(200); - const opts = (agentCommand.mock.calls[0] as unknown[] | undefined)?.[0]; - const message = (opts as { message?: string } | undefined)?.message ?? ""; + const message = getFirstAgentMessage(); expectMessageContext(message, { history: ["User: What's the weather?", "Assistant: Checking the weather."], current: ["Tool: Sunny, 70F."], diff --git a/src/gateway/server-runtime-config.test.ts b/src/gateway/server-runtime-config.test.ts index 9f7c631dea9..74e06ce41c3 100644 --- a/src/gateway/server-runtime-config.test.ts +++ b/src/gateway/server-runtime-config.test.ts @@ -49,6 +49,17 @@ describe("resolveGatewayRuntimeConfig", () => { }, expectedBindHost: "127.0.0.1", }, + { + name: "loopback binding with loopback cidr proxy", + cfg: { + gateway: { + bind: "loopback" as const, + auth: TRUSTED_PROXY_AUTH, + trustedProxies: ["127.0.0.0/8"], + }, + }, + expectedBindHost: "127.0.0.1", + }, ])("allows $name", async ({ cfg, expectedBindHost }) => { const result = await resolveGatewayRuntimeConfig({ cfg, port: 18789 }); expect(result.authMode).toBe("trusted-proxy"); diff --git a/src/gateway/startup-auth.test.ts b/src/gateway/startup-auth.test.ts index 78a389ef848..07cd724e91c 100644 --- a/src/gateway/startup-auth.test.ts +++ b/src/gateway/startup-auth.test.ts @@ -39,6 +39,19 @@ describe("ensureGatewayStartupAuth", () => { mocks.writeConfigFile.mockReset(); }); + async function expectNoTokenGeneration(cfg: OpenClawConfig, mode: string) { + const result = await ensureGatewayStartupAuth({ + cfg, + env: {} as NodeJS.ProcessEnv, + persist: true, + }); + + expect(result.generatedToken).toBeUndefined(); + expect(result.persistedGeneratedToken).toBe(false); + expect(result.auth.mode).toBe(mode); + expect(mocks.writeConfigFile).not.toHaveBeenCalled(); + } + it("generates and persists a token when startup auth is missing", async () => { const result = await ensureGatewayStartupAuth({ cfg: {}, @@ -79,64 +92,43 @@ describe("ensureGatewayStartupAuth", () => { }); it("does not generate in password mode", async () => { - const cfg: OpenClawConfig = { - gateway: { - auth: { - mode: "password", + await expectNoTokenGeneration( + { + gateway: { + auth: { + mode: "password", + }, }, }, - }; - const result = await ensureGatewayStartupAuth({ - cfg, - env: {} as NodeJS.ProcessEnv, - persist: true, - }); - - expect(result.generatedToken).toBeUndefined(); - expect(result.persistedGeneratedToken).toBe(false); - expect(result.auth.mode).toBe("password"); - expect(mocks.writeConfigFile).not.toHaveBeenCalled(); + "password", + ); }); it("does not generate in trusted-proxy mode", async () => { - const cfg: OpenClawConfig = { - gateway: { - auth: { - mode: "trusted-proxy", - trustedProxy: { userHeader: "x-forwarded-user" }, + await expectNoTokenGeneration( + { + gateway: { + auth: { + mode: "trusted-proxy", + trustedProxy: { userHeader: "x-forwarded-user" }, + }, }, }, - }; - const result = await ensureGatewayStartupAuth({ - cfg, - env: {} as NodeJS.ProcessEnv, - persist: true, - }); - - expect(result.generatedToken).toBeUndefined(); - expect(result.persistedGeneratedToken).toBe(false); - expect(result.auth.mode).toBe("trusted-proxy"); - expect(mocks.writeConfigFile).not.toHaveBeenCalled(); + "trusted-proxy", + ); }); it("does not generate in explicit none mode", async () => { - const cfg: OpenClawConfig = { - gateway: { - auth: { - mode: "none", + await expectNoTokenGeneration( + { + gateway: { + auth: { + mode: "none", + }, }, }, - }; - const result = await ensureGatewayStartupAuth({ - cfg, - env: {} as NodeJS.ProcessEnv, - persist: true, - }); - - expect(result.generatedToken).toBeUndefined(); - expect(result.persistedGeneratedToken).toBe(false); - expect(result.auth.mode).toBe("none"); - expect(mocks.writeConfigFile).not.toHaveBeenCalled(); + "none", + ); }); it("treats undefined token override as no override", async () => { diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index 648b80a1a17..3a2ec73607b 100644 --- a/src/gateway/tools-invoke-http.test.ts +++ b/src/gateway/tools-invoke-http.test.ts @@ -198,6 +198,17 @@ const allowAgentsListForMain = () => { }; }; +const postToolsInvoke = async (params: { + port: number; + headers?: Record; + body: Record; +}) => + await fetch(`http://127.0.0.1:${params.port}/tools/invoke`, { + method: "POST", + headers: { "content-type": "application/json", ...params.headers }, + body: JSON.stringify(params.body), + }); + const invokeAgentsList = async (params: { port: number; headers?: Record; @@ -207,11 +218,7 @@ const invokeAgentsList = async (params: { if (params.sessionKey) { body.sessionKey = params.sessionKey; } - return await fetch(`http://127.0.0.1:${params.port}/tools/invoke`, { - method: "POST", - headers: { "content-type": "application/json", ...params.headers }, - body: JSON.stringify(body), - }); + return await postToolsInvoke({ port: params.port, headers: params.headers, body }); }; const invokeTool = async (params: { @@ -232,11 +239,7 @@ const invokeTool = async (params: { if (params.sessionKey) { body.sessionKey = params.sessionKey; } - return await fetch(`http://127.0.0.1:${params.port}/tools/invoke`, { - method: "POST", - headers: { "content-type": "application/json", ...params.headers }, - body: JSON.stringify(body), - }); + return await postToolsInvoke({ port: params.port, headers: params.headers, body }); }; const invokeAgentsListAuthed = async (params: { sessionKey?: string } = {}) => diff --git a/src/hooks/install.test.ts b/src/hooks/install.test.ts index 9eb32f8e22b..e5eeb16c01e 100644 --- a/src/hooks/install.test.ts +++ b/src/hooks/install.test.ts @@ -71,6 +71,19 @@ async function expectUnsupportedNpmSpec( expect(result.error).toContain("unsupported npm spec"); } +function expectInstallFailureContains( + result: Awaited>, + snippets: string[], +) { + expect(result.ok).toBe(false); + if (result.ok) { + throw new Error("expected install failure"); + } + for (const snippet of snippets) { + expect(result.error).toContain(snippet); + } +} + describe("installHooksFromArchive", () => { it.each([ { @@ -125,13 +138,7 @@ describe("installHooksFromArchive", () => { archivePath: fixture.archivePath, hooksDir: fixture.hooksDir, }); - - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toContain("failed to extract archive"); - expect(result.error).toContain(tc.expectedDetail); + expectInstallFailureContains(result, ["failed to extract archive", tc.expectedDetail]); }); it.each([ @@ -149,12 +156,7 @@ describe("installHooksFromArchive", () => { archivePath: fixture.archivePath, hooksDir: fixture.hooksDir, }); - - expect(result.ok).toBe(false); - if (result.ok) { - return; - } - expect(result.error).toContain("reserved path segment"); + expectInstallFailureContains(result, ["reserved path segment"]); }); }); diff --git a/src/infra/npm-pack-install.test.ts b/src/infra/npm-pack-install.test.ts index 7378df1c98f..a0e08663b48 100644 --- a/src/infra/npm-pack-install.test.ts +++ b/src/infra/npm-pack-install.test.ts @@ -14,6 +14,62 @@ vi.mock("./install-source-utils.js", async (importOriginal) => { }); describe("installFromNpmSpecArchive", () => { + const baseSpec = "@openclaw/test@1.0.0"; + const baseArchivePath = "/tmp/openclaw-test.tgz"; + + const mockPackedSuccess = (overrides?: { + resolvedSpec?: string; + integrity?: string; + name?: string; + version?: string; + }) => { + vi.mocked(packNpmSpecToArchive).mockResolvedValue({ + ok: true, + archivePath: baseArchivePath, + metadata: { + resolvedSpec: overrides?.resolvedSpec ?? baseSpec, + integrity: overrides?.integrity ?? "sha512-same", + ...(overrides?.name ? { name: overrides.name } : {}), + ...(overrides?.version ? { version: overrides.version } : {}), + }, + }); + }; + + const runInstall = async (overrides: { + expectedIntegrity?: string; + onIntegrityDrift?: (payload: { + spec: string; + expectedIntegrity: string; + actualIntegrity: string; + resolvedSpec: string; + }) => boolean | Promise; + warn?: (message: string) => void; + installFromArchive: (params: { + archivePath: string; + }) => Promise<{ ok: boolean; [k: string]: unknown }>; + }) => + await installFromNpmSpecArchive({ + tempDirPrefix: "openclaw-test-", + spec: baseSpec, + timeoutMs: 1000, + expectedIntegrity: overrides.expectedIntegrity, + onIntegrityDrift: overrides.onIntegrityDrift, + warn: overrides.warn, + installFromArchive: overrides.installFromArchive, + }); + + const expectWrappedOkResult = ( + result: Awaited>, + installResult: Record, + ) => { + expect(result.ok).toBe(true); + if (!result.ok) { + throw new Error("expected ok result"); + } + expect(result.installResult).toEqual(installResult); + return result; + }; + beforeEach(() => { vi.mocked(packNpmSpecToArchive).mockReset(); vi.mocked(withTempDir).mockClear(); @@ -36,52 +92,45 @@ describe("installFromNpmSpecArchive", () => { }); it("returns resolution metadata and installer result on success", async () => { - vi.mocked(packNpmSpecToArchive).mockResolvedValue({ - ok: true, - archivePath: "/tmp/openclaw-test.tgz", - metadata: { - name: "@openclaw/test", - version: "1.0.0", - resolvedSpec: "@openclaw/test@1.0.0", - integrity: "sha512-same", - }, - }); + mockPackedSuccess({ name: "@openclaw/test", version: "1.0.0" }); const installFromArchive = vi.fn(async () => ({ ok: true as const, target: "done" })); - const result = await installFromNpmSpecArchive({ - tempDirPrefix: "openclaw-test-", - spec: "@openclaw/test@1.0.0", - timeoutMs: 1000, + const result = await runInstall({ expectedIntegrity: "sha512-same", installFromArchive, }); - expect(result.ok).toBe(true); - if (!result.ok) { - return; - } - expect(result.installResult).toEqual({ ok: true, target: "done" }); - expect(result.integrityDrift).toBeUndefined(); - expect(result.npmResolution.resolvedSpec).toBe("@openclaw/test@1.0.0"); - expect(result.npmResolution.resolvedAt).toBeTruthy(); + const okResult = expectWrappedOkResult(result, { ok: true, target: "done" }); + expect(okResult.integrityDrift).toBeUndefined(); + expect(okResult.npmResolution.resolvedSpec).toBe("@openclaw/test@1.0.0"); + expect(okResult.npmResolution.resolvedAt).toBeTruthy(); expect(installFromArchive).toHaveBeenCalledWith({ archivePath: "/tmp/openclaw-test.tgz" }); }); - it("aborts when integrity drift callback rejects drift", async () => { - vi.mocked(packNpmSpecToArchive).mockResolvedValue({ - ok: true, - archivePath: "/tmp/openclaw-test.tgz", - metadata: { - resolvedSpec: "@openclaw/test@1.0.0", - integrity: "sha512-new", - }, + it("proceeds when integrity drift callback accepts drift", async () => { + mockPackedSuccess({ integrity: "sha512-new" }); + const onIntegrityDrift = vi.fn(async () => true); + const installFromArchive = vi.fn(async () => ({ ok: true as const, id: "plugin-accept" })); + + const result = await runInstall({ + expectedIntegrity: "sha512-old", + onIntegrityDrift, + installFromArchive, }); + + const okResult = expectWrappedOkResult(result, { ok: true, id: "plugin-accept" }); + expect(okResult.integrityDrift).toEqual({ + expectedIntegrity: "sha512-old", + actualIntegrity: "sha512-new", + }); + expect(onIntegrityDrift).toHaveBeenCalledTimes(1); + }); + + it("aborts when integrity drift callback rejects drift", async () => { + mockPackedSuccess({ integrity: "sha512-new" }); const installFromArchive = vi.fn(async () => ({ ok: true as const })); - const result = await installFromNpmSpecArchive({ - tempDirPrefix: "openclaw-test-", - spec: "@openclaw/test@1.0.0", - timeoutMs: 1000, + const result = await runInstall({ expectedIntegrity: "sha512-old", onIntegrityDrift: async () => false, installFromArchive, @@ -95,32 +144,18 @@ describe("installFromNpmSpecArchive", () => { }); it("warns and proceeds on drift when no callback is configured", async () => { - vi.mocked(packNpmSpecToArchive).mockResolvedValue({ - ok: true, - archivePath: "/tmp/openclaw-test.tgz", - metadata: { - resolvedSpec: "@openclaw/test@1.0.0", - integrity: "sha512-new", - }, - }); + mockPackedSuccess({ integrity: "sha512-new" }); const warn = vi.fn(); const installFromArchive = vi.fn(async () => ({ ok: true as const, id: "plugin-1" })); - const result = await installFromNpmSpecArchive({ - tempDirPrefix: "openclaw-test-", - spec: "@openclaw/test@1.0.0", - timeoutMs: 1000, + const result = await runInstall({ expectedIntegrity: "sha512-old", warn, installFromArchive, }); - expect(result.ok).toBe(true); - if (!result.ok) { - return; - } - expect(result.installResult).toEqual({ ok: true, id: "plugin-1" }); - expect(result.integrityDrift).toEqual({ + const okResult = expectWrappedOkResult(result, { ok: true, id: "plugin-1" }); + expect(okResult.integrityDrift).toEqual({ expectedIntegrity: "sha512-old", actualIntegrity: "sha512-new", }); @@ -130,26 +165,15 @@ describe("installFromNpmSpecArchive", () => { }); it("returns installer failures to callers for domain-specific handling", async () => { - vi.mocked(packNpmSpecToArchive).mockResolvedValue({ - ok: true, - archivePath: "/tmp/openclaw-test.tgz", - metadata: { resolvedSpec: "@openclaw/test@1.0.0", integrity: "sha512-same" }, - }); + mockPackedSuccess({ integrity: "sha512-same" }); const installFromArchive = vi.fn(async () => ({ ok: false as const, error: "install failed" })); - const result = await installFromNpmSpecArchive({ - tempDirPrefix: "openclaw-test-", - spec: "@openclaw/test@1.0.0", - timeoutMs: 1000, + const result = await runInstall({ expectedIntegrity: "sha512-same", installFromArchive, }); - expect(result.ok).toBe(true); - if (!result.ok) { - return; - } - expect(result.installResult).toEqual({ ok: false, error: "install failed" }); - expect(result.integrityDrift).toBeUndefined(); + const okResult = expectWrappedOkResult(result, { ok: false, error: "install failed" }); + expect(okResult.integrityDrift).toBeUndefined(); }); }); diff --git a/src/infra/retry.test.ts b/src/infra/retry.test.ts index d4d66dcb792..dfba7cabd6b 100644 --- a/src/infra/retry.test.ts +++ b/src/infra/retry.test.ts @@ -1,32 +1,32 @@ import { describe, expect, it, vi } from "vitest"; import { retryAsync } from "./retry.js"; -describe("retryAsync", () => { - async function runRetryAfterCase(options: { - maxDelayMs: number; - retryAfterMs: number; - expectedDelayMs: number; - }) { - vi.useFakeTimers(); - try { - const fn = vi.fn().mockRejectedValueOnce(new Error("boom")).mockResolvedValueOnce("ok"); - const delays: number[] = []; - const promise = retryAsync(fn, { - attempts: 2, - minDelayMs: 0, - maxDelayMs: options.maxDelayMs, - jitter: 0, - retryAfterMs: () => options.retryAfterMs, - onRetry: (info) => delays.push(info.delayMs), - }); - await vi.runAllTimersAsync(); - await expect(promise).resolves.toBe("ok"); - expect(delays[0]).toBe(options.expectedDelayMs); - } finally { - vi.useRealTimers(); - } +async function runRetryAfterCase(params: { + minDelayMs: number; + maxDelayMs: number; + retryAfterMs: number; +}): Promise { + vi.useFakeTimers(); + try { + const fn = vi.fn().mockRejectedValueOnce(new Error("boom")).mockResolvedValueOnce("ok"); + const delays: number[] = []; + const promise = retryAsync(fn, { + attempts: 2, + minDelayMs: params.minDelayMs, + maxDelayMs: params.maxDelayMs, + jitter: 0, + retryAfterMs: () => params.retryAfterMs, + onRetry: (info) => delays.push(info.delayMs), + }); + await vi.runAllTimersAsync(); + await expect(promise).resolves.toBe("ok"); + return delays; + } finally { + vi.useRealTimers(); } +} +describe("retryAsync", () => { it("returns on first success", async () => { const fn = vi.fn().mockResolvedValue("ok"); const result = await retryAsync(fn, 3, 10); @@ -74,20 +74,18 @@ describe("retryAsync", () => { expect(fn).toHaveBeenCalledTimes(1); }); - it.each([ - { - name: "uses retryAfterMs when provided", - maxDelayMs: 1000, - retryAfterMs: 500, - expectedDelayMs: 500, - }, - { - name: "clamps retryAfterMs to maxDelayMs", - maxDelayMs: 100, - retryAfterMs: 500, - expectedDelayMs: 100, - }, - ])("$name", async ({ maxDelayMs, retryAfterMs, expectedDelayMs }) => { - await runRetryAfterCase({ maxDelayMs, retryAfterMs, expectedDelayMs }); + it("uses retryAfterMs when provided", async () => { + const delays = await runRetryAfterCase({ minDelayMs: 0, maxDelayMs: 1000, retryAfterMs: 500 }); + expect(delays[0]).toBe(500); + }); + + it("clamps retryAfterMs to maxDelayMs", async () => { + const delays = await runRetryAfterCase({ minDelayMs: 0, maxDelayMs: 100, retryAfterMs: 500 }); + expect(delays[0]).toBe(100); + }); + + it("clamps retryAfterMs to minDelayMs", async () => { + const delays = await runRetryAfterCase({ minDelayMs: 250, maxDelayMs: 1000, retryAfterMs: 50 }); + expect(delays[0]).toBe(250); }); }); diff --git a/src/infra/system-run-command.test.ts b/src/infra/system-run-command.test.ts index b375c07913d..74dce641fdc 100644 --- a/src/infra/system-run-command.test.ts +++ b/src/infra/system-run-command.test.ts @@ -7,6 +7,16 @@ import { } from "./system-run-command.js"; describe("system run command helpers", () => { + function expectRawCommandMismatch(params: { argv: string[]; rawCommand: string }) { + const res = validateSystemRunCommandConsistency(params); + expect(res.ok).toBe(false); + if (res.ok) { + throw new Error("unreachable"); + } + expect(res.message).toContain("rawCommand does not match command"); + expect(res.details?.code).toBe("RAW_COMMAND_MISMATCH"); + } + test("formatExecCommand quotes args with spaces", () => { expect(formatExecCommand(["echo", "hi there"])).toBe('echo "hi there"'); }); @@ -39,16 +49,10 @@ describe("system run command helpers", () => { }); test("validateSystemRunCommandConsistency rejects mismatched rawCommand vs direct argv", () => { - const res = validateSystemRunCommandConsistency({ + expectRawCommandMismatch({ argv: ["uname", "-a"], rawCommand: "echo hi", }); - expect(res.ok).toBe(false); - if (res.ok) { - throw new Error("unreachable"); - } - expect(res.message).toContain("rawCommand does not match command"); - expect(res.details?.code).toBe("RAW_COMMAND_MISMATCH"); }); test("validateSystemRunCommandConsistency accepts rawCommand matching sh wrapper argv", () => { @@ -60,16 +64,17 @@ describe("system run command helpers", () => { }); test("validateSystemRunCommandConsistency rejects cmd.exe /c trailing-arg smuggling", () => { - const res = validateSystemRunCommandConsistency({ + expectRawCommandMismatch({ argv: ["cmd.exe", "/d", "/s", "/c", "echo", "SAFE&&whoami"], rawCommand: "echo", }); - expect(res.ok).toBe(false); - if (res.ok) { - throw new Error("unreachable"); - } - expect(res.message).toContain("rawCommand does not match command"); - expect(res.details?.code).toBe("RAW_COMMAND_MISMATCH"); + }); + + test("validateSystemRunCommandConsistency rejects mismatched rawCommand vs sh wrapper argv", () => { + expectRawCommandMismatch({ + argv: ["/bin/sh", "-lc", "echo hi"], + rawCommand: "echo bye", + }); }); test("resolveSystemRunCommand requires command when rawCommand is present", () => { diff --git a/src/infra/tailscale.test.ts b/src/infra/tailscale.test.ts index ceaaf4f8461..db402e51521 100644 --- a/src/infra/tailscale.test.ts +++ b/src/infra/tailscale.test.ts @@ -12,6 +12,16 @@ const { } = tailscale; const tailscaleBin = expect.stringMatching(/tailscale$/i); +function createRuntimeWithExitError() { + return { + error: vi.fn(), + log: vi.fn(), + exit: ((code: number) => { + throw new Error(`exit ${code}`); + }) as (code: number) => never, + }; +} + describe("tailscale helpers", () => { let envSnapshot: ReturnType; @@ -46,31 +56,47 @@ describe("tailscale helpers", () => { it("ensureGoInstalled installs when missing and user agrees", async () => { const exec = vi.fn().mockRejectedValueOnce(new Error("no go")).mockResolvedValue({}); // brew install go const prompt = vi.fn().mockResolvedValue(true); - const runtime = { - error: vi.fn(), - log: vi.fn(), - exit: ((code: number) => { - throw new Error(`exit ${code}`); - }) as (code: number) => never, - }; + const runtime = createRuntimeWithExitError(); await ensureGoInstalled(exec as never, prompt, runtime); expect(exec).toHaveBeenCalledWith("brew", ["install", "go"]); }); + it("ensureGoInstalled exits when missing and user declines install", async () => { + const exec = vi.fn().mockRejectedValueOnce(new Error("no go")); + const prompt = vi.fn().mockResolvedValue(false); + const runtime = createRuntimeWithExitError(); + + await expect(ensureGoInstalled(exec as never, prompt, runtime)).rejects.toThrow("exit 1"); + + expect(runtime.error).toHaveBeenCalledWith( + "Go is required to build tailscaled from source. Aborting.", + ); + expect(exec).toHaveBeenCalledTimes(1); + }); + it("ensureTailscaledInstalled installs when missing and user agrees", async () => { const exec = vi.fn().mockRejectedValueOnce(new Error("missing")).mockResolvedValue({}); const prompt = vi.fn().mockResolvedValue(true); - const runtime = { - error: vi.fn(), - log: vi.fn(), - exit: ((code: number) => { - throw new Error(`exit ${code}`); - }) as (code: number) => never, - }; + const runtime = createRuntimeWithExitError(); await ensureTailscaledInstalled(exec as never, prompt, runtime); expect(exec).toHaveBeenCalledWith("brew", ["install", "tailscale"]); }); + it("ensureTailscaledInstalled exits when missing and user declines install", async () => { + const exec = vi.fn().mockRejectedValueOnce(new Error("missing")); + const prompt = vi.fn().mockResolvedValue(false); + const runtime = createRuntimeWithExitError(); + + await expect(ensureTailscaledInstalled(exec as never, prompt, runtime)).rejects.toThrow( + "exit 1", + ); + + expect(runtime.error).toHaveBeenCalledWith( + "tailscaled is required for user-space funnel. Aborting.", + ); + expect(exec).toHaveBeenCalledTimes(1); + }); + it("enableTailscaleServe attempts normal first, then sudo", async () => { // 1. First attempt fails // 2. Second attempt (sudo) succeeds diff --git a/src/infra/unhandled-rejections.fatal-detection.test.ts b/src/infra/unhandled-rejections.fatal-detection.test.ts index 912cab55fd8..6d5f3f5e9f0 100644 --- a/src/infra/unhandled-rejections.fatal-detection.test.ts +++ b/src/infra/unhandled-rejections.fatal-detection.test.ts @@ -37,6 +37,16 @@ describe("installUnhandledRejectionHandler - fatal detection", () => { process.exit = originalExit; }); + function emitUnhandled(reason: unknown): void { + process.emit("unhandledRejection", reason, Promise.resolve()); + } + + function expectExitCodeFromUnhandled(reason: unknown, expected: number[]): void { + exitCalls = []; + emitUnhandled(reason); + expect(exitCalls).toEqual(expected); + } + describe("fatal errors", () => { it("exits on fatal runtime codes", () => { const fatalCases = [ @@ -46,10 +56,7 @@ describe("installUnhandledRejectionHandler - fatal detection", () => { ] as const; for (const { code, message } of fatalCases) { - exitCalls = []; - const err = Object.assign(new Error(message), { code }); - process.emit("unhandledRejection", err, Promise.resolve()); - expect(exitCalls).toEqual([1]); + expectExitCodeFromUnhandled(Object.assign(new Error(message), { code }), [1]); } expect(consoleErrorSpy).toHaveBeenCalledWith( @@ -67,10 +74,7 @@ describe("installUnhandledRejectionHandler - fatal detection", () => { ] as const; for (const { code, message } of configurationCases) { - exitCalls = []; - const err = Object.assign(new Error(message), { code }); - process.emit("unhandledRejection", err, Promise.resolve()); - expect(exitCalls).toEqual([1]); + expectExitCodeFromUnhandled(Object.assign(new Error(message), { code }), [1]); } expect(consoleErrorSpy).toHaveBeenCalledWith( @@ -92,9 +96,7 @@ describe("installUnhandledRejectionHandler - fatal detection", () => { ]; for (const transientErr of transientCases) { - exitCalls = []; - process.emit("unhandledRejection", transientErr, Promise.resolve()); - expect(exitCalls).toEqual([]); + expectExitCodeFromUnhandled(transientErr, []); } expect(consoleWarnSpy).toHaveBeenCalledWith( @@ -106,13 +108,22 @@ describe("installUnhandledRejectionHandler - fatal detection", () => { it("exits on generic errors without code", () => { const genericErr = new Error("Something went wrong"); - process.emit("unhandledRejection", genericErr, Promise.resolve()); - - expect(exitCalls).toEqual([1]); + expectExitCodeFromUnhandled(genericErr, [1]); expect(consoleErrorSpy).toHaveBeenCalledWith( "[openclaw] Unhandled promise rejection:", expect.stringContaining("Something went wrong"), ); }); + + it("does not exit on AbortError and logs suppression warning", () => { + const abortErr = new Error("This operation was aborted"); + abortErr.name = "AbortError"; + + expectExitCodeFromUnhandled(abortErr, []); + expect(consoleWarnSpy).toHaveBeenCalledWith( + "[openclaw] Suppressed AbortError:", + expect.stringContaining("This operation was aborted"), + ); + }); }); }); diff --git a/src/infra/watch-node.test.ts b/src/infra/watch-node.test.ts index c7f75c662ea..69adbab7fc4 100644 --- a/src/infra/watch-node.test.ts +++ b/src/infra/watch-node.test.ts @@ -9,13 +9,18 @@ const createFakeProcess = () => execPath: "/usr/local/bin/node", }) as unknown as NodeJS.Process; +const createWatchHarness = () => { + const child = Object.assign(new EventEmitter(), { + kill: vi.fn(), + }); + const spawn = vi.fn(() => child); + const fakeProcess = createFakeProcess(); + return { child, spawn, fakeProcess }; +}; + describe("watch-node script", () => { it("wires node watch to run-node with watched source/config paths", async () => { - const child = Object.assign(new EventEmitter(), { - kill: vi.fn(), - }); - const spawn = vi.fn(() => child); - const fakeProcess = createFakeProcess(); + const { child, spawn, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], @@ -54,11 +59,7 @@ describe("watch-node script", () => { }); it("terminates child on SIGINT and returns shell interrupt code", async () => { - const child = Object.assign(new EventEmitter(), { - kill: vi.fn(), - }); - const spawn = vi.fn(() => child); - const fakeProcess = createFakeProcess(); + const { child, spawn, fakeProcess } = createWatchHarness(); const runPromise = runWatchMain({ args: ["gateway", "--force"], @@ -74,4 +75,22 @@ describe("watch-node script", () => { expect(fakeProcess.listenerCount("SIGINT")).toBe(0); expect(fakeProcess.listenerCount("SIGTERM")).toBe(0); }); + + it("terminates child on SIGTERM and returns shell terminate code", async () => { + const { child, spawn, fakeProcess } = createWatchHarness(); + + const runPromise = runWatchMain({ + args: ["gateway", "--force"], + process: fakeProcess, + spawn, + }); + + fakeProcess.emit("SIGTERM"); + const exitCode = await runPromise; + + expect(exitCode).toBe(143); + expect(child.kill).toHaveBeenCalledWith("SIGTERM"); + expect(fakeProcess.listenerCount("SIGINT")).toBe(0); + expect(fakeProcess.listenerCount("SIGTERM")).toBe(0); + }); }); diff --git a/src/line/auto-reply-delivery.test.ts b/src/line/auto-reply-delivery.test.ts index a75d5f42756..40371393a2b 100644 --- a/src/line/auto-reply-delivery.test.ts +++ b/src/line/auto-reply-delivery.test.ts @@ -26,6 +26,14 @@ const createLocationMessage = (location: { }); describe("deliverLineAutoReply", () => { + const baseDeliveryParams = { + to: "line:user:1", + replyToken: "token", + replyTokenUsed: false, + accountId: "acc", + textLimit: 5000, + }; + function createDeps(overrides?: Partial) { const replyMessageLine = vi.fn(async () => ({})); const pushMessageLine = vi.fn(async () => ({})); @@ -72,13 +80,9 @@ describe("deliverLineAutoReply", () => { const { deps, replyMessageLine, pushMessagesLine, createQuickReplyItems } = createDeps(); const result = await deliverLineAutoReply({ + ...baseDeliveryParams, payload: { text: "hello", channelData: { line: lineData } }, lineData, - to: "line:user:1", - replyToken: "token", - replyTokenUsed: false, - accountId: "acc", - textLimit: 5000, deps, }); @@ -108,13 +112,9 @@ describe("deliverLineAutoReply", () => { }); const result = await deliverLineAutoReply({ + ...baseDeliveryParams, payload: { channelData: { line: lineData } }, lineData, - to: "line:user:1", - replyToken: "token", - replyTokenUsed: false, - accountId: "acc", - textLimit: 5000, deps, }); @@ -151,13 +151,9 @@ describe("deliverLineAutoReply", () => { }); await deliverLineAutoReply({ + ...baseDeliveryParams, payload: { text: "hello", channelData: { line: lineData } }, lineData, - to: "line:user:1", - replyToken: "token", - replyTokenUsed: false, - accountId: "acc", - textLimit: 5000, deps, }); @@ -181,4 +177,33 @@ describe("deliverLineAutoReply", () => { const replyOrder = replyMessageLine.mock.invocationCallOrder[0]; expect(pushOrder).toBeLessThan(replyOrder); }); + + it("falls back to push when reply token delivery fails", async () => { + const lineData = { + flexMessage: { altText: "Card", contents: { type: "bubble" } }, + }; + const failingReplyMessageLine = vi.fn(async () => { + throw new Error("reply failed"); + }); + const { deps, pushMessagesLine } = createDeps({ + processLineMessage: () => ({ text: "", flexMessages: [] }), + chunkMarkdownText: () => [], + replyMessageLine: failingReplyMessageLine as LineAutoReplyDeps["replyMessageLine"], + }); + + const result = await deliverLineAutoReply({ + ...baseDeliveryParams, + payload: { channelData: { line: lineData } }, + lineData, + deps, + }); + + expect(result.replyTokenUsed).toBe(true); + expect(failingReplyMessageLine).toHaveBeenCalledTimes(1); + expect(pushMessagesLine).toHaveBeenCalledWith( + "line:user:1", + [createFlexMessage("Card", { type: "bubble" })], + { accountId: "acc" }, + ); + }); }); diff --git a/src/line/webhook-node.test.ts b/src/line/webhook-node.test.ts index 27b489ae672..c3840ec92df 100644 --- a/src/line/webhook-node.test.ts +++ b/src/line/webhook-node.test.ts @@ -37,6 +37,20 @@ function createPostWebhookTestHarness(rawBody: string, secret = "secret") { return { bot, handler, secret }; } +const runSignedPost = async (params: { + handler: (req: IncomingMessage, res: ServerResponse) => Promise; + rawBody: string; + secret: string; + res: ServerResponse; +}) => + await params.handler( + { + method: "POST", + headers: { "x-line-signature": sign(params.rawBody, params.secret) }, + } as unknown as IncomingMessage, + params.res, + ); + describe("createLineNodeWebhookHandler", () => { it("returns 200 for GET", async () => { const bot = { handleWebhook: vi.fn(async () => {}) }; @@ -68,6 +82,17 @@ describe("createLineNodeWebhookHandler", () => { expect(bot.handleWebhook).not.toHaveBeenCalled(); }); + it("returns 405 for non-GET/non-POST methods", async () => { + const { bot, handler } = createPostWebhookTestHarness(JSON.stringify({ events: [] })); + + const { res, headers } = createRes(); + await handler({ method: "PUT", headers: {} } as unknown as IncomingMessage, res); + + expect(res.statusCode).toBe(405); + expect(headers.allow).toBe("GET, POST"); + expect(bot.handleWebhook).not.toHaveBeenCalled(); + }); + it("rejects missing signature when events are non-empty", async () => { const rawBody = JSON.stringify({ events: [{ type: "message" }] }); const { bot, handler } = createPostWebhookTestHarness(rawBody); @@ -98,13 +123,7 @@ describe("createLineNodeWebhookHandler", () => { const { bot, handler, secret } = createPostWebhookTestHarness(rawBody); const { res } = createRes(); - await handler( - { - method: "POST", - headers: { "x-line-signature": sign(rawBody, secret) }, - } as unknown as IncomingMessage, - res, - ); + await runSignedPost({ handler, rawBody, secret, res }); expect(res.statusCode).toBe(200); expect(bot.handleWebhook).toHaveBeenCalledWith( @@ -117,13 +136,7 @@ describe("createLineNodeWebhookHandler", () => { const { bot, handler, secret } = createPostWebhookTestHarness(rawBody); const { res } = createRes(); - await handler( - { - method: "POST", - headers: { "x-line-signature": sign(rawBody, secret) }, - } as unknown as IncomingMessage, - res, - ); + await runSignedPost({ handler, rawBody, secret, res }); expect(res.statusCode).toBe(400); expect(bot.handleWebhook).not.toHaveBeenCalled();