Repository navigation
feat(audit): add core audit log table, service and automatic recording - #901
aXenDeveloper wants to merge 5 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
React Doctor found 7 new issues in 6 files · 7 warnings · score 87 / 100 (Great) · 0 fixed · vs 7 warnings
Reviewed by React Doctor for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc03d4791e
ℹ️ 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".
| const stack = frames.get(c) ?? []; | ||
| stack.push(frame); | ||
| frames.set(c, stack); |
There was a problem hiding this comment.
Isolate audit frames by asynchronous execution
When multiple queue tasks run concurrently, these frames are shared rather than nested: runClaimedQueueTasks runs up to five workers with the same Hono context, and each worker calls withAuditContext. Concurrent content schedules also add their own schedule metadata and correlation IDs. An entry from one task can therefore inherit another task's scheduleId, scheduledByUserId, or correlation ID, corrupting the audit trail. Store frames in execution-local context, such as AsyncLocalStorage, instead of a mutable stack keyed only by the request.
Useful? React with 👍 / 👎.
| after: sanitizeAuditValue(after[name]) ?? null, | ||
| before: | ||
| before === undefined | ||
| ? null | ||
| : (sanitizeAuditValue(before[name]) ?? null), |
There was a problem hiding this comment.
Redact sensitive field names before exposing revision values
For an editorial content type with a sensitive scalar field such as apiKey or password, the detail response exposes its revision values verbatim. Unlike sanitizeAuditChanges, sanitizeAuditValue cannot recognize a sensitive field name when called only on its scalar value, so the automatic redaction promised by the Content Engine audit configuration is bypassed here. Check the field name with isSensitiveAuditKey and redact both values before including them in the detail response.
Useful? React with 👍 / 👎.
| const excluded = new Set(definition.audit.excludeFields); | ||
| const names = [ | ||
| ...new Set(revision.changedFields.map(path => path.split(".")[0])), | ||
| ].filter(name => !excluded.has(name)); |
There was a problem hiding this comment.
Honor trackChanges when resolving revision details
When an editorial content type sets audit: { trackChanges: false }, recording correctly omits changes and changed-field metadata, but this detail reader reconstructs both from the linked revision anyway. Consequently, opening an audit entry still reveals field names and before/after values despite the documented operation-only setting. Apply definition.audit.trackChanges before deriving revision fields, while retaining any revision summary that does not disclose changes.
Useful? React with 👍 / 👎.
Adds core_audit_logs with a sanitizing, transaction-aware writer
(c.get("audit").record), an auditDiff helper and trusted context frames
for background work. Content Engine writes are audited inside their
transactions (revision captures for editorial types, the plain services
otherwise), and roles, staff permissions, users, AI and notification
settings, report reasons and the moderation workflow record their own
entries atomically with their changes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSeRx3Xr1PGRJ5q7wvs7jK
…ention and export Adds scoped list/detail/entity/export/settings routes, moderator scope enforcement, a daily retention job, CSV export with formula escaping, the AdminCP System -> Activity Logs and ModeratorCP -> Activity screens with filters and a detail sheet, an Activity row panel on every Content Engine list, the content type audit option and developer documentation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSeRx3Xr1PGRJ5q7wvs7jK
…filter Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSeRx3Xr1PGRJ5q7wvs7jK
… paths Regression tests for the audit writer and its frames, moderator scope enforcement (including another scope's id answering 404), admin routes and sensitive-field gating, export escaping, retention, role and staff recording, the report workflow, plain content updates and scheduled publishing attributed to the system. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSeRx3Xr1PGRJ5q7wvs7jK
cc03d47 to
c35d125
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c35d125256
ℹ️ 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".
| const encodeCursor = (createdAt: Date, id: number): string => | ||
| Buffer.from(JSON.stringify([createdAt.toISOString(), id])).toString( | ||
| "base64url", | ||
| ); |
There was a problem hiding this comment.
Preserve timestamp precision in audit pagination cursors
When a page boundary falls within multiple audit entries written in the same transaction, the next page can silently skip the remaining entries. core_audit_logs.createdAt uses PostgreSQL's microsecond-precision defaultNow(), but converting it through a JavaScript Date truncates it to milliseconds. The tuple predicate in listAuditLogsWithDetails then compares against a timestamp earlier than the actual boundary, excluding entries sharing its original timestamp regardless of their IDs. This affects the admin, moderator, and entity-history lists. Preserve the database timestamp as text in the cursor and cast it back for comparison, as the existing pagination helpers do.
Useful? React with 👍 / 👎.
- Admin logs at /admin/core/staff/admins/logs and moderator logs at /admin/core/staff/moderators/logs, linked from each staff list with a Logs button; export and retention settings live on both pages - New `staff` list/export filter: `moderator` keeps the moderator area, `admin` every other area (AdminCP, member and system activity) - Remove the System -> Activity Logs page and nav item, the ModeratorCP Activity page and nav item, the moderation activity API routes and the moderator activity_logs permission - Hide the Area column when every listed entry shares one area - Docs: Content Engine activity logs page moves to the Content Engine section; audit docs point at the staff logs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSeRx3Xr1PGRJ5q7wvs7jK
|
|
||
| const excludeFields = [...(audit?.excludeFields ?? [])]; | ||
| for (const field of excludeFields) { | ||
| if (!fieldNames.includes(field)) { |
There was a problem hiding this comment.
React Doctor · react-doctor/js-set-map-lookups (warning)
This scales poorly because array.includes() inside a loop scans the whole list every time. Use a Set for constant-time lookups.
Fix → Use a Set or Map when you check for the same items over and over. Array.includes/find scans the whole list each time
| tx: ContentDatabase, | ||
| entries: readonly AuditPendingEntry[], | ||
| ): Promise<void> => { | ||
| for (const entry of entries) await writeAuditLog(c, entry, tx); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This for…of loop waits before starting the next iteration. If iterations perform independent asynchronous work, consider bounded concurrency; await syntax alone does not establish a speedup.
Fix → Consider concurrent calls only for independent asynchronous work. Shared queues or synchronous work may not benefit. Preserve resource limits, transaction ordering, and failure/cancellation semantics; observe all callback promises.
| c, | ||
| tx, | ||
| held | ||
| .filter(entry => entry !== created && !folded.includes(entry)) |
There was a problem hiding this comment.
React Doctor · react-doctor/js-set-map-lookups (warning)
This scales poorly because array.includes() inside a loop scans the whole list every time. Use a Set for constant-time lookups.
Fix → Use a Set or Map when you check for the same items over and over. Array.includes/find scans the whole list each time
| * The body of the detail sheet: who, when, where and what changed, with the | ||
| * content revision the entry points at when there is one. | ||
| */ | ||
| export const ActivityDetailContent = ({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-high-complexity-react-function (warning)
ActivityDetailContent has cyclomatic complexity 21, cognitive complexity 22, and maximum nesting depth 2, so its React logic is hard to understand and change. Extract independent branches into components or hooks.
Fix → Extract independent render branches and state logic into focused components or hooks until the control flow is easy to follow.
| <div | ||
| aria-label={t("label")} | ||
| className="bg-card ring-foreground/10 flex flex-col gap-3 rounded-xl px-4 py-3 shadow-xs ring-1" | ||
| role="search" |
There was a problem hiding this comment.
React Doctor · react-doctor/prefer-tag-over-role (warning)
Screen reader users get more reliable semantics from <search> than role="search", so use <search> instead.
Fix → Use the matching HTML element when one exists so browsers and assistive tech get native semantics.
| targetType, | ||
| }: ActivityHistoryProps) => { | ||
| const t = useTranslations("core.activity_logs"); | ||
| const labels = useActivityLabels(); |
There was a problem hiding this comment.
React Doctor · react-doctor/rerender-memo-with-default-value (warning)
This custom Hook default creates a new array every render and forwards it to a Hook dependency.
Fix → Move it to the top of the file: const EMPTY_ITEMS: Item[] = [], then use that as the default value
Adds core_audit_logs with a sanitizing, transaction-aware writer
(c.get("audit").record), an auditDiff helper and trusted context frames
for background work. Content Engine writes are audited inside their
transactions (revision captures for editorial types, the plain services
otherwise), and roles, staff permissions, users, AI and notification
settings, report reasons and the moderation workflow record their own
entries atomically with their changes.
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01PSeRx3Xr1PGRJ5q7wvs7jK