Skip to content

Commit f865717

Browse files
committed
fix(microsoft-word): reject non-Word targets, scope SharePoint access, and tighten input bounds
1 parent 1f53dae commit f865717

15 files changed

Lines changed: 279 additions & 40 deletions

File tree

apps/docs/content/docs/en/integrations/microsoft_word.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ List or search Microsoft Word (.docx) documents in OneDrive or SharePoint. Non-W
248248
| `query` | string | No | Search text matched against file name, metadata, and content. If omitted, the documents directly inside the folder are listed. |
249249
| `folderId` | string | No | The ID of the folder to list or search within. If omitted, the drive root is used. |
250250
| `driveId` | string | No | The ID of the drive to list from. Required for SharePoint. If omitted, uses the personal OneDrive. |
251-
| `pageSize` | number | No | Maximum number of items to request from Microsoft Graph \(1-$\{MAX_PAGE_SIZE\}, default $\{DEFAULT_PAGE_SIZE\}\) |
251+
| `pageSize` | number | No | Maximum number of items to request from Microsoft Graph \(1-200, default 50\) |
252252
| `pageToken` | string | No | Continuation URL from a previous response's nextPageToken, used to fetch the next page |
253253

254254
#### Output

apps/sim/app/api/tools/microsoft_word/append/route.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,49 @@ describe('POST /api/tools/microsoft_word/append', () => {
129129
)
130130
})
131131

