diff --git a/CLAUDE.md b/CLAUDE.md index 310b46b0..1f7349fd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -301,7 +301,7 @@ Two-stage filtering eliminates false positives: ### `.ralphrc` Parsing (Issue #346) -`.ralphrc` is repository-controlled, so it is parsed as data and never sourced. The grammar and the command-key policy live in **lib/ralphrc.sh** (`ralphrc_parse_line`, `ralphrc_value_allowed`, `ralphrc_get`) and are shared by every reader: `load_ralphrc()` in ralph_loop.sh, `ralph_import.sh` (and `ralph-queue`, which sources it; env `CLAUDE_CODE_CMD` now wins there too) and `tools/inspect-allowed-tools.sh` — never add a new `.ralphrc` reader that greps or sources the file. `load_ralphrc()`: only `KEY=VALUE` lines (optional `export`, quotes, trailing `# comment`, CRLF tolerated) for allowlisted keys are applied via `printf -v`; values are literal — `$`, backticks, `\` and control characters are rejected in every value even when single-quoted (values reach bash arithmetic, where `a[$(cmd)]` executes, and `echo -e`), plus `;|&<>()` and quotes when unquoted; rejected lines are skipped with a WARN, as are unknown keys and malformed lines. Bad lines are never fatal, so the `_env_*` precedence restore always runs. Every allowlisted key (except the legacy aliases, `PROMPT_FILE` and `RALPH_DIR`) needs an `_env_*` snapshot + restore line, or the file beats the environment (Issue #369, structural guard test); keys a lib defaults at source time (`CB_*` thresholds, sandbox, sync) are snapshotted in the block BEFORE the lib source block — snapshotting after would record the lib default as an env value and make `.ralphrc` ineffective (guarded by the real-script tests in `test_ralphrc_security.bats`). Keys written by ralph-enable for reference only (`PROJECT_NAME`, `PROJECT_TYPE`, `TASK_SOURCES`, …) are accepted silently. Command-bearing keys accept only stock values from the file — `CLAUDE_CODE_CMD` (`claude` / `npx @anthropic-ai/claude-code`), `SANDBOX_DOCKER_IMAGE` (`ralph-sandbox:latest` / the GHCR image), `SANDBOX_E2B_TEMPLATE` (`base`), `SANDBOX_DOCKER_NETWORK` (`bridge`/`none`, never `host`), `RALPH_SHELL_INIT_FILE`, `SANDBOX_E2B_SANDBOX_ID` and `RALPH_DIR` (none — RALPH_DIR is env-only, Issue #352: an exported value relocates all state, and `validate_ralph_integrity` maps its `.ralph` entries onto it); every CLI flag that sets a `.ralphrc` key is captured as `_cli_*` and re-applied after `load_ralphrc` (structural guard test enforces it); repo-controlled values are printed only through `ralphrc_display` (control chars and backslashes → `?`, safe under `echo -e`); `--prompt` now wins over `PROMPT_FILE` from `.ralphrc` (`_cli_PROMPT_FILE`); custom values come from the environment or `--sandbox-image`/`--sandbox-template`. Bash 3.2-safe (no associative arrays). Tests: `tests/unit/test_ralphrc_security.bats` (incl. the issue's reproduction run against the real `ralph_loop.sh --dry-run`). +`.ralphrc` is repository-controlled, so it is parsed as data and never sourced. The grammar and the command-key policy live in **lib/ralphrc.sh** (`ralphrc_parse_line`, `ralphrc_value_allowed`, `ralphrc_get`) and are shared by every reader: `load_ralphrc()` in ralph_loop.sh, `ralph_import.sh` (and `ralph-queue`, which sources it; env `CLAUDE_CODE_CMD` now wins there too) and `tools/inspect-allowed-tools.sh` — never add a new `.ralphrc` reader that greps or sources the file. `load_ralphrc()`: only `KEY=VALUE` lines (optional `export`, quotes, trailing `# comment`, CRLF tolerated) for allowlisted keys are applied via `printf -v`; values are literal — `$`, backticks, `\` and control characters are rejected in every value even when single-quoted (values reach bash arithmetic, where `a[$(cmd)]` executes, and `echo -e`), plus `;|&<>()` and quotes when unquoted; rejected lines are skipped with a WARN, as are unknown keys and malformed lines. Bad lines are never fatal, so the `_env_*` precedence restore always runs. Every allowlisted key (except the legacy aliases, `PROMPT_FILE` and `RALPH_DIR`) needs an `_env_*` snapshot + restore line, or the file beats the environment (Issue #369, structural guard test); keys a lib defaults at source time (`CB_*` thresholds, sandbox, sync) are snapshotted in the block BEFORE the lib source block — snapshotting after would record the lib default as an env value and make `.ralphrc` ineffective (guarded by the real-script tests in `test_ralphrc_security.bats`). Keys written by ralph-enable for reference only (`PROJECT_NAME`, `PROJECT_TYPE`, `TASK_SOURCES`, …) are accepted silently. Command-bearing keys accept only stock values from the file — `CLAUDE_CODE_CMD` (`claude` / `npx @anthropic-ai/claude-code`), `SANDBOX_DOCKER_IMAGE` (`ralph-sandbox:latest` / the GHCR image), `SANDBOX_E2B_TEMPLATE` (`base`), `SANDBOX_DOCKER_NETWORK` (`bridge`/`none`, never `host`), `RALPH_SHELL_INIT_FILE`, `SANDBOX_E2B_SANDBOX_ID` and `RALPH_DIR` (none — RALPH_DIR is env-only, Issue #352: an exported value relocates all state, and `validate_ralph_integrity` maps its `.ralph` entries onto it); every CLI flag that sets a `.ralphrc` key is captured as `_cli_*` and re-applied after `load_ralphrc` (structural guard test enforces it); `--monitor` puts every non-empty `_env_*` snapshot on the tmux loop pane's command as `KEY=%q-value` (derived from the snapshots, so new keys forward automatically), because a running tmux server gives panes its own environment (Issue #378); repo-controlled values are printed only through `ralphrc_display` (control chars and backslashes → `?`, safe under `echo -e`); `--prompt` now wins over `PROMPT_FILE` from `.ralphrc` (`_cli_PROMPT_FILE`); custom values come from the environment or `--sandbox-image`/`--sandbox-template`. Bash 3.2-safe (no associative arrays). Tests: `tests/unit/test_ralphrc_security.bats` (incl. the issue's reproduction run against the real `ralph_loop.sh --dry-run`). ### File Protection (Issue #149) diff --git a/ralph_loop.sh b/ralph_loop.sh index 6cfafdf0..e5771e19 100755 --- a/ralph_loop.sh +++ b/ralph_loop.sh @@ -589,13 +589,24 @@ setup_tmux_session() { tmux send-keys -t "$session_name:${base_win}.${pane2}" "${env_prefix}'$ralph_home/ralph_monitor.sh'" Enter fi + # The user's explicit environment rides on the loop command for the same + # reason, so it still beats .ralphrc in the pane (Issue #378). Derived from + # the _env_* snapshots: a new snapshotted key is forwarded automatically. + # This runs before main() loads .ralphrc, so a non-empty snapshot is + # always the user's own value. CLI flags appended below still win. + # Like the RALPH_DIR prefix, %q output assumes a bash-compatible pane shell. + local loop_env="" snap + for snap in "${!_env_@}"; do + [[ -n "${!snap}" ]] && loop_env+="${snap#_env_}=$(printf '%q' "${!snap}") " + done + # Start ralph loop in the left pane (exclude tmux flag to avoid recursion) # Forward all CLI parameters that were set by the user local ralph_cmd if command -v ralph &> /dev/null; then - ralph_cmd="${env_prefix}ralph" + ralph_cmd="${env_prefix}${loop_env}ralph" else - ralph_cmd="${env_prefix}'$ralph_home/ralph_loop.sh'" + ralph_cmd="${env_prefix}${loop_env}'$ralph_home/ralph_loop.sh'" fi # Always use --live mode in tmux for real-time streaming @@ -691,9 +702,10 @@ setup_tmux_session() { # validation ever runs, so explicit CLI --sync-* flags with the docker # provider must be rejected now — silently dropping them would make # --monitor behave differently from a plain run (CodeRabbit, PR #305). - # Env-supplied SYNC_* with docker is merely not forwarded (a plain run - # ignores it the same way); an empty provider may still become e2b via - # the child's .ralphrc, so it forwards. + # Env-supplied SYNC_* with docker is not forwarded as a flag (it rides + # in the env prefix above, which a docker child ignores just like a plain + # run does); an empty provider may still become e2b via the child's + # .ralphrc, so it forwards. if [[ "${SANDBOX_PROVIDER:-}" == "docker" ]]; then if [[ -n "${_cli_SYNC_INCLUDE:-}${_cli_SYNC_EXCLUDE:-}" ]]; then log_status "ERROR" "--sync-include/--sync-exclude do not apply to --sandbox docker (the bind mount shares the whole project in real time)" diff --git a/tests/integration/test_tmux_integration.bats b/tests/integration/test_tmux_integration.bats index 3063def1..c3fb2614 100644 --- a/tests/integration/test_tmux_integration.bats +++ b/tests/integration/test_tmux_integration.bats @@ -721,3 +721,72 @@ assert_tmux_called_with() { pane1_line=$(grep -E "tmux send-keys -t [^ ]+\.1 " "$TMUX_CALL_LOG" | head -1) [[ "$pane1_line" == *"tail -f '/tmp/ralph-abs-state/live.log'"* ]] || { echo "$pane1_line"; false; } } + +# Issue #378: a running tmux server gives panes its own environment, so the +# user's explicit environment values must ride on the loop pane's command. +_pane0_cmd() { + # The logged line is "tmux send-keys -t :.0 Enter" + local line + line=$(grep -E "tmux send-keys -t [^ ]+\.0 " "$TMUX_CALL_LOG" | head -1) + line="${line#tmux send-keys -t * }" + printf '%s' "${line% Enter}" +} + +@test "setup_tmux_session forwards every env-snapshotted key to the loop pane (#378)" { + local keys key + keys=$(grep -oE '^_env_[A-Z0-9_]+=' "${BATS_TEST_DIRNAME}/../../ralph_loop.sh" | sed 's/^_env_//; s/=$//' | sort -u) + [[ $(wc -l <<<"$keys") -ge 40 ]] + for key in $keys; do + printf -v "_env_$key" '%s' "v-$key" + done + + run setup_tmux_session + [ "$status" -eq 0 ] + + local cmd; cmd=$(_pane0_cmd) + for key in $keys; do + [[ "$cmd" == *"$key=v-$key "* ]] || fail "$key not forwarded: $cmd" + done + # The monitor pane only needs RALPH_DIR + [[ $(grep -cE "tmux send-keys -t [^ ]+\.2 .*CB_NO_PROGRESS_THRESHOLD=" "$TMUX_CALL_LOG") -eq 0 ]] +} + +@test "setup_tmux_session quotes forwarded env values byte-exact with no injection (#378)" { + mkdir -p "$TEST_TEMP_DIR/bin" + cat > "$TEST_TEMP_DIR/bin/ralph" << 'SH' +#!/bin/bash +printf '%s' "$CLAUDE_MODEL" > "$OUT_DIR/model" +printf '%s' "$OPTIONAL_SECTIONS" > "$OUT_DIR/sections" +printf '%s' "$CLAUDE_CODE_CMD" > "$OUT_DIR/cmd" +SH + chmod +x "$TEST_TEMP_DIR/bin/ralph" + export PATH="$TEST_TEMP_DIR/bin:$PATH" OUT_DIR="$TEST_TEMP_DIR" + + local model='a b; touch PWNED_1; $(touch PWNED_2) `touch PWNED_3`' + local sections="Nice to Have, it's \"Future\" \\ & | > <" + local cmd_val=$'npx @anthropic-ai/claude-code\ttab\nnewline' + _env_CLAUDE_MODEL="$model" _env_OPTIONAL_SECTIONS="$sections" _env_CLAUDE_CODE_CMD="$cmd_val" + run setup_tmux_session + [ "$status" -eq 0 ] + + # Run the pane command the way the pane's shell would + bash -c "$(_pane0_cmd)" + assert_equal "$(cat "$TEST_TEMP_DIR/model")" "$model" + assert_equal "$(cat "$TEST_TEMP_DIR/sections")" "$sections" + assert_equal "$(cat "$TEST_TEMP_DIR/cmd")" "$cmd_val" + [[ ! -e PWNED_1 && ! -e PWNED_2 && ! -e PWNED_3 ]] +} + +@test "setup_tmux_session forwards only keys set in the environment (#378)" { + _env_CB_NO_PROGRESS_THRESHOLD="10" + _env_CLAUDE_MODEL="" + + run setup_tmux_session + [ "$status" -eq 0 ] + + local cmd; cmd=$(_pane0_cmd) + [[ "$cmd" == *"CB_NO_PROGRESS_THRESHOLD=10 "* ]] + # Empty or absent snapshots leave .ralphrc in charge inside the pane + [[ "$cmd" != *"CLAUDE_MODEL="* ]] + [[ "$cmd" != *"MAX_CONSECUTIVE_TEST_LOOPS="* ]] +}