fix(microsoft-word): make conditional writes service-enforced and return 400 for bad input (#7094)

* fix(microsoft-word): make conditional writes service-enforced and return 400 for bad input

* fix(microsoft-word): split large document uploads into sequential fragments

Author: waleedlatif1

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

@@ -60,6 +60,21 @@ async function docxResponse() {
   }
 }
 
+/** The `createUploadSession` response carrying the preauthenticated upload URL. */
+function uploadSessionResponse(uploadUrl = 'https://sn3302.up.1drv.com/up/session-abc') {
+  const body = { uploadUrl, expirationDateTime: '2026-01-01T00:00:00Z' }
+  return {
+    ok: true,
+    status: 200,
+    statusText: '',
+    headers: new Headers(),
+    body: null,
+    text: async () => JSON.stringify(body),
+    json: async () => body,
+    arrayBuffer: async () => new ArrayBuffer(0),
+  }
+}
+
 function preconditionFailedResponse() {
   return {
     ok: false,
@@ -88,11 +103,11 @@ beforeEach(() => {
 })
 
 describe('POST /api/tools/microsoft_word/append', () => {
-  it('uploads the edit when the document has not changed, guarded by its content tag', async () => {
+  it('writes through a conditional upload session carrying the content tag', async () => {
     mockSecureFetchWithPinnedIP
       .mockResolvedValueOnce(itemResponse('tag-1'))
       .mockResolvedValueOnce(await docxResponse())
-      .mockResolvedValueOnce(itemResponse('tag-1'))
+      .mockResolvedValueOnce(uploadSessionResponse())
       .mockResolvedValueOnce(itemResponse('tag-2'))
 
     const response = await POST(createMockRequest('POST', baseBody))
@@ -105,16 +120,26 @@ describe('POST /api/tools/microsoft_word/append', () => {
     expect(data.success).toBe(true)
     expect(data.output.updatedContent).toBe(true)
 
+    // The precondition rides on the session creation, which Graph documents as
+    // returning 412 on a mismatch.
+    const sessionCall = mockSecureFetchWithPinnedIP.mock.calls.find((call) =>
+      String(call[0]).endsWith('/createUploadSession')
+    )
+    expect(sessionCall?.[2]).toMatchObject({ method: 'POST' })
+    expect(sessionCall?.[2].headers).toMatchObject({ 'if-match': 'tag-1' })
+
+    // Bytes go to the preauthenticated URL, and must not carry the bearer token.
     const uploadCall = mockSecureFetchWithPinnedIP.mock.calls.at(-1)
+    expect(uploadCall?.[0]).toBe('https://sn3302.up.1drv.com/up/session-abc')
     expect(uploadCall?.[2]).toMatchObject({ method: 'PUT' })
-    expect(uploadCall?.[2].headers).toMatchObject({ 'if-match': 'tag-1' })
+    expect(uploadCall?.[2].headers.Authorization).toBeUndefined()
   })
 
-  it('refuses to overwrite a document that changed while the edit was in flight', async () => {
+  it('refuses to overwrite when the service rejects the precondition', async () => {
     mockSecureFetchWithPinnedIP
       .mockResolvedValueOnce(itemResponse('tag-1'))
       .mockResolvedValueOnce(await docxResponse())
-      .mockResolvedValueOnce(itemResponse('tag-2'))
+      .mockResolvedValueOnce(preconditionFailedResponse())
 
     const response = await POST(createMockRequest('POST', baseBody))
 
@@ -123,12 +148,21 @@ describe('POST /api/tools/microsoft_word/append', () => {
     expect(data.success).toBe(false)
     expect(data.error).toMatch(/no other change was overwritten/)
 
-    // The conflict is detected before the PUT, so nothing was written.
+    // The session was refused, so no bytes were ever sent.
     expect(mockSecureFetchWithPinnedIP.mock.calls.some((call) => call[2]?.method === 'PUT')).toBe(
       false
     )
   })
 
+  it('maps a malformed document ID to a client error, not a server error', async () => {
+    const response = await POST(createMockRequest('POST', { ...baseBody, documentId: 'bad/../id' }))
+
+    expect(response.status).toBe(400)
+    const data = (await response.json()) as { success: boolean; error: string }
+    expect(data.success).toBe(false)
+    expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
   it('refuses to write Word bytes over a drive item that is not a .docx', async () => {
     const pdf = {
       id: 'doc-abc',
@@ -201,41 +235,11 @@ describe('POST /api/tools/microsoft_word/append', () => {
     )
   })
 
-  it('refuses to write when the re-read reports no version', async () => {
-    const untagged = {
-      id: 'doc-abc',
-      name: 'notes.docx',
-      file: {
-        mimeType: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
-      },
-    }
+  it('surfaces a conflict raised when the bytes commit', async () => {
     mockSecureFetchWithPinnedIP
       .mockResolvedValueOnce(itemResponse('tag-1'))
       .mockResolvedValueOnce(await docxResponse())
-      .mockResolvedValueOnce({
-        ok: true,
-        status: 200,
-        statusText: '',
-        headers: new Headers(),
-        body: null,
-        text: async () => JSON.stringify(untagged),
-        json: async () => untagged,
-        arrayBuffer: async () => new ArrayBuffer(0),
-      })
-
-    const response = await POST(createMockRequest('POST', baseBody))
-
-    expect(response.status).toBe(409)
-    expect(mockSecureFetchWithPinnedIP.mock.calls.some((call) => call[2]?.method === 'PUT')).toBe(
-      false
-    )
-  })
-
-  it('surfaces a 412 from the upload precondition as the same conflict', async () => {
-    mockSecureFetchWithPinnedIP
-      .mockResolvedValueOnce(itemResponse('tag-1'))
-      .mockResolvedValueOnce(await docxResponse())
-      .mockResolvedValueOnce(itemResponse('tag-1'))
+      .mockResolvedValueOnce(uploadSessionResponse())
       .mockResolvedValueOnce(preconditionFailedResponse())
 
     const response = await POST(createMockRequest('POST', baseBody))

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

@@ -5,14 +5,13 @@ import { parseRequest } from '@/lib/api/server'
 import { checkInternalAuth } from '@/lib/auth/hybrid'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
-import { appendParagraphsToDocx, DOCX_MIME_TYPE } from '@/lib/microsoft-word/document.server'
+import { appendParagraphsToDocx } from '@/lib/microsoft-word/document.server'
 import {
-  assertContentUnchanged,
   downloadDocumentContent,
   fetchDocumentItem,
+  replaceContentIfUnchanged,
   requireContentTag,
   toDocumentMetadata,
-  uploadDocumentContent,
 } from '@/lib/microsoft-word/graph.server'
 import { microsoftWordErrorResponse } from '@/app/api/tools/microsoft_word/utils'
 import { getDocumentBasePath } from '@/tools/microsoft_word/utils'
@@ -61,15 +60,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       })
     }
 
-    await assertContentUnchanged(basePath, accessToken, contentTag)
-
-    const item = await uploadDocumentContent(
-      `${basePath}/content`,
-      accessToken,
-      buffer,
-      DOCX_MIME_TYPE,
-      contentTag
-    )
+    const item = await replaceContentIfUnchanged(basePath, accessToken, buffer, contentTag)
 
     logger.info(`[${requestId}] Appended to Word document`, {
       documentId,

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

@@ -0,0 +1,68 @@
+/**
+ * @vitest-environment node
+ */
+import {
+  createMockRequest,
+  hybridAuthMockFns,
+  inputValidationMock,
+  inputValidationMockFns,
+} from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
+
+import { POST } from '@/app/api/tools/microsoft_word/create-from-template/route'
+
+const { mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP } = inputValidationMockFns
+
+const baseBody = {
+  accessToken: 'token-123',
+  templateDocumentId: 'template-abc',
+  name: 'Acme Agreement',
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
+    success: true,
+    userId: 'user-1',
+    authType: 'internal_jwt',
+  })
+  mockValidateUrlWithDNS.mockResolvedValue({
+    isValid: true,
+    resolvedIP: '93.184.216.34',
+    originalHostname: 'graph.microsoft.com',
+  })
+})
+
+describe('POST /api/tools/microsoft_word/create-from-template', () => {
+  it('rejects an empty placeholder key rather than failing mid-rewrite', async () => {
+    // A blank placeholder would match at every position in the document.
+    const response = await POST(
+      createMockRequest('POST', { ...baseBody, replacements: { '': 'Acme Corp' } })
+    )
+
+    expect(response.status).toBe(400)
+    const data = (await response.json()) as { error?: string }
+    expect(JSON.stringify(data)).toMatch(/placeholder/i)
+    expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('rejects an empty placeholder key inside the JSON-string form too', async () => {
+    const response = await POST(
+      createMockRequest('POST', { ...baseBody, replacements: '{"  ": "Acme Corp"}' })
+    )
+
+    expect(response.status).toBe(400)
+    expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('rejects a replacements value that is not an object mapping', async () => {
+    const response = await POST(
+      createMockRequest('POST', { ...baseBody, replacements: '["a","b"]' })
+    )
+
+    expect(response.status).toBe(400)
+    expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+})

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

@@ -0,0 +1,68 @@
+/**
+ * @vitest-environment node
+ */
+import {
+  createMockRequest,
+  hybridAuthMockFns,
+  inputValidationMock,
+  inputValidationMockFns,
+} from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
+
+import { POST } from '@/app/api/tools/microsoft_word/create/route'
+
+const { mockValidateUrlWithDNS, mockSecureFetchWithPinnedIP } = inputValidationMockFns
+
+const baseBody = {
+  accessToken: 'token-123',
+  name: 'Q3 Report',
+  content: 'Hello',
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
+    success: true,
+    userId: 'user-1',
+    authType: 'internal_jwt',
+  })
+  mockValidateUrlWithDNS.mockResolvedValue({
+    isValid: true,
+    resolvedIP: '93.184.216.34',
+    originalHostname: 'graph.microsoft.com',
+  })
+})
+
+describe('POST /api/tools/microsoft_word/create', () => {
+  it('rejects a whitespace-only name as a client error, not a server error', async () => {
+    // The contract's min(1) accepts a space; the name is only known to be
+    // unusable once the extension helper trims it.
+    const response = await POST(createMockRequest('POST', { ...baseBody, name: '   ' }))
+
+    expect(response.status).toBe(400)
+    const data = (await response.json()) as { success: boolean; error: string }
+    expect(data.success).toBe(false)
+    expect(data.error).toMatch(/Document name is required/)
+    expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('rejects a malformed folder ID as a client error', async () => {
+    const response = await POST(
+      createMockRequest('POST', { ...baseBody, folderId: 'not a valid id' })
+    )
+
+    expect(response.status).toBe(400)
+    const data = (await response.json()) as { success: boolean }
+    expect(data.success).toBe(false)
+    expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+
+  it('rejects a malformed drive ID as a client error', async () => {
+    const response = await POST(createMockRequest('POST', { ...baseBody, driveId: 'bad/../drive' }))
+
+    expect(response.status).toBe(400)
+    expect(mockSecureFetchWithPinnedIP).not.toHaveBeenCalled()
+  })
+})

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

@@ -5,14 +5,13 @@ import { parseRequest } from '@/lib/api/server'
 import { checkInternalAuth } from '@/lib/auth/hybrid'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
-import { DOCX_MIME_TYPE, replaceTextInDocx } from '@/lib/microsoft-word/document.server'
+import { replaceTextInDocx } from '@/lib/microsoft-word/document.server'
 import {
-  assertContentUnchanged,
   downloadDocumentContent,
   fetchDocumentItem,
+  replaceContentIfUnchanged,
   requireContentTag,
   toDocumentMetadata,
-  uploadDocumentContent,
 } from '@/lib/microsoft-word/graph.server'
 import { microsoftWordErrorResponse } from '@/app/api/tools/microsoft_word/utils'
 import { getDocumentBasePath } from '@/tools/microsoft_word/utils'
@@ -66,15 +65,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       })
     }
 
-    await assertContentUnchanged(basePath, accessToken, contentTag)
-
-    const item = await uploadDocumentContent(
-      `${basePath}/content`,
-      accessToken,
-      buffer,
-      DOCX_MIME_TYPE,
-      contentTag
-    )
+    const item = await replaceContentIfUnchanged(basePath, accessToken, buffer, contentTag)
 
     logger.info(`[${requestId}] Replaced text in Word document`, {
       documentId,

apps/sim/app/api/tools/microsoft_word/utils.ts

@@ -3,6 +3,7 @@ import { getErrorMessage } from '@sim/utils/errors'
 import { NextResponse } from 'next/server'
 import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
 import { GraphRequestError } from '@/lib/microsoft-word/graph.server'
+import { MicrosoftWordInputError } from '@/tools/microsoft_word/errors'
 
 /**
  * Projects an error raised while talking to Microsoft Graph — or while building
@@ -20,7 +21,11 @@ export function microsoftWordErrorResponse(
   logger.error(`[${requestId}] Microsoft Word ${operation} failed`, { error: message })
 
   const status =
-    error instanceof GraphRequestError ? error.status : isPayloadSizeLimitError(error) ? 413 : 500
+    error instanceof GraphRequestError || error instanceof MicrosoftWordInputError
+      ? error.status
+      : isPayloadSizeLimitError(error)
+        ? 413
+        : 500
 
   return NextResponse.json({ success: false, error: message }, { status })
 }

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

@@ -119,11 +119,18 @@ const MAX_DOCUMENT_CONTENT_LENGTH = 2_000_000
 const MAX_REPLACEMENTS_LENGTH = 200_000
 
 /** Whether a string parses as a JSON object (not an array or scalar). */
-function isJsonObjectString(value: string): boolean {
+function isPlaceholderMap(value: unknown): boolean {
+  if (typeof value !== 'object' || value === null || Array.isArray(value)) return false
+  // A blank placeholder would match at every position in the document, so it is
+  // rejected here rather than failing mid-rewrite with an unattributable 500.
+  return Object.keys(value).every((key) => key.trim().length > 0)
+}
+
+/** Whether a string parses as a JSON object whose keys are all usable placeholders. */
+function isPlaceholderMapString(value: string): boolean {
   if (!value.trim()) return true
   try {
-    const parsed: unknown = JSON.parse(value)
-    return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)
+    return isPlaceholderMap(JSON.parse(value))
   } catch {
     return false
   }
@@ -140,10 +147,12 @@ const wordReplacementsSchema = z
     z
       .string()
       .refine(
-        isJsonObjectString,
-        'Placeholder values must be a JSON object mapping each placeholder to its value'
+        isPlaceholderMapString,
+        'Placeholder values must be a JSON object mapping each non-empty placeholder to its value'
       ),
-    z.record(z.string(), z.union([z.string(), z.number(), z.boolean(), z.null()])),
+    z
+      .record(z.string(), z.union([z.string(), z.number(), z.boolean(), z.null()]))
+      .refine(isPlaceholderMap, 'Every placeholder must be a non-empty string'),
   ])
   .refine(
     (value) =>