feat: ManageSieve STARTTLS + clearer TLS-mismatch errors + broader connection test

The "Email filters" screen was failing with WRONG_VERSION_NUMBER because the
ManageSieve client was opening implicit-TLS sockets on port 4190, while RFC
5804 servers (Stalwart, Dovecot, Cyrus) listen plaintext on 4190 and expect
STARTTLS. ManageSieveClient.connect now opens plaintext, reads the capability
greeting, sends STARTTLS, hands the socket to SecureSocket.secure(), and
re-reads capabilities on the encrypted stream.

The same WRONG_VERSION_NUMBER error can hit IMAP/SMTP when the SSL toggle and
the chosen port disagree (e.g. SSL=on with SMTP port 587). New helper
lib/data/imap/tls_error.dart translates that BoringSSL error into a
TlsModeMismatchException naming the host/port and suggesting which port goes
with which TLS mode. connectImap, connectSmtp, and the ManageSieve TLS
upgrade all funnel through rethrowAsTlsHint so the same readable message
reaches the UI regardless of which protocol failed.

ConnectionTestService previously only verified IMAP/JMAP, so SMTP and
ManageSieve misconfig silently passed the "Try connection" button on the
edit-account screen and only surfaced when the user later tried to send
mail or open Email filters. After IMAP succeeds, the service now also
verifies SMTP (always — sending mail requires it) and ManageSieve (only
when manageSieveHost is explicitly set, since the section is collapsed by
default). Failures are prefixed with "SMTP:" or "ManageSieve:" so the
user can tell which leg of the connection is broken.

connectionTestServiceProvider now also watches smtpConnectProvider so the
E2E integration tests' plaintext SMTP override applies to the connection
check as well.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

LATER.md

@@ -4,6 +4,16 @@ Are errors written to syncLog ?
 
 ---
 
+Error and Crash reporting to central server.
+
+But this needs a central service at sharedinbox.de
+
+Data will be sent only after confirmation.
+
+The user must decide: send crash report, or accept select "I prefer not to send crash report".
+
+---
+
 Taskfile: Debug logs with start+end timestamp for debugging. Each stdout/stderr in one file. How to
 get this?
 

done.md

@@ -6,6 +6,45 @@ Tasks get moved from next.md to done.md
 
 ## Tasks
 
+## ManageSieve uses STARTTLS; clearer TLS-mismatch errors; broader connection check
+
+The "Email filters" screen failed for IMAP accounts with
+`HandshakeException: WRONG_VERSION_NUMBER(tls_record.cc:127)` because the
+ManageSieve client was opening an implicit-TLS socket to port 4190, while
+the server (Stalwart and other RFC 5804 implementations) listens plaintext
+on 4190 and expects a `STARTTLS` upgrade. The plaintext capability greeting
+landed in the TLS parser, which (correctly) rejected it.
+
+`ManageSieveClient.connect` (`lib/data/imap/managesieve_client.dart`) now
+follows RFC 5804 §1.7: it opens a plaintext socket, reads the capability
+greeting, and — when `useTls` is true — sends `STARTTLS`, waits for `OK`,
+detaches the plaintext listener, hands the raw socket to
+`SecureSocket.secure()`, and re-reads capabilities on the secured stream.
+The previous "implicit TLS, no STARTTLS" mode is gone; if the server does
+not advertise `STARTTLS`, the client throws a clear error pointing the
+user at the SSL toggle.
+
+`WRONG_VERSION_NUMBER` is also produced for SMTP (and IMAP) when the SSL
+toggle and the chosen port disagree — e.g. SSL=on with port 587 (which is
+STARTTLS-only). New helper `lib/data/imap/tls_error.dart` translates that
+specific BoringSSL error into a `TlsModeMismatchException` with the host,
+port, and a hint about which port matches which TLS mode (465/587, 993/143,
+4190). `connectImap`, `connectSmtp`, and the ManageSieve TLS upgrade now
+all funnel through `rethrowAsTlsHint` so the same readable message reaches
+the UI regardless of which protocol failed.
+
+`ConnectionTestService` (`lib/core/services/connection_test_service.dart`)
+previously only verified IMAP/JMAP, so SMTP and ManageSieve misconfig
+silently passed the "Try connection" button on the edit-account screen
+and only surfaced when the user later tried to send mail or open Email
+filters. After IMAP succeeds, the service now also verifies SMTP (always
+for IMAP accounts — sending mail requires it) and ManageSieve (only when
+`manageSieveHost` is explicitly set, since the section is collapsed and
+opt-in by default). Failures are prefixed with `SMTP:` or `ManageSieve:`
+so the user can tell which leg of the connection is broken. New tests in
+`test/unit/connection_test_service_test.dart` cover SMTP-failure
+surfacing, the opt-in skip path, and ManageSieve-failure surfacing.
+
 ## Sieve filter editing for IMAP accounts (ManageSieve)
 
 The "Email filters" entry was previously hidden for IMAP accounts because

