fix(security): residual session mint + ops router authz - #953
Conversation
Review of stale hardening commit 041129ff against current origin/main found most findings already fixed (login-email 410, reconciliation admin, v1 settleInferenceOnce, HTTPS defaults, standup gate, signed session cookie). Residual gaps closed here: - H1: POST /api/session still minted any role without credential proof; require apiKey and validate against backend before signing cookie. - L1/M1: fallback GET diagnostics and recovery GET status were open; gate both routers with requireAdminAuth. Also archives June/July security review docs under docs/security/ and adds a residual review note explaining what was already fixed vs deferred (C3 per-provider HMAC, H2 renter key hashing, H5 deps). Not included: CLAUDE.md / .claude/settings.local.json (local agent config).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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: 07a6e884ea
ℹ️ 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".
| await fetch('/api/session', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ role: metadata.role }), | ||
| body: JSON.stringify({ role: metadata.role, apiKey }), | ||
| }) |
There was a problem hiding this comment.
Now that /api/session can return 401 when the backend credential check fails, this helper still treats any HTTP response as success because fetch only throws on network errors. In those cases the caller continues to save dc1_user_data and redirect to /admin, /renter/*, or /provider/* without the httpOnly cookie, so middleware immediately bounces the user back to auth instead of showing the real sign-in error; check res.ok and throw before persisting local user data.
Useful? React with 👍 / 👎.
|
|
||
| // SECURITY: ops diagnostics + simulate trigger - no external consumer. | ||
| // Gate the whole router (GET status/bottlenecks/disconnects were open). | ||
| router.use(requireAdminAuth); |
There was a problem hiding this comment.
This router-level middleware now runs before GET /api/fallback/status, but the launch readiness integration app mounts this router without setting DC1_ADMIN_TOKEN or sending an admin token, and requireAdminAuth returns 503 when that env var is absent. The existing backend/tests/integration/launch-health-status.test.js still asserts an unauthenticated status request returns 200, so this change breaks that readiness test unless the test/setup is updated to authenticate or the endpoint remains exempt.
Useful? React with 👍 / 👎.
1 participant
Summary
Deferred
Test plan
Merging does not deploy backend (founder approval required).