Harden terminal retries and OTA cleanup

docs/lora_ota_automation.md

@@ -282,8 +282,8 @@ while its raw USB CLI is used to start TempRadio:
 
 If the source is already on the exact TempRadio tuple through a scheduled or
 manual operation, `--source-already-temp` lets a TCP source run without a raw
-CLI link. The script cannot verify or extend that source window, so leave a
-comfortable time margin.
+CLI link. The script cannot verify, extend, or shorten that source window, so
+leave a comfortable time margin.
 
 Use `--controller-baud` or `--source-baud` only for a build whose corresponding
 interface is genuinely configured to another speed.
@@ -303,14 +303,18 @@ files differ, select one explicitly:
 If no ready mOTA is usable, it searches `.bin` and `.hex` members for a valid,
 matching `EndF`, then builds the platform-appropriate container. Every result
 is structurally checked by the runner and independently passed through
-`motatool verify` before any radio changes.
+`motatool verify` before any radio changes. Direct firmware and mOTA inputs, as
+well as individual ZIP members, are rejected above 64 MiB before being loaded.
 
 Useful controls:
 
 - `--public-key signer.key.pub` requires a particular Ed25519 signer during
   verification.
 - `--sign-key signer.key` signs a newly built container.
-- `--no-install` downloads and verifies the image but leaves it staged.
+- `--no-install` downloads and verifies the image but leaves it staged. By
+  default the runner then schedules the target, relays, and a script-configured
+  source back to their normal radios. Combining it with
+  `--leave-controller-radio` deliberately preserves the TempRadio topology.
 - `--allow-non-upgrade` deliberately permits the same or an older version.
 - `--replace-active-download` deliberately discards a different update already
   downloading or staged on the target. Without it, that update is preserved.
@@ -348,11 +352,15 @@ the destination.
    discovery, the transfer timeout, final polling, and install checks.
 6. Start `motatool serve`, discover the exact eight-hex manifest ID, request
    `ota pull <id> flash`, and poll until that same ID reports ready. A seeder
-   process exit stops the run immediately.
+   process exit stops the run immediately. For `--no-install`, schedule all
+   script-controlled nodes back to their normal radios before restoring the
+   controller, unless `--leave-controller-radio` was requested.
 7. Recheck that exact ID, give the target a short final TempRadio safety window,
    and request `ota install`. Then shorten each relay's TempRadio window so the
-   normal multi-hop route returns, restore the controller, wait for reboot, and
-   require the new running identity and exact package version.
+   normal multi-hop route returns, stop the seeder, shorten the source window,
+   restore the controller, wait for reboot, and require the new running identity
+   and exact package version. A source supplied with `--source-already-temp` is
+   never modified.
    `--leave-controller-radio` moves the controller back to TempRadio only after
    this normal-channel verification.
 
@@ -391,10 +399,11 @@ served mOTA, `motatool-serve.log`, extracted build inputs when needed, and
 
 ## Interruption and recovery
 
