Skip to content

Commit 1f53dae

Browse files
committed
fix(microsoft-word): guard document edits against concurrent overwrites
1 parent 2de12d1 commit 1f53dae

14 files changed

Lines changed: 254 additions & 19 deletions

File tree

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Integrate Microsoft Word into the workflow. Create .docx documents from text, fi
7979

8080
### Create Microsoft Word Document
8181

82-
Create a new Microsoft Word (.docx) document in OneDrive or SharePoint from text content. Supports Markdown headings (# ## ###), bullets (-), and inline **bold** / *italic*.
82+
Create a new Microsoft Word (.docx) document in OneDrive or SharePoint from text content. Supports Markdown headings (# ## ###), bullets (-), and inline **bold** / *italic*. An existing document with the same name is never overwritten — the new one is given a unique name instead.
8383

8484
#### Input
8585

@@ -105,7 +105,7 @@ Create a new Microsoft Word (.docx) document in OneDrive or SharePoint from text
105105

106106
### Create Microsoft Word Document from Template
107107

108-
Copy an existing Microsoft Word (.docx) template to a new document and fill its placeholders. The template keeps all of its formatting, styles, headers, and footers, and is never modified.
108+
Copy an existing Microsoft Word (.docx) template to a new document and fill its placeholders. The template keeps all of its formatting, styles, headers, and footers, and is never modified. An existing document with the same name is never overwritten — the new one is given a unique name instead.
109109

110110
#### Input
111111

@@ -185,7 +185,7 @@ Replace the entire contents of an existing Microsoft Word (.docx) document with
185185

186186
### Append to Microsoft Word Document
187187

188-
Append plain-text paragraphs to the end of an existing Microsoft Word (.docx) document, leaving the existing content and formatting intact.
188+
Append plain-text paragraphs to the end of an existing Microsoft Word (.docx) document, leaving the existing content and formatting intact. Fails rather than overwriting if someone else changed the document while the edit was in flight.
189189

190190
#### Input
191191

@@ -211,7 +211,7 @@ Append plain-text paragraphs to the end of an existing Microsoft Word (.docx) do
211211

212212
### Replace Text in Microsoft Word Document
213213

214-
Find and replace text throughout a Microsoft Word (.docx) document, including its headers and footers. Use this to fill placeholders in a template document.
214+
Find and replace text throughout a Microsoft Word (.docx) document, including its headers and footers. Use this to fill placeholders in a template document. Fails rather than overwriting if someone else changed the document while the edit was in flight.
215215

216216
#### Input
217217

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import {
5+
createMockRequest,
6+
hybridAuthMockFns,
7+
inputValidationMock,
8+
inputValidationMockFns,
9+
} from '@sim/testing'
10+
import { beforeEach, describe, expect, it, vi } from 'vitest'
11+
12+
vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
13+
14+
import { buildDocxFromContent } from '@/lib/microsoft-word/document.server'
15+
import { POST } from '@/app/api/tools/microsoft_word/append/route'
16+
17+
const { mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP } = inputValidationMockFns
18+
19+
const PINNED_IP = '93.184.216.34'
20+
21+
const baseBody = {
22+
accessToken: 'token-123',
23+
documentId: 'doc-abc',
24+
content: 'Appended paragraph',
25+
}
26+
27+
/** A Graph `driveItem` metadata response carrying a content tag. */
28+
function itemResponse(cTag: string) {
29+
const body = {
30+
id: 'doc-abc',
31+
name: 'notes.docx',
32+
cTag,
33+
file: { mimeType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' },
34+
}
35+
return {
36+
ok: true,
37+
status: 200,
38+
statusText: '',
39+
headers: new Headers(),
40+
body: null,
41+
text: async () => JSON.stringify(body),
42+
json: async () => body,
43+
arrayBuffer: async () => new ArrayBuffer(0),
44+
}
45+
}
46+
47+
/** A Graph content response carrying a real `.docx` package. */
48+
async function docxResponse() {
49+
const buffer = await buildDocxFromContent('Existing paragraph')
50+
return {
51+
ok: true,
52+
status: 200,
53+
statusText: '',
54+
headers: new Headers(),
55+
body: null,
56+
text: async () => '',
57+
json: async () => ({}),
58+
arrayBuffer: async () =>
59+
buffer.buffer.slice(buffer.byteOffset, buffer.byteOffset + buffer.byteLength),
60+
}
61+
}
62+
63+
function preconditionFailedResponse() {
64+
return {
65+
ok: false,
66+
status: 412,
67+
statusText: 'Precondition Failed',
68+
headers: new Headers(),
69+
body: null,
70+
text: async () => '',
71+
json: async () => ({}),
72+
arrayBuffer: async () => new ArrayBuffer(0),
73+
}
74+
}
75+
76+
beforeEach(() => {
77+
vi.clearAllMocks()
78+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
79+
success: true,
80+
userId: 'user-1',
81+
authType: 'internal_jwt',
82+
})
83+
mockValidateUrlWithDNS.mockResolvedValue({
84+
isValid: true,
85+
resolvedIP: PINNED_IP,
86+
originalHostname: 'graph.microsoft.com',
87+
})
88+
})
89+
90+
describe('POST /api/tools/microsoft_word/append', () => {
91+
it('uploads the edit when the document has not changed, guarded by its content tag', async () => {
92+
mockSecureFetchWithPinnedIP
93+
.mockResolvedValueOnce(itemResponse('tag-1'))
94+
.mockResolvedValueOnce(await docxResponse())
95+
.mockResolvedValueOnce(itemResponse('tag-1'))
96+
.mockResolvedValueOnce(itemResponse('tag-2'))
97+
98+
const response = await POST(createMockRequest('POST', baseBody))
99+
100+
expect(response.status).toBe(200)
101+
const data = (await response.json()) as {
102+
success: boolean
103+
output: { updatedContent: boolean }
104+
}
105+
expect(data.success).toBe(true)
106+
expect(data.output.updatedContent).toBe(true)
107+
108+
const uploadCall = mockSecureFetchWithPinnedIP.mock.calls.at(-1)
109+
expect(uploadCall?.[2]).toMatchObject({ method: 'PUT' })
110+
expect(uploadCall?.[2].headers).toMatchObject({ 'if-match': 'tag-1' })
111+
})
112+
113+
it('refuses to overwrite a document that changed while the edit was in flight', async () => {
114+
mockSecureFetchWithPinnedIP
115+
.mockResolvedValueOnce(itemResponse('tag-1'))
116+
.mockResolvedValueOnce(await docxResponse())
117+
.mockResolvedValueOnce(itemResponse('tag-2'))
118+
119+
const response = await POST(createMockRequest('POST', baseBody))
120+
121+
expect(response.status).toBe(409)
122+
const data = (await response.json()) as { success: boolean; error: string }
123+
expect(data.success).toBe(false)
124+
expect(data.error).toMatch(/no other change was overwritten/)
125+
126+
// The conflict is detected before the PUT, so nothing was written.
127+
expect(mockSecureFetchWithPinnedIP.mock.calls.some((call) => call[2]?.method === 'PUT')).toBe(
128+
false
129+
)
130+
})
131+
132+
it('surfaces a 412 from the upload precondition as the same conflict', async () => {
133+
mockSecureFetchWithPinnedIP
134+
.mockResolvedValueOnce(itemResponse('tag-1'))
135+
.mockResolvedValueOnce(await docxResponse())
136+
.mockResolvedValueOnce(itemResponse('tag-1'))
137+
.mockResolvedValueOnce(preconditionFailedResponse())
138+
139+
const response = await POST(createMockRequest('POST', baseBody))
140+
141+
expect(response.status).toBe(409)
142+
const data = (await response.json()) as { error: string }
143+
expect(data.error).toMatch(/no other change was overwritten/)
144+
})
145+
})

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ import { generateRequestId } from '@/lib/core/utils/request'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
88
import { appendParagraphsToDocx, DOCX_MIME_TYPE } from '@/lib/microsoft-word/document.server'
99
import {
10+
assertContentUnchanged,
1011
downloadDocumentContent,
1112
fetchDocumentItem,
13+
getContentTag,
1214
toDocumentMetadata,
1315
uploadDocumentContent,
1416
} from '@/lib/microsoft-word/graph.server'
@@ -42,16 +44,19 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4244

4345
try {
4446
const basePath = getDocumentBasePath(documentId, driveId ?? undefined)
45-
await fetchDocumentItem(basePath, accessToken)
47+
const contentTag = getContentTag(await fetchDocumentItem(basePath, accessToken))
4648

4749
const existingBuffer = await downloadDocumentContent(basePath, accessToken)
4850
const updatedBuffer = await appendParagraphsToDocx(existingBuffer, content)
4951

52+
await assertContentUnchanged(basePath, accessToken, contentTag)
53+
5054
const item = await uploadDocumentContent(
5155
`${basePath}/content`,
5256
accessToken,
5357
updatedBuffer,
54-
DOCX_MIME_TYPE
58+
DOCX_MIME_TYPE,
59+
contentTag
5560
)
5661

5762
logger.info(`[${requestId}] Appended to Word document`, { documentId, size: item.size })

apps/sim/app/api/tools/microsoft_word/create-from-template/route.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from '@/lib/microsoft-word/graph.server'
1919
import { microsoftWordErrorResponse } from '@/app/api/tools/microsoft_word/utils'
2020
import {
21+
buildCreateUploadUrl,
2122
ensureDocxExtension,
2223
getDocumentBasePath,
2324
getDriveBasePath,
@@ -71,7 +72,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
7172
const parentPath = folderId?.trim()
7273
? getFolderBasePath(folderId, driveId ?? undefined)
7374
: `${getDriveBasePath(driveId ?? undefined)}/root`
74-
const uploadUrl = `${parentPath}:/${encodeURIComponent(fileName)}:/content`
75+
const uploadUrl = buildCreateUploadUrl(parentPath, fileName)
7576

7677
const item = await uploadDocumentContent(uploadUrl, accessToken, filled.buffer, DOCX_MIME_TYPE)
7778

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { buildDocxFromContent, DOCX_MIME_TYPE } from '@/lib/microsoft-word/docum
99
import { toDocumentMetadata, uploadDocumentContent } from '@/lib/microsoft-word/graph.server'
1010
import { microsoftWordErrorResponse } from '@/app/api/tools/microsoft_word/utils'
1111
import {
12+
buildCreateUploadUrl,
1213
ensureDocxExtension,
1314
getDriveBasePath,
1415
getFolderBasePath,
@@ -46,7 +47,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4647
const parentPath = folderId?.trim()
4748
? getFolderBasePath(folderId, driveId ?? undefined)
4849
: `${getDriveBasePath(driveId ?? undefined)}/root`
49-
const uploadUrl = `${parentPath}:/${encodeURIComponent(fileName)}:/content`
50+
const uploadUrl = buildCreateUploadUrl(parentPath, fileName)
5051

5152
const documentBuffer = await buildDocxFromContent(content ?? '', name)
5253
const item = await uploadDocumentContent(uploadUrl, accessToken, documentBuffer, DOCX_MIME_TYPE)

apps/sim/app/api/tools/microsoft_word/replace-text/route.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ import { generateRequestId } from '@/lib/core/utils/request'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
88
import { DOCX_MIME_TYPE, replaceTextInDocx } from '@/lib/microsoft-word/document.server'
99
import {
10+
assertContentUnchanged,
1011
downloadDocumentContent,
1112
fetchDocumentItem,
13+
getContentTag,
1214
toDocumentMetadata,
1315
uploadDocumentContent,
1416
} from '@/lib/microsoft-word/graph.server'
@@ -44,6 +46,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4446
try {
4547
const basePath = getDocumentBasePath(documentId, driveId ?? undefined)
4648
const existingItem = await fetchDocumentItem(basePath, accessToken)
49+
const contentTag = getContentTag(existingItem)
4750

4851
const existingBuffer = await downloadDocumentContent(basePath, accessToken)
4952
const { buffer, occurrencesChanged } = await replaceTextInDocx(
@@ -63,11 +66,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
6366
})
6467
}
6568

69+
await assertContentUnchanged(basePath, accessToken, contentTag)
70+
6671
const item = await uploadDocumentContent(
6772
`${basePath}/content`,
6873
accessToken,
6974
buffer,
70-
DOCX_MIME_TYPE
75+
DOCX_MIME_TYPE,
76+
contentTag
7177
)
7278

7379
logger.info(`[${requestId}] Replaced text in Word document`, {

apps/sim/lib/microsoft-word/graph.server.ts

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,28 @@ interface GraphDriveItem {
1414
webUrl?: string
1515
createdDateTime?: string
1616
lastModifiedDateTime?: string
17+
/** An eTag for the item's content, unchanged when only metadata changes. */
18+
cTag?: string
19+
/** An eTag for the whole item, metadata included. */
20+
eTag?: string
1721
file?: { mimeType?: string }
1822
folder?: Record<string, unknown>
1923
}
2024

25+
/**
26+
* The token that identifies the exact content an edit was based on.
27+
*
28+
* `cTag` is the right one: Graph documents it as "an eTag for the content of the
29+
* item" that does not move when only metadata changes, so a rename will not
30+
* spuriously abort an edit. `eTag` is the fallback for the shapes where Graph
31+
* omits `cTag`.
32+
*
33+
* @see https://learn.microsoft.com/en-us/graph/api/resources/driveitem
34+
*/
35+
export function getContentTag(item: GraphDriveItem): string | undefined {
36+
return item.cTag ?? item.eTag
37+
}
38+
2139
/** Thrown when Microsoft Graph rejects a request, carrying its HTTP status. */
2240
export class GraphRequestError extends Error {
2341
constructor(
@@ -143,27 +161,72 @@ export async function downloadConvertedContent(
143161
return Buffer.from(await response.arrayBuffer())
144162
}
145163

164+
/** Message shown when someone else changed the document mid-edit. */
165+
const CONFLICT_MESSAGE =
166+
'The document changed in OneDrive or SharePoint after Sim read it, so the edit was not applied and no other change was overwritten. Run the operation again to edit the current version.'
167+
168+
/** Raised instead of overwriting a document that changed since it was read. */
169+
export function documentChangedError(): GraphRequestError {
170+
return new GraphRequestError(CONFLICT_MESSAGE, 409)
171+
}
172+
173+
/**
174+
* Aborts a read-modify-write when the document's content changed since it was
175+
* read. Without this, two overlapping edits both succeed and the later upload
176+
* silently discards the earlier one.
177+
*
178+
* `expected` being undefined means Graph returned neither tag for this item, so
179+
* there is nothing to compare and the caller proceeds unguarded — the
180+
* `if-match` header on the upload is the remaining line of defense.
181+
*/
182+
export async function assertContentUnchanged(
183+
basePath: string,
184+
accessToken: string,
185+
expected: string | undefined
186+
): Promise<void> {
187+
if (!expected) return
188+
189+
const current = getContentTag(await fetchDocumentItem(basePath, accessToken))
190+
if (current && current !== expected) {
191+
throw documentChangedError()
192+
}
193+
}
194+
146195
/**
147196
* Uploads bytes as a drive item's content and returns the resulting item.
148197
*
198+
* `ifMatch` carries the content tag the upload is based on. Graph documents
199+
* `if-match` (and its `412 Precondition Failed` response) on the metadata
200+
* update, not on this content endpoint, so it is sent as a second line of
201+
* defense rather than the guarantee: an `If-Match` a server does not implement
202+
* is ignored, and the caller's {@link assertContentUnchanged} check is what
203+
* actually decides whether the write is safe. A `412` is surfaced as the same
204+
* conflict either way.
205+
*
149206
* @see https://learn.microsoft.com/en-us/graph/api/driveitem-put-content
207+
* @see https://learn.microsoft.com/en-us/graph/api/driveitem-update
150208
*/
151209
export async function uploadDocumentContent(
152210
url: string,
153211
accessToken: string,
154212
content: Buffer,
155-
mimeType: string
213+
mimeType: string,
214+
ifMatch?: string
156215
): Promise<GraphDriveItem> {
157216
const response = await graphFetch(url, 'documentUploadUrl', {
158217
method: 'PUT',
159218
headers: {
160219
Authorization: `Bearer ${accessToken}`,
161220
'Content-Type': mimeType,
162221
'Content-Length': String(content.length),
222+
...(ifMatch ? { 'if-match': ifMatch } : {}),
163223
},
164224
body: content,
165225
})
166226

227+
if (response.status === 412) {
228+
throw documentChangedError()
229+
}
167230
if (!response.ok) await raiseGraphError(response)
168231

169232
return (await response.json()) as GraphDriveItem

apps/sim/tools/generated/tool-metadata.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

apps/sim/tools/microsoft_word/append.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export const appendTool: ToolConfig<MicrosoftWordToolParams, MicrosoftWordUpdate
99
id: 'microsoft_word_append',
1010
name: 'Append to Microsoft Word Document',
1111
description:
12-
'Append plain-text paragraphs to the end of an existing Microsoft Word (.docx) document, leaving the existing content and formatting intact.',
12+
'Append plain-text paragraphs to the end of an existing Microsoft Word (.docx) document, leaving the existing content and formatting intact. Fails rather than overwriting if someone else changed the document while the edit was in flight.',
1313
version: '1.0',
1414
errorExtractor: ErrorExtractorId.MICROSOFT_GRAPH_ERRORS,
1515

0 commit comments

Comments
 (0)