Compare commits
9 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| bffd5043dc | |||
| 76a4d9142f | |||
| a7fb92acd8 | |||
| 515a95a79d | |||
| 0bace7615a | |||
| c0d3f16519 | |||
| 508c537deb | |||
| d99dba037c | |||
| 9a878bd885 |
@@ -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)
|
||||||
|
|
||||||
@@ -148,7 +152,13 @@ class GitHttpHandler(BaseHTTPRequestHandler):
|
|||||||
key, _, value = line.decode("latin1").partition(":")
|
key, _, value = line.decode("latin1").partition(":")
|
||||||
value = value.strip()
|
value = value.strip()
|
||||||
if key.lower() == "status":
|
if key.lower() == "status":
|
||||||
|
try:
|
||||||
status = int(value.split()[0])
|
status = int(value.split()[0])
|
||||||
|
except (ValueError, IndexError):
|
||||||
|
self.log_message(
|
||||||
|
"malformed CGI Status header %r; using 500", value,
|
||||||
|
)
|
||||||
|
status = 500
|
||||||
else:
|
else:
|
||||||
headers.append((key, value))
|
headers.append((key, value))
|
||||||
self.send_response(status)
|
self.send_response(status)
|
||||||
|
|||||||
+104
-12
@@ -49,33 +49,125 @@ def _resolve_one_bottle(
|
|||||||
repos_cache[name] = _resolve_repos_raw({}, child_raw)
|
repos_cache[name] = _resolve_repos_raw({}, child_raw)
|
||||||
return bottle
|
return bottle
|
||||||
|
|
||||||
if not isinstance(parent_name_raw, str):
|
# Normalize to list, accepting both str and list[str].
|
||||||
|
raw_list: list[object]
|
||||||
|
if isinstance(parent_name_raw, str):
|
||||||
|
raw_list = [parent_name_raw]
|
||||||
|
elif isinstance(parent_name_raw, list):
|
||||||
|
raw_list = parent_name_raw
|
||||||
|
else:
|
||||||
raise ManifestError(
|
raise ManifestError(
|
||||||
f"bottle '{name}' extends must be a string "
|
f"bottle '{name}' extends must be a string or list of strings "
|
||||||
f"(was {type(parent_name_raw).__name__})"
|
f"(was {type(parent_name_raw).__name__})"
|
||||||
)
|
)
|
||||||
parent_name: str = parent_name_raw
|
|
||||||
if parent_name == name:
|
# Validate each entry before resolving any of them.
|
||||||
|
parent_names: list[str] = []
|
||||||
|
for i, pname in enumerate(raw_list):
|
||||||
|
if not isinstance(pname, str):
|
||||||
raise ManifestError(
|
raise ManifestError(
|
||||||
f"bottle '{name}' extends itself; remove the "
|
f"bottle '{name}' extends[{i}] must be a string "
|
||||||
f"self-reference"
|
f"(was {type(pname).__name__})"
|
||||||
)
|
)
|
||||||
if parent_name not in raws:
|
parent_names.append(pname)
|
||||||
|
if pname == name:
|
||||||
|
raise ManifestError(
|
||||||
|
f"bottle '{name}' extends itself; remove the self-reference"
|
||||||
|
)
|
||||||
|
if pname not in raws:
|
||||||
avail = ", ".join(sorted(raws.keys())) or "(none)"
|
avail = ", ".join(sorted(raws.keys())) or "(none)"
|
||||||
raise ManifestError(
|
raise ManifestError(
|
||||||
f"bottle '{name}' extends '{parent_name}' which is not "
|
f"bottle '{name}' extends '{pname}' which is not "
|
||||||
f"defined. Available bottles: {avail}"
|
f"defined. Available bottles: {avail}"
|
||||||
)
|
)
|
||||||
parent = _resolve_one_bottle(
|
|
||||||
parent_name, raws, cache, repos_cache, seen + (name,)
|
combined_parent, combined_repos_raw = _fold_parents(
|
||||||
|
parent_names, raws, cache, repos_cache, seen + (name,)
|
||||||
)
|
)
|
||||||
merged_repos_raw = _resolve_repos_raw(repos_cache[parent_name], child_raw)
|
merged_repos_raw = _resolve_repos_raw(combined_repos_raw, child_raw)
|
||||||
bottle = _merge_bottles(parent, child_raw, merged_repos_raw, name)
|
bottle = _merge_bottles(combined_parent, child_raw, merged_repos_raw, name)
|
||||||
cache[name] = bottle
|
cache[name] = bottle
|
||||||
repos_cache[name] = merged_repos_raw
|
repos_cache[name] = merged_repos_raw
|
||||||
return bottle
|
return bottle
|
||||||
|
|
||||||
|
|
||||||
|
def _fold_parents(
|
||||||
|
parent_names: list[str],
|
||||||
|
raws: dict[str, dict[str, object]],
|
||||||
|
cache: dict[str, ManifestBottle],
|
||||||
|
repos_cache: dict[str, dict[str, object]],
|
||||||
|
seen: tuple[str, ...],
|
||||||
|
) -> tuple[ManifestBottle, dict[str, object]]:
|
||||||
|
"""Resolve each parent and fold them left-to-right.
|
||||||
|
|
||||||
|
Later parents win over earlier ones on conflict. The `seen` tuple
|
||||||
|
carries the current bottle's name so cycle detection works across
|
||||||
|
every parent edge in the multi-parent graph."""
|
||||||
|
first = parent_names[0]
|
||||||
|
effective = _resolve_one_bottle(first, raws, cache, repos_cache, seen)
|
||||||
|
effective_repos_raw = repos_cache[first]
|
||||||
|
for pname in parent_names[1:]:
|
||||||
|
later = _resolve_one_bottle(pname, raws, cache, repos_cache, seen)
|
||||||
|
later_repos_raw = repos_cache[pname]
|
||||||
|
effective, effective_repos_raw = _fold_two_bottles(
|
||||||
|
effective, effective_repos_raw, later, later_repos_raw
|
||||||
|
)
|
||||||
|
return effective, effective_repos_raw
|
||||||
|
|
||||||
|
|
||||||
|
def _fold_two_bottles(
|
||||||
|
earlier: ManifestBottle,
|
||||||
|
earlier_repos_raw: dict[str, object],
|
||||||
|
later: ManifestBottle,
|
||||||
|
later_repos_raw: dict[str, object],
|
||||||
|
) -> tuple[ManifestBottle, dict[str, object]]:
|
||||||
|
"""Combine two resolved parent bottles; later wins over earlier."""
|
||||||
|
from .manifest import ManifestBottle, ManifestGitUser
|
||||||
|
from .manifest_egress import ManifestEgressConfig
|
||||||
|
from .manifest_git import parse_git_gate_config
|
||||||
|
from .manifest_util import as_json_object
|
||||||
|
|
||||||
|
merged_env = {**earlier.env, **later.env}
|
||||||
|
|
||||||
|
merged_git_user = ManifestGitUser(
|
||||||
|
name=later.git_user.name or earlier.git_user.name,
|
||||||
|
email=later.git_user.email or earlier.git_user.email,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Repos: union by name; for same-name entries, later wins per-field.
|
||||||
|
# Unlike _resolve_repos_raw, an empty later_repos_raw means "no repos
|
||||||
|
# declared" — it does NOT clear the earlier parent's repos.
|
||||||
|
names = list(earlier_repos_raw) + [
|
||||||
|
n for n in later_repos_raw if n not in earlier_repos_raw
|
||||||
|
]
|
||||||
|
merged_repos_raw: dict[str, object] = {
|
||||||
|
n: {
|
||||||
|
**as_json_object(earlier_repos_raw.get(n, {}), "earlier parent repo"),
|
||||||
|
**as_json_object(later_repos_raw.get(n, {}), "later parent repo"),
|
||||||
|
}
|
||||||
|
for n in names
|
||||||
|
}
|
||||||
|
if merged_repos_raw:
|
||||||
|
merged_git, _ = parse_git_gate_config("_fold", {"repos": merged_repos_raw})
|
||||||
|
else:
|
||||||
|
merged_git = ()
|
||||||
|
|
||||||
|
# Egress: routes concatenate; scalar fields use last-wins.
|
||||||
|
merged_egress = ManifestEgressConfig(
|
||||||
|
routes=earlier.egress.routes + later.egress.routes,
|
||||||
|
Log=later.egress.Log,
|
||||||
|
)
|
||||||
|
|
||||||
|
return ManifestBottle(
|
||||||
|
env=merged_env,
|
||||||
|
agent_provider=later.agent_provider,
|
||||||
|
git=merged_git,
|
||||||
|
git_user=merged_git_user,
|
||||||
|
egress=merged_egress,
|
||||||
|
supervise=later.supervise,
|
||||||
|
), merged_repos_raw
|
||||||
|
|
||||||
|
|
||||||
def _merge_bottles(
|
def _merge_bottles(
|
||||||
parent: ManifestBottle,
|
parent: ManifestBottle,
|
||||||
child_raw: dict[str, object],
|
child_raw: dict[str, object],
|
||||||
|
|||||||
@@ -87,5 +87,7 @@ def load_bottle_chain_from_dir(
|
|||||||
parent = fm.get("extends")
|
parent = fm.get("extends")
|
||||||
if isinstance(parent, str):
|
if isinstance(parent, str):
|
||||||
to_load.append(parent)
|
to_load.append(parent)
|
||||||
|
elif isinstance(parent, list):
|
||||||
|
to_load.extend(p for p in parent if isinstance(p, str))
|
||||||
|
|
||||||
return resolve_bottles(raws)[bottle_name]
|
return resolve_bottles(raws)[bottle_name]
|
||||||
|
|||||||
@@ -90,19 +90,19 @@ def parse_jsonrpc(body: bytes) -> JsonRpcRequest:
|
|||||||
try:
|
try:
|
||||||
raw = json.loads(body)
|
raw = json.loads(body)
|
||||||
except json.JSONDecodeError as e:
|
except json.JSONDecodeError as e:
|
||||||
raise _RpcError(ERR_PARSE, f"parse error: {e}") from e
|
raise _RpcClientError(ERR_PARSE, f"parse error: {e}") from e
|
||||||
if not isinstance(raw, dict):
|
if not isinstance(raw, dict):
|
||||||
raise _RpcError(ERR_INVALID_REQUEST, "request must be a JSON object")
|
raise _RpcClientError(ERR_INVALID_REQUEST, "request must be a JSON object")
|
||||||
if raw.get("jsonrpc") != JSONRPC_VERSION:
|
if raw.get("jsonrpc") != JSONRPC_VERSION:
|
||||||
raise _RpcError(ERR_INVALID_REQUEST, "jsonrpc field must be '2.0'")
|
raise _RpcClientError(ERR_INVALID_REQUEST, "jsonrpc field must be '2.0'")
|
||||||
method = raw.get("method")
|
method = raw.get("method")
|
||||||
if not isinstance(method, str):
|
if not isinstance(method, str):
|
||||||
raise _RpcError(ERR_INVALID_REQUEST, "method must be a string")
|
raise _RpcClientError(ERR_INVALID_REQUEST, "method must be a string")
|
||||||
params = raw.get("params", {})
|
params = raw.get("params", {})
|
||||||
if params is None:
|
if params is None:
|
||||||
params = {}
|
params = {}
|
||||||
if not isinstance(params, dict):
|
if not isinstance(params, dict):
|
||||||
raise _RpcError(ERR_INVALID_PARAMS, "params must be an object")
|
raise _RpcClientError(ERR_INVALID_PARAMS, "params must be an object")
|
||||||
rpc_id = raw.get("id", _NO_ID)
|
rpc_id = raw.get("id", _NO_ID)
|
||||||
is_notification = rpc_id is _NO_ID
|
is_notification = rpc_id is _NO_ID
|
||||||
return JsonRpcRequest(
|
return JsonRpcRequest(
|
||||||
@@ -117,12 +117,23 @@ _NO_ID = object()
|
|||||||
|
|
||||||
|
|
||||||
class _RpcError(Exception):
|
class _RpcError(Exception):
|
||||||
|
"""Base class for all typed RPC errors that surface as JSON-RPC error responses."""
|
||||||
def __init__(self, code: int, message: str):
|
def __init__(self, code: int, message: str):
|
||||||
super().__init__(message)
|
super().__init__(message)
|
||||||
self.code = code
|
self.code = code
|
||||||
self.message = message
|
self.message = message
|
||||||
|
|
||||||
|
|
||||||
|
class _RpcClientError(_RpcError):
|
||||||
|
"""Caller sent a bad request; returned verbatim, no server-side logging."""
|
||||||
|
|
||||||
|
|
||||||
|
class _RpcInternalError(_RpcError):
|
||||||
|
"""Server-side fault; logged at ERROR with cause, always returns ERR_INTERNAL."""
|
||||||
|
def __init__(self, message: str) -> None:
|
||||||
|
super().__init__(ERR_INTERNAL, message)
|
||||||
|
|
||||||
|
|
||||||
def jsonrpc_result(request_id: object, result: object) -> bytes:
|
def jsonrpc_result(request_id: object, result: object) -> bytes:
|
||||||
payload = {"jsonrpc": JSONRPC_VERSION, "id": request_id, "result": result}
|
payload = {"jsonrpc": JSONRPC_VERSION, "id": request_id, "result": result}
|
||||||
return (json.dumps(payload) + "\n").encode("utf-8")
|
return (json.dumps(payload) + "\n").encode("utf-8")
|
||||||
@@ -290,7 +301,7 @@ def validate_proposed_file(tool: str, content: str) -> None:
|
|||||||
catches obvious paste-errors / wrong-tool selections before they
|
catches obvious paste-errors / wrong-tool selections before they
|
||||||
enter the queue."""
|
enter the queue."""
|
||||||
if not content.strip():
|
if not content.strip():
|
||||||
raise _RpcError(ERR_INVALID_PARAMS, f"{tool}: proposed file is empty")
|
raise _RpcClientError(ERR_INVALID_PARAMS, f"{tool}: proposed file is empty")
|
||||||
if tool == _sv.TOOL_CAPABILITY_BLOCK:
|
if tool == _sv.TOOL_CAPABILITY_BLOCK:
|
||||||
# Dockerfiles are too varied to validate syntactically beyond
|
# Dockerfiles are too varied to validate syntactically beyond
|
||||||
# non-empty. The operator reads the diff in the TUI.
|
# non-empty. The operator reads the diff in the TUI.
|
||||||
@@ -299,17 +310,17 @@ def validate_proposed_file(tool: str, content: str) -> None:
|
|||||||
try:
|
try:
|
||||||
config = load_config(content)
|
config = load_config(content)
|
||||||
except ValueError as e:
|
except ValueError as e:
|
||||||
raise _RpcError(
|
raise _RpcClientError(
|
||||||
ERR_INVALID_PARAMS,
|
ERR_INVALID_PARAMS,
|
||||||
f"{tool}: proposed routes.yaml is not valid: {e}",
|
f"{tool}: proposed routes.yaml is not valid: {e}",
|
||||||
) from e
|
) from e
|
||||||
if config.log != LOG_OFF:
|
if config.log != LOG_OFF:
|
||||||
raise _RpcError(
|
raise _RpcClientError(
|
||||||
ERR_INVALID_PARAMS,
|
ERR_INVALID_PARAMS,
|
||||||
f"{tool}: proposed routes.yaml must not change egress logging",
|
f"{tool}: proposed routes.yaml must not change egress logging",
|
||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}")
|
raise _RpcClientError(ERR_INVALID_PARAMS, f"unknown tool {tool!r}")
|
||||||
|
|
||||||
|
|
||||||
# --- MCP handlers ----------------------------------------------------------
|
# --- MCP handlers ----------------------------------------------------------
|
||||||
@@ -382,17 +393,17 @@ def handle_tools_call(
|
|||||||
doesn't need operator approval."""
|
doesn't need operator approval."""
|
||||||
name = params.get("name")
|
name = params.get("name")
|
||||||
if not isinstance(name, str):
|
if not isinstance(name, str):
|
||||||
raise _RpcError(ERR_INVALID_PARAMS, "tools/call missing 'name'")
|
raise _RpcClientError(ERR_INVALID_PARAMS, "tools/call missing 'name'")
|
||||||
if name == _sv.TOOL_LIST_EGRESS_ROUTES:
|
if name == _sv.TOOL_LIST_EGRESS_ROUTES:
|
||||||
return handle_list_egress_routes(typing.cast(dict[str, object], params.get("arguments", {})), config)
|
return handle_list_egress_routes(typing.cast(dict[str, object], params.get("arguments", {})), config)
|
||||||
|
|
||||||
args_raw = params.get("arguments", {})
|
args_raw = params.get("arguments", {})
|
||||||
if not isinstance(args_raw, dict):
|
if not isinstance(args_raw, dict):
|
||||||
raise _RpcError(ERR_INVALID_PARAMS, "tools/call 'arguments' must be an object")
|
raise _RpcClientError(ERR_INVALID_PARAMS, "tools/call 'arguments' must be an object")
|
||||||
|
|
||||||
justification = args_raw.get("justification")
|
justification = args_raw.get("justification")
|
||||||
if not isinstance(justification, str) or not justification.strip():
|
if not isinstance(justification, str) or not justification.strip():
|
||||||
raise _RpcError(
|
raise _RpcClientError(
|
||||||
ERR_INVALID_PARAMS,
|
ERR_INVALID_PARAMS,
|
||||||
f"{name}: 'justification' is required and must be a non-empty string",
|
f"{name}: 'justification' is required and must be a non-empty string",
|
||||||
)
|
)
|
||||||
@@ -401,13 +412,13 @@ def handle_tools_call(
|
|||||||
file_field = PROPOSED_FILE_FIELD[name]
|
file_field = PROPOSED_FILE_FIELD[name]
|
||||||
proposed_file = args_raw.get(file_field)
|
proposed_file = args_raw.get(file_field)
|
||||||
if not isinstance(proposed_file, str):
|
if not isinstance(proposed_file, str):
|
||||||
raise _RpcError(
|
raise _RpcClientError(
|
||||||
ERR_INVALID_PARAMS,
|
ERR_INVALID_PARAMS,
|
||||||
f"{name}: '{file_field}' is required and must be a string",
|
f"{name}: '{file_field}' is required and must be a string",
|
||||||
)
|
)
|
||||||
validate_proposed_file(name, proposed_file)
|
validate_proposed_file(name, proposed_file)
|
||||||
else:
|
else:
|
||||||
raise _RpcError(ERR_INVALID_PARAMS, f"unknown tool {name!r}")
|
raise _RpcClientError(ERR_INVALID_PARAMS, f"unknown tool {name!r}")
|
||||||
|
|
||||||
proposal = _sv.Proposal.new(
|
proposal = _sv.Proposal.new(
|
||||||
bottle_slug=config.bottle_slug,
|
bottle_slug=config.bottle_slug,
|
||||||
@@ -416,7 +427,10 @@ def handle_tools_call(
|
|||||||
justification=justification,
|
justification=justification,
|
||||||
current_file_hash=_sv.sha256_hex(proposed_file),
|
current_file_hash=_sv.sha256_hex(proposed_file),
|
||||||
)
|
)
|
||||||
|
try:
|
||||||
_sv.write_proposal(config.queue_dir, proposal)
|
_sv.write_proposal(config.queue_dir, proposal)
|
||||||
|
except OSError as e:
|
||||||
|
raise _RpcInternalError(f"failed to write proposal to queue: {e}") from e
|
||||||
sys.stderr.write(
|
sys.stderr.write(
|
||||||
f"supervise: queued proposal {proposal.id} ({name}) "
|
f"supervise: queued proposal {proposal.id} ({name}) "
|
||||||
f"for bottle {config.bottle_slug}; waiting for operator...\n"
|
f"for bottle {config.bottle_slug}; waiting for operator...\n"
|
||||||
@@ -436,7 +450,10 @@ def handle_tools_call(
|
|||||||
"content": [{"type": "text", "text": text}],
|
"content": [{"type": "text", "text": text}],
|
||||||
"isError": False,
|
"isError": False,
|
||||||
}
|
}
|
||||||
|
try:
|
||||||
_sv.archive_proposal(config.queue_dir, proposal.id)
|
_sv.archive_proposal(config.queue_dir, proposal.id)
|
||||||
|
except OSError as e:
|
||||||
|
raise _RpcInternalError(f"failed to archive proposal: {e}") from e
|
||||||
|
|
||||||
text = format_response_text(response)
|
text = format_response_text(response)
|
||||||
return {
|
return {
|
||||||
@@ -512,7 +529,7 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
req = parse_jsonrpc(body)
|
req = parse_jsonrpc(body)
|
||||||
except _RpcError as e:
|
except _RpcClientError as e:
|
||||||
self._write_jsonrpc(jsonrpc_error(None, e.code, e.message))
|
self._write_jsonrpc(jsonrpc_error(None, e.code, e.message))
|
||||||
return
|
return
|
||||||
|
|
||||||
@@ -520,11 +537,19 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
result = self._dispatch(req, config)
|
result = self._dispatch(req, config)
|
||||||
except _RpcError as e:
|
except _RpcClientError as e:
|
||||||
self._write_jsonrpc(jsonrpc_error(req.id, e.code, e.message))
|
self._write_jsonrpc(jsonrpc_error(req.id, e.code, e.message))
|
||||||
return
|
return
|
||||||
except Exception as e: # noqa: W0718 — catch-all for RPC dispatch errors
|
except _RpcInternalError as e:
|
||||||
sys.stderr.write(f"supervise: internal error: {e}\n")
|
cause = e.__cause__
|
||||||
|
detail = f": {cause}" if cause else ""
|
||||||
|
sys.stderr.write(f"supervise: internal error: {e.message}{detail}\n")
|
||||||
|
sys.stderr.flush()
|
||||||
|
self._write_jsonrpc(jsonrpc_error(req.id, ERR_INTERNAL, "internal error"))
|
||||||
|
return
|
||||||
|
except Exception as e: # noqa: W0718 — unexpected errors
|
||||||
|
sys.stderr.write(f"supervise: unexpected error: {type(e).__name__}: {e}\n")
|
||||||
|
sys.stderr.flush()
|
||||||
self._write_jsonrpc(jsonrpc_error(req.id, ERR_INTERNAL, "internal error"))
|
self._write_jsonrpc(jsonrpc_error(req.id, ERR_INTERNAL, "internal error"))
|
||||||
return
|
return
|
||||||
|
|
||||||
@@ -543,7 +568,7 @@ class MCPHandler(http.server.BaseHTTPRequestHandler):
|
|||||||
return handle_tools_list(req.params)
|
return handle_tools_list(req.params)
|
||||||
if method == "tools/call":
|
if method == "tools/call":
|
||||||
return handle_tools_call(req.params, config)
|
return handle_tools_call(req.params, config)
|
||||||
raise _RpcError(ERR_METHOD_NOT_FOUND, f"method not found: {method}")
|
raise _RpcClientError(ERR_METHOD_NOT_FOUND, f"method not found: {method}")
|
||||||
|
|
||||||
def _write_jsonrpc(self, body: bytes) -> None:
|
def _write_jsonrpc(self, body: bytes) -> None:
|
||||||
self.send_response(200)
|
self.send_response(200)
|
||||||
|
|||||||
@@ -0,0 +1,166 @@
|
|||||||
|
# PRD prd-new: Multi-parent `extends:` for bottles
|
||||||
|
|
||||||
|
- **Status:** Draft
|
||||||
|
- **Author:** didericis
|
||||||
|
- **Created:** 2026-06-25
|
||||||
|
- **Issue:** #268
|
||||||
|
- **Extends:** PRD 0025 (`0025-bottle-extends.md`)
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Allow a bottle's `extends:` field to accept either a single bottle name (existing
|
||||||
|
behavior) or a list of bottle names (new). Multiple parents are resolved
|
||||||
|
independently and folded left-to-right into a single effective parent before the
|
||||||
|
child is merged on top. This lets orthogonal concerns (base env, networking/egress,
|
||||||
|
agent provider) live in separate bottles and be composed without forcing them into a
|
||||||
|
linear chain.
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
PRD 0025 shipped single-parent `extends:` and listed "No multi-parent inheritance"
|
||||||
|
as a non-goal. In practice, users want to compose multiple orthogonal bottles — a
|
||||||
|
base environment, a networking profile, and an agent-provider override — without
|
||||||
|
creating a three-level linear chain that couples unrelated parents to each other.
|
||||||
|
The linear chain workaround has two problems:
|
||||||
|
|
||||||
|
1. **Ordering constraint.** `networking extends base` works, but then
|
||||||
|
`agent extends networking` can't also pick up `base` without going through
|
||||||
|
`networking`, coupling two unrelated concerns.
|
||||||
|
|
||||||
|
2. **Quadratic duplication.** N orthogonal bottles require O(N²) chain variants
|
||||||
|
(one chain per permutation of applied concerns).
|
||||||
|
|
||||||
|
Multi-parent `extends:` removes both constraints: each orthogonal concern stays in
|
||||||
|
its own bottle, and the child bottle is the only place that names the combination.
|
||||||
|
|
||||||
|
## Goals / Success Criteria
|
||||||
|
|
||||||
|
- `extends:` accepts a list of strings in addition to a plain string.
|
||||||
|
- Backward compat: existing single-string `extends:` is unchanged.
|
||||||
|
- Parents are resolved left-to-right; later entries win on conflict.
|
||||||
|
- Child wins over all parents (unchanged from PRD 0025).
|
||||||
|
- Cycle detection covers multi-parent graphs, not just linear chains.
|
||||||
|
- Diamond inheritance: a shared ancestor is resolved once (via the existing cache).
|
||||||
|
- Invalid list entries (non-string, undefined bottle, self-reference) die at parse
|
||||||
|
with clear messages.
|
||||||
|
- `manifest_loader.py`'s `load_bottle_chain_from_dir` enqueues all parents from a
|
||||||
|
list `extends:` so the resolver sees every bottle in the graph.
|
||||||
|
|
||||||
|
## Non-goals
|
||||||
|
|
||||||
|
- No change to the agent-vs-bottle trust boundary (PRD 0025 "Alternatives
|
||||||
|
considered" option 2 stays rejected).
|
||||||
|
- No MRO / C3 linearization. Left-to-right fold is sufficient for the expected use
|
||||||
|
cases.
|
||||||
|
- No preflight display of per-field provenance across multiple parents (same open
|
||||||
|
question as PRD 0025; remains a follow-up).
|
||||||
|
|
||||||
|
## Design
|
||||||
|
|
||||||
|
### Schema
|
||||||
|
|
||||||
|
`extends:` now accepts either form:
|
||||||
|
|
||||||
|
```yaml
|
||||||
|
# single parent (unchanged)
|
||||||
|
extends: base
|
||||||
|
|
||||||
|
# multiple parents (new)
|
||||||
|
extends: [base, networking]
|
||||||
|
```
|
||||||
|
|
||||||
|
Both forms are normalized to a list internally. A list with one element behaves
|
||||||
|
identically to the string form.
|
||||||
|
|
||||||
|
### Merge rules for multi-parent fold
|
||||||
|
|
||||||
|
Parents are folded pairwise left-to-right before the child merge. For each step in
|
||||||
|
the fold, the "earlier" bottle is the running accumulator and the "later" bottle is
|
||||||
|
the next parent. Rules per field:
|
||||||
|
|
||||||
|
| Field | Fold rule |
|
||||||
|
|--------------------|--------------------------------------------------------------|
|
||||||
|
| `env` | dict merge; later wins on key collision |
|
||||||
|
| `git-gate.user` | per-field overlay; later's non-empty fields win |
|
||||||
|
| `git-gate.repos` | union by name; for same-name entries, later wins per-field |
|
||||||
|
| `egress.routes` | concatenate (earlier first, later appended) |
|
||||||
|
| `egress.log` | later wins (last-wins) |
|
||||||
|
| `agent_provider` | later wins (last-wins) |
|
||||||
|
| `supervise` | later wins (last-wins) |
|
||||||
|
|
||||||
|
After the fold, the combined parent is merged against the child using the existing
|
||||||
|
PRD 0025 rules (child always wins). The child's `egress.routes` appends to the
|
||||||
|
combined parent's concatenated routes; `validate_egress_routes` runs once on the
|
||||||
|
final merged set and catches duplicate hosts.
|
||||||
|
|
||||||
|
### Algorithm
|
||||||
|
|
||||||
|
```
|
||||||
|
extends: [p1, p2, p3]
|
||||||
|
|
||||||
|
fold:
|
||||||
|
combined = resolve(p1)
|
||||||
|
combined = fold_two(combined, resolve(p2))
|
||||||
|
combined = fold_two(combined, resolve(p3))
|
||||||
|
|
||||||
|
merge:
|
||||||
|
result = _merge_bottles(combined, child_raw, name)
|
||||||
|
```
|
||||||
|
|
||||||
|
`fold_two(earlier, later)` applies the rules in the table above. Cycle detection
|
||||||
|
(the `seen` tuple) is passed to each parent resolution call unchanged — if any
|
||||||
|
parent's chain circles back to the current bottle, it is caught. The `cache` dict
|
||||||
|
ensures a shared ancestor is only resolved once across all parents.
|
||||||
|
|
||||||
|
### Error cases
|
||||||
|
|
||||||
|
| Condition | Error message shape |
|
||||||
|
|----------------------------------------|------------------------------------------------------------------|
|
||||||
|
| `extends` is not a string or list | `extends must be a string or list of strings (was <type>)` |
|
||||||
|
| A list entry is not a string | `extends[<i>] must be a string (was <type>)` |
|
||||||
|
| A list entry names an undefined bottle | `extends '<name>' which is not defined. Available bottles: ...` |
|
||||||
|
| A list entry is the bottle itself | `extends itself; remove the self-reference` |
|
||||||
|
| Cycle through any parent edge | `is in an extends cycle: <chain>` |
|
||||||
|
|
||||||
|
## Implementation
|
||||||
|
|
||||||
|
### `bot_bottle/manifest_extends.py`
|
||||||
|
|
||||||
|
- `_resolve_one_bottle`: accept `str | list[str]` for `extends`; normalize to list;
|
||||||
|
validate each entry; for a single-entry list fall through to the existing
|
||||||
|
single-parent path; for multiple entries call `_fold_parents` then
|
||||||
|
`_merge_bottles`.
|
||||||
|
- `_fold_parents(parent_names, raws, cache, repos_cache, seen)`: resolve each
|
||||||
|
parent and fold pairwise left-to-right; return `(effective_bottle,
|
||||||
|
effective_repos_raw)`.
|
||||||
|
- `_fold_two_bottles(earlier, earlier_repos_raw, later, later_repos_raw)`: apply
|
||||||
|
the fold rules above; return `(folded_bottle, folded_repos_raw)`.
|
||||||
|
|
||||||
|
### `bot_bottle/manifest_loader.py`
|
||||||
|
|
||||||
|
- `load_bottle_chain_from_dir`: when `extends` is a list, enqueue all parent names
|
||||||
|
for loading (previously only `isinstance(parent, str)` was handled).
|
||||||
|
|
||||||
|
### `tests/unit/test_manifest_extends.py`
|
||||||
|
|
||||||
|
- `TestExtendsErrors.test_non_string_extends_dies`: update to use an integer
|
||||||
|
`extends` value (a list is now valid).
|
||||||
|
- New class `TestExtendsMultiParent` covering all cases listed in the issue.
|
||||||
|
|
||||||
|
## Testing strategy
|
||||||
|
|
||||||
|
Unit tests via `ManifestIndex.from_json_obj` (same resolver surface used by all
|
||||||
|
paths). No integration test changes needed — downstream code consumes the already-
|
||||||
|
merged bottle and is unchanged.
|
||||||
|
|
||||||
|
Test cases:
|
||||||
|
- Two-parent list: env union, egress routes concat, git repos union
|
||||||
|
- Last-parent-wins on scalar (supervise, agent_provider)
|
||||||
|
- Child wins over all parents on conflict
|
||||||
|
- Diamond: two parents share an ancestor; ancestor resolved once
|
||||||
|
- Single-element list: identical to string form
|
||||||
|
- Non-string extends value → ManifestError
|
||||||
|
- Non-string list entry → ManifestError
|
||||||
|
- Undefined bottle in list → ManifestError
|
||||||
|
- Self-reference in list → ManifestError
|
||||||
|
- Cycle through multi-parent edge → ManifestError
|
||||||
@@ -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
|
||||||
@@ -256,6 +312,57 @@ class TestGitHttpBackend(unittest.TestCase):
|
|||||||
os.environ["GIT_GATE_ACCESS_HOOK"] = value
|
os.environ["GIT_GATE_ACCESS_HOOK"] = value
|
||||||
|
|
||||||
|
|
||||||
|
class TestMalformedStatusHeader(unittest.TestCase):
|
||||||
|
"""Malformed CGI Status: headers must not propagate as unhandled exceptions;
|
||||||
|
the handler should fall back to HTTP 500."""
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
from http.server import ThreadingHTTPServer
|
||||||
|
import tempfile
|
||||||
|
self._tmp = tempfile.mkdtemp()
|
||||||
|
os.environ["GIT_PROJECT_ROOT"] = self._tmp
|
||||||
|
self._server = ThreadingHTTPServer(("127.0.0.1", 0), GitHttpHandler)
|
||||||
|
self._thread = threading.Thread(
|
||||||
|
target=self._server.serve_forever, daemon=True,
|
||||||
|
)
|
||||||
|
self._thread.start()
|
||||||
|
self._port = self._server.server_port
|
||||||
|
|
||||||
|
def tearDown(self):
|
||||||
|
self._server.shutdown()
|
||||||
|
self._server.server_close()
|
||||||
|
os.environ.pop("GIT_PROJECT_ROOT", None)
|
||||||
|
import shutil
|
||||||
|
shutil.rmtree(self._tmp, ignore_errors=True)
|
||||||
|
|
||||||
|
def _get_with_backend_response(self, cgi_response: bytes) -> int:
|
||||||
|
with mock.patch(
|
||||||
|
"bot_bottle.git_http_backend.subprocess.run",
|
||||||
|
return_value=mock.Mock(returncode=0, stdout=cgi_response),
|
||||||
|
):
|
||||||
|
req = urllib.request.Request(
|
||||||
|
f"http://127.0.0.1:{self._port}/repo.git/info/refs",
|
||||||
|
method="GET",
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
with urllib.request.urlopen(req, timeout=3) as resp:
|
||||||
|
return resp.status
|
||||||
|
except urllib.error.HTTPError as e: # type: ignore
|
||||||
|
return e.code
|
||||||
|
|
||||||
|
def test_empty_status_value_returns_500(self):
|
||||||
|
status = self._get_with_backend_response(
|
||||||
|
b"Status: \r\nContent-Type: text/plain\r\n\r\n"
|
||||||
|
)
|
||||||
|
self.assertEqual(500, status)
|
||||||
|
|
||||||
|
def test_non_numeric_status_returns_500(self):
|
||||||
|
status = self._get_with_backend_response(
|
||||||
|
b"Status: bad\r\nContent-Type: text/plain\r\n\r\n"
|
||||||
|
)
|
||||||
|
self.assertEqual(500, status)
|
||||||
|
|
||||||
|
|
||||||
class TestContentLengthBounds(unittest.TestCase):
|
class TestContentLengthBounds(unittest.TestCase):
|
||||||
"""PRD 0041: malformed or oversized Content-Length is rejected before
|
"""PRD 0041: malformed or oversized Content-Length is rejected before
|
||||||
git http-backend is invoked."""
|
git http-backend is invoked."""
|
||||||
|
|||||||
@@ -423,9 +423,182 @@ class TestExtendsErrors(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertIn("extends cycle", msg)
|
self.assertIn("extends cycle", msg)
|
||||||
|
|
||||||
def test_non_string_extends_dies(self):
|
def test_non_string_non_list_extends_dies(self):
|
||||||
msg = _error_message(_build, child={"extends": ["base"]})
|
msg = _error_message(_build, child={"extends": 123})
|
||||||
self.assertIn("extends must be a string", msg)
|
self.assertIn("extends must be a string or list of strings", msg)
|
||||||
|
|
||||||
|
def test_list_entry_non_string_dies(self):
|
||||||
|
msg = _error_message(_build, child={"extends": [123]})
|
||||||
|
self.assertIn("extends[0] must be a string", msg)
|
||||||
|
|
||||||
|
|
||||||
|
class TestExtendsMultiParent(unittest.TestCase):
|
||||||
|
"""extends: [p1, p2, ...] — multi-parent composition (issue #268)."""
|
||||||
|
|
||||||
|
_GIT_A = {"url": "ssh://git@host-a/a.git", "key": {"provider": "static", "path": "/k"}}
|
||||||
|
_GIT_B = {"url": "ssh://git@host-b/b.git", "key": {"provider": "static", "path": "/k"}}
|
||||||
|
|
||||||
|
def test_single_element_list_same_as_string(self):
|
||||||
|
m = _build(
|
||||||
|
base={"env": {"X": "1"}},
|
||||||
|
child={"extends": ["base"]},
|
||||||
|
)
|
||||||
|
self.assertEqual({"X": "1"}, dict(m.bottles["child"].env))
|
||||||
|
|
||||||
|
def test_two_parents_env_union(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"env": {"A": "1"}},
|
||||||
|
p2={"env": {"B": "2"}},
|
||||||
|
child={"extends": ["p1", "p2"]},
|
||||||
|
)
|
||||||
|
self.assertEqual({"A": "1", "B": "2"}, dict(m.bottles["child"].env))
|
||||||
|
|
||||||
|
def test_two_parents_env_last_wins_on_collision(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"env": {"X": "from-p1"}},
|
||||||
|
p2={"env": {"X": "from-p2"}},
|
||||||
|
child={"extends": ["p1", "p2"]},
|
||||||
|
)
|
||||||
|
self.assertEqual("from-p2", m.bottles["child"].env["X"])
|
||||||
|
|
||||||
|
def test_child_wins_over_all_parents(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"env": {"X": "from-p1"}},
|
||||||
|
p2={"env": {"X": "from-p2"}},
|
||||||
|
child={"extends": ["p1", "p2"], "env": {"X": "from-child"}},
|
||||||
|
)
|
||||||
|
self.assertEqual("from-child", m.bottles["child"].env["X"])
|
||||||
|
|
||||||
|
def test_two_parents_supervise_last_wins(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"supervise": False},
|
||||||
|
p2={"supervise": True},
|
||||||
|
child={"extends": ["p1", "p2"]},
|
||||||
|
)
|
||||||
|
self.assertTrue(m.bottles["child"].supervise)
|
||||||
|
|
||||||
|
def test_child_supervise_overrides_all_parents(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"supervise": True},
|
||||||
|
p2={"supervise": True},
|
||||||
|
child={"extends": ["p1", "p2"], "supervise": False},
|
||||||
|
)
|
||||||
|
self.assertFalse(m.bottles["child"].supervise)
|
||||||
|
|
||||||
|
def test_two_parents_egress_routes_concatenated(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"egress": {"routes": [{"host": "a.example.com"}]}},
|
||||||
|
p2={"egress": {"routes": [{"host": "b.example.com"}]}},
|
||||||
|
child={"extends": ["p1", "p2"]},
|
||||||
|
)
|
||||||
|
hosts = [r.Host for r in m.bottles["child"].egress.routes]
|
||||||
|
self.assertEqual(["a.example.com", "b.example.com"], hosts)
|
||||||
|
|
||||||
|
def test_child_egress_appends_after_combined_parents(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"egress": {"routes": [{"host": "a.example.com"}]}},
|
||||||
|
p2={"egress": {"routes": [{"host": "b.example.com"}]}},
|
||||||
|
child={
|
||||||
|
"extends": ["p1", "p2"],
|
||||||
|
"egress": {"routes": [{"host": "c.example.com"}]},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
hosts = [r.Host for r in m.bottles["child"].egress.routes]
|
||||||
|
self.assertEqual(["a.example.com", "b.example.com", "c.example.com"], hosts)
|
||||||
|
|
||||||
|
def test_two_parents_git_repos_union(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"git-gate": {"repos": {"a": self._GIT_A}}},
|
||||||
|
p2={"git-gate": {"repos": {"b": self._GIT_B}}},
|
||||||
|
child={"extends": ["p1", "p2"]},
|
||||||
|
)
|
||||||
|
names = {e.Name for e in m.bottles["child"].git}
|
||||||
|
self.assertEqual({"a", "b"}, names)
|
||||||
|
|
||||||
|
def test_two_parents_git_same_name_later_wins_per_field(self):
|
||||||
|
# Both parents declare the same repo name. p2's `key` wins; p1's
|
||||||
|
# `host_key` is preserved because p2 doesn't override it.
|
||||||
|
p1_entry = {
|
||||||
|
"url": "ssh://git@host-a/repo.git",
|
||||||
|
"host_key": "ecdsa AAAA",
|
||||||
|
"key": {"provider": "static", "path": "/k1"},
|
||||||
|
}
|
||||||
|
p2_entry = {
|
||||||
|
"url": "ssh://git@host-a/repo.git", # required, same url
|
||||||
|
"key": {"provider": "gitea", "forge_token_env": "TOK"},
|
||||||
|
}
|
||||||
|
m = _build(
|
||||||
|
p1={"git-gate": {"repos": {"repo": p1_entry}}},
|
||||||
|
p2={"git-gate": {"repos": {"repo": p2_entry}}},
|
||||||
|
child={"extends": ["p1", "p2"]},
|
||||||
|
)
|
||||||
|
entries = m.bottles["child"].git
|
||||||
|
self.assertEqual(1, len(entries))
|
||||||
|
e = entries[0]
|
||||||
|
self.assertEqual("ssh://git@host-a/repo.git", e.Upstream)
|
||||||
|
self.assertEqual("ecdsa AAAA", e.KnownHostKey)
|
||||||
|
self.assertEqual("gitea", e.Key.provider)
|
||||||
|
|
||||||
|
def test_p1_repos_preserved_when_p2_has_none(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"git-gate": {"repos": {"a": self._GIT_A}}},
|
||||||
|
p2={"env": {"X": "1"}},
|
||||||
|
child={"extends": ["p1", "p2"]},
|
||||||
|
)
|
||||||
|
names = [e.Name for e in m.bottles["child"].git]
|
||||||
|
self.assertEqual(["a"], names)
|
||||||
|
|
||||||
|
def test_diamond_shared_ancestor_resolved_once(self):
|
||||||
|
# a <- b, a <- c; child extends [b, c]
|
||||||
|
# `a` must be resolved once and cached.
|
||||||
|
m = _build(
|
||||||
|
a={"env": {"FROM_A": "1"}, "supervise": False},
|
||||||
|
b={"extends": "a", "env": {"FROM_B": "1"}},
|
||||||
|
c={"extends": "a", "env": {"FROM_C": "1"}},
|
||||||
|
child={"extends": ["b", "c"]},
|
||||||
|
)
|
||||||
|
child = m.bottles["child"]
|
||||||
|
self.assertEqual("1", child.env["FROM_A"])
|
||||||
|
self.assertEqual("1", child.env["FROM_B"])
|
||||||
|
self.assertEqual("1", child.env["FROM_C"])
|
||||||
|
# supervise=False from `a` threads through both b and c; c is the
|
||||||
|
# later parent so its effective supervise (False) wins.
|
||||||
|
self.assertFalse(child.supervise)
|
||||||
|
|
||||||
|
def test_three_parents_env_fold_order(self):
|
||||||
|
m = _build(
|
||||||
|
p1={"env": {"X": "1", "A": "a"}},
|
||||||
|
p2={"env": {"X": "2", "B": "b"}},
|
||||||
|
p3={"env": {"X": "3", "C": "c"}},
|
||||||
|
child={"extends": ["p1", "p2", "p3"]},
|
||||||
|
)
|
||||||
|
env = dict(m.bottles["child"].env)
|
||||||
|
self.assertEqual("3", env["X"])
|
||||||
|
self.assertEqual("a", env["A"])
|
||||||
|
self.assertEqual("b", env["B"])
|
||||||
|
self.assertEqual("c", env["C"])
|
||||||
|
|
||||||
|
def test_undefined_bottle_in_list_dies(self):
|
||||||
|
msg = _error_message(
|
||||||
|
_build,
|
||||||
|
base={"env": {}},
|
||||||
|
child={"extends": ["base", "ghost"]},
|
||||||
|
)
|
||||||
|
self.assertIn("extends 'ghost'", msg)
|
||||||
|
self.assertIn("not defined", msg)
|
||||||
|
|
||||||
|
def test_self_reference_in_list_dies(self):
|
||||||
|
msg = _error_message(_build, child={"extends": ["child"]})
|
||||||
|
self.assertIn("extends itself", msg)
|
||||||
|
|
||||||
|
def test_cycle_through_multi_parent_edge_dies(self):
|
||||||
|
msg = _error_message(
|
||||||
|
_build,
|
||||||
|
a={"extends": ["b", "c"]},
|
||||||
|
b={},
|
||||||
|
c={"extends": "a"},
|
||||||
|
)
|
||||||
|
self.assertIn("extends cycle", msg)
|
||||||
|
|
||||||
|
|
||||||
class TestExtendsAvailableInBottleKeys(unittest.TestCase):
|
class TestExtendsAvailableInBottleKeys(unittest.TestCase):
|
||||||
|
|||||||
@@ -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()
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ import supervise as _sv # noqa: E402 # type: ignore
|
|||||||
|
|
||||||
from bot_bottle import supervise_server # noqa: E402
|
from bot_bottle import supervise_server # noqa: E402
|
||||||
from bot_bottle.supervise_server import (
|
from bot_bottle.supervise_server import (
|
||||||
|
ERR_INTERNAL,
|
||||||
ERR_INVALID_PARAMS,
|
ERR_INVALID_PARAMS,
|
||||||
ERR_INVALID_REQUEST,
|
ERR_INVALID_REQUEST,
|
||||||
ERR_METHOD_NOT_FOUND,
|
ERR_METHOD_NOT_FOUND,
|
||||||
@@ -29,7 +30,9 @@ from bot_bottle.supervise_server import (
|
|||||||
PROPOSED_FILE_FIELD,
|
PROPOSED_FILE_FIELD,
|
||||||
ServerConfig,
|
ServerConfig,
|
||||||
TOOL_DEFINITIONS,
|
TOOL_DEFINITIONS,
|
||||||
|
_RpcClientError,
|
||||||
_RpcError,
|
_RpcError,
|
||||||
|
_RpcInternalError,
|
||||||
_response_timeout_from_env,
|
_response_timeout_from_env,
|
||||||
format_response_text,
|
format_response_text,
|
||||||
handle_initialize,
|
handle_initialize,
|
||||||
@@ -77,6 +80,65 @@ class TestValidation(unittest.TestCase):
|
|||||||
self.assertIn("must not change egress logging", cm.exception.message)
|
self.assertIn("must not change egress logging", cm.exception.message)
|
||||||
|
|
||||||
|
|
||||||
|
# --- Error taxonomy --------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
class TestRpcErrorTaxonomy(unittest.TestCase):
|
||||||
|
def test_rpc_client_error_is_rpc_error(self):
|
||||||
|
e = _RpcClientError(ERR_INVALID_PARAMS, "bad param")
|
||||||
|
self.assertIsInstance(e, _RpcError)
|
||||||
|
self.assertEqual(ERR_INVALID_PARAMS, e.code)
|
||||||
|
self.assertEqual("bad param", e.message)
|
||||||
|
|
||||||
|
def test_rpc_internal_error_is_rpc_error(self):
|
||||||
|
e = _RpcInternalError("disk full")
|
||||||
|
self.assertIsInstance(e, _RpcError)
|
||||||
|
self.assertEqual(ERR_INTERNAL, e.code)
|
||||||
|
self.assertEqual("disk full", e.message)
|
||||||
|
|
||||||
|
def test_rpc_internal_error_preserves_cause(self):
|
||||||
|
cause = OSError("no space left on device")
|
||||||
|
try:
|
||||||
|
raise _RpcInternalError("failed to write") from cause
|
||||||
|
except _RpcInternalError as e:
|
||||||
|
self.assertIs(cause, e.__cause__)
|
||||||
|
|
||||||
|
def test_parse_error_is_client_error(self):
|
||||||
|
with self.assertRaises(_RpcClientError):
|
||||||
|
parse_jsonrpc(b"{bad json")
|
||||||
|
|
||||||
|
def test_validation_error_is_client_error(self):
|
||||||
|
with self.assertRaises(_RpcClientError):
|
||||||
|
validate_proposed_file(_sv.TOOL_EGRESS_ALLOW, "routes: nope\n")
|
||||||
|
|
||||||
|
def test_unknown_tool_in_tools_call_is_client_error(self):
|
||||||
|
config = ServerConfig(bottle_slug="dev", queue_dir=Path("/unused"))
|
||||||
|
with self.assertRaises(_RpcClientError) as cm:
|
||||||
|
handle_tools_call({"name": "no-such-tool", "arguments": {}}, config)
|
||||||
|
self.assertEqual(ERR_INVALID_PARAMS, cm.exception.code)
|
||||||
|
|
||||||
|
|
||||||
|
class TestRpcInternalErrorOnIoFailure(unittest.TestCase):
|
||||||
|
def test_write_proposal_os_error_raises_internal(self):
|
||||||
|
config = ServerConfig(
|
||||||
|
bottle_slug="dev",
|
||||||
|
queue_dir=Path("/dev/null/cannot-exist"),
|
||||||
|
)
|
||||||
|
with self.assertRaises(_RpcInternalError) as cm:
|
||||||
|
handle_tools_call(
|
||||||
|
{
|
||||||
|
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||||
|
"arguments": {
|
||||||
|
"dockerfile": "FROM python:3.13\n",
|
||||||
|
"justification": "x",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
config,
|
||||||
|
)
|
||||||
|
self.assertEqual(ERR_INTERNAL, cm.exception.code)
|
||||||
|
self.assertIsNotNone(cm.exception.__cause__)
|
||||||
|
|
||||||
|
|
||||||
# --- JSON-RPC parsing ------------------------------------------------------
|
# --- JSON-RPC parsing ------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@@ -469,6 +531,26 @@ class TestHttpEndToEnd(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
self.assertEqual(ERR_METHOD_NOT_FOUND, result["error"]["code"]) # type: ignore[index]
|
self.assertEqual(ERR_METHOD_NOT_FOUND, result["error"]["code"]) # type: ignore[index]
|
||||||
|
|
||||||
|
def test_internal_error_returns_err_internal_over_http(self):
|
||||||
|
with patch.object(
|
||||||
|
supervise_server._sv, "write_proposal",
|
||||||
|
side_effect=OSError("disk full"),
|
||||||
|
):
|
||||||
|
result = self._post_jsonrpc({
|
||||||
|
"jsonrpc": "2.0",
|
||||||
|
"id": 99,
|
||||||
|
"method": "tools/call",
|
||||||
|
"params": {
|
||||||
|
"name": _sv.TOOL_CAPABILITY_BLOCK,
|
||||||
|
"arguments": {
|
||||||
|
"dockerfile": "FROM python:3.13\n",
|
||||||
|
"justification": "x",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
self.assertIn("error", result)
|
||||||
|
self.assertEqual(ERR_INTERNAL, result["error"]["code"]) # type: ignore[index]
|
||||||
|
|
||||||
def test_health_endpoint(self):
|
def test_health_endpoint(self):
|
||||||
conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=5)
|
conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=5)
|
||||||
try:
|
try:
|
||||||
|
|||||||
Reference in New Issue
Block a user