chore: cleanup uploaded tmp files

app/app.js

@@ -19,6 +19,7 @@ const DataConnection = require('./src/db/dataConnection');
 const dataConnection = new DataConnection();
 const { eventStreamService } = require('./src/components/eventStreamService');
 const clamAvScanner = require('./src/components/clamAvScanner');
+const uploadCleanup = require('./src/forms/file/uploadCleanup');
 
 const statusService = require('./src/components/statusService');
 statusService.registerConnection('dataConnection', 'Database', dataConnection, 'checkAll', 'checkConnection');
@@ -56,6 +57,10 @@ if (process.env.NODE_ENV !== 'test') {
   // Initialize connections and wait for completion
   statusService.initializeAllConnections();
   app.use(httpLogger);
+
+  if (config.has('files.uploads.cleanup')) {
+    uploadCleanup.startUploadCleanupScheduler(config.get('files.uploads.cleanup'));
+  }
 }
 
 const statusPath = `${config.get('server.basePath')}${config.get('server.apiPath')}/status`;
@@ -203,6 +208,7 @@ function cleanup() {
 
   log.info('Cleaning up...', { function: 'cleanup' });
   clearInterval(probeId);
+  uploadCleanup.stopUploadCleanupScheduler();
 
   eventStreamService.closeConnection();
   dataConnection.close(() => process.exit());

app/config/custom-environment-variables.json

@@ -14,7 +14,13 @@
       "fileKey": "FILES_UPLOADS_FILEKEY",
       "fileMaxSize": "FILES_UPLOADS_FILEMAXSIZE",
       "fileMinSize": "FILES_UPLOADS_FILEMINSIZE",
-      "path": "FILES_UPLOADS_PATH"
+      "path": "FILES_UPLOADS_PATH",
+      "cleanup": {
+        "enabled": "FILES_UPLOADS_CLEANUP_ENABLED",
+        "staleAgeMinutes": "FILES_UPLOADS_CLEANUP_STALEAGEMINUTES",
+        "intervalMinutes": "FILES_UPLOADS_CLEANUP_INTERVALMINUTES",
+        "batchSize": "FILES_UPLOADS_CLEANUP_BATCHSIZE"
+      }
     },
     "permanent": "FILES_PERMANENT",
     "localStorage": {

app/config/default.json

@@ -13,7 +13,13 @@
       "fileMaxSize": "25MB",
       "fileMaxSizeBytes": "25000000",
       "fileMinSize": "0KB",
-      "path": "files"
+      "path": "files",
+      "cleanup": {
+        "enabled": true,
+        "staleAgeMinutes": 1440,
+        "intervalMinutes": 60,
+        "batchSize": 100
+      }
     },
     "permanent": "objectStorage",
     "localStorage": {

app/src/forms/file/middleware/upload.js

@@ -8,7 +8,20 @@
 
 const Problem = require('api-problem');
 
-let fileUploadsDir = os.tmpdir();
+// Multer stages uploads in a dedicated subdirectory of the OS temp dir (rather
+// than the shared temp root) so the upload cleanup sweeper can safely target
+// only CHEFS upload temp files and never touch unrelated files in /tmp.
+const UPLOADS_SUBDIR = 'chefs-uploads';
+
+// The default staging directory: a dedicated subdir of the OS temp dir. Shared
+// by fileSetup() and getFileUploadsDir() so the resolved path is identical
+// whether it is read before or after init().
+const defaultUploadsDir = () => path.join(fs.realpathSync(os.tmpdir()), UPLOADS_SUBDIR);
+
+// Resolved lazily by getFileUploadsDir() (or explicitly by fileSetup via init()).
+// Left unset so a read before init() still returns the correct default rather
+// than the bare OS temp dir.
+let fileUploadsDir;
 let maxFileSize = bytes.parse('25MB');
 let maxFileCount = 1;
 
@@ -61,7 +74,7 @@
 };
 
 const fileSetup = (options) => {
-  fileUploadsDir = (options && options.dir) || process.env.FILE_UPLOADS_DIR || fs.realpathSync(os.tmpdir());
+  fileUploadsDir = (options && options.dir) || process.env.FILE_UPLOADS_DIR || defaultUploadsDir();
   try {
     fs.ensureDirSync(fileUploadsDir);
   } catch (error) {
@@ -130,6 +143,11 @@
    * @returns the file uploads directory.
    */
   getFileUploadsDir() {
+    // Fall back to the default if read before init() so callers never receive
+    // the bare OS temp dir (which the sweeper would not be scoped to).
+    if (!fileUploadsDir) {
+      fileUploadsDir = defaultUploadsDir();
+    }
     return fileUploadsDir;
   },
 

app/src/forms/file/middleware/virusScan.js

@@ -1,45 +1,26 @@
-const fs = require('fs').promises;
-const path = require('path');
-const os = require('os');
 const Problem = require('api-problem');
 const clamAvScanner = require('../../../components/clamAvScanner');
 const log = require('../../../components/log')(module.filename);
+const { fileUpload } = require('./upload');
+const uploadCleanup = require('../uploadCleanup');
 
 /**
  * Validates that a file path is within a specified base directory.
  * @param {string} filePath - The file path to validate.
- * @param {string} [baseDirectory=os.tmpdir()] - The base directory to restrict file operations.
+ * @param {string} [baseDirectory] - The base directory to restrict file operations.
  * @returns {string} - The resolved and validated file path.
  * @throws {Error} - Throws an error if the file path is outside the base directory.
  */
-const validateFilePath = (filePath, baseDirectory = os.tmpdir()) => {
-  // Resolve the absolute path of the base directory
-  const resolvedBaseDir = path.resolve(baseDirectory);
-
-  // Resolve the absolute path of the file
-  const resolvedFilePath = path.resolve(resolvedBaseDir, filePath);
-
-  // Ensure the resolved file path is within the base directory
-  if (!resolvedFilePath.startsWith(resolvedBaseDir)) {
-    throw new Error(`Invalid file path: ${filePath} is outside the allowed directory.`);
-  }
-
-  return resolvedFilePath;
+const validateFilePath = (filePath, baseDirectory) => {
+  return uploadCleanup.resolveUploadPath(filePath, baseDirectory || fileUpload.getFileUploadsDir());
 };
 
 /**
  * Removes an infected file from the filesystem.
  * @param {string} filePath - The path of the file to remove.
  */
 const removeInfected = async (filePath) => {
-  try {
-    if (validateFilePath(filePath)) {
-      await fs.unlink(filePath);
-      log.info(`Deleted infected file: ${filePath}`);
-    }
-  } catch (error) {
-    log.error(`Could not delete infected file: ${filePath}. ${error.message}`);
-  }
+  await uploadCleanup.removeUploadedFile(filePath, 'infected');
 };
 
 /**
@@ -66,21 +47,22 @@
     return next();
   }
 
+  const { path: filePath, originalname: fileName } = req.file;
+
   try {
-    const { path: filePath, originalname: fileName } = req.file;
     const scanResult = await clamAvScanner.scanFile(filePath);
 
     log.info(`${fileName} scanned. Is infected? ${scanResult.isInfected}. Viruses: ${scanResult.viruses || 'None'}`);
 
     if (scanResult.isInfected) {
-      const validatedPath = validateFilePath(filePath);
-      await removeInfected(validatedPath);
+      await removeInfected(filePath);
       return next(createVirusProblem(fileName, scanResult.viruses));
     }
 
     next();
   } catch (error) {
-    log.error(`Error scanning file: ${req.file?.originalname || 'unknown'}. ${error.message}`);
+    log.error(`Error scanning file: ${fileName || 'unknown'}. ${error.message}`);
+    await uploadCleanup.removeUploadedFile(filePath, 'scan-error');
     next(error);
   }
 };

app/src/forms/file/service.js

@@ -2,8 +2,10 @@ const config = require('config');
 const uuid = require('uuid');
 const path = require('path');
 const { FileStorage } = require('../common/models');
+const { StorageTypes } = require('../common/constants');
 const log = require('../../components/log')(module.filename);
 const storageService = require('./storage/storageService');
+const uploadCleanup = require('./uploadCleanup');
 
 const PERMANENT_STORAGE = config.get('files.permanent');
 
@@ -58,14 +60,23 @@ const validateFileSecurity = (file) => {
 
 const service = {
   create: async (data, currentUser, folder = 'uploads') => {
-    validateFileSecurity(data);
-
+    const tempPath = data?.path;
     let trx;
+    let uploadResult;
+    let obj;
+    // Once the transaction commits, the file record and its stored object are
+    // durable and reference each other. Anything that fails after this point
+    // (e.g. the read-back below) must NOT trigger compensation, or we would roll
+    // back an already-committed transaction and delete the object that a
+    // committed row still points at - orphaning the record.
+    let committed = false;
 
     try {
+      validateFileSecurity(data);
+
       trx = await FileStorage.startTransaction();
 
-      const obj = {};
+      obj = {};
       obj.id = uuid.v4();
       obj.storage = folder;
       obj.originalName = data.originalname;
@@ -74,21 +85,38 @@ const service = {
       obj.path = data.path;
       obj.createdBy = currentUser?.usernameIdp || 'public';
 
-      const uploadResult = await storageService.upload(obj);
+      uploadResult = await storageService.upload(obj);
       obj.path = uploadResult.path;
       obj.storage = uploadResult.storage;
-      // console.log('INSERTING FILE', {
-      //   storage: obj.storage,
-      //   path: obj.path,
-      //   folder,
-      // });
       await FileStorage.query(trx).insert(obj);
 
       await trx.commit();
+      committed = true;
+
+      if (uploadResult.storage === StorageTypes.OBJECT_STORAGE) {
+        await uploadCleanup.removeUploadedFile(tempPath, 'object-storage-upload-success');
+      }
+
       const result = await service.read(obj.id);
       return result;
     } catch (err) {
-      if (trx) await trx.rollback();
+      // Compensation only applies while the work is still undoable. After a
+      // successful commit the object is legitimately referenced (and for local
+      // storage the temp file IS that object), so leave everything in place.
+      if (!committed) {
+        if (trx) await trx.rollback();
+
+        if (uploadResult && obj?.id) {
+          await service.deleteStorageObject({
+            id: obj.id,
+            path: uploadResult.path,
+            storage: uploadResult.storage,
+          });
+        }
+
+        await uploadCleanup.removeUploadedFile(tempPath, 'create-failure');
+      }
+
       throw err;
     }
   },