From 84c6a68eaecb6cbd4b5908cbb85a57aa67deac9b Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 8 Jul 2026 10:03:32 +0200 Subject: [PATCH] [web] enforce size limit on individual files when cloning project (#35211) * [web] enforce size limit on individual files when cloning project Co-authored-by: Brian Gough * [web] improve test coverage * [agents] speed up running frontend tests * [web] gracefully handle project too large error --------- Co-authored-by: Brian Gough GitOrigin-RevId: c7b35a3a9688e15a9bb51c61816da82d39e0d29d --- .../history-v1/api/controllers/projects.js | 5 + services/history-v1/api/schema.js | 1 + .../acceptance/js/api/project_blobs.test.js | 64 ++++++++++ .../src/Features/History/HistoryManager.mjs | 30 +++-- .../Features/Project/ProjectController.mjs | 12 ++ .../Features/Project/ProjectDuplicator.mjs | 5 +- .../web/frontend/extracted-translations.json | 1 + .../clone-project-modal-content.tsx | 40 ++++--- services/web/locales/en.json | 1 + .../components/clone-project-modal.test.tsx | 46 ++++++++ .../HistoryManager.sequential.test.mjs | 109 ++++++++++++++++++ .../src/Project/ProjectController.test.mjs | 54 +++++++++ .../src/Project/ProjectDuplicator.test.mjs | 19 ++- 13 files changed, 365 insertions(+), 22 deletions(-) diff --git a/services/history-v1/api/controllers/projects.js b/services/history-v1/api/controllers/projects.js index 2b79beffc6..cc22326a56 100644 --- a/services/history-v1/api/controllers/projects.js +++ b/services/history-v1/api/controllers/projects.js @@ -492,6 +492,11 @@ async function copyProjectBlob(req, res, next) { ) return render.notFound(res) } + if (query.sizeLimit > 0 && sourceBlob.getByteLength() > query.sizeLimit) { + return res.status(HTTPStatus.REQUEST_ENTITY_TOO_LARGE).json({ + size: sourceBlob.getByteLength(), + }) + } // Exit early if the blob exists in the target project. // This will also catch global blobs, which always exist. if (targetBlob) { diff --git a/services/history-v1/api/schema.js b/services/history-v1/api/schema.js index 797a68c96f..a150a16705 100644 --- a/services/history-v1/api/schema.js +++ b/services/history-v1/api/schema.js @@ -143,6 +143,7 @@ const schemas = { }), query: z.object({ copyFrom: z.string(), + sizeLimit: z.coerce.number().optional(), }), body: z.any().optional(), }), diff --git a/services/history-v1/test/acceptance/js/api/project_blobs.test.js b/services/history-v1/test/acceptance/js/api/project_blobs.test.js index b325d11428..b9de19a7f7 100644 --- a/services/history-v1/test/acceptance/js/api/project_blobs.test.js +++ b/services/history-v1/test/acceptance/js/api/project_blobs.test.js @@ -298,6 +298,70 @@ describe('Project blobs API', function () { expect(newBlobResponseText).to.equal(fileContents.toString()) }) + it('rejects copying a blob that exceeds the size limit', async function () { + const targetProjectId = '456' + const targetClient = + await testServer.createClientForProject(targetProjectId) + const targetToken = testServer.createTokenForProject(targetProjectId) + const url = new URL( + testServer.url( + `/api/projects/${targetProjectId}/blobs/${testFiles.HELLO_TXT_HASH}` + ) + ) + url.searchParams.append('copyFrom', projectId) + url.searchParams.append( + 'sizeLimit', + (testFiles.HELLO_TXT_BYTE_LENGTH - 1).toString() + ) + + const response = await fetch(url, { + method: 'POST', + headers: { Authorization: `Bearer ${targetToken}` }, + }) + expect(response.status).to.equal(HTTPStatus.REQUEST_ENTITY_TOO_LARGE) + const body = await response.json() + expect(body.size).to.equal(testFiles.HELLO_TXT_BYTE_LENGTH) + + // check that it did not copy the blob to the target project + await expectHttpError( + targetClient.apis.Project.getProjectBlob({ + project_id: targetProjectId, + hash: testFiles.HELLO_TXT_HASH, + }), + HTTPStatus.NOT_FOUND + ) + }) + + it('copies a blob that is within the size limit', async function () { + const targetProjectId = '456' + const targetClient = + await testServer.createClientForProject(targetProjectId) + const targetToken = testServer.createTokenForProject(targetProjectId) + const url = new URL( + testServer.url( + `/api/projects/${targetProjectId}/blobs/${testFiles.HELLO_TXT_HASH}` + ) + ) + url.searchParams.append('copyFrom', projectId) + url.searchParams.append( + 'sizeLimit', + testFiles.HELLO_TXT_BYTE_LENGTH.toString() + ) + + const response = await fetch(url, { + method: 'POST', + headers: { Authorization: `Bearer ${targetToken}` }, + }) + expect(response.status).to.equal(HTTPStatus.CREATED) + + const newBlobResponse = await targetClient.apis.Project.getProjectBlob({ + project_id: targetProjectId, + hash: testFiles.HELLO_TXT_HASH, + }) + const newBlobResponseText = await newBlobResponse.data.text() + expect(newBlobResponseText).to.equal(fileContents.toString()) + }) + it('skips copying a blob to another project if it already exists', async function () { const targetProjectId = '456' const targetClient = diff --git a/services/web/app/src/Features/History/HistoryManager.mjs b/services/web/app/src/Features/History/HistoryManager.mjs index 0e95daa197..7ebe7625f8 100644 --- a/services/web/app/src/Features/History/HistoryManager.mjs +++ b/services/web/app/src/Features/History/HistoryManager.mjs @@ -14,7 +14,11 @@ import ProjectGetter from '../Project/ProjectGetter.mjs' import HistoryBackupDeletionHandler from './HistoryBackupDeletionHandler.mjs' import { db, waitForDb } from '../../infrastructure/mongodb.mjs' import Metrics from '@overleaf/metrics' -import { NotFoundError, HistoryResyncPendingError } from '../Errors/Errors.js' +import { + NotFoundError, + HistoryResyncPendingError, + FileTooLargeError, +} from '../Errors/Errors.js' const HISTORY_V1_URL = settings.apis.v1_history.url const HISTORY_V1_BASIC_AUTH = { @@ -173,15 +177,27 @@ async function uploadBlobFromDisk(historyId, hash, byteLength, fsPath) { }) } -async function copyBlob(sourceHistoryId, targetHistoryId, hash) { - const url = `${HISTORY_V1_URL}/projects/${targetHistoryId}/blobs/${hash}` - await fetchNothing( - `${url}?${new URLSearchParams({ copyFrom: sourceHistoryId })}`, - { +async function copyBlob(sourceHistoryId, targetHistoryId, hash, sizeLimit) { + const url = new URL( + `${HISTORY_V1_URL}/projects/${targetHistoryId}/blobs/${hash}` + ) + url.searchParams.set('copyFrom', sourceHistoryId) + if (sizeLimit) url.searchParams.set('sizeLimit', sizeLimit) + try { + await fetchNothing(url, { method: 'POST', basicAuth: HISTORY_V1_BASIC_AUTH, + }) + } catch (err) { + if (err instanceof RequestFailedError && err.response.status === 413) { + let size + try { + ;({ size } = JSON.parse(err.body)) + } catch {} + throw new FileTooLargeError('file too large', { size }) } - ) + throw err + } } async function requestBlobWithProjectId( diff --git a/services/web/app/src/Features/Project/ProjectController.mjs b/services/web/app/src/Features/Project/ProjectController.mjs index d677673358..69b0a4b123 100644 --- a/services/web/app/src/Features/Project/ProjectController.mjs +++ b/services/web/app/src/Features/Project/ProjectController.mjs @@ -57,6 +57,7 @@ import UserSettingsHelper from './UserSettingsHelper.mjs' import AiFeatureUsageRateLimiter from '../../infrastructure/rate-limiters/AiFeatureUsageRateLimiter.mjs' import WorkbenchRateLimiter from '../../infrastructure/rate-limiters/WorkbenchRateLimiter.mjs' import PermissionsManager from '../Authorization/PermissionsManager.mjs' +import { FileTooLargeError } from '../Errors/Errors.js' const { checkUserPermissions } = PermissionsManager.promises const { isPaidSubscription } = SubscriptionHelper @@ -309,6 +310,17 @@ const _ProjectController = { projectId, userId: currentUser._id, }) + const { path, size } = OError.getFullInfo(err) + if (err instanceof FileTooLargeError && path) { + res.status(413).json({ + message: { + text: 'file too large to copy', + key: 'file_too_large_to_copy', + info: { path, size }, + }, + }) + return + } return next(err) } }, diff --git a/services/web/app/src/Features/Project/ProjectDuplicator.mjs b/services/web/app/src/Features/Project/ProjectDuplicator.mjs index 8735fb961a..ad4ae76cea 100644 --- a/services/web/app/src/Features/Project/ProjectDuplicator.mjs +++ b/services/web/app/src/Features/Project/ProjectDuplicator.mjs @@ -1,6 +1,7 @@ import { callbackify } from 'node:util' import Path from 'node:path' import logger from '@overleaf/logger' +import Settings from '@overleaf/settings' import OError from '@overleaf/o-error' import { promiseMapWithLimit } from '@overleaf/promise-utils' import { Doc } from '../../models/Doc.mjs' @@ -332,11 +333,13 @@ async function _copyFiles( await HistoryManager.promises.copyBlob( sourceHistoryId, targetHistoryId, - file.hash + file.hash, + Settings.maxUploadSize ) return { createdBlob: true, file, path } } catch (err) { throw OError.tag(err, 'unexpected error copying blob', { + path: path.replace(/^\//, ''), sourceProjectId: sourceProject._id, targetProjectId: targetProject._id, sourceFile, diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 2b5318e23c..8711a16ba5 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -696,6 +696,7 @@ "file_or_folder_name_already_exists": "", "file_outline": "", "file_size": "", + "file_too_large_to_copy": "", "file_tree": "", "files_cannot_include_invalid_characters": "", "files_selected": "", diff --git a/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.tsx b/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.tsx index d4b9d185bf..ecfac180cc 100644 --- a/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.tsx +++ b/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.tsx @@ -1,7 +1,7 @@ /* eslint-disable jsx-a11y/no-autofocus */ import { FormEvent, useCallback, useMemo, useState } from 'react' -import { useTranslation } from 'react-i18next' -import { postJSON } from '../../../infrastructure/fetch-json' +import { Trans, useTranslation } from 'react-i18next' +import { postJSON, FetchError } from '@/infrastructure/fetch-json.ts' import { CloneProjectTag } from './clone-project-tag' import { OLModalBody, @@ -16,6 +16,7 @@ import OLFormControl from '@/shared/components/ol/ol-form-control' import OLFormLabel from '@/shared/components/ol/ol-form-label' import OLButton from '@/shared/components/ol/ol-button' import { Tag } from '../../../../../app/src/Features/Tags/types' +import getMeta from '@/utils/meta.ts' export default function CloneProjectModalContent({ handleHide, @@ -35,8 +36,9 @@ export default function CloneProjectModalContent({ projectTags: Tag[] }) { const { t } = useTranslation() + const { maxUploadSize } = getMeta('ol-ExposedSettings') - const [error, setError] = useState() + const [error, setError] = useState(null) const [clonedProjectName, setClonedProjectName] = useState( `${projectName} (Copy)` ) @@ -57,7 +59,7 @@ export default function CloneProjectModalContent({ return } - setError(false) + setError(null) setInFlight(true) // clone the project @@ -71,12 +73,8 @@ export default function CloneProjectModalContent({ // open the cloned project handleAfterCloned(data, clonedProjectTags) }) - .catch(({ response, data }) => { - if (response?.status === 400) { - setError(data.message) - } else { - setError(true) - } + .catch(err => { + setError(err) }) .finally(() => { setInFlight(false) @@ -128,9 +126,25 @@ export default function CloneProjectModalContent({ {error && ( ]} + i18nKey="file_too_large_to_copy" + values={{ + size: (error.data.message.info.size / 1024 / 1024).toFixed( + 1 + ), + limit: Math.floor(maxUploadSize / 1024 / 1024), + path: error.data.message.info.path, + }} + shouldUnescape + tOptions={{ interpolation: { escapeValue: true } }} + > + ) : error.response?.status === 400 ? ( + error.data.message + ) : ( + t('generic_something_went_wrong') + ) } type="error" /> diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 4b09dc9620..9558632221 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -904,6 +904,7 @@ "file_outline": "File outline", "file_size": "File size", "file_too_large": "File too large", + "file_too_large_to_copy": "File <0>__path__ (__size__MiB) exceeds the maximum file size of __limit__MiB.", "file_tree": "File tree", "files_cannot_include_invalid_characters": "File name is empty or contains invalid characters", "files_selected": "files selected.", diff --git a/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.tsx b/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.tsx index 96e7be6258..9d7743f636 100644 --- a/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.tsx +++ b/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.tsx @@ -199,4 +199,50 @@ describe('', function () { expect(button.disabled).to.be.false expect(cancelButton.disabled).to.be.false }) + + it('handles a file too large error response', async function () { + const matcher = 'express:/project/:projectId/clone' + + // default test maxUploadSize is 5 MiB (see reset-meta helper) + fetchMock.postOnce(matcher, { + status: 413, + body: { + message: { + text: 'file too large to copy', + key: 'file_too_large_to_copy', + info: { path: 'huge.pdf', size: 8 * 1024 * 1024 }, + }, + }, + }) + + const handleHide = sinon.stub() + const openProject = sinon.stub() + + renderWithEditorContext( + , + contextProps + ) + + const button: HTMLButtonElement = await screen.findByRole('button', { + name: 'Copy', + }) + + fireEvent.click(button) + await fetchMock.callHistory.flush(true) + + expect(fetchMock.callHistory.done(matcher)).to.be.true + expect(openProject).not.to.be.called + + // the path is rendered inside a element, interpolated with the + // file size and the configured upload limit + const code = await screen.findByText('huge.pdf') + expect(code.tagName).to.equal('CODE') + expect(code.parentElement?.textContent).to.equal( + 'File huge.pdf (8.0MiB) exceeds the maximum file size of 5MiB.' + ) + }) }) diff --git a/services/web/test/unit/src/History/HistoryManager.sequential.test.mjs b/services/web/test/unit/src/History/HistoryManager.sequential.test.mjs index d527a1251a..f3cef76fd0 100644 --- a/services/web/test/unit/src/History/HistoryManager.sequential.test.mjs +++ b/services/web/test/unit/src/History/HistoryManager.sequential.test.mjs @@ -1,6 +1,8 @@ import { beforeAll, beforeEach, describe, it, vi, expect } from 'vitest' import sinon from 'sinon' import mongodb from 'mongodb-legacy' +import { RequestFailedError } from '@overleaf/fetch-utils' +import { FileTooLargeError } from '../../../../app/src/Features/Errors/Errors.js' import { cleanupTestDatabase, db, @@ -9,6 +11,12 @@ import { const { ObjectId } = mongodb +// Ensure the module under test and this test file share the same +// FileTooLargeError class, so `instanceof` checks line up. +vi.mock('../../../../app/src/Features/Errors/Errors.js', () => + vi.importActual('../../../../app/src/Features/Errors/Errors.js') +) + const MODULE_PATH = '../../../../app/src/Features/History/HistoryManager' const GLOBAL_BLOBS = [ @@ -40,6 +48,7 @@ describe('HistoryManager', function () { ctx.FetchUtils = { fetchJson: sinon.stub(), fetchNothing: sinon.stub().resolves(), + RequestFailedError, } ctx.projectHistoryUrl = 'http://project_history.example.com' ctx.v1HistoryUrl = 'http://v1_history.example.com' @@ -364,6 +373,106 @@ describe('HistoryManager', function () { }) }) + describe('copyBlob', function () { + const sourceHistoryId = 'source-history-id' + const targetHistoryId = 'target-history-id' + const hash = 'abcdef0123456789abcdef0123456789abcdef01' + + it('should call the v1-history service without a size limit', async function (ctx) { + await ctx.HistoryManager.promises.copyBlob( + sourceHistoryId, + targetHistoryId, + hash + ) + expect(ctx.FetchUtils.fetchNothing).to.have.been.calledOnce + const [url, opts] = ctx.FetchUtils.fetchNothing.getCall(0).args + expect(url.toString()).to.equal( + `${ctx.v1HistoryUrl}/projects/${targetHistoryId}/blobs/${hash}?copyFrom=${sourceHistoryId}` + ) + expect(opts).to.deep.equal({ + method: 'POST', + basicAuth: { + user: ctx.v1HistoryUser, + password: ctx.v1HistoryPassword, + }, + }) + }) + + it('should pass the size limit as a query parameter', async function (ctx) { + await ctx.HistoryManager.promises.copyBlob( + sourceHistoryId, + targetHistoryId, + hash, + 1234 + ) + const [url] = ctx.FetchUtils.fetchNothing.getCall(0).args + expect(url.searchParams.get('copyFrom')).to.equal(sourceHistoryId) + expect(url.searchParams.get('sizeLimit')).to.equal('1234') + }) + + it('should throw a FileTooLargeError with the size on a 413 response', async function (ctx) { + ctx.FetchUtils.fetchNothing.rejects( + new RequestFailedError( + `${ctx.v1HistoryUrl}/projects/${targetHistoryId}/blobs/${hash}`, + { method: 'POST' }, + { status: 413 }, + JSON.stringify({ size: 9000 }) + ) + ) + await expect( + ctx.HistoryManager.promises.copyBlob( + sourceHistoryId, + targetHistoryId, + hash, + 1234 + ) + ).to.be.rejected.then(err => { + expect(err).to.be.instanceof(FileTooLargeError) + expect(err.info.size).to.equal(9000) + }) + }) + + it('should throw a FileTooLargeError even if the 413 body is not valid JSON', async function (ctx) { + ctx.FetchUtils.fetchNothing.rejects( + new RequestFailedError( + `${ctx.v1HistoryUrl}/projects/${targetHistoryId}/blobs/${hash}`, + { method: 'POST' }, + { status: 413 }, + 'not json' + ) + ) + await expect( + ctx.HistoryManager.promises.copyBlob( + sourceHistoryId, + targetHistoryId, + hash, + 1234 + ) + ).to.be.rejected.then(err => { + expect(err).to.be.instanceof(FileTooLargeError) + expect(err.info.size).to.be.undefined + }) + }) + + it('should rethrow non-413 request failures', async function (ctx) { + const err = new RequestFailedError( + `${ctx.v1HistoryUrl}/projects/${targetHistoryId}/blobs/${hash}`, + { method: 'POST' }, + { status: 500 }, + '' + ) + ctx.FetchUtils.fetchNothing.rejects(err) + await expect( + ctx.HistoryManager.promises.copyBlob( + sourceHistoryId, + targetHistoryId, + hash, + 1234 + ) + ).to.be.rejectedWith(err) + }) + }) + describe('deleteProject', function () { const projectId = new ObjectId() const historyId = new ObjectId() diff --git a/services/web/test/unit/src/Project/ProjectController.test.mjs b/services/web/test/unit/src/Project/ProjectController.test.mjs index b7f1e2b970..2a5afa99c2 100644 --- a/services/web/test/unit/src/Project/ProjectController.test.mjs +++ b/services/web/test/unit/src/Project/ProjectController.test.mjs @@ -3,8 +3,15 @@ import { beforeEach, describe, it, vi, expect } from 'vitest' import path from 'node:path' import sinon from 'sinon' import mongodb from 'mongodb-legacy' +import { FileTooLargeError } from '../../../../app/src/Features/Errors/Errors.js' const { ObjectId } = mongodb +// Ensure the module under test and this test file share the same +// FileTooLargeError class, so `instanceof` checks in the controller line up. +vi.mock('../../../../app/src/Features/Errors/Errors.js', () => + vi.importActual('../../../../app/src/Features/Errors/Errors.js') +) + const MODULE_PATH = path.join( import.meta.dirname, '../../../../app/src/Features/Project/ProjectController' @@ -769,6 +776,53 @@ describe('ProjectController', function () { ctx.ProjectController.cloneProject(ctx.req, ctx.res) }) }) + + it('should respond with a 413 when a file is too large to copy', async function (ctx) { + const err = new FileTooLargeError('file too large', { + path: 'huge.pdf', + size: 123456789, + }) + ctx.ProjectDuplicator.promises.duplicate = sinon.stub().rejects(err) + await new Promise(resolve => { + ctx.res.status = sinon.stub().returns(ctx.res) + ctx.res.json = json => { + ctx.res.status.should.have.been.calledWith(413) + expect(json).to.deep.equal({ + message: { + text: 'file too large to copy', + key: 'file_too_large_to_copy', + info: { path: 'huge.pdf', size: 123456789 }, + }, + }) + resolve() + } + ctx.ProjectController.cloneProject(ctx.req, ctx.res) + }) + }) + + it('should forward a FileTooLargeError without a path to next (e.g. project too large)', async function (ctx) { + const err = new FileTooLargeError('Project is too large') + ctx.ProjectDuplicator.promises.duplicate = sinon.stub().rejects(err) + await new Promise(resolve => { + const next = forwardedErr => { + expect(forwardedErr).to.equal(err) + resolve() + } + ctx.ProjectController.cloneProject(ctx.req, ctx.res, next) + }) + }) + + it('should forward other errors to next', async function (ctx) { + const err = new Error('boom') + ctx.ProjectDuplicator.promises.duplicate = sinon.stub().rejects(err) + await new Promise(resolve => { + const next = forwardedErr => { + expect(forwardedErr).to.equal(err) + resolve() + } + ctx.ProjectController.cloneProject(ctx.req, ctx.res, next) + }) + }) }) describe('newProject', function () { diff --git a/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs b/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs index 1fe237c5ac..79dc114ea7 100644 --- a/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs +++ b/services/web/test/unit/src/Project/ProjectDuplicator.test.mjs @@ -1,6 +1,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import sinon from 'sinon' import mongodb from 'mongodb-legacy' +import OError from '@overleaf/o-error' +import Settings from '@overleaf/settings' const { ObjectId } = mongodb @@ -340,7 +342,8 @@ describe('ProjectDuplicator', function () { ctx.HistoryManager.promises.copyBlob.should.have.been.calledWith( ctx.project.overleaf.history.id, ctx.newProject.overleaf.history.id, - file.hash + file.hash, + Settings.maxUploadSize ) } }) @@ -447,6 +450,20 @@ describe('ProjectDuplicator', function () { ) ).to.be.rejectedWith('copy blob error') }) + + it('should tag the error with the file path', async function (ctx) { + ctx.file0.hash = '500' + await expect( + ctx.ProjectDuplicator.promises.duplicate( + ctx.owner, + ctx.project._id, + 'name' + ) + ).to.be.rejected.then(err => { + // path is tagged without the leading slash + expect(OError.getFullInfo(err).path).to.equal('file0') + }) + }) }) describe('when there is an error', function () {