diff --git a/opencollab/adapters/git_patch.py b/opencollab/adapters/git_patch.py index d55e3665..081402f5 100644 --- a/opencollab/adapters/git_patch.py +++ b/opencollab/adapters/git_patch.py @@ -36,16 +36,18 @@ def _git_command_and_config_guard(*, allow_core_worktree: bool = False) -> tuple "for config_scope in $config_scopes; do " f"unsafe_config=$({git_command} config \"$config_scope\" --includes --name-only --get-regexp " "'^(diff\\..*\\.(command|textconv)|filter\\..*|" - f"core\\.(excludesfile|sparsecheckout|sparsecheckoutcone{worktree_setting}))$' " + f"core\\.(excludesfile{worktree_setting}))$' " "2>/dev/null); " 'config_rc=$?; if [ "$config_rc" -eq 0 ]; then ' "echo \"unsafe repository Git configuration: $unsafe_config\" >&2; exit 125; " 'elif [ "$config_rc" -ne 1 ]; then exit "$config_rc"; fi; ' - f'fsmonitor=$({git_command} config "$config_scope" --includes --type=bool ' - "--get core.fsmonitor 2>/dev/null); fsmonitor_rc=$?; " - 'if [ "$fsmonitor_rc" -eq 0 ] && [ "$fsmonitor" = false ]; then :; ' - 'elif [ "$fsmonitor_rc" -ne 1 ]; then ' - "echo 'unsafe repository Git configuration: core.fsmonitor' >&2; exit 125; fi; " + "for boolean_key in core.fsmonitor core.sparsecheckout core.sparsecheckoutcone; do " + f'config_value=$({git_command} config "$config_scope" --includes --type=bool ' + '--get "$boolean_key" 2>/dev/null); config_rc=$?; ' + 'if [ "$config_rc" -eq 0 ] && [ "$config_value" = false ]; then :; ' + 'elif [ "$config_rc" -ne 1 ]; then ' + 'echo "unsafe repository Git configuration: $boolean_key" >&2; exit 125; fi; ' + "done; " "done; " f'info_attributes=$({git_command} rev-parse --git-path info/attributes) || exit 125; ' 'if [ -L "$info_attributes" ] || { [ -e "$info_attributes" ] && [ ! -f "$info_attributes" ]; }; then ' @@ -164,13 +166,11 @@ def guarded_staged_diff_command( ``core.attributesFile`` and ``diff.ignoreSubmodules`` are pinned by an explicit ``-c``, which outranks any repository-local value, so a repository that marks a file undiffable or hides a gitlink change cannot do either - here. ``core.fsmonitor`` is judged by its value: a repository that turns a - monitor off is hardening itself and is allowed through, while one that turns - a monitor on is refused, because a monitor that lies about what changed can - only be trusted or refused, never overridden. Everything else that could - bend the reading -- a redirected working tree, a sparse checkout, an - excludes file, and the clean/smudge filters and external diff drivers a - tracked ``.gitattributes`` can still reach -- is refused outright. + here. ``core.fsmonitor``, ``core.sparsecheckout``, and + ``core.sparsecheckoutcone`` accept Git boolean false values or an absent + setting. Enabled or malformed values are refused. Redirected working trees, + excludes files, clean/smudge filters, and external diff drivers reachable + through tracked ``.gitattributes`` are refused outright. A refusal exits 125 rather than reporting a patch this command cannot vouch for. diff --git a/tests/runtime/test_disabled_sparse_checkout.py b/tests/runtime/test_disabled_sparse_checkout.py new file mode 100644 index 00000000..8c18e02b --- /dev/null +++ b/tests/runtime/test_disabled_sparse_checkout.py @@ -0,0 +1,75 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from opencollab.adapters.env import LocalEnvironment, WorktreeEnvironment +from opencollab.adapters.tools.git_diff import GitDiffTool +from opencollab.adapters.worktree_pool import WorktreePool +from opencollab.application.tool_execution import ToolRuntime + + +def _git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), *args], check=True, text=True, capture_output=True, + ).stdout + + +@pytest.mark.parametrize("disabled_sparse", [False, True]) +async def test_full_checkout_exports_after_sparse_checkout_disable(tmp_path, monkeypatch, disabled_sparse): + monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1") + monkeypatch.setenv("GIT_CONFIG_GLOBAL", "/dev/null") + source = tmp_path / "source" + source.mkdir() + for name, content in (("src/app.py", "value = 1\n"), ("docs/guide.txt", "guide\n")): + path = source / name + path.parent.mkdir(exist_ok=True) + path.write_text(content, encoding="utf-8") + _git(source, "init", "-q") + _git(source, "add", ".") + _git(source, "-c", "user.name=Test User", "-c", "user.email=test@example.invalid", + "-c", "commit.gpgsign=false", "commit", "-qm", "base") + if disabled_sparse: + _git(source, "sparse-checkout", "init", "--cone") + _git(source, "sparse-checkout", "set", "src") + assert not (source / "docs/guide.txt").exists() + _git(source, "sparse-checkout", "disable") + for key in ("core.sparsecheckout", "core.sparsecheckoutcone"): + assert _git(source, "config", "--type=bool", "--get", key) == "false\n" + assert (source / "docs/guide.txt").read_text(encoding="utf-8") == "guide\n" + assert _git(source, "status", "--porcelain") == "" + assert all(line.startswith("H ") for line in _git(source, "ls-files", "-t").splitlines()) + + local = LocalEnvironment(str(source)) + try: + result = await GitDiffTool().execute_with_runtime({}, ToolRuntime(local, None, None)) + assert result == "Status: working tree clean.\n\ndiff vs HEAD: (no changes)" + finally: + await local.cleanup() + + env = WorktreeEnvironment(str(source)) + candidate = Path(await env.setup()) + try: + await env.write_file("src/app.py", "value = 2\n") + expected = _git(candidate, "diff", "--binary", "--full-index", "--no-color", + "--unified=3", "--src-prefix=a/", "--dst-prefix=b/", "HEAD") + assert "+value = 2\n" in expected + assert await env.get_diff() == expected + assert _git(candidate, "diff", "--cached") == "" + finally: + await env.cleanup() + assert not candidate.exists() + + pool = WorktreePool(str(source), use_worktrees=True, enable_adoption=True) + try: + worker = await pool.acquire("coder") + pooled_candidate = Path(worker.workspace) + await worker.write_file("src/app.py", "value = 2\n") + assert await worker.get_diff() == expected + finally: + await pool.release() + assert not pooled_candidate.exists() + assert _git(source, "status", "--porcelain") == "" + assert (source / "src/app.py").read_text(encoding="utf-8") == "value = 1\n" diff --git a/tests/tools/test_git_patch.py b/tests/tools/test_git_patch.py index b9e13aef..dc59339a 100644 --- a/tests/tools/test_git_patch.py +++ b/tests/tools/test_git_patch.py @@ -205,6 +205,59 @@ def test_patch_rejects_worktree_scoped_sparse_checkout(tmp_path): assert result.returncode == 125 assert "core.sparsecheckout" in result.stderr +@pytest.mark.parametrize("scope", ["--local", "--worktree"]) +@pytest.mark.parametrize("value", ["false", "FaLsE", "no", "off", "0", ""]) +def test_patch_allows_disabled_sparse_boolean_values(tmp_path, scope, value): + repo = _repo(tmp_path) + (repo / "tracked.txt").write_text("new\n", encoding="utf-8") + expected = _extract(repo) + assert expected.returncode == 0, expected.stderr + _git(repo, "config", "extensions.worktreeConfig", "true") + for key in ("core.sparsecheckout", "core.sparsecheckoutcone"): + _git(repo, "config", scope, key, value) + + result = _extract(repo) + + assert result.returncode == 0, result.stderr + assert result.stdout == expected.stdout + +@pytest.mark.parametrize("scope", ["--local", "--worktree"]) +@pytest.mark.parametrize("key", ["core.sparsecheckout", "core.sparsecheckoutcone"]) +@pytest.mark.parametrize("value", ["true", "yes", "on", "1", "-1", "2", "invalid", None]) +def test_patch_rejects_enabled_or_invalid_sparse_values(tmp_path, scope, key, value): + repo = _repo(tmp_path) + _git(repo, "config", "extensions.worktreeConfig", "true") + if value is None: + config = Path(_git(repo, "rev-parse", "--git-path", "config.worktree" if scope == "--worktree" + else "config").stdout.strip()) + with (repo / config).open("a", encoding="utf-8") as stream: + stream.write(f"[core]\n\t{key.split('.')[1]}\n") + else: + _git(repo, "config", scope, key, value) + + result = _extract(repo) + + assert result.returncode == 125 + assert f"unsafe repository Git configuration: {key}" in result.stderr + +@pytest.mark.parametrize("sparse_index", [False, True]) +def test_patch_rejects_active_sparse_checkout(tmp_path, sparse_index): + repo = _repo(tmp_path) + for directory in ("visible", "hidden"): + (repo / directory).mkdir() + (repo / directory / "file.txt").write_text("tracked\n", encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-qm", "add directories") + _git(repo, "sparse-checkout", "init", "--cone", "--sparse-index" if sparse_index else "--no-sparse-index") + _git(repo, "sparse-checkout", "set", "visible") + assert not (repo / "hidden" / "file.txt").exists() + assert "S hidden/" in _git(repo, "ls-files", "--sparse", "-t").stdout + + result = _extract(repo) + + assert result.returncode == 125 + assert "core.sparsecheckout" in result.stderr + def test_patch_rejects_transform_config_loaded_through_include(tmp_path): repo = _repo(tmp_path) (repo / "payload.txt").write_text("candidate\n", encoding="utf-8")