diff --git a/CLAUDE.md b/CLAUDE.md index 3de979b..0f52ec6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,3 +134,19 @@ that's what causes the version and the Docker tags to drift apart. - `probe_iframe_embeddable` must never raise and must default to `False` (redirect) on any ambiguity (network error, timeout, non-2xx, unparseable header) — the redirect page is the one path that always works, so failure must never accidentally select the iframe path instead. +- `render_page` must keep escaping `link` for HTML-attribute contexts (`html.escape(..., quote=True)` + → `link_attr`) and JSON-encoding it for the inline-JS context (`json.dumps` → `link_js`) — never + interpolate the raw string again. The admin override isn't guaranteed to match + `LIVETRACK_LINK_RE`, so this is the only thing standing between an admin-set value containing + quotes/`<>` and breaking out of an attribute or the JS string literal. +- `/admin/link` must keep rejecting (`400`) any submitted link that doesn't fullmatch + `LIVETRACK_LINK_RE` before it ever reaches `state.set_link`/`probe()` — this is what keeps the + admin override from being usable to probe/redirect to arbitrary internal URLs. +- `_check_auth` must keep using `secrets.compare_digest`, not `==`, for both the username and + password comparison. +- `__main__.py` starts the server via `waitress.create_server(...)` + `server.run()`, not the + `serve()` shortcut — the signal handler needs a `server` handle to call `.close()` on. Without + it, SIGTERM is silently swallowed by `handle_signal` and the container only stops on Docker's + SIGKILL, skipping `EmailMonitor`'s clean IMAP logout. +- The `Dockerfile`'s `HEALTHCHECK` reads `WEB_PORT` from the environment at check time (defaulting + to `8080`) rather than hard-coding a port — keep it that way if `WEB_PORT` stays configurable. diff --git a/Dockerfile b/Dockerfile index 0c1864b..0bb6b9f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -44,7 +44,10 @@ USER waypoint EXPOSE 8080 VOLUME ["/data"] +# Reads WEB_PORT at check time (falling back to 8080) rather than +# hard-coding it, since the CMD's JSON form doesn't get shell/env +# substitution -- only the python process it execs does. HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ - CMD ["python", "-c", "import sys,urllib.request; sys.exit(0 if urllib.request.urlopen('http://127.0.0.1:8080/healthz', timeout=3).status == 200 else 1)"] + CMD ["python", "-c", "import os,sys,urllib.request; port=os.environ.get('WEB_PORT','8080'); sys.exit(0 if urllib.request.urlopen(f'http://127.0.0.1:{port}/healthz', timeout=3).status == 200 else 1)"] CMD ["python", "-m", "waypoint"] diff --git a/README.md b/README.md index c6bbba6..ead16fe 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,9 @@ At `/admin` (Basic Auth): - **Status dashboard** — current link, when it was last updated, IMAP connection status, last error. - **Manual override** — set the current link directly, or clear it (e.g. to show the "offline" - page when no activity is live). + page when no activity is live). The link must match the same + `https://livetrack.garmin.com/session/.../token/...` shape as an email-extracted one (`400` on + anything else). - **Recent history** — the last 20 links that were published, with timestamp and source (`email` or `admin`), including whether each was served as an iframe or a redirect. diff --git a/tests/test_render_page.py b/tests/test_render_page.py index 6354c6d..c5624b1 100644 --- a/tests/test_render_page.py +++ b/tests/test_render_page.py @@ -1,12 +1,19 @@ """Tests for the pure HTML renderer in waypoint.web.""" import datetime as dt +import json -from waypoint.web import render_page +from waypoint.web import OFFLINE_REFRESH_SECONDS, render_page LINK = "https://livetrack.garmin.com/session/xxxx/token/YYYY" NOW = dt.datetime(2026, 7, 9, 12, 0, 0) +# Not something the email extractor or the (validating) admin route would +# ever hand render_page, but render_page itself must still neutralize it -- +# defense in depth, since it's a pure function with no knowledge of the +# caller. +MALICIOUS_LINK = 'https://evil.example/">' + def test_redirect_page_contains_link_meta_refresh_and_countdown(): html = render_page(LINK, use_iframe=False, countdown=15, now=NOW) @@ -27,6 +34,34 @@ def test_offline_page_when_no_link_is_available(): html = render_page(None, use_iframe=False, countdown=10, now=NOW) assert LINK not in html - assert "http-equiv=\"refresh\"" not in html # Some human-readable indication that there is currently no active session. assert "no active" in html.lower() or "offline" in html.lower() + + +def test_offline_page_polls_for_a_session_starting(): + # The copy promises this page updates itself once a session goes live -- + # it needs an actual refresh mechanism to back that up, otherwise a + # viewer who opens the link early is stuck until they manually reload. + html = render_page(None, use_iframe=False, countdown=10, now=NOW) + + assert f'' in html + + +def test_iframe_page_escapes_link_in_html_attributes(): + html = render_page(MALICIOUS_LINK, use_iframe=True, countdown=10, now=NOW) + + assert "" not in html + assert "<script>" in html + + +def test_redirect_page_escapes_link_in_meta_and_encodes_in_js(): + html = render_page(MALICIOUS_LINK, use_iframe=False, countdown=10, now=NOW) + + assert "" not in html + assert "<script>" in html + # Plain json.dumps alone would still leak the literal "" into + # the page, which closes the surrounding " in json.dumps(MALICIOUS_LINK) # sanity: the raw case would be unsafe + assert "\\u003cscript\\u003e" in html diff --git a/tests/test_web.py b/tests/test_web.py index 97dda96..ce60b34 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -145,6 +145,28 @@ def test_admin_set_link_probes_embeddability(config, state): assert state.iframe_ok is False +def test_admin_set_link_rejects_non_livetrack_url(client, state): + resp = client.post( + "/admin/link", + data={"link": "javascript:alert(1)"}, + headers=_basic_auth_header("admin", "s3cret"), + ) + + assert resp.status_code == 400 + assert state.current_link is None + + +def test_admin_set_link_rejects_non_garmin_domain(client, state): + resp = client.post( + "/admin/link", + data={"link": "https://evil.example/session/a/token/1"}, + headers=_basic_auth_header("admin", "s3cret"), + ) + + assert resp.status_code == 400 + assert state.current_link is None + + def test_admin_can_clear_link(client, state): state.set_link("https://livetrack.garmin.com/session/c/token/3", source="email", iframe_ok=True) diff --git a/waypoint/__main__.py b/waypoint/__main__.py index bb7ebd6..615a5a9 100644 --- a/waypoint/__main__.py +++ b/waypoint/__main__.py @@ -11,7 +11,7 @@ import threading from dotenv import load_dotenv -from waitress import serve +from waitress import create_server from .config import Config from .email_monitor import EmailMonitor @@ -47,9 +47,20 @@ def main() -> None: monitor = EmailMonitor(config, state) + app = create_app(config, state) + # create_server (rather than the serve() shortcut) gives us a handle to + # close from the signal handler -- serve() blocks forever and installs + # no signal handling of its own, so without this, SIGTERM would just get + # silently overridden by handle_signal below and the container would + # sit until Docker's SIGKILL, skipping the monitor's clean IMAP logout. + server = create_server(app, host="0.0.0.0", port=config.web_port) + shutting_down = threading.Event() + def handle_signal(signum, frame): log.info("Shutting down Garmin LiveTrack...") monitor.stop() + shutting_down.set() + server.close() signal.signal(signal.SIGINT, handle_signal) signal.signal(signal.SIGTERM, handle_signal) @@ -57,9 +68,22 @@ def handle_signal(signum, frame): monitor_thread = threading.Thread(target=monitor.run, name="email-monitor", daemon=True) monitor_thread.start() - app = create_app(config, state) log.info(f"Starting web server on port {config.web_port}...") - serve(app, host="0.0.0.0", port=config.web_port) + try: + server.run() + except OSError: + # server.close() (above) closes the listening socket while + # server.run()'s select() loop may be blocked on it -- waitress + # surfaces that as a "Bad file descriptor" OSError rather than a + # clean return. That's expected once we've asked for a shutdown; + # anything else is a real failure and should still propagate. + if not shutting_down.is_set(): + raise + log.info("Web server stopped.") + finally: + # Give the monitor thread a chance to finish process_new_emails() + # and log out of IMAP cleanly before the process exits. + monitor_thread.join(timeout=15) if __name__ == "__main__": diff --git a/waypoint/web.py b/waypoint/web.py index 7a5f5c7..3002ff6 100644 --- a/waypoint/web.py +++ b/waypoint/web.py @@ -7,6 +7,8 @@ """ import functools +import json +import secrets from datetime import datetime, timezone from html import escape from typing import Callable, Optional @@ -14,9 +16,29 @@ from flask import Flask, Response, abort, jsonify, redirect, request, url_for from .config import Config +from .email_monitor import LIVETRACK_LINK_RE from .link_probe import probe_iframe_embeddable from .state import AppState +# How often the "no active session" placeholder polls for a session having +# started, so a viewer who opens the shared link early doesn't have to +# manually reload once one goes live. +OFFLINE_REFRESH_SECONDS = 30 + + +def _js_string_literal(value: str) -> str: + """JSON-encode `value` for safe embedding inside an inline " would close the surrounding + - +

Garmin LiveTrack

The LiveTrack session cannot be embedded directly.

-

Open LiveTrack in a new tab

+

Open LiveTrack in a new tab

Last updated: {timestamp}

@@ -97,7 +129,7 @@ def render_page(link: Optional[str], *, use_iframe: bool, countdown: int, now: d Garmin LiveTrack - Redirecting - +