Skip to content

security: close 6 CRITICAL findings from 2026-04-08 audit#232

Merged
thewrz merged 5 commits intomainfrom
fix/critical-security-2026-04-08
Apr 9, 2026
Merged

security: close 6 CRITICAL findings from 2026-04-08 audit#232
thewrz merged 5 commits intomainfrom
fix/critical-security-2026-04-08

Conversation

@thewrz
Copy link
Copy Markdown
Owner

@thewrz thewrz commented Apr 9, 2026

Summary

Closes all 6 CRITICAL vulnerabilities identified in the full-stack security audit from 2026-04-08.

  • CRIT-1 — JWT accepted-algorithm list was sourced from env variable. JWT_ALGORITHM=none could silently disable signature verification. Fixed: hardcoded algorithms=["HS256"]
  • CRIT-2 — JWTs had no revocation mechanism (24-hour TTL, no jti, no version). Fixed: token_version column on User, tv claim in JWT, POST /api/auth/logout endpoint
  • CRIT-3 — Kiosk pairing IDOR: any DJ could pair a kiosk to another DJ's event. Fixed: target-event ownership check
  • CRIT-4 — Kiosk reassignment IDOR: same vulnerability on the assign endpoint. Fixed: same check
  • CRIT-5 — Unauthenticated SSE stream with no rate limit or existence check (DoS + passive eavesdropping). Fixed: @limiter.limit("10/minute") + event existence/expiry/archived validation
  • CRIT-6 — Third-party GitHub Actions pinned to mutable tags, not commit SHAs. Fixed: SHA-pinned codecov/codecov-action and softprops/action-gh-release, plus a guardrail test

TDD methodology

Every fix followed strict RED-GREEN-REFACTOR:

  1. RED: wrote failing test(s) that express the security invariant
  2. GREEN: made the minimum production-code change to pass the tests
  3. REFACTOR: verified all 1657 existing tests still pass

22 new tests across 4 new files + 1 extended file. 1 existing test updated: test_auth_jwt_algorithm.py::test_decode_accepts_valid_hs256 needed "tv": 0 added to test data after CRIT-2 landed (the decode_token function now rejects tokens missing the tv claim).

Files changed

Production (10 files):

  • server/app/services/auth.py — hardcoded _JWT_ALGORITHM, tv claim extraction
  • server/app/api/deps.pytoken_version comparison in get_current_user
  • server/app/api/auth.pytv in login token, new /logout endpoint
  • server/app/api/kiosk.py_assert_caller_owns_event helper, called in both handlers
  • server/app/api/sse.py — rate limit + existence check + expiry/archived validation
  • server/app/models/user.pytoken_version column
  • server/app/schemas/auth.pytoken_version in TokenData
  • server/alembic/versions/033_add_user_token_version.py — migration
  • .github/workflows/ci.yml — SHA-pinned codecov action
  • .github/workflows/release.yml — SHA-pinned softprops action

Tests (5 files):

  • server/tests/test_auth_jwt_algorithm.py — 5 tests (CRIT-1)
  • server/tests/test_auth_jwt_revocation.py — 5 tests (CRIT-2)
  • server/tests/test_kiosk_api.py — 4 new IDOR tests (CRIT-3+4)
  • server/tests/test_sse_security.py — 4 tests (CRIT-5)
  • server/tests/test_workflow_pins.py — 4 parametrized tests (CRIT-6)

Test plan

  • All 1679 backend tests pass (87.64% coverage, above 85% threshold)
  • ruff check + format clean
  • bandit clean
  • alembic check: "No new upgrade operations detected"
  • Remote CI green (all 5 jobs)
  • Manual smoke test: kiosk IDOR returns 403
  • Manual smoke test: SSE 404 for unknown events, 429 on rapid connects
  • Manual smoke test: logout invalidates token
  • Manual smoke test: alg=none token rejected

thewrz added 5 commits April 8, 2026 23:30
The accepted JWT algorithm list was sourced from settings.jwt_algorithm,
meaning an operator (or attacker with env-write access) setting
JWT_ALGORITHM=none could silently disable signature verification.

Hardcodes HS256 in create_access_token and decode_token. Adds 5 TDD
guards in tests/test_auth_jwt_algorithm.py.

See docs/security/audit-2026-04-08.md CRIT-1.
…RIT-4)

Before this fix, any authenticated DJ could pair or reassign a kiosk
to an event owned by another DJ by supplying the victim's 6-char event
code. Both complete_kiosk_pairing and assign_kiosk fetched the event
by code and proceeded without checking event.created_by_user_id.

Extracts a helper _assert_caller_owns_event that allows admin bypass
and raises 403 for DJs attempting to act on events they do not own.
Called after the 'Event not found' check in both handlers.

New TestKioskIdor class in test_kiosk_api.py adds 4 guards:
- DJ cannot pair to victim's event (kiosk stays in 'pairing' state)
- DJ cannot reassign their own kiosk to victim's event
- Admin CAN pair to any event
- Admin CAN reassign any kiosk to any event

All 17 existing test_kiosk_api.py tests and 52 test_kiosk.py tests
continue to pass. See docs/security/audit-2026-04-08.md CRIT-3, CRIT-4.
Before this fix, GET /api/public/events/{code}/stream had no rate limit,
no auth, and no event-existence check. Unauthenticated attackers could:
(a) Open unlimited long-lived SSE connections (DoS via FD exhaustion)
(b) Brute-force 6-char event codes to passively eavesdrop on real-time
    request, now-playing, and bridge-status events

Now validates event exists and is not archived/expired before subscribing,
and applies @limiter.limit('10/minute') at the connection-open boundary.

New test_sse_security.py adds 4 guards:
- 404 for unknown event code
- 404/410 for archived event
- 404/410 for expired event
- 429 after exceeding 10 connections/minute

All 120 SSE-related tests pass (existing test_events.py, test_event_bus.py,
test_bridge_commands.py all green).

See docs/security/audit-2026-04-08.md CRIT-5.
Adds token_version column to User (migration 033) and 'tv' claim to
every JWT. On decode, tv must be present and match user.token_version;
otherwise 401. POST /api/auth/logout bumps the counter.

See docs/security/audit-2026-04-08.md CRIT-2.
Third-party actions were pinned to mutable tags (e.g. @v6, @v2).
A compromised tag could exfiltrate GITHUB_TOKEN, WINGET_PAT, and
inject malicious code into the bridge-app installer binaries.

Pinned actions:
- codecov/codecov-action@v6 -> @57e3a136b779b5...  (v6)
- softprops/action-gh-release@v2 -> @153bb8e0440...  (v2)

First-party actions (actions/*, github/*) are exempt — they are
maintained by GitHub and present lower supply-chain risk.

New test_workflow_pins.py guardrail runs on every CI run and fails
if any third-party action uses a non-SHA ref.

See docs/security/audit-2026-04-08.md CRIT-6.
@thewrz thewrz merged commit d4e43c3 into main Apr 9, 2026
8 checks passed
@thewrz thewrz deleted the fix/critical-security-2026-04-08 branch April 9, 2026 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant