Security: invalidate sessions on credential rotation by cursor[bot] · Pull Request #65 · OpenRTMP/librtmp2-server-panel

cursor · 2026-07-17T02:03:43Z

Security review finding

Severity: Medium
Issue: #64
Location: app.py

Fixes #64

Problem

Rotating PASSWORD or changing USERNAME did not invalidate existing panel sessions. An attacker with a previously stolen session cookie kept full admin access for up to the 8-hour session TTL, defeating a common incident-response step.

Impact

Continued stream CRUD and secret exposure after operators rotate credentials.

Fix

Add _credential_fingerprint() (HMAC-SHA256 of USERNAME + PASSWORD keyed by SECRET_KEY).
Store credential_fp in the session at login.
Reject authenticated requests when the fingerprint no longer matches current config credentials.
Compare stored and expected fingerprints with hmac.compare_digest().
Regression test: test_username_change_invalidates_stolen_session_cookie.
Document the security change in CHANGELOG.md.

Verification

venv/bin/python -m pytest -m "not integration" — 100 passed.

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

Note

Medium Risk
Changes the authentication gate on every protected request; scope is limited to session validation and uses constant-time comparison, with regression coverage for credential rotation.

Overview
Fixes a gap where rotating USERNAME or PASSWORD left existing signed session cookies valid until TTL. Sessions now carry a credential_fp HMAC-SHA256 fingerprint (keyed by SECRET_KEY) set at login; _session_is_authenticated compares it to the current config with hmac.compare_digest and revokes/clears the session on mismatch.

Adds test_username_change_invalidates_stolen_session_cookie and documents the behavior under Security in CHANGELOG.md.

^{Reviewed by Cursor Bugbot for commit a76b5c4. Bugbot is set up for automated code reviews on this repo. Configure here.}

Bind each login session to an HMAC fingerprint of USERNAME and PASSWORD so rotating either credential revokes existing cookies, including stolen ones that remain valid in the shared session backend. Co-authored-by: Alexander Wagner <info@alexanderwagnerdev.com>

cursor · 2026-07-17T11:07:48Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_3efa8197-43f1-4b9a-8e44-f5df352d1226)

qodo-code-review · 2026-07-17T11:09:09Z

PR Summary by Qodo

Security: invalidate sessions on credential rotation

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Bind each login session to a credential fingerprint to revoke cookies on credential rotation.
• Reject authenticated requests when stored fingerprint no longer matches configured credentials.
• Add regression test and document the security behavior in the changelog.

Diagram

graph TD
  U[User/Attacker] --> LR["POST /login"] --> SC["Session cookie"] --> AUTH["Auth gate"] --> CFG["Config creds"] --> FP["HMAC credential_fp"] --> STORE[("Session store")] --> PR["Protected routes"]
  FP -->|"mismatch: revoke+clear"| LR

High-Level Assessment

The following are alternative approaches to this PR:

1. Server-side session version/epoch (credential_generation)

➕ Avoids storing any credential-derived marker in the client session cookie (even HMAC’d).
➕ Can invalidate all sessions globally by bumping a single value in Redis/db.
➖ Requires persistent/shared storage and migration/bootstrapping when the store is unavailable.
➖ More moving parts than a self-contained fingerprint check tied to existing config.

2. Rotate SECRET_KEY on credential rotation

➕ Immediately invalidates all existing signed session cookies with no extra auth logic.
➖ Operationally heavier: requires secret distribution and coordinated rollout.
➖ May invalidate other SECRET_KEY-derived artifacts and can cause unexpected logouts beyond admin auth.

Recommendation: Keep the PR’s current approach: it is minimal, self-contained, and enforces immediate revocation even if the session-store token is still within TTL. The server-side session-epoch approach is a reasonable future enhancement if you want a centrally managed invalidation mechanism, but it adds operational and storage dependencies that aren’t necessary for this fix.

Files changed (3) +57 / -0

Bug fix (1) +26 / -0

