Nc fix/attachments (#8478)

* fix: sanitize attachment input properly & throw if invalid

* fix: looked up attachments

* fix: avoid extra slash on attachment path

---------

Co-authored-by: mertmit <mertmit99@gmail.com>

packages/nocodb-sdk/src/lib/globals.ts

@@ -140,6 +140,7 @@ export enum NcErrorType {
   INVALID_LIMIT_VALUE = 'INVALID_LIMIT_VALUE',
   INVALID_FILTER = 'INVALID_FILTER',
   INVALID_SHARED_VIEW_PASSWORD = 'INVALID_SHARED_VIEW_PASSWORD',
+  INVALID_ATTACHMENT_JSON = 'INVALID_ATTACHMENT_JSON',
   NOT_IMPLEMENTED = 'NOT_IMPLEMENTED',
   INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR',
   DATABASE_ERROR = 'DATABASE_ERROR',

packages/nocodb/src/db/BaseModelSqlv2.ts

@@ -5491,7 +5491,13 @@ class BaseModelSqlv2 {
           }
 
           if (d[col.id]?.length) {
-            for (const attachment of d[col.id]) {
+            for (let i = 0; i < d[col.id].length; i++) {
+              if (typeof d[col.id][i] === 'string') {
+                d[col.id][i] = JSON.parse(d[col.id][i]);
+              }
+
+              const attachment = d[col.id][i];
+
               // we expect array of array of attachments in case of lookup
               if (Array.isArray(attachment)) {
                 for (const lookedUpAttachment of attachment) {
@@ -6477,17 +6483,34 @@ class BaseModelSqlv2 {
       }
       if (column.uidt === UITypes.Attachment) {
         if (data[column.column_name]) {
+          try {
+            if (typeof data[column.column_name] === 'string') {
+              data[column.column_name] = JSON.parse(data[column.column_name]);
+            }
+          } catch (e) {
+            NcError.invalidAttachmentJson(data[column.column_name]);
+          }
+
           if (Array.isArray(data[column.column_name])) {
-            for (let attachment of data[column.column_name]) {
-              attachment = extractProps(attachment, [
+            const sanitizedAttachments = [];
+            for (const attachment of data[column.column_name]) {
+              if (!('url' in attachment) && !('path' in attachment)) {
+                NcError.unprocessableEntity(
+                  'Attachment object must contain either url or path',
+                );
+              }
+              sanitizedAttachments.push(
+                extractProps(attachment, [
                   'url',
                   'path',
                   'title',
                   'mimetype',
                   'size',
                   'icon',
-              ]);
+                ]),
+              );
             }
+            data[column.column_name] = JSON.stringify(sanitizedAttachments);
           }
         }
       } else if (

packages/nocodb/src/helpers/catchError.ts

@@ -525,6 +525,11 @@ const errorHelpers: {
     message: 'Invalid shared view password',
     code: 403,
   },
+  [NcErrorType.INVALID_ATTACHMENT_JSON]: {
+    message: (payload: string) =>
+      `Invalid JSON for attachment field: ${payload}`,
+    code: 400,
+  },
   [NcErrorType.NOT_IMPLEMENTED]: {
     message: (feature: string) => `${feature} is not implemented`,
     code: 501,
@@ -688,6 +693,13 @@ export class NcError {
     });
   }
 
+  static invalidAttachmentJson(payload: string, args?: NcErrorArgs) {
+    throw new NcBaseErrorv2(NcErrorType.INVALID_ATTACHMENT_JSON, {
+      params: payload,
+      ...args,
+    });
+  }
+
   static notImplemented(feature: string = 'Feature', args?: NcErrorArgs) {
     throw new NcBaseErrorv2(NcErrorType.NOT_IMPLEMENTED, {
       params: feature,

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

@@ -73,7 +73,11 @@ export class AttachmentsService {
           if (!url) {
             // then store the attachment path only
             // url will be constructed in `useAttachmentCell`
-            attachment.path = `download/${filePath.join('/')}/${fileName}`;
+            attachment.path = path.join(
+              'download',
+              filePath.join('/'),
+              fileName,
+            );
 
             attachment.signedPath = await PresignedUrl.getSignedUrl({
               path: attachment.path.replace(/^download\//, ''),