Security review fixes: XSS, cookie leak, scheduler stall, optional login#60
Merged
Conversation
renderFeed() built each feed card via innerHTML with unescaped activity/athlete names and stat labels straight from the Strava feed. Add an escapeHtml() helper and apply it to every Strava-sourced value in feed.js; also stop toast() from interpolating its message into innerHTML. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh
… gate, harden Docker deploy defaults - SHA-pin every third-party GitHub Action (checkout, setup-python, upload-artifact, release-please-action, docker/*) for supply-chain safety, keeping a version comment for readability. - Add .github/scripts/check_module_coverage.py: enforces the >=90% pure-module coverage target CLAUDE.md already documented but CI never actually checked (coverage.py has no built-in per-file threshold). - Dockerfile: replace the curl-based healthcheck with stdlib urllib (drops the curl apt dependency); clarify why KUDOSY_HOST must stay 0.0.0.0 inside the container. - docker-compose.yaml: bind the published port to 127.0.0.1 by default (no auth in front of the API otherwise — see the login-screen commit), add security_opt/cap_drop hardening. - .dockerignore: exclude *.har (browser network captures can contain session cookies). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh
Security: - GET /api/config no longer returns the raw Strava session cookie (only hasCookie + a masked preview); PUT /api/config now merges+validates instead of writing raw untyped data, and keeps the existing cookie when the request omits/empties it. - Add a strict Content-Security-Policy (nonce-based for the one inline importmap script) plus X-Content-Type-Options/X-Frame-Options/Referrer-Policy on every response; disable the auto-generated FastAPI docs (would break under this CSP anyway). - Validate athlete_id/activity_id path params as numeric before they're interpolated into Strava URLs; harden cursor-pagination parsing against non-numeric drift instead of raising a bare 500. Robustness: - Fix the scheduler: reschedule-after-run now happens in finally (previously an exception in job_fn skipped it entirely), and an overlapping tick reschedules instead of silently dropping the chain — both could previously stall the scheduler forever. - A run that stops early (rate limit / consecutive failures) is now reported as success=False with its own notification wording, instead of looking identical to a normal completed run. - Batch kudoed-activity persistence into one read+write per run instead of one per kudo sent. - Retry idempotent Strava GETs on transient network errors; GET /api/feed converts a network failure into a clean 502 instead of an unhandled 500; log a warning when a Strava stat item's label goes fully unrecognized (early signal for feed-format drift). Feature: optional login screen (auth.py) — inactive unless KUDOSY_AUTH_PASSWORD is set, so existing unauthenticated deployments are unaffected. Signed session cookie (HMAC-SHA256, HttpOnly, timing-safe verify), simple login-attempt lockout, and a frontend overlay that blocks the app until authenticated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh
applyStaticTranslations() ran after ensureAuthenticated(), which blocks on the login overlay when KUDOSY_AUTH_PASSWORD is set. The overlay's own labels stayed in the raw fallback text baked into index.html regardless of detected browser/stored language, while errors surfaced via t() (e.g. a wrong-password message) correctly used the real language — producing a visibly mismatched-language UI during login. Found via live browser verification (agent-browser) of the login overlay; moving the translation call before the auth gate fixes it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh
…rman), fill missing STRAVA_UNREACHABLE translation
The new Access Control section/comments used a German heading ("## Zugriffsschutz") that was then quoted verbatim in docker-compose.yaml, CLAUDE.md, models.py and settings.py — inconsistent with this project's convention of English docs/comments (the app's own UI strings are the separately-maintained multi-language i18n.js catalog, which is unaffected). Renamed to "Access Control" everywhere it's referenced.
Also added the error.STRAVA_UNREACHABLE translation (all 5 languages) that was missing since that error code was introduced — without it, the German backend fallback message would have leaked through untranslated to non-German UI languages.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh
t(key) silently falls back to German when a key is missing for the current language, so a gap is invisible at runtime — exactly what happened here: the entire Statistics tab (tab.stats + 14 stats.* keys) was never translated for fr/es/it, a pre-existing gap this test caught immediately. Fixed those 15 missing translations so the new test starts from a clean baseline. Exports MESSAGES from i18n.js (additive, no behavior change) so the test can load the real ES module via Node rather than hand-parsing the file with regex — this project has no JS test runner (pytest is the only test tool per CLAUDE.md), so the test shells out to node and skips cleanly if it's unavailable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh
Names the specific CLI (agent-browser — isolated Chrome via CDP, no Playwright/Puppeteer dep) instead of just gesturing at 'a headless/dedicated tool', with a quick command-flow example and the immutable-cache gotcha (?v=... assets need a fresh session, not just reload, to pick up an edited static file) discovered while verifying the login-overlay in this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up from a full project review (3 parallel Sonnet explorations: backend architecture,
security/Strava integration, tests/frontend/hygiene). Fixes a chained XSS→cookie-theft path,
a scheduler bug that could permanently stall the auto-kudos job, and adds an opt-in login
screen — plus assorted robustness, CI, and Docker hardening. All changes are backward
compatible: nothing here changes default behavior for existing installs beyond the security
fixes themselves (login is off unless
KUDOSY_AUTH_PASSWORDis set).Security
feed.js): feed cards were built viainnerHTMLwith unescaped Stravaactivity/athlete names — a crafted activity title from any followed athlete could execute
script in the page. Added an
escapeHtml()helper and applied it everywhere a Strava-sourcedstring is interpolated; also fixed a latent instance in
dom.js'stoast().GET /api/configno longer returns the raw_strava4_sessioncookie(only
hasCookie+ a masked preview).PUT /api/confignow merges + validates instead ofwriting raw untyped data, and keeps the existing cookie when the request omits/empties it.
Content-Security-Policy(nonce-based for the one inlineimportmap script),
X-Content-Type-Options,X-Frame-Options,Referrer-Policyon everyresponse. Auto-generated FastAPI docs disabled (would break under this CSP anyway).
athlete_id/activity_idmust be numeric before beinginterpolated into Strava URLs via
str.format.auth.py): inactive unlessKUDOSY_AUTH_PASSWORDis set.Signed session cookie (HMAC-SHA256,
HttpOnly, timing-safe verify), simple login-lockout,and a frontend overlay that blocks the app until authenticated. See README "Zugriffsschutz".
Robustness
finally(previously anexception in
job_fnskipped it entirely), and an overlapping tick reschedules instead ofsilently dropping the chain — both could previously stall the scheduler forever.
success=Falsewith distinct notification wording, instead of looking identical to anormal completed run.
GET /api/feedconverts anetwork failure into a clean 502 instead of an unhandled 500; early-warning log when a
Strava stat label goes fully unrecognized (feed-format drift signal).
CI / Docker
.github/scripts/check_module_coverage.py: actually enforces the ≥90% pure-modulecoverage target CLAUDE.md already documented but CI never checked.
curl→ stdliburllib(drops thecurlapt dependency).docker-compose.yaml: published port bound to127.0.0.1by default, addedsecurity_opt/cap_drophardening.Public-repo housekeeping
Verified the repo/releases/Docker image are correctly public post-switch (no action needed —
documented in commit history for the record);
.gitignoretightened for local Claude Codeoverrides.
Test plan
ruff check/ruff format --check/mypy src/pytestall green (569 tests,92% overall coverage, all 4 "pure module" targets ≥90%)
against a running server via curl
against a running container)
/,/api/status,/api/config(cookieredaction),
/api/auth-status, security headers all confirmed correctno isolated browser-automation tool was available in this environment (per CLAUDE.md,
the user's live browser must never be driven for this)
🤖 Generated with Claude Code
https://claude.ai/code/session_01FXccJ9FUpxNxAqHDmhZ6Bh