lib/core/services/connection_test_service.dart

@@ -4,6 +4,7 @@ import 'package:enough_mail/enough_mail.dart' as imap;
 import 'package:http/http.dart' as http;
 import 'package:sharedinbox/core/models/account.dart';
 import 'package:sharedinbox/data/imap/imap_client_factory.dart';
+import 'package:sharedinbox/data/imap/managesieve_client.dart';
 
 typedef ImapConnectForTestFn = Future<imap.ImapClient> Function(
   Account,
@@ -11,6 +12,25 @@ typedef ImapConnectForTestFn = Future<imap.ImapClient> Function(
   String password,
 );
 
+typedef SmtpConnectForTestFn = Future<imap.SmtpClient> Function(
+  Account,
+  String username,
+  String password,
+);
+
+typedef ManageSieveConnectForTestFn = Future<ManageSieveClient> Function({
+  required String host,
+  required int port,
+  required bool useTls,
+});
+
+Future<ManageSieveClient> _defaultManageSieveConnect({
+  required String host,
+  required int port,
+  required bool useTls,
+}) =>
+    ManageSieveClient.connect(host: host, port: port, useTls: useTls);
+
 abstract class ConnectionTestService {
   /// Verifies credentials and returns the effective username used.
   /// Throws a descriptive [Exception] on failure.
@@ -21,16 +41,31 @@ class ConnectionTestServiceImpl implements ConnectionTestService {
   ConnectionTestServiceImpl(
     this._httpClient, {
     ImapConnectForTestFn imapConnect = connectImap,
-  }) : _imapConnect = imapConnect;
+    SmtpConnectForTestFn smtpConnect = connectSmtp,
+    ManageSieveConnectForTestFn manageSieveConnect = _defaultManageSieveConnect,
+  })  : _imapConnect = imapConnect,
+        _smtpConnect = smtpConnect,
+        _manageSieveConnect = manageSieveConnect;
 
   final http.Client _httpClient;
   final ImapConnectForTestFn _imapConnect;
+  final SmtpConnectForTestFn _smtpConnect;
+  final ManageSieveConnectForTestFn _manageSieveConnect;
 
   @override
   Future<String> testConnection(Account account, String password) async {
     switch (account.type) {
       case AccountType.imap:
-        return _testImap(account, password);
+        final username = await _testImap(account, password);
+        // Also verify SMTP — without this, send-mail would fail later with
+        // no warning at account-edit time.
+        await _testSmtp(account, username, password);
+        // ManageSieve is opt-in: only test it if the user has explicitly
+        // filled in the host (the section is collapsed by default).
+        if (account.manageSieveHost.trim().isNotEmpty) {
+          await _testManageSieve(account, username, password);
+        }
+        return username;
       case AccountType.jmap:
         return _testJmap(account, password);
     }
@@ -63,6 +98,53 @@ class ConnectionTestServiceImpl implements ConnectionTestService {
     throw lastError!;
   }
 
+  Future<void> _testSmtp(
+    Account account,
+    String username,
+    String password,
+  ) async {
+    final imap.SmtpClient client;
+    try {
+      client = await _smtpConnect(account, username, password);
+    } catch (e) {
+      throw Exception('SMTP: $e');
+    }
+    try {
+      await client.quit();
+    } catch (_) {/* best-effort */}
+  }
+
+  Future<void> _testManageSieve(
+    Account account,
+    String username,
+    String password,
+  ) async {
+    final host = account.manageSieveHost.trim().isNotEmpty
+        ? account.manageSieveHost.trim()
+        : account.imapHost;
+    final ManageSieveClient client;
+    try {
+      client = await _manageSieveConnect(
+        host: host,
+        port: account.manageSievePort,
+        useTls: account.manageSieveSsl,
+      );
+    } catch (e) {
+      throw Exception('ManageSieve: $e');
+    }
+    try {
+      await client.authenticatePlain(username, password);
+    } catch (e) {
+      try {
+        await client.logout();
+      } catch (_) {/* best-effort */}
+      throw Exception('ManageSieve: $e');
+    }
+    try {
+      await client.logout();
+    } catch (_) {/* best-effort */}
+  }
+
   Future<String> _testJmap(Account account, String password) async {
     final jmapUrl = account.jmapUrl;
     if (jmapUrl == null || jmapUrl.isEmpty) {

lib/data/imap/imap_client_factory.dart

@@ -3,6 +3,7 @@ import 'dart:async';
 import 'package:enough_mail/enough_mail.dart';
 
 import 'package:sharedinbox/core/models/account.dart';
+import 'package:sharedinbox/data/imap/tls_error.dart';
 
 typedef ImapConnectFn = Future<ImapClient> Function(
   Account account,
@@ -30,11 +31,15 @@ Future<ImapClient> connectImap(
     defaultResponseTimeout: const Duration(seconds: 20),
     isLogEnabled: verboseBuffer != null,
   );
-  await client.connectToServer(
-    account.imapHost,
-    account.imapPort,
-    isSecure: account.imapSsl,
-  );
+  try {
+    await client.connectToServer(
+      account.imapHost,
+      account.imapPort,
+      isSecure: account.imapSsl,
+    );
+  } catch (e, st) {
+    rethrowAsTlsHint(e, st, account.imapHost, account.imapPort);
+  }
   await client.login(username, password);
   return client;
 }
@@ -57,15 +62,23 @@ Future<SmtpClient> connectSmtp(
       atIndex != -1 ? account.email.substring(atIndex + 1) : account.smtpHost;
 
   final client = SmtpClient(clientDomain);
-  await client.connectToServer(
-    account.smtpHost,
-    account.smtpPort,
-    isSecure: account.smtpSsl,
-  );
+  try {
+    await client.connectToServer(
+      account.smtpHost,
+      account.smtpPort,
+      isSecure: account.smtpSsl,
+    );
+  } catch (e, st) {
+    rethrowAsTlsHint(e, st, account.smtpHost, account.smtpPort);
+  }
   await client.ehlo();
   if (!account.smtpSsl) {
     // STARTTLS required on submission port (587). No plaintext fallback.
-    await client.startTls();
+    try {
+      await client.startTls();
+    } catch (e, st) {
+      rethrowAsTlsHint(e, st, account.smtpHost, account.smtpPort);
+    }
   }
   await client.authenticate(username, password);
   return client;

lib/data/imap/managesieve_client.dart

@@ -5,15 +5,15 @@ import 'dart:typed_data';
 
 import 'package:sharedinbox/data/imap/imap_client_factory.dart'
     show verboseLogKey;
+import 'package:sharedinbox/data/imap/tls_error.dart';
 
 /// Minimal ManageSieve (RFC 5804) client used by [SieveRepository] to
 /// list / fetch / upload / activate / delete server-side Sieve scripts on
 /// IMAP-style mail servers (e.g. Stalwart, Dovecot, Cyrus).
 ///
 /// Only the small subset needed by the app is implemented:
-/// AUTHENTICATE PLAIN, LISTSCRIPTS, GETSCRIPT, PUTSCRIPT, SETACTIVE,
-/// DELETESCRIPT, LOGOUT. STARTTLS is not implemented — connect with
-/// implicit TLS instead (the default port 4190 layout used by Stalwart).
+/// STARTTLS, AUTHENTICATE PLAIN, LISTSCRIPTS, GETSCRIPT, PUTSCRIPT,
+/// SETACTIVE, DELETESCRIPT, LOGOUT.
 class ManageSieveClient {
   ManageSieveClient._(this._socket, this._source);
 
@@ -21,7 +21,11 @@ class ManageSieveClient {
   final _ByteSource _source;
   bool _closed = false;
 
-  /// Connects to [host]:[port] and reads the capability greeting.
+  /// Connects to [host]:[port] in plaintext, reads the capability greeting,
+  /// and — when [useTls] is true — upgrades the connection with STARTTLS per
+  /// RFC 5804 §1.7 before returning. Implicit-TLS ManageSieve is non-standard
+  /// and not supported here; if [useTls] is true the server must advertise
+  /// the `STARTTLS` capability.
   static Future<ManageSieveClient> connect({
     required String host,
     required int port,
@@ -29,20 +33,85 @@ class ManageSieveClient {
     Duration timeout = const Duration(seconds: 20),
   }) async {
     // ignore: close_sinks  // Stored in client and closed in logout().
-    final Socket socket = useTls
-        ? await SecureSocket.connect(host, port, timeout: timeout)
-        : await Socket.connect(host, port, timeout: timeout);
-    final source = _ByteSource();
-    socket.listen(
+    Socket socket = await Socket.connect(host, port, timeout: timeout);
+    var source = _ByteSource();
+    var sub = _attach(socket, source);
+    try {
+      final greeting = await _readResponseFromSource(source);
+      if (greeting.status != _Status.ok) {
+        throw ManageSieveException(
+          'Capability greeting failed: ${greeting.message}',
+        );
+      }
+      if (!useTls) {
+        return ManageSieveClient._(socket, source);
+      }
+      if (!_advertisesStartTls(greeting.dataLines)) {
+        throw const ManageSieveException(
+          'Server does not advertise STARTTLS — disable SSL for ManageSieve '
+          'in account settings, or use a server that supports STARTTLS.',
+        );
+      }
+      await _writeLineTo(socket, 'STARTTLS');
+      final ok = await _readResponseFromSource(source);
+      if (ok.status != _Status.ok) {
+        throw ManageSieveException('STARTTLS rejected: ${ok.message}');
+      }
+      // Detach plaintext listener before handing the socket to TLS.
+      await sub.cancel();
+      try {
+        socket = await SecureSocket.secure(socket, host: host);
+      } catch (e, st) {
+        rethrowAsTlsHint(e, st, host, port);
+      }
+      source = _ByteSource();
+      sub = _attach(socket, source);
+      // RFC 5804 §1.7: server re-sends capability listing on the secured
+      // connection. Read and discard it so the response stream stays aligned.
+      final reCap = await _readResponseFromSource(source);
+      if (reCap.status != _Status.ok) {
+        throw ManageSieveException(
+          'Post-STARTTLS capability failed: ${reCap.message}',
+        );
+      }
+      return ManageSieveClient._(socket, source);
+    } catch (_) {
+      await sub.cancel();
+      try {
+        await socket.close();
+      } catch (_) {/* best-effort */}
+      rethrow;
+    }
+  }
+
+  static StreamSubscription<List<int>> _attach(
+    Socket socket,
+    _ByteSource source,
+  ) {
+    return socket.listen(
       source._add,
       onDone: () => source._close(),
       onError: (Object e, StackTrace _) => source._close(e),
       cancelOnError: true,
     );
-    final client = ManageSieveClient._(socket, source);
-    // Greeting / capability response.
-    await client._readResponse();
-    return client;
+  }
+
+  static bool _advertisesStartTls(List<String> capLines) {
+    for (final line in capLines) {
+      // Capability tokens are quoted-strings, e.g. `"STARTTLS"`. Match the
+      // first token on each line, case-insensitively.
+      final m = RegExp(r'^\s*"?([A-Za-z0-9-]+)"?').firstMatch(line);
+      if (m != null && m.group(1)!.toUpperCase() == 'STARTTLS') return true;
+    }
+    return false;
+  }
+
+  static Future<void> _writeLineTo(Socket socket, String line) async {
+    final log = Zone.current[verboseLogKey] as StringBuffer?;
+    if (log != null) log.writeln('SIEVE → $line');
+    socket.add(utf8.encode(line));
+    socket.add(_crlf);
+    await socket.flush();
   }
 
   /// Authenticates with SASL PLAIN. Throws [ManageSieveException] on failure.
@@ -157,20 +226,22 @@ class ManageSieveClient {
     }
   }
 
-  Future<_Response> _readResponse() async {
+  Future<_Response> _readResponse() => _readResponseFromSource(_source);
+
+  static Future<_Response> _readResponseFromSource(_ByteSource source) async {
     final dataLines = <String>[];
     final log = Zone.current[verboseLogKey] as StringBuffer?;
     while (true) {
-      final lineBytes = await _source.takeLine();
+      final lineBytes = await source.takeLine();
       var line = utf8.decode(lineBytes);
       // Detect a trailing literal: line ends with `{NNN}` or `{NNN+}`.
       final lit = RegExp(r'\{(\d+)\+?\}\s*$').firstMatch(line);
       if (lit != null) {
         final n = int.parse(lit.group(1)!);
-        final body = await _source.takeBytes(n);
+        final body = await source.takeBytes(n);
         // After the literal, the server emits the rest of the line — typically
         // empty plus CRLF. Consume it so the next iteration is line-aligned.
-        await _source.takeLine();
+        await source.takeLine();
         line = utf8.decode(body);
         if (log != null) {
           log.writeln('SIEVE ← <literal $n bytes>');

lib/data/imap/tls_error.dart

@@ -0,0 +1,40 @@
+/// Wraps a low-level TLS handshake failure (typically from `dart:io`) into a
+/// message that points the user at the most likely fix: a TLS-mode mismatch
+/// between the client (implicit TLS or plaintext) and the server's listener
+/// on that port.
+///
+/// `WRONG_VERSION_NUMBER` is BoringSSL's way of saying "I tried to read a TLS
+/// record but the bytes don't look like TLS at all" — almost always because
+/// the server on that port is plaintext or expects STARTTLS first.
+class TlsModeMismatchException implements Exception {
+  TlsModeMismatchException(this.host, this.port, this.original);
+  final String host;
+  final int port;
+  final Object original;
+
+  @override
+  String toString() =>
+      "TLS mode mismatch on $host:$port — the server isn't speaking implicit "
+      "TLS on this port. Try toggling 'SSL/TLS' off (server uses STARTTLS), "
+      'or change the port (e.g. 465 for implicit-TLS SMTP, 587 for SMTP+'
+      'STARTTLS, 993 for IMAPS, 143 for IMAP+STARTTLS, 4190 for ManageSieve+'
+      'STARTTLS). Original error: $original';
+}
+
+/// If [error] is a TLS handshake failure caused by a wrong-version-number
+/// (i.e. the server is not speaking TLS), throw a [TlsModeMismatchException]
+/// with [host]/[port] context. Otherwise rethrow [error] unchanged.
+Never rethrowAsTlsHint(
+  Object error,
+  StackTrace stack,
+  String host,
+  int port,
+) {
+  if (error.toString().contains('WRONG_VERSION_NUMBER')) {
+    Error.throwWithStackTrace(
+      TlsModeMismatchException(host, port, error),
+      stack,
+    );
+  }
+  Error.throwWithStackTrace(error, stack);
+}

lib/di.dart

@@ -102,6 +102,7 @@ final connectionTestServiceProvider = Provider<ConnectionTestService>((ref) {
   return ConnectionTestServiceImpl(
     ref.watch(httpClientProvider),
     imapConnect: ref.watch(imapConnectProvider),
+    smtpConnect: ref.watch(smtpConnectProvider),
   );
 });
 

next.md

@@ -16,4 +16,9 @@ Git repo should not contain unknown files.
 
 Then commit.
 
+Then push
+
 ## Tasks
+
+Plain-text connections only via localhost.
+Dont show in ui, except host is localhost.

test/unit/connection_test_service_test.dart

@@ -46,6 +46,7 @@ ConnectionTestServiceImpl _makeService({
       if (imapError != null) throw imapError;
       return fakeImap ?? FakeImapClient();
     },
+    smtpConnect: (account, username, password) async => FakeSmtpClient(),
   );
 }
 
@@ -83,6 +84,7 @@ void main() {
           }
           return FakeImapClient();
         },
+        smtpConnect: (_, __, ___) async => FakeSmtpClient(),
       );
       final result = await svc.testConnection(_imapAccount, 'pw');
       expect(result, 'alice');
@@ -93,12 +95,70 @@ void main() {
       final svc = ConnectionTestServiceImpl(
         MockClient((_) async => http.Response('', 200)),
         imapConnect: (_, __, ___) async => throw Exception('auth failed'),
+        smtpConnect: (_, __, ___) async => FakeSmtpClient(),
       );
       expect(
         () => svc.testConnection(_imapAccount, 'pw'),
         throwsException,
       );
     });
+
+    test('reports SMTP failure after IMAP success', () async {
+      final svc = ConnectionTestServiceImpl(
+        MockClient((_) async => http.Response('', 200)),
+        imapConnect: (_, __, ___) async => FakeImapClient(),
+        smtpConnect: (_, __, ___) async => throw Exception('smtp boom'),
+      );
+      expect(
+        () => svc.testConnection(_imapAccount, 'pw'),
+        throwsA(predicate((e) => e.toString().contains('SMTP: '))),
+      );
+    });
+
+    test('skips ManageSieve when manageSieveHost is empty', () async {
+      var sieveCalled = false;
+      final svc = ConnectionTestServiceImpl(
+        MockClient((_) async => http.Response('', 200)),
+        imapConnect: (_, __, ___) async => FakeImapClient(),
+        smtpConnect: (_, __, ___) async => FakeSmtpClient(),
+        manageSieveConnect: ({
+          required String host,
+          required int port,
+          required bool useTls,
+        }) async {
+          sieveCalled = true;
+          throw Exception('should not be called');
+        },
+      );
+      await svc.testConnection(_imapAccount, 'pw');
+      expect(sieveCalled, false);
+    });
+
+    test('reports ManageSieve failure when host is set', () async {
+      const accountWithSieve = Account(
+        id: 'acc-1',
+        displayName: 'Alice',
+        email: 'alice@example.com',
+        imapHost: 'imap.example.com',
+        smtpHost: 'smtp.example.com',
+        manageSieveHost: 'sieve.example.com',
+      );
+      final svc = ConnectionTestServiceImpl(
+        MockClient((_) async => http.Response('', 200)),
+        imapConnect: (_, __, ___) async => FakeImapClient(),
+        smtpConnect: (_, __, ___) async => FakeSmtpClient(),
+        manageSieveConnect: ({
+          required String host,
+          required int port,
+          required bool useTls,
+        }) async =>
+            throw Exception('sieve boom'),
+      );
+      expect(
+        () => svc.testConnection(accountWithSieve, 'pw'),
+        throwsA(predicate((e) => e.toString().contains('ManageSieve: '))),
+      );
+    });
   });
 
   group('ConnectionTestServiceImpl JMAP', () {

test/unit/fake_imap.dart

@@ -16,3 +16,11 @@ class FakeImapClient extends imap.ImapClient {
   @override
   Future<dynamic> logout() async {}
 }
+
+/// Minimal fake SMTP client; only `quit` is exercised by ConnectionTestService.
+class FakeSmtpClient extends imap.SmtpClient {
+  FakeSmtpClient() : super('fake.host');
+
+  @override
+  Future<imap.SmtpResponse> quit() async => imap.SmtpResponse(const []);
+}