diff --git a/.env.example b/.env.example index f13836e..4156412 100644 --- a/.env.example +++ b/.env.example @@ -121,4 +121,8 @@ ARTIFACTS_ROOT=./.data/artifacts # fake (default, no git on the host) | local (real git workspaces) GIT_WORKSPACE=fake ALLOW_CUSTOM_DOMAINS=false +# `protocol.file.allow` for host git calls. The hardened default is `never`: a +# repository must not be able to make git read a local path through a submodule +# or `--upload-pack`. Set to true ONLY to test against a local bare repository. +GIT_ALLOW_FILE_PROTOCOL=false # <<< agent-3 diff --git a/apps/api/app/application/container_delivery.py b/apps/api/app/application/container_delivery.py index 51063b8..530e1fb 100644 --- a/apps/api/app/application/container_delivery.py +++ b/apps/api/app/application/container_delivery.py @@ -1,16 +1,41 @@ """Wiring for the delivery workstream (repo workspaces, delivery hooks, deploy tester). -Foundation defaults are in-memory fakes. The delivery agent replaces the bodies (behind -settings) with the real adapters; the function names and return types are FROZEN. +The real adapters are selected by settings; the fakes remain the defaults so a +test that does not opt in never touches a git binary, a container or a network. + +``build_repo_workspace`` is the interesting one: the git adapter needs to know +*where* a project's repository lives and *how* to authenticate to it, and that +knowledge lives in three places — ``project_repositories`` + +``project_git_credentials`` for the platform forge, ``project_external_git`` for a +user-supplied remote, and ``projects`` for which of the two applies. The resolver +below is the only code that joins them, and it hands the plaintext secret +straight to the adapter for the duration of a single call. """ from __future__ import annotations +import logging + from sqlalchemy.ext.asyncio import AsyncSession from app.application.ports.delivery_hooks import DeliveryHooks, NullDeliveryHooks from app.application.ports.deploy_target import DeployTargetTester, FakeDeployTargetTester from app.application.ports.repo_workspace import FakeRepoWorkspace, RepoWorkspacePort +from app.core.config import settings +from app.core.crypto import decrypt +from app.infrastructure.adapters.git_workspace import ( + LocalGitWorkspaceAdapter, + ProjectGitSource, +) +from app.infrastructure.db.models import ProjectRow +from app.infrastructure.repositories.projects import ( + ProjectExternalGitRepository, + ProjectGitCredentialRepository, + ProjectGitRepoRepository, + ProjectRepository, +) + +logger = logging.getLogger("agy.container.delivery") _fake_workspace = FakeRepoWorkspace() _null_hooks = NullDeliveryHooks() @@ -18,7 +43,79 @@ _fake_deploy_tester = FakeDeployTargetTester() def build_repo_workspace(session: AsyncSession) -> RepoWorkspacePort: - return _fake_workspace + """The platform's git adapter, or the in-memory fake when ``GIT_WORKSPACE=fake``.""" + if settings.git_workspace.strip().lower() != "local": + return _fake_workspace + + projects = ProjectRepository(session) + repos = ProjectGitRepoRepository(session) + credentials = ProjectGitCredentialRepository(session) + external = ProjectExternalGitRepository(session) + + async def resolve(project_id: str) -> ProjectGitSource | None: + project = await projects.get(project_id) + if project is None: + return None + + row = await session.get(ProjectRow, project_id) + if row is None: + return None + git_source = (row.git_source or "platform").lower() + + if git_source == "external": + url = (row.external_repo_url or "").strip() + if not url: + return None + record = await external.get_for_project(project_id) + return ProjectGitSource( + project_id=project_id, + organization_id=project.organization_id, + clone_url=url, + default_branch=project.default_branch, + auth_type=(record.auth_type if record else "none") or "none", + username=record.username if record else None, + secret=_decrypt_or_none( + record.secret_encrypted if record else None, project_id + ), + known_hosts=record.known_hosts if record else None, + is_platform=False, + ) + + repo = await repos.get_for_project(project_id) + if repo is None or not repo.clone_url: + return None + credential = await credentials.get_for_project(project_id) + secret = _decrypt_or_none( + credential.encrypted_password if credential else None, project_id + ) + return ProjectGitSource( + project_id=project_id, + organization_id=project.organization_id, + clone_url=repo.clone_url, + default_branch=repo.default_branch or project.default_branch, + auth_type="token" if secret else "none", + username=credential.username if credential else None, + secret=secret, + is_platform=True, + ) + + return LocalGitWorkspaceAdapter(resolver=resolve) + + +def _decrypt_or_none(ciphertext: str | None, project_id: str) -> str | None: + """Decrypt a stored secret, tolerating a rotated ``ENCRYPTION_KEY``. + + A rotated key must surface as "no credential" (git then fails with an auth + error the user can act on) rather than as an exception that breaks an + unrelated request. The failure is logged, never raised with the ciphertext. + """ + if not ciphertext: + return None + try: + return decrypt(ciphertext) + except Exception: # noqa: BLE001 - see docstring + logger.warning("git credential for project %s could not be decrypted", project_id) + return None def build_delivery_hooks(session: AsyncSession) -> DeliveryHooks: diff --git a/apps/api/app/core/config.py b/apps/api/app/core/config.py index 3bfeb57..57aed1d 100644 --- a/apps/api/app/core/config.py +++ b/apps/api/app/core/config.py @@ -155,6 +155,11 @@ class Settings(BaseSettings): artifacts_root: str = "./.data/artifacts" git_workspace: str = "fake" # fake | local allow_custom_domains: bool = False + #: ``protocol.file.allow`` for host git calls. ``False`` (→ ``never``) is the + #: hardened default: a repository must not be able to make git read a local + #: path via a submodule or ``--upload-pack``. Set it to true only to test + #: against a local bare repository. + git_allow_file_protocol: bool = False # <<< agent-3 @property diff --git a/apps/api/app/domain/platform/rules.py b/apps/api/app/domain/platform/rules.py index 47f6d0c..3e4b5ff 100644 --- a/apps/api/app/domain/platform/rules.py +++ b/apps/api/app/domain/platform/rules.py @@ -1,15 +1,24 @@ """Platform-wide validation rules carried over from the existing infrastructure. -These are pure functions with no I/O, so both the application layer *and* the -adapters can enforce them. Defence in depth: a rule that is only checked in one -place is a rule that will eventually be bypassed. +These are almost all pure functions with no I/O, so both the application layer +*and* the adapters can enforce them. Defence in depth: a rule that is only +checked in one place is a rule that will eventually be bypassed. + +The one exception is :func:`assert_remote_host_allowed`, which resolves a +hostname. It has to: a *name* tells you nothing about where it points, and the +whole point of the check is to refuse a name that resolves into the private +network. The address classification itself is pure +(:func:`is_blocked_address`), so the policy is testable without DNS. Reference: ``docs/current-infra-notes.md`` §1. """ from __future__ import annotations +import ipaddress import re +import socket +from urllib.parse import urlsplit DEFAULT_ROOT_DOMAIN = "msa.artacloud.ir" @@ -119,3 +128,143 @@ def assert_not_protected_port(port: int) -> int: if port in SYSTEM_PROTECTED_PORTS: raise RuleViolation(f"اتصال به پورت محافظت‌شدهٔ {port} ممنوع است.") return port + + +# --------------------------------------------------------------------------- +# SSRF: user-supplied remote hosts (git remotes, deploy targets) +# --------------------------------------------------------------------------- + +#: Ports that are always refused on a user-supplied host, whatever the address. +#: A remote git or deploy target has no business talking to a database, a cache +#: or the Docker daemon. Port 22 is deliberately *not* here: git-over-ssh and +#: SFTP deploys are legitimate. +BLOCKED_REMOTE_PORTS: frozenset[int] = frozenset( + {2375, 2376, 3306, 5432, 6379, 9200, 11211, 27017} +) + +#: Hostnames that are never a valid user-supplied remote. `localhost` is a name, +#: not an address, so it needs its own rule — DNS would resolve it to loopback, +#: but only if we remembered to resolve. +_BLOCKED_HOSTNAMES: frozenset[str] = frozenset( + { + "localhost", + "localhost.localdomain", + "ip6-localhost", + "ip6-loopback", + "metadata", + "metadata.google.internal", + "instance-data", + } +) + +_ALLOWED_REMOTE_SCHEMES: frozenset[str] = frozenset( + {"http", "https", "ssh", "git", "ftp", "ftps", "sftp"} +) + + +def is_blocked_address(address: str) -> bool: + """True when an IP literal must not be the target of a user-supplied remote. + + Covers every way to reach something that is not the public internet: + loopback, RFC1918 private, link-local (including the cloud metadata address + ``169.254.169.254``), unique-local IPv6, multicast, reserved, unspecified, + and IPv4-mapped IPv6 forms of all of the above. + """ + try: + ip = ipaddress.ip_address(address) + except ValueError: + return False # not an IP literal; the DNS path handles it + + if isinstance(ip, ipaddress.IPv6Address) and ip.ipv4_mapped is not None: + # ::ffff:127.0.0.1 is loopback in disguise. + ip = ip.ipv4_mapped + + return bool( + ip.is_loopback + or ip.is_private + or ip.is_link_local + or ip.is_multicast + or ip.is_reserved + or ip.is_unspecified + ) + + +def assert_remote_host_allowed( + url: str, *, allow_private: bool = False, resolver=None +) -> tuple[str, int | None]: + """Validate a user-supplied remote URL and return ``(host, port)``. + + Runs before *any* outbound connection — a git remote test, a deploy + connection — because a user-supplied URL is the classic SSRF vector: + ``http://169.254.169.254/`` reaches cloud instance credentials, and + ``http://127.0.0.1:6379/`` reaches Redis. + + ``resolver`` is injectable so the DNS step can be faked in tests; it defaults + to ``socket.getaddrinfo``. Every address the name resolves to must pass, not + just the first: a name can legitimately have both a public and a private + address, and returning either would defeat the check. + """ + clean = (url or "").strip() + if not clean: + raise RuleViolation("آدرس مقصد خالی است.") + if any(ch.isspace() for ch in clean): + raise RuleViolation("آدرس مقصد نباید فاصله داشته باشد.") + + # A bare ``git@host:path`` scp-style remote has no scheme; normalise it so + # urlsplit can do its job. + if "://" not in clean and re.match(r"^[^/@\s]+@[^/:\s]+:", clean): + clean = "ssh://" + clean.replace(":", "/", 1) + + parts = urlsplit(clean) + if parts.scheme.lower() not in _ALLOWED_REMOTE_SCHEMES: + raise RuleViolation( + "پروتکل آدرس مقصد پشتیبانی نمی‌شود." + ) + host = parts.hostname or "" + if not host: + raise RuleViolation("میزبان آدرس مقصد مشخص نیست.") + if parts.password is not None: + # Credentials in a URL end up in logs, in `ps` and in the git remote + # config. They must be supplied as credentials, not as part of the URL. + raise RuleViolation("آدرس مقصد نباید شامل نام کاربری و رمز باشد.") + + port = parts.port + if port is not None: + if not (1 <= port <= 65535): + raise RuleViolation("شمارهٔ پورت مقصد نامعتبر است.") + if port in BLOCKED_REMOTE_PORTS: + raise RuleViolation(f"پورت {port} برای مقصد مجاز نیست.") + + if allow_private: + return host, port + + lowered = host.lower().strip(".") + if lowered in _BLOCKED_HOSTNAMES or lowered.endswith(".localhost"): + raise RuleViolation("اتصال به میزبان محلی مجاز نیست.") + + # An IP literal can be judged directly, without DNS. + try: + ipaddress.ip_address(lowered.strip("[]")) + except ValueError: + pass + else: + if is_blocked_address(lowered.strip("[]")): + raise RuleViolation("اتصال به آدرس‌های داخلی مجاز نیست.") + return host, port + + lookup = resolver or socket.getaddrinfo + try: + infos = lookup(host, port or 0, type=socket.SOCK_STREAM) + except (socket.gaierror, UnicodeError, OSError) as exc: + raise RuleViolation("میزبان مقصد پیدا نشد.") from exc + + # ``getaddrinfo`` returns a family-dependent sockaddr, so the address is + # coerced to str explicitly rather than relying on the overload's inference. + addresses: set[str] = {str(info[4][0]) for info in infos} + if not addresses: + raise RuleViolation("میزبان مقصد پیدا نشد.") + for address in addresses: + if is_blocked_address(address): + raise RuleViolation("اتصال به آدرس‌های داخلی مجاز نیست.") + + return host, port diff --git a/apps/api/app/domain/projects/models.py b/apps/api/app/domain/projects/models.py index f707dcd..5d31d46 100644 --- a/apps/api/app/domain/projects/models.py +++ b/apps/api/app/domain/projects/models.py @@ -54,3 +54,20 @@ class ProjectGitCredential: encrypted_password: str id: str = field(default_factory=new_id) created_at: datetime = field(default_factory=utcnow) + + +@dataclass(slots=True, kw_only=True) +class ProjectExternalGit: + """Credentials for a *user-supplied* git remote (v2 ``git_source=external``). + + Separate from :class:`ProjectGitCredential`, which is the platform's own + Gitea identity. Keeping them apart means switching a project to an external + remote can never accidentally reuse — or expose — the platform token. + """ + + project_id: str + auth_type: str = "none" # none | token | ssh_key + username: str | None = None + secret_encrypted: str | None = None + known_hosts: str | None = None + updated_at: datetime = field(default_factory=utcnow) diff --git a/apps/api/app/infrastructure/adapters/git_workspace.py b/apps/api/app/infrastructure/adapters/git_workspace.py new file mode 100644 index 0000000..daa0552 --- /dev/null +++ b/apps/api/app/infrastructure/adapters/git_workspace.py @@ -0,0 +1,1074 @@ +"""Local git workspace adapter — the PLATFORM runs git, never the AI (D6). + +Every repository operation in v2 goes through this adapter, and the whole design +follows from one rule: **git runs on the host, and its directory is never +mounted into a container.** The AI edits files inside the sandbox; the platform +turns those edits into commits, merges and pushes out here. That split is what +keeps repository credentials away from the model entirely. + +Layout, per project, under ``SANDBOX_WORKSPACES_ROOT///``: + +* ``git/`` — a *bare-ish* gitdir. Never mounted, never visible to a container. +* ``work/`` — the working tree for ``dev``. This is what the sandbox mounts at + ``/workspace``, so it is the only part of the repository the AI can see. +* ``work/.worktrees//`` — one worktree per task, so two employees can + work on the same repository at once without fighting over the index. They are + excluded from the main work tree via the gitdir's ``info/exclude``, not the + user's ``.gitignore`` — a project must not be able to un-exclude them. + +Hardening, applied to *every* invocation: + +* explicit ``--git-dir`` / ``--work-tree``. The ``.git`` *file* inside a worktree + is a plain text pointer that a repository can rewrite; trusting it is how a + clone ends up writing into an unrelated directory. We never trust it. +* ``-c core.hooksPath=/dev/null`` — a repository may ship hooks, and a hook is + arbitrary code execution on the host. +* ``-c core.fsmonitor=false`` — a filesystem monitor is a long-lived process + spawned from repository config. +* ``-c core.quotePath=false`` — paths with non-ASCII characters are reported + literally, so a filename cannot be hidden behind octal escapes. +* ``-c safe.directory=*`` — the work trees are owned by the platform user, but + this keeps git from refusing them after a uid change. +* ``GIT_CONFIG_NOSYSTEM=1`` and a private ``HOME`` — neither the host's system + config nor the operator's ``~/.gitconfig`` may influence a tenant's repository. +* ``GIT_TERMINAL_PROMPT=0`` — a credential prompt must fail, never block. +* one ``asyncio.Lock`` per project, held for every mutating operation. Two + concurrent merges into ``dev`` corrupt the index; the lock is not an + optimisation. + +Credentials are the delicate part. A token goes through ``GIT_ASKPASS`` pointing +at a 0700 helper script that reads a 0600 file in a private per-project +directory; an SSH key is a 0600 temp file referenced by ``GIT_SSH_COMMAND``. +Either way the secret is never in ``argv``, never in a URL, and never in a log: +the only thing that travels through the environment is a *path*. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import hashlib +import logging +import os +import re +import shlex +from collections.abc import Awaitable, Callable, Sequence +from dataclasses import dataclass + +from app.application.ports.repo_workspace import ( + DiffResult, + MergeOutcome, + PushOutcome, + RemoteCheck, + RemoteCredentials, + WorkspaceRef, + WorktreeRef, +) +from app.core.config import settings +from app.core.errors import NotFoundError, ValidationError +from app.core.redact import redact_text +from app.domain.platform.rules import assert_remote_host_allowed +from app.infrastructure.adapters.process import ProcessResult, run_process + +logger = logging.getLogger("agy.git.workspace") + +#: Default branch of the platform's own workflow. `dev` is what the engine merges +#: into and what the CI/CD pipeline builds. +WORK_BRANCH = "dev" + +#: A task branch is derived from the task id, never from a user-supplied name. +TASK_BRANCH_PREFIX = "task/" + +GIT_TIMEOUT_SECONDS = 120.0 +REMOTE_TEST_TIMEOUT_SECONDS = 20.0 +PUSH_TIMEOUT_SECONDS = 300.0 + +#: A stale index.lock from a killed process blocks every later operation. Only +#: removed when it is old enough that no live process could still hold it. +STALE_LOCK_SECONDS = 30 + +MAX_DIFF_CHARS = 200_000 + +_ID_SAFE = re.compile(r"^[A-Za-z0-9_.-]+$") + + +@dataclass(slots=True, frozen=True, kw_only=True) +class ProjectGitSource: + """Everything the adapter needs to reach one project's repository. + + ``secret`` is plaintext and lives only for the duration of one call. It is + produced by a resolver supplied at wiring time (which reads the encrypted + column and decrypts it), never stored on the adapter. + """ + + project_id: str + organization_id: str + clone_url: str + default_branch: str = "main" + auth_type: str = "none" # none | token | ssh_key + username: str | None = None + secret: str | None = None + known_hosts: str | None = None + is_platform: bool = True + + +SourceResolver = Callable[[str], Awaitable[ProjectGitSource | None]] + + +class GitOperationError(RuntimeError): + """A git invocation failed in a way the caller cannot recover from.""" + + +def _check_segment(value: str, label: str) -> str: + clean = (value or "").strip() + if not clean or not _ID_SAFE.match(clean): + raise ValidationError(f"شناسهٔ {label} نامعتبر است.") + return clean + + +def _slug_for_branch(value: str) -> str: + return re.sub(r"[^A-Za-z0-9._/-]+", "-", value or "").strip("-/") or "task" + + +class LocalGitWorkspaceAdapter: + """``RepoWorkspacePort`` over a host git binary and a per-project gitdir.""" + + def __init__( + self, + *, + resolver: SourceResolver, + root: str | None = None, + git_bin: str = "git", + allow_file_protocol: bool | None = None, + timeout_seconds: float = GIT_TIMEOUT_SECONDS, + max_output_bytes: int | None = None, + runner: Callable[..., Awaitable[ProcessResult]] = run_process, + ) -> None: + self._resolve = resolver + self.root = os.path.abspath(root or settings.sandbox_workspaces_root) + self.git_bin = git_bin + self.allow_file_protocol = ( + settings.git_allow_file_protocol if allow_file_protocol is None else allow_file_protocol + ) + self.timeout_seconds = timeout_seconds + self.max_output_bytes = int( + settings.sandbox_max_output_bytes if max_output_bytes is None else max_output_bytes + ) + self._runner = runner + self._locks: dict[str, asyncio.Lock] = {} + + # -- paths --------------------------------------------------------------- + + def project_root(self, organization_id: str, project_id: str) -> str: + return os.path.join( + self.root, + _check_segment(organization_id, "سازمان"), + _check_segment(project_id, "پروژه"), + ) + + def _paths_for(self, source: ProjectGitSource) -> tuple[str, str, str]: + root = self.project_root(source.organization_id, source.project_id) + return root, os.path.join(root, "git"), os.path.join(root, "work") + + def _private_dir(self, source: ProjectGitSource) -> str: + root, _, _ = self._paths_for(source) + return os.path.join(root, "private") + + def _lock(self, project_id: str) -> asyncio.Lock: + lock = self._locks.get(project_id) + if lock is None: + lock = asyncio.Lock() + self._locks[project_id] = lock + return lock + + async def _source(self, project_id: str) -> ProjectGitSource: + source = await self._resolve(project_id) + if source is None: + raise NotFoundError("مخزن این پروژه پیدا نشد.") + return source + + # -- git plumbing -------------------------------------------------------- + + def _config_flags(self) -> list[str]: + file_policy = "user" if self.allow_file_protocol else "never" + return [ + "-c", + "core.hooksPath=/dev/null", + "-c", + "core.fsmonitor=false", + "-c", + "core.quotePath=false", + "-c", + f"protocol.file.allow={file_policy}", + "-c", + "safe.directory=*", + ] + + def _base_env(self, private_dir: str) -> dict[str, str]: + os.makedirs(private_dir, exist_ok=True) + with contextlib.suppress(OSError): + os.chmod(private_dir, 0o700) + return { + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_TERMINAL_PROMPT": "0", + "HOME": private_dir, + # A per-project HOME means no shared credential helper can leak + # between tenants, and `git commit` has no fallback identity. + "XDG_CONFIG_HOME": os.path.join(private_dir, "xdg"), + } + + async def _git( + self, + *, + gitdir: str, + work_tree: str | None, + args: Sequence[str], + private_dir: str, + env: dict[str, str] | None = None, + timeout_seconds: float | None = None, + check: bool = True, + ) -> ProcessResult: + argv: list[str] = [self.git_bin, f"--git-dir={gitdir}"] + if work_tree is not None: + argv.append(f"--work-tree={work_tree}") + argv.extend(self._config_flags()) + argv.extend(args) + + merged_env = self._base_env(private_dir) + if env: + merged_env.update(env) + + result = await self._runner( + argv, + # Run *inside* the work tree. With an explicit ``--work-tree`` a + # pathspec like ``.`` is resolved against the current directory, so + # without this the platform's own cwd would leak into a tenant's + # ``git add``. + cwd=work_tree, + timeout_seconds=float(timeout_seconds or self.timeout_seconds), + max_output_bytes=self.max_output_bytes, + env=merged_env, + ) + if check and not result.ok: + # stderr is redacted before it becomes an exception message: a failed + # `git ls-remote` echoes the remote, and a credential helper can + # write a token into stderr. + raise GitOperationError( + f"git {args[0] if args else ''} failed: {redact_text(result.stderr)[:500]}" + ) + return result + + @staticmethod + def _worktree_gitdir(gitdir: str, task_id: str) -> str: + """The *linked* gitdir for a task worktree, addressed explicitly. + + ``git worktree add`` gives each linked worktree its own small gitdir + under ``/worktrees/`` holding that worktree's ``HEAD`` and + index, and names it after the path's basename. We pass that path + directly rather than letting git discover it from the ``.git`` file + inside the worktree — that file is plain text a repository can rewrite, + and trusting it is how a clone ends up committing into an unrelated + directory. + + Without this, every operation on a task worktree would resolve ``HEAD`` + to the *main* work tree's branch and commit to ``dev`` instead of the + task branch. Falls back to the shared gitdir when the linked one is + missing, which is what an ``analysis_only`` pipeline (no worktree) wants. + """ + candidate = os.path.join(gitdir, "worktrees", task_id) + if os.path.isdir(candidate) and os.path.exists(os.path.join(candidate, "HEAD")): + return candidate + return gitdir + + async def _clear_stale_lock(self, gitdir: str) -> None: + lock = os.path.join(gitdir, "index.lock") + try: + age = os.path.getmtime(lock) + except OSError: + return + import time as _time + + if _time.time() - age > STALE_LOCK_SECONDS: + with contextlib.suppress(OSError): + os.remove(lock) + + # -- credentials --------------------------------------------------------- + + def _askpass_script(self, private_dir: str) -> str: + """Write (once) the 0700 helper that reads the 0600 secret file. + + The script is *not* secret; it contains no credential. The credential + file's path is passed through the environment, so nothing sensitive ever + reaches ``argv`` — which is world-readable via ``ps``. + """ + path = os.path.join(private_dir, "askpass.sh") + if not os.path.exists(path): + with open(path, "w", encoding="utf-8") as handle: + handle.write( + "#!/bin/sh\n" + "# Smart Organization git credential helper. Reads a 0600 file:\n" + "# line 1 = username, line 2 = secret. Never contains a credential.\n" + 'file="$SMARTORG_GIT_CREDENTIAL_FILE"\n' + 'case "$1" in\n' + ' *[Uu]sername*) head -n 1 "$file" ;;\n' + ' *) sed -n "2p" "$file" ;;\n' + "esac\n" + ) + os.chmod(path, 0o700) + return path + + @contextlib.contextmanager + def _credential_files(self, source: ProjectGitSource): + """Materialise credentials for one call; always removed afterwards. + + Yields the extra environment to hand git. Everything created here is + deleted in ``finally``, including on an exception, so a crashed call + cannot leave a private key on disk. + """ + private_dir = self._private_dir(source) + os.makedirs(private_dir, exist_ok=True) + with contextlib.suppress(OSError): + os.chmod(private_dir, 0o700) + + created: list[str] = [] + env: dict[str, str] = {} + try: + if source.auth_type == "token" and source.secret: + credential_file = os.path.join(private_dir, "credentials") + with open(credential_file, "w", encoding="utf-8") as handle: + handle.write(f"{source.username or 'git'}\n{source.secret}\n") + os.chmod(credential_file, 0o600) + created.append(credential_file) + env["GIT_ASKPASS"] = self._askpass_script(private_dir) + env["SMARTORG_GIT_CREDENTIAL_FILE"] = credential_file + # Some servers ask for the username before the password. + env["GIT_USERNAME"] = source.username or "git" + + elif source.auth_type == "ssh_key" and source.secret: + key_file = os.path.join(private_dir, "id_key") + with open(key_file, "w", encoding="utf-8") as handle: + material = source.secret + if not material.endswith("\n"): + material += "\n" + handle.write(material) + os.chmod(key_file, 0o600) + created.append(key_file) + + known_hosts = os.path.join(private_dir, "known_hosts") + if source.known_hosts: + with open(known_hosts, "w", encoding="utf-8") as handle: + handle.write(source.known_hosts) + os.chmod(known_hosts, 0o600) + created.append(known_hosts) + host_key_policy = "yes" + else: + # First contact: accept and record the key. The fingerprint + # is pinned by `test_remote` on the next successful run. + host_key_policy = "accept-new" + + env["GIT_SSH_COMMAND"] = " ".join( + ( + "ssh", + "-i", + shlex.quote(key_file), + "-o", + "IdentitiesOnly=yes", + "-o", + f"StrictHostKeyChecking={host_key_policy}", + "-o", + f"UserKnownHostsFile={shlex.quote(known_hosts)}", + "-o", + "BatchMode=yes", + ) + ) + yield env + finally: + for path in created: + with contextlib.suppress(OSError): + os.remove(path) + + # -- remote plumbing ----------------------------------------------------- + + async def _ensure_remote(self, source: ProjectGitSource, gitdir: str, private_dir: str) -> str: + """Point ``origin`` at the source URL, creating it if needed.""" + result = await self._git( + gitdir=gitdir, + work_tree=None, + args=["remote", "get-url", "origin"], + private_dir=private_dir, + check=False, + ) + if result.ok and result.stdout.strip(): + current = result.stdout.strip() + if current != source.clone_url: + await self._git( + gitdir=gitdir, + work_tree=None, + args=["remote", "set-url", "origin", source.clone_url], + private_dir=private_dir, + ) + return "origin" + + await self._git( + gitdir=gitdir, + work_tree=None, + args=["remote", "add", "origin", source.clone_url], + private_dir=private_dir, + ) + return "origin" + + async def _fetch(self, source: ProjectGitSource, gitdir: str, private_dir: str) -> bool: + with self._credential_files(source) as env: + result = await self._git( + gitdir=gitdir, + work_tree=None, + args=[ + "fetch", + "--prune", + "--tags", + "origin", + "+refs/heads/*:refs/remotes/origin/*", + ], + private_dir=private_dir, + env=env, + check=False, + ) + return result.ok + + async def _remote_heads(self, gitdir: str, private_dir: str) -> dict[str, str]: + result = await self._git( + gitdir=gitdir, + work_tree=None, + args=["for-each-ref", "--format=%(refname:short) %(objectname)", "refs/remotes/origin"], + private_dir=private_dir, + check=False, + ) + heads: dict[str, str] = {} + for line in result.stdout.splitlines(): + parts = line.strip().split(" ", 1) + if len(parts) != 2: + continue + name = parts[0].removeprefix("origin/") + heads[name] = parts[1] + return heads + + async def _branch_exists(self, gitdir: str, private_dir: str, branch: str) -> bool: + result = await self._git( + gitdir=gitdir, + work_tree=None, + args=["rev-parse", "--verify", "--quiet", f"refs/heads/{branch}"], + private_dir=private_dir, + check=False, + ) + return result.ok and bool(result.stdout.strip()) + + # -- workspace ----------------------------------------------------------- + + async def ensure_workspace(self, *, project_id: str) -> WorkspaceRef: + source = await self._source(project_id) + root, gitdir, work = self._paths_for(source) + private_dir = self._private_dir(source) + + async with self._lock(project_id): + os.makedirs(root, exist_ok=True) + # The work tree must exist before `git checkout`: git refuses to + # materialise a work tree it cannot see. + os.makedirs(work, exist_ok=True) + for directory in (root, gitdir, work, private_dir): + with contextlib.suppress(OSError): + os.chmod(directory, 0o700) + + if not os.path.isdir(os.path.join(gitdir, "objects")): + os.makedirs(gitdir, exist_ok=True) + await self._git( + gitdir=gitdir, + work_tree=None, + args=["init", "--bare", "--quiet"], + private_dir=private_dir, + ) + # A bare repo with an explicit work tree: this is what lets the + # same gitdir serve the main tree and every task worktree. + await self._git( + gitdir=gitdir, + work_tree=None, + args=["config", "core.bare", "false"], + private_dir=private_dir, + ) + # Keep task worktrees out of the main tree's status without + # touching the project's own .gitignore. + exclude = os.path.join(gitdir, "info", "exclude") + os.makedirs(os.path.dirname(exclude), exist_ok=True) + with open(exclude, "w", encoding="utf-8") as handle: + handle.write("# managed by the platform: task worktrees live here\n") + handle.write(".worktrees/\n") + + await self._ensure_remote(source, gitdir, private_dir) + fetched = await self._fetch(source, gitdir, private_dir) + + heads = await self._remote_heads(gitdir, private_dir) if fetched else {} + if not await self._branch_exists(gitdir, private_dir, WORK_BRANCH): + base = source.default_branch if source.default_branch in heads else None + if base is None and heads: + base = next(iter(heads)) + if base is not None: + await self._git( + gitdir=gitdir, + work_tree=None, + args=["branch", "-f", WORK_BRANCH, f"refs/remotes/origin/{base}"], + private_dir=private_dir, + ) + else: + # An empty repository: give it a root commit so `dev` exists + # and the pipeline has something to build. + await self._create_root_commit(gitdir, work, private_dir) + + await self._checkout(gitdir, work, private_dir, WORK_BRANCH) + + if source.is_platform and heads.get(WORK_BRANCH) is None: + await self._push_branch(source, gitdir, private_dir, WORK_BRANCH) + + await self._clear_stale_lock(gitdir) + + return WorkspaceRef( + project_id=project_id, + host_path=work, + sandbox_path="/workspace", + default_branch=WORK_BRANCH, + ) + + async def _create_root_commit(self, gitdir: str, work: str, private_dir: str) -> None: + await self._git( + gitdir=gitdir, + work_tree=work, + args=["symbolic-ref", "HEAD", f"refs/heads/{WORK_BRANCH}"], + private_dir=private_dir, + ) + await self._git( + gitdir=gitdir, + work_tree=work, + args=[ + "-c", + "user.name=Smart Organization", + "-c", + "user.email=platform@smart-org.local", + "commit", + "--allow-empty", + "--quiet", + "-m", + "chore: initialize repository", + ], + private_dir=private_dir, + ) + + async def _checkout(self, gitdir: str, work: str, private_dir: str, branch: str) -> None: + await self._git( + gitdir=gitdir, + work_tree=work, + args=["checkout", "-f", "--quiet", branch], + private_dir=private_dir, + ) + + async def _push_branch( + self, source: ProjectGitSource, gitdir: str, private_dir: str, branch: str + ) -> PushOutcome: + with self._credential_files(source) as env: + result = await self._git( + gitdir=gitdir, + work_tree=None, + args=["push", "origin", f"refs/heads/{branch}:refs/heads/{branch}"], + private_dir=private_dir, + env=env, + timeout_seconds=PUSH_TIMEOUT_SECONDS, + check=False, + ) + return self._push_outcome(result, gitdir, private_dir, branch) + + def _push_outcome( + self, result: ProcessResult, gitdir: str, private_dir: str, branch: str + ) -> PushOutcome: + combined = f"{result.stdout}\n{result.stderr}" + rejected = "non-fast-forward" in combined or "[rejected]" in combined + if result.ok: + return PushOutcome(ok=True, sha=None, message="") + return PushOutcome( + ok=False, + rejected=rejected, + message=redact_text(combined)[:400] or "push failed", + ) + + # -- worktrees ----------------------------------------------------------- + + async def create_task_worktree( + self, *, project_id: str, task_id: str, base: str = WORK_BRANCH + ) -> WorktreeRef: + source = await self._source(project_id) + _, gitdir, work = self._paths_for(source) + private_dir = self._private_dir(source) + safe_task = _check_segment(task_id, "کار") + branch = f"{TASK_BRANCH_PREFIX}{safe_task}" + path = os.path.join(work, ".worktrees", safe_task) + + async with self._lock(project_id): + os.makedirs(os.path.dirname(path), exist_ok=True) + if os.path.isdir(path): + # Idempotent: a retried turn must not fail because the worktree + # is already there. + return self._worktree_ref(project_id, safe_task, branch, path) + + base_branch = ( + base if await self._branch_exists(gitdir, private_dir, base) else WORK_BRANCH + ) + result = await self._git( + gitdir=gitdir, + work_tree=work, + args=["worktree", "add", "-b", branch, path, base_branch], + private_dir=private_dir, + check=False, + ) + if not result.ok: + # The branch may survive from a previous attempt; reuse it. + if "already exists" in result.stderr: + result = await self._git( + gitdir=gitdir, + work_tree=work, + args=["worktree", "add", path, branch], + private_dir=private_dir, + check=False, + ) + if not result.ok: + raise GitOperationError( + f"could not create worktree: {redact_text(result.stderr)[:400]}" + ) + + return self._worktree_ref(project_id, safe_task, branch, path) + + @staticmethod + def _worktree_ref(project_id: str, task_id: str, branch: str, path: str) -> WorktreeRef: + return WorktreeRef( + project_id=project_id, + task_id=task_id, + branch=branch, + host_path=path, + sandbox_path=f"/workspace/.worktrees/{task_id}", + ) + + async def remove_worktree(self, *, worktree: WorktreeRef) -> None: + source = await self._source(worktree.project_id) + _, gitdir, work = self._paths_for(source) + private_dir = self._private_dir(source) + + async with self._lock(worktree.project_id): + await self._git( + gitdir=gitdir, + work_tree=work, + args=["worktree", "remove", "--force", worktree.host_path], + private_dir=private_dir, + check=False, + ) + await self._git( + gitdir=gitdir, + work_tree=None, + args=["worktree", "prune"], + private_dir=private_dir, + check=False, + ) + with contextlib.suppress(OSError): + os.rmdir(worktree.host_path) + + # -- diff ---------------------------------------------------------------- + + async def diff(self, *, worktree: WorktreeRef) -> DiffResult: + source = await self._source(worktree.project_id) + _, shared_gitdir, _ = self._paths_for(source) + gitdir = self._worktree_gitdir(shared_gitdir, worktree.task_id) + private_dir = self._private_dir(source) + tree = worktree.host_path + + status_result = await self._git( + gitdir=gitdir, + work_tree=tree, + args=["status", "--porcelain=v1", "-uall", "-z"], + private_dir=private_dir, + check=False, + ) + changed = _parse_status_z(status_result.stdout) + + diff_result = await self._git( + gitdir=gitdir, + work_tree=tree, + args=["diff", "--no-color", "--no-ext-diff", "HEAD"], + private_dir=private_dir, + check=False, + ) + text = diff_result.stdout + if len(text) > MAX_DIFF_CHARS: + text = text[:MAX_DIFF_CHARS] + "\n…[diff truncated]\n" + + escaping: list[str] = [] + largest = 0 + for relative in changed: + absolute = os.path.join(tree, relative) + try: + if os.path.islink(absolute): + target = os.path.realpath(absolute) + root_real = os.path.realpath(tree) + if target != root_real and not target.startswith(root_real + os.sep): + escaping.append(relative) + continue + if os.path.isfile(absolute): + largest = max(largest, os.path.getsize(absolute)) + except OSError: + continue + + return DiffResult( + text=text, + changed_files=sorted(changed), + escaping_symlinks=sorted(escaping), + largest_file_bytes=largest, + ) + + # -- commit / merge / push ----------------------------------------------- + + async def commit(self, *, worktree: WorktreeRef, message: str, author: str) -> str | None: + source = await self._source(worktree.project_id) + _, shared_gitdir, _ = self._paths_for(source) + gitdir = self._worktree_gitdir(shared_gitdir, worktree.task_id) + private_dir = self._private_dir(source) + tree = worktree.host_path + + async with self._lock(worktree.project_id): + await self._git( + gitdir=gitdir, + work_tree=tree, + args=["add", "-A", "--", "."], + private_dir=private_dir, + ) + staged = await self._git( + gitdir=gitdir, + work_tree=tree, + args=["diff", "--cached", "--quiet", "--exit-code"], + private_dir=private_dir, + check=False, + ) + if staged.exit_code == 0: + return None # nothing to commit + + # Lower-cased: the local part of an email address is + # case-insensitive in practice, and a display name like "Alice" + # would otherwise produce a mixed-case address in the log. + email = f"{_slug_for_branch(author).lower()}@smart-org.local" + await self._git( + gitdir=gitdir, + work_tree=tree, + args=[ + "-c", + f"user.name={author or 'AI employee'}", + "-c", + f"user.email={email}", + "commit", + "--quiet", + "--no-verify", + "-m", + message, + ], + private_dir=private_dir, + ) + head = await self._git( + gitdir=gitdir, + work_tree=tree, + args=["rev-parse", "HEAD"], + private_dir=private_dir, + ) + return head.stdout.strip() or None + + async def sync_with_base(self, *, worktree: WorktreeRef) -> MergeOutcome: + source = await self._source(worktree.project_id) + _, shared_gitdir, _ = self._paths_for(source) + gitdir = self._worktree_gitdir(shared_gitdir, worktree.task_id) + private_dir = self._private_dir(source) + tree = worktree.host_path + + async with self._lock(worktree.project_id): + # "The base moved" means *this branch* gained commits from the + # merge — that is the signal the engine uses to decide the tests + # must run again. Comparing the base branch's sha before and after + # would always be equal: merging does not move the base. + before = await self._head_of(gitdir, private_dir) + result = await self._git( + gitdir=gitdir, + work_tree=tree, + args=[ + "-c", + "user.name=Smart Organization", + "-c", + "user.email=platform@smart-org.local", + "merge", + "--no-ff", + "--no-edit", + WORK_BRANCH, + ], + private_dir=private_dir, + check=False, + ) + if result.ok: + after = await self._head_of(gitdir, private_dir) + return MergeOutcome(ok=True, sha=after, base_changed=before != after) + + conflicts = await self._conflict_files(gitdir, tree, private_dir) + await self._git( + gitdir=gitdir, + work_tree=tree, + args=["merge", "--abort"], + private_dir=private_dir, + check=False, + ) + return MergeOutcome( + ok=False, + conflict_files=conflicts, + message=redact_text(result.stderr)[:400], + ) + + async def merge_into( + self, *, project_id: str, branch: str, target: str = WORK_BRANCH + ) -> MergeOutcome: + source = await self._source(project_id) + _, gitdir, work = self._paths_for(source) + private_dir = self._private_dir(source) + + async with self._lock(project_id): + before = await self._rev_parse(gitdir, private_dir, target) + + # The main work tree is the platform's; forcing the checkout keeps a + # stray edit from blocking a merge, and nothing of value lives there. + checkout = await self._git( + gitdir=gitdir, + work_tree=work, + args=["checkout", "-f", "--quiet", target], + private_dir=private_dir, + check=False, + ) + if not checkout.ok: + return MergeOutcome( + ok=False, message=redact_text(checkout.stderr)[:400] + ) + + result = await self._git( + gitdir=gitdir, + work_tree=work, + args=[ + "-c", + "user.name=Smart Organization", + "-c", + "user.email=platform@smart-org.local", + "merge", + "--no-ff", + "--no-edit", + branch, + ], + private_dir=private_dir, + check=False, + ) + if result.ok: + after = await self._rev_parse(gitdir, private_dir, target) + return MergeOutcome( + ok=True, sha=after, base_changed=before != after and before is not None + ) + + conflicts = await self._conflict_files(gitdir, work, private_dir) + # Never leave a half-merge behind: the next operation on this work + # tree would otherwise fail with "you have unmerged paths". + await self._git( + gitdir=gitdir, + work_tree=work, + args=["merge", "--abort"], + private_dir=private_dir, + check=False, + ) + return MergeOutcome( + ok=False, + conflict_files=conflicts, + message=redact_text(result.stderr)[:400], + ) + + async def _conflict_files(self, gitdir: str, tree: str, private_dir: str) -> list[str]: + result = await self._git( + gitdir=gitdir, + work_tree=tree, + args=["diff", "--name-only", "--diff-filter=U"], + private_dir=private_dir, + check=False, + ) + return [line.strip() for line in result.stdout.splitlines() if line.strip()] + + async def _head_of(self, gitdir: str, private_dir: str) -> str | None: + """The sha ``HEAD`` currently points at, or ``None`` on an unborn branch.""" + result = await self._git( + gitdir=gitdir, + work_tree=None, + args=["rev-parse", "--verify", "--quiet", "HEAD"], + private_dir=private_dir, + check=False, + ) + return result.stdout.strip() or None + + async def _rev_parse(self, gitdir: str, private_dir: str, ref: str) -> str | None: + result = await self._git( + gitdir=gitdir, + work_tree=None, + args=["rev-parse", "--verify", "--quiet", f"refs/heads/{ref}"], + private_dir=private_dir, + check=False, + ) + return result.stdout.strip() or None + + async def push(self, *, project_id: str, branch: str) -> PushOutcome: + source = await self._source(project_id) + _, gitdir, _ = self._paths_for(source) + private_dir = self._private_dir(source) + + async with self._lock(project_id): + outcome = await self._push_branch(source, gitdir, private_dir, branch) + if not outcome.ok and outcome.rejected: + # The remote moved ahead. Fetch so the caller can merge and + # retry — never force, which would destroy someone else's work. + await self._fetch(source, gitdir, private_dir) + return outcome + + async def head(self, *, project_id: str, branch: str) -> str | None: + source = await self._source(project_id) + _, gitdir, _ = self._paths_for(source) + return await self._rev_parse(gitdir, self._private_dir(source), branch) + + # -- remote testing ------------------------------------------------------ + + async def test_remote( + self, *, url: str, credentials: RemoteCredentials + ) -> RemoteCheck: + """Probe a user-supplied remote without leaking anything about it. + + The SSRF check runs *first*, before a single byte leaves the host: a + user-supplied URL is an obvious route to ``http://169.254.169.254`` or an + internal service. The check resolves the host and refuses private, + loopback and link-local addresses unless the operator has allowlisted + them (``DEPLOY_ALLOW_PRIVATE_HOSTS``). + """ + clean_url = (url or "").strip() + if not clean_url: + return RemoteCheck(ok=False, message="آدرس مخزن خالی است.") + + try: + assert_remote_host_allowed( + clean_url, allow_private=settings.deploy_allow_private_hosts + ) + except Exception as exc: # noqa: BLE001 - translated to a plain message + return RemoteCheck(ok=False, message=str(exc)) + + source = ProjectGitSource( + project_id="probe", + organization_id="probe", + clone_url=clean_url, + auth_type=credentials.auth_type, + username=credentials.username, + secret=credentials.secret, + ) + private_dir = os.path.join(self.root, ".probe") + os.makedirs(private_dir, exist_ok=True) + with contextlib.suppress(OSError): + os.chmod(private_dir, 0o700) + + with self._credential_files(source) as env: + result = await self._runner( + [ + self.git_bin, + *self._config_flags(), + "ls-remote", + "--heads", + clean_url, + ], + timeout_seconds=REMOTE_TEST_TIMEOUT_SECONDS, + max_output_bytes=self.max_output_bytes, + env={**self._base_env(private_dir), **env}, + ) + + if result.timed_out: + return RemoteCheck(ok=False, message="اتصال به مخزن در زمان مجاز برقرار نشد.") + if not result.ok: + return RemoteCheck(ok=False, message=_friendly_remote_error(result.stderr)) + + branches: list[str] = [] + for line in result.stdout.splitlines(): + parts = line.split() + if len(parts) >= 2 and parts[1].startswith("refs/heads/"): + branches.append(parts[1][len("refs/heads/") :]) + + default_branch = "main" if "main" in branches else (branches[0] if branches else None) + return RemoteCheck( + ok=True, + default_branch=default_branch, + branches=sorted(branches), + message="اتصال به مخزن برقرار شد.", + ) + + +def _parse_status_z(raw: str) -> list[str]: + """Parse ``status --porcelain=v1 -z`` into changed paths. + + ``-z`` separates entries with NUL and, for a rename or copy, emits the + destination and the source as two NUL-terminated fields after the status + code. Splitting on newlines instead would mangle a filename containing a + newline — which is exactly the filename an attacker would choose. + """ + fields = raw.split("\x00") + changed: list[str] = [] + index = 0 + while index < len(fields): + entry = fields[index] + if not entry or len(entry) < 4: + index += 1 + continue + code = entry[:2] + path = entry[3:] + changed.append(path) + if code[0] in ("R", "C"): + # The original path follows in its own field. + index += 1 + index += 1 + return changed + + +def _friendly_remote_error(stderr: str) -> str: + """Translate git's stderr into a plain message with no secrets in it. + + Deliberately never returns the raw stderr: git echoes the remote URL, and a + credential helper can write a token to stderr on failure. + """ + lowered = (stderr or "").lower() + if "authentication failed" in lowered or "could not read username" in lowered: + return "احراز هویت ناموفق بود. نام کاربری و رمز/توکن را بررسی کنید." + if "permission denied (publickey" in lowered: + return "کلید SSH پذیرفته نشد." + if "repository not found" in lowered or "not found" in lowered: + return "مخزن پیدا نشد یا دسترسی به آن وجود ندارد." + if "could not resolve host" in lowered or "name or service not known" in lowered: + return "میزبان مخزن پیدا نشد." + if "connection refused" in lowered or "timed out" in lowered: + return "اتصال به مخزن برقرار نشد." + if "host key verification failed" in lowered: + return "کلید میزبان تأیید نشد." + return "اتصال به مخزن برقرار نشد." + + +def fingerprint_for(secret: str) -> str: + """Stable short digest of a secret, for logging "the same one" without it.""" + return hashlib.sha256(secret.encode("utf-8", "replace")).hexdigest()[:12] + + +__all__ = [ + "GIT_TIMEOUT_SECONDS", + "GitOperationError", + "LocalGitWorkspaceAdapter", + "ProjectGitSource", + "TASK_BRANCH_PREFIX", + "WORK_BRANCH", + "fingerprint_for", +] diff --git a/apps/api/app/infrastructure/repositories/projects.py b/apps/api/app/infrastructure/repositories/projects.py index a22609a..3cc11f1 100644 --- a/apps/api/app/infrastructure/repositories/projects.py +++ b/apps/api/app/infrastructure/repositories/projects.py @@ -6,14 +6,17 @@ from __future__ import annotations from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from app.domain.base import utcnow from app.domain.projects.models import ( Project, + ProjectExternalGit, ProjectGitCredential, ProjectGitRepo, ) from app.infrastructure.db.models import ( PreviewDomainRow, PreviewPortRow, + ProjectExternalGitRow, ProjectGitCredentialRow, ProjectRepositoryRow, ProjectRow, @@ -36,6 +39,53 @@ def _repo_to_domain(row: ProjectRepositoryRow) -> ProjectGitRepo: ) +class ProjectExternalGitRepository: + """Persistence for a user-supplied remote's credentials. + + The secret is stored encrypted and is *never* returned by this repository: + callers that need it (the git workspace adapter) ask for it explicitly + through :meth:`get_secret`, which keeps the plaintext out of every code path + that only needs to render a status. + """ + + def __init__(self, session: AsyncSession) -> None: + self.session = session + + async def get_for_project(self, project_id: str) -> ProjectExternalGit | None: + row = await self.session.get(ProjectExternalGitRow, project_id) + return _external_git_to_domain(row) if row else None + + async def upsert(self, record: ProjectExternalGit) -> ProjectExternalGit: + row = await self.session.get(ProjectExternalGitRow, record.project_id) + if row is None: + row = ProjectExternalGitRow(project_id=record.project_id) + self.session.add(row) + row.auth_type = record.auth_type + row.username = record.username + row.secret_encrypted = record.secret_encrypted + row.known_hosts = record.known_hosts + row.updated_at = utcnow() + await self.session.flush() + return _external_git_to_domain(row) + + async def delete_for_project(self, project_id: str) -> None: + row = await self.session.get(ProjectExternalGitRow, project_id) + if row is not None: + await self.session.delete(row) + await self.session.flush() + + +def _external_git_to_domain(row: ProjectExternalGitRow) -> ProjectExternalGit: + return ProjectExternalGit( + project_id=row.project_id, + auth_type=row.auth_type, + username=row.username, + secret_encrypted=row.secret_encrypted, + known_hosts=row.known_hosts, + updated_at=row.updated_at, + ) + + class ProjectGitRepoRepository: """Persistence for the Git repository attached to a project.""" diff --git a/apps/api/pyproject.toml b/apps/api/pyproject.toml index 8a49a17..885b470 100644 --- a/apps/api/pyproject.toml +++ b/apps/api/pyproject.toml @@ -61,6 +61,13 @@ ignore = ["B008"] asyncio_mode = "auto" testpaths = ["tests"] filterwarnings = ["ignore::DeprecationWarning"] +# `docker` marks tests that need a real container runtime, `deploy_it` those +# that need a live sshd/ftp server. Both are skipped unless the matching +# environment flag is set, so the default run needs no infrastructure. +markers = [ + "docker: needs a running docker/podman (set SANDBOX_DOCKER_IT=1)", + "deploy_it: needs a live ssh/ftp test server (set DEPLOY_IT=1)", +] [tool.mypy] python_version = "3.12" diff --git a/apps/api/tests/test_git_workspace.py b/apps/api/tests/test_git_workspace.py new file mode 100644 index 0000000..d192d99 --- /dev/null +++ b/apps/api/tests/test_git_workspace.py @@ -0,0 +1,693 @@ +"""§v2-A3.2 — the platform's git adapter, exercised against real git. + +These tests run a real ``git`` binary against a local bare repository standing in +for the remote. Nothing is faked except the *remote's location*: the flags, the +locking, the worktree dance and the credential plumbing are the real thing, +because those are precisely the parts that cannot be verified by a mock. + +``protocol.file.allow=user`` is passed explicitly. The production default is +``never`` — a repository must not be able to make git read a local path through a +submodule or ``--upload-pack`` — but a local bare repo *is* a local path, so the +hardening has to be relaxed for the test to be able to run at all. That is why +the setting exists rather than being hard-coded. +""" + +from __future__ import annotations + +import asyncio +import os + +import pytest + +from app.core.errors import NotFoundError +from app.domain.platform.rules import RuleViolation, assert_remote_host_allowed, is_blocked_address +from app.infrastructure.adapters.git_workspace import ( + LocalGitWorkspaceAdapter, + ProjectGitSource, +) +from app.infrastructure.adapters.process import ProcessResult, run_process + +# --------------------------------------------------------------------------- +# helpers +# --------------------------------------------------------------------------- + + +async def _git(args: list[str], *, cwd: str, ok: bool = True) -> ProcessResult: + result = await run_process( + ["git", *args], + cwd=cwd, + timeout_seconds=60, + env={ + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_TERMINAL_PROMPT": "0", + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@example.com", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@example.com", + }, + ) + if ok: + assert result.ok, f"git {' '.join(args)} failed: {result.stderr}" + return result + + +async def _make_remote(tmp_path) -> str: + """A local bare repository with one commit on ``main``; returns its path.""" + remote = tmp_path / "remote.git" + seed = tmp_path / "seed" + await _git(["init", "--quiet", "--bare", str(remote)], cwd=str(tmp_path)) + await _git(["init", "--quiet", str(seed)], cwd=str(tmp_path)) + (seed / "README.md").write_text("hello\n") + await _git(["add", "-A"], cwd=str(seed)) + await _git(["commit", "--quiet", "-m", "init"], cwd=str(seed)) + await _git(["branch", "-M", "main"], cwd=str(seed)) + await _git(["remote", "add", "origin", str(remote)], cwd=str(seed)) + await _git(["push", "--quiet", "origin", "main"], cwd=str(seed)) + return str(remote) + + +def _resolver(clone_url: str, *, auth_type: str = "none", secret: str | None = None, + username: str | None = None): + async def resolve(project_id: str) -> ProjectGitSource | None: + if project_id == "missing": + return None + return ProjectGitSource( + project_id=project_id, + organization_id="org1", + clone_url=clone_url, + default_branch="main", + auth_type=auth_type, + username=username, + secret=secret, + ) + + return resolve + + +def _adapter(tmp_path, clone_url: str, **kwargs) -> LocalGitWorkspaceAdapter: + kwargs.setdefault("allow_file_protocol", True) + kwargs.setdefault("runner", run_process) + return LocalGitWorkspaceAdapter( + resolver=_resolver(clone_url, **kwargs.pop("credentials", {})), + root=str(tmp_path / "root"), + **kwargs, + ) + + +# --------------------------------------------------------------------------- +# ensure_workspace +# --------------------------------------------------------------------------- + + +async def test_ensure_workspace_clones_and_creates_dev(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + + ref = await adapter.ensure_workspace(project_id="proj1") + + assert ref.default_branch == "dev" + assert os.path.isdir(ref.host_path), "the work tree must exist for the sandbox mount" + assert (tmp_path / "root" / "org1" / "proj1" / "work" / "README.md").is_file() + assert await adapter.head(project_id="proj1", branch="dev") is not None + # `dev` was pushed to the platform remote because it did not exist there. + assert await adapter.head(project_id="proj1", branch="dev") is not None + + +async def test_ensure_workspace_is_idempotent(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + + first = await adapter.ensure_workspace(project_id="proj1") + (tmp_path / "root" / "org1" / "proj1" / "work" / "new.txt").write_text("x") + second = await adapter.ensure_workspace(project_id="proj1") + + assert first.host_path == second.host_path + assert (tmp_path / "root" / "org1" / "proj1" / "work" / "new.txt").is_file() + + +async def test_ensure_workspace_rejects_an_unknown_project(tmp_path): + adapter = _adapter(tmp_path, str(tmp_path / "nothing.git")) + with pytest.raises(NotFoundError): + await adapter.ensure_workspace(project_id="missing") + + +async def test_gitdir_is_not_inside_the_work_tree(tmp_path): + """The git dir must never be reachable from the sandbox mount (D6).""" + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + ref = await adapter.ensure_workspace(project_id="proj1") + + gitdir = tmp_path / "root" / "org1" / "proj1" / "git" + assert gitdir.is_dir() + assert not os.path.exists(os.path.join(ref.host_path, ".git")) + assert not str(gitdir).startswith(ref.host_path + os.sep) + + +# --------------------------------------------------------------------------- +# worktrees +# --------------------------------------------------------------------------- + + +async def test_task_worktree_is_created_and_removed(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + assert worktree.branch == "task/t1" + assert worktree.sandbox_path == "/workspace/.worktrees/t1" + assert os.path.isdir(worktree.host_path) + + await adapter.remove_worktree(worktree=worktree) + assert not os.path.isdir(worktree.host_path) + + +async def test_task_worktree_is_invisible_in_the_main_work_tree(tmp_path): + """`info/exclude` keeps .worktrees out of `status` without touching .gitignore.""" + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + ref = await adapter.ensure_workspace(project_id="proj1") + await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + result = await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", ref.host_path, + "status", "--porcelain=v1", "-uall"], + cwd=ref.host_path, + ) + assert result.stdout.strip() == "", "the worktree dir must not show up as untracked" + assert not os.path.exists(os.path.join(ref.host_path, ".gitignore")) + + +async def test_create_task_worktree_is_idempotent(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + + first = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + second = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + assert first.host_path == second.host_path + + +# --------------------------------------------------------------------------- +# diff / commit +# --------------------------------------------------------------------------- + + +async def test_diff_reports_a_new_directory_thanks_to_uall(tmp_path): + """`git status` without ``-uall`` hides new directories — a real agybot bug.""" + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + (tmp_path / "root" / "org1" / "proj1" / "work" / ".worktrees" / "t1" / "src").mkdir() + ( + tmp_path / "root" / "org1" / "proj1" / "work" / ".worktrees" / "t1" / "src" / "a.py" + ).write_text("print('hi')\n") + + diff = await adapter.diff(worktree=worktree) + + assert diff.changed_files == ["src/a.py"] + assert diff.is_empty is False + + +async def test_diff_detects_an_escaping_symlink(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + outside = tmp_path / "outside.txt" + outside.write_text("secret") + os.symlink(outside, os.path.join(worktree.host_path, "escape.txt")) + + diff = await adapter.diff(worktree=worktree) + + assert diff.escaping_symlinks == ["escape.txt"] + + +async def test_commit_returns_none_when_there_is_nothing_to_commit(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + assert await adapter.commit(worktree=worktree, message="empty", author="AI") is None + + +async def test_commit_records_the_author_and_returns_a_sha(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + target = os.path.join(worktree.host_path, "feature.py") + with open(target, "w", encoding="utf-8") as handle: + handle.write("x = 1\n") + + sha = await adapter.commit(worktree=worktree, message="task/t1: add feature", author="Alice") + + assert sha and len(sha) == 40 + shared_gitdir = tmp_path / "root" / "org1" / "proj1" / "git" + log = await _git( + ["--git-dir", str(shared_gitdir), "log", "-1", "--format=%an <%ae>|%s", "task/t1"], + cwd=str(tmp_path), + ) + assert log.stdout.strip() == "Alice |task/t1: add feature" + + # ...and specifically to `task/t1`, not to `dev`. A shared gitdir makes this + # the easiest thing in the world to get wrong. + task_head = await _git( + ["--git-dir", str(shared_gitdir), "rev-parse", "task/t1"], cwd=str(tmp_path) + ) + assert task_head.stdout.strip() == sha + dev_head = await _git( + ["--git-dir", str(shared_gitdir), "rev-parse", "dev"], cwd=str(tmp_path) + ) + assert dev_head.stdout.strip() != sha, "the commit must not land on dev directly" + + +# --------------------------------------------------------------------------- +# merge / push / head +# --------------------------------------------------------------------------- + + +async def test_merge_into_dev_and_push(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + with open(os.path.join(worktree.host_path, "a.txt"), "w", encoding="utf-8") as handle: + handle.write("a\n") + sha = await adapter.commit(worktree=worktree, message="task/t1: a", author="AI") + + merged = await adapter.merge_into(project_id="proj1", branch="task/t1", target="dev") + assert merged.ok is True, merged.message + + # `--no-ff` means dev gets a merge *commit*, not the task commit itself, so + # the task sha is a parent of dev's new head rather than dev's head. + dev_head = await adapter.head(project_id="proj1", branch="dev") + assert dev_head == merged.sha + assert dev_head != sha + ancestry = await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "rev-list", "--parents", "-1", "dev"], + cwd=str(tmp_path), + ) + assert sha in ancestry.stdout.split() + + pushed = await adapter.push(project_id="proj1", branch="dev") + assert pushed.ok is True, pushed.message + + # The remote really received it. + ls = await _git(["--git-dir", remote, "rev-parse", "dev"], cwd=str(tmp_path)) + assert ls.stdout.strip() == dev_head + + +async def test_merge_conflict_leaves_a_clean_tree(tmp_path): + """A half-merge would break every later operation (agybot bug H3). + + The task branch has to fork *before* ``dev`` moves, otherwise the merge is a + trivial fast-forward and there is nothing to conflict about. + """ + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + + # 1. The task branches off dev as it stands. + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + with open(os.path.join(worktree.host_path, "README.md"), "w", encoding="utf-8") as handle: + handle.write("task version\n") + await adapter.commit(worktree=worktree, message="task/t1: edit", author="AI") + + # 2. dev moves on, touching the same file. + main_work = tmp_path / "root" / "org1" / "proj1" / "work" + (main_work / "README.md").write_text("dev version\n") + await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(main_work), "add", "-A"], + cwd=str(main_work), + ) + await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(main_work), "commit", "--quiet", "-m", "dev edit"], + cwd=str(main_work), + ) + + outcome = await adapter.merge_into(project_id="proj1", branch="task/t1", target="dev") + + assert outcome.ok is False + assert outcome.conflict_files == ["README.md"] + + # No merge in progress: the next status is clean. + status = await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(main_work), "status", "--porcelain=v1", "-uall"], + cwd=str(main_work), + ) + assert status.stdout.strip() == "" + assert not os.path.exists( + tmp_path / "root" / "org1" / "proj1" / "git" / "MERGE_HEAD" + ) + + +async def test_sync_with_base_reports_that_the_base_moved(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + # Nothing moved yet. + quiet = await adapter.sync_with_base(worktree=worktree) + assert quiet.ok is True and quiet.base_changed is False + + # Move dev forward. + main_work = tmp_path / "root" / "org1" / "proj1" / "work" + (main_work / "other.txt").write_text("x\n") + await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(main_work), "add", "-A"], + cwd=str(main_work), + ) + await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(main_work), "commit", "--quiet", "-m", "dev moved"], + cwd=str(main_work), + ) + + moved = await adapter.sync_with_base(worktree=worktree) + assert moved.ok is True + assert moved.base_changed is True + assert os.path.isfile(os.path.join(worktree.host_path, "other.txt")) + + +async def test_concurrent_merges_are_serialised(tmp_path): + """Without the per-project lock two merges corrupt the index.""" + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + + for task in ("t1", "t2"): + worktree = await adapter.create_task_worktree(project_id="proj1", task_id=task) + with open( + os.path.join(worktree.host_path, f"{task}.txt"), "w", encoding="utf-8" + ) as handle: + handle.write(f"{task}\n") + await adapter.commit(worktree=worktree, message=f"task/{task}", author="AI") + + first, second = await asyncio.gather( + adapter.merge_into(project_id="proj1", branch="task/t1", target="dev"), + adapter.merge_into(project_id="proj1", branch="task/t2", target="dev"), + ) + + assert first.ok is True and second.ok is True + status = await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(tmp_path / "root" / "org1" / "proj1" / "work"), + "status", "--porcelain=v1", "-uall"], + cwd=str(tmp_path), + ) + assert status.stdout.strip() == "" + + +async def test_push_reports_a_rejection_without_forcing(tmp_path): + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + + # Someone else advances the remote's dev, so our push is no longer a + # fast-forward. `origin/dev` is what the clone checked out, not a fresh + # branch off the default one. + other = tmp_path / "other" + await _git(["clone", "--quiet", remote, str(other)], cwd=str(tmp_path)) + await _git(["checkout", "--quiet", "-B", "dev", "origin/dev"], cwd=str(other)) + (other / "divergent.txt").write_text("other\n") + await _git(["add", "-A"], cwd=str(other)) + await _git(["commit", "--quiet", "-m", "divergent"], cwd=str(other)) + await _git(["push", "--quiet", "origin", "dev"], cwd=str(other)) + + # Local dev moves a different way. + main_work = tmp_path / "root" / "org1" / "proj1" / "work" + (main_work / "mine.txt").write_text("mine\n") + await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(main_work), "add", "-A"], + cwd=str(main_work), + ) + await _git( + ["--git-dir", str(tmp_path / "root" / "org1" / "proj1" / "git"), + "--work-tree", str(main_work), "commit", "--quiet", "-m", "mine"], + cwd=str(main_work), + ) + + outcome = await adapter.push(project_id="proj1", branch="dev") + + assert outcome.ok is False + assert outcome.rejected is True, outcome.message + + +# --------------------------------------------------------------------------- +# hardening +# --------------------------------------------------------------------------- + + +async def test_hooks_are_never_executed(tmp_path): + """A repository may ship hooks; a hook is arbitrary host code execution.""" + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + + gitdir = tmp_path / "root" / "org1" / "proj1" / "git" + hooks = gitdir / "hooks" + hooks.mkdir(parents=True, exist_ok=True) + marker = tmp_path / "HOOK_RAN" + hook = hooks / "post-commit" + hook.write_text(f"#!/bin/sh\ntouch {marker}\n") + os.chmod(hook, 0o755) + + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + with open(os.path.join(worktree.host_path, "a.txt"), "w", encoding="utf-8") as handle: + handle.write("a\n") + await adapter.commit(worktree=worktree, message="task/t1", author="AI") + + assert not marker.exists(), "a post-commit hook must not run" + + +async def test_a_tampered_dot_git_file_is_ignored(tmp_path): + """The `.git` pointer inside a worktree is attacker-controlled text.""" + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + await adapter.ensure_workspace(project_id="proj1") + worktree = await adapter.create_task_worktree(project_id="proj1", task_id="t1") + + decoy = tmp_path / "decoy" + await _git(["init", "--quiet", str(decoy)], cwd=str(tmp_path)) + with open(os.path.join(worktree.host_path, ".git"), "w", encoding="utf-8") as handle: + handle.write(f"gitdir: {decoy}\n") + + with open(os.path.join(worktree.host_path, "a.txt"), "w", encoding="utf-8") as handle: + handle.write("a\n") + sha = await adapter.commit(worktree=worktree, message="task/t1", author="AI") + + assert sha is not None + # The commit landed on OUR task branch, not in the decoy. + shared_gitdir = tmp_path / "root" / "org1" / "proj1" / "git" + real = await _git( + ["--git-dir", str(shared_gitdir), "rev-parse", "task/t1"], cwd=str(tmp_path) + ) + assert real.stdout.strip() == sha + decoy_log = await _git( + ["--git-dir", str(decoy), "log", "--oneline"], cwd=str(tmp_path), ok=False + ) + assert decoy_log.exit_code != 0 or "task/t1" not in decoy_log.stdout + + +async def test_credentials_never_appear_in_argv(tmp_path): + """argv is world-readable via `ps`; a secret must travel another way.""" + secret = "ghp_thisisasecrettokenvalue0123456789" + seen: list[tuple[str, ...]] = [] + + async def recording_runner(argv, **kwargs): # noqa: ANN001, ANN003 + seen.append(tuple(argv)) + return await run_process(argv, **kwargs) + + remote = await _make_remote(tmp_path) + adapter = LocalGitWorkspaceAdapter( + resolver=_resolver(remote, auth_type="token", secret=secret, username="git"), + root=str(tmp_path / "root"), + allow_file_protocol=True, + runner=recording_runner, + ) + + # A token against a local path fails, but the *attempt* is what we assert on. + await adapter.ensure_workspace(project_id="proj1") + + assert seen, "the adapter must have invoked git" + for argv in seen: + joined = " ".join(argv) + assert secret not in joined, f"secret leaked into argv: {argv}" + + +async def test_the_credential_file_is_removed_after_the_call(tmp_path): + remote = await _make_remote(tmp_path) + adapter = LocalGitWorkspaceAdapter( + resolver=_resolver(remote, auth_type="token", secret="s3cret-token-value", username="git"), + root=str(tmp_path / "root"), + allow_file_protocol=True, + ) + await adapter.ensure_workspace(project_id="proj1") + + private = tmp_path / "root" / "org1" / "proj1" / "private" + assert private.is_dir() + assert not (private / "credentials").exists(), "the 0600 secret file must be deleted" + assert not (private / "id_key").exists() + + +async def test_the_askpass_helper_is_private_and_holds_no_secret(tmp_path): + remote = await _make_remote(tmp_path) + adapter = LocalGitWorkspaceAdapter( + resolver=_resolver(remote, auth_type="token", secret="s3cret-token-value", username="git"), + root=str(tmp_path / "root"), + allow_file_protocol=True, + ) + await adapter.ensure_workspace(project_id="proj1") + + helper = tmp_path / "root" / "org1" / "proj1" / "private" / "askpass.sh" + assert helper.is_file() + assert oct(helper.stat().st_mode)[-3:] == "700" + assert "s3cret-token-value" not in helper.read_text() + + +# --------------------------------------------------------------------------- +# SSRF: the shared host check +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "address", + [ + "127.0.0.1", + "10.0.0.1", + "172.16.0.1", + "192.168.1.1", + "169.254.169.254", + "::1", + "fd00::1", + "fe80::1", + "0.0.0.0", + ], +) +def test_is_blocked_address_covers_the_dangerous_ranges(address: str): + assert is_blocked_address(address) is True + + +@pytest.mark.parametrize("address", ["8.8.8.8", "1.1.1.1", "2606:4700::1111"]) +def test_is_blocked_address_allows_public_addresses(address: str): + assert is_blocked_address(address) is False + + +@pytest.mark.parametrize( + "url", + [ + "http://169.254.169.254/latest/meta-data/", + "http://127.0.0.1:6379/", + "http://localhost/repo.git", + "https://10.0.0.5/repo.git", + "http://[::1]/repo.git", + "https://user:pass@example.com/repo.git", + "file:///etc/passwd", + "https://example.com:6379/repo.git", + ], +) +def test_assert_remote_host_allowed_refuses_dangerous_targets(url: str): + with pytest.raises(RuleViolation): + assert_remote_host_allowed(url, resolver=lambda *a, **k: []) + + +def test_assert_remote_host_allowed_accepts_a_public_host(): + def resolver(host, port, **kwargs): # noqa: ANN001, ANN003 + return [(2, 1, 6, "", ("93.184.216.34", port))] + + host, port = assert_remote_host_allowed( + "https://git.example.com/team/app.git", resolver=resolver + ) + assert host == "git.example.com" + assert port is None + + +def test_assert_remote_host_allowed_refuses_a_name_that_resolves_private(): + """A public-looking name can resolve into the private network.""" + + def resolver(host, port, **kwargs): # noqa: ANN001, ANN003 + return [(2, 1, 6, "", ("10.1.2.3", port))] + + with pytest.raises(RuleViolation): + assert_remote_host_allowed("https://internal.example.com/repo.git", resolver=resolver) + + +def test_assert_remote_host_allowed_can_be_told_to_allow_private(): + host, _ = assert_remote_host_allowed( + "http://127.0.0.1:8080/repo.git", allow_private=True + ) + assert host == "127.0.0.1" + + +def test_assert_remote_host_allowed_accepts_scp_style_ssh(): + def resolver(host, port, **kwargs): # noqa: ANN001, ANN003 + return [(2, 1, 6, "", ("93.184.216.34", port))] + + host, _ = assert_remote_host_allowed("git@git.example.com:team/app.git", resolver=resolver) + assert host == "git.example.com" + + +async def test_test_remote_refuses_a_private_host(tmp_path): + adapter = _adapter(tmp_path, "https://example.com/x.git") + from app.application.ports.repo_workspace import RemoteCredentials + + check = await adapter.test_remote( + url="http://169.254.169.254/repo.git", credentials=RemoteCredentials() + ) + assert check.ok is False + assert "داخلی" in check.message + + +async def test_test_remote_lists_branches_of_a_local_repo(tmp_path, monkeypatch): + """The ls-remote path, with the SSRF check stubbed out. + + The check itself is covered by the tests above; here the subject is the + parsing of `git ls-remote --heads` and the branch list it produces. A local + bare repo is used as the remote because the suite must not need a network. + """ + import app.infrastructure.adapters.git_workspace as module + from app.application.ports.repo_workspace import RemoteCredentials + + monkeypatch.setattr(module, "assert_remote_host_allowed", lambda *a, **k: ("local", None)) + + remote = await _make_remote(tmp_path) + adapter = _adapter(tmp_path, remote) + + check = await adapter.test_remote( + url=f"file://{remote}", credentials=RemoteCredentials() + ) + + assert check.ok is True, check.message + assert check.branches == ["main"] + assert check.default_branch == "main" + + +async def test_test_remote_reports_a_missing_repository(tmp_path, monkeypatch): + import app.infrastructure.adapters.git_workspace as module + from app.application.ports.repo_workspace import RemoteCredentials + + monkeypatch.setattr(module, "assert_remote_host_allowed", lambda *a, **k: ("local", None)) + + adapter = _adapter(tmp_path, "https://example.com/x.git") + check = await adapter.test_remote( + url=f"file://{tmp_path}/does-not-exist.git", credentials=RemoteCredentials() + ) + + assert check.ok is False + assert check.message diff --git a/docs/plan/reports/agent-3-phase-2.md b/docs/plan/reports/agent-3-phase-2.md new file mode 100644 index 0000000..06fc35d --- /dev/null +++ b/docs/plan/reports/agent-3-phase-2.md @@ -0,0 +1,140 @@ +# Agent 3 — Phase 2 report: git workspaces + +Date: 2026-10-13 · Branch: `dev` + +## What was done + +### 2.1–2.4 `LocalGitWorkspaceAdapter` (`app/infrastructure/adapters/git_workspace.py`) + +Implements every method of `RepoWorkspacePort` with a real host `git` binary. + +**Layout** under `SANDBOX_WORKSPACES_ROOT///`: +`git/` (shared gitdir, never mounted), `work/` (the `dev` work tree, mounted at +`/workspace`), `work/.worktrees//` (one per task), `private/` (0700, holds +credentials for the duration of one call only). `.worktrees/` is excluded through the +**gitdir's** `info/exclude`, never the project's `.gitignore`. + +**Hardening on every call** (asserted by test): explicit `--git-dir`/`--work-tree`; +`core.hooksPath=/dev/null`, `core.fsmonitor=false`, `core.quotePath=false`, +`safe.directory=*`; `GIT_CONFIG_NOSYSTEM=1`; a private `HOME` per project; +`GIT_TERMINAL_PROMPT=0`; `protocol.file.allow` configurable (see the deviation below); +one `asyncio.Lock` per project held for every mutating operation. + +**Credentials.** A token goes through a 0700 `GIT_ASKPASS` script that reads a 0600 +file containing `username\nsecret`; an SSH key is a 0600 temp file referenced by +`GIT_SSH_COMMAND` with `IdentitiesOnly=yes` and a per-project `known_hosts` +(`StrictHostKeyChecking=yes` once pinned, `accept-new` for TOFU on first contact). +Both are deleted in a `finally`. Only a **path** ever travels through the environment — +never the secret itself — so nothing sensitive reaches `argv` (world-readable via `ps`). + +**`test_remote`** runs the shared SSRF check first, then `git ls-remote --heads` with a +20 s timeout, and returns a plain Persian message from a closed set of causes. Raw +stderr is never returned: git echoes the remote URL and a credential helper can write a +token into stderr, so every message goes through `app.core.redact`. + +**`merge_into`** forces the checkout of the target in the main work tree, merges +`--no-ff`, and on conflict collects the unmerged paths and runs `merge --abort` before +returning — a half-merge would break every later operation. **`push`** never forces: +a rejection is reported with `rejected=True` after a fetch. + +### The bug this phase existed to find: linked worktrees need their own gitdir + +`git --git-dir= --work-tree=` does **not** address the task branch. +`--git-dir` names the *main* gitdir, whose `HEAD` is `dev`, so `git commit` in a task +worktree commits straight to `dev` and the task branch never moves. The first version of +this adapter had exactly that bug; it was caught by a test that asserts the task sha is +*not* `dev`'s head. + +The fix is to address the **linked gitdir** that `git worktree add` creates: +`/worktrees/`, which holds that worktree's own `HEAD` and index. It is +passed explicitly rather than discovered from the worktree's `.git` file — that file is +plain text a repository can rewrite, and the test that tampers with it proves the +tampered value is ignored. `_worktree_gitdir()` falls back to the shared gitdir when no +linked one exists, which is the `analysis_only` case (no worktree). + +### 2.3 SSRF check (`app/domain/platform/rules.py`) + +`is_blocked_address()` (pure) classifies an IP literal: loopback, RFC1918 private, +link-local — including the cloud metadata address `169.254.169.254` — unique-local IPv6, +multicast, reserved, unspecified, and IPv4-mapped IPv6 forms of all of them. +`assert_remote_host_allowed()` validates the URL (scheme allowlist, no embedded +credentials, blocked ports) and then resolves the host, refusing if **any** returned +address is blocked — a name can legitimately have both a public and a private address. +The resolver is injectable so the DNS step is fakeable in tests. `localhost` and friends +are refused by name as well as by resolution. + +### 2.5 Wiring (`container_delivery.py`) + +`build_repo_workspace` returns the fake unless `GIT_WORKSPACE=local`, then builds a +resolver that joins `projects` + `project_repositories`/`project_git_credentials` (platform) +or `project_external_git` (external) and hands the decrypted secret to the adapter for one +call. A rotated `ENCRYPTION_KEY` degrades to "no credential" with a log line rather than an +exception. + +Supporting additions: `ProjectExternalGit` domain model + +`ProjectExternalGitRepository` (in Agent 3's own files), and +`app/infrastructure/db/columns.py` is untouched. + +## Files changed + +| File | Change | +|---|---| +| `app/infrastructure/adapters/git_workspace.py` | new — the git adapter (~700 lines) | +| `app/domain/platform/rules.py` | SSRF: `is_blocked_address`, `assert_remote_host_allowed`, `BLOCKED_REMOTE_PORTS` | +| `app/domain/projects/models.py` | `ProjectExternalGit` | +| `app/infrastructure/repositories/projects.py` | `ProjectExternalGitRepository` | +| `app/application/container_delivery.py` | real `build_repo_workspace` + resolver | +| `app/core/config.py`, `.env.example` | `GIT_ALLOW_FILE_PROTOCOL` | +| `pyproject.toml` | registered the `docker` / `deploy_it` markers | +| `tests/test_git_workspace.py` | new — 48 tests | + +## Test output + +``` +apps/api: 394 passed (was 346; +48) + ruff check . -> All checks passed! + mypy app -> Success: no issues found in 128 source files +apps/web: 67 passed (unchanged — no frontend work in this phase) +``` + +The 48 new tests run **real git** against a local bare repository standing in for the +remote; nothing about git itself is mocked. + +## Deviations from the TODO + +1. **`protocol.file.allow` is a setting, not a constant.** The TODO fixes it at `never`. + That is the right production default (it stops a repository from making git read a + local path via a submodule or `--upload-pack`), but it also blocks the local bare repo + the TODO's own test plan asks for. `GIT_ALLOW_FILE_PROTOCOL=false` keeps `never` as the + default and lets the test suite opt in. +2. **The SSRF check lives in `rules.py` as instructed, and it resolves DNS.** That is the + one I/O call in an otherwise pure module; the module docstring now says so, and the + address classification itself is pure and separately tested. +3. **`base_changed` for `sync_with_base` is defined as "the task branch moved".** The + port docstring says "True when the base moved, so the engine must re-run the tests" — + comparing the base branch's sha before and after a merge into the task branch would + always be equal, so the signal would never fire. It is computed from the worktree's + own `HEAD` instead. +4. **`.git` files are not read at all.** The TODO says "never trust the `.git` file + inside a worktree"; the adapter goes further and never opens it, using the linked + gitdir path instead. + +## Open questions / CHANGE REQUESTS + +- **CHANGE REQUEST (for integration):** `container_delivery.build_repo_workspace(session)` + is called with a request-scoped `AsyncSession`. The git adapter's resolver uses that + session, so a `RepoWorkspacePort` handed out by a long-lived worker is only valid while + its session lives. Phase 3's delivery worker must build the port per pass (which is what + `build_repo_workspace(session)` in `worker_session()` already implies). No contract + change needed — noting it so the engine and delivery workers do not cache the port + across sessions. + +## Known limitations + +- `git worktree add` names the linked gitdir after the worktree path's basename. The + adapter relies on that (``), with a fallback to the shared gitdir. A task id + containing characters git rewrites would silently degrade to the fallback; task ids are + platform-generated hex, so this is theoretical. +- Shallow clones are not supported; `ensure_workspace` fetches full history, which is what + the diff guard and merge need. +- No `@pytest.mark.docker` integration test — Phase 2 needs no container.