diff --git a/app/ledger/service.py b/app/ledger/service.py index 3812285..295ae46 100644 --- a/app/ledger/service.py +++ b/app/ledger/service.py @@ -6,11 +6,10 @@ import re from datetime import UTC, datetime from decimal import Decimal, InvalidOperation -from typing import Any, cast +from typing import Any from urllib.parse import urlparse from sqlalchemy import case, func, select, update -from sqlalchemy.engine import CursorResult from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session @@ -657,19 +656,16 @@ def pay_bounty( reserve_account = reserve_account_for_bounty(bounty.id) if get_balance(session, reserve_account) < bounty.reward_microunits: raise LedgerError("bounty reserve balance too low") - claimed = cast( - CursorResult[Any], - session.execute( - update(Bounty) - .where(Bounty.id == bounty.id, Bounty.awards_paid < Bounty.max_awards) - .values( - awards_paid=Bounty.awards_paid + 1, - status=case( - (Bounty.awards_paid + 1 >= Bounty.max_awards, "paid"), - else_="open", - ), - ) - ), + claimed = session.execute( + update(Bounty) + .where(Bounty.id == bounty.id, Bounty.awards_paid < Bounty.max_awards) + .values( + awards_paid=Bounty.awards_paid + 1, + status=case( + (Bounty.awards_paid + 1 >= Bounty.max_awards, "paid"), + else_="open", + ), + ) ) if claimed.rowcount != 1: raise LedgerError("bounty already paid") @@ -737,13 +733,10 @@ def close_bounty( raise LedgerError("bounty is not open") _clean_required_text(closed_by, "closed_by", 80) clean_reference = validate_public_url(reference or bounty.issue_url) - claimed = cast( - CursorResult[Any], - session.execute( - update(Bounty) - .where(Bounty.id == bounty.id, Bounty.status == "open") - .values(status="closed") - ), + claimed = session.execute( + update(Bounty) + .where(Bounty.id == bounty.id, Bounty.status == "open") + .values(status="closed") ) if claimed.rowcount != 1: raise LedgerError("bounty is not open") diff --git a/app/main.py b/app/main.py index 7e7c4f9..ef34a74 100644 --- a/app/main.py +++ b/app/main.py @@ -9,7 +9,7 @@ from datetime import UTC, datetime, timedelta from pathlib import Path from typing import Annotated, Any -from urllib.parse import urlencode, urlsplit, urlunsplit +from urllib.parse import unquote, urlencode, urlsplit, urlunsplit import httpx from fastapi import Depends, FastAPI, Form, HTTPException, Query, Request @@ -278,13 +278,17 @@ def _oauth_configured(settings: Settings) -> bool: def _safe_next_path(next_path: str | None) -> str: + decoded_next_path = unquote(next_path) if next_path else "" if ( not next_path or not next_path.startswith("/") or next_path.startswith("//") or len(next_path) > 2048 or "\\" in next_path + or decoded_next_path.startswith("//") + or "\\" in decoded_next_path or any(ord(char) < 32 or 127 <= ord(char) < 160 for char in next_path) + or any(ord(char) < 32 or 127 <= ord(char) < 160 for char in decoded_next_path) ): return "/me" return next_path @@ -1530,6 +1534,14 @@ def output_format_arg() -> str: raise ValueError("format must be text or json") return normalized + def optional_repo_selector_arg() -> str | None: + repo = optional_clean_str_arg("repo") + if repo is None: + return None + if len(repo) > 200: + raise ValueError("repo is too long") + return repo.lower() + def mcp_issue_number_search_value(query_text: str) -> int | None: if not query_text.isdigit(): return None @@ -1756,8 +1768,11 @@ def optional_bool_arg(field: str, default: bool = False) -> bool: output_format = output_format_arg() has_bounty_id = "bounty_id" in args and args.get("bounty_id") is not None has_issue_number = "issue_number" in args and args.get("issue_number") is not None + repo_selector = optional_repo_selector_arg() if has_bounty_id and has_issue_number: raise ValueError("use bounty_id or issue_number, not both") + if repo_selector is not None and not has_issue_number: + raise ValueError("repo can only be used with issue_number") if has_bounty_id: bounty = session.get(Bounty, positive_int_arg("bounty_id")) if bounty is None: @@ -1768,12 +1783,12 @@ def optional_bool_arg(field: str, default: bool = False) -> bool: else work_proof_guidance(bounty) ) if has_issue_number: - bounties = session.scalars( - select(Bounty) - .where(Bounty.issue_number == positive_int_arg("issue_number")) - .order_by(Bounty.id.desc()) - .limit(2) - ).all() + issue_query = select(Bounty).where( + Bounty.issue_number == positive_int_arg("issue_number") + ) + if repo_selector is not None: + issue_query = issue_query.where(func.lower(Bounty.repo) == repo_selector) + bounties = session.scalars(issue_query.order_by(Bounty.id.desc()).limit(2)).all() if not bounties: return "bounty not found" if len(bounties) > 1: diff --git a/app/mcp.py b/app/mcp.py index 92567a8..5d1c028 100644 --- a/app/mcp.py +++ b/app/mcp.py @@ -38,7 +38,10 @@ {"name": "get_proof", "description": "Get a public proof by hash"}, { "name": "submit_work_proof", - "description": "Return submission instructions, optionally for a bounty_id or issue_number", + "description": ( + "Return submission instructions, optionally for a bounty_id or issue_number " + "and repo, with text or json format" + ), }, ] diff --git a/tests/test_api_mcp.py b/tests/test_api_mcp.py index adf2a36..dfa4fef 100644 --- a/tests/test_api_mcp.py +++ b/tests/test_api_mcp.py @@ -1542,6 +1542,59 @@ def test_mcp_submit_work_proof_reports_unknown_bounty(sqlite_url: str) -> None: assert result["result"]["content"][0]["text"] == "bounty not found" +def test_mcp_submit_work_proof_scopes_issue_number_by_repo(sqlite_url: str) -> None: + create_schema(sqlite_url) + with session_scope(sqlite_url) as session: + ensure_genesis(session) + create_bounty( + session, + repo="ramimbo/mergework", + issue_number=284, + issue_url="https://github.com/ramimbo/mergework/issues/284", + title="First bounty", + reward_mrwk="100", + acceptance="First acceptance.", + ) + target = create_bounty( + session, + repo="example/mergework", + issue_number=284, + issue_url="https://github.com/example/mergework/issues/284", + title="Second bounty", + reward_mrwk="250", + acceptance="Second acceptance.", + ) + target_id = target.id + + client = TestClient(create_app(database_url=sqlite_url, webhook_secret="secret")) + + response = client.post( + "/mcp", + json={ + "jsonrpc": "2.0", + "id": 28, + "method": "tools/call", + "params": { + "name": "submit_work_proof", + "arguments": { + "issue_number": 284, + "repo": "Example/MergeWork", + "format": "json", + }, + }, + }, + ) + + result = response.json()["result"] + structured = result["structuredContent"] + assert json.loads(result["content"][0]["text"]) == structured + assert structured["bounty_id"] == target_id + assert structured["repository"] == "example/mergework" + assert structured["title"] == "Second bounty" + assert structured["reward_mrwk"] == "250" + assert structured["acceptance"] == "Second acceptance." + + @pytest.mark.parametrize( ("arguments", "request_id"), [ @@ -1552,6 +1605,10 @@ def test_mcp_submit_work_proof_reports_unknown_bounty(sqlite_url: str) -> None: ({"bounty_id": 1, "issue_number": 1}, 25), ({"format": "xml"}, 26), ({"format": 1}, 27), + ({"repo": "ramimbo/mergework"}, 29), + ({"bounty_id": 1, "repo": "ramimbo/mergework"}, 30), + ({"issue_number": 1, "repo": 1}, 31), + ({"issue_number": 1, "repo": "a" * 201}, 32), ], ) def test_mcp_submit_work_proof_rejects_invalid_bounty_selectors( diff --git a/tests/test_security.py b/tests/test_security.py index a1288f0..875ca48 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -856,7 +856,10 @@ def test_admin_bounty_api_accepts_multi_award_count( ("https://evil.example/me", "/me"), ("//evil.example/me", "/me"), ("/\\evil.example/me", "/me"), + ("/%2f%2fevil.example/me", "/me"), + ("/%5cevil.example/me", "/me"), ("/me\nLocation: https://evil.example", "/me"), + ("/me%0d%0aLocation:%20https://evil.example", "/me"), ("/me" + chr(0x85), "/me"), ("/me\x7f", "/me"), ("/" + ("a" * 2048), "/me"),