Skip to content

Commit 430be31

Browse files
authored
Security hardening + Dependabot remediation (#165)
* Security hardening and Dependabot remediation * Remove advanced CodeQL workflow; rely on default setup
1 parent 4252f9c commit 430be31

7 files changed

Lines changed: 150 additions & 43 deletions

File tree

.github/dependabot.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
version: 2
2+
updates:
3+
- package-ecosystem: "npm"
4+
directory: "/"
5+
schedule:
6+
interval: "weekly"
7+
day: "monday"
8+
time: "06:00"
9+
timezone: "UTC"
10+
open-pull-requests-limit: 10
11+
groups:
12+
npm-and-yarn:
13+
patterns:
14+
- "*"
15+
16+
- package-ecosystem: "github-actions"
17+
directory: "/"
18+
schedule:
19+
interval: "weekly"
20+
day: "monday"
21+
time: "06:30"
22+
timezone: "UTC"

package-lock.json

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/commands/config.test.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ describe('handleConfigCommand - init guard', () => {
8787
// Should write to .ralph/.env.local
8888
expect(writeSpy).toHaveBeenCalledWith(
8989
envLocalPath,
90-
'TAVILY_API_KEY=tvly-test-key-1234567890\n'
90+
'TAVILY_API_KEY=tvly-test-key-1234567890\n',
91+
{ mode: 0o600 }
9192
);
9293

9394
// Should set environment variable
@@ -212,15 +213,27 @@ describe('handleConfigCommand - init guard', () => {
212213

213214
// Test tavily
214215
await handleConfigCommand(['set', 'tavily', 'tvly-key-1234567890'], mockState);
215-
expect(writeSpy).toHaveBeenLastCalledWith(envLocalPath, 'TAVILY_API_KEY=tvly-key-1234567890\n');
216+
expect(writeSpy).toHaveBeenLastCalledWith(
217+
envLocalPath,
218+
'TAVILY_API_KEY=tvly-key-1234567890\n',
219+
{ mode: 0o600 }
220+
);
216221

217222
// Test context7
218223
await handleConfigCommand(['set', 'context7', 'c7-key-1234567890'], mockState);
219-
expect(writeSpy).toHaveBeenLastCalledWith(envLocalPath, 'CONTEXT7_API_KEY=c7-key-1234567890\n');
224+
expect(writeSpy).toHaveBeenLastCalledWith(
225+
envLocalPath,
226+
'CONTEXT7_API_KEY=c7-key-1234567890\n',
227+
{ mode: 0o600 }
228+
);
220229

221230
// Test braintrust
222231
await handleConfigCommand(['set', 'braintrust', 'bt-key-1234567890'], mockState);
223-
expect(writeSpy).toHaveBeenLastCalledWith(envLocalPath, 'BRAINTRUST_API_KEY=bt-key-1234567890\n');
232+
expect(writeSpy).toHaveBeenLastCalledWith(
233+
envLocalPath,
234+
'BRAINTRUST_API_KEY=bt-key-1234567890\n',
235+
{ mode: 0o600 }
236+
);
224237
});
225238

226239
it('persists /config set cli codex into ralph.config.cjs', async () => {

src/generator/templates.test.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -324,15 +324,17 @@ describe('feature-loop.sh.tmpl — CLI adapter routing', () => {
324324

325325
it('supports codex exec and codex exec resume JSON paths', () => {
326326
const template = readFeatureLoopTemplate();
327-
expect(template).toContain('echo "codex --ask-for-approval \\"$CODEX_APPROVAL_POLICY\\" --sandbox \\"$CODEX_SANDBOX\\" exec -C \\"$APP_DIR\\" --model \\"${model}\\"${codex_extra}"');
327+
expect(template).toContain('echo "codex --ask-for-approval ${CODEX_APPROVAL_POLICY} --sandbox ${CODEX_SANDBOX} exec --cd . --model ${model}${codex_extra}"');
328328
expect(template).toContain('DISABLE_MCP_IN_AUTOMATED_NORM=$(echo "$DISABLE_MCP_IN_AUTOMATED" | tr \'[:upper:]\' \'[:lower:]\')');
329329
expect(template).toContain('if [ "${RALPH_AUTOMATED:-}" = "1" ] && [ "$DISABLE_MCP_IN_AUTOMATED_NORM" = "true" ]; then');
330-
expect(template).toContain(`codex_extra=" -c 'mcp_servers={}'"`);
330+
expect(template).toContain('codex_extra=" -c mcp_servers={}"');
331331
expect(template).toContain('claude -p --output-format json --permission-mode ${CLAUDE_PERMISSION_MODE} --model ${model}');
332-
expect(template).toContain('eval "$claude_cmd --json --output-last-message \\"$LAST_MESSAGE_FILE\\" -"');
332+
expect(template).toContain('read -r -a cmd_parts <<< "$claude_cmd"');
333+
expect(template).toContain('"${cmd_parts[@]}" --json --output-last-message "$LAST_MESSAGE_FILE" -');
333334
expect(template).toContain('local resume_cmd="${claude_cmd/ exec / exec resume }"');
334-
expect(template).toContain('resume_cmd="${resume_cmd/ -C \\"$APP_DIR\\"/}"');
335-
expect(template).toContain('cd "$APP_DIR" && eval "$resume_cmd \\"$session_id\\" - --json --output-last-message \\"$LAST_MESSAGE_FILE\\""');
335+
expect(template).toContain('resume_cmd="${resume_cmd/ --cd ./}"');
336+
expect(template).toContain('"${resume_parts[@]}" "$session_id" - --json --output-last-message "$LAST_MESSAGE_FILE"');
337+
expect(template).not.toContain('eval "$');
336338
});
337339

338340
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', () => {
389391
it('parses review-fix output with implementation CLI adapter', () => {
390392
const template = readFeatureLoopTemplate();
391393
expect(template).toContain('local impl_cli="$CODING_CLI"');
392-
expect(template).toContain('impl_cmd="$IMPL_CMD --json --output-last-message \\"$LAST_MESSAGE_FILE\\""');
394+
expect(template).toContain('read -r -a impl_cmd_parts <<< "$IMPL_CMD"');
395+
expect(template).toContain('"${impl_cmd_parts[@]}" --json --output-last-message "$LAST_MESSAGE_FILE" -');
393396
expect(template).toContain('extract_session_result "${CLAUDE_OUTPUT}.raw" "$impl_cli"');
394397
expect(template).toContain('accumulate_tokens_from_session "$LAST_SESSION_ID" "${CLAUDE_OUTPUT}.raw" "$impl_cli"');
395398
});

src/templates/scripts/feature-loop.sh.tmpl

Lines changed: 73 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,10 @@ fi
236236
build_cli_cmd() {
237237
local cli="$1"
238238
local model="$2"
239+
if [[ ! "$model" =~ ^[A-Za-z0-9._:/=-]+$ ]]; then
240+
echo "ERROR: Invalid model value '$model'. Only alphanumeric, dot, underscore, colon, slash, equals and hyphen are allowed." >&2
241+
return 1
242+
fi
239243
case "$cli" in
240244
claude)
241245
echo "claude -p --output-format json --permission-mode ${CLAUDE_PERMISSION_MODE} --model ${model}"
@@ -244,9 +248,10 @@ build_cli_cmd() {
244248
local codex_extra=""
245249
# Avoid MCP startup deadlocks in unattended loop runs.
246250
if [ "${RALPH_AUTOMATED:-}" = "1" ] && [ "$DISABLE_MCP_IN_AUTOMATED_NORM" = "true" ]; then
247-
codex_extra=" -c 'mcp_servers={}'"
251+
codex_extra=" -c mcp_servers={}"
248252
fi
249-
echo "codex --ask-for-approval \"$CODEX_APPROVAL_POLICY\" --sandbox \"$CODEX_SANDBOX\" exec -C \"$APP_DIR\" --model \"${model}\"${codex_extra}"
253+
# Keep command string token-safe. run_claude_* parses it as an argument array.
254+
echo "codex --ask-for-approval ${CODEX_APPROVAL_POLICY} --sandbox ${CODEX_SANDBOX} exec --cd . --model ${model}${codex_extra}"
250255
;;
251256
*)
252257
echo "ERROR: Unsupported CLI '$cli'" >&2
@@ -332,12 +337,14 @@ fi
332337
run_claude_prompt() {
333338
local prompt_file="$1"
334339
local claude_cmd="$2"
335-
if [[ "$claude_cmd" == codex* ]]; then
340+
local -a cmd_parts=()
341+
read -r -a cmd_parts <<< "$claude_cmd"
342+
if [[ "${cmd_parts[0]:-}" == "codex" ]]; then
336343
LAST_RUN_CLI="codex"
337-
{ cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && eval "$claude_cmd --json --output-last-message \"$LAST_MESSAGE_FILE\" -")
344+
{ cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && "${cmd_parts[@]}" --json --output-last-message "$LAST_MESSAGE_FILE" -)
338345
else
339346
LAST_RUN_CLI="claude"
340-
{ cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | $claude_cmd
347+
{ cat "$prompt_file" | envsubst; echo "$AUTOMATION_FOOTER"; } | "${cmd_parts[@]}"
341348
fi
342349
}
343350

@@ -346,6 +353,10 @@ run_claude_resume() {
346353
local session_id="$1"
347354
local continuation_prompt="$2"
348355
local claude_cmd="$3"
356+
if [[ ! "$session_id" =~ ^[A-Za-z0-9._:-]+$ ]]; then
357+
echo "WARNING: Refusing to resume with unsafe session id '$session_id'" >&2
358+
return 1
359+
fi
349360
if [[ "$claude_cmd" == codex* ]]; then
350361
LAST_RUN_CLI="codex"
351362
local resume_cmd="${claude_cmd/ exec / exec resume }"
@@ -354,18 +365,22 @@ run_claude_resume() {
354365
return 1
355366
fi
356367
# codex exec resume does not accept -C/--cd; resume from APP_DIR instead.
357-
resume_cmd="${resume_cmd/ -C \"$APP_DIR\"/}"
358-
resume_cmd="${resume_cmd/ --cd \"$APP_DIR\"/}"
359-
{ echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && eval "$resume_cmd \"$session_id\" - --json --output-last-message \"$LAST_MESSAGE_FILE\"")
368+
resume_cmd="${resume_cmd/ --cd ./}"
369+
resume_cmd="${resume_cmd/ -C ./}"
370+
local -a resume_parts=()
371+
read -r -a resume_parts <<< "$resume_cmd"
372+
{ echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | (cd "$APP_DIR" && "${resume_parts[@]}" "$session_id" - --json --output-last-message "$LAST_MESSAGE_FILE")
360373
else
361374
LAST_RUN_CLI="claude"
362-
# Insert --resume "$session_id" before the -p flag
363-
local resume_cmd="${claude_cmd/ -p / --resume \"$session_id\" -p }"
375+
# Insert --resume before the -p flag.
376+
local resume_cmd="${claude_cmd/ -p / --resume ${session_id} -p }"
364377
if [ "$resume_cmd" = "$claude_cmd" ]; then
365378
echo "WARNING: --resume injection failed, -p flag not found in command" >&2
366379
return 1
367380
fi
368-
{ echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | $resume_cmd
381+
local -a resume_parts=()
382+
read -r -a resume_parts <<< "$resume_cmd"
383+
{ echo "$continuation_prompt"; echo "$AUTOMATION_FOOTER"; } | "${resume_parts[@]}"
369384
fi
370385
}
371386

@@ -931,15 +946,29 @@ except Exception:
931946
run_review_fix() {
932947
local findings
933948
local impl_cli="$CODING_CLI"
934-
local impl_cmd="$IMPL_CMD"
935949
findings=$(extract_review_findings "${CLAUDE_OUTPUT}.raw")
950+
local -a impl_cmd_parts=()
951+
read -r -a impl_cmd_parts <<< "$IMPL_CMD"
936952
if [ "$impl_cli" = "codex" ]; then
937953
LAST_RUN_CLI="codex"
938-
impl_cmd="$IMPL_CMD --json --output-last-message \"$LAST_MESSAGE_FILE\""
954+
cat <<FIXEOF | "${impl_cmd_parts[@]}" --json --output-last-message "$LAST_MESSAGE_FILE" - 2>&1 | tee "${CLAUDE_OUTPUT}.raw" || true
955+
## Code Review Findings
956+
957+
The following issues were found during code review:
958+
959+
${findings}
960+
961+
## Task
962+
963+
Fix each issue listed above. Run git diff $DEFAULT_BRANCH to see the current changes, then:
964+
1. Fix each issue referenced in the review
965+
2. Run tests to verify fixes
966+
3. Commit and push the fixes
967+
Do NOT propose completion options or ask interactive questions. Just fix, test, commit, push.
968+
FIXEOF
939969
else
940970
LAST_RUN_CLI="claude"
941-
fi
942-
cat <<FIXEOF | eval "$impl_cmd" 2>&1 | tee "${CLAUDE_OUTPUT}.raw" || true
971+
cat <<FIXEOF | "${impl_cmd_parts[@]}" 2>&1 | tee "${CLAUDE_OUTPUT}.raw" || true
943972
## Code Review Findings
944973
945974
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
954983
3. Commit and push the fixes
955984
Do NOT propose completion options or ask interactive questions. Just fix, test, commit, push.
956985
FIXEOF
986+
fi
957987
extract_session_result "${CLAUDE_OUTPUT}.raw" "$impl_cli"
958988
accumulate_tokens_from_session "$LAST_SESSION_ID" "${CLAUDE_OUTPUT}.raw" "$impl_cli"
959989
}
960990

991+
validate_simple_command() {
992+
local command="$1"
993+
if [[ "$command" =~ [\;\&\|\<\>\`\$\(\)] ]]; then
994+
return 1
995+
fi
996+
if [[ "$command" =~ [[:cntrl:]] ]]; then
997+
return 1
998+
fi
999+
return 0
1000+
}
1001+
1002+
run_test_command() {
1003+
if ! validate_simple_command "$TEST_COMMAND"; then
1004+
echo "ERROR: commands.test contains unsupported shell operators. Use a plain command and arguments." >&2
1005+
return 2
1006+
fi
1007+
local -a test_cmd_parts=()
1008+
read -r -a test_cmd_parts <<< "$TEST_COMMAND"
1009+
if [ ${#test_cmd_parts[@]} -eq 0 ]; then
1010+
echo "ERROR: commands.test resolved to an empty command." >&2
1011+
return 2
1012+
fi
1013+
(cd "$APP_DIR" && "${test_cmd_parts[@]}")
1014+
}
1015+
9611016
# Normalize test failure lines: extract test name, strip timing, deduplicate.
9621017
# This makes baseline comparison stable across runs where timing values change.
9631018
normalize_test_failures() {
@@ -968,7 +1023,7 @@ normalize_test_failures() {
9681023
# Returns 0 if tests pass OR all failures are pre-existing (captured at baseline).
9691024
check_tests_pass_or_baseline() {
9701025
local test_output
971-
test_output=$( (cd "$APP_DIR" && eval "$TEST_COMMAND") 2>&1 )
1026+
test_output=$(run_test_command 2>&1)
9721027
local exit_code=$?
9731028

9741029
if [ $exit_code -eq 0 ]; then
@@ -1152,11 +1207,11 @@ fi
11521207
# Capture baseline test failures for pre-existing failure detection
11531208
BASELINE_FAILURES_FILE="/tmp/ralph-loop-${FEATURE}.baseline-failures"
11541209
echo "Capturing baseline test failures..."
1155-
if (cd "$APP_DIR" && eval "$TEST_COMMAND" 2>&1) > /dev/null 2>&1; then
1210+
if run_test_command > /dev/null 2>&1; then
11561211
echo "Baseline: all tests passing"
11571212
: > "$BASELINE_FAILURES_FILE"
11581213
else
1159-
(cd "$APP_DIR" && eval "$TEST_COMMAND" 2>&1) | normalize_test_failures > "$BASELINE_FAILURES_FILE" 2>/dev/null || true
1214+
run_test_command 2>&1 | normalize_test_failures > "$BASELINE_FAILURES_FILE" 2>/dev/null || true
11601215
BASELINE_COUNT=$(wc -l < "$BASELINE_FAILURES_FILE" | tr -d ' ')
11611216
echo "Baseline: $BASELINE_COUNT pre-existing test failure(s) recorded"
11621217
fi

src/utils/env.test.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,16 @@ describe('writeKeysToEnvFile', () => {
271271
vi.spyOn(fs, 'existsSync').mockReturnValue(false);
272272
vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
273273
vi.spyOn(fs, 'writeFileSync').mockReturnValue(undefined);
274+
vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined);
274275

275276
writeKeysToEnvFile(filePath, keys);
276277

277278
expect(fs.writeFileSync).toHaveBeenCalledWith(
278279
filePath,
279-
'OPENAI_API_KEY=sk-test-123\n'
280+
'OPENAI_API_KEY=sk-test-123\n',
281+
{ mode: 0o600 }
280282
);
283+
expect(fs.chmodSync).toHaveBeenCalledWith(filePath, 0o600);
281284
});
282285

283286
it('merges keys into existing file content (preserves other keys)', () => {
@@ -288,6 +291,9 @@ describe('writeKeysToEnvFile', () => {
288291
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
289292
vi.spyOn(fs, 'readFileSync').mockReturnValue(existingContent);
290293
vi.spyOn(fs, 'writeFileSync').mockReturnValue(undefined);
294+
vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined);
295+
vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined);
296+
vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined);
291297

292298
writeKeysToEnvFile(filePath, keys);
293299

@@ -322,13 +328,16 @@ describe('writeKeysToEnvFile', () => {
322328
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
323329
vi.spyOn(fs, 'readFileSync').mockReturnValue(existingContent);
324330
vi.spyOn(fs, 'writeFileSync').mockReturnValue(undefined);
331+
vi.spyOn(fs, 'chmodSync').mockReturnValue(undefined);
325332

326333
writeKeysToEnvFile(filePath, keys);
327334

328335
expect(fs.writeFileSync).toHaveBeenCalledWith(
329336
filePath,
330-
existingContent
337+
existingContent,
338+
{ mode: 0o600 }
331339
);
340+
expect(fs.chmodSync).toHaveBeenCalledWith(filePath, 0o600);
332341
});
333342

334343
it('skips keys with empty string values', () => {

src/utils/env.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,12 @@ export function writeKeysToEnvFile(filePath: string, keys: Record<string, string
8686
}
8787
}
8888

89-
fs.writeFileSync(filePath, envContent);
89+
fs.writeFileSync(filePath, envContent, { mode: 0o600 });
90+
try {
91+
fs.chmodSync(filePath, 0o600);
92+
} catch {
93+
// Best-effort hardening; ignore chmod failures on unusual filesystems.
94+
}
9095
}
9196

9297
/**

0 commit comments

Comments
 (0)