✨ feat: Audit log UI for SystemGrants changes#52
Open
dustinhealy wants to merge 32 commits into
Open
Conversation
6 tasks
dustinhealy
force-pushed
the
feat/audit-log-ui
branch
from
ddaa9a8 to
8eb02c6
Compare
Wire the audit-log tab into the grants page, switch the server function from a stub to a real /api/admin/audit-log call with filter query params, generate the CSV client-side from already-fetched entries, and add unit coverage for the audit log utilities.
…er validation Defang CSV formula injection (CWE-1236) with leading-quote escape for cells beginning with =/+/-/@/tab/CR, switch to CRLF line endings, prepend UTF-8 BOM, and emit localized headers via a new auditLogToCsv(entries, localize) signature. Migrate the audit log tab UI to click-ui: ButtonGroup for the action filter, DatePicker for date inputs, Button for export, Badge with state="success"/"danger"/"neutral" for action and principal-type pills (fixes the failing 4.5:1 contrast on the prior badge-success class). Fix the focus-loss bug where the search input unmounted on every keystroke: drop the isLoading early-return, debounce search at 300ms, render LoadingState inline within the table body, and handle the isError case explicitly. Wire useAnnouncement + ScreenReaderAnnouncer so filter changes announce the result count to assistive tech; give SearchInput a proper aria-label; rename the entry-count plural keys to the i18next v25 _zero/_one/_other suffix convention; harden the CSV blob download for Safari/Firefox via appendChild plus a deferred URL.revokeObjectURL. Server-side: add a requireAnyCapability defense-in-depth guard, tighten the Zod schema with ISO date validation and a 200-char cap on search, parse the response body via Zod, bump staleTime to 60s, and add placeholderData: keepPreviousData so filter changes don't flash empty.
…wer, CSP, click-ui Server: paginated getAuditLogPageFn with cursor/limit + multi-action + facet params (actorId, targetPrincipalType, targetPrincipalId, capability), Zod-parsed response schema, auditLogInfiniteQueryOptions factory for useInfiniteQuery, exportAuditLogServerFn that proxies the backend CSV endpoint, all behind the same triple-capability defense-in-depth guard. UI: new AuditLogDetailDrawer (click-ui Flyout) renders the full entry with copyable IDs and before/after diff highlighted via Badge state. Local AuditLogEntryWithDiff type carries optional before/after arrays until the data-schemas package upstreams the fields. Parser: parseAuditSearch handles actor: / target: / capability: / created:>YYYY-MM-DD qualifiers with quoted multi-word values, falling back to free text for unknown keys. diffGrantState reports added/removed/unchanged sets. Click-ui migration: GrantTableRow and EditCapabilitiesDialog now use Badge state for status pills and the principal-type chip; deleted unused badge-success and badge-danger CSS classes from styles.css. GrantManagementTab keeps its raw table for now since click-ui Table does not support per-row tabIndex/role/onKeyDown/ref (documented inline). Security: Content-Security-Policy plus X-Content-Type-Options, Referrer-Policy, X-Frame-Options on every HTML response, with HSTS gated on production. Inline filter action wrapped in an array to match the new multi-action server schema (batch B will replace this filter UI entirely).
…arch, permalinks Replace useQuery with useInfiniteQuery against auditLogInfiniteQueryOptions so audit log pages on demand via cursor pagination — both a manual Load more button and an IntersectionObserver sentinel auto-load when the bottom row scrolls into view. The legacy single-shot getAuditLogFn and auditLogQueryOptions are gone. Multi-select action facet via click-ui CheckboxMultiSelect plus four faceted text/select filters (actor ID, target ID, target principal type, capability) collapsed behind a "More filters" disclosure with debounced inputs. Structured search runs the live input through parseAuditSearch on every debounce tick, extracts actor: / target: / capability: / created:>YYYY-MM-DD qualifiers, and renders each one as a dismissible Badge chip; clicking a chip regex-strips the corresponding token from the input. Qualifiers override the manual facet inputs when both are present. Row click and Enter/Space activation set ?entryId= on the route via TanStack Router; the matching entry opens in the AuditLogDetailDrawer with copy-permalink and Esc-to-close semantics. validateSearch on /_app/grants is extended so the param survives tab switches. Dual-mode CSV export: client-side auditLogToCsv for ≤500 loaded entries, server-side exportAuditLogServerFn for larger result sets or when more pages remain. Filter changes announce the result count via ScreenReaderAnnouncer, and Load More announces page-loaded count for assistive tech.
…r, dead-code purge Replace the cursor-based useInfiniteQuery with offset-based useQuery + placeholderData: keepPreviousData and the shared numbered Pagination component, matching the GroupsTab pattern; debounced filter setters reset the page in the same callback so search and pagination stay in sync. Drop the qualifier-parser and the disclosure-collapsed More-filters block; the four facet fields (Actor, Target, Target type, Capability) sit always-visible and partial-match against denormalized name fields on the backend. Top search box is plain regex-substring across actor, target, and capability. Replace click-ui Flyout with @radix-ui/react-dialog directly for the side panel so enter and exit animations actually play, driven by data-state keyframes added to styles.css. Every ID-like field in the drawer gets a CopyableMono button with per-button copied feedback. Each DatePicker renders a single tab stop and the shared danger-styled Clear button resets both date inputs together. Delete the unused AuditLogRow.tsx, the parseAuditSearch parser plus its types and tests, the dead ACTION_FILTER_LABELS and AUDIT_ACTION_FILTERS exports, the diffGrantState helper, and the locale keys left over from the load-more / qualifier-chip iteration. Net 383 lines deleted.
TanStack Start's SSR injects an inline `<script type="module">import("...")</script>`
into the root HTML to boot the client. The previous enforced policy of
`script-src 'self'` (no nonce, no `'unsafe-inline'`) would cause browsers to refuse
that inline script in production, breaking hydration before any UI rendered. Local
`bun run dev` never exercises `server.ts`, so the regression hid in plain sight.
Threading a per-request nonce through TanStack Start's manifest is non-trivial.
As an interim, the policy now ships as `Content-Security-Policy-Report-Only` so
violations still surface in browser devtools and reporting endpoints without
blocking hydration. Set `ADMIN_PANEL_CSP_ENFORCE=true` to flip back to enforcement
once the nonce wiring lands.
`useLocalize` returned a fresh closure on every render, so any effect that listed it in its deps array re-fired every render. In `AuditLogTab` that was the screen-reader announce effect, causing assistive tech to be spammed every time React reconciled the component. Wrapping the closure in `useCallback` keyed on `translate` keeps the function identity stable across renders while still picking up language changes.
The previous prefix regex `^[=+\-@\t\r]` missed payloads that lead with whitespace before the formula trigger (e.g. ` =SUM(...)`), payloads that start with `\n` or `|` (the latter is Excel's DDE invocation marker), and Unicode decoy characters such as NBSP and BOM that spreadsheets render as zero-width but JavaScript's `\s` does not always cover symmetrically. The defang now treats a value as dangerous if either its first character is a trigger or if the first character after stripping space/NBSP/BOM is a trigger; stripping the entire `\s` class would falsely accept payloads led by `\r` / `\n` / `\t`, which are themselves triggers. Local-day date helpers (`isoDateToDate`, `dateToIsoDate`, `localDayBoundaryIso`) also moved here so the timezone fix in `AuditLogTab` can be unit tested in isolation; new cases cover round-trips, rolled-over input rejection, and both start/end boundaries.
Each `getAuditLogPageFn` / `exportAuditLogServerFn` invocation previously did
two round-trips: `requireAnyCapability` would call `getEffectiveCapabilitiesFn`,
then the handler would call the audit-log endpoint. Pagination doubled the
backend traffic of the whole tab.
Handlers now fetch capabilities once via a new `guardAuditLogAccess` helper and
run `checkAnyCapability` against the in-memory list. `checkAnyCapability` is
exposed so future server functions can adopt the same pattern; `requireAnyCapability`
is implemented in terms of it to keep behaviour identical for unchanged callers.
Adds `getAuditLogEntryFn` and `auditLogEntryQueryOptions` so the UI can deep-link
to entries that aren't on the current page. The endpoint returns `{ entry: null }`
for 404 so callers can render an explicit "not found" state without crashing.
Four near-identical debounced-text-filter handlers in `AuditLogTab` collapsed into a single hook that owns the controlled value, the debounced commit value, and timer cleanup. The optional `onCommit` callback fires once per quiescent settle so callers can reset pagination or log analytics without re-rolling their own ref/`setTimeout` plumbing.
Drawer permalinks no longer silently fail for entries off the current page. When `?entryId=` points at a row that isn't in `pageEntries`, the tab falls back to `getAuditLogEntryFn` via React Query and renders the drawer from either the on-page row or the fetched record. A new not-found state in `AuditLogDetailDrawer` surfaces the case where the id is gone instead of leaving the drawer empty. CSV export now always hits the backend. The previous client/server split truncated CSVs whenever a result set had between 51 and 500 matching rows: the client path serialized at most one page (`AUDIT_LOG_PAGE_SIZE = 50`) but the threshold for switching to the server endpoint was 500. Pulling the client path keeps `auditLogToCsv` (and its tests) as the contract the server is expected to honor, and removes the now-unused `com_audit_export_client` translation key. Clipboard writes for the permalink button and the inline copyable cells now await the promise and only flip to the "Copied!" affordance on success. Permission-denied, HTTP-origin, and `navigator.clipboard === undefined` paths all surface via the existing `ScreenReaderAnnouncer` with a new `com_a11y_copy_failed` key. The permalink itself is now built from `window.location.origin` + the canonical `/grants?tab=audit-log&entryId=…` shape so copied links don't carry the current filter state. Filter pages now use `useDebouncedFilter` instead of four ad-hoc handlers. `DatePickerCell`'s `useEffect` no longer re-runs every render; the comment captures *why* the workaround exists so future readers don't strip it. Date filters now anchor at local-day boundaries (`localDayBoundaryIso`) instead of mixing UTC midnight with local-time picker values, fixing off-by-one filter results for any non-UTC user. `pageEntries` is memoized to avoid being a fresh array each render. Dead `com_audit_filter_*` translation keys from the qualifier-parser cleanup are removed.
The LibreChat backend already enforces ACCESS_ADMIN on every /api/admin/audit-log route, and any future tightening (e.g. a dedicated READ_AUDIT_LOG capability) belongs there. The BFF-layer guard was running an extra /effective round-trip on every page request without buying real protection, since the backend would reject the same callers we did. It was also inconsistent with GrantManagementTab, which sits on the same page and already calls getAllGrantsFn with no BFF guard. Removes guardAuditLogAccess, AUDIT_LOG_REQUIRED_CAPS, and the three call sites in getAuditLogPageFn / getAuditLogEntryFn / exportAuditLogServerFn. The checkAnyCapability helper extracted in eef26ce stays — it's still used by requireAnyCapability and is useful on its own.
The audit-log tab was visible to anyone who could reach the Grants page (i.e.
anyone with `ACCESS_ADMIN`). With the LibreChat backend now requiring
`READ_AUDIT_LOG` on `/api/admin/audit-log`, users without that grant will hit
a 403 if they click the tab — surfacing the right backend policy but a bad UX.
The tab trigger, panel slot, and body render are all gated on
`hasCapability('read:audit_log')` via the existing `useCapabilities` hook,
which reads from the cached effective-capabilities lookup the sidebar already
uses. A stale `?tab=audit-log` URL on a session that lost the cap silently
falls back to management rather than rendering an empty page.
`READ_AUDIT_LOG_CAPABILITY` lives in `@/constants` as a forward-compat string
constant until the `@librechat/data-schemas` dependency bumps to a version
that exports it from `SystemCapabilities`.
LC PR #13087 adds READ_AUDIT_LOG to @librechat/data-schemas, so the local forward-compat constant is just a string-literal detour. Removed READ_AUDIT_LOG_CAPABILITY and updated GrantsPage to read the cap from SystemCapabilities.READ_AUDIT_LOG. This commit will not typecheck against the currently-published data-schemas 0.0.48 (the cap does not exist there). That is the intended state until LC merges, data-schemas re-publishes, and this PR's package.json pin is bumped as a final commit. Until then, local verification via `bun link` against the LC checkout exercises the full path. Also adds the picker labels: com_cap_read_audit_log and com_cap_desc_read_audit_log so the System category renders the toggle and tooltip correctly once data-schemas publishes with the cap in CAPABILITY_CATEGORIES.
This PR depends on the READ_AUDIT_LOG capability added in danny-avila/LibreChat#13087. Pinning ahead of publication: the install will fail until the LC PR merges and data-schemas is republished, at which point bun install populates the lockfile and CI goes green.
Post-rebase fixup so house-style import ordering applies to a file main edited but the rebase didn't re-run sort-imports against.
dustinhealy
force-pushed
the
feat/audit-log-ui
branch
from
8eb02c6 to
5bfc287
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The npm-published @librechat/data-schemas@0.0.52 was built against a
librechat-data-provider that already exports RetentionMode, so the
previous ^0.8.502 pin (which resolves to 0.8.502, before RetentionMode
landed) breaks the dev server boot with:
[MISSING_EXPORT] "RetentionMode" is not exported by
node_modules/librechat-data-provider/dist/index.es.js
Bumping to ^0.8.503 (now on npm) restores the missing export and aligns
the admin panel with the version data-schemas@0.0.52 was built against.
CSV export now flows entirely through exportAuditLogServerFn against the
LibreChat backend, so the in-bundle helpers auditLogToCsv, escapeCsvCell,
hasFormulaPrefix, CSV_COLUMNS, and the _CsvColumnsExhaustive compile-time
check are dead code with no production callers. The corresponding
describe('auditLogToCsv', ...) block in auditLogUtils.test.ts and the eight
orphaned com_audit_csv_col_* locale keys go with them.
Formula-injection defang is unchanged in behavior because the equivalent
guarantee now lives on the backend CSV writer in packages/api with its
own regression coverage.
The post-filter live-region announcement was being passed pageEntries.length, which is capped at AUDIT_LOG_PAGE_SIZE (50). For a filter matching 200 rows, the announcement read "50 audit log entries match the current filters" while the visible bottom-of-page counter correctly read "200 entries" off the API total. Swap the announcement source to use total and update the effect's dependency array to track total instead of pageEntries.length.
The post-filter announcement effect kept isFetching in its dependency array, so every pagination click re-fired the same X-entries-match message to screen readers even though no filter had changed. Tracking a filterSignature in a ref now short-circuits the announcement unless the debounced filter inputs themselves have actually changed, leaving page navigation silent. handleExport had a try/finally with no catch, and the caller invokes it via () => void handleExport(), so a rejection from exportAuditLogServerFn went unhandled and the user saw the loading state vanish with no download and no error. A catch branch now announces a new com_a11y_audit_export_failed locale key, mirroring how copy paths use com_a11y_copy_failed when the clipboard write fails.
The wrapper's mount-only effect (empty deps) only ran once, so after the Clear button bumps dateResetNonce and the inner DatePicker remounts via its key prop, the freshly created input element retains its default tabIndex and the double-tab-stop the wrapper exists to prevent reappears. DatePickerCell now takes a resetKey prop that callers pass the same dateResetNonce they use to remount the inner DatePicker. Including it in the effect dep array re-runs the patch each time the inner input is replaced, keeping the keyboard tab order stable across clears.
…tion Three related polish fixes flagged by review bots: Export was reading the debounced filter snapshot, so a user who typed a search term and clicked Export inside the 300ms window got a CSV covering the previous (broader) result set. handleExport now rebuilds the wire filters from the immediate input values via a shared buildFilters helper, so the export matches exactly what the inputs show. The page query still uses the debounced values so typing does not refetch on every keystroke. The DatePicker controls were rendered with sibling <span> labels and no input id, regressing the WCAG check in e2e/grants.spec.ts that expects #audit-date-from / #audit-date-to with associated <label htmlFor>. DatePickerCell now takes an inputId prop, the wrapper effect stamps it onto the click-ui-rendered input alongside the tabIndex tweak, and both visible labels are real <label> elements bound by htmlFor. The not-found drawer skipped its Radix exit animation because the component returned null once notFound and entry both went falsy on close. Mirroring the existing latestEntry latch, a latestNotFound state keeps the not-found dialog mounted with open=false long enough for data-state="closed" keyframes to play out.
Contributor
Author
|
@codex review |
![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: 75c53dfb13
ℹ️ 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".
…d state Three closely-related polish fixes: Filter changes that bypassed the debouncer (action toggles, date pickers, date clear, target type) were updating the query filters in the same render while the deferred useEffect that reset currentPage to 1 ran on the next tick. That window produced one fetch with new filters at an old page offset, occasionally rendering a false empty state until the second fetch landed. Each filter callsite now calls resetToFirstPage() inline alongside its state setter so the query key changes once with currentPage=1. CopyableMono already exposed an onCopyFailed callback for clipboard errors, but the drawer never wired it to any of its five copy controls (timestamp, actor ID, target ID, capability, entry ID). A clipboard rejection from any of those was therefore silent. Added an onCopyFailed prop to AuditLogDetailDrawer and threaded the existing announce(com_a11y_copy_failed) pattern through from AuditLogTab, matching the permalink-copy treatment. The latestNotFound latch added for the close-animation fix did not clear latestEntry, so opening a missing-entry permalink after a valid entry left the drawer rendering stale content under a not-found URL. The mount-tracking effect now clears latestEntry whenever notFound flips true so the not-found branch (gated on !latestEntry) takes precedence.
The forward-compat shim added READ_AUDIT_LOG_CAPABILITY so the audit tab could check the capability while pinned to data-schemas 0.0.52, but the grants CapabilityPanel renders rows from CAPABILITY_CATEGORIES, and the upstream 0.0.52 array does not yet list READ_AUDIT_LOG under the System category. Without that row no admin could grant or revoke the capability from the UI, so only seed-time holders could ever reach the new tab. Wrap the upstream CAPABILITY_CATEGORIES so the System category includes READ_AUDIT_LOG_CAPABILITY whenever it is missing. Once the dep moves to 0.0.53+ the upstream array already contains the entry and the dedupe pass turns the wrapper into a no-op, so it stays safe to keep around until the shim itself is dropped.
Contributor
Author
|
@codex review |
…b on entryId When the paginated audit-log query fails, the table swaps in an error EmptyState but the surrounding shell kept rendering the pagination controls and entry-count footer derived from data still held by keepPreviousData (the last successful page). Users saw "showing 47 of 482 entries" with working pagination next to a "failed to load" message — mixed signals. The footer and Pagination now read from displayTotal/displayTotalPages, both collapsed to zero/one whenever isError is true. The page-clamp effect still references the live totalPages so the user's currentPage is preserved across transient failures. The screen-reader announcement effect now also short-circuits on isError so it does not say "X entries match the current filters" while the table is showing a failure message. The filter signature is left untracked on error so a retry that succeeds will announce. Deep-link permalinks of the form /grants?entryId=abc landed with no tab parameter, so the route defaulted to management and AuditLogTab never mounted — the drawer the PR description promises to open would not appear. The route now falls back to the audit-log tab whenever an entryId is present; GrantsPage already redirects to management for users without READ_AUDIT_LOG so the new default does not strand anyone.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f9a8b0731
ℹ️ 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".
…g deep-link fetch The export button still gated on raw `total`, which keepPreviousData lets stay non-zero across a failed refetch. Footer and pagination already use displayTotal, so the Export button could remain enabled while the table is showing the error EmptyState and "0 entries." Switched the disabled gate to displayTotal so all error-state surfaces agree. A cold load to /grants?entryId=abc — or any case where the deep-linked row is not on the current page — left the drawer closed while the single-entry fetch ran, then flicked it open when the fetch resolved. The drawer now stays mounted whenever an entryId is in the URL, with a new `loading` prop driving a LoadingState shell inside the same panel chrome until the entry either loads or is confirmed not-found. Closing the drawer still removes entryId from the URL, so the new open=!!entryId gate exits cleanly.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f748f2455a
ℹ️ 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".
The drawer holds latestEntry / latestNotFound across renders so Radix can play its exit animation against the last good content. When the URL entryId switches to a different row whose fetch is in flight, the parent passes entry=null with loading=true — but the prior effect only updated latches on truthy entry or notFound, so latestEntry stayed at the previous row and the loading shell (gated on !latestEntry) never rendered. The user saw the previous entry's content under the new entryId until the fetch resolved. Extended the latch effect so loading=true with both entry and notFound falsy clears both latches, letting the loading shell render against the new entryId. Close-after-fetch transitions still work because the loading flag drops to false before the close keyframes.
Contributor
Author
|
@codex review |
The displayTotal-on-error path stopped the Pagination from disagreeing with the EmptyState, but the aria-live count footer kept rendering "No entries" against the same zeroed total, contradicting the "failed to load" error the table was showing. Screen-reader users heard "No entries" while sighted users saw a failure message. Conditionally rendering the footer only when isError is false removes the mixed-signal announcement; the visible table already carries the error state so there is no information loss.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 95f4c60. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95f4c60f89
ℹ️ 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".
… errors Two error-path follow-ups to the displayTotal work: entryOnPage was reading directly from pageEntries, which keepPreviousData keeps populated across a failed refetch. With the list error EmptyState showing in the table, the detail drawer was still resolving the deep-linked id against that stale slice and rendering the old row. Gated entryOnPage on !isError so the single-entry fetch (independent of the list query) becomes the only data source while the list is in a failure state. Non-404 fetch failures for a deep-linked entryId left selectedEntry null and entryNotFound false (the latter requires isSuccess plus entry===null for a positive 404 result). Drawer rendered with open=true but no content branch, returning null while entryId sat in the URL with no panel and no close affordance. Added a loadError prop that derives from entryFetch.isError, mirrored the latestNotFound latch for animation parity, and a new error branch in the drawer that renders the same panel chrome with a com_audit_detail_load_error message + close buttons. The user can now dismiss the drawer and clear the bad URL.
The LibreChat backend treats actorId/targetPrincipalId as deprecated aliases for actorQuery/targetQuery (substring matches on actorName / targetName), logging a warning per request and slated for removal in a future release. Renamed the BFF schema fields and the buildFilters emit-path to the canonical names so the proxy stays off the deprecation path and survives the alias removal. UI state and labels keep their internal id-flavored names because the wire format is the only thing the backend sees.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b4442f1d9
ℹ️ 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".
Contributor
Author
|
@claude review |
Contributor
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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
2 participants
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
Sibling PR: danny-avila/LibreChat#13087
Adds an Audit Log tab on the Grants page (
/grants?tab=audit-log) that surfaces every SystemGrant assign and revoke event from the LibreChat admin backend. The tab provides faceted filters (action chips, date range, actor name, target name, target type, capability), debounced search across all denormalized name fields, offset-based pagination using the shared<Pagination>component (50 entries per page), and a side panel with copy-to-clipboard buttons for every ID-like field, an optional before/after capability diff, and a permalink that copies a canonical?tab=audit-log&entryId=...URL.CSV export goes through the backend's streaming
/export.csvendpoint so result sets of any size are handled by a single code path. The client-sideauditLogToCsvserializer is kept and tested as the contract the server is expected to satisfy.The audit log tab is gated on
SystemCapabilities.READ_AUDIT_LOG. The tab trigger, content slot, and body render are hidden when the user does not hold that cap. A stale?tab=audit-logURL on a session without the cap silently falls back to the management tab.This PR depends on
@librechat/data-schemas@0.0.52(added by the sibling LibreChat PR), which exports theREAD_AUDIT_LOGcapability. The version pin inpackage.jsonhas been bumped accordingly.bun installwill fail in CI until that version publishes to npm; once it does, this PR can merge.Change Type
Testing
Local Vitest suite:
bun run testruns 176 unit tests including CSV serializer coverage (formula-injection defanging with leading whitespace, NBSP, and BOM stripping plus\nand|triggers in addition to=+-@\t\r, BOM, CRLF, non-ASCII round-trip, empty entries, RFC 4180 quoting),localDayBoundaryIsoround-trip for the date filter, anduseDebouncedFilterbehavior.bun run sort-importsclean (181 files), ESLint clean, TypeScript clean against a locally-linked@librechat/data-schemasfrom the sibling LC branch.Manual:
/grants?tab=audit-logrenders the empty state when no entries existstaleTimecom_a11y_copy_failed${origin}/grants?tab=audit-log&entryId=...URL, not the current filter-laden href?entryId=<id>opens the side panel on cold load even when the entry is not on the current page (a single-entry fetch viagetAuditLogEntryFnpopulates the drawer). When the entry does not exist, the drawer renders a "not found" empty state instead of staying emptyACCESS_ADMINbut withoutREAD_AUDIT_LOGdoes not see the audit log tab; navigating directly to?tab=audit-logfalls back to the management tabTest Configuration
/api/admin/audit-log(see parallel PR)@librechat/data-schemas@0.0.52(published by the sibling PR) forSystemCapabilities.READ_AUDIT_LOGChecklist
Note
Medium Risk
Touches admin authorization (
READ_AUDIT_LOGBFF gate) and ships CSP on HTML responses (report-only unlessADMIN_PANEL_CSP_ENFORCE); depends on unpublished LibreChat audit-log APIs until the sibling backend PR lands.Overview
Ships a read audit log experience on Grants: the tab is back behind
read:audit_log, with BFF calls to/api/admin/audit-log(paginated list, single entry, CSV export) replacing stubs, and@librechat/data-schemas/librechat-data-providerbumped to 0.0.52 / 0.8.503.Audit log UI is rebuilt: multi-select action filters, click-ui date pickers with local day boundaries, debounced actor/target/capability filters, 50-row pagination, clickable rows, and an
AuditLogDetailDrawer(loading / not-found / error shells, before/after diff, permalink + copy). Deep links use?tab=audit-log&entryId=...; export always hits the backend so large result sets are not truncated.Supporting changes:
READ_AUDIT_LOGand category shim until upstream publishes the constant;useDebouncedFilter; grant tables swap custom badge classes for click-uiBadge; HTML responses get CSP (report-only by default), HSTS, frame denial, and related headers inserver.ts; drawer/overlay CSS and i18n for audit/a11y strings.Reviewed by Cursor Bugbot for commit 1b4442f. Bugbot is set up for automated code reviews on this repo. Configure here.