Skip to content
Merged
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
12 changes: 9 additions & 3 deletions prompts/dev-lead/fix-bot-comment.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`):
Expand Down
78 changes: 64 additions & 14 deletions scripts/dev-lead-fix-reviews.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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***'
Comment thread
don-petry marked this conversation as resolved.
}

# 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 </details> so the wrapping <details> stays intact.
post_no_changes() {
local intent="$1"
local _summary
_summary=$(read_session_summary)
if [ -n "$_summary" ]; then
post_reviews_terminal "$intent" "no-changes" \
"<details><summary>Agent reasoning</summary>

\`\`\`
_summary=$(read_session_summary || true)
local msg="No actionable items found."
if [[ -n "$_summary" ]]; then
# Neutralise any </details> in summary so it cannot close the outer block.
_summary=$(printf '%s' "$_summary" | sed 's|</details>|<\\/details>|g')
local fence
fence=$(pick_fence "$_summary")
msg="<details><summary>Agent reasoning</summary>

${fence}
${_summary}
\`\`\`
${fence}
</details>"
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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
40 changes: 33 additions & 7 deletions scripts/engine.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<details><summary>Click to expand session logs</summary>" >> "$GITHUB_STEP_SUMMARY"
echo "" >> "$GITHUB_STEP_SUMMARY"
cat "$_tmp" >> "$GITHUB_STEP_SUMMARY"
echo "" >> "$GITHUB_STEP_SUMMARY"
echo "</details>" >> "$GITHUB_STEP_SUMMARY"
# HTML-escape the redacted log so literal </details> or </summary> in
# the agent output cannot break the wrapping <details> block on the run
# summary page.
{
echo "## Dev-Lead session output"
echo "<details><summary>Click to expand session logs</summary>"
echo ""
echo "<pre>"
sed 's|&|\&amp;|g; s|<|\&lt;|g; s|>|\&gt;|g' /tmp/dev-lead-session-output.txt
echo "</pre>"
echo ""
echo "</details>"
} >> "$GITHUB_STEP_SUMMARY" || \
echo "::warning::Failed to append session output to GITHUB_STEP_SUMMARY" >&2
Comment thread
don-petry marked this conversation as resolved.
fi
cp "$_tmp" /tmp/dev-lead-session-output.txt
rm -f "$_tmp"
fi
return "$rc"
Expand Down
121 changes: 86 additions & 35 deletions tests/dev-lead/unit/test_engine_writer.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 </details> which must be escaped in the summary so it
# cannot close the wrapping <details> block on the run summary page.
cat > "$STUB_BIN_DIR/claude" << 'STUB'
#!/usr/bin/env bash
echo "danger zone </details> 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 </details> must be HTML-escaped → only the wrapping closer remains
! grep -F "danger zone </details>" "$summary"
grep -F "danger zone &lt;/details&gt;" "$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
}
Loading
Loading