From 88997109b3c2008ffbd66a3d6f9b502a4bfd0bfb Mon Sep 17 00:00:00 2001 From: Tu Pham Date: Tue, 26 May 2026 08:51:34 +0700 Subject: [PATCH] Extract admin payout helpers Refs #320 --- app/ledger/service.py | 37 +++++++++------------ app/main.py | 48 ++++++--------------------- app/payouts.py | 44 +++++++++++++++++++++++++ tests/test_payouts.py | 75 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 61 deletions(-) create mode 100644 app/payouts.py create mode 100644 tests/test_payouts.py diff --git a/app/ledger/service.py b/app/ledger/service.py index 3812285..2a40537 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: 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") diff --git a/app/main.py b/app/main.py index 7e7c4f9..cdd6efe 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 @@ -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"], diff --git a/app/payouts.py b/app/payouts.py new file mode 100644 index 0000000..d5e48fa --- /dev/null +++ b/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) + ) diff --git a/tests/test_payouts.py b/tests/test_payouts.py new file mode 100644 index 0000000..983269a --- /dev/null +++ b/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