feat(api): expand the public v2 files surface - #6160
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview List and upload behavior changes: Shared implementation: Session routes for file content and share now call new orchestration helpers ( Public API guardrails: No presigned upload on v2; share PUT rejects caller-supplied Reviewed by Cursor Bugbot for commit 850f742. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Adds folder support, rename/restore, move, bulk archive, share, and content replace to /api/v2/files, so managing files by API no longer stops at upload + download + archive-one. Routes are thin: auth -> parse -> perform* -> serialize. Share and content replace get their orchestration extracted first so the session routes and the public ones cannot diverge on the effective-authType resolution, the EE public-sharing gate, or the storage-quota classification. Presigned upload stays session-only: presign does an advisory quota check and the real debit happens in the separate register step, so a caller that never registers leaves unaccounted bytes with no reaper. The buffered multipart path debits inside uploadWorkspaceFile's own transaction.
5c245f5 to
6bb9e7e
Compare
|
@cursor review |
Bugbot round 1. The v2 routes map errorCode straight to a status, so every manager failure that arrived unclassified became a 500 for what is really a caller-fixable 400 or 404. - Folder manager throws OrchestrationError: missing target/folder -> not_found, reparent cycle / self-parent / restore-into-archived-workspace -> validation. - File manager does the same for the in-transaction 'File not found' paths that the earlier pass missed. - updateWorkspaceFileContent's outer catch re-wrapped everything in a bare Error, which stripped the class off StorageLimitExceededError and the new not_found alike. It now rethrows a classified failure untouched and attaches cause to the generic wrap, so asOrchestrationError can still walk the chain. - Every remaining perform* gained the asOrchestrationError branch. - renameWorkspaceFile returned the pre-update read, so the v2 PATCH reported a stale updatedAt; it now returns the timestamp it actually wrote. Docs: upload auto-suffixes a duplicate name rather than rejecting it, matching the in-app uploader. The description claimed 409 and was simply wrong.
getWorkspaceFile swallows a query failure and returns null unless throwOnError is set, so a transient blip on the post-upload read reported as 'file could not be read back'. Distinguish the two: a real null after a just-committed write is an invariant break, a query failure is itself.
|
@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 5bb4c37. Configure here.
File folders already live in the shared folder table as resourceType 'file' (#6045 cut them over, #6051 dropped workspace_file_folders), and the remaining file-specific folder machinery is being folded into the generic folder engine. Publishing /api/v2/files/folders/** would pin that transitional split into a public contract we'd then have to keep or break. Files stay folder-aware — folderId/folderPath on the projection, folderId on upload, and the move route — because a folder id is a folder.id and survives the unification untouched. Folder management belongs on /api/v2/folders once that surface serves resourceType 'file'; until then there is no v2 way to enumerate file folders, which is the deliberate gap. The orchestration classification fixes stay: the internal routes and the copilot file-folder tools still call those perform* functions.
|
@cursor review |
Bugbot round 2. uploadWorkspaceFile had the same outer-catch rewrap that
updateWorkspaceFileContent did, so a blown storage quota reached the route as a
bare Error and the v2 handler recovered the status by substring-matching the
message. Any rewording silently demoted a 413 to a 500.
- uploadWorkspaceFile rethrows a classified failure untouched and attaches cause
to the generic wrap.
- FileConflictError is now an OrchestrationError('conflict'), so a duplicate name
classifies like every other conflict. Its 'FILE_EXISTS' discriminator had no
readers and is gone; the instanceof checks elsewhere still hold.
- The v2 upload handler uses v2CaughtOrchestrationError, dropping all three
string matches.
Also documents that bulk-archive is best-effort: unknown or already-archived ids
are skipped rather than failing the call, and deletedItems is what actually
happened. That asymmetry with the single-id DELETE was undocumented.
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 850f742. Configure here.
| export interface PerformRestoreWorkspaceFileFolderResult { | ||
| success: boolean | ||
| error?: string | ||
| errorCode?: WorkspaceFilesOrchestrationErrorCode |
There was a problem hiding this comment.
Bulk archive single ID 404
Medium Severity
POST /api/v2/files/bulk-archive calls the same performDeleteWorkspaceFileItems helper as single-file DELETE. When exactly one fileId (or one folderId) is sent and nothing is archived, the helper returns not_found instead of 200 with zero counts. That contradicts the documented best-effort contract where missing, wrong-workspace, and already-archived ids are skipped.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 850f742. Configure here.
1 participant
Summary
/api/v2/filesfrom list + upload + download + archive-one to cover what the internal files surface does: folders, rename, restore, move, bulk archive, share, and content replacev2FileSchemagainsfolderId/folderPath/updatedAt, the list gainsscope=active|archived(Recently Deleted was unreachable, which made the restore endpoints useless), and upload gainsfolderIdfolderId/folderPathon the projection,folderIdon upload, andPOST /api/v2/files/move) but this PR publishes no folder CRUD. File folders already live in the sharedfoldertable asresourceType: 'file'(feat(folders): cut file folders over, and give knowledge bases and tables folders #6045 cut them over, chore(db): drop the legacy folder tables and adopt the deferred folder_id FKs #6051 droppedworkspace_file_folders), and the remaining file-specific folder machinery is being folded into the generic folder engine — publishing/api/v2/files/folders/**would pin that transitional split into a public contract. A folder id is afolder.idand survives the unification, so the file-side fields are safe; folder management belongs on/api/v2/foldersonce it servesresourceType: 'file'lib/workspace-files/orchestration/first (performUpsertWorkspaceFileShare,performGetWorkspaceFileShare,performUpdateWorkspaceFileContent) and the internal routes now call it — one implementation, not a parallel oneWorkspaceFilesOrchestrationErrorCodewas a narrower private copy of the sharedOrchestrationErrorCode; reconciled onto the shared union so every route usesv2ErrorForOrchestrationand the hand-rolled status switch infiles/[fileId]/route.tsis goneapp/api/v2/files/utils.ts(toV2File,toV2FileFolder)openapi-v2-files-audit.json, all under the existingFilestag — no new sidebar entriesTwo deliberate departures from the internal surface:
/registerstep, so presign → PUT bytes → never register leaves unaccounted bytes in the bucket with no orphan reaper. The buffered multipart path debits insideuploadWorkspaceFile's own transaction, so it's the only public path.token. It exists so the UI can copy a link before saving; over an API key it lets a caller mint predictable public URLs (any string ≥16 chars, no entropy check) and a collision surfaces as an unhandled unique-index 500. v2 tokens are always server-generated. The OpenAPI description also spells out that disable is not revoke —upsertFileSharepreserves the token and stored password/allow-list, so re-enabling resurrects the identical URL.Also fixed at the source rather than propagated:
updateWorkspaceFileContent,renameWorkspaceFile, andrestoreWorkspaceFilenow throw a classifiedOrchestrationError('not_found')instead of a bareError, so nothing downstream string-sniffs'File not found'or'Storage limit exceeded'(the latter was already a classifiedpayload_too_large).Out of scope, on purpose: bulk zip download (~150 lines of budget accounting and archiver wiring with no lib home — needs its own extraction pass), style / csv-preview / compiled-check / inline-image (editor-internal), and
/api/tools/file/manage's write/append/compress/decompress.Known gaps (not fixed here)
No v2 way to enumerate file folders.
/api/v2/folderscurrently rejectsresourceType=file, so a caller can setfolderIdon upload/move only with an id it already holds. This is the deliberate consequence of not publishing a transitional folder surface; it closes when the generic folder engine takes over file folders (it needs the path-segment name rules and the per-workspace advisory lock first).v2 list pagination is still in-memory —
listWorkspaceFilesreturns the full set for the scope and the route sorts and slices. Addingscopemakes the set larger, so this gets worse before it gets better.Shares have no expiry column, no per-workspace cap, and no creation-side rate limit.
The EE
disablePublicFileSharingpolicy resolves againstgetUserPermissionConfig(userId, workspaceId)— worth confirming it behaves for a workspace-scoped API key, whoseuserIdis the billed account rather than the acting human.Duplicate-name semantics are unchanged and shared: both surfaces auto-suffix via
allocateUniqueWorkspaceFileName, so an upload can return201with anamedifferent from the one sent — clients must readnameback off the response.CONFLICTfires only when a unique name can't be allocated after several attempts. (My first draft of the OpenAPI description claimed 409 on any collision; Bugbot caught it and it's corrected.)PATCH /api/v2/files/{fileId}is the exact-or-fail path.v2 access denials stay
403rather than being masked as404on reads — that matches every existing v2 route (v2WorkspaceAccessError); masking would be a surface-wide change, not a files-only one.Type of Change
Testing
bun run check:api-validation:strict— passes (route baseline bumped 1041 → 1049 for the 8 new route files)bun run check:openapi— passes, 7 specs / 94 operations / 85 contracts cross-checkedbun run type-check— passesbun run lint:check— passesbunx vitest run app/api/v2/files lib/workspace-files lib/uploads app/api/workspaces— 48 files, 425 tests passingroute.test.tsper route file: gate-off, invalid input, 403, 429, and happy path each, plus targeted cases for share PUT rejecting an unknowntoken, a bare re-enable keeping the existing token,scope=archivedlisting only archived items, and an upload landing in the folder named byfolderIdChecklist