fix: improve & unify attachment handling (#9095)

* fix: improve & unify attachment handling

* fix: attachment column mapping for public insert

* fix: PR requested changes

packages/nocodb/src/controllers/attachments-secure.controller.ts

@@ -14,8 +14,6 @@ import {
   UseGuards,
   UseInterceptors,
 } from '@nestjs/common';
-import hash from 'object-hash';
-import moment from 'moment';
 import { AnyFilesInterceptor } from '@nestjs/platform-express';
 import { Response } from 'express';
 import type { AttachmentReqType, FileType } from 'nocodb-sdk';
@@ -47,11 +45,8 @@ export class AttachmentsSecureController {
     @UploadedFiles() files: Array<FileType>,
     @Req() req: NcRequest & { user: { id: string } },
   ) {
-    const path = `${moment().format('YYYY/MM/DD')}/${hash(req.user.id)}`;
-
     const attachments = await this.attachmentsService.upload({
       files: files,
-      path: path,
       req,
     });
 
@@ -66,11 +61,8 @@ export class AttachmentsSecureController {
     @Body() body: Array<AttachmentReqType>,
     @Req() req: NcRequest & { user: { id: string } },
   ) {
-    const path = `${moment().format('YYYY/MM/DD')}/${hash(req.user.id)}`;
-
     const attachments = await this.attachmentsService.uploadViaURL({
       urls: body,
-      path,
       req,
     });
 

packages/nocodb/src/db/BaseModelSqlv2.ts

@@ -770,7 +770,7 @@ class BaseModelSqlv2 {
       column_name: string;
       filterArr?: Filter[];
     }[],
-    view: View,
+    _view: View,
   ) {
     try {
       const columns = await this.model.getColumns(this.context);
@@ -1072,7 +1072,7 @@ class BaseModelSqlv2 {
       filterArr?: Filter[];
       sortArr?: Sort[];
     }[],
-    view: View,
+    _view: View,
   ) {
     const columns = await this.model.getColumns(this.context);
     const aliasColObjMap = await this.model.getAliasColObjMap(
@@ -7916,18 +7916,11 @@ class BaseModelSqlv2 {
               if (Array.isArray(attachment)) {
                 for (const lookedUpAttachment of attachment) {
                   if (lookedUpAttachment?.path) {
-                    let relativePath = lookedUpAttachment.path.replace(
-                      /^download\//,
-                      '',
-                    );
-
                     promises.push(
-                      PresignedUrl.getSignedUrl({
+                      PresignedUrl.signAttachment({
-                        pathOrUrl: relativePath,
+                        attachment: lookedUpAttachment,
-                        preview: true,
-                        mimetype: lookedUpAttachment.mimetype,
                         filename: lookedUpAttachment.title,
-                      }).then((r) => (lookedUpAttachment.signedPath = r)),
+                      }),
                     );
 
                     if (!lookedUpAttachment.mimetype?.startsWith('image/')) {
@@ -7940,58 +7933,39 @@ class BaseModelSqlv2 {
                       card_cover: {},
                     };
 
-                    relativePath = `thumbnails/${relativePath}`;
+                    const thumbnailPath = `thumbnails/${lookedUpAttachment.path.replace(
-
+                      /^download\//,
-                    promises.push(
+                      '',
-                      PresignedUrl.getSignedUrl({
+                    )}`;
-                        pathOrUrl: `${relativePath}/tiny.jpg`,
+
-                        preview: true,
+                    for (const key of Object.keys(
-                        mimetype: 'image/jpeg',
+                      lookedUpAttachment.thumbnails,
-                        filename: lookedUpAttachment.title,
+                    )) {
-                      }).then(
+                      promises.push(
-                        (r) =>
+                        PresignedUrl.signAttachment({
-                          (lookedUpAttachment.thumbnails.tiny.signedPath = r),
+                          attachment: {
-                      ),
+                            ...lookedUpAttachment,
-                    );
+                            path: `${thumbnailPath}/${key}.jpg`,
-                    promises.push(
+                          },
-                      PresignedUrl.getSignedUrl({
+                          filename: lookedUpAttachment.title,
-                        pathOrUrl: `${relativePath}/small.jpg`,
+                          mimetype: 'image/jpeg',
-                        preview: true,
+                          nestedKeys: ['thumbnails', key],
-                        mimetype: 'image/jpeg',
+                        }),
-                        filename: lookedUpAttachment.title,
+                      );
-                      }).then(
+                    }
-                        (r) =>
-                          (lookedUpAttachment.thumbnails.small.signedPath = r),
-                      ),
-                    );
-                    promises.push(
-                      PresignedUrl.getSignedUrl({
-                        pathOrUrl: `${relativePath}/card_cover.jpg`,
-                        preview: true,
-                        mimetype: 'image/jpeg',
-                        filename: lookedUpAttachment.title,
-                      }).then(
-                        (r) =>
-                          (lookedUpAttachment.thumbnails.card_cover.signedPath =
-                            r),
-                      ),
-                    );
                   } else if (lookedUpAttachment?.url) {
-                    let relativePath = lookedUpAttachment.url;
                     promises.push(
-                      PresignedUrl.getSignedUrl({
+                      PresignedUrl.signAttachment({
-                        pathOrUrl: relativePath,
+                        attachment: lookedUpAttachment,
-                        preview: true,
-                        mimetype: lookedUpAttachment.mimetype,
                         filename: lookedUpAttachment.title,
-                      }).then((r) => (lookedUpAttachment.signedUrl = r)),
+                      }),
                     );
 
                     if (!lookedUpAttachment.mimetype?.startsWith('image/')) {
                       continue;
                     }
 
-                    relativePath = relativePath.replace(
+                    const thumbnailUrl = lookedUpAttachment.url.replace(
                       'nc/uploads',
                       'nc/thumbnails',
                     );
@@ -8002,59 +7976,40 @@ class BaseModelSqlv2 {
                       card_cover: {},
                     };
 
-                    promises.push(
+                    for (const key of Object.keys(
-                      PresignedUrl.getSignedUrl({
+                      lookedUpAttachment.thumbnails,
-                        pathOrUrl: `${relativePath}/tiny.jpg`,
+                    )) {
-                        preview: true,
+                      promises.push(
-                        mimetype: 'image/jpeg',
+                        PresignedUrl.signAttachment({
-                        filename: lookedUpAttachment.title,
+                          attachment: {
-                      }).then(
+                            ...lookedUpAttachment,
-                        (r) =>
+                            url: `${thumbnailUrl}/${key}.jpg`,
-                          (lookedUpAttachment.thumbnails.tiny.signedUrl = r),
+                          },
-                      ),
+                          filename: lookedUpAttachment.title,
-                    );
+                          mimetype: 'image/jpeg',
-                    promises.push(
+                          nestedKeys: ['thumbnails', key],
-                      PresignedUrl.getSignedUrl({
+                        }),
-                        pathOrUrl: `${relativePath}/small.jpg`,
+                      );
-                        preview: true,
+                    }
-                        mimetype: 'image/jpeg',
-                        filename: lookedUpAttachment.title,
-                      }).then(
-                        (r) =>
-                          (lookedUpAttachment.thumbnails.small.signedUrl = r),
-                      ),
-                    );
-                    promises.push(
-                      PresignedUrl.getSignedUrl({
-                        pathOrUrl: `${relativePath}/card_cover.jpg`,
-                        preview: true,
-                        mimetype: 'image/jpeg',
-                        filename: lookedUpAttachment.title,
-                      }).then(
-                        (r) =>
-                          (lookedUpAttachment.thumbnails.card_cover.signedUrl =
-                            r),
-                      ),
-                    );
                   }
                 }
               } else {
                 if (attachment?.path) {
-                  let relativePath = attachment.path.replace(/^download\//, '');
-
                   promises.push(
-                    PresignedUrl.getSignedUrl({
+                    PresignedUrl.signAttachment({
-                      pathOrUrl: relativePath,
+                      attachment,
-                      preview: true,
-                      mimetype: attachment.mimetype,
                       filename: attachment.title,
-                    }).then((r) => (attachment.signedPath = r)),
+                    }),
                   );
+
                   if (!attachment.mimetype?.startsWith('image/')) {
                     continue;
                   }
 
-                  relativePath = `thumbnails/${relativePath}`;
+                  const thumbnailPath = `thumbnails/${attachment.path.replace(
+                    /^download\//,
+                    '',
+                  )}`;
 
                   attachment.thumbnails = {
                     tiny: {},
@@ -8062,82 +8017,51 @@ class BaseModelSqlv2 {
                     card_cover: {},
                   };
 
-                  promises.push(
+                  for (const key of Object.keys(attachment.thumbnails)) {
-                    PresignedUrl.getSignedUrl({
+                    promises.push(
-                      pathOrUrl: `${relativePath}/tiny.jpg`,
+                      PresignedUrl.signAttachment({
-                      preview: true,
+                        attachment: {
-                      mimetype: 'image/jpeg',
+                          ...attachment,
-                      filename: attachment.title,
+                          path: `${thumbnailPath}/${key}.jpg`,
-                    }).then((r) => (attachment.thumbnails.tiny.signedPath = r)),
+                        },
-                  );
+                        filename: attachment.title,
-                  promises.push(
+                        mimetype: 'image/jpeg',
-                    PresignedUrl.getSignedUrl({
+                        nestedKeys: ['thumbnails', key],
-                      pathOrUrl: `${relativePath}/small.jpg`,
+                      }),
-                      preview: true,
+                    );
-                      mimetype: 'image/jpeg',
+                  }
-                      filename: attachment.title,
-                    }).then(
-                      (r) => (attachment.thumbnails.small.signedPath = r),
-                    ),
-                  );
-                  promises.push(
-                    PresignedUrl.getSignedUrl({
-                      pathOrUrl: `${relativePath}/card_cover.jpg`,
-                      preview: true,
-                      mimetype: 'image/jpeg',
-                      filename: attachment.title,
-                    }).then(
-                      (r) => (attachment.thumbnails.card_cover.signedPath = r),
-                    ),
-                  );
                 } else if (attachment?.url) {
-                  let relativePath = attachment.url;
-
                   promises.push(
-                    PresignedUrl.getSignedUrl({
+                    PresignedUrl.signAttachment({
-                      pathOrUrl: relativePath,
+                      attachment,
-                      preview: true,
-                      mimetype: attachment.mimetype,
                       filename: attachment.title,
-                    }).then((r) => (attachment.signedUrl = r)),
+                    }),
                   );
 
-                  relativePath = relativePath.replace(
+                  const thumbhailUrl = attachment.url.replace(
                     'nc/uploads',
                     'nc/thumbnails',
                   );
+
                   attachment.thumbnails = {
                     tiny: {},
                     small: {},
                     card_cover: {},
                   };
 
-                  promises.push(
+                  for (const key of Object.keys(attachment.thumbnails)) {
-                    PresignedUrl.getSignedUrl({
+                    promises.push(
-                      pathOrUrl: `${relativePath}/tiny.jpg`,
+                      PresignedUrl.signAttachment({
-                      preview: true,
+                        attachment: {
-                      mimetype: 'image/jpeg',
+                          ...attachment,
-                      filename: attachment.title,
+                          url: `${thumbhailUrl}/${key}.jpg`,
-                    }).then((r) => (attachment.thumbnails.tiny.signedUrl = r)),
+                        },
-                  );
+                        filename: attachment.title,
-                  promises.push(
+                        mimetype: 'image/jpeg',
-                    PresignedUrl.getSignedUrl({
+                        nestedKeys: ['thumbnails', key],
-                      pathOrUrl: `${relativePath}/small.jpg`,
+                      }),
-                      preview: true,
+                    );
-                      mimetype: 'image/jpeg',
+                  }
-                      filename: attachment.title,
-                    }).then((r) => (attachment.thumbnails.small.signedUrl = r)),
-                  );
-                  promises.push(
-                    PresignedUrl.getSignedUrl({
-                      pathOrUrl: `${relativePath}/card_cover.jpg`,
-                      preview: true,
-                      mimetype: 'image/jpeg',
-                      filename: attachment.title,
-                    }).then(
-                      (r) => (attachment.thumbnails.card_cover.signedUrl = r),
-                    ),
-                  );
                 }
               }
             }

packages/nocodb/src/helpers/attachmentHelpers.ts

@@ -1,4 +1,8 @@
+import path from 'path';
 import mime from 'mime/lite';
+import slash from 'slash';
+import { getToolDir } from '~/utils/nc-config';
+import { NcError } from '~/helpers/catchError';
 
 const previewableMimeTypes = ['image', 'pdf', 'video', 'audio'];
 
@@ -21,3 +25,32 @@ export function isPreviewAllowed(args: { mimetype?: string; path?: string }) {
 
   return false;
 }
+
+// method for validate/normalise the path for avoid path traversal attack
+export function validateAndNormaliseLocalPath(
+  fileOrFolderPath: string,
+  throw404 = false,
+): string {
+  fileOrFolderPath = slash(fileOrFolderPath);
+
+  const toolDir = getToolDir();
+
+  // Get the absolute path to the base directory
+  const absoluteBasePath = path.resolve(toolDir, 'nc');
+
+  // Get the absolute path to the file
+  const absolutePath = path.resolve(
+    path.join(toolDir, ...fileOrFolderPath.replace(toolDir, '').split('/')),
+  );
+
+  // Check if the resolved path is within the intended directory
+  if (!absolutePath.startsWith(absoluteBasePath)) {
+    if (throw404) {
+      NcError.notFound();
+    } else {
+      NcError.badRequest('Invalid path');
+    }
+  }
+
+  return absolutePath;
+}

packages/nocodb/src/models/FormView.ts

@@ -238,26 +238,14 @@ export default class FormView implements FormViewType {
             formAttachments[key] = deserializeJSON(formAttachments[key]);
           }
 
-          if (formAttachments[key]?.path) {
+          promises.push(
-            promises.push(
+            PresignedUrl.signAttachment(
-              PresignedUrl.getSignedUrl(
+              {
-                {
+                attachment: formAttachments[key],
-                  pathOrUrl: formAttachments[key].path.replace(
+              },
-                    /^download\//,
+              ncMeta,
-                    '',
+            ),
-                  ),
+          );
-                },
-                ncMeta,
-              ).then((r) => (formAttachments[key].signedPath = r)),
-            );
-          } else if (formAttachments[key]?.url) {
-            promises.push(
-              PresignedUrl.getSignedUrl(
-                { pathOrUrl: formAttachments[key].url },
-                ncMeta,
-              ).then((r) => (formAttachments[key].signedUrl = r)),
-            );
-          }
         }
         await Promise.all(promises);
       }

packages/nocodb/src/models/PresignedUrl.ts

@@ -207,4 +207,62 @@ export default class PresignedUrl {
     // return the url
     return tempUrl;
   }
+
+  public static async signAttachment(
+    param: {
+      attachment: {
+        url?: string;
+        path?: string;
+        mimetype: string;
+        signedPath?: string;
+        signedUrl?: string;
+      };
+      preview?: boolean;
+      mimetype?: string;
+      filename?: string;
+      expireSeconds?: number;
+      // allow writing to nested property instead of root (used for thumbnails)
+      nestedKeys?: string[];
+    },
+    ncMeta = Noco.ncMeta,
+  ) {
+    const {
+      nestedKeys = [],
+      attachment,
+      preview = true,
+      mimetype,
+      ...extra
+    } = param;
+
+    const nestedObj = nestedKeys.reduce((acc, key) => {
+      if (acc[key]) {
+        return acc[key];
+      }
+
+      acc[key] = {};
+      return acc[key];
+    }, attachment);
+
+    if (attachment?.path) {
+      nestedObj.signedPath = await PresignedUrl.getSignedUrl(
+        {
+          pathOrUrl: attachment.path.replace(/^download\//, ''),
+          preview,
+          mimetype: mimetype || attachment.mimetype,
+          ...(extra ? { ...extra } : {}),
+        },
+        ncMeta,
+      );
+    } else if (attachment?.url) {
+      nestedObj.signedUrl = await PresignedUrl.getSignedUrl(
+        {
+          pathOrUrl: attachment.url,
+          preview,
+          mimetype: mimetype || attachment.mimetype,
+          ...(extra ? { ...extra } : {}),
+        },
+        ncMeta,
+      );
+    }
+  }
 }

packages/nocodb/src/modules/jobs/jobs/thumbnail-generator/thumbnail-generator.processor.ts

@@ -4,12 +4,11 @@ import { Process, Processor } from '@nestjs/bull';
 import { Job } from 'bull';
 import { Logger } from '@nestjs/common';
 import sharp from 'sharp';
-import axios from 'axios';
+import type { IStorageAdapterV2 } from 'nc-plugin';
 import type { AttachmentResType } from 'nocodb-sdk';
 import type { ThumbnailGeneratorJobData } from '~/interface/Jobs';
 import { JOBS_QUEUE, JobTypes } from '~/interface/Jobs';
 import NcPluginMgrv2 from '~/helpers/NcPluginMgrv2';
-import { PresignedUrl } from '~/models';
 import { AttachmentsService } from '~/services/attachments.service';
 
 const attachmentPreviews = ['image/'];
@@ -43,24 +42,32 @@ export class ThumbnailGeneratorProcessor {
 
       return await Promise.all(thumbnailPromises);
     } catch (error) {
-      this.logger.error('Failed to generate thumbnails', error);
+      this.logger.error('Failed to generate thumbnails', error.stack as string);
     }
   }
 
   private async generateThumbnail(
     attachment: AttachmentResType,
   ): Promise<{ [key: string]: string }> {
-    const { file, relativePath } = await this.getFileData(attachment);
-
-    const thumbnailPaths = {
-      card_cover: path.join('nc', 'thumbnails', relativePath, 'card_cover.jpg'),
-      small: path.join('nc', 'thumbnails', relativePath, 'small.jpg'),
-      tiny: path.join('nc', 'thumbnails', relativePath, 'tiny.jpg'),
-    };
-
     try {
       const storageAdapter = await NcPluginMgrv2.storageAdapter();
 
+      const { file, relativePath } = await this.getFileData(
+        attachment,
+        storageAdapter,
+      );
+
+      const thumbnailPaths = {
+        card_cover: path.join(
+          'nc',
+          'thumbnails',
+          relativePath,
+          'card_cover.jpg',
+        ),
+        small: path.join('nc', 'thumbnails', relativePath, 'small.jpg'),
+        tiny: path.join('nc', 'thumbnails', relativePath, 'tiny.jpg'),
+      };
+
       await Promise.all(
         Object.entries(thumbnailPaths).map(async ([size, thumbnailPath]) => {
           let height;
@@ -101,49 +108,37 @@ export class ThumbnailGeneratorProcessor {
       return thumbnailPaths;
     } catch (error) {
       this.logger.error(
-        `Failed to generate thumbnails for ${attachment.path}`,
+        `Failed to generate thumbnails for ${
-        error,
+          attachment.path ?? attachment.url
+        }`,
+        error.stack as string,
       );
     }
   }
 
   private async getFileData(
     attachment: AttachmentResType,
+    storageAdapter: IStorageAdapterV2,
   ): Promise<{ file: Buffer; relativePath: string }> {
-    let url, signedUrl, file;
     let relativePath;
 
     if (attachment.path) {
-      relativePath = attachment.path.replace(/^download\//, '');
+      relativePath = path.join(
-      url = await PresignedUrl.getSignedUrl({
+        'nc',
-        pathOrUrl: relativePath,
+        'uploads',
-        preview: false,
+        attachment.path.replace(/^download\//, ''),
-        filename: attachment.title,
+      );
-        mimetype: attachment.mimetype,
-      });
-
-      const fullPath = await PresignedUrl.getPath(`${url}`);
-      const [fpath] = fullPath.split('?');
-      const tempPath = await this.attachmentsService.getFile({
-        path: path.join('nc', 'uploads', fpath),
-      });
-      relativePath = fpath;
-      file = tempPath.path;
     } else if (attachment.url) {
-      relativePath = decodeURI(new URL(attachment.url).pathname);
+      relativePath = decodeURI(new URL(attachment.url).pathname).replace(
-
+        /^\/+/,
-      signedUrl = await PresignedUrl.getSignedUrl({
+        '',
-        pathOrUrl: attachment.url,
+      );
-        preview: false,
-        filename: attachment.title,
-        mimetype: attachment.mimetype,
-      });
-
-      file = (await axios({ url: signedUrl, responseType: 'arraybuffer' }))
-        .data as Buffer;
     }
 
-    relativePath = relativePath.replace(/^.*?(?=\/noco)/, '');
+    const file = await storageAdapter.fileRead(relativePath);
+
+    // remove /nc/uploads/ or nc/uploads/ from the path
+    relativePath = relativePath.replace(/^\/?nc\/uploads\//, '');
 
     return { file, relativePath };
   }

packages/nocodb/src/plugins/s3/S3.ts

@@ -107,15 +107,28 @@ export default class S3 implements IStorageAdapterV2 {
   }
 
   public async fileRead(key: string): Promise<any> {
+    const command = new GetObjectCommand({
+      Key: key,
+      Bucket: this.input.bucket,
+    });
+
+    const { Body } = await this.s3Client.send(command);
+
+    const fileStream = Body as Readable;
+
     return new Promise((resolve, reject) => {
-      this.s3Client.getObject({ Key: key } as any, (err, data) => {
+      const chunks: any[] = [];
-        if (err) {
+      fileStream.on('data', (chunk) => {
-          return reject(err);
+        chunks.push(chunk);
-        }
+      });
-        if (!data?.Body) {
+
-          return reject(data);
+      fileStream.on('end', () => {
-        }
+        const buffer = Buffer.concat(chunks);
-        return resolve(data.Body);
+        resolve(buffer);
+      });
+
+      fileStream.on('error', (err) => {
+        reject(err);
       });
     });
   }

packages/nocodb/src/plugins/storage/Local.ts

@@ -6,14 +6,13 @@ import axios from 'axios';
 import { useAgent } from 'request-filtering-agent';
 import type { IStorageAdapterV2, XcFile } from 'nc-plugin';
 import type { Readable } from 'stream';
-import { NcError } from '~/helpers/catchError';
+import { validateAndNormaliseLocalPath } from '~/helpers/attachmentHelpers';
-import { getToolDir } from '~/utils/nc-config';
 
 export default class Local implements IStorageAdapterV2 {
   constructor() {}
 
   public async fileCreate(key: string, file: XcFile): Promise<any> {
-    const destPath = this.validateAndNormalisePath(key);
+    const destPath = validateAndNormaliseLocalPath(key);
     try {
       await mkdirp(path.dirname(destPath));
       const data = await promisify(fs.readFile)(file.path);
@@ -26,7 +25,7 @@ export default class Local implements IStorageAdapterV2 {
   }
 
   async fileCreateByUrl(key: string, url: string): Promise<any> {
-    const destPath = this.validateAndNormalisePath(key);
+    const destPath = validateAndNormaliseLocalPath(key);
     return new Promise((resolve, reject) => {
       axios
         .get(url, {
@@ -68,7 +67,7 @@ export default class Local implements IStorageAdapterV2 {
     stream: Readable,
   ): Promise<void> {
     return new Promise((resolve, reject) => {
-      const destPath = this.validateAndNormalisePath(key);
+      const destPath = validateAndNormaliseLocalPath(key);
       try {
         mkdirp(path.dirname(destPath)).then(() => {
           const writableStream = fs.createWriteStream(destPath);
@@ -83,12 +82,12 @@ export default class Local implements IStorageAdapterV2 {
   }
 
   public async fileReadByStream(key: string): Promise<Readable> {
-    const srcPath = this.validateAndNormalisePath(key);
+    const srcPath = validateAndNormaliseLocalPath(key);
     return fs.createReadStream(srcPath, { encoding: 'utf8' });
   }
 
   public async getDirectoryList(key: string): Promise<string[]> {
-    const destDir = this.validateAndNormalisePath(key);
+    const destDir = validateAndNormaliseLocalPath(key);
     return fs.promises.readdir(destDir);
   }
 
@@ -100,7 +99,7 @@ export default class Local implements IStorageAdapterV2 {
   public async fileRead(filePath: string): Promise<any> {
     try {
       const fileData = await fs.promises.readFile(
-        this.validateAndNormalisePath(filePath, true),
+        validateAndNormaliseLocalPath(filePath, true),
       );
       return fileData;
     } catch (e) {
@@ -115,29 +114,4 @@ export default class Local implements IStorageAdapterV2 {
   test(): Promise<boolean> {
     return Promise.resolve(false);
   }
-
-  // method for validate/normalise the path for avoid path traversal attack
-  public validateAndNormalisePath(
-    fileOrFolderPath: string,
-    throw404 = false,
-  ): string {
-    // Get the absolute path to the base directory
-    const absoluteBasePath = path.resolve(getToolDir(), 'nc');
-
-    // Get the absolute path to the file
-    const absolutePath = path.resolve(
-      path.join(getToolDir(), ...fileOrFolderPath.split('/')),
-    );
-
-    // Check if the resolved path is within the intended directory
-    if (!absolutePath.startsWith(absoluteBasePath)) {
-      if (throw404) {
-        NcError.notFound();
-      } else {
-        NcError.badRequest('Invalid path');
-      }
-    }
-
-    return absolutePath;
-  }
 }

packages/nocodb/src/services/attachments.service.ts

@@ -7,11 +7,12 @@ import slash from 'slash';
 import PQueue from 'p-queue';
 import axios from 'axios';
 import sharp from 'sharp';
+import hash from 'object-hash';
+import moment from 'moment';
 import type { AttachmentReqType, FileType } from 'nocodb-sdk';
 import type { NcRequest } from '~/interface/config';
 import { AppHooksService } from '~/services/app-hooks/app-hooks.service';
 import NcPluginMgrv2 from '~/helpers/NcPluginMgrv2';
-import Local from '~/plugins/storage/Local';
 import mimetypes, { mimeIcons } from '~/utils/mimeTypes';
 import { PresignedUrl } from '~/models';
 import { utf8ify } from '~/helpers/stringHelpers';
@@ -19,6 +20,7 @@ import { NcError } from '~/helpers/catchError';
 import { IJobsService } from '~/modules/jobs/jobs-service.interface';
 import { JobTypes } from '~/interface/Jobs';
 import { RootScopes } from '~/utils/globals';
+import { validateAndNormaliseLocalPath } from '~/helpers/attachmentHelpers';
 
 interface AttachmentObject {
   url?: string;
@@ -41,7 +43,12 @@ export class AttachmentsService {
     private readonly jobsService: IJobsService,
   ) {}
 
-  async upload(param: { path?: string; files: FileType[]; req: NcRequest }) {
+  async upload(param: { files: FileType[]; req?: NcRequest; path?: string }) {
+    const userId = param.req?.user.id || 'anonymous';
+
+    param.path =
+      param.path || `${moment().format('YYYY/MM/DD')}/${hash(userId)}`;
+
     // TODO: add getAjvValidatorMw
     const filePath = this.sanitizeUrlPath(
       param.path?.toString()?.split('/') || [''],
@@ -106,7 +113,7 @@ export class AttachmentsService {
             ...tempMetadata,
           };
 
-          await this.signAttachment({ attachment });
+          await PresignedUrl.signAttachment({ attachment });
 
           attachments.push(attachment);
         } catch (e) {
@@ -141,10 +148,15 @@ export class AttachmentsService {
   }
 
   async uploadViaURL(param: {
-    path?: string;
     urls: AttachmentReqType[];
-    req: NcRequest;
+    req?: NcRequest;
+    path?: string;
   }) {
+    const userId = param.req?.user.id || 'anonymous';
+
+    param.path =
+      param.path || `${moment().format('YYYY/MM/DD')}/${hash(userId)}`;
+
     const filePath = this.sanitizeUrlPath(
       param?.path?.toString()?.split('/') || [''],
     );
@@ -222,7 +234,7 @@ export class AttachmentsService {
             ...tempMetadata,
           };
 
-          await this.signAttachment({ attachment });
+          await PresignedUrl.signAttachment({ attachment });
 
           attachments.push(attachment);
         } catch (e) {
@@ -258,16 +270,11 @@ export class AttachmentsService {
     path: string;
     type: string;
   }> {
-    // get the local storage adapter to display local attachments
-    const storageAdapter = new Local();
     const type =
       mimetypes[path.extname(param.path).split('/').pop().slice(1)] ||
       'text/plain';
 
-    const filePath = storageAdapter.validateAndNormalisePath(
+    const filePath = validateAndNormaliseLocalPath(param.path, true);
-      slash(param.path),
-      true,
-    );
     return { path: filePath, type };
   }
 
@@ -292,7 +299,7 @@ export class AttachmentsService {
       NcError.genericNotFound('Attachment', urlOrPath);
     }
 
-    await this.signAttachment({
+    await PresignedUrl.signAttachment({
       attachment: fileObject,
       preview: false,
       filename: fileObject.title,
@@ -308,31 +315,6 @@ export class AttachmentsService {
     };
   }
 
-  async signAttachment(param: {
-    attachment: AttachmentObject;
-    preview?: boolean;
-    filename?: string;
-    expireSeconds?: number;
-  }) {
-    const { attachment, preview = true, ...extra } = param;
-
-    if (attachment?.path) {
-      attachment.signedPath = await PresignedUrl.getSignedUrl({
-        pathOrUrl: attachment.path.replace(/^download\//, ''),
-        preview,
-        mimetype: attachment.mimetype,
-        ...(extra ? { ...extra } : {}),
-      });
-    } else if (attachment?.url) {
-      attachment.signedUrl = await PresignedUrl.getSignedUrl({
-        pathOrUrl: attachment.url,
-        preview,
-        mimetype: attachment.mimetype,
-        ...(extra ? { ...extra } : {}),
-      });
-    }
-  }
-
   sanitizeUrlPath(paths) {
     return paths.map((url) => url.replace(/[/.?#]+/g, '_'));
   }

packages/nocodb/src/services/public-datas.service.ts

@@ -1,28 +1,19 @@
-import path from 'path';
 import { forwardRef, Inject, Injectable } from '@nestjs/common';
-import { nanoid } from 'nanoid';
+import { UITypes, ViewTypes } from 'nocodb-sdk';
-import { populateUniqueFileName, UITypes, ViewTypes } from 'nocodb-sdk';
-import slash from 'slash';
 import { nocoExecute } from 'nc-help';
-
-import sharp from 'sharp';
 import type { LinkToAnotherRecordColumn } from '~/models';
 import type { NcContext } from '~/interface/config';
 import { Column, Model, Source, View } from '~/models';
 import { NcError } from '~/helpers/catchError';
 import getAst from '~/helpers/getAst';
-import NcPluginMgrv2 from '~/helpers/NcPluginMgrv2';
 import { PagedResponseImpl } from '~/helpers/PagedResponse';
 import { getColumnByIdOrName } from '~/helpers/dataHelpers';
 import NcConnectionMgrv2 from '~/utils/common/NcConnectionMgrv2';
-import { mimeIcons } from '~/utils/mimeTypes';
-import { utf8ify } from '~/helpers/stringHelpers';
 import { replaceDynamicFieldWithValue } from '~/db/BaseModelSqlv2';
 import { Filter } from '~/models';
 import { IJobsService } from '~/modules/jobs/jobs-service.interface';
-import { JobTypes } from '~/interface/Jobs';
-import { RootScopes } from '~/utils/globals';
 import { DatasService } from '~/services/datas.service';
+import { AttachmentsService } from '~/services/attachments.service';
 
 // todo: move to utils
 export function sanitizeUrlPath(paths) {
@@ -35,6 +26,7 @@ export class PublicDatasService {
     protected datasService: DatasService,
     @Inject(forwardRef(() => 'JobsService'))
     protected readonly jobsService: IJobsService,
+    protected readonly attachmentsService: AttachmentsService,
   ) {}
 
   async dataList(
@@ -354,8 +346,6 @@ export class PublicDatasService {
       NcError.sourceDataReadOnly(source.alias);
     }
 
-    const base = await source.getProject(context);
-
     const baseModel = await Model.getBaseModelSQL(context, {
       id: model.id,
       viewId: view?.id,
@@ -389,7 +379,6 @@ export class PublicDatasService {
     }, {});
 
     const attachments = {};
-    const storageAdapter = await NcPluginMgrv2.storageAdapter();
 
     for (const file of param.files || []) {
       // remove `_` prefix and `[]` suffix
@@ -397,66 +386,17 @@ export class PublicDatasService {
         .toString('utf-8')
         .replace(/^_|\[\d*]$/g, '');
 
-      const filePath = sanitizeUrlPath([
-        'noco',
-        base.title,
-        model.title,
-        fieldName,
-      ]);
-
       if (
         fieldName in fields &&
         fields[fieldName].uidt === UITypes.Attachment
       ) {
         attachments[fieldName] = attachments[fieldName] || [];
-        let originalName = utf8ify(file.originalname);
 
-        originalName = populateUniqueFileName(
+        attachments[fieldName].push(
-          originalName,
+          ...(await this.attachmentsService.upload({
-          attachments[fieldName].map((att) => att?.title),
+            files: [file],
+          })),
         );
-
-        const tempMeta: {
-          width?: number;
-          height?: number;
-        } = {};
-
-        if (file.mimetype.startsWith('image')) {
-          try {
-            const meta = await sharp(file.path, {
-              limitInputPixels: false,
-            }).metadata();
-            tempMeta.width = meta.width;
-            tempMeta.height = meta.height;
-          } catch (e) {
-            // Ignore-if file is not image
-          }
-        }
-
-        const fileName = `${nanoid(18)}${path.extname(originalName)}`;
-
-        const url = await storageAdapter.fileCreate(
-          slash(path.join('nc', 'uploads', ...filePath, fileName)),
-          file,
-        );
-        let attachmentPath;
-
-        // if `url` is null, then it is local attachment
-        if (!url) {
-          // then store the attachment path only
-          // url will be constructed in `useAttachmentCell`
-          attachmentPath = `download/${filePath.join('/')}/${fileName}`;
-        }
-
-        attachments[fieldName].push({
-          ...(url ? { url } : {}),
-          ...(attachmentPath ? { path: attachmentPath } : {}),
-          title: originalName,
-          mimetype: file.mimetype,
-          size: file.size,
-          icon: mimeIcons[path.extname(originalName).slice(1)] || undefined,
-          ...tempMeta,
-        });
       }
     }
 
@@ -477,77 +417,17 @@ export class PublicDatasService {
     }
 
     for (const file of uploadByUrlAttachments) {
-      const filePath = sanitizeUrlPath([
-        'noco',
-        base.title,
-        model.title,
-        file.fieldName,
-      ]);
-
       attachments[file.fieldName] = attachments[file.fieldName] || [];
 
-      const fileName = `${nanoid(18)}${path.extname(
+      attachments[file.fieldName].unshift(
-        file?.fileName || file.url.split('/').pop(),
+        ...(await this.attachmentsService.uploadViaURL({
-      )}`;
+          urls: [file.url],
-
+        })),
-      const { url, data } = await storageAdapter.fileCreateByUrl(
-        slash(path.join('nc', 'uploads', ...filePath, fileName)),
-        file.url,
-      );
-
-      const tempMetadata: {
-        width?: number;
-        height?: number;
-      } = {};
-
-      try {
-        const metadata = await sharp(data, {
-          limitInputPixels: true,
-        }).metadata();
-
-        if (metadata.width && metadata.height) {
-          tempMetadata.width = metadata.width;
-          tempMetadata.height = metadata.height;
-        }
-      } catch (e) {
-        // Might be invalid image - ignore
-      }
-
-      let attachmentPath: string | undefined;
-
-      // if `attachmentUrl` is null, then it is local attachment
-      if (!url) {
-        // then store the attachment path only
-        // url will be constructed in `useAttachmentCell`
-        attachmentPath = `download/${filePath.join('/')}/${fileName}`;
-      }
-
-      // add attachement in uploaded order
-      attachments[file.fieldName].splice(
-        file.uploadIndex ?? attachments[file.fieldName].length,
-        0,
-        {
-          ...(url ? { url: url } : {}),
-          ...(attachmentPath ? { path: attachmentPath } : {}),
-          title: file.fileName,
-          mimetype: file.mimetype,
-          size: file.size,
-          icon: mimeIcons[path.extname(fileName).slice(1)] || undefined,
-          ...tempMetadata,
-        },
       );
     }
 
     for (const [column, data] of Object.entries(attachments)) {
       insertObject[column] = JSON.stringify(data);
-
-      await this.jobsService.add(JobTypes.ThumbnailGenerator, {
-        attachments: data,
-        context: {
-          base_id: RootScopes.ROOT,
-          workspace_id: RootScopes.ROOT,
-        },
-      });
     }
 
     return await baseModel.nestedInsert(insertObject, null);