diff --git a/internal/agent/export_test.go b/internal/agent/export_test.go
index df2493484..a1c02d686 100644
--- a/internal/agent/export_test.go
+++ b/internal/agent/export_test.go
@@ -47,6 +47,28 @@ func MarkSideEffectCommittedForTest(w http.ResponseWriter) {
markSideEffectCommitted(w)
}
+// WriteMagicMessageForTest exposes writeMagicMessage so the external
+// agent_test package can verify the magic-link result page's HTML-escape
+// contract directly (writeMagicMessage is the single place that escapes
+// body; driving every handler path that can reach it would duplicate that
+// coverage without testing anything writeMagicMessage itself doesn't
+// already own).
+func WriteMagicMessageForTest(w http.ResponseWriter, status int, title, body string) {
+ writeMagicMessage(w, status, title, body)
+}
+
+// TruncatePreviewBytesForTest exposes truncatePreviewBytes for a direct
+// unit test of the rune-boundary-safe byte cap on the confirm page's body
+// preview.
+func TruncatePreviewBytesForTest(s string, maxBytes int) string {
+ return truncatePreviewBytes(s, maxBytes)
+}
+
+// MaxBodyPreviewBytesForTest is the confirm page's body-preview byte cap,
+// exposed so tests assert against the real constant instead of a hardcoded
+// copy that could drift from it.
+const MaxBodyPreviewBytesForTest = maxBodyPreviewBytes
+
// BuildBlockedOutboundEventForTest exposes buildBlockedOutboundEvent (and the
// blockAuditID soft-ref it is keyed on) so the external agent_test package can
// drive the exact event the emit path constructs through a real outbox +
diff --git a/internal/agent/hitl_magic_api.go b/internal/agent/hitl_magic_api.go
index 29f0ca6c8..364af9bc1 100644
--- a/internal/agent/hitl_magic_api.go
+++ b/internal/agent/hitl_magic_api.go
@@ -306,7 +306,7 @@ func (a *API) magicApprove(w http.ResponseWriter, r *http.Request, messageID, us
return
}
if supErr := a.checkSuppressionStrict(r.Context(), userID, agent.ID, sendReq); supErr != nil {
- writeMagicMessage(w, supErr.Status, "Cannot send", html.EscapeString(supErr.Msg))
+ writeMagicMessage(w, supErr.Status, "Cannot send", supErr.Msg)
return
}
// Flow-cap re-check — same rationale and ordering as ApprovePendingCore
@@ -314,11 +314,11 @@ func (a *API) magicApprove(w http.ResponseWriter, r *http.Request, messageID, us
// metered at accept, so approve time is the enforcement point.
if qerr := a.checkApproveQuota(r.Context(), userID,
identity.UniqueRecipientCount(sendReq.To, sendReq.CC, sendReq.BCC)); qerr != nil {
- writeMagicMessage(w, qerr.Status, "Cannot send", html.EscapeString(qerr.Msg))
+ writeMagicMessage(w, qerr.Status, "Cannot send", qerr.Msg)
return
}
if uerr := prepareManagedUnsubscribe(r.Context(), a.unsubscribeIssuer, a.fromDomain, userID, agent, &sendReq, true); uerr != nil {
- writeMagicMessage(w, uerr.Status, "Cannot send", html.EscapeString(uerr.Msg))
+ writeMagicMessage(w, uerr.Status, "Cannot send", uerr.Msg)
return
}
sent, handled, aerr := a.approveOutboundAsyncWithRequest(r.Context(), agent, messageID, userID, draft, identity.PendingApprovalEdit{}, sendReq, nil)
@@ -330,7 +330,7 @@ func (a *API) magicApprove(w http.ResponseWriter, r *http.Request, messageID, us
log.Printf("[mail:%s] dir=outbound type=%s status=%s agent=%s to_count=%d to_domains=%v approved=magic-link:user:%s delivery=async",
sent.ID, sent.Type, sent.Status, agent.EmailAddress(), len(sent.ToRecipients), logredact.AddressDomains(sent.ToRecipients), userID)
writeMagicResult(w, http.StatusOK, "Approved",
- fmt.Sprintf("Your message to %s has been queued for delivery.", html.EscapeString(firstRecipient(sent.ToRecipients))),
+ fmt.Sprintf("Your message to %s has been queued for delivery.", firstRecipient(sent.ToRecipients)),
viewMessageCTA(agent.EmailAddress(), draft.ConversationID, sent.ID))
return
}
@@ -349,8 +349,7 @@ func (a *API) magicApprove(w http.ResponseWriter, r *http.Request, messageID, us
default:
var ve *outbound.ValidationError
if errors.As(err, &ve) {
- writeMagicMessage(w, http.StatusBadRequest, "Cannot send",
- html.EscapeString(ve.Error()))
+ writeMagicMessage(w, http.StatusBadRequest, "Cannot send", ve.Error())
return
}
log.Printf("[api] magic-approve send failed: msg=%s err=%v", messageID, err)
@@ -367,7 +366,7 @@ func (a *API) magicApprove(w http.ResponseWriter, r *http.Request, messageID, us
writeMagicResult(w, http.StatusOK,
"Approved",
- fmt.Sprintf("Your message to %s has been sent.", html.EscapeString(firstRecipient(sent.ToRecipients))),
+ fmt.Sprintf("Your message to %s has been sent.", firstRecipient(sent.ToRecipients)),
viewMessageCTA(agent.EmailAddress(), draft.ConversationID, sent.ID))
}
@@ -386,7 +385,7 @@ func writeMagicApproveError(w http.ResponseWriter, messageID string, err error)
default:
var ve *outbound.ValidationError
if errors.As(err, &ve) {
- writeMagicMessage(w, http.StatusBadRequest, "Cannot send", html.EscapeString(ve.Error()))
+ writeMagicMessage(w, http.StatusBadRequest, "Cannot send", ve.Error())
return
}
log.Printf("[api] magic-approve accept failed: msg=%s err=%v", messageID, err)
@@ -783,6 +782,11 @@ func setMagicHeaders(w http.ResponseWriter, status int) {
w.Header().Set("Referrer-Policy", "no-referrer")
// Discourage indexing if a link ever leaks publicly.
w.Header().Set("X-Robots-Tag", "noindex, nofollow")
+ // Defense in depth: these pages emit no `,
+ "ampersand": "Tom & Jerry",
+ "quote_attr": `">
`,
+ }
+ for name, body := range cases {
+ t.Run(name, func(t *testing.T) {
+ rec := httptest.NewRecorder()
+ agent.WriteMagicMessageForTest(rec, http.StatusBadRequest, "Cannot send", body)
+ got := rec.Body.String()
+
+ if strings.Contains(got, body) {
+ t.Fatalf("result page contains the raw, unescaped body %q:\n%s", body, got)
+ }
+ want := html.EscapeString(body)
+ if !strings.Contains(got, want) {
+ t.Fatalf("result page missing the escaped body %q:\n%s", want, got)
+ }
+ if doubled := html.EscapeString(want); strings.Contains(got, doubled) {
+ t.Fatalf("result page double-escaped the body (found %q):\n%s", doubled, got)
+ }
+ })
+ }
+}
+
+// --- Confirm-page body-preview byte cap (issue #118 item 2) ---
+
+func TestTruncatePreviewBytesStaysUnderCapOnRuneBoundary(t *testing.T) {
+ // 金 is 3 bytes in UTF-8; a cap that isn't a multiple of 3 forces the
+ // truncation point to fall mid-rune unless it backs off to the last
+ // complete rune, exactly what the maintainer's own suggested
+ // `[:maxPreviewBytes/4]` snippet (rune-count, not byte-count) got wrong.
+ s := strings.Repeat("金", 10)
+ got := agent.TruncatePreviewBytesForTest(s, 7)
+ if !utf8.ValidString(got) {
+ t.Fatalf("truncated string is not valid UTF-8: %q", got)
+ }
+ if !strings.HasPrefix(s, strings.TrimSuffix(got, "\n[truncated]")) {
+ t.Fatalf("truncated output is not a prefix of the input: got %q", got)
+ }
+
+ if got := agent.TruncatePreviewBytesForTest("short", 100); got != "short" {
+ t.Errorf("under the cap should be returned unchanged, got %q", got)
+ }
+}
+
+func TestMagicLinkConfirmPageCapsBodyPreview(t *testing.T) {
+ server, store, signer, _ := setupMagicLinkAPI(t)
+ a, _ := prepareHITLAgent(t, store, "preview-cap")
+ big := strings.Repeat("a", agent.MaxBodyPreviewBytesForTest*2)
+ msg := issuePendingWithBody(t, store, a.ID, big)
+
+ tok, _ := signer.Sign(msg.ID, approvaltoken.ActionApprove, time.Now().Add(1*time.Hour))
+ resp, _ := http.Get(server.URL + "/v1/approve?t=" + url.QueryEscape(tok))
+ body := readBody(t, resp)
+
+ if strings.Contains(body, big) {
+ t.Fatalf("confirm page echoed the full %d-byte body untruncated", len(big))
+ }
+ if len(body) >= len(big) {
+ t.Errorf("confirm page response is %d bytes, want well under the %d-byte body (preview should be capped at %d)",
+ len(body), len(big), agent.MaxBodyPreviewBytesForTest)
}
}