Compare commits

..

7 Commits

Author SHA1 Message Date
didericis-claude 515a95a79d fix: escape quotes/newlines in YAML and gitconfig emitters
test / unit (pull_request) Successful in 35s
test / integration (pull_request) Successful in 17s
lint / lint (push) Successful in 1m48s
test / unit (push) Successful in 32s
test / integration (push) Successful in 16s
Update Quality Badges / update-badges (push) Successful in 1m18s
Closes #258.

`egress_render_routes` and `_render_match_entry` now pass all manifest
strings (host, auth_scheme, token_env, path/header values) through
`_yaml_str_escape` before interpolating into double-quoted YAML scalars,
preventing stray `"` or newlines from corrupting routes.yaml.

`git_gate_render_gitconfig` now calls `_gitconfig_validate_value` on
each Upstream value (and the derived alias) before writing the
`insteadOf` line, rejecting any value containing a newline that would
inject arbitrary gitconfig keys.
2026-06-25 04:23:13 -04:00
didericis-claude 0bace7615a refactor: rename GIT_GATE_DAEMON_TIMEOUT_SECS to GIT_GATE_TIMEOUT_SECS
test / unit (pull_request) Successful in 33s
test / integration (pull_request) Successful in 17s
lint / lint (push) Successful in 1m48s
test / unit (push) Successful in 35s
test / integration (push) Successful in 17s
Update Quality Badges / update-badges (push) Successful in 1m20s
The constant now covers the daemon path, the HTTP backend access-hook,
and the git http-backend CGI subprocess, so 'daemon' in the name was
too narrow. Updated the comment to list all three current uses.
2026-06-25 04:12:43 -04:00
didericis-claude c0d3f16519 refactor: import GIT_GATE_DAEMON_TIMEOUT_SECS instead of duplicating the value 2026-06-25 04:12:43 -04:00
didericis-claude 508c537deb fix: add explicit timeouts to subprocess and HTTP calls in git-gate paths
Closes #255. Without timeouts, a hung upstream during the access-hook
or git http-backend CGI call (git_http_backend.py) and a stalled Gitea
API during deploy-key provisioning (contrib/gitea/deploy_key_provisioner.py)
could wedge a sidecar indefinitely. Adds GIT_HTTP_BACKEND_TIMEOUT_SECS
(30s) to both subprocess.run calls in the HTTP backend, mirroring the
existing GIT_GATE_DAEMON_TIMEOUT_SECS on the daemon path. Adds
_API_TIMEOUT_SECS (30s) and _KEYGEN_TIMEOUT_SECS (10s) to the Gitea
provisioner's urlopen and ssh-keygen calls. Tests verify the timeout
values are forwarded in all four call sites.
2026-06-25 04:12:43 -04: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
didericis 0f72843150 fix(macos-container): anchor relative Dockerfile path to build context
test / unit (pull_request) Successful in 33s
test / integration (pull_request) Successful in 17s
lint / lint (push) Successful in 1m49s
test / unit (push) Successful in 33s
test / integration (push) Successful in 18s
Update Quality Badges / update-badges (push) Successful in 1m19s
`container build` resolves -f relative to the current working directory,
not the build context, so builds failed from any cwd other than the repo
root. Anchor a relative Dockerfile to the context before passing it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-25 03:27:46 -04:00
9 changed files with 323 additions and 30 deletions
@@ -68,6 +68,11 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None:
_ensure_builder_dns() _ensure_builder_dns()
args = [_CONTAINER, "build", "-t", ref, "--dns", dns_server()] args = [_CONTAINER, "build", "-t", ref, "--dns", dns_server()]
if dockerfile: if dockerfile:
# `container build` resolves -f relative to the current working
# directory, not the build context. Anchor a relative Dockerfile to
# the context so builds work from any cwd.
if not os.path.isabs(dockerfile):
dockerfile = os.path.join(context, dockerfile)
args.extend(["-f", dockerfile]) args.extend(["-f", dockerfile])
args.append(context) args.append(context)
subprocess.run(args, check=True) subprocess.run(args, check=True)
@@ -21,6 +21,11 @@ from pathlib import Path
from ...deploy_key_provisioner import DeployKeyCollisionError, DeployKeyProvisioner from ...deploy_key_provisioner import DeployKeyCollisionError, DeployKeyProvisioner
# Timeout for ssh-keygen and Gitea API HTTP calls. A hung Gitea instance at
# prepare time would stall bottle launch indefinitely without this bound.
_API_TIMEOUT_SECS = 30
_KEYGEN_TIMEOUT_SECS = 10
class GiteaDeployKeyProvisioner(DeployKeyProvisioner): class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
"""Manages deploy keys on a Gitea instance.""" """Manages deploy keys on a Gitea instance."""
@@ -46,6 +51,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
check=True, check=True,
stdout=subprocess.DEVNULL, stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
timeout=_KEYGEN_TIMEOUT_SECS,
) )
private_key = key_path.read_bytes() private_key = key_path.read_bytes()
public_key = key_path.with_suffix(".pub").read_text().strip() public_key = key_path.with_suffix(".pub").read_text().strip()
@@ -67,7 +73,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
method="POST", method="POST",
) )
try: try:
with urllib.request.urlopen(req) as resp: with urllib.request.urlopen(req, timeout=_API_TIMEOUT_SECS) as resp:
body = json.loads(resp.read()) body = json.loads(resp.read())
except urllib.error.HTTPError as exc: except urllib.error.HTTPError as exc:
_body = _read_error_body(exc) _body = _read_error_body(exc)
@@ -98,7 +104,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
method="DELETE", method="DELETE",
) )
try: try:
with urllib.request.urlopen(req): with urllib.request.urlopen(req, timeout=_API_TIMEOUT_SECS):
pass pass
except urllib.error.HTTPError as exc: except urllib.error.HTTPError as exc:
if exc.code == 404: if exc.code == 404:
+6 -6
View File
@@ -43,10 +43,10 @@ from .manifest import ManifestBottle, ManifestGitEntry
# Short network alias for git-gate inside the sidecar bundle. The # Short network alias for git-gate inside the sidecar bundle. The
# agent's `.gitconfig` insteadOf rewrites resolve through this name. # agent's `.gitconfig` insteadOf rewrites resolve through this name.
GIT_GATE_HOSTNAME = "git-gate" GIT_GATE_HOSTNAME = "git-gate"
# Bound half-open git client sessions. If an agent/tool runner is # Shared timeout (seconds) for all git-gate subprocess and CGI calls:
# interrupted during push, git daemon should reap the receive-pack # git daemon (--timeout/--init-timeout), the access-hook subprocess in
# child instead of keeping the gate wedged indefinitely. # git_http_backend, and the git http-backend CGI subprocess.
GIT_GATE_DAEMON_TIMEOUT_SECS = 15 GIT_GATE_TIMEOUT_SECS = 15
@dataclass(frozen=True) @dataclass(frozen=True)
@@ -228,8 +228,8 @@ def git_gate_render_entrypoint(upstreams: tuple[GitGateUpstream, ...]) -> str:
"", "",
"exec git daemon \\", "exec git daemon \\",
" --reuseaddr \\", " --reuseaddr \\",
f" --timeout={GIT_GATE_DAEMON_TIMEOUT_SECS} \\", f" --timeout={GIT_GATE_TIMEOUT_SECS} \\",
f" --init-timeout={GIT_GATE_DAEMON_TIMEOUT_SECS} \\", f" --init-timeout={GIT_GATE_TIMEOUT_SECS} \\",
" --base-path=/git \\", " --base-path=/git \\",
" --export-all \\", " --export-all \\",
" --enable=receive-pack \\", " --enable=receive-pack \\",
+11 -1
View File
@@ -16,6 +16,8 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
from pathlib import Path from pathlib import Path
from urllib.parse import urlsplit from urllib.parse import urlsplit
from .git_gate import GIT_GATE_TIMEOUT_SECS
DEFAULT_PORT = 9420 DEFAULT_PORT = 9420
@@ -47,6 +49,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
[hook_path, "upload-pack", str(repo_dir), peer, peer], [hook_path, "upload-pack", str(repo_dir), peer, peer],
capture_output=True, capture_output=True,
check=False, check=False,
timeout=GIT_GATE_TIMEOUT_SECS,
) )
if hook.returncode != 0: if hook.returncode != 0:
detail = (hook.stderr or hook.stdout).decode( detail = (hook.stderr or hook.stdout).decode(
@@ -110,6 +113,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
env=env, env=env,
capture_output=True, capture_output=True,
check=False, check=False,
timeout=GIT_GATE_TIMEOUT_SECS,
) )
self._write_cgi_response(proc.stdout) self._write_cgi_response(proc.stdout)
@@ -148,7 +152,13 @@ class GitHttpHandler(BaseHTTPRequestHandler):
key, _, value = line.decode("latin1").partition(":") key, _, value = line.decode("latin1").partition(":")
value = value.strip() value = value.strip()
if key.lower() == "status": 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: else:
headers.append((key, value)) headers.append((key, value))
self.send_response(status) self.send_response(status)
+46 -21
View File
@@ -90,19 +90,19 @@ def parse_jsonrpc(body: bytes) -> JsonRpcRequest:
try: try:
raw = json.loads(body) raw = json.loads(body)
except json.JSONDecodeError as e: 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): 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: 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") method = raw.get("method")
if not isinstance(method, str): 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", {}) params = raw.get("params", {})
if params is None: if params is None:
params = {} params = {}
if not isinstance(params, dict): 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) rpc_id = raw.get("id", _NO_ID)
is_notification = rpc_id is _NO_ID is_notification = rpc_id is _NO_ID
return JsonRpcRequest( return JsonRpcRequest(
@@ -117,12 +117,23 @@ _NO_ID = object()
class _RpcError(Exception): class _RpcError(Exception):
"""Base class for all typed RPC errors that surface as JSON-RPC error responses."""
def __init__(self, code: int, message: str): def __init__(self, code: int, message: str):
super().__init__(message) super().__init__(message)
self.code = code self.code = code
self.message = message 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: def jsonrpc_result(request_id: object, result: object) -> bytes:
payload = {"jsonrpc": JSONRPC_VERSION, "id": request_id, "result": result} payload = {"jsonrpc": JSONRPC_VERSION, "id": request_id, "result": result}
return (json.dumps(payload) + "\n").encode("utf-8") 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 catches obvious paste-errors / wrong-tool selections before they
enter the queue.""" enter the queue."""
if not content.strip(): 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: if tool == _sv.TOOL_CAPABILITY_BLOCK:
# Dockerfiles are too varied to validate syntactically beyond # Dockerfiles are too varied to validate syntactically beyond
# non-empty. The operator reads the diff in the TUI. # non-empty. The operator reads the diff in the TUI.
@@ -299,17 +310,17 @@ def validate_proposed_file(tool: str, content: str) -> None:
try: try:
config = load_config(content) config = load_config(content)
except ValueError as e: except ValueError as e:
raise _RpcError( raise _RpcClientError(
ERR_INVALID_PARAMS, ERR_INVALID_PARAMS,
f"{tool}: proposed routes.yaml is not valid: {e}", f"{tool}: proposed routes.yaml is not valid: {e}",
) from e ) from e
if config.log != LOG_OFF: if config.log != LOG_OFF:
raise _RpcError( raise _RpcClientError(
ERR_INVALID_PARAMS, ERR_INVALID_PARAMS,
f"{tool}: proposed routes.yaml must not change egress logging", f"{tool}: proposed routes.yaml must not change egress logging",
) )
else: else:
raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}") raise _RpcClientError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}")
# --- MCP handlers ---------------------------------------------------------- # --- MCP handlers ----------------------------------------------------------
@@ -382,17 +393,17 @@ def handle_tools_call(
doesn't need operator approval.""" doesn't need operator approval."""
name = params.get("name") name = params.get("name")
if not isinstance(name, str): 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: if name == _sv.TOOL_LIST_EGRESS_ROUTES:
return handle_list_egress_routes(typing.cast(dict[str, object], params.get("arguments", {})), config) return handle_list_egress_routes(typing.cast(dict[str, object], params.get("arguments", {})), config)
args_raw = params.get("arguments", {}) args_raw = params.get("arguments", {})
if not isinstance(args_raw, dict): 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") justification = args_raw.get("justification")
if not isinstance(justification, str) or not justification.strip(): if not isinstance(justification, str) or not justification.strip():
raise _RpcError( raise _RpcClientError(
ERR_INVALID_PARAMS, ERR_INVALID_PARAMS,
f"{name}: 'justification' is required and must be a non-empty string", 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] file_field = PROPOSED_FILE_FIELD[name]
proposed_file = args_raw.get(file_field) proposed_file = args_raw.get(file_field)
if not isinstance(proposed_file, str): if not isinstance(proposed_file, str):
raise _RpcError( raise _RpcClientError(
ERR_INVALID_PARAMS, ERR_INVALID_PARAMS,
f"{name}: '{file_field}' is required and must be a string", f"{name}: '{file_field}' is required and must be a string",
) )
validate_proposed_file(name, proposed_file) validate_proposed_file(name, proposed_file)
else: 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( proposal = _sv.Proposal.new(
bottle_slug=config.bottle_slug, bottle_slug=config.bottle_slug,
@@ -416,7 +427,10 @@ def handle_tools_call(
justification=justification, justification=justification,
current_file_hash=_sv.sha256_hex(proposed_file), 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( sys.stderr.write(
f"supervise: queued proposal {proposal.id} ({name}) " f"supervise: queued proposal {proposal.id} ({name}) "
f"for bottle {config.bottle_slug}; waiting for operator...\n" f"for bottle {config.bottle_slug}; waiting for operator...\n"
@@ -436,7 +450,10 @@ def handle_tools_call(
"content": [{"type": "text", "text": text}], "content": [{"type": "text", "text": text}],
"isError": False, "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) text = format_response_text(response)
return { return {
@@ -512,7 +529,7 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
try: try:
req = parse_jsonrpc(body) req = parse_jsonrpc(body)
except _RpcError as e: except _RpcClientError as e:
self._write_jsonrpc(jsonrpc_error(None, e.code, e.message)) self._write_jsonrpc(jsonrpc_error(None, e.code, e.message))
return return
@@ -520,11 +537,19 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
try: try:
result = self._dispatch(req, config) result = self._dispatch(req, config)
except _RpcError as e: except _RpcClientError as e:
self._write_jsonrpc(jsonrpc_error(req.id, e.code, e.message)) self._write_jsonrpc(jsonrpc_error(req.id, e.code, e.message))
return return
except Exception as e: # noqa: W0718 — catch-all for RPC dispatch errors except _RpcInternalError as e:
sys.stderr.write(f"supervise: internal error: {e}\n") 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")) self._write_jsonrpc(jsonrpc_error(req.id, ERR_INTERNAL, "internal error"))
return return
@@ -543,7 +568,7 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
return handle_tools_list(req.params) return handle_tools_list(req.params)
if method == "tools/call": if method == "tools/call":
return handle_tools_call(req.params, config) 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: def _write_jsonrpc(self, body: bytes) -> None:
self.send_response(200) self.send_response(200)
@@ -10,6 +10,8 @@ from unittest.mock import MagicMock, patch
from bot_bottle.contrib.gitea.deploy_key_provisioner import ( from bot_bottle.contrib.gitea.deploy_key_provisioner import (
GiteaDeployKeyProvisioner, GiteaDeployKeyProvisioner,
_API_TIMEOUT_SECS,
_KEYGEN_TIMEOUT_SECS,
_split_owner_repo, _split_owner_repo,
) )
from bot_bottle.deploy_key_provisioner import DeployKeyCollisionError from bot_bottle.deploy_key_provisioner import DeployKeyCollisionError
@@ -83,6 +85,25 @@ class TestCreate(unittest.TestCase):
self.assertEqual(str(fake_key_id), key_id) self.assertEqual(str(fake_key_id), key_id)
self.assertEqual(fake_private, private_bytes) self.assertEqual(fake_private, private_bytes)
def test_create_passes_timeout_to_ssh_keygen_and_urlopen(self):
provisioner = _provisioner()
with patch(
"bot_bottle.contrib.gitea.deploy_key_provisioner.subprocess.run"
) as mock_run, patch(
"bot_bottle.contrib.gitea.deploy_key_provisioner.urllib.request.urlopen"
) as mock_urlopen, patch(
"bot_bottle.contrib.gitea.deploy_key_provisioner.Path.read_bytes",
return_value=b"PRIVATE",
), patch(
"bot_bottle.contrib.gitea.deploy_key_provisioner.Path.read_text",
return_value="ssh-ed25519 AAAA\n",
):
mock_urlopen.return_value = _urlopen_response({"id": 1})
provisioner.create("owner/repo", "title")
self.assertEqual(_KEYGEN_TIMEOUT_SECS, mock_run.call_args.kwargs.get("timeout"))
self.assertEqual(_API_TIMEOUT_SECS, mock_urlopen.call_args.kwargs.get("timeout"))
def test_create_raises_on_http_error(self): def test_create_raises_on_http_error(self):
provisioner = _provisioner() provisioner = _provisioner()
with patch( with patch(
@@ -139,6 +160,16 @@ class TestDelete(unittest.TestCase):
self.assertIn("/api/v1/repos/didericis/bot-bottle/keys/99", req.full_url) self.assertIn("/api/v1/repos/didericis/bot-bottle/keys/99", req.full_url)
self.assertEqual("DELETE", req.get_method()) self.assertEqual("DELETE", req.get_method())
def test_delete_passes_timeout_to_urlopen(self):
provisioner = _provisioner()
with patch(
"bot_bottle.contrib.gitea.deploy_key_provisioner.urllib.request.urlopen"
) as mock_urlopen:
mock_urlopen.return_value = _urlopen_response({})
provisioner.delete("owner/repo", "7")
self.assertEqual(_API_TIMEOUT_SECS, mock_urlopen.call_args.kwargs.get("timeout"))
def test_delete_tolerates_404(self): def test_delete_tolerates_404(self):
provisioner = _provisioner() provisioner = _provisioner()
with patch( with patch(
+107
View File
@@ -9,6 +9,7 @@ import urllib.request
from pathlib import Path from pathlib import Path
from unittest import mock from unittest import mock
from bot_bottle.git_gate import GIT_GATE_TIMEOUT_SECS
from bot_bottle.git_http_backend import GitHttpHandler, MAX_BODY_BYTES from bot_bottle.git_http_backend import GitHttpHandler, MAX_BODY_BYTES
@@ -150,6 +151,61 @@ class TestGitHttpBackend(unittest.TestCase):
) )
self.assertEqual("git/test", env["HTTP_USER_AGENT"]) self.assertEqual("git/test", env["HTTP_USER_AGENT"])
def test_subprocess_calls_include_timeout(self):
"""Both subprocess.run calls (access-hook and git http-backend) must
pass timeout= so a hung upstream cannot wedge the sidecar."""
from http.server import ThreadingHTTPServer
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
(root / "repo.git").mkdir()
old_root = os.environ.get("GIT_PROJECT_ROOT")
os.environ["GIT_PROJECT_ROOT"] = str(root)
self.addCleanup(self._restore_env, old_root)
old_hook = os.environ.get("GIT_GATE_ACCESS_HOOK")
hook = root / "access-hook"
hook.write_text("#!/bin/sh\nexit 0\n")
hook.chmod(0o700)
os.environ["GIT_GATE_ACCESS_HOOK"] = str(hook)
self.addCleanup(self._restore_hook, old_hook)
server = ThreadingHTTPServer(("127.0.0.1", 0), GitHttpHandler)
thread = threading.Thread(target=server.serve_forever, daemon=True)
thread.start()
self.addCleanup(server.shutdown)
self.addCleanup(server.server_close)
backend_response = (
b"Status: 200 OK\r\n"
b"Content-Type: application/x-git-upload-pack-result\r\n"
b"\r\n"
b"0000"
)
calls = [
subprocess.CompletedProcess(["hook"], 0, b"", b""),
subprocess.CompletedProcess(["git"], 0, backend_response, b""),
]
with mock.patch(
"bot_bottle.git_http_backend.subprocess.run",
side_effect=calls,
) as run:
req = urllib.request.Request(
f"http://127.0.0.1:{server.server_port}"
"/repo.git/git-upload-pack",
data=b"",
method="POST",
)
with urllib.request.urlopen(req, timeout=5):
pass
for call in run.call_args_list:
self.assertEqual(
GIT_GATE_TIMEOUT_SECS,
call.kwargs.get("timeout"),
f"subprocess.run call missing timeout: {call}",
)
def test_access_hook_denial_is_logged_to_stdout(self): def test_access_hook_denial_is_logged_to_stdout(self):
"""When the access-hook exits non-zero we still return 403 to the """When the access-hook exits non-zero we still return 403 to the
client, but the hook's stderr must also appear on the handler's client, but the hook's stderr must also appear on the handler's
@@ -256,6 +312,57 @@ class TestGitHttpBackend(unittest.TestCase):
os.environ["GIT_GATE_ACCESS_HOOK"] = value 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): class TestContentLengthBounds(unittest.TestCase):
"""PRD 0041: malformed or oversized Content-Length is rejected before """PRD 0041: malformed or oversized Content-Length is rejected before
git http-backend is invoked.""" git http-backend is invoked."""
+27
View File
@@ -73,6 +73,33 @@ resolver #2
) )
self.assertTrue(run.call_args_list[-1].kwargs["check"]) self.assertTrue(run.call_args_list[-1].kwargs["check"])
def test_build_image_anchors_relative_dockerfile_to_context(self):
status = util.subprocess.CompletedProcess(
args=[],
returncode=0,
stdout=(
'[{"status":{"state":"running"},'
'"configuration":{"dns":{"nameservers":["9.9.9.9"]}}}]'
),
stderr="",
)
with patch.object(util.subprocess, "run", return_value=status) as run, \
patch.object(util.os, "environ", {
"BOT_BOTTLE_MACOS_CONTAINER_DNS": "9.9.9.9",
}):
util.build_image(
"bot-bottle-sidecars:latest",
"/repo",
dockerfile="Dockerfile.sidecars",
)
self.assertEqual(
[
"container", "build", "-t", "bot-bottle-sidecars:latest",
"--dns", "9.9.9.9", "-f", "/repo/Dockerfile.sidecars", "/repo",
],
run.call_args_list[-1].args[0],
)
def test_commit_container_execs_tar_and_builds_image(self): def test_commit_container_execs_tar_and_builds_image(self):
# stderr is bytes because subprocess.run uses stderr=PIPE without text=True # stderr is bytes because subprocess.run uses stderr=PIPE without text=True
completed = util.subprocess.CompletedProcess( completed = util.subprocess.CompletedProcess(
+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 import supervise_server # noqa: E402
from bot_bottle.supervise_server import ( from bot_bottle.supervise_server import (
ERR_INTERNAL,
ERR_INVALID_PARAMS, ERR_INVALID_PARAMS,
ERR_INVALID_REQUEST, ERR_INVALID_REQUEST,
ERR_METHOD_NOT_FOUND, ERR_METHOD_NOT_FOUND,
@@ -29,7 +30,9 @@ from bot_bottle.supervise_server import (
PROPOSED_FILE_FIELD, PROPOSED_FILE_FIELD,
ServerConfig, ServerConfig,
TOOL_DEFINITIONS, TOOL_DEFINITIONS,
_RpcClientError,
_RpcError, _RpcError,
_RpcInternalError,
_response_timeout_from_env, _response_timeout_from_env,
format_response_text, format_response_text,
handle_initialize, handle_initialize,
@@ -77,6 +80,65 @@ class TestValidation(unittest.TestCase):
self.assertIn("must not change egress logging", cm.exception.message) 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 ------------------------------------------------------ # --- JSON-RPC parsing ------------------------------------------------------
@@ -469,6 +531,26 @@ class TestHttpEndToEnd(unittest.TestCase):
) )
self.assertEqual(ERR_METHOD_NOT_FOUND, result["error"]["code"]) # type: ignore[index] 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): def test_health_endpoint(self):
conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=5) conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=5)
try: try: