Frontend hardening#8
Conversation
Greptile SummaryThis PR introduces a nonce-based replay-prevention layer for the admin authentication system, a Durable Object-backed rate limiter for support form submissions, a settlement epoch state machine with freshness tracking, and a shared pool-data cache to eliminate redundant fetches across components.
Confidence Score: 3/5The auth hardening is a meaningful improvement overall, but the nonce-not-consumed path for non-admin wallets is a gap in the one-time-use invariant that should be fixed before deploying to production. The nonce replay window for wallets that pass signature verification but are not currently admin is a real, exploitable gap: if a wallet is granted admin status within the 5-minute nonce TTL after a prior rejected request, the old signed challenge can be replayed to authorize admin actions. The admin wallet list refreshes every 60 seconds, so the overlap window is genuine. The client-side challenge response validation gap is a defence-in-depth miss that makes the admin signing ceremony easier to subvert. ts/apps/admin/lib/geoblocking-auth.ts (nonce not consumed for non-admin wallets) and ts/apps/admin/lib/admin-auth-client.ts (challenge response fields not validated against originals) need the most attention before merge.
|
| Filename | Overview |
|---|---|
| ts/apps/admin/lib/geoblocking-auth.ts | Updated to consume nonces after admin verification; nonce is NOT consumed when a wallet has a valid signature but is not admin, leaving the challenge replayable within the 5-minute TTL |
| ts/apps/admin/lib/admin-auth-client.ts | Client helper that fetches a challenge and signs it; missing validation that the server-returned method/route/bodyHash match the originals |
| ts/apps/admin/lib/admin-auth-nonce-durable-object.ts | New Durable Object implementing one-time nonce storage with transactional issue/consume operations; alarm-based cleanup is correct |
| ts/apps/admin/lib/admin-auth-nonce.ts | Server-side helpers for issuing and consuming nonce challenges; route normalization blocks the challenge endpoint itself |
| ts/apps/admin/app/api/admin-auth/challenge/route.ts | New challenge-issuance endpoint; validates input types, blocks caching, dispatches correctly |
| ts/apps/admin/components/settlement-card.tsx | Replaces boolean epochOpen with a three-state EpochState machine and epochStateFresh flag; all operator actions gate on freshness |
| ts/apps/web/lib/support-rate-limit-durable-object.ts | New Durable Object implementing sliding-window rate limiting; transactional storage and alarm cleanup are correct |
| ts/apps/web/lib/support-requests.ts | Added dual rate limiting backed by a Durable Object; IP detection falls back to user-controllable headers after cf-connecting-ip |
| ts/apps/web/lib/poolDataClient.ts | New module-level cache and in-flight deduplication for pool data; clean implementation |
| ts/apps/web/hooks/usePoolStats.ts | Refactored to use shared poolDataClient cache; properly distinguishes loading vs refreshing states |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client as Admin Client
participant ChallengeAPI as /api/admin-auth/challenge
participant NonceDO as AdminAuthNonce DO
participant ProtectedAPI as Protected API Route
participant GeoblockingAuth as geoblocking-auth.ts
participant WalletDO as Admin Wallet on-chain
Client->>ChallengeAPI: POST method route bodyHash
ChallengeAPI->>NonceDO: POST /issue challenge
NonceDO-->>ChallengeAPI: 204 stored
ChallengeAPI-->>Client: challenge with nonce issuedAt
Client->>Client: signMessage challenge
Client->>ProtectedAPI: request with auth headers
ProtectedAPI->>GeoblockingAuth: authorizeAdminAccess
GeoblockingAuth->>GeoblockingAuth: verifySignature
GeoblockingAuth->>WalletDO: getAuthorizedAdminWallets
WalletDO-->>GeoblockingAuth: masterWallet
alt wallet is admin
GeoblockingAuth->>NonceDO: POST /consume
NonceDO-->>GeoblockingAuth: 204 consumed
GeoblockingAuth-->>ProtectedAPI: ok true isAdmin true
ProtectedAPI-->>Client: 200 OK
else wallet not admin
GeoblockingAuth-->>ProtectedAPI: ok true isAdmin false
ProtectedAPI-->>Client: 403 Forbidden
Note over NonceDO: Nonce NOT consumed - replayable 5 min
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client as Admin Client
participant ChallengeAPI as /api/admin-auth/challenge
participant NonceDO as AdminAuthNonce DO
participant ProtectedAPI as Protected API Route
participant GeoblockingAuth as geoblocking-auth.ts
participant WalletDO as Admin Wallet on-chain
Client->>ChallengeAPI: POST method route bodyHash
ChallengeAPI->>NonceDO: POST /issue challenge
NonceDO-->>ChallengeAPI: 204 stored
ChallengeAPI-->>Client: challenge with nonce issuedAt
Client->>Client: signMessage challenge
Client->>ProtectedAPI: request with auth headers
ProtectedAPI->>GeoblockingAuth: authorizeAdminAccess
GeoblockingAuth->>GeoblockingAuth: verifySignature
GeoblockingAuth->>WalletDO: getAuthorizedAdminWallets
WalletDO-->>GeoblockingAuth: masterWallet
alt wallet is admin
GeoblockingAuth->>NonceDO: POST /consume
NonceDO-->>GeoblockingAuth: 204 consumed
GeoblockingAuth-->>ProtectedAPI: ok true isAdmin true
ProtectedAPI-->>Client: 200 OK
else wallet not admin
GeoblockingAuth-->>ProtectedAPI: ok true isAdmin false
ProtectedAPI-->>Client: 403 Forbidden
Note over NonceDO: Nonce NOT consumed - replayable 5 min
end
Comments Outside Diff (2)
-
ts/apps/admin/lib/geoblocking-auth.ts, line 278-296 (link)Nonce not consumed for non-admin wallets
In
authorizeAdminAccess, when a wallet has a valid signature but is not in the admin set, the function returns{ ok: true, isAdmin: false }without consuming the nonce. The signed challenge remains valid for the full 5-minute TTL window and can be replayed. If a wallet is added to the on-chain admin list within that window (cache refresh is 60 s), the attacker could replay the original signed request and gain access. The same pattern exists inauthorizeGeoblockingUpdate(which returns{ ok: false, error: "Connected wallet is not authorized" }without consuming).consumeAdminAuthNonceshould be called for any request that passes signature verification, regardless of admin status. -
ts/apps/web/lib/support-requests.ts, line 99-117 (link)IP rate limit bypassable via user-controlled headers
getClientIpfalls through tox-real-ipand thenx-forwarded-forwhencf-connecting-ipis absent. Both fallback headers are freely settable by the client; anyone reaching the worker without going through Cloudflare's proxy can send an arbitrary IP and bypass the per-IP window entirely. Consider logging a warning or rejecting the request when the CF header is absent instead of trusting user-supplied headers.
Reviews (1): Last reviewed commit: "refactor: cards and admin harneding" | Re-trigger Greptile
| const data = (await response.json().catch(() => null)) as | ||
| | (AdminAuthRequestChallenge & { error?: unknown }) | ||
| | null; | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error( | ||
| typeof data?.error === "string" | ||
| ? data.error | ||
| : "Failed to prepare admin authorization", | ||
| ); | ||
| } | ||
|
|
||
| if ( | ||
| !data || | ||
| typeof data.method !== "string" || | ||
| typeof data.route !== "string" || | ||
| typeof data.bodyHash !== "string" || | ||
| typeof data.issuedAt !== "string" || | ||
| typeof data.nonce !== "string" | ||
| ) { | ||
| throw new Error("Admin authorization challenge is malformed"); | ||
| } | ||
|
|
||
| return data; | ||
| } |
There was a problem hiding this comment.
Challenge response fields not validated against request
requestAdminAuthChallenge verifies only that the fields exist and are strings, but does not assert that data.method, data.route, and data.bodyHash match the original input.method, input.route, and input.bodyHash. If the server ever returns a challenge for a different route or method than the client requested, the admin wallet would sign and authorise the wrong operation with no client-side indication. The three fields should be compared against the normalised inputs before the wallet is asked to sign.
No description provided.