Skip to content

fix(milestones): reject attaching an issue from a different repository - #241

Merged
chonilius merged 1 commit into
MergeFi:mainfrom
oluwagbemigaphilip:fix/mergifi-59-18-26-57
Aug 27, 2026
Merged

fix(milestones): reject attaching an issue from a different repository#241
chonilius merged 1 commit into
MergeFi:mainfrom
oluwagbemigaphilip:fix/mergifi-59-18-26-57

Conversation

@oluwagbemigaphilip

@oluwagbemigaphilip oluwagbemigaphilip commented Aug 27, 2026

Copy link
Copy Markdown

What changed

MilestonesService.addIssue now rejects attaching an issue whose repositoryId doesn't match the target milestone's repositoryId, throwing a BadRequestException naming both repository IDs.

Why

resolveIssue splits a milestone's remaining budget evenly across whatever issues are currently attached, with no per-issue repository check. Since addIssue never validated the issue and milestone belonged to the same repository, a cross-repository issue (copy-pasted ID, client-side picker bug, wrong UUID) could get attached and silently dilute a sponsor's repository-scoped budget toward unrelated work.

Note on scope

The issue also flagged POST /milestones/:id/issues/:issueId as unguarded. In the current code that route already carries @UseGuards(JwtAuthGuard, RolesGuard) + @Roles(UserRole.MAINTAINER) — verified in milestones.controller.ts. That part doesn't apply as filed, so it's not touched here.

How verified

  • Added a test asserting cross-repo attachment is rejected with the exact error message, and issueRepo.save is never called.
  • Updated the 3 existing same-repository attachment tests to set matching repositoryIds explicitly.
  • npx jest milestones.service.spec.ts — 19/19 passing.
  • npx tsc --noEmit — no new errors introduced (pre-existing unrelated errors in other files confirmed present on main before this change).

Batch context

closes #18
closes #26
closes #57
Closes #59

MilestonesService.addIssue reassigned an issue's milestoneId with no
check that the issue actually belongs to the milestone's own
repository. Since resolveIssue splits the milestone's budget evenly
across whatever's attached with no per-issue repository check either,
a cross-repository issue attached via a copy-pasted ID or a client
bug would silently dilute a sponsor's repository-scoped budget.

Rejects with a BadRequestException naming both repository IDs when
issue.repositoryId !== milestone.repositoryId. Note: the controller's
POST /milestones/:id/issues/:issueId route already carries
JwtAuthGuard + RolesGuard(MAINTAINER) in current code, so the auth-gap
concern this issue also raised does not apply as filed.

Closes MergeFi#59
@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the chonilius' projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Aug 27, 2026

Copy link
Copy Markdown

@oluwagbemigaphilip Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@chonilius
chonilius merged commit cad0966 into MergeFi:main Aug 27, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment