From 31eff96614647cc4e70ce1eef082b821cde1dbeb Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 16 Apr 2026 17:22:52 +0200 Subject: [PATCH] feat: Enhance external rule handling in Sieve parser and store #201 --- components/settings/filter-settings.tsx | 202 ++++--- lib/__tests__/sieve-generator.test.ts | 9 +- lib/jmap/sieve-types.ts | 5 + lib/sieve/__tests__/external-rules.test.ts | 267 +++++++++ .../__tests__/fixtures/mixed-origins.sieve | 44 ++ lib/sieve/__tests__/parser.test.ts | 15 +- lib/sieve/generator.ts | 66 +- lib/sieve/parser.ts | 565 ++++++++++++++++-- locales/en/common.json | 4 +- stores/__tests__/filter-store.test.ts | 15 +- stores/filter-store.ts | 69 ++- 11 files changed, 1115 insertions(+), 146 deletions(-) create mode 100644 lib/sieve/__tests__/external-rules.test.ts create mode 100644 lib/sieve/__tests__/fixtures/mixed-origins.sieve diff --git a/components/settings/filter-settings.tsx b/components/settings/filter-settings.tsx index ed28bc97..a0dd780a 100644 --- a/components/settings/filter-settings.tsx +++ b/components/settings/filter-settings.tsx @@ -23,8 +23,13 @@ import { Filter, RotateCcw, PalmtreeIcon, + Lock, } from "lucide-react"; +function isReadonlyRule(r: FilterRule): boolean { + return r.origin === "external" || r.origin === "opaque"; +} + function RuleSummary({ rule }: { rule: FilterRule }) { const t = useTranslations("settings.filters"); @@ -429,90 +434,137 @@ export function FilterSettings() { {!isOpaque && rules.length > 0 && (
- {rules.map((rule, index) => ( -
handleDragStart(e, index)} - onDragOver={(e) => handleDragOver(e, index)} - onDrop={(e) => handleDrop(e, index)} - onDragEnd={handleDragEnd} - className={`flex items-start gap-3 p-3 rounded-md border transition-colors ${ - dragOverIndex === index - ? "border-primary bg-primary/5" - : "border-border hover:bg-muted/50" - } ${!rule.enabled ? "opacity-60" : ""}`} - > + {rules.map((rule, index) => { + const readonly = isReadonlyRule(rule); + + if (readonly) { + const label = rule.originLabel || t("origin_external"); + const tooltip = t("managed_by_tooltip", { source: label }); + const hasStructuredSummary = + rule.origin === "external" && + rule.conditions.length > 0 && + rule.actions.length > 0; + return ( +
+
+ +
+ +
+
+

+ {rule.name} +

+ + {label} + +
+ {hasStructuredSummary ? ( + expandedFilterView ? ( + + ) : ( + + ) + ) : rule.rawBlock ? ( + 
+                          {rule.rawBlock.trim()}
+
+ ) : null} +
+
+ ); + } + + return (
handleDragStart(e, index)} + onDragOver={(e) => handleDragOver(e, index)} + onDrop={(e) => handleDrop(e, index)} + onDragEnd={handleDragEnd} + className={`flex items-start gap-3 p-3 rounded-md border transition-colors ${ + dragOverIndex === index + ? "border-primary bg-primary/5" + : "border-border hover:bg-muted/50" + } ${!rule.enabled ? "opacity-60" : ""}`} > - -
+
+ +
-
- handleToggle(rule.id)} - /> -
+
+ handleToggle(rule.id)} + /> +
-
{ - setEditingRule(rule); - setShowRuleModal(true); - }} - role="button" - tabIndex={0} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); +
{ setEditingRule(rule); setShowRuleModal(true); - } - }} - > -

- {rule.name} -

- {expandedFilterView ? ( - + }} + role="button" + tabIndex={0} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + setEditingRule(rule); + setShowRuleModal(true); + } + }} + > +

+ {rule.name} +

+ {expandedFilterView ? ( + + ) : ( + + )} +
+ + {deleteConfirmId === rule.id ? ( +
+ + +
) : ( - + )}
- - {deleteConfirmId === rule.id ? ( -
- - -
- ) : ( - - )} -
- ))} + ); + })}
)} diff --git a/lib/__tests__/sieve-generator.test.ts b/lib/__tests__/sieve-generator.test.ts index da4fe6cd..b66b47a1 100644 --- a/lib/__tests__/sieve-generator.test.ts +++ b/lib/__tests__/sieve-generator.test.ts @@ -196,10 +196,15 @@ describe('sieve generator', () => { expect(result.vacation?.isEnabled).toBe(true); }); - it('should mark as opaque when real filter rules exist alongside vacation', () => { + it('parses filter rules alongside vacation as external when no metadata is present', () => { const script = `require ["vacation", "fileinto"];\n\nvacation "Away";\n\nif header :contains "From" "boss@example.com" {\n fileinto "Important";\n}\n`; const result = parseScript(script); - expect(result.isOpaque).toBe(true); + // New behavior: preserve both the vacation statement (as opaque) and + // the if-block (as a structured external rule) instead of dropping them. + expect(result.isOpaque).toBe(false); + const ifRule = result.rules.find(r => r.origin === 'external'); + expect(ifRule?.conditions[0]).toMatchObject({ field: 'from', comparator: 'contains', value: 'boss@example.com' }); + expect(ifRule?.actions[0]).toEqual({ type: 'move', value: 'Important' }); }); it('should handle Stalwart :mime format vacation script', () => { diff --git a/lib/jmap/sieve-types.ts b/lib/jmap/sieve-types.ts index 1f8e4035..26aecc40 100644 --- a/lib/jmap/sieve-types.ts +++ b/lib/jmap/sieve-types.ts @@ -39,6 +39,8 @@ export interface FilterAction { value?: string; } +export type FilterOrigin = 'bulwark' | 'external' | 'opaque'; + export interface FilterRule { id: string; name: string; @@ -47,6 +49,9 @@ export interface FilterRule { conditions: FilterCondition[]; actions: FilterAction[]; stopProcessing: boolean; + origin?: FilterOrigin; + originLabel?: string; + rawBlock?: string; } export interface VacationSieveConfig { diff --git a/lib/sieve/__tests__/external-rules.test.ts b/lib/sieve/__tests__/external-rules.test.ts new file mode 100644 index 00000000..a5d3175f --- /dev/null +++ b/lib/sieve/__tests__/external-rules.test.ts @@ -0,0 +1,267 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { parseScript } from '../parser'; +import { generateScript } from '../generator'; +import type { FilterRule } from '@/lib/jmap/sieve-types'; + +function makeBulwarkRule(overrides: Partial = {}): FilterRule { + return { + id: 'bw-1', + name: 'Bulwark Rule', + enabled: true, + matchType: 'all', + conditions: [{ field: 'from', comparator: 'contains', value: 'test@example.com' }], + actions: [{ type: 'move', value: 'Archive' }], + stopProcessing: false, + ...overrides, + }; +} + +describe('external rule preservation (issue #201)', () => { + describe('parser — external rule recognition', () => { + it('parses a Roundcube-style rule with "# rule:[Name]" comment', () => { + const script = `require ["fileinto"];\n\n# rule:[Archive Newsletters]\nif header :contains "List-Id" "news" {\n fileinto "Newsletters";\n}\n`; + const result = parseScript(script); + + expect(result.isOpaque).toBe(false); + expect(result.rules).toHaveLength(1); + const rule = result.rules[0]; + expect(rule.origin).toBe('external'); + expect(rule.originLabel).toBe('Roundcube'); + expect(rule.name).toBe('Archive Newsletters'); + expect(rule.conditions[0]).toMatchObject({ + field: 'header', + comparator: 'contains', + value: 'news', + headerName: 'List-Id', + }); + expect(rule.actions[0]).toEqual({ type: 'move', value: 'Newsletters' }); + }); + + it('labels rules near a Nextcloud marker comment', () => { + const script = `require ["fileinto"];\n\n# Nextcloud Mail - begin\nif header :contains "Subject" "invoice" {\n fileinto "Finance";\n}\n# Nextcloud Mail - end\n`; + const result = parseScript(script); + expect(result.rules[0].originLabel).toBe('Nextcloud'); + }); + + it('falls back to "External" label when no known marker is present', () => { + const script = `require ["fileinto"];\n\nif header :contains "From" "boss@corp.com" {\n fileinto "Important";\n}\n`; + const result = parseScript(script); + expect(result.rules[0].originLabel).toBe('External'); + }); + + it('parses anyof/allof conditions in external rules', () => { + const script = `require ["fileinto"];\n\nif anyof(header :contains "From" "a@x.com", header :contains "From" "b@x.com") {\n fileinto "VIP";\n}\n`; + const result = parseScript(script); + expect(result.rules[0].matchType).toBe('any'); + expect(result.rules[0].conditions).toHaveLength(2); + }); + + it('parses negated conditions (not header :is)', () => { + const script = `if not header :is "From" "spam@x.com" {\n keep;\n}\n`; + const result = parseScript(script); + expect(result.rules[0].conditions[0]).toMatchObject({ + field: 'from', + comparator: 'not_is', + value: 'spam@x.com', + }); + }); + + it('marks unrecognized blocks as opaque but preserves their raw text', () => { + const script = `require ["relational"];\n\nif header :value "ge" :comparator "i;ascii-numeric" "X-Priority" ["3"] {\n keep;\n}\n`; + const result = parseScript(script); + expect(result.isOpaque).toBe(false); + const rule = result.rules[0]; + expect(rule.origin).toBe('opaque'); + expect(rule.rawBlock).toContain('if header :value'); + }); + + it('collects all external require tokens', () => { + const script = `require ["fileinto", "imap4flags", "body"];\n\nif header :is "Subject" "hi" { fileinto "A"; }\n`; + const result = parseScript(script); + expect(result.externalRequires).toEqual(expect.arrayContaining(['fileinto', 'imap4flags', 'body'])); + }); + }); + + describe('parser — mixed Bulwark + external', () => { + it('returns Bulwark rules from metadata and external rules from the rest', () => { + const bulwark = [makeBulwarkRule({ name: 'Bulwark A' })]; + const bulwarkScript = generateScript(bulwark); + const mixedScript = `${bulwarkScript}\n# External appended by Nextcloud\nif header :contains "List-Id" "devs" {\n fileinto "Dev";\n}\n`; + + const result = parseScript(mixedScript); + expect(result.isOpaque).toBe(false); + expect(result.rules.length).toBeGreaterThanOrEqual(2); + + const bulwarkParsed = result.rules.filter(r => !r.origin || r.origin === 'bulwark'); + const externalParsed = result.rules.filter(r => r.origin === 'external'); + expect(bulwarkParsed).toHaveLength(1); + expect(bulwarkParsed[0].name).toBe('Bulwark A'); + expect(externalParsed).toHaveLength(1); + expect(externalParsed[0].originLabel).toBe('Nextcloud'); + }); + + it('does not return Bulwark-emitted if-blocks as external duplicates', () => { + const bulwark = [makeBulwarkRule({ name: 'My Bulwark Rule' })]; + const script = generateScript(bulwark); + const result = parseScript(script); + + // Only the metadata-sourced rule, no duplicate "external" entry. + expect(result.rules).toHaveLength(1); + expect(result.rules[0].origin).toBeUndefined(); + }); + }); + + describe('generator — external splice', () => { + it('appends external rawBlocks verbatim after Bulwark-managed output', () => { + const externalRule: FilterRule = { + id: 'ext-0', + name: 'External', + enabled: true, + matchType: 'all', + conditions: [{ field: 'header', comparator: 'contains', value: 'x', headerName: 'List-Id' }], + actions: [{ type: 'move', value: 'Lists' }], + stopProcessing: false, + origin: 'external', + originLabel: 'Nextcloud', + rawBlock: '# Nextcloud Mail\nif header :contains "List-Id" "x" {\n fileinto "Lists";\n}\n', + }; + const rules: FilterRule[] = [makeBulwarkRule(), externalRule]; + const script = generateScript(rules); + + expect(script).toContain('# Rule: Bulwark Rule'); + expect(script).toContain('# Nextcloud Mail'); + expect(script).toContain('# --- External rules (managed outside Bulwark) ---'); + const bulwarkIdx = script.indexOf('# Rule: Bulwark Rule'); + const externalIdx = script.indexOf('# Nextcloud Mail'); + expect(bulwarkIdx).toBeLessThan(externalIdx); + }); + + it('unions external requires into the top-level require line', () => { + const script = generateScript([makeBulwarkRule()], undefined, { + externalRequires: ['fileinto', 'imap4flags', 'body'], + }); + const requireLine = script.split('\n').find(l => l.startsWith('require'))!; + expect(requireLine).toContain('"fileinto"'); + expect(requireLine).toContain('"imap4flags"'); + expect(requireLine).toContain('"body"'); + }); + + it('strips origin/rawBlock/originLabel from Bulwark rules when writing metadata', () => { + const bulwarkWithJunk: FilterRule = { + ...makeBulwarkRule(), + origin: 'bulwark', + originLabel: 'shouldnotbehere', + rawBlock: 'shouldnotbehere', + }; + const script = generateScript([bulwarkWithJunk]); + const match = script.match(/@metadata:begin\n(.*)\n@metadata:end/); + const metadata = JSON.parse(match![1]); + expect(metadata.rules[0]).not.toHaveProperty('origin'); + expect(metadata.rules[0]).not.toHaveProperty('originLabel'); + expect(metadata.rules[0]).not.toHaveProperty('rawBlock'); + }); + + it('never writes external rules into metadata', () => { + const ext: FilterRule = { + id: 'ext-0', + name: 'Ext', + enabled: true, + matchType: 'all', + conditions: [{ field: 'from', comparator: 'is', value: 'x@y' }], + actions: [{ type: 'keep' }], + stopProcessing: false, + origin: 'external', + rawBlock: '# ext\nif header :is "From" "x@y" { keep; }', + }; + const script = generateScript([makeBulwarkRule(), ext]); + const match = script.match(/@metadata:begin\n(.*)\n@metadata:end/); + const metadata = JSON.parse(match![1]); + expect(metadata.rules).toHaveLength(1); + expect(metadata.rules[0].name).toBe('Bulwark Rule'); + }); + }); + + describe('fixture: mixed-origins.sieve', () => { + const fixture = readFileSync( + join(__dirname, 'fixtures', 'mixed-origins.sieve'), + 'utf-8', + ); + + it('identifies Bulwark, Roundcube, Nextcloud, External, and opaque rules', () => { + const result = parseScript(fixture); + + expect(result.isOpaque).toBe(false); + expect(result.vacation).toBeUndefined(); + + const byOrigin = { + bulwark: result.rules.filter(r => !r.origin || r.origin === 'bulwark'), + external: result.rules.filter(r => r.origin === 'external'), + opaque: result.rules.filter(r => r.origin === 'opaque'), + }; + + expect(byOrigin.bulwark).toHaveLength(2); + expect(byOrigin.external.length).toBeGreaterThanOrEqual(3); + expect(byOrigin.opaque).toHaveLength(1); + + const labels = byOrigin.external.map(r => r.originLabel); + expect(labels).toContain('Roundcube'); + expect(labels).toContain('Nextcloud'); + expect(labels).toContain('External'); + + const opaqueRule = byOrigin.opaque[0]; + expect(opaqueRule.rawBlock).toContain(':comparator "i;ascii-numeric"'); + }); + + it('preserves unknown-Sieve content through save round-trip', () => { + const parsed = parseScript(fixture); + const regenerated = generateScript(parsed.rules, parsed.vacation, { + externalRequires: parsed.externalRequires, + }); + + // The unparseable construct must appear verbatim in the regenerated script. + expect(regenerated).toContain(':comparator "i;ascii-numeric"'); + // Require tokens from the external content are preserved. + expect(regenerated).toContain('"relational"'); + // Bulwark rules are still present. + expect(regenerated).toContain('# Rule: Archive newsletters'); + }); + }); + + describe('round-trip', () => { + it('preserves external rules through parse → generate → parse', () => { + const initial = `require ["fileinto", "imap4flags"];\n\n# rule:[VIP]\nif header :contains "From" "boss@company.com" {\n fileinto "VIP";\n addflag "\\\\Flagged";\n}\n\n# Nextcloud Mail - begin\nif header :contains "Subject" "invoice" {\n fileinto "Finance";\n}\n# Nextcloud Mail - end\n`; + + const firstParse = parseScript(initial); + expect(firstParse.rules).toHaveLength(2); + + const regenerated = generateScript(firstParse.rules, firstParse.vacation, { + externalRequires: firstParse.externalRequires, + }); + const secondParse = parseScript(regenerated); + + expect(secondParse.rules).toHaveLength(2); + const names = secondParse.rules.map(r => r.name).sort(); + expect(names).toContain('VIP'); + }); + + it('does not destroy external rules when Bulwark regenerates after an edit', () => { + const initial = `${generateScript([makeBulwarkRule({ name: 'Mine' })])}\n# rule:[Untouchable]\nif header :is "X-Spam" "yes" {\n discard;\n}\n`; + + const parsed = parseScript(initial); + const externalBefore = parsed.rules.filter(r => r.origin === 'external'); + expect(externalBefore).toHaveLength(1); + + // Simulate a user edit — update the Bulwark rule name + const edited = parsed.rules.map(r => (r.origin === 'external' || r.origin === 'opaque' ? r : { ...r, name: 'Mine (edited)' })); + const regenerated = generateScript(edited, parsed.vacation, { externalRequires: parsed.externalRequires }); + const reparsed = parseScript(regenerated); + + const externalAfter = reparsed.rules.filter(r => r.origin === 'external'); + expect(externalAfter).toHaveLength(1); + expect(externalAfter[0].name).toBe('Untouchable'); + expect(externalAfter[0].conditions[0]).toMatchObject({ field: 'header', headerName: 'X-Spam' }); + }); + }); +}); diff --git a/lib/sieve/__tests__/fixtures/mixed-origins.sieve b/lib/sieve/__tests__/fixtures/mixed-origins.sieve new file mode 100644 index 00000000..74195eaa --- /dev/null +++ b/lib/sieve/__tests__/fixtures/mixed-origins.sieve @@ -0,0 +1,44 @@ +/* @metadata:begin +{"version":1,"rules":[{"id":"bw-news","name":"Archive newsletters","enabled":true,"matchType":"any","conditions":[{"field":"header","comparator":"contains","value":"unsubscribe","headerName":"List-Unsubscribe"},{"field":"from","comparator":"contains","value":"newsletter@"}],"actions":[{"type":"move","value":"Newsletters"}],"stopProcessing":false},{"id":"bw-vip","name":"Flag VIP senders","enabled":true,"matchType":"any","conditions":[{"field":"from","comparator":"is","value":"ceo@company.com"},{"field":"from","comparator":"is","value":"board@company.com"}],"actions":[{"type":"star"},{"type":"mark_read"}],"stopProcessing":false}]} +@metadata:end */ + +require ["body", "copy", "fileinto", "imap4flags", "relational"]; + +# Rule: Archive newsletters +if anyof(header :contains "List-Unsubscribe" "unsubscribe", header :contains "From" "newsletter@") { + fileinto "Newsletters"; +} + +# Rule: Flag VIP senders +if anyof(header :is "From" "ceo@company.com", header :is "From" "board@company.com") { + addflag "\\Flagged"; + addflag "\\Seen"; +} + +# --- External rules (managed outside Bulwark) --- + +# rule:[Finance — auto-file invoices] +if allof(header :contains "From" "billing@", header :contains "Subject" "invoice") { + fileinto :copy "Finance/Invoices"; + keep; +} + +# Nextcloud Mail - begin +# Filter installed by Nextcloud Mail app +if header :contains "Subject" "[Support]" { + fileinto "Support"; +} +# Nextcloud Mail - end + +# A handwritten rule without a tool-specific marker. +# Bulwark should recognize this as generic "External" and preserve it. +if not header :is "X-Spam-Status" "No" { + fileinto "Junk"; +} + +# A rule using a Sieve construct Bulwark's visual editor does not understand. +# It must survive round-trips verbatim, shown to the user as read-only. +if header :value "ge" :comparator "i;ascii-numeric" "X-Priority" ["3"] { + fileinto "LowPriority"; + stop; +} diff --git a/lib/sieve/__tests__/parser.test.ts b/lib/sieve/__tests__/parser.test.ts index b8412be1..3d6af548 100644 --- a/lib/sieve/__tests__/parser.test.ts +++ b/lib/sieve/__tests__/parser.test.ts @@ -25,10 +25,14 @@ describe('parseScript', () => { expect(result.rules).toEqual(rules); }); - it('returns isOpaque for missing metadata', () => { + it('parses external rules when no Bulwark metadata is present', () => { const result = parseScript('require ["fileinto"];\nif header :contains "From" "x" { fileinto "Y"; }'); - expect(result.isOpaque).toBe(true); - expect(result.rules).toEqual([]); + expect(result.isOpaque).toBe(false); + expect(result.rules).toHaveLength(1); + expect(result.rules[0].origin).toBe('external'); + expect(result.rules[0].conditions[0]).toMatchObject({ field: 'from', comparator: 'contains', value: 'x' }); + expect(result.rules[0].actions[0]).toEqual({ type: 'move', value: 'Y' }); + expect(result.externalRequires).toContain('fileinto'); }); it('returns isOpaque for corrupted JSON', () => { @@ -90,9 +94,10 @@ describe('parseScript', () => { expect(result.isOpaque).toBe(true); }); - it('returns isOpaque for empty string', () => { + it('treats an empty string as an empty, editable script (not opaque)', () => { const result = parseScript(''); - expect(result.isOpaque).toBe(true); + expect(result.isOpaque).toBe(false); + expect(result.rules).toEqual([]); }); describe('round-trip', () => { diff --git a/lib/sieve/generator.ts b/lib/sieve/generator.ts index 22beab57..996fd841 100644 --- a/lib/sieve/generator.ts +++ b/lib/sieve/generator.ts @@ -111,11 +111,47 @@ function computeRequires(rules: FilterRule[], vacation?: VacationSieveConfig): s } } - return [...extensions].sort(); + return [...extensions]; } -export function generateScript(rules: FilterRule[], vacation?: VacationSieveConfig): string { - const metadata: FilterMetadata = { version: 1, rules }; +function stripRuleForMetadata(r: FilterRule): Omit { + return { + id: r.id, + name: r.name, + enabled: r.enabled, + matchType: r.matchType, + conditions: r.conditions, + actions: r.actions, + stopProcessing: r.stopProcessing, + }; +} + +export interface GenerateOptions { + /** + * Require extensions used by external (non-Bulwark) rules that we must + * preserve in the top-level `require` directive. Duplicates with Bulwark's + * own requires are deduplicated. + */ + externalRequires?: string[]; +} + +export function generateScript( + rules: FilterRule[], + vacation?: VacationSieveConfig, + options: GenerateOptions = {}, +): string { + // Partition rules by origin. Treat missing origin as 'bulwark' for back-compat. + const bulwarkRules: FilterRule[] = []; + const externalRules: FilterRule[] = []; + for (const r of rules) { + if (r.origin && r.origin !== 'bulwark') externalRules.push(r); + else bulwarkRules.push(r); + } + + const metadata: FilterMetadata = { + version: 1, + rules: bulwarkRules.map(stripRuleForMetadata) as FilterRule[], + }; if (vacation?.isEnabled) { metadata.vacation = vacation; } @@ -127,9 +163,12 @@ export function generateScript(rules: FilterRule[], vacation?: VacationSieveConf lines.push('@metadata:end */'); lines.push(''); - const requires = computeRequires(rules, vacation); - if (requires.length > 0) { - lines.push(`require [${requires.map(r => `"${r}"`).join(', ')}];`); + const bulwarkRequires = computeRequires(bulwarkRules, vacation); + const externalRequires = options.externalRequires ?? []; + const allRequires = [...new Set([...bulwarkRequires, ...externalRequires])].sort(); + + if (allRequires.length > 0) { + lines.push(`require [${allRequires.map(r => `"${r}"`).join(', ')}];`); } if (vacation?.isEnabled) { @@ -143,9 +182,9 @@ export function generateScript(rules: FilterRule[], vacation?: VacationSieveConf lines.push(`vacation ${vacationParts.join(' ')};`); } - const enabledRules = rules.filter(r => r.enabled); + const enabledBulwarkRules = bulwarkRules.filter(r => r.enabled); - for (const rule of enabledRules) { + for (const rule of enabledBulwarkRules) { if (rule.conditions.length === 0 || rule.actions.length === 0) { debug.warn('filters', `Skipping rule "${rule.name}": empty conditions or actions`); continue; @@ -182,6 +221,17 @@ export function generateScript(rules: FilterRule[], vacation?: VacationSieveConf 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) { + 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(''); return lines.join('\n'); } diff --git a/lib/sieve/parser.ts b/lib/sieve/parser.ts index a2533809..a1845926 100644 --- a/lib/sieve/parser.ts +++ b/lib/sieve/parser.ts @@ -1,17 +1,33 @@ -import type { FilterRule, FilterMetadata, VacationSieveConfig } from '@/lib/jmap/sieve-types'; +import type { + FilterAction, + FilterCondition, + FilterComparator, + FilterConditionField, + FilterMetadata, + FilterRule, + VacationSieveConfig, +} from '@/lib/jmap/sieve-types'; import { debug } from '@/lib/debug'; export interface ParseResult { rules: FilterRule[]; isOpaque: boolean; vacation?: VacationSieveConfig; + externalRequires: string[]; } -const OPAQUE: ParseResult = { rules: [], isOpaque: true }; +const OPAQUE: ParseResult = { rules: [], isOpaque: true, externalRequires: [] }; const METADATA_BEGIN = '/* @metadata:begin'; const METADATA_END = '@metadata:end */'; +const FIELD_FROM_HEADER: Record = { + from: 'from', + to: 'to', + cc: 'cc', + subject: 'subject', +}; + function isValidCondition(c: unknown): boolean { if (!c || typeof c !== 'object') return false; const cond = c as Record; @@ -42,83 +58,556 @@ function isValidRule(rule: unknown): rule is FilterRule { /** * Detect Stalwart-generated vacation-only scripts (no metadata). - * These contain `vacation` command but no other filter logic we need to preserve. */ function detectVacationOnlyScript(content: string): ParseResult | null { - // Must contain a vacation command if (!/\bvacation\b/.test(content)) return null; - // Strip requires, comments, and whitespace to see if only vacation remains const stripped = content .replace(/^\s*require\s+\[[^\]]*\]\s*;/gm, '') .replace(/#[^\n]*/g, '') .replace(/\/\*[\s\S]*?\*\//g, '') .trim(); - // Strip quoted string *contents* before checking for structural keywords so that - // message body text like "if you need urgent help..." doesn't cause false rejection. const structural = stripped.replace(/"(?:[^"\\]|\\.)*"/g, '""'); - // Check there are no if/elsif/else filter blocks if (/\b(?:if|elsif|else)\b/.test(structural)) return null; - - // Must still have a vacation command after stripping boilerplate if (!/\bvacation\b/.test(structural)) return null; - // Extract subject if present (:subject "...") const subjectMatch = stripped.match(/:subject\s+"((?:[^"\\]|\\.)*)"/); - const subject = subjectMatch ? subjectMatch[1].replace(/\\"/g, '"').replace(/\\\\/g, '\\') : ''; + const subject = subjectMatch ? unescapeSieveString(subjectMatch[1]) : ''; - // Extract the body text. Stalwart uses :mime format where the body is a full MIME - // message. Extract the plain text after the Content-Transfer-Encoding header. - // Handle both LF and CRLF line endings. let textBody = ''; const mimeBodyMatch = stripped.match(/Content-Transfer-Encoding:[^\r\n]*\r?\n\r?\n([\s\S]*?)"[\s\S]*?;/); if (mimeBodyMatch) { textBody = mimeBodyMatch[1].trim(); } else { - // Plain format: last quoted string argument in the vacation statement const allQuoted = [...stripped.matchAll(/"((?:[^"\\]|\\.)*)"/g)]; const last = allQuoted[allQuoted.length - 1]; - if (last) { - textBody = last[1].replace(/\\"/g, '"').replace(/\\\\/g, '\\'); - } + if (last) textBody = unescapeSieveString(last[1]); } return { rules: [], isOpaque: false, vacation: { isEnabled: true, subject, textBody }, + externalRequires: [], }; } +function unescapeSieveString(s: string): string { + return s.replace(/\\(.)/g, '$1'); +} + +function skipStringLit(s: string, i: number): number { + i++; + while (i < s.length) { + if (s[i] === '\\') { i += 2; continue; } + if (s[i] === '"') return i + 1; + i++; + } + return i; +} + +function skipHashComment(s: string, i: number): number { + while (i < s.length && s[i] !== '\n') i++; + return i; +} + +function skipBlockComment(s: string, i: number): number { + const end = s.indexOf('*/', i + 2); + return end === -1 ? s.length : end + 2; +} + +function skipStatement(s: string, i: number): number { + while (i < s.length) { + const c = s[i]; + if (c === '"') { i = skipStringLit(s, i); continue; } + if (c === '#') { i = skipHashComment(s, i); continue; } + if (c === '/' && s[i + 1] === '*') { i = skipBlockComment(s, i); continue; } + if (c === ';') return i + 1; + i++; + } + return i; +} + +function skipBalanced(s: string, i: number, open: string, close: string): number { + let depth = 0; + while (i < s.length) { + const c = s[i]; + if (c === '"') { i = skipStringLit(s, i); continue; } + if (c === '#') { i = skipHashComment(s, i); continue; } + if (c === '/' && s[i + 1] === '*') { i = skipBlockComment(s, i); continue; } + if (c === open) { depth++; i++; continue; } + if (c === close) { + depth--; + i++; + if (depth === 0) return i; + continue; + } + i++; + } + return i; +} + +function skipIfStatement(s: string, i: number): number { + // positioned after 'if' keyword; skip through condition expression and body braces + while (i < s.length && s[i] !== '{') { + const c = s[i]; + if (c === '"') { i = skipStringLit(s, i); continue; } + if (c === '(') { i = skipBalanced(s, i, '(', ')'); continue; } + if (c === '#') { i = skipHashComment(s, i); continue; } + if (c === '/' && s[i + 1] === '*') { i = skipBlockComment(s, i); continue; } + i++; + } + if (i >= s.length) return i; + return skipBalanced(s, i, '{', '}'); +} + +interface TopBlock { + kind: 'require' | 'if' | 'vacation' | 'other'; + raw: string; // from start-of-leading-text to end of statement + statement: string; // the statement itself (no leading comments/whitespace) + startIdx: number; + endIdx: number; +} + +function scanTopLevel(content: string): TopBlock[] { + const blocks: TopBlock[] = []; + let i = 0; + let segmentStart = 0; + + const consume = (kind: TopBlock['kind'], stmtStart: number, stmtEnd: number) => { + blocks.push({ + kind, + raw: content.slice(segmentStart, stmtEnd), + statement: content.slice(stmtStart, stmtEnd), + startIdx: segmentStart, + endIdx: stmtEnd, + }); + segmentStart = stmtEnd; + }; + + while (i < content.length) { + // Skip whitespace + while (i < content.length && /\s/.test(content[i])) i++; + if (i >= content.length) break; + + const c = content[i]; + + // Comments (stay attached to next block as leading text) + if (c === '#') { i = skipHashComment(content, i); continue; } + if (c === '/' && content[i + 1] === '*') { i = skipBlockComment(content, i); continue; } + + // Identifier + const m = /^[a-zA-Z_][a-zA-Z0-9_]*/.exec(content.slice(i)); + if (!m) { i++; continue; } + + const ident = m[0]; + const stmtStart = i; + i += ident.length; + + if (ident === 'require') { + i = skipStatement(content, i); + consume('require', stmtStart, i); + } else if (ident === 'if') { + i = skipIfStatement(content, i); + consume('if', stmtStart, i); + } else if (ident === 'vacation') { + i = skipStatement(content, i); + consume('vacation', stmtStart, i); + } else { + i = skipStatement(content, i); + consume('other', stmtStart, i); + } + } + + return blocks; +} + +function extractRequireTokens(stmt: string): string[] { + const mList = /require\s+\[([\s\S]*?)\]\s*;/.exec(stmt); + if (mList) return [...mList[1].matchAll(/"([^"]+)"/g)].map(x => x[1]); + const mSingle = /require\s+"([^"]+)"\s*;/.exec(stmt); + return mSingle ? [mSingle[1]] : []; +} + +/** + * Extract the last contiguous block of comments immediately preceding a + * statement — comments separated from the statement by a blank line are not + * considered its leading commentary (they likely belong to the previous + * block, e.g. a trailing "# Nextcloud Mail - end" marker). + */ +function lastCommentChunk(leading: string): string { + const parts = leading.split(/\r?\n\s*\r?\n/).map(s => s.trim()).filter(Boolean); + return parts.length ? parts[parts.length - 1] : ''; +} + +function detectOriginLabel(leading: string): string { + const chunk = lastCommentChunk(leading); + const lower = chunk.toLowerCase(); + if (/rule:\s*\[/i.test(chunk) || /roundcube|managesieve/.test(lower)) return 'Roundcube'; + if (/nextcloud/.test(lower)) return 'Nextcloud'; + if (/horde|ingo/.test(lower)) return 'Horde'; + if (/kolab/.test(lower)) return 'Kolab'; + if (/dovecot/.test(lower)) return 'Dovecot'; + if (/thunderbird/.test(lower)) return 'Thunderbird'; + return 'External'; +} + +function extractName(leading: string, fallback: string): string { + // Roundcube: "# rule:[Name]" + const rc = leading.match(/#\s*rule:\s*\[([^\]]+)\]/i); + if (rc) return rc[1].trim(); + + // "# Rule: Name" + const rr = leading.match(/#\s*Rule:\s*(.+?)\s*$/mi); + if (rr) return rr[1].trim(); + + // Last non-empty trimmed comment line + const lines = leading.split('\n').map(l => l.replace(/^\s*#\s*/, '').trim()).filter(Boolean); + const last = lines[lines.length - 1]; + if (last && last.length <= 80 && !/^\/\*|\*\/$/.test(last)) return last; + + return fallback; +} + +function splitTopLevelComma(s: string): string[] { + const parts: string[] = []; + let depth = 0; + let start = 0; + let i = 0; + while (i < s.length) { + const c = s[i]; + if (c === '"') { i = skipStringLit(s, i); continue; } + if (c === '(' || c === '[' || c === '{') { depth++; i++; continue; } + if (c === ')' || c === ']' || c === '}') { depth--; i++; continue; } + if (c === ',' && depth === 0) { + parts.push(s.slice(start, i)); + start = i + 1; + } + i++; + } + parts.push(s.slice(start)); + return parts.map(p => p.trim()).filter(Boolean); +} + +function splitStatements(body: string): string[] { + const stmts: string[] = []; + let start = 0; + let i = 0; + while (i < body.length) { + const c = body[i]; + if (c === '"') { i = skipStringLit(body, i); continue; } + if (c === '#') { i = skipHashComment(body, i); continue; } + if (c === '/' && body[i + 1] === '*') { i = skipBlockComment(body, i); continue; } + if (c === ';') { + stmts.push(body.slice(start, i)); + start = i + 1; + } + i++; + } + const tail = body.slice(start).trim(); + if (tail) stmts.push(tail); + return stmts.map(s => s.trim()).filter(Boolean); +} + +function normalizeHeaderName(name: string): { field: FilterConditionField; headerName?: string } { + const lc = name.toLowerCase(); + if (FIELD_FROM_HEADER[lc]) return { field: FIELD_FROM_HEADER[lc] }; + return { field: 'header', headerName: name }; +} + +function parseAtom(raw: string): FilterCondition | null { + let s = raw.trim(); + let negated = false; + + if (/^not\b/.test(s)) { + negated = true; + s = s.replace(/^not\s*/, '').trim(); + if (s.startsWith('(') && s.endsWith(')')) { + s = s.slice(1, -1).trim(); + } + } + + let m = /^header\s+:(contains|is|matches)\s+"((?:[^"\\]|\\.)*)"\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) { + const [, tag, headerName, rawValue] = m; + const value = unescapeSieveString(rawValue); + const { field, headerName: customHeaderName } = normalizeHeaderName(unescapeSieveString(headerName)); + + let comparator: FilterComparator; + if (tag === 'contains') { + comparator = negated ? 'not_contains' : 'contains'; + } else if (tag === 'is') { + comparator = negated ? 'not_is' : 'is'; + } else { + // :matches — distinguish starts_with / ends_with / matches + const starPositions = [...value].reduce((acc, ch, idx) => (ch === '*' ? [...acc, idx] : acc), []); + if (starPositions.length === 1 && starPositions[0] === value.length - 1) { + comparator = 'starts_with'; + const cond: FilterCondition = { field, comparator, value: value.slice(0, -1) }; + if (customHeaderName !== undefined) cond.headerName = customHeaderName; + return cond; + } + if (starPositions.length === 1 && starPositions[0] === 0) { + comparator = 'ends_with'; + const cond: FilterCondition = { field, comparator, value: value.slice(1) }; + if (customHeaderName !== undefined) cond.headerName = customHeaderName; + return cond; + } + comparator = 'matches'; + } + + const cond: FilterCondition = { field, comparator, value }; + if (customHeaderName !== undefined) cond.headerName = customHeaderName; + return cond; + } + + m = /^body\s+:(contains|is)\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) { + return { field: 'body', comparator: m[1] === 'is' ? 'is' : 'contains', value: unescapeSieveString(m[2]) }; + } + + m = /^size\s+:(over|under)\s+(\d+)$/.exec(s); + if (m) { + return { field: 'size', comparator: m[1] === 'over' ? 'greater_than' : 'less_than', value: m[2] }; + } + + return null; +} + +function parseCondition(raw: string): { matchType: 'all' | 'any'; conditions: FilterCondition[] } | null { + const s = raw.trim(); + if (!s) return null; + + const allMatch = /^allof\s*\(([\s\S]*)\)$/.exec(s); + const anyMatch = /^anyof\s*\(([\s\S]*)\)$/.exec(s); + let matchType: 'all' | 'any' = 'all'; + let inner: string; + if (allMatch) { matchType = 'all'; inner = allMatch[1]; } + else if (anyMatch) { matchType = 'any'; inner = anyMatch[1]; } + else inner = s; + + const parts = splitTopLevelComma(inner); + const conditions: FilterCondition[] = []; + for (const part of parts) { + const atom = parseAtom(part); + if (!atom) return null; + conditions.push(atom); + } + return { matchType, conditions }; +} + +function parseAction(raw: string): FilterAction | null { + const s = raw.trim(); + + let m = /^fileinto\s+:copy\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) return { type: 'copy', value: unescapeSieveString(m[1]) }; + + m = /^fileinto\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) return { type: 'move', value: unescapeSieveString(m[1]) }; + + m = /^redirect\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) return { type: 'forward', value: unescapeSieveString(m[1]) }; + + m = /^addflag\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) { + const flag = unescapeSieveString(m[1]); + if (flag === '\\Seen') return { type: 'mark_read' }; + if (flag === '\\Flagged') return { type: 'star' }; + if (flag.startsWith('$label:')) return { type: 'add_label', value: flag.slice('$label:'.length) }; + return null; + } + + m = /^reject\s+"((?:[^"\\]|\\.)*)"$/.exec(s); + if (m) return { type: 'reject', value: unescapeSieveString(m[1]) }; + + if (/^discard$/.test(s)) return { type: 'discard' }; + if (/^keep$/.test(s)) return { type: 'keep' }; + if (/^stop$/.test(s)) return { type: 'stop' }; + + return null; +} + +function parseIfBlockToRule(block: TopBlock, idPrefix: string, index: number): FilterRule | null { + const stmt = block.statement; + const afterIf = stmt.replace(/^if\s+/, ''); + const braceIdx = afterIf.indexOf('{'); + const lastBraceIdx = afterIf.lastIndexOf('}'); + if (braceIdx === -1 || lastBraceIdx === -1 || lastBraceIdx < braceIdx) return null; + + const condStr = afterIf.slice(0, braceIdx).trim(); + const bodyStr = afterIf.slice(braceIdx + 1, lastBraceIdx).trim(); + + const cond = parseCondition(condStr); + if (!cond || cond.conditions.length === 0) return null; + + const actionStmts = splitStatements(bodyStr); + const actions: FilterAction[] = []; + for (const st of actionStmts) { + const a = parseAction(st); + if (!a) return null; + actions.push(a); + } + if (actions.length === 0) return null; + + let stopProcessing = false; + if (actions.length > 0 && actions[actions.length - 1].type === 'stop') { + const hasNonStop = actions.some(a => a.type !== 'stop'); + if (hasNonStop) { + stopProcessing = true; + actions.pop(); + } + } + + const leading = block.raw.slice(0, block.statement ? block.raw.length - block.statement.length : 0); + const originLabel = detectOriginLabel(leading); + const name = extractName(leading, `Rule ${index + 1}`); + + return { + id: `${idPrefix}-${index}`, + name, + enabled: true, + matchType: cond.matchType, + conditions: cond.conditions, + actions, + stopProcessing, + origin: 'external', + originLabel, + rawBlock: block.raw, + }; +} + +function makeOpaqueRule(block: TopBlock, idPrefix: string, index: number): FilterRule { + const leading = block.raw.slice(0, block.raw.length - block.statement.length); + const originLabel = detectOriginLabel(leading); + const name = extractName(leading, `External rule ${index + 1}`); + return { + id: `${idPrefix}-${index}`, + name, + enabled: true, + matchType: 'all', + conditions: [], + actions: [], + stopProcessing: false, + origin: 'opaque', + originLabel, + rawBlock: block.raw, + }; +} + +function parseExternalRules( + content: string, + idPrefix: string, +): { rules: FilterRule[]; externalRequires: string[]; hasContent: boolean } { + const blocks = scanTopLevel(content); + const rules: FilterRule[] = []; + const externalRequires: string[] = []; + let index = 0; + let sawAnyStatement = false; + + for (const block of blocks) { + if (block.kind === 'require') { + sawAnyStatement = true; + for (const tok of extractRequireTokens(block.statement)) { + if (!externalRequires.includes(tok)) externalRequires.push(tok); + } + continue; + } + + if (block.kind === 'if') { + sawAnyStatement = true; + const rule = parseIfBlockToRule(block, idPrefix, index); + rules.push(rule ?? makeOpaqueRule(block, idPrefix, index)); + index++; + continue; + } + + // vacation/other: treat as opaque preserved block + sawAnyStatement = true; + rules.push(makeOpaqueRule(block, idPrefix, index)); + index++; + } + + return { rules, externalRequires, hasContent: sawAnyStatement }; +} + export function parseScript(content: string): ParseResult { const beginIdx = content.indexOf(METADATA_BEGIN); - if (beginIdx === -1) { - // No metadata — check if it's a Stalwart vacation-only script - return detectVacationOnlyScript(content) || OPAQUE; + + if (beginIdx !== -1) { + const endIdx = content.indexOf(METADATA_END, beginIdx); + if (endIdx === -1) return OPAQUE; + + const jsonStart = beginIdx + METADATA_BEGIN.length; + const jsonStr = content.slice(jsonStart, endIdx).trim(); + + let metadata: FilterMetadata; + try { + metadata = JSON.parse(jsonStr); + } catch (e) { + debug.warn('filters', 'Failed to parse Sieve metadata JSON:', e); + return OPAQUE; + } + + if (!metadata || metadata.version !== 1) return OPAQUE; + if (!Array.isArray(metadata.rules)) return OPAQUE; + + for (const rule of metadata.rules) { + if (!isValidRule(rule)) return OPAQUE; + } + + // Scan the portion AFTER the metadata block for external rules. + const afterMetadata = content.slice(endIdx + METADATA_END.length); + const external = parseExternalRules(afterMetadata, 'ext'); + + // Parsed bulwark rules intentionally omit an explicit `origin` field so + // round-trip equality with metadata-only callers holds. Absence of origin + // is treated as 'bulwark' everywhere downstream. + const bulwarkRules: FilterRule[] = metadata.rules; + + // Exclude requires and the vacation line that we emit ourselves from externalRequires. + const externalRequires = external.externalRequires; + + // Drop any external "rules" that are really the bulwark-managed if-blocks or vacation. + // Recognizable by the leading comment "# Rule: " or "# Vacation auto-reply". + const filteredExternal = external.rules.filter(r => { + const raw = r.rawBlock || ''; + if (/#\s*Rule:\s*/.test(raw) && r.origin === 'external') { + // If the name matches a bulwark rule name exactly, treat as bulwark-emitted + const match = raw.match(/#\s*Rule:\s*(.+?)\s*$/m); + const name = match ? match[1].trim() : ''; + if (bulwarkRules.some(b => b.name === name)) return false; + } + if (/#\s*Vacation auto-reply/i.test(raw)) return false; + return true; + }); + + return { + rules: [...bulwarkRules, ...filteredExternal], + isOpaque: false, + vacation: metadata.vacation, + externalRequires, + }; } - const endIdx = content.indexOf(METADATA_END, beginIdx); - if (endIdx === -1) return OPAQUE; + // No metadata — check vacation-only first + const vacationOnly = detectVacationOnlyScript(content); + if (vacationOnly) return vacationOnly; - const jsonStart = beginIdx + METADATA_BEGIN.length; - const jsonStr = content.slice(jsonStart, endIdx).trim(); + // Try to parse the whole script as external rules. + const external = parseExternalRules(content, 'ext'); - let metadata: FilterMetadata; - try { - metadata = JSON.parse(jsonStr); - } catch (e) { - debug.warn('filters', 'Failed to parse Sieve metadata JSON:', e); - return OPAQUE; + if (!external.hasContent) { + // Entirely empty or whitespace/comments only — treat as empty, editable. + return { rules: [], isOpaque: false, externalRequires: [] }; } - if (!metadata || metadata.version !== 1) return OPAQUE; - if (!Array.isArray(metadata.rules)) return OPAQUE; - - for (const rule of metadata.rules) { - if (!isValidRule(rule)) return OPAQUE; + // If at least one block parsed into a structured rule, expose them as external. + const anyParsed = external.rules.some(r => r.origin === 'external'); + if (anyParsed || external.rules.length > 0) { + return { rules: external.rules, isOpaque: false, externalRequires: external.externalRequires }; } - return { rules: metadata.rules, isOpaque: false, vacation: metadata.vacation }; + return OPAQUE; } diff --git a/locales/en/common.json b/locales/en/common.json index 8f08b43d..9714f915 100644 --- a/locales/en/common.json +++ b/locales/en/common.json @@ -1416,7 +1416,9 @@ "rule_summary": { "conditions_count": "{count, plural, one {# condition} other {# conditions}}", "actions_count": "{count, plural, one {# action} other {# actions}}" - } + }, + "origin_external": "External", + "managed_by_tooltip": "Managed by {source}. Edit it in that app, or use the raw Sieve editor." }, "templates": { "title": "Email Templates", diff --git a/stores/__tests__/filter-store.test.ts b/stores/__tests__/filter-store.test.ts index a16fdac1..d5b1c660 100644 --- a/stores/__tests__/filter-store.test.ts +++ b/stores/__tests__/filter-store.test.ts @@ -151,15 +151,26 @@ describe('filter-store', () => { }); describe('fetchFilters', () => { - it('should set isOpaque for scripts without metadata', async () => { + it('parses external rules from scripts without metadata', async () => { const mockClient = { getSieveCapabilities: () => null, getSieveScripts: async () => [{ id: 's1', name: 'main', blobId: 'b1', isActive: true }], getSieveScriptContent: async () => 'require ["fileinto"];\nif header :contains "From" "x" { fileinto "Y"; }', }; await useFilterStore.getState().fetchFilters(mockClient as unknown as IJMAPClient); + expect(useFilterStore.getState().isOpaque).toBe(false); + expect(useFilterStore.getState().rules).toHaveLength(1); + expect(useFilterStore.getState().rules[0].origin).toBe('external'); + }); + + it('sets isOpaque for truly unparseable content', async () => { + const mockClient = { + getSieveCapabilities: () => null, + getSieveScripts: async () => [{ id: 's1', name: 'main', blobId: 'b1', isActive: true }], + getSieveScriptContent: async () => '/* @metadata:begin\n{corrupt\n@metadata:end */', + }; + await useFilterStore.getState().fetchFilters(mockClient as unknown as IJMAPClient); expect(useFilterStore.getState().isOpaque).toBe(true); - expect(useFilterStore.getState().rules).toEqual([]); }); it('should parse rules from metadata-bearing script', async () => { diff --git a/stores/filter-store.ts b/stores/filter-store.ts index ef828356..7d48442b 100644 --- a/stores/filter-store.ts +++ b/stores/filter-store.ts @@ -16,6 +16,7 @@ interface FilterStore { isOpaque: boolean; rawScript: string; vacationSettings: VacationSieveConfig | null; + externalRequires: string[]; setSupported: (supported: boolean) => void; fetchFilters: (client: IJMAPClient) => Promise; @@ -43,6 +44,7 @@ export const useFilterStore = create()((set, get) => ({ isOpaque: false, rawScript: '', vacationSettings: null, + externalRequires: [], setSupported: (supported) => set({ isSupported: supported }), @@ -74,10 +76,22 @@ export const useFilterStore = create()((set, get) => ({ if (result.isOpaque) { debug.log('filters', 'Sieve script is opaque (hand-edited)'); - set({ isLoading: false, isOpaque: true, rules: [], vacationSettings: result.vacation || null }); + set({ + isLoading: false, + isOpaque: true, + rules: [], + vacationSettings: result.vacation || null, + externalRequires: result.externalRequires, + }); } else { debug.log('filters', 'Parsed', result.rules.length, 'filter rules'); - set({ isLoading: false, isOpaque: false, rules: result.rules, vacationSettings: result.vacation || null }); + set({ + isLoading: false, + isOpaque: false, + rules: result.rules, + vacationSettings: result.vacation || null, + externalRequires: result.externalRequires, + }); } } catch (error) { debug.error('Failed to fetch filters:', error); @@ -91,13 +105,13 @@ export const useFilterStore = create()((set, get) => ({ saveFilters: async (client) => { set({ isSaving: true, error: null }); try { - const { isOpaque, rawScript, rules, activeScriptId, vacationSettings } = get(); + const { isOpaque, rawScript, rules, activeScriptId, vacationSettings, externalRequires } = get(); let content: string; if (isOpaque) { content = rawScript; } else { - content = generateScript(rules, vacationSettings || undefined); + content = generateScript(rules, vacationSettings || undefined, { externalRequires }); } if (activeScriptId) { @@ -124,40 +138,60 @@ export const useFilterStore = create()((set, get) => ({ }, addRule: (rule) => { - set((state) => ({ rules: [...state.rules, rule] })); + // Insert new bulwark rules before external/opaque rules so Bulwark's + // managed section stays contiguous. + set((state) => { + const bulwark = state.rules.filter(r => !r.origin || r.origin === 'bulwark'); + const external = state.rules.filter(r => r.origin === 'external' || r.origin === 'opaque'); + return { rules: [...bulwark, rule, ...external] }; + }); }, updateRule: (ruleId, updates) => { set((state) => ({ - rules: state.rules.map(r => r.id === ruleId ? { ...r, ...updates } : r), + rules: state.rules.map(r => { + if (r.id !== ruleId) return r; + if (r.origin === 'external' || r.origin === 'opaque') return r; // read-only + return { ...r, ...updates }; + }), })); }, deleteRule: (ruleId) => { set((state) => ({ - rules: state.rules.filter(r => r.id !== ruleId), + rules: state.rules.filter(r => { + if (r.id !== ruleId) return true; + return r.origin === 'external' || r.origin === 'opaque'; + }), })); }, reorderRules: (ruleIds) => { + // Only reorder bulwark rules; external rules always stay at the end in + // their original order. set((state) => { - const ruleMap = new Map(state.rules.map(r => [r.id, r])); - const reordered = ruleIds.map(id => ruleMap.get(id)).filter(Boolean) as FilterRule[]; - return { rules: reordered }; + const bulwarkMap = new Map( + state.rules.filter(r => !r.origin || r.origin === 'bulwark').map(r => [r.id, r]), + ); + const external = state.rules.filter(r => r.origin === 'external' || r.origin === 'opaque'); + const reordered = ruleIds.map(id => bulwarkMap.get(id)).filter(Boolean) as FilterRule[]; + return { rules: [...reordered, ...external] }; }); }, toggleRule: (ruleId) => { set((state) => ({ - rules: state.rules.map(r => - r.id === ruleId ? { ...r, enabled: !r.enabled } : r - ), + rules: state.rules.map(r => { + if (r.id !== ruleId) return r; + if (r.origin === 'external' || r.origin === 'opaque') return r; // read-only + return { ...r, enabled: !r.enabled }; + }), })); }, setRawScript: (content) => set({ rawScript: content }), - resetToVisualBuilder: () => set({ isOpaque: false, rawScript: '', rules: [] }), + resetToVisualBuilder: () => set({ isOpaque: false, rawScript: '', rules: [], externalRequires: [] }), syncVacationToScript: async (client, vacation) => { try { @@ -173,6 +207,7 @@ export const useFilterStore = create()((set, get) => ({ const activeScript = scripts.find(s => s.isActive) || scripts[0]; let rules = previousRules; + let externalRequires = get().externalRequires; // If there's an active script, try to parse our metadata from it. // If the server overwrote it (no metadata), fall back to stored rules. @@ -181,11 +216,12 @@ export const useFilterStore = create()((set, get) => ({ const parsed = parseScript(content); if (!parsed.isOpaque) { rules = parsed.rules; + externalRequires = parsed.externalRequires; } } // Generate a combined script with our metadata, rules, and vacation - const content = generateScript(rules, vacation.isEnabled ? vacation : undefined); + const content = generateScript(rules, vacation.isEnabled ? vacation : undefined, { externalRequires }); if (activeScript) { // Preserve the script's current activation state — don't pass activate: true @@ -198,6 +234,7 @@ export const useFilterStore = create()((set, get) => ({ rules, vacationSettings: vacation, isOpaque: false, + externalRequires, }); } else { // Don't activate; there may be a server-managed 'vacation' script active. @@ -209,6 +246,7 @@ export const useFilterStore = create()((set, get) => ({ rules, vacationSettings: vacation, isOpaque: false, + externalRequires, }); } @@ -229,5 +267,6 @@ export const useFilterStore = create()((set, get) => ({ isOpaque: false, rawScript: '', vacationSettings: null, + externalRequires: [], }), }));