mirror of
https://github.com/kuhyx/screen-locker.git
synced 2026-11-10 19:07:16 +01:00
Make the lock overlay's sport selector usable, and focus its form fields
The manual-workout sport selector was a `tk.OptionMenu`, and while the screen
was locked it could not be changed. A posted Tk menu is a separate
override-redirect toplevel that takes the Tk grab for itself, and gatelock's
1s recovery tick attacks it from both sides: `_surfaces.enforce()` lifts the
lock surface over the menu, and `_reassert_grab()` read the grab sitting on
the menu as "the grab was lost" and yanked it back. The menu opened on click
and died within a second. On 2026-07-26 that forced an hour-long walk to be
logged through the table-tennis form, with the reason typed into `racket`.
Radio buttons instead. They are ordinary children of the surface, so they
lift with it and live inside the root's grab tree -- neither mechanism can
reach them. That is a structural guarantee, not a tuned workaround; the
alternative, teaching gatelock to skip its lift while a popup is posted,
would open a window where something could cover the lock. They are also the
first version of this control styled from the shared palette, where the
OptionMenu was default grey on the dark overlay.
Nothing on a lock surface may open a popup, so two halves enforce it. The
static half walks each module's AST for popup-creating names -- AST rather
than a source regex because the modules explain this bug in prose that names
the very widgets being banned. Its module list is derived ("imports tkinter",
minus a short reasoned exemption list) rather than hardcoded, so a new lock
screen is in scope the day it is written; a hardcoded list drafted today
would already have missed `_sick_dialog`. The behavioural half is
`scripts/verify_lock_popup_safety.py`: it builds the real form with real Tk,
replays both hostile behaviours for five ticks, and re-runs the original
failure so the mechanism stays demonstrated rather than described. It
re-execs itself under Xvfb -- a global grab on the developer's own session
would freeze it -- and a CI job runs it, because the pytest suite replaces
the whole `tk` module and so cannot host it.
Separately, no field ever took keyboard focus. Under a global grab that means
typing goes nowhere with no other window on screen to reveal why. The
manual-workout and sick-justification forms now each name their own first
field via a `focus=` flag; `on_focus_ready` genuinely has nothing to focus
(it fires before those screens exist) and its docstring now says that instead
of claiming the lock window has no typed input at all.
`scripts/fix_2026_07_26_sport.py` re-files that walk as sport "other",
rebuilt through the form's own draft/validate/build path so it carries
exactly the writer's key set, and re-signed. `start_time` and the date are
untouched so `workout_id` stays stable and the phone converges instead of
gaining a duplicate. Kept as the audit trail for the edit; the walk's details
come from the entry and `--details`, so nothing personal lands in a tracked
file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f617be72b4
commit
c0aae7d05b
22
.github/workflows/python-tests.yml
vendored
22
.github/workflows/python-tests.yml
vendored
@ -26,3 +26,25 @@ jobs:
|
||||
|
||||
- name: Run tests with coverage
|
||||
run: python -m pytest -v
|
||||
|
||||
# The pytest suite replaces the whole `tk` module so it can never reach a
|
||||
# display, which means the lock-screen widget behaviour it guards cannot be
|
||||
# asserted there. This job runs the real-Tk half against a throwaway X
|
||||
# server. See scripts/verify_lock_popup_safety.py.
|
||||
lock-popup-safety:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: "3.12"
|
||||
|
||||
- name: Install dependencies
|
||||
run: |
|
||||
sudo apt-get update
|
||||
sudo apt-get install -y xvfb python3-tk
|
||||
pip install -r requirements.txt
|
||||
|
||||
- name: Sport selector survives gatelock's recovery tick
|
||||
run: python3 scripts/verify_lock_popup_safety.py
|
||||
|
||||
2
.gitignore
vendored
2
.gitignore
vendored
@ -17,6 +17,8 @@ htmlcov/
|
||||
|
||||
# Runtime state files — personal workout/health data, never public.
|
||||
screen_locker/workout_log.json
|
||||
# ...including the timestamped backups a correction script leaves behind.
|
||||
screen_locker/*.json.bak-*
|
||||
screen_locker/sick_history.json
|
||||
screen_locker/extra_benefits_state.json
|
||||
screen_locker/scheduled_skips.json
|
||||
|
||||
@ -121,7 +121,9 @@ class ManualWorkoutDialogMixin(ManualWorkoutFormWidgetsMixin):
|
||||
|
||||
self._mw_section(form, "Basics")
|
||||
self._mw_sport_row(form)
|
||||
self._mw_vars["start_time"] = self._mw_entry(form, "Start time (HH:MM):")
|
||||
self._mw_vars["start_time"] = self._mw_entry(
|
||||
form, "Start time (HH:MM):", focus=True
|
||||
)
|
||||
self._mw_vars["end_time"] = self._mw_entry(form, "End time (HH:MM):")
|
||||
|
||||
self._mw_section(form, "Location & logistics")
|
||||
@ -195,7 +197,24 @@ class ManualWorkoutDialogMixin(ManualWorkoutFormWidgetsMixin):
|
||||
).pack(side="left", padx=10)
|
||||
|
||||
def _mw_sport_row(self, parent: tk.Widget) -> None:
|
||||
"""Add the sport-selector dropdown; swaps the activity-details section."""
|
||||
"""Add the sport-selector radio buttons; swaps the activity section.
|
||||
|
||||
CANONICAL ACCOUNT OF THE 2026-07-26 BUG -- other files point here.
|
||||
|
||||
This was a ``tk.OptionMenu``. A posted Tk menu is a separate
|
||||
override-redirect toplevel that takes the Tk grab for itself, and
|
||||
gatelock's 1 s recovery tick killed it within a second of opening:
|
||||
``_surfaces.enforce()`` lifts the lock surface over the menu, and
|
||||
``_reassert_grab()`` saw the grab sitting on the menu rather than the
|
||||
root and yanked it back. The selector stayed stuck on "Table tennis"
|
||||
while locked, and a walk got logged in the table-tennis form.
|
||||
|
||||
Radio buttons are ordinary children of the surface: they lift with it
|
||||
and live inside the root's grab tree, so neither mechanism can reach
|
||||
them. Nothing on a lock surface may open a popup window --
|
||||
``tests/test_no_popup_widgets.py`` enforces that statically and
|
||||
``scripts/verify_lock_popup_safety.py`` behaviourally.
|
||||
"""
|
||||
self._mw_sport_var = tk.StringVar(
|
||||
value=_manual_workout.SPORT_LABELS[_manual_workout.SPORT_TABLE_TENNIS]
|
||||
)
|
||||
@ -207,12 +226,25 @@ class ManualWorkoutDialogMixin(ManualWorkoutFormWidgetsMixin):
|
||||
fg=self._colors.fg,
|
||||
bg=self._colors.bg,
|
||||
).pack(side="left", padx=5)
|
||||
tk.OptionMenu(
|
||||
row,
|
||||
self._mw_sport_var,
|
||||
*_manual_workout.SPORT_LABELS.values(),
|
||||
command=self._on_mw_sport_changed,
|
||||
).pack(side="left", padx=5)
|
||||
for label in _manual_workout.SPORT_LABELS.values():
|
||||
tk.Radiobutton(
|
||||
row,
|
||||
text=label,
|
||||
value=label,
|
||||
variable=self._mw_sport_var,
|
||||
# Radiobutton's command takes no argument -- bind the label.
|
||||
command=lambda chosen=label: self._on_mw_sport_changed(chosen),
|
||||
font=(self._colors.font_family, 16),
|
||||
fg=self._colors.fg,
|
||||
bg=self._colors.bg,
|
||||
activeforeground=self._colors.fg,
|
||||
activebackground=self._colors.bg,
|
||||
# Without selectcolor the indicator is a white blob on the dark
|
||||
# overlay; the OptionMenu was the one unstyled widget in this
|
||||
# form, which likely added to it reading as dead.
|
||||
selectcolor=self._colors.field_bg,
|
||||
highlightthickness=0,
|
||||
).pack(side="left", padx=5)
|
||||
self._mw_grid(parent, row, full=True)
|
||||
|
||||
def _on_mw_sport_changed(self, selected_label: str) -> None:
|
||||
|
||||
@ -74,12 +74,21 @@ class ManualWorkoutFormWidgetsMixin:
|
||||
)
|
||||
self._mw_grid(parent, label, full=True)
|
||||
|
||||
def _mw_entry(self, parent: tk.Widget, label: str) -> tk.StringVar:
|
||||
def _mw_entry(
|
||||
self, parent: tk.Widget, label: str, *, focus: bool = False
|
||||
) -> tk.StringVar:
|
||||
"""Add a half-width label+entry cell and return its backing StringVar.
|
||||
|
||||
Builds its own cell inline (rather than reusing ``_add_label_entry``,
|
||||
which is pack-based and shared with the sick dialog) so it slots into
|
||||
the two-column grid. Paste stays disabled via ``disable_paste``.
|
||||
|
||||
Args:
|
||||
parent: The grid master to place the cell in.
|
||||
label: Text shown above the entry.
|
||||
focus: Give this entry the keyboard focus; see
|
||||
``_ui_widgets.UIWidgetsMixin._add_label_entry`` for why exactly
|
||||
one field per form should ask.
|
||||
"""
|
||||
var = tk.StringVar()
|
||||
cell = tk.Frame(parent, bg=self._colors.bg)
|
||||
@ -101,6 +110,8 @@ class ManualWorkoutFormWidgetsMixin:
|
||||
)
|
||||
entry.pack(fill="x", pady=4)
|
||||
disable_paste(entry)
|
||||
if focus:
|
||||
entry.focus_set()
|
||||
self._mw_grid(parent, cell)
|
||||
return var
|
||||
|
||||
|
||||
@ -103,6 +103,7 @@ class SickDialogMixin:
|
||||
parent,
|
||||
label="Symptom (e.g. fever, nausea):",
|
||||
variable=self._sick_symptom_var,
|
||||
focus=True,
|
||||
)
|
||||
self._add_label_entry(
|
||||
parent,
|
||||
|
||||
@ -132,12 +132,18 @@ class UIWidgetsMixin:
|
||||
*,
|
||||
label: str,
|
||||
variable: tk.StringVar,
|
||||
focus: bool = False,
|
||||
) -> None:
|
||||
"""Add a label + single-line entry pair on every monitor.
|
||||
|
||||
All copies share one ``variable``, so typing on any screen is the
|
||||
same text everywhere and reading it back needs no idea which monitor
|
||||
the user actually used.
|
||||
|
||||
``focus`` gives the primary surface's copy the keyboard focus. Exactly
|
||||
one field per screen should ask: the lock takes a global grab, so with
|
||||
nothing focused the user's typing goes nowhere and there is no other
|
||||
window on screen to make that obvious.
|
||||
"""
|
||||
row = parent.child_frame(bg=self._colors.bg)
|
||||
row.pack(pady=8, fill="x")
|
||||
@ -161,3 +167,5 @@ class UIWidgetsMixin:
|
||||
entries.pack(side="top", anchor="w", pady=4)
|
||||
for entry in entries:
|
||||
disable_paste(entry)
|
||||
if focus:
|
||||
entries.first.focus_set()
|
||||
|
||||
@ -73,7 +73,13 @@ class WindowSetupMixin:
|
||||
self.container.discard(surface.output_name)
|
||||
|
||||
def on_focus_ready(self, surface: SurfaceInfo | None) -> None:
|
||||
"""No typed-input field in the lock window; nothing to focus.
|
||||
"""Nothing to focus yet: the lock's first screen has no input field.
|
||||
|
||||
This fires once, when the lock is mapped and grabbed, and at that
|
||||
moment the screen is the phone-check status -- buttons only. The
|
||||
screens that DO take typed input (the manual-workout form, the sick
|
||||
dialog) are painted later, in response to a button, so each focuses
|
||||
its own first field as it is built rather than relying on this hook.
|
||||
|
||||
``surface`` is None when no output is live at all -- the lock is
|
||||
held with nothing to show, which is still a valid state.
|
||||
|
||||
@ -227,3 +227,24 @@ class TestSubmitManualWorkoutForm:
|
||||
# -> "zero matches played" validation error.
|
||||
locker._mw_error_label.config.assert_called_once()
|
||||
locker._on_manual_workout_saved.assert_not_called()
|
||||
|
||||
|
||||
class TestInitialKeyboardFocus:
|
||||
"""Exactly one field takes focus when a typed-input screen is built."""
|
||||
|
||||
def test_manual_workout_form_focuses_one_entry(
|
||||
self, mock_tk: MagicMock, mock_sys_exit: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
locker = create_locker(mock_tk, tmp_path)
|
||||
with patch.object(_manual_workout, "is_budget_exhausted", return_value=False):
|
||||
locker._show_manual_workout_form()
|
||||
focused = mock_tk.Entry.return_value.focus_set.call_count
|
||||
assert focused == 1, f"expected exactly one focused entry, got {focused}"
|
||||
|
||||
def test_sick_justification_form_focuses_one_entry(
|
||||
self, mock_tk: MagicMock, mock_sys_exit: MagicMock, tmp_path: Path
|
||||
) -> None:
|
||||
locker = create_locker(mock_tk, tmp_path)
|
||||
locker._build_justification_form(had_commitment=False)
|
||||
focused = mock_tk.Entry.return_value.focus_set.call_count
|
||||
assert focused == 1, f"expected exactly one focused entry, got {focused}"
|
||||
|
||||
96
screen_locker/tests/test_no_popup_widgets.py
Normal file
96
screen_locker/tests/test_no_popup_widgets.py
Normal file
@ -0,0 +1,96 @@
|
||||
"""Guard: no lock-surface UI module may build a popup window.
|
||||
|
||||
A Tk popup is a separate override-redirect window that also takes the Tk grab
|
||||
for itself, so gatelock's recovery tick covers it and disowns it within a
|
||||
second of opening. ``_manual_workout_dialog._mw_sport_row`` has the mechanism
|
||||
and the incident; ``scripts/verify_lock_popup_safety.py`` is the behavioural
|
||||
half of this guard and proves the fix still holds under a live recovery tick
|
||||
(run by the ``lock-popup-safety`` job in ``.github/workflows/python-tests.yml``).
|
||||
|
||||
The set of modules under the ban is DERIVED, not listed: anything in the
|
||||
package that both imports ``tkinter`` and paints onto the shared container is
|
||||
in scope automatically, so a new lock screen is covered the day it is written
|
||||
rather than the day someone remembers to add it here.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
PACKAGE_DIR = Path(__file__).resolve().parent.parent
|
||||
|
||||
EXEMPT: dict[str, str] = {
|
||||
"status_view.py": "plain tk.Tk with no grab and no recovery loop",
|
||||
"_heat_skip.py": "own throwaway root, destroyed before the lock exists",
|
||||
}
|
||||
"""Modules that touch Tk but never paint onto a gatelock lock surface.
|
||||
|
||||
Kept deliberately short and reasoned: every entry is a module allowed to open
|
||||
a popup, so an unjustified addition here is how the guard gets hollowed out.
|
||||
"""
|
||||
|
||||
BANNED = frozenset(
|
||||
{
|
||||
"OptionMenu",
|
||||
"Combobox",
|
||||
"tk_popup",
|
||||
"post",
|
||||
"messagebox",
|
||||
"simpledialog",
|
||||
"filedialog",
|
||||
"colorchooser",
|
||||
"Toplevel",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def lock_surface_modules() -> list[str]:
|
||||
"""Every package module that touches Tk on behalf of the lock window.
|
||||
|
||||
Importing ``tkinter`` is the whole test: the mixins build into a parent
|
||||
handed to them, so there is no reliable in-module marker saying "this ends
|
||||
up on a surface". Anything that genuinely does not is named in ``EXEMPT``
|
||||
with its reason.
|
||||
"""
|
||||
return [
|
||||
path.name
|
||||
for path in sorted(PACKAGE_DIR.glob("*.py"))
|
||||
if "import tkinter" in path.read_text(encoding="utf-8")
|
||||
and path.name not in EXEMPT
|
||||
]
|
||||
|
||||
|
||||
def symbols(module_name: str) -> set[str]:
|
||||
"""Every attribute and bare name referenced in a module's code.
|
||||
|
||||
An AST walk rather than a source regex: every banned widget is either an
|
||||
``ast.Attribute`` (``tk.OptionMenu``) or an ``ast.Name`` (a bare import),
|
||||
so docstrings, comments and string literals are excluded for free. The
|
||||
modules under test explain this very bug in prose that names the banned
|
||||
widgets, and a regex would have to strip that back out.
|
||||
"""
|
||||
tree = ast.parse((PACKAGE_DIR / module_name).read_text(encoding="utf-8"))
|
||||
return {n.attr for n in ast.walk(tree) if isinstance(n, ast.Attribute)} | {
|
||||
n.id for n in ast.walk(tree) if isinstance(n, ast.Name)
|
||||
}
|
||||
|
||||
|
||||
def test_the_module_list_is_not_empty() -> None:
|
||||
"""A derived list that silently went empty would pass every other test."""
|
||||
assert lock_surface_modules(), (
|
||||
"no lock-surface modules detected -- the markers in _SURFACE_MARKERS "
|
||||
"have probably drifted, which would make this whole guard a no-op"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("module_name", lock_surface_modules())
|
||||
def test_lock_surface_module_creates_no_popup(module_name: str) -> None:
|
||||
"""Fail if a lock-surface module references any popup-creating API."""
|
||||
offenders = sorted(symbols(module_name) & BANNED)
|
||||
assert not offenders, (
|
||||
f"{module_name} uses popup-creating Tk API {offenders}; a popup on a "
|
||||
"lock surface is covered and loses its grab within one recovery tick"
|
||||
)
|
||||
179
scripts/fix_2026_07_26_sport.py
Executable file
179
scripts/fix_2026_07_26_sport.py
Executable file
@ -0,0 +1,179 @@
|
||||
#!/usr/bin/env python3
|
||||
"""One-off correction: re-file the 2026-07-26 walk as sport "other".
|
||||
|
||||
On 2026-07-26 the manual-workout sport selector could not be changed while the
|
||||
screen was locked (``_manual_workout_dialog._mw_sport_row`` has the mechanism),
|
||||
so an hour-long walk had to be logged through the table-tennis form. The
|
||||
workout is genuine and its credit is correct; only its *shape* is wrong.
|
||||
|
||||
This rebuilds that one entry through the same ``ManualWorkoutDraft`` ->
|
||||
``validate_manual_workout`` -> ``build_entry`` path the form itself uses, so
|
||||
the result is the entry the form would have produced, not an approximation of
|
||||
it, and re-signs it with the writer's own HMAC key.
|
||||
|
||||
``start_time`` and the date are deliberately untouched: ``workout_id`` is
|
||||
derived from them, so keeping it stable means the phone-side CRDT copy
|
||||
converges on the corrected record rather than gaining a duplicate.
|
||||
|
||||
The walk's own details are NOT hardcoded here -- they are read from the entry
|
||||
and from ``--details`` -- because this file is tracked and the log is not.
|
||||
|
||||
Kept in the repo after its single run as the audit trail for the edit; it is
|
||||
idempotent, so re-running it reports that there is nothing to do.
|
||||
|
||||
Usage (needs the root-owned key at /etc/workout-locker/hmac.key):
|
||||
sudo python3 scripts/fix_2026_07_26_sport.py # show the diff
|
||||
sudo python3 scripts/fix_2026_07_26_sport.py --apply # write it
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
from datetime import datetime, timezone
|
||||
import json
|
||||
import logging
|
||||
from pathlib import Path
|
||||
import shutil
|
||||
import sys
|
||||
|
||||
from gatelock.log_integrity import compute_entry_hmac, verify_entry_hmac
|
||||
|
||||
from screen_locker import _manual_workout
|
||||
from screen_locker._log_io import read_raw_log
|
||||
|
||||
# ``print`` is banned repo-wide, so the diff this script exists to show goes
|
||||
# through a logger wired to stdout in main().
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
LOG_FILE = REPO_ROOT / "screen_locker" / "workout_log.json"
|
||||
TARGET_DATE = "2026-07-26"
|
||||
TARGET_START = "03:30"
|
||||
TARGET_WORKOUT_ID = _manual_workout.manual_sync_record_id(TARGET_DATE, TARGET_START)
|
||||
|
||||
ACTIVITY_LABEL = "walking"
|
||||
DEFAULT_DETAILS = (
|
||||
"Pure walking: walked for ~an hour and did ~5 km in total at my regular "
|
||||
"walking pace, on the route in the location field."
|
||||
)
|
||||
|
||||
|
||||
def corrected_workout_data(old: dict, details: str) -> dict[str, object]:
|
||||
"""Rebuild ``old`` as the ``sport: "other"`` entry the form would produce.
|
||||
|
||||
Goes through the real draft/validate/build path rather than editing keys,
|
||||
so the result carries exactly the writer's key set -- no table-tennis
|
||||
leftovers, no fields a hand-written dict would forget.
|
||||
"""
|
||||
draft = _manual_workout.ManualWorkoutDraft(
|
||||
sport=_manual_workout.SPORT_OTHER,
|
||||
start_time=str(old["start_time"]),
|
||||
end_time=str(old["end_time"]),
|
||||
location_name=str(old["location_name"]),
|
||||
transport_method=str(old["transport_method"]),
|
||||
cost=str(old["cost"]),
|
||||
rpe=int(old["rpe"]),
|
||||
went_well=str(old["went_well"]),
|
||||
to_improve=str(old["to_improve"]),
|
||||
overall_feeling=str(old["overall_feeling"]),
|
||||
reservation_phone=str(old.get("reservation_phone", "")),
|
||||
pain_or_injury=str(old.get("pain_or_injury", "none")),
|
||||
# "this was pure walking" was a placeholder the table-tennis form
|
||||
# forced into these; they are not technique or warm-up values.
|
||||
techniques_practiced="",
|
||||
warm_up_minutes="",
|
||||
activity_type_other=ACTIVITY_LABEL,
|
||||
activity_details=details,
|
||||
equipment="",
|
||||
)
|
||||
error = _manual_workout.validate_manual_workout(draft)
|
||||
if error is not None:
|
||||
sys.exit(f"The corrected entry would not pass the form's own rules: {error}")
|
||||
return _manual_workout.build_entry(draft)
|
||||
|
||||
|
||||
def find_target(logs: dict) -> dict:
|
||||
"""Return the one entry to correct, or exit with a reason."""
|
||||
entries = logs.get(TARGET_DATE)
|
||||
if not isinstance(entries, list):
|
||||
sys.exit(f"No list of entries for {TARGET_DATE} in {LOG_FILE}")
|
||||
matches = [
|
||||
e
|
||||
for e in entries
|
||||
if isinstance(e, dict) and e.get("workout_id") == TARGET_WORKOUT_ID
|
||||
]
|
||||
if len(matches) != 1:
|
||||
sys.exit(
|
||||
f"Expected exactly one entry with workout_id {TARGET_WORKOUT_ID} "
|
||||
f"on {TARGET_DATE}, found {len(matches)}"
|
||||
)
|
||||
return matches[0]
|
||||
|
||||
|
||||
def main() -> int:
|
||||
"""Show, and optionally apply, the correction."""
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument(
|
||||
"--apply", action="store_true", help="write the change (default: dry run)"
|
||||
)
|
||||
parser.add_argument(
|
||||
"--details",
|
||||
default=DEFAULT_DETAILS,
|
||||
help="what the activity actually was (goes in activity_details)",
|
||||
)
|
||||
args = parser.parse_args()
|
||||
logging.basicConfig(level=logging.INFO, format="%(message)s", stream=sys.stdout)
|
||||
|
||||
logs = read_raw_log(LOG_FILE)
|
||||
entry = find_target(logs)
|
||||
|
||||
if not verify_entry_hmac(entry):
|
||||
sys.exit(
|
||||
"Refusing to touch an entry whose existing HMAC does not verify "
|
||||
"(bad signature, or the key at /etc/workout-locker/hmac.key is "
|
||||
"unreadable -- run under sudo)."
|
||||
)
|
||||
|
||||
old_data = entry["workout_data"]
|
||||
if not isinstance(old_data, dict):
|
||||
sys.exit("workout_data is not an object")
|
||||
|
||||
new_data = corrected_workout_data(old_data, args.details)
|
||||
if new_data == old_data:
|
||||
_logger.info("Already corrected; nothing to do.")
|
||||
return 0
|
||||
|
||||
for key in sorted(set(old_data) | set(new_data)):
|
||||
before, after = old_data.get(key, "<absent>"), new_data.get(key, "<absent>")
|
||||
if before != after:
|
||||
_logger.info(" %s:\n - %r\n + %r", key, before, after)
|
||||
|
||||
if not args.apply:
|
||||
_logger.info("\nDry run. Re-run with --apply to write.")
|
||||
return 0
|
||||
|
||||
stamp = datetime.now(tz=timezone.utc).strftime("%Y%m%dT%H%M%SZ")
|
||||
backup = LOG_FILE.with_suffix(f".json.bak-{stamp}")
|
||||
shutil.copy2(LOG_FILE, backup)
|
||||
_logger.info("\nBacked up to %s", backup)
|
||||
|
||||
entry["workout_data"] = new_data
|
||||
entry.pop("hmac", None)
|
||||
signature = compute_entry_hmac(entry)
|
||||
if signature is None:
|
||||
sys.exit("Could not sign the corrected entry; log left untouched.")
|
||||
entry["hmac"] = signature
|
||||
# Match write_signed_entry's serialization exactly, so the next real
|
||||
# workout does not produce a spurious whitespace diff.
|
||||
with LOG_FILE.open("w") as handle:
|
||||
json.dump(logs, handle, indent=2)
|
||||
|
||||
reread = find_target(read_raw_log(LOG_FILE))
|
||||
if not verify_entry_hmac(reread):
|
||||
sys.exit("Written entry does not verify! Restore from the backup above.")
|
||||
_logger.info("Corrected entry written and its HMAC verifies.")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
299
scripts/verify_lock_popup_safety.py
Executable file
299
scripts/verify_lock_popup_safety.py
Executable file
@ -0,0 +1,299 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Prove the lock-screen sport selector survives gatelock's recovery tick.
|
||||
|
||||
``_manual_workout_dialog._mw_sport_row`` has the mechanism and the incident.
|
||||
``screen_locker/tests/test_no_popup_widgets.py`` keeps popups out of the lock
|
||||
path statically; this script is the *behavioural* half. It builds the real
|
||||
``ManualWorkoutDialogMixin`` form with real Tk, runs gatelock's two hostile
|
||||
behaviours for five ticks with the "Other" sport selected, and checks the
|
||||
selection and the swapped-in fields are still there. It also re-runs the
|
||||
original failure against a ``tk.OptionMenu``, so the mechanism stays
|
||||
demonstrated rather than merely described -- if that check ever stops
|
||||
reproducing, this harness has drifted from what gatelock actually does.
|
||||
|
||||
Fidelity gap, deliberate: ``HostileLock`` replays the two behaviours rather
|
||||
than calling ``RecoveryLoop.tick()``, which would need a real ``SurfaceSet``
|
||||
managing real per-output windows on this throwaway display. It borrows the
|
||||
real ``holds_grab()`` so the predicate cannot drift; a THIRD hostile behaviour
|
||||
added to ``tick()`` would not be covered here.
|
||||
|
||||
Not a pytest test because ``screen_locker/tests/conftest.py`` replaces the
|
||||
whole ``tk`` module for every test so the suite can never reach a real
|
||||
display. This script re-executes itself under Xvfb instead, so a global grab
|
||||
can never freeze the developer's own session. Run it directly, or let CI:
|
||||
|
||||
python3 ~/screen-locker/scripts/verify_lock_popup_safety.py
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from pathlib import Path
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tkinter as tk
|
||||
from typing import TYPE_CHECKING
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from gatelock import LockConfig, RecoveryLoop
|
||||
|
||||
from screen_locker import _manual_workout
|
||||
from screen_locker._manual_workout_dialog import ManualWorkoutDialogMixin
|
||||
from screen_locker._surface_group import FrameGroup
|
||||
from screen_locker._ui_widgets import UIWidgetsMixin
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
TICK_MS = 1000
|
||||
TICKS = 5
|
||||
INNER_ENV = "_LOCK_POPUP_CHECK_INNER"
|
||||
|
||||
|
||||
def reexec_under_xvfb() -> int:
|
||||
"""Re-run this script on a throwaway X display, returning its exit code."""
|
||||
xvfb_run = shutil.which("xvfb-run")
|
||||
if xvfb_run is None:
|
||||
_logger.error(
|
||||
"xvfb-run not found. Install it with: sudo pacman -S --needed "
|
||||
"xorg-server-xvfb"
|
||||
)
|
||||
return 1
|
||||
env = dict(os.environ, **{INNER_ENV: "1"})
|
||||
# Absolute argv, no shell.
|
||||
return subprocess.run(
|
||||
[
|
||||
xvfb_run,
|
||||
"-a",
|
||||
"-s",
|
||||
"-screen 0 1600x1200x24",
|
||||
sys.executable,
|
||||
str(Path(__file__).resolve()),
|
||||
],
|
||||
check=False,
|
||||
env=env,
|
||||
).returncode
|
||||
|
||||
|
||||
class HostileLock:
|
||||
"""gatelock's two recovery-tick behaviours over a real Tk root."""
|
||||
|
||||
def __init__(self, root: tk.Tk) -> None:
|
||||
"""Take the global grab and start ticking."""
|
||||
self.root = root
|
||||
self.ticks = 0
|
||||
# A RecoveryLoop built only to borrow gatelock's real grab predicate;
|
||||
# this harness drives the ticks itself, so the collaborators are never
|
||||
# called -- the same MagicMock stand-ins gatelock's own tests use.
|
||||
self._loop = RecoveryLoop(
|
||||
root, LockConfig(mode="hard"), MagicMock(), MagicMock(), MagicMock()
|
||||
)
|
||||
root.grab_set_global()
|
||||
root.after(TICK_MS, self._tick)
|
||||
|
||||
def _tick(self) -> None:
|
||||
"""Lift the surface and re-take the grab, exactly as gatelock does."""
|
||||
self.ticks += 1
|
||||
self.root.lift()
|
||||
if not self._loop.holds_grab():
|
||||
self.root.grab_set_global()
|
||||
self.root.after(TICK_MS, self._tick)
|
||||
|
||||
|
||||
def run_ticks(
|
||||
root: tk.Tk, judge: Callable[[], bool], setup: Callable[[], None] | None = None
|
||||
) -> bool:
|
||||
"""Tick hostilely TICKS times, then let ``judge`` say what survived.
|
||||
|
||||
``setup`` runs AFTER the lock has taken its grab, which is the order the
|
||||
real thing has: the user opens a widget on an already-grabbed lock. Posting
|
||||
a menu first would just let the lock's initial ``grab_set_global()`` take
|
||||
it, proving nothing about the recovery tick.
|
||||
"""
|
||||
verdict = False
|
||||
|
||||
def decide() -> None:
|
||||
nonlocal verdict
|
||||
verdict = judge()
|
||||
|
||||
HostileLock(root)
|
||||
if setup is not None:
|
||||
root.after(200, setup)
|
||||
root.after(TICK_MS * TICKS + 400, decide)
|
||||
root.after(TICK_MS * TICKS + 600, root.quit)
|
||||
root.mainloop()
|
||||
root.destroy()
|
||||
return verdict
|
||||
|
||||
|
||||
def leaf(name: object) -> str:
|
||||
"""A Tk window path shortened to its last component, "root" for ``.``."""
|
||||
if not name:
|
||||
return "<nothing>"
|
||||
return str(name).rsplit(".", 1)[-1] or "root"
|
||||
|
||||
|
||||
def grab_holder(root: tk.Tk) -> str:
|
||||
"""The leaf name of whatever currently holds the grab."""
|
||||
return leaf(root.tk.call("grab", "current", root))
|
||||
|
||||
|
||||
def check_optionmenu_still_fails() -> bool:
|
||||
"""Show the original mechanism: a posted menu gets covered by the lift."""
|
||||
root = tk.Tk()
|
||||
root.geometry("800x600+0+0")
|
||||
surface = tk.Toplevel(root)
|
||||
surface.overrideredirect(boolean=True)
|
||||
surface.geometry("800x600+0+0")
|
||||
var = tk.StringVar(value="Table tennis")
|
||||
option = tk.OptionMenu(surface, var, "Table tennis", "Other")
|
||||
option.pack()
|
||||
root.update()
|
||||
|
||||
menu = surface.nametowidget(option["menu"])
|
||||
|
||||
def open_menu() -> None:
|
||||
menu.tk_popup(100, 100)
|
||||
root.update()
|
||||
|
||||
def judge() -> bool:
|
||||
"""A click where the menu appears must no longer reach the menu."""
|
||||
centre_x = menu.winfo_rootx() + menu.winfo_width() // 2
|
||||
centre_y = menu.winfo_rooty() + menu.winfo_height() // 2
|
||||
hit = root.winfo_containing(centre_x, centre_y)
|
||||
_logger.info(
|
||||
" OptionMenu after %d ticks: grab=%s click_lands_on=%s",
|
||||
TICKS,
|
||||
grab_holder(root),
|
||||
leaf(hit),
|
||||
)
|
||||
return hit is not menu
|
||||
|
||||
return run_ticks(root, judge, setup=open_menu)
|
||||
|
||||
|
||||
class FormHost(UIWidgetsMixin, ManualWorkoutDialogMixin):
|
||||
"""The two hooks the mixin requires, over a real Tk root."""
|
||||
|
||||
def __init__(self, root: tk.Tk) -> None:
|
||||
"""Set only the attributes the form actually reads."""
|
||||
self.root = root
|
||||
self.demo_mode = False
|
||||
self.log_file = (
|
||||
Path(__file__).resolve().parent.parent
|
||||
/ "screen_locker"
|
||||
/ "workout_log.json"
|
||||
)
|
||||
self._colors = LockConfig()
|
||||
self.container = FrameGroup.single(root, bg=self._colors.bg)
|
||||
|
||||
def _on_manual_workout_saved(self, entry: dict[str, object]) -> None:
|
||||
"""Unused here; the form is never submitted."""
|
||||
|
||||
def _on_manual_workout_cancelled(self) -> None:
|
||||
"""Unused here; BACK is never pressed."""
|
||||
|
||||
# Public wrappers so this script never reaches into the mixin's
|
||||
# private surface from outside the class.
|
||||
def render(self) -> None:
|
||||
"""Paint the manual-workout form into the container."""
|
||||
self._show_manual_workout_form()
|
||||
|
||||
def state(self) -> tuple[str, bool, bool]:
|
||||
"""(selected sport code, other fields shown, table-tennis shown)."""
|
||||
return (
|
||||
self._current_mw_sport(),
|
||||
bool(self._mw_other_frame.winfo_ismapped()),
|
||||
bool(self._mw_tt_frame.winfo_ismapped()),
|
||||
)
|
||||
|
||||
|
||||
def radios(widget: tk.Misc) -> list[tk.Radiobutton]:
|
||||
"""Every Radiobutton under ``widget``, depth-first."""
|
||||
found: list[tk.Radiobutton] = []
|
||||
for child in widget.winfo_children():
|
||||
if isinstance(child, tk.Radiobutton):
|
||||
found.append(child)
|
||||
found.extend(radios(child))
|
||||
return found
|
||||
|
||||
|
||||
def check_real_form_survives() -> bool:
|
||||
"""Build the production form, pick "Other", hold it through the ticks."""
|
||||
root = tk.Tk()
|
||||
root.geometry("1600x1200+0+0")
|
||||
host = FormHost(root)
|
||||
host.render()
|
||||
root.update()
|
||||
|
||||
buttons = radios(root)
|
||||
labels = [b.cget("text") for b in buttons]
|
||||
_logger.info(" sport radio buttons rendered: %s", labels)
|
||||
if len(buttons) != len(_manual_workout.SPORT_LABELS):
|
||||
_logger.error(
|
||||
" expected one radio button per sport (%d), got %d",
|
||||
len(_manual_workout.SPORT_LABELS),
|
||||
len(labels),
|
||||
)
|
||||
root.destroy()
|
||||
return False
|
||||
|
||||
def pick_other() -> None:
|
||||
buttons[1].invoke()
|
||||
root.update()
|
||||
|
||||
def judge() -> bool:
|
||||
"""The Other sport must still be selected, with its own fields shown."""
|
||||
sport, other_shown, tt_shown = host.state()
|
||||
_logger.info(
|
||||
" radio buttons after %d ticks: grab=%s sport=%r other_fields=%s "
|
||||
"tt_fields=%s",
|
||||
TICKS,
|
||||
grab_holder(root),
|
||||
sport,
|
||||
other_shown,
|
||||
tt_shown,
|
||||
)
|
||||
return sport == _manual_workout.SPORT_OTHER and other_shown and not tt_shown
|
||||
|
||||
return run_ticks(root, judge, setup=pick_other)
|
||||
|
||||
|
||||
def main() -> int:
|
||||
"""Run both checks under Xvfb and report which held."""
|
||||
logging.basicConfig(level=logging.INFO, format="%(message)s", stream=sys.stdout)
|
||||
if os.environ.get(INNER_ENV) != "1":
|
||||
return reexec_under_xvfb()
|
||||
|
||||
_logger.info("The mechanism (a popup on a lock surface):")
|
||||
mechanism_reproduced = check_optionmenu_still_fails()
|
||||
_logger.info("The fix (radio buttons in the production form):")
|
||||
fix_holds = check_real_form_survives()
|
||||
|
||||
if not mechanism_reproduced:
|
||||
_logger.error(
|
||||
"The OptionMenu was NOT covered by the recovery tick. This check no "
|
||||
"longer reproduces the failure it exists to guard against -- either "
|
||||
"gatelock stopped lifting its surfaces, or this harness has drifted."
|
||||
)
|
||||
if not fix_holds:
|
||||
_logger.error(
|
||||
"The production sport selector did NOT survive %d recovery ticks. "
|
||||
"The 2026-07-26 lock-screen bug is back.",
|
||||
TICKS,
|
||||
)
|
||||
if mechanism_reproduced and fix_holds:
|
||||
_logger.info(
|
||||
"\nOK: a popup still dies under the tick; the shipped radio-button "
|
||||
"selector survives it."
|
||||
)
|
||||
return 0
|
||||
return 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Loading…
Reference in New Issue
Block a user