fix: disable debug=True / Werkzeug debugger exposure (closes #9)#20
fix: disable debug=True / Werkzeug debugger exposure (closes #9)#20
Conversation
The Werkzeug debugger is a documented remote-code-execution primitive. app.py was hard-coding `debug=True`, which exposed RCE to anyone who could reach the listening port — a misconfigured `--host`, an SSH tunnel, or a careless reverse proxy was enough. - Remove the `debug=True` literal from app.py. - Default debug OFF. Opt-in via either `--debug` CLI flag or `FLASK_DEBUG=1` env var (truthy = "1" / "true" / "yes", case-insensitive, whitespace-tolerant). - Print a stderr WARNING when debug is enabled, naming the RCE risk and reminding the operator to bind only to loopback. - Gate the auto-reloader on the same flag. Live-tested all four matrix cells: (default off / --debug / FLASK_DEBUG=1 / FLASK_DEBUG=0). Bogus paths under debug-off return a plain Flask 404, not the Werkzeug debugger console. Helper `resolve_debug_flag(env_value, cli_flag)` lives in `utils/debug_flag.py` so it can be unit-tested without importing Flask (matching the existing test convention in tests/test_cli_args.py). Regression coverage in tests/test_cli_args.py adds 8 cases: - default-off, env-truthy, env-falsey, CLI override - argparse `--debug` default + explicit - source-level guard that fails if `debug=True` is reintroduced
Old guard: `self.assertNotIn("debug=True", src)` — substring match.
That misses cosmetic variants like `debug = True` (with spaces),
multi-line `debug=\n True`, or any other form that produces the
same runtime semantics. CodeRabbit correctly flagged it as evadable.
Replaced with an `ast.walk(tree)` over the parsed app.py: find any
`ast.Call` whose keywords contain `debug=True` as a literal Constant.
Catches every cosmetic variant by definition.
Failure message includes the offending line number(s) and the
rationale (issue #9), so a future CI break is immediately
debuggable.
Verified by injecting `debug = True` (with spaces — the form the
old check missed) into app.py:
- Old check: would have passed (false negative).
- New check: failed with `[136]` and the issue-#9 message.
Then reverted the inject; test passes again.
42/42 tests still pass on the actual app.py.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces unconditional debug=True with an opt-in resolver and CLI flag; integrates resolve_debug_flag() into app startup, prints a security warning when enabled, updates app.run() behavior, adds unit tests and AST regression checks, and documents the new opt-in behavior. ChangesDebug Mode Security: Opt-In Flag & Werkzeug Debugger Exposure Remediation
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Parser
participant Resolver
participant App
User->>CLI: run command (maybe --debug)
CLI->>Parser: parse args (includes --debug)
Parser->>Resolver: provide cli_flag and env FLASK_DEBUG
Resolver->>App: return debug_enabled
App->>App: print stderr warning if debug_enabled
App->>App: call app.run(debug=debug_enabled, use_reloader=...)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
bradjin8
left a comment
There was a problem hiding this comment.
READ.md - mention for the debug is off by default.
PR/issue are security-sensitive; a short note that debug is off by default and enabled only with --debug or FLASK_DEBUG would match operator expectations. Optional; you asked not to expand scope unless useful.
…note
- AST guard now handles ast.NameConstant (Py3.7) and **{"debug":True}
dict-spread bypass; helper extracted for unit testing.
- README: opt-in note for the Werkzeug debugger, including that
FLASK_ENV=development is NOT consulted (only FLASK_DEBUG=1).
- Replace em dashes in app.py comments with ASCII to silence GitHub's
non-ASCII banner on review.
README.md: combine both adjacent additions to the Quick Start area — keep our FLASK_DEBUG opt-in note above the new Tests section that landed on master via PR #23.
Problem
app.pywas hard-codingdebug=Truein itsapp.run(...)call. The Werkzeug debugger that ships with Flask debug mode is a documented remote-code-execution primitive — anyone reaching the listening port (and guessing the PIN, which is generated from machine-stable inputs) can execute arbitrary Python in the server process. The default127.0.0.1bind is the only thing standing between the operator and RCE; a misconfigured--host, an SSH tunnel, or a careless reverse proxy is enough to expose it.There was also no way to opt out — the literal was unconditional.
Change
Three files:
app.py— removeddebug=True. Default debug now off. Opt-in via either--debugCLI flag orFLASK_DEBUG=1env var. When debug is enabled, prints a stderr WARNING naming the RCE risk + reminding the operator to bind only to loopback. Auto-reloader gated on the same flag.utils/debug_flag.py— new tiny module withresolve_debug_flag(env_value, cli_flag). Lives outsideapp.pyso it can be unit-tested without importing Flask (matching the existing_build_app_parsermirror convention in the test suite). Truthy env values:"1","true","yes"— case-insensitive, whitespace-tolerant. CLI flag wins.tests/test_cli_args.py— newTestDebugFlagGatingclass: 8 cases covering env-truthy / env-falsey / env-unset / CLI-override + argparse--debugdefault & explicit + a source-level guard that fails ifdebug=Trueis ever re-introduced as a literal inapp.py.Test plan
Unit:
python3 -m unittest tests.test_cli_args→ 42/42 OK (was 34, +8 new).Live smoke (all four matrix cells, on a real
flask>=3.0install):Debug modeDebugger is active!--debugFLASK_DEBUG=1FLASK_DEBUG=0Bogus paths in trials A/D return a plain Flask 404 — not the orange Werkzeug debugger console. The WARNING prints once per process (parent + reloader child = 2× when the reloader is on, which is correct).
Severity
Critical — Werkzeug debugger exposure is a documented RCE pathway. Listed as a Critical / 1pt item in the eval week-1 plan for
cppa-cursor-browser.Closes #9.
Summary by CodeRabbit
New Features
Tests
Documentation