Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.d/fixed-557-nps-relay-register-ip-slot.md
Original file line number Diff line number Diff line change
@@ -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).
15 changes: 15 additions & 0 deletions netlify/nps-relay/relay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)", () => {
Expand Down
12 changes: 9 additions & 3 deletions netlify/nps-relay/relay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand All @@ -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,
Expand Down
Loading