diff --git a/apps/sim/lib/file-parsers/json-parser.test.ts b/apps/sim/lib/file-parsers/json-parser.test.ts index 0a4d110e087..99b779d75f4 100644 --- a/apps/sim/lib/file-parsers/json-parser.test.ts +++ b/apps/sim/lib/file-parsers/json-parser.test.ts @@ -33,4 +33,29 @@ describe('JSON parser complexity limits', () => { code: 'invalid_format', }) }) + + /** + * Text extraction errors reach Chat as the read's error, and egress redaction recognizes only + * whole secrets, so a message quoting a clipped excerpt of the file could carry part of one. + */ + it.each([ + [ + 'JSON', + parseJSONBuffer, + `{"config": {"key": sk_live_FIXTURE_0123456789abcdefghij${'x'.repeat(200)}}}`, + ], + [ + 'JSONL', + parseJSONLBuffer, + `{"ok": true}\n{"key": "${'x'.repeat(80)}sk_live_FIXTURE_0123456789abcdefghij`, + ], + ] as const)( + 'names where invalid %s fails without quoting the file', + async (_kind, parse, text) => { + const error = await parse(Buffer.from(text)).catch((caught: unknown) => caught) + expect(error).toMatchObject({ code: 'invalid_format' }) + expect(String((error as Error).message)).not.toContain('sk_l') + expect(String((error as Error).message)).not.toContain('xxxx') + } + ) }) diff --git a/apps/sim/lib/file-parsers/json-parser.ts b/apps/sim/lib/file-parsers/json-parser.ts index 562e4cbcd43..b8e9c236fe2 100644 --- a/apps/sim/lib/file-parsers/json-parser.ts +++ b/apps/sim/lib/file-parsers/json-parser.ts @@ -213,6 +213,18 @@ export function stripJsonComments(text: string): string { return out.replace(/,(\s*[}\]])/g, '$1') } +/** V8's excerpt of the input: `Unexpected token 's', "…{"key": sk_l"... is not valid JSON`. */ +const QUOTED_INPUT_EXCERPT = /,\s*(?:\.\.\.)?".*"(?:\.\.\.)?\s+is not valid JSON$/s + +/** + * A parse failure's message without the clipped excerpt of the input V8 quotes in it. The message + * reaches Chat as the read's error, where redaction matches whole secrets only, so an excerpt cut + * through a secret would carry the part that survived the cut. + */ +function describeSyntaxError(error: unknown): string { + return getErrorMessage(error, 'Unknown error').replace(QUOTED_INPUT_EXCERPT, '') +} + function parseJsonContent(buffer: Uint8Array): FileParseResult { const decoded = decodeTextBuffer(buffer) try { @@ -226,7 +238,7 @@ function parseJsonContent(buffer: Uint8Array): FileParseResult { if (lenient) return lenient throw new FileParserError( 'invalid_format', - `Invalid JSON: ${getErrorMessage(error, 'Unknown error')}`, + `Invalid JSON: ${describeSyntaxError(error)}`, error ) } @@ -282,7 +294,7 @@ function parseJsonLinesContent(buffer: Uint8Array): FileParseResult { } catch (error) { throw new FileParserError( 'invalid_format', - `Invalid JSONL on line ${lineNumber}: ${line.slice(0, 100)}`, + `Invalid JSONL on line ${lineNumber}: ${describeSyntaxError(error)}`, error ) } diff --git a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts index d4496a4f677..b4df8bd8642 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts @@ -1,6 +1,6 @@ import { sleep } from '@sim/utils/helpers' import { isRecordLike } from '@sim/utils/object' -import { truncate } from '@sim/utils/string' +import { escapeRegExp } from '@sim/utils/string' import type { ReadFileTextResponse } from 'sim/embed' import { listCatalogTools } from '@/lib/catalog/application/list-tools' import { readBlockCatalog } from '@/lib/catalog/application/read-block-catalog' @@ -14,6 +14,9 @@ import { agentCliOk, } from '@/lib/mothership/agent-cli/types' import { buildIntegrationToolSchemas } from '@/lib/mothership/chat/payload' +import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection' +import { OPAQUE_RESOLVED_SECRET_REPLACEMENT } from '@/executor/utils/resolved-secret-matcher' +import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' /** * `grep [--scope a,b] [--in ] [-i] [-C n] [-A n] [-B n] [--count] [--limit n]` — @@ -430,8 +433,36 @@ function compilePattern(raw: string, ignoreCase: boolean): (line: string) => boo return (line) => (ignoreCase ? line.toLowerCase() : line).includes(needle) } +/** What redaction leaves in place of a secret: a `{{NAME}}` label or the opaque marker. */ +const REDACTION_MARKER = new RegExp( + `\\{\\{[^{}]+\\}\\}|${escapeRegExp(OPAQUE_RESOLVED_SECRET_REPLACEMENT)}`, + 'g' +) + +/** Clips redacted text, cutting before a redaction marker rather than through it. */ function clip(line: string): string { - return truncate(line, MAX_LINE_CHARS, '… [line truncated]') + if (line.length <= MAX_LINE_CHARS) return line + let cut = MAX_LINE_CHARS + for (const marker of line.matchAll(REDACTION_MARKER)) { + if (marker.index >= cut) break + if (marker.index + marker[0].length > cut) { + cut = marker.index + break + } + } + return `${line.slice(0, cut)}… [line truncated]` +} + +/** + * Model egress redacts whole secrets only, so a line clipped, or a multi-line secret split into + * lines, would carry a fragment it cannot recognize. The text is redacted whole first, through the + * same provenance egress uses; null means it could not be, and the resource is not searched. + * Without a registry egress withholds the whole result, so there is nothing to redact against. + */ +function redactForModel(text: string, egress: ResolvedSecretTraceRegistry | undefined) { + if (!egress) return text + const projection = projectResolvedSecretModelContent(text, egress) + return projection.safe && typeof projection.value === 'string' ? projection.value : null } function didYouMean(scope: string): string { @@ -587,9 +618,17 @@ export const universalGrepCommand: AgentCliEngine = { } return now } + const egress = runtime.resolvedSecretTraceRegistry?.forkForPropagatedEntries() for (const resource of candidates) { if (resource.text === null) continue - const lines = resource.text.split('\n') + const text = redactForModel(resource.text, egress) + if (text === null) { + bounded.reportIncomplete( + `${resource.scope}/${resource.label} (${resource.id}): could not be checked for secret values, so it was not searched.` + ) + continue + } + const lines = text.split('\n') const selected = new Map() const remainingLines = limit - out.length let nextContextLine = 0 diff --git a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts index 893d05af5e5..e45545e1f16 100644 --- a/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts +++ b/apps/sim/lib/mothership/agent-cli/file-provenance.test.ts @@ -497,6 +497,89 @@ describe('file provenance at the actual CLI and model-result boundary', () => { ) }) + describe('grep reshapes a file line only after redacting it', () => { + const secret = 'sk_live_P3_FIXTURE_0123456789abcdefghij' + const otherSecret = 'pk_test_P3_FIXTURE_ZYXWVUTSRQPONMLKJIH' + const keyBlock = [ + '-----BEGIN FIXTURE KEY-----', + 'MIIBFIXTUREKEYLINEONE0123456789', + 'MIIBFIXTUREKEYLINETWO0123456789', + '-----END FIXTURE KEY-----', + ].join('\n') + const LINE_CHARS = 2_000 + const pad = (from: number, to: number) => 'x'.repeat(to - from) + + /** A secret leaks as soon as its first or last four characters, or one of its lines, cross. */ + function fragments(value: string): string[] { + return [value.slice(0, 4), value.slice(-4), ...value.split('\n')] + } + + async function grepSecretFile(text: string, pattern: string) { + const plaintexts: Record = { + 'cipher-file': secret, + 'cipher-other': otherSecret, + 'cipher-key': keyBlock, + } + mocks.decrypt.mockImplementation(async (cipher: string) => ({ + decrypted: plaintexts[cipher], + })) + mocks.buffer.mockResolvedValue(Buffer.from(text)) + vi.stubGlobal('fetch', async () => + Response.json({ + data: [{ ...file, size: Buffer.byteLength(text), updatedAt: revision.toISOString() }], + nextCursor: null, + }) + ) + resetDbChainMock() + queueTableRows(workspaceFiles, [ + sidecarRow('exact', revision, [ + { name: 'FILE_SECRET', encryptedValue: 'cipher-file', sourceUserId: 'reader' }, + { name: 'OTHER_SECRET', encryptedValue: 'cipher-other', sourceUserId: 'reader' }, + { name: 'KEY_BLOCK', encryptedValue: 'cipher-key', sourceUserId: 'reader' }, + ]), + ]) + const trace = registry() + const result = await runCli(grepFiles(pattern), trace) + const projection = inspectToolResultForCopilot(result, trace, 'sim_cli') + expect(projection.safe).toBe(true) + return JSON.stringify(projection.result) + } + + it.each([ + ['straddling the clip', `NEEDLE ${pad(7, LINE_CHARS - 10)}${secret} tail`], + [ + 'whose redaction marker would straddle the clip', + `NEEDLE ${pad(7, LINE_CHARS - 5)}${secret}`, + ], + ['wholly past the clip', `NEEDLE ${pad(7, LINE_CHARS + 50)}${secret} tail`], + [ + 'repeated around the clip with another secret', + `NEEDLE ${otherSecret} ${pad(48, LINE_CHARS - 12)}${secret}${otherSecret}${secret}`, + ], + ])('never returns a fragment of a secret %s', async (_case, line) => { + const projected = await grepSecretFile(`${line}\nsecond line`, 'NEEDLE') + expect(projected).toContain('NEEDLE') + expect(projected).toContain('[line truncated]') + for (const fragment of [...fragments(secret), ...fragments(otherSecret)]) { + expect(projected).not.toContain(fragment) + } + const withoutMarkers = projected + .replaceAll('{{FILE_SECRET}}', '') + .replaceAll('{{OTHER_SECRET}}', '') + .replaceAll('[REDACTED_SECRET]', '') + expect(withoutMarkers).not.toContain('{{') + expect(withoutMarkers).not.toContain('[RED') + }) + + it('never returns one line of a multi-line secret', async () => { + const projected = await grepSecretFile( + `config start\n${keyBlock}\nconfig end`, + 'FIXTURE KEY|KEYLINE' + ) + for (const fragment of fragments(keyBlock)) expect(projected).not.toContain(fragment) + }) + }) + it('refuses an unknown file through the actual CLI text read without loading its bytes', async () => { classify('unknown') const result = await runCli(readFile) diff --git a/apps/sim/lib/mothership/agent-cli/index.ts b/apps/sim/lib/mothership/agent-cli/index.ts index 2bf711263e1..5a7d35340d8 100644 --- a/apps/sim/lib/mothership/agent-cli/index.ts +++ b/apps/sim/lib/mothership/agent-cli/index.ts @@ -131,6 +131,7 @@ async function executeBoundAgentCliRequest( } : {}), ...(context.chatId !== undefined ? { chatId: context.chatId } : {}), + resolvedSecretTraceRegistry: context.resolvedSecretTraceRegistry, signal: context.signal, }, invocation.flags diff --git a/apps/sim/lib/mothership/agent-cli/types.ts b/apps/sim/lib/mothership/agent-cli/types.ts index f20c60d9f2e..a210e7042be 100644 --- a/apps/sim/lib/mothership/agent-cli/types.ts +++ b/apps/sim/lib/mothership/agent-cli/types.ts @@ -1,6 +1,7 @@ import type { Principal } from '@sim/auth/principal' import type { CopilotChatDelegationContext } from '@/lib/mothership/auth/application-delegation' import type { AgentCliRawResult } from '@/lib/mothership/generated/agent-cli' +import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' /** * Sim's half of the mothership↔CLI translation layer: generic execution PRIMITIVES. * The worker owns the grammar (what commands exist, how argv parses, pipes, help, the @@ -36,6 +37,11 @@ export interface AgentCliRuntime { /** The human the command acts as — reference resolution and grants scope to them. */ userId: string chatId?: string + /** + * The invocation's secret provenance, which model egress redacts the result with. Egress + * matches whole secrets, so an engine that clips or splits text it read redacts it here first. + */ + resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry signal?: AbortSignal }