From b5018c8645c5266832130d859953130e0849ca27 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 05:53:29 -0500 Subject: [PATCH 1/3] =?UTF-8?q?gitea-saas-core#7:=20delivery=20receipts=20?= =?UTF-8?q?=E2=80=94=20idempotent=20debit=20on=20merge=20+=20client=20lock?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement delivery.py: debit_for_merge (idempotency key deliver:repo:pr:sha), on_merge_webhook (tier rate from config/env, receipt comment, floor/overdue client lock), and a server-mountable handle_merge_webhook (X-API-Key auth, tolerant Gitea payload parsing). Injectable core/gitea/intake seams; tests hermetic with FakeGitea + tmp SQLite. --- gitea_saas/delivery.py | 300 +++++++++++++++++++++++++++++++++++++++++ tests/test_delivery.py | 291 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 591 insertions(+) create mode 100644 gitea_saas/delivery.py create mode 100644 tests/test_delivery.py diff --git a/gitea_saas/delivery.py b/gitea_saas/delivery.py new file mode 100644 index 0000000..34db08a --- /dev/null +++ b/gitea_saas/delivery.py @@ -0,0 +1,300 @@ +"""Delivery receipts (issue #7): idempotent debit on merge + client lock. + +Money half of the delivery loop. On a merged PR in a client repo this +module: + +* debits the client's ledger exactly once -- the idempotency key is derived + from ``repo`` + ``pr_number`` + ``head_sha``, so duplicate webhook + deliveries can never double-apply a debit or double-post a receipt; +* posts a delivery receipt comment on the PR/issue ("Delivered ✓ ..."); +* locks the client (future intake queues instead of shipping) when the + balance after the debit falls below the debit floor, or when the debit + would push the balance negative (``overdue``) -- the delivered work + stands, new work waits. + +Everything is seam-injectable (``core`` = billing core, ``gitea`` = comment +posting seam, ``intake`` = intake factory / lock seam) so tests are fully +hermetic: FakeGitea + tmp-DB, no network, no real accounts. + +The real GiteaClient/config/intake_factory modules land in sibling issues +(#4/#5/#6); this module only depends on duck-typed seams so it does not +need those files to exist yet. + +Remote wiring: ``handle_merge_webhook(payload, provided_key)`` returns a +``(result_dict, http_status)`` pair (401 without a valid X-API-Key, 400 for +a malformed payload, 200 otherwise) -- the server.py webhook route +(issue #6's file, not touched here) just mounts it. +""" + +from __future__ import annotations + +import json +import os +from collections.abc import Mapping +from typing import Any, Optional + +# --- defaults (overridable via env, mirror issue #6's config.py) ---------- + +DEFAULT_TIERS: dict[str, dict[str, Any]] = { + "gold": {"rate_minor": 100}, # $1.00 / issue + "standard": {"rate_minor": 25}, # $0.25 / issue + "free": {"rate_minor": 0}, # never debited +} +DEFAULT_RATE_MINOR = 100 # SAAS_RATE_MINOR (100 = $1.00 / issue) +DEFAULT_FLOOR_MINOR = 500 # SAAS_DEBIT_FLOOR_MINOR ($5.00; matches issue #4) + +KEY_PREFIX = "deliver" + +# Error codes (structured, mirror billing-core conventions). +UNAUTHORIZED = "UNAUTHORIZED" +BAD_PAYLOAD = "BAD_PAYLOAD" +UNKNOWN_CLIENT = "UNKNOWN_CLIENT" +INTERNAL = "INTERNAL" + + +def format_minor(amount_minor: Optional[int]) -> str: + """Render an integer minor-unit amount as ``$X.YY`` (100 -> "$1.00").""" + if amount_minor is None: + amount = 0 + else: + try: + amount = max(int(amount_minor), 0) + except (TypeError, ValueError): + amount = 0 + return f"${amount // 100}.{amount % 100:02d}" + + +def _int_env(env: Mapping[str, str], key: str, default: int) -> int: + try: + return int(env.get(key, "")) + except (TypeError, ValueError): + return default + + +def _parse_tiers(env: Mapping[str, str]) -> dict[str, dict[str, Any]]: + """Tiers from ``SAAS_TIERS_JSON`` merged over defaults (config, not code).""" + tiers = {tier: dict(cfg) for tier, cfg in DEFAULT_TIERS.items()} + raw = env.get("SAAS_TIERS_JSON") + if raw: + try: + overrides = json.loads(raw) + except ValueError: + overrides = None + if isinstance(overrides, dict): + for tier, cfg in overrides.items(): + if isinstance(cfg, dict): + tiers[tier] = {**tiers.get(tier, {}), **cfg} + return tiers + + +def default_config(env: Optional[Mapping[str, str]] = None) -> dict[str, Any]: + """Delivery config. ``env`` is an explicit mapping in tests; production + reads the process environment. + """ + source: Mapping[str, str] = os.environ if env is None else env + return { + "tiers": _parse_tiers(source), + "rate_minor": _int_env(source, "SAAS_RATE_MINOR", DEFAULT_RATE_MINOR), + "floor_minor": _int_env(source, "SAAS_DEBIT_FLOOR_MINOR", DEFAULT_FLOOR_MINOR), + "api_key": source.get("SAAS_API_KEY"), + } + + +def _deliver_key(repo: str, pr_number: int, head_sha: str) -> str: + return f"{KEY_PREFIX}:{repo}:{pr_number}:{head_sha}" + + +class Delivery: + """Delivery-receipt flow, wired to injectable seams.""" + + def __init__(self, core, gitea=None, intake=None, + config: Optional[dict[str, Any]] = None): + self.core = core + self.gitea = gitea # seam: post_comment(repo, number, body) + self.intake = intake # seam: lock(client_id) + self.config = dict(config) if config is not None else default_config() + + # ------------------------------------------------------------------ + # Auth (mounted by the server.py webhook route) + # ------------------------------------------------------------------ + + def authorized(self, provided_key: Optional[str]) -> bool: + """Fail-closed: a request is authorized only when the configured + ``SAAS_API_KEY`` is set and matches the provided key exactly.""" + expected = self.config.get("api_key") + if not expected: + return False + return bool(provided_key) and provided_key == expected + + # ------------------------------------------------------------------ + # The debit primitive + # ------------------------------------------------------------------ + + def debit_for_merge(self, client_id: int, repo: str, pr_number: int, + head_sha: str, rate_minor: int) -> dict[str, Any]: + """Idempotent debit for a merged PR. Returns ``{ok, balance, ...}``. + + Idempotency key = ``deliver:::`` -- replaying the + same key (duplicate webhook delivery) is a no-op in the billing core + and returns ``replayed: True`` with the existing balance. + """ + key = _deliver_key(repo, pr_number, head_sha) + ok, details = self.core.debit( + client_id, rate_minor, key, + ref_type="pr", ref_id=f"{repo}#{pr_number}") + result: dict[str, Any] = {"ok": ok, "balance": details.get("balance"), + "idempotency_key": key} + result.update(details) + return result + + # ------------------------------------------------------------------ + # Receipt comment + lock + # ------------------------------------------------------------------ + + @staticmethod + def _receipt_comment(debit: dict[str, Any], balance: Optional[int], + overdue: bool) -> str: + rate = debit.get("amount_minor") + if overdue: + return ("Delivered \u2713 \u2014 balance after: " + f"{format_minor(balance)} \u2014 **overdue** " + f"(debit {format_minor(rate)} rejected; work delivered, " + "client locked, new work queues).") + if not rate: + return "Delivered \u2713 \u2014 no charge (tier rate 0)." + return f"Delivered \u2713 \u2014 balance after: {format_minor(balance)}" + + def _post_receipt(self, repo: str, pr_number: int, comment: str) -> None: + if self.gitea is not None: + self.gitea.post_comment(repo, pr_number, comment) + + def _lock_client(self, client_id: int) -> bool: + """Lock the client so intake queues future submits. Returns whether + the lock was applied (False when no intake seam is wired yet).""" + if self.intake is None: + return False + try: + self.intake.lock(client_id) + return True + except Exception: + return False + + # ------------------------------------------------------------------ + # Webhook flow + # ------------------------------------------------------------------ + + def _tier_rate(self, client_id: int) -> tuple[Optional[int], Optional[dict]]: + ok, details = self.core.get_client(client_id) + if not ok: + return None, details.get("error") + tier = (details.get("client") or {}).get("tier") or "free" + tiers = self.config.get("tiers") or {} + rate = (tiers.get(tier) or {}).get("rate_minor") + if rate is None: + rate = self.config.get("rate_minor", DEFAULT_RATE_MINOR) + if not isinstance(rate, int) or isinstance(rate, bool): + return None, {"code": INTERNAL, "message": f"invalid rate_minor for tier {tier!r}"} + return rate, None + + def on_merge_webhook(self, client_id: int, repo: str, pr_number: int, + head_sha: str) -> dict[str, Any]: + """Run the full delivery flow for one merge event. + + Returns a structured dict (never raises). ``ok`` reflects that the + webhook was handled -- a rejected debit does NOT block the merge; it + is recorded as ``overdue`` and the client is locked instead. A + duplicate webhook (same repo/pr/head_sha) is a no-op and posts no + second comment. + """ + rate, err = self._tier_rate(client_id) + if err is not None: + return {"ok": False, "error": err, "comment_posted": False, + "locked": False} + assert rate is not None # err is None => rate resolved + + if rate <= 0: + # Free / zero-rate tier: receipt only, no debit, no lock. + _, bdet = self.core.get_balance(client_id) + balance = bdet.get("balance") + comment = self._receipt_comment({"amount_minor": 0}, balance, False) + self._post_receipt(repo, pr_number, comment) + return {"ok": True, "rate_minor": 0, "balance": balance, + "overdue": False, "replayed": False, "locked": False, + "comment": comment} + + debit = self.debit_for_merge(client_id, repo, pr_number, head_sha, rate) + + if debit.get("replayed"): + # Duplicate webhook delivery of the same merge: already debited + # and receipted -- no second debit, no second comment. + return {"ok": True, "replayed": True, "balance": debit.get("balance"), + "rate_minor": rate, "overdue": False, "locked": False, + "comment": None, "debit": debit} + + balance = debit.get("balance") + overdue = not debit.get("ok") + floor = int(self.config.get("floor_minor", DEFAULT_FLOOR_MINOR)) + below_floor = (not overdue) and balance is not None and balance < floor + + locked = False + if overdue or below_floor: + locked = self._lock_client(client_id) + + comment = self._receipt_comment(debit, balance, overdue) + self._post_receipt(repo, pr_number, comment) + + return {"ok": True, "replayed": False, "balance": balance, + "rate_minor": rate, "overdue": overdue, + "below_floor": below_floor, "locked": locked, + "comment": comment, "debit": debit} + + # ------------------------------------------------------------------ + # Server.py mountable handler (payload parsing + auth) + # ------------------------------------------------------------------ + + def handle_merge_webhook(self, payload: Any, + provided_key: Optional[str] = None + ) -> tuple[dict[str, Any], int]: + """Tolerant webhook entry point returning ``(result, http_status)``. + + Payload shape (proxy mapper / manual POST): + ``{repo, pr, head_sha, client_id}`` + Also tolerant of Gitea's own merged ``pull_request`` event where + ``repository.full_name`` + ``pull_request.number`` / + ``pull_request.head.sha`` are present and ``client_id`` comes from + the mapper. + + Status codes: 401 without a valid key, 400 for malformed/missing + fields, 200 otherwise. + """ + if not self.authorized(provided_key): + return ({"ok": False, "error": {"code": UNAUTHORIZED, + "message": "bad or missing X-API-Key"}}), 401 + if not isinstance(payload, dict): + return ({"ok": False, "error": {"code": BAD_PAYLOAD, + "message": "expected JSON object"}}), 400 + + repo = payload.get("repo") + if not repo and isinstance(payload.get("repository"), dict): + repo = payload["repository"].get("full_name") + pr = payload.get("pr") + if pr is None and isinstance(payload.get("pull_request"), dict): + pr = payload["pull_request"].get("number") + head_sha = payload.get("head_sha") + if not head_sha and isinstance(payload.get("pull_request"), dict): + head_sha = (payload["pull_request"].get("head") or {}).get("sha") + client_id = payload.get("client_id") + + if not repo or pr is None or not head_sha or client_id is None: + return ({"ok": False, "error": {"code": BAD_PAYLOAD, + "message": "payload must include " + "repo, pr, head_sha, " + "client_id"}}), 400 + + try: + result = self.on_merge_webhook(int(client_id), str(repo), + int(pr), str(head_sha)) + except (TypeError, ValueError) as e: + return ({"ok": False, "error": {"code": BAD_PAYLOAD, + "message": f"invalid fields: {e}"}}), 400 + return result, 200 diff --git a/tests/test_delivery.py b/tests/test_delivery.py new file mode 100644 index 0000000..e98115e --- /dev/null +++ b/tests/test_delivery.py @@ -0,0 +1,291 @@ +"""Hermetic tests for delivery receipts (issue #7). + +No network, no real Gitea, no real accounts: FakeGitea + tmp-SQLite-DB only. +Run with: python -m pytest tests/test_delivery.py -q +""" + +import sys +from pathlib import Path + +import pytest + +# The package is a PEP 420 namespace package; make the project root importable +# regardless of how pytest was invoked. +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from gitea_saas.billing_core import BillingCore # noqa: E402 +from gitea_saas.delivery import ( # noqa: E402 + Delivery, + default_config, + format_minor, +) + +REPO = "client-org/widgets" +PR = 42 +HEAD = "abc123deadbeef" + + +class FakeGitea: + """Records comment posts; hermetic stand-in for the real GiteaClient.""" + + def __init__(self): + self.comments = [] + + def post_comment(self, repo, number, body): + self.comments.append({"repo": repo, "number": number, "body": body}) + + +class FakeIntake: + """Records lock calls; hermetic stand-in for issue #4's IntakeFactory.""" + + def __init__(self): + self.locked = [] + + def lock(self, client_id): + self.locked.append(client_id) + + +@pytest.fixture +def core(tmp_path): + core = BillingCore(tmp_path / "billing.db") + # seed a funded client; helpers credit below + return core + + +def make_client(core, name="alice", tier="gold"): + ok, details = core.create_client(name, tier=tier) + assert ok, details + return details["client_id"] + + +def credit(core, client_id, amount_minor): + ok, details = core.credit(client_id, amount_minor, f"seed:{client_id}:{amount_minor}") + assert ok, details + return details["balance"] + + +def make_delivery(core, gitea=None, intake=None, config=None): + return Delivery(core, gitea=gitea, intake=intake, config=config) + + +# --------------------------------------------------------------------------- +# Merge webhook -> one debit at tier rate, balance reflected, receipt posted +# --------------------------------------------------------------------------- + +def test_merge_webhook_one_debit_at_tier_rate_with_receipt(core): + gitea = FakeGitea() + intake = FakeIntake() + client_id = make_client(core, tier="gold") # gold rate = 100 ($1.00) + credit(core, client_id, 10000) # $100.00 + + d = make_delivery(core, gitea=gitea, intake=intake) + result = d.on_merge_webhook(client_id, REPO, PR, HEAD) + + assert result["ok"] is True + assert result["overdue"] is False + assert result["rate_minor"] == 100 + assert result["balance"] == 9900 # 10000 - 100 + assert result["replayed"] is False + # exactly one receipt comment, mentioning balance + delivered check + assert len(gitea.comments) == 1 + assert "Delivered \u2713" in gitea.comments[0]["body"] + assert "$99.00" in gitea.comments[0]["body"] + # debit recorded in ledger with the delivery idempotency key + pr ref + ok, ledger = core.list_ledger(client_id) + assert ok + debits = [e for e in ledger["entries"] if e["kind"] == "debit"] + assert len(debits) == 1 + assert debits[0]["idempotency_key"] == f"deliver:{REPO}:{PR}:{HEAD}" + assert debits[0]["ref_type"] == "pr" + assert debits[0]["ref_id"] == f"{REPO}#{PR}" + # balance above floor -> client NOT locked + assert intake.locked == [] + + +# --------------------------------------------------------------------------- +# Duplicate webhook -> no second debit, no second comment +# --------------------------------------------------------------------------- + +def test_duplicate_webhook_no_second_debit_no_second_comment(core): + gitea = FakeGitea() + intake = FakeIntake() + client_id = make_client(core, tier="gold") + credit(core, client_id, 10000) + + d = make_delivery(core, gitea=gitea, intake=intake) + first = d.on_merge_webhook(client_id, REPO, PR, HEAD) + second = d.on_merge_webhook(client_id, REPO, PR, HEAD) # duplicate delivery + + assert first["ok"] is True + assert second["ok"] is True + assert second["replayed"] is True + assert second["balance"] == first["balance"] == 9900 + # exactly one debit row, one comment + ok, ledger = core.list_ledger(client_id) + assert ok + debits = [e for e in ledger["entries"] if e["kind"] == "debit"] + assert len(debits) == 1 + assert len(gitea.comments) == 1 + assert intake.locked == [] + + +# --------------------------------------------------------------------------- +# Balance below floor after debit -> client locked (future intake queues) +# --------------------------------------------------------------------------- + +def test_balance_below_floor_locks_client(core): + gitea = FakeGitea() + intake = FakeIntake() + client_id = make_client(core, tier="gold") + credit(core, client_id, 550) # $5.50; after $1.00 debit -> $4.50 < $5.00 floor + + d = make_delivery(core, gitea=gitea, intake=intake) + result = d.on_merge_webhook(client_id, REPO, PR, HEAD) + + assert result["ok"] is True + assert result["below_floor"] is True + assert result["locked"] is True + assert intake.locked == [client_id] + assert result["balance"] == 450 + assert len(gitea.comments) == 1 + + +# --------------------------------------------------------------------------- +# Debit would go negative -> merge NOT blocked, overdue noted, client locked +# --------------------------------------------------------------------------- + +def test_negative_debit_does_not_block_merge_overdue_and_lock(core): + gitea = FakeGitea() + intake = FakeIntake() + client_id = make_client(core, tier="gold") + credit(core, client_id, 50) # $0.50 < $1.00 debit -> would go negative + + d = make_delivery(core, gitea=gitea, intake=intake) + result = d.on_merge_webhook(client_id, REPO, PR, HEAD) + + # merge not blocked: webhook reports handled ok + assert result["ok"] is True + assert result["overdue"] is True + assert result["debit"]["ok"] is False # debit itself rejected + assert result["balance"] is None + assert intake.locked == [client_id] # client locked for new work + # receipt comment records overdue + assert len(gitea.comments) == 1 + assert "overdue" in gitea.comments[0]["body"] + # no debit row was written (fail-closed core wrote nothing) + ok, ledger = core.list_ledger(client_id) + assert ok + debits = [e for e in ledger["entries"] if e["kind"] == "debit"] + assert debits == [] + # balance unchanged + _, bal = core.get_balance(client_id) + assert bal["balance"] == 50 + + +# --------------------------------------------------------------------------- +# Webhook without auth -> 401 +# --------------------------------------------------------------------------- + +def test_webhook_without_auth_401(core): + client_id = make_client(core) + credit(core, client_id, 10000) + d = make_delivery(core, config=default_config({"SAAS_API_KEY": "sekret"})) + payload = {"repo": REPO, "pr": PR, "head_sha": HEAD, "client_id": client_id} + + result, status = d.handle_merge_webhook(payload, provided_key=None) + assert status == 401 + assert result["error"]["code"] == "UNAUTHORIZED" + + result, status = d.handle_merge_webhook(payload, provided_key="wrong") + assert status == 401 + + +def test_webhook_with_auth_ok(core): + gitea = FakeGitea() + client_id = make_client(core, tier="gold") + credit(core, client_id, 10000) + d = make_delivery(core, gitea=gitea, + config=default_config({"SAAS_API_KEY": "sekret"})) + payload = {"repo": REPO, "pr": PR, "head_sha": HEAD, "client_id": client_id} + + result, status = d.handle_merge_webhook(payload, provided_key="sekret") + assert status == 200 + assert result["ok"] is True + assert result["balance"] == 9900 + assert len(gitea.comments) == 1 + + +def test_webhook_bad_payload_400(core): + d = make_delivery(core, config=default_config({"SAAS_API_KEY": "sekret"})) + # missing client_id + result, status = d.handle_merge_webhook( + {"repo": REPO, "pr": PR, "head_sha": HEAD}, provided_key="sekret") + assert status == 400 + assert result["error"]["code"] == "BAD_PAYLOAD" + # non-dict payload + result, status = d.handle_merge_webhook(["nope"], provided_key="sekret") + assert status == 400 + + +def test_webhook_tolerant_gitea_shape(core): + """Gitea's own pull_request event shape is tolerated (full_name+number).""" + gitea = FakeGitea() + client_id = make_client(core, tier="gold") + credit(core, client_id, 10000) + d = make_delivery(core, gitea=gitea, + config=default_config({"SAAS_API_KEY": "sekret"})) + payload = { + "action": "closed", + "repository": {"full_name": REPO}, + "pull_request": {"number": PR, "merged": True, + "head": {"sha": HEAD}}, + "client_id": client_id, + } + result, status = d.handle_merge_webhook(payload, provided_key="sekret") + assert status == 200 + assert result["ok"] is True + assert result["balance"] == 9900 + assert len(gitea.comments) == 1 + + +# --------------------------------------------------------------------------- +# Tier rate from config (not hardcoded) + SAAS_TIERS_JSON env override +# --------------------------------------------------------------------------- + +def test_rate_from_tier_config_override(core): + gitea = FakeGitea() + client_id = make_client(core, tier="standard") # standard default 25 + credit(core, client_id, 10000) + config = default_config({"SAAS_TIERS_JSON": '{"standard": {"rate_minor": 77}}'}) + d = make_delivery(core, gitea=gitea, config=config) + result = d.on_merge_webhook(client_id, REPO, PR, HEAD) + assert result["rate_minor"] == 77 + assert result["balance"] == 10000 - 77 + + +def test_zero_rate_tier_no_debit(core): + gitea = FakeGitea() + intake = FakeIntake() + client_id = make_client(core, tier="free") # free rate = 0 + credit(core, client_id, 10000) + d = make_delivery(core, gitea=gitea, intake=intake) + result = d.on_merge_webhook(client_id, REPO, PR, HEAD) + assert result["ok"] is True + assert result["rate_minor"] == 0 + assert result["balance"] == 10000 # no debit + assert intake.locked == [] # never locked on rate-0 + assert "no charge" in gitea.comments[0]["body"] + ok, ledger = core.list_ledger(client_id) + assert ok + assert [e for e in ledger["entries"] if e["kind"] == "debit"] == [] + + +# --------------------------------------------------------------------------- +# format_minor helper +# --------------------------------------------------------------------------- + +def test_format_minor(): + assert format_minor(100) == "$1.00" + assert format_minor(0) == "$0.00" + assert format_minor(450) == "$4.50" + assert format_minor(None) == "$0.00" -- 2.43.0 From 80958ef9b924ca0ea480ccb771d93bd1fbcc8590 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 06:40:41 -0500 Subject: [PATCH 2/3] gitea-saas-core#7: constant-time API-key check in authorized() (bounce fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the == string comparison at delivery.py:127 with hmac.compare_digest so the shared secret never leaks through response timing (repo convention: stripe_client.py:88, server.py:171). The isinstance/empty guards run first so compare_digest's TypeError on non-str input is never reached — unset config, None, and non-str keys stay fail-closed False. Five direct authorized() tests added (16 delivery tests / 42 full suite green). --- gitea_saas/delivery.py | 17 ++++++++++++++--- tests/test_delivery.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/gitea_saas/delivery.py b/gitea_saas/delivery.py index 34db08a..1fab89d 100644 --- a/gitea_saas/delivery.py +++ b/gitea_saas/delivery.py @@ -28,6 +28,7 @@ a malformed payload, 200 otherwise) -- the server.py webhook route from __future__ import annotations +import hmac import json import os from collections.abc import Mapping @@ -119,12 +120,22 @@ class Delivery: # ------------------------------------------------------------------ def authorized(self, provided_key: Optional[str]) -> bool: - """Fail-closed: a request is authorized only when the configured - ``SAAS_API_KEY`` is set and matches the provided key exactly.""" + """Fail-closed, constant-time: a request is authorized only when + the configured ``SAAS_API_KEY`` is set and matches the provided + key exactly. + + The comparison uses :func:`hmac.compare_digest` (no timing side + channel on the shared secret's prefix). ``compare_digest`` raises + ``TypeError`` on non-str input, so the ``isinstance`` guard runs + first — unset/empty config and non-str/empty keys all return + ``False``. + """ expected = self.config.get("api_key") if not expected: return False - return bool(provided_key) and provided_key == expected + if not isinstance(provided_key, str) or not provided_key: + return False + return hmac.compare_digest(provided_key, expected) # ------------------------------------------------------------------ # The debit primitive diff --git a/tests/test_delivery.py b/tests/test_delivery.py index e98115e..6be23e1 100644 --- a/tests/test_delivery.py +++ b/tests/test_delivery.py @@ -289,3 +289,35 @@ def test_format_minor(): assert format_minor(0) == "$0.00" assert format_minor(450) == "$4.50" assert format_minor(None) == "$0.00" + + +# --------------------------------------------------------------------------- +# authorized(): fail-closed, constant-time API-key check (hmac.compare_digest) +# --------------------------------------------------------------------------- + +def test_authorized_matching_key(): + d = make_delivery(core=None, config={"api_key": "sekret"}) + assert d.authorized("sekret") is True + + +def test_authorized_wrong_key(): + d = make_delivery(core=None, config={"api_key": "sekret"}) + assert d.authorized("sekret-wrong") is False + assert d.authorized("") is False + + +def test_authorized_fail_closed_without_configured_key(): + d = make_delivery(core=None, config=default_config({})) # no SAAS_API_KEY + assert d.authorized("anything") is False + + +def test_authorized_none_provided_key_is_false_not_typeerror(): + d = make_delivery(core=None, config={"api_key": "sekret"}) + assert d.authorized(None) is False + + +def test_authorized_non_str_provided_key_is_false_not_typeerror(): + # hmac.compare_digest raises TypeError on bytes-vs-str; the isinstance + # guard must reject non-str input before the comparison runs. + d = make_delivery(core=None, config={"api_key": "sekret"}) + assert d.authorized(b"sekret") is False # type: ignore[arg-type] -- 2.43.0 From a991d1331722ec971cfce0844444b67f95db1394 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 11:20:55 -0500 Subject: [PATCH 3/3] gitea-saas-core#7: adopt #16 core shape (create_client id key, flat get_client) --- gitea_saas/delivery.py | 2 +- tests/test_delivery.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gitea_saas/delivery.py b/gitea_saas/delivery.py index 1fab89d..81730c6 100644 --- a/gitea_saas/delivery.py +++ b/gitea_saas/delivery.py @@ -198,7 +198,7 @@ class Delivery: ok, details = self.core.get_client(client_id) if not ok: return None, details.get("error") - tier = (details.get("client") or {}).get("tier") or "free" + tier = details.get("tier") or "free" # flat record since #16 (was nested "client") tiers = self.config.get("tiers") or {} rate = (tiers.get(tier) or {}).get("rate_minor") if rate is None: diff --git a/tests/test_delivery.py b/tests/test_delivery.py index 6be23e1..f1e0df3 100644 --- a/tests/test_delivery.py +++ b/tests/test_delivery.py @@ -55,7 +55,7 @@ def core(tmp_path): def make_client(core, name="alice", tier="gold"): ok, details = core.create_client(name, tier=tier) assert ok, details - return details["client_id"] + return details["id"] # record key is "id" since #16 (was "client_id") def credit(core, client_id, amount_minor): -- 2.43.0