Harden git_gate.py shell rendering #166
@@ -29,6 +29,7 @@ backend-specific and lives on concrete subclasses (see
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import shlex
|
||||
from abc import ABC, abstractmethod
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
@@ -207,10 +208,7 @@ def git_gate_render_entrypoint(upstreams: tuple[GitGateUpstream, ...]) -> str:
|
||||
"mkdir -p /git",
|
||||
]
|
||||
for u in upstreams:
|
||||
# Single-quote args so URL/path content (containing : and /)
|
||||
# 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.append(f"init_repo {shlex.quote(u.name)} {shlex.quote(u.upstream_url)}")
|
||||
lines.extend([
|
||||
"",
|
||||
"exec git daemon \\",
|
||||
|
||||
@@ -2,10 +2,16 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from dataclasses import dataclass
|
||||
|
||||
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:
|
||||
if value is None:
|
||||
@@ -94,6 +100,11 @@ class GitEntry:
|
||||
raise ManifestError(
|
||||
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}]"
|
||||
d = as_json_object(raw, f"bottle '{bottle_name}' {label}")
|
||||
for k in d:
|
||||
|
||||
@@ -76,14 +76,18 @@ class TestEntrypointRender(unittest.TestCase):
|
||||
)
|
||||
script = git_gate_render_entrypoint(ups)
|
||||
self.assertIn("#!/bin/sh", script)
|
||||
self.assertIn(
|
||||
"init_repo 'bot-bottle' "
|
||||
"'ssh://git@gitea.dideric.is:30009/didericis/bot-bottle.git'",
|
||||
script,
|
||||
# shlex.quote leaves safe strings unquoted; verify via token parse.
|
||||
import shlex as _shlex
|
||||
lines_with_init = [l for l in script.splitlines() if l.startswith("init_repo ")]
|
||||
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(
|
||||
"init_repo 'foo' 'ssh://git@github.com/didericis/foo.git'",
|
||||
script,
|
||||
self.assertEqual(
|
||||
["init_repo", "foo", "ssh://git@github.com/didericis/foo.git"],
|
||||
_shlex.split(lines_with_init[1]),
|
||||
)
|
||||
# Daemon line is what keeps PID 1 alive.
|
||||
self.assertIn("exec git daemon", script)
|
||||
@@ -108,6 +112,41 @@ class TestEntrypointRender(unittest.TestCase):
|
||||
self.assertNotIn("init_repo '", 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):
|
||||
def test_pre_receive_hook_has_two_phases(self):
|
||||
|
||||
@@ -185,6 +185,51 @@ class TestGitEntryCrossValidation(unittest.TestCase):
|
||||
"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):
|
||||
msg = ""
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user