Skip to content

Disable public Flask debug entrypoints#5686

Open
dazer1234 wants to merge 1 commit into
Scottcjn:mainfrom
dazer1234:codex-disable-public-flask-debug
Open

Disable public Flask debug entrypoints#5686
dazer1234 wants to merge 1 commit into
Scottcjn:mainfrom
dazer1234:codex-disable-public-flask-debug

Conversation

@dazer1234
Copy link
Copy Markdown
Contributor

@dazer1234 dazer1234 commented May 18, 2026

Summary

  • Disables public Flask debug mode entrypoints to avoid exposing interactive debugger behavior.

Validation

  • py_compile and diff checks passed at branch creation; focused Flask tests blocked locally because Flask is not installed.

Bounty reference: Scottcjn/rustchain-bounties#71 (Ongoing Bug Bounty Program)
Bounty fit: Security hardening bug: public Flask debug entrypoints could expose Werkzeug debugger behavior.
RTC payout wallet: RTC0a1c0ce2204390bc49ecf9780fe894da9dc3d92c

Implemented with OpenAI Codex assistance.

@github-actions
Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/S PR: 11-50 lines labels May 18, 2026
Copy link
Copy Markdown
Contributor

@HCIE2054 HCIE2054 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@guangningsun
Copy link
Copy Markdown
Contributor

PR Review — PR #5686

What I reviewed:

  • bridge/bridge_api.py, contributor_registry.py, explorer/app.py — three files changed from debug=True to debug=False

Observations:

  1. This is a security fix — debug=True in Flask exposes the interactive debugger and Werkzeug console on publicly accessible servers, allowing RCE through carefully crafted traceback requests
  2. All three files follow the same pattern: app.run(host="0.0.0.0", port=..., debug=False) — consistent change across all affected entry points
  3. explorer/app.py is missing a newline at end of file (\ No newline at end of file) — minor style issue but not a functional problem
  4. The debug=False change in contributor_registry.py is inside an if __name__ == '__main__': block — this only affects direct execution, not gunicorn or other WSGI runners, which is appropriate since those have their own deployment-level debug settings

LGTM. Security fix is straightforward and correct. The missing newline is cosmetic.

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work on this PR. 🚀

@Auren-Innovation
Copy link
Copy Markdown

Code Review: PR #5686

Title: Disable public Flask debug entrypoints
Review Type: Security
Recommended Reward: 12-15 RTC


Summary

This PR sets debug=False on app.run() calls in five Flask entrypoints that were previously running with debug=True while bound to 0.0.0.0 (all interfaces). It also adds a unit test that asserts none of those files contain debug=True or debug = True strings, preventing regression.

Critical

None

Warning

  • Binding to 0.0.0.0 remains unchanged — While disabling debug mode removes the Werkzeug interactive debugger (the most severe exposure), all five services still bind to all network interfaces. In a production or staging environment, these services are reachable from any network the host is on. Even without debug mode, Flask application errors will still produce traceback-laden 500 responses by default unless PROPAGATE_EXCEPTIONS is set to False or a custom error handler is installed. The combination of 0.0.0.0 binding and default error disclosure is a defense-in-depth gap. Fix: add host="127.0.0.1" to each app.run() call, or document that a reverse proxy / firewall is expected to restrict access.

  • app.run() calls are likely dead code in production — These app.run(debug=True) calls at the bottom of each module are development convenience entrypoints, not production server configurations (production would use gunicorn, uwsgi, etc.). The real risk is if someone runs these files directly in a production-like environment. The PR correctly disables debug, but does not add a guard that prevents accidental direct execution in production. Consider adding an if __name__ == "__main__" guard (some files may lack it) or a startup warning when the module is executed directly.

  • Test only checks for debug=True literal — The test in test_flask_debug_disabled.py searches for the string "debug=True" in source files. This can be bypassed with debug = True (with spaces around =) which is handled, but also with indirect patterns like debug=(1==1), debug=bool(1), or debug=env_var == "true". A more robust approach would be to parse the AST and check the keyword argument value of app.run() calls.

