fix(net): three plaintext-guard fixes for 2.2.0, and CI on release PRs

lib/core/utils/plaintext_destination_guard.dart

@@ -138,6 +138,10 @@ class PlaintextDestinationGuard {
 /// promise is about the app's traffic to a user-supplied address and not
 /// about one call site. Anything given this client is covered, including
 /// call sites that do not exist yet.
+///
+/// **Redirects are not followed.** They would be followed below this layer,
+/// where the guard never sees them, so a 30x comes back to the caller
+/// unfollowed rather than becoming an unchecked second destination.
 class GuardedPlaintextClient extends http.BaseClient {
   final http.Client _inner;
   final PlaintextDestinationGuard _guard;
@@ -148,25 +152,49 @@ class GuardedPlaintextClient extends http.BaseClient {
   @override
   Future<http.StreamedResponse> send(http.BaseRequest request) async {
     final approved = await _guard.approve(request.url);
-    if (approved == request.url) return _inner.send(request);
-    return _inner.send(_reboundTo(approved, request));
+    final outgoing = approved == request.url
+        ? request
+        : _reboundTo(approved, request);
+
+    // Redirects are followed inside `dart:io`, below `BaseClient.send`, so a
+    // hop never comes back through here and never meets [approve]. Left on,
+    // a server answering 30x with a public `http://` target would have that
+    // connection made — from a check that reported the destination private.
+    // The guard cannot vouch for a hop it never sees, so it does not let one
+    // happen: a redirect is returned to the caller as the 30x it is.
+    //
+    // This holds for `https://` too, which [approve] waves through: an
+    // encrypted first hop says nothing about where a `Location` points.
+    if (outgoing.followRedirects) outgoing.followRedirects = false;
+
+    return _inner.send(outgoing);
   }
 
   /// Rebuilds the request against the approved address, keeping the original
   /// authority in the `host` header so a name-based server still routes it.
   ///
+  /// **The port is part of that authority.** A local model server is almost
+  /// never on 80 — `http://ollama.lan:11434` is the ordinary shape — and a
+  /// `Host: ollama.lan` that drops the `:11434` is a different authority from
+  /// the one that was typed. A reverse proxy or a strict server routes by
+  /// what that header says, so it has to keep saying it. The port is written
+  /// only when the URL gave one, so a plain `http://ollama.lan` still sends
+  /// the bare name rather than a redundant `:80`.
+  ///
   /// Only [http.Request] can be rebuilt — its body is bytes already. A
   /// streamed request is passed through **after** the check, which still
   /// refuses a public destination; it only loses the address pinning. The app
   /// sends no streamed requests to a user-supplied endpoint, and a wrong
   /// guess about how to re-wrap one would be worse than the gap.
   http.BaseRequest _reboundTo(Uri url, http.BaseRequest request) {
     if (request is! http.Request) return request;
+    final origin = request.url;
     return http.Request(request.method, url)
       ..headers.addAll(request.headers)
-      ..headers['host'] = request.url.host
+      ..headers['host'] = origin.hasPort
+          ? '${origin.host}:${origin.port}'
+          : origin.host
       ..bodyBytes = request.bodyBytes
-      ..followRedirects = request.followRedirects
       ..maxRedirects = request.maxRedirects
       ..persistentConnection = request.persistentConnection;
   }

test/unit_test/plaintext_destination_guard_test.dart

@@ -5,12 +5,36 @@ import 'package:http/http.dart' as http;
 import 'package:opennutritracker/core/utils/plaintext_destination_guard.dart';
 
 /// Records the request that reached the socket, or the fact that none did.
+///
+/// [sentCount] records how many times [GuardedPlaintextClient] called through.
+/// It cannot see a redirect being followed — that happens inside `dart:io`,
+/// below this fake — so it pins that the guard itself sends once, and nothing
+/// more than that.
 class _RecordingClient extends http.BaseClient {
   http.BaseRequest? sent;
+  int sentCount = 0;
+
+  /// Returned instead of a bare 200 when a test needs a specific status —
+  /// a 30x, for the redirect cases.
+  final http.StreamedResponse? response;
+
+  _RecordingClient({this.response});
 
   @override
   Future<http.StreamedResponse> send(http.BaseRequest request) async {
     sent = request;
+    sentCount++;
+    // The injected response keeps the request association too, so an
+    // assertion on `response.request` reads the same either way.
+    final canned = response;
+    if (canned != null) {
+      return http.StreamedResponse(
+        canned.stream,
+        canned.statusCode,
+        headers: canned.headers,
+        request: request,
+      );
+    }
     return http.StreamedResponse(const Stream.empty(), 200, request: request);
   }
 }
@@ -185,11 +209,30 @@ void main() {
       );
 
       expect(inner.sent!.url.host, '192.168.1.5');
-      expect(inner.sent!.headers['host'], 'ollama.lan');
+      // With the port. `ollama.lan` alone is a different authority from the
+      // `ollama.lan:11434` that was typed, and a local model server is
+      // essentially never on 80, so dropping it is the common case rather
+      // than the corner one.
+      expect(inner.sent!.headers['host'], 'ollama.lan:11434');
       expect(inner.sent!.headers['content-type'], 'application/json');
       expect((inner.sent! as http.Request).body, 'payload');
     });
 
+    test('a URL without a port sends the bare name, not a redundant :80',
+        () async {
+      final inner = _RecordingClient();
+      final client = GuardedPlaintextClient(
+        inner,
+        guard: PlaintextDestinationGuard(
+          lookup: (_) async => [_v4('192.168.1.5')],
+        ),
+      );
+
+      await client.post(Uri.parse('http://ollama.lan/v1'), body: 'payload');
+
+      expect(inner.sent!.headers['host'], 'ollama.lan');
+    });
+
     test('an https request reaches the socket untouched', () async {
       final inner = _RecordingClient();
       final client = GuardedPlaintextClient(inner);
@@ -218,6 +261,73 @@ void main() {
       expect(inner.sent, isNull);
     });
 
+    test('redirects are not followed, on the rebound path', () async {
+      // The gap this closes: `dart:io` follows a 30x below `BaseClient.send`,
+      // so the hop never re-enters the guard. A private server answering with
+      // a public `http://` Location would have had that connection made, out
+      // of a check that reported the destination private.
+      final inner = _RecordingClient();
+      final client = GuardedPlaintextClient(
+        inner,
+        guard: PlaintextDestinationGuard(
+          lookup: (_) async => [_v4('192.168.1.5')],
+        ),
+      );
+
+      await client.post(
+        Uri.parse('http://ollama.lan:11434/v1/chat/completions'),
+        body: 'payload',
+      );
+
+      expect(inner.sent!.followRedirects, isFalse);
+    });
+
+    test('redirects are not followed on the https pass-through either', () async {
+      // `approve` waves https straight through, so the pass-through branch
+      // needs its own guarantee: an encrypted first hop says nothing about
+      // where a `Location` header points.
+      final inner = _RecordingClient();
+      final client = GuardedPlaintextClient(inner);
+
+      await client.post(
+        Uri.parse('https://ollama.example.com/v1/chat/completions'),
+        body: 'payload',
+      );
+
+      expect(inner.sent!.followRedirects, isFalse);
+    });
+
+    test('a 30x comes back to the caller as a response, not a second hop', () async {
+      // Contract, not regression: the following happens inside `dart:io`,
+      // below this fake, so this test passes with or without the fix above.
+      // It pins what a caller sees — a 30x surfacing rather than being
+      // swallowed — while the two tests above are the ones that fail if
+      // `followRedirects` is ever turned back on.
+      final inner = _RecordingClient(
+        response: http.StreamedResponse(
+          const Stream.empty(),
+          302,
+          headers: {'location': 'http://93.184.216.34/v1/chat/completions'},
+        ),
+      );
+      final client = GuardedPlaintextClient(
+        inner,
+        guard: PlaintextDestinationGuard(
+          lookup: (_) async => [_v4('192.168.1.5')],
+        ),
+      );
+
+      final response = await client.post(
+        Uri.parse('http://ollama.lan:11434/v1/chat/completions'),
+        body: 'payload',
+      );
+
+      // Surfaced rather than chased. The caller sees a failed request; what
+      // it does not see is a payload delivered to 93.184.216.34.
+      expect(response.statusCode, 302);
+      expect(inner.sentCount, 1);
+    });
+
     test('the exception names the host and nothing else', () async {
       // Raised on a request that may be carrying a photograph of somebody's
       // dinner. The path and body must not reach a log through it.
@@ -234,4 +344,111 @@ void main() {
       }
     });
   });
