Compare commits
3 Commits
5696e5870e
...
acb9cd67c6
| Author | SHA1 | Date | |
|---|---|---|---|
| acb9cd67c6 | |||
| d90ab7e646 | |||
| 8ea90adcaf |
@@ -26,7 +26,7 @@ jobs:
|
|||||||
- name: Run pylint
|
- name: Run pylint
|
||||||
run: |
|
run: |
|
||||||
# Run pylint on all Python files in the repo
|
# Run pylint on all Python files in the repo
|
||||||
find . -name '*.py' -not -path './.venv/*' -not -path './.git/*' | xargs pylint --fail-under=8.0 || true
|
find . -name '*.py' -not -path './.venv/*' -not -path './.git/*' | xargs pylint --fail-under=8.0
|
||||||
|
|
||||||
- name: Run pyright
|
- name: Run pyright
|
||||||
run: |
|
run: |
|
||||||
|
|||||||
+17
-1
@@ -204,6 +204,7 @@ def git_gate_render_entrypoint(upstreams: tuple[GitGateUpstream, ...]) -> str:
|
|||||||
" git -C \"$repo\" config git-gate.identityFile \"$keyfile\"",
|
" git -C \"$repo\" config git-gate.identityFile \"$keyfile\"",
|
||||||
" git -C \"$repo\" config git-gate.knownHosts \"$hostsfile\"",
|
" git -C \"$repo\" config git-gate.knownHosts \"$hostsfile\"",
|
||||||
" git -C \"$repo\" config receive.denyCurrentBranch ignore",
|
" git -C \"$repo\" config receive.denyCurrentBranch ignore",
|
||||||
|
" git -C \"$repo\" config receive.advertisePushOptions true",
|
||||||
" git -C \"$repo\" config http.receivepack true",
|
" git -C \"$repo\" config http.receivepack true",
|
||||||
" install -m 755 /etc/git-gate/pre-receive \"$repo/hooks/pre-receive\"",
|
" install -m 755 /etc/git-gate/pre-receive \"$repo/hooks/pre-receive\"",
|
||||||
"}",
|
"}",
|
||||||
@@ -280,6 +281,21 @@ if [ ! -f "$hostsfile" ]; then
|
|||||||
fi
|
fi
|
||||||
ssh_cmd="ssh -i $keyfile -o UserKnownHostsFile=$hostsfile -o StrictHostKeyChecking=yes -o IdentitiesOnly=yes -o BatchMode=yes -o ConnectTimeout=10"
|
ssh_cmd="ssh -i $keyfile -o UserKnownHostsFile=$hostsfile -o StrictHostKeyChecking=yes -o IdentitiesOnly=yes -o BatchMode=yes -o ConnectTimeout=10"
|
||||||
|
|
||||||
|
push_option_count=${GIT_PUSH_OPTION_COUNT:-0}
|
||||||
|
case "$push_option_count" in
|
||||||
|
''|*[!0-9]*)
|
||||||
|
echo "git-gate: invalid GIT_PUSH_OPTION_COUNT=$push_option_count" >&2
|
||||||
|
exit 1
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
set --
|
||||||
|
i=0
|
||||||
|
while [ "$i" -lt "$push_option_count" ]; do
|
||||||
|
opt=$(printenv "GIT_PUSH_OPTION_$i" || :)
|
||||||
|
set -- "$@" --push-option="$opt"
|
||||||
|
i=$((i + 1))
|
||||||
|
done
|
||||||
|
|
||||||
while IFS=' ' read -r old new ref; do
|
while IFS=' ' read -r old new ref; do
|
||||||
[ -z "$ref" ] && continue
|
[ -z "$ref" ] && continue
|
||||||
if [ "$new" = "$zero" ]; then
|
if [ "$new" = "$zero" ]; then
|
||||||
@@ -288,7 +304,7 @@ while IFS=' ' read -r old new ref; do
|
|||||||
refspec="$new:$ref"
|
refspec="$new:$ref"
|
||||||
fi
|
fi
|
||||||
echo "git-gate: forwarding $ref to origin" >&2
|
echo "git-gate: forwarding $ref to origin" >&2
|
||||||
if ! GIT_SSH_COMMAND="$ssh_cmd" git push origin "$refspec" 1>&2; then
|
if ! GIT_SSH_COMMAND="$ssh_cmd" git push "$@" origin "$refspec" 1>&2; then
|
||||||
echo "git-gate: upstream push failed for $ref" >&2
|
echo "git-gate: upstream push failed for $ref" >&2
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -19,8 +19,8 @@ from urllib.parse import urlsplit
|
|||||||
|
|
||||||
DEFAULT_PORT = 9420
|
DEFAULT_PORT = 9420
|
||||||
|
|
||||||
# Body-size cap matching supervise_server.py's 1 MiB limit.
|
# Bound memory use while still allowing ordinary git push packfiles.
|
||||||
MAX_BODY_BYTES = 1 * 1024 * 1024
|
MAX_BODY_BYTES = 100 * 1024 * 1024
|
||||||
|
|
||||||
|
|
||||||
class GitHttpHandler(BaseHTTPRequestHandler):
|
class GitHttpHandler(BaseHTTPRequestHandler):
|
||||||
|
|||||||
@@ -13,13 +13,13 @@ Add Content-Length validation and a body-size cap to `git_http_backend.py` so ma
|
|||||||
|
|
||||||
`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.
|
`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.
|
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.
|
||||||
|
|
||||||
## Goals / Success Criteria
|
## Goals / Success Criteria
|
||||||
|
|
||||||
- A missing or non-numeric Content-Length returns HTTP 400.
|
- A missing or non-numeric Content-Length returns HTTP 400.
|
||||||
- A negative 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.
|
- A body larger than the cap (100 MiB) returns HTTP 413.
|
||||||
- Valid Git smart-HTTP pushes and fetches continue to work.
|
- 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.
|
- Unit tests cover: missing length, non-numeric length, negative length, over-cap length, and a valid push/fetch passthrough.
|
||||||
|
|
||||||
@@ -43,12 +43,12 @@ Out of scope:
|
|||||||
|
|
||||||
## Design
|
## 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.
|
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 100 MiB) and return 413 if exceeded. Read exactly `min(content_length, MAX_BODY_BYTES)` bytes.
|
||||||
|
|
||||||
## Testing Strategy
|
## Testing Strategy
|
||||||
|
|
||||||
- Unit tests using `unittest.mock` to drive the handler with crafted headers.
|
- 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.
|
- Test cases: no Content-Length header, `Content-Length: abc`, `Content-Length: -1`, a declared length above `MAX_BODY_BYTES`, and a normal small POST body.
|
||||||
|
|
||||||
Run:
|
Run:
|
||||||
|
|
||||||
|
|||||||
@@ -98,6 +98,9 @@ class TestEntrypointRender(unittest.TestCase):
|
|||||||
# Smart HTTP receive-pack uses the same bare repos and hooks
|
# Smart HTTP receive-pack uses the same bare repos and hooks
|
||||||
# as git-daemon, so repos must opt in to HTTP pushes too.
|
# as git-daemon, so repos must opt in to HTTP pushes too.
|
||||||
self.assertIn("http.receivepack true", script)
|
self.assertIn("http.receivepack true", script)
|
||||||
|
# The gate must advertise push-option support so clients can
|
||||||
|
# pass forge-specific options through to the pre-receive hook.
|
||||||
|
self.assertIn("receive.advertisePushOptions true", script)
|
||||||
# The access-hook is what makes fetch a mirror operation
|
# The access-hook is what makes fetch a mirror operation
|
||||||
# against the upstream (PRD 0008 v1.1).
|
# against the upstream (PRD 0008 v1.1).
|
||||||
self.assertIn("--access-hook=/etc/git-gate/access-hook", script)
|
self.assertIn("--access-hook=/etc/git-gate/access-hook", script)
|
||||||
@@ -153,7 +156,8 @@ class TestHookRender(unittest.TestCase):
|
|||||||
hook = git_gate_render_hook()
|
hook = git_gate_render_hook()
|
||||||
# Phase 1: gitleaks. Phase 2: forward to origin.
|
# Phase 1: gitleaks. Phase 2: forward to origin.
|
||||||
self.assertIn("gitleaks git", hook)
|
self.assertIn("gitleaks git", hook)
|
||||||
self.assertIn("git push origin", hook)
|
self.assertIn("git push", hook)
|
||||||
|
self.assertIn("origin \"$refspec\"", hook)
|
||||||
# KnownHostKey absence is fail-closed.
|
# KnownHostKey absence is fail-closed.
|
||||||
self.assertIn("refusing to push", hook)
|
self.assertIn("refusing to push", hook)
|
||||||
# Stdin is buffered to a tempfile so both phases can re-read.
|
# Stdin is buffered to a tempfile so both phases can re-read.
|
||||||
@@ -177,6 +181,17 @@ class TestHookRender(unittest.TestCase):
|
|||||||
self.assertIn("BatchMode=yes", hook)
|
self.assertIn("BatchMode=yes", hook)
|
||||||
self.assertIn("ConnectTimeout=", hook)
|
self.assertIn("ConnectTimeout=", hook)
|
||||||
|
|
||||||
|
def test_forward_preserves_push_options(self):
|
||||||
|
# Git exposes push options to pre-receive hooks as
|
||||||
|
# GIT_PUSH_OPTION_COUNT + indexed GIT_PUSH_OPTION_N variables.
|
||||||
|
# Forward them as first-class argv entries so spaces and shell
|
||||||
|
# metacharacters inside option values remain data.
|
||||||
|
hook = git_gate_render_hook()
|
||||||
|
self.assertIn("push_option_count=${GIT_PUSH_OPTION_COUNT:-0}", hook)
|
||||||
|
self.assertIn('opt=$(printenv "GIT_PUSH_OPTION_$i" || :)', hook)
|
||||||
|
self.assertIn('set -- "$@" --push-option="$opt"', hook)
|
||||||
|
self.assertIn('git push "$@" origin "$refspec"', hook)
|
||||||
|
|
||||||
|
|
||||||
class TestAccessHookRender(unittest.TestCase):
|
class TestAccessHookRender(unittest.TestCase):
|
||||||
def test_access_hook_refreshes_origin_on_upload_pack(self):
|
def test_access_hook_refreshes_origin_on_upload_pack(self):
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import urllib.request
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
from bot_bottle.git_http_backend import GitHttpHandler
|
from bot_bottle.git_http_backend import GitHttpHandler, MAX_BODY_BYTES
|
||||||
|
|
||||||
|
|
||||||
class TestGitHttpBackend(unittest.TestCase):
|
class TestGitHttpBackend(unittest.TestCase):
|
||||||
@@ -305,9 +305,8 @@ class TestContentLengthBounds(unittest.TestCase):
|
|||||||
self.assertEqual(400, status)
|
self.assertEqual(400, status)
|
||||||
|
|
||||||
def test_oversized_content_length_returns_413(self):
|
def test_oversized_content_length_returns_413(self):
|
||||||
# Declare 2 MiB — over the 1 MiB cap.
|
|
||||||
status = self._post("/repo.git/git-receive-pack",
|
status = self._post("/repo.git/git-receive-pack",
|
||||||
content_length_header=str(2 * 1024 * 1024))
|
content_length_header=str(MAX_BODY_BYTES + 1))
|
||||||
self.assertEqual(413, status)
|
self.assertEqual(413, status)
|
||||||
|
|
||||||
def test_valid_small_body_passes_through(self):
|
def test_valid_small_body_passes_through(self):
|
||||||
|
|||||||
Reference in New Issue
Block a user