diff --git a/docs/gitea-execution-profiles.md b/docs/gitea-execution-profiles.md index 9f3e5b1..19a6c7b 100644 --- a/docs/gitea-execution-profiles.md +++ b/docs/gitea-execution-profiles.md @@ -179,6 +179,30 @@ allowed/forbidden membership check): therefore never silently widen permissions. - An empty or missing `allowed_operations` list denies everything. +## Issue comments versus PR reviews (#126) + +Issue discussion comments and PR reviews are different capabilities and are +gated by different operations: + +- **Issue comments** (`gitea_list_issue_comments`, `gitea_create_issue_comment`) + post to and read from an issue's discussion thread + (`/issues/{n}/comments`). Listing requires `gitea.read`; creating requires + `gitea.issue.comment`. They never submit review verdicts. +- **PR reviews** (`gitea_review_pr`, `gitea_submit_pr_review`) submit + approve/request-changes/comment verdicts on pull requests + (`/pulls/{n}/reviews`) and are gated by the `gitea.pr.*` family + (`gitea.pr.review`, `gitea.pr.approve`, `gitea.pr.request_changes`, + `gitea.pr.comment`). + +A profile holding the full PR review/merge set still cannot post issue +discussion comments unless it also allows `gitea.issue.comment`, and vice +versa — neither family implies the other. Both comment tools require an +explicit issue number; the target repo comes only from the standard +remote/org/repo arguments. Create operations are audit-logged +(`create_issue_comment`) when `GITEA_AUDIT_LOG` is configured, errors are +redacted, and normal output contains no endpoint URLs +(`GITEA_MCP_REVEAL_ENDPOINTS=1` is the local admin opt-in for web links). + ## Identity and fail-closed rules Before **any** mutating action, a workflow must know both: diff --git a/mcp_server.py b/mcp_server.py index 4972ac5..e44b00f 100644 --- a/mcp_server.py +++ b/mcp_server.py @@ -1364,6 +1364,145 @@ def gitea_view_issue( } +def _issue_comment_gate(op: str) -> list[str]: + """Profile permission check for issue-comment tools (#126). + + Issue discussion comments are gated separately from the gitea.pr.* + review/merge family: listing requires ``gitea.read``, creating requires + ``gitea.issue.comment``. Returns a list of block reasons (empty = allowed); + an unreadable profile fails closed. + """ + try: + profile = get_profile() + except Exception as exc: + return [f"profile could not be resolved (fail closed): {_redact(str(exc))}"] + op_ok, op_reason = gitea_config.check_operation( + op, profile["allowed_operations"], profile["forbidden_operations"]) + if op_ok: + return [] + if op_reason == "no-allowed-operations": + return ["profile has no configured allowed operations (fail closed)"] + if op_reason == "forbidden": + return [f"profile forbids '{op}'"] + if op_reason == "invalid-forbidden-entry": + return ["profile has an unrecognized forbidden operation entry (fail closed)"] + return [f"profile is not allowed to {op}"] + + +@mcp.tool() +def gitea_list_issue_comments( + issue_number: int, + limit: int = 50, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """List discussion comments on a Gitea issue. + + Read-only. Issue discussion comments are distinct from PR reviews: this + reads the issue comment thread and never touches review endpoints. The + profile must allow ``gitea.read`` (fail closed otherwise). + + Normal output is LLM-safe: no endpoint URLs. Set + GITEA_MCP_REVEAL_ENDPOINTS=1 (admin/debug opt-in) to include each + comment's web link. + + Args: + issue_number: The issue number whose comments to list (required). + limit: Max number of comments to return (default: 50). + remote: Known instance — 'dadeschools' or 'prgs'. + host: Override the Gitea host. + org: Override the owner/organization. + repo: Override the repository name. + + Returns: + dict with 'success', 'issue_number', and 'comments' (each with 'id', + 'author', 'created_at', 'updated_at', 'body'); on a permission block, + 'success' False and 'reasons' with no API call made. + """ + reasons = _issue_comment_gate("gitea.read") + if reasons: + return {"success": False, "issue_number": issue_number, + "reasons": reasons} + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + api = f"{repo_api_url(h, o, r)}/issues/{issue_number}/comments" + comments = api_request("GET", api, auth) or [] + reveal = _reveal_endpoints() + out = [] + for c in comments[:limit]: + entry = { + "id": c["id"], + "author": (c.get("user") or {}).get("login", ""), + "created_at": c.get("created_at"), + "updated_at": c.get("updated_at"), + "body": c.get("body", ""), + } + if reveal: + entry["url"] = c.get("html_url") + out.append(entry) + return {"success": True, "issue_number": issue_number, "comments": out} + + +@mcp.tool() +def gitea_create_issue_comment( + issue_number: int, + body: str, + remote: str = "dadeschools", + host: str | None = None, + org: str | None = None, + repo: str | None = None, +) -> dict: + """Post a markdown comment to a Gitea issue's discussion thread. + + Issue discussion comments are distinct from PR reviews: this posts to the + issue comment thread only and never submits review verdicts. The profile + must allow ``gitea.issue.comment`` — gated separately from the gitea.pr.* + review/merge operations (fail closed otherwise). The target issue is + always explicit; there is no inference beyond the standard remote + defaults used by every tool. + + Normal output is LLM-safe: comment id + issue number, no endpoint URLs. + Set GITEA_MCP_REVEAL_ENDPOINTS=1 (admin/debug opt-in) to include the + comment's web link. Errors are redacted before being raised. + + Args: + issue_number: The issue number to comment on (required). + body: Markdown comment body (required, non-empty). + remote: Known instance — 'dadeschools' or 'prgs'. + host: Override the Gitea host. + org: Override the owner/organization. + repo: Override the repository name. + + Returns: + dict with 'success', 'comment_id', and 'issue_number' ('url' only + with the reveal opt-in); on a permission block or empty body, + 'success'/'performed' False and 'reasons' with no API call made. + """ + reasons = _issue_comment_gate("gitea.issue.comment") + if not (body or "").strip(): + reasons.append("comment body must be a non-empty string") + if reasons: + return {"success": False, "performed": False, + "issue_number": issue_number, "reasons": reasons} + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) + api = f"{repo_api_url(h, o, r)}/issues/{issue_number}/comments" + try: + with _audited("create_issue_comment", host=h, remote=remote, org=o, + repo=r, issue_number=issue_number, + request_metadata={"body_chars": len(body)}): + data = api_request("POST", api, auth, {"body": body}) + except Exception as exc: + raise RuntimeError(_redact(str(exc))) from None + result = {"success": True, "performed": True, + "comment_id": data["id"], "issue_number": issue_number} + if _reveal_endpoints(): + result["url"] = data.get("html_url") + return result + + @mcp.tool() def gitea_whoami( remote: str = "dadeschools", diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 50d0c3d..83817ce 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -31,6 +31,8 @@ from mcp_server import ( # noqa: E402 gitea_get_profile, gitea_check_pr_eligibility, gitea_submit_pr_review, + gitea_list_issue_comments, + gitea_create_issue_comment, ) from gitea_auth import get_profile # noqa: E402 @@ -1903,3 +1905,199 @@ class TestEndpointRedaction(unittest.TestCase): from mcp_server import gitea_audit_config result = gitea_audit_config() self.assertFalse(result["configured"]) + + +# --------------------------------------------------------------------------- +# Issue comment tools (#126) +# --------------------------------------------------------------------------- +class TestIssueCommentTools(unittest.TestCase): + """gitea_list_issue_comments / gitea_create_issue_comment (#126). + + Issue discussion comments are distinct from PR reviews: they hit the + /issues/{n}/comments endpoint, are gated on gitea.read (list) and + gitea.issue.comment (create) — never on the gitea.pr.* review/merge + family — and their normal output is LLM-safe (no endpoint URLs; the + GITEA_MCP_REVEAL_ENDPOINTS opt-in restores links for local diagnostics). + """ + + AUTHOR_ENV = { + "GITEA_PROFILE_NAME": "gitea-author", + "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment", + } + + @staticmethod + def _comment(cid=101, login="alice", body="hello world"): + return { + "id": cid, + "user": {"login": login}, + "body": body, + "created_at": "2026-07-03T00:00:00Z", + "updated_at": "2026-07-03T01:00:00Z", + "html_url": ( + "https://gitea.example.com/o/r/issues/9#issuecomment-%d" % cid + ), + } + + # -- list ---------------------------------------------------------------- + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_lists_comments(self, _auth, mock_api): + mock_api.return_value = [self._comment(101), self._comment(102, "bob")] + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + result = gitea_list_issue_comments(issue_number=9, remote="prgs") + self.assertTrue(result["success"]) + self.assertEqual(result["issue_number"], 9) + self.assertEqual(len(result["comments"]), 2) + first = result["comments"][0] + self.assertEqual(first["id"], 101) + self.assertEqual(first["author"], "alice") + self.assertEqual(first["body"], "hello world") + self.assertEqual(first["created_at"], "2026-07-03T00:00:00Z") + self.assertEqual(first["updated_at"], "2026-07-03T01:00:00Z") + url = mock_api.call_args[0][1] + self.assertIn("/issues/9/comments", url) + self.assertEqual(mock_api.call_args[0][0], "GET") + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_list_output_has_no_urls_by_default(self, _auth, mock_api): + mock_api.return_value = [self._comment()] + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + result = gitea_list_issue_comments(issue_number=9, remote="prgs") + blob = json.dumps(result) + self.assertNotIn("https://", blob) + self.assertNotIn("http://", blob) + self.assertNotIn("url", blob) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_list_reveal_opt_in_includes_url(self, _auth, mock_api): + mock_api.return_value = [self._comment()] + env = dict(self.AUTHOR_ENV, GITEA_MCP_REVEAL_ENDPOINTS="1") + with patch.dict(os.environ, env, clear=True): + result = gitea_list_issue_comments(issue_number=9, remote="prgs") + self.assertIn("issuecomment-101", result["comments"][0]["url"]) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_list_blocked_without_read_permission(self, _auth, mock_api): + env = {"GITEA_PROFILE_NAME": "gitea-writer-only", + "GITEA_ALLOWED_OPERATIONS": "gitea.issue.comment"} + with patch.dict(os.environ, env, clear=True): + result = gitea_list_issue_comments(issue_number=9, remote="prgs") + self.assertFalse(result["success"]) + self.assertTrue(result["reasons"]) + mock_api.assert_not_called() + + def test_list_unknown_remote_raises(self): + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + with self.assertRaises(ValueError): + gitea_list_issue_comments(issue_number=9, remote="nope") + + # -- create ---------------------------------------------------------------- + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_creates_comment(self, _auth, mock_api): + mock_api.return_value = self._comment(555, "gitea-author", "posted") + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + result = gitea_create_issue_comment( + issue_number=9, body="posted", remote="prgs") + self.assertTrue(result["success"]) + self.assertEqual(result["comment_id"], 555) + self.assertEqual(result["issue_number"], 9) + self.assertEqual(mock_api.call_args[0][0], "POST") + url = mock_api.call_args[0][1] + self.assertIn("/issues/9/comments", url) + self.assertIn("gitea.prgs.cc", url) + self.assertIn("Scaled-Tech-Consulting", url) + self.assertEqual(mock_api.call_args[0][3], {"body": "posted"}) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_create_output_has_no_urls_by_default(self, _auth, mock_api): + mock_api.return_value = self._comment(555) + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + result = gitea_create_issue_comment( + issue_number=9, body="posted", remote="prgs") + blob = json.dumps(result) + self.assertNotIn("https://", blob) + self.assertNotIn("http://", blob) + self.assertNotIn("url", blob) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_create_reveal_opt_in_includes_url(self, _auth, mock_api): + mock_api.return_value = self._comment(555) + env = dict(self.AUTHOR_ENV, GITEA_MCP_REVEAL_ENDPOINTS="1") + with patch.dict(os.environ, env, clear=True): + result = gitea_create_issue_comment( + issue_number=9, body="posted", remote="prgs") + self.assertIn("issuecomment-555", result["url"]) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_review_permissions_do_not_grant_issue_comments(self, _auth, mock_api): + env = {"GITEA_PROFILE_NAME": "gitea-reviewer", + "GITEA_ALLOWED_OPERATIONS": + "gitea.read,gitea.pr.review,gitea.pr.comment," + "gitea.pr.approve,gitea.pr.merge"} + with patch.dict(os.environ, env, clear=True): + result = gitea_create_issue_comment( + issue_number=9, body="posted", remote="prgs") + self.assertFalse(result["success"]) + self.assertFalse(result["performed"]) + self.assertTrue(result["reasons"]) + mock_api.assert_not_called() + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_create_forbidden_overrides_allowed(self, _auth, mock_api): + env = {"GITEA_PROFILE_NAME": "gitea-author", + "GITEA_ALLOWED_OPERATIONS": "gitea.read,gitea.issue.comment", + "GITEA_FORBIDDEN_OPERATIONS": "gitea.issue.comment"} + with patch.dict(os.environ, env, clear=True): + result = gitea_create_issue_comment( + issue_number=9, body="posted", remote="prgs") + self.assertFalse(result["success"]) + mock_api.assert_not_called() + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_create_empty_body_blocked(self, _auth, mock_api): + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + result = gitea_create_issue_comment( + issue_number=9, body=" ", remote="prgs") + self.assertFalse(result["success"]) + mock_api.assert_not_called() + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_create_missing_issue_error_is_redacted(self, _auth, mock_api): + mock_api.side_effect = RuntimeError( + "Gitea API error 404 for issue (auth was token abc123secret)") + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + with self.assertRaises(RuntimeError) as cm: + gitea_create_issue_comment( + issue_number=99999, body="posted", remote="prgs") + msg = str(cm.exception) + self.assertNotIn("abc123secret", msg) + self.assertIn("404", msg) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) + def test_create_never_touches_review_endpoints(self, _auth, mock_api): + mock_api.return_value = self._comment(555) + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + gitea_create_issue_comment( + issue_number=9, body="posted", remote="prgs") + for call in mock_api.call_args_list: + self.assertNotIn("reviews", call[0][1]) + self.assertNotIn("/pulls/", call[0][1]) + + def test_create_unknown_remote_raises(self): + with patch.dict(os.environ, self.AUTHOR_ENV, clear=True): + with self.assertRaises(ValueError): + gitea_create_issue_comment( + issue_number=9, body="x", remote="nope")