132+
it('refuses to write Word bytes over a drive item that is not a .docx', async () => {
133+
const pdf = {
134+
id: 'doc-abc',
135+
name: 'invoice.pdf',
136+
cTag: 'tag-1',
137+
file: { mimeType: 'application/pdf' },
138+
}
139+
mockSecureFetchWithPinnedIP.mockResolvedValueOnce({
140+
ok: true,
141+
status: 200,
142+
statusText: '',
143+
headers: new Headers(),
144+
body: null,
145+
text: async () => JSON.stringify(pdf),
146+
json: async () => pdf,
147+
arrayBuffer: async () => new ArrayBuffer(0),
148+
})
149+
150+
const response = await POST(createMockRequest('POST', baseBody))
151+
152+
expect(response.status).toBe(400)
153+
const data = (await response.json()) as { error: string }
154+
expect(data.error).toMatch(/not a Word document/)
155+
expect(mockSecureFetchWithPinnedIP.mock.calls.some((call) => call[2]?.method === 'PUT')).toBe(
156+
false
157+
)
158+
})
159+
160+
it('reports a no-op without writing when the content adds no paragraph', async () => {
161+
mockSecureFetchWithPinnedIP
162+
.mockResolvedValueOnce(itemResponse('tag-1'))
163+
.mockResolvedValueOnce(await docxResponse())
164+
165+
const response = await POST(createMockRequest('POST', { ...baseBody, content: ' \n \n' }))
166+
167+
expect(response.status).toBe(200)
168+
const data = (await response.json()) as { output: { updatedContent: boolean } }
169+
expect(data.output.updatedContent).toBe(false)
170+
expect(mockSecureFetchWithPinnedIP.mock.calls.some((call) => call[2]?.method === 'PUT')).toBe(
171+
false
172+
)
173+
})
174+
132175
it('surfaces a 412 from the upload precondition as the same conflict', async () => {
133176
mockSecureFetchWithPinnedIP
134177
.mockResolvedValueOnce(itemResponse('tag-1'))

apps/sim/app/api/tools/microsoft_word/append/route.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,38 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4444

4545
try {
4646
const basePath = getDocumentBasePath(documentId, driveId ?? undefined)
47-
const contentTag = getContentTag(await fetchDocumentItem(basePath, accessToken))
47+
const existingItem = await fetchDocumentItem(basePath, accessToken)
48+
const contentTag = getContentTag(existingItem)
4849

4950
const existingBuffer = await downloadDocumentContent(basePath, accessToken)
50-
const updatedBuffer = await appendParagraphsToDocx(existingBuffer, content)
51+
const { buffer, paragraphsAppended } = await appendParagraphsToDocx(existingBuffer, content)
52+
53+
if (paragraphsAppended === 0) {
54+
logger.info(`[${requestId}] No paragraphs to append; document left untouched`, { documentId })
55+
return NextResponse.json({
56+
success: true,
57+
output: {
58+
updatedContent: false,
59+
metadata: toDocumentMetadata(existingItem, documentId),
60+
},
61+
})
62+
}
5163

5264
await assertContentUnchanged(basePath, accessToken, contentTag)
5365

5466
const item = await uploadDocumentContent(
5567
`${basePath}/content`,
5668
accessToken,
57-
updatedBuffer,
69+
buffer,
5870
DOCX_MIME_TYPE,
5971
contentTag
6072
)
6173

62-
logger.info(`[${requestId}] Appended to Word document`, { documentId, size: item.size })
74+
logger.info(`[${requestId}] Appended to Word document`, {
75+
documentId,
76+
paragraphsAppended,
77+
size: item.size,
78+
})
6379

6480
return NextResponse.json({
6581
success: true,

apps/sim/app/api/tools/onedrive/folders/route.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
3434
const validation = onedriveFoldersQuerySchema.safeParse({
3535
credentialId: searchParams.get('credentialId') ?? '',
3636
query: searchParams.get('query') ?? undefined,
37+
driveId: searchParams.get('driveId') ?? undefined,
3738
})
3839
if (!validation.success) {
3940
logger.warn(`[${requestId}] Invalid folders request data`, {
@@ -44,7 +45,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4445
{ status: 400 }
4546
)
4647
}
47-
const { credentialId } = validation.data
48+
const { credentialId, driveId } = validation.data
4849
const query = validation.data.query ?? ''
4950

5051
const credentialIdValidation = validateMicrosoftGraphId(credentialId, 'credentialId')
@@ -70,7 +71,19 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
7071
return NextResponse.json({ error: 'Failed to obtain valid access token' }, { status: 401 })
7172
}
7273

73-
let url = `https://graph.microsoft.com/v1.0/me/drive/root/children?$filter=folder ne null&$select=id,name,folder,webUrl,createdDateTime,lastModifiedDateTime&$top=${ONEDRIVE_FOLDERS_PAGE_SIZE}`
74+
// Scope to the requested drive so a SharePoint-targeted block does not list
75+
// the signed-in user's personal OneDrive folders instead.
76+
let drivePath = 'me/drive'
77+
if (driveId) {
78+
const driveIdValidation = validateMicrosoftGraphId(driveId, 'driveId')
79+
if (!driveIdValidation.isValid) {
80+
logger.warn(`[${requestId}] Invalid drive ID`, { error: driveIdValidation.error })
81+
return NextResponse.json({ error: driveIdValidation.error }, { status: 400 })
82+
}
83+
drivePath = `drives/${driveId}`
84+
}
85+
86+
let url = `https://graph.microsoft.com/v1.0/${drivePath}/root/children?$filter=folder ne null&$select=id,name,folder,webUrl,createdDateTime,lastModifiedDateTime&$top=${ONEDRIVE_FOLDERS_PAGE_SIZE}`
7487

7588
if (query) {
7689
url += `&$search="${encodeURIComponent(query)}"`

apps/sim/blocks/blocks/microsoft_word.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,8 @@ export const MicrosoftWordBlock: BlockConfig<MicrosoftWordResponse> = {
260260
selectorKey: 'onedrive.folders',
261261
requiredScopes: [],
262262
mimeType: 'application/vnd.microsoft.graph.folder',
263-
placeholder: 'Leave empty for the OneDrive root',
264-
dependsOn: ['credential'],
263+
placeholder: 'Leave empty to use the drive root',
264+
dependsOn: ['credential', 'driveId'],
265265
mode: 'basic',
266266
condition: { field: 'operation', value: ['create', 'create_from_template'] },
267267
},
@@ -291,8 +291,8 @@ export const MicrosoftWordBlock: BlockConfig<MicrosoftWordResponse> = {
291291
selectorKey: 'onedrive.folders',
292292
requiredScopes: [],
293293
mimeType: 'application/vnd.microsoft.graph.folder',
294-
placeholder: 'Leave empty for the OneDrive root',
295-
dependsOn: ['credential'],
294+
placeholder: 'Leave empty to use the drive root',
295+
dependsOn: ['credential', 'driveId'],
296296
mode: 'basic',
297297
condition: { field: 'operation', value: 'list' },
298298
},

apps/sim/hooks/selectors/providers/microsoft/selectors.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,13 @@ export const microsoftSelectors = {
227227
'selectors',
228228
'onedrive.folders',
229229
context.oauthCredential ?? 'none',
230+
context.driveId ?? 'none',
230231
],
231232
enabled: ({ context }) => Boolean(context.oauthCredential),
232233
fetchList: async ({ context, signal }: SelectorQueryArgs) => {
233234
const credentialId = ensureCredential(context, 'onedrive.folders')
234235
const data = await requestJson(selectorContracts.onedriveFoldersSelectorContract, {
235-
query: { credentialId },
236+
query: { credentialId, driveId: context.driveId },
236237
signal,
237238
})
238239
return (data.files || []).map((file) => ({

apps/sim/lib/api/contracts/selectors/microsoft.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,14 @@ export const onedriveFolderQuerySchema = z.object({
6868
})
6969

7070
export const onedriveFilesQuerySchema = credentialIdQueryWithSearchSchema
71-
export const onedriveFoldersQuerySchema = credentialIdQueryWithSearchSchema
71+
/**
72+
* Folder listing is drive-scoped like the file listing above: without `driveId`
73+
* the picker would always show the signed-in user's OneDrive, even for a block
74+
* pointed at a SharePoint document library.
75+
*/
76+
export const onedriveFoldersQuerySchema = credentialIdQueryWithSearchSchema.extend({
77+
driveId: z.string().optional(),
78+
})
7279
export const outlookFoldersQuerySchema = credentialIdQuerySchema
7380

7481
export const outlookFoldersSelectorContract = defineGetSelector(

apps/sim/lib/api/contracts/tools/microsoft.ts

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,47 @@ export const dataverseUploadFileBodySchema = z.object({
111111
*/
112112
const MAX_DOCUMENT_CONTENT_LENGTH = 2_000_000
113113

114+
/**
115+
* Ceiling on the serialized placeholder map. Replacement values are substituted
116+
* into a package that is rewritten in memory, so they need the same kind of
117+
* bound the generated document text has.
118+
*/
119+
const MAX_REPLACEMENTS_LENGTH = 200_000
120+
121+
/** Whether a string parses as a JSON object (not an array or scalar). */
122+
function isJsonObjectString(value: string): boolean {
123+
if (!value.trim()) return true
124+
try {
125+
const parsed: unknown = JSON.parse(value)
126+
return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)
127+
} catch {
128+
return false
129+
}
130+
}
131+
132+
/**
133+
* The placeholder map, accepted either as an object from the editor or as the
134+
* JSON string a variable reference resolves to. Both forms are bounded and both
135+
* must describe an object, so malformed caller input is a 400 rather than a 500
136+
* raised later while the template is being filled.
137+
*/
138+
const wordReplacementsSchema = z
139+
.union([
140+
z
141+
.string()
142+
.refine(
143+
isJsonObjectString,
144+
'Placeholder values must be a JSON object mapping each placeholder to its value'
145+
),
146+
z.record(z.string(), z.union([z.string(), z.number(), z.boolean(), z.null()])),
147+
])
148+
.refine(
149+
(value) =>
150+
(typeof value === 'string' ? value.length : JSON.stringify(value).length) <=
151+
MAX_REPLACEMENTS_LENGTH,
152+
'Placeholder values are too long'
153+
)
154+
114155
const wordDocumentIdSchema = z.string().min(1, 'Document ID is required')
115156
const wordDriveIdSchema = z.string().optional().nullable()
116157

@@ -148,13 +189,7 @@ export const microsoftWordCreateFromTemplateBodySchema = z.object({
148189
accessToken: accessTokenSchema,
149190
templateDocumentId: z.string().min(1, 'Template document ID is required'),
150191
name: z.string().min(1, 'Document name is required').max(255, 'Document name is too long'),
151-
replacements: z
152-
.union([
153-
z.string(),
154-
z.record(z.string(), z.union([z.string(), z.number(), z.boolean(), z.null()])),
155-
])
156-
.optional()
157-
.nullable(),
192+
replacements: wordReplacementsSchema.optional().nullable(),
158193
matchCase: z.boolean().optional().nullable(),
159194
folderId: z.string().optional().nullable(),
160195
driveId: wordDriveIdSchema,

apps/sim/lib/microsoft-word/document.server.test.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ describe('buildDocxFromContent', () => {
100100
describe('appendParagraphsToDocx', () => {
101101
it('keeps the existing content and adds the new paragraphs after it', async () => {
102102
const original = await buildDocxFromContent('Existing paragraph')
103-
const updated = await appendParagraphsToDocx(original, 'Appended paragraph')
103+
const { buffer: updated } = await appendParagraphsToDocx(original, 'Appended paragraph')
104104
const text = await extractDocxText(updated)
105105

106106
expect(text).toContain('Existing paragraph')
@@ -110,7 +110,7 @@ describe('appendParagraphsToDocx', () => {
110110

111111
it('inserts before the body-level section properties', async () => {
112112
const original = await buildDocxFromContent('Existing paragraph')
113-
const updated = await appendParagraphsToDocx(original, 'Appended paragraph')
113+
const { buffer: updated } = await appendParagraphsToDocx(original, 'Appended paragraph')
114114
const xml = await readDocumentXml(updated)
115115

116116
const appendedIndex = xml.indexOf('Appended paragraph')
@@ -122,7 +122,7 @@ describe('appendParagraphsToDocx', () => {
122122

123123
it('preserves every other part of the original package', async () => {
124124
const original = await buildDocxFromContent('# Heading\n- bullet')
125-
const updated = await appendParagraphsToDocx(original, 'More')
125+
const { buffer: updated } = await appendParagraphsToDocx(original, 'More')
126126

127127
const originalNames = Object.keys((await JSZip.loadAsync(original)).files).sort()
128128
const updatedNames = Object.keys((await JSZip.loadAsync(updated)).files).sort()
@@ -132,7 +132,7 @@ describe('appendParagraphsToDocx', () => {
132132

133133
it('escapes XML metacharacters instead of emitting broken markup', async () => {
134134
const original = await buildDocxFromContent('Existing')
135-
const updated = await appendParagraphsToDocx(original, 'a < b & c > d')
135+
const { buffer: updated } = await appendParagraphsToDocx(original, 'a < b & c > d')
136136
const xml = await readDocumentXml(updated)
137137

138138
expect(xml).toContain('a &lt; b &amp; c &gt; d')
@@ -141,12 +141,27 @@ describe('appendParagraphsToDocx', () => {
141141

142142
it('skips blank lines so appended text does not grow trailing paragraphs', async () => {
143143
const original = await buildDocxFromContent('Existing')
144-
const updated = await appendParagraphsToDocx(original, 'one\n\n\ntwo')
144+
const { buffer: updated } = await appendParagraphsToDocx(original, 'one\n\n\ntwo')
145145
const xml = await readDocumentXml(updated)
146146

147147
expect(xml.match(/<w:p><w:r><w:t xml:space="preserve">/g)).toHaveLength(2)
148148
})
149149

150+
it('reports a no-op for whitespace-only content and leaves the package untouched', async () => {
151+
const original = await buildDocxFromContent('Existing')
152+
const result = await appendParagraphsToDocx(original, ' \n\n \n')
153+
154+
expect(result.paragraphsAppended).toBe(0)
155+
expect(result.buffer).toBe(original)
156+
})
157+
158+
it('reports how many paragraphs it appended', async () => {
159+
const original = await buildDocxFromContent('Existing')
160+
const result = await appendParagraphsToDocx(original, 'one\ntwo\nthree')
161+
162+
expect(result.paragraphsAppended).toBe(3)
163+
})
164+
150165
it('rejects an archive that is not a Word package', async () => {
151166
const zip = new JSZip()
152167
zip.file('hello.txt', 'not a word document')
@@ -158,6 +173,22 @@ describe('appendParagraphsToDocx', () => {
158173
})
159174
})
160175

176+
describe('extractDocxText', () => {
177+
it('reads a blank Word document as empty text rather than failing', async () => {
178+
const blank = await buildDocxFromContent('')
179+
180+
await expect(extractDocxText(blank)).resolves.toBe('')
181+
})
182+
183+
it('still fails on an archive that is not a Word package', async () => {
184+
const zip = new JSZip()
185+
zip.file('hello.txt', 'not a word document')
186+
const buffer = await zip.generateAsync({ type: 'nodebuffer' })
187+
188+
await expect(extractDocxText(buffer)).rejects.toThrow()
189+
})
190+
})
191+
161192
describe('replaceTextInDocx', () => {
162193
it('replaces text inside a single run and keeps that run formatting', async () => {
163194
const original = await buildDocxWithRuns([

0 commit comments

Comments
 (0)