fix(containers): compose profile and Git mounts safely
Stage profile sources per instance and compose overlapping bind mounts so Docker cannot mask Git content or leave writable files root-owned.\n\n- preserve shared Git clones while applying profile overlays\n- add mount composition and ownership regression coverage\n- update OpenSpec tracking
This commit is contained in:
@@ -2,13 +2,16 @@
|
||||
|
||||
import hashlib
|
||||
import uuid
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from src.services.tool.instance_service import (
|
||||
_chown_staged_mounts,
|
||||
_get_repository_mount_name,
|
||||
_stack_profile_mounts_with_git_mounts,
|
||||
_stage_profile_mounts,
|
||||
clone_git_repo,
|
||||
modify_compose_file,
|
||||
prepare_manifest_instance,
|
||||
@@ -123,151 +126,185 @@ class TestCloneGitRepo:
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestStackProfileMountsWithGitMounts:
|
||||
"""Tests for _stack_profile_mounts_with_git_mounts."""
|
||||
"""Tests for composing profile and Git mounts without Docker masking."""
|
||||
|
||||
def test_exact_overlap_keeps_profile_files_out_of_git_source(
|
||||
self, tmp_path
|
||||
def test_stages_profile_source_under_instance_directory(self, tmp_path) -> None:
|
||||
"""Profile sources must be instance-local before ownership is fixed."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
profile_source = tmp_path / "profile" / "settings.json"
|
||||
profile_source.parent.mkdir(parents=True)
|
||||
profile_source.write_text("{}")
|
||||
|
||||
staged = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.pi/settings.json"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert staged[0]["source"].startswith(str(instance_dir))
|
||||
assert staged[0]["source"] != str(profile_source)
|
||||
assert Path(staged[0]["source"]).read_text() == "{}"
|
||||
|
||||
def test_staged_profile_source_is_chowned_for_container_user(
|
||||
self, monkeypatch, tmp_path
|
||||
) -> None:
|
||||
"""Overlaps must not dirty the shared Git checkout."""
|
||||
"""The ownership pass must include the instance-local profile copy."""
|
||||
from src.services.tool import instance_service
|
||||
|
||||
instance_dir = tmp_path / "instance"
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("{}")
|
||||
staged = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.pi"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
chown = MagicMock()
|
||||
monkeypatch.setattr(instance_service, "_chown_path", chown)
|
||||
|
||||
_chown_staged_mounts(staged, str(instance_dir), 1000, 1000)
|
||||
|
||||
chown.assert_called_once_with(staged[0]["source"], 1000, 1000)
|
||||
|
||||
def test_exact_overlap_creates_instance_local_composite(self, tmp_path) -> None:
|
||||
"""Profile files extend a Git root without mutating its shared clone."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git" / "repo-clone"
|
||||
git_source.mkdir(parents=True)
|
||||
(git_source / "existing.txt").write_text("from git")
|
||||
|
||||
profile_source = tmp_path / "profile" / "home_user_.pi"
|
||||
profile_source.mkdir(parents=True)
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("{}")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi",
|
||||
"type": "bind",
|
||||
"readonly": False,
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi",
|
||||
"type": "bind",
|
||||
}
|
||||
],
|
||||
str(instance_dir),
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
assert (git_source / "existing.txt").read_text() == "from git"
|
||||
assert not (git_source / "settings.json").exists()
|
||||
assert len(result) == 1
|
||||
composite = result[0]
|
||||
assert composite["target"] == "/home/user/.pi"
|
||||
assert composite["source"].startswith(str(instance_dir))
|
||||
assert not (tmp_path / "git" / "repo-clone" / "settings.json").exists()
|
||||
assert (
|
||||
tmp_path / "git" / "repo-clone" / "existing.txt"
|
||||
).read_text() == "from git"
|
||||
assert (tmp_path / "instance" / "mounts" / "composites").exists()
|
||||
assert (Path(composite["source"]) / "existing.txt").read_text() == "from git"
|
||||
assert (Path(composite["source"]) / "settings.json").read_text() == "{}"
|
||||
|
||||
def test_descendant_overlap_keeps_sources_isolated(self, tmp_path) -> None:
|
||||
"""A child profile mount must not mutate the shared Git checkout."""
|
||||
def test_descendant_profile_mount_extends_git_root(self, tmp_path) -> None:
|
||||
"""Nested targets are composed at the Git root, preserving siblings."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
(git_source / "README").write_text("repo")
|
||||
|
||||
profile_source = tmp_path / "profile" / "agent"
|
||||
profile_source.mkdir(parents=True)
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("x")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi/agent",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.pi/agent"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
assert len(result) == 1
|
||||
composite = Path(result[0]["source"])
|
||||
assert result[0]["target"] == "/home/user/.pi"
|
||||
assert (composite / "README").read_text() == "repo"
|
||||
assert (composite / "agent" / "settings.json").read_text() == "x"
|
||||
assert not (git_source / "agent").exists()
|
||||
assert (git_source / "README").read_text() == "repo"
|
||||
|
||||
def test_non_overlapping_mounts_left_untouched(self, tmp_path) -> None:
|
||||
"""Profile mounts that do not overlap a git mount are returned as-is."""
|
||||
def test_file_profile_mount_extends_git_root(self, tmp_path) -> None:
|
||||
"""A file bind mount is composed into the Git directory, not masked."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "config").write_text("c")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.config",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
|
||||
def test_git_source_file_does_not_consume_profile_mount(self, tmp_path) -> None:
|
||||
"""If the overlapping git-mount source is a file, the profile mount
|
||||
cannot be merged and must be kept."""
|
||||
git_source = tmp_path / "file.txt"
|
||||
git_source.write_text("file")
|
||||
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "settings.json").write_text("{}")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{
|
||||
"source": str(git_source),
|
||||
"target": "/home/user/.pi/file.txt",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
|
||||
def test_profile_source_file_does_not_mutate_git_source(self, tmp_path) -> None:
|
||||
"""A profile file must not be copied into a shared Git checkout."""
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
|
||||
(git_source / "README").write_text("repo")
|
||||
profile_source = tmp_path / "settings.json"
|
||||
profile_source.write_text("{}")
|
||||
|
||||
profile_mounts = [
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi/settings.json",
|
||||
"type": "bind",
|
||||
}
|
||||
]
|
||||
git_mount_volumes = [
|
||||
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||
]
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[
|
||||
{
|
||||
"source": str(profile_source),
|
||||
"target": "/home/user/.pi/settings.json",
|
||||
}
|
||||
],
|
||||
str(instance_dir),
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mount_volumes
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert result == profile_mounts
|
||||
assert not (git_source / "settings.json").exists()
|
||||
assert len(result) == 1
|
||||
composite = Path(result[0]["source"])
|
||||
assert result[0]["target"] == "/home/user/.pi"
|
||||
assert (composite / "README").read_text() == "repo"
|
||||
assert (composite / "settings.json").read_text() == "{}"
|
||||
|
||||
def test_parent_profile_mount_extends_nested_git_mount(self, tmp_path) -> None:
|
||||
"""A profile parent mount keeps Git content and its own sibling files."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
(git_source / "plugin.toml").write_text("git")
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
(profile_source / "config.toml").write_text("profile")
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user"}], str(instance_dir)
|
||||
)
|
||||
result = _stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts,
|
||||
[{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
|
||||
assert len(result) == 1
|
||||
composite = Path(result[0]["source"])
|
||||
assert result[0]["target"] == "/home/user"
|
||||
assert (composite / "config.toml").read_text() == "profile"
|
||||
assert (composite / ".pi" / "plugin.toml").read_text() == "git"
|
||||
|
||||
def test_non_overlapping_mounts_remain_separate(self, tmp_path) -> None:
|
||||
"""Unrelated profile and Git mounts retain their independent sources."""
|
||||
instance_dir = tmp_path / "instance"
|
||||
git_source = tmp_path / "git"
|
||||
git_source.mkdir()
|
||||
profile_source = tmp_path / "profile"
|
||||
profile_source.mkdir()
|
||||
|
||||
profile_mounts = _stage_profile_mounts(
|
||||
[{"source": str(profile_source), "target": "/home/user/.config"}],
|
||||
str(instance_dir),
|
||||
)
|
||||
git_mounts = [{"source": str(git_source), "target": "/home/user/.pi"}]
|
||||
|
||||
assert (
|
||||
_stack_profile_mounts_with_git_mounts(
|
||||
profile_mounts, git_mounts, str(instance_dir)
|
||||
)
|
||||
== profile_mounts + git_mounts
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
|
||||
Reference in New Issue
Block a user