feat(platform): add audit log immutability, AI usage logging, and export#1405
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR introduces comprehensive audit logging infrastructure for AI usage tracking and compliance verification. The changes add admin-only CSV/JSON export functionality for audit logs via a new Convex Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Implement tamper-evident hash chain for audit logs (SHA-256 integrity hashes linking each entry to its predecessor), hash chain verification endpoint (admin-only), CSV/JSON export with filters, configurable per-org retention via governance policy, and AI usage audit logging for agent completions, budget-blocked requests, and OpenAI-compat mode. Closes #1364 Closes #1218
larryro
force-pushed
the
feat/issue-1364-audit-immutable
branch
from
0290330 to
0e73c9f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/convex/openai_compat/internal_actions.ts (1)
391-439:⚠️ Potential issue | 🟠 MajorAudit logging is incorrectly gated by token usage.
At Line 391, the token check controls both ledger increment and
ai.completionlogging. Completions with zero/missing token usage will not be audited.Proposed fix
- if (inputTokens > 0 || outputTokens > 0) { - const costCents = estimateCostCents(modelId, inputTokens, outputTokens); + const hasTokenUsage = inputTokens > 0 || outputTokens > 0; + const costCents = hasTokenUsage + ? estimateCostCents(modelId, inputTokens, outputTokens) + : undefined; + if (hasTokenUsage) { await ctx .runMutation( internal.governance.internal_mutations.incrementUsageLedger, @@ }); + } - // AI audit log for OpenAI-compat client tool mode - await ctx - .runMutation(internal.audit_logs.internal_mutations.createAuditLog, { + // AI audit log for OpenAI-compat client tool mode + await ctx + .runMutation(internal.audit_logs.internal_mutations.createAuditLog, { organizationId: args.organizationId, actorId: args.userId ?? 'system', @@ metadata: { model: modelId, inputTokens, outputTokens, totalTokens: inputTokens + outputTokens, - costEstimateCents: costCents, + ...(costCents != null && { costEstimateCents: costCents }), threadId, agentType: 'openai_compat', toolCallCount: toolCalls.length, }, - }) - .catch((error) => { - console.error( - '[OpenAI-compat:clientTools] Failed to write AI audit log:', - error, - ); - }); - } + }) + .catch((error) => { + console.error( + '[OpenAI-compat:clientTools] Failed to write AI audit log:', + error, + ); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/openai_compat/internal_actions.ts` around lines 391 - 439, The audit log creation for ai.completion is incorrectly inside the token-usage conditional, so completions with zero or missing tokens never get audited; move the call to ctx.runMutation(internal.audit_logs.internal_mutations.createAuditLog, ...) so it always executes (or run it in a separate branch) while keeping the usage ledger increment via ctx.runMutation(internal.governance.internal_mutations.incrementUsageLedger, ...) only when inputTokens > 0 || outputTokens > 0; ensure the metadata still includes inputTokens/outputTokens/totalTokens and toolCalls.length (use threadId, modelId, args.userId ?? 'system' as before).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/settings/audit-logs/components/audit-log-table.tsx`:
- Around line 47-54: The onSuccess handler for the export action currently only
skips window.open when data.url is null but still shows the success toast;
change onSuccess (the callback handling data.url) to treat a missing URL as a
failure: if data.url is falsy return early and show an error toast (use toast
with variant: 'destructive' or 'error', an appropriate title via
t('logs.audit.export.failed') or a clear message, and optionally the original
data.fileName as context), otherwise proceed to window.open(...) and the success
toast; ensure you reference the same onSuccess function, data.url, window.open
call and toast invocation when making the change.
- Around line 260-264: The AI-only branch hides real metadata keys; update the
render so that when selectedLog.category === 'ai' you still render the generic
metadata block (using selectedLog.metadata) in addition to AiMetadataSection, or
extend AiMetadataSection to include any unknown keys; specifically ensure
selectedLog.metadata keys such as threadId, agentType, and toolCallCount are
included by either (a) changing the condition around AiMetadataSection to not
short-circuit the generic metadata render, or (b) enhancing AiMetadataSection to
iterate over and display all metadata keys when present (falling back to the
existing specialized view for known fields).
In `@services/platform/convex/audit_logs/export_audit_logs.ts`:
- Around line 52-65: The CSV headers array (headers) and the row construction
(around the code that writes rows at the current export block) omit
metadata-related fields—add 'metadata', 'changedFields', 'previousState', and
'newState' to the headers and include corresponding values when building each
CSV row (serialize objects/arrays as JSON strings so nested data is preserved);
in addition, if you want AI-specific quick-access columns, extract
model/tokens/cost from audit.metadata into separate columns or ensure they are
present inside the serialized metadata field. Update the headers variable and
the row-generation logic (the code that maps an audit record into CSV columns)
to output these serialized fields.
- Around line 75-83: The CSV exporter currently builds a string "str" and only
quotes values with commas/quotes/newlines but doesn't neutralize formula
injection; update the logic in export_audit_logs.ts where "str" is created to
detect values that begin with '=', '+', '-', or '@' (e.g. using a regex like
/^[=+\-@]/) and prefix them with a single quote before applying the existing CSV
escaping (doubling internal quotes and wrapping in quotes if needed); ensure the
prefixing happens after converting non-strings to String(val) but before the
replace(/"/g, '""') and the existing contains check so formula-like cells are
safely quoted/escaped when returned.
In `@services/platform/convex/audit_logs/helpers.ts`:
- Around line 145-155: The current code sets previousHash from
lastEntry?.integrityHash ?? '' which makes the first post-rollout entry a new
genesis when the latest row has no hash; update the insertion logic that
computes previousHash (the query that sets lastEntry and the previousHash
variable) to instead find the most recent auditLogs row with a non-empty
integrityHash (e.g., add a filter like integrityHash != '' or use an index/query
that returns only hashed rows) and use that hash as previousHash; additionally
update verify_integrity.ts to explicitly skip and report any leading unhashed
prefix (count how many unhashed rows were skipped) and start
recomputing/verification from the first row that contains a non-empty
integrityHash so legacy unhashed rows do not break the chain.
In `@services/platform/convex/audit_logs/verify_integrity.ts`:
- Around line 50-52: The verification can falsely accept a truncated suffix
because previousExpectedHash is seeded from the first loaded record of a paged
fetch; change the logic so you detect and handle truncated fetches instead of
treating a page boundary as the chain root: when using maxEntries
(args.maxEntries) and iterating records (verifiedCount, previousExpectedHash),
ensure you either (A) fetch additional pages until you reach the true genesis
(previousHash === ''), or (B) if the fetch returns exactly maxEntries and you
have not seen the genesis, treat the chain as truncated and return valid: false
(or an explicit "incomplete" result). Also fix the seed for previousExpectedHash
so it is derived consistently from the chain traversal (e.g., start from the
newest record's stored hash and walk backwards) rather than assuming the first
page's previousHash is the root.
In `@services/platform/convex/lib/agent_completion/on_agent_complete.ts`:
- Around line 196-234: The audit-log creation call using
ctx.runMutation(internal.audit_logs.internal_mutations.createAuditLog) can be
skipped when usage/cost conditions gate that block, causing
'ai.completion_failed' events with zero-usage to be silently dropped; move or
duplicate the audit-log promise so that a call is always scheduled for failures
(result.error) regardless of the token/cost branch, or change the surrounding
condition to always allow audit logging when result.error is truthy; ensure the
metadata still includes available fields (model, provider, durationMs, threadId,
agentType, agentSlug) and keep the existing .catch that logs failures using
agentType and threadId to preserve error visibility.
In `@services/platform/convex/lib/helpers/audit_hash.test.ts`:
- Around line 30-33: Add a persistence round-trip test that verifies optional
fields dropped by persistence produce the same canonical hash: create an object
with an optional field set to undefined (e.g., { errorMessage: undefined }),
compute its canonical/hash via canonicalizeForTest (and any audit hash helper
used elsewhere like computeAuditHash if present), then simulate the stored form
with the optional field omitted (e.g., {}), recompute the canonical/hash for
that stored form, and assert the two hashes (or canonical strings) are equal;
add this case alongside the existing null/undefined checks and also add
equivalent assertions in the related block referenced around lines 84-109 to
cover the same persistence round-trip scenario for success rows.
In `@services/platform/convex/lib/helpers/audit_hash.ts`:
- Around line 30-48: The canonicalize function must match Convex/JSON
persistence: object keys with value === undefined should be omitted (so in the
isRecord branch filter out keys whose value is undefined in addition to
EXCLUDED_FIELDS) and array elements that are undefined should serialize as null
(so in the Array.isArray branch map undefined items to canonicalize(null) or the
'null' token rather than the current 'undefined'). Update canonicalize
accordingly (refer to canonicalize, EXCLUDED_FIELDS) so createAuditLog and
verify_integrity.ts produce matching hashes.
In `@services/platform/lib/shared/schemas/governance.ts`:
- Around line 121-123: The PolicyType/POLICY_TYPES union is missing the new
'audit_retention' variant, causing type drift with auditRetentionConfigSchema;
update the shared POLICY_TYPES array (and the derived PolicyType union/enum) to
include 'audit_retention' so clients and server use the same policy kinds, and
ensure any switch/handler that matches on PolicyType (or functions like
validatePolicy or parsePolicy) accepts this new type and references
auditRetentionConfigSchema for validation.
In `@services/platform/messages/de-CH.json`:
- Around line 53-94: Remove the duplicate localization entries under the
"logs.audit" subtree in the de-CH file so automatic fallback to the base "de"
locale works: delete the entire "aiMetadata" object, the "categories.ai" key,
the whole "export" object, the whole "verification" object, and the whole
"retention" object (i.e., remove "logs.audit.aiMetadata",
"logs.audit.categories.ai", "logs.audit.export", "logs.audit.verification", and
"logs.audit.retention").
In `@services/platform/messages/en.json`:
- Line 1696: The "validDescription" message currently uses a plain string
("{count} entries verified, no tampering detected") which yields incorrect
grammar for singular; update the "validDescription" value to use ICU
pluralization so it renders correctly for count=1 vs other counts (e.g., use a
plural format keyed on {count} with singular and plural variants and include the
count placeholder in each branch). Locate the "validDescription" JSON key and
replace its string with an ICU pluralized message that handles one vs other
(ensuring the placeholder {count} remains present).
---
Outside diff comments:
In `@services/platform/convex/openai_compat/internal_actions.ts`:
- Around line 391-439: The audit log creation for ai.completion is incorrectly
inside the token-usage conditional, so completions with zero or missing tokens
never get audited; move the call to
ctx.runMutation(internal.audit_logs.internal_mutations.createAuditLog, ...) so
it always executes (or run it in a separate branch) while keeping the usage
ledger increment via
ctx.runMutation(internal.governance.internal_mutations.incrementUsageLedger,
...) only when inputTokens > 0 || outputTokens > 0; ensure the metadata still
includes inputTokens/outputTokens/totalTokens and toolCalls.length (use
threadId, modelId, args.userId ?? 'system' as before).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 53eb8955-3860-40c6-bec9-dffff7aae07b
⛔ Files ignored due to path filters (1)
services/platform/convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (22)
services/platform/app/features/settings/audit-logs/components/audit-log-table.tsxservices/platform/app/routes/dashboard/$id/settings/logs.tsxservices/platform/convex/audit_logs/actions.tsservices/platform/convex/audit_logs/export_audit_logs.tsservices/platform/convex/audit_logs/helpers.tsservices/platform/convex/audit_logs/internal_mutations.tsservices/platform/convex/audit_logs/internal_queries.tsservices/platform/convex/audit_logs/schema.tsservices/platform/convex/audit_logs/validators.tsservices/platform/convex/audit_logs/verify_integrity.tsservices/platform/convex/governance/mutations.tsservices/platform/convex/governance/schema.tsservices/platform/convex/lib/agent_chat/start_agent_chat.tsservices/platform/convex/lib/agent_completion/on_agent_complete.tsservices/platform/convex/lib/helpers/audit_hash.test.tsservices/platform/convex/lib/helpers/audit_hash.tsservices/platform/convex/openai_compat/internal_actions.tsservices/platform/lib/shared/schemas/governance.tsservices/platform/messages/de-AT.jsonservices/platform/messages/de-CH.jsonservices/platform/messages/de.jsonservices/platform/messages/en.json
Comment on lines
+47
to
+54
| onSuccess: (data) => { | ||
| if (data.url) { | ||
| window.open(data.url, '_blank', 'noopener,noreferrer'); | ||
| } | ||
| toast({ | ||
| title: t('logs.audit.export.complete'), | ||
| description: data.fileName, | ||
| }); |
There was a problem hiding this comment.
Treat null download URLs as a failed export.
Lines 48-54 only skip window.open when data.url is null, but they still show the success toast. That leaves the user with “export complete” and no file. Return early with an error toast, or expose a fallback download path, when the action cannot produce a URL.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/audit-logs/components/audit-log-table.tsx`
around lines 47 - 54, The onSuccess handler for the export action currently only
skips window.open when data.url is null but still shows the success toast;
change onSuccess (the callback handling data.url) to treat a missing URL as a
failure: if data.url is falsy return early and show an error toast (use toast
with variant: 'destructive' or 'error', an appropriate title via
t('logs.audit.export.failed') or a clear message, and optionally the original
data.fileName as context), otherwise proceed to window.open(...) and the success
toast; ensure you reference the same onSuccess function, data.url, window.open
call and toast invocation when making the change.
Comment on lines
+260
to
264
| {selectedLog.category === 'ai' && selectedLog.metadata ? ( | ||
| <AiMetadataSection metadata={selectedLog.metadata} t={t} /> | ||
| ) : ( | ||
| selectedLog.metadata && | ||
| Object.keys(selectedLog.metadata).length > 0 && ( |
There was a problem hiding this comment.
The specialized AI view hides real metadata fields.
Line 260 bypasses the generic metadata section for every ai log, but Lines 292-356 only render a subset of keys. The OpenAI-compat writer currently emits threadId, agentType, and toolCallCount, so those values disappear from the dialog instead of being inspectable.
Also applies to: 292-356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/audit-logs/components/audit-log-table.tsx`
around lines 260 - 264, The AI-only branch hides real metadata keys; update the
render so that when selectedLog.category === 'ai' you still render the generic
metadata block (using selectedLog.metadata) in addition to AiMetadataSection, or
extend AiMetadataSection to include any unknown keys; specifically ensure
selectedLog.metadata keys such as threadId, agentType, and toolCallCount are
included by either (a) changing the condition around AiMetadataSection to not
short-circuit the generic metadata render, or (b) enhancing AiMetadataSection to
iterate over and display all metadata keys when present (falling back to the
existing specialized view for known fields).
Comment on lines
+52
to
+65
| const headers = [ | ||
| 'timestamp', | ||
| 'action', | ||
| 'category', | ||
| 'actorEmail', | ||
| 'actorId', | ||
| 'actorType', | ||
| 'actorRole', | ||
| 'resourceType', | ||
| 'resourceId', | ||
| 'resourceName', | ||
| 'status', | ||
| 'errorMessage', | ||
| ]; |
There was a problem hiding this comment.
CSV export drops the new audit details.
Lines 52-65 define the entire CSV surface, and it excludes metadata, changedFields, previousState, and newState. For the new ai category, that means model/token/cost details never make it into CSV exports, so one of the headline audit additions is missing from this format.
Also applies to: 89-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/audit_logs/export_audit_logs.ts` around lines 52 -
65, The CSV headers array (headers) and the row construction (around the code
that writes rows at the current export block) omit metadata-related fields—add
'metadata', 'changedFields', 'previousState', and 'newState' to the headers and
include corresponding values when building each CSV row (serialize
objects/arrays as JSON strings so nested data is preserved); in addition, if you
want AI-specific quick-access columns, extract model/tokens/cost from
audit.metadata into separate columns or ensure they are present inside the
serialized metadata field. Update the headers variable and the row-generation
logic (the code that maps an audit record into CSV columns) to output these
serialized fields.
Comment on lines
+75
to
+83
| const str = | ||
| typeof val === 'string' | ||
| ? val | ||
| : typeof val === 'number' || typeof val === 'boolean' | ||
| ? String(val) | ||
| : JSON.stringify(val); | ||
| if (str.includes(',') || str.includes('"') || str.includes('\n')) { | ||
| return '"' + str.replace(/"/g, '""') + '"'; | ||
| } |
There was a problem hiding this comment.
Harden CSV cells against formula injection.
The current escaping only handles CSV syntax. It still emits values starting with =, +, -, or @ unchanged, so exported audit data can execute as formulas in Excel/Sheets. Prefix formula-like cells before writing them out.
Suggested fix
const str =
typeof val === 'string'
? val
: typeof val === 'number' || typeof val === 'boolean'
? String(val)
: JSON.stringify(val);
- if (str.includes(',') || str.includes('"') || str.includes('\n')) {
- return '"' + str.replace(/"/g, '""') + '"';
+ const safe =
+ /^[=+\-@]/.test(str) ? `'${str}` : str;
+ if (
+ safe.includes(',') ||
+ safe.includes('"') ||
+ safe.includes('\n')
+ ) {
+ return '"' + safe.replace(/"/g, '""') + '"';
}
- return str;
+ return safe;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const str = | |
| typeof val === 'string' | |
| ? val | |
| : typeof val === 'number' || typeof val === 'boolean' | |
| ? String(val) | |
| : JSON.stringify(val); | |
| if (str.includes(',') || str.includes('"') || str.includes('\n')) { | |
| return '"' + str.replace(/"/g, '""') + '"'; | |
| } | |
| const str = | |
| typeof val === 'string' | |
| ? val | |
| : typeof val === 'number' || typeof val === 'boolean' | |
| ? String(val) | |
| : JSON.stringify(val); | |
| const safe = | |
| /^[=+\-@]/.test(str) ? `'${str}` : str; | |
| if ( | |
| safe.includes(',') || | |
| safe.includes('"') || | |
| safe.includes('\n') | |
| ) { | |
| return '"' + safe.replace(/"/g, '""') + '"'; | |
| } | |
| return safe; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/audit_logs/export_audit_logs.ts` around lines 75 -
83, The CSV exporter currently builds a string "str" and only quotes values with
commas/quotes/newlines but doesn't neutralize formula injection; update the
logic in export_audit_logs.ts where "str" is created to detect values that begin
with '=', '+', '-', or '@' (e.g. using a regex like /^[=+\-@]/) and prefix them
with a single quote before applying the existing CSV escaping (doubling internal
quotes and wrapping in quotes if needed); ensure the prefixing happens after
converting non-strings to String(val) but before the replace(/"/g, '""') and the
existing contains check so formula-like cells are safely quoted/escaped when
returned.
Comment on lines
+145
to
+155
| // Look up the most recent audit log for this organization to chain hashes | ||
| const lastEntry = await ctx.db | ||
| .query('auditLogs') | ||
| .withIndex('by_organizationId_and_timestamp', (q) => | ||
| q.eq('organizationId', args.organizationId), | ||
| ) | ||
| .order('desc') | ||
| .first(); | ||
|
|
||
| const previousHash = lastEntry?.integrityHash ?? ''; | ||
|
|
There was a problem hiding this comment.
Handle legacy audit rows before restarting the chain.
lastEntry?.integrityHash ?? '' turns the first post-rollout entry into a new genesis block whenever the latest existing row has no hash yet. verify_integrity.ts recomputes across stored rows, so orgs with pre-existing audit logs will report a broken chain immediately even without tampering. Either backfill hashes before enabling verification, or have verification explicitly skip/report the unhashed prefix and start from the first hashed row.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/audit_logs/helpers.ts` around lines 145 - 155, The
current code sets previousHash from lastEntry?.integrityHash ?? '' which makes
the first post-rollout entry a new genesis when the latest row has no hash;
update the insertion logic that computes previousHash (the query that sets
lastEntry and the previousHash variable) to instead find the most recent
auditLogs row with a non-empty integrityHash (e.g., add a filter like
integrityHash != '' or use an index/query that returns only hashed rows) and use
that hash as previousHash; additionally update verify_integrity.ts to explicitly
skip and report any leading unhashed prefix (count how many unhashed rows were
skipped) and start recomputing/verification from the first row that contains a
non-empty integrityHash so legacy unhashed rows do not break the chain.
Comment on lines
+30
to
+33
| it('handles null and undefined', () => { | ||
| expect(canonicalizeForTest(null)).toBe('null'); | ||
| expect(canonicalizeForTest(undefined)).toBe('undefined'); | ||
| }); |
There was a problem hiding this comment.
Add a persistence round-trip case for optional fields.
The new suite never checks the shape that comes back from Convex after optional fields are omitted. A case like hashing { errorMessage: undefined } and then recomputing from the stored {} form would catch the mismatch that breaks verification for common success rows. As per coding guidelines, "Tests should cover happy paths, edge cases, and error conditions".
Also applies to: 84-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/lib/helpers/audit_hash.test.ts` around lines 30 -
33, Add a persistence round-trip test that verifies optional fields dropped by
persistence produce the same canonical hash: create an object with an optional
field set to undefined (e.g., { errorMessage: undefined }), compute its
canonical/hash via canonicalizeForTest (and any audit hash helper used elsewhere
like computeAuditHash if present), then simulate the stored form with the
optional field omitted (e.g., {}), recompute the canonical/hash for that stored
form, and assert the two hashes (or canonical strings) are equal; add this case
alongside the existing null/undefined checks and also add equivalent assertions
in the related block referenced around lines 84-109 to cover the same
persistence round-trip scenario for success rows.
Comment on lines
+30
to
+48
| function canonicalize(value: unknown): string { | ||
| if (value === undefined) { | ||
| return 'undefined'; | ||
| } | ||
|
|
||
| if (value === null) { | ||
| return 'null'; | ||
| } | ||
|
|
||
| if (Array.isArray(value)) { | ||
| return '[' + value.map((item) => canonicalize(item)).join(',') + ']'; | ||
| } | ||
|
|
||
| if (isRecord(value)) { | ||
| const sortedKeys = Object.keys(value).sort(); | ||
| const entries = sortedKeys | ||
| .filter((key) => !EXCLUDED_FIELDS.has(key)) | ||
| .map((key) => JSON.stringify(key) + ':' + canonicalize(value[key])); | ||
| return '{' + entries.join(',') + '}'; |
There was a problem hiding this comment.
Make canonicalization match the persisted Convex document shape.
Right now object properties with undefined are hashed as "field":undefined, but those fields are omitted once the audit row is stored. createAuditLog passes many optional fields this way, and verify_integrity.ts recomputes from the stored row, so perfectly valid entries will fail verification whenever an optional field is absent. The canonical form needs to mirror JSON/Convex persistence semantics: drop undefined object keys and normalize undefined array elements the same way JSON does.
Suggested fix
function canonicalize(value: unknown): string {
- if (value === undefined) {
- return 'undefined';
- }
-
if (value === null) {
return 'null';
}
if (Array.isArray(value)) {
- return '[' + value.map((item) => canonicalize(item)).join(',') + ']';
+ return (
+ '[' +
+ value
+ .map((item) => (item === undefined ? 'null' : canonicalize(item)))
+ .join(',') +
+ ']'
+ );
}
if (isRecord(value)) {
const sortedKeys = Object.keys(value).sort();
const entries = sortedKeys
- .filter((key) => !EXCLUDED_FIELDS.has(key))
+ .filter(
+ (key) => !EXCLUDED_FIELDS.has(key) && value[key] !== undefined,
+ )
.map((key) => JSON.stringify(key) + ':' + canonicalize(value[key]));
return '{' + entries.join(',') + '}';
}
return JSON.stringify(value);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function canonicalize(value: unknown): string { | |
| if (value === undefined) { | |
| return 'undefined'; | |
| } | |
| if (value === null) { | |
| return 'null'; | |
| } | |
| if (Array.isArray(value)) { | |
| return '[' + value.map((item) => canonicalize(item)).join(',') + ']'; | |
| } | |
| if (isRecord(value)) { | |
| const sortedKeys = Object.keys(value).sort(); | |
| const entries = sortedKeys | |
| .filter((key) => !EXCLUDED_FIELDS.has(key)) | |
| .map((key) => JSON.stringify(key) + ':' + canonicalize(value[key])); | |
| return '{' + entries.join(',') + '}'; | |
| function canonicalize(value: unknown): string { | |
| if (value === null) { | |
| return 'null'; | |
| } | |
| if (Array.isArray(value)) { | |
| return ( | |
| '[' + | |
| value | |
| .map((item) => (item === undefined ? 'null' : canonicalize(item))) | |
| .join(',') + | |
| ']' | |
| ); | |
| } | |
| if (isRecord(value)) { | |
| const sortedKeys = Object.keys(value).sort(); | |
| const entries = sortedKeys | |
| .filter( | |
| (key) => !EXCLUDED_FIELDS.has(key) && value[key] !== undefined, | |
| ) | |
| .map((key) => JSON.stringify(key) + ':' + canonicalize(value[key])); | |
| return '{' + entries.join(',') + '}'; | |
| } | |
| return JSON.stringify(value); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/convex/lib/helpers/audit_hash.ts` around lines 30 - 48, The
canonicalize function must match Convex/JSON persistence: object keys with value
=== undefined should be omitted (so in the isRecord branch filter out keys whose
value is undefined in addition to EXCLUDED_FIELDS) and array elements that are
undefined should serialize as null (so in the Array.isArray branch map undefined
items to canonicalize(null) or the 'null' token rather than the current
'undefined'). Update canonicalize accordingly (refer to canonicalize,
EXCLUDED_FIELDS) so createAuditLog and verify_integrity.ts produce matching
hashes.
Comment on lines
+121
to
+123
| export const auditRetentionConfigSchema = z.object({ | ||
| retentionDays: z.number().int().min(30).max(365), | ||
| }); |
There was a problem hiding this comment.
Policy type union is out of sync with the new audit_retention schema.
You added auditRetentionConfigSchema, but shared POLICY_TYPES/PolicyType still excludes 'audit_retention'. This causes client/server type drift for governance policy handling.
🔧 Suggested fix
export const POLICY_TYPES = [
'system_prompt',
'budgets',
'default_models',
'upload_policy',
'retention_policy',
'feature_flags',
'pii_config',
'model_access',
+ 'audit_retention',
] as const;As per coding guidelines, “ALWAYS share validation schemas between client and server using Zod… DO NOT duplicate validation logic.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const auditRetentionConfigSchema = z.object({ | |
| retentionDays: z.number().int().min(30).max(365), | |
| }); | |
| export const POLICY_TYPES = [ | |
| 'system_prompt', | |
| 'budgets', | |
| 'default_models', | |
| 'upload_policy', | |
| 'retention_policy', | |
| 'feature_flags', | |
| 'pii_config', | |
| 'model_access', | |
| 'audit_retention', | |
| ] as const; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/lib/shared/schemas/governance.ts` around lines 121 - 123,
The PolicyType/POLICY_TYPES union is missing the new 'audit_retention' variant,
causing type drift with auditRetentionConfigSchema; update the shared
POLICY_TYPES array (and the derived PolicyType union/enum) to include
'audit_retention' so clients and server use the same policy kinds, and ensure
any switch/handler that matches on PolicyType (or functions like validatePolicy
or parsePolicy) accepts this new type and references auditRetentionConfigSchema
for validation.
Comment on lines
+53
to
+94
| "logs": { | ||
| "audit": { | ||
| "categories": { | ||
| "ai": "KI" | ||
| }, | ||
| "export": { | ||
| "csv": "CSV exportieren", | ||
| "json": "JSON exportieren", | ||
| "csvLabel": "Audit-Protokolle als CSV exportieren", | ||
| "jsonLabel": "Audit-Protokolle als JSON exportieren", | ||
| "inProgress": "Exportieren...", | ||
| "complete": "Export abgeschlossen", | ||
| "error": "Export fehlgeschlagen" | ||
| }, | ||
| "aiMetadata": { | ||
| "title": "KI-Nutzungsdetails", | ||
| "model": "Modell", | ||
| "provider": "Anbieter", | ||
| "inputTokens": "Eingabe-Token", | ||
| "outputTokens": "Ausgabe-Token", | ||
| "totalTokens": "Token gesamt", | ||
| "cost": "Geschätzte Kosten", | ||
| "duration": "Dauer", | ||
| "agent": "Agent", | ||
| "tools": "Verwendete Tools" | ||
| }, | ||
| "verification": { | ||
| "button": "Integrität prüfen", | ||
| "inProgress": "Prüfung läuft...", | ||
| "valid": "Kette gültig", | ||
| "validDescription": "{count} Einträge geprüft, keine Manipulation erkannt", | ||
| "broken": "Kette unterbrochen", | ||
| "brokenDescription": "Integritätsbruch erkannt bei Eintrag {logId}" | ||
| }, | ||
| "retention": { | ||
| "label": "Aufbewahrungsdauer", | ||
| "days": "Tage", | ||
| "minHint": "Minimum: 30 Tage", | ||
| "maxHint": "Maximum: 365 Tage", | ||
| "saved": "Aufbewahrungsrichtlinie gespeichert" | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
DE_FILE="$(fd -i '^de\.json$' services/platform/messages | head -n1)"
DE_CH_FILE="$(fd -i '^de-CH\.json$' services/platform/messages | head -n1)"
python - "$DE_FILE" "$DE_CH_FILE" <<'PY'
import json, sys
de_path, de_ch_path = sys.argv[1], sys.argv[2]
with open(de_path, "r", encoding="utf-8") as f:
de = json.load(f)
with open(de_ch_path, "r", encoding="utf-8") as f:
de_ch = json.load(f)
def get(d, path):
cur = d
for p in path:
if not isinstance(cur, dict) or p not in cur:
return None
cur = cur[p]
return cur
target = ["settings", "logs", "audit"]
de_audit = get(de, target) or {}
de_ch_audit = get(de_ch, target) or {}
def flatten(obj, prefix=""):
out = {}
if isinstance(obj, dict):
for k, v in obj.items():
key = f"{prefix}.{k}" if prefix else k
out.update(flatten(v, key))
else:
out[prefix] = obj
return out
de_flat = flatten(de_audit)
de_ch_flat = flatten(de_ch_audit)
same = sorted([k for k, v in de_ch_flat.items() if k in de_flat and de_flat[k] == v])
only_ch = sorted([k for k in de_ch_flat if k not in de_flat])
print("de-CH keys identical to de:")
for k in same:
print(" -", k)
print("\nde-CH keys missing in de (expected only if intentionally variant-local):")
for k in only_ch:
print(" -", k)
PYRepository: tale-project/tale
Length of output: 816
Remove all identical keys from de-CH.
All 28 keys in the settings.logs.audit subtree are identical to the base de locale and must be removed to enable automatic fallback. These include: aiMetadata.* (all subkeys), categories.ai, export.*, retention.*, and verification.*.
🧰 Tools
🪛 Checkov (3.2.513)
[low] 71-72: Base64 High Entropy String
(CKV_SECRET_6)
[low] 72-73: Base64 High Entropy String
(CKV_SECRET_6)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/de-CH.json` around lines 53 - 94, Remove the
duplicate localization entries under the "logs.audit" subtree in the de-CH file
so automatic fallback to the base "de" locale works: delete the entire
"aiMetadata" object, the "categories.ai" key, the whole "export" object, the
whole "verification" object, and the whole "retention" object (i.e., remove
"logs.audit.aiMetadata", "logs.audit.categories.ai", "logs.audit.export",
"logs.audit.verification", and "logs.audit.retention").
| "button": "Verify integrity", | ||
| "inProgress": "Verifying...", | ||
| "valid": "Chain valid", | ||
| "validDescription": "{count} entries verified, no tampering detected", |
There was a problem hiding this comment.
Fix singular/plural grammar in verification success text.
Line 1696 should use ICU pluralization; otherwise count=1 renders as “1 entries verified”.
💡 Suggested change
- "validDescription": "{count} entries verified, no tampering detected",
+ "validDescription": "{count, plural, one {# entry verified, no tampering detected} other {# entries verified, no tampering detected}}",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` at line 1696, The "validDescription"
message currently uses a plain string ("{count} entries verified, no tampering
detected") which yields incorrect grammar for singular; update the
"validDescription" value to use ICU pluralization so it renders correctly for
count=1 vs other counts (e.g., use a plural format keyed on {count} with
singular and plural variants and include the count placeholder in each branch).
Locate the "validDescription" JSON key and replace its string with an ICU
pluralized message that handles one vs other (ensuring the placeholder {count}
remains present).
Add missing Logs tab to settings navigation so audit logs are accessible from the UI. Resolve actor IDs to emails by querying the members list on the frontend and in the export action, so both the table and exported CSV/JSON show user emails instead of raw IDs.
This was referenced Apr 13, 2026
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
aiaudit log category with automatic logging for agent completions, budget-blocked requests, and OpenAI-compat client-tool modeTest plan
bun run --filter @tale/platform lint)bun run --filter @tale/platform typecheck)ai.budget_blockedaudit entriesCloses #1364
Closes #1218
Summary by CodeRabbit
Release Notes
New Features
Localization