diff --git a/prompts/dev-lead/fix-bot-comment.md b/prompts/dev-lead/fix-bot-comment.md index 894e46667..8ec9f83df 100644 --- a/prompts/dev-lead/fix-bot-comment.md +++ b/prompts/dev-lead/fix-bot-comment.md @@ -44,9 +44,15 @@ gh api graphql -f query=' } }' -F owner="${REPO%%/*}" -F repo="${REPO##*/}" -F pr=${PR_NUMBER} \ | jq --arg actor "${ACTOR}" ' - .data.repository.pullRequest.reviewThreads.nodes - | map(select(.isResolved == false - and .comments.nodes[0].author.login == $actor))' + # GraphQL omits the "[bot]" suffix that GitHub Actions includes in the + # login (e.g. ACTOR=coderabbitai[bot] → author.login=coderabbitai). + # Match on both forms so threads from any trusted bot can be resolved. + (.data.repository.pullRequest.reviewThreads.nodes + | map(select(.isResolved == false + and ( + .comments.nodes[0].author.login == $actor + or .comments.nodes[0].author.login == ($actor | gsub("\\[bot\\]$"; "")) + ))))' ``` Then resolve each thread you addressed (and any from this bot marked `isOutdated: true`): diff --git a/scripts/dev-lead-fix-reviews.sh b/scripts/dev-lead-fix-reviews.sh index 4e347ef30..57dbe4bb4 100755 --- a/scripts/dev-lead-fix-reviews.sh +++ b/scripts/dev-lead-fix-reviews.sh @@ -86,28 +86,73 @@ ${summary}" gh pr comment "$PR_NUMBER" --repo "$REPO" --body "$body" 2>/dev/null || true } +# redact_secrets: scrub common credential token formats from stdin → stdout. +# Defense-in-depth before publishing agent session output to a PR comment — +# Claude Code's session log can include curl/gh invocations whose stderr leaks +# tokens, or echoed environment variables. Patterns cover GitHub, OpenAI/ +# Anthropic, AWS, Google OAuth, generic bearer tokens, and PEM private keys. +redact_secrets() { + # The PEM range (-----BEGIN ... -----END ...) uses sed's c\ range-change + # so the *entire* multiline block is replaced — header line alone leaves + # the key body lines intact and still leakable. + sed -E \ + -e 's/(gh[opsu]|ghr)_[A-Za-z0-9_]{20,}/***REDACTED-GH-TOKEN***/g' \ + -e 's/github_pat_[A-Za-z0-9_]{20,}/***REDACTED-GH-PAT***/g' \ + -e 's/sk-(ant-)?[A-Za-z0-9_-]{20,}/***REDACTED-API-KEY***/g' \ + -e 's/AKIA[A-Z0-9]{16}/***REDACTED-AWS-KEY***/g' \ + -e 's/AIza[A-Za-z0-9_-]{35}/***REDACTED-GOOGLE-KEY***/g' \ + -e 's|ya29\.[A-Za-z0-9_-]+|***REDACTED-GOOGLE-OAUTH***|g' \ + -e 's/[Bb]earer [A-Za-z0-9._-]{20,}/Bearer ***REDACTED***/g' \ + -e '/-----BEGIN [A-Z ]*PRIVATE KEY-----/,/-----END [A-Z ]*PRIVATE KEY-----/c\ +***REDACTED-PRIVATE-KEY***' +} + +# read_session_summary: reads the last 10 non-blank lines of the agent session +# output and redacts any embedded credentials. Empty output if the file is +# missing (e.g. dry-run paths that never invoked the writer). Redaction runs +# over the full log *before* tailing, so PEM blocks that straddle the tail +# boundary are fully redacted (header in the discarded window, body in the +# kept window would otherwise leak as plaintext key material). read_session_summary() { local log="/tmp/dev-lead-session-output.txt" - [ -f "$log" ] || return 0 - # The agent output format is always at the end; grab last non-empty paragraph - tail -30 "$log" | sed '/^[[:space:]]*$/d' | tail -10 + [[ -f "$log" ]] || return 0 + redact_secrets < "$log" | tail -30 | sed '/^[[:space:]]*$/d' | tail -10 +} + +# pick_fence: emit a tilde fence longer than any tilde run in $1 (min 4). +# Ensures the wrapping ~~~~ code block cannot be terminated early by content +# that happens to contain a tilde sequence. +pick_fence() { + local content="$1" + local fence="~~~~" + while printf '%s' "$content" | grep -qF "$fence"; do + fence="${fence}~" + done + printf '%s' "$fence" } +# post_no_changes: posts a terminal no-changes marker with redacted agent +# reasoning when available, or a plain fallback when the session log is +# absent. Picks a tilde fence that cannot be broken by the content, and +# neutralises any literal so the wrapping
stays intact. post_no_changes() { local intent="$1" local _summary - _summary=$(read_session_summary) - if [ -n "$_summary" ]; then - post_reviews_terminal "$intent" "no-changes" \ - "
Agent reasoning - -\`\`\` + _summary=$(read_session_summary || true) + local msg="No actionable items found." + if [[ -n "$_summary" ]]; then + # Neutralise any
in summary so it cannot close the outer block. + _summary=$(printf '%s' "$_summary" | sed 's|
|<\\/details>|g') + local fence + fence=$(pick_fence "$_summary") + msg="
Agent reasoning + +${fence} ${_summary} -\`\`\` +${fence}
" - else - post_reviews_terminal "$intent" "no-changes" fi + post_reviews_terminal "$intent" "no-changes" "$msg" } # notify_coderabbit_resolve: posts @coderabbitai resolve if coderabbitai[bot]'s @@ -372,7 +417,12 @@ case "$INTENT_TYPE" in export PR_NUMBER PR_URL="https://github.com/${REPO}/pull/${PR_NUMBER}" export REPO HEAD_SHA export BASE_REF="${BASE_REF:-main}" + # Normalise to GraphQL's author.login form: the GitHub Actions event login + # includes a "[bot]" suffix for bots, but GraphQL author.login omits it. + # Without this, the prompt's `author.login == ${TRIGGERING_REVIEWER}` check + # never matches threads from coderabbitai/chatgpt-codex/etc. export TRIGGERING_REVIEWER="${TRIGGERING_REVIEWER:-}" + TRIGGERING_REVIEWER="${TRIGGERING_REVIEWER%\[bot\]}" OPEN_THREADS_JSON=$(gh api graphql -f query=' query($owner:String!,$repo:String!,$pr:Int!) { repository(owner:$owner, name:$repo) { @@ -430,7 +480,7 @@ case "$INTENT_TYPE" in if commit_and_push "on-mention"; then post_reviews_terminal "on-mention" "applied" "Changes committed and pushed." else - post_no_changes "on-mention" + post_reviews_terminal "on-mention" "no-changes" "Engine ran but made no changes." fi fi exit "$rc" @@ -461,7 +511,7 @@ case "$INTENT_TYPE" in post_reviews_terminal "review-changes" "applied" "Changes committed and pushed." else notify_coderabbit_resolve - post_no_changes "review-changes" + post_reviews_terminal "review-changes" "no-changes" "No changes were needed for this PR." fi try_enable_auto_merge fi diff --git a/scripts/engine.sh b/scripts/engine.sh index 7517cff4b..75342c2cd 100644 --- a/scripts/engine.sh +++ b/scripts/engine.sh @@ -580,15 +580,41 @@ run_writer() { _record_engine_tokens "writer" "$REVIEW_ENGINE" "$model" "$prompt_file" "$_tmp" fi if [ -n "$_tmp" ]; then + # Redact once: write secret-scrubbed content to the persisted path, then + # use that redacted file as the source for the step summary so credentials + # cannot leak via /tmp persistence or the public run summary. Patterns + # kept in sync with scripts/dev-lead-fix-reviews.sh:redact_secrets. + rm -f /tmp/dev-lead-session-output.txt + if ! sed -E \ + -e 's/(gh[opsu]|ghr)_[A-Za-z0-9_]{20,}/***REDACTED-GH-TOKEN***/g' \ + -e 's/github_pat_[A-Za-z0-9_]{20,}/***REDACTED-GH-PAT***/g' \ + -e 's/sk-(ant-)?[A-Za-z0-9_-]{20,}/***REDACTED-API-KEY***/g' \ + -e 's/AKIA[A-Z0-9]{16}/***REDACTED-AWS-KEY***/g' \ + -e 's/AIza[A-Za-z0-9_-]{35}/***REDACTED-GOOGLE-KEY***/g' \ + -e 's|ya29\.[A-Za-z0-9_-]+|***REDACTED-GOOGLE-OAUTH***|g' \ + -e 's/[Bb]earer [A-Za-z0-9._-]{20,}/Bearer ***REDACTED***/g' \ + -e '/-----BEGIN [A-Z ]*PRIVATE KEY-----/,/-----END [A-Z ]*PRIVATE KEY-----/c\ +***REDACTED-PRIVATE-KEY***' \ + "$_tmp" > /tmp/dev-lead-session-output.txt; then + : > /tmp/dev-lead-session-output.txt 2>/dev/null || true + echo "::warning::Failed to redact/persist /tmp/dev-lead-session-output.txt" >&2 + fi if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then - echo "## Dev-Lead session output" >> "$GITHUB_STEP_SUMMARY" - echo "
Click to expand session logs" >> "$GITHUB_STEP_SUMMARY" - echo "" >> "$GITHUB_STEP_SUMMARY" - cat "$_tmp" >> "$GITHUB_STEP_SUMMARY" - echo "" >> "$GITHUB_STEP_SUMMARY" - echo "
" >> "$GITHUB_STEP_SUMMARY" + # HTML-escape the redacted log so literal or in + # the agent output cannot break the wrapping
block on the run + # summary page. + { + echo "## Dev-Lead session output" + echo "
Click to expand session logs" + echo "" + echo "
"
+        sed 's|&|\&|g; s|<|\<|g; s|>|\>|g' /tmp/dev-lead-session-output.txt
+        echo "
" + echo "" + echo "
" + } >> "$GITHUB_STEP_SUMMARY" || \ + echo "::warning::Failed to append session output to GITHUB_STEP_SUMMARY" >&2 fi - cp "$_tmp" /tmp/dev-lead-session-output.txt rm -f "$_tmp" fi return "$rc" diff --git a/tests/dev-lead/unit/test_engine_writer.bats b/tests/dev-lead/unit/test_engine_writer.bats index 662ba0fef..941d0721a 100644 --- a/tests/dev-lead/unit/test_engine_writer.bats +++ b/tests/dev-lead/unit/test_engine_writer.bats @@ -23,7 +23,6 @@ setup() { teardown() { rm -f "$GITHUB_ENV" "$GITHUB_OUTPUT" "$TEST_PROMPT" - rm -f /tmp/dev-lead-session-output.txt rm -rf "$STUB_BIN_DIR" if [ -n "${TEST_OWNED_TOKEN_LOG:-}" ] && [ "${TOKEN_LOG_FILE:-}" = "$TEST_OWNED_TOKEN_LOG" ]; then rm -f "$TEST_OWNED_TOKEN_LOG" @@ -470,81 +469,133 @@ STUB [ ! -s "$log" ] } -# ── session output persistence tests ───────────────────────────────────────── - -@test "session: run_writer copies output to /tmp/dev-lead-session-output.txt on success" { +@test "writer: run_writer persists session output to /tmp/dev-lead-session-output.txt" { _source_engine "claude" - export STUB_ENGINE_EXIT=0 - export STUB_ENGINE_RESPONSE="PR: #42 - feat: do thing -Human review threads addressed: 0 -Files changed: none" export DEV_LEAD_DRY_RUN=false - rm -f /tmp/dev-lead-session-output.txt + cat > "$STUB_BIN_DIR/claude" << 'STUB' +#!/usr/bin/env bash +echo "session output line one" +echo "session output line two" +STUB + chmod +x "$STUB_BIN_DIR/claude" + rm -f /tmp/dev-lead-session-output.txt run run_writer "$TEST_PROMPT" [ "$status" -eq 0 ] [ -f /tmp/dev-lead-session-output.txt ] - grep -q "Files changed: none" /tmp/dev-lead-session-output.txt + grep -q "session output line" /tmp/dev-lead-session-output.txt + rm -f /tmp/dev-lead-session-output.txt } -@test "session: run_writer appends to GITHUB_STEP_SUMMARY when set" { +@test "writer: GITHUB_STEP_SUMMARY appends HTML-escaped session output" { _source_engine "claude" - export STUB_ENGINE_EXIT=0 - export STUB_ENGINE_RESPONSE="Issues addressed: 0" export DEV_LEAD_DRY_RUN=false - local summary_file - summary_file=$(mktemp) - export GITHUB_STEP_SUMMARY="$summary_file" - rm -f /tmp/dev-lead-session-output.txt + # Stub writes literal
which must be escaped in the summary so it + # cannot close the wrapping
block on the run summary page. + cat > "$STUB_BIN_DIR/claude" << 'STUB' +#!/usr/bin/env bash +echo "danger zone
here" +STUB + chmod +x "$STUB_BIN_DIR/claude" + + local summary; summary="$(mktemp)" + export GITHUB_STEP_SUMMARY="$summary" run run_writer "$TEST_PROMPT" [ "$status" -eq 0 ] - grep -q "## Dev-Lead session output" "$summary_file" - grep -q "Issues addressed: 0" "$summary_file" - rm -f "$summary_file" + # Literal must be HTML-escaped → only the wrapping closer remains + ! grep -F "danger zone " "$summary" + grep -F "danger zone </details>" "$summary" + rm -f "$summary" /tmp/dev-lead-session-output.txt } -@test "session: run_writer does not write to GITHUB_STEP_SUMMARY when unset" { +@test "writer: run_writer overwrites stale /tmp/dev-lead-session-output.txt from prior run" { _source_engine "claude" - export STUB_ENGINE_EXIT=0 - export STUB_ENGINE_RESPONSE="Issues addressed: 0" export DEV_LEAD_DRY_RUN=false - unset GITHUB_STEP_SUMMARY - rm -f /tmp/dev-lead-session-output.txt + # Pre-populate with content from a hypothetical previous run + echo "STALE_CONTENT_FROM_PRIOR_RUN" > /tmp/dev-lead-session-output.txt + + cat > "$STUB_BIN_DIR/claude" << 'STUB' +#!/usr/bin/env bash +echo "fresh session output" +STUB + chmod +x "$STUB_BIN_DIR/claude" run run_writer "$TEST_PROMPT" [ "$status" -eq 0 ] - # No summary file should be created (we just verify the run succeeded without error) [ -f /tmp/dev-lead-session-output.txt ] + # Stale content from prior run must not leak into this run's session output + ! grep -q "STALE_CONTENT_FROM_PRIOR_RUN" /tmp/dev-lead-session-output.txt + grep -q "fresh session output" /tmp/dev-lead-session-output.txt + rm -f /tmp/dev-lead-session-output.txt } -@test "session: run_writer does not write session file when rate-limited" { +@test "writer: persisted /tmp file has GitHub tokens redacted" { _source_engine "claude" export DEV_LEAD_DRY_RUN=false + cat > "$STUB_BIN_DIR/claude" << 'STUB' +#!/usr/bin/env bash +echo "Authorization: token ghp_abcdefghijklmnopqrstuvwxyz1234567890ABCD" +echo "ok" +STUB + chmod +x "$STUB_BIN_DIR/claude" + rm -f /tmp/dev-lead-session-output.txt + run run_writer "$TEST_PROMPT" + + [ "$status" -eq 0 ] + # Raw token must not survive to /tmp; only the redacted marker remains + ! grep -F "ghp_abcdefghijklmnopqrstuvwxyz1234567890ABCD" /tmp/dev-lead-session-output.txt + grep -F "***REDACTED-GH-TOKEN***" /tmp/dev-lead-session-output.txt + rm -f /tmp/dev-lead-session-output.txt +} + +@test "writer: GITHUB_STEP_SUMMARY has secrets redacted before HTML escape" { + _source_engine "claude" + export DEV_LEAD_DRY_RUN=false cat > "$STUB_BIN_DIR/claude" << 'STUB' #!/usr/bin/env bash -echo "You've hit your limit · resets 11:20pm (UTC)" -exit 1 +echo "Authorization: Bearer sk-ant-secretkey1234567890abcdefghij" STUB chmod +x "$STUB_BIN_DIR/claude" + local summary; summary="$(mktemp)" + export GITHUB_STEP_SUMMARY="$summary" + run run_writer "$TEST_PROMPT" - [ "$status" -eq 2 ] - [ ! -f /tmp/dev-lead-session-output.txt ] + [ "$status" -eq 0 ] + ! grep -F "sk-ant-secretkey1234567890abcdefghij" "$summary" + grep -F "***REDACTED-API-KEY***" "$summary" + rm -f "$summary" /tmp/dev-lead-session-output.txt } -@test "session: run_writer dry-run does not write session file" { +@test "writer: PEM private key block fully redacted in /tmp persistence" { _source_engine "claude" - export DEV_LEAD_DRY_RUN=true - rm -f /tmp/dev-lead-session-output.txt + export DEV_LEAD_DRY_RUN=false + cat > "$STUB_BIN_DIR/claude" << 'STUB' +#!/usr/bin/env bash +echo "-----BEGIN RSA PRIVATE KEY-----" +echo "MIIEowIBAAKCAQEAuVERYSENSITIVEKEYBODYWITHTONSOFCHARSANDMOREDATAHERE" +echo "ENDOFBODYDATAGOESHEREMOREMOREMOREMOREMOREMORE" +echo "-----END RSA PRIVATE KEY-----" +echo "after-key" +STUB + chmod +x "$STUB_BIN_DIR/claude" + rm -f /tmp/dev-lead-session-output.txt run run_writer "$TEST_PROMPT" [ "$status" -eq 0 ] - [ ! -f /tmp/dev-lead-session-output.txt ] + # Neither header, body, nor footer of the PEM block may survive + ! grep -F "BEGIN RSA PRIVATE KEY" /tmp/dev-lead-session-output.txt + ! grep -F "uVERYSENSITIVEKEYBODY" /tmp/dev-lead-session-output.txt + ! grep -F "END RSA PRIVATE KEY" /tmp/dev-lead-session-output.txt + grep -F "***REDACTED-PRIVATE-KEY***" /tmp/dev-lead-session-output.txt + # Content outside the PEM block survives + grep -F "after-key" /tmp/dev-lead-session-output.txt + rm -f /tmp/dev-lead-session-output.txt } diff --git a/tests/dev-lead/unit/test_fix_reviews.bats b/tests/dev-lead/unit/test_fix_reviews.bats index 559ddf34d..7ec0ac746 100644 --- a/tests/dev-lead/unit/test_fix_reviews.bats +++ b/tests/dev-lead/unit/test_fix_reviews.bats @@ -9,7 +9,6 @@ GH_STUBS_DIR="$SCRIPT_DIR/tests/dev-lead/fixtures/stubs" setup() { export GITHUB_ENV="$(mktemp)" export GITHUB_OUTPUT="$(mktemp)" - rm -f /tmp/dev-lead-session-output.txt STUB_BIN_DIR="$(mktemp -d)" cp "$STUB_ENGINES_DIR/stub-claude" "$STUB_BIN_DIR/claude" @@ -54,7 +53,6 @@ GHEOF teardown() { rm -f "$GITHUB_ENV" "$GITHUB_OUTPUT" - rm -f /tmp/dev-lead-session-output.txt rm -rf "$STUB_BIN_DIR" } @@ -430,29 +428,178 @@ STUB [[ "$output" == *"terminal marker"* || "$output" == *"[dry-run]"* ]] } -@test "fix-reviews: no-changes dry-run: does not post visible heading in comment body" { - export INTENT_TYPE="fix-reviews" - export DEV_LEAD_DRY_RUN="true" - export HEAD_SHA="ddd444eee555" +@test "post_no_changes: posts visible heading with fallback when no session log" { + # Run from a non-git tmpdir so commit_and_push reports no changes → no-changes path + local tmpdir + tmpdir="$(mktemp -d)" + rm -f /tmp/dev-lead-session-output.txt - run bash "$FIX_REVIEWS_SCRIPT" + run bash -c " + cd '$tmpdir' + export INTENT_TYPE=fix-reviews DEV_LEAD_DRY_RUN=true + export PR_NUMBER=54 HEAD_SHA=abc123 REPO='petry-projects/.github-private' + export REVIEW_ENGINE=claude BASE_REF=main PROMPTS_DIR='$SCRIPT_DIR/prompts/dev-lead' + export PATH=\"$STUB_BIN_DIR:\$PATH\" + bash '$FIX_REVIEWS_SCRIPT' + " 2>&1 + rm -rf "$tmpdir" + + [[ "$output" == *"## Dev-Lead — fix-reviews (no-changes)"* ]] + [[ "$output" == *"No actionable items found"* ]] +} - [ "$status" -eq 0 ] - # no-changes posts only the invisible HTML marker — no visible ## heading - [[ "$output" != *"## Dev-Lead —"* ]] +@test "post_no_changes: includes agent reasoning when session log is present" { + local tmpdir + tmpdir="$(mktemp -d)" + local session_log="/tmp/dev-lead-session-output.txt" + printf 'Agent determined no code changes required.\n' > "$session_log" + + run bash -c " + cd '$tmpdir' + export INTENT_TYPE=fix-reviews DEV_LEAD_DRY_RUN=true + export PR_NUMBER=54 HEAD_SHA=abc123 REPO='petry-projects/.github-private' + export REVIEW_ENGINE=claude BASE_REF=main PROMPTS_DIR='$SCRIPT_DIR/prompts/dev-lead' + export PATH=\"$STUB_BIN_DIR:\$PATH\" + bash '$FIX_REVIEWS_SCRIPT' + " 2>&1 + rm -rf "$tmpdir" "$session_log" + + [[ "$output" == *"## Dev-Lead — fix-reviews (no-changes)"* ]] + [[ "$output" == *"Agent determined no code changes"* ]] } -@test "fix-reviews: no-changes dry-run review-changes: does not post visible heading" { - export INTENT_TYPE="review-changes" - export DEV_LEAD_DRY_RUN="true" - export HEAD_SHA="ddd444eee555" - export PR_TITLE="Test PR" - export PR_DESCRIPTION="A test pull request" +@test "post_no_changes: redacts GitHub tokens from session log" { + local tmpdir + tmpdir="$(mktemp -d)" + local session_log="/tmp/dev-lead-session-output.txt" + # Embed a fake GitHub PAT — redact_secrets should mask it before publishing. + # Build the token at runtime so the literal doesn't appear in this source file + # (avoids tripping gitleaks on our own test fixture). + local fake_prefix='ghp' fake_body='_abcdefghij1234567890ABCDEFGHIJ' + printf 'curl -H "Auth: %s%s" url\n' "$fake_prefix" "$fake_body" > "$session_log" + + run bash -c " + cd '$tmpdir' + export INTENT_TYPE=fix-reviews DEV_LEAD_DRY_RUN=true + export PR_NUMBER=54 HEAD_SHA=abc123 REPO='petry-projects/.github-private' + export REVIEW_ENGINE=claude BASE_REF=main PROMPTS_DIR='$SCRIPT_DIR/prompts/dev-lead' + export PATH=\"$STUB_BIN_DIR:\$PATH\" + bash '$FIX_REVIEWS_SCRIPT' + " 2>&1 + rm -rf "$tmpdir" "$session_log" + + [[ "$output" == *"***REDACTED-GH-TOKEN***"* ]] + [[ "$output" != *"${fake_prefix}${fake_body}"* ]] +} - run bash "$FIX_REVIEWS_SCRIPT" +@test "post_no_changes: pick_fence outgrows tilde sequences in content" { + local tmpdir + tmpdir="$(mktemp -d)" + local session_log="/tmp/dev-lead-session-output.txt" + # Content has a 4-tilde sequence — fence must be 5+ to wrap cleanly + printf 'output line one\n~~~~ this looks like a fence\noutput line two\n' > "$session_log" + + run bash -c " + cd '$tmpdir' + export INTENT_TYPE=fix-reviews DEV_LEAD_DRY_RUN=true + export PR_NUMBER=54 HEAD_SHA=abc123 REPO='petry-projects/.github-private' + export REVIEW_ENGINE=claude BASE_REF=main PROMPTS_DIR='$SCRIPT_DIR/prompts/dev-lead' + export PATH=\"$STUB_BIN_DIR:\$PATH\" + bash '$FIX_REVIEWS_SCRIPT' + " 2>&1 + rm -rf "$tmpdir" "$session_log" + + # Fence must be longer than the embedded 4-tilde run + [[ "$output" == *"~~~~~"* ]] +} - [ "$status" -eq 0 ] - [[ "$output" != *"## Dev-Lead —"* ]] +@test "post_no_changes: redacts entire PEM private key block, not just header" { + local tmpdir + tmpdir="$(mktemp -d)" + local session_log="/tmp/dev-lead-session-output.txt" + # Build the PEM markers at runtime so the literal "-----BEGIN RSA PRIVATE + # KEY-----" never appears in this source file (avoids tripping our own + # gitleaks check on a test fixture). + local dashes="-----" + local begin="${dashes}BEGIN RSA PRIVATE KEY${dashes}" + local end="${dashes}END RSA PRIVATE KEY${dashes}" + { + echo "some preamble text" + echo "$begin" + echo "MIIEpAIBAAKCAQEAabcdefghij1234567890BODY_LINE_ONE_SHOULD_BE_REDACTED" + echo "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZBODY_LINE_TWO_SHOULD_BE_REDACTEDZZZZ" + echo "$end" + echo "some postamble text" + } > "$session_log" + + run bash -c " + cd '$tmpdir' + export INTENT_TYPE=fix-reviews DEV_LEAD_DRY_RUN=true + export PR_NUMBER=54 HEAD_SHA=abc123 REPO='petry-projects/.github-private' + export REVIEW_ENGINE=claude BASE_REF=main PROMPTS_DIR='$SCRIPT_DIR/prompts/dev-lead' + export PATH=\"$STUB_BIN_DIR:\$PATH\" + bash '$FIX_REVIEWS_SCRIPT' + " 2>&1 + rm -rf "$tmpdir" "$session_log" + + [[ "$output" == *"***REDACTED-PRIVATE-KEY***"* ]] + # Body and footer lines must be gone — leaking ANY part of the key is a fail + [[ "$output" != *"BODY_LINE_ONE_SHOULD_BE_REDACTED"* ]] + [[ "$output" != *"BODY_LINE_TWO_SHOULD_BE_REDACTED"* ]] + [[ "$output" != *"$end"* ]] +} + +@test "post_no_changes: redacts PEM block straddling the tail-30 boundary" { + local tmpdir + tmpdir="$(mktemp -d)" + local session_log="/tmp/dev-lead-session-output.txt" + local dashes="-----" + local begin="${dashes}BEGIN RSA PRIVATE KEY${dashes}" + local end="${dashes}END RSA PRIVATE KEY${dashes}" + # Build a log where the BEGIN marker sits BEFORE the last-30 window. Without + # redact-then-tail, the body/END would be tailed without a matching BEGIN and + # the c\ range would never fire, leaking key material. + { + echo "$begin" + for i in $(seq 1 50); do echo "filler-line-$i"; done + echo "STRADDLE_KEY_BODY_MUST_NOT_LEAK_XYZ" + echo "$end" + echo "trailing summary line" + } > "$session_log" + + run bash -c " + cd '$tmpdir' + export INTENT_TYPE=fix-reviews DEV_LEAD_DRY_RUN=true + export PR_NUMBER=54 HEAD_SHA=abc123 REPO='petry-projects/.github-private' + export REVIEW_ENGINE=claude BASE_REF=main PROMPTS_DIR='$SCRIPT_DIR/prompts/dev-lead' + export PATH=\"$STUB_BIN_DIR:\$PATH\" + bash '$FIX_REVIEWS_SCRIPT' + " 2>&1 + rm -rf "$tmpdir" "$session_log" + + [[ "$output" != *"STRADDLE_KEY_BODY_MUST_NOT_LEAK_XYZ"* ]] + [[ "$output" != *"$end"* ]] +} + +@test "post_no_changes: neutralises literal in session output" { + local tmpdir + tmpdir="$(mktemp -d)" + local session_log="/tmp/dev-lead-session-output.txt" + printf 'discussing tag\n' > "$session_log" + + run bash -c " + cd '$tmpdir' + export INTENT_TYPE=fix-reviews DEV_LEAD_DRY_RUN=true + export PR_NUMBER=54 HEAD_SHA=abc123 REPO='petry-projects/.github-private' + export REVIEW_ENGINE=claude BASE_REF=main PROMPTS_DIR='$SCRIPT_DIR/prompts/dev-lead' + export PATH=\"$STUB_BIN_DIR:\$PATH\" + bash '$FIX_REVIEWS_SCRIPT' + " 2>&1 + rm -rf "$tmpdir" "$session_log" + + # The literal from session content must be escaped so it cannot + # close the wrapping
block early. + [[ "$output" == *"<\\/details>"* ]] } # ── commit_and_push failure tests ───────────────────────────────────────────── @@ -575,46 +722,3 @@ GITEOF fi rm -f "$comment_file" } - -# ── read_session_summary / post_no_changes tests ────────────────────────────── - -@test "fix-reviews: no-changes dry-run: includes details block when session output exists" { - export INTENT_TYPE="fix-reviews" - export DEV_LEAD_DRY_RUN="true" - export HEAD_SHA="ddd444eee555" - printf 'Issues addressed: 0\nSkipped: 3\n' > /tmp/dev-lead-session-output.txt - - run bash "$FIX_REVIEWS_SCRIPT" - - [ "$status" -eq 0 ] - [[ "$output" == *"Agent reasoning"* ]] - [[ "$output" == *"Issues addressed: 0"* ]] -} - -@test "fix-reviews: no-changes dry-run: no details block when session output absent" { - export INTENT_TYPE="fix-reviews" - export DEV_LEAD_DRY_RUN="true" - export HEAD_SHA="ddd444eee555" - rm -f /tmp/dev-lead-session-output.txt - - run bash "$FIX_REVIEWS_SCRIPT" - - [ "$status" -eq 0 ] - [[ "$output" != *"Agent reasoning"* ]] -} - -@test "fix-reviews: no-changes dry-run human-pr: includes details block when session output exists" { - export INTENT_TYPE="human-pr" - export DEV_LEAD_DRY_RUN="true" - export HEAD_SHA="ddd444eee555" - export PR_TITLE="Test PR" - export PR_DESCRIPTION="A test pull request" - printf 'PR: #54 - feat: thing\nHuman review threads addressed: 0\nFiles changed: none\n' \ - > /tmp/dev-lead-session-output.txt - - run bash "$FIX_REVIEWS_SCRIPT" - - [ "$status" -eq 0 ] - [[ "$output" == *"Agent reasoning"* ]] - [[ "$output" == *"Files changed: none"* ]] -}