From 5b2ba2cb7e999c734ecd7c5d326946e8da7cfc8d Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Wed, 15 May 2024 14:51:11 +0530
Subject: [PATCH] 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        |  1 +
 packages/nocodb/src/db/BaseModelSqlv2.ts      | 43 ++++++++++++++-----
 packages/nocodb/src/helpers/catchError.ts     | 12 ++++++
 .../src/services/attachments.service.ts       |  6 ++-
 4 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/packages/nocodb-sdk/src/lib/globals.ts b/packages/nocodb-sdk/src/lib/globals.ts
index a7b2e65d62..e0b4ec186e 100644
--- a/packages/nocodb-sdk/src/lib/globals.ts
+++ b/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',
diff --git a/packages/nocodb/src/db/BaseModelSqlv2.ts b/packages/nocodb/src/db/BaseModelSqlv2.ts
index 602d4ff529..0192b57a3f 100644
--- a/packages/nocodb/src/db/BaseModelSqlv2.ts
+++ b/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, [
-                'url',
-                'path',
-                'title',
-                'mimetype',
-                'size',
-                'icon',
-              ]);
+            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 (
diff --git a/packages/nocodb/src/helpers/catchError.ts b/packages/nocodb/src/helpers/catchError.ts
index 93005a58fd..0c7c6e2a4f 100644
--- a/packages/nocodb/src/helpers/catchError.ts
+++ b/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,
diff --git a/packages/nocodb/src/services/attachments.service.ts b/packages/nocodb/src/services/attachments.service.ts
index 197c7f3d74..8d12642628 100644
--- a/packages/nocodb/src/services/attachments.service.ts
+++ b/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\//, ''),