Codex-generated pull request#29
Conversation
…ssues-for-deployment
…hes-for-deployment
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideModernizes the marketing and intelligence surfaces, introduces a campaign engine UI, and adds a lightweight admin center with file-backed storage, auth, and inbox tooling wired into the contact API and environment-driven configuration. Sequence diagram for contact form submission into admin inboxsequenceDiagram
actor Visitor
participant ContactPage as ContactPage_/contact-us
participant ContactApi as API_contact_POST
participant ContactStore as contactMessages_store
participant SMTP as SMTP_provider
Visitor->>ContactPage: Fill and submit contact form
ContactPage->>ContactApi: POST /api/contact
ContactApi->>ContactApi: Validate required fields
ContactApi->>ContactStore: createContactMessage(sourcePage, firstName, lastName, email, phone, serviceInterest, messageBody)
ContactStore-->>ContactApi: ContactMessage record
alt SMTP configured
ContactApi->>SMTP: sendMail(admin_notification)
ContactApi->>SMTP: sendMail(user_confirmation)
SMTP-->>ContactApi: Delivery responses
else SMTP not configured
ContactApi-->>ContactApi: Skip outbound email
end
ContactApi-->>ContactPage: 200 OK { success: true }
ContactPage-->>Visitor: Show success message
Sequence diagram for admin authentication and route protectionsequenceDiagram
actor Admin
participant AdminLogin as AdminLoginPage
participant AdminLoginApi as API_admin_login_POST
participant AdminUsers as adminUsers_store
participant AdminAuth as adminAuth_token
participant Browser as Browser_cookies
participant Proxy as proxy_middleware
participant AdminInbox as AdminInboxPage
Admin->>AdminLogin: Open /admin/login
AdminLogin->>AdminLoginApi: POST /api/admin/login { email, password }
AdminLoginApi->>AdminUsers: authenticateUser(email, password)
AdminUsers-->>AdminLoginApi: AdminUser | null
alt valid user
AdminLoginApi->>AdminAuth: createSessionToken(userId, email, name, role)
AdminAuth-->>AdminLoginApi: token
AdminLoginApi->>Browser: Set ADMIN_COOKIE=token
AdminLoginApi-->>AdminLogin: 200 OK
AdminLogin->>AdminInbox: router.push(/admin/inbox)
Admin->>Proxy: Request /admin/inbox
Proxy->>Browser: Read ADMIN_COOKIE
Proxy->>AdminAuth: readSessionToken(token)
AdminAuth-->>Proxy: SessionPayload
Proxy-->>AdminInbox: Allow request
AdminInbox-->>Admin: Render inbox UI
else invalid user
AdminLoginApi-->>AdminLogin: 401 Invalid credentials
AdminLogin-->>Admin: Show error message
end
ER diagram for contact messages, admin users, and campaign data model stubserDiagram
AdminUser {
string id
string email
string passwordHash
string name
string role
boolean active
}
ContactMessage {
string id
string createdAt
string sourcePage
string firstName
string lastName
string email
string phone
string serviceInterest
string messageBody
string status
string assignedToUserId
string orgId
string tags
}
Contact {
string id
string email
string firstName
string lastName
string tags
string status
}
Segment {
string id
string name
string filters
int contactCount
}
Campaign {
string id
string name
string mode
string status
string segmentId
string scheduledAtUtc
}
Recipient {
string id
string campaignId
string contactId
string deliveryStatus
}
Event {
string id
string recipientId
string eventType
string atUtc
}
SuppressionListEntry {
string id
string email
string reason
string createdAtUtc
}
AuditLog {
string id
string actor
string action
string targetId
string createdAtUtc
}
NotificationOutbox {
string id
string channel
string status
string payloadRef
string createdAtUtc
}
AdminUser ||--o{ ContactMessage : assigned_to
Contact ||--o{ Recipient : receives
Segment ||--o{ Campaign : filters
Campaign ||--o{ Recipient : targets
Recipient ||--o{ Event : generates
Contact ||--o{ SuppressionListEntry : may_have
Campaign ||--o{ AuditLog : changes_logged_in
NotificationOutbox ||--o{ AuditLog : delivery_logged_in
Updated class diagram for contact message storage and admin inbox toolingclassDiagram
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(input)
+listContactMessages(filters)
+updateContactMessage(id, updates)
+toCsv(messages)
}
class ContactApiRoute {
+POST(request)
}
class AdminInboxApiRoute {
+GET(request)
+PATCH(request)
}
class AdminInboxPage {
+updateMessageAction(formData)
}
ContactMessageStore --> ContactMessage : manages
ContactApiRoute ..> ContactMessageStore : uses
AdminInboxApiRoute ..> ContactMessageStore : uses
AdminInboxPage ..> ContactMessageStore : updates_via_actions
Updated class diagram for admin users, auth, and protected admin centerclassDiagram
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()
+authenticateUser(email, password)
}
class SessionPayload {
string userId
string email
string name
AdminRole role
number exp
}
class AdminAuth {
+createSessionToken(userId, email, name, role)
+readSessionToken(token)
+getCurrentAdminSession()
+hasRole(role, allowedRoles)
}
class AdminLoginApiRoute {
+POST(request)
}
class AdminLogoutApiRoute {
+POST(request)
}
class AdminLoginPage {
+onSubmit(event)
}
class AdminLayout {
+render(children)
}
class AdminUsersPage {
+render()
}
class AdminProxyMiddleware {
+proxy(request)
}
AdminUserStore --> AdminUser : manages
AdminLoginApiRoute ..> AdminUserStore : authenticateUser
AdminLoginApiRoute ..> AdminAuth : createSessionToken
AdminLogoutApiRoute ..> AdminAuth : clear_cookie
AdminLoginPage ..> AdminLoginApiRoute : fetch
AdminLayout ..> AdminAuth : getCurrentAdminSession
AdminUsersPage ..> AdminAuth : getCurrentAdminSession
AdminUsersPage ..> AdminUserStore : listAdminUsers
AdminProxyMiddleware ..> AdminAuth : readSessionToken
AdminAuth --> SessionPayload : uses
Updated class diagram for campaign engine data model stubs and guardrailsclassDiagram
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 {
+campaignEngineGuardrails: string[]
}
Segment "1" --> "many" Campaign : selected_by
Campaign "1" --> "many" Recipient : sends_to
Recipient "1" --> "many" Event : tracked_by
Contact "1" --> "many" Recipient : receives
Contact "1" --> "many" SuppressionListEntry : may_be_suppressed_by
Campaign "1" --> "many" AuditLog : operations_logged_in
NotificationOutbox "1" --> "many" AuditLog : delivery_logged_in
CampaignEngineGuardrails ..> Campaign : governs_behavior
Updated class diagram for environment configuration and layout componentsclassDiagram
class AppEnv {
+string appName
+string supportEmail
+string supportPhone
+string dataProviderApiKey
+string newsIngestionWebhookSecret
+string emailProviderApiKey
+string emailSendingDomain
+string databaseUrl
+string auditLogSink
}
class Navbar {
+navigation
+productNavigation
+mobileNavigation
+isProductRoute(pathname)
}
class Footer {
+render()
}
class HomePage {
+trustMetrics
+services
+architectureSurfaces
+intelligencePreview
+render()
}
class IntelligencePage {
+filters
+feedCards
+render()
}
class CampaignsPage {
+analytics
+queueRows
+dataModelStubs
+render()
}
class RoadmapPage {
+roadmapPhases
+render()
}
class RootLayout {
+metadata
+render(children)
}
AppEnv <.. Navbar : uses_appEnv
AppEnv <.. Footer : uses_appEnv
AppEnv <.. HomePage : uses_appEnv
AppEnv <.. ContactPage : uses_appEnv
RootLayout ..> Navbar : includes
RootLayout ..> Footer : includes
RootLayout ..> HomePage : wraps
RootLayout ..> IntelligencePage : wraps
RootLayout ..> CampaignsPage : wraps
RootLayout ..> RoadmapPage : wraps
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The new
src/proxy.tswithproxy()andconfigwon’t actually run unless it’s wired up as a Next.jsmiddleware.tsdefault export; consider moving this intomiddleware.tsor otherwise registering it so admin route protection is enforced. - The JSON stores under
data/(admin-users.json,contact-messages.json) are now committed to the repo, which risks shipping sample credentials/PII; it would be safer to add these to.gitignoreand seed them at runtime instead. - The
appEnvhelper mixes public and secret-like values (e.g. API keys) and is imported by client components such asNavbarandFooter; consider splitting this into a client-safe config and a server-only config to prevent accidental exposure of sensitive env vars in the client bundle.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `src/proxy.ts` with `proxy()` and `config` won’t actually run unless it’s wired up as a Next.js `middleware.ts` default export; consider moving this into `middleware.ts` or otherwise registering it so admin route protection is enforced.
- The JSON stores under `data/` (`admin-users.json`, `contact-messages.json`) are now committed to the repo, which risks shipping sample credentials/PII; it would be safer to add these to `.gitignore` and seed them at runtime instead.
- The `appEnv` helper mixes public and secret-like values (e.g. API keys) and is imported by client components such as `Navbar` and `Footer`; consider splitting this into a client-safe config and a server-only config to prevent accidental exposure of sensitive env vars in the client bundle.
## Individual Comments
### Comment 1
<location> `src/lib/contactMessages.ts:61-67` </location>
<code_context>
+): Promise<ContactMessage> {
+ const messages = await readMessages();
+
+ const record: ContactMessage = {
+ id: crypto.randomUUID(),
+ createdAt: new Date().toISOString(),
+ status: 'open',
+ assignedToUserId: null,
+ orgId: null,
+ tags: [],
+ ...input,
+ };
</code_context>
<issue_to_address>
**issue (bug_risk):** crypto.randomUUID is used without importing crypto, which will throw at runtime.
In Node, the global `crypto` is the Web Crypto API and does not provide `randomUUID` like `node:crypto` does, so this call will fail. Import and use `randomUUID` from `node:crypto` (or import `crypto` from `node:crypto`) instead, for example:
```ts
import { randomUUID } from 'node:crypto';
const record: ContactMessage = {
id: randomUUID(),
// ...
};
```
</issue_to_address>
### Comment 2
<location> `src/lib/adminAuth.ts:40-48` </location>
<code_context>
+ return null;
+ }
+
+ const [encodedPayload, signature] = token.split('.');
+
+ if (!encodedPayload || !signature) {
+ return null;
+ }
+
+ const expected = sign(encodedPayload);
+
+ if (!crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected))) {
+ return null;
+ }
</code_context>
<issue_to_address>
**🚨 issue (security):** timingSafeEqual is called without checking length equality, which can throw and bypass auth instead of failing gracefully.
In `readSessionToken`, `crypto.timingSafeEqual` is called on `signature` and `expected` without normalizing/validating lengths. If they differ, Node throws instead of returning `false`, which can break or bypass auth handling. Guard on length first and compare buffers:
```ts
const sigBuf = Buffer.from(signature, 'hex');
const expectedBuf = Buffer.from(expected, 'hex');
if (sigBuf.length !== expectedBuf.length) {
return null;
}
if (!crypto.timingSafeEqual(sigBuf, expectedBuf)) {
return null;
}
```
This keeps the comparison constant‑time while avoiding runtime errors for malformed tokens.
</issue_to_address>
### Comment 3
<location> `src/app/api/admin/inbox/route.ts:4-13` </location>
<code_context>
+export async function GET(request: Request) {
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Admin inbox API endpoints rely solely on middleware for auth; adding server-side session checks would harden access control.
These `GET`/`PATCH` handlers assume admin auth is enforced only by routing/middleware. If that layer is ever misconfigured or skipped, the endpoints effectively become public despite exposing sensitive contact data and allowing status/assignee changes.
Consider adding an explicit session + role check here (e.g. `getCurrentAdminSession()` with a role assertion) and returning `401/403` on failure, so the handlers themselves still enforce access control even if middleware is bypassed.
Suggested implementation:
```typescript
import { NextResponse } from 'next/server';
import { listContactMessages, toCsv, updateContactMessage } from '@/lib/contactMessages';
import { getCurrentAdminSession } from '@/lib/auth';
```
```typescript
export async function GET(request: Request) {
const session = await getCurrentAdminSession();
if (!session) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
}
if (session.user?.role !== 'admin') {
return NextResponse.json({ error: 'Forbidden' }, { status: 403 });
}
const url = new URL(request.url);
```
To fully apply this pattern across the inbox admin API and keep access control consistent:
1. Ensure `getCurrentAdminSession` exists in `@/lib/auth` (or adjust the import path) and that it returns a session object with a `user.role` field (or adapt the role check to your existing session shape).
2. Apply the same session + role check at the top of the `PATCH` handler (and any other handlers in this file) so write operations are also protected if middleware or routing is misconfigured.
3. If your project distinguishes between authentication errors (401) and authorization errors (403) differently, adjust the status codes and error payloads to match your existing API conventions.
</issue_to_address>
### Comment 4
<location> `src/app/admin/inbox/page.tsx:25-32` </location>
<code_context>
+
+export default async function InboxPage({
+ searchParams,
+}: {
+ searchParams: Promise<{ q?: string; status?: 'all' | MessageStatus }>;
+}) {
+ const params = await searchParams;
+ const q = params.q || '';
+ const status = params.status || 'all';
</code_context>
<issue_to_address>
**suggestion:** Typing searchParams as a Promise is misleading and unnecessary, even though await on a plain object works at runtime.
In the app router, `searchParams` is already a plain object, not a promise, so this typing is incorrect and the `await` is unnecessary. Please type it directly as:
```ts
export default async function InboxPage({
searchParams,
}: {
searchParams: { q?: string; status?: 'all' | MessageStatus };
}) {
const q = searchParams.q || '';
const status = searchParams.status || 'all';
// ...
}
```
```suggestion
export default async function InboxPage({
searchParams,
}: {
searchParams: { q?: string; status?: 'all' | MessageStatus };
}) {
const q = searchParams.q || '';
const status = searchParams.status || 'all';
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const record: ContactMessage = { | ||
| id: crypto.randomUUID(), | ||
| createdAt: new Date().toISOString(), | ||
| status: 'open', | ||
| assignedToUserId: null, | ||
| orgId: null, | ||
| tags: [], |
There was a problem hiding this comment.
issue (bug_risk): crypto.randomUUID is used without importing crypto, which will throw at runtime.
In Node, the global crypto is the Web Crypto API and does not provide randomUUID like node:crypto does, so this call will fail. Import and use randomUUID from node:crypto (or import crypto from node:crypto) instead, for example:
import { randomUUID } from 'node:crypto';
const record: ContactMessage = {
id: randomUUID(),
// ...
};| const [encodedPayload, signature] = token.split('.'); | ||
|
|
||
| if (!encodedPayload || !signature) { | ||
| return null; | ||
| } | ||
|
|
||
| const expected = sign(encodedPayload); | ||
|
|
||
| if (!crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected))) { |
There was a problem hiding this comment.
🚨 issue (security): timingSafeEqual is called without checking length equality, which can throw and bypass auth instead of failing gracefully.
In readSessionToken, crypto.timingSafeEqual is called on signature and expected without normalizing/validating lengths. If they differ, Node throws instead of returning false, which can break or bypass auth handling. Guard on length first and compare buffers:
const sigBuf = Buffer.from(signature, 'hex');
const expectedBuf = Buffer.from(expected, 'hex');
if (sigBuf.length !== expectedBuf.length) {
return null;
}
if (!crypto.timingSafeEqual(sigBuf, expectedBuf)) {
return null;
}This keeps the comparison constant‑time while avoiding runtime errors for malformed tokens.
| export async function GET(request: Request) { | ||
| const url = new URL(request.url); | ||
| const status = url.searchParams.get('status') || undefined; | ||
| const q = url.searchParams.get('q') || undefined; | ||
| const format = url.searchParams.get('format'); | ||
|
|
||
| const messages = await listContactMessages({ | ||
| q, | ||
| status: status === 'open' || status === 'triaged' || status === 'closed' ? status : undefined, | ||
| }); |
There was a problem hiding this comment.
🚨 suggestion (security): Admin inbox API endpoints rely solely on middleware for auth; adding server-side session checks would harden access control.
These GET/PATCH handlers assume admin auth is enforced only by routing/middleware. If that layer is ever misconfigured or skipped, the endpoints effectively become public despite exposing sensitive contact data and allowing status/assignee changes.
Consider adding an explicit session + role check here (e.g. getCurrentAdminSession() with a role assertion) and returning 401/403 on failure, so the handlers themselves still enforce access control even if middleware is bypassed.
Suggested implementation:
import { NextResponse } from 'next/server';
import { listContactMessages, toCsv, updateContactMessage } from '@/lib/contactMessages';
import { getCurrentAdminSession } from '@/lib/auth';export async function GET(request: Request) {
const session = await getCurrentAdminSession();
if (!session) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
}
if (session.user?.role !== 'admin') {
return NextResponse.json({ error: 'Forbidden' }, { status: 403 });
}
const url = new URL(request.url);To fully apply this pattern across the inbox admin API and keep access control consistent:
- Ensure
getCurrentAdminSessionexists in@/lib/auth(or adjust the import path) and that it returns a session object with auser.rolefield (or adapt the role check to your existing session shape). - Apply the same session + role check at the top of the
PATCHhandler (and any other handlers in this file) so write operations are also protected if middleware or routing is misconfigured. - If your project distinguishes between authentication errors (401) and authorization errors (403) differently, adjust the status codes and error payloads to match your existing API conventions.
| export default async function InboxPage({ | ||
| searchParams, | ||
| }: { | ||
| searchParams: Promise<{ q?: string; status?: 'all' | MessageStatus }>; | ||
| }) { | ||
| const params = await searchParams; | ||
| const q = params.q || ''; | ||
| const status = params.status || 'all'; |
There was a problem hiding this comment.
suggestion: Typing searchParams as a Promise is misleading and unnecessary, even though await on a plain object works at runtime.
In the app router, searchParams is already a plain object, not a promise, so this typing is incorrect and the await is unnecessary. Please type it directly as:
export default async function InboxPage({
searchParams,
}: {
searchParams: { q?: string; status?: 'all' | MessageStatus };
}) {
const q = searchParams.q || '';
const status = searchParams.status || 'all';
// ...
}| export default async function InboxPage({ | |
| searchParams, | |
| }: { | |
| searchParams: Promise<{ q?: string; status?: 'all' | MessageStatus }>; | |
| }) { | |
| const params = await searchParams; | |
| const q = params.q || ''; | |
| const status = params.status || 'all'; | |
| export default async function InboxPage({ | |
| searchParams, | |
| }: { | |
| searchParams: { q?: string; status?: 'all' | MessageStatus }; | |
| }) { | |
| const q = searchParams.q || ''; | |
| const status = searchParams.status || 'all'; |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3d93530bc
ℹ️ 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".
| if (isAdminRoute && isPublicAdminPath(pathname)) { | ||
| return NextResponse.next(); |
There was a problem hiding this comment.
Exempt login API route from admin auth middleware
Because the matcher includes /api/admin/:path*, unauthenticated POST /api/admin/login requests also go through this guard, but the allowlist only bypasses /admin/login page routes. As written, fresh login attempts are intercepted and return 401 before the login handler runs, so admins cannot sign in from a logged-out state.
Useful? React with 👍 / 👎.
|
|
||
| const expected = sign(encodedPayload); | ||
|
|
||
| if (!crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected))) { |
There was a problem hiding this comment.
Check signature length before timingSafeEqual
readSessionToken passes the cookie-provided signature directly into crypto.timingSafeEqual without ensuring both buffers have equal length. In Node, mismatched lengths throw ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH, so a malformed gem_admin_session cookie can trigger a 500 on protected admin requests instead of being treated as an invalid session.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| function authSecret(): string { | ||
| return process.env.ADMIN_AUTH_SECRET || 'change-me-auth-secret'; |
There was a problem hiding this comment.
Remove hardcoded fallback for session signing key
Using a built-in default for ADMIN_AUTH_SECRET means any environment that forgets to set this variable signs and validates admin session cookies with a publicly known key. In that configuration, an attacker can forge a valid cookie payload (including elevated roles) and bypass authentication entirely.
Useful? React with 👍 / 👎.
Codex generated this pull request, but encountered an unexpected error after generation. This is a placeholder PR message.
Codex Task
Summary by Sourcery
Rebrand the public marketing experience into a unified one-page GEM CYBER enterprise platform and introduce an initial admin center with file-backed contact message storage and role-based access.
New Features:
Enhancements:
Documentation: