Skip to content

fix(reopen): honor collaborator closes and paginate issue-events for last-closer - #1185

Merged
JSONbored merged 5 commits into
mainfrom
codex/propose-fix-for-reopen-guard-vulnerability
Jun 24, 2026
Merged

fix(reopen): honor collaborator closes and paginate issue-events for last-closer#1185
JSONbored merged 5 commits into
mainfrom
codex/propose-fix-for-reopen-guard-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent contributor reopens from bypassing the one-shot close policy when a real repo maintainer (admin/maintain/write) closed the PR.\
  • Avoid missing the true last closer when the issue-events timeline is paginated and the most recent closed event appears beyond the first page.\

Description

  • Walk GET /repos/{owner}/{repo}/issues/{issue_number}/events pages (per_page=100) in getLastCloserLogin so the latest closed event is discovered across all pages (src/github/pr-actions.ts).\
  • Replace the brittle owner/admin-only maintainer check with a live collaborator-permission lookup via getRepositoryCollaboratorPermission, treating admin, maintain, and write as maintainer authority when deciding to allow a reopen (src/queue/processors.ts).\
  • Add regression tests covering paginated issue-events and the reopen-reclose flow when a write collaborator closed the PR (test/unit/github-pr-actions.test.ts, test/unit/queue.test.ts).\

Testing

  • Ran targeted unit tests with npx vitest run test/unit/github-pr-actions.test.ts test/unit/queue.test.ts -t "one-shot reopen prevention|paginated issue events", and the new/regression tests passed.\
  • Verified npm run typecheck and git diff --check succeeded.\
  • Attempted npm run test:ci (full local gate); execution reached unrelated existing timeouts and coverage post-processing failed with TypeError: jsTokens is not a function, so full CI was not completed here.\
  • Attempted npm audit --audit-level=moderate; the npm audit endpoint returned 403 Forbidden so the audit could not be completed in this environment.

Codex Task

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.80%. Comparing base (46d09b3) to head (26fa374).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1185      +/-   ##
==========================================
+ Coverage   94.69%   94.80%   +0.10%     
==========================================
  Files         157      157              
  Lines       19077    19083       +6     
  Branches     6906     6909       +3     
==========================================
+ Hits        18065    18091      +26     
+ Misses        424      399      -25     
- Partials      588      593       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jun 24, 2026
…on branches

Branch gaps in the new pagination loop and collaborator-permission check:
- getLastCloserLogin: add catch-path test (request throws → null) and
  null-actor test (actor: null on closed event → null via ??)
- maybeRecloseDisallowedReopen: add fast-path admin test (ADMIN_GITHUB_LOGINS
  hit → no reclose), null-closer test (empty events → unknown closer → allow),
  bot-closer test (gittensory[bot] closed → reclose), self-close test
  (contributor closed → non-maintainer closer → allow), and permission-API-
  error test (fetch throws → null → non-maintainer → allow)
@JSONbored
JSONbored merged commit a21dc3b into main Jun 24, 2026
17 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-reopen-guard-vulnerability branch June 24, 2026 07:46

@superagent-security superagent-security Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superagent found 1 security concern(s).

Comment thread src/github/pr-actions.ts
const closes = events.filter((entry) => entry.event === "closed");
return closes.length > 0 ? (closes[closes.length - 1]?.actor?.login ?? null) : null;
let lastCloser: string | null = null;
for (let page = 1; ; page += 1) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Unbounded pagination loop in issue-events fetch lacks page limit

Unbounded loop fetches every issue-events page with no maximum limit.

Add a maximum page limit (e.g., 10 pages) to prevent rate limit exhaustion.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/github/pr-actions.ts">
<violation number="1" location="src/github/pr-actions.ts:123">
<priority>P2</priority>
<title>Unbounded pagination loop in issue-events fetch lacks page limit</title>
<evidence>The new code introduces a `for (let page = 1; ; page += 1)` loop with no maximum page limit to walk all issue events pages. For PRs with thousands of events, this will make an excessive number of authenticated GitHub API requests, risking rate limit exhaustion and queue processor stalls.</evidence>
<recommendation>Cap pagination at a reasonable maximum (e.g., `page &lt;= 10`) and return `lastCloser` early if the limit is reached. Document the cap in a code comment.</recommendation>
</violation>
</file>

@superagent-security superagent-security Bot added the pr:flagged PR flagged for review by security analysis. label Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:flagged PR flagged for review by security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant