fix(security): block cross-site state changes (ROADMAP 2.2)

CHANGELOG.md

@@ -4,8 +4,13 @@ All notable changes to WalkingDad will be documented in this file.
 
 ## [Unreleased]
 
+### Changed
+
+- **`/reconnect` is now POST-only**, like the belt-control routes. The Connect and Try Again buttons on the connecting screen look and work the same.
+
 ### Fixed
 
+- **Other websites can no longer control the treadmill** (ROADMAP 2.2). A page open in your browser could previously submit Start, speed, Clear History, Bluetooth reconnect, or Shutdown requests to WalkingDad behind your back. Requests that come from another website are now refused. Using WalkingDad from any device on your network works exactly as before.
 - **Pause and speed buttons no longer error out when Bluetooth isn't running.** If the Bluetooth connection had already gone away, pressing Pause or a speed button showed a server error page. Pause now still pauses the session, and the speed buttons do nothing.
 - **A corrupted crash-recovery file no longer stops the app from starting.** If `session_state.json` contained unreadable (non-UTF-8) bytes, startup crashed. Startup now ignores the file, the same as an unparseable one.
 

README.md

@@ -19,7 +19,7 @@ The official WalkingPad experience is a bloated mobile app that wants your email
 - **Console-style interface**: Active, Paused, and Start screens read like the WalkingPad's own onboard display, with one large tabular-digit reading up top and secondary stats in a compact readout strip below
 - **Session history**: Sessions saved to a local SQLite database (`walkingdad.db`) with full stats, pauses, and per-second speed/distance/steps samples; last 10 shown on the start screen. Includes CSV export and history clearing. An existing `session_history.json` from an older version is imported automatically on first launch (the original is kept as `session_history.json.bak-<timestamp>`).
 - **Crash recovery**: If the server crashes or restarts mid-session, your stats aren't lost. The start screen offers to restore the interrupted session (paused, ready to resume) or discard it.
-- **Settings page**: Gear icon in the header lets you change any setting (device name, speed limits, port, and more) from the browser without editing files
+- **Settings page**: Gear icon in the header lets you change settings (device name, speed limits, port, and more) from the browser without editing files
 - **Dark mode**: Three-state toggle (Light → Dark → System) with localStorage persistence
 - **Color themes**: Independent of the Light/Dark/System toggle, via the palette icon in the header. Each theme tints the whole surface, not just buttons, and follows into the browser's own chrome (tab color, etc).
     - Standard palette: Slate is the default; the rest run in spectral order:
@@ -83,7 +83,7 @@ The app opens your browser automatically at `http://127.0.0.1:5001`. A console w
 
 ## Usage
 
-1. Power on your WalkingPad. The app connects automatically on startup (up to 3 retries with exponential backoff).
+1. Power on your WalkingPad. The app connects automatically on startup (up to 3 attempts with exponential backoff).
 2. Click **Start** to begin a session. Stats update in real time.
 3. Adjust speed with the control buttons, or click **Pause** to stop the belt.
 4. If you step off the pad, the app auto-pauses. Click **Resume** to pick back up.
@@ -114,7 +114,7 @@ Then Share → Copy iCloud Link in the Shortcuts app, and swap `_APPLE_HEALTH_SH
 
 ## Configuration
 
-All settings can be changed from the **Settings page** (gear icon in the header), or by editing `config.json` directly. Copy `config.json.example` to `config.json` to get started; running without the file uses the built-in defaults shown below.
+Every setting except `database_path` can be changed from the **Settings page** (gear icon in the header); all of them can be set by editing `config.json` directly. Copy `config.json.example` to `config.json` to get started; running without the file uses the built-in defaults shown below.
 
 | Key | Default | Description |
 |---|---|---|
@@ -127,9 +127,10 @@ All settings can be changed from the **Settings page** (gear icon in the header)
 | `resume_grace_period_seconds` | `7` | Seconds before auto-pause can trigger after start/resume (minimum 3) |
 | `history_display_limit` | `10` | Sessions shown on the start screen |
 | `stale_pause_timeout_minutes` | `30` | A session paused this long is ended and saved automatically; `0` disables |
-| `host` | `"0.0.0.0"` | Network interface to bind |
+| `host` | `"0.0.0.0"` | Network interface to bind. Any device that can reach it can control the treadmill (other websites can't); use `"127.0.0.1"` to allow only this computer |
 | `port` | `5001` | Server port |
 | `waitress_threads` | `16` | Server worker thread count (4-128) |
+| `apple_health_export_enabled` | `false` | Shows the Log to Apple Health prompt after each session; see [Apple Health Export](#apple-health-export) |
 | `apple_health_shortcut_name` | `"Log WalkingDad Workout"` | Must match the installed Shortcut's name exactly; see [Apple Health Export](#apple-health-export) |
 | `database_path` | `"walkingdad.db"` | SQLite database file (relative to the app directory) |
 

ROADMAP.md

@@ -136,15 +136,15 @@ All user-tunable settings are now loaded from an optional `config.json` file, wi
 
 ---
 
-### 2.2 Route Security
-- **Status:** Planned
-- **Priority:** High
-- **Problem:** Routes like `/start`, `/pause`, `/increase_speed` have no authentication or CSRF protection. Since the server binds to `0.0.0.0`, any device on the local network could control the treadmill.
-- **Solution:** Add a simple secret token mechanism:
-    - Generate a random token on startup (stored in config)
-    - Require `?token=...` query parameter or `X-Auth-Token` header on all action routes
-    - Display the token in the UI and use it automatically for frontend requests
-- **Alternative:** Restrict server to `127.0.0.1` only (breaks network access but is simpler).
+### ✅ 2.2 Route Security
+**Status:** ✅ Complete
+**Priority:** High
+**Files Modified:** `app.py`, `templates/connecting.html`, `tests/test_csrf.py`, `tests/test_routes.py`
+
+Routes like `/start`, `/pause`, `/increase_speed` had no CSRF protection, so any website open in a browser on the network could control the treadmill. A `before_request` guard (`_block_cross_site_posts()`) now rejects any non-GET request whose `Origin` isn't the app's own origin (scheme, host, and port), or whose `Sec-Fetch-Site` is `cross-site` or `same-site` (catches it when an extension or proxy strips `Origin`). Requests without either header (curl, `run.py`'s `/shutdown`) pass. `/reconnect` became POST-only (the Connect / Try Again links on the connecting screen are now form buttons), so every state-changing route goes through the guard.
+
+- **Not done, deliberately:** a per-device token. The app is LAN-only and devices on the LAN are trusted by design; the treadmill is only reachable over Bluetooth from home, and anyone at home can press its buttons anyway. A token would add a sign-in step for every phone, break scripts, and add a credential to manage, guarding against a threat that doesn't apply. Revisit only if the app is ever exposed beyond the LAN (tunnel, Tailscale).
+- **Known limit:** a deliberate DNS-rebinding attack makes `Origin` match the app's origin and gets through; that takes an attack targeted at this app specifically, so it's accepted. The fix would be a `Host` allowlist (IP literals, `localhost`, the machine's own name), at the cost of breaking access through custom DNS names (`.lan`, Tailscale).
 
 ---
 
@@ -321,7 +321,7 @@ Shipping this surfaced a real gap it needed to close first: a dead BLE connectio
 - **Problem:** The app always displays imperial units (mph, miles). Users who prefer metric must mentally convert or edit code constants.
 - **Solution:** Add a unit toggle (Imperial ↔ Metric) in the header that switches between mph/miles and km/h/km in real time. Store preference in `localStorage`.
 - **Implementation:**
-    - Return both imperial and metric values from `/stats` JSON endpoint
+    - Return both imperial and metric values from `_build_stats_payload()` (shared by `/stats_stream` and `/stats`)
     - Frontend toggles display based on user preference
     - Add toggle button next to the theme toggle in the header
 

app.py

@@ -989,9 +989,8 @@ async def _cancel_task(task: asyncio.Task | None) -> None:
 
     Shared by every module-level task this app tracks (stats monitor, idle
     watchdog, in-flight belt sequence) -- callers pass their own task
-    variable; this never reassigns it, matching the previous per-task
-    helpers' behavior (the global gets overwritten next time a new task is
-    created, same as before).
+    variable; this never reassigns it (the global gets overwritten next
+    time a new task is created).
     """
     if task and not task.done():
         task.cancel()
@@ -1070,8 +1069,8 @@ async def _wake_and_start_belt(target_speed_kmh: float | None = None, try_light_
 
     try_light_wake=True (Resume only) first attempts _try_light_wake().
     Falls back to _full_wake_sequence() -- identical to the
-    try_light_wake=False path -- if that can't be confirmed, so reliability
-    is never worse than before this change, only sometimes gentler on the
+    try_light_wake=False path -- if that can't be confirmed, so it's never
+    less reliable than the full sequence, only sometimes gentler on the
     WalkingPad's own display.
     """
     async def _body():
@@ -1346,7 +1345,7 @@ def _handle_signal_shutdown(signum, frame):
             return
         _shutting_down = True
 
-    # Set UI-visible flag early so the next /stats poll can inform the browser
+    # Set UI-visible flag early so the next /stats_stream tick can inform the browser
     _server_stopping = True
     logging.info(f"Received signal {signum}, initiating graceful shutdown...")
 
@@ -1365,12 +1364,29 @@ def _handle_signal_shutdown(signum, frame):
         except Exception as exc:
             logging.debug(f"Error stopping BLE loop from signal: {exc}")
 
-    # Give the browser ~2 s to receive the last /stats response (stopping: true),
+    # Give the browser ~2 s to receive the last /stats_stream frame (stopping: true),
     # render the shutdown message, and then force-kill the process.
     time.sleep(2)
     os._exit(0)
 
 
+# ── CSRF guard (ROADMAP 2.2) ────────────────────────────────────────────
+@app.before_request
+def _block_cross_site_posts():
+    """LAN devices are trusted by design (the app is LAN-only); this only stops
+    another website open in a browser from driving the treadmill. Browsers
+    send Origin on every cross-origin POST, including plain form submits;
+    Sec-Fetch-Site still catches it when an extension or proxy strips Origin.
+    "same-site" is rejected too: another port on the same host counts as same-site."""
+    if request.method in ("GET", "HEAD"):
+        return None
+    origin = request.headers.get("Origin")
+    if (request.headers.get("Sec-Fetch-Site") in ("cross-site", "same-site")
+            or (origin and origin != request.host_url.rstrip("/"))):
+        return "Cross-site request blocked.", 403
+    return None
+
+
 # ── Flask routes ────────────────────────────────────────────────────────
 @app.route("/")
 def root():
@@ -1443,10 +1459,9 @@ def _end_session(stale: bool = False):
         async def _end_belt_sequence():
             global _belt_sequence_task, _belt_transitioning
             # Registers itself as _belt_sequence_task and sets
-            # _belt_transitioning like its three siblings (start/pause/resume)
-            # -- previously it did neither, the one belt sequence that didn't
-            # follow the pattern, so nothing else could observe or cancel an
-            # in-flight End Session the way it can for the others.
+            # _belt_transitioning like its three siblings (start/pause/resume),
+            # so an in-flight End Session can be observed and cancelled the
+            # same way as the others.
             await _cancel_task(_belt_sequence_task)
             _belt_sequence_task = asyncio.current_task()
             _belt_transitioning = True
@@ -1668,7 +1683,7 @@ def settings_page():
     )
 
 
-@app.route("/reconnect")
+@app.route("/reconnect", methods=["POST"])
 def reconnect():
     if not connected and not connecting:
         _start_ble_thread()
@@ -1983,7 +1998,7 @@ def _build_stats_payload() -> dict:
     """Build the stats snapshot dict from current global state.
 
     Single source of truth for the wire payload shape, shared by the
-    polling /stats endpoint and the SSE broadcaster.
+    /stats endpoint and the SSE broadcaster.
     """
     return {
         "is_connected": connected,
@@ -2096,7 +2111,7 @@ def shutdown():
             return jsonify({"status": "shutting_down"})
         _shutting_down = True
 
-    # Set UI-visible flag immediately so the next /stats poll informs the browser
+    # Set UI-visible flag immediately so the next /stats_stream tick informs the browser
     _server_stopping = True
     logging.info("Graceful shutdown initiated via HTTP...")
 
@@ -2121,7 +2136,7 @@ def shutdown():
     # Use os._exit(0) here because Waitress catches SystemExit from sys.exit(0)
     # and continues running, which would prevent the server from actually stopping.
     def _deferred_exit():
-        time.sleep(5)  # Give browser time to receive /stats with stopping:true
+        time.sleep(5)  # Give browser time to receive a /stats_stream frame with stopping:true
         logging.info("Exiting process after graceful shutdown...")
         os._exit(0)
 

run.py

@@ -100,8 +100,7 @@ def main():
 
     server_process = subprocess.Popen(
         # --threads: each open SSE connection (/stats_stream) holds a worker
-        # thread for its entire lifetime, unlike the old short-lived polling
-        # requests. Defaults well above the Waitress default of 4 (see
+        # thread for its entire lifetime. Defaults well above the Waitress default of 4 (see
         # config.py's waitress_threads) so several concurrent devices can
         # each hold a stream open alongside action POSTs (start/pause/speed)
         # without stalling; tunable via config.json like every other setting.

templates/base.html

@@ -112,11 +112,8 @@
 
     <style>
         /* Custom properties from the design file */
-        /* Slate is the default (absence of data-color-theme, same
-           optimization the picker already used for the old default).
-           These neutrals are Tailwind's slate-50/900 scale, unchanged from
-           before this was Slate's identity; only the accent moved from
-           blue to gray. */
+        /* Slate is the default (absence of data-color-theme). Neutrals are
+           Tailwind's slate-50/900 scale with a gray accent. */
         :root {
             --background-color: #f8fafc; /* bg-slate-50 */
             --card-color: #e7edf4;
@@ -319,8 +316,7 @@
             --accent-hover: #1e7aa8;
         }
 
-        /* Color theme: Blue (240°). No longer the implicit default (Slate
-           is now); this is a full explicit override like every other hue. */
+        /* Color theme: Blue (240°). A full explicit override like every other hue. */
         [data-color-theme="blue"] {
             --background-color: #f4f4fa;
             --card-color: #e5e5f2;

templates/connecting.html

@@ -10,8 +10,8 @@ <h1 class="console-hero-status">
             <p class="text-muted mt-2 mb-0" style="font-size:0.9rem;">Please ensure your WalkingPad is turned on.</p>
             <script>
             // Reload only once something has actually changed (connected, or the
-            // background retry loop has given up), never on a fixed timer like
-            // the old <meta refresh> did, so ambient theme effects (falling
+            // background retry loop has given up), not on a fixed timer, so
+            // ambient theme effects (falling
             // snow/leaves, etc.) keep running smoothly instead of visibly
             // resetting every few seconds while nothing's actually changed yet.
             // Not startStatsStream() from base.html: that helper treats
@@ -38,11 +38,15 @@ <h1 class="console-hero-status">
         {% elif connection_failed %}
             <h1 class="console-hero-status text-danger">CONNECTION FAILED</h1>
             <p class="text-muted mt-2 mb-3" style="font-size:0.9rem;">Could not find the WalkingPad. Please ensure it's on and in range.</p>
-            <a href="{{ url_for('reconnect') }}" class="btn btn-custom-primary">Try Again</a>
+            <form method="post" action="{{ url_for('reconnect') }}" style="display:contents">
+                <button type="submit" class="btn btn-custom-primary">Try Again</button>
+            </form>
         {% else %}
             <h1 class="console-hero-status text-danger">DISCONNECTED</h1>
             <div class="mt-3">
-                <a href="{{ url_for('reconnect') }}" class="btn btn-custom-primary">Connect</a>
+                <form method="post" action="{{ url_for('reconnect') }}" style="display:contents">
+                    <button type="submit" class="btn btn-custom-primary">Connect</button>
+                </form>
             </div>
         {% endif %}
     </div>

tests/test_csrf.py

@@ -0,0 +1,53 @@
+import pytest
+
+import app
+
+
+@pytest.fixture
+def client(app_state):
+    return app.app.test_client()
+
+
+@pytest.fixture
+def side_effects(monkeypatch, app_state):
+    """Every guarded route's effect, recorded: BLE coroutines, history wipe, config write, process exit."""
+    calls = []
+    monkeypatch.setattr(app, "_clear_session_history", lambda: calls.append("clear_history"))
+    monkeypatch.setattr(app, "_write_config", lambda updates: calls.append("write_config"))
+    monkeypatch.setattr(app.os, "_exit", lambda code: calls.append("exit"))
+    monkeypatch.setattr(app, "_start_ble_thread", lambda: calls.append("start_ble_thread"))
+    return lambda: calls + [c.__name__ for c in app_state]
+
+
+@pytest.mark.parametrize("headers", [
+    {"Origin": "http://evil.example"},
+    {"Origin": "null"},
+    {"Origin": "http://localhost:9999"},
+    {"Origin": "https://localhost"},
+    {"Sec-Fetch-Site": "cross-site"},
+    {"Sec-Fetch-Site": "same-site"},
+    {"Origin": "http://localhost", "Sec-Fetch-Site": "cross-site"},
+])
+@pytest.mark.parametrize("path", ["/start", "/shutdown", "/clear_history", "/settings", "/reconnect"])
+def test_cross_site_post_blocked(client, side_effects, monkeypatch, headers, path):
+    # Disconnected only for /reconnect: it's a no-op when connected, /start a no-op when not.
+    monkeypatch.setattr(app, "connected", path != "/reconnect")
+    assert client.post(path, headers=headers).status_code == 403
+    assert side_effects() == []
+    assert app._server_stopping is False
+
+
+@pytest.mark.parametrize("headers", [
+    {},
+    {"Origin": "http://localhost"},
+    {"Origin": "http://localhost", "Sec-Fetch-Site": "same-origin"},
+    {"Sec-Fetch-Site": "none"},
+])
+def test_same_origin_or_unmarked_post_allowed(client, side_effects, headers):
+    assert client.post("/clear_history", headers=headers).status_code == 200
+    assert side_effects() == ["clear_history"]
+
+
+def test_cross_site_get_allowed(client):
+    headers = {"Origin": "http://evil.example", "Sec-Fetch-Site": "cross-site"}
+    assert client.get("/stats", headers=headers).status_code == 200