Skip to content

Commit 5cb88fd

Browse files
authored
fix(file-search): skip base64-dominated files and redact query errors from task failures (#8004)
* fix(file-search): skip base64-dominated files and redact query errors from task failures * fix(file-search): count wrapped base64 blocks and verify trigram estimate against pg_trgm
1 parent 5eece6e commit 5cb88fd

10 files changed

Lines changed: 365 additions & 10 deletions

File tree

‎.github/workflows/test-build.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,12 @@ jobs:
149149
KNOWLEDGE_ACL_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim
150150
run: bunx vitest run --mode integration lib/workspace-files/search/dispatcher.integration.ts
151151

152+
- name: Verify file search trigram estimate against pg_trgm
153+
working-directory: apps/sim
154+
env:
155+
KNOWLEDGE_ACL_TEST_DATABASE_URL: postgresql://postgres:postgres@127.0.0.1:5432/sim_auth_scim
156+
run: bunx vitest run --mode integration lib/workspace-files/search/index-plan.integration.ts
157+
152158
- name: Verify SCIM and administration over real HTTP
153159
working-directory: apps/sim
154160
env:
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { DrizzleQueryError } from 'drizzle-orm/errors'
5+
import { describe, expect, it } from 'vitest'
6+
import { redactDatabaseQueryError } from '@/lib/core/errors/database-query-error'
7+
8+
const BOUND_VALUE = 'confidential user text'
9+
10+
function queryError(code: string, message: string): DrizzleQueryError {
11+
return new DrizzleQueryError(
12+
'insert into "t" values ($1)',
13+
[BOUND_VALUE],
14+
Object.assign(new Error(message), { code })
15+
)
16+
}
17+
18+
describe('redactDatabaseQueryError', () => {
19+
it('reports a query failure by code and known cancellation reason only', () => {
20+
const original = queryError('57014', 'canceling statement due to statement timeout')
21+
const redacted = redactDatabaseQueryError(new Error('wrapper', { cause: original }), 'Insert')
22+
expect(redacted).toBeInstanceOf(Error)
23+
expect((redacted as Error).message).toBe('Insert failed (57014, statement_timeout)')
24+
expect((redacted as Error).stack).not.toContain(BOUND_VALUE)
25+
})
26+
it('omits an unrecognized reason', () => {
27+
const redacted = redactDatabaseQueryError(
28+
queryError('23505', `duplicate key value ${BOUND_VALUE}`),
29+
'Insert'
30+
)
31+
expect((redacted as Error).message).toBe('Insert failed (23505)')
32+
})
33+
it('returns errors without a query unchanged', () => {
34+
const error = new Error('storage unavailable')
35+
expect(redactDatabaseQueryError(error, 'Insert')).toBe(error)
36+
})
37+
})

‎apps/sim/lib/core/errors/database-query-error.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { findCause } from '@sim/utils/errors'
1+
import { findCause, getPostgresCancellationReason, getPostgresErrorCode } from '@sim/utils/errors'
22
import { DrizzleQueryError } from 'drizzle-orm/errors'
33

44
/**
@@ -8,3 +8,18 @@ import { DrizzleQueryError } from 'drizzle-orm/errors'
88
export function findDatabaseQueryError(error: unknown): DrizzleQueryError | undefined {
99
return findCause(error, (cause): cause is DrizzleQueryError => cause instanceof DrizzleQueryError)
1010
}
11+
12+
/**
13+
* Replaces a Drizzle query failure with an error naming only its database code, for surfaces such as
14+
* task runners that record a thrown error's message and stack verbatim. The original stays in
15+
* `cause` for in-process diagnostics. Errors without a query are returned unchanged.
16+
*/
17+
export function redactDatabaseQueryError(error: unknown, operation: string): unknown {
18+
const queryError = findDatabaseQueryError(error)
19+
if (!queryError) return error
20+
const code = getPostgresErrorCode(queryError) ?? 'no error code'
21+
const reason = getPostgresCancellationReason(queryError)
22+
return new Error(`${operation} failed (${reason ? `${code}, ${reason}` : code})`, {
23+
cause: error,
24+
})
25+
}

‎apps/sim/lib/workspace-files/search/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Workspace file search
22

3-
PostgreSQL stores complete extracted text in bounded chunks. Object storage remains the source of truth. A source and its extracted UTF-8 text must each fit within 25 MiB. Unsupported, degraded, oversized, or partially extracted files are excluded in full. There is no row-count or line-count coverage limit.
3+
PostgreSQL stores complete extracted text in bounded chunks. Object storage remains the source of truth. A source and its extracted UTF-8 text must each fit within 25 MiB. Unsupported, degraded, oversized, or partially extracted files are excluded in full. So is text that is mostly an encoded payload: when unbroken base64-alphabet runs of at least 256 characters that mix upper case, lower case, and digits make up half the text and more than 32 KiB, as with binaries stored as base64 text or SVGs embedding data URIs, the file is skipped as `encoded_content`. A run that fills whole lines at least 60 characters wide continues across line breaks, so base64 wrapped at the usual 64 or 76 columns counts as one payload. Smaller encoded values, such as a signature in a config file or lockfile integrity hashes, stay searchable, as do single-case runs such as hex digests and DNA sequences. There is no row-count or line-count coverage limit.
44

55
## Storage and publication
66

@@ -12,7 +12,7 @@ PostgreSQL stores complete extracted text in bounded chunks. Object storage rema
1212

1313
Workers download and extract outside database transactions, then insert batches of at most 250 rows / 128 KiB. Each batch checks the build token and lease. Publication locks the canonical file, build, and revision in that order, verifies the stored chunk count, and changes the visible pointer only after every batch succeeds. Old dispatch failure callbacks cannot overwrite newer dispatches or successful builds.
1414

15-
The chunk GIN index uses `fastupdate = off`. Each bounded insert updates the main index directly instead of appending to a shared pending list. With deferred updates enabled, even a small insert can cross the pending-list threshold and synchronously merge accumulated work from other files. Direct updates trade some bulk-write throughput for avoiding that foreground cleanup cliff. They do not eliminate normal index I/O, vacuum, or storage contention; the row and worker limits still apply. The 128 KiB batch budget bounds direct index work per transaction without changing the 25 MiB file coverage limit. Dense text with many distinct trigrams and an index working set larger than the available cache can still exceed the statement deadline. Capacity validation must include that cache pressure, not only a small corpus or a row count.
15+
The chunk GIN index uses `fastupdate = off`. Each bounded insert updates the main index directly instead of appending to a shared pending list. With deferred updates enabled, even a small insert can cross the pending-list threshold and synchronously merge accumulated work from other files. Direct updates trade some bulk-write throughput for avoiding that foreground cleanup cliff. They do not eliminate normal index I/O, vacuum, or storage contention; the row and worker limits still apply. The 128 KiB batch budget bounds direct index work per transaction without changing the 25 MiB file coverage limit. Dense text with many distinct trigrams and an index working set larger than the available cache can still exceed the statement deadline. Capacity validation must include that cache pressure, not only a small corpus or a row count. A batch that takes at least two seconds, including one a statement timeout cancels, is logged with its rows, bytes, and estimated trigram key count (pg_trgm's extraction, reproduced exactly for the database's `en_US.UTF-8` ctype), so a key-based batch budget can be sized from production timings. A failed query is reported to the task runner by error code only; Drizzle's message carries the bound file text.
1616

1717
Indexing transactions have separate limits from search: ten seconds per statement, five seconds waiting for a lock, and thirty seconds total on PostgreSQL 17. The outer limit leaves time for ordinary statement cancellation and rollback instead of terminating the connection at the same ten-second deadline. PostgreSQL 16 uses the compatible idle-transaction guard. A canceled batch remains unpublished; the existing task retry starts a fresh fenced build, and cleanup retires the previous attempt. This does not automatically retry revisions already marked failed.
1818

‎apps/sim/lib/workspace-files/search/constants.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ export const FILE_SEARCH_MAX_RESULTS = 200
1919

2020
export const FILE_SEARCH_MAX_SOURCE_BYTES = 25 * 1024 * 1024
2121
export const FILE_SEARCH_MAX_EXTRACTED_BYTES = 25 * 1024 * 1024
22+
/**
23+
* Unbroken base64-alphabet runs at least this long are encoded payloads (data URIs, binaries
24+
* stored as base64 text), not searchable text. Each chunk of one yields thousands of distinct
25+
* rare trigrams, which makes its direct GIN insert far slower than ordinary text.
26+
*/
27+
export const FILE_SEARCH_ENCODED_RUN_MIN_CHARS = 256
28+
/**
29+
* A run continues across a line break when it fills a line at least this wide, so base64 wrapped
30+
* at the usual 64 or 76 columns (MIME, PEM, Python's `encodebytes`) counts as one payload.
31+
*/
32+
export const FILE_SEARCH_ENCODED_WRAP_MIN_CHARS = 60
33+
/** Text is excluded when encoded runs are at least this share of it... */
34+
export const FILE_SEARCH_ENCODED_EXCLUSION_RATIO = 0.5
35+
/** ...and span more than a few chunks, so a small config carrying one signature stays searchable. */
36+
export const FILE_SEARCH_ENCODED_EXCLUSION_MIN_BYTES = 32 * 1024
2237
export const FILE_SEARCH_MAX_PREVIEW_BYTES = 2 * 1024
2338
export const FILE_SEARCH_CHUNK_BYTES = 8 * 1024
2439
export const FILE_SEARCH_CANDIDATE_PAGE_SIZE = 16
@@ -48,6 +63,8 @@ export const FILE_SEARCH_RECONCILE_INTERVAL_MS = 60 * 60 * 1000
4863
export const FILE_SEARCH_INSERT_BATCH_ROWS = 250
4964
/** Direct GIN writes perform index work in each insert, so transactions use smaller byte batches. */
5065
export const FILE_SEARCH_INSERT_BATCH_BYTES = 128 * 1024
66+
/** Batches at least this slow are logged with their estimated trigram key count. */
67+
export const FILE_SEARCH_SLOW_INSERT_BATCH_MS = 2000
5168

5269
/** Index writes allow statement cancellation before the outer transaction terminates its session. */
5370
export const FILE_SEARCH_INDEX_TRANSACTION_LIMITS = {
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import postgres from 'postgres'
5+
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
6+
import { estimateTrigramKeys } from '@/lib/workspace-files/search/index-plan'
7+
8+
describe('trigram key estimate against pg_trgm', () => {
9+
const databaseUrl = process.env.KNOWLEDGE_ACL_TEST_DATABASE_URL
10+
if (!databaseUrl) throw new Error('Trigram estimate tests require a disposable local database')
11+
const target = new URL(databaseUrl)
12+
if (
13+
!['postgres:', 'postgresql:'].includes(target.protocol) ||
14+
!['localhost', '127.0.0.1'].includes(target.hostname) ||
15+
(!target.pathname.startsWith('/sim_acl_test') && target.pathname !== '/sim_auth_scim')
16+
) {
17+
throw new Error('File search tests require a disposable local integration database')
18+
}
19+
const connection = postgres(databaseUrl, { max: 1, onnotice: () => {} })
20+
21+
beforeAll(async () => {
22+
await connection`CREATE EXTENSION IF NOT EXISTS pg_trgm`
23+
})
24+
25+
afterAll(async () => {
26+
await connection.end()
27+
})
28+
29+
it('runs where the estimate is specified', async () => {
30+
const [{ ctype }] = await connection<{ ctype: string }[]>`
31+
SELECT datctype AS ctype FROM pg_database WHERE datname = current_database()`
32+
expect(ctype).toMatch(/^en_US\.utf-?8$/i)
33+
})
34+
35+
it.each([
36+
['prose', 'The quick brown fox jumps over the lazy dog. The dog sleeps.'],
37+
['identifiers', 'snake_case CamelCase kebab-case 123abc x86_64 v2.1.0'],
38+
['a lockfile hash', '"sha512-cjQ7ZlQ0Mv3b47hABuTevyTuYN4i+loJKGeV9flcCgIK37cCXRh+L1bd=="'],
39+
['accented text', 'Café naïve ÉCOLE über straße'],
40+
['CJK text', '日本語のテキスト検索 テスト'],
41+
['Greek text', 'Ελληνικά κείμενα ΑΒΓ'],
42+
['emoji', 'emoji 🙂🙂 party 🎉 done'],
43+
['punctuation only', '--- ___ ... !!!'],
44+
])('matches show_trgm for %s', async (_, text) => {
45+
const [{ keys }] = await connection<{ keys: number }[]>`
46+
SELECT coalesce(array_length(show_trgm(${text}), 1), 0)::int AS keys`
47+
expect(estimateTrigramKeys(text)).toBe(keys)
48+
})
49+
})

‎apps/sim/lib/workspace-files/search/index-plan.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, it } from 'vitest'
22
import {
3+
estimateTrigramKeys,
34
iterateFileSearchChunks,
45
planFileSearchIndex,
56
} from '@/lib/workspace-files/search/index-plan'
@@ -56,6 +57,52 @@ describe('file search chunk packing', () => {
5657
planFileSearchIndex({ text: 'x'.repeat(25 * 1024 * 1024 + 1), partial: false }, signal)
5758
).toThrow('extracted_text_too_large')
5859
})
60+
it.each([
61+
['a binary stored as base64 text', 'iVBORw0KGgo'.repeat(10_000)],
62+
[
63+
'an SVG embedding a data URI',
64+
`<svg><title>sheet</title><image href="data:image/jpeg;base64,${'/9j/4AAQ'.repeat(8000)}"/></svg>`,
65+
],
66+
['space-separated runs at the minimum run length', `${'aB3'.repeat(86)} `.repeat(500)],
67+
['base64 wrapped at 76 columns', `${'aB3d'.repeat(19)}\n`.repeat(2000)],
68+
[
69+
'a PEM-style block wrapped at 64 columns with CRLF',
70+
`-----BEGIN DATA-----\r\n${`${'Qk9z'.repeat(16)}\r\n`.repeat(2000)}-----END DATA-----\r\n`,
71+
],
72+
])('excludes %s before producing any chunks', (_, text) => {
73+
expect(() => planFileSearchIndex({ text, partial: false }, signal)).toThrow('encoded_content')
74+
})
75+
it.each([
76+
[
77+
'a small config carrying one signature',
78+
`{"$schema":"https://example.com/schema.json","signature":"${'Qk9'.repeat(200)}"}`,
79+
],
80+
[
81+
'a lockfile whose integrity hashes are short runs',
82+
`"pkg": ["pkg@1.0.0", "", {}, "sha512-${'Ab1+'.repeat(22)}=="],\n\n`.repeat(2000),
83+
],
84+
['space-separated runs just short of the run length', `${'aB3'.repeat(85)} `.repeat(500)],
85+
['one short token per line', `${'aB3d'.repeat(10)}\n`.repeat(5000)],
86+
['lines that each end in a long hash', `checksum ${'aB3d'.repeat(19)}\n`.repeat(2000)],
87+
['an unwrapped DNA sequence', `>chr1\n${'ACGT'.repeat(20_000)}\n`],
88+
['a hex digest dump', `${'deadbeef0123'.repeat(5000)}\n`],
89+
[
90+
'prose that embeds an encoded payload smaller than itself',
91+
`${'Quarterly results and notes. '.repeat(5000)}${'aB3d'.repeat(10_000)}`,
92+
],
93+
])('keeps %s searchable', (_, text) => {
94+
expect(() => planFileSearchIndex({ text, partial: false }, signal)).not.toThrow()
95+
})
96+
it.each([
97+
['cat', 4],
98+
['Cat CAT cat', 4],
99+
['foo|bar', 8],
100+
['a', 2],
101+
['', 0],
102+
['--- ___ ...', 0],
103+
])('estimates pg_trgm keys for %j', (text, expected) => {
104+
expect(estimateTrigramKeys(text)).toBe(expected)
105+
})
59106
it('stops iteration when aborted', () => {
60107
const controller = new AbortController()
61108
const chunks = iterateFileSearchChunks(

‎apps/sim/lib/workspace-files/search/index-plan.ts‎

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,79 @@ import { Buffer } from 'node:buffer'
22
import {
33
FILE_SEARCH_CANDIDATE_LITERAL_CHARS,
44
FILE_SEARCH_CHUNK_BYTES,
5+
FILE_SEARCH_ENCODED_EXCLUSION_MIN_BYTES,
6+
FILE_SEARCH_ENCODED_EXCLUSION_RATIO,
7+
FILE_SEARCH_ENCODED_RUN_MIN_CHARS,
8+
FILE_SEARCH_ENCODED_WRAP_MIN_CHARS,
59
FILE_SEARCH_MAX_EXTRACTED_BYTES,
610
} from '@/lib/workspace-files/search/constants'
711
import type { ExtractedIndexText } from '@/lib/workspace-files/search/extract'
812

9-
export type FileSearchExclusionReason = 'extracted_text_too_large' | 'incomplete_extraction'
13+
export type FileSearchExclusionReason =
14+
| 'extracted_text_too_large'
15+
| 'incomplete_extraction'
16+
| 'encoded_content'
17+
18+
const TRIGRAM_WORD = /[\p{L}\p{N}]+/gu
19+
20+
/**
21+
* Bytes inside long base64 runs, in one linear pass. A run counts only when it mixes upper case,
22+
* lower case, and digits, as real base64 does; single-case runs such as hex digests or DNA
23+
* sequences are searchable text with few distinct trigrams. A run that fills whole lines of at
24+
* least the wrap width continues across line breaks, so wrapped base64 counts as one payload while
25+
* a hash inside a line still ends with it. Runs are ASCII, so chars are bytes.
26+
*/
27+
function countEncodedBytes(text: string): number {
28+
let encoded = 0
29+
let run = 0
30+
let runFillsLines = false
31+
let lineChars = 0
32+
let upper = false
33+
let lower = false
34+
let digit = false
35+
const endRun = () => {
36+
if (run >= FILE_SEARCH_ENCODED_RUN_MIN_CHARS && upper && lower && digit) encoded += run
37+
run = 0
38+
upper = lower = digit = false
39+
}
40+
for (let i = 0; i < text.length; i++) {
41+
const code = text.charCodeAt(i)
42+
const isUpper = code >= 65 && code <= 90
43+
const isLower = code >= 97 && code <= 122
44+
const isDigit = code >= 48 && code <= 57
45+
if (isUpper || isLower || isDigit || code === 43 || code === 47) {
46+
if (run === 0) runFillsLines = lineChars === 0
47+
run++
48+
lineChars++
49+
upper ||= isUpper
50+
lower ||= isLower
51+
digit ||= isDigit
52+
} else if (code === 10) {
53+
if (!(run > 0 && runFillsLines && lineChars >= FILE_SEARCH_ENCODED_WRAP_MIN_CHARS)) endRun()
54+
lineChars = 0
55+
} else if (code !== 13 || text.charCodeAt(i + 1) !== 10) {
56+
endRun()
57+
lineChars++
58+
}
59+
}
60+
endRun()
61+
return encoded
62+
}
63+
64+
/**
65+
* Distinct keys `gin_trgm_ops` extracts from `content`, mirroring pg_trgm: lowercased alphanumeric
66+
* words, each padded with two leading spaces and one trailing space. This matches PostgreSQL under
67+
* the `en_US.UTF-8` ctype; its hashing of multibyte trigrams can only merge keys, so the count is
68+
* never an underestimate.
69+
*/
70+
export function estimateTrigramKeys(content: string): number {
71+
const keys = new Set<string>()
72+
for (const [word] of content.toLowerCase().matchAll(TRIGRAM_WORD)) {
73+
const padded = [' ', ' ', ...word, ' ']
74+
for (let i = 2; i < padded.length; i++) keys.add(padded[i - 2] + padded[i - 1] + padded[i])
75+
}
76+
return keys.size
77+
}
1078

1179
export class FileSearchExclusionError extends Error {
1280
constructor(readonly reason: FileSearchExclusionReason) {
@@ -30,16 +98,27 @@ export interface FileSearchIndexPlan {
3098
indexedBytes: number
3199
}
32100

33-
/** Admission happens before any chunks are written; incomplete extraction never becomes searchable. */
101+
/**
102+
* Admission happens before any chunks are written; incomplete extraction never becomes searchable,
103+
* and neither does text that is mostly an encoded payload.
104+
*/
34105
export function planFileSearchIndex(
35106
extracted: ExtractedIndexText,
36107
signal: AbortSignal
37108
): FileSearchIndexPlan {
38109
signal.throwIfAborted()
39110
if (extracted.partial) throw new FileSearchExclusionError('incomplete_extraction')
40-
if (Buffer.byteLength(extracted.text, 'utf8') > FILE_SEARCH_MAX_EXTRACTED_BYTES) {
111+
const textBytes = Buffer.byteLength(extracted.text, 'utf8')
112+
if (textBytes > FILE_SEARCH_MAX_EXTRACTED_BYTES) {
41113
throw new FileSearchExclusionError('extracted_text_too_large')
42114
}
115+
const encodedBytes = countEncodedBytes(extracted.text)
116+
if (
117+
encodedBytes > FILE_SEARCH_ENCODED_EXCLUSION_MIN_BYTES &&
118+
encodedBytes >= textBytes * FILE_SEARCH_ENCODED_EXCLUSION_RATIO
119+
) {
120+
throw new FileSearchExclusionError('encoded_content')
121+
}
43122
const bytes = Buffer.from(extracted.text.replace(/\r(?=\n|$)/g, ''), 'utf8')
44123
let lineCount = 1
45124
for (const byte of bytes) if (byte === 10) lineCount++

0 commit comments

Comments
 (0)