Fix temp file leaks in dumpFolder on both success and error paths (#31304)

GitOrigin-RevId: b5d8586a86030fb887cbd7dadd9bbb7cd64a788c

services/web/app/src/Features/History/RestoreManager.mjs

@@ -1,5 +1,6 @@
 import Settings from '@overleaf/settings'
 import Path from 'node:path'
+import fs from 'node:fs'
 import FileWriter from '../../infrastructure/FileWriter.mjs'
 import Metrics from '../../infrastructure/Metrics.mjs'
 import FileSystemImportManager from '../Uploads/FileSystemImportManager.mjs'
@@ -34,30 +35,34 @@ const RestoreManager = {
       version,
       pathname
     )
-    const basename = Path.basename(pathname)
-    let dirname = Path.dirname(pathname)
-    if (dirname === '.') {
-      // no directory
-      dirname = ''
-    }
-    const parentFolderId = await RestoreManager._findOrCreateFolder(
-      projectId,
-      dirname,
-      userId
-    )
-    const addEntityWithName = async name =>
-      await FileSystemImportManager.promises.addEntity(
-        userId,
+    try {
+      const basename = Path.basename(pathname)
+      let dirname = Path.dirname(pathname)
+      if (dirname === '.') {
+        // no directory
+        dirname = ''
+      }
+      const parentFolderId = await RestoreManager._findOrCreateFolder(
         projectId,
-        parentFolderId,
-        name,
-        fsPath,
-        false
+        dirname,
+        userId
       )
-    return await RestoreManager._addEntityWithUniqueName(
-      addEntityWithName,
-      basename
-    )
+      const addEntityWithName = async name =>
+        await FileSystemImportManager.promises.addEntity(
+          userId,
+          projectId,
+          parentFolderId,
+          name,
+          fsPath,
+          false
+        )
+      return await RestoreManager._addEntityWithUniqueName(
+        addEntityWithName,
+        basename
+      )
+    } finally {
+      await fs.promises.unlink(fsPath).catch(() => {})
+    }
   },
 
   async revertFile(userId, projectId, version, pathname, options = {}) {
@@ -188,20 +193,24 @@ const RestoreManager = {
         historyId,
         snapshotFile
       )
-      const newFile = await EditorController.promises.upsertFile(
-        projectId,
-        parentFolderId,
-        basename,
-        fsPath,
-        fileMetadata,
-        options.origin,
-        userId
-      )
+      try {
+        const newFile = await EditorController.promises.upsertFile(
+          projectId,
+          parentFolderId,
+          basename,
+          fsPath,
+          fileMetadata,
+          options.origin,
+          userId
+        )
 
-      endTimer({ type: 'file' })
-      return {
-        _id: newFile._id,
-        type: 'file',
+        endTimer({ type: 'file' })
+        return {
+          _id: newFile._id,
+          type: 'file',
+        }
+      } finally {
+        await fs.promises.unlink(fsPath).catch(() => {})
       }
     }
 

services/web/app/src/Features/LinkedFiles/LinkedFilesHandler.mjs

@@ -1,3 +1,4 @@
+import fs from 'node:fs'
 import FileWriter from '../../infrastructure/FileWriter.mjs'
 import EditorController from '../Editor/EditorController.mjs'
 import ProjectLocator from '../Project/ProjectLocator.mjs'
@@ -63,15 +64,19 @@ const LinkedFilesHandler = {
       readStream
     )
 
-    return await EditorController.promises.upsertFile(
-      projectId,
-      parentFolderId,
-      name,
-      fsPath,
-      linkedFileData,
-      'upload',
-      userId
-    )
+    try {
+      return await EditorController.promises.upsertFile(
+        projectId,
+        parentFolderId,
+        name,
+        fsPath,
+        linkedFileData,
+        'upload',
+        userId
+      )
+    } finally {
+      await fs.promises.unlink(fsPath).catch(() => {})
+    }
   },
 
   async importContent(
@@ -87,15 +92,19 @@ const LinkedFilesHandler = {
       content
     )
 
-    return await EditorController.promises.upsertFile(
-      projectId,
-      parentFolderId,
-      name,
-      fsPath,
-      linkedFileData,
-      'upload',
-      userId
-    )
+    try {
+      return await EditorController.promises.upsertFile(
+        projectId,
+        parentFolderId,
+        name,
+        fsPath,
+        linkedFileData,
+        'upload',
+        userId
+      )
+    } finally {
+      await fs.promises.unlink(fsPath).catch(() => {})
+    }
   },
 }
 

services/web/test/unit/src/History/RestoreManager.test.mjs

@@ -263,6 +263,7 @@ describe('RestoreManager', function () {
 
   afterEach(function () {
     tk.reset()
+    vi.resetModules()
   })
 
   describe('restoreFileFromV2', function () {
@@ -349,6 +350,35 @@ describe('RestoreManager', function () {
           .should.equal(true)
       })
     })
+
+    describe('when addEntity throws an error', function () {
+      beforeEach(function (ctx) {
+        ctx.pathname = 'foo.tex'
+        ctx.FileSystemImportManager.promises.addEntity = sinon
+          .stub()
+          .rejects(new Error('Failed to add entity'))
+        ctx.fsUnlink = sinon.stub().resolves()
+        vi.doMock('node:fs', () => ({
+          default: { promises: { unlink: ctx.fsUnlink } },
+        }))
+      })
+
+      it('should clean up the temporary file and propagate the error', async function (ctx) {
+        let error
+        try {
+          await ctx.RestoreManager.promises.restoreFileFromV2(
+            ctx.user_id,
+            ctx.project_id,
+            ctx.version,
+            ctx.pathname
+          )
+        } catch (err) {
+          error = err
+        }
+        expect(error).to.exist
+        expect(error.message).to.equal('Failed to add entity')
+      })
+    })
   })
 
   describe('_findOrCreateFolder', function () {