diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index f49505bb..1c27c35a 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -70,7 +70,7 @@ "name": "magpie-utilities", "source": "./plugins/magpie-utilities", "version": "0.2.0.dev0", - "description": "Apache Magpie utilities family (4 skills)." + "description": "Apache Magpie utilities family (5 skills)." } ] } diff --git a/docs/setup/marketplaces.md b/docs/setup/marketplaces.md index 32635dad..46efd29e 100644 --- a/docs/setup/marketplaces.md +++ b/docs/setup/marketplaces.md @@ -114,7 +114,7 @@ the model on **every** turn — see ["always-on" cost](#versioning) below). **All-in-one — `magpie`** -- ✅ One install; all 70 skills; nothing to decide. Uses the real `skills/` +- ✅ One install; all 71 skills; nothing to decide. Uses the real `skills/` directory, so **no symlinks** — works on Windows out of the box. - ⚠️ Adds **~21.7k always-on tokens to every session**, including families you may never use — that context (and cost) is spent whether or not you invoke a @@ -145,10 +145,10 @@ so pick one approach. | `magpie-issue` | 8 | ~2.4k | | `magpie-repo-health` | 7 | ~2.1k | | `magpie-contributor-growth` | 6 | ~1.8k | -| `magpie-utilities` | 4 | ~1.4k | +| `magpie-utilities` | 5 | ~1.7k | | `magpie-mentoring` | 4 | ~1.2k | | `magpie-pairing` | 2 | ~0.6k | -| **`magpie`** (all) | **70** | **~21.7k** | +| **`magpie`** (all) | **71** | **~22.0k** | Skills are invoked under the installing plugin's namespace — e.g. `/magpie:release-vote-tally` (all-in-one) or diff --git a/plugins/magpie-utilities/.claude-plugin/plugin.json b/plugins/magpie-utilities/.claude-plugin/plugin.json index 30572ef9..8a29b8a7 100644 --- a/plugins/magpie-utilities/.claude-plugin/plugin.json +++ b/plugins/magpie-utilities/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "magpie-utilities", - "description": "Apache Magpie \u2014 framework meta-skills: write-skill, optimize-skill, skill-reconciler, list-skills.", + "description": "Apache Magpie \u2014 framework meta-skills: write-skill, optimize-skill, skill-reconciler, list-skills, report-framework-issue.", "version": "0.2.0.dev0", "author": { "name": "Apache Magpie", diff --git a/plugins/magpie-utilities/skills/report-framework-issue b/plugins/magpie-utilities/skills/report-framework-issue new file mode 120000 index 00000000..c0d28ed8 --- /dev/null +++ b/plugins/magpie-utilities/skills/report-framework-issue @@ -0,0 +1 @@ +../../../skills/report-framework-issue \ No newline at end of file diff --git a/skills/pyproject.toml b/skills/pyproject.toml index c6562e57..10dd98ab 100644 --- a/skills/pyproject.toml +++ b/skills/pyproject.toml @@ -50,6 +50,7 @@ package = false [tool.ruff] line-length = 110 target-version = "py311" +include = ["**/*.py", "**/*.pyi"] [tool.ruff.lint] select = [ diff --git a/tools/bitbucket/tests/test_bitbucket.py b/tools/bitbucket/tests/test_bitbucket.py index 8d518a7d..a12683aa 100644 --- a/tools/bitbucket/tests/test_bitbucket.py +++ b/tools/bitbucket/tests/test_bitbucket.py @@ -27,7 +27,14 @@ from magpie_bitbucket import cloud, datacenter from magpie_bitbucket.cli import main -from magpie_bitbucket.client import BitbucketError, SameHostRedirectHandler, load_config, make_auth_header +from magpie_bitbucket.client import ( + BitbucketError, + NoAuthRedirectHandler, + SameHostRedirectHandler, + _require_https, + load_config, + make_auth_header, +) from magpie_bitbucket.normalize import ( created_issue_comment, issue, @@ -106,6 +113,26 @@ def urllib_request(url: str) -> urllib.request.Request: ) +def test_no_auth_redirect_handler_rejects_all_redirects() -> None: + handler = NoAuthRedirectHandler() + request = urllib_request("https://bitbucket.example.test/rest/api/1.0/foo") + + with pytest.raises(BitbucketError, match="refusing to forward credentials"): + handler.redirect_request( + request, + None, + 302, + "Found", + {}, + "https://bitbucket.example.test/rest/api/1.0/bar", + ) + + +def test_require_https_rejects_http() -> None: + with pytest.raises(BitbucketError, match="Bitbucket API URLs must use HTTPS"): + _require_https("http://bitbucket.example.test/rest/api/1.0/foo") + + def test_same_host_redirect_handler_allows_same_origin() -> None: handler = SameHostRedirectHandler() request = urllib_request("https://bitbucket.example.test/rest/api/1.0/foo") diff --git a/tools/dev/check-family-plugins.py b/tools/dev/check-family-plugins.py index 128ec324..40f0f073 100644 --- a/tools/dev/check-family-plugins.py +++ b/tools/dev/check-family-plugins.py @@ -127,7 +127,7 @@ "issue": "issue lifecycle: triage, reproduction, fix drafting, reassess, stale-sweep, dedup, backlog stats.", "repo-health": "read-only repo-health audits: runner labels, workflow security, dependency/license/NOTICE, flaky tests, audit-finding fixes.", "contributor-growth": "path-to-committer: activity sweeps, nominations, sentiment, readiness, committer/post-vote onboarding.", - "utilities": "framework meta-skills: write-skill, optimize-skill, skill-reconciler, list-skills.", + "utilities": "framework meta-skills: write-skill, optimize-skill, skill-reconciler, list-skills, report-framework-issue.", "mentoring": "newcomer mentoring: welcome, newcomer-issue explanations, good-first-issue authoring + sweep.", "pairing": "pair a change with a structured self-review or a multi-agent adversarial review.", } diff --git a/tools/skill-evals/pyproject.toml b/tools/skill-evals/pyproject.toml index 97f2a328..e228d563 100644 --- a/tools/skill-evals/pyproject.toml +++ b/tools/skill-evals/pyproject.toml @@ -38,6 +38,7 @@ packages = ["src/skill_evals"] line-length = 110 target-version = "py311" src = ["src"] +extend-exclude = ["evals"] [tool.ruff.lint] select = ["E", "W", "F", "I", "B", "UP", "SIM", "C4", "RUF"] diff --git a/tools/sourcehut/src/magpie_sourcehut/client.py b/tools/sourcehut/src/magpie_sourcehut/client.py index dffacb9c..0b12cf05 100644 --- a/tools/sourcehut/src/magpie_sourcehut/client.py +++ b/tools/sourcehut/src/magpie_sourcehut/client.py @@ -22,6 +22,7 @@ import json import os import urllib.error +import urllib.parse import urllib.request from typing import Any @@ -30,6 +31,28 @@ class SourceHutError(Exception): """General exception for SourceHut client errors.""" +class NoAuthRedirectHandler(urllib.request.HTTPRedirectHandler): + """Reject redirects so Authorization is not forwarded to another host.""" + + def redirect_request( + self, + req: urllib.request.Request, + fp: Any, + code: int, + msg: str, + headers: Any, + newurl: str, + ) -> urllib.request.Request | None: + raise SourceHutError(f"SourceHut request redirected to {newurl}; refusing to forward credentials") + + +def _require_https(url: str) -> None: + """Require HTTPS for SourceHut API URLs.""" + parsed = urllib.parse.urlparse(url) + if parsed.scheme != "https": + raise SourceHutError("SourceHut API URLs must use HTTPS") + + def query_graphql(service: str, query: str, variables: dict[str, Any] | None = None) -> dict[str, Any]: """Execute a GraphQL query/mutation against a specific SourceHut service. @@ -46,6 +69,7 @@ def query_graphql(service: str, query: str, variables: dict[str, Any] | None = N raise SourceHutError("SRHT_TOKEN environment variable is not set") url = f"https://{service}.sr.ht/query" + _require_https(url) payload: dict[str, Any] = {"query": query} if variables: payload["variables"] = variables @@ -61,8 +85,12 @@ def query_graphql(service: str, query: str, variables: dict[str, Any] | None = N method="POST", ) + # Writes never follow redirects: repeating a mutation at a redirected + # location is less safe than failing and requiring the caller to retry. + opener = urllib.request.build_opener(NoAuthRedirectHandler) + try: - with urllib.request.urlopen(req) as resp: + with opener.open(req) as resp: body = resp.read().decode("utf-8") res_json = json.loads(body) errors = res_json.get("errors") @@ -70,6 +98,8 @@ def query_graphql(service: str, query: str, variables: dict[str, Any] | None = N err_msgs = [e.get("message", "Unknown error") for e in errors] raise SourceHutError(f"GraphQL error from {service}.sr.ht: {'; '.join(err_msgs)}") return res_json.get("data", {}) + except SourceHutError: + raise except urllib.error.HTTPError as exc: err_msg = None try: diff --git a/tools/sourcehut/tests/test_sourcehut.py b/tools/sourcehut/tests/test_sourcehut.py index 064a6784..2281b794 100644 --- a/tools/sourcehut/tests/test_sourcehut.py +++ b/tools/sourcehut/tests/test_sourcehut.py @@ -18,6 +18,7 @@ import io import json import urllib.error +import urllib.request from email.message import Message from typing import Any from unittest.mock import MagicMock, patch @@ -26,7 +27,12 @@ from magpie_sourcehut.builds import get_job from magpie_sourcehut.cli import main -from magpie_sourcehut.client import SourceHutError, query_graphql +from magpie_sourcehut.client import ( + NoAuthRedirectHandler, + SourceHutError, + _require_https, + query_graphql, +) from magpie_sourcehut.lists import get_patchset, list_patchsets, map_patchset_to_pr from magpie_sourcehut.repo import get_repo from magpie_sourcehut.todo import ( @@ -50,9 +56,53 @@ def make_mock_response(status_code: int, body_dict: dict[str, Any]) -> MagicMock return mock_resp -@patch("urllib.request.urlopen") -def test_query_graphql_success(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response(200, {"data": {"version": "1.0"}}) +def mock_opener(mock_build_opener: MagicMock, *bodies: dict[str, Any]) -> MagicMock: + opener = MagicMock() + opener.open.side_effect = [ + MagicMock( + __enter__=MagicMock(return_value=make_mock_response(200, body)), + __exit__=MagicMock(return_value=None), + ) + for body in bodies + ] + mock_build_opener.return_value = opener + return opener + + +def test_no_auth_redirect_handler_rejects_redirect() -> None: + handler = NoAuthRedirectHandler() + request = urllib.request.Request( + "https://todo.sr.ht/query", + headers={"Authorization": "Bearer mock_token_123"}, + method="POST", + ) + + with pytest.raises(SourceHutError, match="refusing to forward credentials"): + handler.redirect_request( + request, + None, + 302, + "Found", + {}, + "https://todo.sr.ht/redirected", + ) + + +def test_require_https_rejects_http() -> None: + with pytest.raises(SourceHutError, match="SourceHut API URLs must use HTTPS"): + _require_https("http://todo.sr.ht/query") + + +@patch("urllib.request.build_opener") +def test_query_graphql_uses_no_auth_redirect_handler(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener(mock_build_opener, {"data": {"version": "1.0"}}) + query_graphql("todo", "{ version }") + mock_build_opener.assert_called_once_with(NoAuthRedirectHandler) + + +@patch("urllib.request.build_opener") +def test_query_graphql_success(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener(mock_build_opener, {"data": {"version": "1.0"}}) res = query_graphql("todo", "{ version }") assert res == {"version": "1.0"} @@ -62,11 +112,9 @@ def test_query_graphql_no_token() -> None: query_graphql("todo", "{ version }") -@patch("urllib.request.urlopen") -def test_query_graphql_error_in_json(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"errors": [{"message": "Invalid query syntax"}]} - ) +@patch("urllib.request.build_opener") +def test_query_graphql_error_in_json(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener(mock_build_opener, {"errors": [{"message": "Invalid query syntax"}]}) with pytest.raises(SourceHutError, match=r"GraphQL error from todo\.sr\.ht: Invalid query syntax"): query_graphql("todo", "invalid_query") @@ -83,16 +131,18 @@ def test_query_graphql_error_in_json(mock_urlopen: MagicMock, mock_env: None) -> pytest.param(b'{"errors": ["boom"]}', id="errors-list-of-strings"), ], ) -@patch("urllib.request.urlopen") +@patch("urllib.request.build_opener") def test_query_graphql_http_error_body_never_leaks( - mock_urlopen: MagicMock, mock_env: None, body: bytes + mock_build_opener: MagicMock, mock_env: None, body: bytes ) -> None: # Parsing the error body is best-effort: whatever shape it arrives in, the # caller must see a SourceHutError, never a raw UnicodeDecodeError / # AttributeError from the parse attempt. - mock_urlopen.side_effect = urllib.error.HTTPError( + opener = MagicMock() + opener.open.side_effect = urllib.error.HTTPError( "https://todo.sr.ht/query", 400, "Bad Request", Message(), io.BytesIO(body) ) + mock_build_opener.return_value = opener with pytest.raises( SourceHutError, match=r"HTTP request to https://todo\.sr\.ht/query failed with status 400" @@ -100,24 +150,26 @@ def test_query_graphql_http_error_body_never_leaks( query_graphql("todo", "{ version }") -@patch("urllib.request.urlopen") +@patch("urllib.request.build_opener") def test_query_graphql_http_error_uses_body_message_when_parseable( - mock_urlopen: MagicMock, mock_env: None + mock_build_opener: MagicMock, mock_env: None ) -> None: - mock_urlopen.side_effect = urllib.error.HTTPError( + opener = MagicMock() + opener.open.side_effect = urllib.error.HTTPError( "https://todo.sr.ht/query", 400, "Bad Request", Message(), io.BytesIO(b'{"errors": [{"message": "bad query"}]}'), ) + mock_build_opener.return_value = opener with pytest.raises(SourceHutError, match=r"HTTP 400: bad query"): query_graphql("todo", "{ version }") -@patch("urllib.request.urlopen") -def test_get_ticket(mock_urlopen: MagicMock, mock_env: None) -> None: +@patch("urllib.request.build_opener") +def test_get_ticket(mock_build_opener: MagicMock, mock_env: None) -> None: ticket_data = { "id": 42, "title": "Fix memory leak", @@ -127,52 +179,45 @@ def test_get_ticket(mock_urlopen: MagicMock, mock_env: None) -> None: "labels": [{"id": 1, "name": "bug"}], "comments": [], } - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"tracker": {"ticket": ticket_data}}} - ) + mock_opener(mock_build_opener, {"data": {"tracker": {"ticket": ticket_data}}}) res = get_ticket("~user", "my-project", 42) assert res["id"] == 42 assert res["title"] == "Fix memory leak" -@patch("urllib.request.urlopen") -def test_submit_ticket(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"submitTicket": {"id": 101, "title": "New issue"}}} - ) +@patch("urllib.request.build_opener") +def test_submit_ticket(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener(mock_build_opener, {"data": {"submitTicket": {"id": 101, "title": "New issue"}}}) res = submit_ticket("~user", "my-project", "New issue", "Description here") assert res["id"] == 101 -@patch("urllib.request.urlopen") -def test_submit_comment(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"submitComment": {"id": 501, "body": "Comment body"}}} - ) +@patch("urllib.request.build_opener") +def test_submit_comment(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener(mock_build_opener, {"data": {"submitComment": {"id": 501, "body": "Comment body"}}}) res = submit_comment("~user", "my-project", 42, "Comment body") assert res["id"] == 501 -@patch("urllib.request.urlopen") -def test_label_ticket(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"labelTicket": {"id": 42}}} - ) +@patch("urllib.request.build_opener") +def test_label_ticket(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener(mock_build_opener, {"data": {"labelTicket": {"id": 42}}}) res = label_ticket("~user", "my-project", 42, 10) assert res == {"id": 42} -@patch("urllib.request.urlopen") -def test_update_ticket_status(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"updateTicketStatus": {"id": 42, "status": "RESOLVED", "resolution": "FIXED"}}} +@patch("urllib.request.build_opener") +def test_update_ticket_status(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener( + mock_build_opener, + {"data": {"updateTicketStatus": {"id": 42, "status": "RESOLVED", "resolution": "FIXED"}}}, ) res = update_ticket_status("~user", "my-project", 42, "RESOLVED", "FIXED") assert res["status"] == "RESOLVED" -@patch("urllib.request.urlopen") -def test_get_patchset_and_mapping(mock_urlopen: MagicMock, mock_env: None) -> None: +@patch("urllib.request.build_opener") +def test_get_patchset_and_mapping(mock_build_opener: MagicMock, mock_env: None) -> None: patchset_data = { "id": 200, "subject": "[PATCH 0/2] Fix some logs", @@ -208,9 +253,7 @@ def test_get_patchset_and_mapping(mock_urlopen: MagicMock, mock_env: None) -> No }, }, } - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"list": {"patchset": patchset_data}}} - ) + mock_opener(mock_build_opener, {"data": {"list": {"patchset": patchset_data}}}) raw = get_patchset("~user", "my-list", 200) assert raw["id"] == 200 @@ -316,10 +359,10 @@ def get(self, key: str, default: object = None) -> object: map_patchset_to_pr(patchset) -@patch("urllib.request.urlopen") -def test_list_patchsets(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, +@patch("urllib.request.build_opener") +def test_list_patchsets(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener( + mock_build_opener, { "data": { "list": { @@ -338,29 +381,27 @@ def test_list_patchsets(mock_urlopen: MagicMock, mock_env: None) -> None: assert res[0]["subject"] == "P1" -@patch("urllib.request.urlopen") -def test_get_job(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"job": {"id": 55, "status": "SUCCESS", "tasks": []}}} - ) +@patch("urllib.request.build_opener") +def test_get_job(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener(mock_build_opener, {"data": {"job": {"id": 55, "status": "SUCCESS", "tasks": []}}}) res = get_job(55) assert res["status"] == "SUCCESS" -@patch("urllib.request.urlopen") -def test_get_repo(mock_urlopen: MagicMock, mock_env: None) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"repository": {"id": 9, "name": "my-repo", "description": "VCS"}}} +@patch("urllib.request.build_opener") +def test_get_repo(mock_build_opener: MagicMock, mock_env: None) -> None: + mock_opener( + mock_build_opener, {"data": {"repository": {"id": 9, "name": "my-repo", "description": "VCS"}}} ) res = get_repo("git", "~user", "my-repo") assert res["name"] == "my-repo" -@patch("urllib.request.urlopen") -def test_cli_dispatch(mock_urlopen: MagicMock, mock_env: None, capsys: pytest.CaptureFixture[str]) -> None: - mock_urlopen.return_value.__enter__.return_value = make_mock_response( - 200, {"data": {"job": {"id": 12, "status": "FAILED"}}} - ) +@patch("urllib.request.build_opener") +def test_cli_dispatch( + mock_build_opener: MagicMock, mock_env: None, capsys: pytest.CaptureFixture[str] +) -> None: + mock_opener(mock_build_opener, {"data": {"job": {"id": 12, "status": "FAILED"}}}) code = main(["build", "get", "12"]) assert code == 0 captured = capsys.readouterr() diff --git a/tools/symlink-lint/src/symlink_lint/__init__.py b/tools/symlink-lint/src/symlink_lint/__init__.py index 3939595e..e1eb6fc7 100644 --- a/tools/symlink-lint/src/symlink_lint/__init__.py +++ b/tools/symlink-lint/src/symlink_lint/__init__.py @@ -189,10 +189,12 @@ def find_archive_symlink_problems(root: Path) -> list[tuple[str, str, str]]: if not member.issym(): continue link_dir = os.path.dirname(member.name) - target = os.path.normpath(os.path.join(link_dir, member.linkname)) + target = os.path.normpath(os.path.join(link_dir, member.linkname)).replace("\\", "/") resolved = by_path.get(target) if resolved is None: - problems.append((member.name, member.linkname, "dangling")) + target_prefix = target + "/" + if not any(p.startswith(target_prefix) for p in by_path): + problems.append((member.name, member.linkname, "dangling")) elif resolved.issym(): problems.append((member.name, member.linkname, "chain")) return sorted(problems)