From 57e1534df8b54da342d243471ece4b5c845170a3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 3 Mar 2026 01:05:30 +0000 Subject: [PATCH] refactor(tests): consolidate repeated setup helpers --- src/agents/openai-ws-connection.test.ts | 88 +++---- src/agents/sessions-spawn-hooks.test.ts | 104 +++++---- .../auth-choice.apply-helpers.test.ts | 156 ++++++++----- src/commands/gateway-status.test.ts | 103 ++++---- src/config/env-substitution.test.ts | 128 ++++------ src/pairing/pairing-store.test.ts | 221 ++++++++++-------- src/secrets/runtime.test.ts | 139 ++++++----- src/security/windows-acl.test.ts | 79 +++---- 8 files changed, 494 insertions(+), 524 deletions(-) diff --git a/src/agents/openai-ws-connection.test.ts b/src/agents/openai-ws-connection.test.ts index 13769bd65..64afd9d0b 100644 --- a/src/agents/openai-ws-connection.test.ts +++ b/src/agents/openai-ws-connection.test.ts @@ -185,6 +185,20 @@ async function connectManagerAndGetSocket(manager: OpenAIWebSocketManager) { return sock; } +async function createConnectedManager( + opts?: ConstructorParameters[0], +): Promise<{ manager: OpenAIWebSocketManager; sock: MockWS }> { + const manager = buildManager(opts); + const sock = await connectManagerAndGetSocket(manager); + return { manager, sock }; +} + +function connectIgnoringFailure(manager: OpenAIWebSocketManager): Promise { + return manager.connect("sk-test").catch(() => { + /* ignore rejection */ + }); +} + // ───────────────────────────────────────────────────────────────────────────── // Tests // ───────────────────────────────────────────────────────────────────────────── @@ -259,11 +273,7 @@ describe("OpenAIWebSocketManager", () => { describe("send()", () => { it("sends a JSON-serialized event over the socket", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); const event: ResponseCreateEvent = { type: "response.create", @@ -286,11 +296,7 @@ describe("OpenAIWebSocketManager", () => { }); it("includes previous_response_id when provided", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); const event: ResponseCreateEvent = { type: "response.create", @@ -309,11 +315,7 @@ describe("OpenAIWebSocketManager", () => { describe("onMessage()", () => { it("calls handler for each incoming message", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); const received: OpenAIWebSocketEvent[] = []; manager.onMessage((e) => received.push(e)); @@ -332,11 +334,7 @@ describe("OpenAIWebSocketManager", () => { }); it("returns an unsubscribe function that stops delivery", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); const received: OpenAIWebSocketEvent[] = []; const unsubscribe = manager.onMessage((e) => received.push(e)); @@ -349,11 +347,7 @@ describe("OpenAIWebSocketManager", () => { }); it("supports multiple simultaneous handlers", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); const calls: number[] = []; manager.onMessage(() => calls.push(1)); @@ -373,11 +367,7 @@ describe("OpenAIWebSocketManager", () => { }); it("is updated when a response.completed event is received", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); const completedEvent: ResponseCompletedEvent = { type: "response.completed", @@ -389,11 +379,7 @@ describe("OpenAIWebSocketManager", () => { }); it("tracks the most recent completed response", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); sock.simulateMessage({ type: "response.completed", @@ -408,11 +394,7 @@ describe("OpenAIWebSocketManager", () => { }); it("is not updated for non-completed events", async () => { - const manager = buildManager(); - const connectPromise = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await connectPromise; + const { manager, sock } = await createConnectedManager(); sock.simulateMessage({ type: "response.in_progress", response: makeResponse("resp_x") }); @@ -549,11 +531,7 @@ describe("OpenAIWebSocketManager", () => { describe("warmUp()", () => { it("sends a response.create event with generate: false", async () => { - const manager = buildManager(); - const p = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await p; + const { manager, sock } = await createConnectedManager(); manager.warmUp({ model: "gpt-5.2", instructions: "You are helpful." }); @@ -566,11 +544,7 @@ describe("OpenAIWebSocketManager", () => { }); it("includes tools when provided", async () => { - const manager = buildManager(); - const p = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await p; + const { manager, sock } = await createConnectedManager(); manager.warmUp({ model: "gpt-5.2", @@ -612,9 +586,7 @@ describe("OpenAIWebSocketManager", () => { it("emits error event on WebSocket socket error", async () => { const manager = buildManager({ maxRetries: 0 }); - const p = manager.connect("sk-test").catch(() => { - /* ignore rejection */ - }); + const p = connectIgnoringFailure(manager); const errors = attachErrorCollector(manager); lastSocket().simulateError(new Error("SSL handshake failed")); @@ -625,9 +597,7 @@ describe("OpenAIWebSocketManager", () => { it("handles multiple successive socket errors without crashing", async () => { const manager = buildManager({ maxRetries: 0 }); - const p = manager.connect("sk-test").catch(() => { - /* ignore rejection */ - }); + const p = connectIgnoringFailure(manager); const errors = attachErrorCollector(manager); // Fire two errors in quick succession — previously the second would @@ -646,11 +616,7 @@ describe("OpenAIWebSocketManager", () => { describe("full turn sequence", () => { it("tracks previous_response_id across turns and sends continuation correctly", async () => { - const manager = buildManager(); - const p = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await p; + const { manager, sock } = await createConnectedManager(); const received: OpenAIWebSocketEvent[] = []; manager.onMessage((e) => received.push(e)); diff --git a/src/agents/sessions-spawn-hooks.test.ts b/src/agents/sessions-spawn-hooks.test.ts index 41f87dd33..793fd8c6a 100644 --- a/src/agents/sessions-spawn-hooks.test.ts +++ b/src/agents/sessions-spawn-hooks.test.ts @@ -87,6 +87,52 @@ async function runSessionThreadSpawnAndGetError(params: { return result.details as { error?: string; childSessionKey?: string }; } +async function getDiscordThreadSessionTool() { + return await getSessionsSpawnTool({ + agentSessionKey: "main", + agentChannel: "discord", + agentAccountId: "work", + agentTo: "channel:123", + agentThreadId: "456", + }); +} + +async function executeDiscordThreadSessionSpawn(toolCallId: string) { + const tool = await getDiscordThreadSessionTool(); + return await tool.execute(toolCallId, { + task: "do thing", + thread: true, + mode: "session", + }); +} + +function getSpawnedEventCall(): Record { + const [event] = (hookRunnerMocks.runSubagentSpawned.mock.calls[0] ?? []) as unknown as [ + Record, + ]; + return event; +} + +function expectErrorResultMessage(result: { details: unknown }, pattern: RegExp): void { + expect(result.details).toMatchObject({ status: "error" }); + const details = result.details as { error?: string }; + expect(details.error).toMatch(pattern); +} + +function expectThreadBindFailureCleanup( + details: { childSessionKey?: string }, + pattern: RegExp, +): void { + expect(details.error).toMatch(pattern); + expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); + expectSessionsDeleteWithoutAgentStart(); + const deleteCall = findGatewayRequest("sessions.delete"); + expect(deleteCall?.params).toMatchObject({ + key: details.childSessionKey, + emitLifecycleHooks: false, + }); +} + describe("sessions_spawn subagent lifecycle hooks", () => { beforeEach(() => { resetSubagentRegistryForTests(); @@ -226,9 +272,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { expect(result.details).toMatchObject({ status: "accepted", runId: "run-1", mode: "run" }); expect(hookRunnerMocks.runSubagentSpawning).toHaveBeenCalledTimes(1); - const [event] = (hookRunnerMocks.runSubagentSpawned.mock.calls[0] ?? []) as unknown as [ - Record, - ]; + const event = getSpawnedEventCall(); expect(event).toMatchObject({ mode: "run", threadRequested: true, @@ -243,14 +287,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { error: "Unable to create or bind a Discord thread for this subagent session.", }, }); - expect(details.error).toMatch(/thread/i); - expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); - expectSessionsDeleteWithoutAgentStart(); - const deleteCall = findGatewayRequest("sessions.delete"); - expect(deleteCall?.params).toMatchObject({ - key: details.childSessionKey, - emitLifecycleHooks: false, - }); + expectThreadBindFailureCleanup(details, /thread/i); }); it("returns error when thread binding is not marked ready", async () => { @@ -261,14 +298,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { threadBindingReady: false, }, }); - expect(details.error).toMatch(/unable to create or bind a thread/i); - expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); - expectSessionsDeleteWithoutAgentStart(); - const deleteCall = findGatewayRequest("sessions.delete"); - expect(deleteCall?.params).toMatchObject({ - key: details.childSessionKey, - emitLifecycleHooks: false, - }); + expectThreadBindFailureCleanup(details, /unable to create or bind a thread/i); }); it("rejects mode=session when thread=true is not requested", async () => { @@ -283,9 +313,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { mode: "session", }); - expect(result.details).toMatchObject({ status: "error" }); - const details = result.details as { error?: string }; - expect(details.error).toMatch(/requires thread=true/i); + expectErrorResultMessage(result, /requires thread=true/i); expect(hookRunnerMocks.runSubagentSpawning).not.toHaveBeenCalled(); expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); const callGatewayMock = getCallGatewayMock(); @@ -305,9 +333,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { mode: "session", }); - expect(result.details).toMatchObject({ status: "error" }); - const details = result.details as { error?: string }; - expect(details.error).toMatch(/only discord/i); + expectErrorResultMessage(result, /only discord/i); expect(hookRunnerMocks.runSubagentSpawning).toHaveBeenCalledTimes(1); expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); expectSessionsDeleteWithoutAgentStart(); @@ -315,19 +341,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { it("runs subagent_ended cleanup hook when agent start fails after successful bind", async () => { mockAgentStartFailure(); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "main", - agentChannel: "discord", - agentAccountId: "work", - agentTo: "channel:123", - agentThreadId: "456", - }); - - const result = await tool.execute("call7", { - task: "do thing", - thread: true, - mode: "session", - }); + const result = await executeDiscordThreadSessionSpawn("call7"); expect(result.details).toMatchObject({ status: "error" }); expect(hookRunnerMocks.runSubagentEnded).toHaveBeenCalledTimes(1); @@ -354,19 +368,7 @@ describe("sessions_spawn subagent lifecycle hooks", () => { it("falls back to sessions.delete cleanup when subagent_ended hook is unavailable", async () => { hookRunnerMocks.hasSubagentEndedHook = false; mockAgentStartFailure(); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "main", - agentChannel: "discord", - agentAccountId: "work", - agentTo: "channel:123", - agentThreadId: "456", - }); - - const result = await tool.execute("call8", { - task: "do thing", - thread: true, - mode: "session", - }); + const result = await executeDiscordThreadSessionSpawn("call8"); expect(result.details).toMatchObject({ status: "error" }); expect(hookRunnerMocks.runSubagentEnded).not.toHaveBeenCalled(); diff --git a/src/commands/auth-choice.apply-helpers.test.ts b/src/commands/auth-choice.apply-helpers.test.ts index 9e5810791..37a701cee 100644 --- a/src/commands/auth-choice.apply-helpers.test.ts +++ b/src/commands/auth-choice.apply-helpers.test.ts @@ -44,12 +44,40 @@ function createPromptSpies(params?: { confirmResult?: boolean; textResult?: stri return { confirm, note, text }; } +function createPromptAndCredentialSpies(params?: { confirmResult?: boolean; textResult?: string }) { + return { + ...createPromptSpies(params), + setCredential: vi.fn(async () => undefined), + }; +} + async function ensureMinimaxApiKey(params: { + config?: Parameters[0]["config"]; confirm: WizardPrompter["confirm"]; + note?: WizardPrompter["note"]; + select?: WizardPrompter["select"]; text: WizardPrompter["text"]; setCredential: Parameters[0]["setCredential"]; - config?: Parameters[0]["config"]; secretInputMode?: Parameters[0]["secretInputMode"]; +}) { + return await ensureMinimaxApiKeyInternal({ + config: params.config, + prompter: createPrompter({ + confirm: params.confirm, + note: params.note, + select: params.select, + text: params.text, + }), + secretInputMode: params.secretInputMode, + setCredential: params.setCredential, + }); +} + +async function ensureMinimaxApiKeyInternal(params: { + config?: Parameters[0]["config"]; + prompter: WizardPrompter; + secretInputMode?: Parameters[0]["secretInputMode"]; + setCredential: Parameters[0]["setCredential"]; }) { return await ensureApiKeyFromEnvOrPrompt({ config: params.config ?? {}, @@ -58,7 +86,7 @@ async function ensureMinimaxApiKey(params: { promptMessage: "Enter key", normalize: (value) => value.trim(), validate: () => undefined, - prompter: createPrompter({ confirm: params.confirm, text: params.text }), + prompter: params.prompter, secretInputMode: params.secretInputMode, setCredential: params.setCredential, }); @@ -71,13 +99,8 @@ async function ensureMinimaxApiKeyWithEnvRefPrompter(params: { setCredential: Parameters[0]["setCredential"]; text: WizardPrompter["text"]; }) { - return await ensureApiKeyFromEnvOrPrompt({ - config: params.config ?? {}, - provider: "minimax", - envLabel: "MINIMAX_API_KEY", - promptMessage: "Enter key", - normalize: (value) => value.trim(), - validate: () => undefined, + return await ensureMinimaxApiKeyInternal({ + config: params.config, prompter: createPrompter({ select: params.select, text: params.text, note: params.note }), secretInputMode: "ref", setCredential: params.setCredential, @@ -102,6 +125,55 @@ async function runEnsureMinimaxApiKeyFlow(params: { confirmResult: boolean; text return { result, setCredential, confirm, text }; } +async function runMaybeApplyHuggingFaceToken(tokenProvider: string) { + const setCredential = vi.fn(async () => undefined); + const result = await maybeApplyApiKeyFromOption({ + token: " opt-key ", + tokenProvider, + expectedProviders: ["huggingface"], + normalize: (value) => value.trim(), + setCredential, + }); + return { result, setCredential }; +} + +function expectMinimaxEnvRefCredentialStored(setCredential: ReturnType) { + expect(setCredential).toHaveBeenCalledWith( + { source: "env", provider: "default", id: "MINIMAX_API_KEY" }, + "ref", + ); +} + +async function ensureWithOptionEnvOrPrompt(params: { + token: string; + tokenProvider: string; + expectedProviders: string[]; + provider: string; + envLabel: string; + confirm: WizardPrompter["confirm"]; + note: WizardPrompter["note"]; + noteMessage: string; + noteTitle: string; + setCredential: Parameters[0]["setCredential"]; + text: WizardPrompter["text"]; +}) { + return await ensureApiKeyFromOptionEnvOrPrompt({ + token: params.token, + tokenProvider: params.tokenProvider, + config: {}, + expectedProviders: params.expectedProviders, + provider: params.provider, + envLabel: params.envLabel, + promptMessage: "Enter key", + normalize: (value) => value.trim(), + validate: () => undefined, + prompter: createPrompter({ confirm: params.confirm, note: params.note, text: params.text }), + setCredential: params.setCredential, + noteMessage: params.noteMessage, + noteTitle: params.noteTitle, + }); +} + afterEach(() => { restoreMinimaxEnv(); vi.restoreAllMocks(); @@ -116,30 +188,14 @@ describe("normalizeTokenProviderInput", () => { describe("maybeApplyApiKeyFromOption", () => { it("stores normalized token when provider matches", async () => { - const setCredential = vi.fn(async () => undefined); - - const result = await maybeApplyApiKeyFromOption({ - token: " opt-key ", - tokenProvider: "huggingface", - expectedProviders: ["huggingface"], - normalize: (value) => value.trim(), - setCredential, - }); + const { result, setCredential } = await runMaybeApplyHuggingFaceToken("huggingface"); expect(result).toBe("opt-key"); expect(setCredential).toHaveBeenCalledWith("opt-key", undefined); }); it("matches provider with whitespace/case normalization", async () => { - const setCredential = vi.fn(async () => undefined); - - const result = await maybeApplyApiKeyFromOption({ - token: " opt-key ", - tokenProvider: " HuGgInGfAcE ", - expectedProviders: ["huggingface"], - normalize: (value) => value.trim(), - setCredential, - }); + const { result, setCredential } = await runMaybeApplyHuggingFaceToken(" HuGgInGfAcE "); expect(result).toBe("opt-key"); expect(setCredential).toHaveBeenCalledWith("opt-key", undefined); @@ -192,11 +248,10 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { process.env.MINIMAX_API_KEY = "env-key"; delete process.env.MINIMAX_OAUTH_TOKEN; - const { confirm, text } = createPromptSpies({ + const { confirm, text, setCredential } = createPromptAndCredentialSpies({ confirmResult: true, textResult: "prompt-key", }); - const setCredential = vi.fn(async () => undefined); const result = await ensureMinimaxApiKey({ confirm, @@ -206,10 +261,7 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { }); expect(result).toBe("env-key"); - expect(setCredential).toHaveBeenCalledWith( - { source: "env", provider: "default", id: "MINIMAX_API_KEY" }, - "ref", - ); + expectMinimaxEnvRefCredentialStored(setCredential); expect(text).not.toHaveBeenCalled(); }); @@ -217,11 +269,10 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { delete process.env.MINIMAX_API_KEY; delete process.env.MINIMAX_OAUTH_TOKEN; - const { confirm, text } = createPromptSpies({ + const { confirm, text, setCredential } = createPromptAndCredentialSpies({ confirmResult: true, textResult: "prompt-key", }); - const setCredential = vi.fn(async () => undefined); await expect( ensureMinimaxApiKey({ @@ -268,10 +319,7 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { }); expect(result).toBe("env-key"); - expect(setCredential).toHaveBeenCalledWith( - { source: "env", provider: "default", id: "MINIMAX_API_KEY" }, - "ref", - ); + expectMinimaxEnvRefCredentialStored(setCredential); expect(note).toHaveBeenCalledWith( expect.stringContaining("Could not validate provider reference"), "Reference check failed", @@ -304,26 +352,23 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { describe("ensureApiKeyFromOptionEnvOrPrompt", () => { it("uses opts token and skips note/env/prompt", async () => { - const { confirm, note, text } = createPromptSpies({ + const { confirm, note, text, setCredential } = createPromptAndCredentialSpies({ confirmResult: true, textResult: "prompt-key", }); - const setCredential = vi.fn(async () => undefined); - const result = await ensureApiKeyFromOptionEnvOrPrompt({ + const result = await ensureWithOptionEnvOrPrompt({ token: " opts-key ", tokenProvider: " HUGGINGFACE ", - config: {}, expectedProviders: ["huggingface"], provider: "huggingface", envLabel: "HF_TOKEN", - promptMessage: "Enter key", - normalize: (value) => value.trim(), - validate: () => undefined, - prompter: createPrompter({ confirm, note, text }), - setCredential, + confirm, + note, noteMessage: "Hugging Face note", noteTitle: "Hugging Face", + setCredential, + text, }); expect(result).toBe("opts-key"); @@ -337,26 +382,23 @@ describe("ensureApiKeyFromOptionEnvOrPrompt", () => { delete process.env.MINIMAX_OAUTH_TOKEN; process.env.MINIMAX_API_KEY = "env-key"; - const { confirm, note, text } = createPromptSpies({ + const { confirm, note, text, setCredential } = createPromptAndCredentialSpies({ confirmResult: true, textResult: "prompt-key", }); - const setCredential = vi.fn(async () => undefined); - const result = await ensureApiKeyFromOptionEnvOrPrompt({ + const result = await ensureWithOptionEnvOrPrompt({ token: "opts-key", tokenProvider: "openai", - config: {}, expectedProviders: ["minimax"], provider: "minimax", envLabel: "MINIMAX_API_KEY", - promptMessage: "Enter key", - normalize: (value) => value.trim(), - validate: () => undefined, - prompter: createPrompter({ confirm, note, text }), - setCredential, + confirm, + note, noteMessage: "MiniMax note", noteTitle: "MiniMax", + setCredential, + text, }); expect(result).toBe("env-key"); diff --git a/src/commands/gateway-status.test.ts b/src/commands/gateway-status.test.ts index b95c6e68a..559bec14e 100644 --- a/src/commands/gateway-status.test.ts +++ b/src/commands/gateway-status.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import type { RuntimeEnv } from "../runtime.js"; import { withEnvAsync } from "../test-utils/env.js"; const loadConfig = vi.fn(() => ({ @@ -134,15 +135,33 @@ function createRuntimeCapture() { return { runtime, runtimeLogs, runtimeErrors }; } +function asRuntimeEnv(runtime: ReturnType["runtime"]): RuntimeEnv { + return runtime as unknown as RuntimeEnv; +} + +function makeRemoteGatewayConfig(url: string, token = "rtok", localToken = "ltok") { + return { + gateway: { + mode: "remote", + remote: { url, token }, + auth: { token: localToken }, + }, + }; +} + +async function runGatewayStatus( + runtime: ReturnType["runtime"], + opts: { timeout: string; json?: boolean; ssh?: string; sshAuto?: boolean; sshIdentity?: string }, +) { + const { gatewayStatusCommand } = await import("./gateway-status.js"); + await gatewayStatusCommand(opts, asRuntimeEnv(runtime)); +} + describe("gateway-status command", () => { it("prints human output by default", async () => { const { runtime, runtimeLogs, runtimeErrors } = createRuntimeCapture(); - const { gatewayStatusCommand } = await import("./gateway-status.js"); - await gatewayStatusCommand( - { timeout: "1000" }, - runtime as unknown as import("../runtime.js").RuntimeEnv, - ); + await runGatewayStatus(runtime, { timeout: "1000" }); expect(runtimeErrors).toHaveLength(0); expect(runtimeLogs.join("\n")).toContain("Gateway Status"); @@ -153,11 +172,7 @@ describe("gateway-status command", () => { it("prints a structured JSON envelope when --json is set", async () => { const { runtime, runtimeLogs, runtimeErrors } = createRuntimeCapture(); - const { gatewayStatusCommand } = await import("./gateway-status.js"); - await gatewayStatusCommand( - { timeout: "1000", json: true }, - runtime as unknown as import("../runtime.js").RuntimeEnv, - ); + await runGatewayStatus(runtime, { timeout: "1000", json: true }); expect(runtimeErrors).toHaveLength(0); const parsed = JSON.parse(runtimeLogs.join("\n")) as Record; @@ -176,11 +191,7 @@ describe("gateway-status command", () => { sshStop.mockClear(); probeGateway.mockClear(); - const { gatewayStatusCommand } = await import("./gateway-status.js"); - await gatewayStatusCommand( - { timeout: "1000", json: true, ssh: "me@studio" }, - runtime as unknown as import("../runtime.js").RuntimeEnv, - ); + await runGatewayStatus(runtime, { timeout: "1000", json: true, ssh: "me@studio" }); expect(startSshPortForward).toHaveBeenCalledTimes(1); expect(probeGateway).toHaveBeenCalled(); @@ -198,24 +209,14 @@ describe("gateway-status command", () => { it("skips invalid ssh-auto discovery targets", async () => { const { runtime } = createRuntimeCapture(); await withEnvAsync({ USER: "steipete" }, async () => { - loadConfig.mockReturnValueOnce({ - gateway: { - mode: "remote", - remote: { url: "", token: "" }, - auth: { token: "ltok" }, - }, - }); + loadConfig.mockReturnValueOnce(makeRemoteGatewayConfig("", "", "ltok")); discoverGatewayBeacons.mockResolvedValueOnce([ { tailnetDns: "-V" }, { tailnetDns: "goodhost" }, ]); startSshPortForward.mockClear(); - const { gatewayStatusCommand } = await import("./gateway-status.js"); - await gatewayStatusCommand( - { timeout: "1000", json: true, sshAuto: true }, - runtime as unknown as import("../runtime.js").RuntimeEnv, - ); + await runGatewayStatus(runtime, { timeout: "1000", json: true, sshAuto: true }); expect(startSshPortForward).toHaveBeenCalledTimes(1); const call = startSshPortForward.mock.calls[0]?.[0] as { target: string }; @@ -226,13 +227,9 @@ describe("gateway-status command", () => { it("infers SSH target from gateway.remote.url and ssh config", async () => { const { runtime } = createRuntimeCapture(); await withEnvAsync({ USER: "steipete" }, async () => { - loadConfig.mockReturnValueOnce({ - gateway: { - mode: "remote", - remote: { url: "ws://peters-mac-studio-1.sheep-coho.ts.net:18789", token: "rtok" }, - auth: { token: "ltok" }, - }, - }); + loadConfig.mockReturnValueOnce( + makeRemoteGatewayConfig("ws://peters-mac-studio-1.sheep-coho.ts.net:18789"), + ); resolveSshConfig.mockResolvedValueOnce({ user: "steipete", host: "peters-mac-studio-1.sheep-coho.ts.net", @@ -241,11 +238,7 @@ describe("gateway-status command", () => { }); startSshPortForward.mockClear(); - const { gatewayStatusCommand } = await import("./gateway-status.js"); - await gatewayStatusCommand( - { timeout: "1000", json: true }, - runtime as unknown as import("../runtime.js").RuntimeEnv, - ); + await runGatewayStatus(runtime, { timeout: "1000", json: true }); expect(startSshPortForward).toHaveBeenCalledTimes(1); const call = startSshPortForward.mock.calls[0]?.[0] as { @@ -260,21 +253,11 @@ describe("gateway-status command", () => { it("falls back to host-only when USER is missing and ssh config is unavailable", async () => { const { runtime } = createRuntimeCapture(); await withEnvAsync({ USER: "" }, async () => { - loadConfig.mockReturnValueOnce({ - gateway: { - mode: "remote", - remote: { url: "wss://studio.example:18789", token: "rtok" }, - auth: { token: "ltok" }, - }, - }); + loadConfig.mockReturnValueOnce(makeRemoteGatewayConfig("wss://studio.example:18789")); resolveSshConfig.mockResolvedValueOnce(null); startSshPortForward.mockClear(); - const { gatewayStatusCommand } = await import("./gateway-status.js"); - await gatewayStatusCommand( - { timeout: "1000", json: true }, - runtime as unknown as import("../runtime.js").RuntimeEnv, - ); + await runGatewayStatus(runtime, { timeout: "1000", json: true }); const call = startSshPortForward.mock.calls[0]?.[0] as { target: string; @@ -286,13 +269,7 @@ describe("gateway-status command", () => { it("keeps explicit SSH identity even when ssh config provides one", async () => { const { runtime } = createRuntimeCapture(); - loadConfig.mockReturnValueOnce({ - gateway: { - mode: "remote", - remote: { url: "wss://studio.example:18789", token: "rtok" }, - auth: { token: "ltok" }, - }, - }); + loadConfig.mockReturnValueOnce(makeRemoteGatewayConfig("wss://studio.example:18789")); resolveSshConfig.mockResolvedValueOnce({ user: "me", host: "studio.example", @@ -301,11 +278,11 @@ describe("gateway-status command", () => { }); startSshPortForward.mockClear(); - const { gatewayStatusCommand } = await import("./gateway-status.js"); - await gatewayStatusCommand( - { timeout: "1000", json: true, sshIdentity: "/tmp/explicit_id" }, - runtime as unknown as import("../runtime.js").RuntimeEnv, - ); + await runGatewayStatus(runtime, { + timeout: "1000", + json: true, + sshIdentity: "/tmp/explicit_id", + }); const call = startSshPortForward.mock.calls[0]?.[0] as { identity?: string; diff --git a/src/config/env-substitution.test.ts b/src/config/env-substitution.test.ts index 30ad33343..1b3c3f64f 100644 --- a/src/config/env-substitution.test.ts +++ b/src/config/env-substitution.test.ts @@ -1,15 +1,46 @@ import { describe, expect, it } from "vitest"; import { MissingEnvVarError, resolveConfigEnvVars } from "./env-substitution.js"; +type SubstitutionScenario = { + name: string; + config: unknown; + env: Record; + expected: unknown; +}; + +type MissingEnvScenario = { + name: string; + config: unknown; + env: Record; + varName: string; + configPath: string; +}; + +function expectResolvedScenarios(scenarios: SubstitutionScenario[]) { + for (const scenario of scenarios) { + const result = resolveConfigEnvVars(scenario.config, scenario.env); + expect(result, scenario.name).toEqual(scenario.expected); + } +} + +function expectMissingScenarios(scenarios: MissingEnvScenario[]) { + for (const scenario of scenarios) { + try { + resolveConfigEnvVars(scenario.config, scenario.env); + expect.fail(`${scenario.name}: expected MissingEnvVarError`); + } catch (err) { + expect(err, scenario.name).toBeInstanceOf(MissingEnvVarError); + const error = err as MissingEnvVarError; + expect(error.varName, scenario.name).toBe(scenario.varName); + expect(error.configPath, scenario.name).toBe(scenario.configPath); + } + } +} + describe("resolveConfigEnvVars", () => { describe("basic substitution", () => { it("substitutes direct, inline, repeated, and multi-var patterns", () => { - const scenarios: Array<{ - name: string; - config: unknown; - env: Record; - expected: unknown; - }> = [ + const scenarios: SubstitutionScenario[] = [ { name: "single env var", config: { key: "${FOO}" }, @@ -36,21 +67,13 @@ describe("resolveConfigEnvVars", () => { }, ]; - for (const scenario of scenarios) { - const result = resolveConfigEnvVars(scenario.config, scenario.env); - expect(result, scenario.name).toEqual(scenario.expected); - } + expectResolvedScenarios(scenarios); }); }); describe("nested structures", () => { it("substitutes variables in nested objects and arrays", () => { - const scenarios: Array<{ - name: string; - config: unknown; - env: Record; - expected: unknown; - }> = [ + const scenarios: SubstitutionScenario[] = [ { name: "nested object", config: { outer: { inner: { key: "${API_KEY}" } } }, @@ -81,22 +104,13 @@ describe("resolveConfigEnvVars", () => { }, ]; - for (const scenario of scenarios) { - const result = resolveConfigEnvVars(scenario.config, scenario.env); - expect(result, scenario.name).toEqual(scenario.expected); - } + expectResolvedScenarios(scenarios); }); }); describe("missing env var handling", () => { it("throws MissingEnvVarError with var name and config path details", () => { - const scenarios: Array<{ - name: string; - config: unknown; - env: Record; - varName: string; - configPath: string; - }> = [ + const scenarios: MissingEnvScenario[] = [ { name: "missing top-level var", config: { key: "${MISSING}" }, @@ -127,28 +141,13 @@ describe("resolveConfigEnvVars", () => { }, ]; - for (const scenario of scenarios) { - try { - resolveConfigEnvVars(scenario.config, scenario.env); - expect.fail(`${scenario.name}: expected MissingEnvVarError`); - } catch (err) { - expect(err, scenario.name).toBeInstanceOf(MissingEnvVarError); - const error = err as MissingEnvVarError; - expect(error.varName, scenario.name).toBe(scenario.varName); - expect(error.configPath, scenario.name).toBe(scenario.configPath); - } - } + expectMissingScenarios(scenarios); }); }); describe("escape syntax", () => { it("handles escaped placeholders alongside regular substitutions", () => { - const scenarios: Array<{ - name: string; - config: unknown; - env: Record; - expected: unknown; - }> = [ + const scenarios: SubstitutionScenario[] = [ { name: "escaped placeholder stays literal", config: { key: "$${VAR}" }, @@ -187,21 +186,13 @@ describe("resolveConfigEnvVars", () => { }, ]; - for (const scenario of scenarios) { - const result = resolveConfigEnvVars(scenario.config, scenario.env); - expect(result, scenario.name).toEqual(scenario.expected); - } + expectResolvedScenarios(scenarios); }); }); describe("pattern matching rules", () => { it("leaves non-matching placeholders unchanged", () => { - const scenarios: Array<{ - name: string; - config: unknown; - env: Record; - expected: unknown; - }> = [ + const scenarios: SubstitutionScenario[] = [ { name: "$VAR (no braces)", config: { key: "$VAR" }, @@ -228,19 +219,11 @@ describe("resolveConfigEnvVars", () => { }, ]; - for (const scenario of scenarios) { - const result = resolveConfigEnvVars(scenario.config, scenario.env); - expect(result, scenario.name).toEqual(scenario.expected); - } + expectResolvedScenarios(scenarios); }); it("substitutes valid uppercase/underscore placeholder names", () => { - const scenarios: Array<{ - name: string; - config: unknown; - env: Record; - expected: unknown; - }> = [ + const scenarios: SubstitutionScenario[] = [ { name: "underscore-prefixed name", config: { key: "${_UNDERSCORE_START}" }, @@ -255,10 +238,7 @@ describe("resolveConfigEnvVars", () => { }, ]; - for (const scenario of scenarios) { - const result = resolveConfigEnvVars(scenario.config, scenario.env); - expect(result, scenario.name).toEqual(scenario.expected); - } + expectResolvedScenarios(scenarios); }); }); @@ -287,12 +267,7 @@ describe("resolveConfigEnvVars", () => { describe("real-world config patterns", () => { it("substitutes provider, gateway, and base URL config values", () => { - const scenarios: Array<{ - name: string; - config: unknown; - env: Record; - expected: unknown; - }> = [ + const scenarios: SubstitutionScenario[] = [ { name: "provider API keys", config: { @@ -342,10 +317,7 @@ describe("resolveConfigEnvVars", () => { }, ]; - for (const scenario of scenarios) { - const result = resolveConfigEnvVars(scenario.config, scenario.env); - expect(result, scenario.name).toEqual(scenario.expected); - } + expectResolvedScenarios(scenarios); }); }); }); diff --git a/src/pairing/pairing-store.test.ts b/src/pairing/pairing-store.test.ts index a99e9ca9b..c323c153d 100644 --- a/src/pairing/pairing-store.test.ts +++ b/src/pairing/pairing-store.test.ts @@ -63,13 +63,101 @@ async function writeAllowFromFixture(params: { allowFrom: string[]; accountId?: string; }) { - const oauthDir = resolveOAuthDir(process.env, params.stateDir); - await fs.mkdir(oauthDir, { recursive: true }); - const suffix = params.accountId ? `-${params.accountId}` : ""; - await writeJsonFixture(path.join(oauthDir, `${params.channel}${suffix}-allowFrom.json`), { - version: 1, - allowFrom: params.allowFrom, + await writeJsonFixture( + resolveAllowFromFilePath(params.stateDir, params.channel, params.accountId), + { + version: 1, + allowFrom: params.allowFrom, + }, + ); +} + +async function createTelegramPairingRequest(accountId: string, id = "12345") { + const created = await upsertChannelPairingRequest({ + channel: "telegram", + accountId, + id, }); + expect(created.created).toBe(true); + return created; +} + +async function seedTelegramAllowFromFixtures(params: { + stateDir: string; + scopedAccountId: string; + scopedAllowFrom: string[]; + legacyAllowFrom?: string[]; +}) { + await writeAllowFromFixture({ + stateDir: params.stateDir, + channel: "telegram", + allowFrom: params.legacyAllowFrom ?? ["1001"], + }); + await writeAllowFromFixture({ + stateDir: params.stateDir, + channel: "telegram", + accountId: params.scopedAccountId, + allowFrom: params.scopedAllowFrom, + }); +} + +async function assertAllowFromCacheInvalidation(params: { + stateDir: string; + readAllowFrom: () => Promise; + readSpy: { + mockRestore: () => void; + }; +}) { + const first = await params.readAllowFrom(); + const second = await params.readAllowFrom(); + expect(first).toEqual(["1001"]); + expect(second).toEqual(["1001"]); + expect(params.readSpy).toHaveBeenCalledTimes(1); + + await writeAllowFromFixture({ + stateDir: params.stateDir, + channel: "telegram", + accountId: "yy", + allowFrom: ["10022"], + }); + const third = await params.readAllowFrom(); + expect(third).toEqual(["10022"]); + expect(params.readSpy).toHaveBeenCalledTimes(2); +} + +async function expectAccountScopedEntryIsolated(entry: string, accountId = "yy") { + const accountScoped = await readChannelAllowFromStore("telegram", process.env, accountId); + const channelScoped = await readLegacyChannelAllowFromStore("telegram"); + expect(accountScoped).toContain(entry); + expect(channelScoped).not.toContain(entry); +} + +async function readScopedAllowFromPair(accountId: string) { + const asyncScoped = await readChannelAllowFromStore("telegram", process.env, accountId); + const syncScoped = readChannelAllowFromStoreSync("telegram", process.env, accountId); + return { asyncScoped, syncScoped }; +} + +async function withAllowFromCacheReadSpy(params: { + stateDir: string; + createReadSpy: () => { + mockRestore: () => void; + }; + readAllowFrom: () => Promise; +}) { + await writeAllowFromFixture({ + stateDir: params.stateDir, + channel: "telegram", + accountId: "yy", + allowFrom: ["1001"], + }); + const readSpy = params.createReadSpy(); + await assertAllowFromCacheInvalidation({ + stateDir: params.stateDir, + readAllowFrom: params.readAllowFrom, + readSpy, + }); + readSpy.mockRestore(); } describe("pairing store", () => { @@ -197,21 +285,13 @@ describe("pairing store", () => { entry: "12345", }); - const accountScoped = await readChannelAllowFromStore("telegram", process.env, "yy"); - const channelScoped = await readLegacyChannelAllowFromStore("telegram"); - expect(accountScoped).toContain("12345"); - expect(channelScoped).not.toContain("12345"); + await expectAccountScopedEntryIsolated("12345"); }); }); it("approves pairing codes into account-scoped allowFrom via pairing metadata", async () => { await withTempStateDir(async () => { - const created = await upsertChannelPairingRequest({ - channel: "telegram", - accountId: "yy", - id: "12345", - }); - expect(created.created).toBe(true); + const created = await createTelegramPairingRequest("yy"); const approved = await approveChannelPairingCode({ channel: "telegram", @@ -219,21 +299,13 @@ describe("pairing store", () => { }); expect(approved?.id).toBe("12345"); - const accountScoped = await readChannelAllowFromStore("telegram", process.env, "yy"); - const channelScoped = await readLegacyChannelAllowFromStore("telegram"); - expect(accountScoped).toContain("12345"); - expect(channelScoped).not.toContain("12345"); + await expectAccountScopedEntryIsolated("12345"); }); }); it("filters approvals by account id and ignores blank approval codes", async () => { await withTempStateDir(async () => { - const created = await upsertChannelPairingRequest({ - channel: "telegram", - accountId: "yy", - id: "12345", - }); - expect(created.created).toBe(true); + const created = await createTelegramPairingRequest("yy"); const blank = await approveChannelPairingCode({ channel: "telegram", @@ -303,20 +375,14 @@ describe("pairing store", () => { it("does not read legacy channel-scoped allowFrom for non-default account ids", async () => { await withTempStateDir(async (stateDir) => { - await writeAllowFromFixture({ + await seedTelegramAllowFromFixtures({ stateDir, - channel: "telegram", - allowFrom: ["1001", "*", "1002", "1001"], - }); - await writeAllowFromFixture({ - stateDir, - channel: "telegram", - accountId: "yy", - allowFrom: ["1003"], + scopedAccountId: "yy", + scopedAllowFrom: ["1003"], + legacyAllowFrom: ["1001", "*", "1002", "1001"], }); - const asyncScoped = await readChannelAllowFromStore("telegram", process.env, "yy"); - const syncScoped = readChannelAllowFromStoreSync("telegram", process.env, "yy"); + const { asyncScoped, syncScoped } = await readScopedAllowFromPair("yy"); expect(asyncScoped).toEqual(["1003"]); expect(syncScoped).toEqual(["1003"]); }); @@ -324,20 +390,13 @@ describe("pairing store", () => { it("does not fall back to legacy allowFrom when scoped file exists but is empty", async () => { await withTempStateDir(async (stateDir) => { - await writeAllowFromFixture({ + await seedTelegramAllowFromFixtures({ stateDir, - channel: "telegram", - allowFrom: ["1001"], - }); - await writeAllowFromFixture({ - stateDir, - channel: "telegram", - accountId: "yy", - allowFrom: [], + scopedAccountId: "yy", + scopedAllowFrom: [], }); - const asyncScoped = await readChannelAllowFromStore("telegram", process.env, "yy"); - const syncScoped = readChannelAllowFromStoreSync("telegram", process.env, "yy"); + const { asyncScoped, syncScoped } = await readScopedAllowFromPair("yy"); expect(asyncScoped).toEqual([]); expect(syncScoped).toEqual([]); }); @@ -389,12 +448,10 @@ describe("pairing store", () => { it("reads legacy channel-scoped allowFrom for default account", async () => { await withTempStateDir(async (stateDir) => { - await writeAllowFromFixture({ stateDir, channel: "telegram", allowFrom: ["1001"] }); - await writeAllowFromFixture({ + await seedTelegramAllowFromFixtures({ stateDir, - channel: "telegram", - accountId: "default", - allowFrom: ["1002"], + scopedAccountId: "default", + scopedAllowFrom: ["1002"], }); const scoped = await readChannelAllowFromStore("telegram", process.env, DEFAULT_ACCOUNT_ID); @@ -404,12 +461,10 @@ describe("pairing store", () => { it("uses default-account allowFrom when account id is omitted", async () => { await withTempStateDir(async (stateDir) => { - await writeAllowFromFixture({ stateDir, channel: "telegram", allowFrom: ["1001"] }); - await writeAllowFromFixture({ + await seedTelegramAllowFromFixtures({ stateDir, - channel: "telegram", - accountId: DEFAULT_ACCOUNT_ID, - allowFrom: ["1002"], + scopedAccountId: DEFAULT_ACCOUNT_ID, + scopedAllowFrom: ["1002"], }); const asyncScoped = await readChannelAllowFromStore("telegram", process.env); @@ -421,59 +476,21 @@ describe("pairing store", () => { it("reuses cached async allowFrom reads and invalidates on file updates", async () => { await withTempStateDir(async (stateDir) => { - await writeAllowFromFixture({ + await withAllowFromCacheReadSpy({ stateDir, - channel: "telegram", - accountId: "yy", - allowFrom: ["1001"], + createReadSpy: () => vi.spyOn(fs, "readFile"), + readAllowFrom: () => readChannelAllowFromStore("telegram", process.env, "yy"), }); - const readSpy = vi.spyOn(fs, "readFile"); - - const first = await readChannelAllowFromStore("telegram", process.env, "yy"); - const second = await readChannelAllowFromStore("telegram", process.env, "yy"); - expect(first).toEqual(["1001"]); - expect(second).toEqual(["1001"]); - expect(readSpy).toHaveBeenCalledTimes(1); - - await writeAllowFromFixture({ - stateDir, - channel: "telegram", - accountId: "yy", - allowFrom: ["10022"], - }); - const third = await readChannelAllowFromStore("telegram", process.env, "yy"); - expect(third).toEqual(["10022"]); - expect(readSpy).toHaveBeenCalledTimes(2); - readSpy.mockRestore(); }); }); it("reuses cached sync allowFrom reads and invalidates on file updates", async () => { await withTempStateDir(async (stateDir) => { - await writeAllowFromFixture({ + await withAllowFromCacheReadSpy({ stateDir, - channel: "telegram", - accountId: "yy", - allowFrom: ["1001"], + createReadSpy: () => vi.spyOn(fsSync, "readFileSync"), + readAllowFrom: async () => readChannelAllowFromStoreSync("telegram", process.env, "yy"), }); - const readSpy = vi.spyOn(fsSync, "readFileSync"); - - const first = readChannelAllowFromStoreSync("telegram", process.env, "yy"); - const second = readChannelAllowFromStoreSync("telegram", process.env, "yy"); - expect(first).toEqual(["1001"]); - expect(second).toEqual(["1001"]); - expect(readSpy).toHaveBeenCalledTimes(1); - - await writeAllowFromFixture({ - stateDir, - channel: "telegram", - accountId: "yy", - allowFrom: ["10022"], - }); - const third = readChannelAllowFromStoreSync("telegram", process.env, "yy"); - expect(third).toEqual(["10022"]); - expect(readSpy).toHaveBeenCalledTimes(2); - readSpy.mockRestore(); }); }); }); diff --git a/src/secrets/runtime.test.ts b/src/secrets/runtime.test.ts index e569dc24d..6d3c73afa 100644 --- a/src/secrets/runtime.test.ts +++ b/src/secrets/runtime.test.ts @@ -10,6 +10,39 @@ import { prepareSecretsRuntimeSnapshot, } from "./runtime.js"; +const OPENAI_ENV_KEY_REF = { source: "env", provider: "default", id: "OPENAI_API_KEY" } as const; + +function createOpenAiEnvModelsConfig(): NonNullable { + return { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: OPENAI_ENV_KEY_REF, + models: [], + }, + }, + }; +} + +function createOpenAiFileModelsConfig(): NonNullable { + return { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey: { source: "file", provider: "default", id: "/providers/openai/apiKey" }, + models: [], + }, + }, + }; +} + +function loadAuthStoreWithProfiles(profiles: AuthProfileStore["profiles"]): AuthProfileStore { + return { + version: 1, + profiles, + }; +} + describe("secrets runtime snapshot", () => { afterEach(() => { clearSecretsRuntimeSnapshot(); @@ -17,15 +50,7 @@ describe("secrets runtime snapshot", () => { it("resolves env refs for config and auth profiles", async () => { const config: OpenClawConfig = { - models: { - providers: { - openai: { - baseUrl: "https://api.openai.com/v1", - apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, - models: [], - }, - }, - }, + models: createOpenAiEnvModelsConfig(), skills: { entries: { "review-pr": { @@ -44,14 +69,13 @@ describe("secrets runtime snapshot", () => { REVIEW_SKILL_API_KEY: "sk-skill-ref", }, agentDirs: ["/tmp/openclaw-agent-main"], - loadAuthStore: () => ({ - version: 1, - profiles: { + loadAuthStore: () => + loadAuthStoreWithProfiles({ "openai:default": { type: "api_key", provider: "openai", key: "old-openai", - keyRef: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + keyRef: OPENAI_ENV_KEY_REF, }, "github-copilot:default": { type: "token", @@ -64,8 +88,7 @@ describe("secrets runtime snapshot", () => { provider: "openai", key: "${OPENAI_API_KEY}", }, - }, - }), + }), }); expect(snapshot.config.models?.providers?.openai?.apiKey).toBe("sk-env-openai"); @@ -95,17 +118,14 @@ describe("secrets runtime snapshot", () => { config, env: { MY_TOKEN: "resolved-token-value" }, agentDirs: ["/tmp/openclaw-agent-main"], - loadAuthStore: ((_agentDir?: string) => - ({ - version: 1, - profiles: { - "custom:inline-token": { - type: "token", - provider: "custom", - token: { source: "env", provider: "default", id: "MY_TOKEN" }, - }, + loadAuthStore: () => + loadAuthStoreWithProfiles({ + "custom:inline-token": { + type: "token", + provider: "custom", + token: { source: "env", provider: "default", id: "MY_TOKEN" }, }, - }) as unknown as AuthProfileStore) as (agentDir?: string) => AuthProfileStore, + }), }); const profile = snapshot.authStores[0]?.store.profiles["custom:inline-token"] as Record< @@ -125,17 +145,14 @@ describe("secrets runtime snapshot", () => { config, env: { MY_KEY: "resolved-key-value" }, agentDirs: ["/tmp/openclaw-agent-main"], - loadAuthStore: ((_agentDir?: string) => - ({ - version: 1, - profiles: { - "custom:inline-key": { - type: "api_key", - provider: "custom", - key: { source: "env", provider: "default", id: "MY_KEY" }, - }, + loadAuthStore: () => + loadAuthStoreWithProfiles({ + "custom:inline-key": { + type: "api_key", + provider: "custom", + key: { source: "env", provider: "default", id: "MY_KEY" }, }, - }) as unknown as AuthProfileStore) as (agentDir?: string) => AuthProfileStore, + }), }); const profile = snapshot.authStores[0]?.store.profiles["custom:inline-key"] as Record< @@ -159,17 +176,14 @@ describe("secrets runtime snapshot", () => { }, agentDirs: ["/tmp/openclaw-agent-main"], loadAuthStore: () => - ({ - version: 1, - profiles: { - "custom:explicit-keyref": { - type: "api_key", - provider: "custom", - keyRef: { source: "env", provider: "default", id: "PRIMARY_KEY" }, - key: { source: "env", provider: "default", id: "SHADOW_KEY" }, - }, + loadAuthStoreWithProfiles({ + "custom:explicit-keyref": { + type: "api_key", + provider: "custom", + keyRef: { source: "env", provider: "default", id: "PRIMARY_KEY" }, + key: { source: "env", provider: "default", id: "SHADOW_KEY" }, }, - }) as unknown as AuthProfileStore, + }), }); const profile = snapshot.authStores[0]?.store.profiles["custom:explicit-keyref"] as Record< @@ -264,13 +278,7 @@ describe("secrets runtime snapshot", () => { }, }, models: { - providers: { - openai: { - baseUrl: "https://api.openai.com/v1", - apiKey: { source: "file", provider: "default", id: "/providers/openai/apiKey" }, - models: [], - }, - }, + ...createOpenAiFileModelsConfig(), }, }, agentDirs: ["/tmp/openclaw-agent-main"], @@ -285,28 +293,18 @@ describe("secrets runtime snapshot", () => { it("activates runtime snapshots for loadConfig and ensureAuthProfileStore", async () => { const prepared = await prepareSecretsRuntimeSnapshot({ config: { - models: { - providers: { - openai: { - baseUrl: "https://api.openai.com/v1", - apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, - models: [], - }, - }, - }, + models: createOpenAiEnvModelsConfig(), }, env: { OPENAI_API_KEY: "sk-runtime" }, agentDirs: ["/tmp/openclaw-agent-main"], - loadAuthStore: () => ({ - version: 1, - profiles: { + loadAuthStore: () => + loadAuthStoreWithProfiles({ "openai:default": { type: "api_key", provider: "openai", - keyRef: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + keyRef: OPENAI_ENV_KEY_REF, }, - }, - }), + }), }); activateSecretsRuntimeSnapshot(prepared); @@ -331,14 +329,13 @@ describe("secrets runtime snapshot", () => { await fs.writeFile( path.join(mainAgentDir, "auth-profiles.json"), JSON.stringify({ - version: 1, - profiles: { + ...loadAuthStoreWithProfiles({ "openai:default": { type: "api_key", provider: "openai", - keyRef: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + keyRef: OPENAI_ENV_KEY_REF, }, - }, + }), }), "utf8", ); diff --git a/src/security/windows-acl.test.ts b/src/security/windows-acl.test.ts index 25f31bc57..5f7b86da8 100644 --- a/src/security/windows-acl.test.ts +++ b/src/security/windows-acl.test.ts @@ -34,6 +34,29 @@ function aclEntry(params: { }; } +function expectSinglePrincipal(entries: WindowsAclEntry[], principal: string): void { + expect(entries).toHaveLength(1); + expect(entries[0].principal).toBe(principal); +} + +function expectTrustedOnly( + entries: WindowsAclEntry[], + options?: { env?: NodeJS.ProcessEnv; expectedTrusted?: number }, +): void { + const summary = summarizeWindowsAcl(entries, options?.env); + expect(summary.trusted).toHaveLength(options?.expectedTrusted ?? 1); + expect(summary.untrustedWorld).toHaveLength(0); + expect(summary.untrustedGroup).toHaveLength(0); +} + +function expectInspectSuccess( + result: Awaited>, + expectedEntries: number, +): void { + expect(result.ok).toBe(true); + expect(result.entries).toHaveLength(expectedEntries); +} + describe("windows-acl", () => { describe("resolveWindowsUserPrincipal", () => { it("returns DOMAIN\\USERNAME when both are present", () => { @@ -91,8 +114,7 @@ Successfully processed 1 files`; const output = `C:\\test\\file.txt BUILTIN\\Users:(DENY)(W) BUILTIN\\Administrators:(F)`; const entries = parseIcaclsOutput(output, "C:\\test\\file.txt"); - expect(entries).toHaveLength(1); - expect(entries[0].principal).toBe("BUILTIN\\Administrators"); + expectSinglePrincipal(entries, "BUILTIN\\Administrators"); }); it("skips status messages", () => { @@ -128,8 +150,7 @@ Successfully processed 1 files`; const output = `C:\\test\\file.txt random:message C:\\test\\file.txt BUILTIN\\Administrators:(F)`; const entries = parseIcaclsOutput(output, "C:\\test\\file.txt"); - expect(entries).toHaveLength(1); - expect(entries[0].principal).toBe("BUILTIN\\Administrators"); + expectSinglePrincipal(entries, "BUILTIN\\Administrators"); }); it("handles quoted target paths", () => { @@ -220,11 +241,7 @@ Successfully processed 1 files`; describe("summarizeWindowsAcl — SID-based classification", () => { it("classifies SYSTEM SID (S-1-5-18) as trusted", () => { - const entries: WindowsAclEntry[] = [aclEntry({ principal: "S-1-5-18" })]; - const summary = summarizeWindowsAcl(entries); - expect(summary.trusted).toHaveLength(1); - expect(summary.untrustedWorld).toHaveLength(0); - expect(summary.untrustedGroup).toHaveLength(0); + expectTrustedOnly([aclEntry({ principal: "S-1-5-18" })]); }); it("classifies BUILTIN\\Administrators SID (S-1-5-32-544) as trusted", () => { @@ -236,25 +253,16 @@ Successfully processed 1 files`; it("classifies caller SID from USERSID env var as trusted", () => { const callerSid = "S-1-5-21-1824257776-4070701511-781240313-1001"; - const entries: WindowsAclEntry[] = [aclEntry({ principal: callerSid })]; - const env = { USERSID: callerSid }; - const summary = summarizeWindowsAcl(entries, env); - expect(summary.trusted).toHaveLength(1); - expect(summary.untrustedGroup).toHaveLength(0); + expectTrustedOnly([aclEntry({ principal: callerSid })], { + env: { USERSID: callerSid }, + }); }); it("matches SIDs case-insensitively and trims USERSID", () => { - const entries: WindowsAclEntry[] = [ - aclEntry({ - principal: "s-1-5-21-1824257776-4070701511-781240313-1001", - }), - ]; - const env = { - USERSID: " S-1-5-21-1824257776-4070701511-781240313-1001 ", - }; - const summary = summarizeWindowsAcl(entries, env); - expect(summary.trusted).toHaveLength(1); - expect(summary.untrustedGroup).toHaveLength(0); + expectTrustedOnly( + [aclEntry({ principal: "s-1-5-21-1824257776-4070701511-781240313-1001" })], + { env: { USERSID: " S-1-5-21-1824257776-4070701511-781240313-1001 " } }, + ); }); it("classifies unknown SID as group (not world)", () => { @@ -310,8 +318,7 @@ Successfully processed 1 files`; const result = await inspectWindowsAcl("C:\\test\\file.txt", { exec: mockExec, }); - expect(result.ok).toBe(true); - expect(result.entries).toHaveLength(2); + expectInspectSuccess(result, 2); expect(mockExec).toHaveBeenCalledWith("icacls", ["C:\\test\\file.txt"]); }); @@ -335,8 +342,7 @@ Successfully processed 1 files`; const result = await inspectWindowsAcl("C:\\test\\file.txt", { exec: mockExec, }); - expect(result.ok).toBe(true); - expect(result.entries).toHaveLength(2); + expectInspectSuccess(result, 2); }); }); @@ -475,24 +481,15 @@ Successfully processed 1 files`; describe("summarizeWindowsAcl — localized SYSTEM account names", () => { it("classifies French SYSTEM (AUTORITE NT\\Système) as trusted", () => { - const entries: WindowsAclEntry[] = [aclEntry({ principal: "AUTORITE NT\\Système" })]; - const { trusted, untrustedGroup } = summarizeWindowsAcl(entries); - expect(trusted).toHaveLength(1); - expect(untrustedGroup).toHaveLength(0); + expectTrustedOnly([aclEntry({ principal: "AUTORITE NT\\Système" })]); }); it("classifies German SYSTEM (NT-AUTORITÄT\\SYSTEM) as trusted", () => { - const entries: WindowsAclEntry[] = [aclEntry({ principal: "NT-AUTORITÄT\\SYSTEM" })]; - const { trusted, untrustedGroup } = summarizeWindowsAcl(entries); - expect(trusted).toHaveLength(1); - expect(untrustedGroup).toHaveLength(0); + expectTrustedOnly([aclEntry({ principal: "NT-AUTORITÄT\\SYSTEM" })]); }); it("classifies Spanish SYSTEM (AUTORIDAD NT\\SYSTEM) as trusted", () => { - const entries: WindowsAclEntry[] = [aclEntry({ principal: "AUTORIDAD NT\\SYSTEM" })]; - const { trusted, untrustedGroup } = summarizeWindowsAcl(entries); - expect(trusted).toHaveLength(1); - expect(untrustedGroup).toHaveLength(0); + expectTrustedOnly([aclEntry({ principal: "AUTORIDAD NT\\SYSTEM" })]); }); it("French Windows full scenario: user + Système only → no untrusted", () => {