didericis/bot-bottle
1
0
Fork 0
Code Issues 7 Pull Requests 15 Actions Packages Projects Releases Wiki Activity

fix: add explicit timeouts to subprocess and HTTP calls in git-gate paths

Browse Source 
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.
This commit is contained in:
didericis (claude)
2026-06-25 06:59:09 +00:00
committed by didericis
parent d99dba037c
commit 508c537deb
4 changed files with 107 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bot_bottle/contrib/gitea/deploy_key_provisioner.py
+8 -2
View File
@@ -21,6 +21,11 @@ from pathlib import Path
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):
"""Manages deploy keys on a Gitea instance."""
@@ -46,6 +51,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
check=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
timeout=_KEYGEN_TIMEOUT_SECS,
)
private_key = key_path.read_bytes()
public_key = key_path.with_suffix(".pub").read_text().strip()
@@ -67,7 +73,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
method="POST",
)
try:
with urllib.request.urlopen(req) as resp:
with urllib.request.urlopen(req, timeout=_API_TIMEOUT_SECS) as resp:
body = json.loads(resp.read())
except urllib.error.HTTPError as exc:
_body = _read_error_body(exc)
@@ -98,7 +104,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
method="DELETE",
)
try:
with urllib.request.urlopen(req):
with urllib.request.urlopen(req, timeout=_API_TIMEOUT_SECS):
pass
except urllib.error.HTTPError as exc:
if exc.code == 404:
bot_bottle/git_http_backend.py
+8
View File
@@ -22,6 +22,12 @@ DEFAULT_PORT = 9420
# Bound memory use while still allowing ordinary git push packfiles.
MAX_BODY_BYTES = 100 * 1024 * 1024
# Timeout for the access-hook subprocess and git http-backend CGI subprocess.
# Mirrors GIT_GATE_DAEMON_TIMEOUT_SECS so both HTTP and daemon paths share the
# same bound: a hung upstream fetch in the access-hook or a stalled CGI child
# cannot wedge the sidecar indefinitely.
GIT_HTTP_BACKEND_TIMEOUT_SECS = 30
class GitHttpHandler(BaseHTTPRequestHandler):
server_version = "bot-bottle-git-http/1"
@@ -47,6 +53,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
[hook_path, "upload-pack", str(repo_dir), peer, peer],
capture_output=True,
check=False,
timeout=GIT_HTTP_BACKEND_TIMEOUT_SECS,
)
if hook.returncode != 0:
detail = (hook.stderr or hook.stdout).decode(
@@ -110,6 +117,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
env=env,
capture_output=True,
check=False,
timeout=GIT_HTTP_BACKEND_TIMEOUT_SECS,
)
self._write_cgi_response(proc.stdout)
tests/unit/test_contrib_gitea_deploy_key.py
+31
View File
@@ -10,6 +10,8 @@ from unittest.mock import MagicMock, patch
from bot_bottle.contrib.gitea.deploy_key_provisioner import (
GiteaDeployKeyProvisioner,
_API_TIMEOUT_SECS,
_KEYGEN_TIMEOUT_SECS,
_split_owner_repo,
)
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(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):
provisioner = _provisioner()
with patch(
@@ -139,6 +160,16 @@ class TestDelete(unittest.TestCase):
self.assertIn("/api/v1/repos/didericis/bot-bottle/keys/99", req.full_url)
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):
provisioner = _provisioner()
with patch(
tests/unit/test_git_http_backend.py
+60 -1
View File
@@ -9,7 +9,11 @@ import urllib.request
from pathlib import Path
from unittest import mock
from bot_bottle.git_http_backend import GitHttpHandler, MAX_BODY_BYTES
from bot_bottle.git_http_backend import (
GIT_HTTP_BACKEND_TIMEOUT_SECS,
GitHttpHandler,
MAX_BODY_BYTES,
)
class TestGitHttpBackend(unittest.TestCase):
@@ -150,6 +154,61 @@ class TestGitHttpBackend(unittest.TestCase):
)
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_HTTP_BACKEND_TIMEOUT_SECS,
call.kwargs.get("timeout"),
f"subprocess.run call missing timeout: {call}",
)
def test_access_hook_denial_is_logged_to_stdout(self):
"""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