Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 36c5b7025b | |||
| 515a95a79d | |||
| 0bace7615a | |||
| c0d3f16519 | |||
| 508c537deb |
@@ -21,6 +21,11 @@ from pathlib import Path
|
|||||||
|
|
||||||
from ...deploy_key_provisioner import DeployKeyCollisionError, DeployKeyProvisioner
|
from ...deploy_key_provisioner import DeployKeyCollisionError, DeployKeyProvisioner
|
||||||
|
|
||||||
|
# Timeout for ssh-keygen and Gitea API HTTP calls. A hung Gitea instance at
|
||||||
|
# prepare time would stall bottle launch indefinitely without this bound.
|
||||||
|
_API_TIMEOUT_SECS = 30
|
||||||
|
_KEYGEN_TIMEOUT_SECS = 10
|
||||||
|
|
||||||
|
|
||||||
class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
|
class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
|
||||||
"""Manages deploy keys on a Gitea instance."""
|
"""Manages deploy keys on a Gitea instance."""
|
||||||
@@ -46,6 +51,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
|
|||||||
check=True,
|
check=True,
|
||||||
stdout=subprocess.DEVNULL,
|
stdout=subprocess.DEVNULL,
|
||||||
stderr=subprocess.DEVNULL,
|
stderr=subprocess.DEVNULL,
|
||||||
|
timeout=_KEYGEN_TIMEOUT_SECS,
|
||||||
)
|
)
|
||||||
private_key = key_path.read_bytes()
|
private_key = key_path.read_bytes()
|
||||||
public_key = key_path.with_suffix(".pub").read_text().strip()
|
public_key = key_path.with_suffix(".pub").read_text().strip()
|
||||||
@@ -67,7 +73,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
|
|||||||
method="POST",
|
method="POST",
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
with urllib.request.urlopen(req) as resp:
|
with urllib.request.urlopen(req, timeout=_API_TIMEOUT_SECS) as resp:
|
||||||
body = json.loads(resp.read())
|
body = json.loads(resp.read())
|
||||||
except urllib.error.HTTPError as exc:
|
except urllib.error.HTTPError as exc:
|
||||||
_body = _read_error_body(exc)
|
_body = _read_error_body(exc)
|
||||||
@@ -98,7 +104,7 @@ class GiteaDeployKeyProvisioner(DeployKeyProvisioner):
|
|||||||
method="DELETE",
|
method="DELETE",
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
with urllib.request.urlopen(req):
|
with urllib.request.urlopen(req, timeout=_API_TIMEOUT_SECS):
|
||||||
pass
|
pass
|
||||||
except urllib.error.HTTPError as exc:
|
except urllib.error.HTTPError as exc:
|
||||||
if exc.code == 404:
|
if exc.code == 404:
|
||||||
|
|||||||
+21
-10
@@ -210,6 +210,17 @@ def egress_token_env_map(
|
|||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
def _yaml_str_escape(s: str) -> str:
|
||||||
|
"""Escape a string for use inside a YAML double-quoted scalar."""
|
||||||
|
return (
|
||||||
|
s.replace("\\", "\\\\")
|
||||||
|
.replace('"', '\\"')
|
||||||
|
.replace("\n", "\\n")
|
||||||
|
.replace("\r", "\\r")
|
||||||
|
.replace("\t", "\\t")
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _route_to_yaml_fields(r: Route) -> dict[str, object]:
|
def _route_to_yaml_fields(r: Route) -> dict[str, object]:
|
||||||
fields: dict[str, object] = {"host": r.host}
|
fields: dict[str, object] = {"host": r.host}
|
||||||
if r.auth_scheme and r.token_env:
|
if r.auth_scheme and r.token_env:
|
||||||
@@ -272,12 +283,12 @@ def _render_match_entry(entry: dict[str, object]) -> list[str]:
|
|||||||
for pd in entry["paths"]: # type: ignore[union-attr]
|
for pd in entry["paths"]: # type: ignore[union-attr]
|
||||||
pd_dict: dict[str, str] = pd # type: ignore[assignment]
|
pd_dict: dict[str, str] = pd # type: ignore[assignment]
|
||||||
if "type" in pd_dict:
|
if "type" in pd_dict:
|
||||||
lines.append(f' - type: "{pd_dict["type"]}"')
|
lines.append(f' - type: "{_yaml_str_escape(pd_dict["type"])}"')
|
||||||
lines.append(f' value: "{pd_dict["value"]}"')
|
lines.append(f' value: "{_yaml_str_escape(pd_dict["value"])}"')
|
||||||
else:
|
else:
|
||||||
lines.append(f' - value: "{pd_dict["value"]}"')
|
lines.append(f' - value: "{_yaml_str_escape(pd_dict["value"])}"')
|
||||||
if "methods" in entry:
|
if "methods" in entry:
|
||||||
methods_str = ", ".join(f'"{m}"' for m in entry["methods"]) # type: ignore[union-attr]
|
methods_str = ", ".join(f'"{_yaml_str_escape(m)}"' for m in entry["methods"]) # type: ignore[union-attr]
|
||||||
prefix = " - " if first_key else " "
|
prefix = " - " if first_key else " "
|
||||||
lines.append(f'{prefix}methods: [{methods_str}]')
|
lines.append(f'{prefix}methods: [{methods_str}]')
|
||||||
first_key = False
|
first_key = False
|
||||||
@@ -287,8 +298,8 @@ def _render_match_entry(entry: dict[str, object]) -> list[str]:
|
|||||||
first_key = False
|
first_key = False
|
||||||
for hd in entry["headers"]: # type: ignore[union-attr]
|
for hd in entry["headers"]: # type: ignore[union-attr]
|
||||||
hd_dict: dict[str, str] = hd # type: ignore[assignment]
|
hd_dict: dict[str, str] = hd # type: ignore[assignment]
|
||||||
lines.append(f' - name: "{hd_dict["name"]}"')
|
lines.append(f' - name: "{_yaml_str_escape(hd_dict["name"])}"')
|
||||||
lines.append(f' value: "{hd_dict["value"]}"')
|
lines.append(f' value: "{_yaml_str_escape(hd_dict["value"])}"')
|
||||||
if first_key:
|
if first_key:
|
||||||
lines.append(" - {}")
|
lines.append(" - {}")
|
||||||
return lines
|
return lines
|
||||||
@@ -308,10 +319,10 @@ def egress_render_routes(
|
|||||||
return "\n".join(lines) + "\n"
|
return "\n".join(lines) + "\n"
|
||||||
for r in routes:
|
for r in routes:
|
||||||
f = _route_to_yaml_fields(r)
|
f = _route_to_yaml_fields(r)
|
||||||
lines.append(f' - host: "{f["host"]}"')
|
lines.append(f' - host: "{_yaml_str_escape(str(f["host"]))}"')
|
||||||
if "auth_scheme" in f:
|
if "auth_scheme" in f:
|
||||||
lines.append(f' auth_scheme: "{f["auth_scheme"]}"')
|
lines.append(f' auth_scheme: "{_yaml_str_escape(str(f["auth_scheme"]))}"')
|
||||||
lines.append(f' token_env: "{f["token_env"]}"')
|
lines.append(f' token_env: "{_yaml_str_escape(str(f["token_env"]))}"')
|
||||||
if "matches" in f:
|
if "matches" in f:
|
||||||
lines.append(" matches:")
|
lines.append(" matches:")
|
||||||
for entry in f["matches"]: # type: ignore[union-attr]
|
for entry in f["matches"]: # type: ignore[union-attr]
|
||||||
@@ -331,7 +342,7 @@ def egress_render_routes(
|
|||||||
items_str = ", ".join(f'"{x}"' for x in dv)
|
items_str = ", ".join(f'"{x}"' for x in dv)
|
||||||
lines.append(f" {dk}: [{items_str}]")
|
lines.append(f" {dk}: [{items_str}]")
|
||||||
elif isinstance(dv, str):
|
elif isinstance(dv, str):
|
||||||
lines.append(f' {dk}: "{dv}"')
|
lines.append(f' {dk}: "{_yaml_str_escape(dv)}"')
|
||||||
return "\n".join(lines) + "\n"
|
return "\n".join(lines) + "\n"
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+17
-6
@@ -43,10 +43,10 @@ from .manifest import ManifestBottle, ManifestGitEntry
|
|||||||
# Short network alias for git-gate inside the sidecar bundle. The
|
# Short network alias for git-gate inside the sidecar bundle. The
|
||||||
# agent's `.gitconfig` insteadOf rewrites resolve through this name.
|
# agent's `.gitconfig` insteadOf rewrites resolve through this name.
|
||||||
GIT_GATE_HOSTNAME = "git-gate"
|
GIT_GATE_HOSTNAME = "git-gate"
|
||||||
# Bound half-open git client sessions. If an agent/tool runner is
|
# Shared timeout (seconds) for all git-gate subprocess and CGI calls:
|
||||||
# interrupted during push, git daemon should reap the receive-pack
|
# git daemon (--timeout/--init-timeout), the access-hook subprocess in
|
||||||
# child instead of keeping the gate wedged indefinitely.
|
# git_http_backend, and the git http-backend CGI subprocess.
|
||||||
GIT_GATE_DAEMON_TIMEOUT_SECS = 15
|
GIT_GATE_TIMEOUT_SECS = 15
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
@@ -112,6 +112,15 @@ def git_gate_upstreams_for_bottle(bottle: ManifestBottle) -> tuple[GitGateUpstre
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _gitconfig_validate_value(field: str, value: str) -> None:
|
||||||
|
"""Raise ValueError if value contains characters that break gitconfig line syntax."""
|
||||||
|
if "\n" in value or "\r" in value:
|
||||||
|
raise ValueError(
|
||||||
|
f"git-gate: {field} contains a newline, which would inject "
|
||||||
|
f"arbitrary gitconfig keys; rejecting manifest entry"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def git_gate_render_gitconfig(
|
def git_gate_render_gitconfig(
|
||||||
entries: tuple[ManifestGitEntry, ...], gate_host: str, *, scheme: str = "git",
|
entries: tuple[ManifestGitEntry, ...], gate_host: str, *, scheme: str = "git",
|
||||||
) -> str:
|
) -> str:
|
||||||
@@ -136,6 +145,7 @@ def git_gate_render_gitconfig(
|
|||||||
"# fetch-from-upstream-before-every-upload-pack via access-hook).\n",
|
"# fetch-from-upstream-before-every-upload-pack via access-hook).\n",
|
||||||
]
|
]
|
||||||
for entry in entries:
|
for entry in entries:
|
||||||
|
_gitconfig_validate_value(f"repos[{entry.Name!r}].url", entry.Upstream)
|
||||||
out.append(f'[url "{scheme}://{gate_host}/{entry.Name}.git"]\n')
|
out.append(f'[url "{scheme}://{gate_host}/{entry.Name}.git"]\n')
|
||||||
out.append(f"\tinsteadOf = {entry.Upstream}\n")
|
out.append(f"\tinsteadOf = {entry.Upstream}\n")
|
||||||
if entry.RemoteKey and entry.RemoteKey != entry.UpstreamHost:
|
if entry.RemoteKey and entry.RemoteKey != entry.UpstreamHost:
|
||||||
@@ -148,6 +158,7 @@ def git_gate_render_gitconfig(
|
|||||||
f"ssh://{entry.UpstreamUser}@{entry.RemoteKey}{port}/"
|
f"ssh://{entry.UpstreamUser}@{entry.RemoteKey}{port}/"
|
||||||
f"{entry.UpstreamPath}"
|
f"{entry.UpstreamPath}"
|
||||||
)
|
)
|
||||||
|
_gitconfig_validate_value(f"repos[{entry.Name!r}].url (resolved alias)", alias)
|
||||||
out.append(f"\tinsteadOf = {alias}\n")
|
out.append(f"\tinsteadOf = {alias}\n")
|
||||||
return "".join(out)
|
return "".join(out)
|
||||||
|
|
||||||
@@ -217,8 +228,8 @@ def git_gate_render_entrypoint(upstreams: tuple[GitGateUpstream, ...]) -> str:
|
|||||||
"",
|
"",
|
||||||
"exec git daemon \\",
|
"exec git daemon \\",
|
||||||
" --reuseaddr \\",
|
" --reuseaddr \\",
|
||||||
f" --timeout={GIT_GATE_DAEMON_TIMEOUT_SECS} \\",
|
f" --timeout={GIT_GATE_TIMEOUT_SECS} \\",
|
||||||
f" --init-timeout={GIT_GATE_DAEMON_TIMEOUT_SECS} \\",
|
f" --init-timeout={GIT_GATE_TIMEOUT_SECS} \\",
|
||||||
" --base-path=/git \\",
|
" --base-path=/git \\",
|
||||||
" --export-all \\",
|
" --export-all \\",
|
||||||
" --enable=receive-pack \\",
|
" --enable=receive-pack \\",
|
||||||
|
|||||||
@@ -16,6 +16,8 @@ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from urllib.parse import urlsplit
|
from urllib.parse import urlsplit
|
||||||
|
|
||||||
|
from .git_gate import GIT_GATE_TIMEOUT_SECS
|
||||||
|
|
||||||
|
|
||||||
DEFAULT_PORT = 9420
|
DEFAULT_PORT = 9420
|
||||||
|
|
||||||
@@ -47,6 +49,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
|
|||||||
[hook_path, "upload-pack", str(repo_dir), peer, peer],
|
[hook_path, "upload-pack", str(repo_dir), peer, peer],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
check=False,
|
check=False,
|
||||||
|
timeout=GIT_GATE_TIMEOUT_SECS,
|
||||||
)
|
)
|
||||||
if hook.returncode != 0:
|
if hook.returncode != 0:
|
||||||
detail = (hook.stderr or hook.stdout).decode(
|
detail = (hook.stderr or hook.stdout).decode(
|
||||||
@@ -110,6 +113,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
|
|||||||
env=env,
|
env=env,
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
check=False,
|
check=False,
|
||||||
|
timeout=GIT_GATE_TIMEOUT_SECS,
|
||||||
)
|
)
|
||||||
self._write_cgi_response(proc.stdout)
|
self._write_cgi_response(proc.stdout)
|
||||||
|
|
||||||
|
|||||||
@@ -10,6 +10,8 @@ from unittest.mock import MagicMock, patch
|
|||||||
|
|
||||||
from bot_bottle.contrib.gitea.deploy_key_provisioner import (
|
from bot_bottle.contrib.gitea.deploy_key_provisioner import (
|
||||||
GiteaDeployKeyProvisioner,
|
GiteaDeployKeyProvisioner,
|
||||||
|
_API_TIMEOUT_SECS,
|
||||||
|
_KEYGEN_TIMEOUT_SECS,
|
||||||
_split_owner_repo,
|
_split_owner_repo,
|
||||||
)
|
)
|
||||||
from bot_bottle.deploy_key_provisioner import DeployKeyCollisionError
|
from bot_bottle.deploy_key_provisioner import DeployKeyCollisionError
|
||||||
@@ -83,6 +85,25 @@ class TestCreate(unittest.TestCase):
|
|||||||
self.assertEqual(str(fake_key_id), key_id)
|
self.assertEqual(str(fake_key_id), key_id)
|
||||||
self.assertEqual(fake_private, private_bytes)
|
self.assertEqual(fake_private, private_bytes)
|
||||||
|
|
||||||
|
def test_create_passes_timeout_to_ssh_keygen_and_urlopen(self):
|
||||||
|
provisioner = _provisioner()
|
||||||
|
with patch(
|
||||||
|
"bot_bottle.contrib.gitea.deploy_key_provisioner.subprocess.run"
|
||||||
|
) as mock_run, patch(
|
||||||
|
"bot_bottle.contrib.gitea.deploy_key_provisioner.urllib.request.urlopen"
|
||||||
|
) as mock_urlopen, patch(
|
||||||
|
"bot_bottle.contrib.gitea.deploy_key_provisioner.Path.read_bytes",
|
||||||
|
return_value=b"PRIVATE",
|
||||||
|
), patch(
|
||||||
|
"bot_bottle.contrib.gitea.deploy_key_provisioner.Path.read_text",
|
||||||
|
return_value="ssh-ed25519 AAAA\n",
|
||||||
|
):
|
||||||
|
mock_urlopen.return_value = _urlopen_response({"id": 1})
|
||||||
|
provisioner.create("owner/repo", "title")
|
||||||
|
|
||||||
|
self.assertEqual(_KEYGEN_TIMEOUT_SECS, mock_run.call_args.kwargs.get("timeout"))
|
||||||
|
self.assertEqual(_API_TIMEOUT_SECS, mock_urlopen.call_args.kwargs.get("timeout"))
|
||||||
|
|
||||||
def test_create_raises_on_http_error(self):
|
def test_create_raises_on_http_error(self):
|
||||||
provisioner = _provisioner()
|
provisioner = _provisioner()
|
||||||
with patch(
|
with patch(
|
||||||
@@ -139,6 +160,16 @@ class TestDelete(unittest.TestCase):
|
|||||||
self.assertIn("/api/v1/repos/didericis/bot-bottle/keys/99", req.full_url)
|
self.assertIn("/api/v1/repos/didericis/bot-bottle/keys/99", req.full_url)
|
||||||
self.assertEqual("DELETE", req.get_method())
|
self.assertEqual("DELETE", req.get_method())
|
||||||
|
|
||||||
|
def test_delete_passes_timeout_to_urlopen(self):
|
||||||
|
provisioner = _provisioner()
|
||||||
|
with patch(
|
||||||
|
"bot_bottle.contrib.gitea.deploy_key_provisioner.urllib.request.urlopen"
|
||||||
|
) as mock_urlopen:
|
||||||
|
mock_urlopen.return_value = _urlopen_response({})
|
||||||
|
provisioner.delete("owner/repo", "7")
|
||||||
|
|
||||||
|
self.assertEqual(_API_TIMEOUT_SECS, mock_urlopen.call_args.kwargs.get("timeout"))
|
||||||
|
|
||||||
def test_delete_tolerates_404(self):
|
def test_delete_tolerates_404(self):
|
||||||
provisioner = _provisioner()
|
provisioner = _provisioner()
|
||||||
with patch(
|
with patch(
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ from bot_bottle.egress import (
|
|||||||
Egress,
|
Egress,
|
||||||
EgressPlan,
|
EgressPlan,
|
||||||
EgressRoute,
|
EgressRoute,
|
||||||
|
_yaml_str_escape,
|
||||||
egress_agent_env_entries,
|
egress_agent_env_entries,
|
||||||
egress_manifest_routes,
|
egress_manifest_routes,
|
||||||
egress_render_routes,
|
egress_render_routes,
|
||||||
@@ -419,6 +420,76 @@ class TestRenderRoutes(unittest.TestCase):
|
|||||||
self.assertEqual(LOG_BLOCKS, cfg.log)
|
self.assertEqual(LOG_BLOCKS, cfg.log)
|
||||||
|
|
||||||
|
|
||||||
|
class TestYamlStrEscape(unittest.TestCase):
|
||||||
|
"""_yaml_str_escape produces safe YAML double-quoted scalar content."""
|
||||||
|
|
||||||
|
def test_plain_string_unchanged(self):
|
||||||
|
self.assertEqual("api.example.com", _yaml_str_escape("api.example.com"))
|
||||||
|
|
||||||
|
def test_double_quote_escaped(self):
|
||||||
|
self.assertEqual('\\"', _yaml_str_escape('"'))
|
||||||
|
|
||||||
|
def test_backslash_escaped(self):
|
||||||
|
self.assertEqual("\\\\", _yaml_str_escape("\\"))
|
||||||
|
|
||||||
|
def test_newline_escaped(self):
|
||||||
|
self.assertEqual("\\n", _yaml_str_escape("\n"))
|
||||||
|
|
||||||
|
def test_carriage_return_escaped(self):
|
||||||
|
self.assertEqual("\\r", _yaml_str_escape("\r"))
|
||||||
|
|
||||||
|
def test_tab_escaped(self):
|
||||||
|
self.assertEqual("\\t", _yaml_str_escape("\t"))
|
||||||
|
|
||||||
|
def test_combined(self):
|
||||||
|
self.assertEqual('\\"\\n\\\\', _yaml_str_escape('"\n\\'))
|
||||||
|
|
||||||
|
|
||||||
|
class TestRenderRoutesEscaping(unittest.TestCase):
|
||||||
|
"""Stray quotes/newlines in manifest strings do not corrupt routes.yaml."""
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _parsed(routes) -> list[dict]: # type: ignore
|
||||||
|
return parse_yaml_subset(egress_render_routes(routes))["routes"] # type: ignore
|
||||||
|
|
||||||
|
def test_host_with_double_quote_round_trips(self):
|
||||||
|
routes = (EgressRoute(host='bad"host.example'),)
|
||||||
|
parsed = self._parsed(routes)
|
||||||
|
self.assertEqual('bad"host.example', parsed[0]["host"])
|
||||||
|
|
||||||
|
def test_host_with_newline_round_trips(self):
|
||||||
|
routes = (EgressRoute(host="host\nextra.example"),)
|
||||||
|
parsed = self._parsed(routes)
|
||||||
|
self.assertEqual("host\nextra.example", parsed[0]["host"])
|
||||||
|
|
||||||
|
def test_auth_scheme_with_double_quote_round_trips(self):
|
||||||
|
routes = (EgressRoute(
|
||||||
|
host="api.example",
|
||||||
|
auth_scheme='Bear"er',
|
||||||
|
token_env="EGRESS_TOKEN_0",
|
||||||
|
),)
|
||||||
|
parsed = self._parsed(routes)
|
||||||
|
self.assertEqual('Bear"er', parsed[0]["auth_scheme"])
|
||||||
|
|
||||||
|
def test_path_value_with_double_quote_round_trips(self):
|
||||||
|
from bot_bottle.egress_addon_core import PathMatch, MatchEntry
|
||||||
|
routes = (EgressRoute(
|
||||||
|
host="api.example",
|
||||||
|
matches=(MatchEntry(paths=(PathMatch(type="prefix", value='/v1/"quoted"/'),)),),
|
||||||
|
),)
|
||||||
|
parsed = self._parsed(routes)
|
||||||
|
self.assertEqual('/v1/"quoted"/', parsed[0]["matches"][0]["paths"][0]["value"])
|
||||||
|
|
||||||
|
def test_header_value_with_double_quote_round_trips(self):
|
||||||
|
from bot_bottle.egress_addon_core import HeaderMatch, MatchEntry
|
||||||
|
routes = (EgressRoute(
|
||||||
|
host="api.example",
|
||||||
|
matches=(MatchEntry(headers=(HeaderMatch(name="x-h", value='val"ue'),)),),
|
||||||
|
),)
|
||||||
|
parsed = self._parsed(routes)
|
||||||
|
self.assertEqual('val"ue', parsed[0]["matches"][0]["headers"][0]["value"])
|
||||||
|
|
||||||
|
|
||||||
class TestResolveTokenValues(unittest.TestCase):
|
class TestResolveTokenValues(unittest.TestCase):
|
||||||
def test_reads_host_env(self):
|
def test_reads_host_env(self):
|
||||||
out = egress_resolve_token_values(
|
out = egress_resolve_token_values(
|
||||||
|
|||||||
@@ -9,6 +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_gate import GIT_GATE_TIMEOUT_SECS
|
||||||
from bot_bottle.git_http_backend import GitHttpHandler, MAX_BODY_BYTES
|
from bot_bottle.git_http_backend import GitHttpHandler, MAX_BODY_BYTES
|
||||||
|
|
||||||
|
|
||||||
@@ -150,6 +151,61 @@ class TestGitHttpBackend(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertEqual("git/test", env["HTTP_USER_AGENT"])
|
self.assertEqual("git/test", env["HTTP_USER_AGENT"])
|
||||||
|
|
||||||
|
def test_subprocess_calls_include_timeout(self):
|
||||||
|
"""Both subprocess.run calls (access-hook and git http-backend) must
|
||||||
|
pass timeout= so a hung upstream cannot wedge the sidecar."""
|
||||||
|
from http.server import ThreadingHTTPServer
|
||||||
|
|
||||||
|
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)
|
||||||
|
old_hook = os.environ.get("GIT_GATE_ACCESS_HOOK")
|
||||||
|
hook = root / "access-hook"
|
||||||
|
hook.write_text("#!/bin/sh\nexit 0\n")
|
||||||
|
hook.chmod(0o700)
|
||||||
|
os.environ["GIT_GATE_ACCESS_HOOK"] = str(hook)
|
||||||
|
self.addCleanup(self._restore_hook, old_hook)
|
||||||
|
|
||||||
|
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)
|
||||||
|
|
||||||
|
backend_response = (
|
||||||
|
b"Status: 200 OK\r\n"
|
||||||
|
b"Content-Type: application/x-git-upload-pack-result\r\n"
|
||||||
|
b"\r\n"
|
||||||
|
b"0000"
|
||||||
|
)
|
||||||
|
calls = [
|
||||||
|
subprocess.CompletedProcess(["hook"], 0, b"", b""),
|
||||||
|
subprocess.CompletedProcess(["git"], 0, backend_response, b""),
|
||||||
|
]
|
||||||
|
with mock.patch(
|
||||||
|
"bot_bottle.git_http_backend.subprocess.run",
|
||||||
|
side_effect=calls,
|
||||||
|
) as run:
|
||||||
|
req = urllib.request.Request(
|
||||||
|
f"http://127.0.0.1:{server.server_port}"
|
||||||
|
"/repo.git/git-upload-pack",
|
||||||
|
data=b"",
|
||||||
|
method="POST",
|
||||||
|
)
|
||||||
|
with urllib.request.urlopen(req, timeout=5):
|
||||||
|
pass
|
||||||
|
|
||||||
|
for call in run.call_args_list:
|
||||||
|
self.assertEqual(
|
||||||
|
GIT_GATE_TIMEOUT_SECS,
|
||||||
|
call.kwargs.get("timeout"),
|
||||||
|
f"subprocess.run call missing timeout: {call}",
|
||||||
|
)
|
||||||
|
|
||||||
def test_access_hook_denial_is_logged_to_stdout(self):
|
def test_access_hook_denial_is_logged_to_stdout(self):
|
||||||
"""When the access-hook exits non-zero we still return 403 to the
|
"""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
|
client, but the hook's stderr must also appear on the handler's
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import unittest
|
|||||||
|
|
||||||
from bot_bottle.git_gate import (
|
from bot_bottle.git_gate import (
|
||||||
GIT_GATE_HOSTNAME,
|
GIT_GATE_HOSTNAME,
|
||||||
|
_gitconfig_validate_value,
|
||||||
git_gate_render_gitconfig,
|
git_gate_render_gitconfig,
|
||||||
)
|
)
|
||||||
from bot_bottle.manifest import ManifestIndex
|
from bot_bottle.manifest import ManifestIndex
|
||||||
@@ -90,5 +91,42 @@ class TestGitGateGitconfigRender(unittest.TestCase):
|
|||||||
self.assertNotIn("gitea.dideric.is", out)
|
self.assertNotIn("gitea.dideric.is", out)
|
||||||
|
|
||||||
|
|
||||||
|
class TestGitconfigValidateValue(unittest.TestCase):
|
||||||
|
"""_gitconfig_validate_value rejects values that would inject gitconfig keys."""
|
||||||
|
|
||||||
|
def test_normal_url_passes(self):
|
||||||
|
_gitconfig_validate_value("url", "ssh://git@github.com/owner/repo.git")
|
||||||
|
|
||||||
|
def test_newline_in_url_raises(self):
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
_gitconfig_validate_value("url", "ssh://git@github.com/owner/\nrepo.git")
|
||||||
|
|
||||||
|
def test_carriage_return_in_url_raises(self):
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
_gitconfig_validate_value("url", "ssh://git@github.com/\rrepo.git")
|
||||||
|
|
||||||
|
def test_error_message_names_field(self):
|
||||||
|
with self.assertRaises(ValueError, msg="error should name the field") as ctx:
|
||||||
|
_gitconfig_validate_value("repos['bad'].url", "ssh://host/\npath")
|
||||||
|
self.assertIn("repos['bad'].url", str(ctx.exception))
|
||||||
|
|
||||||
|
|
||||||
|
class TestGitconfigRenderRejectsNewlineInUpstream(unittest.TestCase):
|
||||||
|
"""git_gate_render_gitconfig raises on Upstream values with newlines."""
|
||||||
|
|
||||||
|
def test_newline_in_upstream_raises(self):
|
||||||
|
m = ManifestIndex.from_json_obj({
|
||||||
|
"bottles": {"dev": {"git-gate": {"repos": {
|
||||||
|
"evil": {
|
||||||
|
"url": "ssh://git@github.com/owner/\nfake-key = injected\nrepo.git",
|
||||||
|
"key": {"provider": "static", "path": "/dev/null"},
|
||||||
|
},
|
||||||
|
}}}},
|
||||||
|
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
|
||||||
|
})
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
git_gate_render_gitconfig(m.bottles["dev"].git, GIT_GATE_HOSTNAME)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user