From df8d17b7683afb0dd13023694c44aaabaeb68e0b Mon Sep 17 00:00:00 2001 From: Kai Date: Sun, 25 Oct 2026 22:21:51 +0800 Subject: [PATCH] fix(git): accept disabled sparse checkout settings Read sparse checkout settings with Git boolean parsing so full checkouts restored by sparse-checkout disable can export patches and acquire candidate worktrees. Reject enabled and malformed values in local and worktree configuration, with real Git regressions for both configuration scopes and active sparse indexes. --- opencollab/adapters/git_patch.py | 26 +++---- .../runtime/test_disabled_sparse_checkout.py | 75 +++++++++++++++++++ tests/tools/test_git_patch.py | 53 +++++++++++++ 3 files changed, 141 insertions(+), 13 deletions(-) create mode 100644 tests/runtime/test_disabled_sparse_checkout.py 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")