From 6c31c8d0c69a7789138a40b11534025675ba7c98 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:27:14 +0200 Subject: [PATCH] fix: stop filters saved on the phone from moving spam out of Junk Stalwart only files spam into Junk when no rule decided where the message goes, so the webmail now adds `not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50"` to every move and copy rule when the server supports `spamtestplus` and `relational`, with a per-rule "Also move messages marked as spam" opt-in stored as `includeSpam` (6241ed61). Saving filters on the phone dropped the guard and the opt-in, so every folder rule started pulling spam out of Junk again, and opted-in rules lost their setting. Port the guard and keep `includeSpam` in the rules metadata. The rule editor gets the opt-in switch while the rule moves or copies mail. The round-trip fixtures now cover the guard and a script with every newer feature plus a hand-written rule. Both clients add two blank lines before a script's external rules on each save, so that fixture is compared with what the webmail itself writes when it saves it again. --- src/components/filters/FilterRuleModal.tsx | 17 +++++- .../sieve/__tests__/fixtures/webmail/index.ts | 46 +++++++++++++++ .../fixtures/webmail/spam-guard.sieve | 30 ++++++++++ .../webmail/stalwart-full.resaved.sieve | 47 +++++++++++++++ .../fixtures/webmail/stalwart-full.sieve | 45 ++++++++++++++ src/lib/sieve/__tests__/generator.test.ts | 34 +++++++++++ .../sieve/__tests__/webmail-fixtures.test.ts | 59 ++++++++----------- src/lib/sieve/generator.ts | 42 ++++++++++++- src/lib/sieve/types.ts | 5 ++ src/stores/__tests__/filter-store.test.ts | 43 +++++++++++--- 10 files changed, 323 insertions(+), 45 deletions(-) create mode 100644 src/lib/sieve/__tests__/fixtures/webmail/index.ts create mode 100644 src/lib/sieve/__tests__/fixtures/webmail/spam-guard.sieve create mode 100644 src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve create mode 100644 src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.sieve diff --git a/src/components/filters/FilterRuleModal.tsx b/src/components/filters/FilterRuleModal.tsx index acd8fc52..222d8a28 100644 --- a/src/components/filters/FilterRuleModal.tsx +++ b/src/components/filters/FilterRuleModal.tsx @@ -86,6 +86,7 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F const [conditions, setConditions] = useState(() => seedConditions(rule)); const [actions, setActions] = useState(() => seedActions(rule)); const [stopProcessing, setStopProcessing] = useState(rule?.stopProcessing ?? false); + const [includeSpam, setIncludeSpam] = useState(rule?.includeSpam ?? false); // The Modal stays mounted between opens, so re-seed every field whenever it // is (re)opened for a different rule - otherwise "Add Rule" after editing @@ -98,6 +99,7 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F setConditions(seedConditions(rule)); setActions(seedActions(rule)); setStopProcessing(rule?.stopProcessing ?? false); + setIncludeSpam(rule?.includeSpam ?? false); }, [visible, rule]); const mailboxTargets = useMemo(() => buildMailboxTargets(mailboxes), [mailboxes]); @@ -217,8 +219,11 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F conditions: validConditions, actions: validActions, stopProcessing, + // Only folder moves are kept out of Junk, so the opt-in only means + // something (and is only stored) while the rule has one. + ...(includeSpam && validActions.some((a) => ACTIONS_WITH_MAILBOX.has(a.type)) ? { includeSpam: true } : {}), }); - }, [name, conditions, actions, matchType, stopProcessing, rule, onSave, t, mailboxTargets]); + }, [name, conditions, actions, matchType, stopProcessing, includeSpam, rule, onSave, t, mailboxTargets]); return ( @@ -443,6 +448,16 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F + + {/* Folder rules skip spam unless the rule opts in (webmail 6241ed61). */} + {actions.some((a) => ACTIONS_WITH_MAILBOX.has(a.type)) && ( + + + {t('settings.filters.include_spam', 'Also move messages marked as spam')} + + + + )} diff --git a/src/lib/sieve/__tests__/fixtures/webmail/index.ts b/src/lib/sieve/__tests__/fixtures/webmail/index.ts new file mode 100644 index 00000000..5eb1583e --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/index.ts @@ -0,0 +1,46 @@ +// Sieve scripts written by the webmail's own generator (jmap-webmail +// lib/sieve/generator.ts at e33ab899), used to check that a save on the +// phone gives back the same script. Each one was generated from the rules in +// its metadata for a server listing `extensions`. +import { existsSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +export interface WebmailFixture { + file: string; + /** The server's sieveExtensions the webmail generated the script for. */ + extensions?: string[]; +} + +// A Stalwart 0.16 style extension list (a subset of what it advertises). +export const STALWART_EXTENSIONS = [ + 'body', 'comparator-i;ascii-casemap', 'comparator-i;ascii-numeric', 'comparator-i;octet', + 'copy', 'date', 'envelope', 'fileinto', 'imap4flags', 'include', 'mailbox', 'mailboxid', + 'mime', 'regex', 'reject', 'relational', 'spamtest', 'spamtestplus', 'subaddress', + 'vacation', 'variables', +]; + +export const WEBMAIL_FIXTURES: WebmailFixture[] = [ + { file: 'vacation-include.sieve' }, + { file: 'folder-targets.sieve', extensions: ['fileinto', 'copy', 'imap4flags', 'mailbox', 'mailboxid'] }, + { + file: 'spam-guard.sieve', + extensions: ['fileinto', 'copy', 'relational', 'spamtest', 'spamtestplus', 'comparator-i;ascii-numeric'], + }, + // Everything at once, plus a hand-written rule the webmail kept. + { file: 'stalwart-full.sieve', extensions: STALWART_EXTENSIONS }, +]; + +export function readWebmailFixture(file: string): string { + // Git may check the fixture out with CRLF line ends on Windows. + return readFileSync(join(__dirname, file), 'utf-8').replace(/\r\n/g, '\n'); +} + +/** + * What the webmail itself writes when it saves the fixture again. That is the + * fixture unchanged, except that both clients add two blank lines in front of + * a script's external rules on every save (`.resaved.sieve`). + */ +export function readWebmailResave(file: string): string { + const resaved = file.replace(/\.sieve$/, '.resaved.sieve'); + return readWebmailFixture(existsSync(join(__dirname, resaved)) ? resaved : file); +} diff --git a/src/lib/sieve/__tests__/fixtures/webmail/spam-guard.sieve b/src/lib/sieve/__tests__/fixtures/webmail/spam-guard.sieve new file mode 100644 index 00000000..8fc8d20d --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/spam-guard.sieve @@ -0,0 +1,30 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"one","name":"Single condition","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"move","value":"Newsletters"}],"stopProcessing":false},{"id":"all","name":"All of two","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"a@example.com"},{"field":"subject","comparator":"contains","value":"report"}],"actions":[{"type":"move","value":"Newsletters"}],"stopProcessing":false},{"id":"any","name":"Any of two","enabled":true,"matchType":"any","conditions":[{"field":"from","comparator":"contains","value":"a@example.com"},{"field":"subject","comparator":"contains","value":"report"}],"actions":[{"type":"copy","value":"Reports"}],"stopProcessing":false},{"id":"optin","name":"Spam too","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"move","value":"Newsletters"}],"stopProcessing":false,"includeSpam":true},{"id":"keep","name":"Allow list","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"keep"}],"stopProcessing":false},{"id":"off","name":"Disabled","enabled":false,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"move","value":"Newsletters"}],"stopProcessing":false}]} +@metadata:end */ + +require ["comparator-i;ascii-numeric", "copy", "fileinto", "relational", "spamtestplus"]; + +# Rule: Single condition +if allof(header :contains "From" "news@example.com", not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") { + fileinto "Newsletters"; +} + +# Rule: All of two +if allof(header :contains "From" "a@example.com", header :contains "Subject" "report", not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") { + fileinto "Newsletters"; +} + +# Rule: Any of two +if allof(anyof(header :contains "From" "a@example.com", header :contains "Subject" "report"), not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") { + fileinto :copy "Reports"; +} + +# Rule: Spam too +if header :contains "From" "news@example.com" { + fileinto "Newsletters"; +} + +# Rule: Allow list +if header :contains "From" "news@example.com" { + fileinto "INBOX"; +} diff --git a/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve b/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve new file mode 100644 index 00000000..79f7e5a5 --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve @@ -0,0 +1,47 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"invoices","name":"Invoices","enabled":true,"matchType":"all","conditions":[{"field":"subject","comparator":"contains","value":"invoice"},{"field":"attachment","comparator":"has_type","value":["pdf","xml"]}],"actions":[{"type":"move","value":"Finance","mailboxId":"m3"},{"type":"mark_read"}],"stopProcessing":true},{"id":"vip","name":"VIP","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"is","value":"ceo@example.com"}],"actions":[{"type":"keep"},{"type":"star"}],"stopProcessing":false},{"id":"fwd","name":"Forward copy","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"forward","value":"assistant@example.com","keepCopy":true}],"stopProcessing":false},{"id":"spam","name":"Spam too","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"copy","value":"Everything","mailboxId":"m9"}],"stopProcessing":false,"includeSpam":true},{"id":"big","name":"Big mail","enabled":true,"matchType":"all","conditions":[{"field":"size","comparator":"greater_than","value":"10000000"}],"actions":[{"type":"reject","value":"Too \"big\""}],"stopProcessing":false}],"includeVacation":true} +@metadata:end */ + +require ["comparator-i;ascii-numeric", "copy", "fileinto", "imap4flags", "include", "mailbox", "mailboxid", "mime", "reject", "relational", "spamtestplus"]; + +# Vacation auto-reply +include :personal :optional "vacation"; + +# Rule: Invoices +if allof(header :contains "Subject" "invoice", header :mime :anychild :matches ["Content-Disposition", "Content-Type"] ["*.pdf*", "*.xml*"], not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") { + addflag "\\Seen"; + fileinto :mailboxid "m3" "Finance"; + stop; +} + +# Rule: VIP +if header :is "From" "ceo@example.com" { + addflag "\\Flagged"; + fileinto "INBOX"; +} + +# Rule: Forward copy +if header :contains "From" "news@example.com" { + redirect :copy "assistant@example.com"; +} + +# Rule: Spam too +if header :contains "From" "news@example.com" { + fileinto :copy :mailboxid "m9" "Everything"; +} + +# Rule: Big mail +if size :over 10000000 { + reject "Too \"big\""; +} + +# --- External rules (managed outside Bulwark) --- + + + + +# Hand-written rule +if header :contains "X-Spam-Flag" "YES" { + fileinto "Junk"; + stop; +} diff --git a/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.sieve b/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.sieve new file mode 100644 index 00000000..885845d9 --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.sieve @@ -0,0 +1,45 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"invoices","name":"Invoices","enabled":true,"matchType":"all","conditions":[{"field":"subject","comparator":"contains","value":"invoice"},{"field":"attachment","comparator":"has_type","value":["pdf","xml"]}],"actions":[{"type":"move","value":"Finance","mailboxId":"m3"},{"type":"mark_read"}],"stopProcessing":true},{"id":"vip","name":"VIP","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"is","value":"ceo@example.com"}],"actions":[{"type":"keep"},{"type":"star"}],"stopProcessing":false},{"id":"fwd","name":"Forward copy","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"forward","value":"assistant@example.com","keepCopy":true}],"stopProcessing":false},{"id":"spam","name":"Spam too","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@example.com"}],"actions":[{"type":"copy","value":"Everything","mailboxId":"m9"}],"stopProcessing":false,"includeSpam":true},{"id":"big","name":"Big mail","enabled":true,"matchType":"all","conditions":[{"field":"size","comparator":"greater_than","value":"10000000"}],"actions":[{"type":"reject","value":"Too \"big\""}],"stopProcessing":false}],"includeVacation":true} +@metadata:end */ + +require ["comparator-i;ascii-numeric", "copy", "fileinto", "imap4flags", "include", "mailbox", "mailboxid", "mime", "reject", "relational", "spamtestplus"]; + +# Vacation auto-reply +include :personal :optional "vacation"; + +# Rule: Invoices +if allof(header :contains "Subject" "invoice", header :mime :anychild :matches ["Content-Disposition", "Content-Type"] ["*.pdf*", "*.xml*"], not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") { + addflag "\\Seen"; + fileinto :mailboxid "m3" "Finance"; + stop; +} + +# Rule: VIP +if header :is "From" "ceo@example.com" { + addflag "\\Flagged"; + fileinto "INBOX"; +} + +# Rule: Forward copy +if header :contains "From" "news@example.com" { + redirect :copy "assistant@example.com"; +} + +# Rule: Spam too +if header :contains "From" "news@example.com" { + fileinto :copy :mailboxid "m9" "Everything"; +} + +# Rule: Big mail +if size :over 10000000 { + reject "Too \"big\""; +} + +# --- External rules (managed outside Bulwark) --- + + +# Hand-written rule +if header :contains "X-Spam-Flag" "YES" { + fileinto "Junk"; + stop; +} diff --git a/src/lib/sieve/__tests__/generator.test.ts b/src/lib/sieve/__tests__/generator.test.ts index 191318e3..da69f7f3 100644 --- a/src/lib/sieve/__tests__/generator.test.ts +++ b/src/lib/sieve/__tests__/generator.test.ts @@ -306,6 +306,40 @@ describe('generateScript', () => { }); }); + describe('spam guard', () => { + const exts = ['fileinto', 'relational', 'spamtest', 'spamtestplus']; + const guard = 'not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50"'; + + it('keeps spam out of folder moves', () => { + const script = generateScript([makeRule()], undefined, { extensions: exts }); + expect(script).toContain(`if allof(header :contains "From" "test@example.com", ${guard}) {`); + expect(script).toContain('require ["comparator-i;ascii-numeric", "fileinto", "relational", "spamtestplus"];'); + }); + + it('extends an allof and wraps an anyof', () => { + const conditions = [ + { field: 'from' as const, comparator: 'contains' as const, value: 'a' }, + { field: 'subject' as const, comparator: 'contains' as const, value: 'b' }, + ]; + const all = generateScript([makeRule({ conditions })], undefined, { extensions: exts }); + expect(all).toContain(`if allof(header :contains "From" "a", header :contains "Subject" "b", ${guard}) {`); + const any = generateScript([makeRule({ conditions, matchType: 'any' })], undefined, { extensions: exts }); + expect(any).toContain(`if allof(anyof(header :contains "From" "a", header :contains "Subject" "b"), ${guard}) {`); + }); + + it('leaves keep rules, opted-in rules and servers without spamtestplus alone', () => { + const keep = makeRule({ actions: [{ type: 'keep' }] }); + const optIn = makeRule({ id: 'r2', name: 'Opt in', includeSpam: true }); + expect(generateScript([keep, optIn], undefined, { extensions: exts })).not.toContain('spamtest'); + expect(generateScript([makeRule()], undefined, { extensions: ['fileinto'] })).not.toContain('spamtest'); + }); + + it('round-trips the opt-in', () => { + const parsed = parseScript(generateScript([makeRule({ includeSpam: true })], undefined, { extensions: exts })); + expect(parsed.rules[0].includeSpam).toBe(true); + }); + }); + describe('vacation include', () => { it('includes the server vacation script before the rules', () => { const script = generateScript([makeRule()], undefined, { includeVacation: true }); diff --git a/src/lib/sieve/__tests__/webmail-fixtures.test.ts b/src/lib/sieve/__tests__/webmail-fixtures.test.ts index 730506d2..dae6b9b3 100644 --- a/src/lib/sieve/__tests__/webmail-fixtures.test.ts +++ b/src/lib/sieve/__tests__/webmail-fixtures.test.ts @@ -1,44 +1,34 @@ // Scripts written by the webmail must survive a save on the phone: parsing // them into rules and generating the script again has to give back the same -// rules and the same Sieve, or saving filters on the phone would silently -// change what the server does (switch the auto-reply off, drop folder ids, -// let spam into folders...). -// -// The fixtures in fixtures/webmail/ were written by the webmail's own -// generator (jmap-webmail lib/sieve/generator.ts at e33ab899), run on the -// rules in each file's metadata for a server listing `extensions`. +// rules and the same Sieve the webmail would write, or saving filters on the +// phone would silently change what the server does (switch the auto-reply +// off, drop folder ids, let spam into folders...). See +// fixtures/webmail/index.ts for where the scripts come from. import { describe, it, expect } from 'vitest'; -import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; import { generateScript } from '../generator'; import { parseScript } from '../parser'; - -interface WebmailFixture { - file: string; - /** The server's sieveExtensions the webmail generated the script for. */ - extensions?: string[]; -} - -const FIXTURES: WebmailFixture[] = [ - { file: 'vacation-include.sieve' }, - { file: 'folder-targets.sieve', extensions: ['fileinto', 'copy', 'imap4flags', 'mailbox', 'mailboxid'] }, -]; - -function readFixture(file: string): string { - // Git may check the fixture out with CRLF line ends on Windows. - return readFileSync(join(__dirname, 'fixtures', 'webmail', file), 'utf-8').replace(/\r\n/g, '\n'); -} +import { readWebmailFixture, readWebmailResave, WEBMAIL_FIXTURES } from './fixtures/webmail'; function metadataRules(script: string): unknown[] { const match = script.match(/@metadata:begin\n(.*)\n@metadata:end/); return JSON.parse(match![1]).rules; } +// Blank lines carry no meaning in Sieve. +function withoutBlankLines(script: string): string { + return script.split('\n').filter((line) => line.trim() !== '').join('\n'); +} + describe('scripts written by the webmail', () => { - for (const { file, extensions } of FIXTURES) { + for (const { file, extensions } of WEBMAIL_FIXTURES) { describe(file, () => { - const script = readFixture(file); + const script = readWebmailFixture(file); const parsed = parseScript(script); + const regenerated = generateScript(parsed.rules, parsed.vacation, { + externalRequires: parsed.externalRequires, + includeVacation: parsed.includeVacation, + extensions, + }); it('parses into the rules the webmail stored', () => { expect(parsed.isOpaque).toBe(false); @@ -46,13 +36,14 @@ describe('scripts written by the webmail', () => { expect(bulwark).toEqual(metadataRules(script)); }); - it('saves back unchanged', () => { - const regenerated = generateScript(parsed.rules, parsed.vacation, { - externalRequires: parsed.externalRequires, - includeVacation: parsed.includeVacation, - extensions, - }); - expect(regenerated).toBe(script); + it('saves back what the webmail would save', () => { + expect(regenerated).toBe(readWebmailResave(file)); + }); + + it('saves back the same script', () => { + expect(withoutBlankLines(regenerated)).toBe(withoutBlankLines(script)); + expect(parseScript(regenerated).rules).toEqual(parsed.rules.map((r) => + r.rawBlock ? { ...r, rawBlock: expect.any(String) } : r)); }); }); } diff --git a/src/lib/sieve/generator.ts b/src/lib/sieve/generator.ts index 17625951..0d97c19a 100644 --- a/src/lib/sieve/generator.ts +++ b/src/lib/sieve/generator.ts @@ -148,7 +148,8 @@ function generateActions(actions: FilterAction[], useMailboxId: boolean): string // A bare `keep;` delivers to the "default place", which on Stalwart is // Junk for a message its spam filter has already classified, so an // allow-list rule made with "Keep" would change nothing. An explicit - // `fileinto "INBOX"` is honored over the spam verdict (#1027). + // `fileinto "INBOX"` is honored over the spam verdict (#1027), but + // only from Stalwart 0.16.22; older versions still move it to Junk. return 'fileinto "INBOX";'; case 'stop': return 'stop;'; @@ -156,7 +157,32 @@ function generateActions(actions: FilterAction[], useMailboxId: boolean): string }); } -function computeRequires(rules: FilterRule[], vacation: VacationSieveConfig | undefined, useMailboxId: boolean): string[] { +// Stalwart maps its spam verdict to 100 % (0.16.0) or to a graded value where +// 50 % is the spam threshold (0.16.19+), so ">= 50" matches "is spam" on both. +const SPAM_GUARD = 'not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50"'; +const SPAM_GUARD_REQUIRES = ['spamtestplus', 'relational', 'comparator-i;ascii-numeric']; + +function supportsSpamGuard(extensions: string[] | undefined): boolean { + return ['spamtestplus', 'relational'].every(e => extensions?.includes(e)); +} + +/** + * A folder move takes a message out of the Junk folder the server would have + * put it in, so move/copy rules skip spam unless the rule opts in. "Keep" + * rules are allow-lists and stay unguarded. + */ +function needsSpamGuard(rule: FilterRule, extensions: string[] | undefined): boolean { + return !rule.includeSpam && + rule.actions.some(a => a.type === 'move' || a.type === 'copy') && + supportsSpamGuard(extensions); +} + +function computeRequires( + rules: FilterRule[], + vacation: VacationSieveConfig | undefined, + useMailboxId: boolean, + serverExtensions: string[] | undefined, +): string[] { const extensions = new Set(); const enabledRules = rules.filter(r => r.enabled); @@ -165,6 +191,9 @@ function computeRequires(rules: FilterRule[], vacation: VacationSieveConfig | un } for (const rule of enabledRules) { + if (needsSpamGuard(rule, serverExtensions)) { + for (const e of SPAM_GUARD_REQUIRES) extensions.add(e); + } for (const condition of rule.conditions) { if (condition.field === 'body') extensions.add('body'); if (condition.field === 'attachment') extensions.add('mime'); @@ -211,6 +240,7 @@ function stripRuleForMetadata(r: FilterRule): Omit 1 && rule.matchType === 'all' + ? `allof(${conditions.join(', ')}, ${SPAM_GUARD})` + : `allof(${conditionStr}, ${SPAM_GUARD})`; + } + const actionLines = generateActions(rule.actions, useMailboxId); if (rule.stopProcessing) { diff --git a/src/lib/sieve/types.ts b/src/lib/sieve/types.ts index 1beaa788..a4a82133 100644 --- a/src/lib/sieve/types.ts +++ b/src/lib/sieve/types.ts @@ -79,6 +79,11 @@ export interface FilterRule { conditions: FilterCondition[]; actions: FilterAction[]; stopProcessing: boolean; + /** + * Also move/copy messages the server marked as spam. Off by default, so a + * folder rule does not pull spam out of Junk. + */ + includeSpam?: boolean; origin?: FilterOrigin; originLabel?: string; rawBlock?: string; diff --git a/src/stores/__tests__/filter-store.test.ts b/src/stores/__tests__/filter-store.test.ts index 98fff1c0..01797b1a 100644 --- a/src/stores/__tests__/filter-store.test.ts +++ b/src/stores/__tests__/filter-store.test.ts @@ -11,11 +11,14 @@ vi.mock('../../api/sieve', () => ({ validateSieveScript: vi.fn(async () => ({ isValid: true })), })); -import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; import * as sieve from '../../api/sieve'; import { generateScript } from '../../lib/sieve/generator'; import type { FilterRule } from '../../lib/sieve/types'; +import { + readWebmailFixture as webmailFixture, + readWebmailResave as webmailResave, + STALWART_EXTENSIONS, +} from '../../lib/sieve/__tests__/fixtures/webmail'; import { isVacationIncludedInFilters, syncVacationWithFilters, useFilterStore } from '../filter-store'; const api = vi.mocked(sieve); @@ -38,11 +41,6 @@ function serveScript(rules: FilterRule[]) { api.getSieveScriptContent.mockResolvedValue(generateScript(rules)); } -function webmailFixture(file: string): string { - return readFileSync(join(__dirname, '../../lib/sieve/__tests__/fixtures/webmail', file), 'utf-8') - .replace(/\r\n/g, '\n'); -} - const WITH_INCLUDE = { implementation: 'test', maxSizeScript: 100000, sieveExtensions: ['fileinto', 'include'], notificationMethods: [], externalLists: [], @@ -229,4 +227,35 @@ describe('saving scripts the webmail wrote', () => { await useFilterStore.getState().saveFilters(); expect(api.updateSieveScript).toHaveBeenCalledWith('s1', script, true, 'own'); }); + + it('keeps the spam guard, its opt-in, the vacation include and external rules', async () => { + const script = webmailFixture('stalwart-full.sieve'); + api.getSieveCapabilities.mockReturnValue({ ...WITH_INCLUDE, sieveExtensions: STALWART_EXTENSIONS }); + api.getSieveScripts.mockResolvedValue([ + { id: 's1', name: 'filters', blobId: 'b1', isActive: true }, + { id: 'v1', name: 'vacation', blobId: 'bv', isActive: false }, + ]); + api.getSieveScriptContent.mockResolvedValue(script); + + await useFilterStore.getState().selectAccount(null); + expect(useFilterStore.getState().rules.find((r) => r.id === 'spam')?.includeSpam).toBe(true); + + await useFilterStore.getState().saveFilters(); + expect(api.updateSieveScript).toHaveBeenCalledWith('s1', webmailResave('stalwart-full.sieve'), true, 'own'); + }); + + it('keeps the rule data through an edit on the phone', async () => { + const script = webmailFixture('stalwart-full.sieve'); + api.getSieveCapabilities.mockReturnValue({ ...WITH_INCLUDE, sieveExtensions: STALWART_EXTENSIONS }); + api.getSieveScripts.mockResolvedValue([{ id: 's1', name: 'filters', blobId: 'b1', isActive: true }]); + api.getSieveScriptContent.mockResolvedValue(script); + + await useFilterStore.getState().selectAccount(null); + useFilterStore.getState().toggleRule('big'); + useFilterStore.getState().toggleRule('big'); + useFilterStore.getState().reorderRules(['vip', 'invoices', 'fwd', 'spam', 'big']); + useFilterStore.getState().reorderRules(['invoices', 'vip', 'fwd', 'spam', 'big']); + await useFilterStore.getState().saveFilters(); + expect(api.updateSieveScript.mock.calls[0][1]).toBe(webmailResave('stalwart-full.sieve')); + }); });