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.
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user