From b5b694acb8f26698d2a07006f8ec18db04a9bd8e Mon Sep 17 00:00:00 2001 From: didericis Date: Tue, 2 Jun 2026 23:02:10 -0400 Subject: [PATCH] fix(git-http): log access-hook denial detail to stdout Previously when the access-hook returned non-zero, git-http would pipe the hook's stderr into the 403 body sent back to the agent's git client but never log it locally, so docker logs just showed `"GET ... 403 -"` with no explanation. Operators had to shell into the sidecar and re-run the hook by hand to find out why a clone was being refused (e.g. upstream SSH unreachable, missing credentials). Route the hook's stderr/stdout through the existing log_message channel before sending the 403, one log line per output line so the default request-log format stays readable. When the hook exits non-zero with no output, log the exit code so the line is still informative. Co-Authored-By: Claude Opus 4.7 --- bot_bottle/git_http_backend.py | 12 ++++ tests/unit/test_git_http_backend.py | 91 +++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/bot_bottle/git_http_backend.py b/bot_bottle/git_http_backend.py index 93e4bf9..6ac0453 100644 --- a/bot_bottle/git_http_backend.py +++ b/bot_bottle/git_http_backend.py @@ -49,6 +49,18 @@ class GitHttpHandler(BaseHTTPRequestHandler): check=False, ) if hook.returncode != 0: + detail = (hook.stderr or hook.stdout).decode( + "utf-8", errors="replace", + ).rstrip() + if detail: + for line in detail.splitlines(): + self.log_message("access-hook denied %s: %s", + parsed.path, line) + else: + self.log_message( + "access-hook denied %s: exit=%d (no output)", + parsed.path, hook.returncode, + ) self.send_response(403) self.send_header("Content-Type", "text/plain; charset=utf-8") self.end_headers() diff --git a/tests/unit/test_git_http_backend.py b/tests/unit/test_git_http_backend.py index 037b6fc..c6c98b9 100644 --- a/tests/unit/test_git_http_backend.py +++ b/tests/unit/test_git_http_backend.py @@ -150,6 +150,97 @@ class TestGitHttpBackend(unittest.TestCase): ) self.assertEqual("git/test", env["HTTP_USER_AGENT"]) + 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 + stdout so docker logs surface *why* — otherwise the agent sees + the message and the operator just sees `403 -`.""" + 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) + + denial = b"git-gate: upstream fetch failed; refusing to serve stale data\n" + with mock.patch( + "bot_bottle.git_http_backend.subprocess.run", + return_value=subprocess.CompletedProcess( + ["hook"], 1, b"", denial, + ), + ): + 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 403") + except urllib.error.HTTPError as e: + self.assertEqual(403, e.code) + self.assertIn(b"upstream fetch failed", e.read()) + + logged = buf.getvalue() + self.assertIn("access-hook denied", logged) + self.assertIn("upstream fetch failed", logged) + + def test_access_hook_denial_without_output_logs_exit_code(self): + """If the hook exits non-zero but produces no stderr/stdout, the + log line should still say *something* — the exit code — instead + of silently emitting an empty line.""" + 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", + return_value=subprocess.CompletedProcess( + ["hook"], 2, b"", b"", + ), + ): + 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 403") + except urllib.error.HTTPError as e: + self.assertEqual(403, e.code) + + logged = buf.getvalue() + self.assertIn("access-hook denied", logged) + self.assertIn("exit=2", logged) + @staticmethod def _restore_env(value: str | None) -> None: if value is None: