From 22374f3e1ce4a4fe128e5d182c295af3b9b6034a Mon Sep 17 00:00:00 2001 From: Agent 3 Date: Wed, 14 Oct 2026 12:07:10 +0330 Subject: [PATCH] feat(delivery): final report + reserved-label guard + Windows test fixes (v2 phase A3-final) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gap-closing on top of the merged A3-3..A3-6 work: - ProjectService.create_preview_domain now refuses a label that is a reserved platform name (git, mcp, api, panel, ...) via PROTECTED_HOST_LABELS. The rules set existed but only the owner-facing validate_extra_host consulted it, so the tenant path could reserve one. - process.py: signal.SIGKILL is resolved through getattr with a SIGTERM fallback, so the module imports and type-checks on Windows where SIGKILL does not exist. - Three more Windows test repairs: a git-created .git must be unlinked and recreated rather than overwritten in place; POSIX-only chmod and symlink assertions are gated on os.name (the security property they assert is checked everywhere, the platform-specific mechanics only where they apply). - docs/plan/reports/agent-3-final.md: the integration hand-off — host requirements, every setting, every endpoint added or changed, the tables, the open CHANGE REQUESTS, a 12-step manual QA script and the known limitations. Checks: ruff clean, mypy clean (136 files), 521 API tests pass, 83 web tests pass, tsc clean, next lint clean. --- .../application/services/project_service.py | 8 + .../app/infrastructure/adapters/process.py | 7 +- apps/api/tests/test_docker_sandbox.py | 5 + apps/api/tests/test_git_workspace.py | 20 +- docs/plan/reports/agent-3-final.md | 215 ++++++++++++++++++ 5 files changed, 252 insertions(+), 3 deletions(-) create mode 100644 docs/plan/reports/agent-3-final.md diff --git a/apps/api/app/application/services/project_service.py b/apps/api/app/application/services/project_service.py index 88e4288..a152cb8 100644 --- a/apps/api/app/application/services/project_service.py +++ b/apps/api/app/application/services/project_service.py @@ -32,6 +32,7 @@ from app.core.crypto import decrypt, encrypt from app.core.errors import ConflictError, NotFoundError, PermissionDeniedError, ValidationError from app.domain.base import ensure_unique_slug, slugify from app.domain.platform.rules import ( + PROTECTED_HOST_LABELS, assert_not_protected_port, assert_port_allocatable, build_host, @@ -195,6 +196,13 @@ class ProjectService: port: int | None = None, ) -> dict: label = validate_subdomain_label(requested_subdomain) + # ``git``/``mcp``/``api``/``panel``… are platform services. A tenant host + # must never be able to shadow one on the same root domain — the rules + # module defines the reserved set (see PROTECTED_HOST_LABELS). + if label in PROTECTED_HOST_LABELS: + raise PermissionDeniedError( + f"نام «{label}» برای سرویس‌های سامانه رزرو شده است." + ) host = build_host(label, settings.preview_root_domain) if await self.domains.get_by_host(host) is not None: diff --git a/apps/api/app/infrastructure/adapters/process.py b/apps/api/app/infrastructure/adapters/process.py index 0906746..0bf537e 100644 --- a/apps/api/app/infrastructure/adapters/process.py +++ b/apps/api/app/infrastructure/adapters/process.py @@ -60,6 +60,11 @@ _TERM_GRACE_SECONDS = 5.0 #: pasted into a command line). Refused rather than silently truncated. MAX_ARGV_ELEMENT_CHARS = 100_000 +#: ``SIGKILL`` does not exist on Windows (it has no equivalent), so it is looked +#: up rather than referenced directly. POSIX is the only *supported* target, but +#: the module must still import — and type-check — on a developer's Windows box. +_SIGKILL: int = getattr(signal, "SIGKILL", signal.SIGTERM) + class ProcessError(RuntimeError): """A host command could not be started at all (binary missing, not executable).""" @@ -184,7 +189,7 @@ async def _terminate(proc: asyncio.subprocess.Process) -> None: return except TimeoutError: pass - _signal_group(signal.SIGKILL) + _signal_group(_SIGKILL) with contextlib.suppress(Exception): await proc.wait() diff --git a/apps/api/tests/test_docker_sandbox.py b/apps/api/tests/test_docker_sandbox.py index b123d49..9abbefd 100644 --- a/apps/api/tests/test_docker_sandbox.py +++ b/apps/api/tests/test_docker_sandbox.py @@ -142,6 +142,11 @@ def test_safe_join_refuses_a_parent_escape(tmp_path): safe_join(str(root), "a/b/../../../etc/passwd") +@pytest.mark.skipif( + os.name == "nt", + reason="creating a symlink on Windows requires SeCreateSymbolicLinkPrivilege " + "(admin or Developer Mode); the path-escape guard is covered on POSIX hosts", +) def test_safe_join_refuses_a_symlink_that_escapes(tmp_path): """A symlink planted inside the work dir must not become a host read.""" root = tmp_path / "work" diff --git a/apps/api/tests/test_git_workspace.py b/apps/api/tests/test_git_workspace.py index d192d99..6c8bbd0 100644 --- a/apps/api/tests/test_git_workspace.py +++ b/apps/api/tests/test_git_workspace.py @@ -213,6 +213,11 @@ async def test_diff_reports_a_new_directory_thanks_to_uall(tmp_path): assert diff.is_empty is False +@pytest.mark.skipif( + os.name == "nt", + reason="creating a symlink on Windows requires SeCreateSymbolicLinkPrivilege " + "(admin or Developer Mode); the adapter's guard is covered on POSIX hosts", +) async def test_diff_detects_an_escaping_symlink(tmp_path): remote = await _make_remote(tmp_path) adapter = _adapter(tmp_path, remote) @@ -485,7 +490,14 @@ async def test_a_tampered_dot_git_file_is_ignored(tmp_path): 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: + + # Replace rather than overwrite: on Windows a git-created ``.git`` cannot be + # written in place (it can only be unlinked and recreated). The assertion + # below is identical either way — what matters is the *content* we plant. + pointer = os.path.join(worktree.host_path, ".git") + if os.path.lexists(pointer): + os.unlink(pointer) + with open(pointer, "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: @@ -557,7 +569,11 @@ async def test_the_askpass_helper_is_private_and_holds_no_secret(tmp_path): helper = tmp_path / "root" / "org1" / "proj1" / "private" / "askpass.sh" assert helper.is_file() - assert oct(helper.stat().st_mode)[-3:] == "700" + # ``chmod`` is a no-op on Windows (NTFS has no POSIX mode bits), so the + # executable bit is only asserted where it exists. The security property + # that matters everywhere — no secret inside the helper — is checked below. + if os.name == "posix": + assert oct(helper.stat().st_mode)[-3:] == "700" assert "s3cret-token-value" not in helper.read_text() diff --git a/docs/plan/reports/agent-3-final.md b/docs/plan/reports/agent-3-final.md new file mode 100644 index 0000000..73489a1 --- /dev/null +++ b/docs/plan/reports/agent-3-final.md @@ -0,0 +1,215 @@ +# Agent 3 — Final report: Delivery & Infra (phases 1–6) + +Date: 2026-10-14 · Branch: `dev` · Supersedes and summarises +`agent-3-phase-1.md` … `agent-3-phase-6.md`. + +This file is the integration hand-off: what exists, what it needs on the host, +every setting and endpoint it added, the tables it owns, the change requests still +open, the manual QA script, and the things deliberately not done. + +The golden property, stated once because every design decision serves it: **the AI +never touches the host, the network, or a credential.** Git runs as the platform +user with a gitdir that is never mounted into a container; code runs inside +hardened containers; user-supplied hosts pass an SSRF check before a byte leaves; +and nothing reaches production without a human click. + +--- + +## 1. What exists + +| Phase | Delivered | +|---|---| +| 1 | `DockerSandboxAdapter` (hardened workspace + demo containers), `process.py` (the single hardened subprocess runner), orphan reaping, infra hidden from non-owners | +| 2 | `LocalGitWorkspaceAdapter` — the platform's own git: per-project gitdir, worktrees per task, per-repo lock, hooks disabled, credentials only ever as a *path* | +| 3 | Live CI/CD: pipeline runs/steps/releases/runtimes, build containers, blue/green demo deploy, live batched logs, remote polling | +| 4 | Owner domain console: list/attach/detach/resync routes, custom FQDNs behind a setting, MCP gateway system route | +| 5 | Production deploy: `asyncssh` + `aioftp` adapters with host-key TOFU, atomic symlink switch, FTP backup-first, `production_deploys` ledger | +| 6 | The Application page (four tabs + live build console + production panel), `/admin/domains`, `/admin/sandbox`, `/projects* → /app` redirect, typed client + contracts | + +Phase 5's adapters are **lazy imports**. `asyncssh` and `aioftp` are dependencies, +but importing them costs real start-up time and neither is needed unless somebody +configures a deploy target. `deploy_target.py` therefore resolves them inside +`_connect()`, which is also what keeps `mypy` honest about an optional import. + +--- + +## 2. Host requirements + +| Requirement | Why | Notes | +|---|---|---| +| `docker` **or** `podman` on `PATH` (`SANDBOX_DOCKER_BIN`) | the only sanctioned way tenant code runs | the flags used are the common subset; the workspace/demo/build containers are `--read-only`, non-root, `--cap-drop ALL`, `no-new-privileges`, cpu/mem/pids capped | +| `git` ≥ 2.20 on `PATH` | the platform runs git itself (D6) | worktrees + `--git-dir`/`--work-tree` | +| `caddy` on `PATH` **and** `import /etc/caddy/agy-preview/*.caddy` in the Caddyfile (`DOMAIN_ROUTER=caddy`) | preview + custom domains | the adapter **refuses to run** if the import line is missing — silently writing a file Caddy never reads would report a success that is false | +| Caddy reload permission | saving a route reloads the live proxy | `caddy reload` needs to signal the caddy service; on a systemd host that is a polkit/sudoers entry, **not** root. The command used is `caddy reload --config `, so granting the service user passwordless `caddy reload` is sufficient. | +| Writable, disk-rich dirs under `SANDBOX_WORKSPACES_ROOT` / `ARTIFACTS_ROOT` | work trees, per-run copies, artifacts, tarballs | default `./.data/...` (gitignored). Per-run copies mean disk grows with project size × runs — see Known limitations | +| `ftps` targets | explicit TLS | a plain `ftp` target is accepted and documented as unencrypted | +| `DEPLOY_IT=1` / `SANDBOX_DOCKER_IT=1` | integration tests | both skip cleanly otherwise | + +Nothing else runs on the host. There is no `shell=True` anywhere in the +workstream; `app/infrastructure/adapters/process.py` is the only module that calls +`asyncio.create_subprocess_exec`. + +--- + +## 3. Settings (agent-3 block, `app/core/config.py`) + +| Setting | Default | Meaning | +|---|---|---| +| `SANDBOX_DOCKER_BIN` | `docker` | container runtime CLI (`podman` works) | +| `SANDBOX_WORKSPACES_ROOT` | `./.data/workspaces` | work trees, gitdirs, run dirs | +| `SANDBOX_DEFAULT_CPUS` / `_MEMORY_MB` | `1.0` / `1024` | container caps | +| `DEMO_PORT_START` / `DEMO_PORT_END` | `8100` / `8999` | demo port pool (inside the managed range) | +| `ARTIFACTS_ROOT` | `./.data/artifacts` | release tarballs, FTP backups | +| `DELIVERY_POLL_SECONDS` | `60` | how often remotes are polled for outside pushes | +| `DEPLOY_ALLOW_PRIVATE_HOSTS` | `false` | escape hatch for a private-network target; **off by default** | +| `GIT_WORKSPACE` | `fake` | `local` enables the real host git adapter | +| `ALLOW_CUSTOM_DOMAINS` | `false` | owner-supplied external FQDNs | +| `GIT_ALLOW_FILE_PROTOCOL` | `false` | `protocol.file.allow=never`; `true` only to test against a local bare repo | +| `SANDBOX_PROVIDER` / `DOMAIN_ROUTER` / `GIT_PROVIDER` | `mock` / `mock` / `mock` | shared with Phase 1; `docker`/`caddy` are the real values | + +`.env.example` carries the same block, and the `docker` / `deploy_it` pytest +markers are registered in `pyproject.toml`. + +--- + +## 4. Endpoints added or changed + +All paths are under `/api`. + +### Delivery — `/organizations/{org_id}/delivery` (tenant-scoped) + +| Method | Path | Role | Notes | +|---|---|---|---| +| GET | `/runs` | member | pipeline run list | +| GET | `/runs/{run_id}` | member | run + its 8 steps + log tails | +| POST | `/runs` | org admin/owner | manual re-run of the `dev` head | +| POST | `/runs/{run_id}/cancel` | org admin/owner | | +| GET | `/releases` | member | release history | +| POST | `/releases/rollback` | org admin/owner | demo rollback, slug-confirmed | +| GET | `/runtime` | member | demo URL, live release, health | +| GET | `/app` | member | app summary for the page header | +| GET | `/git` | member | repo URL + `dev` head + last run | +| GET | `/production` | member | target summary + deploy history | +| POST | `/production/test` | org admin/owner | connection test + host-key TOFU | +| POST | `/production/deploy` | org admin/owner | **human-triggered**, `confirm: ""`, one at a time | +| POST | `/production/rollback` | org admin/owner | | + +### Domains — `/admin/domains` (platform owner) + +| Method | Path | Notes | +|---|---|---| +| GET | `` | every route, app routes and system routes | +| POST | `` | attach a host (label, or a custom FQDN when the setting is on) | +| DELETE | `/{domain_id}` | detach, host typed back as confirmation | +| POST | `/sync` | idempotent re-sync from the database | +| GET | `/suggested` | MCP gateway suggestion | + +### Sandbox — `/organizations/{org_id}/sandbox/*` (changed in Phase 1) + +Mutating routes (create/start/stop workspace, run command, read/write file, +snapshot, preview URL, set limits) became **platform-owner only**. Read-only status +(templates, effective limits, workspace list, command log) stayed member-open. + +### Sandbox console — `/admin/sandbox` (new, platform owner) + +`GET /admin/sandbox/containers` and `POST /admin/sandbox/containers/{id}/stop`, +mounted on a separate `admin_router` inside `sandbox.py` so the tenant router +stays org-scoped. + +### Projects — `/organizations/{org_id}/projects/{pid}/preview/*` (changed in Phase 4) + +All four endpoints (ports, domains create/list/delete) became platform-owner only. + +--- + +## 5. Tables (all in `models_delivery.py`) + +`pipeline_runs`, `pipeline_steps`, `releases`, `app_runtimes`, +`production_deploys`, `system_routes`. Migration +`a3b4c5d6e7f3_delivery_infra` (`down_revision = "a0f0e1d2c3b4"`). + +--- + +## 6. Manual QA script + +Run against a real docker + caddy host with +`SANDBOX_PROVIDER=docker GIT_WORKSPACE=local DOMAIN_ROUTER=caddy`. + +1. **Wizard → app**: create an org, complete the wizard with `git_source=platform`. + Expect a demo host `.` claimed and an `app_runtimes` row with a + `pending` status. +2. **First build**: push a commit to `dev`. Watch `org:` receive + `pipeline.queued` → 8 × `pipeline.step` → `pipeline.status`. Expect `checkout`, + `setup`, `build`, `test`, `package`, `deploy_demo`, `healthcheck`, `route`, a + release marked live, and the demo container on the new port. +3. **Open the demo URL** — the healthcheck step already proved the app answers. +4. **Red build**: push a commit that fails the test command. Expect `failed` with a + Persian summary and the **previous demo still serving on the old port**. +5. **Rollback**: from the app page, roll the demo back. The release flips live, + the container is replaced from the artifact, `release.live` fires. +6. **Outside push**: with `git_source=external`, push to the remote from another + machine. Within `DELIVERY_POLL_SECONDS` a run appears with `trigger=poll`. +7. **Production, SSH**: `POST /production/test` against a local `sshd` container, + then `POST /production/deploy` with `confirm: ""`. Expect a release + dir, the atomic `current` symlink switch, the post-deploy hook, and pruning to + the newest 5. Change the server's host key → the next test must **fail** + ("کلید میزبان تغییر کرده است"). +8. **Production, FTP**: the same against a `pure-ftpd` container; expect a + `ftp-backup-.tar.gz` and, on rollback, that archive re-uploaded. +9. **Domains**: as the owner attach a second label; re-sync twice and check the + second call reports `up_to_date: true` with no Caddy reload. Enable the MCP + system route; `mcp.` must answer through the gateway port. +10. **SSRF**: attempt a production deploy to `127.0.0.1`, `10.0.0.1` and + `169.254.169.254`. All three must be refused before any connection. +11. **Tenant isolation**: a member of org B gets 404 on every org-A delivery and + admin-domains path; a non-admin member gets 403 on every mutation. +12. **Secret hygiene**: set a deploy target, run a deploy, then grep the logs, the + `pipeline_steps.log_tail` and the realtime events for the password. Nothing. + +--- + +## 7. CHANGE REQUESTS (still open, for integration) + +1. **The AI workforce (Agent 2) needs the two delivery hooks wired.** + `container_delivery.build_delivery_hooks(session)` returns the real service; + the wizard (Agent 1) should call `on_app_created` and the engine should call + `on_branch_pushed` after a merge+push. Until then nothing queues runs except + a manual `POST /delivery/runs`. +2. **The app profile is Agent 1's.** The Application page reads + `setup/build/test/start commands`, `app_port` and `healthcheck_path` from it. + The Git tab shows the `dev` head; the app-type badges in the header wait for + Phase A1-2. +3. **The web needs the caller's *org role*** to gate the production buttons + client-side. `MeResponse.organizations` already carries `OrganizationWithRole`. + Today the server is the boundary and the UI renders optimistically. +4. **Reserved-label enforcement now lives in two places** and they must stay in + step. The tenant path is guarded in `ProjectService.create_preview_domain` + (`label in PROTECTED_HOST_LABELS` → `PermissionDeniedError`); the owner path is + guarded inside `rules.validate_extra_host`. A future third entry point that + builds a host should call one of them, not re-derive the rule. + +--- + +## 8. Known limitations (deliberate, documented) + +- **Build containers cannot write into the AI's work tree.** Each run gets a + *copy* of the tree under `/…/runs//` (`.git`/`node_modules` + excluded). `RepoWorkspacePort` is frozen and has no checkout primitive, and a + build writing `dist/` into `work/` would show up in the next task's diff as if + an employee had changed it. +- **Disk per run.** Nothing prunes `runs/`, `artifacts/` or `ftp-backup-*` yet. +- **The healthcheck is a plain GET** with 20 × 3 s retries, no readiness protocol, + no auth on the probe path. Any 2xx/3xx counts as healthy. +- **`snapshot` does not capture the work tree** (bind mount) — image state only. +- **FTP has no atomic switch and no host key.** The deploy is backup-first so + there is always something to undo, but the window is not zero, and the plain + `ftp` kind is unencrypted by definition. +- **No container-level egress policy for build containers** beyond the default + bridge. A hermetic build would need an explicit deny and a package mirror. +- **`routes_fingerprint` compares host:port pairs.** Changing a *label* on a demo + host is a remove+add, which the digest does catch; a change that keeps both + host and port but edits the site block body would not. +- **The AI never commits, merges, pushes or inspects git** — that is the point + (D6), but it also means Agent 2's engine must go through `RepoWorkspacePort` + for all of it, and a git failure surfaces to the customer as a Persian message + rather than a git exit code.