fix(ci): resolve SonarCloud B security rating on new code

[cursor]

Summary

SonarCloud failed the quality gate on main with B Security Rating on New Code (required ≥ A) due to two pythonsecurity:S5145 findings that log user-controlled request data.

Changes

• Stop logging the login username and delete stream_id in app.py; keep exception context via logger.exception() / error-only messages.
• Update test_delete_runtime.py expectations for the revised delete-failure log line.
• Migrate the last two inline WTF_CSRF_ENABLED = False test setups to configure_testing_app().

Verification

• python -m pytest -m "not integration" — 99 passed

  

---

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

---

Note

Low Risk
Logging-only hardening with no auth or API behavior changes; delete failures still surface the API error to the user via flash messages.

Overview
Addresses SonarCloud pythonsecurity:S5145 by removing user-controlled values from application logs while keeping enough context for operations.

On login, when the session backend is unavailable, logging no longer includes the submitted username; it uses logger.exception() with a fixed message so stack traces are still captured.

On delete stream failures, the raw stream_id is replaced with a 12-character stream_tag derived from HMAC-SHA256(SECRET_KEY, stream_id) so ops can correlate log lines without writing attacker-controlled IDs to logs or using a guessable unkeyed hash.

Tests assert the new delete-failure log format and two remaining inline test setups now call configure_testing_app() instead of duplicating CSRF/test flags.

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

[cursor]

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_3ba85cfc-e90d-424d-a159-6e28bf134a92)

[qodo-code-review]

PR Summary by Qodo

Fix SonarCloud S5145 by removing user-controlled data from logs

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Remove username/stream_id from logs to satisfy SonarCloud S5145 on new code.
• Preserve debugging context via exception logging and error-only messages.
• Align tests with new log output and standardize CSRF-disabled test setup.

Diagram

graph TD
  A{{"CI gate"}} --> B{{"SonarCloud"}} --> C["app.py"]
  C --> D["login()"] --> F["Sanitized logs"]
  C --> E["delete_stream()"] --> F

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Redact/normalize user inputs before logging

• ➕ Keeps some diagnostic signal (e.g., partial identifiers) without raw user-controlled strings
• ➕ Can be centralized in a helper for consistent usage
• ➖ More complexity and risk of incomplete redaction
• ➖ May still trigger static analyzers if patterns resemble raw input logging

2. Structured logging with an allowlist of safe fields

• ➕ Clear separation between safe operational context and sensitive/request-controlled fields
• ➕ Scales better as additional logs are added
• ➖ Broader refactor and convention change beyond the immediate Sonar issue
• ➖ Higher review/maintenance overhead for this small fix

Recommendation: The chosen approach (remove username/stream_id from the log messages and rely on logger.exception()/exception text for context) is the lowest-risk way to clear S5145 quickly. Consider a follow-up redaction/allowlist strategy only if operational debugging requires correlating specific user/stream identifiers.

Files changed (4) +7 / -11

Bug fix (1) +3 / -5

app.pySanitize login/delete logging to avoid user-controlled fields +3/-5

Sanitize login/delete logging to avoid user-controlled fields

• Updates the login session-backend failure log to use logger.exception() without interpolating the posted username. Removes stream_id from the delete failure warning while still logging the exception details.

app.py

Tests (3) +4 / -6

test_delete_runtime.pyAdjust delete failure log assertion for sanitized message +1/-2

Adjust delete failure log assertion for sanitized message

• Updates the expected logger.warning() call to match the new delete failure log format without stream_id.

tests/test_delete_runtime.py

test_login_routes.pyUse configure_testing_app() instead of inline CSRF/test config +1/-2

Use configure_testing_app() instead of inline CSRF/test config

• Replaces inline TESTING/WTF_CSRF_ENABLED setup with configure_testing_app(application) in a logout error-path test.

tests/test_login_routes.py

test_logout_validation_failure.pyStandardize test app configuration via configure_testing_app() +2/-2

Standardize test app configuration via configure_testing_app()

• Imports and uses configure_testing_app(application) to avoid duplicating TESTING/WTF_CSRF_ENABLED configuration.

tests/test_logout_validation_failure.py

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 6 rules

✅ Cross-repo context

  Not relevant to this PR: OpenRTMP/librtmp2-server

1. Stream tag easily deanonymized ✓ Resolved 🐞 Bug ⛨ Security ⭐ New

Description

In delete_stream(), stream_tag is computed as an unkeyed sha256(stream_id) prefix, so anyone with
log access can recompute tags for predictable/user-chosen stream IDs and map log entries back to the
original IDs (contrary to the “non-reversible” comment). This weakens the intended privacy benefit
of removing stream_id from logs when IDs are human-chosen or otherwise enumerable.

Code

app.py[R472-476]

+            # Log a non-reversible correlation tag instead of the raw
+            # user-controlled stream_id (SonarCloud pythonsecurity:S5145),
+            # while still letting ops correlate failures to a specific stream.
+            stream_tag = hashlib.sha256(stream_id.encode()).hexdigest()[:12]
+            app.logger.warning("Delete stream failed (stream_tag=%s): %s", stream_tag, exc)

Relevance

⭐⭐ Medium

Team removes user-controlled log values for Sonar/security, but no history enforcing keyed/HMAC tags
vs plain hash prefixes.

PR-#58
PR-#57
PR-#56

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The log tag is currently a plain SHA-256 prefix of the raw stream_id, making it deterministic and
recomputable. Stream IDs are user-controllable (taken from the create stream form), so predictable
IDs are plausible; therefore the tag can be matched back to candidate IDs by anyone with access to
the logs.

