Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ boundaries:

Rules apply to all tasks (single or PRD).

Config safety behavior:
- Invalid config files fall back to defaults with a warning.
- Unsafe prototype-pollution keys are rejected during config parsing.

## AI Engines

```bash
Expand Down
12 changes: 6 additions & 6 deletions cli/src/cli/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@ export async function runLoop(options: RuntimeOptions): Promise<void> {
if (!existsSync(options.prdFile)) {
logError(`${options.prdFile} not found in current directory`);
logInfo(`Create a ${options.prdFile} file with tasks`);
process.exit(1);
throw new Error(`PRD source not found: ${options.prdFile}`);
}
} else if (options.prdSource === "markdown-folder") {
if (!existsSync(options.prdFile)) {
logError(`PRD folder ${options.prdFile} not found`);
logInfo(`Create a ${options.prdFile}/ folder with markdown files containing tasks`);
process.exit(1);
throw new Error(`PRD folder not found: ${options.prdFile}`);
}
}

if (options.prdSource === "github" && !options.githubRepo) {
logError("GitHub repository not specified. Use --github owner/repo");
process.exit(1);
throw new Error("GitHub repository not specified");
}

// Check engine availability
Expand All @@ -61,7 +61,7 @@ export async function runLoop(options: RuntimeOptions): Promise<void> {

if (!available) {
logError(`${engine.name} CLI not found. Make sure '${engine.cliCommand}' is in your PATH.`);
process.exit(1);
throw new Error(`${engine.name} CLI not available`);
}

// Create task source with caching for better performance
Expand Down Expand Up @@ -91,7 +91,7 @@ export async function runLoop(options: RuntimeOptions): Promise<void> {
logError("Cannot run in parallel/branch mode: repository has no commits yet.");
logInfo("Please make an initial commit first:");
logInfo(' git add . && git commit -m "Initial commit"');
process.exit(1);
throw new Error("Repository has no commits yet");
}
}

Expand Down Expand Up @@ -195,6 +195,6 @@ export async function runLoop(options: RuntimeOptions): Promise<void> {
}

if (result.tasksFailed > 0) {
process.exit(1);
throw new Error(`${result.tasksFailed} task(s) failed`);
}
}
6 changes: 3 additions & 3 deletions cli/src/cli/commands/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function runTask(task: string, options: RuntimeOptions): Promise<vo

if (!available) {
logError(`${engine.name} CLI not found. Make sure '${engine.cliCommand}' is in your PATH.`);
process.exit(1);
throw new Error(`${engine.name} CLI not available`);
}

logInfo(`Running task with ${engine.name}...`);
Expand Down Expand Up @@ -129,7 +129,7 @@ export async function runTask(task: string, options: RuntimeOptions): Promise<vo
tasksFailed: 1,
});
notifyTaskFailed(task, result.error || "Unknown error");
process.exit(1);
throw new Error(result.error || "Unknown error");
}
} catch (error) {
const errorMsg = error instanceof Error ? error.message : String(error);
Expand All @@ -140,6 +140,6 @@ export async function runTask(task: string, options: RuntimeOptions): Promise<vo
tasksFailed: 1,
});
notifyTaskFailed(task, errorMsg);
process.exit(1);
throw error instanceof Error ? error : new Error(errorMsg);
}
}
6 changes: 4 additions & 2 deletions cli/src/config/detector.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { existsSync, readFileSync } from "node:fs";
import { basename, join } from "node:path";
import { logDebug } from "../ui/logger.ts";

interface DetectedProject {
name: string;
Expand Down Expand Up @@ -106,8 +107,9 @@ function detectNodeProject(packageJsonPath: string, result: DetectedProject): vo
if (scripts.build) {
result.buildCmd = "npm run build";
}
} catch {
// Ignore parsing errors
} catch (error) {
// BUG FIX: Log parsing errors instead of silently ignoring
logDebug(`Failed to parse package.json: ${error}`);
}
}

Expand Down
4 changes: 2 additions & 2 deletions cli/src/config/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * from "./types.ts";
export * from "./loader.ts";
export * from "./detector.ts";
export * from "./loader.ts";
export * from "./types.ts";
export * from "./writer.ts";
99 changes: 93 additions & 6 deletions cli/src/config/loader.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,58 @@
import { existsSync, readFileSync } from "node:fs";
import { join } from "node:path";
import YAML from "yaml";
import { validateCommand } from "../engines/validation.ts";
import { logDebug, logError, logWarn } from "../ui/logger.ts";
import { type RalphyConfig, RalphyConfigSchema } from "./types.ts";

export const RALPHY_DIR = ".ralphy";

/**
* Recursively check for prototype pollution keys in parsed data
* BUG FIX: Uses recursive traversal instead of string matching to prevent Unicode escape bypasses
*/
function hasPrototypePollution(obj: unknown): boolean {
const MAX_DEPTH = 20;
const MAX_NODES = 10000;
const dangerousKeys = new Set(["__proto__", "constructor", "prototype"]);

if (typeof obj !== "object" || obj === null) return false;

const visited = new Set<unknown>();
const queue: Array<{ value: unknown; depth: number }> = [{ value: obj, depth: 0 }];
let nodesVisited = 0;

while (queue.length > 0) {
const current = queue.shift();
if (!current) continue;

nodesVisited++;
if (nodesVisited > MAX_NODES) {
throw new Error("Config file too complex to validate safely");
}

if (current.depth > MAX_DEPTH) {
throw new Error("Config file nesting exceeds safety limits");
}

if (typeof current.value !== "object" || current.value === null) {
continue;
}

if (visited.has(current.value)) {
continue;
}
visited.add(current.value);

for (const key of Object.keys(current.value)) {
if (dangerousKeys.has(key)) return true;
const value = (current.value as Record<string, unknown>)[key];
queue.push({ value, depth: current.depth + 1 });
}
}

return false;
}
export const CONFIG_FILE = "config.yaml";
export const PROGRESS_FILE = "progress.txt";

Expand Down Expand Up @@ -48,10 +97,27 @@ export function loadConfig(workDir = process.cwd()): RalphyConfig | null {
try {
const content = readFileSync(configPath, "utf-8");
const parsed = YAML.parse(content);

// BUG FIX: Proper prototype pollution protection with recursive check
// The old string-based check was bypassable via Unicode escapes
if (hasPrototypePollution(parsed)) {
throw new Error("Config file contains potentially malicious prototype pollution keys");
}

return RalphyConfigSchema.parse(parsed);
} catch (error) {
// Log error for debugging, but return default config
console.error(`Warning: Failed to parse config at ${configPath}:`, error);
const message = error instanceof Error ? error.message : String(error);
const isSecurityError =
message.includes("too complex") ||
message.includes("nesting exceeds") ||
message.includes("prototype pollution");
if (isSecurityError) {
logError(`Config security violation at ${configPath}: ${message}. Refusing to load config.`);
return null;
}

logWarn(`Invalid config file at ${configPath}: ${message}. Falling back to defaults.`);
logDebug(`Config parse stack: ${error instanceof Error ? error.stack || message : message}`);
return RalphyConfigSchema.parse({});
}
}
Expand All @@ -69,31 +135,52 @@ export function loadRules(workDir = process.cwd()): string[] {
*/
export function loadBoundaries(workDir = process.cwd()): string[] {
const config = loadConfig(workDir);
return config?.boundaries.never_touch ?? [];
return config?.boundaries?.never_touch ?? [];
}

/**
* Get test command from config
*/
export function loadTestCommand(workDir = process.cwd()): string {
const config = loadConfig(workDir);
return config?.commands.test ?? "";
const command = config?.commands.test ?? "";

if (command && !validateCommand(command)) {
logWarn(`Invalid test command in config: "${command}". Falling back to default.`);
return "";
}

return command;
}

/**
* Get lint command from config
*/
export function loadLintCommand(workDir = process.cwd()): string {
const config = loadConfig(workDir);
return config?.commands.lint ?? "";
const command = config?.commands.lint ?? "";

if (command && !validateCommand(command)) {
logWarn(`Invalid lint command in config: "${command}". Falling back to default.`);
return "";
}

return command;
Comment on lines 159 to +168
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium config/loader.ts:149

Suggestion: validateCommand forbids spaces, so multi-word commands in loadTestCommand, loadLintCommand, and loadBuildCommand are treated as invalid and reset to empty strings. Consider splitting the config into command and args and validate only the executable, or relax the validation to allow spaces/common arg syntax to avoid rejecting valid commands.

-	if (command && !validateCommand(command)) {
+	if (command && !validateCommand(command.split(' ')[0])) {
		logWarn(`Invalid lint command in config: "${command}". Falling back to default.`);
		return "";
	}
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file cli/src/config/loader.ts around lines 149-158:

Suggestion: `validateCommand` forbids spaces, so multi-word commands in `loadTestCommand`, `loadLintCommand`, and `loadBuildCommand` are treated as invalid and reset to empty strings. Consider splitting the config into `command` and `args` and validate only the executable, or relax the validation to allow spaces/common arg syntax to avoid rejecting valid commands.

Evidence trail:
cli/src/engines/validation.ts lines 50-56: validateCommand uses regex `/^[a-zA-Z0-9._\-/]+$/` which does not include space character. cli/src/config/loader.ts lines 127-139 (loadTestCommand), 149-161 (loadLintCommand), 166-178 (loadBuildCommand): all call validateCommand and return empty string "" when validation fails.

}

/**
* Get build command from config
*/
export function loadBuildCommand(workDir = process.cwd()): string {
const config = loadConfig(workDir);
return config?.commands.build ?? "";
const command = config?.commands.build ?? "";

if (command && !validateCommand(command)) {
logWarn(`Invalid build command in config: "${command}". Falling back to default.`);
return "";
}

return command;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions cli/src/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const NotificationsSchema = z.object({
discord_webhook: z.string().default(""),
slack_webhook: z.string().default(""),
custom_webhook: z.string().default(""),
telemetry_webhook: z.string().default(""),
});

/**
Expand Down Expand Up @@ -109,6 +110,8 @@ export interface RuntimeOptions {
browserEnabled: "auto" | "true" | "false";
/** Override default model for the engine */
modelOverride?: string;
/** Enable comprehensive OpenCode debugging */
debugOpenCode?: boolean;
/** Skip automatic branch merging after parallel execution */
skipMerge?: boolean;
/** Use lightweight sandboxes instead of git worktrees for parallel execution */
Expand Down Expand Up @@ -142,4 +145,5 @@ export const DEFAULT_OPTIONS: RuntimeOptions = {
githubLabel: "",
autoCommit: true,
browserEnabled: "auto",
debugOpenCode: false,
};
73 changes: 15 additions & 58 deletions cli/src/config/writer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { appendFile } from "node:fs/promises";
import YAML from "yaml";
import { logWarn } from "../ui/logger.ts";
import { detectProject } from "./detector.ts";
import { getConfigPath, getProgressPath, getRalphyDir } from "./loader.ts";
import type { RalphyConfig } from "./types.ts";
Expand Down Expand Up @@ -35,7 +36,7 @@ rules:
# - "Use server actions instead of API routes in Next.js"
#
# Skills/playbooks (optional):
# - "Before coding, read and follow any relevant skill/playbook docs under .opencode/skills, .claude/skills, or .github/skills."
# - "Before coding, read and follow any relevant skill/playbook docs under .opencode/skills or .claude/skills."

# Boundaries - files/folders the AI should not modify
boundaries:
Expand All @@ -49,9 +50,17 @@ boundaries:

/**
* Escape a value for safe YAML string
* BUG FIX: Use YAML library for proper escaping to prevent injection attacks
*/
function escapeYaml(value: string | undefined | null): string {
return (value || "").replace(/"/g, '\\"');
if (!value) return "";
// Keep the value safe inside an existing double-quoted scalar.
// This prevents quote breakout and newline-based injection.
return value
.replace(/\\/g, "\\\\")
.replace(/"/g, '\\"')
.replace(/\r/g, "\\r")
.replace(/\n/g, "\\n");
}
Comment on lines 55 to 64
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High config/writer.ts:53

escapeYaml now uses YAML.stringify, which can return single-quoted strings (e.g., 'foo"bar') or block scalars. createConfigContent wraps this output in double quotes, so a command like echo "hello" becomes name: "'echo \"hello\"'" — a string literal containing single quotes that corrupts the value when parsed. For multi-line strings, wrapping a block scalar in quotes produces invalid YAML.

 function escapeYaml(value: string | undefined | null): string {
 	if (!value) return "";
-	// Use YAML library for proper escaping instead of simple quote replacement
-	// This prevents YAML injection attacks
-	const serialized = YAML.stringify(value).trim();
-	// Remove surrounding quotes added by YAML.stringify for simple strings
-	return serialized.replace(/^"|"$/g, "");
+	// Escape backslashes and double quotes, then wrap in double quotes
+	const escaped = value.replace(/\\/g, "\\\\").replace(/"/g, "\\\"");
+	return `"${escaped}"`;
 }
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file cli/src/config/writer.ts around lines 53-60:

`escapeYaml` now uses `YAML.stringify`, which can return single-quoted strings (e.g., `'foo"bar'`) or block scalars. `createConfigContent` wraps this output in double quotes, so a command like `echo "hello"` becomes `name: "'echo \"hello\"'"` — a string literal containing single quotes that corrupts the value when parsed. For multi-line strings, wrapping a block scalar in quotes produces invalid YAML.

Evidence trail:
cli/src/config/writer.ts lines 50-57: `escapeYaml` function uses `YAML.stringify(value).trim()` and strips only double quotes with `replace(/^"|"/g, "")`. Single quotes and block scalars are not handled.

cli/src/config/writer.ts lines 15-21: `createConfigContent` wraps `escapeYaml` output in double quotes, e.g., `name: "${escapeYaml(detected.name)}"` and `test: "${escapeYaml(detected.testCmd)}"`.

When `YAML.stringify` returns single-quoted strings (for inputs containing double quotes) or block scalars (for multi-line strings), the wrapping in double quotes produces invalid YAML.


/**
Expand Down Expand Up @@ -108,44 +117,8 @@ export function addRule(rule: string, workDir = process.cwd()): void {
writeFileSync(configPath, YAML.stringify(parsed), "utf-8");
}

/** Queue for batching progress writes */
const progressWriteQueue: Map<string, string[]> = new Map();
let flushTimeout: ReturnType<typeof setTimeout> | null = null;

/**
* Flush all pending progress writes to disk
*/
async function flushProgressWrites(): Promise<void> {
if (progressWriteQueue.size === 0) return;

const entries = [...progressWriteQueue.entries()];
progressWriteQueue.clear();
flushTimeout = null;

for (const [path, lines] of entries) {
try {
await appendFile(path, lines.join(""), "utf-8");
} catch {
// Ignore write errors for progress logging
}
}
}

/**
* Schedule a flush of progress writes (debounced)
*/
function scheduleFlush(): void {
if (flushTimeout) return;
flushTimeout = setTimeout(() => {
void flushProgressWrites();
}, 100); // Batch writes within 100ms window
}

/**
* Log a task to the progress file (async, batched)
*
* Performance optimized: uses async I/O and batches writes within 100ms windows
* to reduce file system contention in parallel mode.
* Log a task to the progress file
*/
export function logTaskProgress(
task: string,
Expand All @@ -162,23 +135,7 @@ export function logTaskProgress(
const icon = status === "completed" ? "✓" : "✗";
const line = `- [${icon}] ${timestamp} - ${task}\n`;

// Add to write queue
const existing = progressWriteQueue.get(progressPath) || [];
existing.push(line);
progressWriteQueue.set(progressPath, existing);

// Schedule async flush
scheduleFlush();
}

/**
* Force flush all pending progress writes immediately
* Call this before process exit to ensure all writes are persisted
*/
export async function flushAllProgressWrites(): Promise<void> {
if (flushTimeout) {
clearTimeout(flushTimeout);
flushTimeout = null;
}
await flushProgressWrites();
void appendFile(progressPath, line, "utf-8").catch((error) => {
logWarn(`Failed to append task progress: ${error}`);
});
}
Loading