Skip to content

Commit d4870bd

Browse files
committed
fix(mothership): redact file text before grep clips or splits it
Model egress redacts whole secrets only, so grep's 2,000-character line clip and its per-line split could hand it a fragment of a known secret it cannot recognize. Grep now redacts each resource's full text through the invocation's provenance before splitting, matching and clipping, and the clip never cuts through a redaction marker. JSON and JSONL parse errors no longer quote a clipped excerpt of the file.
1 parent 6511d43 commit d4870bd

6 files changed

Lines changed: 171 additions & 5 deletions

File tree

‎apps/sim/lib/file-parsers/json-parser.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,29 @@ describe('JSON parser complexity limits', () => {
3333
code: 'invalid_format',
3434
})
3535
})
36+
37+
/**
38+
* Text extraction errors reach Chat as the read's error, and egress redaction recognizes only
39+
* whole secrets, so a message quoting a clipped excerpt of the file could carry part of one.
40+
*/
41+
it.each([
42+
[
43+
'JSON',
44+
parseJSONBuffer,
45+
`{"config": {"key": sk_live_FIXTURE_0123456789abcdefghij${'x'.repeat(200)}}}`,
46+
],
47+
[
48+
'JSONL',
49+
parseJSONLBuffer,
50+
`{"ok": true}\n{"key": "${'x'.repeat(80)}sk_live_FIXTURE_0123456789abcdefghij`,
51+
],
52+
] as const)(
53+
'names where invalid %s fails without quoting the file',
54+
async (_kind, parse, text) => {
55+
const error = await parse(Buffer.from(text)).catch((caught: unknown) => caught)
56+
expect(error).toMatchObject({ code: 'invalid_format' })
57+
expect(String((error as Error).message)).not.toContain('sk_l')
58+
expect(String((error as Error).message)).not.toContain('xxxx')
59+
}
60+
)
3661
})

‎apps/sim/lib/file-parsers/json-parser.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,18 @@ export function stripJsonComments(text: string): string {
213213
return out.replace(/,(\s*[}\]])/g, '$1')
214214
}
215215

