From 29eb889a3613d36f9513edd470ab92dbd88b76d4 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Yadav Date: Sat, 25 Apr 2026 18:57:35 +0530 Subject: [PATCH 1/5] feat: implement presigned upload flow for dashboard API (#131) - Add requestUpload handler to generate presigned URLs with quota peeking - Add confirmUpload handler to verify uploads and charge quota atomically - Implement size validation, path traversal protection, and cleanup on failure - Add presigned upload tests for controller logic and route wiring - Update storage docs with presigned flow and S3/R2 CORS requirements --- .../__tests__/routes.projects.storage.test.js | 110 ++++++++ .../storage.presigned.controller.test.js | 258 ++++++++++++++++++ .../src/controllers/project.controller.js | 223 +++++++++++++++ apps/dashboard-api/src/routes/projects.js | 14 +- docs-legacy/storage.md | 31 +++ 5 files changed, 632 insertions(+), 4 deletions(-) create mode 100644 apps/dashboard-api/src/__tests__/routes.projects.storage.test.js create mode 100644 apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js diff --git a/apps/dashboard-api/src/__tests__/routes.projects.storage.test.js b/apps/dashboard-api/src/__tests__/routes.projects.storage.test.js new file mode 100644 index 00000000..bc478a2d --- /dev/null +++ b/apps/dashboard-api/src/__tests__/routes.projects.storage.test.js @@ -0,0 +1,110 @@ +'use strict'; + +jest.mock('../middlewares/authMiddleware', () => + jest.fn((req, _res, next) => { + req.user = { _id: 'dev1' }; + next(); + }), +); + +jest.mock('../middlewares/planEnforcement', () => ({ + attachDeveloper: jest.fn((_req, _res, next) => next()), + checkProjectLimit: jest.fn((_req, _res, next) => next()), + checkCollectionLimit: jest.fn((_req, _res, next) => next()), + checkByokGate: jest.fn((_req, _res, next) => next()), +})); + +jest.mock('@urbackend/common', () => ({ + verifyEmail: jest.fn((_req, _res, next) => next()), + checkAuthEnabled: jest.fn((_req, _res, next) => next()), + loadProjectForAdmin: jest.fn((_req, _res, next) => next()), +})); + +jest.mock('../controllers/userAuth.controller', () => ({ + createAdminUser: jest.fn((_req, res) => res.json({ ok: true })), + resetPassword: jest.fn((_req, res) => res.json({ ok: true })), + getUserDetails: jest.fn((_req, res) => res.json({ ok: true })), + updateAdminUser: jest.fn((_req, res) => res.json({ ok: true })), + listUserSessions: jest.fn((_req, res) => res.json({ ok: true })), + revokeUserSession: jest.fn((_req, res) => res.json({ ok: true })), +})); + +jest.mock('../controllers/project.controller', () => { + const ok = (_req, res) => res.json({ ok: true }); + return { + createProject: jest.fn(ok), + getAllProject: jest.fn(ok), + getSingleProject: jest.fn(ok), + regenerateApiKey: jest.fn(ok), + createCollection: jest.fn(ok), + deleteCollection: jest.fn(ok), + getData: jest.fn(ok), + deleteRow: jest.fn(ok), + insertData: jest.fn(ok), + editRow: jest.fn(ok), + uploadFile: jest.fn(ok), + listFiles: jest.fn(ok), + deleteFile: jest.fn(ok), + deleteAllFiles: jest.fn(ok), + deleteProject: jest.fn(ok), + updateProject: jest.fn(ok), + updateExternalConfig: jest.fn(ok), + deleteExternalDbConfig: jest.fn(ok), + deleteExternalStorageConfig: jest.fn(ok), + analytics: jest.fn(ok), + updateAllowedDomains: jest.fn(ok), + toggleAuth: jest.fn(ok), + updateAuthProviders: jest.fn(ok), + updateCollectionRls: jest.fn(ok), + listMailTemplates: jest.fn(ok), + listGlobalMailTemplates: jest.fn(ok), + getMailTemplate: jest.fn(ok), + createMailTemplate: jest.fn(ok), + updateMailTemplate: jest.fn(ok), + deleteMailTemplate: jest.fn(ok), + requestUpload: jest.fn(ok), + confirmUpload: jest.fn(ok), + }; +}); + +const express = require('express'); +const request = require('supertest'); +const projectsRouter = require('../routes/projects'); +const projectController = require('../controllers/project.controller'); +const authMiddleware = require('../middlewares/authMiddleware'); +const { verifyEmail, loadProjectForAdmin } = require('@urbackend/common'); + +let app; + +beforeEach(() => { + jest.clearAllMocks(); + app = express(); + app.use(express.json()); + app.use('/api/projects', projectsRouter); +}); + +describe('projects storage presigned routes', () => { + test('upload-request route is wired and protected', async () => { + const res = await request(app) + .post('/api/projects/project1/storage/upload-request') + .send({ filename: 'a.txt', contentType: 'text/plain', size: 10 }); + + expect(res.status).toBe(200); + expect(authMiddleware).toHaveBeenCalled(); + expect(verifyEmail).toHaveBeenCalled(); + expect(loadProjectForAdmin).toHaveBeenCalled(); + expect(projectController.requestUpload).toHaveBeenCalledTimes(1); + }); + + test('upload-confirm route is wired and protected', async () => { + const res = await request(app) + .post('/api/projects/project1/storage/upload-confirm') + .send({ filePath: 'project1/a.txt', size: 10 }); + + expect(res.status).toBe(200); + expect(authMiddleware).toHaveBeenCalled(); + expect(verifyEmail).toHaveBeenCalled(); + expect(loadProjectForAdmin).toHaveBeenCalled(); + expect(projectController.confirmUpload).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js b/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js new file mode 100644 index 00000000..c8f2b0e2 --- /dev/null +++ b/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js @@ -0,0 +1,258 @@ +'use strict'; + +jest.mock('crypto', () => { + const actual = jest.requireActual('crypto'); + return { + ...actual, + randomUUID: jest.fn(() => 'mocked-uuid'), + }; +}); + +jest.mock('@urbackend/common', () => { + const mockStorageFrom = { + getPublicUrl: jest.fn(), + remove: jest.fn(), + }; + + const mockSupabaseStorage = { + from: jest.fn(() => mockStorageFrom), + }; + + return { + Project: { + findOne: jest.fn(), + updateOne: jest.fn(), + }, + getStorage: jest.fn(() => ({ + storage: mockSupabaseStorage, + })), + getPresignedUploadUrl: jest.fn(), + verifyUploadedFile: jest.fn(), + isProjectStorageExternal: jest.fn(), + getBucket: jest.fn(() => 'dev-files'), + __mockStorageFrom: mockStorageFrom, + }; +}); + +const { + Project, + getPresignedUploadUrl, + verifyUploadedFile, + isProjectStorageExternal, + __mockStorageFrom: mockStorageFrom, +} = require('@urbackend/common'); + +const controller = require('../controllers/project.controller'); + +const makeRes = () => { + const res = { status: jest.fn(), json: jest.fn() }; + res.status.mockReturnValue(res); + res.json.mockReturnValue(res); + return res; +}; + +const makeProject = (overrides = {}) => ({ + _id: 'project1', + owner: 'dev1', + storageUsed: 100, + storageLimit: 1024 * 1024, + resources: { storage: { isExternal: false } }, + ...overrides, +}); + +const mockFindOneSelect = (project) => { + Project.findOne.mockReturnValue({ + select: jest.fn().mockResolvedValue(project), + }); +}; + +describe('dashboard project.controller presigned upload', () => { + beforeEach(() => { + jest.clearAllMocks(); + process.env.NODE_ENV = 'test'; + }); + + describe('requestUpload', () => { + test('returns 400 for invalid input', async () => { + const req = { + params: { projectId: 'project1' }, + body: { filename: 'a.txt', contentType: 'text/plain', size: 'abc' }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.requestUpload(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + error: 'filename, contentType, and size are required.', + }); + }); + + test('returns 403 when internal quota is exceeded', async () => { + mockFindOneSelect(makeProject({ storageUsed: 900, storageLimit: 1000 })); + isProjectStorageExternal.mockReturnValue(false); + + const req = { + params: { projectId: 'project1' }, + body: { filename: 'a.txt', contentType: 'text/plain', size: 200 }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.requestUpload(req, res); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ error: 'Internal storage limit exceeded.' }); + }); + + test('returns signed URL payload on success', async () => { + mockFindOneSelect(makeProject()); + isProjectStorageExternal.mockReturnValue(false); + getPresignedUploadUrl.mockResolvedValue({ + signedUrl: 'https://signed.example/upload', + token: 't1', + }); + + const req = { + params: { projectId: 'project1' }, + body: { filename: 'my file.txt', contentType: 'text/plain', size: 1234 }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.requestUpload(req, res); + + expect(getPresignedUploadUrl).toHaveBeenCalledWith( + expect.objectContaining({ _id: 'project1' }), + 'project1/mocked-uuid_my_file.txt', + 'text/plain', + 1234, + ); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ + signedUrl: 'https://signed.example/upload', + token: 't1', + filePath: 'project1/mocked-uuid_my_file.txt', + }); + }); + }); + + describe('confirmUpload', () => { + test('returns 403 when project path does not match', async () => { + mockFindOneSelect(makeProject()); + isProjectStorageExternal.mockReturnValue(false); + + const req = { + params: { projectId: 'project1' }, + body: { filePath: 'project2/file.txt', size: 200 }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.confirmUpload(req, res); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ error: 'Access denied.' }); + }); + + test('returns 409 when uploaded file is not visible yet', async () => { + mockFindOneSelect(makeProject()); + isProjectStorageExternal.mockReturnValue(false); + verifyUploadedFile.mockRejectedValue(new Error('File not found after upload')); + mockStorageFrom.remove.mockResolvedValue({ data: null, error: null }); + + const req = { + params: { projectId: 'project1' }, + body: { filePath: 'project1/file.txt', size: 200 }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.confirmUpload(req, res); + + expect(mockStorageFrom.remove).toHaveBeenCalledWith(['project1/file.txt']); + expect(res.status).toHaveBeenCalledWith(409); + expect(res.json).toHaveBeenCalledWith({ + error: 'UPLOAD_NOT_READY', + message: 'Uploaded file is not visible yet. Please retry confirmation.', + }); + }); + + test('returns 400 when declared size mismatches actual size', async () => { + mockFindOneSelect(makeProject()); + isProjectStorageExternal.mockReturnValue(false); + verifyUploadedFile.mockResolvedValue(1024); + mockStorageFrom.remove.mockResolvedValue({ data: null, error: null }); + + const req = { + params: { projectId: 'project1' }, + body: { filePath: 'project1/file.txt', size: 900 }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.confirmUpload(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + error: 'Declared file size does not match uploaded file size.', + }); + expect(mockStorageFrom.remove).toHaveBeenCalledWith(['project1/file.txt']); + }); + + test('charges quota and returns success on internal storage', async () => { + mockFindOneSelect(makeProject()); + isProjectStorageExternal.mockReturnValue(false); + verifyUploadedFile.mockResolvedValue(1024); + Project.updateOne.mockResolvedValue({ matchedCount: 1 }); + mockStorageFrom.getPublicUrl.mockReturnValue({ data: { publicUrl: 'https://cdn.example/p/project1/file.txt' } }); + + const req = { + params: { projectId: 'project1' }, + body: { filePath: 'project1/file.txt', size: 1024 }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.confirmUpload(req, res); + + expect(Project.updateOne).toHaveBeenCalledWith( + { + _id: 'project1', + $or: [ + { storageLimit: -1 }, + { $expr: { $lte: [{ $add: ['$storageUsed', 1024] }, '$storageLimit'] } }, + ], + }, + { $inc: { storageUsed: 1024 } }, + ); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ + message: 'Upload confirmed', + path: 'project1/file.txt', + provider: 'internal', + })); + }); + + test('skips quota charge for external storage', async () => { + mockFindOneSelect(makeProject({ resources: { storage: { isExternal: true } } })); + isProjectStorageExternal.mockReturnValue(true); + verifyUploadedFile.mockResolvedValue(512); + mockStorageFrom.getPublicUrl.mockReturnValue({ data: { publicUrl: null, error: 'No public host' } }); + + const req = { + params: { projectId: 'project1' }, + body: { filePath: 'project1/file.txt', size: 512 }, + user: { _id: 'dev1' }, + }; + const res = makeRes(); + + await controller.confirmUpload(req, res); + + expect(Project.updateOne).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ provider: 'external' })); + }); + }); +}); diff --git a/apps/dashboard-api/src/controllers/project.controller.js b/apps/dashboard-api/src/controllers/project.controller.js index 1029dd15..0d16923c 100644 --- a/apps/dashboard-api/src/controllers/project.controller.js +++ b/apps/dashboard-api/src/controllers/project.controller.js @@ -14,6 +14,7 @@ const { generateApiKey, hashApiKey } = require("@urbackend/common"); const { z } = require("zod"); const { encrypt } = require("@urbackend/common"); const { URL } = require("url"); +const path = require("path"); const { getConnection } = require("@urbackend/common"); const { getCompiledModel } = require("@urbackend/common"); const { QueryEngine } = require("@urbackend/common"); @@ -25,10 +26,16 @@ const { deleteProjectById, } = require("@urbackend/common"); const { isProjectStorageExternal, getBucket } = require("@urbackend/common"); +const { getPresignedUploadUrl } = require("@urbackend/common"); +const { verifyUploadedFile } = require("@urbackend/common"); const { getPublicIp } = require("@urbackend/common"); const { clearCompiledModel } = require("@urbackend/common"); const { createUniqueIndexes } = require("@urbackend/common"); +const MAX_FILE_SIZE = 10 * 1024 * 1024; +const SAFETY_MAX_BYTES = 100 * 1024 * 1024; +const CONFIRM_UPLOAD_SIZE_TOLERANCE_BYTES = 64; + const validateUsersSchema = (schema) => { if (!Array.isArray(schema)) return false; @@ -185,6 +192,54 @@ const sanitizeProjectResponse = (projectObj) => { return projectObj; }; +const parsePositiveSize = (size) => { + const numericSize = Number(size); + if (!Number.isFinite(numericSize) || numericSize <= 0) { + return null; + } + return numericSize; +}; + +const normalizeProjectPath = (projectId, inputPath) => { + if (typeof inputPath !== "string") { + return null; + } + + let decodedPath = inputPath; + try { + decodedPath = decodeURIComponent(inputPath); + } catch { + return null; + } + + const normalizedPath = path.posix.normalize(decodedPath).replace(/^\/+/, ""); + const segments = normalizedPath.split("/").filter(Boolean); + + if (segments.length < 2) { + return null; + } + + if (segments[0] !== String(projectId)) { + return null; + } + + if (segments.some((segment) => segment === "." || segment === "..")) { + return null; + } + + return normalizedPath; +}; + +const bestEffortDeleteUploadedObject = async (project, filePath) => { + try { + const supabase = await getStorage(project); + const bucket = getBucket(project); + await supabase.storage.from(bucket).remove([filePath]); + } catch { + // ignore cleanup failures; the primary response should still be returned + } +}; + module.exports.createProject = async (req, res) => { const session = await mongoose.startSession(); session.startTransaction(); @@ -1164,6 +1219,174 @@ module.exports.deleteAllFiles = async (req, res) => { } }; +module.exports.requestUpload = async (req, res) => { + try { + const { projectId } = req.params; + const { filename, contentType, size } = req.body; + const numericSize = parsePositiveSize(size); + + if (!filename || !contentType || numericSize === null) { + return res + .status(400) + .json({ error: "filename, contentType, and size are required." }); + } + + if (numericSize > MAX_FILE_SIZE) { + return res.status(413).json({ error: "File size exceeds limit." }); + } + + const project = await Project.findOne({ + _id: projectId, + owner: req.user._id, + }).select( + "+resources.storage.config.encrypted +resources.storage.config.iv +resources.storage.config.tag resources.storage.isExternal storageUsed storageLimit", + ); + + if (!project) return res.status(404).json({ error: "Project not found" }); + + const external = isProjectStorageExternal(project); + + // just peek at quota - don't charge yet, upload hasn't happened + if (!external) { + const storageLimit = + typeof project.storageLimit === "number" + ? project.storageLimit + : 20 * 1024 * 1024; + const quotaLimit = + storageLimit === -1 ? SAFETY_MAX_BYTES : storageLimit; + + if ((project.storageUsed || 0) + numericSize > quotaLimit) { + return res.status(403).json({ error: "Internal storage limit exceeded." }); + } + } + + const safeName = filename.replace(/\s+/g, "_"); + const filePath = `${projectId}/${randomUUID()}_${safeName}`; + + const { signedUrl, token } = await getPresignedUploadUrl( + project, + filePath, + contentType, + numericSize, + ); + + return res.status(200).json({ signedUrl, token, filePath }); + } catch (err) { + return res.status(500).json({ + error: "Could not generate upload URL", + details: process.env.NODE_ENV === "development" ? err.message : undefined, + }); + } +}; + +module.exports.confirmUpload = async (req, res) => { + try { + const { projectId } = req.params; + const { filePath, size } = req.body; + const declaredSize = parsePositiveSize(size); + + if (!filePath || declaredSize === null) { + return res.status(400).json({ error: "filePath and size are required." }); + } + + const project = await Project.findOne({ + _id: projectId, + owner: req.user._id, + }).select( + "+resources.storage.config.encrypted +resources.storage.config.iv +resources.storage.config.tag resources.storage.isExternal storageUsed storageLimit", + ); + + if (!project) return res.status(404).json({ error: "Project not found" }); + + const external = isProjectStorageExternal(project); + const normalizedPath = normalizeProjectPath(projectId, filePath); + + // make sure client isn't confirming someone else's file + if (!normalizedPath) { + return res.status(403).json({ error: "Access denied." }); + } + + // verify file actually exists on cloud before touching quota + let actualSize; + try { + actualSize = await verifyUploadedFile(project, normalizedPath); + } catch (err) { + if (err?.message === "File not found after upload") { + await bestEffortDeleteUploadedObject(project, normalizedPath); + return res.status(409).json({ + error: "UPLOAD_NOT_READY", + message: "Uploaded file is not visible yet. Please retry confirmation.", + }); + } + throw err; + } + + if (!Number.isFinite(actualSize) || actualSize <= 0) { + await bestEffortDeleteUploadedObject(project, normalizedPath); + return res.status(500).json({ + error: "Upload confirmation failed", + details: + process.env.NODE_ENV === "development" + ? "Uploaded file size could not be determined" + : undefined, + }); + } + + if (Math.abs(actualSize - declaredSize) > CONFIRM_UPLOAD_SIZE_TOLERANCE_BYTES) { + await bestEffortDeleteUploadedObject(project, normalizedPath); + return res + .status(400) + .json({ error: "Declared file size does not match uploaded file size." }); + } + + // now it's safe to charge quota + if (!external) { + const result = await Project.updateOne( + { + _id: project._id, + $or: [ + { storageLimit: -1 }, + { $expr: { $lte: [{ $add: ["$storageUsed", actualSize] }, "$storageLimit"] } }, + ], + }, + { $inc: { storageUsed: actualSize } }, + ); + + if (result.matchedCount === 0) { + await bestEffortDeleteUploadedObject(project, normalizedPath); + return res.status(403).json({ error: "Internal storage limit exceeded." }); + } + } + + const supabase = await getStorage(project); + const bucket = getBucket(project); + const { data: publicUrlData } = supabase.storage + .from(bucket) + .getPublicUrl(normalizedPath); + + const response = { + message: "Upload confirmed", + path: normalizedPath, + provider: external ? "external" : "internal", + }; + + if (publicUrlData?.publicUrl) { + response.url = publicUrlData.publicUrl; + } else { + response.url = null; + response.warning = + publicUrlData?.error || "Upload confirmed, but a public URL is unavailable."; + } + + return res.status(200).json(response); + } catch (err) { + return res.status(500).json({ + error: "Upload confirmation failed", + details: process.env.NODE_ENV === "development" ? err.message : undefined, + }); + } +}; + module.exports.updateProject = async (req, res) => { try { const { name, siteUrl, resendApiKey, resendFromEmail } = req.body; diff --git a/apps/dashboard-api/src/routes/projects.js b/apps/dashboard-api/src/routes/projects.js index 301a45cb..beac0bbc 100644 --- a/apps/dashboard-api/src/routes/projects.js +++ b/apps/dashboard-api/src/routes/projects.js @@ -2,7 +2,7 @@ const express = require('express'); const router = express.Router(); const authMiddleware = require('../middlewares/authMiddleware'); const { attachDeveloper, checkProjectLimit, checkCollectionLimit, checkByokGate } = require('../middlewares/planEnforcement'); -const {verifyEmail} = require('@urbackend/common') +const { verifyEmail, checkAuthEnabled, loadProjectForAdmin } = require('@urbackend/common'); const multer = require('multer'); const storage = multer.memoryStorage(); @@ -36,7 +36,9 @@ const { getMailTemplate, createMailTemplate, updateMailTemplate, - deleteMailTemplate + deleteMailTemplate, + requestUpload, + confirmUpload } = require("../controllers/project.controller") const { createAdminUser, resetPassword, getUserDetails, updateAdminUser, listUserSessions, revokeUserSession } = require('../controllers/userAuth.controller'); @@ -80,6 +82,11 @@ router.post('/:projectId/storage/upload', authMiddleware, verifyEmail, upload.si // POST REQ FOR DELETE FILE router.post('/:projectId/storage/delete', authMiddleware, verifyEmail, deleteFile); +//SIGNED URL +router.post('/:projectId/storage/upload-request', authMiddleware, verifyEmail, loadProjectForAdmin, requestUpload); +//UPLOAD URL +router.post('/:projectId/storage/upload-confirm', authMiddleware, verifyEmail, loadProjectForAdmin, confirmUpload); + // DELETE REQ FOR PROJECT router.delete('/:projectId', authMiddleware, verifyEmail, deleteProject); @@ -125,8 +132,7 @@ router.patch('/:projectId/auth/providers', authMiddleware, verifyEmail, updateAu router.patch('/:projectId/collections/:collectionName/rls', authMiddleware, verifyEmail, updateCollectionRls); // ADMIN AUTH ROUTES -const {checkAuthEnabled} = require('@urbackend/common'); -const {loadProjectForAdmin} = require('@urbackend/common'); + router.post('/:projectId/admin/users', authMiddleware, loadProjectForAdmin, checkAuthEnabled, createAdminUser); router.patch('/:projectId/admin/users/:userId/password', authMiddleware, loadProjectForAdmin, checkAuthEnabled, resetPassword); diff --git a/docs-legacy/storage.md b/docs-legacy/storage.md index 467b4351..8ee98905 100644 --- a/docs-legacy/storage.md +++ b/docs-legacy/storage.md @@ -66,3 +66,34 @@ If `path` is invalid or already removed, API returns `404`. - `400 Bad Request`: usually missing `file` in multipart form. - `401 Unauthorized`: missing/invalid API key. - `413 Payload Too Large`: file exceeds max size limit. + +## Presigned Upload (Dashboard/Public API) + +For the newer upload flow, file bytes are uploaded directly from the browser to storage. + +Typical flow: + +1. Call backend `upload-request` endpoint with `filename`, `contentType`, and `size`. +2. Receive `signedUrl` and `filePath`. +3. Browser uploads file to `signedUrl` using `PUT`. +4. Call backend `upload-confirm` endpoint to verify file existence and charge quota. + +This avoids proxying file bytes through Node.js and keeps server memory usage predictable. + +## Required Bucket CORS For S3/R2 + +If using AWS S3 or Cloudflare R2 with presigned browser uploads, bucket CORS must allow the dashboard origin. + +Required methods: + +- `PUT` +- `OPTIONS` +- `GET` +- `HEAD` + +Required headers should include at least: + +- `content-type` +- `content-length` + +If preflight CORS is missing or restrictive, browser uploads will fail even when the signed URL is valid. From 42ef7f378cde158506e6977463c20e6545cbb874 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Yadav Date: Sat, 25 Apr 2026 19:20:41 +0530 Subject: [PATCH 2/5] minor changes --- .../storage.presigned.controller.test.js | 112 ++++++++++++------ .../src/controllers/project.controller.js | 92 +++++++------- docs-legacy/storage.md | 6 +- mintlify/docs/guides/storage.mdx | 22 +++- 4 files changed, 149 insertions(+), 83 deletions(-) diff --git a/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js b/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js index c8f2b0e2..53f2e167 100644 --- a/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js +++ b/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js @@ -9,6 +9,14 @@ jest.mock('crypto', () => { }); jest.mock('@urbackend/common', () => { + class AppError extends Error { + constructor(statusCode, message) { + super(message); + this.statusCode = statusCode; + this.status = `${statusCode}`.startsWith('4') ? 'fail' : 'error'; + } + } + const mockStorageFrom = { getPublicUrl: jest.fn(), remove: jest.fn(), @@ -30,6 +38,7 @@ jest.mock('@urbackend/common', () => { verifyUploadedFile: jest.fn(), isProjectStorageExternal: jest.fn(), getBucket: jest.fn(() => 'dev-files'), + AppError, __mockStorageFrom: mockStorageFrom, }; }); @@ -51,6 +60,8 @@ const makeRes = () => { return res; }; +const makeNext = () => jest.fn(); + const makeProject = (overrides = {}) => ({ _id: 'project1', owner: 'dev1', @@ -80,13 +91,14 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.requestUpload(req, res); + await controller.requestUpload(req, res, next); - expect(res.status).toHaveBeenCalledWith(400); - expect(res.json).toHaveBeenCalledWith({ - error: 'filename, contentType, and size are required.', - }); + expect(next).toHaveBeenCalledWith(expect.objectContaining({ + statusCode: 400, + message: 'filename, contentType, and size are required.', + })); }); test('returns 403 when internal quota is exceeded', async () => { @@ -99,11 +111,14 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.requestUpload(req, res); + await controller.requestUpload(req, res, next); - expect(res.status).toHaveBeenCalledWith(403); - expect(res.json).toHaveBeenCalledWith({ error: 'Internal storage limit exceeded.' }); + expect(next).toHaveBeenCalledWith(expect.objectContaining({ + statusCode: 403, + message: 'Internal storage limit exceeded.', + })); }); test('returns signed URL payload on success', async () => { @@ -120,8 +135,9 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.requestUpload(req, res); + await controller.requestUpload(req, res, next); expect(getPresignedUploadUrl).toHaveBeenCalledWith( expect.objectContaining({ _id: 'project1' }), @@ -131,10 +147,15 @@ describe('dashboard project.controller presigned upload', () => { ); expect(res.status).toHaveBeenCalledWith(200); expect(res.json).toHaveBeenCalledWith({ - signedUrl: 'https://signed.example/upload', - token: 't1', - filePath: 'project1/mocked-uuid_my_file.txt', + success: true, + data: { + signedUrl: 'https://signed.example/upload', + token: 't1', + filePath: 'project1/mocked-uuid_my_file.txt', + }, + message: 'Upload URL generated successfully.', }); + expect(next).not.toHaveBeenCalled(); }); }); @@ -149,11 +170,14 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.confirmUpload(req, res); + await controller.confirmUpload(req, res, next); - expect(res.status).toHaveBeenCalledWith(403); - expect(res.json).toHaveBeenCalledWith({ error: 'Access denied.' }); + expect(next).toHaveBeenCalledWith(expect.objectContaining({ + statusCode: 403, + message: 'Access denied.', + })); }); test('returns 409 when uploaded file is not visible yet', async () => { @@ -168,15 +192,15 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.confirmUpload(req, res); + await controller.confirmUpload(req, res, next); expect(mockStorageFrom.remove).toHaveBeenCalledWith(['project1/file.txt']); - expect(res.status).toHaveBeenCalledWith(409); - expect(res.json).toHaveBeenCalledWith({ - error: 'UPLOAD_NOT_READY', + expect(next).toHaveBeenCalledWith(expect.objectContaining({ + statusCode: 409, message: 'Uploaded file is not visible yet. Please retry confirmation.', - }); + })); }); test('returns 400 when declared size mismatches actual size', async () => { @@ -191,13 +215,14 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.confirmUpload(req, res); + await controller.confirmUpload(req, res, next); - expect(res.status).toHaveBeenCalledWith(400); - expect(res.json).toHaveBeenCalledWith({ - error: 'Declared file size does not match uploaded file size.', - }); + expect(next).toHaveBeenCalledWith(expect.objectContaining({ + statusCode: 400, + message: 'Declared file size does not match uploaded file size.', + })); expect(mockStorageFrom.remove).toHaveBeenCalledWith(['project1/file.txt']); }); @@ -214,8 +239,9 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.confirmUpload(req, res); + await controller.confirmUpload(req, res, next); expect(Project.updateOne).toHaveBeenCalledWith( { @@ -228,18 +254,24 @@ describe('dashboard project.controller presigned upload', () => { { $inc: { storageUsed: 1024 } }, ); expect(res.status).toHaveBeenCalledWith(200); - expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ - message: 'Upload confirmed', - path: 'project1/file.txt', - provider: 'internal', - })); + expect(res.json).toHaveBeenCalledWith({ + success: true, + data: { + message: 'Upload confirmed', + path: 'project1/file.txt', + provider: 'internal', + url: 'https://cdn.example/p/project1/file.txt', + }, + message: 'Upload confirmed.', + }); + expect(next).not.toHaveBeenCalled(); }); test('skips quota charge for external storage', async () => { mockFindOneSelect(makeProject({ resources: { storage: { isExternal: true } } })); isProjectStorageExternal.mockReturnValue(true); verifyUploadedFile.mockResolvedValue(512); - mockStorageFrom.getPublicUrl.mockReturnValue({ data: { publicUrl: null, error: 'No public host' } }); + mockStorageFrom.getPublicUrl.mockReturnValue({ data: { publicUrl: null } }); const req = { params: { projectId: 'project1' }, @@ -247,12 +279,24 @@ describe('dashboard project.controller presigned upload', () => { user: { _id: 'dev1' }, }; const res = makeRes(); + const next = makeNext(); - await controller.confirmUpload(req, res); + await controller.confirmUpload(req, res, next); expect(Project.updateOne).not.toHaveBeenCalled(); expect(res.status).toHaveBeenCalledWith(200); - expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ provider: 'external' })); + expect(res.json).toHaveBeenCalledWith({ + success: true, + data: { + message: 'Upload confirmed', + path: 'project1/file.txt', + provider: 'external', + url: null, + warning: 'Upload confirmed, but a public URL is unavailable.', + }, + message: 'Upload confirmed.', + }); + expect(next).not.toHaveBeenCalled(); }); }); }); diff --git a/apps/dashboard-api/src/controllers/project.controller.js b/apps/dashboard-api/src/controllers/project.controller.js index 0d16923c..5b63b386 100644 --- a/apps/dashboard-api/src/controllers/project.controller.js +++ b/apps/dashboard-api/src/controllers/project.controller.js @@ -19,6 +19,7 @@ const { getConnection } = require("@urbackend/common"); const { getCompiledModel } = require("@urbackend/common"); const { QueryEngine } = require("@urbackend/common"); const { storageRegistry } = require("@urbackend/common"); +const { AppError } = require("@urbackend/common"); const { deleteProjectByApiKeyCache, setProjectById, @@ -1219,20 +1220,20 @@ module.exports.deleteAllFiles = async (req, res) => { } }; -module.exports.requestUpload = async (req, res) => { +module.exports.requestUpload = async (req, res, next) => { try { const { projectId } = req.params; const { filename, contentType, size } = req.body; const numericSize = parsePositiveSize(size); if (!filename || !contentType || numericSize === null) { - return res - .status(400) - .json({ error: "filename, contentType, and size are required." }); + return next( + new AppError(400, "filename, contentType, and size are required."), + ); } if (numericSize > MAX_FILE_SIZE) { - return res.status(413).json({ error: "File size exceeds limit." }); + return next(new AppError(413, "File size exceeds limit.")); } const project = await Project.findOne({ @@ -1242,7 +1243,7 @@ module.exports.requestUpload = async (req, res) => { "+resources.storage.config.encrypted +resources.storage.config.iv +resources.storage.config.tag resources.storage.isExternal storageUsed storageLimit", ); - if (!project) return res.status(404).json({ error: "Project not found" }); + if (!project) return next(new AppError(404, "Project not found")); const external = isProjectStorageExternal(project); @@ -1256,7 +1257,7 @@ module.exports.requestUpload = async (req, res) => { storageLimit === -1 ? SAFETY_MAX_BYTES : storageLimit; if ((project.storageUsed || 0) + numericSize > quotaLimit) { - return res.status(403).json({ error: "Internal storage limit exceeded." }); + return next(new AppError(403, "Internal storage limit exceeded.")); } } @@ -1270,23 +1271,25 @@ module.exports.requestUpload = async (req, res) => { numericSize, ); - return res.status(200).json({ signedUrl, token, filePath }); - } catch (err) { - return res.status(500).json({ - error: "Could not generate upload URL", - details: process.env.NODE_ENV === "development" ? err.message : undefined, + return res.status(200).json({ + success: true, + data: { signedUrl, token, filePath }, + message: "Upload URL generated successfully.", }); + } catch (err) { + if (err instanceof AppError) return next(err); + return next(new AppError(500, "Could not generate upload URL")); } }; -module.exports.confirmUpload = async (req, res) => { +module.exports.confirmUpload = async (req, res, next) => { try { const { projectId } = req.params; const { filePath, size } = req.body; const declaredSize = parsePositiveSize(size); if (!filePath || declaredSize === null) { - return res.status(400).json({ error: "filePath and size are required." }); + return next(new AppError(400, "filePath and size are required.")); } const project = await Project.findOne({ @@ -1296,14 +1299,14 @@ module.exports.confirmUpload = async (req, res) => { "+resources.storage.config.encrypted +resources.storage.config.iv +resources.storage.config.tag resources.storage.isExternal storageUsed storageLimit", ); - if (!project) return res.status(404).json({ error: "Project not found" }); + if (!project) return next(new AppError(404, "Project not found")); const external = isProjectStorageExternal(project); const normalizedPath = normalizeProjectPath(projectId, filePath); // make sure client isn't confirming someone else's file if (!normalizedPath) { - return res.status(403).json({ error: "Access denied." }); + return next(new AppError(403, "Access denied.")); } // verify file actually exists on cloud before touching quota @@ -1313,30 +1316,26 @@ module.exports.confirmUpload = async (req, res) => { } catch (err) { if (err?.message === "File not found after upload") { await bestEffortDeleteUploadedObject(project, normalizedPath); - return res.status(409).json({ - error: "UPLOAD_NOT_READY", - message: "Uploaded file is not visible yet. Please retry confirmation.", - }); + return next( + new AppError( + 409, + "Uploaded file is not visible yet. Please retry confirmation.", + ), + ); } throw err; } if (!Number.isFinite(actualSize) || actualSize <= 0) { await bestEffortDeleteUploadedObject(project, normalizedPath); - return res.status(500).json({ - error: "Upload confirmation failed", - details: - process.env.NODE_ENV === "development" - ? "Uploaded file size could not be determined" - : undefined, - }); + return next(new AppError(500, "Uploaded file size could not be determined")); } if (Math.abs(actualSize - declaredSize) > CONFIRM_UPLOAD_SIZE_TOLERANCE_BYTES) { await bestEffortDeleteUploadedObject(project, normalizedPath); - return res - .status(400) - .json({ error: "Declared file size does not match uploaded file size." }); + return next( + new AppError(400, "Declared file size does not match uploaded file size."), + ); } // now it's safe to charge quota @@ -1354,36 +1353,39 @@ module.exports.confirmUpload = async (req, res) => { if (result.matchedCount === 0) { await bestEffortDeleteUploadedObject(project, normalizedPath); - return res.status(403).json({ error: "Internal storage limit exceeded." }); + return next(new AppError(403, "Internal storage limit exceeded.")); } } const supabase = await getStorage(project); const bucket = getBucket(project); - const { data: publicUrlData } = supabase.storage + const { data: publicUrlData, error: apiError } = supabase.storage .from(bucket) .getPublicUrl(normalizedPath); - const response = { + const publicUrl = publicUrlData?.publicUrl; + const provider = publicUrl ? (external ? "external" : "internal") : "external"; + + const responseData = { message: "Upload confirmed", path: normalizedPath, - provider: external ? "external" : "internal", + provider, + url: publicUrl ?? null, }; - if (publicUrlData?.publicUrl) { - response.url = publicUrlData.publicUrl; - } else { - response.url = null; - response.warning = - publicUrlData?.error || "Upload confirmed, but a public URL is unavailable."; + if (!publicUrl) { + responseData.warning = + apiError || "Upload confirmed, but a public URL is unavailable."; } - return res.status(200).json(response); - } catch (err) { - return res.status(500).json({ - error: "Upload confirmation failed", - details: process.env.NODE_ENV === "development" ? err.message : undefined, + return res.status(200).json({ + success: true, + data: responseData, + message: "Upload confirmed.", }); + } catch (err) { + if (err instanceof AppError) return next(err); + return next(new AppError(500, "Upload confirmation failed")); } }; diff --git a/docs-legacy/storage.md b/docs-legacy/storage.md index 8ee98905..2dd5cd39 100644 --- a/docs-legacy/storage.md +++ b/docs-legacy/storage.md @@ -73,10 +73,10 @@ For the newer upload flow, file bytes are uploaded directly from the browser to Typical flow: -1. Call backend `upload-request` endpoint with `filename`, `contentType`, and `size`. +1. Call backend `POST /api/storage/upload-request` with `filename`, `contentType`, and `size`. 2. Receive `signedUrl` and `filePath`. 3. Browser uploads file to `signedUrl` using `PUT`. -4. Call backend `upload-confirm` endpoint to verify file existence and charge quota. +4. Call backend `POST /api/storage/upload-confirm` to verify file existence and charge quota. This avoids proxying file bytes through Node.js and keeps server memory usage predictable. @@ -96,4 +96,6 @@ Required headers should include at least: - `content-type` - `content-length` +Browser `PUT` requests to `signedUrl` should send the file body and include the correct `Content-Type`, and `Content-Length` when your client/runtime allows explicitly setting it. + If preflight CORS is missing or restrictive, browser uploads will fail even when the signed URL is valid. diff --git a/mintlify/docs/guides/storage.mdx b/mintlify/docs/guides/storage.mdx index 2c9f4f37..3f66d4dc 100644 --- a/mintlify/docs/guides/storage.mdx +++ b/mintlify/docs/guides/storage.mdx @@ -51,7 +51,7 @@ Uploads use a presigned URL three-step flow so the binary is sent directly to st ### Step 2 — Upload the binary to storage -Send a `PUT` request to `signedUrl` with the raw file contents and the correct `Content-Type` header. +Send a browser `PUT` request to `signedUrl` with the raw file contents and the correct `Content-Type` header. ```javascript const uploadRes = await fetch(signedUrl, { @@ -181,6 +181,24 @@ console.log(url, provider, path, warning); SDK users (`urbackend-sdk`) do not need to change anything — `client.storage.upload()` uses this flow internally. +## Required Bucket CORS (S3/R2) + +If you use AWS S3 or Cloudflare R2 with presigned browser uploads, configure bucket CORS so browser uploads to `signedUrl` can pass preflight. + +Required methods: + +- `PUT` +- `OPTIONS` +- `GET` +- `HEAD` + +Required headers should include at least: + +- `content-type` +- `content-length` + +Without these CORS rules, browser uploads can fail even when `POST /api/storage/upload-request` and `POST /api/storage/upload-confirm` are correct. + ## Delete a file To delete a file, pass the `path` returned from the upload response. @@ -225,6 +243,6 @@ If the path is invalid or the file has already been removed, the API returns `40 - If you use an external S3/R2 bucket, configure its CORS policy to allow `PUT` requests from your client origin. The presigned URL flow requires this bucket-level setting. + If you use an external S3/R2 bucket, configure bucket CORS to allow `PUT`, `OPTIONS`, `GET`, and `HEAD`, and allow `content-type` and `content-length` headers from your client origin. From 55c0a5e2dd75f8c425403f9e096f699ceb53afd0 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Yadav Date: Sun, 26 Apr 2026 14:17:51 +0530 Subject: [PATCH 3/5] feat(storage): migrate dashboard upload to presigned flow --- .../__tests__/routes.projects.storage.test.js | 9 +- .../storage.presigned.controller.test.js | 59 ++++++++----- .../src/controllers/project.controller.js | 84 +++++-------------- apps/dashboard-api/src/routes/projects.js | 8 -- apps/web-dashboard/src/pages/Storage.jsx | 37 ++++++-- packages/common/src/index.js | 4 + packages/common/src/utils/input.validation.js | 21 +++++ 7 files changed, 124 insertions(+), 98 deletions(-) diff --git a/apps/dashboard-api/src/__tests__/routes.projects.storage.test.js b/apps/dashboard-api/src/__tests__/routes.projects.storage.test.js index bc478a2d..014b9752 100644 --- a/apps/dashboard-api/src/__tests__/routes.projects.storage.test.js +++ b/apps/dashboard-api/src/__tests__/routes.projects.storage.test.js @@ -42,7 +42,6 @@ jest.mock('../controllers/project.controller', () => { deleteRow: jest.fn(ok), insertData: jest.fn(ok), editRow: jest.fn(ok), - uploadFile: jest.fn(ok), listFiles: jest.fn(ok), deleteFile: jest.fn(ok), deleteAllFiles: jest.fn(ok), @@ -84,6 +83,14 @@ beforeEach(() => { }); describe('projects storage presigned routes', () => { + test('legacy proxy upload route is removed', async () => { + const res = await request(app) + .post('/api/projects/project1/storage/upload') + .send({}); + + expect(res.status).toBe(404); + }); + test('upload-request route is wired and protected', async () => { const res = await request(app) .post('/api/projects/project1/storage/upload-request') diff --git a/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js b/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js index 53f2e167..54145c8b 100644 --- a/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js +++ b/apps/dashboard-api/src/__tests__/storage.presigned.controller.test.js @@ -38,6 +38,18 @@ jest.mock('@urbackend/common', () => { verifyUploadedFile: jest.fn(), isProjectStorageExternal: jest.fn(), getBucket: jest.fn(() => 'dev-files'), + sanitizeObjectId: jest.fn((value) => { + if (typeof value !== 'string') return null; + const normalized = value.trim(); + return /^[a-fA-F0-9]{24}$/.test(normalized) ? normalized : null; + }), + sanitizeNonEmptyString: jest.fn((value, options = {}) => { + if (typeof value !== 'string') return null; + const normalized = value.trim(); + if (!normalized) return null; + if (normalized.length > (options.maxLength || 1024)) return null; + return normalized; + }), AppError, __mockStorageFrom: mockStorageFrom, }; @@ -52,6 +64,7 @@ const { } = require('@urbackend/common'); const controller = require('../controllers/project.controller'); +const PROJECT_ID = '507f1f77bcf86cd799439011'; const makeRes = () => { const res = { status: jest.fn(), json: jest.fn() }; @@ -63,7 +76,7 @@ const makeRes = () => { const makeNext = () => jest.fn(); const makeProject = (overrides = {}) => ({ - _id: 'project1', + _id: PROJECT_ID, owner: 'dev1', storageUsed: 100, storageLimit: 1024 * 1024, @@ -86,7 +99,7 @@ describe('dashboard project.controller presigned upload', () => { describe('requestUpload', () => { test('returns 400 for invalid input', async () => { const req = { - params: { projectId: 'project1' }, + params: { projectId: PROJECT_ID }, body: { filename: 'a.txt', contentType: 'text/plain', size: 'abc' }, user: { _id: 'dev1' }, }; @@ -97,7 +110,7 @@ describe('dashboard project.controller presigned upload', () => { expect(next).toHaveBeenCalledWith(expect.objectContaining({ statusCode: 400, - message: 'filename, contentType, and size are required.', + message: 'projectId, filename, contentType, and size are required.', })); }); @@ -106,7 +119,7 @@ describe('dashboard project.controller presigned upload', () => { isProjectStorageExternal.mockReturnValue(false); const req = { - params: { projectId: 'project1' }, + params: { projectId: PROJECT_ID }, body: { filename: 'a.txt', contentType: 'text/plain', size: 200 }, user: { _id: 'dev1' }, }; @@ -130,7 +143,7 @@ describe('dashboard project.controller presigned upload', () => { }); const req = { - params: { projectId: 'project1' }, + params: { projectId: PROJECT_ID }, body: { filename: 'my file.txt', contentType: 'text/plain', size: 1234 }, user: { _id: 'dev1' }, }; @@ -140,8 +153,8 @@ describe('dashboard project.controller presigned upload', () => { await controller.requestUpload(req, res, next); expect(getPresignedUploadUrl).toHaveBeenCalledWith( - expect.objectContaining({ _id: 'project1' }), - 'project1/mocked-uuid_my_file.txt', + expect.objectContaining({ _id: PROJECT_ID }), + `${PROJECT_ID}/mocked-uuid_my_file.txt`, 'text/plain', 1234, ); @@ -151,7 +164,7 @@ describe('dashboard project.controller presigned upload', () => { data: { signedUrl: 'https://signed.example/upload', token: 't1', - filePath: 'project1/mocked-uuid_my_file.txt', + filePath: `${PROJECT_ID}/mocked-uuid_my_file.txt`, }, message: 'Upload URL generated successfully.', }); @@ -165,8 +178,8 @@ describe('dashboard project.controller presigned upload', () => { isProjectStorageExternal.mockReturnValue(false); const req = { - params: { projectId: 'project1' }, - body: { filePath: 'project2/file.txt', size: 200 }, + params: { projectId: PROJECT_ID }, + body: { filePath: '507f1f77bcf86cd799439012/file.txt', size: 200 }, user: { _id: 'dev1' }, }; const res = makeRes(); @@ -187,8 +200,8 @@ describe('dashboard project.controller presigned upload', () => { mockStorageFrom.remove.mockResolvedValue({ data: null, error: null }); const req = { - params: { projectId: 'project1' }, - body: { filePath: 'project1/file.txt', size: 200 }, + params: { projectId: PROJECT_ID }, + body: { filePath: `${PROJECT_ID}/file.txt`, size: 200 }, user: { _id: 'dev1' }, }; const res = makeRes(); @@ -196,7 +209,7 @@ describe('dashboard project.controller presigned upload', () => { await controller.confirmUpload(req, res, next); - expect(mockStorageFrom.remove).toHaveBeenCalledWith(['project1/file.txt']); + expect(mockStorageFrom.remove).toHaveBeenCalledWith([`${PROJECT_ID}/file.txt`]); expect(next).toHaveBeenCalledWith(expect.objectContaining({ statusCode: 409, message: 'Uploaded file is not visible yet. Please retry confirmation.', @@ -210,8 +223,8 @@ describe('dashboard project.controller presigned upload', () => { mockStorageFrom.remove.mockResolvedValue({ data: null, error: null }); const req = { - params: { projectId: 'project1' }, - body: { filePath: 'project1/file.txt', size: 900 }, + params: { projectId: PROJECT_ID }, + body: { filePath: `${PROJECT_ID}/file.txt`, size: 900 }, user: { _id: 'dev1' }, }; const res = makeRes(); @@ -223,7 +236,7 @@ describe('dashboard project.controller presigned upload', () => { statusCode: 400, message: 'Declared file size does not match uploaded file size.', })); - expect(mockStorageFrom.remove).toHaveBeenCalledWith(['project1/file.txt']); + expect(mockStorageFrom.remove).toHaveBeenCalledWith([`${PROJECT_ID}/file.txt`]); }); test('charges quota and returns success on internal storage', async () => { @@ -234,8 +247,8 @@ describe('dashboard project.controller presigned upload', () => { mockStorageFrom.getPublicUrl.mockReturnValue({ data: { publicUrl: 'https://cdn.example/p/project1/file.txt' } }); const req = { - params: { projectId: 'project1' }, - body: { filePath: 'project1/file.txt', size: 1024 }, + params: { projectId: PROJECT_ID }, + body: { filePath: `${PROJECT_ID}/file.txt`, size: 1024 }, user: { _id: 'dev1' }, }; const res = makeRes(); @@ -245,7 +258,7 @@ describe('dashboard project.controller presigned upload', () => { expect(Project.updateOne).toHaveBeenCalledWith( { - _id: 'project1', + _id: PROJECT_ID, $or: [ { storageLimit: -1 }, { $expr: { $lte: [{ $add: ['$storageUsed', 1024] }, '$storageLimit'] } }, @@ -258,7 +271,7 @@ describe('dashboard project.controller presigned upload', () => { success: true, data: { message: 'Upload confirmed', - path: 'project1/file.txt', + path: `${PROJECT_ID}/file.txt`, provider: 'internal', url: 'https://cdn.example/p/project1/file.txt', }, @@ -274,8 +287,8 @@ describe('dashboard project.controller presigned upload', () => { mockStorageFrom.getPublicUrl.mockReturnValue({ data: { publicUrl: null } }); const req = { - params: { projectId: 'project1' }, - body: { filePath: 'project1/file.txt', size: 512 }, + params: { projectId: PROJECT_ID }, + body: { filePath: `${PROJECT_ID}/file.txt`, size: 512 }, user: { _id: 'dev1' }, }; const res = makeRes(); @@ -289,7 +302,7 @@ describe('dashboard project.controller presigned upload', () => { success: true, data: { message: 'Upload confirmed', - path: 'project1/file.txt', + path: `${PROJECT_ID}/file.txt`, provider: 'external', url: null, warning: 'Upload confirmed, but a public URL is unavailable.', diff --git a/apps/dashboard-api/src/controllers/project.controller.js b/apps/dashboard-api/src/controllers/project.controller.js index 5b63b386..e7d9b2c2 100644 --- a/apps/dashboard-api/src/controllers/project.controller.js +++ b/apps/dashboard-api/src/controllers/project.controller.js @@ -9,6 +9,8 @@ const { createCollectionSchema, updateExternalConfigSchema, updateAuthProvidersSchema, + sanitizeObjectId, + sanitizeNonEmptyString, } = require("@urbackend/common"); const { generateApiKey, hashApiKey } = require("@urbackend/common"); const { z } = require("zod"); @@ -253,7 +255,7 @@ module.exports.createProject = async (req, res) => { if (req.projectLimit !== undefined) { const currentCount = await Project.countDocuments( { owner: req.user._id }, - { session } + { session }, ); if (currentCount >= req.projectLimit) { @@ -301,6 +303,7 @@ module.exports.createProject = async (req, res) => { if (err instanceof z.ZodError) { return res.status(400).json({ error: err.issues }); } + res.status(500).json({ error: err.message }); } }; @@ -723,7 +726,6 @@ module.exports.createCollection = async (req, res) => { } catch (err) { await session.abortTransaction(); session.endSession(); - try { if (connection && compiledCollectionName) { clearCompiledModel(connection, compiledCollectionName); @@ -1077,55 +1079,6 @@ module.exports.listFiles = async (req, res) => { } }; -module.exports.uploadFile = async (req, res) => { - try { - const { projectId } = req.params; - const file = req.file; - - if (!file) return res.status(400).json({ error: "No file uploaded" }); - - const project = await Project.findOne({ - _id: projectId, - owner: req.user._id, - }).select( - "+resources.storage.config.encrypted +resources.storage.config.iv +resources.storage.config.tag resources.storage.isExternal storageUsed storageLimit", - ); - if (!project) return res.status(404).json({ error: "Project not found" }); - - const external = isProjectStorageExternal(project); - - if (!external) { - if (project.storageUsed + file.size > project.storageLimit) { - return res.status(403).json({ error: "Storage limit exceeded" }); - } - } - - const supabase = await getStorage(project); - const bucket = getBucket(project); - - const safeName = file.originalname.replace(/\s+/g, "_"); - const path = `${projectId}/${randomUUID()}_${safeName}`; - - const { error } = await supabase.storage - .from(bucket) - .upload(path, file.buffer, { - contentType: file.mimetype, - upsert: false, - }); - - if (error) throw error; - - if (!external) { - project.storageUsed += file.size; - await project.save(); - } - - res.json({ success: true, path }); - } catch (err) { - res.status(500).json({ error: err }); - } -}; - module.exports.deleteFile = async (req, res) => { try { const { projectId } = req.params; @@ -1222,13 +1175,19 @@ module.exports.deleteAllFiles = async (req, res) => { module.exports.requestUpload = async (req, res, next) => { try { - const { projectId } = req.params; + const projectId = sanitizeObjectId(req.params?.projectId); const { filename, contentType, size } = req.body; + const sanitizedFilename = sanitizeNonEmptyString(filename, { + maxLength: 255, + }); + const sanitizedContentType = sanitizeNonEmptyString(contentType, { + maxLength: 255, + }); const numericSize = parsePositiveSize(size); - if (!filename || !contentType || numericSize === null) { + if (!projectId || !sanitizedFilename || !sanitizedContentType || numericSize === null) { return next( - new AppError(400, "filename, contentType, and size are required."), + new AppError(400, "projectId, filename, contentType, and size are required."), ); } @@ -1247,7 +1206,7 @@ module.exports.requestUpload = async (req, res, next) => { const external = isProjectStorageExternal(project); - // just peek at quota - don't charge yet, upload hasn't happened + // Pre-check quota only; actual storage usage is charged after confirmUpload verifies object existence and size. if (!external) { const storageLimit = typeof project.storageLimit === "number" @@ -1261,13 +1220,13 @@ module.exports.requestUpload = async (req, res, next) => { } } - const safeName = filename.replace(/\s+/g, "_"); + const safeName = sanitizedFilename.replace(/\s+/g, "_"); const filePath = `${projectId}/${randomUUID()}_${safeName}`; const { signedUrl, token } = await getPresignedUploadUrl( project, filePath, - contentType, + sanitizedContentType, numericSize, ); @@ -1284,12 +1243,15 @@ module.exports.requestUpload = async (req, res, next) => { module.exports.confirmUpload = async (req, res, next) => { try { - const { projectId } = req.params; + const projectId = sanitizeObjectId(req.params?.projectId); const { filePath, size } = req.body; + const sanitizedFilePath = sanitizeNonEmptyString(filePath, { + maxLength: 1024, + }); const declaredSize = parsePositiveSize(size); - if (!filePath || declaredSize === null) { - return next(new AppError(400, "filePath and size are required.")); + if (!projectId || !sanitizedFilePath || declaredSize === null) { + return next(new AppError(400, "projectId, filePath, and size are required.")); } const project = await Project.findOne({ @@ -1302,7 +1264,7 @@ module.exports.confirmUpload = async (req, res, next) => { if (!project) return next(new AppError(404, "Project not found")); const external = isProjectStorageExternal(project); - const normalizedPath = normalizeProjectPath(projectId, filePath); + const normalizedPath = normalizeProjectPath(projectId, sanitizedFilePath); // make sure client isn't confirming someone else's file if (!normalizedPath) { diff --git a/apps/dashboard-api/src/routes/projects.js b/apps/dashboard-api/src/routes/projects.js index beac0bbc..dbcae35d 100644 --- a/apps/dashboard-api/src/routes/projects.js +++ b/apps/dashboard-api/src/routes/projects.js @@ -3,8 +3,6 @@ const router = express.Router(); const authMiddleware = require('../middlewares/authMiddleware'); const { attachDeveloper, checkProjectLimit, checkCollectionLimit, checkByokGate } = require('../middlewares/planEnforcement'); const { verifyEmail, checkAuthEnabled, loadProjectForAdmin } = require('@urbackend/common'); -const multer = require('multer'); -const storage = multer.memoryStorage(); const { createProject, @@ -17,7 +15,6 @@ const { deleteRow, insertData, editRow, - uploadFile, listFiles, deleteFile, deleteAllFiles, @@ -43,8 +40,6 @@ const { const { createAdminUser, resetPassword, getUserDetails, updateAdminUser, listUserSessions, revokeUserSession } = require('../controllers/userAuth.controller'); -const upload = multer({ storage: storage, limits: { fileSize: 10 * 1024 * 1024 } }); // 10MB Limit - // POST REQ FOR CREATE PROJECT router.post('/', authMiddleware, attachDeveloper, verifyEmail, checkProjectLimit, createProject); @@ -76,9 +71,6 @@ router.patch('/:projectId/collections/:collectionName/data/:id', authMiddleware, // GET REQ FOR FILES router.get('/:projectId/storage/files', authMiddleware, listFiles); -// POST REQ FOR UPLOAD FILE -router.post('/:projectId/storage/upload', authMiddleware, verifyEmail, upload.single('file'), uploadFile); - // POST REQ FOR DELETE FILE router.post('/:projectId/storage/delete', authMiddleware, verifyEmail, deleteFile); diff --git a/apps/web-dashboard/src/pages/Storage.jsx b/apps/web-dashboard/src/pages/Storage.jsx index 61c303fb..afbf50ba 100644 --- a/apps/web-dashboard/src/pages/Storage.jsx +++ b/apps/web-dashboard/src/pages/Storage.jsx @@ -39,18 +39,45 @@ export default function Storage() { toast.error("Account Verification Required. Please verify in Settings."); return; } - const formData = new FormData(); - formData.append('file', file); + setUploading(true); const toastId = toast.loading("Uploading..."); + try { - await api.post(`/api/projects/${projectId}/storage/upload`, formData, { - headers: { 'Content-Type': 'multipart/form-data' } + const requestRes = await api.post(`/api/projects/${projectId}/storage/upload-request`, { + filename: file.name, + contentType: file.type || 'application/octet-stream', + size: file.size, + }); + + const signedUrl = requestRes?.data?.data?.signedUrl; + const filePath = requestRes?.data?.data?.filePath; + + if (!signedUrl || !filePath) { + throw new Error('Could not get upload URL'); + } + + const uploadRes = await fetch(signedUrl, { + method: 'PUT', + headers: { + 'Content-Type': file.type || 'application/octet-stream', + }, + body: file, }); + + if (!uploadRes.ok) { + throw new Error('Direct upload failed'); + } + + await api.post(`/api/projects/${projectId}/storage/upload-confirm`, { + filePath, + size: file.size, + }); + toast.success("File uploaded!", { id: toastId }); fetchFiles(); } catch (err) { - toast.error(err.response?.data?.error || "Upload failed", { id: toastId }); + toast.error(err.response?.data?.message || err.message || "Upload failed", { id: toastId }); } finally { setUploading(false); if (fileInputRef.current) fileInputRef.current.value = ''; diff --git a/packages/common/src/index.js b/packages/common/src/index.js index 3c420201..640e34fb 100644 --- a/packages/common/src/index.js +++ b/packages/common/src/index.js @@ -66,6 +66,8 @@ const { createWebhookSchema, updateWebhookSchema, sendMailSchema, + sanitizeObjectId, + sanitizeNonEmptyString, } = require("./utils/input.validation"); const { garbageCollect, storageGarbageCollect } = require("./utils/GC"); const { generateApiKey, hashApiKey } = require("./utils/api"); @@ -130,6 +132,8 @@ module.exports = { createWebhookSchema, updateWebhookSchema, sendMailSchema, + sanitizeObjectId, + sanitizeNonEmptyString, garbageCollect, storageGarbageCollect, generateApiKey, diff --git a/packages/common/src/utils/input.validation.js b/packages/common/src/utils/input.validation.js index a61b307e..f25ee716 100644 --- a/packages/common/src/utils/input.validation.js +++ b/packages/common/src/utils/input.validation.js @@ -1,4 +1,5 @@ const z = require("zod"); +const mongoose = require("mongoose"); const { MAX_FIELD_DEPTH, UNIQUE_SUPPORTED_TYPES, @@ -294,6 +295,26 @@ module.exports.sanitize = (obj) => { return clean; }; +module.exports.sanitizeObjectId = (value) => { + if (typeof value !== "string") return null; + const normalized = value.trim(); + if (!normalized) return null; + return mongoose.Types.ObjectId.isValid(normalized) ? normalized : null; +}; + +module.exports.sanitizeNonEmptyString = (value, options = {}) => { + if (typeof value !== "string") return null; + + const { maxLength = 1024, allowNullByte = false } = options; + const normalized = value.trim(); + + if (!normalized) return null; + if (normalized.length > maxLength) return null; + if (!allowNullByte && normalized.includes("\0")) return null; + + return normalized; +}; + const emptyToUndefined = z.preprocess( (val) => (val === "" || val === null ? undefined : val), z.string().optional(), From 43c0fd2ef6e290260f4c4db24e68ddaf0efc05b0 Mon Sep 17 00:00:00 2001 From: "mintlify[bot]" <109931778+mintlify[bot]@users.noreply.github.com> Date: Sun, 26 Apr 2026 08:55:42 +0000 Subject: [PATCH 4/5] docs: use sentence case for bucket CORS heading Generated-By: mintlify-agent --- mintlify/docs/guides/storage.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mintlify/docs/guides/storage.mdx b/mintlify/docs/guides/storage.mdx index 3f66d4dc..bdb6cd33 100644 --- a/mintlify/docs/guides/storage.mdx +++ b/mintlify/docs/guides/storage.mdx @@ -181,7 +181,7 @@ console.log(url, provider, path, warning); SDK users (`urbackend-sdk`) do not need to change anything — `client.storage.upload()` uses this flow internally. -## Required Bucket CORS (S3/R2) +## Required bucket CORS (S3/R2) If you use AWS S3 or Cloudflare R2 with presigned browser uploads, configure bucket CORS so browser uploads to `signedUrl` can pass preflight. From e9481f67b024fd78d4871a7c719f9585178b4dca Mon Sep 17 00:00:00 2001 From: "mintlify[bot]" <109931778+mintlify[bot]@users.noreply.github.com> Date: Sun, 26 Apr 2026 08:58:36 +0000 Subject: [PATCH 5/5] docs: clarify CORS AllowedHeaders for presigned uploads Generated-By: mintlify-agent --- mintlify/docs/guides/storage.mdx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mintlify/docs/guides/storage.mdx b/mintlify/docs/guides/storage.mdx index bdb6cd33..09532911 100644 --- a/mintlify/docs/guides/storage.mdx +++ b/mintlify/docs/guides/storage.mdx @@ -192,10 +192,10 @@ Required methods: - `GET` - `HEAD` -Required headers should include at least: +Required `AllowedHeaders` (not `ExposeHeaders`) should include at least: -- `content-type` -- `content-length` +- `content-type` — required for presigned `PUT` requests +- `content-length` — typically safelisted, but some providers still require it to be allow-listed Without these CORS rules, browser uploads can fail even when `POST /api/storage/upload-request` and `POST /api/storage/upload-confirm` are correct.