diff --git a/shortcuts/mail/draft/patch.go b/shortcuts/mail/draft/patch.go index e2c570243..3fc6d3ddd 100644 --- a/shortcuts/mail/draft/patch.go +++ b/shortcuts/mail/draft/patch.go @@ -8,18 +8,12 @@ import ( "mime" "os" "path/filepath" - "regexp" "strings" - "github.com/google/uuid" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/mail/filecheck" ) -// imgSrcRegexp matches and captures the src value. -// It handles both single and double quotes. -var imgSrcRegexp = regexp.MustCompile(`(?i)]*?\s)?src\s*=\s*["']([^"']+)["']`) - var protectedHeaders = map[string]bool{ "message-id": true, "mime-version": true, @@ -39,10 +33,13 @@ func Apply(snapshot *DraftSnapshot, patch Patch) error { return err } } - if err := postProcessInlineImages(snapshot); err != nil { + if err := refreshSnapshot(snapshot); err != nil { + return err + } + if err := validateInlineCIDAfterApply(snapshot); err != nil { return err } - return refreshSnapshot(snapshot) + return validateOrphanedInlineCIDAfterApply(snapshot) } func applyOp(snapshot *DraftSnapshot, op PatchOp, options PatchOptions) error { @@ -526,25 +523,21 @@ func addAttachment(snapshot *DraftSnapshot, path string) error { return nil } -// loadAndAttachInline reads a local image file, validates its format, -// creates a MIME inline part, and attaches it to the snapshot's -// multipart/related container. If container is non-nil it is reused; -// otherwise the container is resolved from the snapshot. -func loadAndAttachInline(snapshot *DraftSnapshot, path, cid, fileName string, container *Part) (*Part, error) { +func addInline(snapshot *DraftSnapshot, path, cid, fileName, contentType string) error { safePath, err := validate.SafeInputPath(path) if err != nil { - return nil, fmt.Errorf("inline image %q: %w", path, err) + return fmt.Errorf("inline image %q: %w", path, err) } info, err := os.Stat(safePath) if err != nil { - return nil, fmt.Errorf("inline image %q: %w", path, err) + return err } if err := checkSnapshotAttachmentLimit(snapshot, info.Size(), nil); err != nil { - return nil, err + return err } content, err := os.ReadFile(safePath) if err != nil { - return nil, fmt.Errorf("inline image %q: %w", path, err) + return err } name := fileName if strings.TrimSpace(name) == "" { @@ -552,30 +545,23 @@ func loadAndAttachInline(snapshot *DraftSnapshot, path, cid, fileName string, co } detectedCT, err := filecheck.CheckInlineImageFormat(name, content) if err != nil { - return nil, fmt.Errorf("inline image %q: %w", path, err) + return err } - inline, err := newInlinePart(safePath, content, cid, name, detectedCT) + inline, err := newInlinePart(path, content, cid, fileName, detectedCT) if err != nil { - return nil, fmt.Errorf("inline image %q: %w", path, err) + return err } - if container == nil { - containerRef := primaryBodyRootRef(&snapshot.Body) - if containerRef == nil || *containerRef == nil { - return nil, fmt.Errorf("draft has no primary body container") - } - container, err = ensureInlineContainerRef(containerRef) - if err != nil { - return nil, fmt.Errorf("inline image %q: %w", path, err) - } + containerRef := primaryBodyRootRef(&snapshot.Body) + if containerRef == nil || *containerRef == nil { + return fmt.Errorf("draft has no primary body container") + } + container, err := ensureInlineContainerRef(containerRef) + if err != nil { + return err } container.Children = append(container.Children, inline) container.Dirty = true - return container, nil -} - -func addInline(snapshot *DraftSnapshot, path, cid, fileName, contentType string) error { - _, err := loadAndAttachInline(snapshot, path, cid, fileName, nil) - return err + return nil } func replaceInline(snapshot *DraftSnapshot, partID, path, cid, fileName, contentType string) error { @@ -776,9 +762,6 @@ func newInlinePart(path string, content []byte, cid, fileName, contentType strin if err := validate.RejectCRLF(cid, "inline cid"); err != nil { return nil, err } - if strings.ContainsAny(cid, " \t<>()") { - return nil, fmt.Errorf("inline cid %q contains invalid characters (spaces, tabs, angle brackets, or parentheses are not allowed)", cid) - } if err := validate.RejectCRLF(fileName, "inline filename"); err != nil { return nil, err } @@ -874,152 +857,59 @@ func removeHeader(headers *[]Header, name string) { *headers = next } -// uriSchemeRegexp matches a URI scheme (RFC 3986: ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) ":"). -var uriSchemeRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.\-]*:`) - -// isLocalFileSrc returns true if src is a local file path. -// Any URI with a scheme (http:, cid:, data:, ftp:, blob:, file:, etc.) -// or protocol-relative URL (//host/...) is rejected. -func isLocalFileSrc(src string) bool { - trimmed := strings.TrimSpace(src) - if trimmed == "" { - return false - } - if strings.HasPrefix(trimmed, "//") { - return false - } - return !uriSchemeRegexp.MatchString(trimmed) -} - -// generateCID returns a random UUID string suitable for use as a Content-ID. -// UUIDs contain only [0-9a-f-], which is inherently RFC-safe and unique, -// avoiding all filename-derived encoding/collision issues. -func generateCID() (string, error) { - id, err := uuid.NewRandom() - if err != nil { - return "", fmt.Errorf("failed to generate CID: %w", err) +// validateInlineCIDAfterApply checks that all CID references in the HTML body +// resolve to actual inline MIME parts. This is called after Apply (editing) to +// prevent broken CID references, but NOT during Parse (where broken CIDs +// should not block opening the draft). +func validateInlineCIDAfterApply(snapshot *DraftSnapshot) error { + htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) + if htmlPart == nil { + return nil } - return id.String(), nil -} - -// resolveLocalImgSrc scans HTML for references, -// creates MIME inline parts for each local file, and returns the HTML -// with those src attributes replaced by cid: URIs. -func resolveLocalImgSrc(snapshot *DraftSnapshot, html string) (string, error) { - matches := imgSrcRegexp.FindAllStringSubmatchIndex(html, -1) - if len(matches) == 0 { - return html, nil + refs := extractCIDRefs(string(htmlPart.Body)) + if len(refs) == 0 { + return nil } - - var container *Part - // Cache resolved paths so the same file is only attached once. - pathToCID := make(map[string]string) - - // Iterate in reverse so that index offsets remain valid after replacement. - for i := len(matches) - 1; i >= 0; i-- { - srcStart, srcEnd := matches[i][2], matches[i][3] - src := html[srcStart:srcEnd] - if !isLocalFileSrc(src) { + cids := make(map[string]bool) + for _, part := range flattenParts(snapshot.Body) { + if part == nil || part.ContentID == "" { continue } - - resolvedPath, err := validate.SafeInputPath(src) - if err != nil { - return "", fmt.Errorf("inline image %q: %w", src, err) - } - - cid, ok := pathToCID[resolvedPath] - if !ok { - fileName := filepath.Base(src) - cid, err = generateCID() - if err != nil { - return "", err - } - pathToCID[resolvedPath] = cid - - container, err = loadAndAttachInline(snapshot, src, cid, fileName, container) - if err != nil { - return "", err - } - } - - html = html[:srcStart] + "cid:" + cid + html[srcEnd:] - } - - return html, nil -} - -// removeOrphanedInlineParts removes inline MIME parts whose ContentID -// is not in the referencedCIDs set from all multipart/related containers. -func removeOrphanedInlineParts(root *Part, referencedCIDs map[string]bool) { - if root == nil { - return - } - if !strings.EqualFold(root.MediaType, "multipart/related") { - for _, child := range root.Children { - removeOrphanedInlineParts(child, referencedCIDs) - } - return + cids[strings.ToLower(part.ContentID)] = true } - kept := make([]*Part, 0, len(root.Children)) - for _, child := range root.Children { - if child == nil { - continue - } - if strings.EqualFold(child.ContentDisposition, "inline") && child.ContentID != "" { - if !referencedCIDs[strings.ToLower(child.ContentID)] { - root.Dirty = true - continue - } + for _, ref := range refs { + if !cids[strings.ToLower(ref)] { + return fmt.Errorf("html body references missing inline cid %q", ref) } - kept = append(kept, child) } - root.Children = kept + return nil } -// postProcessInlineImages is the unified post-processing step that: -// 1. Resolves local to inline CID parts. -// 2. Validates all CID references in HTML resolve to MIME parts. -// 3. Removes orphaned inline MIME parts no longer referenced by HTML. -func postProcessInlineImages(snapshot *DraftSnapshot) error { - htmlPart := findPrimaryBodyPart(snapshot.Body, "text/html") +// validateOrphanedInlineCIDAfterApply checks the reverse direction: every +// inline MIME part with a ContentID must be referenced by the HTML body. +// An orphaned inline part (CID exists but HTML has no ) will +// be displayed as an unexpected attachment by most mail clients. +func validateOrphanedInlineCIDAfterApply(snapshot *DraftSnapshot) error { + htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) if htmlPart == nil { return nil } - - origHTML := string(htmlPart.Body) - // Note: if resolveLocalImgSrc returns an error after partially attaching - // inline parts to the snapshot, those parts are orphaned but will be - // cleaned up by removeOrphanedInlineParts on the next successful Apply. - html, err := resolveLocalImgSrc(snapshot, origHTML) - if err != nil { - return err - } - if html != origHTML { - htmlPart.Body = []byte(html) - htmlPart.Dirty = true - } - - refs := extractCIDRefs(html) + refs := extractCIDRefs(string(htmlPart.Body)) refSet := make(map[string]bool, len(refs)) for _, ref := range refs { refSet[strings.ToLower(ref)] = true } - - cidParts := make(map[string]bool) + var orphaned []string for _, part := range flattenParts(snapshot.Body) { if part == nil || part.ContentID == "" { continue } - cidParts[strings.ToLower(part.ContentID)] = true - } - - for _, ref := range refs { - if !cidParts[strings.ToLower(ref)] { - return fmt.Errorf("html body references missing inline cid %q", ref) + if !refSet[strings.ToLower(part.ContentID)] { + orphaned = append(orphaned, part.ContentID) } } - - removeOrphanedInlineParts(snapshot.Body, refSet) + if len(orphaned) > 0 { + return fmt.Errorf("inline MIME parts have no reference in the HTML body and will appear as unexpected attachments: orphaned cids %v; if you used set_body, make sure the new body preserves all existing cid:... references", orphaned) + } return nil } diff --git a/shortcuts/mail/draft/patch_inline_resolve_test.go b/shortcuts/mail/draft/patch_inline_resolve_test.go deleted file mode 100644 index 7c43886a3..000000000 --- a/shortcuts/mail/draft/patch_inline_resolve_test.go +++ /dev/null @@ -1,773 +0,0 @@ -// Copyright (c) 2026 Lark Technologies Pte. Ltd. -// SPDX-License-Identifier: MIT - -package draft - -import ( - "os" - "regexp" - "strings" - "testing" -) - -// --------------------------------------------------------------------------- -// resolveLocalImgSrc — basic auto-resolve -// --------------------------------------------------------------------------- - -func TestResolveLocalImgSrcBasic(t *testing.T) { - chdirTemp(t) - os.WriteFile("logo.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
Hello
-`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: `
Hello
`}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) - if htmlPart == nil { - t.Fatal("HTML part not found") - } - body := string(htmlPart.Body) - if strings.Contains(body, "./logo.png") { - t.Fatal("local path should have been replaced") - } - // Extract the generated CID from the HTML body. - cidRe := regexp.MustCompile(`src="cid:([^"]+)"`) - m := cidRe.FindStringSubmatch(body) - if m == nil { - t.Fatalf("expected src to contain a cid: reference, got: %s", body) - } - cid := m[1] - // Verify MIME inline part was created with the matching CID. - found := false - for _, part := range flattenParts(snapshot.Body) { - if part != nil && part.ContentID == cid { - found = true - if part.MediaType != "image/png" { - t.Fatalf("expected image/png, got %q", part.MediaType) - } - } - } - if !found { - t.Fatalf("expected inline MIME part with CID %q to be created", cid) - } -} - -// --------------------------------------------------------------------------- -// resolveLocalImgSrc — multiple images -// --------------------------------------------------------------------------- - -func TestResolveLocalImgSrcMultipleImages(t *testing.T) { - chdirTemp(t) - os.WriteFile("a.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - os.WriteFile("b.jpg", []byte{0xFF, 0xD8, 0xFF, 0xE0}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
empty
-`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: `
`}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) - body := string(htmlPart.Body) - cidRe := regexp.MustCompile(`src="cid:([^"]+)"`) - matches := cidRe.FindAllStringSubmatch(body, -1) - if len(matches) != 2 { - t.Fatalf("expected 2 cid: references, got %d in: %s", len(matches), body) - } - if matches[0][1] == matches[1][1] { - t.Fatalf("expected different CIDs for different files, both got: %s", matches[0][1]) - } -} - -// --------------------------------------------------------------------------- -// resolveLocalImgSrc — skips cid/http/data URIs -// --------------------------------------------------------------------------- - -func TestResolveLocalImgSrcSkipsNonLocalSrc(t *testing.T) { - chdirTemp(t) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: multipart/related; boundary="rel" - ---rel -Content-Type: text/html; charset=UTF-8 - -
---rel -Content-Type: image/png; name=existing.png -Content-Disposition: inline; filename=existing.png -Content-ID: -Content-Transfer-Encoding: base64 - -cG5n ---rel-- -`) - htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) - originalBody := string(htmlPart.Body) - - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: originalBody}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - htmlPart = findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) - if string(htmlPart.Body) != originalBody { - t.Fatalf("body should be unchanged, got: %s", string(htmlPart.Body)) - } -} - -// --------------------------------------------------------------------------- -// resolveLocalImgSrc — duplicate file names get unique CIDs -// --------------------------------------------------------------------------- - -func TestResolveLocalImgSrcDuplicateCID(t *testing.T) { - chdirTemp(t) - os.MkdirAll("sub", 0o755) - os.WriteFile("logo.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - os.WriteFile("sub/logo.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
empty
-`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: `
`}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) - body := string(htmlPart.Body) - cidRe := regexp.MustCompile(`src="cid:([^"]+)"`) - matches := cidRe.FindAllStringSubmatch(body, -1) - if len(matches) != 2 { - t.Fatalf("expected 2 cid: references, got %d in: %s", len(matches), body) - } - if matches[0][1] == matches[1][1] { - t.Fatalf("expected different CIDs for different files, both got: %s", matches[0][1]) - } -} - -// --------------------------------------------------------------------------- -// resolveLocalImgSrc — same file referenced multiple times reuses one CID -// --------------------------------------------------------------------------- - -func TestResolveLocalImgSrcSameFileReused(t *testing.T) { - chdirTemp(t) - os.WriteFile("logo.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
empty
-`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: `

