Skip to content

Commit b2992d4

Browse files
Sg312Sg312
committed
fix(mothership): preserve file upload and preview resource context
1 parent 0703ed8 commit b2992d4

9 files changed

Lines changed: 386 additions & 15 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/html-preview-navigation.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/** @vitest-environment jsdom */
2+
import { expect, it, vi } from 'vitest'
3+
import { buildHtmlPreviewDocument } from '@/app/workspace/[workspaceId]/files/components/file-viewer/preview-panel'
4+
5+
it('keeps fragment navigation local, decodes targets and preserves external handoff', () => {
6+
const html = buildHtmlPreviewDocument('<html><head></head><body></body></html>')
7+
const script = html.match(/<script>([\s\S]*?)<\/script>/)?.[1]
8+
expect(script).toBeDefined()
9+
document.body.innerHTML =
10+
'<a href="#section">Section</a><section id="section" tabindex="-1"></section><a href="#long%20name">Encoded</a><section id="long name" tabindex="-1"></section><a href="#missing">Missing</a><a href="https://example.com/help">External</a>'
11+
const localLocation = { hash: '' }
12+
const scroll = vi.fn()
13+
const handoff = vi.fn()
14+
const listeners: Array<[string, EventListener]> = []
15+
const scopedDocument = {
16+
getElementById: document.getElementById.bind(document),
17+
addEventListener: (name: string, listener: EventListener) => listeners.push([name, listener]),
18+
}
19+
for (const section of document.querySelectorAll('section')) section.scrollIntoView = scroll
20+
new Function('document', 'location', 'Element', 'HTMLAnchorElement', 'parent', script!)(
21+
scopedDocument,
22+
localLocation,
23+
Element,
24+
HTMLAnchorElement,
25+
{ postMessage: handoff }
26+
)
27+
const click = listeners.find(([name]) => name === 'click')![1]
28+
for (const [index, hash, focused] of [
29+
[0, '#section', 'section'],
30+
[1, '#long%20name', 'long name'],
31+
[2, '#missing', 'long name'],
32+
] as const) {
33+
const event = new MouseEvent('click', { cancelable: true })
34+
Object.defineProperty(event, 'target', { value: document.querySelectorAll('a')[index] })
35+
click(event)
36+
expect(event.defaultPrevented).toBe(true)
37+
expect(localLocation.hash).toBe(hash)
38+
expect(document.activeElement?.id).toBe(focused)
39+
}
40+
expect(scroll).toHaveBeenCalledTimes(2)
41+
expect(handoff).not.toHaveBeenCalled()
42+
const event = new MouseEvent('click', { cancelable: true })
43+
Object.defineProperty(event, 'target', { value: document.querySelectorAll('a')[3] })
44+
click(event)
45+
expect(event.defaultPrevented).toBe(true)
46+
expect(handoff).toHaveBeenCalledWith({ __simPageNav: 'https://example.com/help' }, '*')
47+
})

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/preview-panel.tsx‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ const HTML_PREVIEW_CSP = [
129129

130130
const HTML_PREVIEW_BOOTSTRAP = `<script>
131131
(() => {
132-
const allowHref = (href) => href.startsWith('#') || /^\\s*javascript:/i.test(href)
132+
const allowHref = (href) => /^\\s*javascript:/i.test(href)
133133
134134
document.addEventListener(
135135
'click',
@@ -138,6 +138,21 @@ const HTML_PREVIEW_BOOTSTRAP = `<script>
138138
const anchor = event.target.closest('a[href]')
139139
if (!(anchor instanceof HTMLAnchorElement)) return
140140
const href = anchor.getAttribute('href') || ''
141+
/** The host's base-uri CSP can reject about:srcdoc; fragments must stay in this document. */
142+
if (href.startsWith('#')) {
143+
event.preventDefault()
144+
location.hash = href
145+
let id = href.slice(1)
146+
try { id = decodeURIComponent(id) } catch {}
147+
const target = document.getElementById(id)
148+
if (target) {
149+
target.scrollIntoView()
150+
target.focus({ preventScroll: true })
151+
} else if (!id) {
152+
window.scrollTo(0, 0)
153+
}
154+
return
155+
}
141156
if (allowHref(href)) return
142157
event.preventDefault()
143158
// The sandbox can neither navigate nor open windows, so hand the click

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

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
assertAuthBinding: vi.fn(),
8+
completeSession: vi.fn(),
9+
finalizePurpose: vi.fn(),
10+
getOwnedSession: vi.fn(),
11+
getPrincipalSession: vi.fn(),
12+
reauthorizeWorkspacePurpose: vi.fn(),
13+
getWorkspaceFile: vi.fn(),
14+
createSession: vi.fn(),
15+
authorizeCreate: vi.fn(),
16+
attribution: vi.fn(),
17+
authorizeOrganizationAttachment: vi.fn(),
18+
authorizeOrganizationLogo: vi.fn(),
19+
}))
20+
21+
vi.mock('@/lib/uploads/contexts/organization-assistant/application', () => ({
22+
authorizeOrganizationAttachmentControl: mocks.authorizeOrganizationAttachment,
23+
createOrganizationAssistantAttachment: vi.fn(),
24+
}))
25+
26+
vi.mock('@/lib/uploads/contexts/organization-logo/application', () => ({
27+
authorizeOrganizationLogoControl: mocks.authorizeOrganizationLogo,
28+
createOrganizationLogoUpload: vi.fn(),
29+
}))
30+
31+
vi.mock('@/lib/uploads/contexts/workspace', () => ({
32+
getWorkspaceFile: mocks.getWorkspaceFile,
33+
}))
34+
35+
vi.mock('@/lib/uploads/upload-session/service', () => ({
36+
abortUploadSession: vi.fn(),
37+
assertUploadSessionAuthBinding: mocks.assertAuthBinding,
38+
completeUploadSession: mocks.completeSession,
39+
createUploadPartUrls: vi.fn(),
40+
createUploadSession: mocks.createSession,
41+
getOwnedUploadSession: mocks.getOwnedSession,
42+
getPrincipalUploadSession: mocks.getPrincipalSession,
43+
}))
44+
45+
vi.mock('@/app/api/files/uploads/finalizers', () => ({
46+
finalizeUploadPurpose: mocks.finalizePurpose,
47+
finalizeWorkspaceFileUpload: vi.fn(),
48+
loadCompletedUploadPurpose: vi.fn(),
49+
loadCompletedWorkspaceFileUpload: vi.fn(),
50+
}))
51+
52+
vi.mock('@/app/api/files/uploads/purposes', () => ({
53+
createPurposeUploadSession: vi.fn(),
54+
reauthorizeUploadPurpose: vi.fn(),
55+
reauthorizeWorkspaceUploadPurpose: mocks.reauthorizeWorkspacePurpose,
56+
resolveUploadAttributionUserId: mocks.attribution,
57+
}))
58+
59+
vi.mock('@/lib/workspace-files/application/workspace-operation-context', () => ({
60+
authorizeWorkspaceFileOperation: mocks.authorizeCreate,
61+
}))
62+
vi.mock('@/lib/folders/queries', () => ({
63+
loadActiveFolderPathIndex: async () => new Map(),
64+
resolveFolderPathFromIndex: () => null,
65+
}))
66+
67+
import { OrchestrationError } from '@/lib/core/orchestration/types'
68+
import { createFileUploadTransport } from '@/lib/mothership/agent-cli/file-upload-transport'
69+
70+
const workspaceId = '7727ef3f-8cf6-4686-b063-2bb006a10785'
71+
const endpoint = 'http://localhost:3400'
72+
const cloudTransfer = { method: 'put', url: 'https://storage.test/signed-object', headers: {} }
73+
const session = {
74+
id: 'upload',
75+
status: 'uploading',
76+
fileName: 'deck.pptx',
77+
contentType: 'application/octet-stream',
78+
fileSize: 4,
79+
expiresAt: new Date('2099-01-01'),
80+
error: null,
81+
uploadToken: 'fixture',
82+
transfer: cloudTransfer,
83+
}
84+
const fallback = vi.fn()
85+
function upload(headers: Record<string, string> = {}) {
86+
const transport = createFileUploadTransport({
87+
endpoint,
88+
workspaceId,
89+
userId: 'reader',
90+
invocation: { userId: 'reader', workspaceId, chatId: 'chat' },
91+
fallback,
92+
uploadProvenance: () => ({ status: 'exact', entries: [] }),
93+
})
94+
return transport(`${endpoint}/api/v2/files/uploads`, {
95+
method: 'POST',
96+
headers: { 'content-type': 'application/json', ...headers },
97+
body: JSON.stringify({
98+
workspaceId,
99+
name: 'deck.pptx',
100+
contentType: 'application/octet-stream',
101+
size: 4,
102+
}),
103+
})
104+
}
105+
beforeEach(() => {
106+
vi.resetAllMocks()
107+
mocks.authorizeCreate.mockResolvedValue(undefined)
108+
mocks.attribution.mockResolvedValue('reader')
109+
mocks.createSession.mockResolvedValue(session)
110+
})
111+
describe('embedded upload application request context', () => {
112+
it.each([
113+
{},
114+
{
115+
origin: 'https://untrusted.test',
116+
host: 'untrusted.test',
117+
'x-forwarded-host': 'untrusted.test',
118+
'x-forwarded-proto': 'https',
119+
},
120+
])(
121+
'supplies the configured origin to the real authorized operation despite headers %j',
122+
async (headers) => {
123+
const response = await upload(headers)
124+
expect(response.status).toBe(200)
125+
expect(mocks.authorizeCreate).toHaveBeenCalledWith(
126+
expect.objectContaining({
127+
kind: 'delegated',
128+
subjectUserId: 'reader',
129+
workspaceId,
130+
audience: 'sim:workspace-files',
131+
}),
132+
expect.objectContaining({ id: 'files.upload.create', minimumRole: 'write' }),
133+
workspaceId
134+
)
135+
expect(mocks.createSession).toHaveBeenCalledWith(
136+
expect.objectContaining({ localOrigin: endpoint, workspaceId, secretProvenance: 'pending' })
137+
)
138+
expect((await response.json()).data.transfer).toEqual(cloudTransfer)
139+
expect(fallback).not.toHaveBeenCalled()
140+
}
141+
)
142+
it('still rejects canonical authorization failure before creating an upload session', async () => {
143+
mocks.authorizeCreate.mockRejectedValue(new OrchestrationError('forbidden', 'Access denied'))
144+
expect((await upload()).status).toBe(403)
145+
expect(mocks.createSession).not.toHaveBeenCalled()
146+
})
147+
})

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ export function createFileUploadTransport(context: {
5353
return context.fallback(input, init)
5454
}
5555
const request = new NextRequest(new Request(input, init))
56+
/** In-process CLI requests have no network origin headers; signing uses the configured endpoint. */
57+
request.headers.set('origin', base.origin)
5658
try {
5759
request.signal.throwIfAborted()
5860
const principal = context.invocation

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ async function executeBoundAgentCliRequest(
172172
} else throw new Error('Service invocation must use the service bridge')
173173
if (resources.length)
174174
result = { ...result, resources: [...resources, ...(result.resources ?? [])] }
175-
if (context.chatOrganizationId && result.resources?.length)
175+
if (result.resources?.length)
176176
result = {
177177
...result,
178178
resources: result.resources.map((effect) => {

‎apps/sim/lib/mothership/agent-cli/resource-owner.test.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/** @vitest-environment node */
2+
import type { EmbeddedCliIdentity } from 'sim/embed'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
const mocks = vi.hoisted(() => ({ target: vi.fn(), transport: vi.fn(), file: vi.fn() }))
6+
vi.mock('@/lib/mothership/application/workspace-target', () => ({
7+
resolveInvocationWorkspace: mocks.target,
8+
}))
9+
vi.mock('@/lib/mothership/agent-cli/scoped-transport', () => ({
10+
createScopedCliTransport: () => mocks.transport,
11+
}))
12+
vi.mock('@/lib/mothership/agent-cli/workbench-file-provenance', () => ({
13+
createWorkbenchFileProvenance: () => ({ observeOutput: vi.fn() }),
14+
}))
15+
vi.mock('@/lib/mothership/agent-cli/file-upload-transport', () => ({
16+
createFileUploadTransport: ({ fallback }: { fallback: typeof fetch }) => fallback,
17+
}))
18+
vi.mock('@/lib/mothership/application/execute-file-use-case', () => ({
19+
executeCopilotFileUseCase: mocks.file,
20+
}))
21+
vi.mock('@/lib/mothership/agent-cli/run-cli', () => ({
22+
async runCli(_argv: string[], identity: EmbeddedCliIdentity) {
23+
const response = await identity.transport!(
24+
`${identity.endpoint}/api/v2/files/uploads/upload/complete?workspaceId=${identity.workspaceId}`,
25+
{ method: 'POST' }
26+
)
27+
return { exitCode: 0, stdout: await response.text(), stderr: '' }
28+
},
29+
}))
30+
31+
import { executeAgentCliRequest } from '@/lib/mothership/agent-cli'
32+
import { getChatResourceKey } from '@/lib/mothership/resources/types'
33+
import { openResourceServerTool } from '@/lib/mothership/tools/server/open-resource'
34+
35+
const first = '00000000-0000-4000-8000-000000000001'
36+
const second = '00000000-0000-4000-8000-000000000002'
37+
const fileId = 'wf_uploaded_file'
38+
const file = {
39+
id: fileId,
40+
name: 'upload-acceptance.txt',
41+
webUrl: `https://sim.test/files/${fileId}`,
42+
size: 59,
43+
type: 'text/plain',
44+
key: 'workspace/upload-acceptance.txt',
45+
folderPath: '/',
46+
uploadedByEmail: 'fixture@example.com',
47+
uploadedAt: '2026-09-16T00:00:00.000Z',
48+
updatedAt: '2026-09-16T00:00:00.000Z',
49+
deletedAt: null,
50+
}
51+
const context = {
52+
userId: 'actor',
53+
workspaceId: first,
54+
chatId: 'chat',
55+
toolCallId: 'call',
56+
requestMode: 'agent',
57+
copilotToolExecution: true,
58+
}
59+
60+
beforeEach(() => {
61+
vi.clearAllMocks()
62+
mocks.target.mockImplementation(async (_context, target) => ({
63+
workspaceId: target ?? first,
64+
userId: 'actor',
65+
permission: 'write',
66+
}))
67+
mocks.file.mockResolvedValue({ file })
68+
mocks.transport.mockImplementation(async () =>
69+
Response.json({
70+
data: {
71+
id: 'upload',
72+
status: 'completed',
73+
name: file.name,
74+
contentType: file.type,
75+
size: file.size,
76+
expiresAt: '2099-01-01T00:00:00.000Z',
77+
error: null,
78+
file,
79+
},
80+
})
81+
)
82+
})
83+
84+
describe('scoped CLI resource ownership', () => {
85+
it('gives a workspace upload completion and explicit open the same resource identity', async () => {
86+
const upload = await executeAgentCliRequest(
87+
{ invocation: { kind: 'cli', argv: ['files', 'upload', '@/tmp/upload-acceptance.txt'] } },
88+
context
89+
)
90+
const opened = await openResourceServerTool.execute(
91+
{ resources: [{ type: 'file', id: fileId }] },
92+
context
93+
)
94+
const effect = upload.resources?.[0]
95+
if (effect?.op !== 'upsert') throw new Error('Upload completion must publish a file')
96+
const resource = effect.resource
97+
expect(resource).toEqual({
98+
type: 'file',
99+
id: fileId,
100+
title: file.name,
101+
path: 'files/upload-acceptance.txt',
102+
workspaceId: first,
103+
})
104+
expect(getChatResourceKey(resource)).toBe(getChatResourceKey(opened.resources[0]))
105+
expect(mocks.transport).toHaveBeenCalledTimes(1)
106+
})
107+
108+
it('keeps organization targets distinct even when a resource ID is reused', async () => {
109+
const keys: string[] = []
110+
for (const workspaceId of [first, second]) {
111+
const result = await executeAgentCliRequest(
112+
{
113+
workspaceId,
114+
invocation: { kind: 'cli', argv: ['files', 'upload', '@/tmp/upload-acceptance.txt'] },
115+
},
116+
{ ...context, workspaceId: undefined, organizationId: 'org' }
117+
)
118+
const effect = result.resources?.[0]
119+
if (effect?.op !== 'upsert') throw new Error('Upload completion must publish a file')
120+
expect(effect.resource.workspaceId).toBe(workspaceId)
121+
keys.push(getChatResourceKey(effect.resource))
122+
}
123+
expect(new Set(keys).size).toBe(2)
124+
})
125+
})

0 commit comments

Comments
 (0)