Repository navigation
feat(audit): add core audit log table, service and automatic recording - #901
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) · 1 fixed · vs 7 warnings
Reviewed by React Doctor for commit |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 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.
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.
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.
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 👍 / 👎.
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.
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 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74fc8b042d
ℹ️ 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".
| queryKey: [...ACTIVITY_QUERY_ROOT, "entity", target], | ||
| retry: false, | ||
| staleTime: OPERATIONAL_STALE_TIME, |
There was a problem hiding this comment.
If an editor opens a record's Activity panel, edits or publishes the record, and reopens Activity within the 15-second freshness window, the panel reuses the cached history and omits the operation that just succeeded. The content mutation transports call invalidateContentAfterWrite in tanstack/admin/content/query.ts, but that helper invalidates only content and search queries; repository-wide search shows the new activity query root is invalidated only when retention settings are saved. Wire activity-history invalidation into the shared content-write completion path so successful mutations immediately appear in the audit UI, as required by AGENTS.md's caching guidance.
AGENTS.md reference: AGENTS.md:L63-L63
Useful? React with 👍 / 👎.
683cf2b to
51f3514
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51f3514dbf
ℹ️ 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".
| <div className="flex flex-1 items-center justify-center"> | ||
| <Spinner size="xl" /> | ||
| </div> |
There was a problem hiding this comment.
When the retention sheet is first opened and its lazy module or settings query is still loading, this fallback displays only a spinner rather than the form-shaped local skeleton required by AGENTS.md. Replace it with a section-scoped FormPendingSkeleton or equivalent skeleton matching the settings form.
AGENTS.md reference: AGENTS.md:L43-L43
Useful? React with 👍 / 👎.
| > | ||
| {t("load_more")} | ||
| </Button> |
There was a problem hiding this comment.
When additional activity entries are available, the Load more action renders without a leading icon. AGENTS.md explicitly requires an aria-hidden lucide-react icon before every action-button label, except dismiss actions. Add the icon here and to the corresponding history pagination button in views/activity-logs/activity-history.tsx.
AGENTS.md reference: AGENTS.md:L83-L83
Useful? React with 👍 / 👎.
…d content
One audit trail for who changed what, when and to which resource.
- core_audit_logs and core_audit_settings tables with a migration;
indexed for newest-first keyset paging, actor, plugin, action, area,
one target's history and moderation scope
- Transaction-aware audit service: c.get("audit").record() / .diff(),
writeAuditLog, withAuditContext frames (actor, area, metadata, scope,
correlation id, intercept), trusted actor resolution from the session,
sanitized and bounded changes and metadata, sensitive keys redacted
- Content Engine audit written inside each write's transaction: create,
edit, delete, duplicate, publish, hide, restore, schedule and every
translation change, linked to the revision it created; per-type
`audit` option (excludeFields, trackChanges, or false)
- Core integrations: roles, staff permissions, users, AI settings and
access, notifications, report reasons, cron runs, report workflow,
approvals and scheduled publishing (attributed to the system)
- Payments: payment.created (the customer) and payment.processing /
succeeded / failed / expired (the system), written in the same
transaction as the status change; amount, currency, provider,
reference and charging plugin in the metadata
- Plugin API: defineAuditActions + buildApiPlugin({ auditActions }) for
the action filter, labels from {pluginId}.audit.actions.*
- AdminCP -> Staff -> Administrators / Moderators -> Logs: filters,
keyset "load more", detail sheet with changes and revision diff, CSV
export (formula-escaped) and retention settings; Area column only when
the list mixes areas; Activity panel on every content row
- Admin activity_logs permissions: can_view, can_view_sensitive,
can_export, can_manage_retention; daily audit-retention cron
- Docs under dev/audit-logs and content-engine/activity-logs, with
screenshots; tests for sanitizing, diffs, CSV, access rules, routes
and recording paths
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSeRx3Xr1PGRJ5q7wvs7jK
51f3514 to
bb5ad7b
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| 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 20, cognitive complexity 21, 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