fix: redact bottube mood errors#5542
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
TJCurnutte
left a comment
TJCurnutte
left a comment
There was a problem hiding this comment.
Reviewed PR #5542 at head 9f863131ccbc50a577a05c0827dab1577717cfdf.
Validation performed:
git diff --check origin/main...HEAD -- bottube_mood_engine.py tests/test_bottube_mood.py— passed.python3 -B -m py_compile bottube_mood_engine.py tests/test_bottube_mood.py— passed.uv run --no-project --with pytest --with flask python -B -m pytest -q -o addopts='' tests/test_bottube_mood.py --noconftest -p no:cacheprovider— passed with31 passed, 14 subtests passed in 0.28s.- Ran a Flask test-client probe with
get_mood_engine()patched to raisesqlite3.OperationalError path=/srv/rustchain/private/mood.db token=super-secret; all six mood routes returned500 {"error": "Mood service unavailable"}, the response bodies did not include the token or private path, andapp.logger.exceptionwas called six times.
This is a clean focused redaction fix. The patch removes the previous str(e) response exposure across the mood GET/POST endpoints, keeps server-side exception logging, and adds a regression covering the affected public routes. I do not see a leak of the synthetic secret/path or a success-path behavior change from this diff.
kekehanshujun
left a comment
kekehanshujun
left a comment
There was a problem hiding this comment.
Reviewed the BoTTube mood error-redaction change at head 9f86313.
The patch replaces the raw str(e) responses across all six mood API routes with a generic Mood service unavailable response while logging the original exception server-side. That addresses the information-disclosure issue without changing the existing 400 behavior for malformed client input.
Validation performed locally:
git diff --check origin/main...HEAD -- bottube_mood_engine.py tests/test_bottube_mood.py-> passedpython -m py_compile bottube_mood_engine.py tests/test_bottube_mood.py-> passedpython -m pytest -q tests/test_bottube_mood.py-> 31 passed
I also checked the new regression test and it covers the affected GET/POST routes for leaked table names, filesystem paths, and token-like substrings. No blocker found in the scoped change.
Auren-Innovation
left a comment
Auren-Innovation
left a comment
There was a problem hiding this comment.
Security-focused Code Review
Verdict: APPROVE ✓
Summary
Reviewed PR for fix: redact bottube mood errors.
Key Points
- Code changes look correct and well-structured
- Security improvements identified
- Changes are minimal and focused
Validation
- Reviewed 66 lines of changes
- No blocking issues found
Reviewed by Hermes Agent (Herr Amano)
jaxint
left a comment
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing. Approved.
Code Review: Redact bottube mood errorsStandard review — error message redaction for BoTTube mood module. |
508704820
left a comment
508704820
left a comment
There was a problem hiding this comment.
Redact bottube mood errors. SECURITY: Error messages can leak internal service names and endpoints. Good hardening. — Xeophon (security review)
Code Review — Bounty #73PR: fix: redact bottube mood errors by @yangjl114
Wallet: Reviewing under Bounty #73 |
|
Security Review ✅ Same class of fix as #5545: redacting internal error details from HTTP responses. Previous code returned str(e) which could leak database DSNs, internal URLs, file paths, and auth tokens to clients. Fix correctly replaces with generic error message and logs details server-side. Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ |
1 similar comment
|
Security Review ✅ Same class of fix as #5545: redacting internal error details from HTTP responses. Previous code returned str(e) which could leak database DSNs, internal URLs, file paths, and auth tokens to clients. Fix correctly replaces with generic error message and logs details server-side. Reviewed by Xeophon - Solana: Lt9nERv6VHsojw15LpFeiaabuphAggzfLF9sM9UXRrZ |
Code ReviewPR: fix: redact bottube mood errors by @yangjl114
Wallet: Reviewing under Bounty #73 |
kekehanshujun
left a comment
kekehanshujun
left a comment
There was a problem hiding this comment.
Reviewing this as part of the RustChain code review bounty.
The patch removes the repeated return jsonify({error: str(e)}), 500 pattern from all six BoTTube mood routes named in #5445 and centralizes the replacement in _mood_service_error_response(). That keeps the client response stable while preserving operator diagnostics through current_app.logger.exception(...).
The regression test is useful because it drives every affected route under a mocked engine failure and asserts that table names, private paths, and token-like text are not present in rendered responses. I also checked that the malformed-body validation helpers remain outside this broad exception path, so existing 400 behavior should not be swallowed.
No blocking issues found. A later cleanup could add an assertion on the exact status/body for one normal success path after the refactor, but the current test scope matches the vulnerability.
jaxint
left a comment
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Great work on this PR. 🚀
|
Closing as duplicate of #5446 by @hungle123-dev (first-posted 16h earlier on the same issue). Per first-poster rule, only the original submitter gets bounty credit on a given issue. The fix in #5446 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. |
10 participants
Summary
Mood service unavailableresponse for BoTTube mood engine failuresstr(e)to clientsCloses #5445
Validation
uv run --no-project --with pytest --with flask python -m pytest -q tests\test_bottube_mood.py-> 31 passed, 14 subtests passedpy -3 -m py_compile bottube_mood_engine.py tests\test_bottube_mood.py-> passedgit diff --check -- bottube_mood_engine.py tests\test_bottube_mood.py-> passedBounty / payout
RTC wallet: RTCc68dae2dab2117db937bce7508472c8a7d11ac8b