Compare commits

..

3 Commits

Author SHA1 Message Date
didericis-codex 731826f966 feat: add ripgrep to agent images 2026-06-25 08:23:37 +00:00
didericis-claude d99dba037c feat(supervise): typed RPC error taxonomy for dispatch
test / unit (pull_request) Successful in 31s
test / integration (pull_request) Successful in 17s
lint / lint (push) Successful in 1m48s
test / unit (push) Successful in 32s
test / integration (push) Successful in 18s
Update Quality Badges / update-badges (push) Successful in 1m20s
Introduce _RpcClientError and _RpcInternalError as distinct subclasses
of _RpcError so the dispatcher can handle bad requests and server-side
faults differently — returning client errors verbatim and logging
internal faults with their cause before replying ERR_INTERNAL.

Wrap write_proposal and archive_proposal IO with _RpcInternalError
so OS failures surface through the typed path instead of the bare
Exception fallback. All existing raise _RpcError(...) call sites
converted to _RpcClientError.

Closes #253
2026-06-25 04:02:39 -04:00
didericis-claude 9a878bd885 fix: guard CGI Status-line parse in _write_cgi_response
test / unit (pull_request) Successful in 32s
test / integration (pull_request) Successful in 16s
lint / lint (push) Successful in 1m47s
test / unit (push) Successful in 32s
test / integration (push) Successful in 16s
Update Quality Badges / update-badges (push) Successful in 1m19s
An empty or non-numeric Status: header from git http-backend raised
ValueError/IndexError that escaped the handler thread. Wrap the parse
in a try/except and fall back to HTTP 500 instead.

