From 7ac9aea797cdebc273aeb242f2f21e809d904c63 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 22:05:08 +0000 Subject: [PATCH] Trust only our own comments for the state marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_state_marker accepted a marker from any comment, so anyone able to comment on a public repo could plant one; if its base matched, the resume would merge an attacker-chosen commit (fork-pushed objects are reachable by hash) into the branch and push it with the action's token. A quote-reply of an old conflict comment could likewise resurrect a stale marker, since HTML comments survive quoting and the newest marker wins. Filter the comments to viewerDidAuthor, i.e. those posted with the same token the action runs under. Also reject markers with missing fields instead of passing empty values to git: a marker missing squash= used to crash the run on update-ref and strand the PR under the label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JHvKryT4QUpHYdNq9YEQxX --- tests/test_conflict_resolution_resume.sh | 21 ++++++++++++++++++++- update-pr-stack.sh | 18 ++++++++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index 205b101..b7c1c88 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -33,7 +33,11 @@ echo "gh $*" >> "$CALLS" if [[ "$1 $2" == "pr view" ]]; then case "$*" in *--json\ labels*) printf '%s\n' "${MOCK_LABELS:-}";; - *--json\ comments*) cat "${MOCK_COMMENTS_FILE:-/dev/null}";; + *--json\ comments*) + # The comments file stands for our own comments only, so the query + # must restrict itself to those. + [[ "$*" == *viewerDidAuthor* ]] || { echo "comments query must filter by viewerDidAuthor" >&2; exit 1; } + cat "${MOCK_COMMENTS_FILE:-/dev/null}";; *) echo "unhandled pr view: $*" >&2; exit 1;; esac elif [[ "$1 $2" == "pr comment" ]]; then @@ -167,5 +171,20 @@ grep -q -- "--base" "$CALLS" && fail "D: base must NOT be edited" [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "D: child was pushed" ok "D: missing target detected, no branch mutation, label removed" +# --------------------------------------------------------------------------- +echo "### Scenario E: malformed state marker -> no mutation" +setup_repo +MOCK_LABELS="autorestack-needs-conflict-resolution" +PR_BASE="parent" +MOCK_COMMENTS_FILE="$WORK/comments.txt" +{ echo "### conflict"; echo; echo ''; } > "$MOCK_COMMENTS_FILE" +run_resume + +grep -q "remove-label autorestack-needs-conflict-resolution" "$CALLS" || fail "E: label not removed" +grep -q "gh pr comment" "$CALLS" || fail "E: no explanatory comment posted" +grep -q -- "--base" "$CALLS" && fail "E: base must NOT be edited" +[[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "E: child was pushed" +ok "E: malformed marker handled, no branch mutation, label removed" + echo echo "All conflict-resume tests passed 🎉 ($PASS scenarios)" diff --git a/update-pr-stack.sh b/update-pr-stack.sh index 504580b..2596fd8 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -39,13 +39,17 @@ format_state_marker() { "$STATE_MARKER_PREFIX" "$1" "$2" "$3" } -# Echoes the most recent state-marker line found in our PR comments, or nothing. -# A failed comments fetch aborts the run: treating it as "no marker" would make -# the caller abandon the resume and drop the conflict label for good. +# Echoes the most recent state-marker line found in PR comments, or nothing. +# Only comments posted with our own token count (viewerDidAuthor): anyone can +# comment a marker, and acting on a forged one would merge and push an +# attacker-chosen commit. A failed comments fetch aborts the run: treating it +# as "no marker" would make the caller abandon the resume and drop the +# conflict label for good. read_state_marker() { local PR_NUMBER="$1" local BODIES - if ! BODIES=$(gh pr view "$PR_NUMBER" --json comments --jq '.comments[].body'); then + if ! BODIES=$(gh pr view "$PR_NUMBER" --json comments \ + --jq '.comments[] | select(.viewerDidAuthor) | .body'); then echo "Error: could not read comments of PR #$PR_NUMBER" >&2 exit 1 fi @@ -252,6 +256,12 @@ continue_after_resolution() { read -r OLD_BASE NEW_TARGET SQUASH_HASH < <(parse_state_marker "$MARKER") echo "Recorded state: base=$OLD_BASE target=$NEW_TARGET squash=$SQUASH_HASH" + if [[ -z "$OLD_BASE" || -z "$NEW_TARGET" || -z "$SQUASH_HASH" ]]; then + echo "âš ī¸ State marker on $PR_BRANCH is malformed; cannot resume safely. Removing the label." + abandon_resume "$PR_NUMBER" "â„šī¸ autorestack found a malformed state marker on this PR, so it will not update the stack automatically. If this PR still needs its base updated, update its base manually." + return + fi + # The PR was left based on the merged parent branch. If the payload shows a # different base, a human retargeted the PR; the recorded target is stale, # so step back before any mutation.