feat: add admin dashboard#120
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a platform admin surface and access control; records sanitized operational events with noisy-event suppression and scheduled pruning; updates DB schema (operational_events, user role/ban, session impersonation); implements admin APIs, router, middleware, frontend admin UI, tests, and documentation/config for new env vars. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend Client
participant Router as Admin Router
participant Middleware as requirePlatformAdmin
participant DB as Database
participant Service as Admin Service
Client->>Router: GET /api/admin/overview
Router->>Middleware: authorize request
Middleware->>DB: SELECT role FROM users WHERE id=...
DB-->>Middleware: user.role
Middleware->>Middleware: isPlatformAdminRole(role)?
alt role is admin
Middleware->>Service: getAdminOverview(env)
Service->>DB: aggregate counts & anomalies
DB-->>Service: stats
Service-->>Router: AdminOverviewResponse
Router-->>Client: 200 + JSON
else not admin
Middleware-->>Client: 403 forbidden
end
sequenceDiagram
participant Handler as InboundEmailHandler
participant Tracker as recordOperationalEventSafely
participant Sanitizer as Metadata Sanitizer
participant DO as FixedWindowRateLimiterDO
participant DB as Database
Handler->>Handler: process inbound email
alt notable outcome (reject/error/etc.)
Handler->>Tracker: trackOperationalEvent(input)
Tracker->>Sanitizer: sanitize & truncate metadata
Sanitizer-->>Tracker: sanitized JSON
Tracker->>DO: check noisy-key consume(window, max)
alt allowed
Tracker->>DB: INSERT INTO operational_events(...)
DB-->>Tracker: insert ok
else suppressed
Tracker-->>Handler: skip insert
end
end
Handler->>Handler: continue processing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
packages/frontend/src/lib/query-keys.ts (1)
7-27: Use an options object foradminAnomaliesquery keys.Seven positional params is pretty fragile here, especially since
severity,type,organizationId,from, andtoare all juststring. Swapping two filters still type-checks but creates a different cache bucket. Mirroring the API's options-object shape makes these keys much harder to misuse.Suggested refactor
- adminAnomalies: ( - page: number, - pageSize: number, - severity: string, - type: string, - organizationId: string, - from: string, - to: string - ) => + adminAnomalies: (options: { + page: number; + pageSize: number; + severity?: string; + type?: string; + organizationId?: string; + from?: string; + to?: string; + }) => [ "app", "admin", "anomalies", - page, - pageSize, - severity, - type, - organizationId, - from, - to, + options.page, + options.pageSize, + options.severity ?? "", + options.type ?? "", + options.organizationId ?? "", + options.from ?? "", + options.to ?? "", ] as const,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/lib/query-keys.ts` around lines 7 - 27, The adminAnomalies query key currently takes seven positional args which is fragile; change adminAnomalies to accept a single options object (e.g., { page, pageSize, severity, type, organizationId, from, to }) and build the key from that object (e.g., ["app","admin","anomalies", options]) or an array that includes each named field from options to preserve granularity, keep the trailing "as const" typing, and update all call sites to pass the new options object shape; ensure the exported type/signature for adminAnomalies is updated so TypeScript reflects the named properties and prevents positional-argument mistakes.packages/backend/src/modules/admin/operational-events.ts (1)
21-23: Tighten sensitive-key matching to reduce accidental over-redaction.Including
textas a free substring can redact non-sensitive keys likecontext. Consider boundary-based matching (word/segment boundaries) so diagnostics stay useful while still protecting secrets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/modules/admin/operational-events.ts` around lines 21 - 23, The SENSITIVE_KEY_PATTERN is too permissive (e.g., matches "text" inside "context"); tighten it by changing the regex in SENSITIVE_KEY_PATTERN to require whole-key or segment boundaries (use word/segment boundaries or separators like start/end, underscore, hyphen) and remove or require segment-bound matching for the "text" token so keys like "context" aren't redacted; update the pattern referenced by SENSITIVE_KEY_PATTERN accordingly and ensure any code using it (e.g., redaction logic) continues to test keys against the new boundary-aware pattern.packages/frontend/src/features/auth/hooks/route-loaders.ts (1)
44-51: Avoid duplicating platform-admin role parsing logic.
isPlatformAdminRolehere mirrors backend logic inpackages/backend/src/platform/auth/admin-access.ts(Line 21-32). Keeping two copies increases drift risk for admin gating behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/features/auth/hooks/route-loaders.ts` around lines 44 - 51, The platform-admin role parsing logic in isPlatformAdminRole is duplicated; replace it by importing a single shared implementation instead of re-implementing parsing here: move or export the canonical admin-check function (the backend's admin-check implementation) into a shared utility module (or export it from the existing auth module) and update this file to import and call that exported function (reference: isPlatformAdminRole and the backend admin-access admin-check implementation) so all code uses one source of truth.packages/contracts/src/index.ts (1)
106-128: Tighten the new date fields to actual date/datetime contracts.These shapes currently accept any non-empty string, but the backend is already serializing them as ISO values in
packages/backend/src/modules/admin/service.ts(Lines 46-56, 248-255, and 309). Using stricter validators here would catch contract drift instead of letting locale strings or malformed timestamps through.♻️ Proposed fix
export const adminActivityDaySchema = z.object({ - date: z.string().min(1), + date: z.iso.date(), generatedAddresses: z.number().int().nonnegative(), receivedEmails: z.number().int().nonnegative(), }); @@ export const adminOrganizationItemSchema = z.object({ id: z.string().min(1), name: z.string().min(1), slug: z.string().min(1), - createdAt: z.string().nullable(), + createdAt: z.string().datetime().nullable(), memberCount: z.number().int().nonnegative(), addressCount: z.number().int().nonnegative(), receivedEmailCount: z.number().int().nonnegative(), sampleEmailCount: z.number().int().nonnegative(), integrationCount: z.number().int().nonnegative(), activeIntegrationCount: z.number().int().nonnegative(), - lastReceivedAt: z.string().nullable(), + lastReceivedAt: z.string().datetime().nullable(), }); @@ export const adminOperationalEventSchema = z.object({ id: z.string().min(1), severity: adminOperationalEventSeveritySchema, type: adminOperationalEventTypeSchema, @@ organizationName: z.string().nullable(), message: z.string().min(1), metadata: z.record(z.string(), z.unknown()).nullable(), - createdAt: z.string().nullable(), + createdAt: z.string().datetime().nullable(), });Also applies to: 139-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/contracts/src/index.ts` around lines 106 - 128, The new date/string fields in the contracts are too permissive: tighten date validation by replacing freeform strings with ISO datetime validators (e.g., use z.string().datetime() or z.string().nullable().datetime() as appropriate) for the fields in adminActivityDaySchema.date and the timestamp fields in adminOrganizationItemSchema (createdAt, lastReceivedAt) and the analogous fields referenced around lines 139-151; update those schema entries (adminActivityDaySchema, adminActivityResponseSchema if it has dates, and adminOrganizationItemSchema) to require ISO/UTC datetime format so the contract matches the backend serialization.packages/backend/src/modules/admin/repo.ts (1)
346-386: Make sureoperational_eventsis indexed for this access pattern.This endpoint always sorts by
createdAt DESCand can also filter byorganizationId,severity, andtype. If the migration hasn't already added supporting indexes, the anomalies tab will slow down quickly as the event log grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/backend/src/modules/admin/repo.ts` around lines 346 - 386, The query in findAdminOperationalEventsPage repeatedly scans operationalEvents ordered by createdAt and filtered by organizationId, severity, and type; add appropriate DB indexes via a migration to speed this access pattern—create a descending index on createdAt (created_at DESC) and one or more composite indexes covering common filter combos (e.g., (organization_id, created_at DESC), (severity, created_at DESC), and/or (type, created_at DESC) or a multicolumn index like (organization_id, severity, type, created_at DESC)) so the query using operationalEvents.createdAt, operationalEvents.organizationId, operationalEvents.severity, and operationalEvents.type can use indexes for ordering and filtering.packages/frontend/src/lib/api.ts (1)
1-5: Use the contract enums for anomaly filters.
severityandtypeare already modeled in@spinupmail/contracts, but this helper widens them back to plainstring. That makes it easy to ship unsupported query values and only find out at runtime when the backend rejects them.♻️ Proposed fix
import type { AdminActivityResponse, + AdminOperationalEventSeverity, AdminOperationalEventsResponse, + AdminOperationalEventType, AdminOrganizationsResponse, AdminOverviewResponse, AddressIntegration, @@ export const listAdminAnomalies = async (options?: { page?: number; pageSize?: number; - severity?: string; - type?: string; + severity?: AdminOperationalEventSeverity; + type?: AdminOperationalEventType; organizationId?: string; from?: string; to?: string; signal?: AbortSignal; }) => {Also applies to: 185-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/lib/api.ts` around lines 1 - 5, The helper that constructs anomaly filters currently widens the severity and type fields back to plain string; import the appropriate enums from `@spinupmail/contracts` (e.g., AnomalySeverity and AnomalyType or the exact enum names exported for anomaly severity/type) and change the types for the severity and type parameters/fields in the anomaly filter helper to those enums instead of string, update any function signatures and local variables that set or return severity/type (the helper that builds query params and the other occurrence around the anomaly filter logic) to use the enum types, and ensure when serializing to query params you use the enum values (no broad string unions or casts) so only supported values are emitted to the backend.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/tests/unit/admin-operational-events.test.ts`:
- Around line 15-17: The test file currently only calls vi.clearAllMocks() in
beforeEach and later replaces console.error with a spy, but never restores it;
add an afterEach that restores the console.error spy (e.g., call
vi.restoreAllMocks() or explicitly restore the spy) so console.error is returned
to its original implementation after tests; refer to the existing
beforeEach/vi.clearAllMocks and the console.error spy usage to implement the
afterEach restoration.
In `@packages/frontend/src/components/app-sidebar.tsx`:
- Around line 106-119: The sidebar's admin link visibility (computed in
visibleNavItems using isPlatformAdminRole) relies on the stale session returned
by useAuth (which wraps authClient.useSession), causing drift from the admin
route loader that uses disableCookieCache: true; update the auth hook (useAuth /
the wrapper around authClient.useSession) to expose a way to force-refresh or
invalidate the cached session (e.g., provide a refreshSession or an option to
call authClient.useSession({ disableCookieCache: true }) when roles may have
changed) and wire the sidebar to call that refresh (or subscribe to the
refreshed session) before computing visibleNavItems so visibility always matches
the loader-enforced role.
In `@packages/frontend/src/pages/admin-page.tsx`:
- Around line 490-499: The onSuccess handler is invalidating sessions using
selectedSessionUser which can be cleared earlier in the "Revoke all" flow,
causing a null key to be invalidated; fix this by capturing the acted-on user's
id into a local variable (e.g., const actedUserId = selectedSessionUser?.id ??
null) before mutating state, then use queryClient.invalidateQueries({ queryKey:
queryKeys.adminUserSessions(actedUserId) }) (and the other invalidations) and
only afterwards call setPendingAction(null) / clear selectedSessionUser so the
correct user's sessions are invalidated; update the onSuccess in the mutation
that currently references selectedSessionUser to use this captured id.
- Around line 847-859: The code is forcing date inputs to UTC by appending "Z"
when building from/to filter timestamps before calling listAdminAnomalies;
remove that conversion so local day boundaries are preserved—build the
timestamps as `${fromDate}T00:00:00` and `${toDate}T23:59:59.999` (no trailing
Z) or alternatively pass the raw date strings and let the backend normalize,
then use those values in the listAdminAnomalies queryFn (referencing variables
fromDate, toDate and the call to listAdminAnomalies).
---
Nitpick comments:
In `@packages/backend/src/modules/admin/operational-events.ts`:
- Around line 21-23: The SENSITIVE_KEY_PATTERN is too permissive (e.g., matches
"text" inside "context"); tighten it by changing the regex in
SENSITIVE_KEY_PATTERN to require whole-key or segment boundaries (use
word/segment boundaries or separators like start/end, underscore, hyphen) and
remove or require segment-bound matching for the "text" token so keys like
"context" aren't redacted; update the pattern referenced by
SENSITIVE_KEY_PATTERN accordingly and ensure any code using it (e.g., redaction
logic) continues to test keys against the new boundary-aware pattern.
In `@packages/backend/src/modules/admin/repo.ts`:
- Around line 346-386: The query in findAdminOperationalEventsPage repeatedly
scans operationalEvents ordered by createdAt and filtered by organizationId,
severity, and type; add appropriate DB indexes via a migration to speed this
access pattern—create a descending index on createdAt (created_at DESC) and one
or more composite indexes covering common filter combos (e.g., (organization_id,
created_at DESC), (severity, created_at DESC), and/or (type, created_at DESC) or
a multicolumn index like (organization_id, severity, type, created_at DESC)) so
the query using operationalEvents.createdAt, operationalEvents.organizationId,
operationalEvents.severity, and operationalEvents.type can use indexes for
ordering and filtering.
In `@packages/contracts/src/index.ts`:
- Around line 106-128: The new date/string fields in the contracts are too
permissive: tighten date validation by replacing freeform strings with ISO
datetime validators (e.g., use z.string().datetime() or
z.string().nullable().datetime() as appropriate) for the fields in
adminActivityDaySchema.date and the timestamp fields in
adminOrganizationItemSchema (createdAt, lastReceivedAt) and the analogous fields
referenced around lines 139-151; update those schema entries
(adminActivityDaySchema, adminActivityResponseSchema if it has dates, and
adminOrganizationItemSchema) to require ISO/UTC datetime format so the contract
matches the backend serialization.
In `@packages/frontend/src/features/auth/hooks/route-loaders.ts`:
- Around line 44-51: The platform-admin role parsing logic in
isPlatformAdminRole is duplicated; replace it by importing a single shared
implementation instead of re-implementing parsing here: move or export the
canonical admin-check function (the backend's admin-check implementation) into a
shared utility module (or export it from the existing auth module) and update
this file to import and call that exported function (reference:
isPlatformAdminRole and the backend admin-access admin-check implementation) so
all code uses one source of truth.
In `@packages/frontend/src/lib/api.ts`:
- Around line 1-5: The helper that constructs anomaly filters currently widens
the severity and type fields back to plain string; import the appropriate enums
from `@spinupmail/contracts` (e.g., AnomalySeverity and AnomalyType or the exact
enum names exported for anomaly severity/type) and change the types for the
severity and type parameters/fields in the anomaly filter helper to those enums
instead of string, update any function signatures and local variables that set
or return severity/type (the helper that builds query params and the other
occurrence around the anomaly filter logic) to use the enum types, and ensure
when serializing to query params you use the enum values (no broad string unions
or casts) so only supported values are emitted to the backend.
In `@packages/frontend/src/lib/query-keys.ts`:
- Around line 7-27: The adminAnomalies query key currently takes seven
positional args which is fragile; change adminAnomalies to accept a single
options object (e.g., { page, pageSize, severity, type, organizationId, from, to
}) and build the key from that object (e.g., ["app","admin","anomalies",
options]) or an array that includes each named field from options to preserve
granularity, keep the trailing "as const" typing, and update all call sites to
pass the new options object shape; ensure the exported type/signature for
adminAnomalies is updated so TypeScript reflects the named properties and
prevents positional-argument mistakes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6edf3f5-82ed-4b13-a7a6-ef9f2c4e9798
📒 Files selected for processing (37)
README.mdpackages/backend/drizzle/0011_wet_sleeper.sqlpackages/backend/drizzle/meta/0011_snapshot.jsonpackages/backend/drizzle/meta/_journal.jsonpackages/backend/src/app/middleware/require-platform-admin.tspackages/backend/src/db/auth.schema.tspackages/backend/src/db/index.tspackages/backend/src/db/operational-event.schema.tspackages/backend/src/db/schema.tspackages/backend/src/index.tspackages/backend/src/modules/admin/operational-events.tspackages/backend/src/modules/admin/repo.tspackages/backend/src/modules/admin/router.tspackages/backend/src/modules/admin/schemas.tspackages/backend/src/modules/admin/service.tspackages/backend/src/modules/inbound-email/handler.tspackages/backend/src/platform/auth/admin-access.tspackages/backend/src/platform/auth/create-auth.tspackages/backend/tests/unit/admin-operational-events.test.tspackages/backend/tests/unit/admin-service.test.tspackages/backend/tests/unit/auth-middleware.test.tspackages/backend/tests/unit/create-auth.test.tspackages/contracts/src/index.tspackages/frontend/src/components/app-sidebar.tsxpackages/frontend/src/features/auth/hooks/route-loaders.tspackages/frontend/src/features/auth/hooks/tests/route-loaders.test.tspackages/frontend/src/features/settings/components/tests/change-password-panel.test.tsxpackages/frontend/src/features/settings/components/tests/two-factor-panel.test.tsxpackages/frontend/src/features/settings/components/tests/user-profile-panel.test.tsxpackages/frontend/src/lib/api.tspackages/frontend/src/lib/auth.tspackages/frontend/src/lib/query-keys.tspackages/frontend/src/lib/tests/api.test.tspackages/frontend/src/lib/tests/query-keys.test.tspackages/frontend/src/main.tsxpackages/frontend/src/pages/admin-page.tsxpackages/frontend/src/pages/tests/protected-layout-page.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
packages/frontend/src/pages/admin-page.tsx (1)
639-650:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCapture the acted-on user before the single-session revoke resolves.
This still reads
selectedSessionUserinonSuccess, so closing the dialog while the revoke is in flight can invalidate thenullkey and drop the audit message context. The same state-capture bug that existed in the “Revoke all” path is still present here.🐛 Suggested direction
- const revokeSessionMutation = useMutation({ - mutationFn: async (sessionToken: string) => { - const result = await authClient.admin.revokeUserSession({ sessionToken }); + const revokeSessionMutation = useMutation({ + mutationFn: async ({ + sessionToken, + userId, + userEmail, + }: { + sessionToken: string; + userId: string; + userEmail: string | null; + }) => { + const result = await authClient.admin.revokeUserSession({ sessionToken }); if (result.error) { throw new Error( readAuthError(result.error, "Unable to revoke session") ); } + return { userId, userEmail }; }, - onSuccess: async () => { - const userId = selectedSessionUser?.id ?? null; + onSuccess: async (_data, { userId, userEmail }) => { await queryClient.invalidateQueries({ queryKey: queryKeys.adminUserSessions(userId), }); - if (userId) { - await recordAdminAuditEvent({ - action: "revoke-session", - targetType: "session", - targetId: userId, - message: `Revoked one session for ${selectedSessionUser?.email ?? "user"}.`, - }).catch(() => undefined); - } + await recordAdminAuditEvent({ + action: "revoke-session", + targetType: "session", + targetId: userId, + message: `Revoked one session for ${userEmail ?? "user"}.`, + }).catch(() => undefined); }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/pages/admin-page.tsx` around lines 639 - 650, The onSuccess handler uses selectedSessionUser during async awaits so closing the dialog can change that state; capture the acted-on user's identity synchronously and reuse it. Inside onSuccess (or better, before any awaits), read and store const userId = selectedSessionUser?.id ?? null and const actedOnEmail = selectedSessionUser?.email ?? "user", then call await queryClient.invalidateQueries({ queryKey: queryKeys.adminUserSessions(userId) }) and use actedOnEmail in the recordAdminAuditEvent message instead of reading selectedSessionUser again; this ensures recordAdminAuditEvent and invalidateQueries use stable values even if selectedSessionUser changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/modules/admin/repo.ts`:
- Around line 104-111: The Recent Audit query currently returns any
operationalEvents whose metadataJson.targetId equals userId; restrict it to only
admin audit events by adding a type-filter to the query (e.g., require
operationalEvents.type (or operationalEvents.eventType) to be IN a set of admin
audit event types). Implement this by introducing a constant like
ADMIN_AUDIT_EVENT_TYPES and combining it with the existing predicate (use
.where(in(operationalEvents.type, ADMIN_AUDIT_EVENT_TYPES)) or an equivalent sql
IN clause) alongside the existing json_extract(...) = userId condition in the
query that builds from operationalEvents and leftJoin organizations.
In `@packages/backend/src/modules/admin/service.ts`:
- Around line 581-589: The metadata spread currently places input.metadata last,
allowing callers to override trusted audit fields; change the merge so that
input.metadata cannot overwrite trusted fields by either spreading
(input.metadata ?? {}) first and then the trusted properties (actorUserId,
actorEmail, action, targetType, targetId, reason) or by explicitly removing
those keys from input.metadata before merging; update the metadata object
construction in the function that builds the audit log to ensure the trusted
fields in metadata always take precedence over any keys provided in
input.metadata.
- Around line 54-58: The helper toIsoString can throw when given an invalid Date
or out-of-range numeric timestamp; update to guard date validity before calling
toISOString by checking date instanceof Date then !Number.isNaN(date.getTime())
before date.toISOString(), and for the numeric branch construct const d = new
Date(value) and verify Number.isFinite(value) and !Number.isNaN(d.getTime())
before calling d.toISOString(); return null for any invalid date/timestamp. Use
the toIsoString function name to locate and modify the logic accordingly.
In `@packages/backend/src/platform/auth/admin-access.ts`:
- Around line 17-20: platformAdminRole currently grants the raw Better Auth
permission "set-role" which lets any platform admin assign any configured role
(including superadmin); remove "set-role" from the permissions in
platformAdminRole (the adminAccessControl.newRole call) and instead implement a
custom handler (e.g., a dedicated set-role endpoint/handler) that enforces an
explicit target-role allowlist and rejects promotions to superadmin unless the
caller has an elevated permission; reference platformAdminRole and the
adminAccessControl.newRole invocation when making this change.
In `@packages/frontend/src/pages/admin-page.tsx`:
- Around line 605-621: The client currently fires recordAdminAuditEvent as a
best-effort call (note the .catch(()=>undefined)) after performing privileged
mutations, which can lead to unlogged admin actions; instead, move the audit
write into the server-side flow by creating a backend endpoint that performs the
admin action and the audit atomically, then change the client code (the code
calling recordAdminAuditEvent and the Promise.all with
queryClient.invalidateQueries and getActionAuditMessage) to call that new
endpoint and remove the best-effort audit call; ensure the client checks the
endpoint response for success/failure and only invalidates caches
(queryKeys.adminUserSessions(actedUserId),
queryKeys.adminUserDetail(actedUserId), ["app","admin","users"]) after the
server confirms both the action and audit were recorded.
- Around line 567-571: The ban handler is discarding the dialog's entered reason
and always sending the hardcoded "Administrative action"; update the branch
where action.type === "ban" to pass the user-entered reason (the dialog field on
the action object, e.g., action.banReason) into authClient.admin.banUser instead
of the literal string, and optionally keep a fallback (e.g., only use
"Administrative action" if action.banReason is empty) so the persisted
ban_reason matches what the admin entered.
- Around line 360-396: The UI currently treats a missing overview as healthy by
using fallbacks; instead, detect overview load failures via
overviewQuery.isError (or when !isLoading && !overview) and render an
error/unknown state rather than defaulting to "healthy" and zero metrics. Update
the render block that reads overview?.system.status and the computed values
generatedDelta and integrationQueueCount to short-circuit when overview is
absent (use overviewQuery.isError or overview === undefined) and show an
explicit error/placeholder UI for the platform status and metrics so API errors
are surfaced to admins.
In `@packages/frontend/src/pages/protected-layout-page.tsx`:
- Around line 77-81: The AppSidebar is receiving onRefreshSession
unconditionally which allows refreshSession to be called when user is null;
change the prop so onRefreshSession is only passed when authenticated (use the
user value) — e.g., conditionally provide onRefreshSession={refreshSession} only
if user is truthy or omit the prop otherwise; update the JSX around AppSidebar
(the onRefreshSession prop passed to AppSidebar in protected-layout-page.tsx)
and keep handleSignOut and user unchanged.
In `@packages/landing/src/content/docs/cloudflare-resources.mdx`:
- Around line 37-43: Replace the incorrect "pnpm wrangler queues create
spinupmail-integration-dispatches" invocations with "pnpm exec wrangler queues
create spinupmail-integration-dispatches": update both the table row labeled
"Integration dispatch queue" and the matching setup guide command so they use
"pnpm exec wrangler" (i.e., change the prefix from "pnpm wrangler" to "pnpm exec
wrangler" for the queue creation command).
In `@packages/landing/src/content/docs/limits-security.mdx`:
- Around line 23-29: The docs table in limits-security.mdx is out of sync with
the worker config: update the documented default values to match the actual
defaults in the worker config and wrangler.toml (e.g. change
MAX_ADDRESSES_PER_ORGANIZATION from `100` to `10`, and EMAIL_MAX_BYTES
(EMAIL_MAX_BYTES / EMAIL_MAX_BYTES constant referenced in
worker-configuration.d.ts and packages/backend/wrangler.toml) from `524288` to
`10485760`), and scan the file for any other mentions of these symbols
(MAX_RECEIVED_EMAILS_PER_ADDRESS, MAX_RECEIVED_EMAILS_PER_ORGANIZATION,
MAX_ADDRESSES_PER_ORGANIZATION, EMAIL_MAX_BYTES) and update them to reflect the
live defaults defined in worker-configuration.d.ts and
packages/backend/wrangler.toml so the page is an accurate live-default
reference.
---
Duplicate comments:
In `@packages/frontend/src/pages/admin-page.tsx`:
- Around line 639-650: The onSuccess handler uses selectedSessionUser during
async awaits so closing the dialog can change that state; capture the acted-on
user's identity synchronously and reuse it. Inside onSuccess (or better, before
any awaits), read and store const userId = selectedSessionUser?.id ?? null and
const actedOnEmail = selectedSessionUser?.email ?? "user", then call await
queryClient.invalidateQueries({ queryKey: queryKeys.adminUserSessions(userId) })
and use actedOnEmail in the recordAdminAuditEvent message instead of reading
selectedSessionUser again; this ensures recordAdminAuditEvent and
invalidateQueries use stable values even if selectedSessionUser changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c567ace-ddee-4cc4-b87f-26029ce0b7ce
📒 Files selected for processing (31)
README.mdpackages/backend/drizzle/0012_flaky_hercules.sqlpackages/backend/drizzle/meta/0012_snapshot.jsonpackages/backend/drizzle/meta/_journal.jsonpackages/backend/src/db/operational-event.schema.tspackages/backend/src/env.d.tspackages/backend/src/index.tspackages/backend/src/modules/admin/operational-events.tspackages/backend/src/modules/admin/repo.tspackages/backend/src/modules/admin/router.tspackages/backend/src/modules/admin/schemas.tspackages/backend/src/modules/admin/service.tspackages/backend/src/platform/auth/admin-access.tspackages/backend/src/platform/auth/create-auth.tspackages/backend/tests/unit/admin-operational-events.test.tspackages/backend/tests/unit/create-auth.test.tspackages/backend/worker-configuration.d.tspackages/backend/wrangler.e2e.tomlpackages/backend/wrangler.tomlpackages/backend/wrangler.toml.examplepackages/contracts/src/index.tspackages/frontend/src/components/app-sidebar.tsxpackages/frontend/src/features/auth/hooks/route-loaders.tspackages/frontend/src/lib/api.tspackages/frontend/src/lib/query-keys.tspackages/frontend/src/lib/tests/query-keys.test.tspackages/frontend/src/pages/admin-page.tsxpackages/frontend/src/pages/protected-layout-page.tsxpackages/landing/src/components/docs/content/docs-content.tspackages/landing/src/content/docs/cloudflare-resources.mdxpackages/landing/src/content/docs/limits-security.mdx
✅ Files skipped from review due to trivial changes (6)
- packages/landing/src/components/docs/content/docs-content.ts
- packages/frontend/src/lib/tests/query-keys.test.ts
- packages/backend/src/env.d.ts
- packages/backend/src/modules/admin/schemas.ts
- packages/backend/worker-configuration.d.ts
- README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/backend/src/index.ts
- packages/frontend/src/features/auth/hooks/route-loaders.ts
- packages/backend/src/platform/auth/create-auth.ts
- packages/frontend/src/lib/query-keys.ts
- packages/frontend/src/components/app-sidebar.tsx
- packages/backend/src/modules/admin/operational-events.ts
- packages/frontend/src/lib/api.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/frontend/src/pages/admin-page.tsx (1)
1617-1634: 💤 Low valueQuery key includes
severity: "info"but queryFn doesn't filter by it.The query key at Lines 1621 specifies
severity: "info", but thequeryFndoesn't pass aseverityparameter tolistAdminAnomalies. This mismatch is confusing—the cache key suggests a severity filter that isn't applied, which could lead to unexpected cache behavior if severity filtering is added later.Consider either removing
severityfrom the query key or passing it to the API call for consistency.♻️ Suggested fix: align queryKey and queryFn
queryKey: queryKeys.adminAnomalies({ page, pageSize: PAGE_SIZE, - severity: "info", + severity: "all", type, organizationId: "", from: "", to: "", }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/src/pages/admin-page.tsx` around lines 1617 - 1634, The queryKey for auditQuery includes severity: "info" but the queryFn calling listAdminAnomalies doesn't pass severity, causing a cache mismatch; update the auditQuery to align them by either (A) removing severity from queryKeys.adminAnomalies(...) if you don't intend to filter by severity, or (B) add a severity variable (e.g., const severity = "info" or a prop/state) and pass it into both queryKeys.adminAnomalies({ ..., severity }) and listAdminAnomalies({ page, pageSize: PAGE_SIZE, type, severity, signal }) so the cache key and the API call remain consistent (adjust symbols auditQuery, queryKeys.adminAnomalies, and listAdminAnomalies).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/modules/admin/repo.ts`:
- Around line 404-407: The count query in anomalies.last24h is including all
severities (so normal admin info events are treated as anomalies); update the
query that selects from operationalEvents (the
db.select(...).from(operationalEvents).where(gte(operationalEvents.createdAt,
anomalySince)) call) to also restrict to severities "warning" and "error" (i.e.,
add an additional WHERE clause filtering operationalEvents.severity to
['warning','error']) so only true warning/error events are counted; mirror the
same severity filter where relevant in related admin audit logic in service.ts
if similar counts are computed there.
In `@packages/backend/src/modules/admin/service.ts`:
- Around line 711-779: The code loads targetUser without users.role and
therefore cannot enforce role-hierarchy checks before mutating privileged
accounts; update the initial query that sets targetUser to also select
users.role, then in each privileged branch (set-role, ban, unban,
revoke-sessions, revoke-session and the impersonation path) compare actorRole
and targetUser.role and reject actions where targetUser.role is equal or higher
than actorRole (unless roleIncludes(actorRole, "superadmin") or other explicit
exceptions apply); use the existing helper roleIncludes and constants
(SUPER_ADMIN_SET_ROLE_ALLOWED_TARGET_ROLES, ADMIN_SET_ROLE_ALLOWED_TARGET_ROLES)
to determine permitted operations and throw AdminActionError(403, "target role
is not allowed") when hierarchy rules forbid the action, and add/adjust tests
covering the actor/target role matrix.
---
Nitpick comments:
In `@packages/frontend/src/pages/admin-page.tsx`:
- Around line 1617-1634: The queryKey for auditQuery includes severity: "info"
but the queryFn calling listAdminAnomalies doesn't pass severity, causing a
cache mismatch; update the auditQuery to align them by either (A) removing
severity from queryKeys.adminAnomalies(...) if you don't intend to filter by
severity, or (B) add a severity variable (e.g., const severity = "info" or a
prop/state) and pass it into both queryKeys.adminAnomalies({ ..., severity })
and listAdminAnomalies({ page, pageSize: PAGE_SIZE, type, severity, signal }) so
the cache key and the API call remain consistent (adjust symbols auditQuery,
queryKeys.adminAnomalies, and listAdminAnomalies).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a188ab50-d004-46c9-bed2-b674ac1579fe
📒 Files selected for processing (12)
packages/backend/src/modules/admin/repo.tspackages/backend/src/modules/admin/router.tspackages/backend/src/modules/admin/service.tspackages/backend/src/platform/auth/admin-access.tspackages/backend/tests/unit/create-auth.test.tspackages/contracts/src/index.tspackages/frontend/src/lib/api.tspackages/frontend/src/pages/admin-page.tsxpackages/frontend/src/pages/protected-layout-page.tsxpackages/landing/src/content/docs/cloudflare-resources.mdxpackages/landing/src/content/docs/installation.mdxpackages/landing/src/content/docs/limits-security.mdx
✅ Files skipped from review due to trivial changes (2)
- packages/landing/src/content/docs/installation.mdx
- packages/backend/src/platform/auth/admin-access.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/frontend/src/pages/protected-layout-page.tsx
- packages/landing/src/content/docs/limits-security.mdx
- packages/backend/tests/unit/create-auth.test.ts
- packages/frontend/src/lib/api.ts
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.
Summary by CodeRabbit
New Features
Documentation