-Ctrl-C stops the seeder, detaches its serial folder, and attempts to restore
-the controller. The target and relays remain on TempRadio only until their
-bounded windows end; rebooting also restores their saved radio settings. A
-partial download remains safe. Once the target is reachable again (after its
+Ctrl-C stops the seeder, detaches its serial folder, makes one best-effort
+request to shorten a source TempRadio window started by the script, and attempts
+to restore the controller. The target and relays remain on TempRadio only until
+their bounded windows end; rebooting also restores their saved radio settings.
+A partial download remains safe. Once the target is reachable again (after its
 TempRadio window ends, or after putting the controller back on that tuple),
 rerunning the same package recognizes its manifest ID and resumes the existing
 session instead of clearing it.

examples/companion_radio/MyMesh.cpp

@@ -3212,10 +3212,18 @@ ContactInfo* MyMesh::getTerminalRecipient() {
 void MyMesh::rememberTerminalAck(ContactInfo& recipient, const char* text,
                                  uint32_t message_timestamp, uint32_t expected_ack,
                                  uint32_t est_timeout,
-                                 const uint8_t packet_retry_key[MAX_HASH_SIZE]) {
-  if (expected_ack == 0) return;
+                                 const uint8_t packet_retry_key[MAX_HASH_SIZE],
+                                 AckTableEntry* replacement_entry) {
+  if (expected_ack == 0) {
+    if (replacement_entry != NULL) {
+      clearExpectedAck(*replacement_entry, false);
+    }
+    return;
+  }
 
-  AckTableEntry& entry = expected_ack_table[next_ack_idx];
+  AckTableEntry& entry = replacement_entry != NULL
+      ? *replacement_entry
+      : expected_ack_table[next_ack_idx];
   clearExpectedAck(entry, false);
   entry.msg_sent = _ms->getMillis();
   entry.expires_at = futureMillis(est_timeout);
@@ -3227,7 +3235,9 @@ void MyMesh::rememberTerminalAck(ContactInfo& recipient, const char* text,
                       (const uint8_t*)text, strlen(text));
   memcpy(entry.retry_key, packet_retry_key, sizeof(entry.retry_key));
   entry.terminal_origin = true;
-  next_ack_idx = (next_ack_idx + 1) % EXPECTED_ACK_TABLE_SIZE;
+  if (replacement_entry == NULL) {
+    next_ack_idx = (next_ack_idx + 1) % EXPECTED_ACK_TABLE_SIZE;
+  }
   expireExpectedAcks();
 }
 
@@ -3289,11 +3299,8 @@ void MyMesh::handleTerminalCommand(char* command) {
       if (result == MSG_SEND_FAILED) {
         Serial.print("  ERROR: unable to send\r\n");
       } else {
-        if (replacement_entry != NULL) {
-          clearExpectedAck(*replacement_entry, false);
-        }
         rememberTerminalAck(*recipient, text, message_timestamp, expected_ack,
-                            est_timeout, packet_retry_key);
+                            est_timeout, packet_retry_key, replacement_entry);
         Serial.printf("  message sent - %s\r\n",
                       result == MSG_SEND_SENT_FLOOD ? "FLOOD" : "DIRECT");
       }

examples/companion_radio/MyMesh.h

@@ -263,10 +263,6 @@ private:
 #ifdef ENABLE_USB_INTERFACE
   ContactInfo* getTerminalRecipient();
   void importTerminalCard(char* command);
-  void rememberTerminalAck(ContactInfo& recipient, const char* text,
-                           uint32_t message_timestamp, uint32_t expected_ack,
-                           uint32_t est_timeout,
-                           const uint8_t packet_retry_key[MAX_HASH_SIZE]);
 #endif
   bool isValidClientRepeatFreq(uint32_t f) const;
   bool hasLocationTelemetryRecipient();
@@ -369,6 +365,13 @@ private:
   void expireExpectedAcks();
   AckTableEntry* findPendingTextMessage(
       const uint8_t text_fingerprint[MAX_HASH_SIZE], uint32_t message_timestamp);
+#ifdef ENABLE_USB_INTERFACE
+  void rememberTerminalAck(ContactInfo& recipient, const char* text,
+                           uint32_t message_timestamp, uint32_t expected_ack,
+                           uint32_t est_timeout,
+                           const uint8_t packet_retry_key[MAX_HASH_SIZE],
+                           AckTableEntry* replacement_entry);
+#endif
 
   #define ADVERT_PATH_TABLE_SIZE   16
   AdvertPath advert_paths[ADVERT_PATH_TABLE_SIZE]; // circular table

tools/lora_ota/lora_ota.py

@@ -54,8 +54,8 @@ TRANSMISSION_RETRY_WINDOW_SECONDS = 90
 TRANSMISSION_RETRY_DELAY_SECONDS = 1
 TRANSMISSION_PROMPT_SECONDS = 10
 TEMP_RADIO_SWITCH_DELAY_SECONDS = 3
-POST_INSTALL_RELAY_MINUTES = 1
-POST_INSTALL_RELAY_MARGIN_SECONDS = 15
+TEMP_RADIO_RETURN_MINUTES = 1
+TEMP_RADIO_RETURN_MARGIN_SECONDS = 15
 INSTALL_TARGET_WINDOW_MINUTES = 3
 # Firmware may hold the apply reboot for up to 15 seconds while its reply
 # drains. Do not interpret "still ready" as a failed install before that cap.
@@ -446,8 +446,25 @@ def parse_intel_hex(raw: bytes) -> bytes:
     return bytes(image)
 
 
+def read_bounded_file(path: Path, max_size: int, description: str) -> bytes:
+    try:
+        size = path.stat().st_size
+    except OSError as exc:
+        raise OtaError(f"cannot inspect {description} {path}: {exc}") from exc
+    if size > max_size:
+        raise OtaError(f"{description} is unexpectedly large: {path}")
+    try:
+        raw = path.read_bytes()
+    except OSError as exc:
+        raise OtaError(f"cannot read {description} {path}: {exc}") from exc
+    # Check again after reading in case the file changed between stat and read.
+    if len(raw) > max_size:
+        raise OtaError(f"{description} is unexpectedly large: {path}")
+    return raw
+
+
 def read_firmware_file(path: Path) -> bytes:
-    raw = path.read_bytes()
+    raw = read_bounded_file(path, MAX_FIRMWARE_IMAGE_SIZE, "firmware file")
     return parse_intel_hex(raw) if path.suffix.lower() == ".hex" else raw
 
 
@@ -594,7 +611,9 @@ def select_firmware_from_zip(
 def load_base_image(path: Path, target: TargetInfo) -> EndFInfo:
     suffix = path.suffix.lower()
     if suffix == ".mota":
-        info = parse_mota(path.read_bytes(), path)
+        info = parse_mota(
+            read_bounded_file(path, MAX_ARCHIVE_MEMBER_SIZE, "mOTA file"), path
+        )
         if not info.is_full:
             raise OtaError("--base mOTA must be a full-image container")
         identity = parse_endf(info.payload)
@@ -723,7 +742,9 @@ def prepare_package(
     new_identity: EndFInfo | None = None
 
     if source.suffix.lower() == ".mota":
-        selected_blob = source.read_bytes()
+        selected_blob = read_bounded_file(
+            source, MAX_ARCHIVE_MEMBER_SIZE, "mOTA file"
+        )
         selected = parse_mota(selected_blob, source)
     elif source.suffix.lower() == ".zip":
         try:
@@ -751,7 +772,9 @@ def prepare_package(
             raise OtaError(f"package is not installable on {target.name}: {reason}")
         output = served_dir / f"{selected.manifest_id.lower()}.mota"
         output.write_bytes(selected_blob if selected_blob is not None else selected.blob)
-        selected = parse_mota(output.read_bytes(), output)
+        selected = parse_mota(
+            read_bounded_file(output, MAX_ARCHIVE_MEMBER_SIZE, "mOTA file"), output
+        )
     else:
         if new_identity is None:
             raise OtaError("could not obtain firmware from the input package")
@@ -789,7 +812,9 @@ def prepare_package(
             command.extend(["--sign", str(args.sign_key.resolve())])
         result = run_checked(command, label="build mOTA", timeout=600)
         print(result.stdout.strip())
-        selected = parse_mota(output.read_bytes(), output)
+        selected = parse_mota(
+            read_bounded_file(output, MAX_ARCHIVE_MEMBER_SIZE, "mOTA file"), output
+        )
         good, reason = compatible_mota(selected, target)
         if not good:
             raise OtaError(f"newly built package is not installable: {reason}")
@@ -1664,10 +1689,10 @@ def shorten_relay_temp_windows(
 ) -> None:
     if not args.relay_values:
         return
-    command = temp_radio_command_for_minutes(args, POST_INSTALL_RELAY_MINUTES)
+    command = temp_radio_command_for_minutes(args, TEMP_RADIO_RETURN_MINUTES)
     print(
         f"[relays] scheduling return to the normal channel in "
-        f"{POST_INSTALL_RELAY_MINUTES} minute"
+        f"{TEMP_RADIO_RETURN_MINUTES} minute"
     )
     for relay_name, relay_password in args.relay_values:
         reply = controller.remote_command(
@@ -1676,6 +1701,42 @@ def shorten_relay_temp_windows(
         require_temp_radio_reply(relay_name, reply)
 
 
+def shorten_target_temp_window(
+    controller: Controller,
+    args: argparse.Namespace,
+) -> None:
+    command = temp_radio_command_for_minutes(args, TEMP_RADIO_RETURN_MINUTES)
+    print(
+        f"[destination] scheduling return to the normal channel in "
+        f"{TEMP_RADIO_RETURN_MINUTES} minute"
+    )
+    reply = controller.remote_command(args.target, command)
+    require_temp_radio_reply(args.target, reply)
+
+
+def shorten_source_temp_window(
+    args: argparse.Namespace,
+    *,
+    check: bool = True,
+) -> bool:
+    if args.source_already_temp:
+        return True
+    command = temp_radio_command_for_minutes(args, TEMP_RADIO_RETURN_MINUTES)
+    print(
+        f"[source] scheduling return to the normal channel in "
+        f"{TEMP_RADIO_RETURN_MINUTES} minute"
+    )
+    output = source_cli_command(args, command, check=check)
+    if not output and not check:
+        print(
+            "[warn] could not shorten the OTA source TempRadio window; "
+            "it will return when its original bounded window ends",
+            file=sys.stderr,
+        )
+        return False
+    return True
+
+
 def verify_installed(
     controller: Controller,
     args: argparse.Namespace,
@@ -1937,6 +1998,7 @@ def main(argv: list[str] | None = None) -> int:
     controller_changed = False
     seeder: SeederProcess | None = None
     seeder_attempted = False
+    source_temp_owned = False
     password = args.password or os.environ.get("MESHCORE_ADMIN_PASSWORD", "")
     try:
         preflight_inputs(args)
@@ -1990,6 +2052,7 @@ def main(argv: list[str] | None = None) -> int:
             require_temp_radio_reply(relay_name, temp_reply)
         if not args.source_already_temp:
             source_cli_command(args, temp_command)
+            source_temp_owned = True
 
         temp_radio = RadioSettings(
             freq, bandwidth, sf, cr, original_radio.repeat
@@ -2006,6 +2069,19 @@ def main(argv: list[str] | None = None) -> int:
 
         if args.no_install:
             print(f"{args.target} is ready to install; leaving the verified update staged.")
+            if args.leave_controller_radio:
+                print(
+                    "[cleanup] preserving TempRadio windows because "
+                    "--leave-controller-radio was requested"
+                )
+            else:
+                shorten_target_temp_window(controller, args)
+                shorten_relay_temp_windows(controller, args)
+                seeder.stop()
+                seeder = None
+                if source_temp_owned:
+                    shorten_source_temp_window(args)
+                    source_temp_owned = False
             return 0
 
         install_confirmed = request_install(controller, args, package)
@@ -2025,10 +2101,13 @@ def main(argv: list[str] | None = None) -> int:
         # Stop seeding before returning the controller to its ordinary channel.
         seeder.stop()
         seeder = None
+        if source_temp_owned and not args.leave_controller_radio:
+            shorten_source_temp_window(args)
+            source_temp_owned = False
         controller.set_radio(original_radio, "restore controller radio for verification")
         controller_changed = False
         relay_wait = (
-            POST_INSTALL_RELAY_MINUTES * 60 + POST_INSTALL_RELAY_MARGIN_SECONDS
+            TEMP_RADIO_RETURN_MINUTES * 60 + TEMP_RADIO_RETURN_MARGIN_SECONDS
             if args.relay_values else 0
         )
         time.sleep(max(args.reboot_wait, relay_wait))
@@ -2057,6 +2136,8 @@ def main(argv: list[str] | None = None) -> int:
             # this is harmless on TCP and explicitly cleans the serial case.
             if seeder_attempted and args.source_serial:
                 source_cli_command(args, "ota folder off", check=False)
+            if source_temp_owned and not args.leave_controller_radio:
+                shorten_source_temp_window(args, check=False)
             if (
                 controller is not None
                 and controller_changed

tools/lora_ota/test_lora_ota.py

@@ -179,6 +179,19 @@ class FormatTests(unittest.TestCase):
         with self.assertRaisesRegex(ota.OtaError, "address span"):
             ota.parse_intel_hex(raw)
 
+    def test_oversized_direct_file_is_rejected_before_read(self) -> None:
+        with tempfile.TemporaryDirectory() as directory:
+            path = Path(directory) / "oversized.mota"
+            path.write_bytes(b"1234")
+            with (
+                mock.patch.object(
+                    Path, "read_bytes",
+                    side_effect=AssertionError("oversized file was read"),
+                ),
+                self.assertRaisesRegex(ota.OtaError, "unexpectedly large"),
+            ):
+                ota.read_bounded_file(path, 3, "mOTA file")
+
 
 class CompatibilityTests(unittest.TestCase):
     def setUp(self) -> None:
@@ -624,6 +637,52 @@ class ReliabilityTests(unittest.TestCase):
         ):
             ota.validate_args(args, parser)
 
+    def test_stage_cleanup_shortens_target_and_relays(self) -> None:
+        class Controller:
+            def __init__(self) -> None:
+                self.commands: list[tuple[str, str, str | None]] = []
+
+            def remote_command(
+                self, target_name: str, command: str, **kwargs: object
+            ) -> str:
+                password = kwargs.get("password")
+                self.commands.append((target_name, command, password))
+                return "OK - temp params for 1 mins"
+
+        controller = Controller()
+        args = argparse.Namespace(
+            target="remote",
+            relay_values=[("relay", "relay-secret")],
+            temp_values=(909.95, 250.0, 7, 5, 120),
+        )
+        ota.shorten_target_temp_window(controller, args)
+        ota.shorten_relay_temp_windows(controller, args)
+        self.assertEqual(
+            controller.commands,
+            [
+                ("remote", "tempradio 909.95,250,7,5,1", None),
+                ("relay", "tempradio 909.95,250,7,5,1", "relay-secret"),
+            ],
+        )
+
+    def test_source_cleanup_only_changes_a_script_owned_window(self) -> None:
+        args = argparse.Namespace(
+            source_already_temp=False,
+            temp_values=(909.95, 250.0, 7, 5, 120),
+        )
+        with mock.patch.object(
+            ota, "source_cli_command", return_value="OK - temp params for 1 mins"
+        ) as source_command:
+            self.assertTrue(ota.shorten_source_temp_window(args))
+        source_command.assert_called_once_with(
+            args, "tempradio 909.95,250,7,5,1", check=True
+        )
+
+        args.source_already_temp = True
+        with mock.patch.object(ota, "source_cli_command") as source_command:
+            self.assertTrue(ota.shorten_source_temp_window(args))
+        source_command.assert_not_called()
+
 
 class MotatoolIntegrationTests(unittest.TestCase):
     @classmethod