Complete action runtime rewrite
This commit is contained in:
@@ -22,7 +22,14 @@ IMAGE_TAG = "flow-e2e:test"
|
||||
|
||||
def _pick_runtime() -> str | None:
|
||||
for binary in ("podman", "docker"):
|
||||
if shutil.which(binary):
|
||||
if not shutil.which(binary):
|
||||
continue
|
||||
result = subprocess.run(
|
||||
[binary, "version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
return binary
|
||||
return None
|
||||
|
||||
|
||||
60
tests/test_adapters_archive.py
Normal file
60
tests/test_adapters_archive.py
Normal file
@@ -0,0 +1,60 @@
|
||||
"""Archive adapter safety tests."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import tarfile
|
||||
import zipfile
|
||||
|
||||
import pytest
|
||||
|
||||
from flow.adapters.archive import ArchiveClient
|
||||
from flow.adapters.filesystem import FileSystem
|
||||
from flow.core.errors import FlowError
|
||||
|
||||
|
||||
def test_extract_tar_uses_safe_member_paths(tmp_path):
|
||||
archive = tmp_path / "ok.tar.gz"
|
||||
with tarfile.open(archive, "w:gz") as tar:
|
||||
content = b"hello"
|
||||
info = tarfile.TarInfo("pkg/bin/tool")
|
||||
info.size = len(content)
|
||||
tar.addfile(info, io.BytesIO(content))
|
||||
|
||||
target = tmp_path / "extract"
|
||||
ArchiveClient(FileSystem()).extract(archive, target)
|
||||
|
||||
assert (target / "pkg" / "bin" / "tool").read_text() == "hello"
|
||||
|
||||
|
||||
def test_rejects_tar_member_parent_traversal(tmp_path):
|
||||
archive = tmp_path / "bad.tar.gz"
|
||||
with tarfile.open(archive, "w:gz") as tar:
|
||||
content = b"bad"
|
||||
info = tarfile.TarInfo("../escape")
|
||||
info.size = len(content)
|
||||
tar.addfile(info, io.BytesIO(content))
|
||||
|
||||
with pytest.raises(FlowError, match="escapes"):
|
||||
ArchiveClient(FileSystem()).extract(archive, tmp_path / "extract")
|
||||
|
||||
|
||||
def test_rejects_zip_member_parent_traversal(tmp_path):
|
||||
archive = tmp_path / "bad.zip"
|
||||
with zipfile.ZipFile(archive, "w") as zf:
|
||||
zf.writestr("../escape", "bad")
|
||||
|
||||
with pytest.raises(FlowError, match="escapes"):
|
||||
ArchiveClient(FileSystem()).extract(archive, tmp_path / "extract")
|
||||
|
||||
|
||||
def test_rejects_tar_symlink_members(tmp_path):
|
||||
archive = tmp_path / "bad-link.tar"
|
||||
with tarfile.open(archive, "w") as tar:
|
||||
info = tarfile.TarInfo("pkg/link")
|
||||
info.type = tarfile.SYMTYPE
|
||||
info.linkname = "/etc/passwd"
|
||||
tar.addfile(info)
|
||||
|
||||
with pytest.raises(FlowError, match="member type"):
|
||||
ArchiveClient(FileSystem()).extract(archive, tmp_path / "extract")
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import subprocess
|
||||
|
||||
from flow.commands.completion import complete
|
||||
from flow.app.completion import complete
|
||||
from flow.core.config import AppConfig, FlowContext
|
||||
from flow.core.console import Console
|
||||
from flow.core.platform import PlatformInfo
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
"""Tests for flow.core.containers."""
|
||||
"""Tests for flow.adapters.containers."""
|
||||
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from flow.core.containers import ContainerRuntime
|
||||
from flow.adapters.containers import ContainerRuntime
|
||||
from flow.core.errors import FlowError
|
||||
|
||||
from tests.fakes import FakeRunner
|
||||
@@ -53,7 +53,7 @@ class TestMode:
|
||||
rootful.write_text("")
|
||||
compat.write_text("")
|
||||
monkeypatch.setattr(
|
||||
"flow.core.containers.ContainerRuntime._socket_candidates",
|
||||
"flow.adapters.containers.ContainerRuntime._socket_candidates",
|
||||
lambda self: [rootful, rootless, compat],
|
||||
)
|
||||
rt = ContainerRuntime(FakeRunner(), mode="podman-rootful", binary="podman")
|
||||
@@ -65,7 +65,7 @@ class TestMode:
|
||||
rootless.write_text("")
|
||||
rootful.write_text("")
|
||||
monkeypatch.setattr(
|
||||
"flow.core.containers.ContainerRuntime._socket_candidates",
|
||||
"flow.adapters.containers.ContainerRuntime._socket_candidates",
|
||||
lambda self: [rootless, rootful],
|
||||
)
|
||||
rt = ContainerRuntime(FakeRunner(), mode="podman", binary="podman")
|
||||
@@ -77,7 +77,7 @@ class TestSocketPath:
|
||||
sock = tmp_path / "docker.sock"
|
||||
sock.write_text("")
|
||||
monkeypatch.setattr(
|
||||
"flow.core.containers.ContainerRuntime._socket_candidates",
|
||||
"flow.adapters.containers.ContainerRuntime._socket_candidates",
|
||||
lambda self: [sock],
|
||||
)
|
||||
rt = ContainerRuntime(FakeRunner(), binary="docker")
|
||||
@@ -85,7 +85,7 @@ class TestSocketPath:
|
||||
|
||||
def test_docker_socket_missing(self, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
"flow.core.containers.ContainerRuntime._socket_candidates",
|
||||
"flow.adapters.containers.ContainerRuntime._socket_candidates",
|
||||
lambda self: [Path("/nonexistent/docker.sock")],
|
||||
)
|
||||
rt = ContainerRuntime(FakeRunner(), binary="docker")
|
||||
@@ -97,7 +97,7 @@ class TestSocketPath:
|
||||
rootless.write_text("")
|
||||
rootful.write_text("")
|
||||
monkeypatch.setattr(
|
||||
"flow.core.containers.ContainerRuntime._socket_candidates",
|
||||
"flow.adapters.containers.ContainerRuntime._socket_candidates",
|
||||
lambda self: [rootless, rootful],
|
||||
)
|
||||
rt = ContainerRuntime(FakeRunner(), binary="podman")
|
||||
@@ -107,7 +107,7 @@ class TestSocketPath:
|
||||
rootful = tmp_path / "rootful.sock"
|
||||
rootful.write_text("")
|
||||
monkeypatch.setattr(
|
||||
"flow.core.containers.ContainerRuntime._socket_candidates",
|
||||
"flow.adapters.containers.ContainerRuntime._socket_candidates",
|
||||
lambda self: [Path("/nonexistent"), rootful],
|
||||
)
|
||||
rt = ContainerRuntime(FakeRunner(), binary="podman")
|
||||
|
||||
@@ -5,10 +5,10 @@ import os
|
||||
|
||||
import pytest
|
||||
|
||||
from flow.core.containers import ContainerRuntime
|
||||
from flow.adapters.containers import ContainerRuntime
|
||||
from flow.core.errors import FlowError
|
||||
from flow.core.runtime import CommandRunner, FileSystem, GitClient, SystemRuntime
|
||||
from flow.core.tmux import TmuxClient
|
||||
from flow.adapters.tmux import TmuxClient
|
||||
|
||||
|
||||
class TestFileSystem:
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
"""Tests for flow.core.tmux."""
|
||||
"""Tests for flow.adapters.tmux."""
|
||||
|
||||
import subprocess
|
||||
|
||||
from flow.core.tmux import TmuxClient, build_new_session_argv
|
||||
from flow.adapters.tmux import TmuxClient, build_new_session_argv
|
||||
|
||||
from tests.fakes import FakeRunner
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ from flow.core.console import Console
|
||||
from flow.core.errors import FlowError
|
||||
from flow.core.platform import PlatformInfo
|
||||
from flow.core.runtime import SystemRuntime
|
||||
from flow.services.bootstrap import BootstrapService
|
||||
from flow.app.bootstrap import BootstrapService
|
||||
|
||||
|
||||
def _make_ctx(manifest=None):
|
||||
@@ -74,8 +74,8 @@ class TestBootstrapService:
|
||||
def install(self, packages, *, dry_run=False):
|
||||
captured["packages"] = packages
|
||||
|
||||
monkeypatch.setattr("flow.services.packages.PackageService", StubPackageService)
|
||||
monkeypatch.setattr("flow.services.dotfiles.DotfilesService.link", lambda self, profile=None: None)
|
||||
monkeypatch.setattr("flow.app.packages.PackageService", StubPackageService)
|
||||
monkeypatch.setattr("flow.app.dotfiles.DotfilesService.link", lambda self, profile=None: None)
|
||||
|
||||
manifest = {
|
||||
"profiles": {
|
||||
@@ -116,7 +116,7 @@ class TestBootstrapService:
|
||||
def test_run_uses_dotfiles_profile_override(self, monkeypatch):
|
||||
captured = {}
|
||||
|
||||
monkeypatch.setattr("flow.services.packages.PackageService.install", lambda self, packages, dry_run=False: None)
|
||||
monkeypatch.setattr("flow.app.packages.PackageService.install", lambda self, packages, dry_run=False: None)
|
||||
|
||||
class StubDotfilesService:
|
||||
def __init__(self, ctx):
|
||||
@@ -125,7 +125,7 @@ class TestBootstrapService:
|
||||
def link(self, profile=None):
|
||||
captured["profile"] = profile
|
||||
|
||||
monkeypatch.setattr("flow.services.dotfiles.DotfilesService", StubDotfilesService)
|
||||
monkeypatch.setattr("flow.app.dotfiles.DotfilesService", StubDotfilesService)
|
||||
|
||||
manifest = {
|
||||
"profiles": {
|
||||
|
||||
@@ -4,11 +4,11 @@ import subprocess
|
||||
|
||||
from flow.core.config import AppConfig, FlowContext
|
||||
from flow.core.console import Console
|
||||
from flow.core.containers import ContainerRuntime
|
||||
from flow.adapters.containers import ContainerRuntime
|
||||
from flow.core.platform import PlatformInfo
|
||||
from flow.core.runtime import SystemRuntime
|
||||
from flow.core import paths
|
||||
from flow.services.containers import ContainerService
|
||||
from flow.app.containers import ContainerService
|
||||
|
||||
from tests.fakes import FakeRunner
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ from flow.core.errors import FlowError, PlanConflict
|
||||
from flow.core.platform import PlatformInfo
|
||||
from flow.core.runtime import SystemRuntime
|
||||
from flow.core import paths
|
||||
from flow.services.dotfiles import DotfilesService
|
||||
from flow.app.dotfiles import DotfilesService
|
||||
from tests.fakes import FakeRunner
|
||||
|
||||
|
||||
|
||||
@@ -14,7 +14,7 @@ from flow.core.platform import PlatformInfo
|
||||
from flow.core.runtime import SystemRuntime
|
||||
from flow.core import paths
|
||||
from flow.domain.packages.models import InstalledPackage, InstalledState, PackageDef
|
||||
from flow.services.packages import PackageService
|
||||
from flow.app.packages import PackageService
|
||||
|
||||
|
||||
def _make_ctx(tmp_path, manifest=None):
|
||||
|
||||
@@ -2,14 +2,18 @@
|
||||
|
||||
import subprocess
|
||||
|
||||
import pytest
|
||||
|
||||
from flow.core.config import AppConfig, FlowContext
|
||||
from flow.core.console import Console
|
||||
from flow.core.errors import FlowError
|
||||
from flow.core.platform import PlatformInfo
|
||||
from flow.core.runtime import SystemRuntime
|
||||
from flow.services.projects import ProjectService
|
||||
from flow.app.projects import ProjectService
|
||||
|
||||
|
||||
def _make_ctx(projects_dir):
|
||||
def _make_ctx(projects_dir, monkeypatch):
|
||||
monkeypatch.setattr("flow.actions.executor.paths.STATE_DIR", projects_dir / ".state")
|
||||
return FlowContext(
|
||||
config=AppConfig(projects_dir=str(projects_dir)),
|
||||
manifest={},
|
||||
@@ -19,25 +23,44 @@ def _make_ctx(projects_dir):
|
||||
)
|
||||
|
||||
|
||||
def _git(*args):
|
||||
return subprocess.run(
|
||||
["git", *[str(arg) for arg in args]],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
)
|
||||
|
||||
|
||||
def _init_repo(path, commit=True):
|
||||
"""Create a git repo with an initial commit."""
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
subprocess.run(["git", "init", str(path)], capture_output=True, check=True)
|
||||
subprocess.run(["git", "-C", str(path), "config", "user.email", "test@test.com"], capture_output=True, check=True)
|
||||
subprocess.run(["git", "-C", str(path), "config", "user.name", "Test"], capture_output=True, check=True)
|
||||
_git("init", path)
|
||||
_git("-C", path, "config", "user.email", "test@test.com")
|
||||
_git("-C", path, "config", "user.name", "Test")
|
||||
if commit:
|
||||
(path / "README.md").write_text("# test")
|
||||
subprocess.run(["git", "-C", str(path), "add", "."], capture_output=True, check=True)
|
||||
subprocess.run(["git", "-C", str(path), "commit", "-m", "init"], capture_output=True, check=True)
|
||||
_git("-C", path, "add", ".")
|
||||
_git("-C", path, "commit", "-m", "init")
|
||||
|
||||
|
||||
def _add_upstream(repo):
|
||||
remote = repo.parent / f"{repo.name}.git"
|
||||
_git("init", "--bare", remote)
|
||||
_git("-C", repo, "remote", "add", "origin", remote)
|
||||
branch = _git("-C", repo, "branch", "--show-current").stdout.strip()
|
||||
_git("-C", repo, "push", "-u", "origin", branch)
|
||||
|
||||
|
||||
class TestProjectService:
|
||||
def test_check_clean_repo(self, tmp_path, capsys):
|
||||
def test_check_clean_repo(self, tmp_path, monkeypatch, capsys):
|
||||
projects = tmp_path / "projects"
|
||||
projects.mkdir()
|
||||
_init_repo(projects / "myrepo")
|
||||
repo = projects / "myrepo"
|
||||
_init_repo(repo)
|
||||
_add_upstream(repo)
|
||||
|
||||
ctx = _make_ctx(projects)
|
||||
ctx = _make_ctx(projects, monkeypatch)
|
||||
svc = ProjectService(ctx)
|
||||
svc.check(fetch=False)
|
||||
|
||||
@@ -45,33 +68,74 @@ class TestProjectService:
|
||||
assert "myrepo" in output
|
||||
assert "clean" in output
|
||||
|
||||
def test_check_uncommitted_changes(self, tmp_path, capsys):
|
||||
def test_check_uncommitted_changes(self, tmp_path, monkeypatch, capsys):
|
||||
projects = tmp_path / "projects"
|
||||
projects.mkdir()
|
||||
_init_repo(projects / "myrepo")
|
||||
(projects / "myrepo" / "new_file.txt").write_text("changes")
|
||||
repo = projects / "myrepo"
|
||||
_init_repo(repo)
|
||||
_add_upstream(repo)
|
||||
(repo / "new_file.txt").write_text("changes")
|
||||
|
||||
ctx = _make_ctx(projects)
|
||||
ctx = _make_ctx(projects, monkeypatch)
|
||||
svc = ProjectService(ctx)
|
||||
svc.check(fetch=False)
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "uncommitted" in output
|
||||
|
||||
def test_check_no_git_repos(self, tmp_path, capsys):
|
||||
def test_check_no_git_repos(self, tmp_path, monkeypatch, capsys):
|
||||
projects = tmp_path / "projects"
|
||||
projects.mkdir()
|
||||
(projects / "not-a-repo").mkdir()
|
||||
|
||||
ctx = _make_ctx(projects)
|
||||
ctx = _make_ctx(projects, monkeypatch)
|
||||
svc = ProjectService(ctx)
|
||||
svc.check(fetch=False)
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "No git" in output
|
||||
|
||||
def test_missing_projects_dir(self, tmp_path, capsys):
|
||||
ctx = _make_ctx(tmp_path / "nonexistent")
|
||||
def test_missing_projects_dir(self, tmp_path, monkeypatch, capsys):
|
||||
ctx = _make_ctx(tmp_path / "nonexistent", monkeypatch)
|
||||
svc = ProjectService(ctx)
|
||||
svc.check(fetch=False)
|
||||
assert "not found" in capsys.readouterr().out
|
||||
|
||||
def test_fetch_failure_raises(self, tmp_path, monkeypatch):
|
||||
projects = tmp_path / "projects"
|
||||
projects.mkdir()
|
||||
repo = projects / "myrepo"
|
||||
_init_repo(repo)
|
||||
_git("-C", repo, "remote", "add", "origin", tmp_path / "missing-origin")
|
||||
|
||||
ctx = _make_ctx(projects, monkeypatch)
|
||||
svc = ProjectService(ctx)
|
||||
|
||||
with pytest.raises(FlowError, match="Fetch remotes for myrepo"):
|
||||
svc.check(fetch=True)
|
||||
|
||||
def test_status_failure_raises(self, tmp_path, monkeypatch):
|
||||
projects = tmp_path / "projects"
|
||||
projects.mkdir()
|
||||
repo = projects / "myrepo"
|
||||
_init_repo(repo)
|
||||
(repo / ".git" / "HEAD").unlink()
|
||||
|
||||
ctx = _make_ctx(projects, monkeypatch)
|
||||
svc = ProjectService(ctx)
|
||||
|
||||
with pytest.raises(FlowError, match="Check working tree for myrepo"):
|
||||
svc.check(fetch=False)
|
||||
|
||||
def test_missing_upstream_raises_instead_of_clean(self, tmp_path, monkeypatch, capsys):
|
||||
projects = tmp_path / "projects"
|
||||
projects.mkdir()
|
||||
_init_repo(projects / "myrepo")
|
||||
|
||||
ctx = _make_ctx(projects, monkeypatch)
|
||||
svc = ProjectService(ctx)
|
||||
|
||||
with pytest.raises(FlowError, match="rev-list"):
|
||||
svc.check(fetch=False)
|
||||
|
||||
assert "clean" not in capsys.readouterr().out
|
||||
|
||||
@@ -7,7 +7,7 @@ from flow.core.console import Console
|
||||
from flow.core.errors import FlowError
|
||||
from flow.core.platform import PlatformInfo
|
||||
from flow.core.runtime import SystemRuntime
|
||||
from flow.services.remote import RemoteService
|
||||
from flow.app.remote import RemoteService
|
||||
|
||||
|
||||
def _make_ctx(targets=None):
|
||||
|
||||
@@ -12,6 +12,14 @@ MUTATING_PATTERNS = (
|
||||
re.compile(r"runtime\.fs\.(create_symlink|remove_symlink|write_json|write_text|write_bytes|copy_file|copy_tree|remove_file|remove_tree)\("),
|
||||
)
|
||||
|
||||
COMMAND_PATTERNS = (
|
||||
re.compile(r"runtime\.runner\.(run|run_shell)\("),
|
||||
re.compile(r"runtime\.git\.run\("),
|
||||
re.compile(r"runtime\.containers\.(run_container|start|stop|kill|rm)\("),
|
||||
re.compile(r"runtime\.tmux\.(new_session|set_option|respawn_pane)\("),
|
||||
re.compile(r"self\.(rt|tmux)\.(run_container|start|stop|kill|rm|new_session|set_option|respawn_pane)\("),
|
||||
)
|
||||
|
||||
ALLOWED_PREFIXES = (
|
||||
Path("src/flow/adapters"),
|
||||
Path("src/flow/actions"),
|
||||
@@ -22,17 +30,12 @@ ALLOWED_FILES = {
|
||||
Path("src/flow/core/paths.py"),
|
||||
}
|
||||
|
||||
SKIPPED_LEGACY_COMMANDS = {
|
||||
Path("src/flow/commands/completion.py"),
|
||||
}
|
||||
|
||||
|
||||
def test_no_direct_filesystem_mutation_outside_action_boundary():
|
||||
root = Path(__file__).resolve().parents[1]
|
||||
offenders: list[str] = []
|
||||
for path in sorted((root / "src" / "flow").rglob("*.py")):
|
||||
rel = path.relative_to(root)
|
||||
if rel in ALLOWED_FILES or rel in SKIPPED_LEGACY_COMMANDS:
|
||||
if rel in ALLOWED_FILES:
|
||||
continue
|
||||
if any(rel.is_relative_to(prefix) for prefix in ALLOWED_PREFIXES):
|
||||
continue
|
||||
@@ -43,3 +46,17 @@ def test_no_direct_filesystem_mutation_outside_action_boundary():
|
||||
offenders.append(f"{rel}:{line_no}: {line.strip()}")
|
||||
|
||||
assert offenders == []
|
||||
|
||||
|
||||
def test_no_direct_command_mutation_outside_action_boundary():
|
||||
root = Path(__file__).resolve().parents[1]
|
||||
offenders: list[str] = []
|
||||
for path in sorted((root / "src" / "flow").rglob("*.py")):
|
||||
rel = path.relative_to(root)
|
||||
if any(rel.is_relative_to(prefix) for prefix in ALLOWED_PREFIXES):
|
||||
continue
|
||||
for line_no, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1):
|
||||
if any(pattern.search(line) for pattern in COMMAND_PATTERNS):
|
||||
offenders.append(f"{rel}:{line_no}: {line.strip()}")
|
||||
|
||||
assert offenders == []
|
||||
|
||||
Reference in New Issue
Block a user