From eeae904867a3cce4e14e1486bdc154fb16d5958c Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 29 Oct 2026 09:30:13 +0100 Subject: [PATCH] fix(server): solve the proof of work and read the revocation notice without awaiting in a loop The proof of work and the bounded read of a 403 body recurse instead of looping, the background-work checks compare against null instead of testing a promise, and the tests repeat their requests through oneByOne. The renewal test loads the stored key before its first request, and the panel test picks the last confirm button without Array.prototype.at. --- .../Admin/ServiceIdentityPanel.test.tsx | 3 ++- .../nest/service-identity/service-key.pow.ts | 15 ++++++----- .../service-identity/service-key.service.ts | 4 +-- .../service-request-identity.ts | 25 ++++++++++++------- .../service-identity/service-key.pow.test.ts | 5 ++-- .../service-key.service.test.ts | 25 ++++++++++--------- .../unit/nest/trek-places.client.test.ts | 13 +++++----- 7 files changed, 51 insertions(+), 39 deletions(-) diff --git a/client/src/components/Admin/ServiceIdentityPanel.test.tsx b/client/src/components/Admin/ServiceIdentityPanel.test.tsx index 2e29816a9..41c4a53f7 100644 --- a/client/src/components/Admin/ServiceIdentityPanel.test.tsx +++ b/client/src/components/Admin/ServiceIdentityPanel.test.tsx @@ -97,7 +97,8 @@ describe('ServiceIdentityPanel', () => { expect(screen.getByText('admin.trekApi.quota.regenerateBody')).toBeInTheDocument(); expect(regenerated).toBe(0); - const confirm = screen.getAllByRole('button', { name: 'admin.trekApi.quota.regenerate' }).at(-1); + const regenerateButtons = screen.getAllByRole('button', { name: 'admin.trekApi.quota.regenerate' }); + const confirm = regenerateButtons[regenerateButtons.length - 1]; if (!confirm) throw new Error('no confirm button'); await user.click(confirm); await waitFor(() => expect(regenerated).toBe(1)); diff --git a/server/src/nest/service-identity/service-key.pow.ts b/server/src/nest/service-identity/service-key.pow.ts index 721d6fc13..b24c9680d 100644 --- a/server/src/nest/service-identity/service-key.pow.ts +++ b/server/src/nest/service-identity/service-key.pow.ts @@ -29,10 +29,13 @@ export function leadingZeroBits(hash: Uint8Array): number { } /** The first decimal nonce whose hash has at least `bits` leading zero bits, tried until `deadlineMs`. */ -export async function solvePow(challenge: ServiceChallenge, deadlineMs: number): Promise { - for (let nonce = 0; Date.now() < deadlineMs; nonce++) { - const hash = await powHash(challenge.c, String(nonce), challenge); - if (leadingZeroBits(hash) >= challenge.bits) return String(nonce); - } - throw new Error('the registration challenge expired before it was solved'); +export function solvePow(challenge: ServiceChallenge, deadlineMs: number): Promise { + return firstNonceFrom(0, challenge, deadlineMs); +} + +async function firstNonceFrom(nonce: number, challenge: ServiceChallenge, deadlineMs: number): Promise { + if (Date.now() >= deadlineMs) throw new Error('the registration challenge expired before it was solved'); + const hash = await powHash(challenge.c, String(nonce), challenge); + if (leadingZeroBits(hash) >= challenge.bits) return String(nonce); + return firstNonceFrom(nonce + 1, challenge, deadlineMs); } diff --git a/server/src/nest/service-identity/service-key.service.ts b/server/src/nest/service-identity/service-key.service.ts index 15c2a317a..b5d8f861d 100644 --- a/server/src/nest/service-identity/service-key.service.ts +++ b/server/src/nest/service-identity/service-key.service.ts @@ -154,7 +154,7 @@ export class ServiceKeyService { /** Test seam: resolves once the background work queued so far has run. */ async whenIdle(): Promise { const work = this.work; - if (!work) return; + if (work === null) return; await work; await this.whenIdle(); } @@ -201,7 +201,7 @@ export class ServiceKeyService { } private settleInBackground(): void { - if (this.work || (this.loaded && !this.wantsRegistration() && !this.keyToRenewNow())) return; + if (this.work !== null || (this.loaded && !this.wantsRegistration() && !this.keyToRenewNow())) return; void this.inBackground(async () => { await this.load(); if (this.wantsRegistration()) { diff --git a/server/src/nest/service-identity/service-request-identity.ts b/server/src/nest/service-identity/service-request-identity.ts index 587322164..ca2ee0281 100644 --- a/server/src/nest/service-identity/service-request-identity.ts +++ b/server/src/nest/service-identity/service-request-identity.ts @@ -61,13 +61,20 @@ async function saysRevoked(response: Response): Promise { async function readUpTo(response: Response, maxChars: number): Promise { const reader = response.body?.getReader(); if (!reader) return ''; - const decoder = new TextDecoder(); - let text = ''; - while (text.length <= maxChars) { - const { done, value } = await reader.read(); - if (done) return text; - text += decoder.decode(value, { stream: true }); - } - await reader.cancel(); - return text; + return readOn(reader, new TextDecoder(), '', maxChars); +} + +async function readOn( + reader: ReadableStreamDefaultReader, + decoder: TextDecoder, + text: string, + maxChars: number, +): Promise { + if (text.length > maxChars) { + await reader.cancel(); + return text; + } + const { done, value } = await reader.read(); + if (done) return text; + return readOn(reader, decoder, text + decoder.decode(value, { stream: true }), maxChars); } diff --git a/server/tests/unit/nest/service-identity/service-key.pow.test.ts b/server/tests/unit/nest/service-identity/service-key.pow.test.ts index dd7a377a4..ab3e8f009 100644 --- a/server/tests/unit/nest/service-identity/service-key.pow.test.ts +++ b/server/tests/unit/nest/service-identity/service-key.pow.test.ts @@ -55,8 +55,9 @@ describe('solvePow', () => { const nonce = await solvePow(cheap, Date.now() + 60_000); expect(nonce).toMatch(/^\d+$/); expect(leadingZeroBits(await powHash(cheap.c, nonce, cheap))).toBeGreaterThanOrEqual(cheap.bits); - for (let earlier = 0; earlier < Number(nonce); earlier++) { - expect(leadingZeroBits(await powHash(cheap.c, String(earlier), cheap))).toBeLessThan(cheap.bits); + const earlier = Array.from({ length: Number(nonce) }, (_, n) => powHash(cheap.c, String(n), cheap)); + for (const hash of await Promise.all(earlier)) { + expect(leadingZeroBits(hash)).toBeLessThan(cheap.bits); } }); diff --git a/server/tests/unit/nest/service-identity/service-key.service.test.ts b/server/tests/unit/nest/service-identity/service-key.service.test.ts index c7a847e5e..91cc4e553 100644 --- a/server/tests/unit/nest/service-identity/service-key.service.test.ts +++ b/server/tests/unit/nest/service-identity/service-key.service.test.ts @@ -19,6 +19,7 @@ import { type StoredIdentity, } from '../../../../src/nest/service-identity/service-key.store'; import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { oneByOne } from '../../../helpers/one-by-one'; import { serviceKeyTokenFor } from '../../../helpers/service-keys'; import { createTestModule, type TestModule } from '../../../helpers/test-module'; @@ -220,15 +221,14 @@ describe('when it never registers', () => { expect((await keys.snapshot()).state).toBe('legacy'); }); - it('SIKEYS-011: never with the index switched off, nor with TREK_SERVICES=off', async () => { - for (const setup of [{ places: { enabled: false } }, { config: { mode: 'off' as const } }]) { - const { keys, issuer } = await build(setup); - expect(keys.bearer()).toBeNull(); - expect((await keys.snapshot()).state).toBe('off'); - expect(issuer.challenge).not.toHaveBeenCalled(); - await t?.close(); - t = null; - } + it.each<[string, Setup]>([ + ['the index switched off', { places: { enabled: false } }], + ['TREK_SERVICES=off', { config: { mode: 'off' } }], + ])('SIKEYS-011: never with %s', async (_, setup) => { + const { keys, issuer } = await build(setup); + expect(keys.bearer()).toBeNull(); + expect((await keys.snapshot()).state).toBe('off'); + expect(issuer.challenge).not.toHaveBeenCalled(); }); it('SIKEYS-012: never for a copy of the index the operator runs', async () => { @@ -391,10 +391,10 @@ describe('renewal', () => { await first.keys.renewIfDue(); await first.keys.renewIfDue(); expect(first.store.dropKey).not.toHaveBeenCalled(); - for (let i = 0; i < 2; i++) { + await oneByOne(Array.from({ length: 2 }), () => { vi.setSystemTime(Date.now() + 10 * 60_000); - await first.keys.renewIfDue(); - } + return first.keys.renewIfDue(); + }); expect(first.store.dropKey).toHaveBeenCalledTimes(1); vi.useRealTimers(); await t?.close(); @@ -440,6 +440,7 @@ describe('renewal', () => { it('SIKEYS-036: a due key is renewed on the next request, not only by the daily check', async () => { const store = stored(); const { keys, issuer } = await build({ store }); + await keys.snapshot(); expect(keys.bearer()).toBe(oldKey); await settled(keys); expect(issuer.renew).toHaveBeenCalledWith(oldKey); diff --git a/server/tests/unit/nest/trek-places.client.test.ts b/server/tests/unit/nest/trek-places.client.test.ts index eb425d2cf..4505ffa52 100644 --- a/server/tests/unit/nest/trek-places.client.test.ts +++ b/server/tests/unit/nest/trek-places.client.test.ts @@ -466,14 +466,13 @@ describe('the circuit breaker', () => { // A 401 or 403 says the service is there and judged the key; the identity // falls back to the shared pool on its own. A 429 means the next request // costs the same and gets the same, like a server error. - for (const status of [401, 403]) { - for (let i = 0; i < 6; i++) { - stubFetch({}, status); - await expect(trekPlacesSearch('x')).rejects.toThrow(String(status)); - } - } + const refusals = [401, 403].flatMap((status) => Array(6).fill(status)); + await oneByOne(refusals, (status) => { + stubFetch({}, status); + return expect(trekPlacesSearch('x')).rejects.toThrow(String(status)); + }); stubFetch({}, 429); - for (let i = 0; i < 4; i++) await expect(trekPlacesSearch('x')).rejects.toThrow(/429/); + await oneByOne(Array.from({ length: 4 }), () => expect(trekPlacesSearch('x')).rejects.toThrow(/429/)); await expect(trekPlacesSearch('x')).rejects.toThrow('unreachable, not retrying yet'); }); });