diff --git a/changelog.d/fixed-557-nps-relay-register-ip-slot.md b/changelog.d/fixed-557-nps-relay-register-ip-slot.md new file mode 100644 index 0000000..9b0b1cc --- /dev/null +++ b/changelog.d/fixed-557-nps-relay-register-ip-slot.md @@ -0,0 +1,2 @@ +- The NPS relay no longer consumes the per-IP registration slot when the daily + registration cap refuses a registration (hivecommons/docs#557). diff --git a/netlify/nps-relay/relay.test.ts b/netlify/nps-relay/relay.test.ts index 5eacb18..b0fa620 100644 --- a/netlify/nps-relay/relay.test.ts +++ b/netlify/nps-relay/relay.test.ts @@ -37,6 +37,7 @@ import { PURPOSE_SUBMIT, RATE_WINDOW_MS, REGISTER_DAY_PREFIX, + REGISTER_IP_RATE_PREFIX, REGISTER_PATH, RELAY_BASE_PATH, SIGNATURE_MAX_SKEW_MS, @@ -488,6 +489,20 @@ describe("POST /api/nps/register", () => { // An already-registered install can still re-register idempotently. expect((await handleRelayRequest(register(), deps())).status).toBe(200); }); + + it("releases the per-IP slot when the day cap refuses the registration", async () => { + const dayKey = `${REGISTER_DAY_PREFIX}${new Date(now).toISOString().slice(0, 10)}`; + store.data.set(dayKey, { count: MAX_REGISTRATIONS_PER_DAY }); + const res = await handleRelayRequest(register(), deps()); + expect(res.status).toBe(429); + expect(store.data.has(`${INSTALL_PREFIX}${hive.installId}`)).toBe(false); + const ipRecords = [...store.data.entries()].filter(([k]) => + k.startsWith(REGISTER_IP_RATE_PREFIX) + ); + for (const [, rec] of ipRecords) { + expect((rec as { times: number[] }).times).toEqual([]); + } + }); }); describe("POST /api/nps (submit)", () => { diff --git a/netlify/nps-relay/relay.ts b/netlify/nps-relay/relay.ts index 3b50301..57bcf1c 100644 --- a/netlify/nps-relay/relay.ts +++ b/netlify/nps-relay/relay.ts @@ -743,10 +743,13 @@ async function handleRegister( } // Reserve the per-IP and per-day slots before creating the install, so a - // limit that is hit never leaves a registration behind. + // limit that is hit never leaves a registration behind. The two writes are + // not atomic together: if the day cap refuses, the IP slot is released so a + // rejected registration does not consume the independent IP quota. + const ipKey = `${REGISTER_IP_RATE_PREFIX}${ipHash(deps)}`; const ipDenied = await reserveSlot( store, - `${REGISTER_IP_RATE_PREFIX}${ipHash(deps)}`, + ipKey, MAX_REGISTRATIONS_PER_IP_PER_WINDOW, now ); @@ -760,7 +763,10 @@ async function handleRegister( if (count >= MAX_REGISTRATIONS_PER_DAY) return { reject: RATE_LIMITED() }; return { next: { count: count + 1 } satisfies DayCounter }; }); - if (dayDenied) return dayDenied; + if (dayDenied) { + await releaseSlot(store, ipKey, now); + return dayDenied; + } const record: InstallRecord = { public_key: reg.public_key,