Skip to content

fix(ai-gateway): sanitize request logs#2807

Closed
kilo-code-bot[bot] wants to merge 2 commits intomainfrom
fix-api-request-log-nul-sanitization
Closed

fix(ai-gateway): sanitize request logs#2807
kilo-code-bot[bot] wants to merge 2 commits intomainfrom
fix-api-request-log-nul-sanitization

Conversation

@kilo-code-bot
Copy link
Copy Markdown
Contributor

@kilo-code-bot kilo-code-bot Bot commented Apr 24, 2026

Summary

  • Sanitize NUL bytes from gateway request bodies before inserting api_request_log rows.
  • Move stripNulBytesInPlace into a shared ai-gateway utility so usage logging and request logging use the same DB-boundary sanitizer.

Verification

  • Not manually tested; this is a DB-boundary sanitization change with no UI path.

Visual Changes

N/A

Reviewer Notes

  • pnpm --filter web typecheck passes locally.
  • pnpm test -- apps/web/src/lib/ai-gateway/processUsage.test.ts could not run locally because Docker is unavailable for the required test Postgres instance.


function sanitizedRequestBody(request: GatewayRequest): GatewayRequest['body'] {
const dirtyFields: string[] = [];
stripNulBytesInPlace(request.body, dirtyFields);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@kilo-code-bot
Copy link
Copy Markdown
Contributor Author

kilo-code-bot Bot commented Apr 24, 2026

Code Review Summary

Status: 1 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/web/src/lib/ai-gateway/strip-nul-bytes.ts 17 The recursive sanitizer skips string elements inside arrays, so request payloads with NUL bytes in arrays like models or transforms can still fail api_request_log.request inserts.
Other Observations (not in diff)

None.

Files Reviewed (1 files)
  • apps/web/src/lib/ai-gateway/strip-nul-bytes.ts - 1 issue

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}]`);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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('');
@chrarnoldus chrarnoldus deleted the fix-api-request-log-nul-sanitization branch April 28, 2026 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants