#5459 Fix visible 'noname' email attachment file (#5472)

* add conditional statement for hiding noname attachment

* wip: update test

* wip: add test for password messages

* #5460 hide number of attachments

* #5460 add live gmail test

* wip: live gmail test

* wip: fix incorrect attachment count

* pr reviews: fix incorrect number of attachments

* bug fix: hide attachment container only when .pgp_pubkey is hidden

* pr reviews: include missing test on gmail.ts

* wip: revert changes on semaphore.yml

* wip: trim blank line

* pr reviews: fix bug

* wip: live-gmail-test

* wip: revert changes on semaphore.yml

* pr reviews: fix minor issues

* wip: fix test for filename

* wip: fix ui for pgp block button

extension/css/cryptup.css

@@ -1443,10 +1443,6 @@ td {
   font-size: 14px;
 }
 
-.pgp_neutral button {
-  width: 120px;
-}
-
 .pgp_insecure {
   border: none;
   border-left: 4px solid #d14836;

extension/js/common/core/attachment.ts

@@ -8,7 +8,7 @@ import { Str } from './common.js';
 export type Attachment$treatAs =
   | 'publicKey'
   | 'privateKey'
-  | 'encryptedMsg' /* may be signed-only (known as 'signedMsg' in MsgBlockType) as well, 
+  | 'encryptedMsg' /* may be signed-only (known as 'signedMsg' in MsgBlockType) as well,
   should probably be renamed to 'cryptoMsg' to not be confused with 'encryptedMsg' in MsgBlockType */
   | 'hidden'
   | 'signature'
@@ -40,7 +40,15 @@ export class Attachment {
   // todo: it'd be better to compile this regex based on the data we have in `treatAs` method
   public static readonly webmailNamePattern =
     /^(((cryptup|flowcrypt)-backup-[a-z0-9]+\.(key|asc))|(.+\.pgp)|(.+\.gpg)|(.+\.asc)|(OpenPGP_signature(.asc)?)|(noname)|(message)|(PGPMIME version identification)|(ATT[0-9]{5})|())$/m;
-  public static readonly encryptedMsgNames = ['msg.asc', 'message.asc', 'encrypted.asc', 'encrypted.eml.pgp', 'Message.pgp', 'openpgp-encrypted-message.asc'];
+  public static readonly encryptedMsgNames = [
+    'msg.asc',
+    'message.asc',
+    'encrypted.asc',
+    'encrypted.eml.pgp',
+    'Message.pgp',
+    'openpgp-encrypted-message.asc',
+    '.asc.pgp',
+  ];
 
   public length = NaN;
   public type: string;
@@ -137,7 +145,7 @@ export class Attachment {
     }
     return (
       (this.type === 'application/pgp-keys' && !this.isPrivateKey()) ||
-      /^(0|0x)?[A-F0-9]{8}([A-F0-9]{8})?.*\.asc$/g.test(this.name) || // name starts with a key id
+      /^(0|0x)?([A-F0-9]{16}|[A-F0-9]{8}([A-F0-9]{8})?)\.asc(\.pgp)?$/i.test(this.name) || // Key ID (8 or 16 characters) with .asc extension (optional .pgp)
       (this.name.toLowerCase().includes('public') && /[A-F0-9]{8}.*\.asc$/g.test(this.name)) || // name contains the word "public", any key id and ends with .asc
       (/\.asc$/.test(this.name) && this.hasData() && Buf.with(this.getData().subarray(0, 100)).toUtfStr().includes('-----BEGIN PGP PUBLIC KEY BLOCK-----'))
     );

extension/js/common/message-renderer.ts

@@ -326,6 +326,11 @@ export class MessageRenderer {
           return 'hidden'; // native attachment should be hidden, the "attachment" goes to the message container
         }
       }
+      if (treatAs === 'plainFile') {
+        if (!a.name || a.name === 'noname') {
+          return 'hidden';
+        }
+      }
       if (treatAs !== 'plainFile') {
         loaderContext.hideAttachment(attachmentSel);
       }

extension/js/content_scripts/webmail/gmail-element-replacer.ts

@@ -440,6 +440,17 @@ export class GmailElementReplacer implements WebmailElementReplacer {
     }
   };
 
