fix: hide dashboard health exception details#5443
Conversation
github-actions
Bot
added
BCOS-L1
kekehanshujun
left a comment
kekehanshujun
left a comment
There was a problem hiding this comment.
Approved. I reviewed the dashboard health exception redaction path and did not find a blocker.
What I checked:
bridge/dashboard_api.pynow logs raw database, Solana RPC, and wRTC mint lookup exceptions server-side withlogger.exception(...)instead of returningstr(e)to clients.- The response shape still preserves per-component health/degraded state, but the client-facing detail strings are generic:
Database unavailable,RPC unavailable, andMint check unavailable. - The new tests inject exception messages containing internal paths, RPC URLs/tokens, and configured mint identifiers, then assert those values are absent from the API response.
Validation on head ffe0ff38f449df8003b5f59a0de115cdb98ac508:
$env:PYTHONPATH=.tmp-pr5443-files; python -m pytest .tmp-pr5443-files\bridge\test_dashboard_api.py::TestBridgeHealth -q --confcutdir=.tmp-pr5443-files-> 7 passed$env:PYTHONPATH=.tmp-pr5443-files; python -m pytest .tmp-pr5443-files\bridge\test_dashboard_api.py -q --confcutdir=.tmp-pr5443-files-> 25 passedpython -m py_compile .tmp-pr5443-files\bridge\dashboard_api.py .tmp-pr5443-files\bridge\test_dashboard_api.py .tmp-pr5443-files\bridge\bridge_api.py-> passed
No blocking issue found in this scope.
HCIE2054
left a comment
HCIE2054
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing!
ZacharyZhang-NY
left a comment
ZacharyZhang-NY
left a comment
There was a problem hiding this comment.
Reviewed PR #5443 at head ffe0ff3.
Validation performed:
- git fetch origin pull/5443/head:review-pr-5443 --force
- git diff --check origin/main...review-pr-5443 -- bridge/dashboard_api.py bridge/test_dashboard_api.py -> passed.
- python -m py_compile bridge/dashboard_api.py bridge/test_dashboard_api.py bridge/bridge_api.py on an extracted PR-head tree -> passed.
- python -m pytest bridge/test_dashboard_api.py::TestBridgeHealth -q --confcutdir= -> 7 passed.
- python -m pytest bridge/test_dashboard_api.py -q --confcutdir= -> 25 passed.
- Checked issue #5442, which scopes the leak to database, Solana RPC, and configured wRTC mint health-check exceptions in
/bridge/dashboard/health. - Compared PR #5457, which is a parallel fix for the same issue.
The health endpoint still reports component status and generic detail strings, while raw database path/RPC URL/mint lookup exception text is logged server-side with logger.exception(...) instead of being returned to clients. This satisfies the issue-scoped redaction behavior; this branch overlaps PR #5457, so the maintainer can choose the preferred implementation.
TJCurnutte
left a comment
TJCurnutte
left a comment
There was a problem hiding this comment.
Scoped review on the dashboard health error redaction change looks good.
Validated head ffe0ff38f449df8003b5f59a0de115cdb98ac508 locally with:
git diff --check origin/main...HEAD -- bridge/dashboard_api.py bridge/test_dashboard_api.py
PYTHONPATH=. python3 -B -m py_compile bridge/dashboard_api.py bridge/test_dashboard_api.py
PYTHONPATH=. uv run --no-project --with pytest --with flask python -B -m pytest -q bridge/test_dashboard_api.py --noconftestResult: 25 passed in 1.85s.
The patch keeps the health endpoint operational while moving raw DB/RPC/mint exception details out of the public JSON body. The new tests monkeypatch secret-bearing database and RPC failures, verify the endpoint reports generic Database unavailable, RPC unavailable, and Mint check unavailable messages, and assert those secret exception strings are absent from the response. Server-side detail is still retained through logger.exception, which is the right boundary.
I did not find a blocker in this scoped pass.
Code Review — Bounty #73PR: fix: hide dashboard health exception details by @hungle123-dev
SummaryThis is a bug fix PR. Changes appear consistent with project patterns. Wallet: Reviewing under Bounty #73 — Code Review Bounty Program |
8 participants
Fixes #5442.
Summary
logger.exception(...)./bridge/dashboard/healthstill reports degraded/offline status without leaking local paths, private RPC URLs, tokens, or internal exception text.Root cause
The dashboard health endpoint converted caught exceptions directly into response details with
str(e). That made diagnostic internals visible to API callers whenever database/RPC/mint checks failed.Validation
python -m pytest bridge/test_dashboard_api.py::TestBridgeHealth::test_health_hides_database_exception_details -qfailed because the response leakedC:/srv/rustchain/private/bridge.db.python -m pytest bridge/test_dashboard_api.py::TestBridgeHealth::test_health_hides_rpc_exception_details -qfailed because the response leakedhttps://internal-solana.local/rpc?token=secret.python -m pytest bridge/test_dashboard_api.py::TestBridgeHealth::test_health_hides_mint_exception_details -qfailed because the response leaked the configured mint lookup exception.python -m pytest bridge/test_dashboard_api.py::TestBridgeHealth -q->7 passed.python -m pytest bridge/test_dashboard_api.py -q->25 passed.python -m py_compile bridge\dashboard_api.py bridge\test_dashboard_api.py-> passed.git diff --check -- bridge/dashboard_api.py bridge/test_dashboard_api.py-> passed.