Skip to content

fix: sanitize internal error details in bot proxy responses#1310

Merged
qin-ctx merged 1 commit intovolcengine:mainfrom
sjhddh:fix/sanitize-bot-proxy-errors
Apr 9, 2026
Merged

fix: sanitize internal error details in bot proxy responses#1310
qin-ctx merged 1 commit intovolcengine:mainfrom
sjhddh:fix/sanitize-bot-proxy-errors

Conversation

@sjhddh
Copy link
Copy Markdown
Contributor

@sjhddh sjhddh commented Apr 8, 2026

Summary

The bot proxy router leaked internal infrastructure details to API clients via error response messages:

  • httpx.RequestError paths (lines 65, 131, 196): formatted the exception as f"Bot service unavailable: {str(e)}", which exposes internal hostnames, IP addresses, port numbers, and connection error specifics from httpx.
  • httpx.HTTPStatusError paths (lines 71, 143, 203): forwarded the raw upstream response body as f"Bot service error: {e.response.text}", which may contain internal service error details, stack traces, or debug output.

This is an information disclosure vulnerability — clients can fingerprint internal network topology and service behavior from error responses alone.

Fix

Replace all six dynamic error detail strings with static generic messages:

  • "Bot service unavailable" for connection/transport errors
  • "Bot service error" for upstream HTTP error responses

No observability is lost: logger.error(...) calls immediately preceding each raise HTTPException already capture the full error context server-side.

Also removes a stray print() debug statement in health_check that was present in the upstream branch.

Related

Related to #1233 (same class of information disclosure via error responses).

Testing

  • On httpx.RequestError: verify response body is {"detail": "Bot service unavailable"} with no hostname/IP/port leakage.
  • On upstream 5xx: verify response body is {"detail": "Bot service error"} with no upstream response body forwarded.
  • On upstream 4xx: client error passthrough (lines 135–138) is unchanged — intentional, as those are client-facing validation errors.
  • SSE stream: verify error events contain {"error": "Bot service unavailable"} / {"error": "Bot service error"} with no internal details.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 8, 2026

CLA assistant check
All committers have signed the CLA.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sjhddh sjhddh force-pushed the fix/sanitize-bot-proxy-errors branch from 0cbf0f7 to 060b89a Compare April 8, 2026 17:06
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

1233 - Fully compliant

Compliant requirements:

  • Fixed global exception handler to return generic "Internal server error" message
  • Changed logging from logger.warning to logger.exception to capture full traceback server-side
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 92
🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Fix global 500 handler internal detail leak

Relevant files:

  • openviking/server/app.py

Sub-PR theme: Sanitize bot proxy error responses + remove debug print

Relevant files:

  • openviking/server/routers/bot.py

⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

PR Code Suggestions ✨

No code suggestions found for the PR.

@qin-ctx qin-ctx merged commit f68f5c7 into volcengine:main Apr 9, 2026
2 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants