fix(git): refuse repository config that runs a program before push, release and gc
An agent that can write a repository's .git/config could make the operator's pkgmgr push, release or branch gc run a program or reroute the push. UNSAFE_CONFIG lists the local keys that do (core.sshCommand, gitProxy, askPass, fsmonitor, alternateRefsCommand, credential helpers, gpg programs, protocol.*, config hooks, gc.recentObjectsHook, http proxy/TLS/cookie keys, remote pack programs and proxy, url rewrites). push and release refuse up front; gc skips its fetch and keeps remote branches. Filter drivers, hooksPath, gpg.format and include stay allowed for git-lfs, husky and per-repo signing. SCAN_FLAGS add --no-pager and switch off config-defined hooks (hook.<event>.enabled=false), which core.hooksPath does not cover. The signing amend, gc's fetch, delete push and branch -D run under them. Remote and ref names sit behind -- in every push, fetch, ls-remote and remote get-url, fetches and pushes no longer recurse into submodules, and a staged gitlink is no longer folded into the signed HEAD. gc skips a compose project that also has containers outside the worktree. 🎯 Fix: 92 % 💥 Break: 4 % 💸 Cost: 91 % ⏱️ Time: 89 % 🛡️ Security: 100 % 🏆 Score: 100 🚦 Gate: ✅ 90/5, cost and time <= 100, security 100 % Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
fb6183838b
commit
4c8b137449
13 files changed
+467
-22
No files matched your search
@@ -109,6 +109,7 @@ class GcFixture:
|
||||
scope: str = "all",
|
||||
activity: bool = False,
|
||||
hooks: dict | None = None,
|
||||
containers: list[gc.Container] | None = None,
|
||||
) -> gc.Repo:
|
||||
with (
|
||||
patch.object(gc, "fetch_forge_facts", return_value=self.forge),
|
||||
@@ -116,7 +117,7 @@ class GcFixture:
|
||||
):
|
||||
return gc.plan_repo(
|
||||
self.repo,
|
||||
[],
|
||||
containers or [],
|
||||
host_ns,
|
||||
merged_into=merged_into or [],
|
||||
sizes=sizes,
|
||||
@@ -388,6 +389,97 @@ class TestBranchGc(GcFixture, unittest.TestCase):
|
||||
self.assertEqual(git(self.repo, "for-each-ref", "refs/heads/-evil"), "")
|
||||
self.assertEqual(git(other, "for-each-ref", "refs/heads/-evil"), "")
|
||||
|
||||
def _fetches(self) -> int:
|
||||
with patch.object(gc, "run", wraps=gc.run) as spy:
|
||||
self.plan = self._plan(host_ns=True, cwds={})
|
||||
return sum("fetch" in call.args[0] for call in spy.call_args_list)
|
||||
|
||||
def test_unsafe_repository_config_skips_fetch_and_remote_deletes(self) -> None:
|
||||
self.assertEqual(self._fetches(), 1)
|
||||
git(self.repo, "config", "core.sshCommand", "false")
|
||||
|
||||
self.assertEqual(self._fetches(), 0)
|
||||
plan = self.plan
|
||||
item = self._items(plan)["merged"]
|
||||
|
||||
self.assertIn(
|
||||
"fetch and remote branch deletes skipped: the repository config sets "
|
||||
"core.sshcommand",
|
||||
plan.warnings,
|
||||
)
|
||||
self.assertIsNone(item.skip)
|
||||
self.assertEqual(item.actions, ["worktree", "local-branch"])
|
||||
|
||||
with patch.object(gc, "_process_cwds", return_value={}):
|
||||
gc.execute(plan, item)
|
||||
|
||||
self.assertEqual(item.result, "done")
|
||||
self.assertFalse(os.path.exists(self.merged_wt))
|
||||
self.assertEqual(git(self.origin, "branch", "--list", "merged"), "merged")
|
||||
|
||||
def test_config_set_after_plan_stops_execute(self) -> None:
|
||||
plan = self._plan(host_ns=True, cwds={})
|
||||
item = self._items(plan)["merged"]
|
||||
git(self.repo, "config", "credential.helper", "false")
|
||||
|
||||
with patch.object(gc, "_process_cwds", return_value={}):
|
||||
gc.execute(plan, item)
|
||||
|
||||
self.assertEqual(
|
||||
item.result, "skipped: repository config sets credential.helper"
|
||||
)
|
||||
self.assertTrue(os.path.isdir(self.merged_wt))
|
||||
self.assertEqual(git(self.origin, "branch", "--list", "merged"), "merged")
|
||||
|
||||
def test_compose_project_with_containers_elsewhere_is_skipped(self) -> None:
|
||||
inside = gc.Container("proj", os.path.join(self.merged_wt, "app"), "img")
|
||||
elsewhere = gc.Container("proj", os.path.join(self.root, "other"), "img")
|
||||
|
||||
alone = self._items(self._plan(host_ns=True, cwds={}, containers=[inside]))
|
||||
shared = self._items(
|
||||
self._plan(host_ns=True, cwds={}, containers=[inside, elsewhere])
|
||||
)
|
||||
|
||||
self.assertIsNone(alone["merged"].skip)
|
||||
self.assertIn("compose-down:proj", alone["merged"].actions)
|
||||
self.assertEqual(
|
||||
shared["merged"].skip,
|
||||
"compose project proj also has containers outside the worktree",
|
||||
)
|
||||
self.assertFalse(shared["merged"].eligible)
|
||||
|
||||
def test_execute_runs_no_repository_hook(self) -> None:
|
||||
marker = os.path.join(self.root, "hooked")
|
||||
hook = os.path.join(self.repo, ".git", "hooks", "reference-transaction")
|
||||
with open(hook, "w") as fh:
|
||||
fh.write(f"#!/bin/sh\ntouch {marker}\n")
|
||||
os.chmod(hook, 0o755)
|
||||
plan = self._plan(host_ns=True, cwds={})
|
||||
item = self._items(plan)["merged"]
|
||||
|
||||
with patch.object(gc, "_process_cwds", return_value={}):
|
||||
gc.execute(plan, item)
|
||||
|
||||
self.assertEqual(item.result, "done")
|
||||
self.assertEqual(git(self.origin, "branch", "--list", "merged"), "")
|
||||
self.assertFalse(os.path.exists(marker))
|
||||
|
||||
def test_local_branch_delete_runs_no_config_hook(self) -> None:
|
||||
marker = os.path.join(self.root, "hooked")
|
||||
git(self.repo, "branch", "local-only", "merged")
|
||||
git(self.repo, "config", "hook.probe.command", f"touch {marker}")
|
||||
git(self.repo, "config", "hook.probe.event", "reference-transaction")
|
||||
plan = self._plan(host_ns=True, cwds={})
|
||||
item = self._items(plan)["local-only"]
|
||||
self.assertEqual(item.actions, ["local-branch"])
|
||||
|
||||
with patch.object(gc, "_process_cwds", return_value={}):
|
||||
gc.execute(plan, item)
|
||||
|
||||
self.assertEqual(item.result, "done")
|
||||
self.assertEqual(git(self.repo, "branch", "--list", "local-only"), "")
|
||||
self.assertFalse(os.path.exists(marker))
|
||||
|
||||
def test_cleanup_json_and_refusal(self) -> None:
|
||||
out = io.StringIO()
|
||||
with (
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
|
||||
from pkgmgr.core.git.commands import delete_remote_branch, push, push_upstream
|
||||
|
||||
REMOTE = "-r"
|
||||
|
||||
|
||||
def git(cwd: str, *args: str) -> str:
|
||||
return subprocess.run(
|
||||
[
|
||||
"git",
|
||||
"-c",
|
||||
"user.name=t",
|
||||
"-c",
|
||||
"user.email=t@example.org",
|
||||
"-c",
|
||||
"commit.gpgsign=false",
|
||||
*args,
|
||||
],
|
||||
cwd=cwd,
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
).stdout.strip()
|
||||
|
||||
|
||||
class TestPushCommandsEndOptions(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self._tmp.cleanup)
|
||||
root = os.path.realpath(self._tmp.name)
|
||||
self.origin = os.path.join(root, "origin.git")
|
||||
self.repo = os.path.join(root, "repo")
|
||||
git(root, "init", "-q", "--bare", "-b", "main", self.origin)
|
||||
git(root, "init", "-q", "-b", "main", self.repo)
|
||||
git(self.repo, "config", f"remote.{REMOTE}.url", self.origin)
|
||||
git(
|
||||
self.repo,
|
||||
"config",
|
||||
f"remote.{REMOTE}.fetch",
|
||||
f"+refs/heads/*:refs/remotes/{REMOTE}/*",
|
||||
)
|
||||
git(self.repo, "commit", "-q", "--allow-empty", "-m", "init")
|
||||
|
||||
def _branches(self) -> str:
|
||||
return git(self.origin, "for-each-ref", "--format=%(refname:short)")
|
||||
|
||||
def test_remote_named_like_an_option(self) -> None:
|
||||
push_upstream(REMOTE, "main", cwd=self.repo)
|
||||
self.assertEqual(self._branches(), "main")
|
||||
self.assertEqual(
|
||||
git(self.repo, "rev-parse", "--abbrev-ref", "@{u}"), f"{REMOTE}/main"
|
||||
)
|
||||
|
||||
git(self.repo, "commit", "-q", "--amend", "--allow-empty", "-m", "rewritten")
|
||||
push(REMOTE, "main:feature", cwd=self.repo)
|
||||
push(REMOTE, "main", force=True, cwd=self.repo)
|
||||
self.assertEqual(
|
||||
git(self.origin, "rev-parse", "main"), git(self.repo, "rev-parse", "HEAD")
|
||||
)
|
||||
|
||||
delete_remote_branch(REMOTE, "feature", cwd=self.repo)
|
||||
self.assertEqual(self._branches(), "main")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -11,6 +11,7 @@ from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.actions.release.retry import retry_release
|
||||
from pkgmgr.actions.release.workflow import _release_impl
|
||||
from pkgmgr.actions.repository.push import (
|
||||
PushRefusedError,
|
||||
push_branch,
|
||||
@@ -240,6 +241,154 @@ class TestPushMirrorLeases(unittest.TestCase):
|
||||
self.assertNotIn("WARN", out)
|
||||
self.assertEqual(self._tip(self.canonical, "refs/heads/feature"), head)
|
||||
|
||||
def test_repository_config_that_runs_a_program_is_refused(self) -> None:
|
||||
self._commit("feature")
|
||||
for key in (
|
||||
"core.sshCommand",
|
||||
"core.gitProxy",
|
||||
"core.askPass",
|
||||
"core.fsmonitor",
|
||||
"core.alternateRefsCommand",
|
||||
"credential.helper",
|
||||
"credential.https://example.org.helper",
|
||||
"gpg.program",
|
||||
"gpg.ssh.program",
|
||||
"gpg.ssh.defaultKeyCommand",
|
||||
"protocol.ext.allow",
|
||||
"hook.probe.command",
|
||||
"gc.recentObjectsHook",
|
||||
"http.proxy",
|
||||
"http.sslVerify",
|
||||
"http.sslCAInfo",
|
||||
"http.sslCAPath",
|
||||
"http.cookieFile",
|
||||
"http.saveCookies",
|
||||
"remote.origin.receivepack",
|
||||
"remote.origin.uploadpack",
|
||||
"remote.origin.vcs",
|
||||
"remote.origin.proxy",
|
||||
"url.x.insteadOf",
|
||||
"url.x.pushInsteadOf",
|
||||
):
|
||||
for push in (self._push, lambda: push_branch(self.repo, [], sign=False)):
|
||||
with self.subTest(key=key, push=push):
|
||||
git(self.repo, "config", key, "false")
|
||||
|
||||
with self.assertRaises(PushRefusedError) as cm:
|
||||
push()
|
||||
|
||||
git(self.repo, "config", "--unset", key)
|
||||
self.assertIn(key.lower(), str(cm.exception))
|
||||
self.assertEqual(self._tip(self.canonical), self.init)
|
||||
self.assertEqual(self._tip(self.mirror), self.init)
|
||||
|
||||
def test_ordinary_repository_config_is_pushed(self) -> None:
|
||||
for key, value in (
|
||||
("core.hooksPath", ".husky/_"),
|
||||
("filter.lfs.process", "git-lfs filter-process"),
|
||||
("gpg.format", "ssh"),
|
||||
("credential.useHttpPath", "true"),
|
||||
("include.path", "../shared.gitconfig"),
|
||||
):
|
||||
git(self.repo, "config", key, value)
|
||||
self._commit("feature")
|
||||
|
||||
self._push()
|
||||
|
||||
self._assert_shipped_signed()
|
||||
|
||||
def test_staged_gitlink_is_not_folded_into_the_signed_head(self) -> None:
|
||||
self._commit("feature")
|
||||
head = git(self.repo, "rev-parse", "HEAD")
|
||||
git(self.repo, "config", "diff.ignoreSubmodules", "all")
|
||||
git(self.repo, "update-index", "--add", "--cacheinfo", f"160000,{head},sub")
|
||||
|
||||
with self.assertRaises(PushRefusedError) as cm:
|
||||
self._push()
|
||||
|
||||
self.assertIn("staged changes", str(cm.exception))
|
||||
self.assertEqual(git(self.repo, "rev-parse", "HEAD"), head)
|
||||
|
||||
def test_unstaged_edit_stays_out_of_the_signed_head(self) -> None:
|
||||
self._commit("feature")
|
||||
Path(self.repo, "feature.txt").write_text("unstaged")
|
||||
|
||||
self._push()
|
||||
|
||||
self._assert_shipped_signed()
|
||||
self.assertEqual(git(self.repo, "status", "--porcelain"), "M feature.txt")
|
||||
|
||||
def test_signing_amend_runs_no_repository_hook(self) -> None:
|
||||
marker = Path(self.root, "hooked")
|
||||
hook = Path(self.repo, ".git", "hooks", "post-commit")
|
||||
hook.write_text(f"#!/bin/sh\ntouch {marker}\n")
|
||||
hook.chmod(0o755)
|
||||
self._commit("feature")
|
||||
marker.unlink()
|
||||
|
||||
self._push()
|
||||
|
||||
self._assert_shipped_signed()
|
||||
self.assertFalse(marker.exists())
|
||||
|
||||
def test_signing_amend_runs_no_config_hook(self) -> None:
|
||||
marker = Path(self.root, "hooked")
|
||||
git(self.repo, "config", "hook.probe.command", f"touch {marker}")
|
||||
git(self.repo, "config", "hook.probe.event", "post-commit")
|
||||
self._commit("feature")
|
||||
hooks_fire = marker.exists()
|
||||
marker.unlink(missing_ok=True)
|
||||
|
||||
try:
|
||||
self._push()
|
||||
except PushRefusedError:
|
||||
pass
|
||||
|
||||
self.assertFalse(marker.exists(), f"config hooks fire on commit: {hooks_fire}")
|
||||
|
||||
def test_mirror_lease_runs_no_textconv(self) -> None:
|
||||
marker = Path(self.root, "textconv")
|
||||
Path(self.repo, ".gitattributes").write_text("* diff=probe\n")
|
||||
git(self.repo, "add", ".gitattributes")
|
||||
git(self.repo, "commit", "-q", "-S", "-m", "attributes")
|
||||
git(self.repo, "push", "-q", self.mirror, "main")
|
||||
git(self.repo, "config", "diff.probe.textconv", f"touch {marker};cat")
|
||||
self._commit("feature")
|
||||
git(self.repo, "diff", "HEAD~1", "HEAD")
|
||||
self.assertTrue(marker.exists())
|
||||
marker.unlink()
|
||||
|
||||
self._push()
|
||||
|
||||
self._assert_shipped_signed()
|
||||
self.assertFalse(marker.exists())
|
||||
|
||||
def test_remote_named_like_an_option_is_pushed(self) -> None:
|
||||
git(self.repo, "config", "remote.-r.url", self.canonical)
|
||||
git(self.repo, "config", "remote.-r.fetch", "+refs/heads/*:refs/remotes/-r/*")
|
||||
git(self.repo, "config", "branch.main.pushRemote", "-r")
|
||||
git(self.repo, "fetch", "-q", "--", "-r")
|
||||
self._commit("feature")
|
||||
|
||||
self._push()
|
||||
|
||||
self.assertEqual(self._tip(self.canonical), git(self.repo, "rev-parse", "HEAD"))
|
||||
|
||||
def test_release_refuses_unsafe_config_before_any_change(self) -> None:
|
||||
head = git(self.repo, "rev-parse", "HEAD")
|
||||
git(self.repo, "config", "core.sshCommand", "false")
|
||||
cwd = os.getcwd()
|
||||
os.chdir(self.repo)
|
||||
self.addCleanup(os.chdir, cwd)
|
||||
|
||||
with redirect_stdout(io.StringIO()), self.assertRaises(PushRefusedError):
|
||||
_release_impl(
|
||||
pyproject_path=os.path.join(self.repo, "pyproject.toml"), force=True
|
||||
)
|
||||
|
||||
self.assertEqual(git(self.repo, "rev-parse", "HEAD"), head)
|
||||
self.assertEqual(git(self.repo, "status", "--porcelain"), "")
|
||||
|
||||
def test_release_retry_refuses_unsigned_range(self) -> None:
|
||||
self._commit("release")
|
||||
git(self.repo, "tag", "-a", "v1.0.0", "-m", "v1.0.0")
|
||||
|
||||
@@ -448,6 +448,16 @@ class TestWorktreeUp(WorktreeFixture, unittest.TestCase):
|
||||
self.assertFalse(os.path.exists(parsed))
|
||||
self.assertFalse(os.path.exists(ran))
|
||||
|
||||
def test_checkout_runs_no_config_hook(self) -> None:
|
||||
marker = os.path.join(self.root, "hooked")
|
||||
git(self.repo, "config", "hook.probe.command", f"touch {marker}")
|
||||
git(self.repo, "config", "hook.probe.event", "post-checkout")
|
||||
|
||||
code, _ = self._up("feature/h", None)
|
||||
|
||||
self.assertEqual(code, 0)
|
||||
self.assertFalse(os.path.exists(marker))
|
||||
|
||||
def test_origin_url_cannot_escape_the_worktrees_dir(self) -> None:
|
||||
git(self.repo, "remote", "set-url", "origin", "https://host/../../escape")
|
||||
|
||||
|
||||
Reference in new issue
Block a user