Suggestion

  • Add app.config["PROPAGATE_EXCEPTIONS"] = False or register a generic 500 error handler in each service to prevent stack trace leakage on unhandled exceptions, since these services are still bound to 0.0.0.0.

  • The test file could assert host="127.0.0.1" instead of or in addition to checking debug status, since the 0.0.0.0 binding is arguably the more persistent exposure.

  • Consider moving app.run() calls behind an environment gate such as if os.environ.get("FLASK_DEV_SERVER"): so they are never accidentally invoked in production without an explicit opt-in.

  • profile_badge_generator.py line 248 — This is the only file that does not explicitly set host="0.0.0.0", it relies on Flask default (127.0.0.1). This is actually the safer default. Consider making the other four files consistent by also omitting host or setting it to 127.0.0.1.

Verdict

Approve — The change is correct and addresses a real security misconfiguration (debug=True on publicly bound Flask servers). The warnings above are defense-in-depth improvements that can be addressed in follow-up PRs. The core fix is sound and the regression test is a practical safeguard.


Review by Herr Amano | 2026-05-19
Wallet: herr-amano

Copy link
Copy Markdown
Contributor

@HCIE2054 HCIE2054 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Copy Markdown

@TJCurnutte TJCurnutte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m requesting changes here. The PR removes several direct Flask debug entrypoints and its new focused test passes, but the stated sweep is incomplete on this head and GitHub reports the branch as dirty.

Validation I ran on head 8efb1bdc886c558473b3467e2b21de52825701ce:

  • git diff --check origin/main...HEAD -- bridge/bridge_api.py contributor_registry.py explorer/app.py keeper_explorer.py profile_badge_generator.py tests/test_flask_debug_disabled.py — passed.
  • python3 -B -m py_compile bridge/bridge_api.py contributor_registry.py explorer/app.py keeper_explorer.py profile_badge_generator.py tests/test_flask_debug_disabled.py — passed.
  • python3 -B -m unittest discover -s tests -p 'test_flask_debug_disabled.py' -v — passed (Ran 1 test ... OK).
  • Repository scan at PR head for debug\s*=\s*True|debug=True still found non-test debug runs.

Concrete issue:

# security_test_payment_widget.py:275
app.run(debug=True, host='0.0.0.0', port=5000)

xss_poc_templates.py:421 also still has app.run(debug=True, port=5001). If those are intentionally local-only/security-fixture entrypoints, the PR should document that boundary and the regression test should encode it. As written, tests/test_flask_debug_disabled.py only checks five hard-coded files, so it misses at least the public-hosted security_test_payment_widget.py case.

Please either include the remaining debug entrypoint(s) in the fix or explicitly exclude them with rationale plus test coverage. This also needs rebase/conflict resolution because GitHub reports mergeable_state=dirty.

@kevinyan911
Copy link
Copy Markdown

kevinyan911 commented May 19, 2026

Code Review: Disable public Flask debug entrypoints

Reviewed PR #5686 | Files: 6 | +59/-6

Security Fix -- Correct and Important

Changing debug=True to debug=False across 5 public entrypoints is the right call. Flask's debug mode exposes the interactive debugger and Werkzeug stack traces to anyone who can reach the endpoint -- a serious security risk. Ship it.


Observation 1 -- The test is well-structured

test_flask_debug_disabled.py correctly uses subTest for parameterized testing across all 5 files. Good: it resolves the repo root via Path(__file__).resolve().parents[1] rather than relying on cwd. One small improvement: the test checks "debug=True" but could also handle "debug = True" (with spaces) for completeness.


Observation 2 -- Consider pinning host to 127.0.0.1 for local services

All 5 entrypoints still bind to host='0.0.0.0'. For local-only services like profile_badge_generator.py on port 5003, binding to 127.0.0.1 instead would add defense-in-depth -- even if debug mode were somehow re-enabled, the debugger would not be accessible externally. Low priority but worth noting.


Summary

Clear security fix with solid regression test. LGTM.

I received RTC compensation for this review.

TRC20 Wallet Address: TGHW2YTps9PntWQRPpkaNcxQhfjyr9oovQ

@wybbb123123
Copy link
Copy Markdown

This closes several literal Flask debug=True entrypoints, but it still leaves public debug switches that can enable Werkzeug debug mode at runtime.

Examples still present on this PR branch:

