fix(security): harden mail handling against header/terminal injection (0.2.5)

.beads/.gitignore

@@ -36,6 +36,7 @@ beads.right.meta.json
 # These files are machine-specific and should not be shared across clones
 .sync.lock
 sync_base.jsonl
+export-state/
 
 # NOTE: Do NOT add negation patterns (e.g., !issues.jsonl) here.
 # They would override fork protection in .git/info/exclude, allowing

.beads/issues.jsonl

@@ -1,9 +1,14 @@
 {"id":"pm-cli-0av","title":"Batch operations - delete/move/flag by query or multiple IDs","status":"closed","priority":0,"issue_type":"task","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:07.771517427-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T17:32:54.946496593-08:00","closed_at":"2026-02-14T17:32:54.946496593-08:00","close_reason":"Implemented batch operations for mail delete, move, and flag commands. All three now accept multiple IDs and support --query flag for query-based operations."}
 {"id":"pm-cli-0db","title":"Address book integration - contacts list/search","status":"closed","priority":2,"issue_type":"task","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:36.801406301-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T18:45:03.511089289-08:00","closed_at":"2026-02-14T18:45:03.511089289-08:00","close_reason":"Implemented contacts management"}
 {"id":"pm-cli-0gn","title":"Thread/conversation support - mail thread \u003cID\u003e to show full conversation","status":"closed","priority":1,"issue_type":"task","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:20.979185868-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T18:13:37.945625664-08:00","closed_at":"2026-02-14T18:13:37.945625664-08:00","close_reason":"Added mail thread command for conversation view"}
+{"id":"pm-cli-0u6","title":"Replace sh -c in mail watch --exec with argv parsing","description":"MEDIUM severity. mail watch --exec uses exec.Command(sh, -c, cmdStr) after template substitution. Latent RCE if future tokens include email content. Replace with shlex-style argv parsing; pass message ID via env var (PM_MSG_ID) instead of string substitution.","status":"closed","priority":1,"issue_type":"bug","owner":"191290+bscott@users.noreply.github.com","created_at":"2026-04-23T09:44:47.266915-07:00","created_by":"Brian Scott","updated_at":"2026-04-27T08:49:45.060819-07:00","closed_at":"2026-04-27T08:49:45.060819-07:00","close_reason":"Closed"}
 {"id":"pm-cli-2pg","title":"Test Unicode subjects - emoji and non-ASCII characters","status":"open","priority":2,"issue_type":"task","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:51.65352926-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T16:04:51.65352926-08:00"}
+{"id":"pm-cli-2sj","title":"Sanitize ANSI escapes and control chars in terminal output of email content","description":"MEDIUM severity. Subject, From, text/HTML body, attachment filenames printed raw to stdout. Attacker emails can embed ANSI escapes for output obscuring, OSC 8 hyperlink spoofing, clipboard attacks. Add sanitizeForTerminal helper applied to text-output sinks only (preserve JSON output unchanged).","status":"closed","priority":1,"issue_type":"bug","owner":"191290+bscott@users.noreply.github.com","created_at":"2026-04-23T09:44:49.661341-07:00","created_by":"Brian Scott","updated_at":"2026-04-27T08:49:45.065384-07:00","closed_at":"2026-04-27T08:49:45.065384-07:00","close_reason":"Closed"}
 {"id":"pm-cli-3dm","title":"Reply workflow - mail reply and mail reply-all","status":"closed","priority":1,"issue_type":"task","assignee":"Brian Scott","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:18.279319484-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T17:04:44.285383471-08:00","closed_at":"2026-02-14T17:04:44.285383471-08:00","close_reason":"Implemented in this session"}
 {"id":"pm-cli-47z","title":"Richer JSON output - thread context, full MIME structure, RFC3339 timestamps","status":"closed","priority":1,"issue_type":"task","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:39.528441667-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T17:58:33.883131405-08:00","closed_at":"2026-02-14T17:58:33.883131405-08:00","close_reason":"Added date_iso (RFC3339) field to JSON output"}
+{"id":"pm-cli-5jz","title":"Enforce loopback-only hosts when InsecureSkipVerify is true","description":"MEDIUM severity. IMAP and SMTP clients set InsecureSkipVerify: true without verifying host is localhost/loopback. Config-file override or --config with remote host lets attacker harvest Bridge password. Add isLoopback check before connecting in imap.Connect and smtp.Send.","status":"closed","priority":1,"issue_type":"bug","owner":"191290+bscott@users.noreply.github.com","created_at":"2026-04-23T09:44:48.467144-07:00","created_by":"Brian Scott","updated_at":"2026-04-27T08:49:45.063908-07:00","closed_at":"2026-04-27T08:49:45.063908-07:00","close_reason":"Closed"}
+{"id":"pm-cli-5px","title":"Fix SMTP header injection via Message-ID in reply/forward","description":"HIGH severity. Message-ID from attacker-controlled emails is copied verbatim into In-Reply-To and References outbound headers with no CRLF stripping. Fix by adding sanitizeHeaderValue helper in smtp/client.go and applying to Subject, InReplyTo, References, From, To, CC.","status":"closed","priority":0,"issue_type":"bug","owner":"191290+bscott@users.noreply.github.com","created_at":"2026-04-23T09:44:02.263632-07:00","created_by":"Brian Scott","updated_at":"2026-04-27T08:49:45.051606-07:00","closed_at":"2026-04-27T08:49:45.051606-07:00","close_reason":"Closed"}
+{"id":"pm-cli-6f8","title":"Fix SMTP Subject header injection via ASCII CRLF","description":"HIGH severity. encodeSubject only applies RFC 2047 Q-encoding if non-ASCII runes present. ASCII CRLF in Subject bypasses encoding and allows injecting Bcc/other headers. Covered by same sanitizeHeaderValue helper as the In-Reply-To fix.","status":"closed","priority":0,"issue_type":"bug","owner":"191290+bscott@users.noreply.github.com","created_at":"2026-04-23T09:44:42.950689-07:00","created_by":"Brian Scott","updated_at":"2026-04-27T08:49:45.056016-07:00","closed_at":"2026-04-27T08:49:45.056016-07:00","close_reason":"Closed"}
 {"id":"pm-cli-73l","title":"Search improvements - body text, boolean operators, attachment filter, size filter","status":"closed","priority":0,"issue_type":"task","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:06.897090494-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T17:33:03.844923792-08:00","closed_at":"2026-02-14T17:33:03.844923792-08:00","close_reason":"Implemented search improvements: body text search, boolean operators (AND/OR/NOT), attachment filter, and size filters"}
 {"id":"pm-cli-7jo","title":"Watch mode - mail watch --unread --exec for monitoring new mail","status":"closed","priority":2,"issue_type":"task","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:34.959997264-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T18:45:03.430100949-08:00","closed_at":"2026-02-14T18:45:03.430100949-08:00","close_reason":"Implemented mail watch command"}
 {"id":"pm-cli-864","title":"Attachment download - mail download \u003cID\u003e \u003cindex\u003e [--output PATH]","status":"closed","priority":0,"issue_type":"task","assignee":"Brian Scott","owner":"bscott@bscott.dev","created_at":"2026-02-14T16:04:05.144964427-08:00","created_by":"Brian Scott","updated_at":"2026-02-14T17:04:44.264126209-08:00","closed_at":"2026-02-14T17:04:44.264126209-08:00","close_reason":"Implemented in this session"}

AGENTS.md

@@ -253,3 +253,29 @@ pm-cli mailbox list --json
 ```
 
 If this returns mailboxes, the connection is working.
+
+## Landing the Plane (Session Completion)
+
+**When ending a work session**, you MUST complete ALL steps below. Work is NOT complete until `git push` succeeds.
+
+**MANDATORY WORKFLOW:**
+
+1. **File issues for remaining work** - Create issues for anything that needs follow-up
+2. **Run quality gates** (if code changed) - Tests, linters, builds
+3. **Update issue status** - Close finished work, update in-progress items
+4. **PUSH TO REMOTE** - This is MANDATORY:
+   ```bash
+   git pull --rebase
+   bd sync
+   git push
+   git status  # MUST show "up to date with origin"
+   ```
+5. **Clean up** - Clear stashes, prune remote branches
+6. **Verify** - All changes committed AND pushed
+7. **Hand off** - Provide context for next session
+
+**CRITICAL RULES:**
+- Work is NOT complete until `git push` succeeds
+- NEVER stop before pushing - that leaves work stranded locally
+- NEVER say "ready to push when you are" - YOU must push
+- If push fails, resolve and retry until it succeeds

docs/commands.md

@@ -532,12 +532,20 @@ pm-cli mail watch --json
 
 # Notify and read the new message
 pm-cli mail watch -e "pm-cli mail read {}"
+
+# Use environment variables instead of {} substitution
+pm-cli mail watch -e 'echo "From: $PM_MSG_FROM | Subject: $PM_MSG_SUBJECT" | logger'
 ```
 
 The watch command:
 - Polls the mailbox at regular intervals
 - Tracks message UIDs to detect new arrivals
 - Optionally executes a command with the message ID substituted for `{}`
+- Exposes message metadata as environment variables to the executed command:
+  `PM_MSG_SEQ`, `PM_MSG_UID` (numeric), `PM_MSG_FROM`, `PM_MSG_SUBJECT`
+  (sanitized for CR/LF). Prefer these over `{}` for non-numeric data —
+  the exec template is passed to `sh -c`, so any string-substituted token
+  carrying email-derived content would be a shell-injection sink.
 - Handles Ctrl+C gracefully for clean shutdown
 - Supports JSON output for integration with scripts and AI agents
 

internal/cli/cli.go

@@ -5,7 +5,7 @@ import (
 	"github.com/bscott/pm-cli/internal/output"
 )
 
-var Version = "0.2.4"
+var Version = "0.2.5"
 
 type Globals struct {
 	JSON     bool   `help:"Output as JSON" name:"json"`

internal/cli/mail.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/bscott/pm-cli/internal/config"
 	"github.com/bscott/pm-cli/internal/imap"
+	"github.com/bscott/pm-cli/internal/safetext"
 	"github.com/bscott/pm-cli/internal/smtp"
 	"github.com/emersion/go-message/mail"
 	"gopkg.in/yaml.v3"
@@ -99,12 +100,12 @@ func (c *MailListCmd) Run(ctx *Context) error {
 			flags = "-"
 		}
 
-		subject := msg.Subject
+		subject := safetext.SanitizeForTerminal(msg.Subject)
 		if len(subject) > 50 {
 			subject = subject[:47] + "..."
 		}
 
-		from := msg.From
+		from := safetext.SanitizeForTerminal(msg.From)
 		if len(from) > 25 {
 			from = from[:22] + "..."
 		}
@@ -167,8 +168,8 @@ func (c *MailReadCmd) Run(ctx *Context) error {
 		for _, att := range attachments {
 			table.AddRow(
 				fmt.Sprintf("%d", att.Index),
-				att.Filename,
-				att.ContentType,
+				safetext.SanitizeForTerminal(att.Filename),
+				safetext.SanitizeForTerminal(att.ContentType),
 				formatSize(att.Size),
 			)
 		}
@@ -226,19 +227,23 @@ func (c *MailReadCmd) Run(ctx *Context) error {
 		return nil
 	}
 
-	fmt.Printf("From:    %s\n", msg.From)
-	fmt.Printf("To:      %s\n", strings.Join(msg.To, ", "))
+	// Sanitize every field derived from the received email before printing.
+	// An attacker sending an email can embed ANSI/OSC escape sequences in
+	// headers and body; writing them to a TTY lets them obscure output or
+	// spoof terminal hyperlinks.
+	fmt.Printf("From:    %s\n", safetext.SanitizeForTerminal(msg.From))
+	fmt.Printf("To:      %s\n", safetext.SanitizeForTerminal(strings.Join(msg.To, ", ")))
 	if len(msg.CC) > 0 {
-		fmt.Printf("CC:      %s\n", strings.Join(msg.CC, ", "))
+		fmt.Printf("CC:      %s\n", safetext.SanitizeForTerminal(strings.Join(msg.CC, ", ")))
 	}
 	fmt.Printf("Date:    %s\n", msg.Date)
-	fmt.Printf("Subject: %s\n", msg.Subject)
+	fmt.Printf("Subject: %s\n", safetext.SanitizeForTerminal(msg.Subject))
 	if msg.MessageID != "" {
-		fmt.Printf("Message-ID: %s\n", msg.MessageID)
+		fmt.Printf("Message-ID: %s\n", safetext.SanitizeForTerminal(msg.MessageID))
 	}
 
 	if c.Headers {
-		fmt.Printf("Flags:   %s\n", strings.Join(msg.Flags, ", "))
+		fmt.Printf("Flags:   %s\n", safetext.SanitizeForTerminal(strings.Join(msg.Flags, ", ")))
 		fmt.Printf("UID:     %d\n", msg.UID)
 		fmt.Printf("Seq:     %d\n", msg.SeqNum)
 	}
@@ -254,22 +259,22 @@ func (c *MailReadCmd) Run(ctx *Context) error {
 		if c.HTML {
 			// Output HTML body directly
 			if htmlBody != "" {
-				fmt.Println(htmlBody)
+				fmt.Println(safetext.SanitizeForTerminal(htmlBody))
 			} else if textBody != "" {
 				// No HTML, output text
-				fmt.Println(textBody)
+				fmt.Println(safetext.SanitizeForTerminal(textBody))
 			} else {
 				fmt.Println("[No body content]")
 			}
 		} else {
 			// Default: output plain text
 			if textBody != "" {
-				fmt.Println(textBody)
+				fmt.Println(safetext.SanitizeForTerminal(textBody))
 			} else if htmlBody != "" {
 				// Convert HTML to plain text
 				text := htmlToText(htmlBody)
 				if text != "" {
-					fmt.Println(text)
+					fmt.Println(safetext.SanitizeForTerminal(text))
 				} else {
 					fmt.Println("[HTML content - use --html to view]")
 				}
@@ -812,12 +817,12 @@ func (c *MailSearchCmd) Run(ctx *Context) error {
 			flags = "-"
 		}
 
-		subject := msg.Subject
+		subject := safetext.SanitizeForTerminal(msg.Subject)
 		if len(subject) > 50 {
 			subject = subject[:47] + "..."
 		}
 
-		from := msg.From
+		from := safetext.SanitizeForTerminal(msg.From)
 		if len(from) > 25 {
 			from = from[:22] + "..."
 		}
@@ -1378,7 +1383,10 @@ func (c *MailDownloadCmd) Run(ctx *Context) error {
 		})
 	}
 
-	fmt.Printf("Saved %s (%d bytes) to %s\n", attachment.Filename, len(attachment.Data), outPath)
+	fmt.Printf("Saved %s (%d bytes) to %s\n",
+		safetext.SanitizeForTerminal(attachment.Filename),
+		len(attachment.Data),
+		safetext.SanitizeForTerminal(outPath))
 	return nil
 }
 
@@ -1773,8 +1781,8 @@ func (c *MailWatchCmd) Run(ctx *Context) error {
 					})
 				} else {
 					fmt.Printf("\n[NEW] %s\n", msg.Date)
-					fmt.Printf("  From:    %s\n", msg.From)
-					fmt.Printf("  Subject: %s\n", msg.Subject)
+					fmt.Printf("  From:    %s\n", safetext.SanitizeForTerminal(msg.From))
+					fmt.Printf("  Subject: %s\n", safetext.SanitizeForTerminal(msg.Subject))
 					fmt.Printf("  ID:      %d\n", msg.SeqNum)
 				}
 
