Skip to content

Commit 6162253

Browse files
committed
fix(microsoft-excel): address PR review comments
- Validate siteId/driveId format in drives route to prevent path traversal - Use direct single-drive endpoint for fetchById instead of filtering full list - Fix dependsOn on sheet/spreadsheet selectors so driveId flows into context - Fix NextRequest type in drives route for build compatibility
1 parent 9669ca4 commit 6162253

3 files changed

Lines changed: 60 additions & 18 deletions

File tree

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

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import { NextResponse } from 'next/server'
2+
import { type NextRequest, NextResponse } from 'next/server'
33
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
44
import { generateRequestId } from '@/lib/core/utils/request'
55
import { refreshAccessTokenIfNeeded } from '@/app/api/auth/oauth/utils'
@@ -20,12 +20,12 @@ interface GraphDrive {
2020
* Used by the microsoft.excel.drives selector to let users pick
2121
* which drive contains their Excel file.
2222
*/
23-
export async function POST(request: Request) {
23+
export async function POST(request: NextRequest) {
2424
const requestId = generateRequestId()
2525

2626
try {
2727
const body = await request.json()
28-
const { credential, workflowId, siteId } = body
28+
const { credential, workflowId, siteId, driveId } = body
2929

3030
if (!credential) {
3131
logger.warn(`[${requestId}] Missing credential in request`)
@@ -37,7 +37,12 @@ export async function POST(request: Request) {
3737
return NextResponse.json({ error: 'Site ID is required' }, { status: 400 })
3838
}
3939

40-
const authz = await authorizeCredentialUse(request as Request, {
40+
if (!/^[\w.,;:-]+$/.test(siteId)) {
41+
logger.warn(`[${requestId}] Invalid siteId format`)
42+
return NextResponse.json({ error: 'Invalid site ID format' }, { status: 400 })
43+
}
44+
45+
const authz = await authorizeCredentialUse(request, {
4146
credentialId: credential,
4247
workflowId,
4348
})
@@ -58,6 +63,35 @@ export async function POST(request: Request) {
5863
)
5964
}
6065

66+
// Single-drive lookup when driveId is provided (used by fetchById)
67+
if (driveId) {
68+
if (!/^[\w-]+$/.test(driveId)) {
69+
return NextResponse.json({ error: 'Invalid drive ID format' }, { status: 400 })
70+
}
71+
72+
const url = `https://graph.microsoft.com/v1.0/sites/${siteId}/drives/${driveId}?$select=id,name,driveType,webUrl`
73+
const response = await fetch(url, {
74+
headers: { Authorization: `Bearer ${accessToken}` },
75+
})
76+
77+
if (!response.ok) {
78+
const errorData = await response
79+
.json()
80+
.catch(() => ({ error: { message: 'Unknown error' } }))
81+
return NextResponse.json(
82+
{ error: errorData.error?.message || 'Failed to fetch drive' },
83+
{ status: response.status }
84+
)
85+
}
86+
87+
const data: GraphDrive = await response.json()
88+
return NextResponse.json(
89+
{ drive: { id: data.id, name: data.name, driveType: data.driveType } },
90+
{ status: 200 }
91+
)
92+
}
93+
94+
// List all drives for the site
6195
const url = `https://graph.microsoft.com/v1.0/sites/${siteId}/drives?$select=id,name,driveType,webUrl`
6296

6397
const response = await fetch(url, {

apps/sim/blocks/blocks/microsoft_excel.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
431431
requiredScopes: [],
432432
mimeType: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
433433
placeholder: 'Select a spreadsheet',
434-
dependsOn: ['credential'],
434+
dependsOn: { all: ['credential'], any: ['driveSelector'] },
435435
mode: 'basic',
436436
},
437437
// Manual Spreadsheet ID (advanced mode)
@@ -441,7 +441,7 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
441441
type: 'short-input',
442442
canonicalParamId: 'spreadsheetId',
443443
placeholder: 'Enter spreadsheet ID',
444-
dependsOn: ['credential'],
444+
dependsOn: { all: ['credential'], any: ['manualDriveId'] },
445445
mode: 'advanced',
446446
},
447447
// Drive ID for SharePoint (advanced mode)
@@ -464,7 +464,10 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
464464
selectorAllowSearch: false,
465465
placeholder: 'Select a sheet',
466466
required: true,
467-
dependsOn: { all: ['credential'], any: ['spreadsheetId', 'manualSpreadsheetId'] },
467+
dependsOn: {
468+
all: ['credential'],
469+
any: ['spreadsheetId', 'manualSpreadsheetId', 'driveSelector'],
470+
},
468471
mode: 'basic',
469472
},
470473
// Manual Sheet Name (advanced mode)
@@ -475,7 +478,10 @@ export const MicrosoftExcelV2Block: BlockConfig<MicrosoftExcelV2Response> = {
475478
canonicalParamId: 'sheetName',
476479
placeholder: 'Name of the sheet/tab (e.g., Sheet1)',
477480
required: true,
478-
dependsOn: ['credential'],
481+
dependsOn: {
482+
all: ['credential'],
483+
any: ['manualDriveId'],
484+
},
479485
mode: 'advanced',
480486
},
481487
// Cell Range (optional for read/write)

apps/sim/hooks/selectors/registry.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,18 +1561,20 @@ const registry: Record<SelectorKey, SelectorDefinition> = {
15611561
fetchById: async ({ context, detailId }: SelectorQueryArgs) => {
15621562
if (!detailId || !context.siteId) return null
15631563
const credentialId = ensureCredential(context, 'microsoft.excel.drives')
1564-
const body = JSON.stringify({
1565-
credential: credentialId,
1566-
workflowId: context.workflowId,
1567-
siteId: context.siteId,
1568-
})
1569-
const data = await fetchJson<{ drives: { id: string; name: string }[] }>(
1564+
const data = await fetchJson<{ drive: { id: string; name: string } }>(
15701565
'/api/tools/microsoft_excel/drives',
1571-
{ method: 'POST', body }
1566+
{
1567+
method: 'POST',
1568+
body: JSON.stringify({
1569+
credential: credentialId,
1570+
workflowId: context.workflowId,
1571+
siteId: context.siteId,
1572+
driveId: detailId,
1573+
}),
1574+
}
15721575
)
1573-
const drive = (data.drives || []).find((d) => d.id === detailId) ?? null
1574-
if (!drive) return null
1575-
return { id: drive.id, label: drive.name }
1576+
if (!data.drive) return null
1577+
return { id: data.drive.id, label: data.drive.name }
15761578
},
15771579
},
15781580
'microsoft.excel': {

0 commit comments

Comments
 (0)