diff --git a/modules/mesh-mail/cmd/mesh-mail/tools.go b/modules/mesh-mail/cmd/mesh-mail/tools.go index 667c7fbd..14c9e9d3 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools.go @@ -725,11 +725,13 @@ func orderByArg(args map[string]any) (string, error) { return s, nil } -// orderArg is 1230: asc or desc; anything else is refused (AC16), before the index is ever asked. +// orderArg is 1230: asc or desc; desc (newest, largest, ...) when left out, as the schema says (review round 1 +// finding 5: this returned ascending on nothing given, contradicting its own description); anything else is +// refused (AC16), before the index is ever asked. func orderArg(args map[string]any) (bool, error) { v, ok := args["order"] if !ok || v == nil { - return false, nil + return true, nil } s, ok := v.(string) if !ok { diff --git a/modules/mesh-mail/cmd/mesh-mail/tools_test.go b/modules/mesh-mail/cmd/mesh-mail/tools_test.go index e81c1f2d..747b1d30 100644 --- a/modules/mesh-mail/cmd/mesh-mail/tools_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/tools_test.go @@ -276,13 +276,58 @@ func TestARefusalSaysWhyTheGrantsCouldNotBeRead(t *testing.T) { // 1228/AC1, 1232: with no filter at all (not even query), mail_search answers the newest mails of the granted // accounts, not a refusal demanding a query. +// 1228/AC1, 1232 (review round 1 finding 7): several mails over two granted accounts and folders, one mail of a +// third account the caller is not granted, and a limit below the granted count — the newest granted mails +// answer, in that exact order, never the ungranted account's, even though its mail is the newest of all. func TestMailSearchWithNoFilterAnswersTheNewestGrantedMail(t *testing.T) { - b, _, _ := aBundle(t) + dir := t.TempDir() + env := Env{Accounts: filepath.Join(dir, "accounts.json"), Grants: filepath.Join(dir, "grants.json"), Secrets: dir, + Status: filepath.Join(dir, "health.json")} + writeFile(t, env.Accounts, `{"accounts": { + "mesh": {"kind": "imap", "address": "jschoubben@novox.be", "incoming": {"host": "mail.novox.be", "port": 993, "security": "tls"}}, + "gmail": {"kind": "imap", "address": "me@gmail.example", "incoming": {"host": "imap.gmail.com", "port": 993, "security": "tls"}}, + "secret": {"kind": "imap", "address": "other@example.org", "incoming": {"host": "mail.example.org", "port": 993, "security": "tls"}}}}`) + writeFile(t, env.Grants, `{"grants": {"mesh": ["triage"], "gmail": ["triage"], "secret": ["other"]}}`) + st := newMemStore() + ctx := context.Background() + at := func(h int) time.Time { return judgedAt.Add(time.Duration(h) * time.Hour) } + for _, m := range []Message{ + {Account: "mesh", Folder: "INBOX", UID: 1, MessageID: "", Subject: "m1", Date: at(1), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mesh", Folder: "INBOX", UID: 2, MessageID: "", Subject: "m2", Date: at(2), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mesh", Folder: "Sent", UID: 1, MessageID: "", Subject: "m3", Date: at(3), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "gmail", Folder: "INBOX", UID: 1, MessageID: "", Subject: "m4", Date: at(4), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "gmail", Folder: "Sent", UID: 1, MessageID: "", Subject: "m5", Date: at(5), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + {Account: "mesh", Folder: "Junk", UID: 1, MessageID: "", Subject: "m6", Date: at(6), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + // The newest of all, but in an account triage is not granted: it must never answer. + {Account: "secret", Folder: "INBOX", UID: 1, MessageID: "", Subject: "s1", Date: at(7), + From: []Person{{Address: "a@example.org"}}, To: []Person{}, Flags: []string{}, Attachments: []AttachmentRef{}}, + } { + if err := st.Add(ctx, m, nil); err != nil { + t.Fatal(err) + } + } + b := &Bundle{Env: env, Index: func(context.Context) (Store, error) { return st, nil }, Now: func() time.Time { return judgedAt }} rt := startBundle(t, b.Tools()) - text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{}) - if isErr || !strings.Contains(text, `"message-id":"att@example.org"`) { + text, isErr := rt.call(SearchTool, "novox.triage", map[string]any{"limit": 3}) + if isErr { t.Fatalf("%v %s", isErr, text) } + if strings.Contains(text, "secret") || strings.Contains(text, "s1@secret") { + t.Fatalf("the ungranted account's mail answered: %s", text) + } + i6, i5, i4 := strings.Index(text, "m6@mesh"), strings.Index(text, "m5@gmail"), strings.Index(text, "m4@gmail") + if i6 < 0 || i5 < 0 || i4 < 0 || !(i6 < i5 && i5 < i4) { + t.Fatalf("not exactly m6, m5, m4 in that (newest-first) order: %s", text) + } + if strings.Contains(text, "m3@mesh") || strings.Contains(text, "m2@mesh") || strings.Contains(text, "m1@mesh") { + t.Fatalf("limit 3 answered more than the 3 newest: %s", text) + } } // 1232/AC16: an unknown order-by, a limit over 50, a date that does not parse, or an account not granted are @@ -394,4 +439,15 @@ func TestMailSearchEndToEndFiltersAndGroupBySender(t *testing.T) { grouped.Senders[0].Address != "alice@example.org" || grouped.Senders[0].Count != 2 { t.Fatalf("%v %s", isErr, text) } + + // 1232/AC17 (review round 1 finding 5): order-by given, order left out, defaults to desc, as the schema's + // own description promises — not asc. "from alice" is a prefix of "from alice again", so ascending would put + // first; descending puts first. + text, isErr = rt.call(SearchTool, "novox.triage", map[string]any{"order-by": "subject"}) + if isErr { + t.Fatalf("%v %s", isErr, text) + } + if first, second := strings.Index(text, "b@mesh"), strings.Index(text, "a@mesh"); first < 0 || second < 0 || first > second { + t.Fatalf("order-by with no order was not desc by default: %s", text) + } }