fix: hide bottube mood exception details#5446
Conversation
kekehanshujun
left a comment
There was a problem hiding this comment.
Approved. I reviewed the BoTTube mood-route exception redaction fix and did not find a blocker.
What I checked:
- All six affected mood routes now route unexpected internal failures through
_mood_service_unavailable(...)instead of returningstr(e)in the JSON response. - The handler still logs the original exception with
logger.exception(...)for server-side debugging. - The new regression test covers the GET and POST routes and injects exception text containing a database table name, private path, and token-like substring; each response returns only the generic
Mood service unavailableerror. - Existing 400 client-validation paths are not touched by the broad exception redaction change.
Validation on head 81b1df35251b423c5eb4f01c608b842ebdc8dd3a:
python -m pytest tests\test_bottube_mood.py::TestMoodBlueprintJsonValidation::test_mood_routes_hide_internal_exception_details -q --confcutdir=.-> 1 passed, 6 subtests passedpython -m pytest tests\test_bottube_mood.py -q --confcutdir=.-> 31 passed, 14 subtests passedpython -m py_compile bottube_mood_engine.py tests\test_bottube_mood.py-> passed
No blocking issue found in this scope.
HCIE2054
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing!
ZacharyZhang-NY
left a comment
There was a problem hiding this comment.
Reviewed PR #5446 at head 81b1df3.
Validation performed:
- git fetch origin pull/5446/head:review-pr-5446 --force
- git diff --check origin/main...review-pr-5446 -- bottube_mood_engine.py tests/test_bottube_mood.py -> passed.
- python -m py_compile bottube_mood_engine.py tests/test_bottube_mood.py on an extracted PR-head tree -> passed.
- python -m pytest tests/test_bottube_mood.py::TestMoodBlueprintJsonValidation::test_mood_routes_hide_internal_exception_details -q --confcutdir= -> 1 passed.
- python -m pytest tests/test_bottube_mood.py -q --confcutdir= -> 31 passed.
- Checked issue #5445, which lists six BoTTube mood routes that reflected raw internal exception text.
- Inspected the route handlers and regression: all six unexpected failure paths now call the shared generic 500 helper while malformed client JSON keeps the existing 400 validation paths.
The six issue-scoped mood routes now return Mood service unavailable on internal failures and keep raw database/path/token-like exception details out of client JSON. The full exception is still logged server-side via logger.exception(...). Approving.
ZacharyZhang-NY
left a comment
There was a problem hiding this comment.
Reviewed PR #5446 at head 81b1df3.
Validation performed:
- Checked issue #5445. The scoped bug is BoTTube mood routes returning raw internal mood-engine exception text to HTTP clients.
- git fetch origin pull/5446/head:review-pr-5446 --force
- git diff --check origin/main...review-pr-5446 -- bottube_mood_engine.py tests/test_bottube_mood.py -> passed.
- python -m py_compile bottube_mood_engine.py tests/test_bottube_mood.py on an extracted PR-head tree -> passed.
- python -m pytest tests/test_bottube_mood.py::TestMoodBlueprintJsonValidation::test_mood_routes_hide_internal_exception_details -q --confcutdir= -> 1 passed.
- python -m pytest tests/test_bottube_mood.py -q --confcutdir= -> 31 passed.
- rg for
return jsonify({"error": str(e)}), 500in bottube_mood_engine.py -> no matches.
The six mood API routes now return a generic Mood service unavailable response while logging full exception details server-side. Approving.
jaxint
left a comment
There was a problem hiding this comment.
LGTM! Thanks for contributing. Approved.
TJCurnutte
left a comment
There was a problem hiding this comment.
Reviewed the diff and validated locally at PR head 81b1df35251b.
I approve this. The change closes the user-visible exception leak in the BoTTube mood routes without removing server-side debuggability: the affected 500 responses now return the generic Mood service unavailable message, while full exception details still go through logging. The regression coverage is useful because it forces SQL/table/path/token-like exception text through the affected routes and checks that clients do not receive it.
Validation I ran:
git diff --check origin/main...HEAD -- bottube_mood_engine.py tests/test_bottube_mood.py
python3 -B -m py_compile bottube_mood_engine.py tests/test_bottube_mood.py
uv run --no-project --with pytest --with flask python -B -m pytest -q tests/test_bottube_mood.py --noconftestResult: 31 passed, 14 subtests passed in 0.53s.
I also ran a source probe confirming the generic Mood service unavailable response is present, the regression test includes sensitive tokens such as agent_mood_history, /srv/rustchain/private, and super-secret, and the old raw str(e) JSON 500 response pattern is no longer present. No blockers from me.
Code Review — Bounty #73PR: fix: hide bottube mood 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 |
Fixes #5445.
Summary
logger.exception(...)for debuggability.Root cause
The route handlers caught broad exceptions and returned raw
str(e)to clients. If the mood engine raised a database, filesystem, or integration error, callers received the raw exception text, including table names, paths, or secret-like substrings.Behavior after fix
The affected routes now return a generic
Mood service unavailableHTTP 500 for internal failures, while malformed/non-object/missing client JSON still uses the existing 400 responses.Validation
Red before fix:
python -m pytest tests/test_bottube_mood.py::TestMoodBlueprintJsonValidation::test_mood_routes_hide_internal_exception_details -q-> failed on all six subtests because the response leakedagent_mood_history,/srv/rustchain/private, andsuper-secret.Green after fix:
python -m pytest tests/test_bottube_mood.py::TestMoodBlueprintJsonValidation::test_mood_routes_hide_internal_exception_details -q-> 1 passed, 6 subtests passedpython -m pytest tests/test_bottube_mood.py -q-> 31 passed, 14 subtests passedpython -m py_compile bottube_mood_engine.py tests\test_bottube_mood.py-> passedgit diff --check -- bottube_mood_engine.py tests/test_bottube_mood.py-> passedrg -n 'return jsonify\(\{"error": str\(e\)\}\), 500' bottube_mood_engine.py-> no matches