Closes #254
2026-06-25 03:47:05 -04:00
6 changed files with 188 additions and 24 deletions
+1 -1
View File
@@ -21,7 +21,7 @@ FROM node:22-slim
# to it) works against egress's bumped TLS without the agent needing
# local DNS.
RUN apt-get update \
&& apt-get install -y --no-install-recommends git ca-certificates curl \
&& apt-get install -y --no-install-recommends git ca-certificates curl ripgrep \
&& rm -rf /var/lib/apt/lists/*
# App-specific deps. Python isn't required by claude-code itself
+1 -1
View File
@@ -6,7 +6,7 @@
FROM node:22-slim
RUN apt-get update \
&& apt-get install -y --no-install-recommends git ca-certificates curl procps \
&& apt-get install -y --no-install-recommends git ca-certificates curl procps ripgrep \
&& rm -rf /var/lib/apt/lists/*
# App-specific deps. Python isn't required by codex itself
+7 -1
View File
@@ -148,7 +148,13 @@ class GitHttpHandler(BaseHTTPRequestHandler):
key, _, value = line.decode("latin1").partition(":")
value = value.strip()
if key.lower() == "status":
status = int(value.split()[0])
try:
status = int(value.split()[0])
except (ValueError, IndexError):
self.log_message(
"malformed CGI Status header %r; using 500", value,
)
status = 500
else:
headers.append((key, value))
self.send_response(status)
+46 -21
View File
@@ -90,19 +90,19 @@ def parse_jsonrpc(body: bytes) -> JsonRpcRequest:
try:
raw = json.loads(body)
except json.JSONDecodeError as e:
raise _RpcError(ERR_PARSE, f"parse error: {e}") from e
raise _RpcClientError(ERR_PARSE, f"parse error: {e}") from e
if not isinstance(raw, dict):
raise _RpcError(ERR_INVALID_REQUEST, "request must be a JSON object")
raise _RpcClientError(ERR_INVALID_REQUEST, "request must be a JSON object")
if raw.get("jsonrpc") != JSONRPC_VERSION:
raise _RpcError(ERR_INVALID_REQUEST, "jsonrpc field must be '2.0'")
raise _RpcClientError(ERR_INVALID_REQUEST, "jsonrpc field must be '2.0'")
method = raw.get("method")
if not isinstance(method, str):
raise _RpcError(ERR_INVALID_REQUEST, "method must be a string")
raise _RpcClientError(ERR_INVALID_REQUEST, "method must be a string")
params = raw.get("params", {})
if params is None:
params = {}
if not isinstance(params, dict):
raise _RpcError(ERR_INVALID_PARAMS, "params must be an object")
raise _RpcClientError(ERR_INVALID_PARAMS, "params must be an object")
rpc_id = raw.get("id", _NO_ID)
is_notification = rpc_id is _NO_ID
return JsonRpcRequest(
@@ -117,12 +117,23 @@ _NO_ID = object()
class _RpcError(Exception):
"""Base class for all typed RPC errors that surface as JSON-RPC error responses."""
def __init__(self, code: int, message: str):
super().__init__(message)
self.code = code
self.message = message
class _RpcClientError(_RpcError):
"""Caller sent a bad request; returned verbatim, no server-side logging."""
class _RpcInternalError(_RpcError):
"""Server-side fault; logged at ERROR with cause, always returns ERR_INTERNAL."""
def __init__(self, message: str) -> None:
super().__init__(ERR_INTERNAL, message)
def jsonrpc_result(request_id: object, result: object) -> bytes:
payload = {"jsonrpc": JSONRPC_VERSION, "id": request_id, "result": result}
return (json.dumps(payload) + "\n").encode("utf-8")
@@ -290,7 +301,7 @@ def validate_proposed_file(tool: str, content: str) -> None:
catches obvious paste-errors / wrong-tool selections before they
enter the queue."""
if not content.strip():
raise _RpcError(ERR_INVALID_PARAMS, f"{tool}: proposed file is empty")
raise _RpcClientError(ERR_INVALID_PARAMS, f"{tool}: proposed file is empty")
if tool == _sv.TOOL_CAPABILITY_BLOCK:
# Dockerfiles are too varied to validate syntactically beyond
# non-empty. The operator reads the diff in the TUI.
@@ -299,17 +310,17 @@ def validate_proposed_file(tool: str, content: str) -> None:
try:
config = load_config(content)
except ValueError as e:
raise _RpcError(
raise _RpcClientError(
ERR_INVALID_PARAMS,
f"{tool}: proposed routes.yaml is not valid: {e}",
) from e
if config.log != LOG_OFF:
raise _RpcError(
raise _RpcClientError(
ERR_INVALID_PARAMS,
f"{tool}: proposed routes.yaml must not change egress logging",
)
else:
raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}")
raise _RpcClientError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}")
# --- MCP handlers ----------------------------------------------------------
@@ -382,17 +393,17 @@ def handle_tools_call(
doesn't need operator approval."""
name = params.get("name")
if not isinstance(name, str):
raise _RpcError(ERR_INVALID_PARAMS, "tools/call missing 'name'")
raise _RpcClientError(ERR_INVALID_PARAMS, "tools/call missing 'name'")
if name == _sv.TOOL_LIST_EGRESS_ROUTES:
return handle_list_egress_routes(typing.cast(dict[str, object], params.get("arguments", {})), config)
args_raw = params.get("arguments", {})
if not isinstance(args_raw, dict):
raise _RpcError(ERR_INVALID_PARAMS, "tools/call 'arguments' must be an object")
raise _RpcClientError(ERR_INVALID_PARAMS, "tools/call 'arguments' must be an object")
justification = args_raw.get("justification")
if not isinstance(justification, str) or not justification.strip():
raise _RpcError(
raise _RpcClientError(
ERR_INVALID_PARAMS,
f"{name}: 'justification' is required and must be a non-empty string",
)
@@ -401,13 +412,13 @@ def handle_tools_call(
file_field = PROPOSED_FILE_FIELD[name]
proposed_file = args_raw.get(file_field)
if not isinstance(proposed_file, str):
raise _RpcError(
raise _RpcClientError(
ERR_INVALID_PARAMS,
f"{name}: '{file_field}' is required and must be a string",
)
validate_proposed_file(name, proposed_file)
else:
raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {name!r}")
raise _RpcClientError(ERR_INVALID_PARAMS, f"unknown tool {name!r}")
proposal = _sv.Proposal.new(
bottle_slug=config.bottle_slug,
@@ -416,7 +427,10 @@ def handle_tools_call(
justification=justification,
current_file_hash=_sv.sha256_hex(proposed_file),
)
_sv.write_proposal(config.queue_dir, proposal)
try:
_sv.write_proposal(config.queue_dir, proposal)
except OSError as e:
raise _RpcInternalError(f"failed to write proposal to queue: {e}") from e
sys.stderr.write(
f"supervise: queued proposal {proposal.id} ({name}) "
f"for bottle {config.bottle_slug}; waiting for operator...\n"
@@ -436,7 +450,10 @@ def handle_tools_call(
"content": [{"type": "text", "text": text}],
"isError": False,
}
_sv.archive_proposal(config.queue_dir, proposal.id)
try:
_sv.archive_proposal(config.queue_dir, proposal.id)
except OSError as e:
raise _RpcInternalError(f"failed to archive proposal: {e}") from e
text = format_response_text(response)
return {
@@ -512,7 +529,7 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
try:
req = parse_jsonrpc(body)
except _RpcError as e:
except _RpcClientError as e:
self._write_jsonrpc(jsonrpc_error(None, e.code, e.message))
return
@@ -520,11 +537,19 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
try:
result = self._dispatch(req, config)
except _RpcError as e:
except _RpcClientError as e:
self._write_jsonrpc(jsonrpc_error(req.id, e.code, e.message))
return
except Exception as e: # noqa: W0718 — catch-all for RPC dispatch errors
sys.stderr.write(f"supervise: internal error: {e}\n")
except _RpcInternalError as e:
cause = e.__cause__
detail = f": {cause}" if cause else ""
sys.stderr.write(f"supervise: internal error: {e.message}{detail}\n")
sys.stderr.flush()
self._write_jsonrpc(jsonrpc_error(req.id, ERR_INTERNAL, "internal error"))
return
except Exception as e: # noqa: W0718 — unexpected errors
sys.stderr.write(f"supervise: unexpected error: {type(e).__name__}: {e}\n")
sys.stderr.flush()
self._write_jsonrpc(jsonrpc_error(req.id, ERR_INTERNAL, "internal error"))
return
@@ -543,7 +568,7 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
return handle_tools_list(req.params)
if method == "tools/call":
return handle_tools_call(req.params, config)
raise _RpcError(ERR_METHOD_NOT_FOUND, f"method not found: {method}")
raise _RpcClientError(ERR_METHOD_NOT_FOUND, f"method not found: {method}")
def _write_jsonrpc(self, body: bytes) -> None:
self.send_response(200)
+51
View File
@@ -256,6 +256,57 @@ class TestGitHttpBackend(unittest.TestCase):
os.environ["GIT_GATE_ACCESS_HOOK"] = value
class TestMalformedStatusHeader(unittest.TestCase):
"""Malformed CGI Status: headers must not propagate as unhandled exceptions;
the handler should fall back to HTTP 500."""
def setUp(self):
from http.server import ThreadingHTTPServer
import tempfile
self._tmp = tempfile.mkdtemp()
os.environ["GIT_PROJECT_ROOT"] = self._tmp
self._server = ThreadingHTTPServer(("127.0.0.1", 0), GitHttpHandler)
self._thread = threading.Thread(
target=self._server.serve_forever, daemon=True,
)
self._thread.start()
self._port = self._server.server_port
def tearDown(self):
self._server.shutdown()
self._server.server_close()
os.environ.pop("GIT_PROJECT_ROOT", None)
import shutil
shutil.rmtree(self._tmp, ignore_errors=True)
def _get_with_backend_response(self, cgi_response: bytes) -> int:
with mock.patch(
"bot_bottle.git_http_backend.subprocess.run",
return_value=mock.Mock(returncode=0, stdout=cgi_response),
):
req = urllib.request.Request(
f"http://127.0.0.1:{self._port}/repo.git/info/refs",
method="GET",
)
try:
with urllib.request.urlopen(req, timeout=3) as resp:
return resp.status
except urllib.error.HTTPError as e: # type: ignore
return e.code
def test_empty_status_value_returns_500(self):
status = self._get_with_backend_response(
b"Status: \r\nContent-Type: text/plain\r\n\r\n"
)
self.assertEqual(500, status)
def test_non_numeric_status_returns_500(self):
status = self._get_with_backend_response(
b"Status: bad\r\nContent-Type: text/plain\r\n\r\n"
)
self.assertEqual(500, status)
class TestContentLengthBounds(unittest.TestCase):
"""PRD 0041: malformed or oversized Content-Length is rejected before
git http-backend is invoked."""
+82
View File
@@ -20,6 +20,7 @@ import supervise as _sv # noqa: E402 # type: ignore
from bot_bottle import supervise_server # noqa: E402
from bot_bottle.supervise_server import (
ERR_INTERNAL,
ERR_INVALID_PARAMS,
ERR_INVALID_REQUEST,
ERR_METHOD_NOT_FOUND,
@@ -29,7 +30,9 @@ from bot_bottle.supervise_server import (
PROPOSED_FILE_FIELD,
ServerConfig,
TOOL_DEFINITIONS,
_RpcClientError,
_RpcError,
_RpcInternalError,
_response_timeout_from_env,
format_response_text,
handle_initialize,
@@ -77,6 +80,65 @@ class TestValidation(unittest.TestCase):
self.assertIn("must not change egress logging", cm.exception.message)
# --- Error taxonomy --------------------------------------------------------
class TestRpcErrorTaxonomy(unittest.TestCase):
def test_rpc_client_error_is_rpc_error(self):
e = _RpcClientError(ERR_INVALID_PARAMS, "bad param")
self.assertIsInstance(e, _RpcError)
self.assertEqual(ERR_INVALID_PARAMS, e.code)
self.assertEqual("bad param", e.message)
def test_rpc_internal_error_is_rpc_error(self):
e = _RpcInternalError("disk full")
self.assertIsInstance(e, _RpcError)
self.assertEqual(ERR_INTERNAL, e.code)
self.assertEqual("disk full", e.message)
def test_rpc_internal_error_preserves_cause(self):
cause = OSError("no space left on device")
try:
raise _RpcInternalError("failed to write") from cause
except _RpcInternalError as e:
self.assertIs(cause, e.__cause__)
def test_parse_error_is_client_error(self):
with self.assertRaises(_RpcClientError):
parse_jsonrpc(b"{bad json")
def test_validation_error_is_client_error(self):
with self.assertRaises(_RpcClientError):
validate_proposed_file(_sv.TOOL_EGRESS_ALLOW, "routes: nope\n")
def test_unknown_tool_in_tools_call_is_client_error(self):
config = ServerConfig(bottle_slug="dev", queue_dir=Path("/unused"))
with self.assertRaises(_RpcClientError) as cm:
handle_tools_call({"name": "no-such-tool", "arguments": {}}, config)
self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code)
class TestRpcInternalErrorOnIoFailure(unittest.TestCase):
def test_write_proposal_os_error_raises_internal(self):
config = ServerConfig(
bottle_slug="dev",
queue_dir=Path("/dev/null/cannot-exist"),
)
with self.assertRaises(_RpcInternalError) as cm:
handle_tools_call(
{
"name": _sv.TOOL_CAPABILITY_BLOCK,
"arguments": {
"dockerfile": "FROM python:3.13\n",
"justification": "x",
},
},
config,
)
self.assertEqual(ERR_INTERNAL, cm.exception.code)
self.assertIsNotNone(cm.exception.__cause__)
# --- JSON-RPC parsing ------------------------------------------------------
@@ -469,6 +531,26 @@ class TestHttpEndToEnd(unittest.TestCase):
)
self.assertEqual(ERR_METHOD_NOT_FOUND, result["error"]["code"]) # type: ignore[index]
def test_internal_error_returns_err_internal_over_http(self):
with patch.object(
supervise_server._sv, "write_proposal",
side_effect=OSError("disk full"),
):
result = self._post_jsonrpc({
"jsonrpc": "2.0",
"id": 99,
"method": "tools/call",
"params": {
"name": _sv.TOOL_CAPABILITY_BLOCK,
"arguments": {
"dockerfile": "FROM python:3.13\n",
"justification": "x",
},
},
})
self.assertIn("error", result)
self.assertEqual(ERR_INTERNAL, result["error"]["code"]) # type: ignore[index]
def test_health_endpoint(self):
conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=5)
try: