diff --git a/modules/mesh-mail/cmd/mesh-mail/ask_test.go b/modules/mesh-mail/cmd/mesh-mail/ask_test.go index ad7144ac..41f20759 100644 --- a/modules/mesh-mail/cmd/mesh-mail/ask_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/ask_test.go @@ -3,27 +3,43 @@ package main // mail_send raises its ask on the operator channel, and messenger's content rule (ADR 0253 §3) refuses any // message that carries a mail address, a URL or a bare host name. ADR 0361 §2 needs the ask to show the // account, the recipients, the subject and the first lines regardless — so the ask must say all of that in -// a form the content rule lets through (novox/hq issue 868, "Decided" note on issue 868). These tests build -// real asks through mail_send and judge them with the real rule, copied verbatim as content_test.go. +// a form the content rule lets through (novox/hq issue 868, "Decided" note and review round 1). These tests +// build real asks through mail_send and judge them with the real rule, copied verbatim as content_rule.go. import ( "encoding/json" "strings" "testing" + + "git.novox.be/novox/mesh-sdk/go/asks" ) -// wholeAsk is every word the operator sees for one ask, the same text messenger's content rule judges as a -// whole (the asks router hands it Headline, Explanation and Details together). +// wholeAsk is every word the operator can ever see for one ask: its Headline, Explanation and Details +// together. Useful for a readability check (nothing unmasked is visible anywhere), but never for judging +// whether messenger would refuse the ask as a whole — judgedWords is that (novox/hq issue 868, review +// round 1): Details is filtered line by line, not judged with the rest (holder.go's sayOn). func wholeAsk(r *sendRouter, id string) string { a := r.published[id] return a.Headline + "\n" + a.Explanation + "\n" + a.Details } -// mustNotBeRefused fails the test naming what the content rule refused, and why. -func mustNotBeRefused(t *testing.T, whole string) { +// judgedWords is exactly the text messenger refuses (or lets through) an ask as a whole on (asks.go's +// TakeAsk): its headline, its explanation, what happens at expiry, and each option's label and effect. +// Details is never part of it. +func judgedWords(a asks.Ask) string { + words := []string{a.Headline, a.Explanation, a.OnExpiry} + for _, o := range a.Options { + words = append(words, o.Label, o.Does) + } + return strings.Join(words, "\n") +} + +// mustNotBeRefused fails the test naming what the content rule refused, and why — judged the way messenger +// itself judges an ask as a whole, on judgedWords alone. +func mustNotBeRefused(t *testing.T, a asks.Ask) { t.Helper() - if refusal, ok := Check(whole); !ok { - t.Fatalf("the content rule refuses the ask: %s\n%s", refusal, whole) + if refusal, ok := Check(judgedWords(a)); !ok { + t.Fatalf("the content rule refuses the ask: %s\n%s", refusal, judgedWords(a)) } } @@ -56,7 +72,7 @@ func TestMailSendAskForOwnAccountRecipientCarriesNoAddress(t *testing.T) { if !strings.Contains(whole, "your account mesh") { t.Errorf("the ask does not name the own account:\n%s", whole) } - mustNotBeRefused(t, whole) + mustNotBeRefused(t, router.published[id]) } // (2) A recipient outside the operator's own accounts is said without its address or host: its first @@ -81,7 +97,7 @@ func TestMailSendAskForOutsideRecipientCarriesNoAddress(t *testing.T) { t.Errorf("the ask does not say %q:\n%s", want, whole) } } - mustNotBeRefused(t, whole) + mustNotBeRefused(t, router.published[id]) } // (3) More than three recipients are counted rather than said one by one. @@ -106,7 +122,7 @@ func TestMailSendAskForManyRecipientsCountsTheRest(t *testing.T) { if !strings.Contains(whole, "and 2 more") { t.Errorf("the ask does not count the recipients past the first three:\n%s", whole) } - mustNotBeRefused(t, whole) + mustNotBeRefused(t, router.published[id]) } // (4) A subject holding a mail address and a link has both masked before the ask carries it. @@ -131,5 +147,72 @@ func TestMailSendAskMasksAnAddressAndALinkInTheSubject(t *testing.T) { if !strings.Contains(whole, "[address]") || !strings.Contains(whole, "[link]") { t.Errorf("the ask does not say [address] and [link] in place of what it masked:\n%s", whole) } - mustNotBeRefused(t, whole) + mustNotBeRefused(t, router.published[id]) +} + +// (5) review round 1: a subject the masking does not catch (an IP, a two-slash path-shaped reference, a +// 40-character hex hash) does not refuse the whole ask: the real content rule (content_rule.go) still judges +// the masked explanation as refused, so the subject and first lines are left out of it and pointed at Details +// instead — the ask still names the account and recipients, and still reaches the operator. +func TestMailSendAskFallsBackToDetailsWhenTheSubjectStillRefuses(t *testing.T) { + for _, subject := range []string{ + "Invoice 2026/10/15", + "server 10.0.0.1", + "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", // 40 hex characters: reHex's shape + } { + t.Run(subject, func(t *testing.T) { + b, router, _ := sendTestBundle(t, Server{Host: "127.0.0.1", Port: 25, Security: "tls"}) + rt := startBundle(t, b.Tools()) + args := map[string]any{"account": "mesh", "to": []any{"ops@example.org"}, "subject": subject, "body": "body"} + text, isErr := rt.call(SendTool, "novox.triage", args) + if isErr { + t.Fatalf("%s", text) + } + var out map[string]any + _ = json.Unmarshal([]byte(text), &out) + id := out["ask"].(string) + a := router.published[id] + mustNotBeRefused(t, a) + if !strings.Contains(a.Explanation, "mesh") { + t.Errorf("the explanation does not name the account:\n%s", a.Explanation) + } + if !strings.Contains(a.Explanation, "o… at example org") { + t.Errorf("the explanation does not name the recipient:\n%s", a.Explanation) + } + if strings.Contains(a.Explanation, subject) { + t.Errorf("the explanation still carries the subject that refused the ask:\n%s", a.Explanation) + } + if !strings.Contains(a.Details, "Details") && !strings.Contains(a.Explanation, "Details") { + t.Errorf("the explanation does not point at Details:\n%s", a.Explanation) + } + if !strings.Contains(a.Details, subject) { + t.Errorf("Details does not carry the subject (filtered line by line by messenger, as the subject's own line): %s", a.Details) + } + }) + } +} + +// (6) review round 1: a recipient written with a display name ("Name ") is matched against the +// operator's own accounts, and masked, by its address alone. +func TestMailSendAskMatchesADisplayNameRecipientByItsAddress(t *testing.T) { + b, router, _ := sendTestBundle(t, Server{Host: "127.0.0.1", Port: 25, Security: "tls"}) + rt := startBundle(t, b.Tools()) + + args := map[string]any{"account": "mesh", "to": []any{"Jochen Schoubben "}, + "subject": "Testing a display name", "body": "body"} + text, isErr := rt.call(SendTool, "novox.triage", args) + if isErr { + t.Fatalf("%s", text) + } + var out map[string]any + _ = json.Unmarshal([]byte(text), &out) + id := out["ask"].(string) + whole := wholeAsk(router, id) + if strings.Contains(whole, "@") || strings.Contains(whole, "Jochen Schoubben") { + t.Errorf("the ask carries the display-name recipient's address or name:\n%s", whole) + } + if !strings.Contains(whole, "your account mesh") { + t.Errorf("the ask does not match the display-name recipient to its own account:\n%s", whole) + } + mustNotBeRefused(t, router.published[id]) } diff --git a/modules/mesh-mail/cmd/mesh-mail/content_test.go b/modules/mesh-mail/cmd/mesh-mail/content_rule.go similarity index 100% rename from modules/mesh-mail/cmd/mesh-mail/content_test.go rename to modules/mesh-mail/cmd/mesh-mail/content_rule.go diff --git a/modules/mesh-mail/cmd/mesh-mail/copies_test.go b/modules/mesh-mail/cmd/mesh-mail/copies_test.go index 78543ed1..7597d998 100644 --- a/modules/mesh-mail/cmd/mesh-mail/copies_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/copies_test.go @@ -1,10 +1,13 @@ package main -// content_test.go is messenger's own content rule (modules/messenger/cmd/messenger/content.go, novox/hq ADR -// 0253 §3), copied here verbatim so mail_send's tests can judge its own asks against the real rule rather -// than a guess at it (novox/hq issue 868). Messenger's content rule stays messenger's: this copy is never -// built into the mesh-mail binary (it is a _test.go file, test-only), and this test pins it to the same -// bytes as its source, the way modules/networkmanager/cmd/uplink-tools/copies_test.go pins uplink.go. +// content_rule.go is messenger's own content rule (modules/messenger/cmd/messenger/content.go, novox/hq ADR +// 0253 §3), copied here verbatim — byte for byte, only the file renamed, since Go allows nothing else to +// differ (novox/hq issue 868, review round 1). mesh-mail carries it at run time, not only in its tests: the +// ask it builds is checked against the real rule before it is published (send.go), so a subject or a first +// line the masking does not catch (an IP, a hardware address, a path-shaped reference, a hex hash, …) falls +// back to a safe explanation rather than refusing the whole ask. Messenger's content rule stays messenger's: +// this is a pinned copy, never a dependency on it, the way modules/networkmanager/cmd/uplink-tools/copies_test.go +// pins its copy of uplink.go. import ( "bytes" @@ -19,12 +22,12 @@ func TestContentRuleCopyMatchesMessengers(t *testing.T) { if err != nil { t.Skipf("messenger's content.go is not beside this module (%v); nothing to pin against", err) } - mine, err := os.ReadFile("content_test.go") + mine, err := os.ReadFile("content_rule.go") if err != nil { t.Fatal(err) } if !bytes.Equal(mine, theirs) { - t.Error("mesh-mail's copy of messenger's content.go (content_test.go) differs from " + src + + t.Error("mesh-mail's copy of messenger's content.go (content_rule.go) differs from " + src + ": change this copy to match messenger's content rule, verbatim, whenever it changes") } } diff --git a/modules/mesh-mail/cmd/mesh-mail/send.go b/modules/mesh-mail/cmd/mesh-mail/send.go index d4043125..bd8f31c6 100644 --- a/modules/mesh-mail/cmd/mesh-mail/send.go +++ b/modules/mesh-mail/cmd/mesh-mail/send.go @@ -214,8 +214,14 @@ func firstChars(s string, n int) string { // Ask is the ask for this request, composed from it alone: the account, the recipients, the subject and the first // lines of the body (novox/hq ADR 0361 §2), in a form the operator channel's content rule (ADR 0253 §3) lets // through: AskTo names recipients without an address (composeAskTo, at AskMailSend time), and the subject and -// body have every mail address and link or host name masked (maskText) before they reach the ask. The mail -// itself is sent unchanged: neither masking touches s.To, s.Subject or s.Body (novox/hq issue 868). +// body have every mail address and link or host name masked (maskText) before they reach the ask. Messenger +// refuses an ask as a whole on its Headline, Explanation, OnExpiry and each option's Label and Does alone +// (asks.go's TakeAsk) — never Details, which it filters line by line instead (holder.go's sayOn) — so once +// those are composed, the real content rule (content_rule.go, mesh-mail's own run-time copy) judges them +// exactly as messenger would. When the masking still leaves something the rule refuses (an IP, a hardware +// address, a path-shaped reference, a hex hash, … novox/hq issue 868, review round 1), the subject and first +// lines are left out of the Explanation and pointed at Details instead, so the ask always reaches the operator. +// The mail itself is sent unchanged: neither masking touches s.To, s.Subject or s.Body. func (s SendRequest) Ask() asks.Ask { to := s.AskTo subject := maskText(s.Subject) @@ -224,18 +230,34 @@ func (s SendRequest) Ask() asks.Ask { if len([]rune(headline)) > asks.HeadlineLength { headline = "Send mail?" } + onExpiry := "nothing is sent" + options := []struct{ id, label, does string }{ + {OptionSend, "Send", "the mail is sent through " + s.Account + "'s outgoing server"}, + {OptionDecline, "Decline", "nothing is sent"}, + } + // judged is exactly the words messenger refuses (or lets through) an ask as a whole on. + judged := func(explanation string) string { + words := []string{headline, explanation, onExpiry} + for _, o := range options { + words = append(words, o.label, o.does) + } + return strings.Join(words, "\n") + } explanation := fmt.Sprintf("From %s to %s: %q.", s.Account, to, subject) if len([]rune(explanation)) > asks.SummaryLength { explanation = fmt.Sprintf("From %s: %q.", s.Account, subject) } + if _, ok := Check(judged(explanation)); !ok { + explanation = fmt.Sprintf("From %s to %s. The subject and first lines are in Details.", s.Account, to) + if len([]rune(explanation)) > asks.SummaryLength { + explanation = "Send mail. The subject and first lines are in Details." + } + } details := fmt.Sprintf("Asked by %s through the mesh-mail module, at %s UTC.\nAccount: %s\nTo: %s\nSubject: %s\n\n%s", s.Caller, s.At.UTC().Format("15:04 on 2 Jan"), s.Account, to, subject, body) a := asks.Ask{ID: s.ID, Headline: headline, Explanation: explanation, Details: details, - Who: asks.Operator, Expires: s.At.Add(SendAskLasts), OnExpiry: "nothing is sent"} - for _, o := range []struct{ id, label, does string }{ - {OptionSend, "Send", "the mail is sent through " + s.Account + "'s outgoing server"}, - {OptionDecline, "Decline", "nothing is sent"}, - } { + Who: asks.Operator, Expires: s.At.Add(SendAskLasts), OnExpiry: onExpiry} + for _, o := range options { binds, _ := asks.ActDigest(s.Act(o.id)) a.Options = append(a.Options, asks.Option{ID: o.id, Label: o.label, Does: o.does, Level: asks.Approve, Binds: binds}) } @@ -454,12 +476,18 @@ func composeAskTo(to []string, accounts []Account) string { } // oneAskRecipient is one recipient's words in the ask: "your account " for one of the accounts given, or -// its first letter, an ellipsis and its domain's labels as words for any other ("j… at example com"). +// its first letter, an ellipsis and its domain's labels as words for any other ("j… at example com"). A +// recipient written with a display name ("Name ") is matched and masked by its address alone, parsed the +// same way AskMailSend already validates it (novox/hq issue 868, review round 1). func oneAskRecipient(addr string, own map[string]string) string { - if name, ok := own[strings.ToLower(addr)]; ok { + bare := addr + if parsed, err := mail.ParseAddress(addr); err == nil { + bare = parsed.Address + } + if name, ok := own[strings.ToLower(bare)]; ok { return "your account " + name } - local, domain, found := strings.Cut(addr, "@") + local, domain, found := strings.Cut(bare, "@") if !found || local == "" { return "a recipient" }