fix: use conditional Blobs writes for NPS relay rate, nonce and install records - #152
Merged
Merged
Conversation
…ll records Bump @netlify/blobs to ^10.7.13 (conditional writes landed in 10.0.0 and reach setJSON in 10.7.12) and update every bounded record in the relay — per-IP and per-install submission limits, per-IP and per-day registration caps, and the per-install nonce list — through a compare-and-swap loop guarded by the record's ETag. Install records are written create-only so an install id is bound to exactly one key even when registrations race. Persistent contention fails closed with 429. Adds MemoryStore ETag/conditional-write emulation and concurrency tests that fire bursts of requests and assert each cap holds. Closes #151 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: sec-check <sec-check@hive.kubestellar.io> Co-authored-by: clubanderson <407614+clubanderson@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for hivecommons-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security Fix
The NPS relay (
netlify/nps-relay/relay.ts) enforced its per-IP / per-install submission limits, per-IP / per-day registration caps, the nonce replay guard and the "one install id, one key" rule with plain read → check →setJSONsequences on Netlify Blobs. Concurrent function invocations all observe the same pre-write record, so every one of those bounds held only for sequential callers.This PR:
@netlify/blobs^8.2.0→^10.7.13(conditional writes landed in 10.0.0 and reachsetJSONin 10.7.12; the Node floor is unchanged, unlike 11.x). The relay only usesget/setJSON/delete/list, which the 9.0.0/10.0.0 breaking changes do not touch.RelayStorewithgetWithMetadata(ETag) and conditionalsetJSON(onlyIfMatch/onlyIfNew), and routes every bounded record through acompareAndSwaploop (CAS_MAX_ATTEMPTS = 4) that re-reads on a lost race and fails closed (429) under persistent contention.onlyIfNew), so two registrations racing for the same install id resolve to exactly one bound key; the loser gets the same 200/409 answers as today.MemoryStoreETags and conditional writes (and makes each call yield, so concurrent handlers interleave) and adds six concurrency tests covering each cap, the replayed-nonce race, the install-id race and the fail-closed path.Validation:
vitest run— 78 files / 750 tests pass (relay: 37);tsc --noEmitclean.Closes #151
Filed by sec-check agent (ACMM L6 — full mode)
— hive: agent=sec-check backend=copilot model=claude-fable-5.1 copilot=1.0.88