# bounties/issue-2312/src/relic_market_api.py
parser.add_argument('--debug', action='store_true', help='Enable debug mode')
...
app.run(host=args.host, port=args.port, debug=args.debug)
# faucet_service/faucet_service.py
parser.add_argument('--debug', action='store_true', help='Enable debug mode')
...
app.run(host=args.host, port=args.port, debug=args.debug)
# tools/bcos_badge_generator.py
parser.add_argument('--debug', action='store_true', help='Enable debug mode')
...
app.run(host=args.host, port=args.port, debug=args.debug)

The new tests/test_flask_debug_disabled.py only checks a fixed list of five files and only rejects literal debug=True / debug = True. It does not scan the remaining Flask entrypoints above, and it does not reject non-literal debug keyword values such as debug=args.debug. That means python ... --debug --host 0.0.0.0 still exposes Flask debug mode on public bind addresses.

The docs also still advertise these unsafe invocations, for example:

tools/BCOS_BADGE_GENERATOR.md: python bcos_badge_generator.py --port 5000 --host 0.0.0.0 --debug
faucet_service/README.md: python faucet_service.py --host 0.0.0.0 --debug
bounties/issue-2312/README.md: python relic_market_api.py --host 0.0.0.0 --port 5000 --debug

Suggested fix: remove/deprecate the public --debug flags or force debug=False at app.run() for these entrypoints. The regression test should scan all public Flask entrypoints and reject any non-literal value flowing into the debug= keyword, not only literal True.

AI assistance disclosure: OpenAI Codex / GPT-5 was used to inspect the diff and prepare this review; the finding was verified against the PR branch contents and the new test logic.

Copy link
Copy Markdown

@crystal-tensor crystal-tensor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved: Security hardening - input validation and error handling improvements.

@JeremyZeng77
Copy link
Copy Markdown

Reviewed this Flask debug-entrypoint hardening PR for the code review bounty.

What I checked:

  • The public standalone entrypoints in bridge/bridge_api.py, contributor_registry.py, explorer/app.py, keeper_explorer.py, and profile_badge_generator.py now pass debug=False instead of enabling the interactive debugger.
  • The new regression test scans the relevant entrypoint files for accidental debug=True reintroduction.
  • No runtime behavior besides debug-mode disabling is changed in the touched entrypoints.

Verification I ran locally:

  • python -m py_compile bridge/bridge_api.py contributor_registry.py explorer/app.py keeper_explorer.py profile_badge_generator.py tests/test_flask_debug_disabled.py -> passed, with an existing keeper_explorer.py string escape SyntaxWarning unrelated to this patch
  • python -B -m pytest -q tests/test_flask_debug_disabled.py --tb=short -p no:cacheprovider -> 1 passed

Assessment: no blocker found. The patch is small, scoped to the public debug exposure, and has a focused regression test.

@kevinyan911
Copy link
Copy Markdown

Code Review: Disable public Flask debug entrypoints

PR: #5686 | Author: @dazer1234 | Files: 5 | Reward Range: 5-10 RTC (Standard)

Security Impact: HIGH

debug=True in Flask enables the werkzeug interactive debugger on public networks, enabling remote code execution. This is a real attack surface.

Files Reviewed

bridge/bridge_api.py, contributor_registry.py, explorer/app.py, keeper_explorer.py, profile_badge_generator.py

All 5 files: disable debug mode in production entrypoints. The fix is minimal, targeted, and correct.

Code Quality: ✓

  • Clean one-liner replacements
  • No side effects
  • Follows least-privilege principle

Non-blocking Suggestion

Consider adding a health check endpoint as a replacement for debug-based troubleshooting.

Recommendation: LGTM ✓

Merging this closes a real attack surface.


Hermes Agent | Code Review Bounty #73

Copy link
Copy Markdown
Contributor

@BossChaos BossChaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — Disable public Flask debug entrypoints

✅ Strengths

  • Security: disable debug endpoints
  • Consistent with the validation pattern seen in other PRs
  • 82 lines changed (36 additions)

⚠️ Issues

1. Validation Coverage
The diff validates the structure but doesn't test:

  • Edge cases (empty strings, null values, oversized payloads)
  • Error messages for invalid input

2. Test Coverage
No tests included. Should add test cases for:

  • Valid input (should pass)
  • Invalid input (should reject with clear error)
  • Boundary conditions

📋 Summary

Good contribution. Security importance: HIGH. Suggested: 12-15 RTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/S PR: 11-50 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.