From f63d739fb34208d6a2653ec4cc811fa76b934d4a Mon Sep 17 00:00:00 2001
From: Linus Rath <139418639+rathlinus@users.noreply.github.com>
Date: Thu, 24 Sep 2026 18:33:25 +0200
Subject: [PATCH] fix: file "Keep" filter rules into the inbox so allow-lists
 beat the spam filter #1027

A bare `keep;` delivers to the default place, which on Stalwart is Junk
for a message its spam filter has already classified, so an allow-list
rule made with "Keep in inbox" changed nothing. The webmail now writes
`fileinto "INBOX";` for it (62465e1e), but saving filters on the phone
wrote `keep;` again and undid that.

The action stays `keep` in the metadata, so scripts written by either
client read back as "Keep in inbox" and save unchanged.
---
 src/lib/sieve/__tests__/generator.test.ts | 13 ++++++---
 src/lib/sieve/__tests__/parser.test.ts    | 32 +++++++++++++++++++++++
 src/lib/sieve/generator.ts                |  7 ++++-
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/src/lib/sieve/__tests__/generator.test.ts b/src/lib/sieve/__tests__/generator.test.ts
index 7e4d5133..6f5a2ae0 100644
--- a/src/lib/sieve/__tests__/generator.test.ts
+++ b/src/lib/sieve/__tests__/generator.test.ts
@@ -209,9 +209,14 @@ describe('generateScript', () => {
       expect(script).toContain('reject "Go away";');
     });
 
-    it('generates keep', () => {
+    it('generates keep as an explicit fileinto "INBOX" (#1027)', () => {
+      // A bare `keep;` lets Stalwart's spam verdict decide the folder, so a
+      // "Keep in inbox" allow-list rule must file into INBOX explicitly.
       const script = generateScript([makeRule({ actions: [{ type: 'keep' }] })]);
-      expect(script).toContain('keep;');
+      expect(script).toContain('fileinto "INBOX";');
+      expect(script).not.toMatch(/^\s*keep;/m);
+      const requireLine = script.split('\n').find(l => l.startsWith('require'))!;
+      expect(requireLine).toContain('"fileinto"');
     });
 
     it('generates stop', () => {
@@ -434,9 +439,9 @@ describe('generateScript', () => {
       expect(requireLine).toContain('"imap4flags"');
     });
 
-    it('generates no require line when only keep/discard/stop/forward actions', () => {
+    it('generates no require line when only discard/stop/forward actions', () => {
       const script = generateScript([makeRule({
-        actions: [{ type: 'keep' }, { type: 'forward', value: 'a@b.com' }],
+        actions: [{ type: 'forward', value: 'a@b.com' }, { type: 'stop' }],
       })]);
       expect(script).not.toContain('require');
     });
diff --git a/src/lib/sieve/__tests__/parser.test.ts b/src/lib/sieve/__tests__/parser.test.ts
index 53fdec23..c4090e30 100644
--- a/src/lib/sieve/__tests__/parser.test.ts
+++ b/src/lib/sieve/__tests__/parser.test.ts
@@ -148,6 +148,38 @@ describe('parseScript', () => {
       const result = parseScript(script);
       expect(result.rules).toEqual(rules);
     });
+
+    it('reads a webmail "Keep in inbox" rule back as keep and saves it unchanged (#1027)', () => {
+      // What the webmail writes since 62465e1e: the action stays `keep` in the
+      // metadata, but the Sieve block files into INBOX explicitly.
+      const rule = makeRule({
+        id: 'allow',
+        name: 'Allow boss',
+        conditions: [{ field: 'from', comparator: 'is', value: 'boss@example.com' }],
+        actions: [{ type: 'keep' }],
+        stopProcessing: true,
+      });
+      const webScript = [
+        '/* @metadata:begin',
+        JSON.stringify({ version: 1, rules: [rule] }),
+        '@metadata:end */',
+        '',
+        'require ["fileinto"];',
+        '',
+        '# Rule: Allow boss',
+        'if header :is "From" "boss@example.com" {',
+        '    fileinto "INBOX";',
+        '    stop;',
+        '}',
+        '',
+      ].join('\n');
+
+      const result = parseScript(webScript);
+      expect(result.isOpaque).toBe(false);
+      expect(result.rules).toEqual([rule]);
+      expect(generateScript(result.rules, result.vacation, { externalRequires: result.externalRequires }))
+        .toBe(webScript);
+    });
   });
 
   describe('validation edge cases', () => {
diff --git a/src/lib/sieve/generator.ts b/src/lib/sieve/generator.ts
index cc3690e0..e850840b 100644
--- a/src/lib/sieve/generator.ts
+++ b/src/lib/sieve/generator.ts
@@ -129,7 +129,11 @@ function generateActions(actions: FilterAction[]): string[] {
       case 'reject':
         return `reject "${escapeString(action.value || '')}";`;
       case 'keep':
-        return 'keep;';
+        // A bare `keep;` delivers to the "default place", which on Stalwart is
+        // Junk for a message its spam filter has already classified, so an
+        // allow-list rule made with "Keep" would change nothing. An explicit
+        // `fileinto "INBOX"` is honored over the spam verdict (#1027).
+        return 'fileinto "INBOX";';
       case 'stop':
         return 'stop;';
     }
@@ -152,6 +156,7 @@ function computeRequires(rules: FilterRule[], vacation?: VacationSieveConfig): s
     for (const action of rule.actions) {
       switch (action.type) {
         case 'move':
+        case 'keep':
           extensions.add('fileinto');
           break;
         case 'copy':