Repository navigation
fix(milestones): create binds sponsorId to the caller, not the DTO - #685
Open
yggdrasilworld wants to merge 3 commits into
Open
yggdrasilworld wants to merge 3 commits into
yggdrasilworld wants to merge 3 commits into
Conversation
create() did sponsorId: dto.sponsorId ?? callerUserId — a client could set an arbitrary sponsorId and own/spoof the milestone's sponsor identity. The money-identity must derive from the authenticated caller, not the request body (MergeFi#40). Now sponsorId is always the caller; dto.sponsorId is ignored. Resolves MergeFi#40 (milestones leg)
|
@yggdrasilworld is attempting to deploy a commit to the chonilius' projects Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
Ready for review — the #40 money-identity bug on milestones: |
…ne specs The stacked merge (172fd0d) left two overlapping invokeOnLockedEscrow calls — the file failed to parse, which also broke the spec suite at collection. Kept the u64-encoded variant (MergeFi#301). milestones.service.spec: synced with upstream drift — fund() takes the callerUserId argument, releasePartial carries the transaction manager as a 5th arg, and the transition error text changed. Added the create() sponsor- binding regression tests for MergeFi#40. 22/22 pass.
The stacked-merge base left files unable to compile or collect:
- escrow.service.ts: overlapping invokeOnLockedEscrow returns (kept u64)
- sponsors.controller.ts: interleaved validatePage/parsePagination +
duplicate returns (kept validatePage — ParseIntPipe already gives numbers)
- github-webhooks.service.spec.ts: missing it()/describe() closers, a
duplicate webhookEventRepo declaration, an uninitialized issueRepo, and
mocks still on the old { where } signature — findIssueByRepoAndNumber is
positional (repoId, number) now
Spec drift synced elsewhere: fund() callerUserId, releasePartial's manager
arg, lowercase role enums, dto-shaped release/splitRelease calls,
checksum-valid fixtures, StrKey stub in the jest mock, and
FundEscrowDto.deadline IsDateString -> IsDate (the @type transform made
IsDateString reject every real deadline — requests with a deadline 400'd).
Author
|
Verification — tests run (real jest, this PR's head)
Also fixed while verifying — this branch's stacked merge left |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What — the #40 money-identity trust bug
milestones.create()didsponsorId: dto.sponsorId ?? callerUserId— a client could set an arbitrarysponsorIdand own/spoof the milestone's sponsor identity. The money-identity must derive from the authenticated caller, not the request body.Fix
sponsorIdis always the caller;dto.sponsorIdis ignored. Pairs with #656 (create-bounty's sponsorId) — this is the same class on the milestones route.Note:
escrow.fund'ssponsorId/funderAddressneed the companion #55 work (that route has no JWT-guard at all) — left for that issue.Resolves #40 (milestones leg)