Merge pull request #990 from kdmukai/bugfix_image_entropy_held_button
What changed, and why it matters
This commit fixes a user-interface bug in SeedSigner's 'image entropy' feature, where a user holding down a button could accidentally skip the live camera preview and the final photo review. Because the generated seed's randomness comes from the photo, skipping the review could let a user unknowingly accept a poor or unintended image. The patch now waits until the button is released before accepting the next press, and adds tests to confirm the behavior.
Reviewers should verify that the release-detection logic covers all hardware button combinations used on supported devices, and that the new tests run green. Consider whether similar held-button patterns exist in other screen flows (e.g., seed entry, signing approval) and apply the same fix if needed.
Security signals we found
UI state machine ignores stale held button input
Prevents unintended final entropy capture from a held button
Prevents auto-acceptance of final entropy image before user review
Adds regression tests for critical entropy-generation interaction flow
Evidence from the diff
The change modifies ToolsImageEntropyLivePreviewScreen and ToolsImageEntropyFinalImageScreen in tools_screens.py. It introduces an is_maybe_still_holding flag so that an ANYCLICK button still held from entering the screen is ignored until all click buttons are observed released; only then does a fresh press capture the final entropy frame. It also adds a release-wait loop before the final-image review wait_for, preventing the same held press from auto-accepting the captured image. A new test file exercises held-button, fresh-click, back-exit, and review-release scenarios with mocked hardware inputs and camera.
Changed components
src/seedsigner/gui/screens/tools_screens.pytests/test_tools_screens.pyToolsImageEntropyLivePreviewScreenToolsImageEntropyFinalImageScreenInspect captured patch +226 / −2
### src/seedsigner/gui/screens/tools_screens.py
@@ -35,6 +35,12 @@ def _run(self):
max_entropy_frames = 50
instructions_font = Fonts.get_font(GUIConstants.get_body_font_name(), GUIConstants.get_button_font_size())
+ # If the user continues holding the button that brought them into this flow, we
+ # have to ensure that it doesn't trigger the ANYCLICK check below, otherwise they
+ # will inadvertently skip all the preview frames and take the final image
+ # immediately.
+ is_maybe_still_holding = True
+
while True:
if self.hw_inputs.check_for_low(HardwareButtonsConstants.KEY_LEFT):
# Have to manually update last input time since we're not in a wait_for loop
@@ -75,8 +81,20 @@ def _run(self):
self.renderer.canvas.paste(frame.crop(box=box))
- # Check for ANYCLICK to take final entropy image
- if self.hw_inputs.check_for_low(keys=HardwareButtonsConstants.KEYS__ANYCLICK):
+ # If the ANYCLICK buttons are detected as being all released (none of them
+ # cause check_for_low to return True), we can be sure that the user isn't
+ # still holding down the initial button press that brought them into this
+ # flow. It is then safe to arm the loop to trigger the final image capture for
+ # whenever the *next* ANYCLICK button is pressed.
+ if not self.hw_inputs.check_for_low(keys=HardwareButtonsConstants.KEYS__ANYCLICK):
+ # Confirmed that all ANYCLICK buttons are released. The next click can now
+ # trigger the final image capture.
+ is_maybe_still_holding = False
+
+ elif not is_maybe_still_holding:
+ # We passed the above check; this is a fresh, explicit click. We can now
+ # capture the final image and exit this loop.
+
# Have to manually update last input time since we're not in a wait_for loop
self.hw_inputs.update_last_input_time()
self.camera.stop_video_stream_mode()
@@ -151,6 +169,13 @@ def _run(self):
)
self.renderer.show_image()
+ # The button click that triggered the final image might still be held down as this
+ # screen appears. We can't let that held button auto-dismiss the final image
+ # review here. Wait until every button has been released before listening for the
+ # accept/reshoot decision.
+ while self.hw_inputs.check_for_low(keys=[HardwareButtonsConstants.KEY_LEFT, HardwareButtonsConstants.KEY_RIGHT] + HardwareButtonsConstants.KEYS__ANYCLICK):
+ time.sleep(0.01)
+
# LEFT = reshoot, RIGHT / ANYCLICK = accept
input = self.hw_inputs.wait_for([HardwareButtonsConstants.KEY_LEFT, HardwareButtonsConstants.KEY_RIGHT] + HardwareButtonsConstants.KEYS__ANYCLICK)
if input == HardwareButtonsConstants.KEY_LEFT:
### tests/test_tools_screens.py
@@ -0,0 +1,199 @@
+import os
+
+from unittest.mock import MagicMock, patch
+
+from PIL import Image
+
+# Must import test base before the Controller
+from base import BaseTest
+
+from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON
+from seedsigner.hardware.camera import Camera
+
+
+"""
+ We don't test other Screens; they mostly have simple UI nav, text entry, or button
+ select behavior. But the image entropy screens have critical user interaction review
+ checks that could impact the security of the generated seed. So explicit tests of
+ those interactions are warranted.
+
+ These tests have to simulate user button press sequences AND duration at various
+ stages of the image entropy live review loop and final image review.
+
+ These tests are not concerned with actual image content, entropy, etc.
+"""
+
+
+def make_noise_frame(width: int = 240, height: int = 240) -> Image.Image:
+ # Random bytes are fine here: tests only need frames that are non-blank and
+ # distinct from one another; nothing here feeds real entropy.
+ return Image.frombytes("RGBA", (width, height), os.urandom(width * height * 4))
+
+
+def make_mock_camera(frames: list) -> MagicMock:
+ """
+ Returns a mocked Camera whose read_video_stream() plays back the given frames in
+ order.
+ """
+ camera = MagicMock()
+ frame_feed = list(frames)
+
+ def read_video_stream(as_image: bool = False):
+ if not frame_feed:
+ raise AssertionError("Camera frame script exhausted; the screen loop should have exited by now")
+ return frame_feed.pop(0)
+
+ camera.read_video_stream.side_effect = read_video_stream
+ return camera
+
+
+def make_mock_hw_inputs(left_script: list = None, anyclick_script: list = None) -> MagicMock:
+ """
+ Returns a mocked HardwareButtons whose check_for_low() plays back scripted
+ responses.
+
+ There are two types of input checks:
+ * check_for_low(specific_key_constant). e.g. was KEY_LEFT (back) pressed?
+
+ * check_for_low(keys=list_of_keys). e.g. was ANYCLICK pressed? (any of the click buttons)
+ """
+ left_feed = list(left_script or [])
+ anyclick_feed = list(anyclick_script or [])
+ hw_inputs = MagicMock()
+
+ def check_for_low(key=None, keys=None):
+ if keys is None:
+ return left_feed.pop(0) if left_feed else False
+ return anyclick_feed.pop(0) if anyclick_feed else False
+
+ hw_inputs.check_for_low.side_effect = check_for_low
+ return hw_inputs
+
+
+
+class ImageEntropyScreenTestBase(BaseTest):
+
+ def setup_method(self):
+ super().setup_method()
+
+ from seedsigner.gui.renderer import Renderer
+
+ # tests/base.py mocks the whole renderer module; give each test a fresh renderer
+ # whose canvas dims are real ints (the screens do math on them). Exposed as a
+ # patch so screen construction restores the original when its `with` exits.
+ self.mock_renderer = MagicMock()
+ self.mock_renderer.canvas_width = 240
+ self.mock_renderer.canvas_height = 240
+ self.renderer_patch = patch.object(Renderer, "get_instance", return_value=self.mock_renderer)
+
+
+
+class TestToolsImageEntropyLivePreviewScreen(ImageEntropyScreenTestBase):
+
+ def build_screen(self, mock_camera: MagicMock, mock_hw_inputs: MagicMock):
+ from seedsigner.gui.screens.tools_screens import ToolsImageEntropyLivePreviewScreen
+
+ # Run within our mocked Renderer context
+ with self.renderer_patch:
+ with patch.object(Camera, "get_instance", return_value=mock_camera):
+ screen = ToolsImageEntropyLivePreviewScreen()
+
+ screen.hw_inputs = mock_hw_inputs
+ return screen
+
+
+ def test_button_held_from_start_never_captures_until_released(self):
+ """
+ A button already held down when the screen starts must not trigger the final
+ image capture; only a fresh press after all buttons have been seen released
+ may capture.
+ """
+ frames = [make_noise_frame() for i in range(5)]
+ mock_camera = make_mock_camera(frames)
+
+ # Button is held for the first two loop passes, released on the third, then
+ # pressed again on the fourth.
+ mock_hw_inputs = make_mock_hw_inputs(anyclick_script=[True, True, False, True])
+ screen = self.build_screen(mock_camera, mock_hw_inputs)
+
+ # The screen returns the live preview frames
+ result = screen._run()
+
+ # The held presses were ignored; the fresh press on the fourth pass captured.
+ # Three frames were collected before the capture pass.
+ assert result == frames[:3]
+ mock_camera.stop_video_stream_mode.assert_called_once()
+
+
+ def test_click_after_release_captures(self):
+ """ Normal use: no buttons pressed at first, then a click captures. """
+ frames = [make_noise_frame() for i in range(2)]
+ mock_camera = make_mock_camera(frames)
+ mock_hw_inputs = make_mock_hw_inputs(anyclick_script=[False, True])
+ screen = self.build_screen(mock_camera, mock_hw_inputs)
+
+ result = screen._run()
+
+ assert result == frames[:1]
+ mock_camera.stop_video_stream_mode.assert_called_once()
+
+
+ def test_back_button_exits_immediately(self):
+ """ KEY_LEFT backs out at any time, even before any frame is read. """
+ mock_camera = make_mock_camera([])
+ mock_hw_inputs = make_mock_hw_inputs(left_script=[True])
+ screen = self.build_screen(mock_camera, mock_hw_inputs)
+
+ result = screen._run()
+
+ assert result == RET_CODE__BACK_BUTTON
+ mock_camera.stop_video_stream_mode.assert_called_once()
+ mock_camera.read_video_stream.assert_not_called()
+
+
+class TestToolsImageEntropyFinalImageScreen(ImageEntropyScreenTestBase):
+
+ def build_screen(self, mock_hw_inputs: MagicMock):
+ from seedsigner.gui.screens.tools_screens import ToolsImageEntropyFinalImageScreen
+
+ # Run within our mocked Renderer context
+ with self.renderer_patch:
+ screen = ToolsImageEntropyFinalImageScreen(final_image=MagicMock())
+ screen.hw_inputs = mock_hw_inputs
+ return screen
+
+
+ def test_held_button_must_be_released_before_review_input(self):
+ """
+ The click that captured the photo can still be held down when the review
+ screen appears; it must be released before accept/reshoot input is read, so
+ one long press can never accept the photo sight unseen.
+ """
+ from seedsigner.hardware.buttons import HardwareButtonsConstants
+
+ # Button held for two polls, released on the third; only then is the real
+ # accept/reshoot decision awaited.
+ mock_hw_inputs = make_mock_hw_inputs(anyclick_script=[True, True, False])
+ mock_hw_inputs.wait_for.return_value = HardwareButtonsConstants.KEY_LEFT
+ screen = self.build_screen(mock_hw_inputs)
+
+ # Screen returns the back button code when the user chooses to reshoot (KEY_LEFT).
+ result = screen._run()
+
+ assert mock_hw_inputs.check_for_low.call_count == 3
+ mock_hw_inputs.wait_for.assert_called_once()
+ assert result == RET_CODE__BACK_BUTTON
+
+
+ def test_accept_returns_none_to_advance(self):
+ """ A (fresh) accept click falls through: the screen returns None. """
+ from seedsigner.hardware.buttons import HardwareButtonsConstants
+
+ mock_hw_inputs = make_mock_hw_inputs(anyclick_script=[False])
+ mock_hw_inputs.wait_for.return_value = HardwareButtonsConstants.KEY_RIGHT
+ screen = self.build_screen(mock_hw_inputs)
+
+ result = screen._run()
+
+ assert result is None
+ mock_hw_inputs.wait_for.assert_called_once()Why this scored 60/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.