From 1142c0b53a46c5e1d6cd4d169303f23fbd4904ab Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Fri, 15 May 2026 07:17:22 -0400 Subject: [PATCH 1/5] feat(mail): quote source message in reply drafts Reply drafts (--reply-to) now quote the source message below the authored body, matching Gmail's web UI default, instead of producing a blank body. The source is fetched with the full body only when quoting; --no-quote keeps the cheaper metadata-only fetch and reproduces the prior blank draft. - Attribution is a fixed en-US "On wrote:" line with the date rendered in the source message's own timezone (never UTC/local); deliberately not localized. Falls back to the raw Date on parse failure. - Plain replies use "> " line prefixes (empty lines become ">"); HTML and markdown replies wrap the quote in Gmail's gmail_quote div/blockquote markup so the Gmail UI collapses it natively. Attribution and source are HTML-escaped to prevent markup injection via display name or address. - Reply mode now allows zero body sources: a bare --reply-to yields a quote-only draft; the leading separator is omitted when there is no authored body. Non-reply mode still requires exactly one body source. - A source with only an HTML part (no plain-text alternative) is shown as escaped tags, by design, to avoid an HTML-to-text dependency. Adds the "Gmail browser parity" design principle to CLAUDE.md, the rule this behavior embodies. Closes #125 --- CLAUDE.md | 4 + README.md | 9 +- integration-tests.md | 6 + internal/cmd/mail/draft.go | 94 ++++++++++- internal/cmd/mail/draft_quote_test.go | 219 ++++++++++++++++++++++++++ internal/cmd/mail/draft_test.go | 6 +- 6 files changed, 329 insertions(+), 9 deletions(-) create mode 100644 internal/cmd/mail/draft_quote_test.go diff --git a/CLAUDE.md b/CLAUDE.md index e34a550..a44629b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,6 +45,10 @@ make install # Install to /usr/local/bin See `docs/golden-principles.md` for the full set of enforced rules. +## Design Principles + +- **Gmail browser parity**: Mail features must integrate with how people expect Gmail to behave in the browser. `gro` is one client among many on a shared mailbox — drafts, quoting, threading, and labels should look and behave like native Gmail when later opened or sent from the web UI. Emit the conventions/markers Gmail's own client recognizes (e.g. wrap reply quotes in `gmail_quote` markup so Gmail collapses them behind its `…` toggle; `Re:` subject handling; RFC threading headers) rather than reimplementing client-side rendering. Prefer Gmail-native parity over technically-valid-but-foreign output. + ## Testing Run tests: `make test` diff --git a/README.md b/README.md index d7fd05f..bb280ef 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,8 @@ gro mail draft --from work@me.com --to a@x.com --subject "Hi" --body "..." # Reply to an existing message (preserves thread, adds In-Reply-To / References) gro mail draft --reply-to --body "thanks, will review" +gro mail draft --reply-to # quote-only reply (no typed text) +gro mail draft --reply-to --no-quote --body "ack" gro mail draft --reply-to --reply-all --body "looping everyone in" gro mail draft --reply-to --subject "Re: customised" --body "..." ``` @@ -301,6 +303,8 @@ Drafts always land in your Gmail Drafts folder for human review. The CLI never c With `--reply-to`, `--to` and `--subject` are derived from the source message (To = original From; Subject = `Re: `, no double prefix). Explicit `--to` / `--cc` / `--subject` flags override the derived values. `--reply-all` adds the original To and Cc as Cc on the reply, filtered to remove your own address (and any `--from` alias). +By default a reply quotes the source message below your text, like Gmail's web UI: an `On wrote:` line followed by the original body (`> ` line prefixes for plain replies; a collapsible `gmail_quote` block for HTML/markdown replies, which Gmail's UI hides behind its “…” toggle). A body source is optional when replying — a bare `--reply-to` yields a quote-only draft. Use `--no-quote` to reply without quoting (with no body source the draft is left blank). Known limitation: a source message that has only an HTML part (no plain-text alternative) is HTML-escaped and shown as visible tags inside the quote — by design, to avoid an HTML-to-text conversion dependency. + ### Calendar Commands All Calendar commands are under `gro calendar` (or `gro cal`): @@ -744,13 +748,14 @@ Flags: -j, --json Output result as JSON --reply-to string Source Gmail message ID to reply to (derives To/Subject/threading) --reply-all Include the source To/Cc as Cc on the reply (requires --reply-to) + --no-quote Reply without quoting the source message (requires --reply-to) ``` -`--body`, `--stdin`, and `--file` are mutually exclusive (exactly one required). `--plain` and `--html` are mutually exclusive. +`--body`, `--stdin`, and `--file` are mutually exclusive; exactly one is required, except in reply mode where all three are optional (a bare reply is just the quote). `--plain` and `--html` are mutually exclusive. Display names in `--to`/`--cc`/`--bcc` (e.g., `Alice `) are stripped; only the email address is sent. Edit the draft in Gmail to set display names. -With `--reply-to`, the draft is threaded onto the source conversation (`In-Reply-To` and `References` headers are set; `Draft.Message.ThreadId` is set to the source thread). `--to` and `--subject` are derived from the source (To = original From; Subject = `Re: ` with no double prefix). Explicit `--to`/`--cc`/`--subject` flags override the derived values. `--reply-all` populates Cc with the source To+Cc minus your authenticated account and any `--from` alias. +With `--reply-to`, the draft is threaded onto the source conversation (`In-Reply-To` and `References` headers are set; `Draft.Message.ThreadId` is set to the source thread). `--to` and `--subject` are derived from the source (To = original From; Subject = `Re: ` with no double prefix). Explicit `--to`/`--cc`/`--subject` flags override the derived values. `--reply-all` populates Cc with the source To+Cc minus your authenticated account and any `--from` alias. The source message is quoted below your text by default (Gmail-style `On wrote:` attribution; `gmail_quote` markup on HTML replies so Gmail collapses it natively); `--no-quote` suppresses the quote. ### gro calendar list diff --git a/integration-tests.md b/integration-tests.md index eb8e604..118c000 100644 --- a/integration-tests.md +++ b/integration-tests.md @@ -592,6 +592,12 @@ DRAFT_MSG_ID=$(./bin/gro mail draft --reply-to "$SRC_ID" --body "T21 reply" --pl | T24 | Explicit Cc override | `./bin/gro mail draft --reply-to "$SRC_ID" --cc "$ME" --body "T24 cc" --plain --json` | Exit 0. Draft Cc is exactly `$ME` (replaces, not merges with, derived Cc). | | T25 | No double Re: prefix | First find a source message whose subject already starts with `Re:`: `RE_SRC=$(./bin/gro mail search "subject:Re:" --json \| jq -r '.[0].id')`. Then: `./bin/gro mail draft --reply-to "$RE_SRC" --body "T25" --plain --json` | Exit 0. Read-back subject is unchanged (no `Re: Re: ` doubling). | | T26 | Reply-all without reply-to | `./bin/gro mail draft --to "$ME" --subject "x" --body "y" --reply-all; echo "exit=$?"` | Non-zero exit. Error mentions `--reply-all requires --reply-to`. | +| T27 | Quoted reply (plain) | `./bin/gro mail draft --reply-to "$SRC_ID" --body "T27 reply" --plain --json` then read back the draft body | Exit 0. Draft body starts with `T27 reply`, then a blank line, an `On wrote:` line, then the source body with every line `> `-prefixed (empty lines are `>`). | +| T28 | Quoted reply (HTML) collapses in Gmail | `./bin/gro mail draft --reply-to "$SRC_ID" --body "T28 **reply**" --json` then open the draft in the Gmail web UI | Exit 0. Draft body contains a `
` / `
` wrapper. **Manual:** opening the draft in Gmail shows the quoted history collapsed behind Gmail's “…” affordance (not automatable — Gmail client-side rendering). | +| T29 | Quote-only reply (no body source) | `./bin/gro mail draft --reply-to "$SRC_ID" --json` then read back | Exit 0. Draft body is exactly the quote block (attribution + quoted source), no leading blank lines, no authored text. Threading headers set as T21. | +| T30 | `--no-quote` reply | `./bin/gro mail draft --reply-to "$SRC_ID" --no-quote --body "T30" --plain --json` then read back | Exit 0. Draft body is exactly `T30` — no attribution, no quote. Threading headers still set. | +| T31 | `--no-quote` bare (blank draft) | `./bin/gro mail draft --reply-to "$SRC_ID" --no-quote --json` then read back | Exit 0. Draft body is empty. Threading headers still set. | +| T32 | `--no-quote` without reply-to | `./bin/gro mail draft --to "$ME" --subject "x" --body "y" --no-quote; echo "exit=$?"` | Non-zero exit. Error mentions `--no-quote requires --reply-to`. | ### Cleanup diff --git a/internal/cmd/mail/draft.go b/internal/cmd/mail/draft.go index 7229764..8e8acf8 100644 --- a/internal/cmd/mail/draft.go +++ b/internal/cmd/mail/draft.go @@ -3,6 +3,7 @@ package mail import ( "bytes" "fmt" + "html" "io" "mime" "net/mail" @@ -33,6 +34,7 @@ func newDraftCommand() *cobra.Command { jsonOut bool replyTo string replyAll bool + noQuote bool ) cmd := &cobra.Command{ @@ -45,7 +47,10 @@ The CLI never calls drafts.send — sending requires explicit action in Gmail. Body input is markdown by default and rendered to HTML. Use --plain for plain text, --html to send raw HTML verbatim. Body source is one of --body, --stdin, -or --file. +or --file (optional when replying — a bare reply is just the quote). + +In reply mode the source message is quoted below your text, like Gmail's web +UI. Use --no-quote to reply without quoting. Examples: gro mail draft --to alice@example.com --subject "Hi" --body "**hello**" @@ -54,6 +59,8 @@ Examples: gro mail draft --to a@x.com --subject "Plain" --body "no md" --plain gro mail draft --to a@x.com --subject "Report" --body "see attached" --attach report.pdf gro mail draft --reply-to --body "thanks, will review" + gro mail draft --reply-to # quote-only reply + gro mail draft --reply-to --no-quote --body "ack" gro mail draft --reply-to --reply-all --body "..."`, RunE: func(cmd *cobra.Command, _ []string) error { // --reply-to with an empty value is a user error (often a shell @@ -69,6 +76,9 @@ Examples: if replyAll && !isReply { return fmt.Errorf("--reply-all requires --reply-to") } + if noQuote && !isReply { + return fmt.Errorf("--no-quote requires --reply-to") + } if !isReply { if !cmd.Flags().Changed("to") { return fmt.Errorf("--to is required") @@ -125,7 +135,10 @@ Examples: return fmt.Errorf("--to must contain at least one address") } - // 4. Body source: exactly one of --body, --stdin, --file. + // 4. Body source: at most one of --body, --stdin, --file. + // Non-reply mode requires exactly one. Reply mode allows zero + // (an empty authored body — the reply is then just the quote, + // matching Gmail's "reply with no typed text" behavior). bodySources := 0 if cmd.Flags().Changed("body") { bodySources++ @@ -136,7 +149,10 @@ Examples: if cmd.Flags().Changed("file") { bodySources++ } - if bodySources != 1 { + if bodySources > 1 { + return fmt.Errorf("at most one of --body, --stdin, or --file may be used") + } + if bodySources == 0 && !isReply { return fmt.Errorf("exactly one of --body, --stdin, or --file is required") } @@ -208,7 +224,7 @@ Examples: references []string ) if isReply { - src, err := client.GetMessage(cmd.Context(), replyTo, false) + src, err := client.GetMessage(cmd.Context(), replyTo, !noQuote) if err != nil { return fmt.Errorf("fetching reply source %s: %w", replyTo, err) } @@ -246,6 +262,27 @@ Examples: if needs.Subject { subject = derived.Subject } + + // Quote the source body (default; suppressed by --no-quote). + // Skip entirely if the source has no body part. The leading + // separator is omitted when there is no authored body, so a + // bare reply is exactly the quote block. + if !noQuote && src.Body != "" { + attrib := replyAttribution(src) + if kind == gmailapi.DraftBodyHTML { + sep := "" + if len(outBody) > 0 { + sep = "\n" + } + outBody = append(outBody, []byte(sep+quoteHTML(attrib, src.Body))...) + } else { + sep := "" + if len(outBody) > 0 { + sep = "\n\n" + } + outBody = append(outBody, []byte(sep+attrib+"\n\n"+quotePlain(src.Body))...) + } + } } // 9b. Post-derivation resolved-recipient validation. @@ -310,6 +347,7 @@ Examples: cmd.Flags().BoolVarP(&jsonOut, "json", "j", false, "Output result as JSON") cmd.Flags().StringVar(&replyTo, "reply-to", "", "Source Gmail message ID to reply to (derives To/Subject/threading)") cmd.Flags().BoolVar(&replyAll, "reply-all", false, "Include the source To/Cc as Cc on the reply (requires --reply-to)") + cmd.Flags().BoolVar(&noQuote, "no-quote", false, "Reply without quoting the source message (requires --reply-to)") return cmd } @@ -431,6 +469,54 @@ func addRePrefix(subject string) string { return "Re: " + subject } +// replyAttribution builds the Gmail-style "On , wrote:" line that +// precedes a quoted reply. The date is rendered in the source message's own +// timezone (the offset parsed from its Date header) — never converted to UTC +// or local — and uses a fixed en-US layout. This is deliberately not localized: +// Gmail's quote-collapse is driven by the gmail_quote markup, not this text, +// and the CLI has no reliable recipient-locale source. On a Date parse failure +// the raw header value is echoed. The returned string is unescaped; the HTML +// branch escapes it before embedding. +func replyAttribution(src *gmailapi.Message) string { + when := src.Date + if t, err := mail.ParseDate(src.Date); err == nil { + when = t.Format("Mon, Jan 2, 2006 at 3:04 PM") + } + return fmt.Sprintf("On %s %s wrote:", when, src.From) +} + +// quotePlain prefixes each line of body for a plain-text reply: a non-empty +// line becomes "> " + line; an empty line becomes ">" (no trailing space, so +// the output is Gmail-faithful and free of trailing whitespace). +func quotePlain(body string) string { + lines := strings.Split(body, "\n") + for i, ln := range lines { + if ln == "" { + lines[i] = ">" + } else { + lines[i] = "> " + ln + } + } + return strings.Join(lines, "\n") +} + +// quoteHTML wraps the attribution and source body in Gmail's own quote markup +// so the Gmail web UI collapses it behind the "…" affordance when the draft is +// opened. Both the attribution and the body are HTML-escaped: src.From carries +// a raw "" and a crafted display name must not inject markup. A source +// that is HTML-only (no plain-text alternative) is therefore shown as escaped +// tags — an accepted limitation that avoids an HTML-to-text dependency. +func quoteHTML(attrib, body string) string { + escBody := strings.ReplaceAll(html.EscapeString(body), "\n", "
\n") + return fmt.Sprintf( + "
%s
\n"+ + "
\n"+ + "%s\n"+ + "
", + html.EscapeString(attrib), escBody, + ) +} + // parseAddressList parses a comma-separated address list. An empty input is OK // and returns nil — the caller decides whether emptiness is an error. func parseAddressList(flag, raw string) ([]string, error) { diff --git a/internal/cmd/mail/draft_quote_test.go b/internal/cmd/mail/draft_quote_test.go new file mode 100644 index 0000000..f1e41a1 --- /dev/null +++ b/internal/cmd/mail/draft_quote_test.go @@ -0,0 +1,219 @@ +package mail + +import ( + "context" + "testing" + + gmailapi "github.com/open-cli-collective/google-readonly/internal/gmail" + "github.com/open-cli-collective/google-readonly/internal/testutil" +) + +// --- Reply quoting tests (#125) --- + +// srcReplyWithBody returns a reply source with a body and a Date whose offset +// is chosen so that converting to UTC changes the weekday, day, and hour — any +// accidental .UTC()/.Local() in the attribution is then caught. +// +// Source-local: Tue, May 12, 2026 at 11:30 PM (-0530). +// UTC would be: Wed, May 13, 2026 at 5:00 AM. +func srcReplyWithBody() *gmailapi.Message { + s := srcReply() + s.Date = "Tue, 12 May 2026 23:30:00 -0530" + s.Body = "Line one\n\nLine two" + return s +} + +const wantAttrib = "On Tue, May 12, 2026 at 11:30 PM Alice wrote:" + +func TestDraftCommand_Reply_QuotesPlainByDefault(t *testing.T) { + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return srcReplyWithBody(), nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "T27 reply", "--plain"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + want := "T27 reply\n\n" + wantAttrib + "\n\n> Line one\n>\n> Line two" + testutil.Equal(t, string(seen.Body), want) + testutil.Equal(t, seen.BodyKind, gmailapi.DraftBodyPlainText) + // No UTC drift: must not contain the UTC-converted day/hour. + testutil.NotContains(t, string(seen.Body), "May 13") + testutil.NotContains(t, string(seen.Body), "5:00 AM") + }) +} + +func TestDraftCommand_Reply_QuotesHTMLWithGmailQuoteMarkup(t *testing.T) { + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return srcReplyWithBody(), nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "T28 **reply**"}) // markdown -> HTML + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + body := string(seen.Body) + testutil.Equal(t, seen.BodyKind, gmailapi.DraftBodyHTML) + testutil.Contains(t, body, "reply") + testutil.Contains(t, body, `
`) + testutil.Contains(t, body, `
`) + // Attribution is HTML-escaped: the raw angle brackets must not + // leak as markup. + testutil.Contains(t, body, "On Tue, May 12, 2026 at 11:30 PM Alice <alice@example.com> wrote:") + testutil.NotContains(t, body, "") + }) +} + +func TestDraftCommand_Reply_HTMLEscapesMarkupInDisplayName(t *testing.T) { + src := srcReplyWithBody() + src.From = `"" ` + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return src, nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "hi"}) // HTML branch + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + body := string(seen.Body) + testutil.NotContains(t, body, "") + testutil.Contains(t, body, "<script>alert(1)</script>") + }) +} + +func TestDraftCommand_Reply_BareReplyIsQuoteOnly(t *testing.T) { + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return srcReplyWithBody(), nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--plain"}) // zero body sources + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + // No authored body => no leading separator: body is exactly the quote. + want := wantAttrib + "\n\n> Line one\n>\n> Line two" + testutil.Equal(t, string(seen.Body), want) + }) +} + +func TestDraftCommand_Reply_NoQuoteWithBody(t *testing.T) { + var seen gmailapi.DraftMessage + sawIncludeBody := true + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, includeBody bool) (*gmailapi.Message, error) { + sawIncludeBody = includeBody + return srcReplyWithBody(), nil + }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--no-quote", "--body", "T30", "--plain"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + testutil.Equal(t, string(seen.Body), "T30") // no attribution, no quote + testutil.False(t, sawIncludeBody) // --no-quote keeps metadata-only fetch + }) +} + +func TestDraftCommand_Reply_DefaultFetchesBody(t *testing.T) { + sawIncludeBody := false + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, includeBody bool) (*gmailapi.Message, error) { + sawIncludeBody = includeBody + return srcReplyWithBody(), nil + }, + CreateDraftFunc: func(_ context.Context, _ gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "x", "--plain"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + testutil.True(t, sawIncludeBody) // default reply fetches full body + }) +} + +func TestDraftCommand_Reply_NoQuoteBareIsBlank(t *testing.T) { + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return srcReplyWithBody(), nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--no-quote", "--plain"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + testutil.Equal(t, string(seen.Body), "") // byte-empty, like a blank draft today + }) +} + +func TestDraftCommand_NoQuote_RequiresReplyTo(t *testing.T) { + cmd := newDraftCommand() + cmd.SetArgs([]string{"--to", "a@x.com", "--subject", "Hi", "--body", "x", "--no-quote"}) + withMockClient(&MockGmailClient{}, func() { + err := cmd.Execute() + testutil.Error(t, err) + testutil.Contains(t, err.Error(), "--no-quote") + testutil.Contains(t, err.Error(), "--reply-to") + }) +} + +func TestDraftCommand_Reply_EmptySourceBodySkipsQuote(t *testing.T) { + src := srcReply() // no Body + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return src, nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "x", "--plain"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + testutil.Equal(t, string(seen.Body), "x") // no attribution appended + }) +} + +func TestDraftCommand_Reply_DateParseFailureFallsBackToRaw(t *testing.T) { + src := srcReplyWithBody() + src.Date = "definitely not a date" + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return src, nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "x", "--plain"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + testutil.Contains(t, string(seen.Body), "On definitely not a date Alice wrote:") + }) +} diff --git a/internal/cmd/mail/draft_test.go b/internal/cmd/mail/draft_test.go index 7a5f70c..cd53e88 100644 --- a/internal/cmd/mail/draft_test.go +++ b/internal/cmd/mail/draft_test.go @@ -20,7 +20,7 @@ func TestDraftCommand_FlagsPresent(t *testing.T) { for _, name := range []string{ "to", "cc", "bcc", "from", "subject", "body", "stdin", "file", "plain", "html", - "attach", "json", + "attach", "json", "no-quote", } { if cmd.Flags().Lookup(name) == nil { t.Errorf("flag %q missing", name) @@ -83,7 +83,7 @@ func TestDraftCommand_RejectsMultipleBodySources(t *testing.T) { withMockClient(&MockGmailClient{}, func() { err := cmd.Execute() testutil.Error(t, err) - testutil.Contains(t, err.Error(), "exactly one of") + testutil.Contains(t, err.Error(), "at most one of") }) } @@ -892,7 +892,7 @@ func TestDraftCommand_LocalValidationBeforeFetch(t *testing.T) { withMockClient(mock, func() { err := cmd.Execute() testutil.Error(t, err) - testutil.Contains(t, err.Error(), "exactly one of") + testutil.Contains(t, err.Error(), "at most one of") }) } From a6d78d71e05d4f9b50b235b3a8e097119b0bf17d Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Fri, 15 May 2026 07:22:17 -0400 Subject: [PATCH 2/5] fix(mail): handle CRLF source bodies and cover default-mode bare reply quotePlain strips a terminal CR per line before classifying, so a CRLF blank line quotes as ">" (not "> \r") and content lines carry no stray CR; output is normalized to LF. Real email text/plain parts are CRLF. Adds a CRLF-source test and a default-mode (markdown/HTML) bare-reply test asserting no leading newline and a leading gmail_quote block. --- internal/cmd/mail/draft.go | 6 +++- internal/cmd/mail/draft_quote_test.go | 47 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/internal/cmd/mail/draft.go b/internal/cmd/mail/draft.go index 8e8acf8..3a2db31 100644 --- a/internal/cmd/mail/draft.go +++ b/internal/cmd/mail/draft.go @@ -487,10 +487,14 @@ func replyAttribution(src *gmailapi.Message) string { // quotePlain prefixes each line of body for a plain-text reply: a non-empty // line becomes "> " + line; an empty line becomes ">" (no trailing space, so -// the output is Gmail-faithful and free of trailing whitespace). +// the output is Gmail-faithful and free of trailing whitespace). A terminal +// CR is stripped per line before classification — real email text/plain parts +// are CRLF, and a CRLF blank line must yield ">" not "> \r". Output is +// normalized to LF. func quotePlain(body string) string { lines := strings.Split(body, "\n") for i, ln := range lines { + ln = strings.TrimSuffix(ln, "\r") if ln == "" { lines[i] = ">" } else { diff --git a/internal/cmd/mail/draft_quote_test.go b/internal/cmd/mail/draft_quote_test.go index f1e41a1..6650fa5 100644 --- a/internal/cmd/mail/draft_quote_test.go +++ b/internal/cmd/mail/draft_quote_test.go @@ -2,6 +2,7 @@ package mail import ( "context" + "strings" "testing" gmailapi "github.com/open-cli-collective/google-readonly/internal/gmail" @@ -217,3 +218,49 @@ func TestDraftCommand_Reply_DateParseFailureFallsBackToRaw(t *testing.T) { testutil.Contains(t, string(seen.Body), "On definitely not a date Alice wrote:") }) } + +// Real email text/plain bodies are CRLF. A CRLF blank line must quote as ">" +// (not "> \r") and content lines must not carry a trailing CR. +func TestDraftCommand_Reply_QuotesCRLFSourceBody(t *testing.T) { + src := srcReplyWithBody() + src.Body = "Line one\r\n\r\nLine two" + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return src, nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "x", "--plain"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + want := "x\n\n" + wantAttrib + "\n\n> Line one\n>\n> Line two" + testutil.Equal(t, string(seen.Body), want) + testutil.NotContains(t, string(seen.Body), "\r") + }) +} + +// The literal product path `gro mail draft --reply-to ` (no flags) defaults +// to markdown/HTML. A bare reply must have no leading newline and begin with +// the gmail_quote block. +func TestDraftCommand_Reply_BareReplyDefaultModeIsQuoteOnlyHTML(t *testing.T) { + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return srcReplyWithBody(), nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src"}) // no body source, no --plain + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + body := string(seen.Body) + testutil.Equal(t, seen.BodyKind, gmailapi.DraftBodyHTML) + testutil.True(t, strings.HasPrefix(body, `
`)) + testutil.Contains(t, body, `
Date: Fri, 15 May 2026 07:27:39 -0400 Subject: [PATCH 3/5] fix(mail): normalize CRLF in HTML quote; strengthen quote tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit quoteHTML also leaked CRLF (a "\r
" before each break) — extract a shared normalizeLF used by both quotePlain and quoteHTML so neither emits stray CRs. Real text/html and text/plain parts decode as CRLF. Tests: add HTML-CRLF coverage, a raw --html reply-quoting case, and an ordering assertion (quote follows the authored body, preceded by the "\n" separator). Fix the replyAttribution doc comment to match the comma-free format the code actually emits. --- internal/cmd/mail/draft.go | 21 ++++++----- internal/cmd/mail/draft_quote_test.go | 51 +++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/internal/cmd/mail/draft.go b/internal/cmd/mail/draft.go index 3a2db31..ee7c318 100644 --- a/internal/cmd/mail/draft.go +++ b/internal/cmd/mail/draft.go @@ -469,7 +469,15 @@ func addRePrefix(subject string) string { return "Re: " + subject } -// replyAttribution builds the Gmail-style "On , wrote:" line that +// normalizeLF collapses CRLF and lone CR to LF. Real email text/plain and +// text/html parts decode as CRLF; quoting must not leak stray CRs into the +// draft body (a "> \r" plain marker, or "\r
" in HTML). +func normalizeLF(s string) string { + s = strings.ReplaceAll(s, "\r\n", "\n") + return strings.ReplaceAll(s, "\r", "\n") +} + +// replyAttribution builds the Gmail-style "On wrote:" line that // precedes a quoted reply. The date is rendered in the source message's own // timezone (the offset parsed from its Date header) — never converted to UTC // or local — and uses a fixed en-US layout. This is deliberately not localized: @@ -487,14 +495,11 @@ func replyAttribution(src *gmailapi.Message) string { // quotePlain prefixes each line of body for a plain-text reply: a non-empty // line becomes "> " + line; an empty line becomes ">" (no trailing space, so -// the output is Gmail-faithful and free of trailing whitespace). A terminal -// CR is stripped per line before classification — real email text/plain parts -// are CRLF, and a CRLF blank line must yield ">" not "> \r". Output is -// normalized to LF. +// the output is Gmail-faithful and free of trailing whitespace). Input is +// normalized to LF first so a CRLF blank line yields ">" not "> \r". func quotePlain(body string) string { - lines := strings.Split(body, "\n") + lines := strings.Split(normalizeLF(body), "\n") for i, ln := range lines { - ln = strings.TrimSuffix(ln, "\r") if ln == "" { lines[i] = ">" } else { @@ -511,7 +516,7 @@ func quotePlain(body string) string { // that is HTML-only (no plain-text alternative) is therefore shown as escaped // tags — an accepted limitation that avoids an HTML-to-text dependency. func quoteHTML(attrib, body string) string { - escBody := strings.ReplaceAll(html.EscapeString(body), "\n", "
\n") + escBody := strings.ReplaceAll(html.EscapeString(normalizeLF(body)), "\n", "
\n") return fmt.Sprintf( "
%s
\n"+ "
\n"+ diff --git a/internal/cmd/mail/draft_quote_test.go b/internal/cmd/mail/draft_quote_test.go index 6650fa5..cfbee7b 100644 --- a/internal/cmd/mail/draft_quote_test.go +++ b/internal/cmd/mail/draft_quote_test.go @@ -70,6 +70,57 @@ func TestDraftCommand_Reply_QuotesHTMLWithGmailQuoteMarkup(t *testing.T) { // leak as markup. testutil.Contains(t, body, "On Tue, May 12, 2026 at 11:30 PM Alice <alice@example.com> wrote:") testutil.NotContains(t, body, "") + // Ordering: the quote follows the authored body and is immediately + // preceded by the "\n" separator we add. + authoredIdx := strings.Index(body, "reply") + quoteStart := strings.Index(body, `
`) + testutil.True(t, authoredIdx >= 0 && quoteStart > authoredIdx) + testutil.Equal(t, body[quoteStart-1], byte('\n')) + }) +} + +// CRLF must not leak into the HTML quote (no "\r" / "\r
"). +func TestDraftCommand_Reply_QuotesHTMLCRLFSourceBody(t *testing.T) { + src := srcReplyWithBody() + src.Body = "Line one\r\n\r\nLine two" + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return src, nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "hi"}) // HTML branch + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + body := string(seen.Body) + testutil.NotContains(t, body, "\r") + testutil.Contains(t, body, "Line one
") + testutil.Contains(t, body, "Line two") + }) +} + +// The HTML quote branch also fires for raw --html replies, not just markdown. +func TestDraftCommand_Reply_RawHTMLBodyStillQuotes(t *testing.T) { + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return srcReplyWithBody(), nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--html", "--body", "

raw body

"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + body := string(seen.Body) + testutil.Equal(t, seen.BodyKind, gmailapi.DraftBodyHTML) + testutil.True(t, strings.HasPrefix(body, "

raw body

")) // raw HTML passed verbatim + testutil.Contains(t, body, `
`) + testutil.Contains(t, body, "Line one
") }) } From 8b698be6304f325da76994096ff99a113df1217c Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Fri, 15 May 2026 07:42:22 -0400 Subject: [PATCH 4/5] fix(mail): nest HTML-only source as HTML in reply quote (Gmail parity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An HTML-only source message (no text/plain alternative — common for alert/marketing/SaaS senders) was HTML-escaped into a wall of visible tags in the quote. Gmail itself nests the original message's HTML inside the gmail_quote blockquote on reply; mirror that. - gmail: extractBodyWithKind reports whether Body came from text/html; Message gains BodyIsHTML. extractBody kept as a thin wrapper so the existing callers/tests are untouched. - mail: quoteHTML nests an HTML source verbatim and only escapes a plain-text source. The attribution line is always escaped regardless (it carries the raw From header / display name). Reverses the earlier "escape everything / documented limitation" decision: the limitation hit a common real-world sender and produced exactly the technically-valid-but-foreign output the Gmail-parity principle rejects. --- README.md | 2 +- internal/cmd/mail/draft.go | 25 +++++++++++----- internal/cmd/mail/draft_quote_test.go | 30 +++++++++++++++++++ internal/gmail/messages.go | 43 +++++++++++++++++---------- internal/gmail/messages_test.go | 39 ++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index bb280ef..c8184bc 100644 --- a/README.md +++ b/README.md @@ -303,7 +303,7 @@ Drafts always land in your Gmail Drafts folder for human review. The CLI never c With `--reply-to`, `--to` and `--subject` are derived from the source message (To = original From; Subject = `Re: `, no double prefix). Explicit `--to` / `--cc` / `--subject` flags override the derived values. `--reply-all` adds the original To and Cc as Cc on the reply, filtered to remove your own address (and any `--from` alias). -By default a reply quotes the source message below your text, like Gmail's web UI: an `On wrote:` line followed by the original body (`> ` line prefixes for plain replies; a collapsible `gmail_quote` block for HTML/markdown replies, which Gmail's UI hides behind its “…” toggle). A body source is optional when replying — a bare `--reply-to` yields a quote-only draft. Use `--no-quote` to reply without quoting (with no body source the draft is left blank). Known limitation: a source message that has only an HTML part (no plain-text alternative) is HTML-escaped and shown as visible tags inside the quote — by design, to avoid an HTML-to-text conversion dependency. +By default a reply quotes the source message below your text, like Gmail's web UI: an `On wrote:` line followed by the original body (`> ` line prefixes for plain replies; a collapsible `gmail_quote` block for HTML/markdown replies, which Gmail's UI hides behind its “…” toggle). A body source is optional when replying — a bare `--reply-to` yields a quote-only draft. Use `--no-quote` to reply without quoting (with no body source the draft is left blank). Source bodies are quoted the way Gmail itself does on reply: a plain-text source is escaped and shown with `> ` / `
`, while an HTML source (common for alert/marketing/SaaS senders that ship no plain-text part) is nested as HTML so it renders normally rather than appearing as escaped tags. The attribution line is always HTML-escaped. ### Calendar Commands diff --git a/internal/cmd/mail/draft.go b/internal/cmd/mail/draft.go index ee7c318..1091418 100644 --- a/internal/cmd/mail/draft.go +++ b/internal/cmd/mail/draft.go @@ -274,7 +274,7 @@ Examples: if len(outBody) > 0 { sep = "\n" } - outBody = append(outBody, []byte(sep+quoteHTML(attrib, src.Body))...) + outBody = append(outBody, []byte(sep+quoteHTML(attrib, src.Body, src.BodyIsHTML))...) } else { sep := "" if len(outBody) > 0 { @@ -511,18 +511,27 @@ func quotePlain(body string) string { // quoteHTML wraps the attribution and source body in Gmail's own quote markup // so the Gmail web UI collapses it behind the "…" affordance when the draft is -// opened. Both the attribution and the body are HTML-escaped: src.From carries -// a raw "" and a crafted display name must not inject markup. A source -// that is HTML-only (no plain-text alternative) is therefore shown as escaped -// tags — an accepted limitation that avoids an HTML-to-text dependency. -func quoteHTML(attrib, body string) string { - escBody := strings.ReplaceAll(html.EscapeString(normalizeLF(body)), "\n", "
\n") +// opened. +// +// The attribution is always HTML-escaped: it carries the raw From header +// (display name + "") and a crafted display name must not inject markup. +// +// The body is treated per its source kind, matching what Gmail itself does on +// reply: an HTML source (bodyIsHTML) is nested as-is so it renders normally; +// a plain-text source is escaped with newlines converted to
. This is the +// difference between a readable quote and a wall of visible tags for the many +// senders (alerts, marketing, SaaS notifications) that are HTML-only. +func quoteHTML(attrib, body string, bodyIsHTML bool) string { + quoted := normalizeLF(body) + if !bodyIsHTML { + quoted = strings.ReplaceAll(html.EscapeString(quoted), "\n", "
\n") + } return fmt.Sprintf( "
%s
\n"+ "
\n"+ "%s\n"+ "
", - html.EscapeString(attrib), escBody, + html.EscapeString(attrib), quoted, ) } diff --git a/internal/cmd/mail/draft_quote_test.go b/internal/cmd/mail/draft_quote_test.go index cfbee7b..06dc9a6 100644 --- a/internal/cmd/mail/draft_quote_test.go +++ b/internal/cmd/mail/draft_quote_test.go @@ -102,6 +102,36 @@ func TestDraftCommand_Reply_QuotesHTMLCRLFSourceBody(t *testing.T) { }) } +// An HTML-only source (no text/plain alternative) must be nested as HTML, +// like Gmail's own reply — not escaped into a wall of visible tags. The +// attribution stays escaped regardless. +func TestDraftCommand_Reply_HTMLSourceNestedUnescaped(t *testing.T) { + src := srcReplyWithBody() + src.Body = "

Hello world

" + src.BodyIsHTML = true + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return src, nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "ack"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + body := string(seen.Body) + // Source HTML embedded verbatim (renders normally in Gmail). + testutil.Contains(t, body, "

Hello world

") + testutil.NotContains(t, body, "<!DOCTYPE") + testutil.NotContains(t, body, "<p>") + // Still wrapped for collapse, and attribution still escaped. + testutil.Contains(t, body, `
hi

"))}}, + }, + } + body, isHTML := extractBodyWithKind(payload) + if body != "hi" || isHTML { + t.Errorf("got (%q, %v), want (\"hi\", false)", body, isHTML) + } + }) + + t.Run("html-only part: isHTML true", func(t *testing.T) { + t.Parallel() + payload := &gmail.MessagePart{ + MimeType: "text/html", + Body: &gmail.MessagePartBody{Data: base64.URLEncoding.EncodeToString([]byte("

only

"))}, + } + body, isHTML := extractBodyWithKind(payload) + if body != "

only

" || !isHTML { + t.Errorf("got (%q, %v), want (\"

only

\", true)", body, isHTML) + } + }) + + t.Run("no body: isHTML false", func(t *testing.T) { + t.Parallel() + body, isHTML := extractBodyWithKind(&gmail.MessagePart{MimeType: "text/plain"}) + if body != "" || isHTML { + t.Errorf("got (%q, %v), want (\"\", false)", body, isHTML) + } + }) +} + func TestMessageStruct(t *testing.T) { t.Parallel() t.Run("message struct has all fields", func(t *testing.T) { From 4712b877cb84dc077eb481e10735ed855f8954f0 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Fri, 15 May 2026 07:50:48 -0400 Subject: [PATCH 5/5] fix(mail): contain quoted HTML to a parsed body fragment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nesting raw source HTML risked container breakout: a source body with "
" could escape the gmail_quote wrapper before Gmail renders it (Gmail sanitizes active content but does not re-balance our quote containment). Parse the source with x/net/html and re-serialize the children, which (1) strips the DOCTYPE// so we no longer nest a full document inside a blockquote, and (2) makes breakout structurally impossible — stray closers are dropped by the parser since we emit a re-parsed tree, not raw bytes. Parse failure degrades safely to escaped (plain-source) treatment. Attribution stays escaped. x/net was already in the module graph (now direct). Also: BodyIsHTML is json:"-" (internal routing bit, not a public --json surface), and a parseMessage(includeBody=true) test proves an HTML-only payload sets both Body and BodyIsHTML. Breakout regression added. --- go.mod | 2 +- internal/cmd/mail/draft.go | 49 ++++++++++++++++++++++++++- internal/cmd/mail/draft_quote_test.go | 45 ++++++++++++++++++++++-- internal/gmail/messages.go | 6 ++-- internal/gmail/messages_test.go | 23 +++++++++++++ 5 files changed, 117 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 3010c91..1c5a4cd 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c github.com/spf13/cobra v1.8.0 github.com/yuin/goldmark v1.8.2 + golang.org/x/net v0.49.0 golang.org/x/oauth2 v0.34.0 google.golang.org/api v0.262.0 ) @@ -54,7 +55,6 @@ require ( go.opentelemetry.io/otel/metric v1.39.0 // indirect go.opentelemetry.io/otel/trace v1.39.0 // indirect golang.org/x/crypto v0.47.0 // indirect - golang.org/x/net v0.49.0 // indirect golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.33.0 // indirect diff --git a/internal/cmd/mail/draft.go b/internal/cmd/mail/draft.go index 1091418..afa0ae3 100644 --- a/internal/cmd/mail/draft.go +++ b/internal/cmd/mail/draft.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" "github.com/yuin/goldmark" "github.com/yuin/goldmark/extension" + xhtml "golang.org/x/net/html" gmailapi "github.com/open-cli-collective/google-readonly/internal/gmail" ) @@ -523,7 +524,9 @@ func quotePlain(body string) string { // senders (alerts, marketing, SaaS notifications) that are HTML-only. func quoteHTML(attrib, body string, bodyIsHTML bool) string { quoted := normalizeLF(body) - if !bodyIsHTML { + if bodyIsHTML { + quoted = htmlBodyFragment(quoted) + } else { quoted = strings.ReplaceAll(html.EscapeString(quoted), "\n", "
\n") } return fmt.Sprintf( @@ -535,6 +538,50 @@ func quoteHTML(attrib, body string, bodyIsHTML bool) string { ) } +// htmlBodyFragment reduces a source HTML body to a balanced fragment suitable +// for nesting inside the gmail_quote blockquote. It parses the HTML and +// re-serializes the children (or the whole tree if there is no body), +// which: (1) drops the DOCTYPE// so we don't nest a full document +// inside a blockquote, and (2) makes container breakout impossible — stray +// "
" in the source cannot escape our wrapper because we +// emit a re-serialized parse tree, not the raw bytes. Active content +// (script/handlers) is intentionally left for Gmail's render-time sanitizer, +// exactly as when the original message was read. On a parse failure the body +// is escaped, degrading safely to the plain-source treatment. +func htmlBodyFragment(s string) string { + doc, err := xhtml.Parse(strings.NewReader(s)) + if err != nil { + return strings.ReplaceAll(html.EscapeString(s), "\n", "
\n") + } + var body *xhtml.Node + var find func(*xhtml.Node) + find = func(n *xhtml.Node) { + if body != nil { + return + } + if n.Type == xhtml.ElementNode && n.Data == "body" { + body = n + return + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + find(c) + } + } + find(doc) + var buf bytes.Buffer + render := func(n *xhtml.Node) { + for c := n.FirstChild; c != nil; c = c.NextSibling { + _ = xhtml.Render(&buf, c) + } + } + if body != nil { + render(body) + } else { + render(doc) + } + return buf.String() +} + // parseAddressList parses a comma-separated address list. An empty input is OK // and returns nil — the caller decides whether emptiness is an error. func parseAddressList(flag, raw string) ([]string, error) { diff --git a/internal/cmd/mail/draft_quote_test.go b/internal/cmd/mail/draft_quote_test.go index 06dc9a6..d568f6d 100644 --- a/internal/cmd/mail/draft_quote_test.go +++ b/internal/cmd/mail/draft_quote_test.go @@ -122,16 +122,55 @@ func TestDraftCommand_Reply_HTMLSourceNestedUnescaped(t *testing.T) { withMockClient(mock, func() { testutil.NoError(t, cmd.Execute()) body := string(seen.Body) - // Source HTML embedded verbatim (renders normally in Gmail). - testutil.Contains(t, body, "

Hello world

") - testutil.NotContains(t, body, "<!DOCTYPE") + // Source HTML rendered as a body fragment (not escaped), so it + // displays normally in Gmail... + testutil.Contains(t, body, "

Hello world

") testutil.NotContains(t, body, "<p>") + // ...but the DOCTYPE// document scaffolding is stripped + // — we must not nest a full document inside a blockquote. + testutil.NotContains(t, body, "") + testutil.NotContains(t, body, "") // Still wrapped for collapse, and attribution still escaped. testutil.Contains(t, body, `
" in the source cannot prematurely close +// our container (we emit a re-parsed tree, not raw bytes), so exactly one +// closing pair exists and the source content stays contained. +func TestDraftCommand_Reply_HTMLSourceCannotBreakOutOfQuote(t *testing.T) { + src := srcReplyWithBody() + src.Body = `

real quote

forged "authored" text

` + src.BodyIsHTML = true + var seen gmailapi.DraftMessage + mock := &MockGmailClient{ + GetMessageFunc: func(_ context.Context, _ string, _ bool) (*gmailapi.Message, error) { return src, nil }, + CreateDraftFunc: func(_ context.Context, msg gmailapi.DraftMessage) (*gmailapi.DraftResult, error) { + seen = msg + return &gmailapi.DraftResult{ID: "d1"}, nil + }, + } + cmd := newDraftCommand() + cmd.SetArgs([]string{"--reply-to", "msg-src", "--body", "ack"}) + withMockClient(mock, func() { + testutil.NoError(t, cmd.Execute()) + body := string(seen.Body) + // Exactly our single wrapper close — the source's stray closers were + // dropped by the parser, so containment holds. + testutil.Equal(t, strings.Count(body, "
"), 1) + testutil.Equal(t, strings.Count(body, "
"), 1) + // Source content is preserved, just contained. + testutil.Contains(t, body, "

real quote

") + testutil.Contains(t, body, "forged") + // The wrapper close is the final thing in the body (nothing escaped + // out after it). + testutil.True(t, strings.HasSuffix(body, "")) + }) +} + // The HTML quote branch also fires for raw --html replies, not just markdown. func TestDraftCommand_Reply_RawHTMLBodyStillQuotes(t *testing.T) { var seen gmailapi.DraftMessage diff --git a/internal/gmail/messages.go b/internal/gmail/messages.go index eea2ff4..d2cecaf 100644 --- a/internal/gmail/messages.go +++ b/internal/gmail/messages.go @@ -22,9 +22,9 @@ type Message struct { Snippet string `json:"snippet"` Body string `json:"body,omitempty"` // BodyIsHTML reports that Body came from the message's text/html part - // (no text/plain alternative). Reply quoting nests HTML sources as HTML - // rather than escaping them. - BodyIsHTML bool `json:"bodyIsHtml,omitempty"` + // (no text/plain alternative). Internal routing bit for reply quoting; + // intentionally excluded from the public --json output surface. + BodyIsHTML bool `json:"-"` Attachments []*Attachment `json:"attachments,omitempty"` Labels []string `json:"labels,omitempty"` Categories []string `json:"categories,omitempty"` diff --git a/internal/gmail/messages_test.go b/internal/gmail/messages_test.go index b1b6f38..8902c0c 100644 --- a/internal/gmail/messages_test.go +++ b/internal/gmail/messages_test.go @@ -427,6 +427,29 @@ func TestParseMessageWithBody(t *testing.T) { if result.Body != bodyText { t.Errorf("got %v, want %v", result.Body, bodyText) } + if result.BodyIsHTML { + t.Errorf("plain payload: BodyIsHTML = true, want false") + } + }) + + t.Run("html-only payload sets Body and BodyIsHTML", func(t *testing.T) { + t.Parallel() + htmlContent := "

HTML only body

" + msg := &gmail.Message{ + Id: "msg-html", + Payload: &gmail.MessagePart{ + MimeType: "text/html", + Headers: []*gmail.MessagePartHeader{{Name: "Subject", Value: "Test"}}, + Body: &gmail.MessagePartBody{Data: base64.URLEncoding.EncodeToString([]byte(htmlContent))}, + }, + } + result := parseMessage(msg, true, nil) + if result.Body != htmlContent { + t.Errorf("Body = %q, want %q", result.Body, htmlContent) + } + if !result.BodyIsHTML { + t.Errorf("html-only payload: BodyIsHTML = false, want true") + } }) t.Run("excludes body when not requested", func(t *testing.T) {