text

`}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) - body := string(htmlPart.Body) - // Both references should resolve to the same CID. - cidRe := regexp.MustCompile(`src="cid:([^"]+)"`) - matches := cidRe.FindAllStringSubmatch(body, -1) - if len(matches) != 2 { - t.Fatalf("expected 2 cid: references, got %d in: %s", len(matches), body) - } - if matches[0][1] != matches[1][1] { - t.Fatalf("expected same CID reused, got %q and %q", matches[0][1], matches[1][1]) - } - // Count inline MIME parts — should be exactly 1. - var count int - for _, part := range flattenParts(snapshot.Body) { - if part != nil && strings.EqualFold(part.ContentDisposition, "inline") { - count++ - } - } - if count != 1 { - t.Fatalf("expected 1 inline part (reused), got %d", count) - } -} - -// --------------------------------------------------------------------------- -// resolveLocalImgSrc — non-image format rejected -// --------------------------------------------------------------------------- - -func TestResolveLocalImgSrcRejectsNonImage(t *testing.T) { - chdirTemp(t) - os.WriteFile("doc.txt", []byte("not an image"), 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
empty
-`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: `
`}}, - }) - if err == nil { - t.Fatal("expected error for non-image file") - } -} - -// --------------------------------------------------------------------------- -// orphan cleanup — delete inline image by removing from body -// --------------------------------------------------------------------------- - -func TestOrphanCleanupOnImgRemoval(t *testing.T) { - snapshot := mustParseFixtureDraft(t, `Subject: Inline -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: multipart/related; boundary="rel" - ---rel -Content-Type: text/html; charset=UTF-8 - -
hello
---rel -Content-Type: image/png; name=logo.png -Content-Disposition: inline; filename=logo.png -Content-ID: -Content-Transfer-Encoding: base64 - -cG5n ---rel-- -`) - // Remove the tag from body. - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: "
hello
"}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - for _, part := range flattenParts(snapshot.Body) { - if part != nil && part.ContentID == "logo" { - t.Fatal("expected orphaned inline part 'logo' to be removed") - } - } -} - -// --------------------------------------------------------------------------- -// orphan cleanup — replace inline image -// --------------------------------------------------------------------------- - -func TestOrphanCleanupOnImgReplace(t *testing.T) { - chdirTemp(t) - os.WriteFile("new.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Inline -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: multipart/related; boundary="rel" - ---rel -Content-Type: text/html; charset=UTF-8 - -
---rel -Content-Type: image/png; name=old.png -Content-Disposition: inline; filename=old.png -Content-ID: -Content-Transfer-Encoding: base64 - -cG5n ---rel-- -`) - // Replace old image reference with a new local file. - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: `
`}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - var foundOld bool - var newInlineCount int - for _, part := range flattenParts(snapshot.Body) { - if part == nil { - continue - } - if part.ContentID == "old" { - foundOld = true - } - if strings.EqualFold(part.ContentDisposition, "inline") && part.ContentID != "" && part.ContentID != "old" { - newInlineCount++ - } - } - if foundOld { - t.Fatal("expected old inline part to be removed") - } - if newInlineCount != 1 { - t.Fatalf("expected 1 new inline part, got %d", newInlineCount) - } -} - -// --------------------------------------------------------------------------- -// set_reply_body — local path resolved, quote block preserved -// --------------------------------------------------------------------------- - -func TestSetReplyBodyResolvesLocalImgSrc(t *testing.T) { - chdirTemp(t) - os.WriteFile("photo.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Re: Hello -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
original reply
quoted text
-`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_reply_body", Value: `
new reply
`}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - htmlPart := findPart(snapshot.Body, snapshot.PrimaryHTMLPartID) - if htmlPart == nil { - t.Fatal("HTML part not found") - } - body := string(htmlPart.Body) - if strings.Contains(body, "./photo.png") { - t.Fatal("local path should have been replaced") - } - cidRe := regexp.MustCompile(`src="cid:([^"]+)"`) - m := cidRe.FindStringSubmatch(body) - if m == nil { - t.Fatalf("expected cid: reference in body, got: %s", body) - } - if !strings.Contains(body, "history-quote-wrapper") { - t.Fatalf("expected quote block preserved, got: %s", body) - } - found := false - for _, part := range flattenParts(snapshot.Body) { - if part != nil && part.ContentID == m[1] { - found = true - } - } - if !found { - t.Fatalf("expected inline MIME part with CID %q to be created", m[1]) - } -} - -// --------------------------------------------------------------------------- -// mixed usage — add_inline + local path in body -// --------------------------------------------------------------------------- - -func TestMixedAddInlineAndLocalPath(t *testing.T) { - chdirTemp(t) - os.WriteFile("a.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - os.WriteFile("b.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
empty
-`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{ - {Op: "add_inline", Path: "a.png", CID: "a"}, - {Op: "set_body", Value: `
`}, - }, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - var foundA bool - var autoResolvedCount int - for _, part := range flattenParts(snapshot.Body) { - if part == nil { - continue - } - if part.ContentID == "a" { - foundA = true - } else if strings.EqualFold(part.ContentDisposition, "inline") && part.ContentID != "" { - autoResolvedCount++ - } - } - if !foundA { - t.Fatal("expected inline part 'a' from add_inline") - } - if autoResolvedCount != 1 { - t.Fatalf("expected 1 auto-resolved inline part for b.png, got %d", autoResolvedCount) - } -} - -// --------------------------------------------------------------------------- -// conflict: add_inline same file + body local path → redundant part cleaned -// --------------------------------------------------------------------------- - -func TestAddInlineSameFileAsLocalPath(t *testing.T) { - chdirTemp(t) - os.WriteFile("logo.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/html; charset=UTF-8 - -
empty
-`) - // add_inline creates CID "logo", but body uses local path instead of cid:logo. - // resolve generates a UUID CID, orphan cleanup removes the unused "logo". - err := Apply(snapshot, Patch{ - Ops: []PatchOp{ - {Op: "add_inline", Path: "logo.png", CID: "logo"}, - {Op: "set_body", Value: `
`}, - }, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - // The explicitly added "logo" CID is orphaned (not referenced in HTML) - // and should be auto-removed. Only the auto-generated CID remains. - var foundLogo bool - var count int - for _, part := range flattenParts(snapshot.Body) { - if part != nil && strings.EqualFold(part.ContentDisposition, "inline") { - count++ - if part.ContentID == "logo" { - foundLogo = true - } - } - } - if foundLogo { - t.Fatal("expected orphaned 'logo' inline part to be removed") - } - if count != 1 { - t.Fatalf("expected 1 inline part after orphan cleanup, got %d", count) - } -} - -// --------------------------------------------------------------------------- -// conflict: remove_inline but body still references its CID → error -// --------------------------------------------------------------------------- - -func TestRemoveInlineButBodyStillReferencesCID(t *testing.T) { - snapshot := mustParseFixtureDraft(t, `Subject: Inline -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: multipart/related; boundary="rel" - ---rel -Content-Type: text/html; charset=UTF-8 - -
---rel -Content-Type: image/png; name=logo.png -Content-Disposition: inline; filename=logo.png -Content-ID: -Content-Transfer-Encoding: base64 - -cG5n ---rel-- -`) - // remove_inline removes the MIME part, but set_body still references cid:logo. - err := Apply(snapshot, Patch{ - Ops: []PatchOp{ - {Op: "remove_inline", Target: AttachmentTarget{CID: "logo"}}, - {Op: "set_body", Value: `
`}, - }, - }) - if err == nil || !strings.Contains(err.Error(), "missing inline cid") { - t.Fatalf("expected missing cid error, got: %v", err) - } -} - -// --------------------------------------------------------------------------- -// conflict: remove_inline + body replaces with local path → works -// --------------------------------------------------------------------------- - -func TestRemoveInlineAndReplaceWithLocalPath(t *testing.T) { - chdirTemp(t) - os.WriteFile("new.png", []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A}, 0o644) - - snapshot := mustParseFixtureDraft(t, `Subject: Inline -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: multipart/related; boundary="rel" - ---rel -Content-Type: text/html; charset=UTF-8 - -
---rel -Content-Type: image/png; name=old.png -Content-Disposition: inline; filename=old.png -Content-ID: -Content-Transfer-Encoding: base64 - -cG5n ---rel-- -`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{ - {Op: "remove_inline", Target: AttachmentTarget{CID: "old"}}, - {Op: "set_body", Value: `
`}, - }, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - var foundOld bool - var newInlineCount int - for _, part := range flattenParts(snapshot.Body) { - if part == nil { - continue - } - if part.ContentID == "old" { - foundOld = true - } - if strings.EqualFold(part.ContentDisposition, "inline") && part.ContentID != "" && part.ContentID != "old" { - newInlineCount++ - } - } - if foundOld { - t.Fatal("expected old inline part to be removed") - } - if newInlineCount != 1 { - t.Fatalf("expected 1 new inline part from local path resolve, got %d", newInlineCount) - } -} - -// --------------------------------------------------------------------------- -// no HTML body — text/plain only draft -// --------------------------------------------------------------------------- - -func TestResolveLocalImgSrcNoHTMLBody(t *testing.T) { - snapshot := mustParseFixtureDraft(t, `Subject: Plain -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 - -Just plain text. -`) - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_body", Value: "Updated plain text."}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } -} - -// --------------------------------------------------------------------------- -// regression: HTML body with Content-ID must not be removed by orphan cleanup -// --------------------------------------------------------------------------- - -func TestOrphanCleanupPreservesHTMLBodyWithContentID(t *testing.T) { - snapshot := mustParseFixtureDraft(t, `Subject: Test -From: Alice -To: Bob -MIME-Version: 1.0 -Content-Type: multipart/related; boundary="rel" - ---rel -Content-Type: text/html; charset=UTF-8 -Content-ID: - -
hello world
---rel -Content-Type: image/png; name=logo.png -Content-Disposition: inline; filename=logo.png -Content-ID: -Content-Transfer-Encoding: base64 - -cG5n ---rel-- -`) - // A metadata-only edit should not destroy the HTML body part even though - // its Content-ID is not referenced by any . - err := Apply(snapshot, Patch{ - Ops: []PatchOp{{Op: "set_subject", Value: "Updated subject"}}, - }) - if err != nil { - t.Fatalf("Apply() error = %v", err) - } - htmlPart := findPrimaryBodyPart(snapshot.Body, "text/html") - if htmlPart == nil { - t.Fatal("HTML body part was deleted by orphan cleanup") - } - if !strings.Contains(string(htmlPart.Body), "hello world") { - t.Fatalf("HTML body content changed unexpectedly: %s", string(htmlPart.Body)) - } -} - -// --------------------------------------------------------------------------- -// helper unit tests -// --------------------------------------------------------------------------- - -func TestIsLocalFileSrc(t *testing.T) { - tests := []struct { - src string - want bool - }{ - {"./logo.png", true}, - {"../images/logo.png", true}, - {"logo.png", true}, - {"/absolute/path/logo.png", true}, - {"cid:logo", false}, - {"CID:logo", false}, - {"http://example.com/img.png", false}, - {"https://example.com/img.png", false}, - {"data:image/png;base64,abc", false}, - {"//cdn.example.com/a.png", false}, - {"blob:https://example.com/uuid", false}, - {"ftp://example.com/file.png", false}, - {"file:///local/file.png", false}, - {"mailto:test@example.com", false}, - {"", false}, - } - for _, tt := range tests { - if got := isLocalFileSrc(tt.src); got != tt.want { - t.Errorf("isLocalFileSrc(%q) = %v, want %v", tt.src, got, tt.want) - } - } -} - -func TestGenerateCID(t *testing.T) { - seen := make(map[string]bool) - for i := 0; i < 100; i++ { - cid, err := generateCID() - if err != nil { - t.Fatalf("generateCID() error = %v", err) - } - if cid == "" { - t.Fatal("generateCID() returned empty string") - } - if strings.ContainsAny(cid, " \t\r\n<>()") { - t.Fatalf("generateCID() returned CID with invalid characters: %q", cid) - } - if seen[cid] { - t.Fatalf("generateCID() returned duplicate CID: %q", cid) - } - seen[cid] = true - } -} - -// --------------------------------------------------------------------------- -// imgSrcRegexp — must not match data-src or similar attribute names -// --------------------------------------------------------------------------- - -func TestImgSrcRegexpSkipsDataSrc(t *testing.T) { - tests := []struct { - name string - html string - want string // expected captured src value, empty if no match - }{ - { - name: "plain src", - html: ``, - want: "./logo.png", - }, - { - name: "src with alt before", - html: `pic`, - want: "./logo.png", - }, - { - name: "data-src before real src", - html: ``, - want: "./logo.png", - }, - { - name: "only data-src, no src", - html: ``, - want: "", - }, - { - name: "x-src before real src", - html: ``, - want: "./real.png", - }, - { - name: "single-quoted src", - html: ``, - want: "./logo.png", - }, - { - name: "multiple spaces before src", - html: ``, - want: "./logo.png", - }, - { - name: "newline before src", - html: "", - want: "./logo.png", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - matches := imgSrcRegexp.FindStringSubmatch(tt.html) - got := "" - if len(matches) > 1 { - got = matches[1] - } - if got != tt.want { - t.Errorf("imgSrcRegexp on %q: got %q, want %q", tt.html, got, tt.want) - } - }) - } -} - -// --------------------------------------------------------------------------- -// newInlinePart — rejects CIDs with spaces or other invalid characters -// --------------------------------------------------------------------------- - -func TestNewInlinePartRejectsInvalidCIDChars(t *testing.T) { - content := []byte{0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A} - for _, bad := range []string{"my logo", "a\tb", "cid", "cid(x)"} { - _, err := newInlinePart("test.png", content, bad, "test.png", "image/png") - if err == nil { - t.Errorf("expected error for CID %q, got nil", bad) - } - } - // Valid CIDs should pass. - for _, good := range []string{"logo", "my-logo", "img_01", "photo.2"} { - _, err := newInlinePart("test.png", content, good, "test.png", "image/png") - if err != nil { - t.Errorf("unexpected error for CID %q: %v", good, err) - } - } -} diff --git a/shortcuts/mail/draft/patch_test.go b/shortcuts/mail/draft/patch_test.go index e3c55a9e0..8572f503d 100644 --- a/shortcuts/mail/draft/patch_test.go +++ b/shortcuts/mail/draft/patch_test.go @@ -460,7 +460,7 @@ func TestRemoveInlineFailsWhenHTMLStillReferencesCID(t *testing.T) { } } -func TestApplySetBodyOrphanedInlineCIDIsAutoRemoved(t *testing.T) { +func TestApplySetBodyOrphanedInlineCIDIsRejected(t *testing.T) { snapshot := mustParseFixtureDraft(t, `Subject: Inline From: Alice To: Bob @@ -480,18 +480,12 @@ Content-Transfer-Encoding: base64 cG5n --rel-- `) - // set_body that drops the existing cid:logo reference → logo is auto-removed + // set_body that drops the existing cid:logo reference → logo becomes orphaned err := Apply(snapshot, Patch{ Ops: []PatchOp{{Op: "set_body", Value: "
replaced body without cid reference
"}}, }) - if err != nil { - t.Fatalf("expected no error, got: %v", err) - } - // The orphaned inline part should be removed from the MIME tree. - for _, part := range flattenParts(snapshot.Body) { - if part != nil && part.ContentID == "logo" { - t.Fatal("expected orphaned inline part 'logo' to be removed") - } + if err == nil || !strings.Contains(err.Error(), "orphaned cids") { + t.Fatalf("expected orphaned cid error, got: %v", err) } } diff --git a/shortcuts/mail/mail_draft_edit.go b/shortcuts/mail/mail_draft_edit.go index 17a383726..99061b8bc 100644 --- a/shortcuts/mail/mail_draft_edit.go +++ b/shortcuts/mail/mail_draft_edit.go @@ -303,13 +303,13 @@ func buildDraftEditPatchTemplate() map[string]interface{} { {"op": "set_recipients", "shape": map[string]interface{}{"field": "to|cc|bcc", "addresses": []map[string]interface{}{{"address": "string", "name": "string(optional)"}}}}, {"op": "add_recipient", "shape": map[string]interface{}{"field": "to|cc|bcc", "address": "string", "name": "string(optional)"}}, {"op": "remove_recipient", "shape": map[string]interface{}{"field": "to|cc|bcc", "address": "string"}}, - {"op": "set_body", "shape": map[string]interface{}{"value": "string (supports — local paths auto-resolved to inline MIME parts)"}}, - {"op": "set_reply_body", "shape": map[string]interface{}{"value": "string (user-authored content only, WITHOUT the quote block; the quote block is re-appended automatically; supports — local paths auto-resolved to inline MIME parts)"}}, + {"op": "set_body", "shape": map[string]interface{}{"value": "string"}}, + {"op": "set_reply_body", "shape": map[string]interface{}{"value": "string (user-authored content only, WITHOUT the quote block; the quote block is re-appended automatically)"}}, {"op": "set_header", "shape": map[string]interface{}{"name": "string", "value": "string"}}, {"op": "remove_header", "shape": map[string]interface{}{"name": "string"}}, {"op": "add_attachment", "shape": map[string]interface{}{"path": "string(relative path)"}}, {"op": "remove_attachment", "shape": map[string]interface{}{"target": map[string]interface{}{"part_id": "string(optional)", "cid": "string(optional)"}}}, - {"op": "add_inline", "shape": map[string]interface{}{"path": "string(relative path)", "cid": "string", "filename": "string(optional)", "content_type": "string(optional)"}, "note": "advanced: prefer in set_body/set_reply_body instead"}, + {"op": "add_inline", "shape": map[string]interface{}{"path": "string(relative path)", "cid": "string", "filename": "string(optional)", "content_type": "string(optional)"}}, {"op": "replace_inline", "shape": map[string]interface{}{"target": map[string]interface{}{"part_id": "string(optional)", "cid": "string(optional)"}, "path": "string(relative path)", "cid": "string(optional)", "filename": "string(optional)", "content_type": "string(optional)"}}, {"op": "remove_inline", "shape": map[string]interface{}{"target": map[string]interface{}{"part_id": "string(optional)", "cid": "string(optional)"}}}, }, @@ -318,8 +318,8 @@ func buildDraftEditPatchTemplate() map[string]interface{} { "group": "subject_and_body", "ops": []map[string]interface{}{ {"op": "set_subject", "shape": map[string]interface{}{"value": "string"}}, - {"op": "set_body", "shape": map[string]interface{}{"value": "string (supports — local paths auto-resolved to inline MIME parts)"}}, - {"op": "set_reply_body", "shape": map[string]interface{}{"value": "string (user-authored content only, WITHOUT the quote block; the quote block is re-appended automatically; supports — local paths auto-resolved to inline MIME parts)"}}, + {"op": "set_body", "shape": map[string]interface{}{"value": "string"}}, + {"op": "set_reply_body", "shape": map[string]interface{}{"value": "string (user-authored content only, WITHOUT the quote block; the quote block is re-appended automatically)"}}, }, }, { @@ -342,7 +342,7 @@ func buildDraftEditPatchTemplate() map[string]interface{} { "ops": []map[string]interface{}{ {"op": "add_attachment", "shape": map[string]interface{}{"path": "string(relative path)"}}, {"op": "remove_attachment", "shape": map[string]interface{}{"target": map[string]interface{}{"part_id": "string(optional)", "cid": "string(optional)"}}}, - {"op": "add_inline", "shape": map[string]interface{}{"path": "string(relative path)", "cid": "string", "filename": "string(optional)", "content_type": "string(optional)"}, "note": "advanced: prefer in set_body/set_reply_body instead"}, + {"op": "add_inline", "shape": map[string]interface{}{"path": "string(relative path)", "cid": "string", "filename": "string(optional)", "content_type": "string(optional)"}}, {"op": "replace_inline", "shape": map[string]interface{}{"target": map[string]interface{}{"part_id": "string(optional)", "cid": "string(optional)"}, "path": "string(relative path)", "cid": "string(optional)", "filename": "string(optional)", "content_type": "string(optional)"}}, {"op": "remove_inline", "shape": map[string]interface{}{"target": map[string]interface{}{"part_id": "string(optional)", "cid": "string(optional)"}}}, }, @@ -359,13 +359,12 @@ func buildDraftEditPatchTemplate() map[string]interface{} { {"situation": "draft created by +reply or +forward (has_quoted_content=true)", "recommended_op": "set_reply_body — replaces only the user-authored portion and automatically preserves the quoted original message; if user explicitly wants to remove the quote, use set_body instead"}, }, "notes": []string{ - "`set_body`/`set_reply_body` support inline images via local file paths: use in the HTML value — the local path is automatically resolved into an inline MIME part with a generated CID; removing or replacing an tag automatically cleans up or replaces the corresponding MIME part; do NOT use `add_inline` for this; example: {\"op\":\"set_body\",\"value\":\"
Hello
\"}", - "`add_inline` is an advanced op for precise CID control only — in most cases, use in `set_body`/`set_reply_body` instead", "`ops` is executed in order", "all file paths (--patch-file and `path` fields in ops) must be relative — no absolute paths or .. traversal", "all body edits MUST go through --patch-file; there is no --set-body flag", "`set_body` replaces the ENTIRE body including any reply/forward quote block; when the draft has both text/plain and text/html, it updates the HTML body and regenerates the plain-text summary, so the input should be HTML", "`set_reply_body` replaces only the user-authored portion of the body and automatically re-appends the trailing reply/forward quote block (generated by +reply or +forward); the value you pass should contain ONLY the new user-authored content WITHOUT the quote block — the quote block will be re-inserted automatically; if the user wants to modify content INSIDE the quote block, use `set_body` instead for full replacement; if the draft has no quote block, it behaves identically to `set_body`", + "`add_inline` only adds the MIME binary part; it does NOT insert an tag into the HTML body; to display the image in the body, you must ALSO use set_body/set_reply_body to insert into the body content; forgetting this causes the inline part to become an orphaned attachment when sent", "`body_kind` only supports text/plain and text/html", "`selector` currently only supports primary", "`remove_attachment` target supports part_id or cid; priority: part_id > cid", diff --git a/skills/lark-mail/references/lark-mail-draft-edit.md b/skills/lark-mail/references/lark-mail-draft-edit.md index 5cb58d29f..6831700ca 100644 --- a/skills/lark-mail/references/lark-mail-draft-edit.md +++ b/skills/lark-mail/references/lark-mail-draft-edit.md @@ -198,9 +198,9 @@ lark-cli mail +draft-edit --draft-id --inspect { "op": "add_inline", "path": "./logo.png", "cid": "logo" } ``` -> **推荐方式:** 直接在 `set_body`/`set_reply_body` 的 HTML 中使用 ``(本地文件路径),系统会自动创建 MIME 内嵌部分、生成 CID 并替换为 `cid:` 引用。删除或替换 `` 标签时,对应的 MIME 部分会自动清理。详见[在正文中插入内嵌图片](#在正文中插入内嵌图片)。 -> -> `add_inline` 仅在需要精确控制 CID 命名时使用。使用时仍需在 HTML 正文中加入 `` 引用。 +> **重要:`add_inline` 仅添加 MIME 二进制部分,不会在 HTML 正文中插入 `` 标签。** +> 如需图片在邮件正文中可见,**必须**同时使用 `set_body` 或 `set_reply_body` 更新 HTML 正文并加入 `` 标签。参见[在正文中插入内嵌图片](#在正文中插入内嵌图片)的完整流程。 +> 如果忘记添加 `` 引用,该内嵌部分在发送时会变成孤立附件。 `replace_inline` @@ -304,18 +304,23 @@ lark-cli mail +draft-edit --draft-id --patch-file ./patch.json ### 在正文中插入内嵌图片 -直接在 `set_body`/`set_reply_body` 的 HTML 中使用本地文件路径即可。系统会自动创建 MIME 内嵌部分并替换为 `cid:` 引用。 +添加内嵌图片需要**两个协同编辑**:(1)通过 `add_inline` 添加 MIME 部分,(2)通过 `set_body` 或 `set_reply_body` 在 HTML 正文中插入 `` 标签。 ```bash -# 1. 查看草稿以获取当前 HTML 正文 +# 1. 查看草稿以获取当前 HTML 正文和已有的内嵌部分 lark-cli mail +draft-edit --draft-id --inspect +# 返回包含: +# projection.body_html_summary: "
原有内容
" +# projection.inline_summary: [{"part_id":"1.1.2","cid":"existing.png", ...}] -# 2. 编写补丁 — 直接使用本地文件路径(注意:回复草稿用 set_reply_body,普通草稿用 set_body) +# 2. 编写补丁(注意:回复草稿用 set_reply_body,普通草稿用 set_body) cat > ./patch.json << 'EOF' { "ops": [ - { "op": "set_body", "value": "
内容
" } - ] + { "op": "set_body", "value": "
原有内容
" }, + { "op": "add_inline", "path": "./new-image.png", "cid": "new-image" } + ], + "options": {} } EOF @@ -323,13 +328,6 @@ EOF lark-cli mail +draft-edit --draft-id --patch-file ./patch.json ``` -内嵌图片的增删改通过 HTML 正文自动联动: -- **添加**:在 HTML 中写 ``,自动创建 MIME 部分 -- **删除**:从 HTML 中移除 `` 标签,对应 MIME 部分自动清理 -- **替换**:将 `src` 改为新的本地路径,旧 MIME 部分自动移除、新部分自动创建 - -> **高级用法:** 需要精确控制 CID 命名时,仍可使用 `add_inline` 手动添加 MIME 部分,并在 HTML 中用 `` 引用。 - ### 使用 patch-file 进行高级编辑 ```bash