Skip to content

fix(http): stop leaking tracebacks in JSONL/RPC error responses #361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RealDiligent wants to merge 1 commit into
base: main
Choose a base branch
from
+18 −2
Open

fix(http): stop leaking tracebacks in JSONL/RPC error responses #361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/vouch/jsonl_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
from __future__ import annotations

import json
import logging
import os
import sys
import traceback
from collections.abc import Callable
from contextvars import ContextVar
from pathlib import Path
Expand All @@ -37,6 +37,8 @@
from .capabilities import capabilities as build_caps
from .context import build_context_pack
from .logging_config import configure_logging

_log = logging.getLogger("vouch.jsonl_server")
from .models import ProposalStatus
from .proposals import (
EXPIRE_ACTOR,
Expand Down Expand Up @@ -761,12 +763,12 @@ def handle_request(envelope: dict) -> dict:
"error": {"code": "invalid_request", "message": str(e)},
}
except Exception as e:
_log.exception("internal error handling %s", method)
return {
"id": req_id, "ok": False,
"error": {
"code": "internal_error",
"message": str(e),
"traceback": traceback.format_exc(),
},
}
Comment on lines 765 to 773

@coderabbitai coderabbitai Bot Jul 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Exception message can still leak sensitive internal details.

Traceback is now correctly kept server-side, but "message": str(e) still forwards the raw exception text to the client over HTTP /rpc. Exception messages frequently embed sensitive internals (file paths, connection strings, IDs, or — as the new test itself demonstrates with RuntimeError("secret internals") — arbitrary secrets a handler author put in an error message). This only partially closes the information-disclosure gap described in the PR objectives.

Consider returning a generic, static message for the internal_error case (logging str(e) server-side alongside the traceback) so unexpected-exception responses never echo handler-controlled text to clients.

🔒 Proposed fix 
     except Exception as e:
-        _log.exception("internal error handling %s", method)
+        _log.exception("internal error handling %s: %s", method, e)
         return {
             "id": req_id, "ok": False,
             "error": {
                 "code": "internal_error",
-                "message": str(e),
+                "message": "internal server error",
             },
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
except Exception as e:
_log.exception("internal error handling %s", method)
return {
"id": req_id, "ok": False,
"error": {
"code": "internal_error",
"message": str(e),
"traceback": traceback.format_exc(),
},
}
except Exception as e:
_log.exception("internal error handling %s: %s", method, e)
return {
"id": req_id, "ok": False,
"error": {
"code": "internal_error",
"message": "internal server error",
},
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vouch/jsonl_server.py` around lines 765 - 773, The `except Exception as
e` handler in `jsonl_server.py` still returns handler-controlled text via
`message: str(e)`, which can leak sensitive internals to `/rpc` clients. Update
the `internal_error` response to use a generic, static message instead of the
raw exception text, and keep the detailed exception only in
`_log.exception("internal error handling %s", method)` so `req_id`/`method` stay
contextual while `str(e)` is not exposed over HTTP.


Expand Down
14 changes: 14 additions & 0 deletions tests/test_jsonl_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,17 @@ def test_jsonl_self_approval_allowed_with_trusted_agent_config(
resp = handle_request({"id": "2", "method": "kb.approve",
"params": {"proposal_id": pid}})
assert resp["ok"]


def test_jsonl_internal_error_omits_traceback(monkeypatch) -> None:
"""HTTP /rpc clients must not receive server tracebacks on unexpected errors."""
from vouch import jsonl_server

def _boom(_params: dict) -> dict:
raise RuntimeError("secret internals")

monkeypatch.setitem(jsonl_server.HANDLERS, "kb.status", _boom)
resp = handle_request({"id": "x", "method": "kb.status", "params": {}})
assert resp["ok"] is False
assert resp["error"]["code"] == "internal_error"
assert "traceback" not in resp["error"]