app.py[463-477]
app.py[339-370]
app.py[18-32]

Agent prompt

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

## Issue description
`delete_stream()` logs `stream_tag = sha256(stream_id)[:12]` and describes it as “non-reversible”, but because it’s an unkeyed deterministic transform of a user-controllable ID, it can be deanonymized by recomputing tags for candidate IDs (especially when IDs are user-supplied/predictable).

## Issue Context
Stream IDs can be provided by the user via the `/streams/new` form, which makes low-entropy IDs plausible. The current approach is useful for preventing direct log injection of the raw ID, but it should not be described/treated as non-reversible anonymization.

## Fix Focus Areas
- app.py[472-476]

## Suggested fix
- Replace `hashlib.sha256(stream_id.encode()).hexdigest()[:12]` with a keyed construction like `hmac.new(app.config["SECRET_KEY"].encode("utf-8"), stream_id.encode("utf-8"), hashlib.sha256).hexdigest()[:12]` (or another server-held secret).
- Alternatively (or additionally), update the comment to avoid claiming “non-reversible” if you intentionally want an unkeyed tag for ops convenience.

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

---

2. Stream ID lost in log ✓ Resolved 🐞 Bug ◔ Observability

Description

In delete_stream(), the warning log on Lrtmp2ApiError no longer includes the stream_id, which makes
it harder to correlate deletion failures to a specific stream during ops/debugging. The handler
already validates stream_id, so this context is available but is dropped in the new log line.

Code

app.py[R468-472]

        try:
            client.delete_stream(stream_id)
        except Lrtmp2ApiError as exc:
-            app.logger.warning("Delete for stream %s failed: %s", stream_id, exc)
+            app.logger.warning("Delete stream failed: %s", exc)
            session["flash_error"] = str(exc)

Relevance

⭐⭐⭐ High

PR #57 accepted delete-failure logging including stream_id; dropping it regresses previously
approved operability improvement.

PR-#57

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The handler validates stream_id but the updated warning log drops it, so the log no longer indicates
which validated stream was being deleted when the exception occurred.

app.py[29-31]
app.py[462-473]
PR-#57

Agent prompt

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

### Issue description
`delete_stream()` logs API deletion failures without identifying which stream was being deleted, reducing diagnostic value of the warning.

### Issue Context
- `stream_id` is already validated by `_is_valid_stream_id()` before the API call.
- This PR removed `stream_id` from the warning to address SonarCloud S5145; reintroducing the raw value may re-trigger the rule.

### Fix Focus Areas
- app.py[462-473]

### Suggested fix
Log a *non-sensitive correlation value* derived from `stream_id` (e.g., a short SHA-256 prefix) or add structured logging fields.

Example:
```py
import hashlib
...
except Lrtmp2ApiError as exc:
   stream_tag = hashlib.sha256(stream_id.encode("utf-8")).hexdigest()[:12]
   app.logger.warning("Delete stream failed (stream_tag=%s): %s", stream_tag, exc)
   session["flash_error"] = str(exc)
```
This preserves correlation without logging the raw user-controlled identifier.

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

---

Previous review results

Review updated until commit e48f568

Results up to commit 203f25c

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

1. Stream ID lost in log ✓ Resolved 🐞 Bug ◔ Observability

Description

In delete_stream(), the warning log on Lrtmp2ApiError no longer includes the stream_id, which makes
it harder to correlate deletion failures to a specific stream during ops/debugging. The handler
already validates stream_id, so this context is available but is dropped in the new log line.

Code

app.py[R468-472]

        try:
            client.delete_stream(stream_id)
        except Lrtmp2ApiError as exc:
-            app.logger.warning("Delete for stream %s failed: %s", stream_id, exc)
+            app.logger.warning("Delete stream failed: %s", exc)
            session["flash_error"] = str(exc)

Relevance

⭐⭐⭐ High

PR #57 accepted delete-failure logging including stream_id; dropping it regresses previously
approved operability improvement.

PR-#57

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The handler validates stream_id but the updated warning log drops it, so the log no longer indicates
which validated stream was being deleted when the exception occurred.

app.py[29-31]
app.py[462-473]
PR-#57

Agent prompt

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

### Issue description
`delete_stream()` logs API deletion failures without identifying which stream was being deleted, reducing diagnostic value of the warning.

### Issue Context
- `stream_id` is already validated by `_is_valid_stream_id()` before the API call.
- This PR removed `stream_id` from the warning to address SonarCloud S5145; reintroducing the raw value may re-trigger the rule.

### Fix Focus Areas
- app.py[462-473]

### Suggested fix
Log a *non-sensitive correlation value* derived from `stream_id` (e.g., a short SHA-256 prefix) or add structured logging fields.

Example:
```py
import hashlib
...
except Lrtmp2ApiError as exc:
   stream_tag = hashlib.sha256(stream_id.encode("utf-8")).hexdigest()[:12]
   app.logger.warning("Delete stream failed (stream_tag=%s): %s", stream_tag, exc)
   session["flash_error"] = str(exc)
```
This preserves correlation without logging the raw user-controlled identifier.

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

---

[AlexanderWagnerDev]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 203f25c869

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cursor]

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_0b42f9ee-81d4-4711-95bb-49596255c153)

[qodo-code-review]

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

[cursor]

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_be26c6ac-86a4-41af-8627-d713ba7b3a5b)

[sonarqubecloud]

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]

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