Skip to content

Commit feebc08

Browse files
fix: signature accidentally attaches to email
1 parent 63d200d commit feebc08

4 files changed

Lines changed: 80 additions & 6 deletions

File tree

extension/chrome/elements/compose-modules/compose-input-module.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ export class ComposeInputModule extends ViewModule<ComposeView> {
6767
public extract = (type: 'text' | 'html', elSel: 'input_text' | 'input_intro', flag?: 'SKIP-ADDONS') => {
6868
let html = this.view.S.cached(elSel)[0].innerHTML;
6969
if (elSel === 'input_text' && flag !== 'SKIP-ADDONS') {
70-
html += this.view.quoteModule.getTripleDotSanitizedFormattedHtmlContent();
70+
// skipFooter: true because footer is already rendered in input when user changes sender alias (issue #6135)
71+
html += this.view.quoteModule.getTripleDotSanitizedFormattedHtmlContent(true);
7172
}
7273
if (type === 'html') {
7374
return Xss.htmlSanitizeKeepBasicTags(html, 'IMG-KEEP');

extension/chrome/elements/compose-modules/compose-quote-module.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,24 @@ export class ComposeQuoteModule extends ViewModule<ComposeView> {
2323
public tripleDotSanitizedHtmlContent: { quote: string | undefined; footer: string | undefined } | undefined;
2424
public messageToReplyOrForward: MessageToReplyOrForward | undefined;
2525

26-
public getTripleDotSanitizedFormattedHtmlContent = (): string => {
27-
// email content order: [myMsg, myFooter, theirQuote]
28-
if (this.tripleDotSanitizedHtmlContent) {
29-
return '<br />' + (this.tripleDotSanitizedHtmlContent.footer || '') + (this.tripleDotSanitizedHtmlContent.quote || '');
26+
/**
27+
* Returns the formatted HTML content for the triple-dot expandable section.
28+
* Email content order: [myMsg, myFooter, theirQuote]
29+
*
30+
* @param skipFooter - If true, skip including the footer when there's no quote.
31+
* Used to prevent duplicate signatures when footer was already
32+
* rendered in input (issue #6135).
33+
*/
34+
public getTripleDotSanitizedFormattedHtmlContent = (skipFooter = false): string => {
35+
if (!this.tripleDotSanitizedHtmlContent) {
36+
return '';
3037
}
31-
return '';
38+
const { footer, quote } = this.tripleDotSanitizedHtmlContent;
39+
// When skipFooter is true and there's no quote, return empty to avoid duplicate footer
40+
if (skipFooter && !quote) {
41+
return '';
42+
}
43+
return '<br />' + (footer || '') + (quote || '');
3244
};
3345

3446
public addSignatureToInput = async () => {

test/source/mock/google/strategies/send-message-strategy.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,11 +318,32 @@ class PlainTextMessageTestStrategy implements ITestMsgStrategy {
318318
};
319319
}
320320

321+
321322
class NoopTestStrategy implements ITestMsgStrategy {
322323
// eslint-disable-next-line @typescript-eslint/no-empty-function
323324
public test = async () => {};
324325
}
325326

327+
class SignatureRemovalTestStrategy implements ITestMsgStrategy {
328+
private readonly signature = 'Test alias signature';
329+
330+
public test = async (parseResult: ParseMsgResult) => {
331+
const mimeMsg = parseResult.mimeMsg;
332+
const kisWithPp = await Config.getKeyInfo(['flowcrypt.compatibility.1pp1', 'flowcrypt.compatibility.2pp1']);
333+
const encryptedData = mimeMsg.text!;
334+
const decrypted = await MsgUtil.decryptMessage({ kisWithPp, encryptedData, verificationPubs: [] });
335+
expect(decrypted.success).to.be.true;
336+
// We expect the body to contain the message text but NOT the signature
337+
const body = decrypted.content?.toUtfStr() || '';
338+
if (!body.includes('Message without signature')) {
339+
throw new HttpClientErr(`Error: Msg Text doesn't contain expected body. Current: '${body}'`);
340+
}
341+
if (body.includes(this.signature)) {
342+
throw new HttpClientErr(`Error: Msg Text contains signature that should have been removed. Current: '${body}'`);
343+
}
344+
};
345+
}
346+
326347
class IncludeQuotedPartTestStrategy implements ITestMsgStrategy {
327348
private readonly quotedContent: string = [
328349
'On 2019-06-14 at 23:24, flowcrypt.compatibility@gmail.com wrote:',
@@ -484,6 +505,8 @@ export class TestBySubjectStrategyContext {
484505
this.strategy = new SaveMessageInStorageStrategy();
485506
} else if (subject.includes('Test Sending Message With Attachment Which Contains Emoji in Filename')) {
486507
this.strategy = new SaveMessageInStorageStrategy();
508+
} else if (subject.includes('Message without signature')) {
509+
this.strategy = new SignatureRemovalTestStrategy();
487510
} else if (subject.includes('Re: FROM: flowcrypt.compatibility@gmail.com, TO: flowcrypt.compatibility@gmail.com + vladimir@flowcrypt.com')) {
488511
this.strategy = new NoopTestStrategy();
489512
} else {

test/source/tests/compose.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,44 @@ export const defineComposeTests = (testVariant: TestVariant, testWithBrowser: Te
104104
})
105105
);
106106

107+
test.only(
108+
'compose - signature must not reappear after manual removal (issue #6135)',
109+
testWithBrowser(async (t, browser) => {
110+
const primarySignature = 'Test primary signature';
111+
const aliasSignature = 'Test alias signature';
112+
const acctAliases = [{
113+
...flowcryptCompatibilityAliasList[0],
114+
signature: aliasSignature,
115+
}];
116+
await BrowserRecipe.setupCommonAcctWithAttester(t, browser, 'compatibility', {
117+
google: { acctAliases, acctPrimarySignature: primarySignature },
118+
attester: { includeHumanKey: true },
119+
});
120+
121+
const composePage = await ComposePageRecipe.openStandalone(t, browser, 'compatibility');
122+
// Select alias email to trigger signature change
123+
await ComposePageRecipe.selectFromOption(composePage, 'flowcryptcompatibility@gmail.com');
124+
let emailBody = await composePage.read('@input-body');
125+
expect(emailBody).to.contain(aliasSignature);
126+
127+
// meaningful user change: delete the signature
128+
// clearInput is not available, so we select all and delete
129+
await composePage.waitAndClick('@input-body');
130+
await composePage.page.keyboard.down('Control');
131+
await composePage.page.keyboard.press('a');
132+
await composePage.page.keyboard.up('Control');
133+
await composePage.page.keyboard.press('Backspace');
134+
await ComposePageRecipe.fillMsg(composePage, { to: 'human@flowcrypt.com' }, 'Message without signature', 'Message without signature');
135+
136+
// Verify signature is gone before sending
137+
emailBody = await composePage.read('@input-body');
138+
expect(emailBody).to.not.contain(aliasSignature);
139+
140+
// Send
141+
await ComposePageRecipe.sendAndClose(composePage);
142+
})
143+
);
144+
107145
test(
108146
'compose - check for sender [flowcrypt.compatibility@gmail.com] from a password-protected email',
109147
testWithBrowser(async (t, browser) => {

0 commit comments

Comments
 (0)