Hide explorer upstream exception details by rogierx · Pull Request #5482 · Scottcjn/Rustchain

rogierx · 2026-05-16T19:39:08Z

Fixes #5449.

Summary

stops the explorer API from returning raw upstream requests exception strings to clients
logs full upstream failures server-side with route-specific context
preserves the existing /api/miners fallback shape with miners: []
adds regression coverage for /api/miners, /api/network/stats, and /api/miner/<id> using a host/token/path-bearing upstream exception

Validation

/tmp/rustchain-5449-venv/bin/python -m pytest tests/test_explorer_app_upstream_errors.py -q -> 3 passed
/tmp/rustchain-5449-venv/bin/python -m py_compile explorer/app.py tests/test_explorer_app_upstream_errors.py
git diff --check

github-actions · 2026-05-16T19:39:17Z

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!

rogierx · 2026-05-16T19:39:34Z

I tried to add the required BCOS tier label, but fork contributors do not have label permissions (AddLabelsToLabelable is denied for this account). Suggested tier: BCOS-L1 for this small information-disclosure hardening patch.

HCIE2054

LGTM!

HoangTechCS-AIE

I validated the functional hardening path on head 92c003371519ef87432a7252af4486711a6fb035:

python3 -m py_compile explorer/app.py tests/test_explorer_app_upstream_errors.py passed
python3 -m pytest tests/test_explorer_app_upstream_errors.py -q passed with 3 passed in 0.07s
git diff --check origin/main...HEAD -- explorer/app.py tests/test_explorer_app_upstream_errors.py passed

The exception redaction itself looks correct: all three affected API routes now catch requests.exceptions.RequestException, log the full upstream failure server-side via app.logger.exception(...), and return a stable generic client-visible error. The /api/miners fallback shape also keeps miners: [], which matches the existing API contract.

Blocking issue before merge: the newly added code file tests/test_explorer_app_upstream_errors.py is missing an SPDX header. The repo enforces this for added code-like files via tools/bcos_spdx_check.py; running python3 tools/bcos_spdx_check.py --base-ref origin/main currently fails with:

BCOS SPDX check failed. Add an SPDX header to the following new files:
- tests/test_explorer_app_upstream_errors.py

Please add the project-appropriate header near the top of the new test file, e.g. # SPDX-License-Identifier: MIT, then this should be ready from the explorer behavior/security side.

ZacharyZhang-NY

Reviewed and verified.

This PR addresses the information-disclosure issue in #5449 for the three affected explorer API routes. The client-visible responses now use the shared generic error string, and /api/miners preserves its existing fallback shape with miners: [].

Validation run:

git fetch origin pull/5482/head:review-pr-5482 --force
git diff --check origin/main...review-pr-5482 -- explorer/app.py tests/test_explorer_app_upstream_errors.py
python -m py_compile explorer/app.py tests/test_explorer_app_upstream_errors.py
python -m pytest tests/test_explorer_app_upstream_errors.py -q

I ran the compile and targeted pytest checks from a temporary archive of the PR head. Result:

3 passed in 0.23s

The regression test covers:

/api/miners         -> {"error": "Upstream RustChain API unavailable", "miners": []}
/api/network/stats  -> {"error": "Upstream RustChain API unavailable"}
/api/miner/alice    -> {"error": "Upstream RustChain API unavailable"}

It also asserts the simulated leaked substrings are absent from the response body: 127.0.0.1, 8000, super-secret, /srv/rustchain/private, and node.py.

No blocking issue found.

HCIE2054

LGTM!

rogierx · 2026-05-16T20:38:22Z

Addressed the SPDX blocker in c04035f: added the missing SPDX header to tests/test_explorer_app_upstream_errors.py. No behavior changes.

HCIE2054

LGTM!

strongkeep-debug

Approving after validating the current head c04035f. The earlier review concern about the new regression file missing an SPDX header is resolved on this head, and the focused upstream-error redaction path still passes.

Focused validation:

python -m py_compile explorer/app.py tests/test_explorer_app_upstream_errors.py
passed

python -m pytest tests/test_explorer_app_upstream_errors.py -q
...                                                                      [100%]
3 passed in 0.44s

