Files
smart-org/docs/plan/reports/agent-3-phase-2.md
T
mjalmoudy94 d41e00ad10 feat(delivery): git workspaces + SSRF host check (v2 phase A3-2)
The platform's side of D6: git runs on the host, in a gitdir that is never
mounted into a container, so repository credentials never reach the model.

- adapters/git_workspace.py: LocalGitWorkspaceAdapter implementing
  RepoWorkspacePort. Bare shared gitdir + a dev work tree mounted at
  /workspace + one linked worktree per task under work/.worktrees/<id>
  (excluded via the gitdir's info/exclude, not the project's .gitignore).
  Explicit --git-dir/--work-tree, core.hooksPath=/dev/null, fsmonitor off,
  quotePath off, GIT_CONFIG_NOSYSTEM, a private HOME per project, prompts off,
  one asyncio.Lock per project. Token via GIT_ASKPASS reading a 0600 file; SSH
  key as a 0600 temp file with a pinned known_hosts. Everything deleted in a
  finally; only a *path* ever travels through the environment.
- The bug this phase existed to find: --git-dir=<shared> does not address a
  linked worktree, so `git commit` in a task worktree committed straight to
  `dev`. Operations now address <gitdir>/worktrees/<task_id> explicitly — and
  never read the worktree's .git file, which a repository can rewrite.
- domain/platform/rules.py: is_blocked_address (pure) + assert_remote_host_allowed
  (resolves, refuses if any address is loopback/private/link-local/metadata).
  Shared by the git remote test and, later, every deploy target.
- ProjectExternalGit domain model + repository; container_delivery wires the
  real adapter behind GIT_WORKSPACE=local with a resolver that joins the
  project, its repo and its (decrypted) credentials.

Tests: 394 API (+48) against real git and a local bare repo; ruff and mypy clean.

Report: docs/plan/reports/agent-3-phase-2.md
2026-10-14 00:18:13 +03:30

7.6 KiB
Raw Blame History

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/<org>/<project>/: git/ (shared gitdir, never mounted), work/ (the dev work tree, mounted at /workspace), work/.worktrees/<task_id>/ (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=<shared> --work-tree=<task 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: <gitdir>/worktrees/<task_id>, 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 (<task_id>), 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.