Skip to content

Commit 1f570d3

Browse files
waleedlatif1waleedlatif1
authored
fix(files): refuse Chat current-file reads no provenance observer records (#8864)
* fix(files): refuse Chat current-file reads no provenance observer records * test(files): run Chat file reads through the production CLI transport composition * test(sandbox): assert the observed file read is admitted, not the recorder call
1 parent 4beed5a commit 1f570d3

8 files changed

Lines changed: 257 additions & 57 deletions

File tree

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

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,11 @@ import { withWorkspaceInvocationScope } from '@/lib/core/application/workspace-i
33
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
44
import { curateBlockDetail } from '@/lib/mothership/agent-cli/curation'
55
import { AUGMENTATION_ENGINES, runEngine } from '@/lib/mothership/agent-cli/engines'
6-
import { createFileReadTransport } from '@/lib/mothership/agent-cli/file-read-transport'
7-
import { createFileUploadTransport } from '@/lib/mothership/agent-cli/file-upload-transport'
86
import { curateKnowledgeDocuments } from '@/lib/mothership/agent-cli/knowledge-curation'
9-
import { createResourceEffectTransport } from '@/lib/mothership/agent-cli/resource-effects'
107
import { runCli } from '@/lib/mothership/agent-cli/run-cli'
11-
import { createScopedCliTransport } from '@/lib/mothership/agent-cli/scoped-transport'
128
import { executeAgentCliService } from '@/lib/mothership/agent-cli/services'
139
import { applySink } from '@/lib/mothership/agent-cli/sink'
14-
import { createTableReadTransport } from '@/lib/mothership/agent-cli/table-read-transport'
15-
import { createTracedCliTransport } from '@/lib/mothership/agent-cli/traced-transport'
10+
import { createAgentCliTransport } from '@/lib/mothership/agent-cli/transport'
1611
import { createWorkbenchFileProvenance } from '@/lib/mothership/agent-cli/workbench-file-provenance'
1712
import { resolveInvocationWorkspace } from '@/lib/mothership/application/workspace-target'
1813
import {
@@ -79,44 +74,22 @@ async function executeBoundAgentCliRequest(
7974
const endpoint = getInternalApiBaseUrl()
8075
const sessionKey = context.chatId ? chatSandboxSessionKey(context.chatId) : null
8176
const files = sessionKey ? createWorkbenchFileProvenance({ ...context, sessionKey }) : undefined
82-
const reads = createFileReadTransport({
83-
endpoint,
84-
transport: createTableReadTransport({
85-
endpoint,
86-
transport: createTracedCliTransport(
87-
endpoint,
88-
createScopedCliTransport(endpoint, invocationIdentity)
89-
),
90-
registry: context.resolvedSecretTraceRegistry,
91-
}),
92-
userId: context.userId,
93-
invocation: invocationIdentity,
94-
registry: context.resolvedSecretTraceRegistry,
95-
...(context.chatId !== undefined ? { chatId: context.chatId } : {}),
96-
...(files ? { trackDownload: files.trackDownload } : {}),
97-
})
9877
const resources: ResourceChange[] = []
9978
const identity: EmbeddedCliIdentity = {
10079
endpoint,
10180
apiKey,
10281
workspaceId: context.workspaceId,
103-
transport: createResourceEffectTransport(
82+
transport: createAgentCliTransport({
10483
endpoint,
105-
files
106-
? createFileUploadTransport({
107-
endpoint,
108-
workspaceId: context.workspaceId,
109-
userId: context.userId,
110-
invocation: invocationIdentity,
111-
fallback: reads,
112-
uploadProvenance: files.uploadProvenance,
113-
})
114-
: reads,
84+
invocation: invocationIdentity,
85+
registry: context.resolvedSecretTraceRegistry,
86+
...(files ? { files } : {}),
11587
resources,
116-
request.invocation.kind === 'cli' ||
88+
observeReads:
89+
request.invocation.kind === 'cli' ||
11790
(request.invocation.kind === 'augmentation' &&
118-
AUGMENTATION_ENGINES[request.invocation.name]?.openReadResources === true)
119-
),
91+
AUGMENTATION_ENGINES[request.invocation.name]?.openReadResources === true),
92+
}),
12093
...(context.signal ? { signal: context.signal } : {}),
12194
}
12295

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { createFileReadTransport } from '@/lib/mothership/agent-cli/file-read-transport'
2+
import { createFileUploadTransport } from '@/lib/mothership/agent-cli/file-upload-transport'
3+
import { createResourceEffectTransport } from '@/lib/mothership/agent-cli/resource-effects'
4+
import { createScopedCliTransport } from '@/lib/mothership/agent-cli/scoped-transport'
5+
import { createTableReadTransport } from '@/lib/mothership/agent-cli/table-read-transport'
6+
import { createTracedCliTransport } from '@/lib/mothership/agent-cli/traced-transport'
7+
import type { createWorkbenchFileProvenance } from '@/lib/mothership/agent-cli/workbench-file-provenance'
8+
import type { CopilotChatDelegationContext } from '@/lib/mothership/auth/application-delegation'
9+
import type { ResourceChange } from '@/lib/mothership/generated/resources'
10+
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
11+
12+
/**
13+
* The transport every Chat CLI invocation and augmentation engine reaches Sim through.
14+
*
15+
* Order is load-bearing: the file read layer sits outside in-process admission so every file
16+
* read and every other `GET` runs under a delivery observer that records the secret provenance
17+
* of the bytes, and resource effects observe what the composed stack returns.
18+
*/
19+
export function createAgentCliTransport(context: {
20+
endpoint: string
21+
invocation: CopilotChatDelegationContext & { workspaceId: string }
22+
registry?: ResolvedSecretTraceRegistry
23+
files?: ReturnType<typeof createWorkbenchFileProvenance>
24+
resources: ResourceChange[]
25+
observeReads: boolean
26+
}): typeof fetch {
27+
const { endpoint, invocation, registry, files } = context
28+
const reads = createFileReadTransport({
29+
endpoint,
30+
transport: createTableReadTransport({
31+
endpoint,
32+
transport: createTracedCliTransport(endpoint, createScopedCliTransport(endpoint, invocation)),
33+
registry,
34+
}),
35+
userId: invocation.userId,
36+
invocation,
37+
registry,
38+
...(invocation.chatId !== undefined ? { chatId: invocation.chatId } : {}),
39+
...(files ? { trackDownload: files.trackDownload } : {}),
40+
})
41+
return createResourceEffectTransport(
42+
endpoint,
43+
files
44+
? createFileUploadTransport({
45+
endpoint,
46+
workspaceId: invocation.workspaceId,
47+
userId: invocation.userId,
48+
invocation,
49+
fallback: reads,
50+
uploadProvenance: files.uploadProvenance,
51+
})
52+
: reads,
53+
context.resources,
54+
context.observeReads
55+
)
56+
}

‎apps/sim/lib/mothership/tools/sandbox-resource-transport.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { createDelegatedPrincipal } from '@sim/testing/factories/principal.factory'
12
import {
23
mothershipWorkspaceTargetMock,
34
mothershipWorkspaceTargetMockFns,
@@ -7,6 +8,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
78
import { isCopilotRequest } from '@/lib/api/server/routes/copilot-request'
89
import { assertWorkspaceInvocationScope } from '@/lib/core/application/workspace-invocation-scope'
910
import { OrchestrationError } from '@/lib/core/orchestration/types'
11+
import {
12+
reportWorkspaceFileDelivery,
13+
requireCopilotWorkspaceFileDeliveryObserver,
14+
} from '@/lib/workspace-files/application/file-delivery-observer'
1015

1116
const { readScope, recordEffects, fetcher, routeMatcher, recordInput, mint } = vi.hoisted(() => ({
1217
readScope: vi.fn(),
@@ -275,6 +280,28 @@ it('cannot deliver a successful unclassified file response when recording its un
275280
expect(fetcher).toHaveBeenCalledOnce()
276281
})
277282

283+
/**
284+
* The file read and download use cases refuse a Chat principal that no delivery observer records,
285+
* so the workbench's file reads depend on this proxy installing one around the route handler.
286+
*/
287+
it('dispatches a workbench file read under a delivery observer the Copilot guard admits', async () => {
288+
routeMatcher.mockReturnValue({ params: { fileId: 'file' }, load: async () => ({ GET: fetcher }) })
289+
fetcher.mockImplementation(async () => {
290+
requireCopilotWorkspaceFileDeliveryObserver(createDelegatedPrincipal({ serviceId: 'copilot' }))
291+
await reportWorkspaceFileDelivery({ status: 'exact', entries: [] })
292+
return new Response('file text')
293+
})
294+
recordInput.mockResolvedValueOnce(undefined)
295+
296+
const response = await proxySandboxResourceRequest(
297+
request('/api/v2/files/file/text?workspaceId=workspace'),
298+
token
299+
)
300+
301+
expect(response.status).toBe(200)
302+
expect(await response.text()).toBe('file text')
303+
})
304+
278305
vi.mock('@/lib/mothership/chat/delegation', () => ({ mintDelegationToken: mint }))
279306

280307
it('never resolves a real credential for an invalid callback scope', async () => {

‎apps/sim/lib/workspace-files/__integration__/copilot-file-versions.integration.ts‎

Lines changed: 148 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/** Chat's in-process CLI reading and reverting workspace file version history as the delegating user. */
1+
/** Chat's in-process CLI reading workspace files and reverting their version history as the delegating user. */
22
import { mkdtempSync } from 'node:fs'
33
import { rm } from 'node:fs/promises'
44
import { tmpdir } from 'node:os'
@@ -35,15 +35,21 @@ import {
3535
createKnowledgeAclFixtureIds,
3636
seedKnowledgeAclFixture,
3737
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
38-
import { createFileReadTransport } from '@/lib/mothership/agent-cli/file-read-transport'
38+
import { runCli } from '@/lib/mothership/agent-cli/run-cli'
3939
import { createScopedCliTransport } from '@/lib/mothership/agent-cli/scoped-transport'
40+
import { createAgentCliTransport } from '@/lib/mothership/agent-cli/transport'
41+
import { createCopilotChatPrincipal } from '@/lib/mothership/auth/application-delegation'
4042
import {
4143
updateWorkspaceFileContent,
4244
uploadWorkspaceFile,
4345
} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
4446
import type { WorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
47+
import { WORKSPACE_FILES_DELEGATION_AUDIENCE } from '@/lib/workspace-files/application/authorization'
48+
import { downloadWorkspaceFileStream } from '@/lib/workspace-files/application/download-workspace-file'
49+
import { WorkspaceFileDeliveryUnobservedError } from '@/lib/workspace-files/application/file-delivery-observer'
4550
import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection'
4651
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
52+
import '@/app/api/v2/files/[fileId]/text/route'
4753
import '@/app/api/v2/files/[fileId]/versions/route'
4854
import '@/app/api/v2/files/[fileId]/versions/[version]/route'
4955
import '@/app/api/v2/files/[fileId]/versions/[version]/content/route'
@@ -121,36 +127,40 @@ describe('chat-delegated file version history', () => {
121127
}
122128

123129
/**
124-
* Chat's composed CLI transport: the provenance-observing read layer over in-process admission.
130+
* Chat's composed CLI transport, the same stack every Chat CLI invocation and engine runs on.
125131
* `layers` drops the outer layers to prove the inner ones hold on their own.
126132
*/
127-
function chatTransport(
133+
function chatFetch(
128134
fixture: { workspaceId: string; organizationId: string },
129135
userId: string,
130136
registry?: ResolvedSecretTraceRegistry,
131137
layers: { observer?: boolean; invocationScope?: boolean } = {}
132-
) {
138+
): typeof fetch {
133139
const invocation = { userId, workspaceId: fixture.workspaceId, chatId: generateId() }
134-
const scoped = createScopedCliTransport(ORIGIN, invocation)
135140
const transport =
136141
layers.observer === false
137-
? scoped
138-
: createFileReadTransport({
142+
? createScopedCliTransport(ORIGIN, invocation)
143+
: createAgentCliTransport({
139144
endpoint: ORIGIN,
140-
transport: scoped,
141-
userId,
142145
invocation,
143146
...(registry ? { registry } : {}),
147+
resources: [],
148+
observeReads: true,
144149
})
145-
return (url: string, init?: RequestInit) =>
150+
return (input, init) =>
146151
layers.invocationScope === false
147-
? transport(`${ORIGIN}${url}`, init)
152+
? transport(input, init)
148153
: withWorkspaceInvocationScope(
149154
{ workspaceId: fixture.workspaceId, organizationId: fixture.organizationId },
150-
() => transport(`${ORIGIN}${url}`, init)
155+
() => transport(input, init)
151156
)
152157
}
153158

159+
function chatTransport(...args: Parameters<typeof chatFetch>) {
160+
const transport = chatFetch(...args)
161+
return (url: string, init?: RequestInit) => transport(`${ORIGIN}${url}`, init)
162+
}
163+
154164
function registryFor(fixture: { workspaceId: string }, userId: string) {
155165
return new ResolvedSecretTraceRegistry([], { userId, workspaceId: fixture.workspaceId })
156166
}
@@ -402,4 +412,129 @@ describe('chat-delegated file version history', () => {
402412
expect(untracked.status).toBe(503)
403413
expect(await untracked.text()).not.toContain(SECRET)
404414
})
415+
416+
/**
417+
* The current-file text read and download admitted Chat whether or not a delivery observer would
418+
* record the secret provenance of the bytes; only the transport Chat composes today installed one.
419+
*/
420+
describe('current-file reads without a delivery observer', () => {
421+
async function revertToSecretVersion(fixture: Awaited<ReturnType<typeof seedVersionedFile>>) {
422+
const reverted = await chatTransport(fixture, fixture.aliceId)(
423+
`/api/v2/files/${fixture.fileId}/versions/2/revert`,
424+
{
425+
method: 'POST',
426+
headers: { 'content-type': 'application/json' },
427+
body: JSON.stringify({ workspaceId: fixture.workspaceId }),
428+
}
429+
)
430+
expect(reverted.status).toBe(200)
431+
}
432+
433+
function inWorkspace<T>(
434+
fixture: { workspaceId: string; organizationId: string },
435+
run: () => T
436+
) {
437+
return withWorkspaceInvocationScope(
438+
{ workspaceId: fixture.workspaceId, organizationId: fixture.organizationId },
439+
run
440+
)
441+
}
442+
443+
function download(
444+
fixture: { workspaceId: string; fileId: string },
445+
principal: Parameters<typeof downloadWorkspaceFileStream.execute>[0]['principal']
446+
) {
447+
return downloadWorkspaceFileStream.execute({
448+
principal,
449+
input: { fileId: fixture.fileId, assertedWorkspaceId: fixture.workspaceId },
450+
})
451+
}
452+
453+
it('refuses a Chat text read before returning the secret', async () => {
454+
const fixture = await seedVersionedFile()
455+
await revertToSecretVersion(fixture)
456+
457+
const response = await chatTransport(fixture, fixture.bobId, undefined, { observer: false })(
458+
`/api/v2/files/${fixture.fileId}/text?workspaceId=${fixture.workspaceId}`
459+
)
460+
461+
expect(response.status).toBe(503)
462+
const body = await response.text()
463+
expect(body).not.toContain(SECRET)
464+
expect(JSON.parse(body)).toMatchObject({ error: { code: 'SERVICE_UNAVAILABLE' } })
465+
})
466+
467+
it('refuses a Chat download before streaming any bytes', async () => {
468+
const fixture = await seedVersionedFile()
469+
const principal = createCopilotChatPrincipal(
470+
{ userId: fixture.bobId, workspaceId: fixture.workspaceId, chatId: generateId() },
471+
WORKSPACE_FILES_DELEGATION_AUDIENCE
472+
)
473+
474+
await expect(inWorkspace(fixture, () => download(fixture, principal))).rejects.toThrow(
475+
WorkspaceFileDeliveryUnobservedError
476+
)
477+
})
478+
479+
it('still serves a workflow run, which no Chat model reads', async () => {
480+
const fixture = await seedVersionedFile()
481+
await revertToSecretVersion(fixture)
482+
const now = Date.now()
483+
484+
const result = await inWorkspace(fixture, () =>
485+
download(fixture, {
486+
kind: 'delegated',
487+
serviceId: 'executor',
488+
subjectUserId: fixture.bobId,
489+
workspaceId: fixture.workspaceId,
490+
delegationId: generateId(),
491+
audience: WORKSPACE_FILES_DELEGATION_AUDIENCE,
492+
issuedAt: new Date(now),
493+
expiresAt: new Date(now + 60_000),
494+
})
495+
)
496+
497+
expect(await new Response(result.stream).text()).toBe(`token=${SECRET}`)
498+
})
499+
500+
it('serves a Chat `files read` through the composed CLI', async () => {
501+
const fixture = await seedVersionedFile()
502+
await revertToSecretVersion(fixture)
503+
const registry = registryFor(fixture, fixture.bobId)
504+
505+
const result = await runCli(
506+
['files', 'read', fixture.fileId],
507+
{
508+
endpoint: ORIGIN,
509+
apiKey: 'mothership-in-process',
510+
workspaceId: fixture.workspaceId,
511+
transport: chatFetch(fixture, fixture.bobId, registry),
512+
},
513+
null
514+
)
515+
516+
expect(result.exitCode, result.stderr).toBe(0)
517+
const projected = projectResolvedSecretModelContent(result.stdout, registry)
518+
if (!projected.safe) throw new Error('File text was withheld')
519+
expect(projected.value).not.toContain(SECRET)
520+
expect(projected.value).toContain('token=[REDACTED_SECRET]')
521+
})
522+
523+
it('serves Chat through the transport that records provenance', async () => {
524+
const fixture = await seedVersionedFile()
525+
await revertToSecretVersion(fixture)
526+
const registry = registryFor(fixture, fixture.bobId)
527+
528+
const response = await chatTransport(
529+
fixture,
530+
fixture.bobId,
531+
registry
532+
)(`/api/v2/files/${fixture.fileId}?workspaceId=${fixture.workspaceId}`)
533+
534+
expect(response.status).toBe(200)
535+
const projected = projectResolvedSecretModelContent(await response.text(), registry)
536+
if (!projected.safe) throw new Error('Downloaded file was withheld')
537+
expect(projected.value).toBe('token=[REDACTED_SECRET]')
538+
})
539+
})
405540
})

‎apps/sim/lib/workspace-files/application/download-workspace-file.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/appl
1717
import {
1818
hasWorkspaceFileDeliveryObserver,
1919
reportWorkspaceFileDelivery,
20+
requireCopilotWorkspaceFileDeliveryObserver,
2021
} from '@/lib/workspace-files/application/file-delivery-observer'
2122
import { fileOperations } from '@/lib/workspace-files/application/operations'
2223
import { resolveRenderedWorkspaceArtifact } from '@/lib/workspace-files/application/resolve-rendered-workspace-artifact'
@@ -100,6 +101,7 @@ async function executeDownloadWorkspaceFileStream({
100101
DownloadWorkspaceFileInput,
101102
ActiveWorkspaceFileContext
102103
>): Promise<DownloadWorkspaceFileStreamResult> {
104+
requireCopilotWorkspaceFileDeliveryObserver(principal)
103105
const file = await getWorkspaceFile(context.workspaceId, context.fileId, {
104106
throwOnError: true,
105107
})

0 commit comments

Comments
 (0)