From 4983d5aae4cc005c4765c4e03a8cf8577ab47ec2 Mon Sep 17 00:00:00 2001 From: Yggdrasil Date: Wed, 14 Oct 2026 01:20:44 -0400 Subject: [PATCH 1/3] test: spec the ttl-cache, users.mapper, and github-webhook-payload utils MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ttl-cache: TTL expiry, single-flight coalescing, invalidate - users.mapper: the public-shape mapping — strips email + githubAccount (the #359 leak the mapper exists to prevent) - github-webhook-payload: the PR/issues payload validators — valid accept, malformed/missing/wrong-typed reject Resolves #598, #599, #595 --- src/analytics/ttl-cache.spec.ts | 49 ++++++++++++ .../github-webhook-payload.util.spec.ts | 76 +++++++++++++++++++ src/users/users.mapper.spec.ts | 51 +++++++++++++ 3 files changed, 176 insertions(+) create mode 100644 src/analytics/ttl-cache.spec.ts create mode 100644 src/github/github-webhook-payload.util.spec.ts create mode 100644 src/users/users.mapper.spec.ts diff --git a/src/analytics/ttl-cache.spec.ts b/src/analytics/ttl-cache.spec.ts new file mode 100644 index 00000000..9266af2a --- /dev/null +++ b/src/analytics/ttl-cache.spec.ts @@ -0,0 +1,49 @@ +import { TtlCache } from './ttl-cache'; + +describe('TtlCache', () => { + it('returns the loaded value on a miss', async () => { + const cache = new TtlCache(60_000); + const loader = jest.fn().mockResolvedValue(42); + expect(await cache.getOrLoad(loader)).toBe(42); + expect(loader).toHaveBeenCalledTimes(1); + }); + + it('serves the cached value within the TTL without reloading', async () => { + const cache = new TtlCache(60_000); + const loader = jest.fn().mockResolvedValue(7); + await cache.getOrLoad(loader); + await cache.getOrLoad(loader); + expect(loader).toHaveBeenCalledTimes(1); + }); + + it('coalesces concurrent misses into one loader call (single-flight)', async () => { + const cache = new TtlCache(60_000); + let resolve: (v: number) => void = () => {}; + const loader = jest.fn(() => new Promise((r) => (resolve = r))); + const p1 = cache.getOrLoad(loader); + const p2 = cache.getOrLoad(loader); + resolve(5); + expect(await Promise.all([p1, p2])).toEqual([5, 5]); + expect(loader).toHaveBeenCalledTimes(1); + }); + + it('reloads after the TTL expires', async () => { + const cache = new TtlCache(1); // 1ms TTL + const loader = jest + .fn() + .mockResolvedValue(1) + .mockResolvedValueOnce(1) + .mockResolvedValue(2); + await cache.getOrLoad(loader); + await new Promise((r) => setTimeout(r, 5)); + expect(await cache.getOrLoad(loader)).toBe(2); + }); + + it('invalidate() forces a reload', async () => { + const cache = new TtlCache(60_000); + const loader = jest.fn().mockResolvedValueOnce(1).mockResolvedValue(2); + await cache.getOrLoad(loader); + cache.invalidate(); + expect(await cache.getOrLoad(loader)).toBe(2); + }); +}); diff --git a/src/github/github-webhook-payload.util.spec.ts b/src/github/github-webhook-payload.util.spec.ts new file mode 100644 index 00000000..65360aa6 --- /dev/null +++ b/src/github/github-webhook-payload.util.spec.ts @@ -0,0 +1,76 @@ +import { + validatePullRequestPayload, + validateIssuesEventPayload, + WebhookPayloadValidationError, +} from './github-webhook-payload.util'; + +const validPR = { + action: 'closed', + number: 42, + pull_request: { + html_url: 'https://github.com/o/r/pull/42', + number: 42, + merged: true, + body: 'fixes', + title: 'the pr', + }, + repository: { id: 1, full_name: 'o/r', html_url: 'https://github.com/o/r' }, +}; + +const validIssue = { + action: 'opened', + issue: { + number: 7, + title: 'the issue', + state: 'open', + updated_at: '2024-01-01T00:00:00Z', + }, + repository: { id: 1, full_name: 'o/r', html_url: 'https://github.com/o/r' }, +}; + +describe('github-webhook-payload.util', () => { + describe('validatePullRequestPayload', () => { + it('accepts a valid pull_request payload', () => { + const out = validatePullRequestPayload(validPR); + expect(out.number).toBe(42); + expect(out.pull_request.merged).toBe(true); + }); + + it('rejects a payload missing pull_request', () => { + expect(() => + validatePullRequestPayload({ action: 'closed', number: 1 }), + ).toThrow(WebhookPayloadValidationError); + }); + + it('rejects a non-boolean merged', () => { + expect(() => + validatePullRequestPayload({ + ...validPR, + pull_request: { ...validPR.pull_request, merged: 'yes' }, + }), + ).toThrow(WebhookPayloadValidationError); + }); + }); + + describe('validateIssuesEventPayload', () => { + it('accepts a valid issues payload', () => { + const out = validateIssuesEventPayload(validIssue); + expect(out.issue.number).toBe(7); + }); + + it('rejects a payload missing the issue record', () => { + expect(() => validateIssuesEventPayload({ action: 'opened' })).toThrow( + WebhookPayloadValidationError, + ); + }); + + it('rejects a non-string issue.state', () => { + expect(() => + validateIssuesEventPayload({ + ...validIssue, + issue: { ...validIssue.issue, state: 5 }, + }), + ).toThrow(WebhookPayloadValidationError); + }); + }); +}); diff --git a/src/users/users.mapper.spec.ts b/src/users/users.mapper.spec.ts new file mode 100644 index 00000000..fb32f594 --- /dev/null +++ b/src/users/users.mapper.spec.ts @@ -0,0 +1,51 @@ +import { toPublicUser } from './users.mapper'; +import { User } from '../common/entities/user.entity'; +import { UserRole } from '../common/enums'; + +const makeUser = (over: Partial = {}): User => + ({ + id: 'u1', + username: 'dev', + displayName: 'Dev', + avatarUrl: 'https://x/y.png', + roles: [UserRole.CONTRIBUTOR], + stellarAddress: 'GADDR', + createdAt: new Date('2024-01-01'), + // the private fields that must NOT leak + email: 'dev@secret.com', + githubAccount: { id: 'g1', accessToken: 'token' }, + ...over, + }) as unknown as User; + +describe('toPublicUser', () => { + it('maps the public fields through', () => { + const dto = toPublicUser(makeUser()); + expect(dto.id).toBe('u1'); + expect(dto.username).toBe('dev'); + expect(dto.stellarAddress).toBe('GADDR'); + expect(dto.createdAt).toEqual(new Date('2024-01-01')); + }); + + it('strips the private fields (email, githubAccount)', () => { + const dto = toPublicUser(makeUser()) as any; + // #359 — the raw entity leaks email + the nested githubAccount; the public + // DTO must not carry them. + expect(dto.email).toBeUndefined(); + expect(dto.githubAccount).toBeUndefined(); + }); + + it('only exposes the declared public shape', () => { + const dto = toPublicUser(makeUser()); + expect(Object.keys(dto).sort()).toEqual( + [ + 'id', + 'username', + 'displayName', + 'avatarUrl', + 'roles', + 'stellarAddress', + 'createdAt', + ].sort(), + ); + }); +}); From 8fca9e751e87c7f12ad7dc30c705cfe7b8ccdaf5 Mon Sep 17 00:00:00 2001 From: yggdrasilworld Date: Sun, 18 Oct 2026 21:24:33 -0400 Subject: [PATCH 2/3] test(github-webhooks): complete the valid issues-event fixture The fixture was missing issue.id + issue.html_url, both required by the validator (and present in real GitHub issues events). --- src/github/github-webhook-payload.util.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/github/github-webhook-payload.util.spec.ts b/src/github/github-webhook-payload.util.spec.ts index 65360aa6..471d25c8 100644 --- a/src/github/github-webhook-payload.util.spec.ts +++ b/src/github/github-webhook-payload.util.spec.ts @@ -20,10 +20,12 @@ const validPR = { const validIssue = { action: 'opened', issue: { + id: 987654, number: 7, title: 'the issue', state: 'open', updated_at: '2024-01-01T00:00:00Z', + html_url: 'https://github.com/o/r/issues/7', }, repository: { id: 1, full_name: 'o/r', html_url: 'https://github.com/o/r' }, }; From e8d84fcef7d9724ac261ed660963176ffebbe2dd Mon Sep 17 00:00:00 2001 From: yggdrasilworld Date: Sun, 18 Oct 2026 21:50:08 -0400 Subject: [PATCH 3/3] fix: repair merge glue that left the branch uncompilable The shared base carried half-written merge artifacts: an orphaned invokeOnLockedEscrow block in escrow.service.ts, duplicated returns plus a dead parsePagination in sponsors.controller.ts, and missing describe/it closers in github-webhooks.service.spec.ts. Aligned each with the intended form so the branch typechecks and its specs load. --- src/escrow/escrow.service.ts | 10 ++-------- src/github/github-webhooks.service.spec.ts | 3 +++ src/sponsors/sponsors.controller.ts | 21 --------------------- 3 files changed, 5 insertions(+), 29 deletions(-) diff --git a/src/escrow/escrow.service.ts b/src/escrow/escrow.service.ts index 6d2dd289..c252993f 100644 --- a/src/escrow/escrow.service.ts +++ b/src/escrow/escrow.service.ts @@ -584,20 +584,14 @@ export class EscrowService { recipients: Array<[string, number]>, manager?: EntityManager, ): Promise { - 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, diff --git a/src/github/github-webhooks.service.spec.ts b/src/github/github-webhooks.service.spec.ts index 45bb6fe8..a35ac3be 100644 --- a/src/github/github-webhooks.service.spec.ts +++ b/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. diff --git a/src/sponsors/sponsors.controller.ts b/src/sponsors/sponsors.controller.ts index b7a48cca..c097bf1e 100644 --- a/src/sponsors/sponsors.controller.ts +++ b/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)); } }