Harden git_gate.py shell rendering #166

Merged
didericis merged 1 commits from fix/shell-injection-git-gate into main 2026-06-03 00:47:34 -04:00
4 changed files with 104 additions and 11 deletions
Showing only changes of commit cc0c952d0b - Show all commits
+2 -4
View File
@@ -29,6 +29,7 @@ backend-specific and lives on concrete subclasses (see
from __future__ import annotations from __future__ import annotations
import shlex
from abc import ABC, abstractmethod from abc import ABC, abstractmethod
from dataclasses import dataclass from dataclasses import dataclass
from pathlib import Path from pathlib import Path
@@ -207,10 +208,7 @@ def git_gate_render_entrypoint(upstreams: tuple[GitGateUpstream, ...]) -> str:
"mkdir -p /git", "mkdir -p /git",
] ]
for u in upstreams: for u in upstreams:
# Single-quote args so URL/path content (containing : and /) lines.append(f"init_repo {shlex.quote(u.name)} {shlex.quote(u.upstream_url)}")
# passes through ash unmangled. Names came through the manifest
# validator so they don't contain a single quote.
lines.append(f"init_repo '{u.name}' '{u.upstream_url}'")
lines.extend([ lines.extend([
"", "",
"exec git daemon \\", "exec git daemon \\",
+11
View File
@@ -2,10 +2,16 @@
from __future__ import annotations from __future__ import annotations
import re
from dataclasses import dataclass from dataclasses import dataclass
from .manifest_util import ManifestError, as_json_object from .manifest_util import ManifestError, as_json_object
# Shell-safe characters for git-gate repo names. Names are embedded in
# the generated entrypoint shell script (shlex.quote is the primary
# defence; this regex is belt-and-suspenders and documents intent).
_GIT_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$")
def _opt_str(value: object, label: str) -> str: def _opt_str(value: object, label: str) -> str:
if value is None: if value is None:
@@ -94,6 +100,11 @@ class GitEntry:
raise ManifestError( raise ManifestError(
f"bottle '{bottle_name}' git-gate.repos has an empty key" f"bottle '{bottle_name}' git-gate.repos has an empty key"
) )
if not _GIT_NAME_RE.match(repo_name):
raise ManifestError(
f"bottle '{bottle_name}' git-gate.repos name {repo_name!r} is invalid; "
f"allowed characters: A-Z a-z 0-9 . _ -"
)
label = f"git-gate.repos[{repo_name!r}]" label = f"git-gate.repos[{repo_name!r}]"
d = as_json_object(raw, f"bottle '{bottle_name}' {label}") d = as_json_object(raw, f"bottle '{bottle_name}' {label}")
for k in d: for k in d:
+46 -7
View File
@@ -76,14 +76,18 @@ class TestEntrypointRender(unittest.TestCase):
) )
script = git_gate_render_entrypoint(ups) script = git_gate_render_entrypoint(ups)
self.assertIn("#!/bin/sh", script) self.assertIn("#!/bin/sh", script)
self.assertIn( # shlex.quote leaves safe strings unquoted; verify via token parse.
"init_repo 'bot-bottle' " import shlex as _shlex
"'ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git'", lines_with_init = [l for l in script.splitlines() if l.startswith("init_repo ")]
script, self.assertEqual(2, len(lines_with_init))
self.assertEqual(
["init_repo", "bot-bottle",
"ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git"],
_shlex.split(lines_with_init[0]),
) )
self.assertIn( self.assertEqual(
"init_repo 'foo' 'ssh://git@github.com/didericis/foo.git'", ["init_repo", "foo", "ssh://git@github.com/didericis/foo.git"],
script, _shlex.split(lines_with_init[1]),
) )
# Daemon line is what keeps PID 1 alive. # Daemon line is what keeps PID 1 alive.
self.assertIn("exec git daemon", script) self.assertIn("exec git daemon", script)
@@ -108,6 +112,41 @@ class TestEntrypointRender(unittest.TestCase):
self.assertNotIn("init_repo '", script) self.assertNotIn("init_repo '", script)
self.assertIn("exec git daemon", script) self.assertIn("exec git daemon", script)
def test_single_quote_in_upstream_url_is_escaped(self):
ups = (GitGateUpstream(
name="myrepo",
upstream_url="ssh://git@host/path'with'quotes.git",
upstream_host="host",
upstream_port="22",
identity_file="/key",
known_host_key="",
),)
script = git_gate_render_entrypoint(ups)
self.assertNotIn(
"init_repo 'myrepo' 'ssh://git@host/path'with'quotes.git'",
script,
)
self.assertIn("init_repo", script)
self.assertIn("path", script)
def test_space_and_semicolon_in_upstream_url_are_escaped(self):
import shlex as _shlex
raw_url = "ssh://git@host/path with spaces;evil.git"
ups = (GitGateUpstream(
name="myrepo",
upstream_url=raw_url,
upstream_host="host",
upstream_port="22",
identity_file="/key",
known_host_key="",
),)
script = git_gate_render_entrypoint(ups)
line = next(l for l in script.splitlines() if l.startswith("init_repo "))
tokens = _shlex.split(line)
self.assertEqual(3, len(tokens))
self.assertEqual("myrepo", tokens[1])
self.assertEqual(raw_url, tokens[2])
class TestHookRender(unittest.TestCase): class TestHookRender(unittest.TestCase):
def test_pre_receive_hook_has_two_phases(self): def test_pre_receive_hook_has_two_phases(self):
+45
View File
@@ -185,6 +185,51 @@ class TestGitEntryCrossValidation(unittest.TestCase):
"agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}},
}) })
def test_name_with_single_quote_dies(self):
with self.assertRaises(ManifestError):
Manifest.from_json_obj(_manifest({
"o'reilly": {
"url": "ssh://git@github.com/foo.git",
"identity": "/dev/null",
},
}))
def test_name_with_space_dies(self):
with self.assertRaises(ManifestError):
Manifest.from_json_obj(_manifest({
"my repo": {
"url": "ssh://git@github.com/foo.git",
"identity": "/dev/null",
},
}))
def test_name_with_semicolon_dies(self):
with self.assertRaises(ManifestError):
Manifest.from_json_obj(_manifest({
"foo;bar": {
"url": "ssh://git@github.com/foo.git",
"identity": "/dev/null",
},
}))
def test_name_with_dollar_dies(self):
with self.assertRaises(ManifestError):
Manifest.from_json_obj(_manifest({
"foo$bar": {
"url": "ssh://git@github.com/foo.git",
"identity": "/dev/null",
},
}))
def test_valid_name_with_dots_and_hyphens_accepted(self):
m = Manifest.from_json_obj(_manifest({
"my.repo-name_1": {
"url": "ssh://git@github.com/foo.git",
"identity": "/dev/null",
},
}))
self.assertEqual("my.repo-name_1", m.bottles["dev"].git[0].Name)
def test_legacy_git_key_dies_with_hint(self): def test_legacy_git_key_dies_with_hint(self):
msg = "" msg = ""
try: try: