Skip to content

Commit 326114d

Browse files
waleedlatif1claude
andcommitted
fix(microsoft-excel): address PR review feedback for SharePoint drive support
- Clear stale driveId/siteId/spreadsheetId when fileSource changes by adding fileSource to dependsOn arrays for siteSelector, driveSelector, and spreadsheetId selectors - Reorder manualDriveId before manualSpreadsheetId in advanced mode for logical top-down flow - Validate spreadsheetId with validateMicrosoftGraphId in getItemBasePath() and sheets route to close injection vector (uses permissive validator that accepts ! chars in OneDrive item IDs) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8b1c88c commit 326114d

File tree

3 files changed

+34
-18
lines changed

3 files changed

+34
-18
lines changed

apps/sim/app/api/tools/microsoft_excel/sheets/route.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
33
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
4-
import { validateAlphanumericId } from '@/lib/core/security/input-validation'
4+
import {
5+
validateAlphanumericId,
6+
validateMicrosoftGraphId,
7+
} from '@/lib/core/security/input-validation'
58
import { generateRequestId } from '@/lib/core/utils/request'
69
import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
710

@@ -63,6 +66,11 @@ export async function GET(request: NextRequest) {
6366
`[${requestId}] Fetching worksheets from Microsoft Graph API for workbook ${spreadsheetId}`
6467
)
6568

69+
const spreadsheetValidation = validateMicrosoftGraphId(spreadsheetId, 'spreadsheetId')
70+
if (!spreadsheetValidation.isValid) {
71+
return NextResponse.json({ error: spreadsheetValidation.error }, { status: 400 })
72+
}
73+
6674
if (driveId) {
6775
const driveIdValidation = validateAlphanumericId(driveId, 'driveId')
6876
if (!driveIdValidation.isValid) {

apps/sim/blocks/blocks/microsoft_excel.ts

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
415415
selectorKey: 'sharepoint.sites',
416416
requiredScopes: [],
417417
placeholder: 'Select a SharePoint site',
418-
dependsOn: ['credential'],
418+
dependsOn: ['credential', 'fileSource'],
419419
condition: { field: 'fileSource', value: 'sharepoint' },
420420
required: { field: 'fileSource', value: 'sharepoint' },
421421
mode: 'basic',
@@ -430,7 +430,7 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
430430
selectorKey: 'microsoft.excel.drives',
431431
selectorAllowSearch: false,
432432
placeholder: 'Select a document library',
433-
dependsOn: ['credential', 'siteSelector'],
433+
dependsOn: ['credential', 'siteSelector', 'fileSource'],
434434
condition: { field: 'fileSource', value: 'sharepoint' },
435435
required: { field: 'fileSource', value: 'sharepoint' },
436436
mode: 'basic',
@@ -446,19 +446,9 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
446446
requiredScopes: [],
447447
mimeType: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
448448
placeholder: 'Select a spreadsheet',
449-
dependsOn: { all: ['credential'], any: ['credential', 'driveSelector'] },
449+
dependsOn: { all: ['credential', 'fileSource'], any: ['credential', 'driveSelector'] },
450450
mode: 'basic',
451451
},
452-
// Manual Spreadsheet ID (advanced mode)
453-
{
454-
id: 'manualSpreadsheetId',
455-
title: 'Spreadsheet ID',
456-
type: 'short-input',
457-
canonicalParamId: 'spreadsheetId',
458-
placeholder: 'Enter spreadsheet ID',
459-
dependsOn: { all: ['credential'], any: ['credential', 'manualDriveId'] },
460-
mode: 'advanced',
461-
},
462452
// Drive ID for SharePoint (advanced mode, only when SharePoint is selected)
463453
{
464454
id: 'manualDriveId',
@@ -469,6 +459,16 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
469459
condition: { field: 'fileSource', value: 'sharepoint' },
470460
mode: 'advanced',
471461
},
462+
// Manual Spreadsheet ID (advanced mode)
463+
{
464+
id: 'manualSpreadsheetId',
465+
title: 'Spreadsheet ID',
466+
type: 'short-input',
467+
canonicalParamId: 'spreadsheetId',
468+
placeholder: 'Enter spreadsheet ID',
469+
dependsOn: { all: ['credential'], any: ['credential', 'manualDriveId'] },
470+
mode: 'advanced',
471+
},
472472
// Sheet Name Selector (basic mode)
473473
{
474474
id: 'sheetName',

apps/sim/tools/microsoft_excel/utils.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { createLogger } from '@sim/logger'
2-
import { validateAlphanumericId } from '@/lib/core/security/input-validation'
2+
import {
3+
validateAlphanumericId,
4+
validateMicrosoftGraphId,
5+
} from '@/lib/core/security/input-validation'
36
import type { ExcelCellValue } from '@/tools/microsoft_excel/types'
47

58
const logger = createLogger('MicrosoftExcelUtils')
@@ -10,10 +13,15 @@ const logger = createLogger('MicrosoftExcelUtils')
1013
* When driveId is omitted, uses /me/drive/items/{itemId} (personal OneDrive).
1114
*/
1215
export function getItemBasePath(spreadsheetId: string, driveId?: string): string {
16+
const spreadsheetValidation = validateMicrosoftGraphId(spreadsheetId, 'spreadsheetId')
17+
if (!spreadsheetValidation.isValid) {
18+
throw new Error(spreadsheetValidation.error)
19+
}
20+
1321
if (driveId) {
14-
const validation = validateAlphanumericId(driveId, 'driveId')
15-
if (!validation.isValid) {
16-
throw new Error(validation.error)
22+
const driveValidation = validateAlphanumericId(driveId, 'driveId')
23+
if (!driveValidation.isValid) {
24+
throw new Error(driveValidation.error)
1725
}
1826
return `https://graph.microsoft.com/v1.0/drives/${driveId}/items/${spreadsheetId}`
1927
}

0 commit comments

Comments
 (0)