Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| "Content-Type": "multipart/form-data", | ||
| }, | ||
| body: formData, | ||
| }); |
There was a problem hiding this comment.
Manual Content-Type header breaks FormData multipart uploads
High Severity
uploadMediaFile manually sets Content-Type: "multipart/form-data" when sending a FormData body via fetch. This prevents the browser from automatically appending the required boundary parameter to the header, so the server cannot parse the multipart body. All uploads through this function will fail. The header line needs to be removed entirely — the browser sets the correct Content-Type with boundary automatically when the body is FormData. The useUploadAsset hook in use-media.tsx correctly omits this header, so only uploadMediaFile is affected.
| variant="outline" | ||
| size="sm" | ||
| data-testid="open-media-picker" | ||
| className="hidden" |
There was a problem hiding this comment.
ImagePicker trigger button hidden in two example apps
Medium Severity
The ImagePicker trigger Button has className="hidden" in the React Router and TanStack examples, making the "Browse Media" button invisible to users. The Next.js example does not have this class. This means users cannot open the media picker from the markdown editor toolbar or CMS file fields in those two example apps. The E2E tests only run against the Next.js example, so this won't be caught by CI.
Additional Locations (1)
| useDeleteFolder: _useDeleteFolder, | ||
| } = require("../../hooks/use-media"); | ||
| return _useDeleteFolder(); | ||
| } |
There was a problem hiding this comment.
Local useDeleteFolder uses require() and has dead code
Medium Severity
The local useDeleteFolder function uses require("../../hooks/use-media") instead of a normal import, despite useDeleteFolder being exported from that module and readily importable alongside the other hooks already imported at the top of the file. Additionally, useDeleteAsset() is called on line 106 but its result (mutateAsync) is never used — it's dead code. The misleading comment ("separate import at top handles this") is incorrect since useDeleteFolder is not imported at the top.
There was a problem hiding this comment.
Security Review — feat/media-plugin-client
Three findings, in priority order. Two are introduced by this PR's changes; one is a latent weakness in the schema layer that this PR newly exposes via the URL-registration UI.
1. MEDIUM — Size-limit check bypassed when file.size is not a number (POST /media/upload)
What changed. The PR replaces instanceof File (which guarantees size is a non-negative integer per the Web API spec) with a duck-type check that only verifies arrayBuffer is a function:
if (
!fileRaw ||
typeof fileRaw !== "object" ||
typeof (fileRaw as any).arrayBuffer !== "function"
) { ... }
const file = fileRaw as Pick<File, "name" | "type" | "size" | "arrayBuffer">;file.size is then used directly in the guard:
if (file.size > maxFileSizeBytes) { ... }Why this matters. In JavaScript, undefined > 10_485_760 evaluates to false. If Better Call's multipart parser produces a file-like object that lacks a numeric size property (e.g., a plain object, or an undici Blob that has not yet set .size), the check silently passes, allowing uploads of arbitrary size regardless of the configured maxFileSizeBytes. The PR author's comment acknowledges that Better Call puts "File instances for file fields", but this assumption is not enforced in the code and could break silently with a parser update.
Remediation. Add an explicit guard immediately after the duck-type check:
if (typeof (fileRaw as any).size !== "number" || (fileRaw as any).size < 0) {
throw ctx.error(400, { message: "File 'size' is missing or invalid" });
}The same principle applies to file.name (must be a non-empty string) and file.type (must be a string), both of which feed into onBeforeUpload hook metadata and validateMimeType.
2. LOW — body.folderId cast to string without a type guard (POST /media/upload)
The multipart body is cast wholesale:
const folderId = (body.folderId as string | undefined) ?? undefined;If Better Call puts a non-string value for folderId (e.g., an array when the client sends the field twice), the raw non-string value is forwarded to getFolderById and then to storageAdapter.upload. Depending on the database adapter, this can produce silently wrong behaviour rather than a 400 error.
Remediation.
const folderId =
typeof body.folderId === "string" && body.folderId ? body.folderId : undefined;3. LOW — z.string().url() accepts javascript: and data: URIs (POST /media/assets — URL registration)
createAssetSchema validates the client-supplied URL with:
url: z.string().url(),Zod's .url() uses the URL constructor, which accepts any protocol. Zod issue #2353 documents that javascript:alert(1) passes validation. The PR's new URL tab (url-tab.tsx) provides a direct UI path for any user with API access to register such a URL. The stored URL is then rendered:
<img src={asset.url} … />Modern browsers do not execute JS from img src, so the direct XSS impact is limited today. However:
- Custom
ImageComponentoverrides could render the URL in anhrefor other executable context. new URL(asset.url, apiBaseURL).hrefis computed and written to the clipboard — ajavascript:href silently survives.
Remediation. Add a protocol allowlist to the schema:
url: z.string().url().refine(
(u) => {
try {
const { protocol } = new URL(u);
return protocol === "https:" || protocol === "http:";
} catch { return false; }
},
{ message: "Only http:// and https:// URLs are allowed" }
),Non-findings / out of scope
- Path traversal in
localAdapter:path.basename()correctly strips directory components before constructing the stored path; no traversal possible fromfile.name. - S3 key injection via
folderId: TheuploadTokenEndpointreplaces the user-suppliedfolderIdwithfolder.idfrom the DB before passing it to the S3 adapter; DB-generated IDs are safe in key paths. - SSRF from URL registration: The server never fetches the registered URL — it only stores it. No SSRF risk.
- Auth by default: All endpoints are unauthenticated unless
onBeforeUpload/onBeforeListAssetsetc. hooks are configured. This is a pre-existing design decision documented in the plugin config, not introduced by this PR.
Sent by Cursor Automation: Find vulnerabilities
| @@ -606,8 +628,7 @@ export const mediaBackendPlugin = (config: MediaBackendConfig) => | |||
| } | |||
There was a problem hiding this comment.
[Security: MEDIUM] Size-limit bypass risk — file.size is not validated as a number.
The duck-type check only confirms arrayBuffer is a function. If Better Call's parser produces a file-like object where size is undefined or not a number, file.size > maxFileSizeBytes evaluates to false and the limit is silently skipped.
Suggest adding an explicit guard right after the duck-type check:
if (typeof (fileRaw as any).size !== "number" || (fileRaw as any).size < 0) {
throw ctx.error(400, { message: "File 'size' is missing or invalid" });
}Similar guards should cover file.name (must be a non-empty string) and file.type (must be a string), as both flow into onBeforeUpload metadata and validateMimeType.
| const buffer = Buffer.from(await file.arrayBuffer()); | ||
| const folderId = | ||
| (formData.get("folderId") as string | undefined) ?? undefined; | ||
| const folderId = (body.folderId as string | undefined) ?? undefined; |
There was a problem hiding this comment.
[Security: LOW] body.folderId is cast to string without a type guard.
If Better Call places a non-string value here (e.g., an array when the field is sent twice), the raw value is forwarded to getFolderById and then to storageAdapter.upload. Prefer an explicit type check:
const folderId =
typeof body.folderId === "string" && body.folderId ? body.folderId : undefined;| size: z.number().int().positive(), | ||
| // Allow 0 for URL-registered assets where size is unknown at registration time. | ||
| size: z.number().int().min(0), | ||
| url: z.string().url(), |
There was a problem hiding this comment.
[Security: LOW] z.string().url() accepts javascript: and data: URIs (Zod issue #2353).
The new URL-registration UI (url-tab.tsx) provides a direct path for users to store arbitrary-protocol URLs. These URLs are later rendered as img src (low direct impact in modern browsers, but custom ImageComponent providers could render them in executable contexts).
Add a protocol allowlist:
url: z.string().url().refine(
(u) => {
try { const { protocol } = new URL(u); return protocol === 'https:' || protocol === 'http:'; }
catch { return false; }
},
{ message: 'Only http:// and https:// URLs are allowed' }
),1 participant
Summary
node: docs and lib version bump will follow in the next PR
Type of change
Checklist
pnpm buildpassespnpm typecheckpassespnpm lintpassesdocs/content/docs/) if consumer-facing types or behavior changedScreenshots
Note
Medium Risk
Adds a new media client plugin plus new upload/URL-registration flows and wires them into multiple existing plugins (blog/CMS/kanban), which can affect asset handling and editor UX across examples. Also changes multipart upload parsing on the media API endpoint, which could impact uploads if the request/body format differs in production.
Overview
Introduces the Media client plugin: a new
/medialibrary route, React Query-powered hooks (useAssets,useFolders, upload/register/delete folder+asset mutations), query keys, and exported client entrypoints/CSS for consumers.Adds a
MediaPickerUI (browse/upload/URL tabs with folders, selection, deletion) plus anImageInputFieldanduploadMediaFilehelper, and integrates picker/field overrides into Blog (featured image + markdown editor insertion), CMS (file fields), and Kanban (task editor image insertion). Example apps (Next.js/React Router/TanStack) are updated to register the media plugin and switch from mock uploads to real media uploads.Updates the media API direct upload endpoint to reliably handle
multipart/form-dataviactx.body(duck-typed file handling) and relaxes asset schema size validation to allow0for URL-registered assets. E2E coverage is expanded with a newsmoke.media.spec.ts, existing CMS upload tests are adjusted to tolerate expected thumbnail 404s, and Playwright defaults now set a fixed viewport.Written by Cursor Bugbot for commit ca3205c. This will update automatically on new commits. Configure here.