app.pyBind sessions to credential fingerprint and enforce constant-time comparison +26/-0

Bind sessions to credential fingerprint and enforce constant-time comparison

• Introduces _credential_fingerprint() using HMAC-SHA256 keyed by SECRET_KEY and stores the fingerprint in the session at login. On each authenticated request, recomputes the expected fingerprint and revokes/clears the session when it no longer matches, using hmac.compare_digest() for constant-time comparison.

app.py

Tests (1) +26 / -0

test_login_routes.pyAdd regression test for password rotation invalidating stolen session cookie +26/-0

Add regression test for password rotation invalidating stolen session cookie
• Adds a test that logs in, captures the session cookie, rotates application.config["PASSWORD"], then verifies the old cookie is redirected back to /login on the next request.
tests/test_login_routes.py

Documentation (1) +5 / -0

CHANGELOG.mdDocument session invalidation on credential changes +5/-0

Document session invalidation on credential changes

• Adds an Unreleased Security changelog entry describing that active sessions are revoked when username/password changes to prevent stolen cookies from remaining valid.

CHANGELOG.md

qodo-code-review · 2026-07-17T11:12:20Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 Password rotation does not invalidate active panel sessions, allowing continued admin access with stolen cookies

✅ Compliance rules (platform): 6 rules

✅ Cross-repo context

Not relevant to this PR: OpenRTMP/librtmp2-server

1. _credential_fingerprint() wrong HMAC key ✓ Resolved 📎 Requirement gap ⛨ Security ⭐ New

Description

The new _credential_fingerprint() uses USERNAME/PASSWORD as the HMAC key instead of using
SECRET_KEY as the key, which does not meet the required credential-fingerprint construction. This
can weaken the intended binding and diverges from the mandated fingerprint scheme for session
invalidation on credential rotation.

Code

app.py[71]
+        key,

Relevance