+  private isAttachmentHideable = (attachment: Attachment, renderStatus: string) => {
+    if (renderStatus === 'hidden') {
+      return true;
+    }
+    const isHideableFile =
+      attachment.type === 'application/pgp-keys' ||
+      attachment.isPublicKey() ||
+      Attachment.encryptedMsgNames.some(filename => attachment.name.includes(filename));
+    return isHideableFile;
+  };
+
   private processAttachments = async (
     msgId: string,
     attachments: Attachment[],
@@ -457,6 +468,7 @@ export class GmailElementReplacer implements WebmailElementReplacer {
     }
     const loaderContext = new GmailLoaderContext(this.factory, msgEl, attachmentsContainerInner);
     attachmentsContainerInner = $(attachmentsContainerInner);
+    attachmentsContainerInner.parent().find(this.sel.numberOfAttachments).hide();
     let nRenderedAttachments = attachments.length;
     for (const a of attachments) {
       const attachmentSel = this.filterAttachments(
@@ -473,21 +485,32 @@ export class GmailElementReplacer implements WebmailElementReplacer {
         messageInfo,
         skipGoogleDrive
       );
-      if (renderStatus === 'hidden') {
+      if (this.isAttachmentHideable(a, renderStatus)) {
         nRenderedAttachments--;
       }
     }
     if (nRenderedAttachments !== attachments.length) {
       // according to #4200, no point in showing "download all" button if at least one attachment is encrypted etc.
       $(this.sel.attachmentsButtons).hide();
     }
-    if (nRenderedAttachments >= 2) {
-      // Aligned with Gmail, the label is shown only if there are 2 or more attachments
-      attachmentsContainerInner.parent().find(this.sel.numberOfAttachmentsDigit).text(nRenderedAttachments);
-      attachmentsContainerInner.parent().find(this.sel.numberOfAttachments).show();
-    }
     if (nRenderedAttachments === 0) {
-      attachmentsContainerInner.parents(this.sel.attachmentsContainerOuter).first().hide();
+      $(this.sel.attachmentsContainerOuter).children('.hp').hide();
+      if ($('.pgp_block').length === 0) {
+        attachmentsContainerInner.parents(this.sel.attachmentsContainerOuter).first().hide();
+      }
+    } else {
+      const elementsToClone = ['span.a2H', 'div.a2b'];
+      const scannedByGmailLabel = $(elementsToClone[0]).first().clone();
+      const scannedByGmailPopover = $(elementsToClone[1]).first().clone(true);
+      // for uniformity reasons especially when Google used "One" for single attachment and numeric representation for multiple.
+      const gmailAttachmentLabelReplacement = $(
+        `<span>${nRenderedAttachments}</span>&nbsp;<span>${nRenderedAttachments > 1 ? 'Attachments' : 'Attachment'}</span>`
+      );
+      attachmentsContainerInner.parent().find(this.sel.numberOfAttachments).empty();
+      gmailAttachmentLabelReplacement.appendTo(attachmentsContainerInner.parent().find(this.sel.numberOfAttachments));
+      scannedByGmailLabel.appendTo(attachmentsContainerInner.parent().find(this.sel.numberOfAttachments));
+      scannedByGmailPopover.appendTo(attachmentsContainerInner.parent().find(this.sel.numberOfAttachments));
+      attachmentsContainerInner.parent().find(this.sel.numberOfAttachments).show();
     }
     if (!skipGoogleDrive) {
       await this.processGoogleDriveAttachments(msgId, loaderContext.msgEl, attachmentsContainerInner, messageInfo);

test/source/tests/decrypt.ts

@@ -2008,23 +2008,23 @@ XZ8r4OC6sguP/yozWlkG+7dDxsgKQVBENeG6Lw==
     );
 
     test(
-      'decrypt - an ambiguous file "noname" should not be recognized as an encrypted message',
+      'decrypt - an ambiguous file "noname" should be hidden and not be recognized as an encrypted message',
       testWithBrowser(async (t, browser) => {
         const threadId1 = '18adb91ebf3ba7b9'; // email attachment "noname" with type img/<image-extension>
         const threadId2 = '18afaa4118afeb62'; // email attachment "noname" with type application/octet-stream
+        const threadId3 = '18b7f6a2b00ad967'; // a password-protected message that is also public key encrypted with noname attachment
         const { acctEmail } = await BrowserRecipe.setupCommonAcctWithAttester(t, browser, 'compatibility');
         const inboxPage1 = await browser.newExtensionPage(t, `chrome/settings/inbox/inbox.htm?acctEmail=${acctEmail}&threadId=${threadId1}`);
-        await inboxPage1.waitAll('iframe');
-        const attachmentFrame1 = await inboxPage1.getFrame(['attachment.htm']);
-        await attachmentFrame1.waitForSelTestState('ready');
-        await attachmentFrame1.waitForContent('@attachment-name', 'noname');
-        await attachmentFrame1.waitForContent('@container-attachment-header', 'PLAIN FILE');
+        await inboxPage1.notPresent('iframe.pgp_block');
+        await inboxPage1.notPresent('@container-attachments');
         const inboxPage2 = await browser.newExtensionPage(t, `chrome/settings/inbox/inbox.htm?acctEmail=${acctEmail}&threadId=${threadId2}`);
-        await inboxPage2.waitAll('iframe');
-        const attachmentFrame2 = await inboxPage2.getFrame(['attachment.htm']);
-        await attachmentFrame2.waitForSelTestState('ready');
-        await attachmentFrame2.waitForContent('@attachment-name', 'noname');
-        await attachmentFrame2.waitForContent('@container-attachment-header', 'PLAIN FILE');
+        await inboxPage2.notPresent('iframe.pgp_block');
+        await inboxPage2.notPresent('@container-attachments');
+        const inboxPage3 = await browser.newExtensionPage(t, `chrome/settings/inbox/inbox.htm?acctEmail=${acctEmail}&threadId=${threadId3}`);
+        const pgpBlock = await inboxPage3.getFrame(['pgp_block.htm']);
+        await inboxPage3.notPresent('@container-attachments');
+        expect(await inboxPage3.isElementPresent('iframe.pgp_block')).to.equal(true);
+        expect(await pgpBlock.isElementPresent('@pgp-encryption')).to.equal(true);
       })
     );
 

test/source/tests/gmail.ts

@@ -563,6 +563,8 @@ export const defineGmailTests = (testVariant: TestVariant, testWithBrowser: Test
         await gmailPage.waitAll(['.aZo'], { visible: false });
         const urls = await gmailPage.getFramesUrls(['/chrome/elements/attachment.htm']);
         expect(urls.length).to.equal(2);
+        expect(await gmailPage.waitForContent('.aVW span:first-child', '2'));
+        expect(await gmailPage.waitForContent('.aVW span.a2H', ' •  Scanned by Gmail'));
         await gmailPage.close();
       })
     );
@@ -591,6 +593,7 @@ export const defineGmailTests = (testVariant: TestVariant, testWithBrowser: Test
         const urls = await gmailPage.getFramesUrls(['/chrome/elements/pgp_pubkey.htm']);
         expect(urls.length).to.equal(1);
         await pageHasSecureReplyContainer(t, browser, gmailPage);
+        expect(await gmailPage.isElementVisible('.aVW')).to.equal(false); // attachment label count should be hidden
         expect(await gmailPage.isElementVisible('.aQH')).to.equal(false); // original attachment container(s) should be hidden
       })
     );