chore: backport the 2.2.0 release-branch fixes to develop (#1006)

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

Carries the plaintext-guard fixes onto the release branch. The first two
were raised in review of #988 and were never fixed there, because the work
landed on a branch targeting `develop` (#1002, since merged) while
`release/2.2.0` is cut from `main`.

- **A redirect bypassed the guard entirely.** `send` validated the initial
  URI and then handed the request to a client that follows redirects by
  default, below `BaseClient.send` where the guard never sees the hop. An
  approved private endpoint answering 30x with a public `http://` target
  would have had that connection made. Automatic redirects are now off, so a
  30x returns to the caller as the response it is — for `https://` too.
- **The Host header dropped the port.** Rebinding `http://ollama.lan:11434`
  to its resolved address wrote `Host: ollama.lan`, a different authority
  from the one typed, and a local model server is essentially never on 80.
- **A zoned link-local address was refused.** `Uri` escapes the zone id, so
  `fe80::1%wlan0` reaches the guard as `fe80::1%25wlan0`, which
  `InternetAddress.tryParse` rejects; the address fell through to a DNS
  lookup that cannot succeed and the caller was told the server is
  unreachable. `dart:io` performs the same unescaping itself before it
  connects, which is why an unguarded client worked. Fail-closed, and it
  predates 2.2.0. Found by audit, not by review.

The review also objected that the guard's tests used a recording fake, which
cannot follow a redirect, so they pinned that `followRedirects` was set
false rather than that it stops the second hop. There is now a group running
against a real `HttpServer` on loopback and a real `dart:io` client, over
both branches through the guard, with a control test proving an unguarded
client really does follow the 302.

All three fixes are mutation-tested: reverting the redirect guard fails four
tests, the port two, the zone two.

Also adds `release/**` to the workflow's `pull_request` trigger. This PR
opened with exactly one check — the Copilot reviewer — while reporting
`CLEAN`, which is the silent failure the trigger list's own comment warns
about, now hit on a release branch.

Cherry-picked from #1002 and from #1005, which is closed in favour of this.
`plaintext_destination_guard.dart` and its test file are byte-identical
across both branches.

(cherry picked from commit ae8ad7c)

* fix(bulk-add): round a converted quantity to the precision the field takes

The amount field accepts at most two decimals: `_quantityPattern` in
bulk_add_screen.dart is both the submit check and the field's input
formatter. A converted imperial quantity carries far more — `1 lb` is
453.59237 g — and the prefill passed it through untouched.

So an ordinary imperial line was unloggable. `1 lb mince` prefilled
"453.59237", the submit check refused it with a message naming only the
field, and because that check validates the whole batch before writing
anything, one such row blocked every correct row beside it. Recovery was
worse: the pattern is anchored, so the formatter matched nothing on the
non-conforming string and the field blanked entirely on the first
keystroke.

Rounding at the prefill is what keeps the two in step. The floor is the
same concern from the other end — a positive quantity below 0.005 would
round to "0.00", which the check rejects for being non-positive, and a
backend serving weight can be that small.

`lb` is a deliberate unit, not a stray one: it is in the model tool
schema enum because the model was otherwise mapping "1 pound of mince"
onto `oz`, so the model path reached this too.

(cherry picked from commit fcb2323)

Author: simonoppowa

.github/workflows/default_workflow.yml

@@ -32,6 +32,13 @@ on:
       # to add it back, and the failure mode is silent — a retargeted PR
       # still reports mergeable with no checks run.
       - 'feature/**'
+      # Release branches, for the same reason and with the same failure mode.
+      # `release/2.2.0` is cut from `main`, so a fix that landed on `develop`
+      # is not in it, and the PR carrying that fix across got exactly one
+      # check — the Copilot reviewer — while reporting `CLEAN`. A release
+      # branch is the last place that should merge unverified: everything on
+      # it is by definition about to become `main`.
+      - 'release/**'
   workflow_dispatch:
 
 jobs:

lib/core/utils/plaintext_destination_guard.dart

@@ -88,6 +88,32 @@ class PlaintextDestinationGuard {
   static Future<List<InternetAddress>> _resolve(String host) =>
       InternetAddress.lookup(host, type: InternetAddressType.any);
 
+  /// Turns the `%25` a URI must spell a zone id with back into the `%` that
+  /// [InternetAddress] parses.
+  ///
+  /// A link-local address needs a zone to be routable on a host with more
+  /// than one interface, and RFC 6874 says a URI escapes it: `Uri` stores
+  /// `http://[fe80::1%wlan0]` with a host of `fe80::1%25wlan0` — typing the
+  /// bare `%` gets you the same thing, so this is not a corner someone has to
+  /// know the RFC to reach. `InternetAddress.tryParse` returns null on that
+  /// escaped form, which sent the address down the DNS branch below, where
+  /// the lookup fails and the caller is told the server is **unreachable** —
+  /// about a link-local server this check exists to *permit*, and which an
+  /// unguarded client reaches perfectly well.
+  ///
+  /// `dart:io` does exactly this substitution itself, in
+  /// `escapeLinkLocalAddress`, before it resolves or connects. Doing it here
+  /// too is what makes the check agree with the socket layer it is guarding.
+  ///
+  /// Anything that still fails to parse falls through to the lookup unchanged,
+  /// so a name is never touched: the `25` is only dropped when it directly
+  /// follows the first `%`.
+  static String _withZoneUnescaped(String host) {
+    final marker = host.indexOf('%');
+    if (marker < 0 || !host.startsWith('25', marker + 1)) return host;
+    return host.replaceRange(marker + 1, marker + 3, '');
+  }
+
   /// Returns the URL to actually request, or throws
   /// [InsecureDestinationException].
   ///
@@ -103,7 +129,7 @@ class PlaintextDestinationGuard {
     if (url.scheme != 'http') return url;
 
     final host = url.host;
-    final literal = InternetAddress.tryParse(host);
+    final literal = InternetAddress.tryParse(_withZoneUnescaped(host));
     if (literal != null) {
       // No lookup to do, and nothing to pin — the user typed the address.
       if (isPrivateDestination(literal)) return url;

lib/features/add_meal/presentation/bloc/bulk_add_bloc.dart

@@ -435,11 +435,11 @@ class BulkAddBloc extends Bloc<BulkAddEvent, BulkAddState> {
     bool usesImperialUnits,
   ) {
     final parsedQuantity = parsed.quantity;
-    if (parsedQuantity != null) return _trimZeros(parsedQuantity);
+    if (parsedQuantity != null) return _amountText(parsedQuantity);
 
     if (meal != null && meal.hasServingValues) {
       final serving = meal.servingQuantity;
-      if (serving != null) return _trimZeros(serving);
+      if (serving != null) return _amountText(serving);
     }
     return usesImperialUnits ? '1' : '100';
   }
@@ -497,9 +497,26 @@ class BulkAddBloc extends Bloc<BulkAddEvent, BulkAddState> {
     return fallback;
   }
 
-  static String _trimZeros(double value) => value == value.roundToDouble()
-      ? value.toInt().toString()
-      : value.toString();
+  /// Formats a quantity for the amount field, which accepts at most two
+  /// decimals — `_quantityPattern` on the bulk-add screen is both the submit
+  /// check and the field's input formatter. A converted imperial quantity has
+  /// many more: `1 lb` is 453.59237 g. Prefilling that verbatim produced a row
+  /// the submit check refused, with a message naming only the field, and
+  /// because that check aborts the batch, one such row blocked every correct
+  /// row beside it. The pattern is anchored, so the formatter then rejected
+  /// every edit too and the field blanked on the first keystroke. Rounding
+  /// here is what keeps the prefill and the check in step.
+  ///
+  /// The floor is the same concern from the other end: a positive quantity
+  /// below 0.005 would round to zero, which the check rejects for being
+  /// non-positive.
+  static String _amountText(double value) {
+    final rounded = double.parse(value.toStringAsFixed(2));
+    final quantity = rounded <= 0 && value > 0 ? 0.01 : rounded;
+    return quantity == quantity.roundToDouble()
+        ? quantity.toInt().toString()
+        : quantity.toString();
+  }
 
   void _onChangeCandidate(
     ChangeRowCandidateEvent event,

test/unit_test/bulk_add_bloc_test.dart

@@ -221,6 +221,78 @@ void main() {
     expect(state.rows.single.unit, 'g');
   });
 
+  // The amount field accepts at most two decimals, and the same anchored
+  // pattern is both the submit check and the field's input formatter. A
+  // prefill it refuses is a row that cannot be logged and cannot be edited
+  // back into shape — the field blanks on the first keystroke — and the
+  // submit check aborts the whole batch on it. Mirrored from
+  // `_quantityPattern` in bulk_add_screen.dart; if that loosens, so can this.
+  final fieldPattern = RegExp(r'^\d+([.,]\d{0,2})?$');
+
+  test(
+    'a pound quantity is prefilled at a precision the field accepts',
+    () async {
+      final bloc = blocWith({
+        'mince': [meal('Mince')],
+      });
+
+      // 1 lb is 453.59237 g. Prefilled verbatim it was unloggable.
+      final state = await parse(bloc, '1 lb mince');
+
+      expect(state.rows.single.amountText, '453.59');
+      expect(state.rows.single.unit, 'g');
+      expect(fieldPattern.hasMatch(state.rows.single.amountText), isTrue);
+    },
+  );
+
+  test('one refused row no longer blocks the rows beside it', () async {
+    final bloc = blocWith({
+      'mince': [meal('Mince')],
+      'rice': [meal('Rice')],
+    });
+
+    final state = await parse(bloc, '1 lb mince, 100 g rice');
+
+    expect(state.rows, hasLength(2));
+    for (final row in state.rows) {
+      expect(
+        fieldPattern.hasMatch(row.amountText),
+        isTrue,
+        reason: '${row.meal?.name}: "${row.amountText}" fails the submit '
+            'check, and that check refuses the entire batch',
+      );
+    }
+  });
+
+  test('a serving weight carrying more decimals is rounded too', () async {
+    final bloc = blocWith({
+      'almonds': [
+        meal('Almonds', servingQuantity: 226.796185, servingUnit: 'g'),
+      ],
+    });
+
+    final state = await parse(bloc, 'almonds');
+
+    expect(state.rows.single.amountText, '226.8');
+    expect(fieldPattern.hasMatch(state.rows.single.amountText), isTrue);
+  });
+
+  test(
+    'a positive quantity below the rounding floor does not become zero',
+    () async {
+      final bloc = blocWith({
+        'saffron': [meal('Saffron', servingQuantity: 0.004, servingUnit: 'g')],
+      });
+
+      final state = await parse(bloc, 'saffron');
+
+      // Rounding alone gives "0.00", which the submit check rejects for
+      // being non-positive — the same dead end by the other route.
+      expect(state.rows.single.amountText, '0.01');
+      expect(fieldPattern.hasMatch(state.rows.single.amountText), isTrue);
+    },
+  );
+
   test('changing candidates falls back from unsupported units', () async {
     final bloc = blocWith({
       'drink': [

test/unit_test/plaintext_destination_guard_test.dart

@@ -168,6 +168,87 @@ void main() {
           InternetAddressType.IPv6);
     });
 
+    // A zone id is what makes a link-local address routable on a host with
+    // more than one interface, and it is the case the guard was silently
+    // breaking: `Uri` escapes the `%` to `%25`, `InternetAddress.tryParse`
+    // refuses that, and the address fell through to a DNS lookup that cannot
+    // succeed — so a link-local server this check exists to *permit* was
+    // reported unreachable, while an unguarded client reached it fine.
+    //
+    // Two things make these tests fussier than they look:
+    //
+    // - `tryParse` resolves a **named** zone against the running machine's
+    //   interfaces, so a hardcoded `%eth0` is null on a host without an eth0.
+    //   The name is asked of the machine instead.
+    // - A **numeric** zone tests nothing at all: `%1` escapes to `%251`,
+    //   which parses happily as scope 251, so the guard never stumbled. Only
+    //   a named zone reproduces the failure.
+    //
+    // There is deliberately no "zoned public address" case: `tryParse`
+    // rejects a zone on anything that is not link-local (`2001:db8::1%lo` is
+    // null), so such a URL was never reaching this branch to begin with.
+    Future<String?> anInterfaceName() async {
+      final interfaces = await NetworkInterface.list(
+        includeLoopback: true,
+        type: InternetAddressType.any,
+      );
+      return interfaces.isEmpty ? null : interfaces.first.name;
+    }
+
+    /// Fails rather than resolving. A zoned literal must be settled by the
+    /// literal branch; reaching the resolver *is* the bug.
+    PlaintextDestinationGuard guardThatMustNotResolve() =>
+        PlaintextDestinationGuard(
+          lookup: (host) async => fail('a zoned literal reached the resolver: $host'),
+        );
+
+    test('a zoned link-local literal is allowed, and left alone', () async {
+      final zone = await anInterfaceName();
+      if (zone == null) {
+        markTestSkipped('no network interface to name a zone with');
+        return;
+      }
+      final url = Uri.parse('http://[fe80::1%$zone]:11434/v1/chat/completions');
+      // What the guard actually sees, and what `tryParse` answers to it.
+      expect(url.host, 'fe80::1%25$zone');
+      expect(InternetAddress.tryParse(url.host), isNull);
+
+      // Returned untouched: the socket layer does its own unescaping, and
+      // rewriting the host here would only cost the zone.
+      expect(await guardThatMustNotResolve().approve(url), url);
+    });
+
+    test('the typed % and the RFC 6874 %25 are the same destination', () async {
+      // Nobody needs to know the RFC to hit this — both spellings land on the
+      // same escaped host, so the plain one cannot behave differently.
+      final zone = await anInterfaceName();
+      if (zone == null) {
+        markTestSkipped('no network interface to name a zone with');
+        return;
+      }
+      final typed = Uri.parse('http://[fe80::1%$zone]:11434/v1');
+      final escaped = Uri.parse('http://[fe80::1%25$zone]:11434/v1');
+      expect(typed.host, escaped.host);
+
+      final guard = guardThatMustNotResolve();
+      expect(await guard.approve(typed), typed);
+      expect(await guard.approve(escaped), escaped);
+    });
+
+    test('a hostname is never rewritten by the zone handling', () async {
+      // The `25` comes off only when it directly follows the first `%`, so a
+      // name still reaches the resolver exactly as it was typed.
+      final seen = <String>[];
+      final guard = PlaintextDestinationGuard(lookup: (host) async {
+        seen.add(host);
+        return [_v4('192.168.1.5')];
+      });
+
+      await guard.approve(Uri.parse('http://ollama.lan:11434/v1'));
+
+      expect(seen, ['ollama.lan']);
+    });
+
     test('a name that does not resolve is not a policy refusal', () async {
       // Telling someone their address is unsafe when it is merely
       // unreachable sends them to fix the wrong thing.