From bd22135906772481e8f4617bbe4da669137f0e5c Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Wed, 8 Jul 2026 16:12:45 -0400 Subject: [PATCH] fix: bind issue comments to explicit worktree path (Closes #560) --- gitea_mcp_server.py | 4 +- tests/test_issue_comment_workspace_guard.py | 257 ++++++++++++++++++++ 2 files changed, 260 insertions(+), 1 deletion(-) create mode 100644 tests/test_issue_comment_workspace_guard.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 5c7446d..bca219d 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -6130,6 +6130,7 @@ def gitea_create_issue_comment( host: str | None = None, org: str | None = None, repo: str | None = None, + worktree_path: str | None = None, ) -> dict: """Post a markdown comment to a Gitea issue's discussion thread. @@ -6151,6 +6152,7 @@ def gitea_create_issue_comment( host: Override the Gitea host. org: Override the owner/organization. repo: Override the repository name. + worktree_path: Optional path to verify branches-only guard. Returns: dict with 'success', 'comment_id', and 'issue_number' ('url' only @@ -6159,7 +6161,7 @@ def gitea_create_issue_comment( (permission blocks also carry a structured 'permission_report', #142). """ - verify_preflight_purity(remote, task="comment_issue") + verify_preflight_purity(remote, worktree_path=worktree_path, task="comment_issue") gate_reasons = _profile_operation_gate("gitea.issue.comment") reasons = list(gate_reasons) if not (body or "").strip(): diff --git a/tests/test_issue_comment_workspace_guard.py b/tests/test_issue_comment_workspace_guard.py new file mode 100644 index 0000000..5359786 --- /dev/null +++ b/tests/test_issue_comment_workspace_guard.py @@ -0,0 +1,257 @@ +import os +import sys +import unittest +from pathlib import Path +from unittest.mock import MagicMock, patch + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import gitea_mcp_server as srv + + +FAKE_AUTH = {"Authorization": "token test-token"} +CONTROL_CHECKOUT_ROOT = str(Path(__file__).resolve().parents[3]) + + +class TestIssueCommentWorkspaceGuard(unittest.TestCase): + AUTHOR_ENV = { + "GITEA_PROFILE_NAME": "gitea-author", + "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment", + } + + def setUp(self): + srv._preflight_whoami_called = True + srv._preflight_capability_called = True + srv._preflight_resolved_role = "author" + srv._preflight_resolved_task = "comment_issue" + srv._preflight_whoami_violation = False + srv._preflight_capability_violation = False + srv._preflight_reviewer_violation_files = [] + + self._orig_in_test = srv._preflight_in_test_mode + srv._preflight_in_test_mode = lambda: False + self.addCleanup(self._restore_preflight_mode) + + def _restore_preflight_mode(self): + srv._preflight_in_test_mode = self._orig_in_test + + def _git_state(self, valid_worktree: str): + def side_effect(path): + real = os.path.realpath(path) + if real == os.path.realpath(CONTROL_CHECKOUT_ROOT): + return { + "current_branch": "master", + "head_sha": "a" * 40, + "porcelain_status": "", + } + if real == os.path.realpath(valid_worktree): + return { + "current_branch": "fix/issue-560-issue-comment-worktree-path", + "head_sha": "b" * 40, + "porcelain_status": "", + } + return { + "current_branch": "other", + "head_sha": "c" * 40, + "porcelain_status": "", + } + + return side_effect + + def _subprocess(self, valid_worktree: str, outside_worktree: str | None = None): + def side_effect(cmd, *args, **kwargs): + result = MagicMock(returncode=0, stdout="") + cwd = "" + if isinstance(cmd, list) and "-C" in cmd: + cwd = os.path.realpath(cmd[cmd.index("-C") + 1]) + + if isinstance(cmd, list) and "--show-toplevel" in cmd: + if cwd == os.path.realpath(valid_worktree): + result.stdout = f"{valid_worktree}\n" + elif outside_worktree and cwd == os.path.realpath(outside_worktree): + result.stdout = f"{outside_worktree}\n" + else: + result.stdout = f"{CONTROL_CHECKOUT_ROOT}\n" + return result + + if isinstance(cmd, list) and "--git-common-dir" in cmd: + if cwd == os.path.realpath(valid_worktree): + result.stdout = f"{CONTROL_CHECKOUT_ROOT}/.git\n" + elif outside_worktree and cwd == os.path.realpath(outside_worktree): + result.stdout = "/tmp/other-repo/.git\n" + else: + result.stdout = f"{CONTROL_CHECKOUT_ROOT}/.git\n" + return result + + return result + + return side_effect + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", return_value="a" * 40) + def test_comment_rejects_control_checkout_without_worktree_path( + self, _remote_sha, mock_api, _auth + ): + valid_worktree = os.path.join( + CONTROL_CHECKOUT_ROOT, + "branches", + "issue-560-issue-comment-worktree-path", + ) + mock_api.return_value = {"id": 42} + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with patch( + "gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", + side_effect=self._git_state(valid_worktree), + ): + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue_comment( + issue_number=557, + body="canonical evidence", + remote="prgs", + ) + self.assertIn("stable control checkout", str(ctx.exception)) + mock_api.assert_not_called() + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", return_value="a" * 40) + @patch("os.path.isdir", return_value=True) + @patch("os.path.exists", return_value=True) + def test_comment_accepts_valid_branches_worktree_path_with_explicit_repo( + self, _exists, _isdir, _remote_sha, mock_api, _auth + ): + valid_worktree = os.path.join( + CONTROL_CHECKOUT_ROOT, + "branches", + "issue-560-issue-comment-worktree-path", + ) + mock_api.return_value = {"id": 43} + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""): + with patch( + "gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", + side_effect=self._git_state(valid_worktree), + ): + with patch( + "gitea_mcp_server.subprocess.run", + side_effect=self._subprocess(valid_worktree), + ): + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + result = srv.gitea_create_issue_comment( + issue_number=557, + body="canonical evidence", + remote="prgs", + org="Scaled-Tech-Consulting", + repo="Gitea-Tools", + worktree_path=valid_worktree, + ) + + self.assertTrue(result["success"]) + self.assertTrue(result["performed"]) + self.assertEqual(result["comment_id"], 43) + method, url, _auth_arg, payload = mock_api.call_args[0] + self.assertEqual(method, "POST") + self.assertIn("/repos/Scaled-Tech-Consulting/Gitea-Tools/issues/557/comments", url) + self.assertEqual(payload, {"body": "canonical evidence"}) + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", return_value="a" * 40) + @patch("os.path.isdir", return_value=True) + @patch("os.path.exists", return_value=True) + def test_comment_rejects_outside_repo_worktree_path( + self, _exists, _isdir, _remote_sha, mock_api, _auth + ): + valid_worktree = os.path.join( + CONTROL_CHECKOUT_ROOT, + "branches", + "issue-560-issue-comment-worktree-path", + ) + outside_worktree = "/tmp/not-gitea-tools-worktree" + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""): + with patch( + "gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", + side_effect=self._git_state(valid_worktree), + ): + with patch( + "gitea_mcp_server.subprocess.run", + side_effect=self._subprocess(valid_worktree, outside_worktree), + ): + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue_comment( + issue_number=557, + body="canonical evidence", + remote="prgs", + worktree_path=outside_worktree, + ) + self.assertIn("does not belong to the target repository", str(ctx.exception)) + mock_api.assert_not_called() + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server.api_request") + def test_comment_still_requires_capability_preflight(self, mock_api, _auth): + srv._preflight_capability_called = False + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with self.assertRaises(RuntimeError) as ctx: + srv.gitea_create_issue_comment( + issue_number=557, + body="canonical evidence", + remote="prgs", + worktree_path=os.path.join( + CONTROL_CHECKOUT_ROOT, + "branches", + "issue-560-issue-comment-worktree-path", + ), + ) + self.assertIn("Task capability", str(ctx.exception)) + mock_api.assert_not_called() + + @patch("gitea_mcp_server._auth", return_value=FAKE_AUTH) + @patch("gitea_mcp_server.api_request") + @patch("gitea_mcp_server.root_checkout_guard.resolve_remote_master_sha", return_value="a" * 40) + @patch("os.path.isdir", return_value=True) + @patch("os.path.exists", return_value=True) + def test_comment_still_requires_issue_comment_permission( + self, _exists, _isdir, _remote_sha, mock_api, _auth + ): + valid_worktree = os.path.join( + CONTROL_CHECKOUT_ROOT, + "branches", + "issue-560-issue-comment-worktree-path", + ) + denied_env = { + "GITEA_PROFILE_NAME": "gitea-author", + "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.pr.comment", + } + with patch.object(srv, "PROJECT_ROOT", CONTROL_CHECKOUT_ROOT): + with patch("gitea_mcp_server._get_workspace_porcelain", return_value=""): + with patch( + "gitea_mcp_server.issue_lock_worktree.read_worktree_git_state", + side_effect=self._git_state(valid_worktree), + ): + with patch( + "gitea_mcp_server.subprocess.run", + side_effect=self._subprocess(valid_worktree), + ): + with patch.dict(os.environ, denied_env, clear=True): + result = srv.gitea_create_issue_comment( + issue_number=557, + body="canonical evidence", + remote="prgs", + worktree_path=valid_worktree, + ) + self.assertFalse(result["success"]) + self.assertFalse(result["performed"]) + self.assertIn("permission_report", result) + self.assertEqual( + result["permission_report"]["missing_permission"], + "gitea.issue.comment", + ) + mock_api.assert_not_called() + + +if __name__ == "__main__": + unittest.main()