The three covered API routes now return stable generic client-visible errors while preserving the /api/miners response shape with miners: []; full upstream exception detail remains server-side only via logging.

jaxint

LGTM! Thanks for contributing. Approved.

HCIE2054

LGTM!

ZacharyZhang-NY

Reviewed PR #5482 at head c04035f.

Validation performed:

Checked issue #5449. The scoped bug is the explorer API returning raw upstream requests exception text on /api/miners, /api/network/stats, and /api/miner/<miner_id>.
git fetch origin pull/5482/head:review-pr-5482 --force
git diff --check origin/main...review-pr-5482 -- explorer/app.py tests/test_explorer_app_upstream_errors.py -> passed.
python -m py_compile explorer/app.py tests/test_explorer_app_upstream_errors.py on an extracted PR-head tree -> passed.
python -m pytest tests/test_explorer_app_upstream_errors.py -q --confcutdir= -> 3 passed.
Ran Flask test-client probes with an upstream exception containing host, port, token, path, and filename details. /api/miners returned 500 with {"error":"Upstream RustChain API unavailable","miners":[]}; /api/network/stats and /api/miner/alice returned 500 with the generic upstream-unavailable error. None of the sensitive substrings appeared in the client-visible response bodies.

The explorer routes now preserve the /api/miners fallback shape while keeping upstream connection details out of client JSON. Approving.

BossChaos · 2026-05-17T03:28:05Z

Code Review — Bounty #73

PR: Hide explorer upstream exception details by @rogierx
Files changed: 2 (+64/-8)

✅ Code change
✅ Input validation present
✅ Error handling present

Summary

This is a code change PR. Changes appear consistent with project patterns.

Wallet: 0xdaE5d307339074A24F579dB48e7c639359D94904

Reviewing under Bounty #73 — Code Review Bounty Program

HCIE2054

LGTM!

HCIE2054

LGTM!

HCIE2054

LGTM!

kekehanshujun

Approved. I reviewed the explorer upstream error redaction patch for issue #5449.

Validation performed:

Applied the PR patch locally against the PR base.
git diff --check -- explorer/app.py tests/test_explorer_app_upstream_errors.py passed; the only local note was the existing CRLF/LF normalization warning for explorer/app.py.
python -B -m py_compile explorer/app.py tests/test_explorer_app_upstream_errors.py passed.
python -B -m pytest -q tests/test_explorer_app_upstream_errors.py --noconftest -> 3 passed.

The implementation replaces client-visible raw requests exception text with a shared generic upstream-unavailable response, keeps the existing /api/miners fallback shape with miners: [], and logs the detailed upstream exception server-side with route-specific context. The regression test covers all three affected API routes and checks that host, token, and private path substrings are not leaked.

BossChaos · 2026-05-17T08:28:02Z

Code Review — Bounty #73

PR: Hide explorer upstream exception details by @rogierx

✅ Code review

Wallet: 0xdaE5d307339074A24F579dB48e7c639359D94904

Reviewing under Bounty #73

508704820

Hide explorer upstream exception details. Good - prevents leaking internal service URLs and error details. Verify all upstream error handlers follow this pattern. - Xeophon (security review)

HCIE2054

LGTM!

HCIE2054

LGTM!

TJCurnutte

Approved. I focused on the explorer upstream-error redaction path.

Validation performed on head c04035f27dbe944a6f8b1317169345ea7284370b:

git diff --check origin/main...HEAD -- explorer/app.py tests/test_explorer_app_upstream_errors.py passed.
python3 -B -m py_compile explorer/app.py tests/test_explorer_app_upstream_errors.py passed.
uv run --no-project --with pytest --with flask --with requests python -B -m pytest -q -o addopts= tests/test_explorer_app_upstream_errors.py --noconftest -p no:cacheprovider passed with 3 passed in 0.23s.
A Flask runtime probe monkeypatched requests.get() to raise HTTPConnectionPool(host='127.0.0.1', port=8000): token=super-secret trace=/srv/rustchain/private/node.py; /api/miners, /api/network/stats, and /api/miner/alice all returned HTTP 500 with the generic Upstream RustChain API unavailable JSON shape and leaks=false for the host/token/private-path strings.

The public response boundary is clean now: upstream exception details stay out of API JSON, /api/miners keeps the existing miners: [] client shape, and the full exception is only logged internally for operators.

508704820 · 2026-05-17T11:30:56Z

Security Review ✅

Same class as #5545: stops explorer from returning raw upstream exception strings. Logs failures server-side, returns generic error.

Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ

HCIE2054

LGTM!

HCIE2054

LGTM!

Scottcjn · 2026-05-18T03:00:56Z

Closing as duplicate of #5450 by @hungle123-dev (first-posted 5h earlier on the same issue).

Per first-poster rule, only the original submitter gets bounty credit on a given issue. The fix in #5450 addresses the same root cause and is already under review.

Appreciate the effort — for next time, please check open PRs against the same issue before filing. Thanks for the contribution.

Hide explorer upstream exception details

92c0033

github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes size/M PR: 51-200 lines labels May 16, 2026

HCIE2054 approved these changes May 16, 2026

View reviewed changes

HoangTechCS-AIE suggested changes May 16, 2026

View reviewed changes

ZacharyZhang-NY approved these changes May 16, 2026

View reviewed changes

ZacharyZhang-NY mentioned this pull request May 16, 2026

[Bounty Claim] PR Review - RustChain PR #5482 (Bounty #73) Scottcjn/rustchain-bounties#10159

Open

HCIE2054 approved these changes May 16, 2026

View reviewed changes

Add SPDX header to explorer upstream tests

c04035f

HCIE2054 approved these changes May 16, 2026

View reviewed changes

strongkeep-debug approved these changes May 16, 2026

View reviewed changes

strongkeep-debug mentioned this pull request May 16, 2026

[Bounty Claim] Code review batch for Bounty #73 Scottcjn/rustchain-bounties#10172

Open

jaxint approved these changes May 16, 2026

View reviewed changes

HCIE2054 approved these changes May 16, 2026

View reviewed changes

ZacharyZhang-NY approved these changes May 16, 2026

View reviewed changes

ZacharyZhang-NY mentioned this pull request May 16, 2026

[Bounty Claim] PR Review - RustChain PR #5482 (Bounty #305) Scottcjn/rustchain-bounties#10231

Open

HCIE2054 approved these changes May 17, 2026

View reviewed changes

kekehanshujun approved these changes May 17, 2026

View reviewed changes

kekehanshujun mentioned this pull request May 17, 2026

[Bounty Claim] PR Review - RustChain PR #5482 (Bounty #73) Scottcjn/rustchain-bounties#10457

Open

508704820 reviewed May 17, 2026

View reviewed changes

HCIE2054 approved these changes May 17, 2026

View reviewed changes

TJCurnutte approved these changes May 17, 2026

View reviewed changes

TJCurnutte mentioned this pull request May 17, 2026

[BOUNTY] Code Review Bounty Program — Review PRs, Earn RTC (100 RTC Pool) Scottcjn/rustchain-bounties#73

Open

508704820 mentioned this pull request May 17, 2026

fix(#5442): hide internal exception details in dashboard health endpoint #5470

Closed

HCIE2054 approved these changes May 17, 2026

View reviewed changes

Scottcjn closed this May 18, 2026

Uh oh!

Conversation

rogierx commented May 16, 2026

Summary

Validation

Uh oh!

github-actions Bot commented May 16, 2026

Uh oh!

rogierx commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HoangTechCS-AIE left a comment

Choose a reason for hiding this comment

Uh oh!

ZacharyZhang-NY left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

rogierx commented May 16, 2026

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

strongkeep-debug left a comment

Choose a reason for hiding this comment

Uh oh!

jaxint left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

ZacharyZhang-NY left a comment

Choose a reason for hiding this comment

Uh oh!

BossChaos commented May 17, 2026

Code Review — Bounty #73

Summary

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

kekehanshujun left a comment

Choose a reason for hiding this comment

Uh oh!

BossChaos commented May 17, 2026

Code Review — Bounty #73

Uh oh!

508704820 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

TJCurnutte left a comment

Choose a reason for hiding this comment

Uh oh!

508704820 commented May 17, 2026

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

HCIE2054 left a comment

Choose a reason for hiding this comment

Uh oh!

Scottcjn commented May 18, 2026

rogierx commented May 16, 2026 •

edited

Loading