+
+  /// The claims above are made against a fake, which cannot follow a redirect
+  /// — `dart:io` does that below `BaseClient.send`, where no fake reaches. So
+  /// these run against a real `HttpServer` on loopback and a real `dart:io`
+  /// client, which is the only place the redirect rule is actually decided.
+  group('against a real socket', () {
+    late HttpServer server;
+    late List<String> paths;
+    late List<String?> hosts;
+    HttpOverrides? savedOverrides;
+
+    setUp(() async {
+      // `flutter_test` installs an override that answers every request with a
+      // canned 400 rather than opening a socket. These tests are about what
+      // the socket layer really does, so they need it out of the way — and
+      // put back, because the rest of the suite may rely on it.
+      savedOverrides = HttpOverrides.current;
+      HttpOverrides.global = null;
+
+      paths = [];
+      hosts = [];
+      server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0);
+      server.listen((request) async {
+        paths.add(request.uri.path);
+        hosts.add(request.headers.value('host'));
+        if (request.uri.path == '/first') {
+          request.response.statusCode = HttpStatus.found;
+          request.response.headers.set('location', '/second');
+        } else {
+          request.response.statusCode = HttpStatus.ok;
+        }
+        await request.response.close();
+      });
+    });
+
+    tearDown(() async {
+      await server.close(force: true);
+      HttpOverrides.global = savedOverrides;
+    });
+
+    test('an unguarded client really does follow the redirect', () async {
+      // The control. Without it the two tests below prove nothing: they would
+      // pass just as well against a server that never redirected, or a client
+      // stack that had stopped following redirects on its own.
+      final client = http.Client();
+      addTearDown(client.close);
+
+      final response =
+          await client.get(Uri.parse('http://127.0.0.1:${server.port}/first'));
+
+      expect(paths, ['/first', '/second']);
+      expect(response.statusCode, 200);
+    });
+
+    test('the guarded client stops at the 30x — literal pass-through',
+        () async {
+      // A private literal is waved through `approve` untouched, so this is
+      // the branch where the request object reaches the socket as the caller
+      // built it. `followRedirects` still has to have been turned off.
+      final client = GuardedPlaintextClient(http.Client());
+      addTearDown(client.close);
+
+      final response =
+          await client.get(Uri.parse('http://127.0.0.1:${server.port}/first'));
+
+      expect(paths, ['/first']);
+      expect(response.statusCode, 302);
+      expect(response.headers['location'], '/second');
+    });
+
+    test('the guarded client stops at the 30x — rebound path', () async {
+      // The same claim on the other branch: a name pinned to its resolved
+      // address, rebuilt into a new request. The rebuild must not hand back a
+      // request that follows redirects.
+      final client = GuardedPlaintextClient(
+        http.Client(),
+        guard: PlaintextDestinationGuard(
+          lookup: (_) async => [_v4('127.0.0.1')],
+        ),
+      );
+      addTearDown(client.close);
+
+      final response = await client
+          .get(Uri.parse('http://ollama.lan:${server.port}/first'));
+
+      expect(paths, ['/first']);
+      expect(response.statusCode, 302);
+    });
+
+    test('the Host header arrives at the server with its port', () async {
+      // Read off the wire rather than off the request object: the header has
+      // to survive `dart:io`, which sets a Host of its own from the
+      // connection URI and would otherwise report 127.0.0.1.
+      final client = GuardedPlaintextClient(
+        http.Client(),
+        guard: PlaintextDestinationGuard(
+          lookup: (_) async => [_v4('127.0.0.1')],
+        ),
+      );
+      addTearDown(client.close);
+
+      await client.get(Uri.parse('http://ollama.lan:${server.port}/second'));
+
+      expect(hosts.single, 'ollama.lan:${server.port}');
+    });
+  });
+
 }