Compare commits
3 Commits
71005d56e2
...
e84e7e6ba4
| Author | SHA1 | Date | |
|---|---|---|---|
| e84e7e6ba4 | |||
| 0969d91d58 | |||
| 52033a6290 |
@@ -19,6 +19,9 @@ from urllib.parse import urlsplit
|
|||||||
|
|
||||||
DEFAULT_PORT = 9420
|
DEFAULT_PORT = 9420
|
||||||
|
|
||||||
|
# Body-size cap matching supervise_server.py's 1 MiB limit.
|
||||||
|
_MAX_BODY_BYTES = 1 * 1024 * 1024
|
||||||
|
|
||||||
|
|
||||||
class GitHttpHandler(BaseHTTPRequestHandler):
|
class GitHttpHandler(BaseHTTPRequestHandler):
|
||||||
server_version = "bot-bottle-git-http/1"
|
server_version = "bot-bottle-git-http/1"
|
||||||
@@ -76,7 +79,18 @@ class GitHttpHandler(BaseHTTPRequestHandler):
|
|||||||
value = self.headers.get(header)
|
value = self.headers.get(header)
|
||||||
if value:
|
if value:
|
||||||
env[variable] = value
|
env[variable] = value
|
||||||
length = int(self.headers.get("content-length", "0") or "0")
|
raw_length = self.headers.get("content-length", "0") or "0"
|
||||||
|
try:
|
||||||
|
length = int(raw_length)
|
||||||
|
except ValueError:
|
||||||
|
self.send_error(400, "Bad Content-Length")
|
||||||
|
return
|
||||||
|
if length < 0:
|
||||||
|
self.send_error(400, "Negative Content-Length")
|
||||||
|
return
|
||||||
|
if length > _MAX_BODY_BYTES:
|
||||||
|
self.send_error(413, "Request body too large")
|
||||||
|
return
|
||||||
body = self.rfile.read(length) if length else b""
|
body = self.rfile.read(length) if length else b""
|
||||||
proc = subprocess.run(
|
proc = subprocess.run(
|
||||||
["git", "http-backend"],
|
["git", "http-backend"],
|
||||||
|
|||||||
@@ -0,0 +1,74 @@
|
|||||||
|
# PRD 0041: Git HTTP Request Bounds
|
||||||
|
|
||||||
|
- **Status:** Active
|
||||||
|
- **Author:** didericis-codex
|
||||||
|
- **Created:** 2026-06-02
|
||||||
|
- **Issue:** #138
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Add Content-Length validation and a body-size cap to `git_http_backend.py` so
|
||||||
|
malformed or oversized smart-HTTP requests fail cleanly rather than crashing
|
||||||
|
the handler or exhausting memory.
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
`bot_bottle/git_http_backend.py` calls `int(self.headers.get("Content-Length",
|
||||||
|
0))` without catching `ValueError`. A request with a non-numeric Content-Length
|
||||||
|
raises an unhandled exception in the request handler.
|
||||||
|
|
||||||
|
The handler reads the full declared length into memory before passing the body
|
||||||
|
to `git http-backend` with no upper bound. A local or compromised client can
|
||||||
|
force arbitrarily high memory use. For comparison, `supervise_server.py` caps
|
||||||
|
request bodies at 1 MiB.
|
||||||
|
|
||||||
|
## Goals / Success Criteria
|
||||||
|
|
||||||
|
- A missing or non-numeric Content-Length returns HTTP 400.
|
||||||
|
- A negative Content-Length returns HTTP 400.
|
||||||
|
- A body larger than the cap (1 MiB, matching `supervise_server.py`) returns
|
||||||
|
HTTP 413.
|
||||||
|
- Valid Git smart-HTTP pushes and fetches continue to work.
|
||||||
|
- Unit tests cover: missing length, non-numeric length, negative length,
|
||||||
|
over-cap length, and a valid push/fetch passthrough.
|
||||||
|
|
||||||
|
## Non-goals
|
||||||
|
|
||||||
|
- No changes to git-gate authentication or route logic.
|
||||||
|
- No changes to `supervise_server.py`.
|
||||||
|
- No streaming / chunked-transfer-encoding support.
|
||||||
|
- No TLS changes.
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
In scope:
|
||||||
|
|
||||||
|
- `bot_bottle/git_http_backend.py` request parsing and body reading.
|
||||||
|
- Unit tests in `tests/unit/test_git_http_backend.py`.
|
||||||
|
|
||||||
|
Out of scope:
|
||||||
|
|
||||||
|
- Integration tests that drive a real Git client through the handler.
|
||||||
|
|
||||||
|
## Design
|
||||||
|
|
||||||
|
Wrap the Content-Length parse in a try/except and return 400 on `ValueError`.
|
||||||
|
Add an explicit check for negative values. After parsing, compare the declared
|
||||||
|
length against a module-level `_MAX_BODY_BYTES` constant (default 1 MiB) and
|
||||||
|
return 413 if exceeded. Read exactly `min(content_length, _MAX_BODY_BYTES)`
|
||||||
|
bytes.
|
||||||
|
|
||||||
|
## Testing Strategy
|
||||||
|
|
||||||
|
- Unit tests using `unittest.mock` to drive the handler with crafted headers.
|
||||||
|
- Test cases: no Content-Length header, `Content-Length: abc`, `Content-Length:
|
||||||
|
-1`, `Content-Length: 2097152` (over cap), and a normal small POST body.
|
||||||
|
|
||||||
|
Run:
|
||||||
|
|
||||||
|
- `python3 -m unittest tests.unit.test_git_http_backend`
|
||||||
|
- `python3 -m unittest discover -s tests/unit`
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
None.
|
||||||
@@ -165,5 +165,77 @@ class TestGitHttpBackend(unittest.TestCase):
|
|||||||
os.environ["GIT_GATE_ACCESS_HOOK"] = value
|
os.environ["GIT_GATE_ACCESS_HOOK"] = value
|
||||||
|
|
||||||
|
|
||||||
|
class TestContentLengthBounds(unittest.TestCase):
|
||||||
|
"""PRD 0041: malformed or oversized Content-Length is rejected before
|
||||||
|
git http-backend is invoked."""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
from http.server import ThreadingHTTPServer
|
||||||
|
import tempfile, os
|
||||||
|
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 _post(self, path: str, *, content_length_header: str,
|
||||||
|
body: bytes = b"x") -> int:
|
||||||
|
req = urllib.request.Request(
|
||||||
|
f"http://127.0.0.1:{self._port}{path}",
|
||||||
|
data=body,
|
||||||
|
method="POST",
|
||||||
|
)
|
||||||
|
req.add_header("Content-Length", content_length_header)
|
||||||
|
req.add_header("Content-Type", "application/x-git-receive-pack-request")
|
||||||
|
try:
|
||||||
|
with urllib.request.urlopen(req, timeout=3) as resp:
|
||||||
|
return resp.status
|
||||||
|
except urllib.error.HTTPError as e:
|
||||||
|
return e.code
|
||||||
|
|
||||||
|
def test_non_numeric_content_length_returns_400(self):
|
||||||
|
status = self._post("/repo.git/git-receive-pack",
|
||||||
|
content_length_header="abc")
|
||||||
|
self.assertEqual(400, status)
|
||||||
|
|
||||||
|
def test_negative_content_length_returns_400(self):
|
||||||
|
status = self._post("/repo.git/git-receive-pack",
|
||||||
|
content_length_header="-1")
|
||||||
|
self.assertEqual(400, status)
|
||||||
|
|
||||||
|
def test_oversized_content_length_returns_413(self):
|
||||||
|
# Declare 2 MiB — over the 1 MiB cap.
|
||||||
|
status = self._post("/repo.git/git-receive-pack",
|
||||||
|
content_length_header=str(2 * 1024 * 1024))
|
||||||
|
self.assertEqual(413, status)
|
||||||
|
|
||||||
|
def test_valid_small_body_passes_through(self):
|
||||||
|
# With a valid Content-Length the handler proceeds into
|
||||||
|
# git http-backend; that will fail (no real git repo) but the
|
||||||
|
# status won't be 400 or 413.
|
||||||
|
with mock.patch("bot_bottle.git_http_backend.subprocess.run") as run:
|
||||||
|
run.return_value = mock.Mock(
|
||||||
|
returncode=0,
|
||||||
|
stdout=(
|
||||||
|
b"Status: 200 OK\r\n"
|
||||||
|
b"Content-Type: application/x-git-receive-pack-result\r\n"
|
||||||
|
b"\r\n"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
status = self._post("/repo.git/git-receive-pack",
|
||||||
|
content_length_header="1", body=b"x")
|
||||||
|
self.assertNotIn(status, (400, 413))
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user