diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..88001b1 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,22 @@ +version: 2 +updates: + - package-ecosystem: "npm" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + time: "06:00" + timezone: "UTC" + open-pull-requests-limit: 10 + groups: + npm-and-yarn: + patterns: + - "*" + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + time: "06:30" + timezone: "UTC" diff --git a/package-lock.json b/package-lock.json index aa4c190..63af97d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1855,9 +1855,9 @@ } }, "node_modules/brace-expansion": { - "version": "5.0.3", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.3.tgz", - "integrity": "sha512-fy6KJm2RawA5RcHkLa1z/ScpBeA762UF9KmZQxwIbDtRJrgLzM10depAiEQ+CXYcoiqW1/m96OAAoke2nE9EeA==", + "version": "5.0.5", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.5.tgz", + "integrity": "sha512-VZznLgtwhn+Mact9tfiwx64fA9erHH/MCXEUfB/0bX/6Fz6ny5EGTXYltMocqg4xFAQZtnO3DHWWXi8RiuN7cQ==", "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" @@ -3494,9 +3494,9 @@ } }, "node_modules/path-to-regexp": { - "version": "0.1.12", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.12.tgz", - "integrity": "sha512-RA1GjUVMnvYFxuqovrEqZoxxW5NUZqbwKtYz/Tt7nXerk0LbLblQmrsgdeOxV5SFHf0UDggjS/bSeOZwt1pmEQ==", + "version": "0.1.13", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.13.tgz", + "integrity": "sha512-A/AGNMFN3c8bOlvV9RreMdrv7jsmF9XIfDeCd87+I8RNg6s78BhJxMu69NEMHBSJFxKidViTEdruRwEk/WIKqA==", "license": "MIT" }, "node_modules/pathe": { @@ -3513,9 +3513,9 @@ "license": "ISC" }, "node_modules/picomatch": { - "version": "4.0.3", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", - "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", + "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", "engines": { @@ -4273,9 +4273,9 @@ } }, "node_modules/vite": { - "version": "7.3.1", - "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.1.tgz", - "integrity": "sha512-w+N7Hifpc3gRjZ63vYBXA56dvvRlNWRczTdmCBBa+CotUzAPf5b7YMdMR/8CQoeYE5LX3W4wj6RYTgonm1b9DA==", + "version": "7.3.2", + "resolved": "https://registry.npmjs.org/vite/-/vite-7.3.2.tgz", + "integrity": "sha512-Bby3NOsna2jsjfLVOHKes8sGwgl4TT0E6vvpYgnAYDIF/tie7MRaFthmKuHx1NSXjiTueXH3do80FMQgvEktRg==", "dev": true, "license": "MIT", "dependencies": { diff --git a/src/commands/config.test.ts b/src/commands/config.test.ts index 4d98594..5dc694c 100644 --- a/src/commands/config.test.ts +++ b/src/commands/config.test.ts @@ -87,7 +87,8 @@ describe('handleConfigCommand - init guard', () => { // Should write to .ralph/.env.local expect(writeSpy).toHaveBeenCalledWith( envLocalPath, - 'TAVILY_API_KEY=tvly-test-key-1234567890\n' + 'TAVILY_API_KEY=tvly-test-key-1234567890\n', + { mode: 0o600 } ); // Should set environment variable @@ -212,15 +213,27 @@ describe('handleConfigCommand - init guard', () => { // Test tavily await handleConfigCommand(['set', 'tavily', 'tvly-key-1234567890'], mockState); - expect(writeSpy).toHaveBeenLastCalledWith(envLocalPath, 'TAVILY_API_KEY=tvly-key-1234567890\n'); + expect(writeSpy).toHaveBeenLastCalledWith( + envLocalPath, + 'TAVILY_API_KEY=tvly-key-1234567890\n', + { mode: 0o600 } + ); // Test context7 await handleConfigCommand(['set', 'context7', 'c7-key-1234567890'], mockState); - expect(writeSpy).toHaveBeenLastCalledWith(envLocalPath, 'CONTEXT7_API_KEY=c7-key-1234567890\n'); + expect(writeSpy).toHaveBeenLastCalledWith( + envLocalPath, + 'CONTEXT7_API_KEY=c7-key-1234567890\n', + { mode: 0o600 } + ); // Test braintrust await handleConfigCommand(['set', 'braintrust', 'bt-key-1234567890'], mockState); - expect(writeSpy).toHaveBeenLastCalledWith(envLocalPath, 'BRAINTRUST_API_KEY=bt-key-1234567890\n'); + expect(writeSpy).toHaveBeenLastCalledWith( + envLocalPath, + 'BRAINTRUST_API_KEY=bt-key-1234567890\n', + { mode: 0o600 } + ); }); it('persists /config set cli codex into ralph.config.cjs', async () => { diff --git a/src/generator/templates.test.ts b/src/generator/templates.test.ts index a160b15..c7f1cab 100644 --- a/src/generator/templates.test.ts +++ b/src/generator/templates.test.ts @@ -324,15 +324,17 @@ describe('feature-loop.sh.tmpl — CLI adapter routing', () => { it('supports codex exec and codex exec resume JSON paths', () => { const template = readFeatureLoopTemplate(); - expect(template).toContain('echo "codex --ask-for-approval \\"$CODEX_APPROVAL_POLICY\\" --sandbox \\"$CODEX_SANDBOX\\" exec -C \\"$APP_DIR\\" --model \\"${model}\\"${codex_extra}"'); + expect(template).toContain('echo "codex --ask-for-approval ${CODEX_APPROVAL_POLICY} --sandbox ${CODEX_SANDBOX} exec --cd . --model ${model}${codex_extra}"'); expect(template).toContain('DISABLE_MCP_IN_AUTOMATED_NORM=$(echo "$DISABLE_MCP_IN_AUTOMATED" | tr \'[:upper:]\' \'[:lower:]\')'); expect(template).toContain('if [ "${RALPH_AUTOMATED:-}" = "1" ] && [ "$DISABLE_MCP_IN_AUTOMATED_NORM" = "true" ]; then'); - expect(template).toContain(`codex_extra=" -c 'mcp_servers={}'"`); + expect(template).toContain('codex_extra=" -c mcp_servers={}"'); expect(template).toContain('claude -p --output-format json --permission-mode ${CLAUDE_PERMISSION_MODE} --model ${model}'); - expect(template).toContain('eval "$claude_cmd --json --output-last-message \\"$LAST_MESSAGE_FILE\\" -"'); + expect(template).toContain('read -r -a cmd_parts <<< "$claude_cmd"'); + expect(template).toContain('"${cmd_parts[@]}" --json --output-last-message "$LAST_MESSAGE_FILE" -'); expect(template).toContain('local resume_cmd="${claude_cmd/ exec / exec resume }"'); - expect(template).toContain('resume_cmd="${resume_cmd/ -C \\"$APP_DIR\\"/}"'); - expect(template).toContain('cd "$APP_DIR" && eval "$resume_cmd \\"$session_id\\" - --json --output-last-message \\"$LAST_MESSAGE_FILE\\""'); + expect(template).toContain('resume_cmd="${resume_cmd/ --cd ./}"'); + expect(template).toContain('"${resume_parts[@]}" "$session_id" - --json --output-last-message "$LAST_MESSAGE_FILE"'); + expect(template).not.toContain('eval "$'); }); it('extracts Codex token usage using multiple key shapes without overcounting repeated events', () => { @@ -389,7 +391,8 @@ describe('feature-loop.sh.tmpl — CLI adapter routing', () => { it('parses review-fix output with implementation CLI adapter', () => { const template = readFeatureLoopTemplate(); expect(template).toContain('local impl_cli="$CODING_CLI"'); - expect(template).toContain('impl_cmd="$IMPL_CMD --json --output-last-message \\"$LAST_MESSAGE_FILE\\""'); + expect(template).toContain('read -r -a impl_cmd_parts <<< "$IMPL_CMD"'); + expect(template).toContain('"${impl_cmd_parts[@]}" --json --output-last-message "$LAST_MESSAGE_FILE" -'); expect(template).toContain('extract_session_result "${CLAUDE_OUTPUT}.raw" "$impl_cli"'); expect(template).toContain('accumulate_tokens_from_session "$LAST_SESSION_ID" "${CLAUDE_OUTPUT}.raw" "$impl_cli"'); }); diff --git a/src/templates/scripts/feature-loop.sh.tmpl b/src/templates/scripts/feature-loop.sh.tmpl index 6f692b3..f51e5c5 100644 --- a/src/templates/scripts/feature-loop.sh.tmpl +++ b/src/templates/scripts/feature-loop.sh.tmpl @@ -236,6 +236,10 @@ fi build_cli_cmd() { local cli="$1" local model="$2" + if [[ ! "$model" =~ ^[A-Za-z0-9._:/=-]+$ ]]; then + echo "ERROR: Invalid model value '$model'. Only alphanumeric, dot, underscore, colon, slash, equals and hyphen are allowed." >&2 + return 1 + fi case "$cli" in claude) echo "claude -p --output-format json --permission-mode ${CLAUDE_PERMISSION_MODE} --model ${model}" @@ -244,9 +248,10 @@ build_cli_cmd() { local codex_extra="" # Avoid MCP startup deadlocks in unattended loop runs. if [ "${RALPH_AUTOMATED:-}" = "1" ] && [ "$DISABLE_MCP_IN_AUTOMATED_NORM" = "true" ]; then - codex_extra=" -c 'mcp_servers={}'" + codex_extra=" -c mcp_servers={}" fi - echo "codex --ask-for-approval \"$CODEX_APPROVAL_POLICY\" --sandbox \"$CODEX_SANDBOX\" exec -C \"$APP_DIR\" --model \"${model}\"${codex_extra}" + # Keep command string token-safe. run_claude_* parses it as an argument array. + echo "codex --ask-for-approval ${CODEX_APPROVAL_POLICY} --sandbox ${CODEX_SANDBOX} exec --cd . --model ${model}${codex_extra}" ;; *) echo "ERROR: Unsupported CLI '$cli'" >&2 @@ -332,12 +337,14 @@ fi run_claude_prompt() { local prompt_file="$1" local claude_cmd="$2" - if [[ "$claude_cmd" == codex* ]]; then + local -a cmd_parts=() + read -r -a cmd_parts <<< "$claude_cmd" + if [[ "${cmd_parts[0]:-}" == "codex" ]]; then LAST_RUN_CLI="codex" - { cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && eval "$claude_cmd --json --output-last-message \"$LAST_MESSAGE_FILE\" -") + { cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && "${cmd_parts[@]}" --json --output-last-message "$LAST_MESSAGE_FILE" -) else LAST_RUN_CLI="claude" - { cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | $claude_cmd + { cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | "${cmd_parts[@]}" fi } @@ -346,6 +353,10 @@ run_claude_resume() { local session_id="$1" local continuation_prompt="$2" local claude_cmd="$3" + if [[ ! "$session_id" =~ ^[A-Za-z0-9._:-]+$ ]]; then + echo "WARNING: Refusing to resume with unsafe session id '$session_id'" >&2 + return 1 + fi if [[ "$claude_cmd" == codex* ]]; then LAST_RUN_CLI="codex" local resume_cmd="${claude_cmd/ exec / exec resume }" @@ -354,18 +365,22 @@ run_claude_resume() { return 1 fi # codex exec resume does not accept -C/--cd; resume from APP_DIR instead. - resume_cmd="${resume_cmd/ -C \"$APP_DIR\"/}" - resume_cmd="${resume_cmd/ --cd \"$APP_DIR\"/}" - { echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && eval "$resume_cmd \"$session_id\" - --json --output-last-message \"$LAST_MESSAGE_FILE\"") + resume_cmd="${resume_cmd/ --cd ./}" + resume_cmd="${resume_cmd/ -C ./}" + local -a resume_parts=() + read -r -a resume_parts <<< "$resume_cmd" + { echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && "${resume_parts[@]}" "$session_id" - --json --output-last-message "$LAST_MESSAGE_FILE") else LAST_RUN_CLI="claude" - # Insert --resume "$session_id" before the -p flag - local resume_cmd="${claude_cmd/ -p / --resume \"$session_id\" -p }" + # Insert --resume before the -p flag. + local resume_cmd="${claude_cmd/ -p / --resume ${session_id} -p }" if [ "$resume_cmd" = "$claude_cmd" ]; then echo "WARNING: --resume injection failed, -p flag not found in command" >&2 return 1 fi - { echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | $resume_cmd + local -a resume_parts=() + read -r -a resume_parts <<< "$resume_cmd" + { echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | "${resume_parts[@]}" fi } @@ -931,15 +946,29 @@ except Exception: run_review_fix() { local findings local impl_cli="$CODING_CLI" - local impl_cmd="$IMPL_CMD" findings=$(extract_review_findings "${CLAUDE_OUTPUT}.raw") + local -a impl_cmd_parts=() + read -r -a impl_cmd_parts <<< "$IMPL_CMD" if [ "$impl_cli" = "codex" ]; then LAST_RUN_CLI="codex" - impl_cmd="$IMPL_CMD --json --output-last-message \"$LAST_MESSAGE_FILE\"" + cat <&1 | tee "${CLAUDE_OUTPUT}.raw" || true +## Code Review Findings + +The following issues were found during code review: + +${findings} + +## Task + +Fix each issue listed above. Run git diff $DEFAULT_BRANCH to see the current changes, then: +1. Fix each issue referenced in the review +2. Run tests to verify fixes +3. Commit and push the fixes +Do NOT propose completion options or ask interactive questions. Just fix, test, commit, push. +FIXEOF else LAST_RUN_CLI="claude" - fi - cat <&1 | tee "${CLAUDE_OUTPUT}.raw" || true + cat <&1 | tee "${CLAUDE_OUTPUT}.raw" || true ## Code Review Findings The following issues were found during code review: @@ -954,10 +983,36 @@ Fix each issue listed above. Run git diff $DEFAULT_BRANCH to see the current cha 3. Commit and push the fixes Do NOT propose completion options or ask interactive questions. Just fix, test, commit, push. FIXEOF + fi extract_session_result "${CLAUDE_OUTPUT}.raw" "$impl_cli" accumulate_tokens_from_session "$LAST_SESSION_ID" "${CLAUDE_OUTPUT}.raw" "$impl_cli" } +validate_simple_command() { + local command="$1" + if [[ "$command" =~ [\;\&\|\<\>\`\$\(\)] ]]; then + return 1 + fi + if [[ "$command" =~ [[:cntrl:]] ]]; then + return 1 + fi + return 0 +} + +run_test_command() { + if ! validate_simple_command "$TEST_COMMAND"; then + echo "ERROR: commands.test contains unsupported shell operators. Use a plain command and arguments." >&2 + return 2 + fi + local -a test_cmd_parts=() + read -r -a test_cmd_parts <<< "$TEST_COMMAND" + if [ ${#test_cmd_parts[@]} -eq 0 ]; then + echo "ERROR: commands.test resolved to an empty command." >&2 + return 2 + fi + (cd "$APP_DIR" && "${test_cmd_parts[@]}") +} + # Normalize test failure lines: extract test name, strip timing, deduplicate. # This makes baseline comparison stable across runs where timing values change. normalize_test_failures() { @@ -968,7 +1023,7 @@ normalize_test_failures() { # Returns 0 if tests pass OR all failures are pre-existing (captured at baseline). check_tests_pass_or_baseline() { local test_output - test_output=$( (cd "$APP_DIR" && eval "$TEST_COMMAND") 2>&1 ) + test_output=$(run_test_command 2>&1) local exit_code=$? if [ $exit_code -eq 0 ]; then @@ -1152,11 +1207,11 @@ fi # Capture baseline test failures for pre-existing failure detection BASELINE_FAILURES_FILE="/tmp/ralph-loop-${FEATURE}.baseline-failures" echo "Capturing baseline test failures..." -if (cd "$APP_DIR" && eval "$TEST_COMMAND" 2>&1) > /dev/null 2>&1; then +if run_test_command > /dev/null 2>&1; then echo "Baseline: all tests passing" : > "$BASELINE_FAILURES_FILE" else - (cd "$APP_DIR" && eval "$TEST_COMMAND" 2>&1) | normalize_test_failures > "$BASELINE_FAILURES_FILE" 2>/dev/null || true + run_test_command 2>&1 | normalize_test_failures > "$BASELINE_FAILURES_FILE" 2>/dev/null || true BASELINE_COUNT=$(wc -l < "$BASELINE_FAILURES_FILE" | tr -d ' ') echo "Baseline: $BASELINE_COUNT pre-existing test failure(s) recorded" fi diff --git a/src/utils/env.test.ts b/src/utils/env.test.ts index 427149b..a11b96c 100644 --- a/src/utils/env.test.ts +++ b/src/utils/env.test.ts @@ -271,13 +271,16 @@ describe('writeKeysToEnvFile', () => { vi.spyOn(fs, 'existsSync').mockReturnValue(false); vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined); vi.spyOn(fs, 'writeFileSync').mockReturnValue(undefined); + vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined); writeKeysToEnvFile(filePath, keys); expect(fs.writeFileSync).toHaveBeenCalledWith( filePath, - 'OPENAI_API_KEY=sk-test-123\n' + 'OPENAI_API_KEY=sk-test-123\n', + { mode: 0o600 } ); + expect(fs.chmodSync).toHaveBeenCalledWith(filePath, 0o600); }); it('merges keys into existing file content (preserves other keys)', () => { @@ -288,6 +291,9 @@ describe('writeKeysToEnvFile', () => { vi.spyOn(fs, 'existsSync').mockReturnValue(true); vi.spyOn(fs, 'readFileSync').mockReturnValue(existingContent); vi.spyOn(fs, 'writeFileSync').mockReturnValue(undefined); + vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined); + vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined); + vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined); writeKeysToEnvFile(filePath, keys); @@ -322,13 +328,16 @@ describe('writeKeysToEnvFile', () => { vi.spyOn(fs, 'existsSync').mockReturnValue(true); vi.spyOn(fs, 'readFileSync').mockReturnValue(existingContent); vi.spyOn(fs, 'writeFileSync').mockReturnValue(undefined); + vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined); writeKeysToEnvFile(filePath, keys); expect(fs.writeFileSync).toHaveBeenCalledWith( filePath, - existingContent + existingContent, + { mode: 0o600 } ); + expect(fs.chmodSync).toHaveBeenCalledWith(filePath, 0o600); }); it('skips keys with empty string values', () => { diff --git a/src/utils/env.ts b/src/utils/env.ts index 2b1cc02..b29c726 100644 --- a/src/utils/env.ts +++ b/src/utils/env.ts @@ -86,7 +86,12 @@ export function writeKeysToEnvFile(filePath: string, keys: Record