From b1850be5d111c02379009ce3a0b96fdbec837204 Mon Sep 17 00:00:00 2001 From: didericis Date: Fri, 17 Jul 2026 21:49:30 -0400 Subject: [PATCH] fix(git-gate): make the gateway access-hook executable regardless of copy transport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cloning/fetching from the git-gate on the Apple-container backend failed with "empty reply from server" (curl exit 52). Root cause: the git-http handler crashed on every upload-pack with PermissionError: [Errno 13] Permission denied: '/etc/git-gate/access-hook' The access-hook is exec'd directly, so it needs the x bit. prepare() stages it 0o700 and trusted the gateway copy to carry that mode. `docker cp` does; the Apple `container cp` (AppleGatewayTransport) does not, landing the hook 0o644 → EACCES. The unhandled exception killed the handler thread, closing the socket with no HTTP response — which the client sees as the opaque empty reply. - provision_git_gate now `chmod +x`es the access-hook on the gateway side after the copy, so it's executable under every transport (docker/apple/firecracker). - git-http handler wraps the access-hook subprocess.run: an OSError / SubprocessError (un-execable, timed out) now fails closed with a 503 instead of crashing the thread into an empty reply — a gate that can't run its hook should deny, visibly. - Updates the now-misleading "docker cp preserves source mode" comment in git_gate.prepare(). Regression tests: provisioning applies +x to the access-hook; the handler returns 503 (not an empty reply) when the hook can't be exec'd. Co-Authored-By: Claude Opus 4.8 --- .../backend/docker/gateway_provision.py | 6 +++ bot_bottle/git_gate.py | 6 ++- bot_bottle/git_http_backend.py | 24 ++++++++--- tests/unit/test_gateway_provision.py | 12 ++++++ tests/unit/test_git_http_backend.py | 42 +++++++++++++++++++ 5 files changed, 82 insertions(+), 8 deletions(-) diff --git a/bot_bottle/backend/docker/gateway_provision.py b/bot_bottle/backend/docker/gateway_provision.py index e7d5596..d1094dc 100644 --- a/bot_bottle/backend/docker/gateway_provision.py +++ b/bot_bottle/backend/docker/gateway_provision.py @@ -94,6 +94,12 @@ def provision_git_gate( transport.exec(["mkdir", "-p", "/etc/git-gate"]) transport.cp_into(str(plan.hook_script), "/etc/git-gate/pre-receive") transport.cp_into(str(plan.access_hook_script), "/etc/git-gate/access-hook") + # The access-hook is exec'd directly (not via `sh`), so it needs the x bit. + # Set it here rather than trusting the copy to carry the staged 0o700: + # `docker cp` preserves source mode, but the Apple `container cp` does not, + # landing the hook 0o644 → EACCES when the git-http handler tries to exec it. + # chmod on the gateway side is backend-neutral and fixes every transport. + transport.exec(["chmod", "+x", "/etc/git-gate/access-hook"]) creds = _creds_dir(bottle_id) transport.exec(["mkdir", "-p", creds]) for u in plan.upstreams: diff --git a/bot_bottle/git_gate.py b/bot_bottle/git_gate.py index 59305d2..d7c0f96 100644 --- a/bot_bottle/git_gate.py +++ b/bot_bottle/git_gate.py @@ -112,8 +112,10 @@ class GitGate(ABC): access_hook = stage_dir / "git_gate_access_hook.sh" access_hook.write_text(git_gate_render_access_hook()) # 0o700 (not 0o600): git daemon execs --access-hook directly, - # not via `sh`, so the script needs the x bit. docker cp - # preserves source mode into the container. + # not via `sh`, so the script needs the x bit. The gateway copy + # does not necessarily preserve this mode (`docker cp` does, the + # Apple `container cp` does not), so provision_git_gate re-applies + # +x on the gateway side — see backend/docker/gateway_provision.py. access_hook.chmod(0o700) upstreams_with_files: list[GitGateUpstream] = [] for u in upstreams: diff --git a/bot_bottle/git_http_backend.py b/bot_bottle/git_http_backend.py index 483ed84..12e09f3 100644 --- a/bot_bottle/git_http_backend.py +++ b/bot_bottle/git_http_backend.py @@ -148,12 +148,24 @@ class GitHttpHandler(BaseHTTPRequestHandler): "GIT_GATE_ACCESS_HOOK", "/etc/git-gate/access-hook", ) peer = self.client_address[0] - hook = subprocess.run( - [hook_path, "upload-pack", str(repo_dir), peer, peer], - capture_output=True, - check=False, - timeout=GIT_GATE_TIMEOUT_SECS, - ) + try: + hook = subprocess.run( + [hook_path, "upload-pack", str(repo_dir), peer, peer], + capture_output=True, + check=False, + timeout=GIT_GATE_TIMEOUT_SECS, + ) + except (OSError, subprocess.SubprocessError) as exc: + # The access-hook couldn't be run (missing, not executable, + # timed out, …). Fail closed with a real HTTP error rather + # than letting the exception kill the handler thread — an + # unhandled exception closes the socket with no response, which + # the client sees as an opaque "empty reply from server". + self.log_message( + "access-hook could not run for %s: %s", parsed.path, exc, + ) + self.send_error(503, "git-gate access-hook unavailable") + return if hook.returncode != 0: detail = (hook.stderr or hook.stdout).decode( "utf-8", errors="replace", diff --git a/tests/unit/test_gateway_provision.py b/tests/unit/test_gateway_provision.py index fe2a4f6..e247038 100644 --- a/tests/unit/test_gateway_provision.py +++ b/tests/unit/test_gateway_provision.py @@ -69,6 +69,18 @@ class TestProvisionGitGate(unittest.TestCase): self.assertEqual(1, len(exec_scripts)) self.assertIn("repo=/git/bottle1/${name}.git", exec_scripts[0][-1]) + def test_makes_access_hook_executable_on_the_gateway(self) -> None: + # Regression: the access-hook is exec'd directly, so it needs the x + # bit. The copy alone can't be trusted to carry the staged 0o700 + # (`docker cp` preserves mode, the Apple `container cp` does not), + # so provisioning must re-apply +x on the gateway side. + calls: list[list[str]] = [] + with patch(_RUN, side_effect=_recorder(calls)): + provision_git_gate(DockerGatewayTransport("gw"), "bottle1", _plan(_up("foo"))) + self.assertIn( + ["docker", "exec", "gw", "chmod", "+x", "/etc/git-gate/access-hook"], calls, + ) + def test_omits_known_hosts_copy_when_absent(self) -> None: calls: list[list[str]] = [] with patch(_RUN, side_effect=_recorder(calls)): diff --git a/tests/unit/test_git_http_backend.py b/tests/unit/test_git_http_backend.py index cda0d32..e340e20 100644 --- a/tests/unit/test_git_http_backend.py +++ b/tests/unit/test_git_http_backend.py @@ -364,6 +364,48 @@ class TestGitHttpBackend(unittest.TestCase): self.assertIn("access-hook denied", logged) self.assertIn("exit=2", logged) + def test_access_hook_that_cannot_run_fails_closed_503(self): + """Regression: when the access-hook can't be exec'd (missing / not + executable — a PermissionError from subprocess.run), the handler must + fail closed with a real HTTP status instead of letting the exception + kill the thread, which closes the socket with no response and the + client sees an opaque "empty reply from server".""" + from http.server import ThreadingHTTPServer + import io + import sys + + 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) + + 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) + + with mock.patch( + "bot_bottle.git_http_backend.subprocess.run", + side_effect=PermissionError(13, "Permission denied"), + ): + buf = io.StringIO() + with mock.patch.object(sys, "stdout", buf): + req = urllib.request.Request( + f"http://127.0.0.1:{server.server_port}" + "/repo.git/info/refs?service=git-upload-pack", + method="GET", + ) + try: + urllib.request.urlopen(req, timeout=5) + self.fail("expected HTTPError 503") + except urllib.error.HTTPError as e: # type: ignore + self.assertEqual(503, e.code) + + self.assertIn("access-hook could not run", buf.getvalue()) + @staticmethod def _restore_env(value: str | None) -> None: if value is None: