From 2167a9d700a281fa4bc6850dc12dd9d453302dc1 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:57:00 +0100 Subject: [PATCH 1/9] fix: keep webmail filter periods, forwarding and exact address rules through a phone save #1152 --- .../webmail/address-comparators.sieve | 32 +++ .../fixtures/webmail/forward-off.sieve | 13 ++ .../sieve/__tests__/fixtures/webmail/index.ts | 20 +- .../webmail/period-forward-audience.sieve | 27 +++ .../webmail/stalwart-full.resaved.sieve | 4 - src/lib/sieve/__tests__/generator.test.ts | 86 +++++++- src/lib/sieve/__tests__/parser.test.ts | 46 +++- .../sieve/__tests__/webmail-fixtures.test.ts | 17 +- src/lib/sieve/generator.ts | 167 +++++++++++++-- src/lib/sieve/parser.ts | 100 +++++++-- src/lib/sieve/period.ts | 87 ++++++++ src/lib/sieve/types.ts | 66 +++++- src/lib/sieve/vacation-audience.ts | 30 +++ src/lib/sieve/vacation-forward.ts | 55 +++++ src/stores/__tests__/filter-store.test.ts | 92 +++++++- src/stores/filter-store.ts | 199 ++++++++++++++++-- 16 files changed, 968 insertions(+), 73 deletions(-) create mode 100644 src/lib/sieve/__tests__/fixtures/webmail/address-comparators.sieve create mode 100644 src/lib/sieve/__tests__/fixtures/webmail/forward-off.sieve create mode 100644 src/lib/sieve/__tests__/fixtures/webmail/period-forward-audience.sieve create mode 100644 src/lib/sieve/period.ts create mode 100644 src/lib/sieve/vacation-audience.ts create mode 100644 src/lib/sieve/vacation-forward.ts diff --git a/src/lib/sieve/__tests__/fixtures/webmail/address-comparators.sieve b/src/lib/sieve/__tests__/fixtures/webmail/address-comparators.sieve new file mode 100644 index 00000000..6d713078 --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/address-comparators.sieve @@ -0,0 +1,32 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"anna","name":"Anna exactly","enabled":true,"matchType":"any","conditions":[{"field":"from","comparator":"address_is","value":"anna@acme.com"},{"field":"cc","comparator":"address_is","value":["anna@acme.com","a.smith@acme.com"]}],"actions":[{"type":"star"}],"stopProcessing":false},{"id":"acme","name":"Acme domain","enabled":true,"matchType":"all","conditions":[{"field":"to","comparator":"domain_is","value":"acme.com"}],"actions":[{"type":"add_label","value":"acme"}],"stopProcessing":false},{"id":"drop","name":"Drop spam sender","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"domain_is","value":"spam.example"}],"actions":[{"type":"discard"}],"stopProcessing":true},{"id":"bounce","name":"Bounce","enabled":true,"matchType":"all","conditions":[{"field":"subject","comparator":"is","value":"win money"}],"actions":[{"type":"reject","value":"No thanks"}],"stopProcessing":true},{"id":"every","name":"Everything else","enabled":true,"matchType":"all","conditions":[{"field":"all","comparator":"any","value":""}],"actions":[{"type":"add_label","value":"seen-by-filter"}],"stopProcessing":false}]} +@metadata:end */ + +require ["imap4flags", "reject"]; + +# Rule: Anna exactly +if anyof(address :is "From" "anna@acme.com", address :is "Cc" ["anna@acme.com", "a.smith@acme.com"]) { + addflag "\\Flagged"; +} + +# Rule: Acme domain +if address :domain :is "To" "acme.com" { + addflag "$label:acme"; +} + +# Rule: Drop spam sender +if address :domain :is "From" "spam.example" { + discard; + stop; +} + +# Rule: Bounce +if header :is "Subject" "win money" { + reject "No thanks"; + stop; +} + +# Rule: Everything else +if true { + addflag "$label:seen-by-filter"; +} diff --git a/src/lib/sieve/__tests__/fixtures/webmail/forward-off.sieve b/src/lib/sieve/__tests__/fixtures/webmail/forward-off.sieve new file mode 100644 index 00000000..c620abbb --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/forward-off.sieve @@ -0,0 +1,13 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"news","name":"News","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"contains","value":"news@"}],"actions":[{"type":"mark_read"}],"stopProcessing":false}],"includeVacation":true,"vacationForward":{"enabled":false,"to":"backup@example.org","keepCopy":true}} +@metadata:end */ + +require ["imap4flags", "include"]; + +# Vacation auto-reply +include :personal :optional "vacation"; + +# Rule: News +if header :contains "From" "news@" { + addflag "\\Seen"; +} diff --git a/src/lib/sieve/__tests__/fixtures/webmail/index.ts b/src/lib/sieve/__tests__/fixtures/webmail/index.ts index 5eb1583e..fb4c850f 100644 --- a/src/lib/sieve/__tests__/fixtures/webmail/index.ts +++ b/src/lib/sieve/__tests__/fixtures/webmail/index.ts @@ -1,7 +1,8 @@ // 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`. +// lib/sieve/generator.ts at e33ab899; the version 2, address comparator and +// forwarding ones at 4aea0976), 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'; @@ -28,6 +29,14 @@ export const WEBMAIL_FIXTURES: WebmailFixture[] = [ }, // Everything at once, plus a hand-written rule the webmail kept. { file: 'stalwart-full.sieve', extensions: STALWART_EXTENSIONS }, + // Version 2 (webmail 1.13+): rule periods, forwarding from the vacation + // card in the vacation's period, the auto-reply for colleagues only. + { file: 'period-forward-audience.sieve', extensions: STALWART_EXTENSIONS }, + // Version 1 with the vacation forwarding switched off but remembered. + { file: 'forward-off.sieve', extensions: STALWART_EXTENSIONS }, + // "is the address" / "has the domain", "all messages", stop after discard + // and reject, names with runs of spaces. + { file: 'address-comparators.sieve', extensions: STALWART_EXTENSIONS }, ]; export function readWebmailFixture(file: string): string { @@ -37,8 +46,9 @@ export function readWebmailFixture(file: string): string { /** * 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`). + * fixture unchanged, except that a script written before 4aea0976 had extra + * blank lines in front of its external rules, which a save now trims + * (`.resaved.sieve`). */ export function readWebmailResave(file: string): string { const resaved = file.replace(/\.sieve$/, '.resaved.sieve'); diff --git a/src/lib/sieve/__tests__/fixtures/webmail/period-forward-audience.sieve b/src/lib/sieve/__tests__/fixtures/webmail/period-forward-audience.sieve new file mode 100644 index 00000000..1b45e629 --- /dev/null +++ b/src/lib/sieve/__tests__/fixtures/webmail/period-forward-audience.sieve @@ -0,0 +1,27 @@ +/* @metadata:begin +{"version":2,"rules":[{"id":"trip","name":"While travelling","enabled":true,"matchType":"all","conditions":[{"field":"from","comparator":"domain_is","value":"client.example"}],"actions":[{"type":"forward","value":"deputy@example.org","keepCopy":true}],"stopProcessing":false,"activeFrom":"2026-11-02T07:00:00.000Z","activeUntil":"2026-11-13T17:30:00.000Z"},{"id":"later","name":"From December","enabled":true,"matchType":"all","conditions":[{"field":"subject","comparator":"contains","value":"report"}],"actions":[{"type":"move","value":"Reports","mailboxId":"m7"}],"stopProcessing":true,"activeFrom":"2026-12-01T00:00:00.000Z"}],"includeVacation":true,"vacationForward":{"enabled":true,"to":"backup@example.org","keepCopy":false,"activeFrom":"2026-11-02T00:00:00.000Z","activeUntil":"2026-11-14T00:00:00.000Z"},"vacationAudience":{"only":"internal","domains":["example.org","example.com"]}} +@metadata:end */ + +require ["comparator-i;ascii-numeric", "copy", "date", "envelope", "fileinto", "include", "mailbox", "mailboxid", "relational", "spamtestplus"]; + +# Vacation auto-reply +if envelope :domain :is "from" ["example.org", "example.com"] { + include :personal :optional "vacation"; +} + +# Vacation forwarding +if allof(anyof(currentdate :zone "+0000" :value "gt" "date" "2026-11-02", allof(currentdate :zone "+0000" :is "date" "2026-11-02", currentdate :zone "+0000" :value "ge" "time" "00:00:00")), anyof(currentdate :zone "+0000" :value "lt" "date" "2026-11-14", allof(currentdate :zone "+0000" :is "date" "2026-11-14", currentdate :zone "+0000" :value "le" "time" "00:00:00")), not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50") { + redirect "backup@example.org"; + stop; +} + +# Rule: While travelling +if allof(anyof(currentdate :zone "+0000" :value "gt" "date" "2026-11-02", allof(currentdate :zone "+0000" :is "date" "2026-11-02", currentdate :zone "+0000" :value "ge" "time" "07:00:00")), anyof(currentdate :zone "+0000" :value "lt" "date" "2026-11-13", allof(currentdate :zone "+0000" :is "date" "2026-11-13", currentdate :zone "+0000" :value "le" "time" "17:30:00")), address :domain :is "From" "client.example") { + redirect :copy "deputy@example.org"; +} + +# Rule: From December +if allof(anyof(currentdate :zone "+0000" :value "gt" "date" "2026-12-01", allof(currentdate :zone "+0000" :is "date" "2026-12-01", currentdate :zone "+0000" :value "ge" "time" "00:00:00")), allof(header :contains "Subject" "report", not spamtest :percent :value "ge" :comparator "i;ascii-numeric" "50")) { + fileinto :mailboxid "m7" "Reports"; + stop; +} diff --git a/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve b/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve index 79f7e5a5..1e169194 100644 --- a/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve +++ b/src/lib/sieve/__tests__/fixtures/webmail/stalwart-full.resaved.sieve @@ -36,10 +36,6 @@ if size :over 10000000 { } # --- External rules (managed outside Bulwark) --- - - - - # Hand-written rule if header :contains "X-Spam-Flag" "YES" { fileinto "Junk"; diff --git a/src/lib/sieve/__tests__/generator.test.ts b/src/lib/sieve/__tests__/generator.test.ts index 737e2218..0cbb833e 100644 --- a/src/lib/sieve/__tests__/generator.test.ts +++ b/src/lib/sieve/__tests__/generator.test.ts @@ -241,25 +241,99 @@ describe('generateScript', () => { expect(matches).toHaveLength(1); }); - it('does not append stop after discard', () => { + // discard and reject only cancel the implicit keep; the rules below + // would still file, flag or forward the message (#1153). + it('appends stop after discard', () => { const script = generateScript([makeRule({ actions: [{ type: 'discard' }], stopProcessing: true, })]); - const matches = script.match(/stop;/g); - expect(matches).toBeNull(); + expect(script).toContain(' discard;\n stop;\n}'); }); - it('does not append stop after reject', () => { + it('appends stop after reject', () => { const script = generateScript([makeRule({ actions: [{ type: 'reject', value: 'No' }], stopProcessing: true, })]); - const matches = script.match(/stop;/g); - expect(matches).toBeNull(); + expect(script).toContain(' reject "No";\n stop;\n}'); }); }); + describe('address comparators', () => { + it('writes "is the address" as an address test', () => { + const script = generateScript([makeRule({ + conditions: [{ field: 'from', comparator: 'address_is', value: 'anna@acme.com' }], + })]); + expect(script).toContain('if address :is "From" "anna@acme.com" {'); + expect(script).not.toContain('header :contains'); + }); + + it('writes "has the domain" as an address :domain test', () => { + const script = generateScript([makeRule({ + conditions: [{ field: 'cc', comparator: 'domain_is', value: ['acme.com', 'acme.org'] }], + })]); + expect(script).toContain('if address :domain :is "Cc" ["acme.com", "acme.org"] {'); + }); + + it('reads both back as the same comparators', () => { + const rules = [ + makeRule({ id: 'a', name: 'A', conditions: [{ field: 'to', comparator: 'address_is', value: 'a@b.c' }] }), + makeRule({ id: 'b', name: 'B', conditions: [{ field: 'from', comparator: 'domain_is', value: 'b.c' }] }), + ]; + expect(parseScript(generateScript(rules)).rules).toEqual(rules); + }); + + it('refuses a comparator it does not know instead of broadening it', () => { + const rule = makeRule({ + conditions: [{ field: 'from', comparator: 'sounds_like' as never, value: 'anna' }], + }); + expect(() => generateScript([rule])).toThrow(/sounds_like/); + }); + + it('refuses an address comparator on the subject', () => { + const rule = makeRule({ + conditions: [{ field: 'subject', comparator: 'address_is', value: 'x' }], + }); + expect(() => generateScript([rule])).toThrow(); + }); + }); + + // #1154: a rule for every message is Sieve `true`. + it('writes the "all messages" condition as true', () => { + const script = generateScript([makeRule({ + conditions: [{ field: 'all', comparator: 'any', value: '' }], + actions: [{ type: 'add_label', value: 'x' }], + })]); + expect(script).toContain('if true {'); + }); + + it('confines a rule with a period to it', () => { + const script = generateScript([makeRule({ activeFrom: '2026-11-02T07:00:00.000Z' })]); + expect(script).toContain('{"version":2,'); + expect(script).toMatch(/require \[[^\]]*"date"[^\]]*"relational"/); + expect(script).toContain('if allof(anyof(currentdate :zone "+0000" :value "gt" "date" "2026-11-02"'); + }); + + it('runs the vacation forwarding ahead of the rules', () => { + const script = generateScript([makeRule()], undefined, { + includeVacation: true, + vacationForward: { enabled: true, to: 'backup@example.org', keepCopy: false }, + }); + const forward = script.indexOf('# Vacation forwarding\nif true {\n redirect "backup@example.org";\n stop;\n}'); + expect(forward).toBeGreaterThan(0); + expect(forward).toBeLessThan(script.indexOf('# Rule: Test Rule')); + }); + + it('includes the auto-reply for colleagues only', () => { + const script = generateScript([], undefined, { + includeVacation: true, + vacationAudience: { only: 'internal', domains: ['example.org'] }, + }); + expect(script).toContain('if envelope :domain :is "from" ["example.org"] {\n include :personal :optional "vacation";\n}'); + expect(script).toMatch(/require \[[^\]]*"envelope"/); + }); + describe('action order and targets', () => { it('sets flags before the move so the moved message keeps them', () => { const script = generateScript([makeRule({ diff --git a/src/lib/sieve/__tests__/parser.test.ts b/src/lib/sieve/__tests__/parser.test.ts index c4090e30..6f9ca616 100644 --- a/src/lib/sieve/__tests__/parser.test.ts +++ b/src/lib/sieve/__tests__/parser.test.ts @@ -42,13 +42,55 @@ describe('parseScript', () => { expect(result.rules).toEqual([]); }); - it('returns isOpaque for version mismatch', () => { - const script = '/* @metadata:begin\n{"version":2,"rules":[]}\n@metadata:end */'; + it('returns isOpaque for a metadata version it does not know', () => { + const script = '/* @metadata:begin\n{"version":3,"rules":[]}\n@metadata:end */'; const result = parseScript(script); expect(result.isOpaque).toBe(true); expect(result.rules).toEqual([]); }); + // Webmail 1.13+ writes version 2 once a rule has a period or the vacation + // card forwards or narrows its recipients (#1152). + it('reads version 2 metadata as its own', () => { + const script = '/* @metadata:begin\n{"version":2,"rules":[]}\n@metadata:end */'; + const result = parseScript(script); + expect(result.isOpaque).toBe(false); + expect(result.rules).toEqual([]); + }); + + it('keeps a rule period and the vacation forwarding and audience through a save', () => { + const rule = makeRule({ activeFrom: '2026-11-02T07:00:00.000Z', activeUntil: '2026-11-13T17:30:00.000Z' }); + const vacationForward = { enabled: true, to: 'backup@example.org', keepCopy: true, activeUntil: '2026-11-14T00:00:00.000Z' }; + const vacationAudience = { only: 'external' as const, domains: ['example.org'] }; + const script = generateScript([rule], undefined, { includeVacation: true, vacationForward, vacationAudience }); + expect(script).toContain('{"version":2,'); + const parsed = parseScript(script); + expect(parsed.isOpaque).toBe(false); + expect(parsed.rules[0]).toMatchObject({ activeFrom: rule.activeFrom, activeUntil: rule.activeUntil }); + expect(parsed.vacationForward).toEqual(vacationForward); + expect(parsed.vacationAudience).toEqual(vacationAudience); + expect(generateScript(parsed.rules, parsed.vacation, { + includeVacation: parsed.includeVacation, + vacationForward: parsed.vacationForward, + vacationAudience: parsed.vacationAudience, + })).toBe(script); + }); + + it('treats a script with an unreadable rule period as hand-edited', () => { + const script = '/* @metadata:begin\n' + JSON.stringify({ version: 2, rules: [makeRule({ activeFrom: 'next monday' })] }) + '\n@metadata:end */'; + expect(parseScript(script).isOpaque).toBe(true); + }); + + // #1153: the comment collapses the run of spaces; compared as written the + // rule came back as someone else's, one more copy on every save. + it('matches a rule whose name has a run of spaces to its comment', () => { + const script = generateScript([makeRule({ name: 'Foo Bar\tBaz' })]); + expect(script).toContain('# Rule: Foo Bar Baz'); + const parsed = parseScript(script); + expect(parsed.rules).toHaveLength(1); + expect(parsed.rules[0].origin).toBeUndefined(); + }); + it('returns isOpaque for empty metadata block', () => { const script = '/* @metadata:begin\n\n@metadata:end */'; const result = parseScript(script); diff --git a/src/lib/sieve/__tests__/webmail-fixtures.test.ts b/src/lib/sieve/__tests__/webmail-fixtures.test.ts index dae6b9b3..38c8a48d 100644 --- a/src/lib/sieve/__tests__/webmail-fixtures.test.ts +++ b/src/lib/sieve/__tests__/webmail-fixtures.test.ts @@ -9,9 +9,13 @@ import { generateScript } from '../generator'; import { parseScript } from '../parser'; import { readWebmailFixture, readWebmailResave, WEBMAIL_FIXTURES } from './fixtures/webmail'; -function metadataRules(script: string): unknown[] { +function metadata(script: string): Record { const match = script.match(/@metadata:begin\n(.*)\n@metadata:end/); - return JSON.parse(match![1]).rules; + return JSON.parse(match![1]); +} + +function metadataRules(script: string): unknown[] { + return metadata(script).rules as unknown[]; } // Blank lines carry no meaning in Sieve. @@ -27,6 +31,8 @@ describe('scripts written by the webmail', () => { const regenerated = generateScript(parsed.rules, parsed.vacation, { externalRequires: parsed.externalRequires, includeVacation: parsed.includeVacation, + vacationForward: parsed.vacationForward, + vacationAudience: parsed.vacationAudience, extensions, }); @@ -36,6 +42,13 @@ describe('scripts written by the webmail', () => { expect(bulwark).toEqual(metadataRules(script)); }); + it('keeps the vacation forwarding and audience the webmail stored', () => { + const stored = metadata(script); + expect(parsed.vacationForward).toEqual(stored.vacationForward); + expect(parsed.vacationAudience).toEqual(stored.vacationAudience); + expect(metadata(regenerated)).toEqual(stored); + }); + it('saves back what the webmail would save', () => { expect(regenerated).toBe(readWebmailResave(file)); }); diff --git a/src/lib/sieve/generator.ts b/src/lib/sieve/generator.ts index d1b4d158..c4b95dda 100644 --- a/src/lib/sieve/generator.ts +++ b/src/lib/sieve/generator.ts @@ -3,8 +3,13 @@ import type { FilterCondition, FilterAction, FilterMetadata, + VacationAudience, + VacationForward, VacationSieveConfig, } from './types'; +import { PERIOD_REQUIRES, hasPeriod, periodTests } from './period'; +import { VACATION_FORWARD_MARKER, isValidVacationForward, normalizeVacationForward } from './vacation-forward'; +import { isValidVacationAudience, normalizeVacationAudience } from './vacation-audience'; // Ported from the webmail's lib/sieve/generator.ts. Emits an RFC 5228 Sieve // script with a leading `@metadata` JSON comment that lets the parser round-trip @@ -18,6 +23,9 @@ const HEADER_MAP: Record = { subject: 'Subject', }; +/** Fields whose header holds addresses, so the `address` test applies. */ +const ADDRESS_FIELDS = new Set(['from', 'to', 'cc']); + function escapeString(value: string): string { return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); } @@ -34,10 +42,11 @@ export function isValidSieveHeaderName(name: string): boolean { /** * A rule's name as it appears in its `# Rule:` comment, which ends at the * line break: a CR or LF in the name would have turned the rest of it into - * script. The parser matches the comment back to the rule by this form. + * script. Every whitespace run becomes one space, as the webmail writes it; + * the parser matches the comment back to the rule by this form. */ export function ruleCommentName(name: string): string { - return name.replace(/[\r\n]+/g, ' ').trim(); + return name.replace(/\s+/g, ' '); } /** A `header` condition whose name the server would refuse. */ @@ -66,9 +75,15 @@ function formatStringArg(values: string[], transform: (s: string) => string = (s function generateCondition(condition: FilterCondition): string { const { field, comparator, value } = condition; + // Every message: there is nothing to compare. + if (field === 'all') return 'true'; + if (field === 'size') { - // Size is numeric, single value only. - const sizeValue = Array.isArray(value) ? value[0] : value; + // Size is numeric, single value only. It is written unquoted, so + // anything but a number (with an optional K/M/G quantifier) would be + // Sieve source; fall back to 0. + const raw = String((Array.isArray(value) ? value[0] : value) ?? '').trim(); + const sizeValue = /^\d+[KMG]?$/i.test(raw) ? raw : '0'; const op = comparator === 'greater_than' ? ':over' : ':under'; return `size ${op} ${sizeValue}`; } @@ -100,7 +115,7 @@ function generateCondition(condition: FilterCondition): string { return `header :mime :anychild :matches ["Content-Disposition", "Content-Type"] ${formatStringArg(normalised, (ext) => `*.${ext}*`)}`; } - const headerName = field === 'header' + const rawHeaderName = field === 'header' ? (condition.headerName?.trim() || 'X-Unknown') : HEADER_MAP[field]; @@ -108,9 +123,22 @@ function generateCondition(condition: FilterCondition): string { // must never be flattened into `header :contains "undefined"`: that would // silently rewrite the user's rule server-side on the next save. Fail the // save instead so the script stays untouched. - if (!headerName) { + if (!rawHeaderName) { throw new Error(`Unsupported filter condition field: ${String(field)}`); } + const headerName = escapeString(rawHeaderName); + + // RFC 5228 §5.1: `address` compares the parsed address, so the display + // name and the angle brackets play no part. `header :contains` would let + // "anna@acme.com" match joanna@acme.com, and `header :matches "*@acme.com"` + // never matches "Anna " because of the closing ">". + if (comparator === 'address_is' || comparator === 'domain_is') { + if (!ADDRESS_FIELDS.has(field)) { + throw new Error(`Unsupported filter comparator for ${String(field)}: ${comparator}`); + } + const part = comparator === 'domain_is' ? ':domain ' : ''; + return `address ${part}:is "${headerName}" ${formatStringArg(values)}`; + } switch (comparator) { case 'contains': @@ -128,7 +156,10 @@ function generateCondition(condition: FilterCondition): string { case 'matches': return `header :matches "${headerName}" ${formatStringArg(values)}`; default: - return `header :contains "${headerName}" ${formatStringArg(values)}`; + // As with an unknown field: a comparator from a newer client written + // as `header :contains` would quietly broaden the rule ("is the + // address" catching every address that merely contains it). + throw new Error(`Unsupported filter comparator: ${String(comparator)}`); } } @@ -176,6 +207,8 @@ function generateActions(actions: FilterAction[], useMailboxId: boolean): string return 'fileinto "INBOX";'; case 'stop': return 'stop;'; + default: + throw new Error(`Unsupported filter action: ${String((action as FilterAction).type)}`); } }); } @@ -185,7 +218,7 @@ function generateActions(actions: FilterAction[], useMailboxId: boolean): string 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 { +export function supportsSpamGuard(extensions: string[] | undefined): boolean { return ['spamtestplus', 'relational'].every(e => extensions?.includes(e)); } @@ -217,6 +250,9 @@ function computeRequires( if (needsSpamGuard(rule, serverExtensions)) { for (const e of SPAM_GUARD_REQUIRES) extensions.add(e); } + if (periodTests(rule)?.length) { + for (const e of PERIOD_REQUIRES) extensions.add(e); + } for (const condition of rule.conditions) { if (condition.field === 'body') extensions.add('body'); if (condition.field === 'attachment') extensions.add('mime'); @@ -264,6 +300,8 @@ function stripRuleForMetadata(r: FilterRule): Omit 0) bulwarkRequires.push(...PERIOD_REQUIRES); + if (forwardSpamGuard) bulwarkRequires.push(...SPAM_GUARD_REQUIRES); + } const externalRequires = options.externalRequires ?? []; const allRequires = [...new Set([...bulwarkRequires, ...externalRequires])].sort(); @@ -339,7 +422,20 @@ export function generateScript( // the parser does not re-import it as an external rule. lines.push(''); lines.push('# Vacation auto-reply'); - lines.push(`include :personal :optional "${VACATION_SCRIPT_NAME}";`); + const include = `include :personal :optional "${VACATION_SCRIPT_NAME}";`; + const audience = metadata.vacationAudience; + if (audience) { + // Only senders from (or not from) the account's own domains. The reply + // goes to the envelope sender, so that is the address to judge: a From + // header can name a colleague on mail from anywhere, and the reply + // meant for colleagues would go to whoever sent it. + const domains = audience.domains.map(d => `"${escapeString(d)}"`).join(', '); + lines.push(`if ${audience.only === 'external' ? 'not ' : ''}envelope :domain :is "from" [${domains}] {`); + lines.push(` ${include}`); + lines.push('}'); + } else { + lines.push(include); + } } if (vacation?.isEnabled) { @@ -353,6 +449,19 @@ export function generateScript( lines.push(`vacation ${vacationParts.join(' ')};`); } + if (forward && forwardPeriod) { + // Spam is not passed on: it stays here for the server's own handling. + const tests = [...forwardPeriod, ...(forwardSpamGuard ? [SPAM_GUARD] : [])]; + const condition = tests.length === 0 ? 'true' : tests.length === 1 ? tests[0] : `allof(${tests.join(', ')})`; + lines.push(''); + lines.push(VACATION_FORWARD_MARKER); + lines.push(`if ${condition} {`); + lines.push(` redirect ${forward.keepCopy ? ':copy ' : ''}"${escapeString(forward.to)}";`); + // Without a copy kept here, no rule below may file it either. + if (!forward.keepCopy) lines.push(' stop;'); + lines.push('}'); + } + const enabledBulwarkRules = bulwarkRules.filter(r => r.enabled); for (const rule of enabledBulwarkRules) { @@ -367,6 +476,13 @@ export function generateScript( console.warn(`[filters] Skipping rule "${rule.name}": a header name the server would refuse`); continue; } + // Without its period a rule would act all the time - a forwarding rule + // would forward for good - so one whose period is unusable is dropped. + const period = periodTests(rule); + if (period === null) { + console.warn(`[filters] Skipping rule "${rule.name}": unusable period`); + continue; + } lines.push(''); lines.push(`# Rule: ${ruleCommentName(rule.name)}`); @@ -389,13 +505,20 @@ export function generateScript( : `allof(${conditionStr}, ${SPAM_GUARD})`; } + // The period comes first: outside it, nothing else is evaluated. + if (period.length > 0) { + conditionStr = `allof(${[...period, conditionStr].join(', ')})`; + } + const actionLines = generateActions(rule.actions, useMailboxId); - if (rule.stopProcessing) { - const lastAction = rule.actions[rule.actions.length - 1]; - if (!lastAction || !['stop', 'discard', 'reject'].includes(lastAction.type)) { - actionLines.push('stop;'); - } + // discard and reject only cancel the implicit keep (RFC 5228 4.4, + // RFC 5429); the script goes on, and the rules below would still act on + // the message. So "stop processing" always writes a stop, unless the + // block has one already: it runs straight through, so any stop in it ends + // the script. + if (rule.stopProcessing && !actionLines.includes('stop;')) { + actionLines.push('stop;'); } lines.push(`if ${conditionStr} {`); @@ -405,15 +528,17 @@ export function generateScript( lines.push('}'); } - // Append preserved external rules verbatim. Each rawBlock already carries its - // own leading comments and trailing whitespace from the source script. - if (externalRules.length > 0) { + // Append preserved external rules verbatim. Each rawBlock carries its own + // leading comments, but also the blank lines around it in the source + // script; trim those and put exactly one blank line between blocks, so a + // save writes the same script as the one before it. + const externalBlocks = externalRules + .map((ext) => (ext.rawBlock ?? '').trim()) + .filter((block) => block.length > 0); + if (externalBlocks.length > 0) { lines.push(''); lines.push('# --- External rules (managed outside Bulwark) ---'); - for (const ext of externalRules) { - if (!ext.rawBlock) continue; - lines.push(ext.rawBlock.replace(/\s+$/, '')); - } + lines.push(externalBlocks.join('\n\n')); } lines.push(''); diff --git a/src/lib/sieve/parser.ts b/src/lib/sieve/parser.ts index 0439ad30..38d9172f 100644 --- a/src/lib/sieve/parser.ts +++ b/src/lib/sieve/parser.ts @@ -5,15 +5,19 @@ import type { FilterConditionField, FilterMetadata, FilterRule, + VacationAudience, + VacationForward, VacationSieveConfig, } from './types'; -import { ruleCommentName } from './generator'; - +import { isPeriodBoundary } from './period'; +import { VACATION_FORWARD_MARKER_RE, isValidVacationForward } from './vacation-forward'; +import { isValidVacationAudience } from './vacation-audience'; // Ported from the webmail's lib/sieve/parser.ts. Parses a Sieve script back // into structured rules. Bulwark-authored scripts carry an `@metadata` JSON // block we trust directly; foreign scripts (Roundcube, Nextcloud, hand-edited) // are best-effort parsed into read-only "external" rules or preserved opaquely. + export interface ParseResult { rules: FilterRule[]; isOpaque: boolean; @@ -21,6 +25,10 @@ export interface ParseResult { externalRequires: string[]; /** The script runs the server's "vacation" script via `include`. */ includeVacation?: boolean; + /** Forwarding from the vacation card, on or off. */ + vacationForward?: VacationForward; + /** Who gets the auto-reply, when not everyone. */ + vacationAudience?: VacationAudience; } const OPAQUE: ParseResult = { rules: [], isOpaque: true, externalRequires: [] }; @@ -69,6 +77,12 @@ function isValidRule(rule: unknown): rule is FilterRule { typeof r.stopProcessing !== 'boolean' ) return false; + // A period that cannot be read would be dropped on the next save and the + // rule would act all the time; treating the script as hand-edited leaves + // it untouched instead. + if (r.activeFrom !== undefined && !isPeriodBoundary(r.activeFrom)) return false; + if (r.activeUntil !== undefined && !isPeriodBoundary(r.activeUntil)) return false; + return r.conditions.every(isValidCondition) && r.actions.every(isValidAction); } @@ -134,6 +148,19 @@ function skipBlockComment(s: string, i: number): number { return end === -1 ? s.length : end + 2; } +// Past a `text:` string: every line up to and including the lone ".". +function skipMultiLineString(s: string, i: number): number { + let line = s.indexOf('\n', i) + 1; + if (line === 0) return s.length; + while (line < s.length) { + const nl = s.indexOf('\n', line); + const next = nl === -1 ? s.length : nl + 1; + if (s.slice(line, next).replace(/\r?\n$/, '') === '.') return next; + line = next; + } + return s.length; +} + function skipStatement(s: string, i: number): number { while (i < s.length) { const c = s[i]; @@ -458,6 +485,22 @@ function parseAtom(raw: string): FilterCondition | null { return null; } + // `address [:all|:domain] :is` on From/To/Cc, as the generator writes the + // address_is / domain_is comparators. Sieve takes tagged arguments in any + // order, so the address part may also follow `:is`. Anything else about an + // address test (another part, another match type, a negation) has no + // builder equivalent and is left for the opaque fallback. + m = /^address\s+(?:(:all|:domain)\s+)?:is\s+(?:(:all|:domain)\s+)?"((?:[^"\\]|\\.)*)"\s+([\s\S]+)$/.exec(s); + if (m) { + const [, partBefore, partAfter, headerName, rawTail] = m; + if (negated || (partBefore && partAfter)) return null; + const field = FIELD_FROM_HEADER[unescapeSieveString(headerName).toLowerCase()]; + const value = parseValueTail(rawTail); + if (!field || field === 'subject' || value === null) return null; + const part = partBefore ?? partAfter; + return { field, comparator: part === ':domain' ? 'domain_is' : 'address_is', value }; + } + m = /^header\s+:(contains|is|matches)\s+"((?:[^"\\]|\\.)*)"\s+([\s\S]+)$/.exec(s); if (m) { const [, tag, headerName, rawTail] = m; @@ -674,14 +717,25 @@ function extractNextcloudRegions(content: string): { requires: string[]; } { const marker = NEXTCLOUD_BLOCK_MARKER; + // Only a comment line counts. The same text inside a string (a rule's value, + // an auto-reply) is data, and taken for a region it would be written back + // as live Sieve. const positions: number[] = []; - let searchFrom = 0; - while (true) { - const idx = content.indexOf(marker, searchFrom); - if (idx === -1) break; - const atLineStart = idx === 0 || content[idx - 1] === '\n'; - if (atLineStart) positions.push(idx); - searchFrom = idx + marker.length; + let i = 0; + while (i < content.length) { + const c = content[i]; + if (c === '"') { i = skipStringLit(content, i); continue; } + if (c === '/' && content[i + 1] === '*') { i = skipBlockComment(content, i); continue; } + if (c === '#') { + if ((i === 0 || content[i - 1] === '\n') && content.startsWith(marker, i)) positions.push(i); + i = skipHashComment(content, i); + continue; + } + if (content.slice(i, i + 5).toLowerCase() === 'text:' && !/\w/.test(content[i - 1] ?? '')) { + i = skipMultiLineString(content, i); + continue; + } + i++; } if (positions.length < 2) { @@ -801,12 +855,21 @@ export function parseScript(content: string): ParseResult { return OPAQUE; } - if (!metadata || metadata.version !== 1) return OPAQUE; + // Version 2 only marks fields older builds do not know (see FilterMetadata). + if (!metadata || (metadata.version !== 1 && metadata.version !== 2)) return OPAQUE; if (!Array.isArray(metadata.rules)) return OPAQUE; for (const rule of metadata.rules) { if (!isValidRule(rule)) return OPAQUE; } + // Forwarding that cannot be read could be written back without its + // period; as with a rule, the script then counts as edited by hand. + if (metadata.vacationForward !== undefined && !isValidVacationForward(metadata.vacationForward)) { + return OPAQUE; + } + if (metadata.vacationAudience !== undefined && !isValidVacationAudience(metadata.vacationAudience)) { + return OPAQUE; + } // Scan the portion AFTER the metadata block for external rules. A prior // Bulwark save may have emitted its "External rules" header here; strip @@ -836,13 +899,20 @@ export function parseScript(content: string): ParseResult { // identifies it as ours. const filteredExternal = external.rules.filter(r => { const raw = r.rawBlock || ''; - const match = raw.match(/#\s*Rule:\s*(.+?)\s*$/m); + // The name ends with its line, and may be empty. + const match = raw.match(/#\s*Rule:[ \t]*(.*?)[ \t]*$/m); if (match) { - const name = match[1].trim(); - // The comment carries the name without line breaks (ruleCommentName). - if (bulwarkRules.some(b => ruleCommentName(b.name) === name)) return false; + // The generator writes the name on one line, its whitespace runs + // collapsed; compared as written, "Foo Bar" would come back as + // someone else's rule, and as one more copy on every save. + const oneLine = (name: string) => name.replace(/\s+/g, ' ').trim(); + const name = oneLine(match[1]); + if (bulwarkRules.some(b => oneLine(b.name) === name)) return false; } if (/#\s*Vacation auto-reply/i.test(raw)) return false; + // The generator writes the forwarding block from the metadata again. + // Only written while switched on, so only then is such a block Bulwark's. + if (metadata.vacationForward?.enabled && VACATION_FORWARD_MARKER_RE.test(raw)) return false; return true; }); @@ -860,6 +930,8 @@ export function parseScript(content: string): ParseResult { vacation: metadata.vacation, externalRequires, ...(metadata.includeVacation === true ? { includeVacation: true } : {}), + ...(metadata.vacationForward ? { vacationForward: metadata.vacationForward } : {}), + ...(metadata.vacationAudience ? { vacationAudience: metadata.vacationAudience } : {}), }; } diff --git a/src/lib/sieve/period.ts b/src/lib/sieve/period.ts new file mode 100644 index 00000000..ad802a04 --- /dev/null +++ b/src/lib/sieve/period.ts @@ -0,0 +1,87 @@ +import type { FilterRule } from './types'; + +/** + * Time periods for filter rules: "only act on mail that arrives from ... + * until ...". Sieve has no rule scheduling of its own; the date extension + * (RFC 5260) lets a rule test the moment the message is being filtered. + */ + +/** Sieve extensions a period needs: `currentdate` and `:value` comparisons. */ +export const PERIOD_REQUIRES = ['date', 'relational'] as const; + +export function supportsPeriods(extensions: readonly string[] | undefined | null): boolean { + return PERIOD_REQUIRES.every((e) => extensions?.includes(e)); +} + +// A moment with its zone spelled out. Without one, Date.parse reads the value +// as local time, which would make the script depend on the browser saving it. +const ISO_WITH_ZONE = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}(?::\d{2}(?:\.\d{1,3})?)?(?:Z|[+-]\d{2}:\d{2})$/; + +/** Whether `value` is a usable period boundary (see FilterRule.activeFrom). */ +export function isPeriodBoundary(value: unknown): value is string { + if (typeof value !== 'string' || !ISO_WITH_ZONE.test(value)) return false; + const time = Date.parse(value); + // In UTC the zone can push it past year 9999 ("9999-12-31T23:59-01:00"), + // which the four-digit year of Sieve's "date" part cannot hold. + return !Number.isNaN(time) && /^\d{4}-/.test(new Date(time).toISOString()); +} + +export function hasPeriod(rule: Pick): boolean { + return rule.activeFrom !== undefined || rule.activeUntil !== undefined; +} + +/** + * The boundary as RFC 5260 "date" (yyyy-mm-dd) and "time" (hh:mm:ss) parts in + * UTC. The RFC fixes both formats, so comparing them as strings works on any + * server - unlike "iso8601", whose zone suffix is up to the implementation. + */ +function utcParts(boundary: string): { date: string; time: string } { + const iso = new Date(Date.parse(boundary)).toISOString(); + return { date: iso.slice(0, 10), time: iso.slice(11, 19) }; +} + +const NOW = 'currentdate :zone "+0000"'; + +/** now >= boundary: a later day, or the same day at or after the time. */ +function atOrAfter(boundary: string): string { + const { date, time } = utcParts(boundary); + return `anyof(${NOW} :value "gt" "date" "${date}", allof(${NOW} :is "date" "${date}", ${NOW} :value "ge" "time" "${time}"))`; +} + +/** now <= boundary: an earlier day, or the same day at or before the time. */ +function atOrBefore(boundary: string): string { + const { date, time } = utcParts(boundary); + return `anyof(${NOW} :value "lt" "date" "${date}", allof(${NOW} :is "date" "${date}", ${NOW} :value "le" "time" "${time}"))`; +} + +/** + * The Sieve tests that confine `rule` to its period: none without a period, + * null when a boundary is unusable. A caller must then drop the rule rather + * than run it without its period - a forwarding rule would otherwise forward + * for good. + */ +export function periodTests(rule: Pick): string[] | null { + const tests: string[] = []; + if (rule.activeFrom !== undefined) { + if (!isPeriodBoundary(rule.activeFrom)) return null; + tests.push(atOrAfter(rule.activeFrom)); + } + if (rule.activeUntil !== undefined) { + if (!isPeriodBoundary(rule.activeUntil)) return null; + tests.push(atOrBefore(rule.activeUntil)); + } + return tests; +} + +export type PeriodStatus = 'scheduled' | 'active' | 'expired'; + +/** Where `now` falls in the rule's period; null for a rule without one. */ +export function periodStatus( + rule: Pick, + now: number = Date.now(), +): PeriodStatus | null { + if (!hasPeriod(rule)) return null; + if (isPeriodBoundary(rule.activeFrom) && now < Date.parse(rule.activeFrom)) return 'scheduled'; + if (isPeriodBoundary(rule.activeUntil) && now > Date.parse(rule.activeUntil)) return 'expired'; + return 'active'; +} diff --git a/src/lib/sieve/types.ts b/src/lib/sieve/types.ts index a4a82133..997dac82 100644 --- a/src/lib/sieve/types.ts +++ b/src/lib/sieve/types.ts @@ -22,7 +22,9 @@ export interface SieveCapabilities { export type FilterConditionField = | 'from' | 'to' | 'cc' | 'subject' | 'header' | 'size' | 'body' - | 'attachment'; + | 'attachment' + // Every message (Sieve `true`): no value, and always the comparator 'any'. + | 'all'; export type FilterComparator = | 'contains' | 'not_contains' @@ -34,7 +36,16 @@ export type FilterComparator = // has_any → message has any attachment (Content-Disposition: attachment) // has_type → message has an attachment whose Content-Type matches `value` // (substring match, e.g. "application/pdf" or "image/") - | 'has_any' | 'has_type'; + | 'has_any' | 'has_type' + // For field === 'from' | 'to' | 'cc', compared against the parsed address + // with the Sieve `address` test rather than the raw header text: + // address_is → the whole address equals `value` (anna@acme.com, but not + // joanna@acme.com) + // domain_is → the domain part equals `value` (acme.com, but neither + // sub.acme.com nor acme.com.evil) + | 'address_is' | 'domain_is' + // For field === 'all': any message at all. + | 'any'; export type FilterActionType = | 'move' | 'copy' | 'forward' @@ -84,6 +95,15 @@ export interface FilterRule { * folder rule does not pull spam out of Junk. */ includeSpam?: boolean; + /** + * The rule only acts on mail that arrives from `activeFrom` until + * `activeUntil`, both inclusive, either one open-ended. Absolute moments + * as ISO 8601 UTC ("2026-10-05T06:00:00.000Z"), so the script does not + * depend on the time zone of whoever saves it next. Compiled to Sieve + * `currentdate` tests (RFC 5260), which need "date" and "relational". + */ + activeFrom?: string; + activeUntil?: string; origin?: FilterOrigin; originLabel?: string; rawBlock?: string; @@ -95,10 +115,50 @@ export interface VacationSieveConfig { textBody: string; } +/** + * Forwarding set up in the vacation card: mail goes on to one address, with + * or without the auto-reply, in the vacation's period. It is not a filter + * rule and the filter list does not show it; it runs ahead of the rules. + */ +export interface VacationForward { + /** The forwarding switch, independent of the auto-reply's. */ + enabled: boolean; + /** The one address mail is forwarded to. */ + to: string; + /** + * Keep the message in this mailbox too (`redirect :copy`). Otherwise it + * is only forwarded, and no filter rule runs on it. + */ + keepCopy: boolean; + /** The auto-reply's period when it was saved, as for FilterRule.activeFrom. */ + activeFrom?: string; + activeUntil?: string; +} + +/** + * Who gets the auto-reply when not everyone should: only senders from the + * account's own domains, or only senders from elsewhere. It is decided on + * the envelope sender while the auto-reply runs from the filters script. + */ +export interface VacationAudience { + only: 'internal' | 'external'; + /** The account's own domains (from its identities) when it was saved. */ + domains: string[]; +} + export interface FilterMetadata { - version: 1; + /** + * 2 once a rule has a period or the vacation's forwarding or recipients are + * stored. Builds that only know version 1 would write such a script back + * without these fields, so they leave anything else alone. + */ + version: 1 | 2; rules: FilterRule[]; vacation?: VacationSieveConfig; + /** See VacationAudience; absent when everyone gets the auto-reply. */ + vacationAudience?: VacationAudience; + /** See VacationForward; kept while switched off, so the card remembers it. */ + vacationForward?: VacationForward; /** * The script runs the server-managed "vacation" script via `include`. * Servers like Stalwart keep one active script, so a VacationResponse diff --git a/src/lib/sieve/vacation-audience.ts b/src/lib/sieve/vacation-audience.ts new file mode 100644 index 00000000..0df8616e --- /dev/null +++ b/src/lib/sieve/vacation-audience.ts @@ -0,0 +1,30 @@ +import type { VacationAudience } from './types'; + +// A host name in the form the identities give it: labels of letters, digits +// and inner hyphens, at least two of them. +const DOMAIN = /^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)+$/; + +/** Whether `value` is an audience Bulwark can write back as it reads. */ +export function isValidVacationAudience(value: unknown): value is VacationAudience { + if (!value || typeof value !== 'object') return false; + const a = value as Record; + if (a.only !== 'internal' && a.only !== 'external') return false; + return Array.isArray(a.domains) && a.domains.length > 0 + && a.domains.every((d) => typeof d === 'string' && DOMAIN.test(d)); +} + +/** The fields Bulwark stores, in a fixed order. */ +export function normalizeVacationAudience(audience: VacationAudience): VacationAudience { + return { only: audience.only, domains: audience.domains }; +} + +/** The domains of the given addresses, lower-cased, each once, in order. */ +export function ownDomains(addresses: string[]): string[] { + const domains: string[] = []; + for (const address of addresses) { + const at = address.lastIndexOf('@'); + const domain = at === -1 ? '' : address.slice(at + 1).trim().toLowerCase(); + if (DOMAIN.test(domain) && !domains.includes(domain)) domains.push(domain); + } + return domains; +} diff --git a/src/lib/sieve/vacation-forward.ts b/src/lib/sieve/vacation-forward.ts new file mode 100644 index 00000000..1c1d6e8c --- /dev/null +++ b/src/lib/sieve/vacation-forward.ts @@ -0,0 +1,55 @@ +import type { VacationForward } from './types'; +import { isValidEmail } from '../recipients'; +import { isPeriodBoundary } from './period'; + +/** + * Forwarding from the vacation card (see VacationForward). The generator + * writes it as its own block after the vacation include, marked by this + * comment; the parser knows the block by it and leaves it to the generator. + */ +export const VACATION_FORWARD_MARKER = '# Vacation forwarding'; + +/** + * The marker as the parser looks for it in a block: the whole line, so a + * comment of someone's own that only begins like it is not taken for it. + */ +export const VACATION_FORWARD_MARKER_RE = /^[ \t]*# Vacation forwarding[ \t]*$/m; + +/** Whether `value` is forwarding Bulwark can write back as it reads. */ +export function isValidVacationForward(value: unknown): value is VacationForward { + if (!value || typeof value !== 'object') return false; + const f = value as Record; + if (typeof f.enabled !== 'boolean' || typeof f.keepCopy !== 'boolean') return false; + if (typeof f.to !== 'string' || !isValidEmail(f.to)) return false; + if (f.activeFrom !== undefined && !isPeriodBoundary(f.activeFrom)) return false; + if (f.activeUntil !== undefined && !isPeriodBoundary(f.activeUntil)) return false; + return true; +} + +/** + * The forwarding with the vacation's period, whatever period it had: it runs + * while the vacation does (an empty date leaves that end open). + */ +export function withVacationPeriod( + forward: Pick, + period: { from: string | null; until: string | null }, +): VacationForward { + return { + enabled: forward.enabled, + to: forward.to, + keepCopy: forward.keepCopy, + ...(period.from ? { activeFrom: period.from } : {}), + ...(period.until ? { activeUntil: period.until } : {}), + }; +} + +/** The fields Bulwark stores, in a fixed order. */ +export function normalizeVacationForward(forward: VacationForward): VacationForward { + return { + enabled: forward.enabled, + to: forward.to, + keepCopy: forward.keepCopy, + ...(forward.activeFrom !== undefined ? { activeFrom: forward.activeFrom } : {}), + ...(forward.activeUntil !== undefined ? { activeUntil: forward.activeUntil } : {}), + }; +} diff --git a/src/stores/__tests__/filter-store.test.ts b/src/stores/__tests__/filter-store.test.ts index 01797b1a..6ab20f4e 100644 --- a/src/stores/__tests__/filter-store.test.ts +++ b/src/stores/__tests__/filter-store.test.ts @@ -19,7 +19,9 @@ import { readWebmailResave as webmailResave, STALWART_EXTENSIONS, } from '../../lib/sieve/__tests__/fixtures/webmail'; -import { isVacationIncludedInFilters, syncVacationWithFilters, useFilterStore } from '../filter-store'; +import { + isVacationIncludedInFilters, readVacationFilters, syncVacationWithFilters, useFilterStore, +} from '../filter-store'; const api = vi.mocked(sieve); @@ -258,4 +260,92 @@ describe('saving scripts the webmail wrote', () => { await useFilterStore.getState().saveFilters(); expect(api.updateSieveScript.mock.calls[0][1]).toBe(webmailResave('stalwart-full.sieve')); }); + + // #1152: webmail 1.13+ writes version 2 for rule periods and the vacation + // card's forwarding and audience. Read as raw Sieve, "Reset to visual + // builder" wiped them; a save must keep them byte for byte. + it('edits a version 2 script and keeps periods, forwarding and audience', async () => { + const script = webmailFixture('period-forward-audience.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); + const s = useFilterStore.getState(); + expect(s.isOpaque).toBe(false); + expect(s.rules.map((r) => r.activeFrom)).toEqual(['2026-11-02T07:00:00.000Z', '2026-12-01T00:00:00.000Z']); + expect(s.vacationForward?.to).toBe('backup@example.org'); + expect(s.vacationAudience?.only).toBe('internal'); + + await useFilterStore.getState().saveFilters(); + expect(api.updateSieveScript).toHaveBeenCalledWith('s1', script, true, 'own'); + }); + + it('keeps the forwarding and audience when the auto-reply is switched off on the phone', async () => { + const script = webmailFixture('period-forward-audience.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 syncVacationWithFilters(false); + const written = api.updateSieveScript.mock.calls[0][1] as string; + expect(written).not.toContain('include :personal'); + expect(written).toContain('# Vacation forwarding'); + expect(written).toContain('"vacationAudience":{"only":"internal","domains":["example.org","example.com"]}'); + expect(written).toContain('"activeUntil":"2026-11-13T17:30:00.000Z"'); + }); + + it('keeps the remembered forwarding of a version 1 script', async () => { + const script = webmailFixture('forward-off.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); + await useFilterStore.getState().saveFilters(); + expect(api.updateSieveScript).toHaveBeenCalledWith('s1', script, true, 'own'); + }); +}); + +describe('readVacationFilters', () => { + it('reports the stored forwarding and audience and what the server can run', async () => { + 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(webmailFixture('period-forward-audience.sieve')); + + const filters = await readVacationFilters(); + expect(filters).toMatchObject({ + includesVacation: true, + forward: { enabled: true, to: 'backup@example.org', keepCopy: false }, + forwardAvailable: true, + audience: { only: 'internal', domains: ['example.org', 'example.com'] }, + audienceAvailable: true, + notRunning: false, + }); + }); + + it('sets up forwarding in the vacation period, creating the script when there is none', async () => { + api.getSieveCapabilities.mockReturnValue({ ...WITH_INCLUDE, sieveExtensions: STALWART_EXTENSIONS }); + api.getSieveScripts.mockResolvedValue([{ id: 'v1', name: 'vacation', blobId: 'bv', isActive: true }]); + api.createSieveScript.mockResolvedValue({ id: 'new', name: 'filters', blobId: 'b', isActive: true }); + + await syncVacationWithFilters( + true, undefined, { enabled: true, to: 'backup@example.org', keepCopy: true }, + undefined, { from: '2026-11-02T00:00:00Z', until: null }, + ); + const [name, content, activate] = api.createSieveScript.mock.calls[0]; + expect(name).toBe('filters'); + expect(activate).toBe(true); + expect(content).toContain('"vacationForward":{"enabled":true,"to":"backup@example.org","keepCopy":true,"activeFrom":"2026-11-02T00:00:00Z"}'); + expect(content).toContain(' redirect :copy "backup@example.org";'); + }); }); diff --git a/src/stores/filter-store.ts b/src/stores/filter-store.ts index 0418a577..0268af15 100644 --- a/src/stores/filter-store.ts +++ b/src/stores/filter-store.ts @@ -1,7 +1,12 @@ import { create } from 'zustand'; -import type { FilterRule, SieveCapabilities, VacationSieveConfig } from '../lib/sieve/types'; +import type { + FilterRule, SieveCapabilities, VacationAudience, VacationForward, VacationSieveConfig, +} from '../lib/sieve/types'; import { parseScript } from '../lib/sieve/parser'; -import { generateScript, VACATION_SCRIPT_NAME } from '../lib/sieve/generator'; +import { generateScript, supportsSpamGuard, VACATION_SCRIPT_NAME } from '../lib/sieve/generator'; +import { supportsPeriods } from '../lib/sieve/period'; +import { isValidVacationForward, withVacationPeriod } from '../lib/sieve/vacation-forward'; +import { isValidVacationAudience, normalizeVacationAudience } from '../lib/sieve/vacation-audience'; import { createSieveScript, getSieveAccountId, @@ -32,6 +37,10 @@ interface FilterStore { externalRequires: string[]; /** The script runs the server's vacation script via `include`. */ includeVacation: boolean; + /** Forwarding from the vacation card; carried through every save. */ + vacationForward: VacationForward | null; + /** Who gets the auto-reply, when not everyone; carried through every save. */ + vacationAudience: VacationAudience | null; selectedAccountId: string | null; fetchFilters: (accountId?: string) => Promise; @@ -63,6 +72,8 @@ export const useFilterStore = create()((set, get) => ({ vacationSettings: null, externalRequires: [], includeVacation: false, + vacationForward: null, + vacationAudience: null, selectedAccountId: null, fetchFilters: async (accountId) => { @@ -102,6 +113,8 @@ export const useFilterStore = create()((set, get) => ({ rawScript: '', isOpaque: false, includeVacation: vacationActive, + vacationForward: null, + vacationAudience: null, }); return; } @@ -121,6 +134,8 @@ export const useFilterStore = create()((set, get) => ({ vacationSettings: result.vacation || null, externalRequires: result.externalRequires, includeVacation: !result.isOpaque && (!!result.includeVacation || vacationActive), + vacationForward: result.isOpaque ? null : result.vacationForward ?? null, + vacationAudience: result.isOpaque ? null : result.vacationAudience ?? null, }); } catch (error) { if (stale()) return; @@ -143,6 +158,8 @@ export const useFilterStore = create()((set, get) => ({ vacationSettings: null, externalRequires: [], includeVacation: false, + vacationForward: null, + vacationAudience: null, }); await get().fetchFilters(accountId ?? undefined); }, @@ -152,7 +169,7 @@ export const useFilterStore = create()((set, get) => ({ try { const { isOpaque, rawScript, rules, activeScriptId, vacationSettings, externalRequires, includeVacation, - selectedAccountId, sieveCapabilities, + vacationForward, vacationAudience, selectedAccountId, sieveCapabilities, } = get(); const accountId = selectedAccountId ?? undefined; @@ -161,6 +178,8 @@ export const useFilterStore = create()((set, get) => ({ : generateScript(rules, vacationSettings || undefined, { externalRequires, includeVacation, + vacationForward, + vacationAudience, extensions: sieveCapabilities?.sieveExtensions, }); @@ -257,6 +276,8 @@ export const useFilterStore = create()((set, get) => ({ vacationSettings: null, externalRequires: [], includeVacation: false, + vacationForward: null, + vacationAudience: null, selectedAccountId: null, }), })); @@ -285,36 +306,184 @@ export async function isVacationIncludedInFilters(accountId?: string): Promise { + const sieveAccountId = accountId ?? getSieveAccountId(); + const capabilities = getSieveCapabilities(sieveAccountId); + const { vacationScript, target, parsed } = await loadManagedScript(sieveAccountId); + const filtersUsable = supportsInclude(capabilities) && !(target && !parsed); + const forward = parsed?.vacationForward ?? null; + const audience = parsed?.vacationAudience ?? null; + return { + includesVacation: !!(vacationScript && target?.isActive && parsed?.includeVacation), + forward, + forwardAvailable: filtersUsable && (canForward(capabilities) || !!forward?.enabled), + audience, + // Told apart by the envelope sender, which the auto-reply goes to. + audienceAvailable: filtersUsable && !!capabilities?.sieveExtensions?.includes('envelope'), + notRunning: storedButIdle(!!target?.isActive, !!vacationScript?.isActive, forward, audience), + }; +} + +function same(a: T | null, b: T | null, normalize: (value: T) => T): boolean { + if (!a || !b) return a === b; + return JSON.stringify(normalize(a)) === JSON.stringify(normalize(b)); +} + +// The same moment, however it is written: the server hands the vacation's +// dates back in its own form. +function sameMoment(a: string | undefined, b: string | undefined): boolean { + if (a === undefined || b === undefined) return a === b; + return a === b || Date.parse(a) === Date.parse(b); +} + +function sameForward(a: VacationForward | null, b: VacationForward | null): boolean { + if (!a || !b) return a === b; + return a.enabled === b.enabled && a.to === b.to && a.keepCopy === b.keepCopy && + sameMoment(a.activeFrom, b.activeFrom) && sameMoment(a.activeUntil, b.activeUntil); +} + /** - * Keep the filters and the auto-reply both running after VacationResponse/set - * (webmail 198a3c0d). + * Keep the filters, the auto-reply and its forwarding running after + * VacationResponse/set (webmail 198a3c0d, 1.13 forwarding). * * Stalwart allows one active Sieve script and turns the auto-reply on by * activating its own "vacation" script, which switches every filter off. * When that happened, re-activate the filters script with an `include` of * the vacation script. When the auto-reply is turned off, drop the include. + * + * `forward` replaces the stored forwarding (null removes it, undefined keeps + * it). Forwarding runs with or without the auto-reply, but only from an + * active filters script, so for it the script is activated even without + * rules, and created when there is none. `audience` likewise replaces who + * gets the auto-reply (null: everyone). `period` is the vacation's, which + * whatever forwarding there is keeps to. */ -export async function syncVacationWithFilters(enabled: boolean, accountId?: string): Promise { +export async function syncVacationWithFilters( + enabled: boolean, + accountId?: string, + forward?: VacationForward | null, + audience?: VacationAudience | null, + period?: { from: string | null; until: string | null }, +): Promise { const sieveAccountId = accountId ?? getSieveAccountId(); const capabilities = getSieveCapabilities(sieveAccountId); - if (enabled && !supportsInclude(capabilities)) return; + const canInclude = supportsInclude(capabilities); + if (enabled && !canInclude && forward === undefined && audience === undefined) return; const { vacationScript, target, parsed } = await loadManagedScript(sieveAccountId); - if (!target || !parsed) return; + const opaque = !!target && !parsed; + const storedForward = parsed?.vacationForward ?? null; + const storedAudience = parsed?.vacationAudience ?? null; + const requestedForward = forward === undefined ? storedForward : forward; + const nextForward = requestedForward && period ? withVacationPeriod(requestedForward, period) : requestedForward; + const nextAudience = audience === undefined ? storedAudience : audience; + const changed = + !sameForward(storedForward, nextForward) || + !same(storedAudience, nextAudience, normalizeVacationAudience); + if (changed) { + // The card only offers these where they can be stored and run. + if (opaque) throw new OpaqueFiltersError(); + if (!canInclude) throw new Error('Forwarding and reply recipients need the Sieve "include" extension'); + // The generator leaves out what it could not read back, and the save + // would look done while nothing forwards, or everyone gets the reply. + if (nextForward && !isValidVacationForward(nextForward)) throw new Error('Unusable vacation forwarding'); + if (nextAudience && !isValidVacationAudience(nextAudience)) throw new Error('Unusable vacation reply recipients'); + } + if ((enabled && !canInclude) || opaque) return; + + const rules = parsed?.rules ?? []; + const forwarding = !!nextForward?.enabled; if (enabled) { - // Only act when the vacation script took over from existing filters. - if (!vacationScript?.isActive || target.isActive || parsed.rules.length === 0) return; - } else if (!parsed.includeVacation) { - return; + // Act when the vacation script took over from filters that must keep + // running, or when something changed. An auto-reply for some senders + // only must run from the filters script: on its own it answers everyone. + const tookOver = !!vacationScript?.isActive && !target?.isActive; + const needsFilters = rules.length > 0 || forwarding || !!nextAudience; + if (!changed && !(tookOver && needsFilters)) return; + } else { + // Forwarding also runs without the auto-reply, from an active filters + // script; turning the auto-reply off can leave no script active. + const forwardingIdle = forwarding && !target?.isActive; + if (!changed && !parsed?.includeVacation && !forwardingIdle) return; } - const content = generateScript(parsed.rules, parsed.vacation, { - externalRequires: parsed.externalRequires, + const content = generateScript(rules, parsed?.vacation, { + externalRequires: parsed?.externalRequires, includeVacation: enabled, + vacationForward: nextForward, + vacationAudience: nextAudience, extensions: capabilities?.sieveExtensions, }); - await updateSieveScript(target.id, content, enabled || target.isActive, sieveAccountId); + const activate = enabled || forwarding || !!target?.isActive; + if (target) { + await updateSieveScript(target.id, content, activate, sieveAccountId); + } else { + await createSieveScript('filters', content, activate, sieveAccountId); + } const store = useFilterStore.getState(); if (store.selectedAccountId === sieveAccountId) { From df1a5ab4b67d1bb1b7d8791d55c5dd85d199f525 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:07:09 +0100 Subject: [PATCH 2/9] feat: set up vacation forwarding, reply recipients, rule periods and all-messages rules on the phone #1152 --- .../__tests__/settings-i18n.test.ts | 2 +- src/components/filters/FilterRuleModal.tsx | 111 ++++++++++--- src/components/settings/FilterSettings.tsx | 20 ++- src/components/settings/VacationSettings.tsx | 149 +++++++++++++++++- .../sieve/__tests__/condition-value.test.ts | 22 +++ src/lib/sieve/__tests__/rule-period.test.ts | 33 ++++ src/lib/sieve/condition-value.ts | 29 +++- src/lib/sieve/rule-period.ts | 37 +++++ src/stores/__tests__/vacation-store.test.ts | 59 ++++++- src/stores/vacation-store.ts | 136 +++++++++++++--- 10 files changed, 542 insertions(+), 56 deletions(-) create mode 100644 src/lib/sieve/__tests__/rule-period.test.ts create mode 100644 src/lib/sieve/rule-period.ts diff --git a/src/components/__tests__/settings-i18n.test.ts b/src/components/__tests__/settings-i18n.test.ts index e22520e3..12f71d1e 100644 --- a/src/components/__tests__/settings-i18n.test.ts +++ b/src/components/__tests__/settings-i18n.test.ts @@ -47,7 +47,7 @@ const TRANSLATED_PANES: Record = { 'settings/device-sync/DeviceSyncAccountRow.tsx': [], 'settings/device-sync/DeviceSyncCollectionsSheet.tsx': [], 'settings/device-sync/status.ts': [], - 'filters/FilterRuleModal.tsx': [], + 'filters/FilterRuleModal.tsx': ['YYYY-MM-DD HH:MM'], 'filters/SieveEditorSheet.tsx': [], }; diff --git a/src/components/filters/FilterRuleModal.tsx b/src/components/filters/FilterRuleModal.tsx index 5564cccd..6461fd15 100644 --- a/src/components/filters/FilterRuleModal.tsx +++ b/src/components/filters/FilterRuleModal.tsx @@ -14,9 +14,10 @@ import { useLocaleStore } from '../../stores/locale-store'; import { useKeywordsStore } from '../../stores/keywords-store'; import { generateUUID } from '../../lib/uuid'; import { + comparatorsFor, inputStringToValue, isConditionValueEmpty, - isHasAnyCondition, + takesNoValue, valueToInputString, } from '../../lib/sieve/condition-value'; import { @@ -29,6 +30,8 @@ import { withMailboxTarget, } from '../../lib/sieve/rule-actions'; import { isValidSieveHeaderName } from '../../lib/sieve/generator'; +import { hasPeriod } from '../../lib/sieve/period'; +import { periodError, periodFromInput, periodToInput } from '../../lib/sieve/rule-period'; import type { Mailbox } from '../../api/types'; import type { FilterRule, @@ -41,18 +44,9 @@ import type { import { SafeAreaModal } from '../SafeAreaModal'; import { MODAL_ORIENTATIONS } from '../../lib/modal-orientations'; -const ALL_FIELDS: FilterConditionField[] = ['from', 'to', 'cc', 'subject', 'header', 'size', 'body', 'attachment']; -const TEXT_COMPARATORS: FilterComparator[] = ['contains', 'not_contains', 'is', 'not_is', 'starts_with', 'ends_with', 'matches']; -const SIZE_COMPARATORS: FilterComparator[] = ['greater_than', 'less_than']; -const ATTACHMENT_COMPARATORS: FilterComparator[] = ['has_any', 'has_type']; +const ALL_FIELDS: FilterConditionField[] = ['from', 'to', 'cc', 'subject', 'header', 'size', 'body', 'attachment', 'all']; -function comparatorsFor(field: FilterConditionField): FilterComparator[] { - if (field === 'size') return SIZE_COMPARATORS; - if (field === 'attachment') return ATTACHMENT_COMPARATORS; - return TEXT_COMPARATORS; -} - -const isHasAny = isHasAnyCondition; +const isHasAny = takesNoValue; function seedConditions(rule?: FilterRule): FilterCondition[] { return rule?.conditions.length ? rule.conditions.map((cond) => ({ ...cond })) : [makeEmptyCondition()]; @@ -73,11 +67,15 @@ interface FilterRuleModalProps { visible: boolean; rule?: FilterRule; mailboxes: Mailbox[]; + /** The server can confine a rule to a period (Sieve "date" and "relational"). */ + periodsSupported?: boolean; onSave: (rule: FilterRule) => void; onClose: () => void; } -export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: FilterRuleModalProps) { +export function FilterRuleModal({ + visible, rule, mailboxes, periodsSupported = false, onSave, onClose, +}: FilterRuleModalProps) { const c = useColors(); const styles = useMemo(() => makeStyles(c), [c]); const t = useLocaleStore((s) => s.t); @@ -90,6 +88,10 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F const [actions, setActions] = useState(() => seedActions(rule)); const [stopProcessing, setStopProcessing] = useState(rule?.stopProcessing ?? false); const [includeSpam, setIncludeSpam] = useState(rule?.includeSpam ?? false); + const hadPeriod = !!rule && hasPeriod(rule); + const [periodOn, setPeriodOn] = useState(hadPeriod); + const [periodStart, setPeriodStart] = useState(periodToInput(rule?.activeFrom)); + const [periodEnd, setPeriodEnd] = useState(periodToInput(rule?.activeUntil)); // 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 @@ -103,8 +105,15 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F setActions(seedActions(rule)); setStopProcessing(rule?.stopProcessing ?? false); setIncludeSpam(rule?.includeSpam ?? false); + setPeriodOn(!!rule && hasPeriod(rule)); + setPeriodStart(periodToInput(rule?.activeFrom)); + setPeriodEnd(periodToInput(rule?.activeUntil)); }, [visible, rule]); + const activeFrom = periodOn ? periodFromInput(periodStart, rule?.activeFrom) : undefined; + const activeUntil = periodOn ? periodFromInput(periodEnd, rule?.activeUntil) : undefined; + const periodProblem = periodOn ? periodError(activeFrom, activeUntil) : null; + const mailboxTargets = useMemo(() => buildMailboxTargets(mailboxes), [mailboxes]); const fieldOptions = useMemo( @@ -225,6 +234,14 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F Alert.alert(t('settings.filters.validation_empty_actions', 'At least one action is required')); return; } + if (periodProblem) { + Alert.alert(periodProblem === 'invalid' + ? t('settings.filters.validation_period_invalid', 'Enter a valid date with a time for the period, or leave the field empty') + : periodProblem === 'empty' + ? t('settings.filters.validation_period_empty', 'Enter a start or end date for the period') + : t('settings.filters.validation_period_order', 'End date must be after start date')); + return; + } onSave({ id: rule?.id || generateUUID(), name: trimmedName, @@ -236,8 +253,15 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F // 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 } : {}), + // Set to undefined when off: the settings page merges the result into + // the stored rule, and a field left out would keep its old value there. + activeFrom: activeFrom ?? undefined, + activeUntil: activeUntil ?? undefined, }); - }, [name, conditions, actions, matchType, stopProcessing, includeSpam, rule, onSave, t, mailboxTargets]); + }, [ + name, conditions, actions, matchType, stopProcessing, includeSpam, rule, onSave, t, mailboxTargets, + periodProblem, activeFrom, activeUntil, + ]); return ( @@ -332,14 +356,16 @@ export function FilterRuleModal({ visible, rule, mailboxes, onSave, onClose }: F /> )} - updateCondition(index, { comparator: v as FilterComparator })} + options={comparatorOptions(condition.field)} + style={{ alignSelf: 'flex-start' }} + /> + )} - {/* has_any takes no value ("an attachment is present"). */} + {/* has_any and "all messages" take no value. */} {!isHasAny(condition) && ( )} + + {/* Offered where the server supports it; a rule that already has a + period always shows it, so it can be removed. */} + {(periodsSupported || hadPeriod) && ( + + + + {t('settings.filters.period_toggle', 'Only active during a period')} + + + + {periodOn && ( + <> + {t('settings.filters.period_start', 'Start date')} + + {t('settings.filters.period_end', 'End date')} + + + {t('settings.filters.period_hint', 'Applies to messages that arrive during this period. Leave a field empty to keep that end open.')} + + + )} + + )} @@ -572,6 +640,7 @@ function makeStyles(c: ThemePalette) { gap: spacing.md, }, optionLabel: { ...typography.body, color: c.text, flex: 1 }, + hint: { ...typography.caption, color: c.mutedForeground }, footer: { flexDirection: 'row', diff --git a/src/components/settings/FilterSettings.tsx b/src/components/settings/FilterSettings.tsx index beeb4461..83d21b7d 100644 --- a/src/components/settings/FilterSettings.tsx +++ b/src/components/settings/FilterSettings.tsx @@ -21,6 +21,7 @@ import { FilterRuleModal } from '../filters/FilterRuleModal'; import { SieveEditorSheet } from '../filters/SieveEditorSheet'; import type { FilterRule } from '../../lib/sieve/types'; import { formatConditionValue, summarizeRule } from '../../lib/sieve/condition-value'; +import { periodStatus, supportsPeriods } from '../../lib/sieve/period'; import Badge from '../Badge'; type Translate = (key: string, fallback?: string) => string; @@ -29,6 +30,18 @@ function isReadonlyRule(r: FilterRule): boolean { return r.origin === 'external' || r.origin === 'opaque'; } +/** Whether a rule with a period is waiting for it, in it, or past it. */ +function PeriodBadge({ rule, t }: { rule: FilterRule; t: Translate }) { + const status = periodStatus(rule); + if (!status) return null; + const label = status === 'scheduled' + ? t('settings.filters.period_status_scheduled', 'scheduled') + : status === 'active' + ? t('settings.filters.period_status_active', 'active') + : t('settings.filters.period_status_expired', 'expired'); + return {label}; +} + // Expanded "IF ... / THEN ..." view (webmail VisualRuleSummary, 1.4.6): every // condition and action as a chip, with the match-type hint after the IF row. function VisualRuleSummary({ rule, t, c }: { rule: FilterRule; t: Translate; c: ThemePalette }) { @@ -147,6 +160,7 @@ export function FilterSettings({ onOpenVacation }: FilterSettingsProps = {}) { selectAccount, saveFilters, addRule, updateRule, deleteRule, reorderRules, toggleRule, setOpaqueScript, resetToVisualBuilder, validateScript, } = useFilterStore(); + const periodsSupported = useFilterStore((s) => supportsPeriods(s.sieveCapabilities?.sieveExtensions)); const [editingRule, setEditingRule] = useState(); const [showRuleModal, setShowRuleModal] = useState(false); @@ -397,7 +411,10 @@ export function FilterSettings({ onOpenVacation }: FilterSettingsProps = {}) { style={{ flex: 1, minWidth: 0 }} onPress={() => { setEditingRule(rule); setShowRuleModal(true); }} > - {rule.name} + + {rule.name} + + {expandedView ? ( ) : ( @@ -487,6 +504,7 @@ export function FilterSettings({ onOpenVacation }: FilterSettingsProps = {}) { visible={showRuleModal} rule={editingRule} mailboxes={mailboxes} + periodsSupported={periodsSupported} onSave={handleSaveRule} onClose={() => { setShowRuleModal(false); setEditingRule(undefined); }} /> diff --git a/src/components/settings/VacationSettings.tsx b/src/components/settings/VacationSettings.tsx index 83801284..60aef49e 100644 --- a/src/components/settings/VacationSettings.tsx +++ b/src/components/settings/VacationSettings.tsx @@ -4,14 +4,18 @@ import { WebView } from 'react-native-webview'; import { AlertTriangle, Eye, EyeOff, Bold, Italic, Underline, List as ListIcon, } from '../icons'; -import { SettingsSection, SettingItem, ToggleSwitch } from './settings-section'; +import { SettingsSection, SettingItem, ToggleSwitch, Select } from './settings-section'; import Button from '../Button'; import RichTextEditor, { type RichTextEditorHandle } from '../RichTextEditor'; import { spacing, radius, typography, type ThemePalette } from '../../theme/tokens'; import { useColors } from '../../theme/colors'; -import { useVacationStore } from '../../stores/vacation-store'; +import { useVacationStore, VacationFiltersError, type VacationForwardSettings } from '../../stores/vacation-store'; import { useManagedAccountStore } from '../../stores/managed-account-store'; import { useLocaleStore } from '../../stores/locale-store'; +import { useSettingsStore } from '../../stores/settings-store'; +import { isValidEmail } from '../../lib/recipients'; +import { ownDomains } from '../../lib/sieve/vacation-audience'; +import type { VacationForward } from '../../lib/sieve/types'; import { htmlToPlainText } from '../../lib/compose-html'; import { stripDangerousTags, escapeHtml } from '../../lib/email-html'; import { @@ -26,6 +30,16 @@ import { // are typed as local wall-clock time and sent as RFC 8621 UTCDate strings; // the optional HTML body is edited in the composer's rich-text editor and // sanitised on save, with the plain-text part derived from it when left blank. +// Forwarding and "reply to internal/external senders only" (webmail 1.13, +// #1152) live in the filters script, so they are offered only where that +// script can carry them (see readVacationFilters). + +type AudienceChoice = 'all' | 'internal' | 'external'; + +function sameForward(a: VacationForwardSettings | null, b: VacationForward | null): boolean { + if (!a || !b) return a === b; + return a.enabled === b.enabled && a.to === b.to && a.keepCopy === b.keepCopy; +} export function VacationSettings() { const c = useColors(); @@ -46,6 +60,13 @@ export function VacationSettings() { const editorRef = useRef(null); // Seed the editor once per load; RichTextEditor only reads initialHtml on mount. const [editorSeed, setEditorSeed] = useState(store.htmlBody || ''); + const [forwardEnabled, setForwardEnabled] = useState(store.forward?.enabled ?? false); + const [forwardTo, setForwardTo] = useState(store.forward?.to ?? ''); + const [forwardKeep, setForwardKeep] = useState(store.forward?.keepCopy ?? false); + const [audienceOnly, setAudienceOnly] = useState(store.audience?.only ?? 'all'); + // "Internal" are the account's own domains, as its identities give them. + const identities = useSettingsStore((s) => s.identities); + const domains = useMemo(() => ownDomains(identities.map((i) => i.email)), [identities]); // The store reuses `error` for save failures too, so remember whether the // initial fetch itself failed (that's the only case that blanks the form). @@ -66,11 +87,41 @@ export function VacationSettings() { setHtmlEnabled(!!store.htmlBody); setHtmlBody(store.htmlBody || ''); setEditorSeed(store.htmlBody || ''); + setForwardEnabled(store.forward?.enabled ?? false); + setForwardTo(store.forward?.to ?? ''); + setForwardKeep(store.forward?.keepCopy ?? false); + setAudienceOnly(store.audience?.only ?? 'all'); }, [ store.hasLoaded, store.isEnabled, store.fromDate, store.toDate, - store.subject, store.textBody, store.htmlBody, + store.subject, store.textBody, store.htmlBody, store.forward, store.audience, ]); + // Who gets the auto-reply as a save stores it: everyone, or the senders + // from (or not from) the account's domains as they are now. The domains + // come from the signed-in user's identities, so a managed (shared) account + // offers no narrowing rather than one built from the wrong domains. + const canNarrow = store.audienceAvailable && !managedAccountId && domains.length > 0; + const audienceSettings = !canNarrow + ? undefined + : audienceOnly === 'all' ? null : { only: audienceOnly, domains }; + const audienceChanged = canNarrow && ( + audienceOnly !== (store.audience?.only ?? 'all') || + (audienceOnly !== 'all' && domains.join(',') !== (store.audience?.domains ?? []).join(',')) + ); + + // Forwarding as a save stores it: an address that is off is kept for next + // time if it is usable, and dropped otherwise. + const forwardAddress = forwardTo.trim(); + const forwardInvalid = store.forwardAvailable && forwardEnabled && !isValidEmail(forwardAddress); + const forwardSettings: VacationForwardSettings | null | undefined = !store.forwardAvailable + ? undefined + : forwardEnabled || isValidEmail(forwardAddress) + ? { enabled: forwardEnabled, to: forwardAddress, keepCopy: forwardKeep } + : null; + // Only what was changed here is sent: the rest stays as the server has it, + // which another device may have changed since this one loaded. + const forwardChanged = forwardSettings !== undefined && !sameForward(forwardSettings, store.forward); + const fromParsed = parseLocalInput(fromDate); const toParsed = parseLocalInput(toDate); const formatError = !isValidLocalInput(fromDate) || !isValidLocalInput(toDate); @@ -99,9 +150,11 @@ export function VacationSettings() { (formatError ? true : localInputToUtcIso(toDate) !== normalizeUtcIso(store.toDate)) || subject !== store.subject || body !== store.textBody || - (htmlEnabled ? htmlBody : '') !== (store.htmlBody || ''); + (htmlEnabled ? htmlBody : '') !== (store.htmlBody || '') || + forwardChanged || + audienceChanged; - const canSave = hasChanges && !endBeforeStart && !formatError && !store.isSaving; + const canSave = hasChanges && !endBeforeStart && !formatError && !forwardInvalid && !store.isSaving; const handleSave = useCallback(async () => { // Read the live editor DOM rather than trusting onChange state (issue #9). @@ -129,6 +182,8 @@ export function VacationSettings() { subject: subject.trim(), textBody, htmlBody: sanitizedHtml, + forward: forwardChanged ? forwardSettings : undefined, + audience: audienceChanged ? audienceSettings : undefined, }); if (!sanitizedHtml) { setHtmlEnabled(false); @@ -137,11 +192,16 @@ export function VacationSettings() { Alert.alert(t('notifications.vacation_saved', 'Vacation responder settings saved')); } catch (err) { Alert.alert( - t('notifications.vacation_save_failed', 'Failed to save vacation responder settings'), + err instanceof VacationFiltersError + ? t('notifications.vacation_filters_save_failed', 'Out of office settings saved, but the forwarding and reply recipients could not be saved') + : t('notifications.vacation_save_failed', 'Failed to save vacation responder settings'), err instanceof Error ? err.message : undefined, ); } - }, [htmlBody, htmlEnabled, body, enabled, fromDate, toDate, subject, store, t]); + }, [ + htmlBody, htmlEnabled, body, enabled, fromDate, toDate, subject, store, t, + forwardChanged, forwardSettings, audienceChanged, audienceSettings, + ]); const title = t('settings.vacation.title', 'Vacation Responder'); const description = t('settings.vacation.description', "Automatically reply to incoming emails while you're away"); @@ -202,8 +262,34 @@ export function VacationSettings() { + {canNarrow && ( + +