216+
/** V8's excerpt of the input: `Unexpected token 's', "…{"key": sk_l"... is not valid JSON`. */
217+
const QUOTED_INPUT_EXCERPT = /,\s*(?:\.\.\.)?".*"(?:\.\.\.)?\s+is not valid JSON$/s
218+
219+
/**
220+
* A parse failure's message without the clipped excerpt of the input V8 quotes in it. The message
221+
* reaches Chat as the read's error, where redaction matches whole secrets only, so an excerpt cut
222+
* through a secret would carry the part that survived the cut.
223+
*/
224+
function describeSyntaxError(error: unknown): string {
225+
return getErrorMessage(error, 'Unknown error').replace(QUOTED_INPUT_EXCERPT, '')
226+
}
227+
216228
function parseJsonContent(buffer: Uint8Array): FileParseResult {
217229
const decoded = decodeTextBuffer(buffer)
218230
try {
@@ -226,7 +238,7 @@ function parseJsonContent(buffer: Uint8Array): FileParseResult {
226238
if (lenient) return lenient
227239
throw new FileParserError(
228240
'invalid_format',
229-
`Invalid JSON: ${getErrorMessage(error, 'Unknown error')}`,
241+
`Invalid JSON: ${describeSyntaxError(error)}`,
230242
error
231243
)
232244
}
@@ -282,7 +294,7 @@ function parseJsonLinesContent(buffer: Uint8Array): FileParseResult {
282294
} catch (error) {
283295
throw new FileParserError(
284296
'invalid_format',
285-
`Invalid JSONL on line ${lineNumber}: ${line.slice(0, 100)}`,
297+
`Invalid JSONL on line ${lineNumber}: ${describeSyntaxError(error)}`,
286298
error
287299
)
288300
}

‎apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { sleep } from '@sim/utils/helpers'
22
import { isRecordLike } from '@sim/utils/object'
3-
import { truncate } from '@sim/utils/string'
3+
import { escapeRegExp } from '@sim/utils/string'
44
import type { ReadFileTextResponse } from 'sim/embed'
55
import { listCatalogTools } from '@/lib/catalog/application/list-tools'
66
import { readBlockCatalog } from '@/lib/catalog/application/read-block-catalog'
@@ -14,6 +14,9 @@ import {
1414
agentCliOk,
1515
} from '@/lib/mothership/agent-cli/types'
1616
import { buildIntegrationToolSchemas } from '@/lib/mothership/chat/payload'
17+
import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection'
18+
import { OPAQUE_RESOLVED_SECRET_REPLACEMENT } from '@/executor/utils/resolved-secret-matcher'
19+
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
1720

1821
/**
1922
* `grep <pattern> [--scope a,b] [--in <world|id|name|world/id>] [-i] [-C n] [-A n] [-B n] [--count] [--limit n]` —
@@ -430,8 +433,36 @@ function compilePattern(raw: string, ignoreCase: boolean): (line: string) => boo
430433
return (line) => (ignoreCase ? line.toLowerCase() : line).includes(needle)
431434
}
432435

436+
/** What redaction leaves in place of a secret: a `{{NAME}}` label or the opaque marker. */
437+
const REDACTION_MARKER = new RegExp(
438+
`\\{\\{[^{}]+\\}\\}|${escapeRegExp(OPAQUE_RESOLVED_SECRET_REPLACEMENT)}`,
439+
'g'
440+
)
441+
442+
/** Clips redacted text, cutting before a redaction marker rather than through it. */
433443
function clip(line: string): string {
434-
return truncate(line, MAX_LINE_CHARS, '… [line truncated]')
444+
if (line.length <= MAX_LINE_CHARS) return line
445+
let cut = MAX_LINE_CHARS
446+
for (const marker of line.matchAll(REDACTION_MARKER)) {
447+
if (marker.index >= cut) break
448+
if (marker.index + marker[0].length > cut) {
449+
cut = marker.index
450+
break
451+
}
452+
}
453+
return `${line.slice(0, cut)}… [line truncated]`
454+
}
455+
456+
/**
457+
* Model egress redacts whole secrets only, so a line clipped, or a multi-line secret split into
458+
* lines, would carry a fragment it cannot recognize. The text is redacted whole first, through the
459+
* same provenance egress uses; null means it could not be, and the resource is not searched.
460+
* Without a registry egress withholds the whole result, so there is nothing to redact against.
461+
*/
462+
function redactForModel(text: string, egress: ResolvedSecretTraceRegistry | undefined) {
463+
if (!egress) return text
464+
const projection = projectResolvedSecretModelContent(text, egress)
465+
return projection.safe && typeof projection.value === 'string' ? projection.value : null
435466
}
436467

437468
function didYouMean(scope: string): string {
@@ -587,9 +618,17 @@ export const universalGrepCommand: AgentCliEngine = {
587618
}
588619
return now
589620
}
621+
const egress = runtime.resolvedSecretTraceRegistry?.forkForPropagatedEntries()
590622
for (const resource of candidates) {
591623
if (resource.text === null) continue
592-
const lines = resource.text.split('\n')
624+
const text = redactForModel(resource.text, egress)
625+
if (text === null) {
626+
bounded.reportIncomplete(
627+
`${resource.scope}/${resource.label} (${resource.id}): could not be checked for secret values, so it was not searched.`
628+
)
629+
continue
630+
}
631+
const lines = text.split('\n')
593632
const selected = new Map<number, boolean>()
594633
const remainingLines = limit - out.length
595634
let nextContextLine = 0

‎apps/sim/lib/mothership/agent-cli/file-provenance.test.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,89 @@ describe('file provenance at the actual CLI and model-result boundary', () => {
497497
)
498498
})
499499

500+
describe('grep reshapes a file line only after redacting it', () => {
501+
const secret = 'sk_live_P3_FIXTURE_0123456789abcdefghij'
502+
const otherSecret = 'pk_test_P3_FIXTURE_ZYXWVUTSRQPONMLKJIH'
503+
const keyBlock = [
504+
'-----BEGIN FIXTURE KEY-----',
505+
'MIIBFIXTUREKEYLINEONE0123456789',
506+
'MIIBFIXTUREKEYLINETWO0123456789',
507+
'-----END FIXTURE KEY-----',
508+
].join('\n')
509+
const LINE_CHARS = 2_000
510+
const pad = (from: number, to: number) => 'x'.repeat(to - from)
511+
512+
/** A secret leaks as soon as its first or last four characters, or one of its lines, cross. */
513+
function fragments(value: string): string[] {
514+
return [value.slice(0, 4), value.slice(-4), ...value.split('\n')]
515+
}
516+
517+
async function grepSecretFile(text: string, pattern: string) {
518+
const plaintexts: Record<string, string> = {
519+
'cipher-file': secret,
520+
'cipher-other': otherSecret,
521+
'cipher-key': keyBlock,
522+
}
523+
mocks.decrypt.mockImplementation(async (cipher: string) => ({
524+
decrypted: plaintexts[cipher],
525+
}))
526+
mocks.buffer.mockResolvedValue(Buffer.from(text))
527+
vi.stubGlobal('fetch', async () =>
528+
Response.json({
529+
data: [{ ...file, size: Buffer.byteLength(text), updatedAt: revision.toISOString() }],
530+
nextCursor: null,
531+
})
532+
)
533+
resetDbChainMock()
534+
queueTableRows(workspaceFiles, [
535+
sidecarRow('exact', revision, [
536+
{ name: 'FILE_SECRET', encryptedValue: 'cipher-file', sourceUserId: 'reader' },
537+
{ name: 'OTHER_SECRET', encryptedValue: 'cipher-other', sourceUserId: 'reader' },
538+
{ name: 'KEY_BLOCK', encryptedValue: 'cipher-key', sourceUserId: 'reader' },
539+
]),
540+
])
541+
const trace = registry()
542+
const result = await runCli(grepFiles(pattern), trace)
543+
const projection = inspectToolResultForCopilot(result, trace, 'sim_cli')
544+
expect(projection.safe).toBe(true)
545+
return JSON.stringify(projection.result)
546+
}
547+
548+
it.each([
549+
['straddling the clip', `NEEDLE ${pad(7, LINE_CHARS - 10)}${secret} tail`],
550+
[
551+
'whose redaction marker would straddle the clip',
552+
`NEEDLE ${pad(7, LINE_CHARS - 5)}${secret}`,
553+
],
554+
['wholly past the clip', `NEEDLE ${pad(7, LINE_CHARS + 50)}${secret} tail`],
555+
[
556+
'repeated around the clip with another secret',
557+
`NEEDLE ${otherSecret} ${pad(48, LINE_CHARS - 12)}${secret}${otherSecret}${secret}`,
558+
],
559+
])('never returns a fragment of a secret %s', async (_case, line) => {
560+
const projected = await grepSecretFile(`${line}\nsecond line`, 'NEEDLE')
561+
expect(projected).toContain('NEEDLE')
562+
expect(projected).toContain('[line truncated]')
563+
for (const fragment of [...fragments(secret), ...fragments(otherSecret)]) {
564+
expect(projected).not.toContain(fragment)
565+
}
566+
const withoutMarkers = projected
567+
.replaceAll('{{FILE_SECRET}}', '')
568+
.replaceAll('{{OTHER_SECRET}}', '')
569+
.replaceAll('[REDACTED_SECRET]', '')
570+
expect(withoutMarkers).not.toContain('{{')
571+
expect(withoutMarkers).not.toContain('[RED')
572+
})
573+
574+
it('never returns one line of a multi-line secret', async () => {
575+
const projected = await grepSecretFile(
576+
`config start\n${keyBlock}\nconfig end`,
577+
'FIXTURE KEY|KEYLINE'
578+
)
579+
for (const fragment of fragments(keyBlock)) expect(projected).not.toContain(fragment)
580+
})
581+
})
582+
500583
it('refuses an unknown file through the actual CLI text read without loading its bytes', async () => {
501584
classify('unknown')
502585
const result = await runCli(readFile)

‎apps/sim/lib/mothership/agent-cli/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ async function executeBoundAgentCliRequest(
131131
}
132132
: {}),
133133
...(context.chatId !== undefined ? { chatId: context.chatId } : {}),
134+
resolvedSecretTraceRegistry: context.resolvedSecretTraceRegistry,
134135
signal: context.signal,
135136
},
136137
invocation.flags

‎apps/sim/lib/mothership/agent-cli/types.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Principal } from '@sim/auth/principal'
22
import type { CopilotChatDelegationContext } from '@/lib/mothership/auth/application-delegation'
33
import type { AgentCliRawResult } from '@/lib/mothership/generated/agent-cli'
4+
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
45
/**
56
* Sim's half of the mothership↔CLI translation layer: generic execution PRIMITIVES.
67
* The worker owns the grammar (what commands exist, how argv parses, pipes, help, the
@@ -36,6 +37,11 @@ export interface AgentCliRuntime {
3637
/** The human the command acts as — reference resolution and grants scope to them. */
3738
userId: string
3839
chatId?: string
40+
/**
41+
* The invocation's secret provenance, which model egress redacts the result with. Egress
42+
* matches whole secrets, so an engine that clips or splits text it read redacts it here first.
43+
*/
44+
resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry
3945
signal?: AbortSignal
4046
}
4147

0 commit comments

Comments
 (0)