Skip to content

Commit 8e14e4d

Browse files
icecrasher321icecrasher321
authored
fix(provenance): align file matching and report withheld attachments (#7867)
* fix(provenance): stop silently dropping model attachments * fix(provenance): keep model turns running after attachment refusal * fix(provenance): apply literal policy before file classification
1 parent 1e7c3dc commit 8e14e4d

11 files changed

Lines changed: 491 additions & 160 deletions

File tree

‎apps/sim/executor/handlers/agent/agent-handler.test.ts‎

Lines changed: 77 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,55 +1040,87 @@ describe('AgentBlockHandler', () => {
10401040
}
10411041
})
10421042

1043-
it('omits only a generated document whose embedded contributor is not model-safe', async () => {
1044-
const key = 'workspace/ws-1/report.pdf'
1045-
mockContext.workspaceId = 'ws-1'
1046-
const hydrationSpy = vi
1047-
.spyOn(userFileBase64, 'hydrateUserFilesWithBase64')
1048-
.mockImplementationOnce(async (files, options) => {
1049-
await options.onServableFileContributors?.(files[0], [
1050-
{
1051-
fileId: 'image-1',
1052-
key: 'workspace/ws-1/image-1.png',
1053-
context: 'workspace',
1054-
contentUpdatedAt: new Date('2026-08-06T00:00:00.000Z'),
1055-
},
1056-
])
1057-
return files.map((file) => ({ ...file, base64: 'JVBERi0=' }))
1058-
})
1059-
mockImportWorkspaceFileSecretProvenanceForModelView.mockResolvedValueOnce(false)
1043+
it.each([
1044+
{ safe: false, includeSafeFile: false },
1045+
{ safe: false, includeSafeFile: true },
1046+
{ safe: true, includeSafeFile: true },
1047+
])(
1048+
'continues after document contributor admission (safe=$safe, mixed=$includeSafeFile)',
1049+
async ({ safe, includeSafeFile }) => {
1050+
const key = 'workspace/ws-1/report.pdf'
1051+
mockContext.workspaceId = 'ws-1'
1052+
const hydrationSpy = vi
1053+
.spyOn(userFileBase64, 'hydrateUserFilesWithBase64')
1054+
.mockImplementationOnce(async (files, options) => {
1055+
await options.onServableFileContributors?.(files[0], [
1056+
{
1057+
fileId: 'image-1',
1058+
key: 'workspace/ws-1/image-1.png',
1059+
context: 'workspace',
1060+
contentUpdatedAt: new Date('2026-08-06T00:00:00.000Z'),
1061+
},
1062+
])
1063+
return files.map((file) => ({ ...file, base64: 'JVBERi0=' }))
1064+
})
1065+
mockImportWorkspaceFileSecretProvenanceForModelView.mockResolvedValueOnce(safe)
10601066

1061-
try {
1062-
mockGetProviderFromModel.mockReturnValue('openai')
1067+
try {
1068+
mockGetProviderFromModel.mockReturnValue('openai')
10631069

1064-
await handler.execute(mockContext, mockBlock, {
1065-
model: 'gpt-4o',
1066-
userPrompt: 'Analyze this document',
1067-
files: [
1068-
{
1069-
id: 'file-1',
1070-
name: 'report.pdf',
1071-
path: `/api/files/serve/${encodeURIComponent(key)}?context=workspace`,
1072-
key,
1073-
size: 128,
1074-
type: 'text/x-python-pdf',
1075-
},
1076-
],
1077-
apiKey: 'test-api-key',
1078-
})
1079-
1080-
expect(mockExecuteProviderRequest.mock.calls[0][1].messages.at(-1)?.files).toEqual([])
1081-
expect(mockImportWorkspaceFileSecretProvenanceForModelView).toHaveBeenCalledWith(
1082-
expect.objectContaining({
1083-
workspaceId: mockContext.workspaceId,
1084-
view: 'opaque',
1085-
identity: expect.objectContaining({ fileId: 'image-1' }),
1070+
await handler.execute(mockContext, mockBlock, {
1071+
model: 'gpt-4o',
1072+
userPrompt: 'Analyze this document',
1073+
files: [
1074+
{
1075+
id: 'file-1',
1076+
name: 'report.pdf',
1077+
path: `/api/files/serve/${encodeURIComponent(key)}?context=workspace`,
1078+
key,
1079+
size: 128,
1080+
type: 'text/x-python-pdf',
1081+
},
1082+
...(includeSafeFile
1083+
? [
1084+
{
1085+
id: 'file-2',
1086+
name: 'safe.pdf',
1087+
path: '/safe.pdf',
1088+
key: 'workspace/ws-1/safe.pdf',
1089+
size: 128,
1090+
type: 'application/pdf',
1091+
},
1092+
]
1093+
: []),
1094+
],
1095+
apiKey: 'test-api-key',
10861096
})
1087-
)
1088-
} finally {
1089-
hydrationSpy.mockRestore()
1097+
1098+
expect(mockExecuteProviderRequest).toHaveBeenCalledOnce()
1099+
const sent = mockExecuteProviderRequest.mock.calls[0][1].messages.at(-1)
1100+
expect(sent.files.map((file: { id: string }) => file.id)).toEqual([
1101+
...(safe ? ['file-1'] : []),
1102+
...(includeSafeFile ? ['file-2'] : []),
1103+
])
1104+
if (safe) {
1105+
expect(sent.content).toBe('Analyze this document')
1106+
} else {
1107+
expect(sent.content).toMatch(
1108+
/^Analyze this document\n\nAttachment error: 1 requested file attachment was not provided/
1109+
)
1110+
expect(JSON.stringify(sent)).not.toContain(key)
1111+
}
1112+
expect(mockImportWorkspaceFileSecretProvenanceForModelView).toHaveBeenCalledWith(
1113+
expect.objectContaining({
1114+
workspaceId: mockContext.workspaceId,
1115+
view: 'opaque',
1116+
identity: expect.objectContaining({ fileId: 'image-1' }),
1117+
})
1118+
)
1119+
} finally {
1120+
hydrationSpy.mockRestore()
1121+
}
10901122
}
1091-
})
1123+
)
10921124

10931125
it('should reject files for providers without attachment support', async () => {
10941126
const inputs = {

‎apps/sim/executor/handlers/agent/agent-handler.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@ import {
3434
type RawFileInput,
3535
tryInferContextFromKey,
3636
} from '@/lib/uploads/utils/file-utils'
37-
import { selectModelBoundFileInputPaths } from '@/lib/uploads/utils/model-input'
37+
import {
38+
appendUnavailableAttachmentNotice,
39+
selectModelBoundFileInputPaths,
40+
} from '@/lib/uploads/utils/model-input'
3841
import { hydrateUserFilesWithBase64 } from '@/lib/uploads/utils/user-file-base64.server'
3942
import { resolveCustomBlockToolBinding } from '@/lib/workflows/custom-blocks/operations'
4043
import {
@@ -1602,8 +1605,13 @@ export class AgentBlockHandler implements BlockHandler {
16021605
)
16031606
}
16041607

1608+
const omittedCount = hydratedFiles.length - modelSafeHydratedFiles.length
16051609
nextMessages[messageIndex] = {
16061610
...message,
1611+
content:
1612+
omittedCount > 0
1613+
? appendUnavailableAttachmentNotice(message.content, omittedCount)
1614+
: message.content,
16071615
files: modelSafeHydratedFiles,
16081616
}
16091617
}

‎apps/sim/lib/copilot/request/lifecycle/run.test.ts‎

Lines changed: 96 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -665,31 +665,110 @@ describe('runCopilotLifecycle', () => {
665665
expect(sent.fileAttachments).toEqual([{ name: 'TOKEN.txt', key: 'safe-key' }])
666666
})
667667

668-
it('omits only unsafe durable attachments before the initial Go request', async () => {
669-
const unsafe = { id: 'wf-unsafe', name: 'unsafe.txt', key: 'workspace/ws-1/unsafe.txt' }
670-
const safe = { id: 'wf-safe', name: 'safe.txt', key: 'workspace/ws-1/safe.txt' }
671-
mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce([safe])
672-
let capturedRequestBody = ''
673-
mockRunStreamLoop.mockImplementationOnce(async (_url: string, request: RequestInit) => {
674-
capturedRequestBody = String(request.body)
675-
})
676-
677-
await runCopilotLifecycle(
678-
{
668+
it.each([
669+
{ key: 'attachments', includeSafeFile: false },
670+
{ key: 'attachments', includeSafeFile: true },
671+
{ key: 'fileAttachments', includeSafeFile: false },
672+
{ key: 'fileAttachments', includeSafeFile: true },
673+
])(
674+
'continues with an error notice for refused $key (mixed=$includeSafeFile)',
675+
async ({ key, includeSafeFile }) => {
676+
const unsafe = {
677+
id: 'wf-private',
678+
name: 'private-filename.txt',
679+
key: 'private-storage-key',
680+
base64: 'private-bytes',
681+
}
682+
const safe = { id: 'wf-safe', name: 'safe.txt', key: 'workspace/ws-1/safe.txt' }
683+
const safeFiles = includeSafeFile ? [safe] : []
684+
mockFilterModelSafeWorkspaceFileAttachments.mockResolvedValueOnce(safeFiles)
685+
const onError = vi.fn()
686+
mockRunStreamLoop.mockImplementationOnce(async (_url, _request, context) => {
687+
context.accumulatedContent = 'I can continue with the available inputs.'
688+
context.completionStatus = MothershipStreamV1CompletionStatus.complete
689+
})
690+
const payload = {
679691
message: 'Review files',
680-
fileAttachments: [unsafe, safe],
692+
[key]: [...safeFiles, unsafe],
681693
workspaceId: 'ws-1',
682694
messageId: 'stream-file-provenance',
683-
},
684-
{
695+
}
696+
const originalPayload = structuredClone(payload)
697+
698+
const result = await runCopilotLifecycle(payload, {
685699
userId: 'user-1',
686700
workspaceId: 'ws-1',
687701
executionContext: { userId: 'user-1', workflowId: '', workspaceId: 'ws-1' },
702+
onError,
703+
})
704+
705+
expect(result).toMatchObject({
706+
success: true,
707+
content: 'I can continue with the available inputs.',
708+
})
709+
expect(onError).not.toHaveBeenCalled()
710+
expect(mockRunStreamLoop).toHaveBeenCalledOnce()
711+
const sent = JSON.parse(String(mockRunStreamLoop.mock.calls[0][1].body))
712+
expect(sent.message).toMatch(
713+
/^Review files\n\nAttachment error: 1 requested file attachment was not provided/
714+
)
715+
expect(sent[key] ?? []).toEqual(safeFiles)
716+
expect(JSON.stringify(sent)).not.toContain('private-')
717+
expect(mockFilterModelSafeWorkspaceFileAttachments).toHaveBeenCalledWith(
718+
[...safeFiles, unsafe],
719+
{ workspaceId: 'ws-1' }
720+
)
721+
expect(payload).toEqual(originalPayload)
722+
}
723+
)
724+
725+
it.each(['messages', 'both', 'attachment-only', 'system-only'])(
726+
'reports combined attachment refusals in %s payloads without changing history',
727+
async (shape) => {
728+
const history = {
729+
role: 'assistant',
730+
content: 'Previous response',
731+
tool_calls: [{ id: 'existing-call' }],
732+
}
733+
const messages =
734+
shape === 'system-only'
735+
? [{ role: 'system', content: 'System context' }]
736+
: [history, { role: 'user', content: 'Review files' }]
737+
const payload = {
738+
...(shape === 'both' ? { message: 'Review files' } : {}),
739+
...(shape === 'attachment-only' ? {} : { messages }),
740+
attachments: [{ key: 'private-first-file' }],
741+
fileAttachments: [{ key: 'private-second-file' }],
688742
}
689-
)
743+
const original = structuredClone(payload)
744+
mockFilterModelSafeWorkspaceFileAttachments
745+
.mockResolvedValueOnce([])
746+
.mockResolvedValueOnce([])
747+
mockRunStreamLoop.mockResolvedValueOnce(undefined)
690748

691-
expect(JSON.parse(capturedRequestBody).fileAttachments).toEqual([safe])
692-
})
749+
const result = await runCopilotLifecycle(payload, {
750+
userId: 'user-1',
751+
workspaceId: 'ws-1',
752+
executionContext: { userId: 'user-1', workflowId: '', workspaceId: 'ws-1' },
753+
})
754+
755+
expect(result.success).toBe(true)
756+
const sent = JSON.parse(String(mockRunStreamLoop.mock.calls[0][1].body))
757+
const notice = 'Attachment error: 2 requested file attachments were not provided'
758+
if (shape === 'both' || shape === 'attachment-only') expect(sent.message).toContain(notice)
759+
if (shape !== 'attachment-only') {
760+
expect(sent.messages[0]).toEqual(messages[0])
761+
expect(sent.messages.at(-1)).toMatchObject({
762+
role: 'user',
763+
content: expect.stringContaining(notice),
764+
})
765+
}
766+
expect(sent).not.toHaveProperty('attachments')
767+
expect(sent).not.toHaveProperty('fileAttachments')
768+
expect(JSON.stringify(sent)).not.toContain('private-')
769+
expect(payload).toEqual(original)
770+
}
771+
)
693772

694773
it('rejects when durable attachment provenance cannot be verified', async () => {
695774
mockFilterModelSafeWorkspaceFileAttachments.mockRejectedValueOnce(new Error('db unavailable'))

‎apps/sim/lib/copilot/request/lifecycle/run.ts‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { PermissionType } from '@sim/platform-authz/workspace'
44
import { getErrorMessage, toError } from '@sim/utils/errors'
55
import { interruptibleSleep, sleep } from '@sim/utils/helpers'
66
import { generateId } from '@sim/utils/id'
7-
import { omit } from '@sim/utils/object'
7+
import { isPlainRecord, omit } from '@sim/utils/object'
88
import { workspaceSearchFiltersSchema } from '@/lib/api/contracts/knowledge/search'
99
import {
1010
type AttributedBillingRequestEnvelope,
@@ -72,6 +72,7 @@ import { env } from '@/lib/core/config/env'
7272
import { isCopilotToolPermissionsEnabled, isHosted } from '@/lib/core/config/env-flags'
7373
import { isWorkspaceCapabilityWithheld } from '@/lib/permission-groups/capability-assertions'
7474
import { filterModelSafeWorkspaceFileAttachments } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
75+
import { appendUnavailableAttachmentNotice } from '@/lib/uploads/utils/model-input'
7576
import type { ExecutorDelegationOrigin } from '@/executor/types'
7677
import { refuseResolvedSecretProjection } from '@/executor/utils/resolved-secret-projection-refusal'
7778
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
@@ -95,11 +96,12 @@ class CopilotModelContentProjectionError extends Error {
9596
}
9697
}
9798

98-
async function omitUnsafeInitialCopilotAttachments(
99+
async function prepareInitialCopilotAttachmentsForModel(
99100
payload: Record<string, unknown>,
100101
workspaceId?: string
101102
): Promise<Record<string, unknown>> {
102103
let projected = payload
104+
let omittedCount = 0
103105
for (const key of ['attachments', 'fileAttachments'] as const) {
104106
if (!Object.hasOwn(projected, key)) continue
105107
const attachments = projected[key]
@@ -129,21 +131,44 @@ async function omitUnsafeInitialCopilotAttachments(
129131
}
130132

131133
if (safeAttachments.length === attachments.length) continue
134+
omittedCount += attachments.length - safeAttachments.length
132135
logger.warn('Omitting Copilot attachments with unsafe secret provenance', {
133136
attachmentCount: attachments.length,
134137
omittedCount: attachments.length - safeAttachments.length,
135138
})
136139
projected =
137140
safeAttachments.length > 0 ? { ...projected, [key]: safeAttachments } : omit(projected, [key])
138141
}
139-
return projected
140-
}
142+
if (omittedCount === 0) return projected
141143

142-
async function filterInitialCopilotAttachmentsForModel(
143-
payload: Record<string, unknown>,
144-
workspaceId?: string
145-
): Promise<Record<string, unknown>> {
146-
return omitUnsafeInitialCopilotAttachments(payload, workspaceId)
144+
if (typeof projected.message === 'string') {
145+
projected = {
146+
...projected,
147+
message: appendUnavailableAttachmentNotice(projected.message, omittedCount),
148+
}
149+
}
150+
if (Array.isArray(projected.messages)) {
151+
const messages: unknown[] = [...projected.messages]
152+
let notified = false
153+
for (let index = messages.length - 1; index >= 0; index--) {
154+
const message = messages[index]
155+
if (!isPlainRecord(message) || message.role !== 'user' || typeof message.content !== 'string')
156+
continue
157+
messages[index] = {
158+
...message,
159+
content: appendUnavailableAttachmentNotice(message.content, omittedCount),
160+
}
161+
notified = true
162+
break
163+
}
164+
if (!notified) {
165+
messages.push({ role: 'user', content: appendUnavailableAttachmentNotice('', omittedCount) })
166+
}
167+
projected = { ...projected, messages }
168+
} else if (typeof projected.message !== 'string') {
169+
projected = { ...projected, message: appendUnavailableAttachmentNotice('', omittedCount) }
170+
}
171+
return projected
147172
}
148173

149174
async function ensureModelEgressRegistry(
@@ -412,7 +437,7 @@ export async function runCopilotLifecycle(
412437
}),
413438
}
414439
}
415-
const modelSafeRequestPayload = await filterInitialCopilotAttachmentsForModel(
440+
const modelSafeRequestPayload = await prepareInitialCopilotAttachmentsForModel(
416441
requestPayload,
417442
lifecycleOptions.workspaceId
418443
)

0 commit comments

Comments
 (0)