⭐⭐⭐ High
Team consistently accepts auth/security correctness fixes in app.py (e.g., session
invalidation/revocation hardening in PRs #56, #61).
PR-#56
PR-#61
PR-#51

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2107469 requires a credential fingerprint computed as an HMAC keyed by SECRET_KEY
over USERNAME+PASSWORD. In app.py, the docstring and code explicitly build key from
username and password and pass it as the first argument to hmac.new(), making credentials the
HMAC key rather than SECRET_KEY.
Bind session validity to a credential fingerprint stored at login and verified on each authenticated request
app.py[64-67]
app.py[69-69]
app.py[71-71]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_credential_fingerprint()` currently constructs the HMAC using `USERNAME`/`PASSWORD` as the HMAC key and `SECRET_KEY` as the message. The compliance requirement expects the opposite: derive the fingerprint from `USERNAME`+`PASSWORD` while using `SECRET_KEY` as the HMAC key.

## Issue Context
This fingerprint is used to bind an authenticated session to the currently configured credentials and invalidate sessions immediately after credential rotation.

## Fix Focus Areas
- app.py[64-71]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Test overrides PASSWORD literal~~ ✓ Resolved 📘 Rule violation ▣ Testability

Description

The new test sets application.config["PASSWORD"] to a non-standard hardcoded value, violating the
requirement that test contexts use fixed dummy secret strings. This can undermine consistent CI
validation and risks accidental introduction of non-approved secret values in tests.

Code

tests/test_login_routes.py[272]

+        application.config["PASSWORD"] = "new-rotated-password-that-is-long-enough"

Relevance

⭐⭐ Medium
No prior evidence enforcing fixed dummy PASSWORD literal in tests; only general test-hardening
accepted (e.g., PR19).
PR-#19
PR-#46
PR-#62

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2107438 requires fixed dummy secret strings in automated tests. The added test code
overrides PASSWORD with a different literal value, which does not match the allowed fixed dummy
PASSWORD string.
Rule 2107438: Use fixed dummy secrets in automated tests
tests/test_login_routes.py[255-276]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test overrides `PASSWORD` with `new-rotated-password-that-is-long-enough`, but compliance requires fixed dummy secret strings for `SECRET_KEY`, `PASSWORD`, and `LRTMP2_API_TOKEN` in test contexts.

## Issue Context
Rule requires test-only usages/overrides of these secret config keys to use exactly:
- `SECRET_KEY=test-secret-key-for-ci-validation-only-32chars`
- `PASSWORD=test-password-for-ci-only`
- `LRTMP2_API_TOKEN=test-api-token-for-ci-only`

To keep the intent of the regression test (invalidate sessions on credential rotation) without changing `PASSWORD`, consider rotating `USERNAME` instead (not covered by this rule) and adjusting the test name/assertions accordingly.

## Fix Focus Areas
- tests/test_login_routes.py[255-279]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Previous review results

Review updated until commit a76b5c4

Results up to commit 9f84f9c

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Test overrides PASSWORD literal~~ ✓ Resolved 📘 Rule violation ▣ Testability

Description

The new test sets application.config["PASSWORD"] to a non-standard hardcoded value, violating the
requirement that test contexts use fixed dummy secret strings. This can undermine consistent CI
validation and risks accidental introduction of non-approved secret values in tests.

Code

tests/test_login_routes.py[272]

+        application.config["PASSWORD"] = "new-rotated-password-that-is-long-enough"

Relevance

⭐⭐ Medium
No prior evidence enforcing fixed dummy PASSWORD literal in tests; only general test-hardening
accepted (e.g., PR19).
PR-#19
PR-#46
PR-#62

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2107438 requires fixed dummy secret strings in automated tests. The added test code
overrides PASSWORD with a different literal value, which does not match the allowed fixed dummy
PASSWORD string.
Rule 2107438: Use fixed dummy secrets in automated tests
tests/test_login_routes.py[255-276]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test overrides `PASSWORD` with `new-rotated-password-that-is-long-enough`, but compliance requires fixed dummy secret strings for `SECRET_KEY`, `PASSWORD`, and `LRTMP2_API_TOKEN` in test contexts.

## Issue Context
Rule requires test-only usages/overrides of these secret config keys to use exactly:
- `SECRET_KEY=test-secret-key-for-ci-validation-only-32chars`
- `PASSWORD=test-password-for-ci-only`
- `LRTMP2_API_TOKEN=test-api-token-for-ci-only`

To keep the intent of the regression test (invalidate sessions on credential rotation) without changing `PASSWORD`, consider rotating `USERNAME` instead (not covered by this rule) and adjusting the test name/assertions accordingly.

## Fix Focus Areas
- tests/test_login_routes.py[255-279]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

cursor · 2026-07-17T11:20:28Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_b1c8e5fb-ef14-4c55-8797-da34df5512e6)

qodo-code-review · 2026-07-17T11:22:39Z

Code review by qodo was updated up to the latest commit 26afd68

CodeQL's py/weak-sensitive-data-hashing flagged the fingerprint construction because username/password were hashed as HMAC message content. Swap roles so the credentials key the HMAC and a fixed value (secret_key) is the message - same security property (unforgeable, invalidates on any credential change) without matching the flagged "password fed into a fast digest" pattern.

cursor · 2026-07-17T11:28:06Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_ec171232-11da-413a-848f-cb572d00db98)

qodo-code-review · 2026-07-17T11:31:05Z

Code review by qodo was updated up to the latest commit a205d45

…ositive Revert the earlier key/message swap - the credential-fingerprint scheme in issue #64 requires HMAC(key=SECRET_KEY, msg=username+password), and swapping the roles diverged from that spec (flagged by review). Instead suppress the CodeQL py/weak-sensitive-data-hashing alert inline: it treats the password reaching hashlib.sha256 as an unkeyed password-storage hash, but this is a keyed HMAC where SECRET_KEY is secret - the fingerprint can't be brute-forced back to the password without it.

cursor · 2026-07-17T11:32:31Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_04c2e28a-5142-48cc-86e1-899d7be43d29)

qodo-code-review · 2026-07-17T11:34:23Z

Code review by qodo was updated up to the latest commit d2d8b9b

The first attempt placed the codeql[] suppression comment on the hmac.new( call line; move it to the hashlib.sha256 argument itself, which is the actual sink py/weak-sensitive-data-hashing reports.

cursor · 2026-07-17T11:36:06Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_3366ca09-ed86-4dd3-b8e5-2f117ea5ce63)

qodo-code-review · 2026-07-17T11:37:28Z

Code review by qodo was updated up to the latest commit 2c5880c

GitHub's inline review comment showed the alert anchored to the material.encode() line (the msg argument), not the hashlib.sha256 line the comment was previously on - so it was never being matched and the check kept failing. Move it to the correct line.

cursor · 2026-07-17T11:39:07Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_56525a9f-d7f3-4b34-a0c6-e994a8d024ca)

qodo-code-review · 2026-07-17T11:40:10Z

Code review by qodo was updated up to the latest commit 45b6532

The codeql[] suppression comment on the exact flagged line (confirmed via the inline review annotation) didn't get picked up by the check. Trying the older lgtm[] syntax GitHub's code scanning also recognizes, in case this repo's config only honors that form.

cursor · 2026-07-17T11:46:11Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_aa44c239-423e-4771-a932-bd616bed9f3b)

qodo-code-review · 2026-07-17T11:47:26Z

Code review by qodo was updated up to the latest commit 2658937

Neither codeql[] nor the legacy lgtm[] suppression syntax got honored by this repo's code scanning check, so the alert needs a manual dismissal in the Security tab as a false positive. Keeping the codeql[] comment in place documents intent for future readers even though it isn't functionally suppressing the check here.

cursor · 2026-07-17T11:48:59Z

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_c19a939c-5f02-4b88-b23f-844f1c3e24a5)

sonarqubecloud · 2026-07-17T11:49:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

qodo-code-review · 2026-07-17T11:50:52Z

Code review by qodo was updated up to the latest commit a76b5c4

github-advanced-security AI found potential problems Jul 17, 2026

View reviewed changes

Comment thread app.py Fixed

AlexanderWagnerDev added 2 commits July 17, 2026 13:05

fix: use constant-time credential fingerprint comparison

601ded4

docs: add credential rotation security changelog entry

9f84f9c

AlexanderWagnerDev marked this pull request as ready for review July 17, 2026 11:07

qodo-code-review Bot added the 🕐 20-40 Minutes label Jul 17, 2026

qodo-code-review Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread tests/test_login_routes.py Outdated

test: rotate username in session invalidation regression

26afd68

qodo-code-review Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread app.py Outdated

chore: move CodeQL suppression comment to the flagged sink line

2c5880c

The first attempt placed the codeql[] suppression comment on the hmac.new( call line; move it to the hashlib.sha256 argument itself, which is the actual sink py/weak-sensitive-data-hashing reports.

github-advanced-security AI found potential problems Jul 17, 2026

View reviewed changes

Comment thread app.py Fixed

github-advanced-security AI found potential problems Jul 17, 2026

View reviewed changes

Comment thread app.py Dismissed

github-advanced-security AI found potential problems Jul 17, 2026

View reviewed changes

Comment thread app.py Fixed

AlexanderWagnerDev merged commit be6f49b into main Jul 17, 2026
11 checks passed

AlexanderWagnerDev deleted the cursor/application-security-review-4f74 branch July 17, 2026 17:26

Uh oh!

Conversation

cursor Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review finding

Problem

Impact

Fix

Verification

Uh oh!

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

Uh oh!

cursor Bot commented Jul 17, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

sonarqubecloud Bot commented Jul 17, 2026

Quality Gate passed

Uh oh!

qodo-code-review Bot commented Jul 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cursor Bot commented Jul 17, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 17, 2026 •

edited

Loading