fix: disable bridge debug server#5488
Conversation
|
Welcome to RustChain! Thanks for your first pull request. Before we review, please make sure:
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
Bot
added
BCOS-L1
ZacharyZhang-NY
left a comment
ZacharyZhang-NY
left a comment
There was a problem hiding this comment.
Reviewed PR #5488 at head $commit.
Validation performed:
- python -m py_compile bridge_api.py test_bridge_api.py on the PR-head extracted �ridge/ files.
- python -m pytest test_bridge_api.py::test_standalone_bridge_server_does_not_enable_debugger -q -> 1 passed.
- git diff --check origin/main...review-pr-5488 -- bridge/bridge_api.py bridge/test_bridge_api.py -> passed.
- Checked issue #5059 comments: the additional Flask debug instance explicitly includes �ridge/bridge_api.py, while PR #5118 covers keeper_explorer.py and contributor_registry.py.
The bridge standalone entrypoint keeps the same host and port while setting debug=False, and the regression guards the bridge source against reintroducing debug=True. Approval.
|
Clarification for my review above: the reviewed head was $commit. Validation performed:
|
ZacharyZhang-NY
left a comment
ZacharyZhang-NY
left a comment
There was a problem hiding this comment.
Reviewed PR #5488 at head cef49fb.
Validation performed:
- Checked issue #5059 context. This PR is scoped to the standalone RIP-305 bridge dev server entrypoint and removes the exposed Flask debugger setting there.
- git fetch origin pull/5488/head:review-pr-5488 --force
- git diff --check origin/main...review-pr-5488 -- bridge/bridge_api.py bridge/test_bridge_api.py -> passed.
- python -m py_compile bridge/bridge_api.py bridge/test_bridge_api.py on an extracted PR-head tree -> passed.
- python -m pytest bridge/test_bridge_api.py::test_standalone_bridge_server_does_not_enable_debugger -q --confcutdir= -> 1 passed.
- python -m pytest bridge/test_bridge_api.py -q --confcutdir= -> 42 passed.
The bridge standalone entrypoint now runs with debug=False, and the focused regression plus full bridge API test file pass. Approving.
kekehanshujun
left a comment
kekehanshujun
left a comment
There was a problem hiding this comment.
Approved. I verified the standalone bridge dev server no longer enables the Flask debugger and did not find a blocking issue.
Validation performed:
python -m py_compile bridge\bridge_api.py bridge\test_bridge_api.pypython -m pytest bridge\test_bridge_api.py::test_standalone_bridge_server_does_not_enable_debugger -qpython -m pytest bridge\test_bridge_api.py -qgit diff --check origin/main...HEAD -- bridge/bridge_api.py bridge/test_bridge_api.py
The focused regression covers the __main__ server entrypoint, and the full bridge API suite passed with 42 tests.
Code Review — Bounty #73PR: fix: disable bridge debug server by @rogierx
SummaryThis is a bug fix PR. Changes appear consistent with project patterns. Wallet: Reviewing under Bounty #73 — Code Review Bounty Program |
TJCurnutte
left a comment
TJCurnutte
left a comment
There was a problem hiding this comment.
Approved. I checked the standalone bridge dev-server hardening and the regression coverage.
Validation performed on head cef49fb7d674c80284a929c57ddb310d2578d874:
git diff --check origin/main...HEAD -- bridge/bridge_api.py bridge/test_bridge_api.pypassed.PYTHONPATH=bridge:. python3 -B -m py_compile bridge/bridge_api.py bridge/test_bridge_api.pypassed.PYTHONPATH=bridge:. uv run --no-project --with pytest --with flask --with requests --with flask-cors python -B -m pytest -q bridge/test_bridge_api.py --noconftest -p no:cacheproviderreturned42 passed in 0.76s.- Source probe returned
debug_true_count 0,debug_false_count 1, andtest_guard_present True.
Reasoning: the only standalone app.run(...) path in bridge/bridge_api.py now binds with debug=False, so the Werkzeug debugger is not exposed when this dev server listens on 0.0.0.0:8096. The added test guards against reintroducing debug=True in that file.
508704820
left a comment
508704820
left a comment
There was a problem hiding this comment.
Disable bridge debug server. CRITICAL - debug servers in production allow unauthenticated access to internal state. Verify: (1) Debug server is disabled in ALL environments, not just production. (2) No other debug endpoints remain enabled. (3) Environment variable to enable debug has a non-obvious default (disabled). - Xeophon (security review)
|
Security Review ✅ Same class as #5531: disables Flask debug=True on the bridge dev server. Prevents Werkzeug debugger RCE. Regression test ensures debug=True cannot be reintroduced. Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ |
jaxint
left a comment
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing. Approved.
|
Claiming the accepted/merged RustChain bounty for this PR. RustChain RTC payout address: RTC88d4b01c4dd3c2c05b07b8d7ff96f4417aeffaff No private wallet material is included. |
9 participants
Summary
Notes
This is intentionally scoped to the bridge instance from the #5059 discussion and avoids touching contributor_registry.py / keeper_explorer.py, which are already covered by PR #5118.
Checks