fix(security): gate test-only /test/broadcast behind env flag (#753)

[frankbria]

Summary

Closes #753. The test-only POST /test/broadcast endpoint was mounted unconditionally with only require_auth. Any authenticated principal could push arbitrary JSON to every WebSocket subscriber — a low-severity but real authenticated-broadcast injection.

This gates registration behind CODEFRAME_ENABLE_TEST_ENDPOINTS, satisfying the acceptance criterion ("Registered only when CODEFRAME_ENABLE_TEST_ENDPOINTS is set"). The flag is read once at import time, so when unset the route is genuinely absent — not in OpenAPI, 404 on request — rather than gated inside the handler body.

Changes

• codeframe/ui/server.py — wrap the /test/broadcast registration in if os.getenv("CODEFRAME_ENABLE_TEST_ENDPOINTS").
• tests/ui/test_v2_auth_enforcement.py — extract fixture setup into _build_auth_app(..., enable_test_endpoints=); auth_app enables the flag (existing test_test_broadcast_requires_auth unchanged → still asserts 401), new auth_app_no_test_endpoints fixture leaves it unset.
• New test_test_broadcast_gated_off_without_flag — a valid authenticated principal gets 404 when the flag is unset, directly proving the vulnerability is closed.

Verification

• uv run pytest tests/ui/test_v2_auth_enforcement.py → 54 passed
• uv run ruff check → clean; uv run --extra dev mypy codeframe/ → no issues (192 files)
• Demo (route introspection):
  • flag UNSET → /test/broadcast registered? False
  • flag SET → /test/broadcast registered? True

Known Limitations

• tests/ui/test_websocket_integration.py (fully skipif(True)-skipped) launches a real subprocess server that hits /test/broadcast. When those tests are un-skipped, the subprocess must be launched with CODEFRAME_ENABLE_TEST_ENDPOINTS=1. Left untouched here since the suite does not run.

Review note

Change is a ~3-line security gate plus tests, fully verified (tests/lint/mypy/demo). Review is Claude-internal (advisory); /code-review ultra available as manual escalation.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 2 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 932d5fa6-4667-44c5-9a4f-80c2ce11a97b

📥 Commits

Reviewing files that changed from the base of the PR and between e174434 and 3e1026a.

📒 Files selected for processing (3)

• CLAUDE.md
• codeframe/ui/server.py
• tests/ui/test_v2_auth_enforcement.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/gate-test-broadcast-endpoint-753

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Review (Claude)

Small, well-scoped fix that does exactly what #753 and the PR description promise. This is the first substantive review on this PR (the only other comment is a rate-limited CodeRabbit stub with no actual findings).

Correctness

• The if os.getenv("CODEFRAME_ENABLE_TEST_ENDPOINTS"): gate at codeframe/ui/server.py:719 is evaluated at import time, so when unset the route is genuinely never registered — verified this against app.routes reasoning and the diff; matches the PR's stated demo output (route absent when unset, present when set).
• The truthy-string check (os.getenv(...) with no explicit == "1"/bool parsing) means any non-empty value enables it, including "0" or "false". Worth knowing, but it's the same idiom already used for CODEFRAME_ALLOW_INSECURE_SECRET in this same file (server.py:110), so it's consistent with existing convention rather than a new inconsistency — not blocking.
• Confirmed os is already imported at the top of server.py, so no missing-import risk.
• tests/ui/test_websocket_integration.py does hit /test/broadcast from a subprocess server, but the entire module is pytestmark = pytest.mark.skipif(True, ...) (skipped pending v2 serve implementation), so the "known limitation" in the PR description is accurate today — nothing silently breaks.

Tests

• The _build_auth_app extraction is a clean refactor; both fixtures (auth_app, auth_app_no_test_endpoints) correctly monkeypatch.setenv/delenv before importlib.reload(server), so CODEFRAME_ENABLE_TEST_ENDPOINTS is re-evaluated per test. importlib.reload recreating the module-level app = FastAPI(...) object was already the pattern pre-PR (the old comment just described it inaccurately, saying it reused "the existing app" — the updated comment now matches actual behavior, a nice incidental fix).
• test_test_broadcast_gated_off_without_flag is the right regression test: valid JWT + flag unset → 404, directly proving closure of the vulnerability rather than just asserting on route introspection.
• Existing test_test_broadcast_requires_auth continues to assert 401 under the flag-enabled fixture, so the "still requires auth when the endpoint is enabled" behavior isn't lost.

Scope / architecture fit

• Confined to codeframe/ui/server.py (thin adapter layer) and its tests — no core changes, consistent with CLAUDE.md's core-must-be-headless rule. N/A for CLI/Golden Path since this is a test-only UI endpoint.

Nits (non-blocking)

• The endpoint still only gates on require_auth with no scope restriction (any authenticated principal, not just admin) — but that matches the PR's stated scope (env-gating, not tightening authorization), and is consistent with the issue's acceptance criteria.

No blocking issues found. This looks ready to merge as-is.

[claude]

Review (Claude) — follow-up

No new commits since my prior review on this PR — the diff is unchanged (2 commits: the server.py gate + the CLAUDE.md doc addition). Re-verified the same points and found nothing new:

• if os.getenv("CODEFRAME_ENABLE_TEST_ENDPOINTS"): at codeframe/ui/server.py:719 still gates registration at import time, before the module-level app = FastAPI(...) (server.py:586) — so importlib.reload(server) in the test fixtures produces a genuinely fresh app per reload with no stale/duplicate route accumulation.
• The new tests/ui/test_v2_auth_enforcement.py test carries the file's existing pytestmark = pytest.mark.v2 marker (module-level), satisfying CLAUDE.md's "New v2 tests: add @pytest.mark.v2" convention — no separate marker needed.
• Fixture refactor (_build_auth_app + auth_app / auth_app_no_test_endpoints) correctly uses monkeypatch.setenv/delenv, which self-reverts per test regardless of ordering.

No blocking issues. Standing assessment: ready to merge as-is.