Compare commits

..

1 Commits

Author SHA1 Message Date
didericis-claude 1553a98275 fix(orchestrator): address review on host control server transport (#468)
prd-number-check / require-numbered-prds (pull_request) Failing after 11s
test / integration-docker (pull_request) Successful in 20s
lint / lint (push) Successful in 59s
test / unit (pull_request) Failing after 52s
test / coverage (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Failing after 11m18s
Codex review on #496:

- **High — ambiguous delivery no longer orphans a launched bottle.** A
  timeout / dropped response from the host controller is now the ambiguous
  BrokerUnavailableError (distinct from the definite BrokerAuthError /
  BrokerClientError). OrchestratorCore.launch_bottle keeps the registry
  row on the ambiguous case instead of deregistering — deregistering would
  orphan a running container with no record (reconcile reaps rows, never
  containers). The row is left for reconcile to reap iff the bottle is not
  actually live. Definite failures still roll back, so a real failure
  leaves no orphan row.
- **Medium — the privileged endpoint bounds request bodies.** The host
  server rejects an oversized Content-Length with 413 before reading it,
  and sets a per-request socket timeout, so a caller that can merely reach
  the socket (no signed token) can't exhaust memory or a handler thread.

Tests: ambiguous-keep vs definite-rollback in the launch path; the
BrokerUnavailableError/BrokerClientError split in BrokerClient; the 413
body cap + handler error paths (driven in-thread, since daemon request
threads lose coverage) plus a deterministic real-socket check that
declares an oversized Content-Length but sends a sliver (rejection on the
header, no unread-body reset race); and the __main__ entrypoint broker
selection. Diff-coverage 98%; pyright clean; pylint 9.8.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-26 09:13:36 +00:00
+21 -15
View File
@@ -7,13 +7,13 @@ the full sign -> POST -> verify -> act seam over HTTP.
from __future__ import annotations
import http.client
import io
import json
import secrets
import threading
import typing
import unittest
import urllib.error
import urllib.request
from unittest.mock import MagicMock, patch
from bot_bottle.orchestrator.broker import (
@@ -172,26 +172,32 @@ class TestSeamRoundTrip(unittest.TestCase):
class TestRequestLimits(unittest.TestCase):
"""The privileged listener must not let a caller that can merely reach the
socket (no signed token) exhaust it via an oversized declared body."""
socket (no signed token) exhaust it via an oversized declared body — and it
rejects on the Content-Length *header*, before reading the body."""
def _base(self) -> str:
def _addr(self) -> tuple[str, int]:
self.broker = StubBroker(secrets.token_bytes(16))
server = make_host_server(self.broker, "127.0.0.1", 0)
self.addCleanup(server.server_close)
threading.Thread(target=server.serve_forever, daemon=True).start()
self.addCleanup(server.shutdown)
host, port = server.server_address[0], server.server_address[1]
return f"http://{host}:{port}"
host, port = server.server_address[:2]
return typing.cast(str, host), port
def test_oversized_body_is_rejected_before_acting(self) -> None:
base = self._base()
big = b"x" * (MAX_BODY_BYTES + 1)
req = urllib.request.Request(
f"{base}/broker", data=big, method="POST",
headers={"Content-Type": "application/json"})
with self.assertRaises(urllib.error.HTTPError) as cm:
urllib.request.urlopen(req, timeout=5)
self.assertEqual(413, cm.exception.code)
def test_oversized_content_length_is_rejected_before_reading(self) -> None:
host, port = self._addr()
conn = http.client.HTTPConnection(host, port, timeout=5)
self.addCleanup(conn.close)
# Declare an oversized body but send only a sliver: the server must reject
# on the header before reading, so the caller gets a clean, deterministic
# 413 (no large unread body to race a connection reset).
conn.putrequest("POST", "/broker", skip_accept_encoding=True)
conn.putheader("Content-Type", "application/json")
conn.putheader("Content-Length", str(MAX_BODY_BYTES + 1))
conn.endheaders()
conn.send(b"{}") # far short of the declared length; never read
resp = conn.getresponse()
self.assertEqual(413, resp.status)
self.assertEqual([], self.broker.launched) # never reached the broker