From 05c625275ef4a110d20c13174eae22094e279f2d Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 7 Jul 2026 15:00:35 -0700 Subject: [PATCH 1/2] ci: split PR severity workflow into classify and apply jobs In this commit, we separate the two concerns in the PR severity workflow: working out the severity, and applying it. The classify job inspects the PR and records its verdict (the severity level, whether to comment, and the comment body) to a few files. A second apply job reads those files and does the mechanical work of setting the label and posting the comment. Pulling the classification apart from the application keeps each job doing one thing and makes the flow easier to follow. The apply job takes the severity the classifier picked and checks it against the known set before touching a label, and posts the comment from a file via --body-file so the body is handled as plain data. We also turn off checkout credential persistence, since neither job needs a git credential on disk. --- .github/workflows/pr-severity.yml | 213 +++++++++++++++++++++--------- scripts/apply-pr-severity.sh | 145 ++++++++++++++++++++ scripts/apply-pr-severity_test.sh | 125 ++++++++++++++++++ 3 files changed, 424 insertions(+), 59 deletions(-) create mode 100755 scripts/apply-pr-severity.sh create mode 100755 scripts/apply-pr-severity_test.sh diff --git a/.github/workflows/pr-severity.yml b/.github/workflows/pr-severity.yml index 6b4897738..e4c14ff22 100644 --- a/.github/workflows/pr-severity.yml +++ b/.github/workflows/pr-severity.yml @@ -1,16 +1,19 @@ name: PR Severity Classification on: - # Use pull_request_target to allow running on fork PRs with access to secrets. - # This is safe because we don't checkout or execute any code from the PR - - # we only read PR metadata (changed files, labels) via the GitHub API. + # Use pull_request_target so the workflow runs on fork PRs with the base + # repository's workflow definition. The classify job below reads PR metadata + # with a read-only token and never checks out or executes PR code; the write + # scope needed to apply the label lives in a separate, model-free job. pull_request_target: types: [opened, synchronize, labeled] +# Default the whole workflow to read-only. Each job opts into exactly the scope +# it needs: classify stays read-only (untrusted PR metadata reaches the model, +# so it must not hold a write token), apply takes pull-requests: write but runs +# no model. permissions: contents: read - pull-requests: write - issues: write concurrency: group: pr-severity-${{ github.event.pull_request.number }} @@ -20,6 +23,14 @@ jobs: classify: name: Classify PR Severity runs-on: ubuntu-latest + # Cap the model run: it fires on every pull_request_target synchronize with + # attacker-controllable input, so bound its runner-minute/token cost rather + # than inheriting GitHub's 6h default. Mirrors the dedupe workflow. + timeout-minutes: 10 + # Read-only: the classifier only inspects PR metadata via the GitHub API. + permissions: + contents: read + pull-requests: read # Skip if PR has skip-severity-check label. # For labeled events, only run if 'reclassify' label was added. if: | @@ -30,43 +41,78 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 1 + # Don't leave the job token in .git/config: nothing here needs a + # persisted git credential, and the classifier runs on untrusted + # fork-PR input. + persist-credentials: false - name: Classify PR with Claude - uses: anthropics/claude-code-action@v1 + # Pinned to a full commit SHA rather than the mutable @v1 tag: this step + # runs on pull_request_target with CLAUDE_CODE_OAUTH_TOKEN and + # GITHUB_TOKEN in-process and is reachable by any fork author, so a + # repointed tag would run attacker-controlled action code with those + # secrets present. Bump deliberately when updating. + uses: anthropics/claude-code-action@ba0aafd4308cbba7165f9f2cdb0cfbed5a3c99ce # v1 + env: + # gh (invoked by the Bash(gh pr view) tool) authenticates from + # GH_TOKEN; set it explicitly so classification doesn't depend on the + # action propagating its github_token input into the tool environment, + # matching the dedupe find-duplicates step. + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} github_token: ${{ secrets.GITHUB_TOKEN }} - # Allow any user since this workflow only reads PR metadata via API - # and doesn't execute any code from the PR. Tool permissions are - # restricted to gh pr commands only. + # Accept any PR author: this job holds only a read-only token, reads + # PR metadata via the API, and writes its verdict to a file. The + # privileged label/comment step runs separately without the model. + # "*" is safe ONLY while this job stays read-only. Before granting it + # a write token or a mutating tool (a write-capable gh subcommand, a + # Bash mutation), replace "*" with an explicit allowlist — otherwise + # any fork author's PR text would steer a privileged model. allowed_non_write_users: "*" - # Allow Claude to manage labels and post comments. - # Keep permissions minimal to limit prompt injection risk. - claude_args: --allowedTools "Bash(gh pr view:*)" "Bash(gh pr edit:*)" "Bash(gh pr comment:*)" + # Pin the model so the severity decision (which gates the label the + # apply job applies) is reproducible from the workflow file and does + # not drift on an action-default change. + model: claude-sonnet-5 + + # The classifier only needs to read PR data and record its verdict to + # the workspace. It has no write-capable gh tools. + # + # Security note: because this runs on pull_request_target, the OAuth + # token above is present in-process while the model reads untrusted + # fork-PR text. The read-only github_token bounds what the model can + # WRITE via the API, but the in-process token's protection rests on + # this allowlist staying minimal — read-only `gh pr view` plus + # `Write`, with no network- or shell-mutating tool a prompt injection + # could use to exfiltrate it. Keep it that way: do not add `Bash` + # verbs beyond `gh pr view`, and never add a tool that can make + # outbound requests. (A stricter design would drop Bash entirely and + # pre-fetch PR metadata via the API in a separate step.) + claude_args: --allowedTools "Bash(gh pr view:*)" "Write" prompt: | You are a PR severity classifier for the lnd (Lightning Network Daemon) repository. ## Tool Constraints - You ONLY have access to these commands: + You ONLY have access to: - `gh pr view` - to read PR metadata - - `gh pr edit` - to add/remove labels - - `gh pr comment` - to post comments + - `Write` - to record your verdict to files - You do NOT have access to `gh api`, `gh label`, or any other - `gh` subcommand. Do not attempt to use them. For ALL label - operations, use `gh pr edit` with `--add-label` or - `--remove-label`. + You do NOT have access to `gh api`, `gh label`, `gh pr edit`, + `gh pr comment`, or any other command. Do not attempt to use them. + You do NOT apply labels or post comments yourself. A separate, + deterministic step reads the files you write and applies the label + and comment. Your job is only to classify and record the result. ## Your Task Analyze PR #${{ github.event.pull_request.number }} and: 1. Determine its severity level based on the files changed - 2. Apply the appropriate severity label - 3. Post a detailed comment explaining your determination + 2. Record the severity, whether a comment should be posted, and the + comment body, to files (see "Output" below). ## Severity Levels @@ -130,51 +176,49 @@ jobs: ## Steps - 1. First, check for existing override labels AND existing severity labels: + 1. Read the current labels AND comments to detect overrides and prior + bot activity: ``` - gh pr view ${{ github.event.pull_request.number }} --json labels --jq '.labels[].name' + gh pr view ${{ github.event.pull_request.number }} --json labels,comments ``` - Note which `severity-*` label (if any) is currently applied. This is - the "previous severity". + Note which `severity-*` label (if any) is currently applied. This + is the "previous severity". Look for the HTML marker + `` in comment bodies to tell whether the + bot has commented before. - 2. If an override label exists (severity-override-*), use that level and skip classification. + 2. If an override label exists (severity-override-*), use that level + and skip classification. - 3. Check for existing bot comments. Look for the HTML marker ``: - ``` - gh pr view ${{ github.event.pull_request.number }} --json comments --jq '.comments[].body' | grep -c 'pr-severity-bot' || true - ``` - This tells you whether the bot has commented before. - - 4. Get the list of changed files: + 3. Get the list of changed files: ``` gh pr view ${{ github.event.pull_request.number }} --json files,additions,deletions ``` - 5. Classify each file and determine the new overall severity. + 4. Classify each file and determine the new overall severity. - 6. **Decide whether to comment.** Only post a comment if EITHER: - - The bot has NOT commented before (no existing comment with ``), OR - - The newly determined severity is DIFFERENT from the previous severity label. + 5. **Decide whether a comment should be posted.** Set should_comment + to "true" only if EITHER: + - The bot has NOT commented before (no existing comment with + ``), OR + - The newly determined severity is DIFFERENT from the previous + severity label. - If the bot already commented AND the severity has NOT changed, just - stop here — do NOT post another comment. Still update the label if - needed (step 7-8), but skip the comment. + Otherwise set should_comment to "false" (the label may still be + updated by the apply step, but no new comment is posted). - 7. Remove any existing severity-* labels (not override labels): - ``` - gh pr edit ${{ github.event.pull_request.number }} --remove-label "severity-critical" 2>/dev/null || true - gh pr edit ${{ github.event.pull_request.number }} --remove-label "severity-high" 2>/dev/null || true - gh pr edit ${{ github.event.pull_request.number }} --remove-label "severity-medium" 2>/dev/null || true - gh pr edit ${{ github.event.pull_request.number }} --remove-label "severity-low" 2>/dev/null || true - ``` + ## Output - 8. Apply the new severity label: - ``` - gh pr edit ${{ github.event.pull_request.number }} --add-label "severity-" - ``` + Record your verdict by writing these files in the current working + directory (the repository root). Do NOT apply labels or comment + yourself. - 9. If you determined in step 6 that a comment should be posted, post it - with your analysis. Use this format: + 1. `severity.txt` - exactly one lowercase word, one of: + `critical`, `high`, `medium`, `low`. Nothing else. + + 2. `should_comment.txt` - exactly `true` or `false`. + + 3. `comment.md` - only if should_comment is `true`. The full comment + markdown, in this format: If this is a severity CHANGE (previous label existed but differs), prepend: `> ⚠️ Severity changed: **** → **** (files changed since last classification)` @@ -203,16 +247,67 @@ jobs: ``` - 10. Post the comment using `gh pr comment`: - ``` - gh pr comment ${{ github.event.pull_request.number }} --body "YOUR_COMMENT_HERE" - ``` + If should_comment is `false`, do not create `comment.md`. - 11. If you decided in step 6 to SKIP commenting, do NOT post any comment. - Just ensure the label is correct and exit. + Keep the comment concise and factual: it is posted verbatim under + the bot's identity, and the apply step defangs any `@`-mentions + and links, so do not rely on them. ## Emoji Mapping - critical: 🔴 - high: 🟠 - medium: 🟡 - low: 🟢 + + - name: Upload classification result + uses: actions/upload-artifact@v4 + with: + name: pr-severity-result + path: | + severity.txt + should_comment.txt + comment.md + if-no-files-found: warn + retention-days: 1 + + apply: + name: Apply Severity Label + runs-on: ubuntu-latest + needs: classify + timeout-minutes: 5 + # Write scope lives here, in a job that runs no model. The only inputs are + # the PR number from the trusted event payload and the classifier's files, + # which are strictly validated before use. + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + # Needed for scripts/apply-pr-severity.sh; no persisted git credential + # is required here. + persist-credentials: false + + - name: Download classification result + uses: actions/download-artifact@v4 + # classify uploads with if-no-files-found: warn, so if the model writes + # no verdict at all (timeout, refusal) no artifact exists and + # download-artifact would otherwise hard-fail the job. Tolerate a + # missing artifact so the no-op guard in the next step is reachable. + continue-on-error: true + with: + name: pr-severity-result + path: result + + - name: Apply label and comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + # Validate the severity, reconcile the label, and sanitize + post the + # model-authored comment. The logic lives in a checked-in script so the + # untrusted-comment sanitizer is unit-tested + # (scripts/apply-pr-severity_test.sh), mirroring how the dedupe workflow + # delegates to scripts/comment-on-duplicates.sh. + run: ./scripts/apply-pr-severity.sh result diff --git a/scripts/apply-pr-severity.sh b/scripts/apply-pr-severity.sh new file mode 100755 index 000000000..311aac8df --- /dev/null +++ b/scripts/apply-pr-severity.sh @@ -0,0 +1,145 @@ +#!/usr/bin/env bash +# +# Applies the PR severity label and posts the classifier's comment. +# +# Reads the classifier's verdict from a result directory (arg $1, default +# "result") and, in this order: +# - validates the severity against the known set, +# - reconciles the severity-* label in a single gh edit, and +# - sanitizes and posts the model-authored comment. +# +# The classify job runs a model on untrusted PR text, so the comment body is +# treated as tainted and passed through sanitize_comment() before posting. +# +# Env: GH_TOKEN (gh auth), PR_NUMBER, REPO. +# Usage: ./apply-pr-severity.sh [RESULT_DIR] + +set -euo pipefail + +# sanitize_comment reads an untrusted comment body on stdin and writes a safe +# version to stdout. GitHub already strips scripts and unsafe HTML from rendered +# comment bodies, so this targets the notification/spam class: @-mentions that +# ping arbitrary users, `#N` cross-references that notify other issues/PRs, +# auto-linked URLs used for phishing, and Markdown links. It deliberately leaves +# the model's
/ markup intact so the comment still renders. +# +# The zero-width non-joiner is spliced in as a literal byte (not a `\xNN` sed +# escape) so the rules work identically under both GNU and BSD sed. Rule order +# matters: the `#`-before-digit defang runs before the bracket rules, which +# themselves emit `[`/`]` — running it after would corrupt those +# entities. Matching only `#` followed by a digit leaves `##`/`###` Markdown +# headings untouched. +sanitize_comment() { + local z + z="$(printf '\xe2\x80\x8c')" + sed -E \ + -e "s/@/@${z}/g" \ + -e "s/www\./www${z}./g" \ + -e 's,[hH][tT][tT][pP][sS]?://,hxxp://,g' \ + -e "s/#([0-9])/#${z}\1/g" \ + -e 's/\[/\[/g' \ + -e 's/\]/\]/g' +} + +main() { + local result_dir="${1:-result}" + : "${GH_TOKEN:?GH_TOKEN is required}" + : "${PR_NUMBER:?PR_NUMBER is required}" + : "${REPO:?REPO is required}" + + # Treat a missing severity.txt and an empty/whitespace-only one the same way: + # both mean the classify job produced no usable verdict (model crash or + # timeout, a missing artifact tolerated by the download step, or a truncated + # write). Warn and degrade rather than laundering a broken run into a silent + # green or failing the check red, since severity classification is advisory. + # A non-empty but unrecognized value still fails below. + # + # Lowercase on read so a stray `Low`/`HIGH` from the model still matches. + local severity="" + if [[ -f "$result_dir/severity.txt" ]]; then + severity="$(tr -d '[:space:]' < "$result_dir/severity.txt" | tr '[:upper:]' '[:lower:]')" + fi + if [[ -z "$severity" ]]; then + echo "::warning::PR severity classifier produced no verdict; PR left unlabeled." + return 0 + fi + + # Strictly validate the severity against the known set — it is the only + # privileged, semantically meaningful output. + case "$severity" in + critical|high|medium|low) ;; + *) + echo "Invalid severity '$severity'; refusing to apply." >&2 + return 1 + ;; + esac + + # Read the PR's current severity-* labels so the reconciliation below removes + # exactly the ones present. (The severity-change banner is authored by the + # model in comment.md, so no previous severity is needed here.) + # + # Fail closed on a read error (rate limit, 5xx): swallowing it would yield + # empty labels, so the loop below would remove nothing yet still add the new + # label — the exact two-label state this single-edit design prevents. With + # set -e, a failed read aborts before any edit. (local is declared separately + # so the assignment's own exit status, not local's, drives set -e.) + local cur_labels + cur_labels="$(gh pr view "$PR_NUMBER" --repo "$REPO" \ + --json labels --jq '.labels[].name')" + + # Reconcile the severity label in a SINGLE gh edit: add the target and remove + # exactly the other severity-* labels currently present. Doing it in one call + # means: + # - if the add fails (label missing from the repo, transient API error), the + # whole edit fails and the PR keeps its prior label rather than being left + # unlabeled; and + # - a run cancelled by `concurrency.cancel-in-progress` can't stop between an + # add and a separate remove, so the PR is never left carrying two labels. + # Only present labels are removed, so gh never errors on a missing one. + local edit_args level + edit_args=(--add-label "severity-$severity") + for level in critical high medium low; do + [[ "$level" == "$severity" ]] && continue + if grep -qx "severity-$level" <<< "$cur_labels"; then + edit_args+=(--remove-label "severity-$level") + fi + done + gh pr edit "$PR_NUMBER" --repo "$REPO" "${edit_args[@]}" + + # Lowercase on read (as with severity) so a `True`/`TRUE` slip from the model + # is honored rather than silently dropping the comment. + local should_comment="false" + if [[ -f "$result_dir/should_comment.txt" ]]; then + should_comment="$(tr -d '[:space:]' < "$result_dir/should_comment.txt" | tr '[:upper:]' '[:lower:]')" + fi + + # Use -s (exists and non-empty): a 0-byte comment.md would otherwise reach + # `gh pr comment --body-file`, which rejects an empty body and would abort + # after the label was already applied. + if [[ "$should_comment" != "true" || ! -s "$result_dir/comment.md" ]]; then + echo "No comment requested; done." + return 0 + fi + + # Sanitize first, then enforce the size cap on the SANITIZED body. Every + # sanitizer rule only grows the byte count (e.g. `[` -> `[`), so a body + # that fits before sanitizing can exceed GitHub's comment limit afterward; + # measuring the post-sanitization size is what actually bounds the request. + sanitize_comment < "$result_dir/comment.md" > "$result_dir/comment.sanitized.md" + + local max_bytes=16384 + if [[ "$(wc -c < "$result_dir/comment.sanitized.md")" -gt "$max_bytes" ]]; then + echo "::warning::classifier comment exceeds ${max_bytes} bytes after sanitization; skipping comment." + return 0 + fi + + # Post from a file (never interpolated into the shell) so the body is handled + # purely as data. + gh pr comment "$PR_NUMBER" --repo "$REPO" \ + --body-file "$result_dir/comment.sanitized.md" +} + +# Allow sourcing (e.g. from the test) without executing main. +if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then + main "$@" +fi diff --git a/scripts/apply-pr-severity_test.sh b/scripts/apply-pr-severity_test.sh new file mode 100755 index 000000000..b2fdc1e11 --- /dev/null +++ b/scripts/apply-pr-severity_test.sh @@ -0,0 +1,125 @@ +#!/usr/bin/env bash +# +# Tests for scripts/apply-pr-severity.sh: the sanitize_comment() filter and the +# main() control flow (label reconciliation and comment gating), the latter with +# gh stubbed via a PATH shim that records its argument lists. +# +# Run: bash scripts/apply-pr-severity_test.sh + +set -euo pipefail + +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=scripts/apply-pr-severity.sh +source "$DIR/apply-pr-severity.sh" + +ZWNJ="$(printf '\xe2\x80\x8c')" +fail=0 + +want() { grep -qF -- "$2" <<< "$OUT" && echo "ok: $1" || { echo "FAIL: $1"; fail=1; }; } +absent() { grep -qF -- "$2" <<< "$OUT" && { echo "FAIL: $1"; fail=1; } || echo "ok: $1"; } +eq() { [[ "$2" == "$3" ]] && echo "ok: $1" || { echo "FAIL: $1 (got '$2' want '$3')"; fail=1; }; } +has() { grep -qF -- "$2" "$GH_CALLS" && echo "ok: $1" || { echo "FAIL: $1 (no gh call: $2)"; fail=1; }; } +lacks() { grep -qF -- "$2" "$GH_CALLS" && { echo "FAIL: $1 (unexpected gh call: $2)"; fail=1; } || echo "ok: $1"; } +in_file(){ grep -qF -- "$3" "$2" && echo "ok: $1" || { echo "FAIL: $1"; fail=1; }; } + +echo "# sanitize_comment" +INPUT='## 🟢 PR Severity: **LOW** +### Analysis +Ping @user, see #123 and org/repo#4567, [x](https://evil), www.evil.net, http://Bad.io. +' +OUT="$(printf '%s' "$INPUT" | sanitize_comment)" +want "## heading preserved" '## 🟢 PR Severity' +want "### heading preserved" '### Analysis' +want "@-mention defanged" "@${ZWNJ}user" +want "#123 defanged" "#${ZWNJ}123" +want "repo#4567 defanged" "#${ZWNJ}4567" +want "https scheme defanged" 'hxxp://evil' +want "http scheme (case-insensitive)" 'hxxp://Bad.io' +want "www. auto-link defanged" "www${ZWNJ}.evil.net" +want "markdown brackets escaped" '[x]' +want "bot marker preserved" '' +absent "no live http(s):// scheme" 'https://' +absent "bracket entity not corrupted by #" "&#${ZWNJ}" + +echo "# main()" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT +mkdir -p "$WORK/bin" +# Fake gh: record the full argument list, and for `pr view` emit the fixture +# labels so main()'s reconciliation has something to diff against. +cat > "$WORK/bin/gh" <<'SH' +#!/usr/bin/env bash +echo "$*" >> "$GH_CALLS" +[[ "$1 $2" == "pr view" ]] && printf '%s' "${GH_FAKE_LABELS:-}" +exit 0 +SH +chmod +x "$WORK/bin/gh" +export PATH="$WORK/bin:$PATH" +export GH_TOKEN=x PR_NUMBER=42 REPO=owner/repo + +# run_main RESULT_DIR: run main() capturing its return code in RC, with a fresh +# GH_CALLS log for the case. +run_main() { export GH_CALLS="$WORK/calls"; : > "$GH_CALLS"; RC=0; main "$1" >/dev/null 2>&1 || RC=$?; } + +# Invalid severity: reject before any gh mutation. +r="$WORK/invalid"; mkdir -p "$r"; printf 'bogus\n' > "$r/severity.txt" +GH_FAKE_LABELS='' run_main "$r" +eq "invalid severity returns 1" "$RC" "1" +lacks "invalid severity: no label edit" "pr edit" + +# No verdict at all: warn and no-op, no gh mutation. +r="$WORK/noverdict"; mkdir -p "$r" +GH_FAKE_LABELS='' run_main "$r" +eq "missing severity.txt returns 0" "$RC" "0" +lacks "missing verdict: no label edit" "pr edit" + +# Empty/whitespace-only severity.txt is handled like a missing one (F30). +r="$WORK/emptysev"; mkdir -p "$r"; printf ' \n' > "$r/severity.txt" +GH_FAKE_LABELS='' run_main "$r" +eq "empty severity.txt returns 0" "$RC" "0" +lacks "empty severity: no label edit" "pr edit" + +# Valid severity, no comment: reconcile labels in one edit, remove only present. +r="$WORK/label"; mkdir -p "$r"; printf 'high\n' > "$r/severity.txt"; printf 'false\n' > "$r/should_comment.txt" +GH_FAKE_LABELS=$'severity-low\nkeep-me\nseverity-medium' run_main "$r" +eq "label-only returns 0" "$RC" "0" +has "adds target label" "--add-label severity-high" +has "removes present severity-low" "--remove-label severity-low" +has "removes present severity-medium" "--remove-label severity-medium" +lacks "does not touch absent critical" "severity-critical" +lacks "no comment when should_comment false" "pr comment" + +# should_comment true with a real body: sanitize and post it. +r="$WORK/comment"; mkdir -p "$r"; printf 'low\n' > "$r/severity.txt"; printf 'true\n' > "$r/should_comment.txt" +printf '## x\nhi @bob\n' > "$r/comment.md" +GH_FAKE_LABELS='' run_main "$r" +eq "comment path returns 0" "$RC" "0" +has "posts sanitized comment" "pr comment 42 --repo owner/repo --body-file" +in_file "posted body defangs @-mention" "$r/comment.sanitized.md" "@${ZWNJ}bob" + +# Capitalized should_comment is honored, not silently dropped (F31). +r="$WORK/truecase"; mkdir -p "$r"; printf 'low\n' > "$r/severity.txt"; printf 'TRUE\n' > "$r/should_comment.txt" +printf '## x\nhi\n' > "$r/comment.md" +GH_FAKE_LABELS='' run_main "$r" +eq "should_comment TRUE returns 0" "$RC" "0" +has "TRUE still posts the comment" "pr comment 42 --repo owner/repo --body-file" + +# should_comment true but empty body (F15): no post. +r="$WORK/empty"; mkdir -p "$r"; printf 'low\n' > "$r/severity.txt"; printf 'true\n' > "$r/should_comment.txt" +: > "$r/comment.md" +GH_FAKE_LABELS='' run_main "$r" +eq "empty comment returns 0" "$RC" "0" +lacks "empty comment: no post" "pr comment" + +# Oversized after sanitization (F22): brackets expand ~5x past the cap; no post. +r="$WORK/big"; mkdir -p "$r"; printf 'low\n' > "$r/severity.txt"; printf 'true\n' > "$r/should_comment.txt" +{ printf '## x\n'; head -c 20000 /dev/zero | tr '\0' '['; } > "$r/comment.md" +GH_FAKE_LABELS='' run_main "$r" +eq "oversized comment returns 0" "$RC" "0" +lacks "oversized comment: no post" "pr comment" + +if [[ "$fail" -ne 0 ]]; then + echo "TESTS FAILED" + exit 1 +fi +echo "ALL TESTS PASSED" From d1ea8687c08cef029be30ac8e75b8ec6913f8564 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 7 Jul 2026 15:00:35 -0700 Subject: [PATCH 2/2] ci: split issue dedupe into find and post jobs In this commit, we give the issue dedupe workflow the same shape: one job finds the duplicate candidates, another posts the comment. The find job records the candidate issue numbers to a file, and the post job hands those numbers to comment-on-duplicates.sh, which already validates each number and renders the comment from a fixed template. Keeping detection and posting apart mirrors how the script is already factored, so the post job ends up a thin wrapper over it. We also drop the unused id-token permission and turn off checkout credential persistence while we're in here. --- .github/workflows/claude-dedupe-issues.yml | 155 ++++++++++++++++++++- 1 file changed, 148 insertions(+), 7 deletions(-) diff --git a/.github/workflows/claude-dedupe-issues.yml b/.github/workflows/claude-dedupe-issues.yml index 5f73298fe..93c052e3b 100644 --- a/.github/workflows/claude-dedupe-issues.yml +++ b/.github/workflows/claude-dedupe-issues.yml @@ -10,26 +10,167 @@ on: required: true type: string +# Default to read-only. The find-duplicates job reads untrusted issue text with +# the model, so it must not hold a write token; post-comment takes issues: write +# but runs no model and only shells out to scripts/comment-on-duplicates.sh, +# which re-validates every issue number it is handed. +permissions: + contents: read + +# Serialize runs for the same issue so an `issues: opened` event and a +# workflow_dispatch for the same number can't both read "no prior comment" and +# double-post. Mirrors pr-severity.yml. +concurrency: + group: claude-dedupe-${{ github.event.issue.number || inputs.issue_number }} + cancel-in-progress: true + jobs: - claude-dedupe-issues: + find-duplicates: runs-on: ubuntu-latest timeout-minutes: 10 + # Read-only: the model inspects the issue and searches for duplicates, then + # records the candidate issue numbers to a file. permissions: contents: read - issues: write - id-token: write + issues: read steps: - name: Checkout repository uses: actions/checkout@v4 + with: + persist-credentials: false - - name: Run Claude Code slash command - uses: anthropics/claude-code-action@v1 + - name: Find duplicate issues with Claude + # Pinned to a full commit SHA rather than the mutable @v1 tag: this step + # feeds untrusted issue text to the model with CLAUDE_CODE_OAUTH_TOKEN in + # process, so a repointed tag would run attacker-controlled action code + # with that secret present. Bump deliberately when updating. + uses: anthropics/claude-code-action@ba0aafd4308cbba7165f9f2cdb0cfbed5a3c99ce # v1 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: github_token: ${{ secrets.GITHUB_TOKEN }} + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + + # Accept any issue author: this job holds only read scope and merely + # records candidate issue numbers to a file; the comment is posted by + # a separate, model-free job. "*" is safe ONLY while this job stays + # read-only. Before granting this job a write token or a mutating tool + # (a write-capable gh subcommand, a Bash mutation), replace "*" with + # an explicit allowlist — otherwise any fork author's issue text would + # steer a privileged model. allowed_non_write_users: "*" model: claude-haiku-4-5-20251001 - prompt: "/dedupe ${{ github.repository }}/issues/${{ github.event.issue.number || inputs.issue_number }}" - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + + # Read-only gh tools plus Write to record the result. No comment or + # edit tools, and no access to the duplicate-comment script. + claude_args: >- + --allowedTools + "Bash(gh issue view:*)" + "Bash(gh search:*)" + "Bash(gh issue list:*)" + "Write" + + prompt: | + Find up to 3 likely duplicate issues for issue + #${{ github.event.issue.number || inputs.issue_number }} in the + ${{ github.repository }} repository. Follow these steps precisely: + + 1. View the issue and check whether it (a) is closed, (b) does not + need deduping (e.g. broad product feedback without a specific + solution, or positive feedback), or (c) already has a duplicates + comment. If any of these hold, write an empty `duplicates.txt` + (create the file with no content) and stop. + + 2. Summarize the issue. + + 3. Search GitHub for duplicates of this issue using several diverse + keyword searches and search approaches, based on the summary. + + 4. Filter out false positives that are likely not actually + duplicates of the original issue. If no plausible duplicates + remain, write an empty `duplicates.txt` and stop. + + 5. Otherwise, write the chosen duplicate issue numbers to a file + named `duplicates.txt` in the current working directory: digits + only, one issue number per line, at most 3 lines. Do not include + `#`, URLs, or any other text. + + Notes: + - Use `gh` to interact with GitHub, not web fetch. + - Do NOT use any tools beyond `gh issue view`, `gh search`, + `gh issue list`, and `Write`. You do NOT post comments; a separate + step does that from the file you write. + - Make a todo list first. + + - name: Upload duplicate candidates + uses: actions/upload-artifact@v4 + with: + name: dedupe-result + path: duplicates.txt + if-no-files-found: warn + retention-days: 1 + + post-comment: + runs-on: ubuntu-latest + needs: find-duplicates + timeout-minutes: 5 + # Write scope lives here, in a job that runs no model. The base issue number + # comes from the trusted event payload, and comment-on-duplicates.sh + # re-validates every candidate issue number before posting. + permissions: + contents: read + issues: write + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Download duplicate candidates + uses: actions/download-artifact@v4 + # find-duplicates uploads with if-no-files-found: warn, so when the + # model writes no file at all (timeout, refusal) no artifact exists and + # download-artifact would otherwise hard-fail the job. Tolerate a + # missing artifact so the no-op guard in the next step is reachable. + continue-on-error: true + with: + name: dedupe-result + path: result + + - name: Post duplicate comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_REPOSITORY: ${{ github.repository }} + BASE_ISSUE: ${{ github.event.issue.number || inputs.issue_number }} + run: | + set -euo pipefail + + # Distinguish a genuine "no duplicates" verdict from a find-duplicates + # run that produced no artifact at all (model crash/timeout, or a + # tolerated missing-artifact download). The latter gets a warning so a + # broken run doesn't read as a healthy no-op, mirroring the pr-severity + # apply step; the present-but-empty case stays a silent no-op. + if [[ ! -f result/duplicates.txt ]]; then + echo "::warning::dedupe find-duplicates produced no result; nothing posted." + exit 0 + fi + if [[ ! -s result/duplicates.txt ]]; then + echo "No duplicate candidates; nothing to post." + exit 0 + fi + + # Extract up to 3 purely-numeric issue ids. comment-on-duplicates.sh + # re-validates these and the base issue (numeric, existing, at most 3) + # before posting. + mapfile -t DUPS < <(grep -oE '^[0-9]+$' result/duplicates.txt | head -n 3) + + if [[ ${#DUPS[@]} -eq 0 ]]; then + echo "No valid numeric duplicate ids; nothing to post." + exit 0 + fi + + ./scripts/comment-on-duplicates.sh \ + --base-issue "$BASE_ISSUE" \ + --potential-duplicates "${DUPS[@]}"