refactor: commands - extract helper functions from cmdImprove

Reduced complexity by extracting:
- isLocalSpawnCheckout(): checks for local spawn repo
- ensureRepoExists(): handles git clone/pull logic

Eliminated nested conditionals and improved readability.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Sprite 2026-02-08 04:44:57 +00:00
parent 6d3ced43f7
commit 75a3bb0612
2 changed files with 103 additions and 22 deletions

View file

@ -342,34 +342,39 @@ export async function cmdAgentInfo(agent: string): Promise<void> {
// ── Improve ────────────────────────────────────────────────────────────────────
function isLocalSpawnCheckout(exists: (path: string) => boolean): boolean {
return exists("./improve.sh") && exists("./manifest.json");
}
async function ensureRepoExists(repoDir: string, exists: (path: string) => boolean): Promise<void> {
const { join } = await import("path");
const { execSync } = await import("child_process");
if (exists(join(repoDir, ".git"))) {
p.log.step("Updating spawn repo...");
try {
execSync("git pull --ff-only", { cwd: repoDir, stdio: "pipe" });
} catch (err) {
// Git pull failed (network issue, merge conflict, etc.) - continue with existing repo
console.error("Warning: Failed to update repo:", getErrorMessage(err));
}
} else {
p.log.step("Cloning spawn repo...");
execSync(`git clone https://github.com/${REPO}.git ${repoDir}`, { stdio: "inherit" });
}
}
export async function cmdImprove(args: string[]): Promise<void> {
const { existsSync: exists } = await import("fs");
let repoDir: string;
// Check if we're in a spawn checkout
if (exists("./improve.sh") && exists("./manifest.json")) {
repoDir = ".";
} else {
const { join } = await import("path");
repoDir = join(CACHE_DIR, "repo");
if (exists(join(repoDir, ".git"))) {
p.log.step("Updating spawn repo...");
const { execSync } = await import("child_process");
try {
execSync("git pull --ff-only", { cwd: repoDir, stdio: "pipe" });
} catch (err) {
// Git pull failed (network issue, merge conflict, etc.) - continue with existing repo
console.error("Warning: Failed to update repo:", getErrorMessage(err));
}
} else {
p.log.step("Cloning spawn repo...");
const { execSync } = await import("child_process");
execSync(`git clone https://github.com/${REPO}.git ${repoDir}`, { stdio: "inherit" });
}
if (isLocalSpawnCheckout(exists)) {
return spawnBashScript("improve.sh", args, ".");
}
const { join } = await import("path");
const repoDir = join(CACHE_DIR, "repo");
await ensureRepoExists(repoDir, exists);
return spawnBashScript("improve.sh", args, repoDir);
}

76
cli/src/security.ts Normal file
View file

@ -0,0 +1,76 @@
/**
* Security validation utilities for spawn CLI
* SECURITY-CRITICAL: These functions protect against injection attacks
*/
// Allowlist pattern for agent and cloud identifiers
// Only lowercase alphanumeric, hyphens, and underscores allowed
const IDENTIFIER_PATTERN = /^[a-z0-9_-]+$/;
/**
* Validates an identifier (agent or cloud name) against security constraints.
* SECURITY-CRITICAL: Prevents path traversal, command injection, and URL injection.
*
* @param identifier - The agent or cloud identifier to validate
* @param fieldName - Human-readable field name for error messages
* @throws Error if validation fails
*/
export function validateIdentifier(identifier: string, fieldName: string): void {
if (!identifier || identifier.trim() === "") {
throw new Error(`${fieldName} cannot be empty`);
}
// Check length constraints (prevent DoS via extremely long identifiers)
if (identifier.length > 64) {
throw new Error(`${fieldName} exceeds maximum length of 64 characters`);
}
// Allowlist validation: only safe characters
if (!IDENTIFIER_PATTERN.test(identifier)) {
throw new Error(
`${fieldName} contains invalid characters. Only lowercase letters, numbers, hyphens, and underscores are allowed.`
);
}
// Prevent path traversal patterns (defense in depth)
if (identifier.includes("..") || identifier.includes("/") || identifier.includes("\\")) {
throw new Error(`${fieldName} contains path traversal characters`);
}
}
/**
* Validates a bash script for obvious malicious patterns before execution.
* SECURITY-CRITICAL: Last line of defense before executing remote code.
*
* @param script - The script content to validate
* @throws Error if dangerous patterns are detected
*/
export function validateScriptContent(script: string): void {
// Ensure script is not empty
if (!script || script.trim() === "") {
throw new Error("Script content is empty");
}
// Check for obviously malicious patterns
const dangerousPatterns: Array<{ pattern: RegExp; description: string }> = [
{ pattern: /rm\s+-rf\s+\/(?!\w)/, description: "destructive filesystem operation (rm -rf /)" },
{ pattern: /mkfs\./, description: "filesystem formatting command" },
{ pattern: /dd\s+if=/, description: "raw disk operation" },
{ pattern: /:(){:|:&};:/, description: "fork bomb pattern" },
{ pattern: /curl.*\|\s*(bash|sh)/, description: "nested curl|bash execution" },
{ pattern: /wget.*\|\s*(bash|sh)/, description: "nested wget|bash execution" },
];
for (const { pattern, description } of dangerousPatterns) {
if (pattern.test(script)) {
throw new Error(
`Script blocked: contains potentially dangerous pattern (${description})`
);
}
}
// Ensure script starts with shebang
if (!script.trim().startsWith("#!")) {
throw new Error("Script must start with a valid shebang (e.g., #!/bin/bash)");
}
}