Conversation
added 3 commits
April 20, 2026 08:16
…ult) Ports the auth + vault key derivation to the browser so sysnode.info can interoperate with the PR-1 backend contract without the server ever seeing the user's password: master = PBKDF2-SHA512(password, NFKC(email), 600_000 iter, 32 bytes) authHash = HKDF-SHA256(master, info="sysnode-auth-v1", 32 bytes) vaultKey = HKDF-SHA256(master, info="sysnode-vault-v1", salt=saltV) Implementation uses WebCrypto (`globalThis.crypto.subtle`) everywhere — no bignum / hash JS. The vault payload is SYSV1-prefixed authenticated AES-GCM, base64url-encoded, so tampering or wrong-key decrypts fail fast with a surfaced error code. Tests cross-check deriveMaster against Node's PBKDF2 and deriveAuthHash against a hand-rolled HKDF-SHA256 reference, so any future drift from the backend contract fails loudly before it ships. Vault tests cover round- trip, IV freshness, magic-prefix validation, wrong-key rejection, and bit-flip tamper detection. setupTests.js polyfills TextEncoder/TextDecoder + WebCrypto for the jsdom 16 environment that ships with react-scripts 5. Made-with: Cursor
Adds the HTTP + state layer that sits between the auth pages and the
backend's /auth and /vault endpoints:
- `src/lib/apiClient.js`: dedicated axios instance with
`withCredentials: true`, auto-attaches `X-CSRF-Token` from the csrf
cookie on every state-changing request, and routes non-`/auth/*` 401s
to an `onAuthLost` callback so a single handler can invalidate the
client-side session. Transport and HTTP errors are normalised into
`{ code, status, details }` so pages can switch on `err.code` without
matching message strings.
- `src/lib/authService.js`: thin façade that derives `authHash` client-
side via `deriveLoginKeys` and maps the backend contract (register /
verify-email / login / logout / me). Split from apiClient so it's
trivial to inject into tests.
- `src/context/AuthContext.js`: React context with `booting ->
anonymous|authenticated` state. On mount it calls `/auth/me` once to
restore sessions after a page refresh, and it exposes `login`,
`register`, `verifyEmail`, `logout`, `refresh`, and `handleAuthLost`.
Logout swallows transport failures intentionally — from the client's
POV the session is gone either way.
axios-mock-adapter added as devDependency to drive the transport tests.
Made-with: Cursor
Wires the auth subsystem into the existing Sysnode app: - `pages/Login`: email/password form with client-side validation, friendly error copy keyed off backend error codes (invalid_credentials, email_not_verified, server_misconfigured, network_error). Redirects to the originally-requested page after login, falling back to /account. - `pages/Register`: email + password + confirm, client-side length and mismatch checks, then a post-submit "check your inbox" screen that explains 30-minute link expiry and offers a retry path. Because the backend always returns 202 (enumeration-resistant), the UI text is worded "if the address you entered is valid..." to match. - `pages/VerifyEmail`: reads token from query string, distinguishes verified / already_verified / invalid_or_expired_token / network error. Uses a ref to guard against StrictMode double-invoke (each token is single-use server-side). - `pages/Account`: minimal view of email + verified chip + sign out. Placeholder for the key-management UI landing in a later PR. - `parts/PrivateRoute`: route guard that shows a neutral "checking your session..." panel during the /auth/me boot fetch so reload doesn't flicker between Sign-in and Account chrome. Supports `component`, `render`, and `children` patterns from RRv5. - `parts/Header`: adds an Account / Sign-in button in the header actions slot, hidden while booting to avoid the flicker above. - `App.js`: wraps the route tree in `<AuthProvider>` and registers `/login`, `/register`, `/verify-email`, and the guarded `/account`. - `App.css`: new `.auth-card`, `.auth-field`, `.auth-input`, `.auth-alert`, `.auth-kv` styles that hang off the existing design tokens (--panel, --panel-outline, --accent, --radius-md) so the auth surface reads as part of the site, not bolted on. Test coverage: form validation, backend-error-code rendering, happy- path navigation to /account on login, verify-email state machine, PrivateRoute redirect/render/boot behaviours. App.test updated to mock the new pages and provide a 401-stub authService so the provider doesn't make real network calls during the routing smoke tests. Made-with: Cursor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b34d08a1af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 2 commits
April 20, 2026 08:31
…(Codex round 1 P1) On slow networks the mount-time /auth/me probe can outlast a quick login click. When it eventually returns 401 its `catch` arm would unconditionally force ANONYMOUS, kicking a freshly-signed-in user back to /login. Introduce a request-generation counter on the provider. Every async operation that writes auth state (refresh / login / logout / handleAuthLost) captures the counter at its START and only writes state if the counter is unchanged at its END. Any newer operation bumps the counter, which atomically invalidates every in-flight older operation, regardless of which promise resolves first. Test reproduces the exact race: a pending mount-time /auth/me is held open, login completes, the mount /auth/me then rejects 401 — state must stay AUTHENTICATED. Fails on main without the counter. Made-with: Cursor
… 1 P2) The previous one-shot `firedRef` guard served StrictMode safety but permanently pinned the page to its first observed token. Same-tab SPA navigation from `/verify-email?token=A` to `?token=B` (e.g. the user clicking a fresh link in the same window) re-fires the effect with updated `location.search`, yet the boolean guard short-circuits and the new token is never submitted — the page keeps displaying stale status until a hard reload. Switch the dedupe key from a bare boolean to the last-submitted token value. Same semantics for StrictMode (a second invoke with the same token is a no-op), but a different token flips the ref and fires the new redemption. Also resets status to BOOTING for the new request so the UI reflects that we're rechecking. Test pushes a second `?token=...` URL through a shared memory history and asserts the service is called twice with the distinct tokens and the final rendered status matches the second call's outcome. Made-with: Cursor
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 449ce34ef1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 2 commits
April 20, 2026 08:41
`parseInt(str, 16)` tolerates partial parses — `parseInt('Ax', 16) === 10`
— so a malformed saltV like `"A0...AZ"` would previously be coerced to a
valid-looking 32-byte array and fed into `deriveVaultKey`. The resulting
AES-GCM key would differ silently from the intended one, leaving the
user with an opaque "decrypt failed" downstream instead of an immediate
input validation error.
Add an `^[0-9a-fA-F]*$` guard over the entire input before any per-byte
parsing. The existing length check stays; together they ensure fromHex
either returns the exact bytes the caller encoded or throws.
Tests cover: a non-hex char inside a 2-char byte window (the exact
parseInt silent-coerce case), odd-length input, and the trailing-junk
form that originally motivated the review comment.
Made-with: Cursor
…dler (Codex round 2 P2)
The default `apiClient` is created at module import time, long before
any React component (and therefore before AuthProvider) has mounted.
Passing `onAuthLost` into that `createApiClient()` call directly would
capture `undefined` in closure, so a 401 on any protected endpoint —
say `/vault` after a session expiry — would be swallowed silently and
the UI would happily keep rendering the "authenticated" chrome until
the user did something that round-tripped /auth/me.
Introduce a mutable handler slot on the apiClient module:
setAuthLostHandler(fn) — register / clear the current handler
apiClient — singleton whose onAuthLost thunk reads the
slot at dispatch time
AuthProvider registers its `handleAuthLost` via a mount-effect and
clears the slot on unmount, so stale providers can't fire after the
tree has changed.
Tests cover: non-auth 401 reaches the registered handler, cleared
slot is a clean no-op, /auth/* 401s are still treated as credential
errors (not session loss), and a re-registered handler atomically
replaces the previous one (mirrors AuthProvider remount semantics).
Made-with: Cursor
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02652e2326
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 2 commits
April 20, 2026 08:52
…ceeds (Codex round 3 P1) The round-1 generation guard claimed its slot at the very start of login(), before awaiting authService.login. When credentials are rejected, that eager bump invalidates any still-pending mount-time refresh — whose eventual ANONYMOUS write is then discarded as stale, stranding the UI in `booting` on top of the failed-login error until another auth op happens. Move `nextGen()` to AFTER `authService.login` resolves. If the login throws (bad creds, 5xx, whatever), the gen counter stays put and the mount refresh's later write still lands correctly. If it succeeds we still atomically invalidate any older in-flight refresh as before. Test reproduces the exact race: pending mount /auth/me + rejected login → mount /auth/me rejects 401 → state MUST end up anonymous. Made-with: Cursor
…mple (Codex round 3 P2)
Passing a `Content-Type` header into `axios.create({ headers })` puts
it in axios's `common` bag, which is merged into every request —
including GETs with no body. In a cross-origin deployment (the common
`REACT_APP_API_BASE` = `https://sysnode-backend.syscoin.dev` setup),
any GET with Content-Type is a non-simple CORS request and forces a
preflight; if the server's OPTIONS response doesn't whitelist the
header, `/auth/me` on boot fails before anything else can happen and
the app is effectively unmountable against a permissively-credentialed
backend.
Drop the global Content-Type. Axios still sets the correct JSON
content-type automatically on POST/PUT/PATCH when the body is an
object, so there's no behavioural regression.
Tests cover: GETs from the instance carry no Content-Type from
defaults (and the common defaults bag is empty), while JSON POSTs
still surface application/json via axios's body-inference path.
Made-with: Cursor
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f22d09117
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 2 commits
April 20, 2026 09:06
…odex round 4 P1)
Login's catch branch was blanket-treating every me() failure as a
transient hiccup and falling back to the shallow `res.user` from
/auth/login. That is wrong for 401: a 401 on the hydrate call means
the Set-Cookie from /auth/login never persisted (cross-origin third-
party-cookie block, SameSite conflict, a browser in strict privacy
mode). Pretending the user is authenticated just unlocks protected
routes that the very next /vault call will 401 on — the worst kind
of silent failure for auth UIs.
Split the catch:
* err.status === 401 -> real failure. Clear state back to ANONYMOUS
and throw a `session_not_established` error
so the Login page can surface targeted copy
("your browser didn't keep the session
cookie — allow cookies for sysnode").
* anything else -> transient (5xx, network blip). Login did
succeed server-side; fall back to the
shallow user as before.
Tests exercise both arms: 401 on hydrate → provider stays anonymous
AND login() rejects with the new error code; 503 on hydrate →
provider reaches AUTHENTICATED via the shallow user. Login page
adds an ERROR_COPY entry for `session_not_established` so the new
failure has human-friendly guidance instead of "Something went
wrong".
Made-with: Cursor
…nges (Codex round 4 P2) The round-1 fix handled the dispatch side of in-app token changes (re-firing verifyEmail when `?token=` changes) but left the response side unguarded. If token A is in flight and the user navigates to token B, A's late .then/.catch would unconditionally call setStatus and could overwrite whatever B landed, showing the wrong screen for the URL on display. Add a monotonic request counter. Each dispatch captures the counter at its start and its .then/.catch short-circuit if the counter has advanced in the meantime — i.e. a newer dispatch has already owned status. Combined with the token-value dedupe that's already in place, this gives us: same token re-seen → skipped at effect start (StrictMode) different token seen → new dispatch, new counter stale dispatch resolves → no-op, current dispatch still owns status Test pins token A open, navigates to B (which rejects), waits for B's "invalid" screen, then resolves A with success — asserts the UI stays on B's invalid screen and never flips to "verified". Made-with: Cursor
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fab24cd52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 2 commits
April 20, 2026 09:21
…rigin (Codex round 5 P1) The old fallback chain (REACT_APP_API_BASE → window.location.origin → http://localhost:3001) made /auth/* silently hit the SPA host in every real browser session, because window.location.origin wins over the localhost literal once you're running in a browser. The sysnode- info SPA server only serves static assets — it does not proxy /auth anywhere — so default login / register / session hydration would reach the wrong origin and fail unless an operator remembered to set the env var. New priority: 1. REACT_APP_API_BASE (explicit override) 2. NODE_ENV === 'production' → https://syscoin.dev (same host as the legacy public client in src/lib/api.js, which is where sysnode-backend is reachable) 3. else → http://localhost:3001 (sysnode-backend dev server) window.location.origin is removed entirely from the fallback chain. Also bump the PBKDF2 normalization test's timeout to 20s to match its siblings — two 600k-iteration SHA-512 derivations routinely exceed Jest's 5s default on local / CI runners. Made-with: Cursor
…ed (Codex round 5 P2)
The previous logout() swallowed every server-side failure and
unconditionally forced ANONYMOUS. That's a security footgun: on a
5xx or network error the session cookie is almost certainly still
valid server-side, so the UI tells the user they're signed out
while a reload would silently restore the session. On a shared /
kiosk machine, the user walks away believing the session is dead.
Split the cases:
401 / 404 on /auth/logout
→ session is ALREADY gone server-side (expired cookie, etc).
Clearing local state is idempotent with reality → clear.
any other failure (5xx, network, CORS)
→ server session likely still alive. Surface a
`logout_failed` error (with .code and .status) so the caller
can prompt retry. Local state stays AUTHENTICATED until the
server confirms.
Account page updated to catch `logout_failed`, keep the user on the
page (no redirect to /login), and render a targeted alert asking
them to retry or close the browser. The sign-out button disables
while the request is in flight.
Tests replace the old "logout always wins" expectation with three
new cases: happy-path sign-out clears locally; 401 from server
still clears locally (session already gone); 503 from server keeps
AUTHENTICATED and rejects with code `logout_failed`.
Made-with: Cursor
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb695af4e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… path (Codex round 6 P2) The round-4 reqIdRef guard only advanced the counter on dispatch, so the malformed-token early-return path (which short-circuits to STATUS_BAD without calling verifyEmail) left the counter untouched. If a previous valid-token request was still in flight, a late \"verified\" response could satisfy myReqId === reqIdRef.current and overwrite the malformed-token invalid screen with a success screen for a token that was no longer on screen. Move the `reqIdRef.current += 1` bump BEFORE the regex check so every new token — valid or malformed — invalidates every in-flight older request. Same guard; wider coverage. Test pins token A's verifyEmail promise open, navigates to a malformed token (goes straight to STATUS_BAD), then resolves A successfully — asserts UI stays on the invalid screen. Made-with: Cursor
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
8 tasks
sidhujag
pushed a commit
that referenced
this pull request
Apr 22, 2026
P1 (NewProposal): align projected payments to superblock boundaries. `buildApproximateSchedule` was treating payment #1 as if it landed on `startEpoch` (`at = start + i * step`), but Syscoin governance payouts only execute on superblocks — the first payable point is the next superblock after `startEpoch`, not the raw start timestamp. For proposals whose `startEpoch` lands just after a superblock, this overestimated how many payments could fit inside the voting window and silently hid the truncation warning. Fix: * Step by `(i + 1) * step` so the schedule starts at the NEXT superblock under the worst-case alignment assumption (the frontend has no network anchor for superblock timing, so erring conservative is correct — it may show one false "won't fit" warning but never hides a real one). * Render the schedule panel for ALL `paymentCount >= 2` proposals, not only those with a non-empty projected list. Otherwise a window shorter than one cycle would suppress the panel AND the truncation warning, recreating the very bug Codex flagged. * Add a dedicated copy variant for `schedule.length === 0` so users with short windows hear "too short to land any" in plain English. * Tighten surrounding explanatory copy to call out that the listed dates are worst-case estimates; real superblocks may run a bit earlier depending on start alignment. Added a regression test for the zero-row case and updated the existing truncation test to exercise a 45-day window that now fits exactly one payment under the corrected math. Made-with: Cursor
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
1 participant
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.
auth to backend auth work from syscoin/sysnode-backend#2