What changed, and why it matters
This commit fixes a CI hardware-test script that was pointing to the wrong firmware file path for the original Trezor (T1B1). It also cleans up how the test launches the firmware-update command and checks whether it succeeded. There is no direct evidence this is a security fix for users; it appears to be a test-infrastructure repair.
No urgent action. If reviewing this code, consider validating or quoting the `file` argument to `headertool` and `trezorctl` to reduce shell-injection risk in the test harness, though this is internal CI tooling.
Security signals we found
Command construction uses shell=True with a user-supplied file path in `run(f"headertool {file}", shell=True, check=True)` and `Popen(trezorctlcmd, shell=True)`
Return-code checking was added, improving failure detection in the test harness
No product firmware or cryptographic code is changed
Evidence from the diff
The change updates ci/hardware_tests/t1_hw_test.sh to use the correct relative path ../../legacy/firmware/firmware-T1B1-*.bin instead of the stale ../../firmware-T1*.bin. In ci/hardware_tests/device/legacy.py, the update_firmware method now runs headertool on the supplied file, invokes trezorctl firmware-update via Popen instead of a backgrounded shell command, and explicitly checks the process return code. These are CI/test-harness robustness improvements.
Changed components
ci/hardware_tests/device/legacy.pyci/hardware_tests/t1_hw_test.shInspect captured patch +16 / −4
diff --git a/ci/hardware_tests/device/legacy.py b/ci/hardware_tests/device/legacy.py
index 83df789d..105ccea1 100644
--- a/ci/hardware_tests/device/legacy.py
+++ b/ci/hardware_tests/device/legacy.py
@@ -1,3 +1,5 @@
+from subprocess import Popen, run
+
import serial
from .device import Device
@@ -15,12 +17,13 @@ class TrezorOne(Device):
def update_firmware(self, file=None):
if file:
+ run(f"headertool {file}", shell=True, check=True) # check if file exists
unofficial = True
- trezorctlcmd = f"firmware-update -s -f {file} &"
+ trezorctlcmd = f"trezorctl firmware-update -s -f {file}"
self.log(f"[software] Updating the firmware to {file}")
else:
unofficial = False
- trezorctlcmd = "firmware-update &"
+ trezorctlcmd = "trezorctl firmware-update"
self.log("[software] Updating the firmware to latest")
self.wait(3)
self._enter_bootloader()
@@ -28,7 +31,9 @@ class TrezorOne(Device):
self.wait(3)
self.check_model("Trezor 1 bootloader")
- self.run_trezorctl(trezorctlcmd)
+ self.log(f"[software/trezorctl] Running '{trezorctlcmd}' in background")
+ process = Popen(trezorctlcmd, shell=True)
+
self.wait(3)
self.touch("right", "click")
self.wait(30)
@@ -42,6 +47,13 @@ class TrezorOne(Device):
self.wait(5)
self.touch("right", "click")
self.wait(15)
+
+ if process.poll() is None:
+ process.kill()
+ process.wait(timeout=1)
+ if process.returncode != 0:
+ raise RuntimeError(f"{trezorctlcmd} failed: {process.returncode}")
+
print(self.check_model("Trezor 1"))
def _enter_bootloader(self):
diff --git a/ci/hardware_tests/t1_hw_test.sh b/ci/hardware_tests/t1_hw_test.sh
index 16a7910a..c4796e1c 100755
--- a/ci/hardware_tests/t1_hw_test.sh
+++ b/ci/hardware_tests/t1_hw_test.sh
@@ -15,5 +15,5 @@ set -x # trace commands
./record_video.sh ${T1_CAMERA} ${SHA} start
(cd ../.. && uv sync)
#uv run python bootstrap.py T1B1 # install official firmware first
-uv run python bootstrap.py T1B1 ../../firmware-T1*.bin
+uv run python bootstrap.py T1B1 ../../legacy/firmware/firmware-T1B1-*.bin
uv run pytest ../../tests/device_tests
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.