feat(sharepoint): validate against Graph API and add delete/update/download tools#5369
Conversation
…wnload tools - Fix bugs found validating existing tools against live Graph docs: malformed nested $expand syntax in get_list, reversed site-resolution precedence in create_list, unsanitized field fallback in add_list_items, missing URL encoding in read_page, and an incorrect root/serverRelativeUrl field shape - Fix V1 block gaps vs V2: upload_file required flag, missing required on pageName/listDisplayName, wrong site-picker mimeType, dead subBlock refs - Add 8 new tools within already-granted scopes for CRUD completion: delete_list_item, get_list_item, delete_page, update_page, publish_page, download_file, delete_file, get_drive_item - Add opt-in stripAuthOnRedirect option to secureFetchWithPinnedIP so the SharePoint file-download route doesn't resend the bearer token to the preauthenticated redirect target (default off, no behavior change elsewhere) - Dedupe read-only list-item field sanitization into a shared utils helper
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Adds Fixes Graph integration bugs: Reviewed by Cursor Bugbot for commit 5a076d5. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…dence - URL-encode siteId/groupId everywhere it's interpolated into a Graph request path (Graph site IDs like "host,guid,guid" contain commas that must be encoded) - addresses Cursor Bugbot findings on update_page, delete_page, publish_page, and applies the same fix consistently across every other sharepoint tool for consistency - Fix create_page's site-resolution precedence (was siteSelector before siteId, inconsistent with every sibling tool)
Greptile SummaryThis PR adds 8 new SharePoint tools (update/publish/delete page, get/delete list item, download/delete file, get drive item), fixes real Graph API bugs in existing tools (site-ID precedence, URL encoding,
Confidence Score: 4/5Safe to merge after fixing the OData nested-expand syntax in get_list.ts; all other changes are well-structured and consistent with existing patterns. The change in get_list.ts from items($expand=fields) to items(expand=fields) removes the required $ prefix from a nested OData system query option. Microsoft Graph silently ignores the malformed inner option, so any caller that sets both includeColumns and includeItems will receive list items with no field values — a silent data-loss regression on a code path that the PR explicitly claims to validate. apps/sim/tools/sharepoint/get_list.ts — the nested $expand syntax needs to be restored to items($expand=fields). Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as Block UI
participant Tool as SharePoint Tool
participant Route as /api/tools/sharepoint/*
participant Graph as Microsoft Graph API
participant CDN as Pre-auth CDN
Note over UI,Graph: New: Download File flow
UI->>Tool: download_file(driveId, driveItemId)
Tool->>Route: "POST /download-file {accessToken, driveId, itemId}"
Route->>Graph: "GET /drives/{id}/items/{id} (metadata)"
Graph-->>Route: "{ name, file: { mimeType } }"
Route->>Graph: "GET /drives/{id}/items/{id}/content"
Graph-->>Route: 302 redirect to CDN URL
Note over Route: stripAuthOnRedirect strips Bearer token
Route->>CDN: GET pre-auth URL (no Authorization)
CDN-->>Route: Binary content
Route-->>Tool: "{ file: { name, mimeType, data (base64), size } }"
Note over UI,Graph: New: Page mutation flow
UI->>Tool: update_page / publish_page / delete_page
Tool->>Graph: "PATCH/POST/DELETE /sites/{siteId}/pages/{pageId}/microsoft.graph.sitePage"
Graph-->>Tool: 200 or 204
Tool-->>UI: "{ success, page/published/deleted }"
Note over UI,Graph: New: List item lookup/delete
UI->>Tool: get_list_item / delete_list_item
Tool->>Graph: "GET or DELETE /sites/{id}/lists/{id}/items/{id}"
Graph-->>Tool: "{ id, fields } or 204"
Tool-->>UI: "{ item } or { deleted, itemId }"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as Block UI
participant Tool as SharePoint Tool
participant Route as /api/tools/sharepoint/*
participant Graph as Microsoft Graph API
participant CDN as Pre-auth CDN
Note over UI,Graph: New: Download File flow
UI->>Tool: download_file(driveId, driveItemId)
Tool->>Route: "POST /download-file {accessToken, driveId, itemId}"
Route->>Graph: "GET /drives/{id}/items/{id} (metadata)"
Graph-->>Route: "{ name, file: { mimeType } }"
Route->>Graph: "GET /drives/{id}/items/{id}/content"
Graph-->>Route: 302 redirect to CDN URL
Note over Route: stripAuthOnRedirect strips Bearer token
Route->>CDN: GET pre-auth URL (no Authorization)
CDN-->>Route: Binary content
Route-->>Tool: "{ file: { name, mimeType, data (base64), size } }"
Note over UI,Graph: New: Page mutation flow
UI->>Tool: update_page / publish_page / delete_page
Tool->>Graph: "PATCH/POST/DELETE /sites/{siteId}/pages/{pageId}/microsoft.graph.sitePage"
Graph-->>Tool: 200 or 204
Tool-->>UI: "{ success, page/published/deleted }"
Note over UI,Graph: New: List item lookup/delete
UI->>Tool: get_list_item / delete_list_item
Tool->>Graph: "GET or DELETE /sites/{id}/lists/{id}/items/{id}"
Graph-->>Tool: "{ id, fields } or 204"
Tool-->>UI: "{ item } or { deleted, itemId }"
Reviews (6): Last reviewed commit: "fix(sharepoint): close V1 block input/ou..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…hrowing - create_page/update_page only escaped quotes when building innerHtml; add a shared escapeHtml() helper that also escapes &, <, > so page content with angle brackets/ampersands doesn't corrupt the canvas layout - add_list_items' fields fallback (sanitized re-derivation when Graph's response omits fields) could throw on a malformed fallback input after the item was already created successfully; catch and fall back to undefined instead of failing an already-successful response
|
@greptile review |
|
@cursor review |
…te_list Both V1 and V2 tools.config.params mapped columnDefinitions onto pageContent whenever columnDefinitions was truthy, with no operation check. Stale list-column JSON left in block state after switching to create_page/ update_page would silently replace the user's intended page text. Gate the mapping on operation === create_list (V1) / sharepoint_create_list (V2).
|
@greptile review |
Greptile flagged the content fetch as unbounded - a large file would buffer entirely in memory before base64-encoding into the JSON response. Pass maxResponseBytes: MAX_FILE_SIZE (100MB, same constant used by the upload path) to secureFetchWithPinnedIP so oversized files reject early with a clear PayloadSizeLimitError instead of exhausting memory.
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 52793b2. Configure here.
Final validation pass (4 parallel audit agents against live Graph docs) surfaced remaining gaps: - apps/sim/app/api/tools/sharepoint/upload/route.ts: siteId/driveId were not encodeURIComponent-ed in the Graph upload URL, unlike every other sharepoint route/tool - same encoding-gap class fixed everywhere else - read_page.ts: transformResponse recomputed siteId with a raw `||` in 3 spots instead of the optionalTrim(...) || optionalTrim(...) || 'root' precedence used by request.url and every sibling tool - SharepointBlock (V1, legacy/hidden-from-toolbar but still executable for existing saved workflows): listItemFields and listItemId were missing `required` for update_list despite the tool requiring them; maxPages/groupId/includeColumns/includeItems/nextPageUrl subBlocks were entirely absent, making those tool params unreachable from the UI; block-level outputs were missing site/pages/content/totalPages/ nextPageUrl/lists/skippedFiles/skippedCount/errors even though the underlying tools return them - V2 already covered all of these
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5a076d5. Configure here.
|
Re: the get_list.ts nested
Our |
|
@greptile review |
1 participant
Summary
$expandsyntax inget_list, reversed site-resolution precedence increate_list, unsanitized field fallback inadd_list_items, missing URL encoding inread_page, and an incorrectroot/serverRelativeUrlfield shape (Graph'srootfacet has no properties)upload_filerequired flag, missingrequiredonpageName/listDisplayName, wrong site-pickermimeType, dead subBlock referencesdelete_list_item,get_list_item,delete_page,update_page,publish_page,download_file,delete_file,get_drive_item— all covered by the OAuth scopes already granted (Sites.Read.All/Sites.ReadWrite.All/Sites.Manage.All), no scope changesstripAuthOnRedirectoption tosecureFetchWithPinnedIPso the new download route doesn't resend the bearer token to the preauthenticated redirect target Graph returns (default off, zero behavior change for existing callers)utils.tshelperType of Change
Testing
Tested manually — verified every tool against live Graph API docs, confirmed backwards compatibility (no renamed/removed tool IDs, params, or outputs), confirmed OAuth scope neutrality, ran lint/typecheck/API-validation-strict clean.
Checklist