Extract admin payout helpers

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: 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",
+            ),
+        )
     )
     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: Any = 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")

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
@@ -55,9 +55,9 @@
     BountyAttempt,
     LedgerEntry,
     Proof,
-    Submission,
     Wallet,
 )
+from app.payouts import existing_payout_proof_for_submission, payout_response_from_proof
 from app.serializers import (
     accepted_work_for_account,
     account_accepted_summary,
@@ -218,40 +218,6 @@ def expire_stale_bounty_attempts(
     session.execute(query.values(status="expired", updated_at=now))
 
 
-def _payout_response_from_proof(proof: Proof, *, status: str) -> dict[str, Any]:
-    data = json.loads(proof.public_json)
-    if not isinstance(data, dict) or data.get("kind") != "bounty_payment":
-        raise HTTPException(status_code=500, detail="invalid proof payload")
-    return {
-        "status": status,
-        "bounty_id": proof.bounty_id,
-        "to_account": data.get("to_account"),
-        "submission_id": proof.submission_id,
-        "submission_url": data.get("submission_url"),
-        "ledger_sequence": proof.ledger_sequence,
-        "ledger_url": f"/ledger/{proof.ledger_sequence}",
-        "proof_hash": proof.hash,
-        "proof_url": f"/proofs/{proof.hash}",
-    }
-
-
-def _existing_payout_proof_for_submission(
-    session: Session, bounty_id: int, submission_url: str
-) -> Proof | None:
-    submission = session.scalar(
-        select(Submission)
-        .where(Submission.bounty_id == bounty_id, Submission.url == submission_url)
-        .limit(1)
-    )
-    if submission is None:
-        return None
-    return session.scalar(
-        select(Proof)
-        .where(Proof.submission_id == submission.id, Proof.kind == "bounty_payment")
-        .limit(1)
-    )
-
-
 def _host_without_port(request: Request) -> str:
     return request.headers.get("host", "").split(":", 1)[0].lower()
 
@@ -278,13 +244,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
@@ -881,18 +851,18 @@ async def api_pay_bounty(
                 proof_payload = json.loads(proof.public_json)
             except LedgerError as exc:
                 if str(exc) == "submission already paid":
-                    existing_proof = _existing_payout_proof_for_submission(
+                    existing_proof = existing_payout_proof_for_submission(
                         session, bounty_id, clean_submission_url
                     )
                     if existing_proof is not None:
                         return JSONResponse(
                             status_code=409,
-                            content=_payout_response_from_proof(
+                            content=payout_response_from_proof(
                                 existing_proof, status="already_paid"
                             ),
                         )
                 raise HTTPException(status_code=400, detail=str(exc)) from exc
-            payout_response = _payout_response_from_proof(proof, status="paid")
+            payout_response = payout_response_from_proof(proof, status="paid")
             payout_response.update(
                 {
                     "bounty_status": bounty_state["status"],

app/payouts.py

@@ -0,0 +1,44 @@
+from __future__ import annotations
+
+import json
+from typing import Any
+
+from fastapi import HTTPException
+from sqlalchemy import select
+from sqlalchemy.orm import Session
+
+from app.models import Proof, Submission
+
+
+def payout_response_from_proof(proof: Proof, *, status: str) -> dict[str, Any]:
+    data = json.loads(proof.public_json)
+    if not isinstance(data, dict) or data.get("kind") != "bounty_payment":
+        raise HTTPException(status_code=500, detail="invalid proof payload")
+    return {
+        "status": status,
+        "bounty_id": proof.bounty_id,
+        "to_account": data.get("to_account"),
+        "submission_id": proof.submission_id,
+        "submission_url": data.get("submission_url"),
+        "ledger_sequence": proof.ledger_sequence,
+        "ledger_url": f"/ledger/{proof.ledger_sequence}",
+        "proof_hash": proof.hash,
+        "proof_url": f"/proofs/{proof.hash}",
+    }
+
+
+def existing_payout_proof_for_submission(
+    session: Session, bounty_id: int, submission_url: str
+) -> Proof | None:
+    submission = session.scalar(
+        select(Submission)
+        .where(Submission.bounty_id == bounty_id, Submission.url == submission_url)
+        .limit(1)
+    )
+    if submission is None:
+        return None
+    return session.scalar(
+        select(Proof)
+        .where(Proof.submission_id == submission.id, Proof.kind == "bounty_payment")
+        .limit(1)
+    )

tests/test_payouts.py

@@ -0,0 +1,75 @@
+from __future__ import annotations
+
+from app.db import create_schema, session_scope
+from app.ledger.service import create_bounty, ensure_genesis, pay_bounty
+from app.payouts import existing_payout_proof_for_submission, payout_response_from_proof
+
+
+def test_payout_response_from_proof_shapes_admin_api_payload(sqlite_url: str) -> None:
+    create_schema(sqlite_url)
+    with session_scope(sqlite_url) as session:
+        ensure_genesis(session)
+        bounty = create_bounty(
+            session,
+            repo="ramimbo/mergework",
+            issue_number=320,
+            issue_url="https://github.com/ramimbo/mergework/issues/320",
+            title="Payout helper extraction",
+            reward_mrwk="15",
+            acceptance="Payout API response helpers should be independently testable.",
+        )
+        proof = pay_bounty(
+            session,
+            bounty_id=bounty.id,
+            to_account="github:alice",
+            submission_url="https://github.com/ramimbo/mergework/pull/320",
+            accepted_by="maintainer",
+            verifier_result={"label": "mrwk:accepted"},
+        )
+
+        payload = payout_response_from_proof(proof, status="paid")
+
+    assert payload == {
+        "status": "paid",
+        "bounty_id": bounty.id,
+        "to_account": "github:alice",
+        "submission_id": proof.submission_id,
+        "submission_url": "https://github.com/ramimbo/mergework/pull/320",
+        "ledger_sequence": proof.ledger_sequence,
+        "ledger_url": f"/ledger/{proof.ledger_sequence}",
+        "proof_hash": proof.hash,
+        "proof_url": f"/proofs/{proof.hash}",
+    }
+
+
+def test_existing_payout_proof_for_submission_finds_duplicate_proof(sqlite_url: str) -> None:
+    create_schema(sqlite_url)
+    submission_url = "https://github.com/ramimbo/mergework/pull/321"
+    with session_scope(sqlite_url) as session:
+        ensure_genesis(session)
+        bounty = create_bounty(
+            session,
+            repo="ramimbo/mergework",
+            issue_number=321,
+            issue_url="https://github.com/ramimbo/mergework/issues/321",
+            title="Duplicate payout helper",
+            reward_mrwk="15",
+            acceptance="Duplicate payout lookup should find the existing proof.",
+        )
+        proof = pay_bounty(
+            session,
+            bounty_id=bounty.id,
+            to_account="github:alice",
+            submission_url=submission_url,
+            accepted_by="maintainer",
+            verifier_result={"label": "mrwk:accepted"},
+        )
+
+        found = existing_payout_proof_for_submission(session, bounty.id, submission_url)
+        missing = existing_payout_proof_for_submission(
+            session, bounty.id, "https://github.com/ramimbo/mergework/pull/999"
+        )
+
+    assert found is not None
+    assert found.hash == proof.hash
+    assert missing is None