feat(files): export markdown as zip with embedded images#4413
feat(files): export markdown as zip with embedded images#4413waleedlatif1 merged 13 commits intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Refactors workspace file reads by renaming server-side Tweaks workflow lock UX: unlock notifications no longer auto-dismiss and don’t disappear when clicked, locked canvases now surface an info notification on drop attempts, admin unlock controls are disabled while read-only, and locked canvas opacity is adjusted. Reviewed by Cursor Bugbot for commit 76a07aa. Configure here. |
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — only P2 style/robustness findings; core export logic, auth, and filename sanitisation are sound. All findings are P2: incomplete control-character stripping in Content-Disposition (functional but non-spec) and missing filename* RFC 5987 encoding for non-ASCII names (browser-tolerated). No P0/P1 issues found. The access-check, deduplication, zip-slip prevention, and rename refactor are all correct. apps/sim/app/api/files/export/[id]/route.ts — two minor header-encoding improvements suggested. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Client (triggerFileDownload)
participant E as GET /api/files/export/[id]
participant S as GET /api/files/serve/…
participant DB as File Metadata DB
participant Stor as Storage Service
C->>C: isMarkdown(record)?
alt markdown
C->>E: GET /api/files/export/{id}
E->>E: checkSessionOrInternalAuth
E->>DB: getFileMetadataById(id)
E->>E: verifyFileAccess
E->>E: isMarkdown(originalName, contentType)?
alt not markdown on server
E-->>S: 302 redirect to serve path
else markdown
E->>Stor: downloadFile(key, context)
E->>E: extract imageIds via VIEW_URL_RE (up to 50)
par fetch all images
E->>DB: getFileMetadataById(imageId)
E->>Stor: downloadFile(imgKey, context)
end
E->>E: deduplicate filenames, rewrite md refs
E->>E: zip.file(mdName, content) + assets/
E-->>C: 200 application/zip
end
else non-markdown
C->>S: GET /api/files/serve/{key}?context=workspace
S-->>C: raw file
end
C->>C: createObjectURL → anchor click → download
Reviews (5): Last reviewed commit: "fix(tests): update mocks for fetchWorksp..." | Re-trigger Greptile |
|
@greptile |
|
@cursor review |
|
@greptile |
|
@cursor review |
|
@greptile |
|
@cursor review |
…edded image count
…n in markdown URL rewrite
…ale lock notification When workspacePermissions loads asynchronously, prevCanAdminRef (which only tracked effectivePermissions.canAdmin) would not detect the change, causing the early-return guard to skip rebuilding the notification with the correct unlock-button visibility. Track the same resolved value (workspacePermissions ?.viewer?.isAdmin ?? effectivePermissions.canAdmin) that is actually used to build the notification.
… client download to uploads/client/download.ts as triggerFileDownload
|
@greptile |
|
@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 76a07aa. Configure here.
Summary
GET /api/files/export/[id]— when the target file is markdown, downloads all embedded workspace images, rewritesrefs to./assets/{filename}, and returns a self-contained zipdownloadWorkspaceFileto route markdown files through the export endpoint automatically; falls back to raw serve for files without an idpath.basename+ strip",\, control chars) prevents header injection and zip path traversalStorageContext(was forced to'workspace', breaking mothership files)Type of Change
Testing
Tested manually
Checklist