diff --git a/python/robomp/src/proxy/server.py b/python/robomp/src/proxy/server.py index e90022f6f..4860aa3e0 100644 --- a/python/robomp/src/proxy/server.py +++ b/python/robomp/src/proxy/server.py @@ -108,6 +108,36 @@ def _require_int(value: Any, field: str) -> int: return value +_SAFE_REF_BODY_RE = re.compile(r"[A-Za-z0-9._/-]+") + + +def _require_fetch_ref(value: Any) -> str: + """Validate the base-branch ref for `/gh/v1/git/fetch_ref`. + + The orchestrator only ever fetches a branch — a bare name (`main`, + `farm/x/y`, `alice/fix-parser`) or `refs/heads/`. Reject anything + `git_ops._branch_refspec` would otherwise pass verbatim into the fetch + refspec: `:` (refspec injection — write arbitrary refs in the shared + pool), a leading `-` (argv option injection), and (via the charset) `*` + `+` `~` `^` `@` `?` `[` `\\`, whitespace, and control bytes; plus the + git-invalid `..` / `//` / leading-or-trailing `/` / trailing `.`|`.lock` + forms. Normal slashy branch names still pass. + """ + ref = _require_str(value, "ref") + body = ref.removeprefix("refs/heads/") + if ( + not body + or ref.startswith("-") + or body.startswith("/") + or body.endswith(("/", ".", ".lock")) + or "//" in body + or ".." in body + or not _SAFE_REF_BODY_RE.fullmatch(body) + ): + raise HTTPException(400, "invalid ref") + return ref + + def _optional_slot_uid(value: Any) -> int | None: if value is None: return None @@ -731,7 +761,7 @@ def create_proxy_app(settings: Settings) -> FastAPI: async def git_fetch_ref_endpoint(request: Request) -> JSONResponse: data = await _json_body(request) repo = _require_str(data.get("repo"), "repo") - ref = _require_str(data.get("ref"), "ref") + ref = _require_fetch_ref(data.get("ref")) target = _pool_dir(settings, repo) remote = await asyncio.to_thread(_origin_remote_auth, target, repo, _resolve_token(settings)) # fetch_ref is intentionally best-effort; never surfaces a 5xx. diff --git a/python/robomp/tests/test_proxy_server.py b/python/robomp/tests/test_proxy_server.py index 26b8df665..36aa94631 100644 --- a/python/robomp/tests/test_proxy_server.py +++ b/python/robomp/tests/test_proxy_server.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import os import platform import subprocess @@ -1203,3 +1204,50 @@ async def test_git_push_rejects_attacker_pushurl(proxy_settings: Settings, upstr ) assert resp.status_code == 400, resp.text assert not _bare_has_branch(upstream_repo, branch) + +# ============================================================================ +# fetch_ref refuses refspec / option injection in `ref` +# ============================================================================ + + +@pytest.mark.parametrize( + "bad_ref", + [ + "refs/heads/x:refs/heads/evil", # `:` -> refspec injection (arbitrary local ref write) + "--upload-pack=env", # leading dash -> argv option injection + "+refs/heads/*:refs/remotes/origin/x", # `+`/`*` wildcard refspec + "refs/heads/*", # wildcard + "a..b", # `..` range token + "ref with space", # whitespace / control + "feature/", # trailing slash + ], +) +async def test_git_fetch_ref_rejects_injection_refs(proxy_settings: Settings, bad_ref: str) -> None: + """`ref` is interpolated into the fetch refspec, so a `:`/leading-dash/ + wildcard ref MUST be refused with 400 — validation runs before any git op, + so no pool needs to exist.""" + app = _build_app(proxy_settings) + body = json.dumps({"repo": "octo/widget", "ref": bad_ref}).encode() + async with await _async_client(app) as client: + resp = await client.post( + "/gh/v1/git/fetch_ref", + content=body, + headers={**_signed("POST", "/gh/v1/git/fetch_ref", body), "Content-Type": "application/json"}, + ) + assert resp.status_code == 400, resp.text + + +async def test_git_fetch_ref_allows_slashy_branch_name(proxy_settings: Settings, upstream_repo: Path) -> None: + """A normal PR-head-style branch (`contrib/fix-parser`) MUST pass validation. + fetch_ref is best-effort, so a clean ref against the staged pool returns 200 + even though that branch isn't on the local upstream.""" + _stage_pool(proxy_settings, upstream_repo) + app = _build_app(proxy_settings) + body = json.dumps({"repo": "octo/widget", "ref": "contrib/fix-parser"}).encode() + async with await _async_client(app) as client: + resp = await client.post( + "/gh/v1/git/fetch_ref", + content=body, + headers={**_signed("POST", "/gh/v1/git/fetch_ref", body), "Content-Type": "application/json"}, + ) + assert resp.status_code == 200, resp.text