From f6c0fce81ea6285148fc3dd7c0a8e1496af25755 Mon Sep 17 00:00:00 2001 From: gxc Date: Sun, 30 Aug 2026 23:29:21 +0800 Subject: [PATCH 1/2] feat: ssh custom git servers via GIT_CLONE_EXTRA_HOSTS Allowlist self-hosted git servers (host[:port] entries, IPv6 bracketed) for generate_cpg cloning over ssh:// only; github.com/gitlab.com keep their https-only posture with per-call token injection (stripped from .git/config after the clone). - validate_repo_url: custom hosts accept ssh:// only; http(s) stays github/gitlab-only, port pinning and the SSRF posture unchanged - git_manager: ssh auth rides in GIT_SSH_COMMAND (key path or full command override) - compose: mount the operator key dir read-only at /keys and derive the in-container key path (/keys/id_ed25519) from GIT_CLONE_SSH_KEYS_HOST_DIR - docs: two-mode path semantics (host-run MCP vs dockerized stack) - tests: unit coverage for the ssh-only validator and clone behavior --- .env.example | 25 ++++ .gitignore | 4 + Dockerfile.mcp | 1 + docker-compose.yml | 17 +++ docs/deployment.md | 65 ++++++++++- docs/security.md | 2 +- src/defaults.py | 17 +++ src/services/git_manager.py | 103 ++++++++++------- src/tools/core_tools.py | 33 ++++-- src/utils/__init__.py | 2 +- src/utils/validators.py | 219 ++++++++++++++++++++++++++++-------- tests/test_git_manager.py | 127 +++++++++++++++++++++ tests/test_validators.py | 117 +++++++++++++++++++ 13 files changed, 627 insertions(+), 105 deletions(-) create mode 100644 tests/test_git_manager.py diff --git a/.env.example b/.env.example index ef7d4e5..5ef4d8d 100644 --- a/.env.example +++ b/.env.example @@ -82,6 +82,31 @@ CPG_QUEUE_BACKEND=durable # Empty = no allowlist. # ALLOWED_SOURCE_ROOTS=/abs/path/to/sources:/abs/path/to/other-sources +# Custom git clone servers (optional) — allow your own git host beyond +# github.com/gitlab.com, e.g. a self-hosted Forgejo on the LAN. +# GIT_CLONE_EXTRA_HOSTS: ','-separated `host[:port]` entries. A bare host allows +# any port on it; `host:port` pins that port only. Listed hosts are cloned via +# ssh:// only (github.com/gitlab.com stay https-only). +# ssh:// auth depends on how the MCP runs (the clone runs where the MCP runs): +# - MCP on the host (`python main.py`): set GIT_CLONE_SSH_KEY_PATH to the key +# FILE on this host, or a full GIT_CLONE_SSH_COMMAND override. +# - Full docker stack (`./scripts/deploy.sh`): set GIT_CLONE_SSH_KEYS_HOST_DIR +# to a HOST directory containing the private key as id_ed25519; compose mounts +# it read-only at /keys in the codebadger-mcp container and the server then +# sees /keys/id_ed25519. GIT_CLONE_SSH_KEY_PATH has no effect in the container +# — do not set it here. With GIT_CLONE_SSH_COMMAND, any path it references +# must exist inside the container (e.g. /keys/...). +# Example Forgejo reachable at ssh://git@192.168.152.14:3000/...: +# GIT_CLONE_EXTRA_HOSTS=192.168.152.14:3000 +# # host-run MCP: +# GIT_CLONE_SSH_KEY_PATH=/path/to/id_ed25519 +# # dockerized stack: +# GIT_CLONE_SSH_KEYS_HOST_DIR=/path/to/keydir # containing id_ed25519 +# GIT_CLONE_EXTRA_HOSTS= +# GIT_CLONE_SSH_KEYS_HOST_DIR= +# GIT_CLONE_SSH_KEY_PATH= +# GIT_CLONE_SSH_COMMAND= + DOCKER_HOST=unix:///var/run/docker.sock # GitHub (optional) — token for cloning private repos. diff --git a/.gitignore b/.gitignore index 0f69609..6cb02c4 100644 --- a/.gitignore +++ b/.gitignore @@ -145,3 +145,7 @@ codebadger.db docker-compose.override.yml scripts/backfill_overlays.sh .claude/settings.json + +# Operator ssh keys for GIT_CLONE_EXTRA_HOSTS clones (compose default host dir). +# Never commit private keys. +.ssh-keys/ diff --git a/Dockerfile.mcp b/Dockerfile.mcp index b93a21e..9ec117a 100644 --- a/Dockerfile.mcp +++ b/Dockerfile.mcp @@ -15,6 +15,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ git \ curl \ ca-certificates \ + openssh-client \ && curl -fsSL "https://download.docker.com/linux/static/stable/${DOCKER_CLI_ARCH}/docker-${DOCKER_CLI_VERSION}.tgz" \ | tar -xz -C /usr/local/bin --strip-components=1 docker/docker \ && docker --version \ diff --git a/docker-compose.yml b/docker-compose.yml index bb8f566..364e57e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -54,6 +54,13 @@ services: # DOCKER_HOST) for a rootless / non-default socket; container side stays fixed. - ${DOCKER_SOCK:-/var/run/docker.sock}:/var/run/docker.sock - ./logs:/app/logs + # Operator ssh key dir for GIT_CLONE_EXTRA_HOSTS clones, mounted READ-ONLY + # at the fixed in-container path /keys. Point GIT_CLONE_SSH_KEYS_HOST_DIR + # (.env) at a HOST dir containing the private key as id_ed25519; the key + # then resolves to /keys/id_ed25519 inside this container (see the env + # below). Unset => an empty dir is mounted and no key path is set — no + # behavior change. + - ${GIT_CLONE_SSH_KEYS_HOST_DIR:-./.ssh-keys}:/keys:ro environment: # 0.0.0.0 = reachable on all interfaces. Set MCP_HOST=127.0.0.1 to bind # loopback only (e.g. when a reverse proxy / socat already fronts it). @@ -96,6 +103,16 @@ services: # this container — local sources live under /app/playground here. CHAT_DEPLOY: ${CHAT_DEPLOY:-false} ALLOWED_SOURCE_ROOTS: ${ALLOWED_SOURCE_ROOTS:-} + # Custom git clone servers (optional) — allowlist your own git host + # (e.g. a LAN Forgejo) beyond github.com/gitlab.com. ssh:// clones auth + # via the mounted key (below) or a full GIT_CLONE_SSH_COMMAND. + GIT_CLONE_EXTRA_HOSTS: ${GIT_CLONE_EXTRA_HOSTS:-} + # In the dockerized stack the key's in-container path is DERIVED from + # GIT_CLONE_SSH_KEYS_HOST_DIR (the host dir mounted at /keys above). + # GIT_CLONE_SSH_KEY_PATH is a host-run-MCP setting and is deliberately + # NOT passed through here — a host path would never resolve in-container. + GIT_CLONE_SSH_KEY_PATH: ${GIT_CLONE_SSH_KEYS_HOST_DIR:+/keys/id_ed25519} + GIT_CLONE_SSH_COMMAND: ${GIT_CLONE_SSH_COMMAND:-} depends_on: codebadger-postgres: condition: service_healthy diff --git a/docs/deployment.md b/docs/deployment.md index ad3921a..7384b3d 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -139,11 +139,13 @@ CHAT_DEPLOY=true # in .env (passed through to the container by compose) With `CHAT_DEPLOY=true` the MCP **refuses `source_type="local"`** and returns a message steering the caller to the safe inputs. What remains: -- **Git repos** — only `https://github.com/…` and `https://gitlab.com/…` are - accepted. The URL is checked twice (a literal `https:///` prefix *and* a - parsed-hostname allowlist) and rejects other hosts, non-`https` schemes - (`git://`, `ssh://`, `file://`), embedded credentials, ports, and look-alike - domains — so a repo URL can't be turned into an SSRF probe. +- **Git repos** — `https://github.com/…` / `https://gitlab.com/…` are accepted, + plus `ssh://…` on hosts the operator listed in `GIT_CLONE_EXTRA_HOSTS` + (see [Custom git servers](#custom-git-servers-git_clone_) below). The URL is + checked twice (a literal `https:///` prefix *and* a parsed-hostname + allowlist) and rejects other hosts, non-`https` schemes (`git://`, `file://`), + embedded credentials, ports, and look-alike domains — so a repo URL can't be + turned into an SSRF probe. - **Pasted snippets** — `source_type="snippet"` with the code in a `` tag; the language is validated/inferred and a mislabeled or ambiguous snippet is refused. Nothing touches the host filesystem. @@ -205,6 +207,59 @@ defaults — note a few differ in the shipped `docker-compose.yml` (called out b | `ALLOWED_SOURCE_ROOTS` | `` (empty) | `:`-separated allowlist of dirs local sources must canonically resolve within (as the MCP sees them, e.g. `/app/playground`). Empty = no allowlist. | | `GITHUB_TOKEN` | `` (empty) | PAT for cloning private repos (never embed it in the URL). | +### Custom git servers (`GIT_CLONE_*`) + +By default `generate_cpg` only clones from `https://github.com/…` / +`https://gitlab.com/…`. To also analyze code on your own git server (e.g. a +self-hosted **Forgejo**/**Gitea** on the LAN), allowlist it with +`GIT_CLONE_EXTRA_HOSTS` — no other change is needed; callers then pass the +repo URL (`ssh://git@192.168.152.14:3000//.git`) as +`source_path` with `source_type='github'`. Custom hosts are **ssh-only** — +http(s) clone URLs are rejected for them. + +```bash +# ','-separated host[:port] entries — the same in both modes below +GIT_CLONE_EXTRA_HOSTS=192.168.152.14:3000 + +# MCP run on the host (`python main.py`): key FILE on this host +GIT_CLONE_SSH_KEY_PATH=/abs/path/to/id_ed25519 + +# Full docker stack (`./scripts/deploy.sh`): HOST DIRECTORY containing the key +# as id_ed25519; compose mounts it read-only at /keys in the codebadger-mcp +# container, so the server sees /keys/id_ed25519 +GIT_CLONE_SSH_KEYS_HOST_DIR=/abs/path/to/keydir +``` + +| Variable | Default | Description | +|---|---|---| +| `GIT_CLONE_EXTRA_HOSTS` | `` (empty) | ','-separated `host[:port]` entries accepted in addition to github.com/gitlab.com (IPv6 goes in brackets, e.g. `[::1]:2222`). A bare host allows **any** port on it; `host:port` pins that port only. Allowlisted hosts are cloned over `ssh://` only; the built-in hosts keep their strict https-only, default-port-only posture unless an operator explicitly lists one here (e.g. `github.com:22` would enable ssh for github.com). | +| `GIT_CLONE_SSH_KEYS_HOST_DIR` | `` (empty) | **Dockerized stack only.** Host directory containing the private key (named `id_ed25519`); docker compose mounts it read-only at `/keys` in the `codebadger-mcp` container and the server resolves the key to the fixed in-container path `/keys/id_ed25519`. Unset, an empty dir is mounted and no key path is configured. | +| `GIT_CLONE_SSH_KEY_PATH` | `` (empty) | **Host-run MCP only — ignored by the dockerized stack** (a host path never resolves inside the `codebadger-mcp` container; do not set it in `.env`). Private key FILE for `ssh://` clones of a custom host. The clone also sets `-o BatchMode=yes -o StrictHostKeyChecking=accept-new`, so a missing key fails fast instead of hanging on a prompt, and the host key is recorded on first contact. | +| `GIT_CLONE_SSH_COMMAND` | `` (empty) | Full ssh command override (passed to git as `GIT_SSH_COMMAND` for the clone); takes precedence over both key settings. In the dockerized stack any key path it references must exist **inside the `codebadger-mcp` container** (e.g. `/keys/…`). | + +Notes: +- **Where does the clone run?** In the MCP process, so every path must make + sense *there*: host-run MCP → host filesystem; full docker stack → inside + the `codebadger-mcp` container. The two key variables above exist because of + this split: `GIT_CLONE_SSH_KEY_PATH` is a host path for host-run MCP, + while `GIT_CLONE_SSH_KEYS_HOST_DIR` is the compose bridge that maps a host + key dir onto the fixed container path `/keys` (hence the container always + sees `/keys/id_ed25519`). For a host-run MCP any key file name works + (`ssh -i` doesn't care); in the dockerized stack the key **must** be named + `id_ed25519` because the in-container path is fixed — or bypass it with + `GIT_CLONE_SSH_COMMAND`. +- Embedded credentials in the `source_path` URL are always rejected. For + github.com/gitlab.com private repos pass the PAT via the `github_token` + argument; it is injected into the clone URL and stripped from `.git/config` + after the clone. +- `ssh://` URLs may carry a username (`git@…`) but not a password; keys/agent + do the auth. scp-style `git@host:path` URLs are not accepted — use + `ssh://git@host[:port]/path` (it carries ports unambiguously). +- The allowlist still blocks every other host (alternate git hosts, look-alike + domains, cloud metadata endpoints, …), so the SSRF posture of + [docs/security.md](security.md) is unchanged — you are explicitly trusting + the hosts you list. + ### Memory & the Joern pool Three distinct memory knobs, easy to confuse — keep them straight: diff --git a/docs/security.md b/docs/security.md index f8a309d..ddca353 100644 --- a/docs/security.md +++ b/docs/security.md @@ -73,7 +73,7 @@ The numbered controls are the boundary checks; each is described below. | # | Boundary | Control | Where | |---|----------|---------|-------| | ① | Tool input → MCP | **Allowlist/format validation of every parameter**: `source_type`, `language` (whitelist), `codebase_hash` (`^[a-f0-9]{16}$`), `github_token` & `branch` (anti URL-/arg-injection, e.g. blocks `--upload-pack`), snippet `code`/`filename`/label, regex `pattern` (length + ReDoS shapes). | `src/utils/validators.py` | -| ①a | Repo URL → clone (**SSRF/undefined-clone prevention**) | **Strict allowlist on remote repos**: only `https://github.com/` or `https://gitlab.com/` (incl. `www.`). Enforced by **two independent gates** — a literal, case-sensitive `https:///` prefix match *and* a parsed-`hostname` allowlist — plus rejection of any non-`https` scheme (`git://`, `ssh://`, `file://`, …), embedded credentials (`user:tok@`), non-default ports, and whitespace/control chars. Blocks userinfo host-smuggling (`https://github.com@evil/…`), internal/metadata hosts, and look-alike domains. | `validators.py` (`validate_repo_url`) | +| ①a | Repo URL → clone (**SSRF/undefined-clone prevention**) | **Strict allowlist on remote repos**: only `https://github.com/` or `https://gitlab.com/` (incl. `www.`) by default. Enforced by **two independent gates** — a literal, case-sensitive `https:///` prefix match *and* a parsed-`hostname` allowlist — plus rejection of any non-`https` scheme (`git://`, `file://`, …), embedded credentials (`user:tok@`), non-default ports, and whitespace/control chars. Blocks userinfo host-smuggling (`https://github.com@evil/…`), internal/metadata hosts, and look-alike domains. **Operator extension**: `GIT_CLONE_EXTRA_HOSTS` (env) explicitly adds `host[:port]` entries which may also be cloned over `ssh://` (username but no password allowed; auth rides in `GIT_SSH_COMMAND`, never the URL) — everything else about the posture (exact-hostname match, no embedded credentials, control chars, ≥`/owner/repo` path) is unchanged, and the injected `github_token` for github.com/gitlab.com is stripped from `.git/config` after the clone. | `validators.py` (`validate_repo_url`), `services/git_manager.py` | | ①b | Snippet code → CPG | **Language validated *and* inferred.** Pasted code is supplied in `` tags (parsed by regex); the declared language must be supported, and a content-signal check **refuses an obviously mislabeled tag** or **ambiguous/undeclared** language — every refusal returns an actionable message rather than building a wrong-language CPG. | `validators.py` (`parse_snippet_blocks`, `validate_and_infer_snippet_language`) | | ② | Source staging | **Path confinement + symlink-safe copy.** Local paths must be absolute, are rejected if they contain null bytes/control chars, then `realpath`-canonicalized (collapsing `..` and resolving symlinks *before* any check) and screened against a system-dir denylist (`/etc`, `/proc`, `/sys`, `/root`, …). An optional `ALLOWED_SOURCE_ROOTS` allowlist hard-contains local sources to named roots. Snapshot reads confined with `realpath`+prefix / `commonpath`; the copy never dereferences symlinks whose target escapes the source tree. | `validators.py` (`resolve_host_path`), `core_tools.py` | | ②a | Deployment posture | **`CHAT_DEPLOY=true` disables `source_type='local'` entirely** so a chat-facing / multi-tenant MCP cannot read arbitrary host paths — callers must use an allowlisted repo URL or a pasted snippet. | `core_tools.py`, `config.py` | diff --git a/src/defaults.py b/src/defaults.py index 49eed77..5321f53 100644 --- a/src/defaults.py +++ b/src/defaults.py @@ -66,6 +66,23 @@ def resolve_redis_url() -> str: # generate_cpg: a chat-facing MCP must never expose arbitrary host filesystem # paths. Callers use a github.com/gitlab.com URL or a pasted snippet instead. CHAT_DEPLOY = False + +# --- Custom git clone servers (self-hosted Forgejo / Gitea / GitLab, ...) ---- +# Beyond the built-in github.com/gitlab.com https allowlist, an operator can +# allowlist their own git server(s) for cloning via generate_cpg. Addresses are +# configured through the environment so a LAN deployment needs no code changes: +# GIT_CLONE_EXTRA_HOSTS ','-separated `host[:port]` entries accepted in +# addition to github.com/gitlab.com. A bare host allows +# any port on it; `host:port` pins that port only. +# Custom hosts are cloned over ssh:// only +# (github.com/gitlab.com stay https-only). +# GIT_CLONE_SSH_KEY_PATH Private key for ssh:// clones of custom hosts. +# GIT_CLONE_SSH_COMMAND Full ssh command override (takes precedence over the +# key path; passed to git as GIT_SSH_COMMAND). +GIT_CLONE_EXTRA_HOSTS = "" +GIT_CLONE_SSH_KEY_PATH = "" +GIT_CLONE_SSH_COMMAND = "" + # Optional ':'-separated allowlist of host directory roots that source_type= # 'local' paths must canonically resolve within. Empty = no allowlist (the # denylist + symlink-resolving canonicalization in resolve_host_path still apply). diff --git a/src/services/git_manager.py b/src/services/git_manager.py index 5b27f05..baf33d9 100644 --- a/src/services/git_manager.py +++ b/src/services/git_manager.py @@ -1,5 +1,10 @@ """ -Git repository manager for cloning and managing GitHub repositories +Git repository manager for cloning and managing remote git repositories. + +Besides the built-in github.com/gitlab.com https allowlist, an operator can +allowlist custom git servers (e.g. a self-hosted Forgejo in the LAN, cloned +over ssh) via the GIT_CLONE_EXTRA_HOSTS / GIT_CLONE_SSH_* environment +variables — see src/defaults.py. """ import asyncio @@ -8,34 +13,17 @@ import re import shutil from typing import Dict, Optional -from urllib.parse import urlparse +from urllib.parse import quote, urlparse import git +from .. import defaults from ..exceptions import GitOperationError, ValidationError from ..utils.validators import validate_github_url logger = logging.getLogger(__name__) -def _mask_token_in_url(url: str) -> str: - """ - Mask authentication tokens in URLs for safe logging. - - Args: - url: URL that may contain a token - - Returns: - URL with token replaced by '***' - """ - # Pattern to match tokens in URLs: scheme://token@host/path - return re.sub( - r"(https?://)[^@\s]+@", - r"\1***@", - url - ) - - def _mask_token_in_text(text: str) -> str: """ Mask authentication tokens in error messages or logs. @@ -46,15 +34,33 @@ def _mask_token_in_text(text: str) -> str: Returns: Text with tokens masked """ - return re.sub( - r"(https?://)[^@\s]+@", - r"\1***@", - text - ) + return re.sub(r"(https?://)[^@\s]+@", r"\1***@", text) + + +def _ssh_clone_env() -> Dict[str, str]: + """Environment for an ssh:// clone of a custom git server. + + git invokes the command in GIT_SSH_COMMAND for every ssh remote. Default to + the operator's full override (GIT_CLONE_SSH_COMMAND) or build one from + GIT_CLONE_SSH_KEY_PATH. BatchMode keeps a missing key/passphrase from + hanging the clone on an interactive prompt; accept-new auto-records the + server's host key on first contact instead of blocking on confirmation. + """ + ssh_cmd = os.getenv("GIT_CLONE_SSH_COMMAND", defaults.GIT_CLONE_SSH_COMMAND).strip() + if not ssh_cmd: + parts = ["ssh"] + key_path = os.getenv( + "GIT_CLONE_SSH_KEY_PATH", defaults.GIT_CLONE_SSH_KEY_PATH + ).strip() + if key_path: + parts += ["-i", key_path] + parts += ["-o", "StrictHostKeyChecking=accept-new", "-o", "BatchMode=yes"] + ssh_cmd = " ".join(parts) + return dict(os.environ, GIT_SSH_COMMAND=ssh_cmd) class GitManager: - """Handles GitHub repository operations""" + """Handles remote git repository operations""" def __init__(self, workspace_root: str): self.workspace_root = workspace_root @@ -68,17 +74,30 @@ async def clone_repository( branch: Optional[str] = None, token: Optional[str] = None, ) -> str: - """Clone a GitHub repository""" + """Clone a repo (https for github/gitlab; ssh for custom hosts)""" try: - # Validate URL + # Validate URL (scheme/host allowlist, no embedded credentials, …) validate_github_url(repo_url) - # Parse URL and inject token if provided - if token: - parsed = urlparse(repo_url) - auth_url = f"{parsed.scheme}://{token}@{parsed.netloc}{parsed.path}" - else: - auth_url = repo_url + parsed = urlparse(repo_url) + auth_url = repo_url + clone_env: Optional[Dict[str, str]] = None + injected_credential = False + + if parsed.scheme in ("http", "https"): + # Built-in github.com/gitlab.com hosts only (the validator + # rejects every other host): the per-call token rides in the + # URL username and is stripped from .git/config afterwards. + if token: + auth_url = ( + f"{parsed.scheme}://{quote(token, safe='')}" + f"@{parsed.netloc}{parsed.path}" + ) + injected_credential = True + elif parsed.scheme == "ssh": + # ssh:// (custom GIT_CLONE_EXTRA_HOSTS server): auth comes from + # the key/agent via GIT_SSH_COMMAND, not from the URL. + clone_env = _ssh_clone_env() # Create target directory os.makedirs(target_path, exist_ok=True) @@ -87,12 +106,12 @@ async def clone_repository( # Clone in a thread pool (git operations are blocking) loop = asyncio.get_event_loop() await loop.run_in_executor( - None, self._do_clone, auth_url, source_path, branch + None, self._do_clone, auth_url, source_path, branch, clone_env ) # Remove the embedded credential from .git/config so the token is # not stored on disk in plaintext. - if token: + if injected_credential: await loop.run_in_executor( None, self._strip_remote_credential, source_path, repo_url ) @@ -108,13 +127,19 @@ async def clone_repository( logger.error(f"Failed to clone repository: {safe_error}") raise GitOperationError(f"Failed to clone repository: {safe_error}") - def _do_clone(self, url: str, target: str, branch: Optional[str]): + def _do_clone( + self, + url: str, + target: str, + branch: Optional[str], + env: Optional[Dict[str, str]] = None, + ): """Blocking clone operation""" try: if branch: - git.Repo.clone_from(url, target, branch=branch, depth=1) + git.Repo.clone_from(url, target, branch=branch, depth=1, env=env) else: - git.Repo.clone_from(url, target, depth=1) + git.Repo.clone_from(url, target, depth=1, env=env) except Exception as e: # Mask tokens in error messages safe_error = _mask_token_in_text(str(e)) diff --git a/src/tools/core_tools.py b/src/tools/core_tools.py index 7e19799..bcd3979 100644 --- a/src/tools/core_tools.py +++ b/src/tools/core_tools.py @@ -1714,13 +1714,16 @@ def register_core_tools(mcp, services: dict): The CPG is cached by a hash of the codebase. Accepted git repositories (source_type='github'): - - ONLY public/private repos on github.com or gitlab.com. - - The URL MUST be an https:// URL of the form: + - Public/private repos on github.com or gitlab.com via https:// URLs of the form: https://github.com// or https://gitlab.com// (gitlab nested subgroups are allowed; a trailing .git is fine). - - Other hosts, schemes (git://, ssh://, http://), embedded credentials, or - custom ports are rejected. Use github_token for a private repo, do NOT embed - the token in the URL. + - Repos on the server's CUSTOM git hosts, when the operator configured + GIT_CLONE_EXTRA_HOSTS (e.g. a self-hosted Forgejo). Those hosts accept + ssh:// URLs (with custom ports), e.g.: + ssh://git@192.168.152.14:3000//.git + - Embedded credentials in the URL are always rejected. Use github_token for a + private github.com/gitlab.com repo — do NOT embed the token in the URL. + (Custom hosts authenticate via the operator's ssh key, not a token.) Pasting code directly (source_type='snippet'): Wrap the code in a tag whose `language` attribute is one of the supported @@ -1746,10 +1749,13 @@ def register_core_tools(mcp, services: dict): This guard does NOT apply to GitHub URLs — size is unknown until cloned. Args: - source_type: One of 'local', 'github' (a github.com/gitlab.com repo), or 'snippet'. + source_type: One of 'local', 'github' (a github.com/gitlab.com repo, or a repo + on a configured GIT_CLONE_EXTRA_HOSTS server), or 'snippet'. source_path: REQUIRED for local (absolute path) and github (an https - github.com/gitlab.com URL). OPTIONAL for snippet — a short label; - when omitted the server derives one from the filename/language. + github.com/gitlab.com URL, or an ssh:// URL on a host the + operator allowlisted via GIT_CLONE_EXTRA_HOSTS). OPTIONAL for + snippet — a short label; when omitted the server derives one from + the filename/language. language: Programming language (java, c, cpp, python, javascript, go, etc.). REQUIRED for local/github. Optional for snippets that carry a tag (the tag wins) or whose language is inferable. @@ -1771,7 +1777,9 @@ def register_core_tools(mcp, services: dict): - This is an async operation. Use get_cpg_status to check progress. - Large codebases may take several minutes to analyze. - Supported languages: c, cpp, java, javascript, python, go, kotlin, csharp, php, ruby, swift. - - Git repos: only https://github.com/... and https://gitlab.com/... are accepted. + - Git repos: only https://github.com/... and https://gitlab.com/... are + accepted, plus ssh:// URLs on hosts the operator configured via the + GIT_CLONE_EXTRA_HOSTS environment variable (e.g. a LAN Forgejo). Examples: generate_cpg( @@ -1787,11 +1795,11 @@ def register_core_tools(mcp, services: dict): ) async def generate_cpg( source_type: Annotated[str, Field(description="One of 'local', 'github', or 'snippet' (code pasted directly into the chat)")], - source_path: Annotated[Optional[str], Field(description="REQUIRED for local (absolute path to source directory) and github (an https URL on github.com or gitlab.com ONLY, e.g. https://github.com/user/repo — other hosts/schemes/credentials/ports are rejected). OPTIONAL for snippet: a short human label for the pasted code (e.g. a function name); when omitted the server derives one from the filename/language.")] = None, + source_path: Annotated[Optional[str], Field(description="REQUIRED for local (absolute path to source directory) and github (an https URL on github.com or gitlab.com ONLY, e.g. https://github.com/user/repo; additionally ssh:// URLs on hosts the operator allowlisted via GIT_CLONE_EXTRA_HOSTS, e.g. ssh://git@192.168.152.14:3000/user/repo.git — embedded credentials are always rejected). OPTIONAL for snippet: a short human label for the pasted code (e.g. a function name); when omitted the server derives one from the filename/language.")] = None, language: Annotated[str, Field(description="Programming language - one of: java, c, cpp, javascript, python, go, kotlin, csharp, ghidra, jimple, php, ruby, swift. REQUIRED for local/github. For a snippet whose code carries a tag, the tag's language wins and this is optional.")] = "", code: Annotated[Optional[str], Field(description="Required when source_type='snippet'. Wrap the code in a ... tag where LANG is a supported language id, e.g. int main(){...}. Multiple blocks are concatenated but must share one language. Ignored for local/github.")] = None, filename: Annotated[Optional[str], Field(description="Optional filename for a snippet (e.g. 'parser.c'); defaults to snippet. from the language. Ignored for local/github.")] = None, - github_token: Annotated[Optional[str], Field(description="GitHub Personal Access Token for private repositories (optional)")] = None, + github_token: Annotated[Optional[str], Field(description="Access token for private github.com/gitlab.com repositories (optional; sent as the clone URL username). Custom GIT_CLONE_EXTRA_HOSTS servers authenticate via the operator's ssh key instead — a token is not used there. Never embed the token in the URL.")] = None, branch: Annotated[Optional[str], Field(description="Specific git branch to checkout (optional, defaults to default branch)")] = None, force: Annotated[bool, Field(description="Skip the large-project size warning. Set to True only after the user has explicitly confirmed they want to analyze the full project.")] = False, include_paths: Annotated[Optional[list], Field(description="C/C++ only: extra header include directories for c2cpg (--include). Relative paths resolve against the source root (e.g. 'include', '_build/include'); absolute paths pass through. Use when a project's generated headers (e.g. a configure/cmake-produced xmlversion.h or config.h) gate code behind feature macros — the source root, any include/ dir, and dirs containing config.h/*version*.h are auto-detected, so this is only needed for non-standard layouts.")] = None, @@ -1827,7 +1835,8 @@ async def generate_cpg( if _cfg and getattr(_cfg.server, "chat_deploy", False): raise ValidationError( "source_type='local' is disabled in this deployment. Provide a " - "github.com or gitlab.com repository URL with source_type='github', " + "github.com or gitlab.com repository URL (or one on a configured " + "GIT_CLONE_EXTRA_HOSTS server) with source_type='github', " "or paste the code with source_type='snippet'." ) # For snippets the code may be wrapped in tags; diff --git a/src/utils/__init__.py b/src/utils/__init__.py index 0df579e..094952f 100644 --- a/src/utils/__init__.py +++ b/src/utils/__init__.py @@ -10,9 +10,9 @@ validate_codebase_hash, validate_cpgql_query, validate_github_url, - validate_repo_url, validate_language, validate_local_path, + validate_repo_url, validate_search_pattern, validate_source_type, validate_timeout, diff --git a/src/utils/validators.py b/src/utils/validators.py index 779d531..695e76a 100644 --- a/src/utils/validators.py +++ b/src/utils/validators.py @@ -5,9 +5,10 @@ import hashlib import os import re -from typing import Optional -from urllib.parse import urlparse +from typing import Dict, Optional +from urllib.parse import ParseResult, urlparse +from .. import defaults from ..exceptions import ValidationError from ..models import SourceType @@ -63,9 +64,10 @@ def validate_codebase_hash(codebase_hash: str) -> None: -# Only these hosts may be cloned. Anything else — alternate git hosts, raw IPs, -# localhost, cloud metadata endpoints (169.254.169.254), etc. — is rejected so a -# repo URL can't be turned into an SSRF probe or an undefined-behavior clone. +# Only these hosts may be cloned by default. Anything else — alternate git hosts, +# raw IPs, localhost, cloud metadata endpoints (169.254.169.254), etc. — is +# rejected so a repo URL can't be turned into an SSRF probe or an +# undefined-behavior clone. ALLOWED_REPO_HOSTS = frozenset( {"github.com", "www.github.com", "gitlab.com", "www.gitlab.com"} ) @@ -79,22 +81,119 @@ def validate_codebase_hash(codebase_hash: str) -> None: sorted(f"https://{host}/" for host in ALLOWED_REPO_HOSTS) ) +# Operators can extend the allowlist with their own git servers (e.g. a LAN +# Forgejo) via GIT_CLONE_EXTRA_HOSTS — see src/defaults.py. Hostnames / IPv4, +# optionally with a port; IPv6 goes in brackets. Used as a building block for +# both the entry syntax below and as a matcher against parsed URL hostnames. +_EXTRA_HOST_ENTRY_RE = re.compile(r"^[A-Za-z0-9._-]+$") +_IPV6_HOST_RE = re.compile(r"^[A-Fa-f0-9:]+$") + + +def _extra_repo_host_entries() -> Dict[str, Optional[set]]: + """Parse GIT_CLONE_EXTRA_HOSTS into ``{hostname: ports | None}``. + + Each entry is ``host`` or ``host:port`` (comma-separated). ``None`` for a + bare host means any port is allowed on it; a set pins the listed ports. + A malformed entry raises ValidationError so a typo'd config fails loudly + instead of silently widening (or narrowing) the allowlist. + """ + raw = os.getenv("GIT_CLONE_EXTRA_HOSTS", defaults.GIT_CLONE_EXTRA_HOSTS) + hosts: Dict[str, Optional[set]] = {} + for entry in raw.split(","): + entry = entry.strip() + if not entry: + continue + if "/" in entry or "@" in entry or "?" in entry or "#" in entry: + raise ValidationError( + f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' " + "(expected 'host' or 'host:port')" + ) + if entry.startswith("["): # [ipv6] or [ipv6]:port + host, closed, suffix = entry[1:].partition("]") + if not closed: + raise ValidationError( + f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (unclosed ']')" + ) + port_str = "" + if suffix: + if not suffix.startswith(":"): + raise ValidationError( + f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' " + "(expected '[host]:port')" + ) + port_str = suffix[1:] + if not _IPV6_HOST_RE.match(host): + raise ValidationError( + f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad IPv6 host)" + ) + elif ":" in entry: + host, port_str = entry.rsplit(":", 1) + if not _EXTRA_HOST_ENTRY_RE.match(host): + raise ValidationError( + f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad host)" + ) + else: + host, port_str = entry, "" + if not _EXTRA_HOST_ENTRY_RE.match(host): + raise ValidationError( + f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad host)" + ) + host = host.lower() + ports: Optional[set] = None + if port_str: + if not port_str.isdigit() or not (1 <= int(port_str) <= 65535): + raise ValidationError( + f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad port)" + ) + ports = {int(port_str)} + if host in hosts: + # A second entry for the same host either widens to any port or + # unions the ports, depending on whether it carries one. + existing = hosts[host] + if existing is None or ports is None: + hosts[host] = None + else: + hosts[host] = existing | ports + else: + hosts[host] = ports + return hosts + + +def is_extra_repo_host(hostname: Optional[str], port: Optional[int]) -> bool: + """True when hostname/port match an operator-configured GIT_CLONE_EXTRA_HOSTS + entry (bare host = any port; `host:port` = that port only).""" + if not hostname: + return False + entries = _extra_repo_host_entries() + key = hostname.lower() + if key not in entries: + return False + allowed_ports = entries[key] + return allowed_ports is None or (port is not None and port in allowed_ports) + def validate_repo_url(url: str) -> bool: - """Strictly validate a remote git repository URL (github.com / gitlab.com). - - Hardened against SSRF and undefined clone behavior. The URL MUST: - * be a string with no whitespace or control characters, - * use the ``https`` scheme (rejects ``git://``, ``ssh://``, ``http://``, - ``file://``, ``data:``, scp-style ``git@host:path``, …), - * carry no embedded credentials (``https://user:tok@…`` is rejected so the - host can't be smuggled past the allowlist via the userinfo field), - * resolve to an exact allowlisted host (``parsed.hostname`` is lowercased - and excludes userinfo/port, so ``github.com@evil.com`` → host ``evil.com`` - → rejected), - * use no non-default port, - * have an ``/owner/repo`` path (gitlab subgroups, i.e. extra segments, are - allowed). + """Strictly validate a remote git repository URL. + + Hardened against SSRF and undefined clone behavior. Accepted URLs: + * ``https://github.com/…`` / ``https://gitlab.com/…`` — the built-in + default: https only, default port only, no embedded credentials; + * ``ssh://[user@]/…`` where ```` is listed in + the operator's ``GIT_CLONE_EXTRA_HOSTS`` (e.g. a self-hosted Forgejo at + ``ssh://git@192.168.152.14:3000``). Custom hosts are ssh-only (no + http(s) cloning), may carry a port (a ``host:port`` entry pins it; a + bare entry allows any), and the URL may carry a username (``git@``) + but no password. + + For every URL, regardless of host: + * no whitespace or control characters, + * no embedded credentials in http(s) URLs (the host can't be smuggled + past the allowlist via the userinfo field), + * hostname matches the allowlist EXACTLY (``parsed.hostname`` is + lowercased and excludes userinfo/port, so ``github.com@evil.com`` → + host ``evil.com`` → rejected; ``github.com.evil.com`` is a different + host and rejected too), + * an ``/owner/repo`` path (nested subgroups / extra segments are fine). """ if not url or not isinstance(url, str): raise ValidationError("Repository URL must be a non-empty string") @@ -105,50 +204,76 @@ def validate_repo_url(url: str) -> bool: "Repository URL must not contain whitespace or control characters" ) - # Literal prefix gate: the string must START with an exact allowed - # `https:///` prefix. Belt-and-suspenders with the parsed hostname - # check below — the literal match is case-sensitive and rejects anything - # that isn't canonically lowercase https://github.com/ or https://gitlab.com/. - if not url.startswith(ALLOWED_REPO_URL_PREFIXES): - raise ValidationError( - "Repository URL must start with one of: " - + ", ".join(ALLOWED_REPO_URL_PREFIXES) - ) - try: parsed = urlparse(url) except Exception as e: raise ValidationError(f"Invalid repository URL: {e}") - if parsed.scheme != "https": + try: + port = parsed.port + except ValueError: + raise ValidationError("Repository URL has an invalid port") + + scheme = parsed.scheme.lower() + + if scheme in ("http", "https"): + if parsed.username or parsed.password: + raise ValidationError( + "Repository URL must not contain embedded credentials" + ) + if parsed.hostname in ALLOWED_REPO_HOSTS: + # Built-in hosts keep the strictest posture: https, default port, + # and a canonical lowercase literal `https:///` prefix + # (belt-and-suspenders with the parsed hostname check — also + # rejects userinfo smuggling and ports before any parsing). + if scheme != "https": + raise ValidationError( + f"Repository URL must use https:// (got '{scheme}')" + ) + if port is not None and port != 443: + raise ValidationError( + "Repository URL must not specify a non-default port" + ) + if not url.startswith(ALLOWED_REPO_URL_PREFIXES): + raise ValidationError( + "Repository URL must start with one of: " + + ", ".join(ALLOWED_REPO_URL_PREFIXES) + ) + return _validate_repo_url_path(parsed) raise ValidationError( - f"Repository URL must use https:// (got '{parsed.scheme or 'no scheme'}')" + "Only github.com and gitlab.com repositories are supported over " + "http(s) (clone a GIT_CLONE_EXTRA_HOSTS server over ssh:// instead; " + f"got host '{parsed.hostname}')" ) - if parsed.username or parsed.password: - raise ValidationError("Repository URL must not contain embedded credentials") - - if parsed.hostname not in ALLOWED_REPO_HOSTS: + if scheme == "ssh": + if parsed.password: + raise ValidationError( + "ssh repository URLs must not contain an embedded password" + ) + if parsed.username and not re.fullmatch(r"[A-Za-z0-9._-]+", parsed.username): + raise ValidationError("Invalid username in ssh repository URL") + if is_extra_repo_host(parsed.hostname, port): + return _validate_repo_url_path(parsed) raise ValidationError( - "Only github.com and gitlab.com repositories are supported " - f"(got host '{parsed.hostname}')" + "ssh:// repository URLs are only supported for hosts configured " + f"in GIT_CLONE_EXTRA_HOSTS (got host '{parsed.hostname}')" ) - try: - port = parsed.port - except ValueError: - raise ValidationError("Repository URL has an invalid port") - if port is not None and port != 443: - raise ValidationError("Repository URL must not specify a non-default port") + raise ValidationError( + f"Repository URL must use https:// (or ssh:// for a host listed " + f"in GIT_CLONE_EXTRA_HOSTS); got '{scheme or 'no scheme'}'" + ) + - # Path must be at least /owner/repo. +def _validate_repo_url_path(parsed: ParseResult) -> bool: + """Path check shared by every accepted scheme: at least /owner/repo.""" parts = [p for p in parsed.path.strip("/").split("/") if p] if len(parts) < 2: raise ValidationError( - "Invalid repository URL. Expected https://github.com/owner/repo " - "or https://gitlab.com/owner/repo" + "Invalid repository URL. Expected https://github.com/owner/repo, " + "https://gitlab.com/owner/repo, or ssh:///owner/repo" ) - return True diff --git a/tests/test_git_manager.py b/tests/test_git_manager.py new file mode 100644 index 0000000..c908291 --- /dev/null +++ b/tests/test_git_manager.py @@ -0,0 +1,127 @@ +"""Unit tests for GitManager cloning from custom git servers. + +Covers the GIT_CLONE_* env configuration: ssh:// clones of custom hosts driven +through GIT_SSH_COMMAND, and the default-host posture (github.com/gitlab.com +https + per-call token) staying exactly as strict as before. Custom +GIT_CLONE_EXTRA_HOSTS servers are ssh-only — http(s) clone URLs are rejected. +""" + +import os + +import git +import pytest + +from src.exceptions import ValidationError +from src.services.git_manager import ( + GitManager, + _ssh_clone_env, +) + + +@pytest.fixture(autouse=True) +def _clean_git_clone_env(monkeypatch): + """Start every test from an empty GIT_CLONE_* configuration.""" + for var in ( + "GIT_CLONE_EXTRA_HOSTS", + "GIT_CLONE_SSH_KEY_PATH", + "GIT_CLONE_SSH_COMMAND", + ): + monkeypatch.delenv(var, raising=False) + + +class TestSshCloneEnv: + """GIT_SSH_COMMAND construction for ssh:// clones of custom hosts.""" + + def test_default_command(self): + env = _ssh_clone_env() + cmd = env["GIT_SSH_COMMAND"] + assert cmd.startswith("ssh ") + # Never hang on a prompt: batch mode + auto-accept the host key. + assert "BatchMode=yes" in cmd + assert "StrictHostKeyChecking=accept-new" in cmd + + def test_key_path_injected(self, monkeypatch): + monkeypatch.setenv("GIT_CLONE_SSH_KEY_PATH", "/keys/id_ed25519") + cmd = _ssh_clone_env()["GIT_SSH_COMMAND"] + assert "-i /keys/id_ed25519" in cmd + + def test_full_command_override_wins(self, monkeypatch): + monkeypatch.setenv("GIT_CLONE_SSH_KEY_PATH", "/keys/ignored") + monkeypatch.setenv("GIT_CLONE_SSH_COMMAND", "ssh -i /other/key -p 2222") + assert _ssh_clone_env()["GIT_SSH_COMMAND"] == "ssh -i /other/key -p 2222" + + def test_env_carries_os_environ(self, monkeypatch): + monkeypatch.setenv("CODEBADGER_TEST_MARKER", "1") + assert _ssh_clone_env().get("CODEBADGER_TEST_MARKER") == "1" + + +class TestCloneRepository: + """clone_repository URL/env behavior (git itself is mocked out).""" + + @pytest.fixture + def recorder(self, monkeypatch): + """Capture git.Repo.clone_from calls instead of cloning.""" + calls = [] + stripped = [] + + def fake_clone_from(url, target, **kwargs): + os.makedirs(target, exist_ok=True) + calls.append({"url": url, "target": target, **kwargs}) + + monkeypatch.setattr(git.Repo, "clone_from", staticmethod(fake_clone_from)) + monkeypatch.setattr( + GitManager, + "_strip_remote_credential", + lambda self, repo_path, clean_url: stripped.append((repo_path, clean_url)), + ) + return {"calls": calls, "stripped": stripped} + + async def test_github_token_injection_unchanged(self, tmp_path, recorder): + # Historic behavior: https://@github.com/... + post-clone strip. + manager = GitManager(str(tmp_path)) + source = await manager.clone_repository( + "https://github.com/user/repo", str(tmp_path / "cb"), token="ghp_abc" + ) + assert source.endswith("/source") + assert len(recorder["calls"]) == 1 + assert recorder["calls"][0]["url"] == "https://ghp_abc@github.com/user/repo" + assert recorder["stripped"] == [(source, "https://github.com/user/repo")] + + async def test_ssh_clone_uses_ssh_command_env( + self, tmp_path, recorder, monkeypatch + ): + monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", "192.168.152.14:3000") + monkeypatch.setenv("GIT_CLONE_SSH_KEY_PATH", "/keys/id_ed25519") + manager = GitManager(str(tmp_path)) + url = "ssh://git@192.168.152.14:3000/ethan/demo.git" + await manager.clone_repository(url, str(tmp_path / "cb"), branch="dev") + call = recorder["calls"][0] + # URL is passed through untouched; auth rides in GIT_SSH_COMMAND. + assert call["url"] == url + assert call["branch"] == "dev" + assert "-i /keys/id_ed25519" in call["env"]["GIT_SSH_COMMAND"] + # No URL credential was injected, so nothing to strip. + assert recorder["stripped"] == [] + + async def test_custom_host_http_url_rejected_before_clone( + self, tmp_path, recorder, monkeypatch + ): + # Custom hosts are ssh-only: even an allowlisted host can't be cloned + # over http(s). + monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", "192.168.152.14:3000") + manager = GitManager(str(tmp_path)) + for url in ( + "http://192.168.152.14:3000/ethan/demo.git", + "https://192.168.152.14:3000/ethan/demo", + ): + with pytest.raises(ValidationError): + await manager.clone_repository(url, str(tmp_path / "cb")) + assert recorder["calls"] == [] + + async def test_off_allowlist_url_rejected_before_clone(self, tmp_path, recorder): + manager = GitManager(str(tmp_path)) + with pytest.raises(ValidationError): + await manager.clone_repository( + "http://192.168.152.14:3000/ethan/demo.git", str(tmp_path / "cb") + ) + assert recorder["calls"] == [] diff --git a/tests/test_validators.py b/tests/test_validators.py index 565d19b..8de86bd 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -277,6 +277,123 @@ def test_literal_prefix_gate(self): validate_github_url(url) +class TestExtraGitHosts: + """GIT_CLONE_EXTRA_HOSTS: operator-allowlisted custom git servers. + + github.com/gitlab.com keep their strict https posture; allowlisted hosts + (e.g. a LAN Forgejo at ssh://git@192.168.152.14:3000) may additionally be + cloned over ssh:// with custom ports — http(s) stays rejected for them. + """ + + HOSTS = "192.168.152.14:3000,git.example.com,[::1]:2222" + + @pytest.fixture(autouse=True) + def _set_hosts(self, monkeypatch): + monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", self.HOSTS) + + def test_custom_http_and_https_urls_rejected(self): + """Custom hosts are ssh-only; http(s) never reaches a clone.""" + for url in [ + "http://192.168.152.14:3000/ethan/demo.git", + "https://192.168.152.14:3000/ethan/demo", + "http://git.example.com/ethan/demo", + "https://git.example.com:8443/ethan/demo", + "http://[::1]:2222/ethan/demo", + ]: + with pytest.raises(ValidationError): + validate_github_url(url) + + def test_ssh_urls_accepted_for_custom_hosts(self): + for url in [ + "ssh://git@192.168.152.14:3000/ethan/demo.git", + "ssh://192.168.152.14:3000/ethan/demo", # username optional + "ssh://git@git.example.com/ethan/demo", + "ssh://git@[::1]:2222/ethan/demo", + ]: + validate_github_url(url) + + def test_port_pinning_enforced(self): + """A `host:port` entry only allows that port on that host.""" + for url in [ + "ssh://git@192.168.152.14:3001/ethan/demo", # wrong port, pinned host + "ssh://git@[::1]:22/ethan/demo", # wrong port, pinned host + "ssh://git@192.168.152.14/ethan/demo", # no port, pinned host + ]: + with pytest.raises(ValidationError): + validate_github_url(url) + + def test_ssh_not_enabled_for_default_hosts(self): + """ssh:// stays rejected for github.com/gitlab.com.""" + for url in [ + "ssh://git@github.com/user/repo", + "ssh://git@gitlab.com/user/repo", + ]: + with pytest.raises(ValidationError): + validate_github_url(url) + + def test_hardening_unchanged_for_custom_hosts(self): + """The SSRF posture applies to allowlisted hosts verbatim.""" + for url in [ + "ssh://git@192.168.152.14:3000/user", # path < /owner/repo + "ssh://user:tok@192.168.152.14:3000/ethan/demo", # embedded credentials + "ssh://evil.com/ethan/demo", # not allowlisted + "ssh://git@192.168.152.14.evil.com:3000/ethan/demo", # look-alike host + "git://192.168.152.14:3000/ethan/demo", # git protocol + "ssh://git:pw@192.168.152.14:3000/ethan/demo", # password in ssh URL + "ssh://bad user@192.168.152.14:3000/ethan/demo", # bad ssh username + "ssh://git@192.168.152.14:3000/ethan/demo\n.git", # control chars + ]: + with pytest.raises(ValidationError): + validate_github_url(url) + + def test_default_hosts_unchanged_when_extra_hosts_set(self): + """Setting the extension doesn't relax the built-in allowlist.""" + with pytest.raises(ValidationError): + validate_github_url("http://github.com/user/repo") # still https-only + + def test_malformed_entries_fail_loudly(self, monkeypatch): + """A typo'd GIT_CLONE_EXTRA_HOSTS must surface, not silently apply.""" + from src.utils.validators import validate_repo_url + + for cfg in [ + "192.168.1.1:0", # port out of range (low) + "host:99999", # port out of range (high) + "host:notaport", # non-numeric port + "[::1", # unclosed IPv6 bracket + "host/path", # path in an entry + "user@host", # userinfo in an entry + ":3000", # empty host + ]: + monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", cfg) + # An ssh URL is what drives the entry parse for a custom host. + with pytest.raises(ValidationError): + validate_repo_url("ssh://git@host.example/a/b") + + def test_is_extra_repo_host(self): + from src.utils.validators import is_extra_repo_host + + assert is_extra_repo_host("192.168.152.14", 3000) is True + assert is_extra_repo_host("192.168.152.14", None) is False # pinned port + assert is_extra_repo_host("git.example.com", 8443) is True # bare host + assert is_extra_repo_host("git.example.com", None) is True + assert is_extra_repo_host("::1", 2222) is True + assert is_extra_repo_host("evil.com", 80) is False + assert is_extra_repo_host(None, 80) is False + + def test_empty_config_rejects_everything_extra(self, monkeypatch): + """Without GIT_CLONE_EXTRA_HOSTS the default posture is unchanged.""" + monkeypatch.delenv("GIT_CLONE_EXTRA_HOSTS", raising=False) + for url in [ + "http://192.168.152.14:3000/a/b", + "https://192.168.152.14:3000/a/b", + "ssh://git@192.168.152.14:3000/a/b", + ]: + with pytest.raises(ValidationError): + validate_github_url(url) + # ... while the built-in hosts keep working. + validate_github_url("https://github.com/user/repo") + + class TestParseSnippetBlocks: """ snippet extraction.""" From 55631a8ce865c39cb932a7083e9b635a190101b4 Mon Sep 17 00:00:00 2001 From: Ahmed Lekssays Date: Mon, 31 Aug 2026 14:12:22 +0300 Subject: [PATCH 2/2] harden the GIT_CLONE_EXTRA_HOSTS allowlist (review follow-ups) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on #28. The default posture (no GIT_CLONE_EXTRA_HOSTS) remains byte-identical to main; these tighten the opt-in extension. - Ports are part of the allowlist. A bare `host` entry now means port 22 only instead of any port, so allowlisting a git server no longer lets a caller who can influence source_path reach every other port on that machine over ssh. `host:port` pins a port as before; the new `host:*` opts into any port explicitly. - Reject port 0 on ssh URLs (urlparse only range-checks the upper bound, so it slipped past the entry parser's own 1..65535 check). - A portless ssh URL now matches against 22 — what a clone would actually dial — so a natural `forge.lan:22` entry no longer rejects `ssh://git@forge.lan/owner/repo`. - Parse GIT_CLONE_EXTRA_HOSTS at startup so a typo fails the boot instead of sitting latent until the first ssh clone, and redact the raw config from the error an MCP caller sees (it is logged for the operator). - ssh usernames must start alphanumeric: a leading '-' reaches ssh's argv as an option. git blocks this downstream; match the file's posture. - Normalize IPv6 hosts through `ipaddress`, so `[::1]` and `[0:0:0:0:0:0:0:1]` are one entry and IPv4-mapped forms parse. - _ssh_clone_env returns only GIT_SSH_COMMAND. GitPython layers env over os.environ, so copying the whole environment (POSTGRES_PASSWORD, GITHUB_TOKEN, JOERN_SERVER_AUTH_PASSWORD) into the Git object was needless secret spread. Shell-quote the key path while here. - New GIT_CLONE_SSH_KNOWN_HOSTS pins the server host key (StrictHostKeyChecking=yes). Unset keeps accept-new, now documented as trust-on-first-use per deploy in the dockerized stack, where the record dies with the container. - Track .ssh-keys/ via .gitkeep so compose doesn't create the default bind mount root-owned in the working tree. - docs/security.md: scope the "two independent gates" claim to the built-in https hosts; the ssh path is the hostname gate plus the port. Co-Authored-By: Claude Opus 5 (1M context) --- .env.example | 13 +++- .gitignore | 6 +- .ssh-keys/.gitkeep | 3 + docker-compose.yml | 5 ++ docs/deployment.md | 13 +++- docs/security.md | 2 +- main.py | 7 ++- src/defaults.py | 14 +++-- src/services/git_manager.py | 35 +++++++++-- src/utils/__init__.py | 2 + src/utils/validators.py | 117 ++++++++++++++++++++++++++++-------- tests/test_git_manager.py | 23 ++++++- tests/test_validators.py | 64 ++++++++++++++++++-- 13 files changed, 251 insertions(+), 53 deletions(-) create mode 100644 .ssh-keys/.gitkeep diff --git a/.env.example b/.env.example index 5ef4d8d..76469ba 100644 --- a/.env.example +++ b/.env.example @@ -84,9 +84,10 @@ CPG_QUEUE_BACKEND=durable # Custom git clone servers (optional) — allow your own git host beyond # github.com/gitlab.com, e.g. a self-hosted Forgejo on the LAN. -# GIT_CLONE_EXTRA_HOSTS: ','-separated `host[:port]` entries. A bare host allows -# any port on it; `host:port` pins that port only. Listed hosts are cloned via -# ssh:// only (github.com/gitlab.com stay https-only). +# GIT_CLONE_EXTRA_HOSTS: ','-separated `host[:port]` entries. A bare host means +# port 22 only; `host:port` pins that port; `host:*` allows any port on it. +# Listed hosts are cloned via ssh:// only (github.com/gitlab.com stay +# https-only). A malformed value fails the server boot. # ssh:// auth depends on how the MCP runs (the clone runs where the MCP runs): # - MCP on the host (`python main.py`): set GIT_CLONE_SSH_KEY_PATH to the key # FILE on this host, or a full GIT_CLONE_SSH_COMMAND override. @@ -102,9 +103,15 @@ CPG_QUEUE_BACKEND=durable # GIT_CLONE_SSH_KEY_PATH=/path/to/id_ed25519 # # dockerized stack: # GIT_CLONE_SSH_KEYS_HOST_DIR=/path/to/keydir # containing id_ed25519 +# Host keys: without GIT_CLONE_SSH_KNOWN_HOSTS the clone uses +# StrictHostKeyChecking=accept-new (trust-on-first-use — and in the dockerized +# stack that record dies with the container, so it is TOFU on every recreate). +# Point it at a known_hosts file to pin the server key instead; in the docker +# stack that path must be IN-CONTAINER, e.g. /keys/known_hosts next to the key. # GIT_CLONE_EXTRA_HOSTS= # GIT_CLONE_SSH_KEYS_HOST_DIR= # GIT_CLONE_SSH_KEY_PATH= +# GIT_CLONE_SSH_KNOWN_HOSTS= # GIT_CLONE_SSH_COMMAND= DOCKER_HOST=unix:///var/run/docker.sock diff --git a/.gitignore b/.gitignore index 6cb02c4..4643887 100644 --- a/.gitignore +++ b/.gitignore @@ -147,5 +147,7 @@ scripts/backfill_overlays.sh .claude/settings.json # Operator ssh keys for GIT_CLONE_EXTRA_HOSTS clones (compose default host dir). -# Never commit private keys. -.ssh-keys/ +# Never commit private keys. The dir itself is tracked (via .gitkeep) so compose +# doesn't create it root-owned when GIT_CLONE_SSH_KEYS_HOST_DIR is unset. +.ssh-keys/* +!.ssh-keys/.gitkeep diff --git a/.ssh-keys/.gitkeep b/.ssh-keys/.gitkeep new file mode 100644 index 0000000..f7b9b79 --- /dev/null +++ b/.ssh-keys/.gitkeep @@ -0,0 +1,3 @@ +# Default mount source for GIT_CLONE_SSH_KEYS_HOST_DIR (docker-compose). +# Place an operator deploy key here as id_ed25519 (and optionally known_hosts). +# Everything in this directory except this file is gitignored. diff --git a/docker-compose.yml b/docker-compose.yml index 364e57e..8db9c84 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -112,6 +112,11 @@ services: # GIT_CLONE_SSH_KEY_PATH is a host-run-MCP setting and is deliberately # NOT passed through here — a host path would never resolve in-container. GIT_CLONE_SSH_KEY_PATH: ${GIT_CLONE_SSH_KEYS_HOST_DIR:+/keys/id_ed25519} + # Optional host-key pinning. Like GIT_CLONE_SSH_COMMAND this is an + # IN-CONTAINER path: drop a known_hosts next to the key and set + # GIT_CLONE_SSH_KNOWN_HOSTS=/keys/known_hosts. Unset => accept-new, whose + # record lives in the container and is lost on every recreate. + GIT_CLONE_SSH_KNOWN_HOSTS: ${GIT_CLONE_SSH_KNOWN_HOSTS:-} GIT_CLONE_SSH_COMMAND: ${GIT_CLONE_SSH_COMMAND:-} depends_on: codebadger-postgres: diff --git a/docs/deployment.md b/docs/deployment.md index 7384b3d..50d3adf 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -218,7 +218,8 @@ repo URL (`ssh://git@192.168.152.14:3000//.git`) as http(s) clone URLs are rejected for them. ```bash -# ','-separated host[:port] entries — the same in both modes below +# ','-separated host[:port] entries — the same in both modes below. +# A bare host means port 22; `host:port` pins that port; `host:*` allows any. GIT_CLONE_EXTRA_HOSTS=192.168.152.14:3000 # MCP run on the host (`python main.py`): key FILE on this host @@ -232,9 +233,10 @@ GIT_CLONE_SSH_KEYS_HOST_DIR=/abs/path/to/keydir | Variable | Default | Description | |---|---|---| -| `GIT_CLONE_EXTRA_HOSTS` | `` (empty) | ','-separated `host[:port]` entries accepted in addition to github.com/gitlab.com (IPv6 goes in brackets, e.g. `[::1]:2222`). A bare host allows **any** port on it; `host:port` pins that port only. Allowlisted hosts are cloned over `ssh://` only; the built-in hosts keep their strict https-only, default-port-only posture unless an operator explicitly lists one here (e.g. `github.com:22` would enable ssh for github.com). | +| `GIT_CLONE_EXTRA_HOSTS` | `` (empty) | ','-separated `host[:port]` entries accepted in addition to github.com/gitlab.com (IPv6 goes in brackets, e.g. `[::1]:2222`). A bare host means **port 22 only**; `host:port` pins that port; `host:*` allows any port on it. Allowlisted hosts are cloned over `ssh://` only; the built-in hosts keep their strict https-only, default-port-only posture unless an operator explicitly lists one here (e.g. `github.com:22` would enable ssh for github.com). Parsed once at startup — a malformed value fails the boot rather than surfacing on the first ssh clone. | | `GIT_CLONE_SSH_KEYS_HOST_DIR` | `` (empty) | **Dockerized stack only.** Host directory containing the private key (named `id_ed25519`); docker compose mounts it read-only at `/keys` in the `codebadger-mcp` container and the server resolves the key to the fixed in-container path `/keys/id_ed25519`. Unset, an empty dir is mounted and no key path is configured. | -| `GIT_CLONE_SSH_KEY_PATH` | `` (empty) | **Host-run MCP only — ignored by the dockerized stack** (a host path never resolves inside the `codebadger-mcp` container; do not set it in `.env`). Private key FILE for `ssh://` clones of a custom host. The clone also sets `-o BatchMode=yes -o StrictHostKeyChecking=accept-new`, so a missing key fails fast instead of hanging on a prompt, and the host key is recorded on first contact. | +| `GIT_CLONE_SSH_KEY_PATH` | `` (empty) | **Host-run MCP only — ignored by the dockerized stack** (a host path never resolves inside the `codebadger-mcp` container; do not set it in `.env`). Private key FILE for `ssh://` clones of a custom host. The clone also sets `-o BatchMode=yes`, so a missing key fails fast instead of hanging on a prompt. | +| `GIT_CLONE_SSH_KNOWN_HOSTS` | `` (empty) | `known_hosts` file pinning the custom servers' host keys (`-o StrictHostKeyChecking=yes`). Unset, the clone falls back to `accept-new`: the key is recorded on first contact, but **in the dockerized stack that record lives in the container and is lost on every recreate**, making it trust-on-first-use each deploy. Like `GIT_CLONE_SSH_COMMAND` this is an in-container path there — put a `known_hosts` in the mounted key dir and set `/keys/known_hosts`. | | `GIT_CLONE_SSH_COMMAND` | `` (empty) | Full ssh command override (passed to git as `GIT_SSH_COMMAND` for the clone); takes precedence over both key settings. In the dockerized stack any key path it references must exist **inside the `codebadger-mcp` container** (e.g. `/keys/…`). | Notes: @@ -255,6 +257,11 @@ Notes: - `ssh://` URLs may carry a username (`git@…`) but not a password; keys/agent do the auth. scp-style `git@host:path` URLs are not accepted — use `ssh://git@host[:port]/path` (it carries ports unambiguously). +- **Ports are part of the allowlist.** A bare `forge.lan` entry only permits + `ssh://…@forge.lan[:22]/…`, so allowlisting a git server does not also expose + every other port on that machine to a caller who can influence `source_path`. + Use `forge.lan:3000` for a non-default ssh port, or `forge.lan:*` to accept + any port on it. - The allowlist still blocks every other host (alternate git hosts, look-alike domains, cloud metadata endpoints, …), so the SSRF posture of [docs/security.md](security.md) is unchanged — you are explicitly trusting diff --git a/docs/security.md b/docs/security.md index ddca353..8ff52e8 100644 --- a/docs/security.md +++ b/docs/security.md @@ -73,7 +73,7 @@ The numbered controls are the boundary checks; each is described below. | # | Boundary | Control | Where | |---|----------|---------|-------| | ① | Tool input → MCP | **Allowlist/format validation of every parameter**: `source_type`, `language` (whitelist), `codebase_hash` (`^[a-f0-9]{16}$`), `github_token` & `branch` (anti URL-/arg-injection, e.g. blocks `--upload-pack`), snippet `code`/`filename`/label, regex `pattern` (length + ReDoS shapes). | `src/utils/validators.py` | -| ①a | Repo URL → clone (**SSRF/undefined-clone prevention**) | **Strict allowlist on remote repos**: only `https://github.com/` or `https://gitlab.com/` (incl. `www.`) by default. Enforced by **two independent gates** — a literal, case-sensitive `https:///` prefix match *and* a parsed-`hostname` allowlist — plus rejection of any non-`https` scheme (`git://`, `file://`, …), embedded credentials (`user:tok@`), non-default ports, and whitespace/control chars. Blocks userinfo host-smuggling (`https://github.com@evil/…`), internal/metadata hosts, and look-alike domains. **Operator extension**: `GIT_CLONE_EXTRA_HOSTS` (env) explicitly adds `host[:port]` entries which may also be cloned over `ssh://` (username but no password allowed; auth rides in `GIT_SSH_COMMAND`, never the URL) — everything else about the posture (exact-hostname match, no embedded credentials, control chars, ≥`/owner/repo` path) is unchanged, and the injected `github_token` for github.com/gitlab.com is stripped from `.git/config` after the clone. | `validators.py` (`validate_repo_url`), `services/git_manager.py` | +| ①a | Repo URL → clone (**SSRF/undefined-clone prevention**) | **Strict allowlist on remote repos**: only `https://github.com/` or `https://gitlab.com/` (incl. `www.`) by default. For the built-in hosts, enforced by **two independent gates** — a literal, case-sensitive `https:///` prefix match *and* a parsed-`hostname` allowlist — plus rejection of any non-`https` scheme (`git://`, `file://`, …), embedded credentials (`user:tok@`), non-default ports, and whitespace/control chars. Blocks userinfo host-smuggling (`https://github.com@evil/…`), internal/metadata hosts, and look-alike domains. **Operator extension**: `GIT_CLONE_EXTRA_HOSTS` (env) explicitly adds `host[:port]` entries which may also be cloned over `ssh://`, gated by the parsed-`hostname` allowlist (the literal-prefix gate is https-only) plus an **exact port match** — a bare entry means port 22, so one allowlisted host is not a licence to reach every port on that machine; `host:*` opts into any. A username but no password is allowed; auth rides in `GIT_SSH_COMMAND`, never the URL. Everything else about the posture (exact-hostname match, no embedded credentials, control chars, ≥`/owner/repo` path) is unchanged; the config is parsed at startup so a typo fails the boot, and the injected `github_token` for github.com/gitlab.com is stripped from `.git/config` after the clone. | `validators.py` (`validate_repo_url`), `services/git_manager.py` | | ①b | Snippet code → CPG | **Language validated *and* inferred.** Pasted code is supplied in `` tags (parsed by regex); the declared language must be supported, and a content-signal check **refuses an obviously mislabeled tag** or **ambiguous/undeclared** language — every refusal returns an actionable message rather than building a wrong-language CPG. | `validators.py` (`parse_snippet_blocks`, `validate_and_infer_snippet_language`) | | ② | Source staging | **Path confinement + symlink-safe copy.** Local paths must be absolute, are rejected if they contain null bytes/control chars, then `realpath`-canonicalized (collapsing `..` and resolving symlinks *before* any check) and screened against a system-dir denylist (`/etc`, `/proc`, `/sys`, `/root`, …). An optional `ALLOWED_SOURCE_ROOTS` allowlist hard-contains local sources to named roots. Snapshot reads confined with `realpath`+prefix / `commonpath`; the copy never dereferences symlinks whose target escapes the source tree. | `validators.py` (`resolve_host_path`), `core_tools.py` | | ②a | Deployment posture | **`CHAT_DEPLOY=true` disables `source_type='local'` entirely** so a chat-facing / multi-tenant MCP cannot read arbitrary host paths — callers must use an allowlisted repo URL or a pasted snippet. | `core_tools.py`, `config.py` | diff --git a/main.py b/main.py index dd66fc0..b2437f8 100644 --- a/main.py +++ b/main.py @@ -33,7 +33,7 @@ QueryExecutor, CodeBrowsingService ) -from src.utils import setup_logging +from src.utils import setup_logging, validate_extra_repo_hosts_config from src.utils import compute_recommendation, current_from_config, render_recommendation from src.startup_tuning import apply_startup_tuning, container_mem_limit_mb, parse_mem_to_mb from src.health import ( @@ -374,6 +374,11 @@ async def app_lifespan(server: FastMCP): ) logger.info("Starting CodeBadger Server") + # Fail the boot on a typo'd GIT_CLONE_EXTRA_HOSTS rather than letting it sit + # latent until the first ssh:// clone (github/gitlab clones would keep + # working, hiding the misconfiguration from the operator). + validate_extra_repo_hosts_config() + # Print the memory-aware configuration envelope before the heavy service # init, flag drift that risks an OOM cascade, and auto-derive an unset Joern # memory budget from host RAM (before the Joern manager is constructed). diff --git a/src/defaults.py b/src/defaults.py index 5321f53..ae71b9a 100644 --- a/src/defaults.py +++ b/src/defaults.py @@ -72,15 +72,21 @@ def resolve_redis_url() -> str: # allowlist their own git server(s) for cloning via generate_cpg. Addresses are # configured through the environment so a LAN deployment needs no code changes: # GIT_CLONE_EXTRA_HOSTS ','-separated `host[:port]` entries accepted in -# addition to github.com/gitlab.com. A bare host allows -# any port on it; `host:port` pins that port only. -# Custom hosts are cloned over ssh:// only -# (github.com/gitlab.com stay https-only). +# addition to github.com/gitlab.com. A bare host means +# port 22 only; `host:port` pins that port; `host:*` +# allows any port on it. Custom hosts are cloned over +# ssh:// only (github.com/gitlab.com stay https-only). +# Parsed at startup — a malformed value fails the boot. # GIT_CLONE_SSH_KEY_PATH Private key for ssh:// clones of custom hosts. +# GIT_CLONE_SSH_KNOWN_HOSTS +# known_hosts file pinning the custom servers' host +# keys (StrictHostKeyChecking=yes). Unset = accept-new, +# i.e. trust-on-first-use. # GIT_CLONE_SSH_COMMAND Full ssh command override (takes precedence over the # key path; passed to git as GIT_SSH_COMMAND). GIT_CLONE_EXTRA_HOSTS = "" GIT_CLONE_SSH_KEY_PATH = "" +GIT_CLONE_SSH_KNOWN_HOSTS = "" GIT_CLONE_SSH_COMMAND = "" # Optional ':'-separated allowlist of host directory roots that source_type= diff --git a/src/services/git_manager.py b/src/services/git_manager.py index baf33d9..33ea1fd 100644 --- a/src/services/git_manager.py +++ b/src/services/git_manager.py @@ -11,6 +11,7 @@ import logging import os import re +import shlex import shutil from typing import Dict, Optional from urllib.parse import quote, urlparse @@ -38,13 +39,22 @@ def _mask_token_in_text(text: str) -> str: def _ssh_clone_env() -> Dict[str, str]: - """Environment for an ssh:// clone of a custom git server. + """Environment overrides for an ssh:// clone of a custom git server. git invokes the command in GIT_SSH_COMMAND for every ssh remote. Default to the operator's full override (GIT_CLONE_SSH_COMMAND) or build one from GIT_CLONE_SSH_KEY_PATH. BatchMode keeps a missing key/passphrase from - hanging the clone on an interactive prompt; accept-new auto-records the - server's host key on first contact instead of blocking on confirmation. + hanging the clone on an interactive prompt. + + Host key policy: with GIT_CLONE_SSH_KNOWN_HOSTS pointing at a known_hosts + file the server key is *pinned* (StrictHostKeyChecking=yes). Without it we + fall back to accept-new, which records the key on first contact — note that + in the dockerized stack that record lives in the container's ~/.ssh and is + lost on every container recreate, so it is trust-on-first-use each deploy. + + Only GIT_SSH_COMMAND is returned: GitPython layers these over os.environ + for the child process, so there is no need (and no reason, given the + secrets in this server's environment) to copy the whole environment. """ ssh_cmd = os.getenv("GIT_CLONE_SSH_COMMAND", defaults.GIT_CLONE_SSH_COMMAND).strip() if not ssh_cmd: @@ -53,10 +63,23 @@ def _ssh_clone_env() -> Dict[str, str]: "GIT_CLONE_SSH_KEY_PATH", defaults.GIT_CLONE_SSH_KEY_PATH ).strip() if key_path: - parts += ["-i", key_path] - parts += ["-o", "StrictHostKeyChecking=accept-new", "-o", "BatchMode=yes"] + # git runs GIT_SSH_COMMAND through a shell, so paths need quoting. + parts += ["-i", shlex.quote(key_path)] + known_hosts = os.getenv( + "GIT_CLONE_SSH_KNOWN_HOSTS", defaults.GIT_CLONE_SSH_KNOWN_HOSTS + ).strip() + if known_hosts: + parts += [ + "-o", + f"UserKnownHostsFile={shlex.quote(known_hosts)}", + "-o", + "StrictHostKeyChecking=yes", + ] + else: + parts += ["-o", "StrictHostKeyChecking=accept-new"] + parts += ["-o", "BatchMode=yes"] ssh_cmd = " ".join(parts) - return dict(os.environ, GIT_SSH_COMMAND=ssh_cmd) + return {"GIT_SSH_COMMAND": ssh_cmd} class GitManager: diff --git a/src/utils/__init__.py b/src/utils/__init__.py index 094952f..3d95034 100644 --- a/src/utils/__init__.py +++ b/src/utils/__init__.py @@ -9,6 +9,7 @@ sanitize_path, validate_codebase_hash, validate_cpgql_query, + validate_extra_repo_hosts_config, validate_github_url, validate_language, validate_local_path, @@ -36,6 +37,7 @@ "validate_codebase_hash", "validate_source_type", "validate_local_path", + "validate_extra_repo_hosts_config", "validate_github_url", "validate_repo_url", "validate_language", diff --git a/src/utils/validators.py b/src/utils/validators.py index 695e76a..49154ec 100644 --- a/src/utils/validators.py +++ b/src/utils/validators.py @@ -3,6 +3,8 @@ """ import hashlib +import ipaddress +import logging import os import re from typing import Dict, Optional @@ -12,6 +14,8 @@ from ..exceptions import ValidationError from ..models import SourceType +logger = logging.getLogger(__name__) + def validate_source_type(source_type: str) -> None: """Validate source type""" @@ -86,16 +90,43 @@ def validate_codebase_hash(codebase_hash: str) -> None: # optionally with a port; IPv6 goes in brackets. Used as a building block for # both the entry syntax below and as a matcher against parsed URL hostnames. _EXTRA_HOST_ENTRY_RE = re.compile(r"^[A-Za-z0-9._-]+$") -_IPV6_HOST_RE = re.compile(r"^[A-Fa-f0-9:]+$") + +# ssh's default port. A bare `host` entry allows ONLY this port: an entry is a +# statement about one git server, not a licence to reach every service on that +# machine. Widening to every port is possible but must be spelled out (`host:*`). +SSH_DEFAULT_PORT = 22 +_ANY_PORT = "*" + + +def _normalize_extra_host(host: str) -> str: + """Canonical comparison key for an allowlist host or a URL hostname. + + Lowercased, and IPv6 literals are collapsed to their canonical compressed + form so ``[0:0:0:0:0:0:0:1]`` and ``[::1]`` are the same entry (urlparse + hands back the literal text between the brackets, un-normalized). + """ + key = host.lower() + if ":" in key: # only an IPv6 literal can contain a colon here + try: + return ipaddress.IPv6Address(key).compressed + except ValueError: + return key + return key def _extra_repo_host_entries() -> Dict[str, Optional[set]]: """Parse GIT_CLONE_EXTRA_HOSTS into ``{hostname: ports | None}``. - Each entry is ``host`` or ``host:port`` (comma-separated). ``None`` for a - bare host means any port is allowed on it; a set pins the listed ports. + Each entry is ``host``, ``host:port`` or ``host:*`` (comma-separated): + + * ``host`` → port 22 only (ssh's default), + * ``host:port`` → that port only, + * ``host:*`` → any port on that host (``None`` in the returned map). + A malformed entry raises ValidationError so a typo'd config fails loudly - instead of silently widening (or narrowing) the allowlist. + instead of silently widening (or narrowing) the allowlist. The message + quotes the offending entry and is for the operator: callers get the + redacted version raised by :func:`is_extra_repo_host`. """ raw = os.getenv("GIT_CLONE_EXTRA_HOSTS", defaults.GIT_CLONE_EXTRA_HOSTS) hosts: Dict[str, Optional[set]] = {} @@ -106,7 +137,7 @@ def _extra_repo_host_entries() -> Dict[str, Optional[set]]: if "/" in entry or "@" in entry or "?" in entry or "#" in entry: raise ValidationError( f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' " - "(expected 'host' or 'host:port')" + "(expected 'host', 'host:port' or 'host:*')" ) if entry.startswith("["): # [ipv6] or [ipv6]:port host, closed, suffix = entry[1:].partition("]") @@ -122,33 +153,34 @@ def _extra_repo_host_entries() -> Dict[str, Optional[set]]: "(expected '[host]:port')" ) port_str = suffix[1:] - if not _IPV6_HOST_RE.match(host): + try: + ipaddress.IPv6Address(host) + except ValueError: raise ValidationError( f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad IPv6 host)" ) - elif ":" in entry: - host, port_str = entry.rsplit(":", 1) - if not _EXTRA_HOST_ENTRY_RE.match(host): - raise ValidationError( - f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad host)" - ) else: - host, port_str = entry, "" + host, sep, port_str = entry.rpartition(":") + if not sep: + host, port_str = port_str, "" if not _EXTRA_HOST_ENTRY_RE.match(host): raise ValidationError( f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad host)" ) - host = host.lower() - ports: Optional[set] = None - if port_str: + host = _normalize_extra_host(host) + # No port on the entry = ssh's default port, NOT "any port". + ports: Optional[set] = {SSH_DEFAULT_PORT} + if port_str == _ANY_PORT: + ports = None + elif port_str: if not port_str.isdigit() or not (1 <= int(port_str) <= 65535): raise ValidationError( f"Invalid GIT_CLONE_EXTRA_HOSTS entry '{entry}' (bad port)" ) ports = {int(port_str)} if host in hosts: - # A second entry for the same host either widens to any port or - # unions the ports, depending on whether it carries one. + # A second entry for the same host either widens to any port + # (`host:*`) or unions the pinned ports. existing = hosts[host] if existing is None or ports is None: hosts[host] = None @@ -159,17 +191,42 @@ def _extra_repo_host_entries() -> Dict[str, Optional[set]]: return hosts +def validate_extra_repo_hosts_config() -> Dict[str, Optional[set]]: + """Parse GIT_CLONE_EXTRA_HOSTS once at startup so a typo fails the boot. + + Without this the config is only parsed when a caller happens to submit an + ssh:// URL, so a misconfiguration would sit latent (github/gitlab clones + keep working) until the first custom-host clone. Raises ValidationError. + """ + return _extra_repo_host_entries() + + def is_extra_repo_host(hostname: Optional[str], port: Optional[int]) -> bool: """True when hostname/port match an operator-configured GIT_CLONE_EXTRA_HOSTS - entry (bare host = any port; `host:port` = that port only).""" + entry (bare host = port 22; ``host:port`` = that port; ``host:*`` = any). + + ``port`` is the port parsed from the URL, or None when it carried none — in + which case ssh's default 22 is what a clone would actually dial, so that is + what gets matched. + """ if not hostname: return False - entries = _extra_repo_host_entries() - key = hostname.lower() + try: + entries = _extra_repo_host_entries() + except ValidationError as e: + # The operator's raw config must not leak to an MCP caller; it is + # logged for them instead. Startup already refuses to boot on this, + # so reaching here means the env changed under a running server. + logger.error("GIT_CLONE_EXTRA_HOSTS is misconfigured: %s", e) + raise ValidationError( + "The repository host allowlist is misconfigured on this server; " + "contact the operator" + ) from None + key = _normalize_extra_host(hostname) if key not in entries: return False allowed_ports = entries[key] - return allowed_ports is None or (port is not None and port in allowed_ports) + return allowed_ports is None or (port or SSH_DEFAULT_PORT) in allowed_ports def validate_repo_url(url: str) -> bool: @@ -181,8 +238,9 @@ def validate_repo_url(url: str) -> bool: * ``ssh://[user@]/…`` where ```` is listed in the operator's ``GIT_CLONE_EXTRA_HOSTS`` (e.g. a self-hosted Forgejo at ``ssh://git@192.168.152.14:3000``). Custom hosts are ssh-only (no - http(s) cloning), may carry a port (a ``host:port`` entry pins it; a - bare entry allows any), and the URL may carry a username (``git@``) + http(s) cloning); the port must be one the entry allows (a bare + ``host`` entry means port 22 only, ``host:port`` pins that port, and + ``host:*`` allows any), and the URL may carry a username (``git@``) but no password. For every URL, regardless of host: @@ -251,8 +309,17 @@ def validate_repo_url(url: str) -> bool: raise ValidationError( "ssh repository URLs must not contain an embedded password" ) - if parsed.username and not re.fullmatch(r"[A-Za-z0-9._-]+", parsed.username): + # Must start alphanumeric: a leading '-' would reach ssh's argv as an + # option rather than a login name. git blocks that downstream too — this + # is the same belt-and-suspenders posture the rest of this file keeps. + if parsed.username and not re.fullmatch( + r"[A-Za-z0-9][A-Za-z0-9._-]*", parsed.username + ): raise ValidationError("Invalid username in ssh repository URL") + # urlparse accepts port 0 (it only range-checks the upper bound), and a + # `host:*` entry would otherwise let it through. + if port is not None and not (1 <= port <= 65535): + raise ValidationError("Repository URL has an invalid port") if is_extra_repo_host(parsed.hostname, port): return _validate_repo_url_path(parsed) raise ValidationError( diff --git a/tests/test_git_manager.py b/tests/test_git_manager.py index c908291..6bf79f0 100644 --- a/tests/test_git_manager.py +++ b/tests/test_git_manager.py @@ -24,6 +24,7 @@ def _clean_git_clone_env(monkeypatch): for var in ( "GIT_CLONE_EXTRA_HOSTS", "GIT_CLONE_SSH_KEY_PATH", + "GIT_CLONE_SSH_KNOWN_HOSTS", "GIT_CLONE_SSH_COMMAND", ): monkeypatch.delenv(var, raising=False) @@ -45,14 +46,32 @@ def test_key_path_injected(self, monkeypatch): cmd = _ssh_clone_env()["GIT_SSH_COMMAND"] assert "-i /keys/id_ed25519" in cmd + def test_key_path_is_shell_quoted(self, monkeypatch): + """git runs GIT_SSH_COMMAND through a shell — a spaced path must survive.""" + monkeypatch.setenv("GIT_CLONE_SSH_KEY_PATH", "/my keys/id_ed25519") + cmd = _ssh_clone_env()["GIT_SSH_COMMAND"] + assert "-i '/my keys/id_ed25519'" in cmd + + def test_known_hosts_pins_the_server_key(self, monkeypatch): + monkeypatch.setenv("GIT_CLONE_SSH_KNOWN_HOSTS", "/keys/known_hosts") + cmd = _ssh_clone_env()["GIT_SSH_COMMAND"] + assert "UserKnownHostsFile=/keys/known_hosts" in cmd + assert "StrictHostKeyChecking=yes" in cmd + assert "accept-new" not in cmd + def test_full_command_override_wins(self, monkeypatch): monkeypatch.setenv("GIT_CLONE_SSH_KEY_PATH", "/keys/ignored") monkeypatch.setenv("GIT_CLONE_SSH_COMMAND", "ssh -i /other/key -p 2222") assert _ssh_clone_env()["GIT_SSH_COMMAND"] == "ssh -i /other/key -p 2222" - def test_env_carries_os_environ(self, monkeypatch): + def test_env_is_the_ssh_command_only(self, monkeypatch): + """GitPython layers env over os.environ, so don't copy the whole thing. + + This server's environment holds POSTGRES_PASSWORD / GITHUB_TOKEN / + JOERN_SERVER_AUTH_PASSWORD; none of it belongs in the Git object. + """ monkeypatch.setenv("CODEBADGER_TEST_MARKER", "1") - assert _ssh_clone_env().get("CODEBADGER_TEST_MARKER") == "1" + assert list(_ssh_clone_env()) == ["GIT_SSH_COMMAND"] class TestCloneRepository: diff --git a/tests/test_validators.py b/tests/test_validators.py index 8de86bd..81606f8 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -285,7 +285,7 @@ class TestExtraGitHosts: cloned over ssh:// with custom ports — http(s) stays rejected for them. """ - HOSTS = "192.168.152.14:3000,git.example.com,[::1]:2222" + HOSTS = "192.168.152.14:3000,git.example.com,[::1]:2222,any.example.com:*" @pytest.fixture(autouse=True) def _set_hosts(self, monkeypatch): @@ -307,8 +307,11 @@ def test_ssh_urls_accepted_for_custom_hosts(self): for url in [ "ssh://git@192.168.152.14:3000/ethan/demo.git", "ssh://192.168.152.14:3000/ethan/demo", # username optional - "ssh://git@git.example.com/ethan/demo", + "ssh://git@git.example.com/ethan/demo", # bare entry => port 22 + "ssh://git@git.example.com:22/ethan/demo", # ... explicitly "ssh://git@[::1]:2222/ethan/demo", + "ssh://git@[0:0:0:0:0:0:0:1]:2222/ethan/demo", # same host, long form + "ssh://git@any.example.com:9999/ethan/demo", # `host:*` => any port ]: validate_github_url(url) @@ -317,11 +320,30 @@ def test_port_pinning_enforced(self): for url in [ "ssh://git@192.168.152.14:3001/ethan/demo", # wrong port, pinned host "ssh://git@[::1]:22/ethan/demo", # wrong port, pinned host - "ssh://git@192.168.152.14/ethan/demo", # no port, pinned host + "ssh://git@192.168.152.14/ethan/demo", # no port => 22, pinned to 3000 ]: with pytest.raises(ValidationError): validate_github_url(url) + def test_bare_entry_does_not_open_every_port(self): + """A bare `host` entry is a git server, not a licence to reach the box. + + Without this, one allowlisted host turns into an ssh-speaking port + scanner for any caller that can influence source_path. + """ + for url in [ + "ssh://git@git.example.com:3000/ethan/demo", + "ssh://git@git.example.com:8080/ethan/demo", + "ssh://git@git.example.com:6379/ethan/demo", + ]: + with pytest.raises(ValidationError): + validate_github_url(url) + + def test_port_zero_rejected(self): + """urlparse only range-checks the upper bound; port 0 must not pass.""" + with pytest.raises(ValidationError): + validate_github_url("ssh://git@any.example.com:0/ethan/demo") + def test_ssh_not_enabled_for_default_hosts(self): """ssh:// stays rejected for github.com/gitlab.com.""" for url in [ @@ -341,6 +363,8 @@ def test_hardening_unchanged_for_custom_hosts(self): "git://192.168.152.14:3000/ethan/demo", # git protocol "ssh://git:pw@192.168.152.14:3000/ethan/demo", # password in ssh URL "ssh://bad user@192.168.152.14:3000/ethan/demo", # bad ssh username + "ssh://-oProxyCommand@192.168.152.14:3000/e/d", # option-shaped username + "ssh://-4@192.168.152.14:3000/ethan/demo", # ... short form "ssh://git@192.168.152.14:3000/ethan/demo\n.git", # control chars ]: with pytest.raises(ValidationError): @@ -363,20 +387,48 @@ def test_malformed_entries_fail_loudly(self, monkeypatch): "host/path", # path in an entry "user@host", # userinfo in an entry ":3000", # empty host + "host:3000:4", # two ports + "[not:an:ipv6::z]", # bracketed but not an address + "host:**", # not the any-port wildcard ]: monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", cfg) # An ssh URL is what drives the entry parse for a custom host. - with pytest.raises(ValidationError): + with pytest.raises(ValidationError) as excinfo: validate_repo_url("ssh://git@host.example/a/b") + # The operator's raw config must not be echoed back to the caller. + assert cfg not in str(excinfo.value) + + def test_malformed_config_fails_at_startup(self, monkeypatch): + """The boot check parses the config with no URL to trigger it.""" + from src.utils.validators import validate_extra_repo_hosts_config + + monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", "host:notaport") + with pytest.raises(ValidationError): + validate_extra_repo_hosts_config() + # ... and a good config parses to the documented shape. + monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", "a.example,b.example:3000,c.example:*") + assert validate_extra_repo_hosts_config() == { + "a.example": {22}, + "b.example": {3000}, + "c.example": None, + } + + def test_malformed_config_does_not_break_default_hosts(self, monkeypatch): + """github/gitlab clones are unaffected by a broken extension config.""" + monkeypatch.setenv("GIT_CLONE_EXTRA_HOSTS", "host:notaport") + validate_github_url("https://github.com/user/repo") def test_is_extra_repo_host(self): from src.utils.validators import is_extra_repo_host assert is_extra_repo_host("192.168.152.14", 3000) is True assert is_extra_repo_host("192.168.152.14", None) is False # pinned port - assert is_extra_repo_host("git.example.com", 8443) is True # bare host - assert is_extra_repo_host("git.example.com", None) is True + assert is_extra_repo_host("git.example.com", 8443) is False # bare = 22 only + assert is_extra_repo_host("git.example.com", 22) is True + assert is_extra_repo_host("git.example.com", None) is True # None => ssh's 22 + assert is_extra_repo_host("any.example.com", 8443) is True # `host:*` assert is_extra_repo_host("::1", 2222) is True + assert is_extra_repo_host("0:0:0:0:0:0:0:1", 2222) is True # normalized assert is_extra_repo_host("evil.com", 80) is False assert is_extra_repo_host(None, 80) is False