fix(security): stop leaking Python tracebacks to HTTP clients#283
Merged
ChuckBuilds merged 3 commits intoChuckBuilds:mainfrom Mar 20, 2026
Merged
fix(security): stop leaking Python tracebacks to HTTP clients#283ChuckBuilds merged 3 commits intoChuckBuilds:mainfrom
ChuckBuilds merged 3 commits intoChuckBuilds:mainfrom
Conversation
Replace 13 instances where traceback.format_exc() was sent in API
JSON responses (via `details=`, `traceback:`, or `details:` keys).
- 5 error_response(details=traceback.format_exc()) → generic message
- 6 jsonify({'traceback': traceback.format_exc()}) → removed key
- 2 jsonify({'details': error_details}) → logger.error() instead
Tracebacks in debug mode (app.py error handlers) are preserved as
they are guarded by app.debug and expected during development.
Co-Authored-By: 5ymb01 <noreply@github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaced per-endpoint traceback exposure with generic "Internal server error" responses and converted print/traceback diagnostics to logger-based logging across API v3 endpoints, preserving HTTP status codes while removing stack traces from error payloads. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web_interface/blueprints/api_v3.py`:
- Around line 413-417: The response is leaking raw exception text to clients via
the error_response call (e.g., the call using ErrorCode.CONFIG_SAVE_FAILED and
f"Error saving schedule configuration: {str(e)}"); instead, remove str(e) from
the client-facing message and return a generic message (e.g., "Error saving
schedule configuration") or a user-safe error id, while logging the full
exception server-side with traceback; update all similar sites that use
str(e)/exception text in error_response (the occurrences you noted) to use the
generic client message and ensure the exception is passed to the server logger
rather than the response.
- Around line 6066-6068: The except block that currently returns a sanitized 500
(the except Exception as e handling that returns
jsonify({'status':'error','message':str(e)}), 500) must log the full exception
before returning: use the existing structured logger (e.g., processLogger or app
logger) and call its exception()/error() with the traceback and a context tag
like "[NHL Recent]" so remote debugging is possible; capture the traceback or
use logger.exception("... [NHL Recent]") to include stack trace, then continue
returning the sanitized JSON 500 response.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f5db7edf-b332-47fd-ac09-dbd55eb0ff7c
📒 Files selected for processing (1)
web_interface/blueprints/api_v3.py
… logging Address CodeRabbit review findings: - Replace str(e) in error_response message fields with generic messages - Replace import logging/traceback + manual format with logger.exception() - Add logger.exception() to 6 jsonify handlers that were swallowing errors - All exception details now logged server-side only, not sent to clients Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web_interface/blueprints/api_v3.py (2)
1015-1037: Avoid duplicate traceback logging insave_raw_main_configerror path.This block now uses
logger.exception(...), but it still manually builds/logs traceback text earlier in the sameexcept, causing duplicate stack traces and mixed logging style.♻️ Proposed cleanup
- except Exception as e: - import logging - import traceback - from src.exceptions import ConfigError - - # Log the full error for debugging - error_msg = f"Error saving raw main config: {str(e)}\n{traceback.format_exc()}" - logging.error(error_msg) - - # Extract more specific error message if it's a ConfigError - logger.exception("[RawConfig] Failed to save raw main config") + except Exception as e: + from src.exceptions import ConfigError + logger.exception("[RawConfig] Failed to save raw main config")As per coding guidelines "Use structured logging with context (e.g.,
[NHL Recent]) for logging messages".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web_interface/blueprints/api_v3.py` around lines 1015 - 1037, In save_raw_main_config remove the manual logging.error/traceback.format_exc() block to avoid duplicate stack traces and rely on the existing logger.exception call for structured logging; replace the logger.exception call's message with a contextual tag (e.g., "[RawConfig] Failed to save raw main config") if it isn't already, ensure you only use logger.exception (not logging.error) in the except block, and keep the existing ConfigError check and error_response logic unchanged.
6054-6055: Use endpoint-specific log context instead of a shared[API] Unexpected errortag.Using the same tag across multiple handlers makes production triage harder. Consider distinct tags per endpoint (e.g.,
[PluginAssetsUpload],[OfTheDayUpload], etc.).🧭 Example pattern
- logger.exception("[API] Unexpected error") + logger.exception("[PluginAssetsUpload] Unexpected error")As per coding guidelines "Use structured logging with context (e.g.,
[NHL Recent]) for logging messages" and "Implement comprehensive logging for remote debugging on Raspberry Pi".Also applies to: 6204-6205, 6251-6252, 6297-6298, 6469-6470, 6497-6498
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web_interface/blueprints/api_v3.py` around lines 6054 - 6055, Replace the generic logger.exception("[API] Unexpected error") message in each route handler with an endpoint-specific structured context: e.g., logger.exception("[PluginAssetsUpload] Unexpected error processing %s %s", request.method, request.path) or logger.exception("[OfTheDayUpload] ...", ...) so logs include the unique tag (PluginAssetsUpload, OfTheDayUpload, etc.), request.method/path or handler name, and the exception (logger.exception preserves traceback). Apply this change for the shown occurrence and the other similar spots (the other logger.exception calls referenced) so each endpoint uses a distinct tag and includes request context for easier triage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web_interface/blueprints/api_v3.py`:
- Around line 1015-1037: In save_raw_main_config remove the manual
logging.error/traceback.format_exc() block to avoid duplicate stack traces and
rely on the existing logger.exception call for structured logging; replace the
logger.exception call's message with a contextual tag (e.g., "[RawConfig] Failed
to save raw main config") if it isn't already, ensure you only use
logger.exception (not logging.error) in the except block, and keep the existing
ConfigError check and error_response logic unchanged.
- Around line 6054-6055: Replace the generic logger.exception("[API] Unexpected
error") message in each route handler with an endpoint-specific structured
context: e.g., logger.exception("[PluginAssetsUpload] Unexpected error
processing %s %s", request.method, request.path) or
logger.exception("[OfTheDayUpload] ...", ...) so logs include the unique tag
(PluginAssetsUpload, OfTheDayUpload, etc.), request.method/path or handler name,
and the exception (logger.exception preserves traceback). Apply this change for
the shown occurrence and the other similar spots (the other logger.exception
calls referenced) so each endpoint uses a distinct tag and includes request
context for easier triage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34c0d4b0-2626-4d72-a762-5baf08dc1ff9
📒 Files selected for processing (1)
web_interface/blueprints/api_v3.py
Address CodeRabbit nitpicks: - Remove manual import logging/traceback + logging.error() that duplicated the logger.exception() call in save_raw_main_config - Apply same fix to save_raw_secrets_config: replace str(e) in client response with generic message, use logger.exception() for server-side Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
traceback.format_exc()was sent in API JSON responses to HTTP clientserror_response(details=traceback.format_exc())→ generic "Internal server error - check server logs"jsonify({'traceback': traceback.format_exc()})→ removedtracebackkey from responsejsonify({'details': error_details})→ replaced withlogger.error()server-side loggingapp.pyerror handlers is preserved (guarded byapp.debug)Rationale
Leaking Python tracebacks to HTTP clients exposes internal file paths, library versions, and code structure. This information can aid attackers in crafting targeted exploits. Server-side logging via
logger.error()preserves debugging capability without client exposure.Test plan
grep -n "details=traceback\|'traceback': traceback\|'details': error_details" web_interface/blueprints/api_v3.py— zero matchesmessagebut notracebackordetailswith stack framesCo-Authored-By: 5ymb01 noreply@github.com
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit
Bug Fixes
Chores