diff --git a/apps/web/src/lib/code-reviews/db/code-reviews.test.ts b/apps/web/src/lib/code-reviews/db/code-reviews.test.ts index 22f2a944b9..cbc920a827 100644 --- a/apps/web/src/lib/code-reviews/db/code-reviews.test.ts +++ b/apps/web/src/lib/code-reviews/db/code-reviews.test.ts @@ -36,6 +36,8 @@ import { findPreviousCompletedReview, updateCodeReviewStatus, resetCodeReviewForRetry, + failReservedQueuedReview, + updatePreviousReviewSummary, } from './code-reviews'; const REPO = `test-org/session-continuation-${Date.now()}`; @@ -1970,6 +1972,49 @@ describe('resetCodeReviewForRetry', () => { }); expect(stored?.status).toBe('pending'); }); + + it('persists sanitized previous summaries while preserving null and valid markdown', async () => { + const reviewId = await insertReview('pending'); + + await updatePreviousReviewSummary(reviewId, { + body: '## Summary\nactual\0NUL, literal \\u0000, and 😀', + headSha: 'previous-head-sha', + }); + + const stored = await db.query.cloud_agent_code_reviews.findFirst({ + where: eq(cloud_agent_code_reviews.id, reviewId), + }); + expect(stored?.previous_summary_body).toBe( + '## Summary\nactual\ufffdNUL, literal \\u0000, and 😀' + ); + expect(stored?.previous_summary_head_sha).toBe('previous-head-sha'); + + await updatePreviousReviewSummary(reviewId, { body: null, headSha: null }); + + const cleared = await db.query.cloud_agent_code_reviews.findFirst({ + where: eq(cloud_agent_code_reviews.id, reviewId), + }); + expect(cleared?.previous_summary_body).toBeNull(); + expect(cleared?.previous_summary_head_sha).toBeNull(); + }); + + it('marks a reserved review failed when its dispatch error contains a NUL character', async () => { + const reservationId = crypto.randomUUID(); + const reviewId = await insertReview('queued', { + dispatch_reservation_id: reservationId, + }); + + await expect( + failReservedQueuedReview(reviewId, reservationId, 'Dispatch failed: actual\0NUL') + ).resolves.toBe(true); + + const stored = await db.query.cloud_agent_code_reviews.findFirst({ + where: eq(cloud_agent_code_reviews.id, reviewId), + }); + expect(stored?.status).toBe('failed'); + expect(stored?.dispatch_reservation_id).toBeNull(); + expect(stored?.error_message).toBe('Dispatch failed: actual\ufffdNUL'); + }); }); describe('listCodeReviews narrows the list DTO', () => { diff --git a/apps/web/src/lib/code-reviews/db/code-reviews.ts b/apps/web/src/lib/code-reviews/db/code-reviews.ts index 00435b7e2b..5af78dcb42 100644 --- a/apps/web/src/lib/code-reviews/db/code-reviews.ts +++ b/apps/web/src/lib/code-reviews/db/code-reviews.ts @@ -32,6 +32,7 @@ import { } from 'drizzle-orm'; import { captureException } from '@sentry/nextjs'; import { logExceptInTest } from '@/lib/utils.server'; +import { sanitizePostgresString } from '@/lib/sanitize-jsonb'; import { CreateReviewParamsSchema } from '../core'; import { assertCouncilCreationAllowed } from '../core/council-entitlement'; import { codeReviewLedgerIntent, settleCodeReviewLedgerRow } from '../code-review-ledger'; @@ -1213,7 +1214,7 @@ export async function failReservedQueuedReview( try { const updateData: Partial = { status: 'failed', - error_message: errorMessage, + error_message: sanitizePostgresString(errorMessage), dispatch_reservation_id: null, completed_at: new Date().toISOString(), updated_at: new Date().toISOString(), @@ -1363,7 +1364,7 @@ export async function updatePreviousReviewSummary( await db .update(cloud_agent_code_reviews) .set({ - previous_summary_body: summary.body, + previous_summary_body: summary.body === null ? null : sanitizePostgresString(summary.body), previous_summary_head_sha: summary.headSha, updated_at: new Date().toISOString(), }) diff --git a/apps/web/src/lib/sanitize-jsonb.test.ts b/apps/web/src/lib/sanitize-jsonb.test.ts index d2a9afd305..6ee80a303a 100644 --- a/apps/web/src/lib/sanitize-jsonb.test.ts +++ b/apps/web/src/lib/sanitize-jsonb.test.ts @@ -1,5 +1,12 @@ import { describe, expect, test } from '@jest/globals'; -import { sanitizeJsonbValue } from './sanitize-jsonb'; +import { sanitizeJsonbValue, sanitizePostgresString } from './sanitize-jsonb'; + +describe('sanitizePostgresString', () => { + test('replaces NUL characters and lone surrogates without changing valid text', () => { + expect(sanitizePostgresString('before\0after\ud800')).toBe('before\ufffdafter\ufffd'); + expect(sanitizePostgresString('`\\u0000` 😀')).toBe('`\\u0000` 😀'); + }); +}); describe('sanitizeJsonbValue', () => { test('replaces JSONB-incompatible characters in nested values and object keys', () => { diff --git a/apps/web/src/lib/sanitize-jsonb.ts b/apps/web/src/lib/sanitize-jsonb.ts index c3afad9cbb..dd614430a3 100644 --- a/apps/web/src/lib/sanitize-jsonb.ts +++ b/apps/web/src/lib/sanitize-jsonb.ts @@ -1,9 +1,9 @@ /** - * PostgreSQL JSONB rejects escaped NUL characters and lone UTF-16 surrogates. + * PostgreSQL text rejects NUL characters; JSONB also rejects lone UTF-16 surrogates. * JavaScript strings can contain both, so repair them before sending values to - * a JSONB column. + * a PostgreSQL column. */ -function sanitizeJsonbString(value: string): string { +export function sanitizePostgresString(value: string): string { if (value.isWellFormed() && !value.includes('\0')) { return value; } @@ -13,7 +13,7 @@ function sanitizeJsonbString(value: string): string { export function sanitizeJsonbValue(value: unknown): unknown { if (typeof value === 'string') { - return sanitizeJsonbString(value); + return sanitizePostgresString(value); } if (Array.isArray(value)) { @@ -23,7 +23,7 @@ export function sanitizeJsonbValue(value: unknown): unknown { if (value !== null && typeof value === 'object') { return Object.fromEntries( Object.entries(value).map(([key, nestedValue]) => [ - sanitizeJsonbString(key), + sanitizePostgresString(key), sanitizeJsonbValue(nestedValue), ]) );