fix: split rate-limit buckets, guard cross-tab network change, enforce error discriminant uniqueness - #1178
Merged
Ejirowebfi merged 1 commit intoAug 28, 2026
Conversation
…e error discriminant uniqueness Closes Favourorg#1162 Closes Favourorg#1163 Closes Favourorg#1164 ## Favourorg#1162 — Rate-limit bucket shared between challenge issuance and authenticated actions Challenge issuance (GET /api/auth/challenge) is now limited by **requester IP** via a new isChallengeRateLimited(ip) function, not by the target Stellar address. Authenticated actions (upload-file, upload-json, unpin) continue to use an address-keyed bucket via the new isActionRateLimited(address) function. The two bucket namespaces (ratelimit:challenge:<ip> vs ratelimit:action:<addr>) are fully independent in both the Vercel KV and in-memory paths, so exhausting one cannot affect the other. - api/_lib/rateLimit.ts: split into isChallengeRateLimited / isActionRateLimited; kept isRateLimited as a deprecated shim for any external callers - api/auth/challenge.ts: GET uses isChallengeRateLimited(clientIp(req)); POST verify uses isActionRateLimited(address) - api/ipfs/upload-file.ts, upload-json.ts, unpin.ts: switched to isActionRateLimited - api/_lib/rateLimit.test.ts: added tests asserting bucket isolation including the griefing scenario (attacker floods challenge endpoint with victim address; victim action quota is unaffected) ## Favourorg#1163 — Cross-tab localStorage network sync can retarget an in-progress form useNetworkGuard now captures the network value active at hook-mount time and sets networkChangedSinceMount=true whenever NetworkContext drifts away from that baseline (e.g. via a cross-tab storage event). Submission is blocked and an explicit 'Network changed to X since you opened this form' banner with an acknowledgement button is shown. Clicking the button calls acknowledgeNetworkChange(), which rebases the snapshot and re-enables the form. - frontend/src/hooks/useNetworkGuard.ts: added networkChangedSinceMount and acknowledgeNetworkChange() to the return value - MintForm, BurnForm, SetMetadataForm, AdminPanel: consume the new values and render the confirmation banner ## Favourorg#1164 — Contract error enum discriminant uniqueness Added test_error_discriminants_are_unique in contracts/token-factory/src/test.rs that exhaustively lists all 27 Error variants, casts each to u32, and asserts uniqueness via a HashSet. A future collision will fail CI before shipping. Added entries for codes 24-27 (TreasuryTransferFailed, BatchSizeExceeded, NoPendingProposal, ProposalExpired) to CONTRACT_ERROR_MESSAGES and CONTRACT_ERROR_REQUIREMENTS in frontend/src/utils/contractErrors.ts. Documented all three fixes in docs/security-triage.md.
7 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 28, 2026
## [1.8.7](v1.8.6...v1.8.7) (2026-08-28) ### Bug Fixes * **auth:** replace timing-unsafe JWT signature comparison with timingSafeEqual ([#1179](#1179)) ([8925440](8925440)), closes [#4](#4) * **contract:** replace single-step upgrade with two-step timelock (issue [#6](#6)) ([#1180](#1180)) ([cb9f98a](cb9f98a)) * split rate-limit buckets, guard cross-tab network change, enforce error discriminant uniqueness ([#1178](#1178)) ([7ddf2c4](7ddf2c4)), closes [#1162](#1162) [#1163](#1163) [#1164](#1164) [#1162](#1162) [#1163](#1163) [#1164](#1164)
|
🎉 This PR is included in version 1.8.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
2 participants
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.
Summary
Fixes three issues from the August 2026 critical-issues audit in a single PR.
Closes #1162
Closes #1163
Closes #1164
#1162 — Rate-limit bucket shared between challenge issuance and authenticated actions (High)
Root cause:
isRateLimited(address)keyed one shared bucket per Stellar address across every call site — including the unauthenticatedGET /api/auth/challenge, where a Stellar address is a public value. An attacker could exhaust a victim's daily quota at zero cost, locking them out of login and all authenticated actions for 24 hours.Changes:
api/_lib/rateLimit.ts: split into two exported functions:isChallengeRateLimited(ip)— keyed on requester IP, limitsGET /api/auth/challenge; defaults to 20/window, 200/day (env:CHALLENGE_RATE_LIMIT_WINDOW,CHALLENGE_RATE_LIMIT_DAY)isActionRateLimited(address)— keyed on wallet address, limits uploads/unpin after proof-of-possession; keeps existing defaults of 10/window, 100/dayisRateLimitedretained as a deprecated shim delegating toisActionRateLimitedapi/auth/challenge.ts:GETnow callsisChallengeRateLimited(clientIp(req));POSTverify callsisActionRateLimited(address)api/ipfs/upload-file.ts,upload-json.ts,unpin.ts: switched toisActionRateLimitedapi/_lib/rateLimit.test.ts: added tests asserting full bucket isolation including the griefing scenario (attacker floods challenge endpoint with victim address; victim action quota untouched)#1163 — Cross-tab
localStoragenetwork sync can retarget an in-progress form (High)Root cause:
useLocalStorage'sstorageevent listener silently updatesNetworkContextwhen another tab writes the persisted network key.StellarContextrebuildsstellarServicepointing at a different factory contract, whileMintForm/BurnForm/SetMetadataForm/AdminPanelretain their stale token address and amount.useNetworkGuardonly checks Freighter-vs-app agreement — it had no way to know the app's own target changed mid-fill.Changes:
frontend/src/hooks/useNetworkGuard.ts: captures the network value at hook-mount time; setsnetworkChangedSinceMount = truewheneverNetworkContextdrifts away (cross-tab or manual switch); returns newacknowledgeNetworkChange()that rebases the snapshot and clears the flag. Blocks submission with a descriptive reason when drift is detected.MintForm,BurnForm,SetMetadataForm,AdminPanel: consumenetworkChangedSinceMountandacknowledgeNetworkChange; render an explicit "Network changed to X since you opened this form — review and confirm" banner with a confirmation button.#1164 — Three contract errors sharing discriminant 24 (Medium-High)
Root cause: The contract already carries the correct unique codes (24
TreasuryTransferFailed, 25BatchSizeExceeded, 26NoPendingProposal, 27ProposalExpired), but there was no automated guard preventing a future collision, and the frontend'sCONTRACT_ERROR_MESSAGESmap was missing all four codes.Changes:
contracts/token-factory/src/test.rs: addedtest_error_discriminants_are_unique— exhaustively lists all 27Errorvariants, casts each tou32, asserts uniqueness viaHashSet. Also asserts the total variant count, so adding a new variant without registering it here fails CI.frontend/src/utils/contractErrors.ts: added entries for codes 24–27 in bothCONTRACT_ERROR_MESSAGESandCONTRACT_ERROR_REQUIREMENTS.Documentation
docs/security-triage.md: added a section explaining why challenge issuance and authenticated actions are rate-limited differently (#1162), why the network-changed guard is separate from the Freighter mismatch check (#1163), and where the error-discriminant uniqueness check lives (#1164).Testing
@rolldown/binding-linux-x64-gnunot installed), which preventsnpm run test:apiandnpm run testfrom running locally. This is a pre-existing environment issue unrelated to these changes — CI on the upstream repo will execute the full suite.tsc --noEmiton both root and frontend tsconfigs) with no new errors introduced by these changes.