diff --git a/shortcuts/mail/draft/htmltext.go b/shortcuts/mail/draft/htmltext.go index 0ef9c17de..fbaa585a8 100644 --- a/shortcuts/mail/draft/htmltext.go +++ b/shortcuts/mail/draft/htmltext.go @@ -123,6 +123,13 @@ func isHTMLBlockBoundary(n *xhtml.Node) bool { } } +// PlainTextFromHTML is the exported wrapper over plainTextFromHTML, so the +// mail package can render an HTML signature as a plain-text fallback when a +// message body is sent in plain-text mode. The conversion logic is unchanged. +func PlainTextFromHTML(raw string) string { + return plainTextFromHTML(raw) +} + // bodyLooksLikeHTML reports whether raw appears to contain HTML markup. // This is intentionally heuristic: it exists to reject obvious plain-text // input when a draft's authored body is text/html. diff --git a/shortcuts/mail/draft/htmltext_test.go b/shortcuts/mail/draft/htmltext_test.go index f640016b5..8d7e1bb0a 100644 --- a/shortcuts/mail/draft/htmltext_test.go +++ b/shortcuts/mail/draft/htmltext_test.go @@ -102,3 +102,10 @@ func TestIsHTMLNonTextTag(t *testing.T) { }) } } + +func TestPlainTextFromHTMLExported(t *testing.T) { + got := PlainTextFromHTML("
Hello world
") + if !strings.Contains(got, "Hello world") { + t.Fatalf("PlainTextFromHTML: expected to contain \"Hello world\", got %q", got) + } +} diff --git a/shortcuts/mail/mail_draft_create.go b/shortcuts/mail/mail_draft_create.go index 8f4875c8b..316029193 100644 --- a/shortcuts/mail/mail_draft_create.go +++ b/shortcuts/mail/mail_draft_create.go @@ -56,6 +56,7 @@ var MailDraftCreate = common.Shortcut{ {Name: "request-receipt", Type: "bool", Desc: "Request a read receipt (Message Disposition Notification, RFC 3798) addressed to the sender. Recipient mail clients may prompt the user, send automatically, or silently ignore — delivery of a receipt is not guaranteed."}, {Name: "template-id", Desc: "Optional. Apply a saved template by ID (decimal integer string) before composing. The template's subject/body/to/cc/bcc/attachments are merged with user-supplied flags (user flags win). Requires --as user."}, signatureFlag, + noSignatureFlag, priorityFlag, eventSummaryFlag, eventStartFlag, eventEndFlag, eventLocationFlag, showLintDetailsFlag, @@ -92,7 +93,7 @@ var MailDraftCreate = common.Shortcut{ if !hasTemplate && strings.TrimSpace(runtime.Str("subject")) == "" { return mailValidationParamError("--subject", "--subject is required; pass the final email subject (or use --template-id)") } - if err := validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")); err != nil { + if err := validateNoSignatureConflict(runtime.Bool("no-signature"), runtime.Str("signature-id")); err != nil { return err } if err := validateEventFlags(runtime); err != nil { @@ -180,12 +181,22 @@ var MailDraftCreate = common.Shortcut{ if strings.TrimSpace(input.Body) == "" { return mailValidationParamError("--body", "effective body is empty after applying template; pass --body explicitly") } - sigResult, err := resolveSignature(ctx, runtime, mailboxID, runtime.Str("signature-id"), runtime.Str("from")) + signatureID := runtime.Str("signature-id") + noSignature := runtime.Bool("no-signature") + senderEmail := resolveComposeSenderEmail(runtime) + // Auto-resolve default signature when neither --no-signature nor --signature-id is set. + if noSignature { + signatureID = "" + } else if signatureID == "" { + signatureID = autoResolveSignatureID(runtime, mailboxID, senderEmail, false) + } + sigResult, err := resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail, + runtime.Str("signature-id") != "", !input.PlainText) if err != nil { return err } rawEML, lintApplied, lintBlocked, err := buildRawEMLForDraftCreate(ctx, runtime, input, sigResult, priority, - templateLargeAttachmentIDs, mailboxID, templateID, templateInlineAttachments, templateSmallAttachments) + templateLargeAttachmentIDs, mailboxID, templateID, templateInlineAttachments, templateSmallAttachments, senderEmail) if err != nil { return err } @@ -241,13 +252,19 @@ func buildRawEMLForDraftCreate( mailboxID, templateID string, templateInlineAttachments []templateInlineRef, templateSmallAttachments []templateAttachmentRef, + senderEmailHint string, ) (rawEMLOut string, lintApplied, lintBlocked []lint.Finding, err error) { // Initialise lint findings as empty (non-nil) slices so callers can // surface them through the envelope unconditionally even on the // plain-text branch. lintApplied, lintBlocked = emptyLintFindings() - senderEmail := resolveComposeSenderEmail(runtime) + // Use the pre-resolved senderEmail when available (avoids a duplicate + // profile API call when Execute already fetched it for auto-resolve). + senderEmail := senderEmailHint + if senderEmail == "" { + senderEmail = resolveComposeSenderEmail(runtime) + } if senderEmail == "" { return "", lintApplied, lintBlocked, mailValidationParamError("--from", "unable to determine sender email; please specify --from explicitly") } @@ -290,7 +307,7 @@ func buildRawEMLForDraftCreate( var composedHTMLBody string var composedTextBody string if input.PlainText { - composedTextBody = input.Body + composedTextBody = injectPlainTextSignature(input.Body, sigResult) bld = bld.TextBody([]byte(composedTextBody)) } else if bodyIsHTML(input.Body) || sigResult != nil { htmlBody := input.Body diff --git a/shortcuts/mail/mail_draft_create_test.go b/shortcuts/mail/mail_draft_create_test.go index 21cbf9304..1b78e2535 100644 --- a/shortcuts/mail/mail_draft_create_test.go +++ b/shortcuts/mail/mail_draft_create_test.go @@ -62,7 +62,7 @@ func TestBuildRawEMLForDraftCreate_ResolvesLocalImages(t *testing.T) { Body: `Hello

Hello world
`, } - rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil) + rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil, "") if err != nil { t.Fatalf("buildRawEMLForDraftCreate() error = %v", err) } @@ -124,7 +124,7 @@ func TestBuildRawEMLForDraftCreate_AutoResolveCountedInSizeLimit(t *testing.T) { Attach: "./big.txt", } - _, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil) + _, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil, "") if err == nil { t.Fatal("expected size limit error when auto-resolved image + attachment exceed 25MB") } @@ -145,7 +145,7 @@ func TestBuildRawEMLForDraftCreate_OrphanedInlineSpecError(t *testing.T) { Inline: `[{"cid":"orphan","file_path":"./unused.png"}]`, } - _, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil) + _, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil, "") if err == nil { t.Fatal("expected error for orphaned --inline CID not referenced in body") } @@ -166,7 +166,7 @@ func TestBuildRawEMLForDraftCreate_MissingCIDRefError(t *testing.T) { Inline: `[{"cid":"present","file_path":"./present.png"}]`, } - _, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil) + _, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil, "") if err == nil { t.Fatal("expected error for missing CID reference") } @@ -183,7 +183,7 @@ func TestBuildRawEMLForDraftCreate_WithPriority(t *testing.T) { Body: `Hello
`, } - rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "1", nil, "", "", nil, nil) + rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "1", nil, "", "", nil, nil, "") if err != nil { t.Fatalf("buildRawEMLForDraftCreate() error = %v", err) } @@ -201,7 +201,7 @@ func TestBuildRawEMLForDraftCreate_NoPriority(t *testing.T) { Body: `Hello
`, } - rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil) + rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil, "") if err != nil { t.Fatalf("buildRawEMLForDraftCreate() error = %v", err) } @@ -237,7 +237,7 @@ func TestBuildRawEMLForDraftCreate_RequestReceiptAddsHeader(t *testing.T) { } rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), - newRuntimeWithFromAndRequestReceipt("sender@example.com", true), input, nil, "", nil, "", "", nil, nil) + newRuntimeWithFromAndRequestReceipt("sender@example.com", true), input, nil, "", nil, "", "", nil, nil, "") if err != nil { t.Fatalf("buildRawEMLForDraftCreate() error = %v", err) } @@ -260,7 +260,7 @@ func TestBuildRawEMLForDraftCreate_RequestReceiptOmittedByDefault(t *testing.T) } rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), - newRuntimeWithFromAndRequestReceipt("sender@example.com", false), input, nil, "", nil, "", "", nil, nil) + newRuntimeWithFromAndRequestReceipt("sender@example.com", false), input, nil, "", nil, "", "", nil, nil, "") if err != nil { t.Fatalf("buildRawEMLForDraftCreate() error = %v", err) } @@ -283,7 +283,7 @@ func TestBuildRawEMLForDraftCreate_PlainTextSkipsResolve(t *testing.T) { PlainText: true, } - rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil) + rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), newRuntimeWithFrom("sender@example.com"), input, nil, "", nil, "", "", nil, nil, "") if err != nil { t.Fatalf("buildRawEMLForDraftCreate() error = %v", err) } @@ -304,7 +304,7 @@ func TestBuildRawEMLForDraftCreate_WithCalendarEvent(t *testing.T) { Body: "Please join us
", } - rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), rt, input, nil, "", nil, "", "", nil, nil) + rawEML, _, _, err := buildRawEMLForDraftCreate(context.Background(), rt, input, nil, "", nil, "", "", nil, nil, "") if err != nil { t.Fatalf("buildRawEMLForDraftCreate() error = %v", err) } diff --git a/shortcuts/mail/mail_draft_edit.go b/shortcuts/mail/mail_draft_edit.go index 76a6f9f9c..0bfbf998a 100644 --- a/shortcuts/mail/mail_draft_edit.go +++ b/shortcuts/mail/mail_draft_edit.go @@ -134,7 +134,7 @@ var MailDraftEdit = common.Shortcut{ for i := range patch.Ops { switch patch.Ops[i].Op { case "insert_signature": - sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, patch.Ops[i].SignatureID, draftFromEmail) + sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, patch.Ops[i].SignatureID, draftFromEmail, true, true) if sigErr != nil { return sigErr } diff --git a/shortcuts/mail/mail_forward.go b/shortcuts/mail/mail_forward.go index 25a0ff0ad..20a14577a 100644 --- a/shortcuts/mail/mail_forward.go +++ b/shortcuts/mail/mail_forward.go @@ -45,6 +45,7 @@ var MailForward = common.Shortcut{ {Name: "subject", Desc: "Optional. Override the auto-generated Fw: subject. When set, the shortcut uses this value verbatim instead of prefixing the original subject."}, {Name: "template-id", Desc: "Optional. Apply a saved template by ID (decimal integer string) before composing. The template's body/to/cc/bcc/attachments are merged into the forward draft (template values appended to user flags / forward-derived values; no de-duplication)."}, signatureFlag, + noSignatureFlag, priorityFlag, eventSummaryFlag, eventStartFlag, eventEndFlag, eventLocationFlag, showLintDetailsFlag}, @@ -96,7 +97,7 @@ var MailForward = common.Shortcut{ return err } } - if err := validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")); err != nil { + if err := validateNoSignatureConflict(runtime.Bool("no-signature"), runtime.Str("signature-id")); err != nil { return err } if err := validateEventFlags(runtime); err != nil { @@ -127,12 +128,7 @@ var MailForward = common.Shortcut{ return err } - signatureID := runtime.Str("signature-id") mailboxID := resolveComposeMailboxID(runtime) - sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, signatureID, runtime.Str("from")) - if sigErr != nil { - return sigErr - } sourceMsg, err := fetchComposeSourceMessage(runtime, mailboxID, messageId) if err != nil { return mailDecorateProblemMessage(err, "failed to fetch original message") @@ -156,6 +152,18 @@ var MailForward = common.Shortcut{ senderEmail = orig.headTo } + // Signature ID is resolved here (after senderEmail is finalised) so DefaultReplyID + // matches the correct usage. The actual image download in resolveSignature is deferred + // to after applyTemplate so the final plainText value (which a template can override + // via IsPlainTextMode) is used for the downloadImages decision. + signatureID := runtime.Str("signature-id") + noSignature := runtime.Bool("no-signature") + if noSignature { + signatureID = "" + } else if signatureID == "" { + signatureID = autoResolveSignatureID(runtime, mailboxID, senderEmail, true /*isReply*/) + } + // --template-id merge (§5.5 Q1-Q5). var templateLargeAttachmentIDs []string var templateInlineAttachments []templateInlineRef @@ -198,6 +206,14 @@ var MailForward = common.Shortcut{ "bccs_count": countAddresses(bccFlag), }) } + // Resolve signature after template processing so plainText reflects any IsPlainTextMode + // override from the template. This avoids downloading HTML signature images when the + // template forces plain-text mode, which could cause CDN 403/5xx or timeout errors. + sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail, + runtime.Str("signature-id") != "", !plainText) + if sigErr != nil { + return sigErr + } subjectOverride := strings.TrimSpace(runtime.Str("subject")) // Post-merge recipient check for --confirm-send + --template-id: @@ -310,7 +326,7 @@ var MailForward = common.Shortcut{ return err } } else { - composedTextBody = buildForwardedMessage(&orig, body) + composedTextBody = buildForwardedMessage(&orig, injectPlainTextSignature(body, sigResult)) bld = bld.TextBody([]byte(composedTextBody)) } // Embed template SMALL non-inline attachments regardless of body mode. diff --git a/shortcuts/mail/mail_reply.go b/shortcuts/mail/mail_reply.go index d4f190894..11acea385 100644 --- a/shortcuts/mail/mail_reply.go +++ b/shortcuts/mail/mail_reply.go @@ -42,6 +42,7 @@ var MailReply = common.Shortcut{ {Name: "subject", Desc: "Optional. Override the auto-generated Re: subject. When set, the shortcut uses this value verbatim instead of prefixing the original subject."}, {Name: "template-id", Desc: "Optional. Apply a saved template by ID (decimal integer string) before composing. The template's body/to/cc/bcc/attachments are appended to the reply-derived values (no de-duplication; see warning in Execute output)."}, signatureFlag, + noSignatureFlag, priorityFlag, eventSummaryFlag, eventStartFlag, eventEndFlag, eventLocationFlag, showLintDetailsFlag}, @@ -93,7 +94,7 @@ var MailReply = common.Shortcut{ if err := validateSendTime(runtime); err != nil { return err } - if err := validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")); err != nil { + if err := validateNoSignatureConflict(runtime.Bool("no-signature"), runtime.Str("signature-id")); err != nil { return err } if err := validateEventFlags(runtime); err != nil { @@ -129,12 +130,7 @@ var MailReply = common.Shortcut{ return err } - signatureID := runtime.Str("signature-id") mailboxID := resolveComposeMailboxID(runtime) - sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, signatureID, runtime.Str("from")) - if sigErr != nil { - return sigErr - } sourceMsg, err := fetchComposeSourceMessage(runtime, mailboxID, messageId) if err != nil { return mailDecorateProblemMessage(err, "failed to fetch original message") @@ -156,6 +152,18 @@ var MailReply = common.Shortcut{ senderEmail = orig.headTo } + // Signature ID is resolved here (after senderEmail is finalised) so DefaultReplyID + // matches the correct usage. The actual image download in resolveSignature is deferred + // to after applyTemplate so the final plainText value (which a template can override + // via IsPlainTextMode) is used for the downloadImages decision. + signatureID := runtime.Str("signature-id") + noSignature := runtime.Bool("no-signature") + if noSignature { + signatureID = "" + } else if signatureID == "" { + signatureID = autoResolveSignatureID(runtime, mailboxID, senderEmail, true /*isReply*/) + } + replyTo := orig.replyTo if replyTo == "" { replyTo = orig.headFrom @@ -208,6 +216,14 @@ var MailReply = common.Shortcut{ "bccs_count": countAddresses(bccFlag), }) } + // Resolve signature after template processing so plainText reflects any IsPlainTextMode + // override from the template. This avoids downloading HTML signature images when the + // template forces plain-text mode, which could cause CDN 403/5xx or timeout errors. + sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail, + runtime.Str("signature-id") != "", !plainText) + if sigErr != nil { + return sigErr + } // --subject (explicit override) takes precedence over auto-generated. subjectOverride := strings.TrimSpace(runtime.Str("subject")) @@ -311,7 +327,7 @@ var MailReply = common.Shortcut{ return err } } else { - composedTextBody = bodyStr + quoted + composedTextBody = injectPlainTextSignature(bodyStr, sigResult) + quoted bld = bld.TextBody([]byte(composedTextBody)) } // Embed template SMALL non-inline attachments regardless of body mode. diff --git a/shortcuts/mail/mail_reply_all.go b/shortcuts/mail/mail_reply_all.go index 66f584e1f..3c2d19bef 100644 --- a/shortcuts/mail/mail_reply_all.go +++ b/shortcuts/mail/mail_reply_all.go @@ -43,6 +43,7 @@ var MailReplyAll = common.Shortcut{ {Name: "subject", Desc: "Optional. Override the auto-generated Re: subject. When set, the shortcut uses this value verbatim instead of prefixing the original subject."}, {Name: "template-id", Desc: "Optional. Apply a saved template by ID (decimal integer string) before composing. The template's body/to/cc/bcc/attachments are appended to the reply-derived values (no de-duplication; see warning in Execute output)."}, signatureFlag, + noSignatureFlag, priorityFlag, eventSummaryFlag, eventStartFlag, eventEndFlag, eventLocationFlag, showLintDetailsFlag}, @@ -94,7 +95,7 @@ var MailReplyAll = common.Shortcut{ if err := validateSendTime(runtime); err != nil { return err } - if err := validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")); err != nil { + if err := validateNoSignatureConflict(runtime.Bool("no-signature"), runtime.Str("signature-id")); err != nil { return err } if err := validateEventFlags(runtime); err != nil { @@ -131,12 +132,7 @@ var MailReplyAll = common.Shortcut{ return err } - signatureID := runtime.Str("signature-id") mailboxID := resolveComposeMailboxID(runtime) - sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, signatureID, runtime.Str("from")) - if sigErr != nil { - return sigErr - } sourceMsg, err := fetchComposeSourceMessage(runtime, mailboxID, messageId) if err != nil { return mailDecorateProblemMessage(err, "failed to fetch original message") @@ -158,6 +154,18 @@ var MailReplyAll = common.Shortcut{ senderEmail = orig.headTo } + // Signature ID is resolved here (after senderEmail is finalised) so DefaultReplyID + // matches the correct usage. The actual image download in resolveSignature is deferred + // to after applyTemplate so the final plainText value (which a template can override + // via IsPlainTextMode) is used for the downloadImages decision. + signatureID := runtime.Str("signature-id") + noSignature := runtime.Bool("no-signature") + if noSignature { + signatureID = "" + } else if signatureID == "" { + signatureID = autoResolveSignatureID(runtime, mailboxID, senderEmail, true /*isReply*/) + } + var removeList []string for _, r := range strings.Split(removeFlag, ",") { if s := strings.TrimSpace(r); s != "" { @@ -218,6 +226,14 @@ var MailReplyAll = common.Shortcut{ "bccs_count": countAddresses(bccFlag), }) } + // Resolve signature after template processing so plainText reflects any IsPlainTextMode + // override from the template. This avoids downloading HTML signature images when the + // template forces plain-text mode, which could cause CDN 403/5xx or timeout errors. + sigResult, sigErr := resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail, + runtime.Str("signature-id") != "", !plainText) + if sigErr != nil { + return sigErr + } subjectOverride := strings.TrimSpace(runtime.Str("subject")) if err := validateRecipientCount(toList, ccList, bccFlag); err != nil { @@ -316,7 +332,7 @@ var MailReplyAll = common.Shortcut{ return err } } else { - composedTextBody = bodyStr + quoted + composedTextBody = injectPlainTextSignature(bodyStr, sigResult) + quoted bld = bld.TextBody([]byte(composedTextBody)) } // Embed template SMALL non-inline attachments regardless of body mode. diff --git a/shortcuts/mail/mail_send.go b/shortcuts/mail/mail_send.go index b536aa827..fde49ac40 100644 --- a/shortcuts/mail/mail_send.go +++ b/shortcuts/mail/mail_send.go @@ -40,6 +40,7 @@ var MailSend = common.Shortcut{ {Name: "request-receipt", Type: "bool", Desc: "Request a read receipt (Message Disposition Notification, RFC 3798) addressed to the sender. Recipient mail clients may prompt the user, send automatically, or silently ignore — delivery of a receipt is not guaranteed."}, {Name: "template-id", Desc: "Optional. Apply a saved template by ID (decimal integer string) before composing. The template's subject/body/to/cc/bcc/attachments are merged with user-supplied flags (user flags win). Requires --as user."}, signatureFlag, + noSignatureFlag, priorityFlag, eventSummaryFlag, eventStartFlag, eventEndFlag, eventLocationFlag, showLintDetailsFlag}, @@ -98,7 +99,7 @@ var MailSend = common.Shortcut{ if err := validateSendTime(runtime); err != nil { return err } - if err := validateSignatureWithPlainText(runtime.Bool("plain-text"), runtime.Str("signature-id")); err != nil { + if err := validateNoSignatureConflict(runtime.Bool("no-signature"), runtime.Str("signature-id")); err != nil { return err } // Resolve the body content first (reading --body-file if set) so @@ -145,6 +146,14 @@ var MailSend = common.Shortcut{ mailboxID := resolveComposeMailboxID(runtime) + // Auto-resolve default signature when neither --no-signature nor --signature-id is set. + noSignature := runtime.Bool("no-signature") + if noSignature { + signatureID = "" + } else if signatureID == "" { + signatureID = autoResolveSignatureID(runtime, mailboxID, senderEmail, false) + } + // --template-id merge: fetch template and apply it to compose state. var templateLargeAttachmentIDs []string var templateInlineAttachments []templateInlineRef @@ -195,7 +204,8 @@ var MailSend = common.Shortcut{ } } - sigResult, err := resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail) + sigResult, err := resolveSignature(ctx, runtime, mailboxID, signatureID, senderEmail, + runtime.Str("signature-id") != "", !plainText) if err != nil { return err } @@ -230,7 +240,7 @@ var MailSend = common.Shortcut{ // `lint_applied[]` / `original_blocked[]` even on the plain-text path. lintApplied, lintBlocked := emptyLintEnvelopeFields() if plainText { - composedTextBody = body + composedTextBody = injectPlainTextSignature(body, sigResult) bld = bld.TextBody([]byte(composedTextBody)) } else if bodyIsHTML(body) || sigResult != nil { // If signature is requested on plain-text body, auto-upgrade to HTML. diff --git a/shortcuts/mail/signature/provider.go b/shortcuts/mail/signature/provider.go index b5a807cc2..d07c0b726 100644 --- a/shortcuts/mail/signature/provider.go +++ b/shortcuts/mail/signature/provider.go @@ -6,6 +6,7 @@ package signature import ( "encoding/json" "net/url" + "strings" "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" @@ -55,6 +56,33 @@ func List(runtime *common.RuntimeContext, mailboxID string) ([]Signature, error) return resp.Signatures, nil } +// DefaultSendID returns the default send-mail signature ID for the given +// sender email address. Returns "" if no default is configured. +// "0" and empty string are treated as "no default" (API convention). +func DefaultSendID(usages []SignatureUsage, emailAddr string) string { + for _, u := range usages { + if strings.EqualFold(u.EmailAddress, emailAddr) { + if u.SendMailSignatureID != "" && u.SendMailSignatureID != "0" { + return u.SendMailSignatureID + } + } + } + return "" +} + +// DefaultReplyID returns the default reply/forward signature ID for the given +// sender email address. Returns "" if no default is configured. +func DefaultReplyID(usages []SignatureUsage, emailAddr string) string { + for _, u := range usages { + if strings.EqualFold(u.EmailAddress, emailAddr) { + if u.ReplySignatureID != "" && u.ReplySignatureID != "0" { + return u.ReplySignatureID + } + } + } + return "" +} + // Get returns a single signature by ID. Returns an error if not found. func Get(runtime *common.RuntimeContext, mailboxID, signatureID string) (*Signature, error) { resp, err := ListAll(runtime, mailboxID) diff --git a/shortcuts/mail/signature/provider_test.go b/shortcuts/mail/signature/provider_test.go new file mode 100644 index 000000000..bc30213d6 --- /dev/null +++ b/shortcuts/mail/signature/provider_test.go @@ -0,0 +1,92 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package signature + +import ( + "testing" +) + +func TestDefaultSendID_Match(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "user@example.com", SendMailSignatureID: "sig-send-1", ReplySignatureID: "sig-reply-1"}, + } + if got := DefaultSendID(usages, "user@example.com"); got != "sig-send-1" { + t.Fatalf("expected sig-send-1, got %q", got) + } +} + +func TestDefaultSendID_CaseInsensitive(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "User@Example.COM", SendMailSignatureID: "sig-send-x"}, + } + if got := DefaultSendID(usages, "user@example.com"); got != "sig-send-x" { + t.Fatalf("expected case-insensitive match, got %q", got) + } +} + +func TestDefaultSendID_NoMatch(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "other@example.com", SendMailSignatureID: "sig-other"}, + } + if got := DefaultSendID(usages, "user@example.com"); got != "" { + t.Fatalf("expected empty string for no match, got %q", got) + } +} + +func TestDefaultSendID_ZeroIDTreatedAsNone(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "user@example.com", SendMailSignatureID: "0"}, + } + if got := DefaultSendID(usages, "user@example.com"); got != "" { + t.Fatalf("expected empty string for ID=0, got %q", got) + } +} + +func TestDefaultSendID_NilUsages(t *testing.T) { + if got := DefaultSendID(nil, "user@example.com"); got != "" { + t.Fatalf("expected empty string for nil usages, got %q", got) + } +} + +func TestDefaultReplyID_Match(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "user@example.com", SendMailSignatureID: "sig-send-1", ReplySignatureID: "sig-reply-2"}, + } + if got := DefaultReplyID(usages, "user@example.com"); got != "sig-reply-2" { + t.Fatalf("expected sig-reply-2, got %q", got) + } +} + +func TestDefaultReplyID_CaseInsensitive(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "User@Example.COM", ReplySignatureID: "sig-reply-x"}, + } + if got := DefaultReplyID(usages, "user@example.com"); got != "sig-reply-x" { + t.Fatalf("expected case-insensitive match, got %q", got) + } +} + +func TestDefaultReplyID_NoMatch(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "other@example.com", ReplySignatureID: "sig-reply-other"}, + } + if got := DefaultReplyID(usages, "user@example.com"); got != "" { + t.Fatalf("expected empty string for no match, got %q", got) + } +} + +func TestDefaultReplyID_ZeroIDTreatedAsNone(t *testing.T) { + usages := []SignatureUsage{ + {EmailAddress: "user@example.com", ReplySignatureID: "0"}, + } + if got := DefaultReplyID(usages, "user@example.com"); got != "" { + t.Fatalf("expected empty string for ID=0, got %q", got) + } +} + +func TestDefaultReplyID_NilUsages(t *testing.T) { + if got := DefaultReplyID(nil, "user@example.com"); got != "" { + t.Fatalf("expected empty string for nil usages, got %q", got) + } +} diff --git a/shortcuts/mail/signature_compose.go b/shortcuts/mail/signature_compose.go index ac7f13cde..f41a1ebb8 100644 --- a/shortcuts/mail/signature_compose.go +++ b/shortcuts/mail/signature_compose.go @@ -5,6 +5,7 @@ package mail import ( "context" + "fmt" "io" "net/http" "net/url" @@ -25,6 +26,54 @@ var signatureFlag = common.Flag{ Desc: "Optional. Signature ID to append after body content. Run `mail +signature` to list available signatures.", } +// noSignatureFlag is shared by all 5 compose shortcuts. +var noSignatureFlag = common.Flag{ + Name: "no-signature", + Type: "bool", + Desc: "Skip automatic default signature insertion. Mutually exclusive with --signature-id.", +} + +// validateNoSignatureConflict returns a structured validation error when +// --no-signature and --signature-id are both set; they are mutually exclusive. +func validateNoSignatureConflict(noSignature bool, signatureID string) error { + if noSignature && signatureID != "" { + return mailValidationParamError("--no-signature", "--no-signature and --signature-id are mutually exclusive") + } + return nil +} + +// autoResolveSignatureID resolves the default signature ID for the given mailbox/sender. +// isReply=true uses DefaultReplyID (+reply/+reply-all/+forward); +// isReply=false uses DefaultSendID (+send/+draft-create). +// Returns "" on API failure (writes stderr warning) or when no default is configured. +func autoResolveSignatureID(runtime *common.RuntimeContext, mailboxID, senderEmail string, isReply bool) string { + resp, err := signature.ListAll(runtime, mailboxID) + if err != nil { + fmt.Fprintf(runtime.IO().ErrOut, + "warning: failed to fetch default signature: %v; sending without signature\n", err) + return "" + } + if isReply { + return signature.DefaultReplyID(resp.Usages, senderEmail) + } + return signature.DefaultSendID(resp.Usages, senderEmail) +} + +// injectPlainTextSignature appends a plain-text rendering of the signature to a +// plain-text body. The HTML signature (sig.RenderedContent) is converted via +// draftpkg.PlainTextFromHTML; inline images are dropped (plain text has none). +// Returns textBody unchanged when sig is nil. +func injectPlainTextSignature(textBody string, sig *signatureResult) string { + if sig == nil { + return textBody + } + sigText := strings.TrimRight(draftpkg.PlainTextFromHTML(sig.RenderedContent), "\n") + if sigText == "" { + return textBody + } + return textBody + "\n\n" + sigText +} + // signatureResult holds the pre-processed signature data ready for HTML injection. type signatureResult struct { ID string @@ -32,16 +81,32 @@ type signatureResult struct { Images []draftpkg.SignatureImage } -// resolveSignature fetches, interpolates, and downloads images for a signature. +// resolveSignature fetches, interpolates, and optionally downloads images for a signature. // fromEmail is the --from address (may be an alias); used to match the correct // sender identity for template interpolation. Pass "" to use the primary address. -func resolveSignature(ctx context.Context, runtime *common.RuntimeContext, mailboxID, signatureID, fromEmail string) (*signatureResult, error) { +// +// userExplicit must be true when the caller obtained signatureID from a user-supplied flag +// (--signature-id); false when the ID was auto-resolved from default usages. When false, +// a "not found" error from the signatures API is treated as graceful degradation (no +// signature) rather than a hard failure — this protects against stale default IDs. +// +// includeImages controls whether inline image attachments are downloaded. Pass false for +// plain-text compose paths to avoid unnecessary network I/O (images are discarded in +// plain-text mode anyway). +func resolveSignature(ctx context.Context, runtime *common.RuntimeContext, mailboxID, signatureID, fromEmail string, userExplicit, includeImages bool) (*signatureResult, error) { if signatureID == "" { return nil, nil } sig, err := signature.Get(runtime, mailboxID, signatureID) if err != nil { + if !userExplicit && errs.IsValidation(err) { + // Stale auto-resolved default signature ID — degrade gracefully instead of + // blocking the entire send/reply/forward operation. + fmt.Fprintf(runtime.IO().ErrOut, + "warning: default signature %q not found in current list; sending without signature\n", signatureID) + return nil, nil + } return nil, err } @@ -50,23 +115,26 @@ func resolveSignature(ctx context.Context, runtime *common.RuntimeContext, mailb senderName, senderEmail := resolveSenderInfo(runtime, mailboxID, fromEmail) rendered := signature.InterpolateTemplate(sig, lang, senderName, senderEmail) - // Download signature inline images. The file_key field contains a - // direct download URL provided by the mail backend. + // Download signature inline images only when the compose path needs them. + // Plain-text paths discard images, so skip the download to avoid unnecessary + // network I/O (and potential failures from expired pre-signed URLs). var images []draftpkg.SignatureImage - for _, img := range sig.Images { - if img.DownloadURL == "" || img.CID == "" { - continue + if includeImages { + for _, img := range sig.Images { + if img.DownloadURL == "" || img.CID == "" { + continue + } + data, ct, err := downloadSignatureImage(runtime, img.DownloadURL, img.ImageName) + if err != nil { + return nil, mailDecorateProblemMessage(err, "failed to download signature image %s", img.ImageName) + } + images = append(images, draftpkg.SignatureImage{ + CID: img.CID, + ContentType: ct, + FileName: img.ImageName, + Data: data, + }) } - data, ct, err := downloadSignatureImage(runtime, img.DownloadURL, img.ImageName) - if err != nil { - return nil, mailDecorateProblemMessage(err, "failed to download signature image %s", img.ImageName) - } - images = append(images, draftpkg.SignatureImage{ - CID: img.CID, - ContentType: ct, - FileName: img.ImageName, - Data: data, - }) } return &signatureResult{ @@ -243,15 +311,3 @@ func signatureCIDs(sig *signatureResult) []string { } return cids } - -// validateSignatureWithPlainText returns an error if both --plain-text and --signature-id are set. -func validateSignatureWithPlainText(plainText bool, signatureID string) error { - if plainText && signatureID != "" { - return mailValidationError("--plain-text and --signature-id are mutually exclusive: signatures require HTML mode"). - WithParams( - mailInvalidParam("--plain-text", "mutually exclusive with --signature-id"), - mailInvalidParam("--signature-id", "requires HTML mode"), - ) - } - return nil -} diff --git a/shortcuts/mail/signature_compose_test.go b/shortcuts/mail/signature_compose_test.go index 51bf63f97..9adf52a35 100644 --- a/shortcuts/mail/signature_compose_test.go +++ b/shortcuts/mail/signature_compose_test.go @@ -4,204 +4,287 @@ package mail import ( + "context" "errors" - "io" - "net/http" - "net/http/httptest" "strings" "testing" + "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/shortcuts/common" + draftpkg "github.com/larksuite/cli/shortcuts/mail/draft" + "github.com/larksuite/cli/shortcuts/mail/emlbuilder" ) -func TestDownloadSignatureImageRejectsInvalidURLs(t *testing.T) { - rt := newDownloadRuntime(t, &http.Client{}) +func TestValidateNoSignatureConflictTypedError(t *testing.T) { + err := validateNoSignatureConflict(true, "sig_123") + if err == nil { + t.Fatal("expected error, got nil") + } + // mailValidationParamError returns *errs.ValidationError. + var valErr *errs.ValidationError + if !errors.As(err, &valErr) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if valErr.Param != "--no-signature" { + t.Errorf("expected Param \"--no-signature\", got %q", valErr.Param) + } + if !strings.Contains(err.Error(), "mutually exclusive") { + t.Errorf("error message = %q, want it to contain \"mutually exclusive\"", err.Error()) + } +} +func TestValidateNoSignatureConflictNoError(t *testing.T) { + if err := validateNoSignatureConflict(false, "sig_123"); err != nil { + t.Fatalf("expected no error when noSignature=false, got %v", err) + } + if err := validateNoSignatureConflict(true, ""); err != nil { + t.Fatalf("expected no error when signatureID empty, got %v", err) + } +} + +func TestInjectPlainTextSignatureNilSig(t *testing.T) { + body := "Hello world" + got := injectPlainTextSignature(body, nil) + if got != body { + t.Fatalf("expected unchanged body %q, got %q", body, got) + } +} + +func TestInjectPlainTextSignatureEmptyHTML(t *testing.T) { + sig := &signatureResult{RenderedContent: "Alice
"} + body := "My message" + got := injectPlainTextSignature(body, sig) + // Result must not end with a bare newline after the signature text. + if strings.HasSuffix(got, "\n") { + t.Fatalf("result should not end with newline, got %q", got) + } + if !strings.Contains(got, "Alice") { + t.Fatalf("expected sig text in result, got %q", got) + } +} + +func TestContentTypeFromFilename(t *testing.T) { cases := []struct { name string - url string + want string }{ - {name: "invalid", url: "https://[::1"}, - {name: "http", url: "http://example.com/sig.png"}, - {name: "no host", url: "https:///sig.png"}, + {"logo.png", "image/png"}, + {"photo.jpg", "image/jpeg"}, + {"photo.jpeg", "image/jpeg"}, + {"anim.gif", "image/gif"}, + {"icon.webp", "image/webp"}, + {"draw.svg", "image/svg+xml"}, + {"bitmap.bmp", "image/bmp"}, + {"data.bin", "application/octet-stream"}, + {"noext", "application/octet-stream"}, + {"UPPER.PNG", "image/png"}, } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - _, _, err := downloadSignatureImage(rt, tc.url, "sig.png") - var internalErr *errs.InternalError - if !errors.As(err, &internalErr) { - t.Fatalf("expected internal error, got %T (%v)", err, err) - } - p, ok := errs.ProblemOf(err) - if !ok { - t.Fatalf("expected typed problem, got %T", err) - } - if p.Subtype != errs.SubtypeInvalidResponse { - t.Fatalf("subtype = %q, want %q", p.Subtype, errs.SubtypeInvalidResponse) - } - }) + got := contentTypeFromFilename(tc.name) + if got != tc.want { + t.Errorf("contentTypeFromFilename(%q) = %q, want %q", tc.name, got, tc.want) + } } } -func TestDownloadSignatureImageHTTPErrorClassification(t *testing.T) { - for _, tc := range []struct { - name string - statusCode int - wantType any - wantSub errs.Subtype - retryable bool - }{ - { - name: "server", - statusCode: http.StatusInternalServerError, - wantType: (*errs.NetworkError)(nil), - wantSub: errs.SubtypeNetworkServer, - retryable: true, +func TestSignatureCIDsNilSig(t *testing.T) { + if cids := signatureCIDs(nil); cids != nil { + t.Fatalf("expected nil slice for nil sig, got %v", cids) + } +} + +func TestSignatureCIDsFiltersEmpty(t *testing.T) { + sig := &signatureResult{ + Images: []draftpkg.SignatureImage{ + {CID: "abc123"}, + {CID: ""}, + {CID: "