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.

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));

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<string> {
-  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<string> {
+  return firstNonceFrom(0, challenge, deadlineMs);
+}
+
+async function firstNonceFrom(nonce: number, challenge: ServiceChallenge, deadlineMs: number): Promise<string> {
+  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);
 }

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<void> {
     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()) {

server/src/nest/service-identity/service-request-identity.ts

@@ -61,13 +61,20 @@ async function saysRevoked(response: Response): Promise<boolean> {
 async function readUpTo(response: Response, maxChars: number): Promise<string> {
   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<Uint8Array>,
+  decoder: TextDecoder,
+  text: string,
+  maxChars: number,
+): Promise<string> {
+  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);
 }

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);
     }
   });
 

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);

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<number>(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');
   });
 });