Skip to content

Fix close-enforcement authorization gap; extract shared helper #4602

Description

@JSONbored

Context

Part of the review-stack architecture audit (parent epic). The audit traced every code path that can
call closePullRequest() and found two populations: the unified planAgentMaintenanceActions planner
(src/settings/agent-actions.ts:587-1244, 9 signal sources, sound design), and 5 hand-written
functions in src/queue/processors.ts that call closePullRequest() directly
, bypassing the planner
entirely (documented as deliberate — procedural-integrity enforcement, not a merit verdict):

  1. closeDraftDodgeAttemptIfBlockedprocessors.ts:12775-12944, close at :12902
  2. recloseDisallowedReopenIfNeeded:12983-13166, close at :13139
  3. closeReviewEvasionSelfCloseIfActive:13205-13435, close at :13339
  4. closeReviewEvasionDraftConversionIfActive:13455-13643, close at :13563
  5. closeRepeatedDraftCyclingIfDetected:13670-13879, close at :13783

The bug

Two of the five are missing the close-autonomy-class check the other three correctly implement:

  • processors.ts:12839-12842closeDraftDodgeAttemptIfBlocked calls
    resolveAgentPermissionReadiness({ autonomy: settings.autonomy, installationPermissions: draftDodgeInstallationPermissions }) without actionClass: "close". Per
    requiredAgentActionPermissions (src/settings/agent-execution.ts:109-121), omitting actionClass
    checks the union of every acting autonomy class's write-permission grant, not specifically close's.
  • processors.ts:6169-6176 — the call site gates only on isAgentConfigured(settings.autonomy)
    (src/settings/autonomy.ts:39-41: true if any class is acting), never on
    resolveAutonomy(settings.autonomy, "close").
  • processors.ts:12983-13166 (recloseDisallowedReopenIfNeeded) has no
    resolveAgentPermissionReadiness call at all, gating only on the same loose
    isAgentConfigured(reopenSettings.autonomy) at :13042.

Contrast with the three correct siblings, which explicitly do
const closeAutonomy = resolveAutonomy(settings.autonomy, "close"); if (closeAutonomy !== "auto") { deny }
at :13231-13232, :13483-13484, :13701-13702, and correctly pass actionClass: "close" at
:13283, :13531, :13749.

Impact

A repo that enables any other autonomy class (e.g. assign: "auto") but deliberately leaves close
unconfigured (documented deny-by-default) can still have PRs auto-closed by the draft-dodge and
reopen-reclose paths. This is a live authorization bypass, not a hypothetical.

Root cause

The 5 close-enforcement paths duplicate ~150-230 lines of near-identical scaffolding each (~980 lines
total: lock claim/release, mode resolution, permission readiness, live freshness re-check, the close
call, the audit-event write) instead of sharing one helper — the classic "same check hand-duplicated N
times, N-2 copies drift" pattern.

Fix

  1. Add actionClass: "close" to the draft-dodge readiness call and an explicit
    closeAutonomy !== "auto" deny to both draft-dodge and reopen-reclose (the immediate security fix).
  2. Extract one enforceDeterministicClose(env, { repoFullName, pr, settings, deliveryId, eventType, lockLabel, isStillJustified, closeComment }) helper owning lock claim/release, mode resolution, the
    close-autonomy check, scoped permission readiness, live freshness re-check, the close call, and the
    audit-event write — leaving each of the 5 call sites only its ~10-30 lines of actually-distinct
    justification logic. Estimated 600-700 line reduction, and makes the omission structurally impossible
    to repeat a sixth time.

Acceptance criteria

  • closeDraftDodgeAttemptIfBlocked and recloseDisallowedReopenIfNeeded correctly deny when
    close autonomy is not "auto", matching the 3 correct siblings.
  • All 5 close paths route through one shared enforceDeterministicClose helper.
  • Regression test: a repo with assign: "auto" but close unconfigured must NOT have a PR
    auto-closed via any of the 5 paths.
  • Full branch coverage on the new helper and all 5 call sites.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

Status
In Progress

Relationships

None yet

Development

No branches or pull requests

Issue actions