From 8446e785cf56672685ec4cb0071ac3e293f8711e Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Fri, 13 Feb 2026 01:22:11 -0800 Subject: [PATCH] test: add 88 tests for OAuth flow functions in shared/common.sh (#843) The OAuth flow is the primary authentication mechanism for spawn users, yet its component functions had zero test coverage. This adds tests for: - validate_oauth_port: port range validation (boundary values, injection) - _generate_csrf_state: CSRF token generation (entropy, uniqueness) - _generate_oauth_html: success/error HTML page generation - _generate_oauth_server_script: Node.js callback server (CSRF, ports) - _validate_oauth_server_args: prerequisite validation (port, state, runtime) - _init_oauth_session: temp directory and CSRF state file creation - cleanup_oauth_session: PID and directory cleanup - exchange_oauth_code: OAuth code-to-key exchange with json_escape security - check_openrouter_connectivity: network reachability fallback chain - Integration: session lifecycle and CSRF security properties Agent: test-engineer Co-authored-by: A <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- .../shared-common-oauth-flow.test.ts | 852 ++++++++++++++++++ 1 file changed, 852 insertions(+) create mode 100644 cli/src/__tests__/shared-common-oauth-flow.test.ts diff --git a/cli/src/__tests__/shared-common-oauth-flow.test.ts b/cli/src/__tests__/shared-common-oauth-flow.test.ts new file mode 100644 index 00000000..e53465b2 --- /dev/null +++ b/cli/src/__tests__/shared-common-oauth-flow.test.ts @@ -0,0 +1,852 @@ +import { describe, it, expect, beforeEach, afterEach } from "bun:test"; +import { resolve, join } from "path"; +import { mkdirSync, rmSync, existsSync, readFileSync, writeFileSync, chmodSync } from "fs"; +import { tmpdir } from "os"; + +/** + * Tests for OAuth flow functions in shared/common.sh. + * + * The OAuth flow is the primary authentication mechanism for spawn users, + * yet its component functions had zero test coverage. This file tests: + * + * - validate_oauth_port: port range validation (1024-65535, numeric only) + * - _generate_csrf_state: CSRF token generation (security-critical) + * - _generate_oauth_html: HTML page generation for OAuth callback + * - _generate_oauth_server_script: Node.js callback server generation + * - _validate_oauth_server_args: prerequisite validation (port, state, runtime) + * - _init_oauth_session: temp directory and CSRF state file creation + * - cleanup_oauth_session: PID and directory cleanup + * - exchange_oauth_code: OAuth code-to-key exchange (json_escape security) + * + * These are SECURITY-CRITICAL: CSRF state prevents OAuth code interception, + * port validation prevents injection, and json_escape in exchange_oauth_code + * prevents JSON injection via crafted OAuth codes. + * + * Agent: test-engineer + */ + +const REPO_ROOT = resolve(import.meta.dir, "../../.."); +const COMMON_SH = resolve(REPO_ROOT, "shared/common.sh"); + +let testDir: string; + +beforeEach(() => { + testDir = join(tmpdir(), `spawn-oauth-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(testDir, { recursive: true }); +}); + +afterEach(() => { + if (existsSync(testDir)) { + rmSync(testDir, { recursive: true, force: true }); + } +}); + +/** + * Run a bash snippet that sources shared/common.sh first. + * Returns { exitCode, stdout, stderr }. + */ +function runBash(script: string, env?: Record): { exitCode: number; stdout: string; stderr: string } { + const fullScript = `source "${COMMON_SH}"\n${script}`; + const { spawnSync } = require("child_process"); + const result = spawnSync("bash", ["-c", fullScript], { + encoding: "utf-8", + timeout: 15000, + stdio: ["pipe", "pipe", "pipe"], + env: { ...process.env, ...env }, + }); + return { + exitCode: result.status ?? 1, + stdout: (result.stdout || "").trim(), + stderr: (result.stderr || "").trim(), + }; +} + +// ── validate_oauth_port ─────────────────────────────────────────────────────── + +describe("validate_oauth_port", () => { + describe("accepts valid ports", () => { + const validPorts = ["1024", "5180", "8080", "9999", "49152", "65535"]; + for (const port of validPorts) { + it(`should accept port ${port}`, () => { + const result = runBash(`validate_oauth_port "${port}"`); + expect(result.exitCode).toBe(0); + }); + } + }); + + describe("rejects privileged ports (below 1024)", () => { + const privilegedPorts = ["0", "1", "22", "80", "443", "1023"]; + for (const port of privilegedPorts) { + it(`should reject port ${port}`, () => { + const result = runBash(`validate_oauth_port "${port}"`); + expect(result.exitCode).toBe(1); + }); + } + }); + + describe("rejects ports above 65535", () => { + it("should reject port 65536", () => { + const result = runBash(`validate_oauth_port "65536"`); + expect(result.exitCode).toBe(1); + }); + + it("should reject port 99999", () => { + const result = runBash(`validate_oauth_port "99999"`); + expect(result.exitCode).toBe(1); + }); + }); + + describe("rejects non-numeric input", () => { + it("should reject alphabetic string", () => { + const result = runBash(`validate_oauth_port "abc"`); + expect(result.exitCode).toBe(1); + }); + + it("should reject empty string", () => { + const result = runBash(`validate_oauth_port ""`); + expect(result.exitCode).toBe(1); + }); + + it("should reject port with spaces", () => { + const result = runBash(`validate_oauth_port "80 80"`); + expect(result.exitCode).toBe(1); + }); + + it("should reject port with special characters", () => { + const result = runBash(`validate_oauth_port "5180;echo"`); + expect(result.exitCode).toBe(1); + }); + + it("should reject negative number", () => { + const result = runBash(`validate_oauth_port "-1"`); + expect(result.exitCode).toBe(1); + }); + + it("should reject decimal number", () => { + const result = runBash(`validate_oauth_port "5180.5"`); + expect(result.exitCode).toBe(1); + }); + }); + + describe("boundary values", () => { + it("should reject port 1023 (just below valid range)", () => { + const result = runBash(`validate_oauth_port "1023"`); + expect(result.exitCode).toBe(1); + }); + + it("should accept port 1024 (lower boundary)", () => { + const result = runBash(`validate_oauth_port "1024"`); + expect(result.exitCode).toBe(0); + }); + + it("should accept port 65535 (upper boundary)", () => { + const result = runBash(`validate_oauth_port "65535"`); + expect(result.exitCode).toBe(0); + }); + + it("should reject port 65536 (just above valid range)", () => { + const result = runBash(`validate_oauth_port "65536"`); + expect(result.exitCode).toBe(1); + }); + }); + + describe("error messages", () => { + it("should show 'must be numeric' for non-numeric input", () => { + const result = runBash(`validate_oauth_port "abc"`); + expect(result.stderr).toContain("must be numeric"); + }); + + it("should show 'must be between' for out-of-range port", () => { + const result = runBash(`validate_oauth_port "80"`); + expect(result.stderr).toContain("must be between"); + }); + }); +}); + +// ── _generate_csrf_state ────────────────────────────────────────────────────── + +describe("_generate_csrf_state", () => { + it("should generate a non-empty string", () => { + const result = runBash("_generate_csrf_state"); + expect(result.exitCode).toBe(0); + expect(result.stdout.length).toBeGreaterThan(0); + }); + + it("should generate hex-only output", () => { + const result = runBash("_generate_csrf_state"); + expect(result.exitCode).toBe(0); + expect(result.stdout).toMatch(/^[0-9a-f]+$/); + }); + + it("should generate at least 16 hex characters (64 bits of entropy)", () => { + const result = runBash("_generate_csrf_state"); + expect(result.exitCode).toBe(0); + expect(result.stdout.length).toBeGreaterThanOrEqual(16); + }); + + it("should generate different values on consecutive calls", () => { + const result = runBash(` + state1=$(_generate_csrf_state) + state2=$(_generate_csrf_state) + if [[ "$state1" == "$state2" ]]; then + echo "SAME" + exit 1 + fi + echo "DIFFERENT" + `); + expect(result.exitCode).toBe(0); + expect(result.stdout).toBe("DIFFERENT"); + }); + + it("should work with openssl fallback", () => { + // Test the primary openssl path (if available) + const result = runBash(` + if command -v openssl &>/dev/null; then + state=$(_generate_csrf_state) + echo "$state" + else + echo "no-openssl" + fi + `); + expect(result.exitCode).toBe(0); + if (result.stdout !== "no-openssl") { + // openssl rand -hex 16 produces exactly 32 hex chars + expect(result.stdout.length).toBe(32); + } + }); + + it("should produce output safe for embedding in URLs and filenames", () => { + const result = runBash("_generate_csrf_state"); + expect(result.exitCode).toBe(0); + // No special characters, spaces, or newlines + expect(result.stdout).not.toContain(" "); + expect(result.stdout).not.toContain("\n"); + expect(result.stdout).not.toContain("/"); + expect(result.stdout).not.toContain("&"); + expect(result.stdout).not.toContain("?"); + }); +}); + +// ── _generate_oauth_html ────────────────────────────────────────────────────── + +describe("_generate_oauth_html", () => { + it("should set OAUTH_SUCCESS_HTML variable", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_SUCCESS_HTML" + `); + expect(result.exitCode).toBe(0); + expect(result.stdout.length).toBeGreaterThan(0); + }); + + it("should set OAUTH_ERROR_HTML variable", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_ERROR_HTML" + `); + expect(result.exitCode).toBe(0); + expect(result.stdout.length).toBeGreaterThan(0); + }); + + it("should produce valid HTML in success page", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_SUCCESS_HTML" + `); + expect(result.stdout).toContain(""); + expect(result.stdout).toContain(""); + }); + + it("should include success message in success HTML", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_SUCCESS_HTML" + `); + expect(result.stdout).toContain("Authentication Successful"); + }); + + it("should include auto-close script in success HTML", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_SUCCESS_HTML" + `); + expect(result.stdout).toContain("window.close"); + }); + + it("should include CSRF protection message in error HTML", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_ERROR_HTML" + `); + expect(result.stdout).toContain("CSRF"); + }); + + it("should include 'Authentication Failed' in error HTML", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_ERROR_HTML" + `); + expect(result.stdout).toContain("Authentication Failed"); + }); + + it("should include 'try again' guidance in error HTML", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_ERROR_HTML" + `); + expect(result.stdout).toContain("try again"); + }); + + it("should include CSS styling in both pages", () => { + const result = runBash(` + _generate_oauth_html + echo "$OAUTH_SUCCESS_HTML" + echo "---SEPARATOR---" + echo "$OAUTH_ERROR_HTML" + `); + const parts = result.stdout.split("---SEPARATOR---"); + expect(parts[0]).toContain("