fix(users): self-check on PATCH stellar-address + controller spec

src/escrow/escrow.service.ts

@@ -584,20 +584,14 @@ export class EscrowService {
     recipients: Array<[string, number]>,
     manager?: EntityManager,
   ): Promise<ContractInvocationResult> {
-    return this.invokeOnLockedEscrow(escrow, operation, () =>
-      this.soroban.invoke(
-        'release',
-        // `release(issue_id: u64, recipients)` — u64-typed on-chain (#301).
-        [u64(this.onChainKeyFor(escrow)), recipients],
-        this.contractOpts(escrow),
-      ),
     return this.invokeOnLockedEscrow(
       escrow,
       operation,
       () =>
         this.soroban.invoke(
           'release',
-          [this.onChainKeyFor(escrow), recipients],
+          // `release(issue_id: u64, recipients)` — u64-typed on-chain (#301).
+          [u64(this.onChainKeyFor(escrow)), recipients],
           this.contractOpts(escrow),
         ),
       manager,

src/github/github-webhooks.service.spec.ts

@@ -869,6 +869,9 @@ describe('GithubWebhooksService', () => {
 
       expect(bountiesService.markInReview).not.toHaveBeenCalled();
       expect(bountiesService.markMergedAndRelease).not.toHaveBeenCalled();
+    });
+  });
+
   // #308: a redelivered X-GitHub-Delivery used to hit the unique constraint
   // on webhook_events.deliveryId and escape handleEvent as a 500, so every
   // redelivery of that event failed forever.

src/sponsors/sponsors.controller.ts

@@ -26,25 +26,6 @@ function validatePage(limit?: number, offset?: number) {
     throw new BadRequestException('offset must be >= 0');
   }
   return { limit, offset };
-/**
- * Parse optional limit/offset query params, rejecting NaN, negatives,
- * non-integers, and limits above MAX_PAGE_LIMIT with a 400 (#280).
- */
-function parsePagination(limit?: string, offset?: string) {
-  const parse = (name: string, raw: string | undefined, min: number, max: number) => {
-    if (raw === undefined || raw === '') return undefined;
-    const n = Number(raw);
-    if (!Number.isInteger(n) || n < min || n > max) {
-      throw new BadRequestException(
-        `${name} must be an integer between ${min} and ${max}`,
-      );
-    }
-    return n;
-  };
-  return {
-    limit: parse('limit', limit, 1, MAX_PAGE_LIMIT),
-    offset: parse('offset', offset, 0, Number.MAX_SAFE_INTEGER),
-  };
 }
 
 @ApiTags('sponsors')
@@ -73,7 +54,6 @@ export class SponsorsController {
   ) {
     this.assertOwnsSponsor(user, id);
     return this.sponsorsService.dashboard(id, validatePage(limit, offset));
-    return this.sponsorsService.dashboard(id, parsePagination(limit, offset));
   }
 
   @ApiBearerAuth()
@@ -91,6 +71,5 @@ export class SponsorsController {
       id,
       validatePage(limit, offset),
     );
-    return this.sponsorsService.milestoneProgress(id, parsePagination(limit, offset));
   }
 }

src/users/users.controller.spec.ts

@@ -0,0 +1,66 @@
+import { ForbiddenException } from '@nestjs/common';
+import { UsersController } from './users.controller';
+import { UsersService } from './users.service';
+import { Reflector } from '@nestjs/core';
+import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
+import { GUARDS_METADATA } from '@nestjs/common/constants';
+
+describe('UsersController (#597)', () => {
+  const usersService = {
+    list: jest.fn(),
+    findById: jest.fn(),
+    setStellarAddress: jest.fn(),
+  } as unknown as UsersService;
+  const controller = new UsersController(usersService);
+
+  it('list delegates to usersService.list', async () => {
+    usersService.list = jest.fn().mockResolvedValue([{ id: 'u1' }]);
+    await expect(controller.list()).resolves.toEqual([{ id: 'u1' }]);
+    expect(usersService.list).toHaveBeenCalled();
+  });
+
+  it('findOne delegates with the parsed id', async () => {
+    usersService.findById = jest.fn().mockResolvedValue({ id: 'u1' });
+    await controller.findOne('u1');
+    expect(usersService.findById).toHaveBeenCalledWith('u1');
+  });
+
+  it('setStellarAddress delegates for the authenticated self', async () => {
+    usersService.setStellarAddress = jest
+      .fn()
+      .mockResolvedValue({ stellarAddress: 'G...' });
+    const req = { user: { userId: 'u1' } } as never;
+    await controller.setStellarAddress(
+      'u1',
+      { stellarAddress: 'GABC' } as never,
+      req,
+    );
+    expect(usersService.setStellarAddress).toHaveBeenCalledWith('u1', 'GABC');
+  });
+
+  // The self-only check (#39) — anyone setting ANOTHER user's stellar-address
+  // (the on-chain payout recipient) is a payout-hijack. Must 403.
+  it("setStellarAddress rejects setting another user's address", async () => {
+    usersService.setStellarAddress = jest.fn();
+    const req = { user: { userId: 'attacker' } } as never;
+    expect(() =>
+      controller.setStellarAddress(
+        'victim-id',
+        { stellarAddress: 'GABC' } as never,
+        req,
+      ),
+    ).toThrow(ForbiddenException);
+    expect(usersService.setStellarAddress).not.toHaveBeenCalled();
+  });
+
+  it('all routes are behind JwtAuthGuard', () => {
+    const reflector = new Reflector();
+    for (const method of ['list', 'findOne', 'setStellarAddress'] as const) {
+      const guards = reflector.get(
+        GUARDS_METADATA,
+        UsersController.prototype[method],
+      );
+      expect(guards).toContain(JwtAuthGuard);
+    }
+  });
+});

src/users/users.controller.ts

@@ -1,12 +1,15 @@
 import {
   Body,
   Controller,
+  ForbiddenException,
   Get,
   Param,
   ParseUUIDPipe,
   Patch,
+  Req,
   UseGuards,
 } from '@nestjs/common';
+import { Request } from 'express';
 import { ApiBearerAuth, ApiProperty, ApiTags } from '@nestjs/swagger';
 import { UsersService } from './users.service';
 import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
@@ -48,7 +51,15 @@ export class UsersController {
   setStellarAddress(
     @Param('id', new ParseUUIDPipe()) id: string,
     @Body() dto: SetStellarAddressDto,
+    @Req() req: Request,
   ) {
+    // Self-only (#39): the URL :id must be the authenticated caller — the
+    // stellar-address is the on-chain payout recipient, so letting anyone set
+    // anyone's address would misdirect escrow releases. Reject non-self.
+    const callerId = (req.user as { userId?: string }).userId;
+    if (callerId !== id) {
+      throw new ForbiddenException('Can only set your own stellar address');
+    }
     return this.usersService.setStellarAddress(id, dto.stellarAddress);
   }
 }