fix(ai-gateway): sanitize request logs#2807
Conversation
|
|
||
| function sanitizedRequestBody(request: GatewayRequest): GatewayRequest['body'] { | ||
| const dirtyFields: string[] = []; | ||
| stripNulBytesInPlace(request.body, dirtyFields); |
There was a problem hiding this comment.
WARNING: Nested request fields are still unsanitized
stripNulBytesInPlace only touches the top-level properties on request.body, but gateway payloads keep the actual prompt text inside nested structures like messages[].content, input[].content, and tool payloads. A NUL byte in one of those strings will still make the api_request_log.request jsonb insert fail, so this change does not fully cover the bug the PR is trying to fix.
There was a problem hiding this comment.
Fixed in f56d7e5 by making stripNulBytesInPlace recurse through arrays and plain objects, so nested prompt/tool strings are sanitized before inserting api_request_log.request.
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)None. Files Reviewed (1 files)
Fix these issues in Kilo Cloud Reviewed by gpt-5.4-20260305 · 293,697 tokens |
| function stripNulBytesFromValue(value: unknown, dirtyFields: string[], path: string): void { | ||
| if (Array.isArray(value)) { | ||
| value.forEach((item, index) => { | ||
| stripNulBytesFromValue(item, dirtyFields, `${path}[${index}]`); |
There was a problem hiding this comment.
WARNING: Arrays of strings still bypass sanitization
The new recursion fixes nested objects, but this branch immediately recurses into each array element without handling the case where the element itself is a string. Values like transforms: ['bad\u0000value'], models: ['bad\u0000value'], or any other string array in the request body will return early at isPlainObject and keep the NUL byte, so api_request_log.request inserts can still fail for those payloads.
| for (const [key, item] of Object.entries(value)) { | ||
| const itemPath = path ? `${path}.${key}` : key; | ||
| if (typeof item === 'string' && item.indexOf('\u0000') >= 0) { | ||
| value[key] = item.split('\u0000').join(''); |
Summary
api_request_logrows.stripNulBytesInPlaceinto a shared ai-gateway utility so usage logging and request logging use the same DB-boundary sanitizer.Verification
Visual Changes
N/A
Reviewer Notes
pnpm --filter web typecheckpasses locally.pnpm test -- apps/web/src/lib/ai-gateway/processUsage.test.tscould not run locally because Docker is unavailable for the required test Postgres instance.