feat: add logs for uploads to s3 (#7975)

* feat: add logs for uploads to s3 * feat: add logs including formid for virus scanning * feat: remove attachmentmetadata from upload log * feat: update error message precision * feat: cast id to string * feat: add attachment metadata to logs to link s3 object to form * feat: remove upload attachment log
opengovsg · Dec 10, 2024 · b61c831 · b61c831
1 parent 9dd7191
commit b61c831
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 26 deletions.
diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts
@@ -166,6 +166,7 @@ export const validateStorageSubmissionParams = celebrate({
  */
 const asyncVirusScanning = (
   responses: ParsedClearFormFieldResponse[],
+  formId: string,
 ): ResultAsync<
   ParsedClearFormFieldResponse,
   | VirusScanFailedError
@@ -176,6 +177,7 @@ const asyncVirusScanning = (
     if (isQuarantinedAttachmentResponse(response)) {
       return SubmissionService.triggerVirusScanThenDownloadCleanFileChain(
         response,
+        formId,
       )
     }
 
@@ -191,6 +193,7 @@ const asyncVirusScanning = (
  */
 const devModeSyncVirusScanning = async (
   responses: ParsedClearFormFieldResponse[],
+  formId: string,
 ): Promise<
   Result<
     ParsedClearFormFieldResponse,
@@ -209,6 +212,7 @@ const devModeSyncVirusScanning = async (
       const attachmentResponse =
         await SubmissionService.triggerVirusScanThenDownloadCleanFileChain(
           response,
+          formId,
         )
       results.push(attachmentResponse)
       if (attachmentResponse.isErr()) break
@@ -246,8 +250,18 @@ export const scanAndRetrieveAttachments = async (
     // run the virus scanning asynchronously for better performance (lower latency).
     // Note on .combine: if any scans or downloads error out, it will short circuit and return the first error.
     isDev
-      ? Result.combine(await devModeSyncVirusScanning(req.body.responses))
-      : await ResultAsync.combine(asyncVirusScanning(req.body.responses))
+      ? Result.combine(
+          await devModeSyncVirusScanning(
+            req.body.responses,
+            req.formsg.formDef._id.toString(),
+          ),
+        )
+      : await ResultAsync.combine(
+          asyncVirusScanning(
+            req.body.responses,
+            req.formsg.formDef._id.toString(),
+          ),
+        )
 
   if (scanAndRetrieveFilesResult.isErr()) {
     logger.error({

diff --git a/...pp/modules/submission/multirespondent-submission/multirespondent-submission.middleware.ts b/...pp/modules/submission/multirespondent-submission/multirespondent-submission.middleware.ts
@@ -184,14 +184,15 @@ type IdTaggedParsedClearAttachmentResponseV3 =
  */
 const asyncVirusScanning = (
   responses: IdTaggedParsedClearAttachmentResponseV3[],
+  formId: string,
 ): ResultAsync<
   IdTaggedParsedClearAttachmentResponseV3,
   | VirusScanFailedError
   | DownloadCleanFileFailedError
   | MaliciousFileDetectedError
 >[] =>
   responses.map((response) =>
-    triggerVirusScanThenDownloadCleanFileChain(response.answer).map(
+    triggerVirusScanThenDownloadCleanFileChain(response.answer, formId).map(
       (attachmentResponse) => ({ ...response, answer: attachmentResponse }),
     ),
   )
@@ -203,6 +204,7 @@ const asyncVirusScanning = (
  */
 const devModeSyncVirusScanning = async (
   responses: IdTaggedParsedClearAttachmentResponseV3[],
+  formId: string,
 ): Promise<
   Result<
     IdTaggedParsedClearAttachmentResponseV3,
@@ -216,6 +218,7 @@ const devModeSyncVirusScanning = async (
     // await to pause for...of loop until the virus scanning and downloading of clean file is completed.
     const attachmentResponse = await triggerVirusScanThenDownloadCleanFileChain(
       response.answer,
+      formId,
     )
     if (attachmentResponse.isErr()) {
       results.push(err(attachmentResponse.error))
@@ -267,10 +270,16 @@ export const scanAndRetrieveAttachments = async (
     // Note on .combine: if any scans or downloads error out, it will short circuit and return the first error.
     isDev
       ? Result.combine(
-          await devModeSyncVirusScanning(attachmentResponsesToRetrieve),
+          await devModeSyncVirusScanning(
+            attachmentResponsesToRetrieve,
+            req.formsg.formDef._id.toString(),
+          ),
         )
       : await ResultAsync.combine(
-          asyncVirusScanning(attachmentResponsesToRetrieve),
+          asyncVirusScanning(
+            attachmentResponsesToRetrieve,
+            req.formsg.formDef._id.toString(),
+          ),
         )
 
   if (scanAndRetrieveFilesResult.isErr()) {

diff --git a/src/app/modules/submission/submission.service.ts b/src/app/modules/submission/submission.service.ts
@@ -432,33 +432,55 @@ export const triggerVirusScanThenDownloadCleanFileChain = <
     | ParsedClearAttachmentFieldResponseV3,
 >(
   response: T,
+  formId: string,
 ): ResultAsync<
   T,
   | VirusScanFailedError
   | DownloadCleanFileFailedError
   | MaliciousFileDetectedError
-> =>
+> => {
+  const logMeta = {
+    action: 'triggerVirusScanThenDownloadCleanFileChain',
+    formId,
+    quarantineFileKey: response.answer,
+  }
   // Step 3: Trigger lambda to scan attachments.
-  triggerVirusScanning(response.answer)
-    .mapErr((error) => {
-      if (error instanceof MaliciousFileDetectedError)
-        return new MaliciousFileDetectedError(response.filename)
-      return error
-    })
-    .map((lambdaOutput) => lambdaOutput.body)
-    // Step 4: Retrieve attachments from the clean bucket.
-    .andThen((cleanAttachment) =>
-      // Retrieve attachment from clean bucket.
-      downloadCleanFile(
-        cleanAttachment.cleanFileKey,
-        cleanAttachment.destinationVersionId,
-      ).map((attachmentBuffer) => ({
-        ...response,
-        // Replace content with attachmentBuffer and answer with filename.
-        content: attachmentBuffer,
-        answer: response.filename,
-      })),
-    )
+  return (
+    triggerVirusScanning(response.answer)
+      .mapErr((error) => {
+        if (error instanceof MaliciousFileDetectedError) {
+          logger.error({
+            message: 'Malicious file detected during lambda virus scan',
+            meta: logMeta,
+            error,
+          })
+          return new MaliciousFileDetectedError(response.filename)
+        }
+        return error
+      })
+      .map((lambdaOutput) => {
+        logger.info({
+          message:
+            'Successfully retrieved clean file from virus scanning lambda',
+          meta: { ...logMeta, cleanFileKey: lambdaOutput.body.cleanFileKey },
+        })
+        return lambdaOutput.body
+      })
+      // Step 4: Retrieve attachments from the clean bucket.
+      .andThen((cleanAttachment) =>
+        // Retrieve attachment from clean bucket.
+        downloadCleanFile(
+          cleanAttachment.cleanFileKey,
+          cleanAttachment.destinationVersionId,
+        ).map((attachmentBuffer) => ({
+          ...response,
+          // Replace content with attachmentBuffer and answer with filename.
+          content: attachmentBuffer,
+          answer: response.filename,
+        })),
+      )
+  )
+}
 
 type AttachmentReducerData = {
   attachmentMetadata: AttachmentMetadata // type alias for Map<string, string>