@@ -1844,12 +1852,23 @@ func (c *MailWatchCmd) checkForNewMessages(ctx *Context, seenUIDs map[uint32]boo
 }
 
 func (c *MailWatchCmd) executeCommand(ctx *Context, msg imap.MessageSummary) {
-	// Replace {} with the message ID
+	// Replace {} with the (numeric, validated) sequence number. Do NOT
+	// add substitution tokens for email-derived string data (From, Subject,
+	// Message-ID, etc.) because this command is passed through `sh -c`;
+	// expose those fields via environment variables instead, where shell
+	// word-splitting still applies but the raw value is not interpolated
+	// into the command text.
 	cmdStr := strings.Replace(c.Exec, "{}", fmt.Sprintf("%d", msg.SeqNum), -1)
 
 	ctx.Formatter.Verbosef("Executing: %s", cmdStr)
 
 	cmd := exec.Command("sh", "-c", cmdStr)
+	cmd.Env = append(os.Environ(),
+		fmt.Sprintf("PM_MSG_SEQ=%d", msg.SeqNum),
+		fmt.Sprintf("PM_MSG_UID=%d", msg.UID),
+		"PM_MSG_FROM="+safetext.SanitizeHeaderValue(msg.From),
+		"PM_MSG_SUBJECT="+safetext.SanitizeHeaderValue(msg.Subject),
+	)
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
 
@@ -1908,18 +1927,18 @@ func (c *MailThreadCmd) Run(ctx *Context) error {
 		if i > 0 {
 			fmt.Println(strings.Repeat("-", 60))
 		}
-		fmt.Printf("\nFrom:    %s\n", msg.From)
-		fmt.Printf("To:      %s\n", msg.To)
+		fmt.Printf("\nFrom:    %s\n", safetext.SanitizeForTerminal(msg.From))
+		fmt.Printf("To:      %s\n", safetext.SanitizeForTerminal(msg.To))
 		fmt.Printf("Date:    %s\n", msg.Date)
-		fmt.Printf("Subject: %s\n", msg.Subject)
+		fmt.Printf("Subject: %s\n", safetext.SanitizeForTerminal(msg.Subject))
 		if !msg.Seen {
 			fmt.Print("[UNREAD] ")
 		}
 		fmt.Printf("(ID: %d)\n", msg.SeqNum)
 		fmt.Println()
 
 		// Show body (truncated for readability)
-		body := msg.Body
+		body := safetext.SanitizeForTerminal(msg.Body)
 		if len(body) > 500 {
 			body = body[:500] + "\n[... truncated ...]"
 		}

internal/imap/client.go

@@ -10,10 +10,29 @@ import (
 	"time"
 
 	"github.com/bscott/pm-cli/internal/config"
+	"github.com/bscott/pm-cli/internal/safetext"
 	"github.com/emersion/go-imap/v2"
 	"github.com/emersion/go-imap/v2/imapclient"
 )
 
+// isLoopbackHost reports whether host is a loopback address. Accepts the
+// literal "localhost" (case-insensitive) or any IP that parses as loopback.
+// Does not resolve DNS; DNS is itself untrusted and we want a hard
+// guarantee that we are speaking to a local process (Proton Bridge).
+func isLoopbackHost(host string) bool {
+	h := strings.ToLower(strings.TrimSpace(host))
+	if h == "" {
+		return false
+	}
+	if h == "localhost" {
+		return true
+	}
+	if ip := net.ParseIP(h); ip != nil {
+		return ip.IsLoopback()
+	}
+	return false
+}
+
 type Client struct {
 	client *imapclient.Client
 	config *config.Config
@@ -115,6 +134,14 @@ func NewClient(cfg *config.Config) (*Client, error) {
 }
 
 func (c *Client) Connect() error {
+	// TLS skip-verify below is only safe against a locally-running Proton
+	// Bridge. Refuse to send the Bridge password to anything that is not a
+	// loopback address, in case the user's config was tampered with or
+	// redirected via --config.
+	if !isLoopbackHost(c.config.Bridge.IMAPHost) {
+		return fmt.Errorf("refusing to connect: IMAP host %q is not a loopback address (Proton Bridge runs on localhost; InsecureSkipVerify is unsafe for remote hosts)", c.config.Bridge.IMAPHost)
+	}
+
 	password, err := c.config.GetPassword()
 	if err != nil {
 		return fmt.Errorf("failed to get password: %w", err)
@@ -1329,23 +1356,34 @@ func (c *Client) DeleteDraft(ids []string) error {
 	return c.DeleteMessages("Drafts", ids, true)
 }
 
+// sanitizeAddressList strips CR/LF from each address and joins with ", ".
+func sanitizeAddressList(addrs []string) string {
+	clean := make([]string, len(addrs))
+	for i, a := range addrs {
+		clean[i] = safetext.SanitizeHeaderValue(a)
+	}
+	return strings.Join(clean, ", ")
+}
+
 // buildDraftMessage creates an RFC822 message from a Draft
 func buildDraftMessage(draft *Draft, from string) string {
 	var sb strings.Builder
 
-	// Headers
-	sb.WriteString(fmt.Sprintf("From: %s\r\n", from))
+	// Headers — strip CR/LF from every value to prevent header injection
+	// if draft data was ever sourced from untrusted input (e.g. a forward
+	// template pulling Subject/From from a received email).
+	sb.WriteString(fmt.Sprintf("From: %s\r\n", safetext.SanitizeHeaderValue(from)))
 
 	if len(draft.To) > 0 {
-		sb.WriteString(fmt.Sprintf("To: %s\r\n", strings.Join(draft.To, ", ")))
+		sb.WriteString(fmt.Sprintf("To: %s\r\n", sanitizeAddressList(draft.To)))
 	}
 
 	if len(draft.CC) > 0 {
-		sb.WriteString(fmt.Sprintf("Cc: %s\r\n", strings.Join(draft.CC, ", ")))
+		sb.WriteString(fmt.Sprintf("Cc: %s\r\n", sanitizeAddressList(draft.CC)))
 	}
 
 	if draft.Subject != "" {
-		sb.WriteString(fmt.Sprintf("Subject: %s\r\n", draft.Subject))
+		sb.WriteString(fmt.Sprintf("Subject: %s\r\n", safetext.SanitizeHeaderValue(draft.Subject)))
 	}
 
 	sb.WriteString(fmt.Sprintf("Date: %s\r\n", time.Now().Format(time.RFC1123Z)))