Codex/upgrade admin routing and UI design l3iq4v#37
Conversation
…ssues-for-deployment
…hes-for-deployment
…app-branch-7t1t73
…app-branch-d3piof Add Bitcoin services inventory and remote-push diagnostics; tighten .gitignore for env files
Reviewer's GuideRebrands the public marketing experience into a single-page GEM CYBER enterprise platform and introduces a new admin center with file-backed data stores, authenticated routing, and operator-focused modules for inbox triage, diagnostics, and roadmap/spec documentation. Sequence diagram for contact submission and admin triage flowsequenceDiagram
actor Visitor
participant Browser as PublicApp_Browser
participant ContactApi as ApiContactRoute
participant Store as ContactMessageStore
participant Smtp as SmtpServer
actor Admin
participant AdminBrowser as AdminApp_Browser
participant AdminInboxPage as AdminInboxPage
participant AdminInboxApi as ApiAdminInboxRoute
Visitor->>Browser: Submit contact form
Browser->>ContactApi: POST /api/contact{name, email, company, phone, service, message, sourcePage}
ContactApi->>Store: createContactMessage(sourcePage, firstName, lastName, email, phone, serviceInterest, messageBody)
Store-->>ContactApi: ContactMessage
alt smtp configured
ContactApi->>Smtp: send admin notification
ContactApi->>Smtp: send visitor confirmation
end
ContactApi-->>Browser: 200 {success: true}
Admin->>AdminBrowser: Open /admin/inbox
AdminBrowser->>AdminInboxPage: Request server component
AdminInboxPage->>AdminInboxApi: listContactMessages(q, status)
AdminInboxApi->>Store: listContactMessages(q, status, assignedToUserId)
Store-->>AdminInboxApi: ContactMessage[]
AdminInboxApi-->>AdminInboxPage: messages
AdminInboxPage-->>AdminBrowser: Render message list
Admin->>AdminInboxPage: Update status/assignee (form submit)
AdminInboxPage->>Store: updateContactMessage(id, status, assignedToUserId)
Store-->>AdminInboxPage: updated ContactMessage
AdminInboxPage-->>AdminBrowser: Revalidated inbox view
Class diagram for admin auth, contact messages, and campaign modelsclassDiagram
class ContactMessage {
+string id
+string createdAt
+string sourcePage
+string firstName
+string lastName
+string email
+string phone
+string serviceInterest
+string messageBody
+MessageStatus status
+string assignedToUserId
+string orgId
+string[] tags
}
class MessageStatus {
<<enumeration>>
open
triaged
closed
}
class ContactMessageStore {
+createContactMessage(sourcePage, firstName, lastName, email, phone, serviceInterest, messageBody) ContactMessage
+listContactMessages(q, status, assignedToUserId) ContactMessage[]
+updateContactMessage(id, status, assignedToUserId, orgId, tags) ContactMessage
+toCsv(messages) string
-ensureStore() void
-readMessages() ContactMessage[]
-writeMessages(messages) void
}
class AdminUser {
+string id
+string email
+string passwordHash
+string name
+AdminRole role
+boolean active
}
class AdminRole {
<<enumeration>>
super_admin
admin
analyst
auditor
}
class AdminUserStore {
+listAdminUsers() AdminUser[]
+authenticateUser(email, password) AdminUser
-hashPassword(password) string
-verifyPassword(password, passwordHash) boolean
-defaultUsers() AdminUser[]
-normalizeUser(user) AdminUser
-ensureStore() void
}
class SessionPayload {
+string userId
+string email
+string name
+AdminRole role
+number exp
}
class AdminAuth {
+string ADMIN_COOKIE
+createSessionToken(userId, email, name, role) string
+readSessionToken(token) SessionPayload
+getCurrentAdminSession() SessionPayload
+hasRole(role, allowedRoles) boolean
-authSecret() string
-sign(payload) string
}
class CampaignMode {
<<enumeration>>
sequence
newsletter
}
class Contact {
+string id
+string email
+string firstName
+string lastName
+string[] tags
+string status
}
class Segment {
+string id
+string name
+string[] filters
+number contactCount
}
class Campaign {
+string id
+string name
+CampaignMode mode
+string status
+string segmentId
+string scheduledAtUtc
}
class Recipient {
+string id
+string campaignId
+string contactId
+string deliveryStatus
}
class Event {
+string id
+string recipientId
+string eventType
+string atUtc
}
class SuppressionListEntry {
+string id
+string email
+string reason
+string createdAtUtc
}
class AuditLog {
+string id
+string actor
+string action
+string targetId
+string createdAtUtc
}
class NotificationOutbox {
+string id
+string channel
+string status
+string payloadRef
+string createdAtUtc
}
class CampaignEngineGuardrails {
<<value_object>>
+string[] rules
}
class AppEnv {
+string appName
+string supportEmail
+string supportPhone
+string dataProviderApiKey
+string newsIngestionWebhookSecret
+string emailProviderApiKey
+string emailSendingDomain
+string databaseUrl
+string auditLogSink
}
class ContactPage
class HomePage
class Navbar
class Footer
ContactMessageStore --> ContactMessage
ContactMessageStore --> MessageStatus
AdminUserStore --> AdminUser
AdminUserStore --> AdminRole
AdminAuth --> SessionPayload
AdminAuth --> AdminRole
Campaign --> CampaignMode
Campaign --> Segment
Recipient --> Campaign
Recipient --> Contact
Event --> Recipient
SuppressionListEntry --> Contact
AuditLog --> AdminUser
NotificationOutbox --> Campaign
CampaignEngineGuardrails --> Campaign
AppEnv ..> ContactPage : used by
AppEnv ..> HomePage : used by
AppEnv ..> Navbar : used by
AppEnv ..> Footer : used by
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…app-branch-nh36mg Add Bitcoin services inventory and remote-push diagnostics; tighten .gitignore for env files
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
src/lib/adminAuth.ts,timingSafeEqualcan throw if the provided signature length differs from the expected one; consider validating lengths before calling it so malformed tokens don’t crash requests. - The
proxyhelper insrc/proxy.tswon’t run unless it’s wired into Next.js middleware (e.g., via amiddleware.tsthat calls it or by renaming it accordingly); ensure it’s actually invoked for the intended admin routes. - On
src/app/admin/inbox/page.tsx, thesearchParamsprop is typed and handled as aPromise, but in the App Router it’s passed as a plain object; updating the type and removingawaitwill align with Next.js conventions and avoid type issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `src/lib/adminAuth.ts`, `timingSafeEqual` can throw if the provided signature length differs from the expected one; consider validating lengths before calling it so malformed tokens don’t crash requests.
- The `proxy` helper in `src/proxy.ts` won’t run unless it’s wired into Next.js middleware (e.g., via a `middleware.ts` that calls it or by renaming it accordingly); ensure it’s actually invoked for the intended admin routes.
- On `src/app/admin/inbox/page.tsx`, the `searchParams` prop is typed and handled as a `Promise`, but in the App Router it’s passed as a plain object; updating the type and removing `await` will align with Next.js conventions and avoid type issues.
## Individual Comments
### Comment 1
<location> `src/lib/contactMessages.ts:112-121` </location>
<code_context>
+
+export async function updateContactMessage(
+ id: string,
+ updates: Partial<Pick<ContactMessage, 'status' | 'assignedToUserId' | 'orgId' | 'tags'>>,
+): Promise<ContactMessage | null> {
+ const messages = await readMessages();
+ const index = messages.findIndex((message) => message.id === id);
+
+ if (index === -1) {
+ return null;
+ }
+
+ const current = messages[index];
+
+ messages[index] = {
+ ...current,
+ ...updates,
+ tags: updates.tags ?? current.tags,
+ };
</code_context>
<issue_to_address>
**issue (bug_risk):** Spreading `updates` can overwrite fields with `undefined` (e.g., status) unexpectedly.
Since `updates` is a `Partial`, callers (e.g. the admin API) can send `{ status: undefined }`, which will overwrite `current.status` when you spread `...updates`. Your special handling only protects `tags`. To avoid this, only merge fields that are explicitly defined, e.g.:
```ts
const next: ContactMessage = {
...current,
...(updates.status !== undefined && { status: updates.status }),
...(updates.assignedToUserId !== undefined && { assignedToUserId: updates.assignedToUserId }),
...(updates.orgId !== undefined && { orgId: updates.orgId }),
tags: updates.tags ?? current.tags,
};
messages[index] = next;
```
This prevents unintentionally overwriting existing values with `undefined`.
</issue_to_address>
### Comment 2
<location> `src/app/api/admin/inbox/route.ts:41-45` </location>
<code_context>
+ return NextResponse.json({ message: 'Missing message id' }, { status: 400 });
+ }
+
+ const updated = await updateContactMessage(body.id, {
+ status: body.status,
+ assignedToUserId: body.assignedToUserId ?? null,
+ tags: body.tags,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** PATCH handler may pass `undefined` status/tags through, relying on `updateContactMessage` to handle them.
Because `status` and `tags` are forwarded directly, omitted/`undefined` values can end up clearing fields in combination with the current `updateContactMessage` behavior. To better match PATCH semantics, only pass properties that are actually present in the request body, e.g.:
```ts
const updates: Parameters<typeof updateContactMessage>[1] = {};
if (body.status) updates.status = body.status;
if ('assignedToUserId' in body) updates.assignedToUserId = body.assignedToUserId ?? null;
if (Array.isArray(body.tags)) updates.tags = body.tags;
const updated = await updateContactMessage(body.id, updates);
```
This avoids unintentionally nullifying fields when they are omitted from the PATCH request.
```suggestion
const updates: Parameters<typeof updateContactMessage>[1] = {};
if (body.status) {
updates.status = body.status;
}
if ('assignedToUserId' in body) {
updates.assignedToUserId = body.assignedToUserId ?? null;
}
if (Array.isArray(body.tags)) {
updates.tags = body.tags;
}
const updated = await updateContactMessage(body.id, updates);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| updates: Partial<Pick<ContactMessage, 'status' | 'assignedToUserId' | 'orgId' | 'tags'>>, | ||
| ): Promise<ContactMessage | null> { | ||
| const messages = await readMessages(); | ||
| const index = messages.findIndex((message) => message.id === id); | ||
|
|
||
| if (index === -1) { | ||
| return null; | ||
| } | ||
|
|
||
| const current = messages[index]; |
There was a problem hiding this comment.
issue (bug_risk): Spreading updates can overwrite fields with undefined (e.g., status) unexpectedly.
Since updates is a Partial, callers (e.g. the admin API) can send { status: undefined }, which will overwrite current.status when you spread ...updates. Your special handling only protects tags. To avoid this, only merge fields that are explicitly defined, e.g.:
const next: ContactMessage = {
...current,
...(updates.status !== undefined && { status: updates.status }),
...(updates.assignedToUserId !== undefined && { assignedToUserId: updates.assignedToUserId }),
...(updates.orgId !== undefined && { orgId: updates.orgId }),
tags: updates.tags ?? current.tags,
};
messages[index] = next;This prevents unintentionally overwriting existing values with undefined.
| const updated = await updateContactMessage(body.id, { | ||
| status: body.status, | ||
| assignedToUserId: body.assignedToUserId ?? null, | ||
| tags: body.tags, | ||
| }); |
There was a problem hiding this comment.
suggestion (bug_risk): PATCH handler may pass undefined status/tags through, relying on updateContactMessage to handle them.
Because status and tags are forwarded directly, omitted/undefined values can end up clearing fields in combination with the current updateContactMessage behavior. To better match PATCH semantics, only pass properties that are actually present in the request body, e.g.:
const updates: Parameters<typeof updateContactMessage>[1] = {};
if (body.status) updates.status = body.status;
if ('assignedToUserId' in body) updates.assignedToUserId = body.assignedToUserId ?? null;
if (Array.isArray(body.tags)) updates.tags = body.tags;
const updated = await updateContactMessage(body.id, updates);This avoids unintentionally nullifying fields when they are omitted from the PATCH request.
| const updated = await updateContactMessage(body.id, { | |
| status: body.status, | |
| assignedToUserId: body.assignedToUserId ?? null, | |
| tags: body.tags, | |
| }); | |
| const updates: Parameters<typeof updateContactMessage>[1] = {}; | |
| if (body.status) { | |
| updates.status = body.status; | |
| } | |
| if ('assignedToUserId' in body) { | |
| updates.assignedToUserId = body.assignedToUserId ?? null; | |
| } | |
| if (Array.isArray(body.tags)) { | |
| updates.tags = body.tags; | |
| } | |
| const updated = await updateContactMessage(body.id, updates); |
…app-branch-9lkoqp
…app-branch-ouc3rk
…app-branch-s5l3hh
| export function proxy(request: NextRequest) { | ||
| const { pathname, search } = request.nextUrl; | ||
|
|
||
| const isAdminRoute = pathname.startsWith('/admin'); | ||
| const isAdminApiRoute = pathname.startsWith('/api/admin'); | ||
|
|
||
| if (!isAdminRoute && !isAdminApiRoute) { | ||
| return NextResponse.next(); | ||
| } | ||
|
|
||
| if (isAdminRoute && isPublicAdminPath(pathname)) { | ||
| return NextResponse.next(); | ||
| } | ||
|
|
||
| const token = request.cookies.get(ADMIN_COOKIE)?.value; | ||
| const session = readSessionToken(token || ''); | ||
|
|
||
| if (session) { | ||
| return NextResponse.next(); | ||
| } | ||
|
|
||
| if (isAdminApiRoute) { | ||
| return NextResponse.json({ message: 'Unauthorized' }, { status: 401 }); | ||
| } | ||
|
|
||
| const loginUrl = new URL('/admin/login', request.url); | ||
| loginUrl.searchParams.set('next', `${pathname}${search}`); | ||
| return NextResponse.redirect(loginUrl); | ||
| } | ||
|
|
||
| export const config = { | ||
| matcher: ['/admin/:path*', '/api/admin/:path*'], | ||
| }; |
There was a problem hiding this comment.
🔴 Middleware file misnamed — admin auth protection is completely inactive
The authentication middleware is placed at src/proxy.ts, but Next.js only auto-discovers middleware from middleware.ts (at project root) or src/middleware.ts. Because no file exists at either of those paths, Next.js never invokes the proxy function. The exported config.matcher and the proxy function are dead code.
Impact: All admin routes are unprotected
Since the middleware never runs, every admin UI route (/admin/inbox, /admin/users, /admin/diagnostics, etc.) and every admin API route (/api/admin/inbox, /api/admin/logout, etc.) is publicly accessible without any authentication. Anyone can view the admin inbox, export CSV data, modify message statuses, and see diagnostic information.
The file should be renamed from src/proxy.ts to src/middleware.ts, and the default export should be the middleware function (Next.js expects a default export or a named export called middleware):
// src/middleware.ts
export { proxy as middleware, config } from './proxy';or simply rename and restructure the file.
Prompt for agents
The middleware file at src/proxy.ts is never loaded by Next.js because it is not at the expected path. Next.js requires middleware to be at either middleware.ts (project root) or src/middleware.ts. Additionally the function must be exported as default or as a named export called 'middleware'.
Option A: Rename src/proxy.ts to src/middleware.ts and rename the exported function from 'proxy' to 'middleware'.
Option B: Create a new file src/middleware.ts that re-exports:
export { proxy as middleware, config } from './proxy';
Either approach will make Next.js discover and invoke the admin auth middleware.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| const expected = sign(encodedPayload); | ||
|
|
||
| if (!crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected))) { |
There was a problem hiding this comment.
🔴 timingSafeEqual throws on mismatched buffer lengths, crashing session validation
At src/lib/adminAuth.ts:48, crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected)) will throw a RangeError: Input buffers must have the same byte length if the signature portion of a cookie token has a different byte length than the expected HMAC hex digest (which is always 64 hex chars).
Root Cause and Impact
The expected value is always a 64-character hex string produced by crypto.createHmac('sha256', ...).digest('hex') at src/lib/adminAuth.ts:20. However, signature is extracted directly from user-supplied cookie data at line 40. If a user (or attacker) sets a malformed gem_admin_session cookie with a signature part of any other length, timingSafeEqual throws instead of returning false.
This error is unhandled — it propagates up through readSessionToken → getCurrentAdminSession (line 68) → the admin layout (src/app/admin/layout.tsx:14), causing a 500 Internal Server Error on every admin page. An attacker can intentionally trigger this by setting a crafted cookie value like abc.short.
The fix is to check buffer lengths before calling timingSafeEqual:
const sigBuf = Buffer.from(signature);
const expBuf = Buffer.from(expected);
if (sigBuf.length !== expBuf.length || !crypto.timingSafeEqual(sigBuf, expBuf)) {
return null;
}| if (!crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected))) { | |
| const sigBuf = Buffer.from(signature); | |
| const expBuf = Buffer.from(expected); | |
| if (sigBuf.length !== expBuf.length || !crypto.timingSafeEqual(sigBuf, expBuf)) { |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary by Sourcery
Rebuild the marketing homepage and intelligence experience into a unified, one-page enterprise platform while introducing an admin center with role-based authentication, inbox triage, and campaign architecture stubs backed by environment-driven configuration.
New Features:
Enhancements:
Build:
Deployment:
Documentation:
Chores: