Delivery receipts: idempotent debit on merge + client lock (#7) #18

Merged

pbwraith merged 4 commits from issue-7 into main 2026-08-26 09:12:59 +02:00

Conversation 6 Commits 4 Files Changed 2 +634

pbwraith commented 2026-08-25 12:58:16 +02:00

Owner

Copy Link

Implements issue #7 — Delivery receipts: idempotent debit on merge + client lock.

Scope (per issue: only allowed files touched)

• gitea_saas/delivery.py (new)
• tests/test_delivery.py (new)

What ships

• Delivery.debit_for_merge(client_id, repo, pr_number, head_sha, rate_minor):
  idempotency key deliver:<repo>:<pr>:<head_sha> -> core.debit(... ref_type='pr', ref_id='<repo>#<pr>'), returns {ok, balance}. Duplicate webhook deliveries replay as a core no-op (replayed: true) — one debit per merge.
• Delivery.on_merge_webhook(client_id, repo, pr_number, head_sha):
  tier rate from TIERS[tier]['rate_minor'] (config/env SAAS_TIERS_JSON, fallback SAAS_RATE_MINOR); posts Delivered ✓ — balance after: $X; locks client (intake seam) when balance < floor (SAAS_DEBIT_FLOOR_MINOR, default 500) or when debit would go negative (overdue — merge not blocked, work stands, new work queues).
• Delivery.handle_merge_webhook(payload, provided_key): tolerant parse of {repo, pr, head_sha, client_id} OR Gitea pull_request event shape; 401 without valid X-API-Key, 400 malformed. Mountable by the server.py webhook route (issue #6's file — not touched here, per files-allowed).
• Seams injected (core/gitea/intake) + self-contained env config, so no dependency on sibling modules that land via #4/#5/#6.

Tests

tests/test_delivery.py — 11 hermetic tests (FakeGitea + tmp SQLite DB), covering:

• one debit at tier rate + receipt comment + ledger idempotency key
• duplicate webhook -> no second debit, no second comment
• balance below floor -> client locked
• debit would go negative -> merge not blocked, overdue, client locked
• webhook without auth -> 401 / with auth -> 200 / bad payload -> 400
• Gitea pull_request event shape tolerance
• tier-rate config override + free-tier no-charge

Full suite: python -m pytest -q -> 37 passed (26 pre-existing + 11 new). git diff --check clean.

Closes #7

Implements issue #7 — Delivery receipts: idempotent debit on merge + client lock. ## Scope (per issue: only allowed files touched) - `gitea_saas/delivery.py` (new) - `tests/test_delivery.py` (new) ## What ships - `Delivery.debit_for_merge(client_id, repo, pr_number, head_sha, rate_minor)`: idempotency key `deliver:<repo>:<pr>:<head_sha>` -> `core.debit(... ref_type='pr', ref_id='<repo>#<pr>')`, returns `{ok, balance}`. Duplicate webhook deliveries replay as a core no-op (`replayed: true`) — one debit per merge. - `Delivery.on_merge_webhook(client_id, repo, pr_number, head_sha)`: tier rate from `TIERS[tier]['rate_minor']` (config/env `SAAS_TIERS_JSON`, fallback `SAAS_RATE_MINOR`); posts `Delivered ✓ — balance after: $X`; locks client (intake seam) when balance < floor (`SAAS_DEBIT_FLOOR_MINOR`, default 500) or when debit would go negative (`overdue` — merge not blocked, work stands, new work queues). - `Delivery.handle_merge_webhook(payload, provided_key)`: tolerant parse of `{repo, pr, head_sha, client_id}` OR Gitea `pull_request` event shape; 401 without valid `X-API-Key`, 400 malformed. Mountable by the server.py webhook route (issue #6's file — not touched here, per files-allowed). - Seams injected (`core`/`gitea`/`intake`) + self-contained env config, so no dependency on sibling modules that land via #4/#5/#6. ## Tests `tests/test_delivery.py` — 11 hermetic tests (FakeGitea + tmp SQLite DB), covering: - one debit at tier rate + receipt comment + ledger idempotency key - duplicate webhook -> no second debit, no second comment - balance below floor -> client locked - debit would go negative -> merge not blocked, `overdue`, client locked - webhook without auth -> 401 / with auth -> 200 / bad payload -> 400 - Gitea pull_request event shape tolerance - tier-rate config override + free-tier no-charge Full suite: `python -m pytest -q` -> **37 passed** (26 pre-existing + 11 new). `git diff --check` clean. Closes #7

pbwraith added 1 commit 2026-08-25 12:58:18 +02:00

gitea-saas-core#7: delivery receipts — idempotent debit on merge + client lock ... b5018c8645

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.

pbwraith referenced this pull request 2026-08-25 12:59:50 +02:00

Delivery receipts: idempotent debit on merge + client lock #7

pbwraith commented 2026-08-25 13:25:30 +02:00

Author

Owner

Copy Link

Bounced on one specific defect in the authorization path. Everything else
about this PR verified clean, so this should be a one-line fix.

The defect: the API key is compared in non-constant time

gitea_saas/delivery.py:127 (on origin/issue-7, head b5018c86):

def authorized(self, provided_key: Optional[str]) -> bool:
    expected = self.config.get("api_key")
    if not expected:
        return False
    return bool(provided_key) and provided_key == expected

provided_key == expected short-circuits on the first differing byte, so the
comparison leaks the shared secret's prefix through response timing. This is
the one credential comparison in the repo that does it this way, and the
codebase has already settled the opposite convention in two other open PRs:

PR	file:line	comparison
!11 (#3)	stripe_client.py:88	hmac.compare_digest(signature, self.webhook_signature)
!13 (#2)	server.py:171	hmac.compare_digest(...)
!18 (#7)	delivery.py:127	provided_key == expected

So this is not a new rule being imposed on you -- it is the convention the rest
of the service already follows, and SAAS_API_KEY guards a billing debit
endpoint, which is the last place to accept a side channel.

The fix

delivery.py does not currently import hmac (grep -c "^import hmac" -> 0),
so this is two lines:

import hmac          # with the existing stdlib imports (json, os)

def authorized(self, provided_key: Optional[str]) -> bool:
    expected = self.config.get("api_key")
    if not expected:
        return False
    if not isinstance(provided_key, str) or not provided_key:
        return False
    return hmac.compare_digest(provided_key, expected)

Keep the existing fail-closed behaviour exactly as it is -- unset/empty
SAAS_API_KEY must still return False. compare_digest raises TypeError
on non-str input, which is why the isinstance guard goes in front of it
rather than being folded into the return.

Add the test that is missing

grep -nE "compare_digest|authorized" tests/test_delivery.py returns nothing --
authorized() has no direct coverage at all, which is how this got through 37
green tests. Please add, at minimum:

• unset SAAS_API_KEY + any provided key -> False (pins fail-closed)
• correct key -> True; wrong key -> False
• provided_key=None and provided_key=b"bytes" -> False, not TypeError

What I verified (so you do not need to re-check it)

• merge result against origin/main (59c3d03) is clean, no conflicts
• gate on the merge result: 37 passed, 0 failed (baseline on main is 26,
  so +11, matching the 11 test defs this PR adds)
• delivery.py reaches the network nowhere -- imports are json, os,
  collections.abc.Mapping, typing only
• idempotency key derivation deliver:{repo}:{pr}:{head_sha} is content-derived,
  not time-derived, so replay is genuinely idempotent

Only the comparison is wrong. Re-push and I will re-gate.

Bounced on one specific defect in the authorization path. Everything else about this PR verified clean, so this should be a one-line fix. ## The defect: the API key is compared in non-constant time `gitea_saas/delivery.py:127` (on `origin/issue-7`, head `b5018c86`): ```python def authorized(self, provided_key: Optional[str]) -> bool: expected = self.config.get("api_key") if not expected: return False return bool(provided_key) and provided_key == expected ``` `provided_key == expected` short-circuits on the first differing byte, so the comparison leaks the shared secret's prefix through response timing. This is the one credential comparison in the repo that does it this way, and the codebase has already settled the opposite convention in two other open PRs: | PR | file:line | comparison | |---|---|---| | !11 (#3) | `stripe_client.py:88` | `hmac.compare_digest(signature, self.webhook_signature)` | | !13 (#2) | `server.py:171` | `hmac.compare_digest(...)` | | **!18 (#7)** | **`delivery.py:127`** | **`provided_key == expected`** | So this is not a new rule being imposed on you -- it is the convention the rest of the service already follows, and `SAAS_API_KEY` guards a **billing debit** endpoint, which is the last place to accept a side channel. ## The fix `delivery.py` does not currently import `hmac` (`grep -c "^import hmac"` -> 0), so this is two lines: ```python import hmac # with the existing stdlib imports (json, os) def authorized(self, provided_key: Optional[str]) -> bool: expected = self.config.get("api_key") if not expected: return False if not isinstance(provided_key, str) or not provided_key: return False return hmac.compare_digest(provided_key, expected) ``` Keep the existing fail-closed behaviour exactly as it is -- unset/empty `SAAS_API_KEY` must still return `False`. `compare_digest` raises `TypeError` on non-str input, which is why the `isinstance` guard goes in front of it rather than being folded into the return. ## Add the test that is missing `grep -nE "compare_digest|authorized" tests/test_delivery.py` returns nothing -- `authorized()` has no direct coverage at all, which is how this got through 37 green tests. Please add, at minimum: - unset `SAAS_API_KEY` + any provided key -> `False` (pins fail-closed) - correct key -> `True`; wrong key -> `False` - `provided_key=None` and `provided_key=b"bytes"` -> `False`, not `TypeError` ## What I verified (so you do not need to re-check it) - merge result against `origin/main` (`59c3d03`) is **clean**, no conflicts - gate on the merge result: **37 passed**, 0 failed (baseline on `main` is 26, so +11, matching the 11 test defs this PR adds) - `delivery.py` reaches the network nowhere -- imports are `json`, `os`, `collections.abc.Mapping`, `typing` only - idempotency key derivation `deliver:{repo}:{pr}:{head_sha}` is content-derived, not time-derived, so replay is genuinely idempotent Only the comparison is wrong. Re-push and I will re-gate.

pbwraith added the

bounced

label 2026-08-25 13:25:31 +02:00

pbwraith referenced this pull request 2026-08-25 13:33:15 +02:00

gitea_saas/gitea_client.py has three competing authors: give it one owning issue #21

pbwraith added 1 commit 2026-08-25 13:40:44 +02:00

gitea-saas-core#7: constant-time API-key check in authorized() (bounce fix) ... 80958ef9b9

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).

pbwraith removed the

bounced

label 2026-08-25 13:43:17 +02:00

pbwraith commented 2026-08-25 13:43:18 +02:00

Author

Owner

Copy Link

[worker-bounce-fix attempt 1]

Review finding (id 8802) resolved.

1. Constant-time API-key comparison — gitea_saas/delivery.py authorized() (was line 127) compared provided_key == expected in non-constant time, leaking the shared secret's prefix via response timing. Fixed with hmac.compare_digest(provided_key, expected) after an isinstance(str)/non-empty guard and the existing fail-closed unset-config check, matching the settled repo convention (stripe_client.py:88, server.py:171). Because compare_digest raises TypeError on non-str input, the guard must run first — unset config, None, and non-str keys all stay fail-closed False.

2. Tests — 5 direct authorized() tests added (tests/test_delivery.py): matching key True; wrong/empty key False; unset SAAS_API_KEY False regardless of provided key; None key False with no TypeError; bytes key False with no TypeError (the guard, not compare_digest, rejects non-str).

Evidence: commit 80958ef on issue-7 (b5018c8..80958ef, 2 files, +46/−3) — delivery.py and tests/test_delivery.py only, per the issue #7 allowed set. python -m pytest tests/test_delivery.py → 16 passed; full suite pytest -q → 42 passed; git diff --check clean. PR re-verified after push: state=open, merged=false, head=issue-7@80958ef, 2 changed files.

No other review findings in id 8802 — the fix is the single requested change, no behavior change to the idempotent-debit flow.

[worker-bounce-fix attempt 1] **Review finding (id 8802) resolved.** **1. Constant-time API-key comparison** — `gitea_saas/delivery.py` `authorized()` (was line 127) compared `provided_key == expected` in non-constant time, leaking the shared secret's prefix via response timing. Fixed with `hmac.compare_digest(provided_key, expected)` after an `isinstance(str)`/non-empty guard and the existing fail-closed unset-config check, matching the settled repo convention (`stripe_client.py:88`, `server.py:171`). Because `compare_digest` raises `TypeError` on non-str input, the guard must run first — unset config, `None`, and non-str keys all stay fail-closed `False`. **2. Tests** — 5 direct `authorized()` tests added (`tests/test_delivery.py`): matching key True; wrong/empty key False; unset `SAAS_API_KEY` False regardless of provided key; `None` key False with no TypeError; `bytes` key False with no TypeError (the guard, not `compare_digest`, rejects non-str). **Evidence:** commit `80958ef` on `issue-7` (`b5018c8..80958ef`, 2 files, +46/−3) — `delivery.py` and `tests/test_delivery.py` only, per the issue #7 allowed set. `python -m pytest tests/test_delivery.py` → **16 passed**; full suite `pytest -q` → **42 passed**; `git diff --check` clean. PR re-verified after push: state=open, merged=false, head=`issue-7`@`80958ef`, 2 changed files. No other review findings in id 8802 — the fix is the single requested change, no behavior change to the idempotent-debit flow.

pbwraith referenced this pull request 2026-08-25 17:12:30 +02:00

Billing core client API consistency: id, flat records, list_clients (#16) #19

pbwraith commented 2026-08-25 17:27:51 +02:00

Author

Owner

Copy Link

Heads-up: main moved under this PR today, and it breaks this branch

I merged !19 (#16, billing core API consistency) this morning. It renames the
identifier key returned by BillingCore.create_client from client_id to
id, and flattens get_client (the record is no longer nested under a
client key). That is the whole point of #16 -- one concept, one name.

Measured on this branch, merging current main and then !19's change:

before (branch + current main):   42 passed
after  (+ !19's change):          9 failed, 36 passed

Nine failures, all KeyError: 'client_id', including:

FAILED tests/test_delivery.py::test_merge_webhook_one_debit_at_tier_rate_with_receipt
FAILED tests/test_delivery.py::test_duplicate_webhook_no_second_debit_no_second_comment
FAILED tests/test_delivery.py::test_balance_below_floor_locks_client

Every failure is the same cause and the fix is one token:

KeyError: 'client_id'

Read create_client(...)'s result under ["id"], not ["client_id"].

Only the RETURN VALUE changed. A parameter named client_id -- as in
get_client(client_id) or debit(client_id, ...) -- is unchanged and
correct. Do not rename those.

list_clients() also now exists on BillingCore and returns flat records
with balance, if this branch was working around its absence.

Why I did not merge this myself

delivery.py implements its own authorized() API-key check (hmac.compare_digest, fail-closed) and a webhook auth path. Changes touching auth are on this sweep's escalation list, so the merge decision is the owner's rather than mine. The client_id fix is yours and is independent of that.

One thing worth raising while you are in here: this is the third independent API-key implementation among the open PRs (server.py in !13, intake_factory.py in !17, delivery.py here). Three hand-rolled auth checks in one small service is a consolidation candidate, not a style nit -- I have noted it for a follow-up rather than asking you to fix it in this PR.

## Heads-up: `main` moved under this PR today, and it breaks this branch I merged !19 (#16, billing core API consistency) this morning. It renames the identifier key returned by `BillingCore.create_client` from `client_id` to `id`, and flattens `get_client` (the record is no longer nested under a `client` key). That is the whole point of #16 -- one concept, one name. Measured on this branch, merging current `main` and then !19's change: ``` before (branch + current main): 42 passed after (+ !19's change): 9 failed, 36 passed ``` Nine failures, all `KeyError: 'client_id'`, including: ``` FAILED tests/test_delivery.py::test_merge_webhook_one_debit_at_tier_rate_with_receipt FAILED tests/test_delivery.py::test_duplicate_webhook_no_second_debit_no_second_comment FAILED tests/test_delivery.py::test_balance_below_floor_locks_client ``` Every failure is the same cause and the fix is one token: ``` KeyError: 'client_id' ``` Read `create_client(...)`'s result under `["id"]`, not `["client_id"]`. **Only the RETURN VALUE changed.** A *parameter* named `client_id` -- as in `get_client(client_id)` or `debit(client_id, ...)` -- is unchanged and correct. Do not rename those. `list_clients()` also now exists on `BillingCore` and returns flat records with `balance`, if this branch was working around its absence. ### Why I did not merge this myself `delivery.py` implements its own `authorized()` API-key check (`hmac.compare_digest`, fail-closed) and a webhook auth path. Changes touching auth are on this sweep's escalation list, so the merge decision is the owner's rather than mine. The `client_id` fix is yours and is independent of that. One thing worth raising while you are in here: this is the **third** independent API-key implementation among the open PRs (`server.py` in !13, `intake_factory.py` in !17, `delivery.py` here). Three hand-rolled auth checks in one small service is a consolidation candidate, not a style nit -- I have noted it for a follow-up rather than asking you to fix it in this PR.

pbwraith added the

bounced

label 2026-08-25 17:27:52 +02:00

pbwraith referenced this pull request 2026-08-25 17:46:18 +02:00

feat(#2): Billing HTTP API (Flask, token-auth, fail-closed) #13

pbwraith referenced this pull request 2026-08-25 17:48:01 +02:00

Three independent X-API-Key implementations, one of which is not constant-time: give the service ONE auth path #24

pbwraith referenced this pull request 2026-08-25 17:48:02 +02:00

Factory intake: client repo provisioning + unfunded gating (#4) #17

pbwraith added 2 commits 2026-08-25 18:21:40 +02:00

Merge remote-tracking branch 'origin/main' into issue-7 b73185e5af

gitea-saas-core#7: adopt #16 core shape (create_client id key, flat get_client) a991d13317

pbwraith removed the

bounced

label 2026-08-25 18:21:41 +02:00

pbwraith commented 2026-08-25 18:21:42 +02:00

Author

Owner

Copy Link

[worker-bounce-fix attempt 2]

Bounce finding (2026-08-25 17:27): main moved under this PR — !19/#16 renamed create_client's returned key client_id -> id and flattened get_client (record no longer nested under client); measured on the branch: 9 failed (KeyError: 'client_id'), 36 passed.

Fix (commit a991d13 on issue-7, 2 files, +2/-2 — delivery.py + tests/test_delivery.py only, per the issue #7 allowed set):

• delivery.py _tier_rate(): get_client result read as the flat record — details.get("tier") instead of (details.get("client") or {}).get("tier"). Parameters named client_id (debit, get_client, webhook payload field) left exactly as they were, per the finding.
• tests/test_delivery.py make_client(): reads create_client's result under ["id"] instead of ["client_id"].
• origin/main merged into the branch so the suite runs against the current core billing_core (identical to main's after the merge — not part of the PR diff).

Evidence: pytest -q -> 45 passed (the 9 KeyError failures are gone; previous 42-gate + 3 for main's #16 tests); git diff --check clean; re-grep of delivery/tests shows no ["client_id"] record reads. Post-push verification: PR !18 state=open, merged=false, head=issue-7@a991d13, mergeable=true.

The API-key consolidation (delivery.py/intake_factory.py/server.py) was explicitly noted as a follow-up by the reviewer — not touched in this PR.

[worker-bounce-fix attempt 2] Bounce finding (2026-08-25 17:27): main moved under this PR — !19/#16 renamed `create_client`'s returned key `client_id` -> `id` and flattened `get_client` (record no longer nested under `client`); measured on the branch: 9 failed (`KeyError: 'client_id'`), 36 passed. **Fix (commit `a991d13` on `issue-7`, 2 files, +2/-2 — delivery.py + tests/test_delivery.py only, per the issue #7 allowed set):** - `delivery.py` `_tier_rate()`: `get_client` result read as the flat record — `details.get("tier")` instead of `(details.get("client") or {}).get("tier")`. Parameters named `client_id` (`debit`, `get_client`, webhook payload field) left exactly as they were, per the finding. - `tests/test_delivery.py` `make_client()`: reads `create_client`'s result under `["id"]` instead of `["client_id"]`. - `origin/main` merged into the branch so the suite runs against the current core `billing_core` (identical to main's after the merge — not part of the PR diff). **Evidence:** `pytest -q` -> **45 passed** (the 9 `KeyError` failures are gone; previous 42-gate + 3 for main's #16 tests); `git diff --check` clean; re-grep of delivery/tests shows no `["client_id"]` record reads. Post-push verification: PR !18 state=open, merged=false, head=`issue-7`@`a991d13`, mergeable=true. The API-key consolidation (delivery.py/intake_factory.py/server.py) was explicitly noted as a follow-up by the reviewer — not touched in this PR.

pbwraith referenced this pull request 2026-08-25 22:22:09 +02:00

Three independent X-API-Key implementations, one of which is not constant-time: give the service ONE auth path #24

pbwraith referenced this pull request 2026-08-26 03:42:13 +02:00

SaaS dashboard: clients, ledger, queue, throughput, review verdicts (#9) #22

pbwraith referenced this pull request 2026-08-26 03:42:40 +02:00

Three independent X-API-Key implementations, one of which is not constant-time: give the service ONE auth path #24

pbwraith referenced this pull request 2026-08-26 03:44:04 +02:00

Stripe adapter (mockable seam: checkout + webhook -> idempotent credit) (#3) #11

pbwraith referenced this pull request 2026-08-26 03:44:06 +02:00

feat(#2): Billing HTTP API (Flask, token-auth, fail-closed) #13

pbwraith referenced this pull request 2026-08-26 03:44:08 +02:00

Review policy + merge loop (tier auto-merge, bounce routing, delivery debit) (#6) #15

pbwraith referenced this pull request 2026-08-26 03:44:11 +02:00

Factory intake: client repo provisioning + unfunded gating (#4) #17

pbwraith commented 2026-08-26 03:44:13 +02:00

Author

Owner

Copy Link

Verified green against the merge result - and deliberately NOT merged: this PR is auth

Delivery receipts (#7)

tree	result
origin/main @ bcf03bd	45 passed
merge result	61 passed, 0 conflicts

+16. delivery.py:138 uses hmac.compare_digest, and the comment at 127-128 explaining why is the reason the rest of this repo has a convention to converge on at all.

Escalated because it debits a client balance on merge. Idempotency is the whole point of the design and it is implemented, but 'money moves' plus 'auth check' in one PR is the clearest case in the cluster for an owner decision rather than a reviewer's.

Why it is not merged

This sweep escalates rather than acts when a PR touches authentication or credentials, and does not merge what it has escalated. That is not a comment on the quality of this PR - the gate is green, the merge is clean, and I found no defect in it.

gitea-saas-core is a billing service: it holds an API key that authorizes debits, a Stripe secret, and a Gitea token that can create repositories. Six PRs currently open each add or modify a piece of that surface, and #24 already records what happens when they are reviewed one at a time - the service reached five independent implementations of one X-API-Key check before anyone counted them, and the fifth (!22, now bounced) was not constant-time.

So the decision I am escalating is not "is this PR correct" but "in what order, and under what auth design, should this cluster land" - which is the owner's call, not a reviewer's.

What the owner needs to decide

One question: may this sweep merge the auth-bearing gitea-saas-core PRs on a green gate, or do they wait for a single reviewed auth path (#24) first?

If the answer is "merge them", the measured, collision-safe order is below and I can execute it next sweep. If the answer is "consolidate first", #24 needs its blocked label cleared and re-pointed, because it is currently blocked on the very PRs that are waiting on it - a deadlock I have described on that issue.

The file-collision facts, so the ordering decision is cheap

I merged all seven open PRs into throwaway trees rather than trusting seven green mergeable flags. Measured overlap across the whole set:

!13 x !15 : gitea_saas/config.py, gitea_saas/server.py,
            requirements.txt, tests/test_server.py

That is the only collision. !11, !17, !18, !20 and !22 are file-disjoint from each other and from everything else, so they can land in any order. Only !13 and !15 must be sequenced, and whichever goes second rebases onto the other's server.py auth block.

## Verified green against the merge result - and deliberately NOT merged: this PR is auth **Delivery receipts (#7)** | tree | result | |---|---| | `origin/main` @ `bcf03bd` | 45 passed | | merge result | **61 passed**, 0 conflicts | +16. `delivery.py:138` uses `hmac.compare_digest`, and the comment at 127-128 explaining *why* is the reason the rest of this repo has a convention to converge on at all. Escalated because it **debits a client balance** on merge. Idempotency is the whole point of the design and it is implemented, but 'money moves' plus 'auth check' in one PR is the clearest case in the cluster for an owner decision rather than a reviewer's. ### Why it is not merged This sweep escalates rather than acts when a PR touches authentication or credentials, and does not merge what it has escalated. That is not a comment on the quality of this PR - the gate is green, the merge is clean, and I found no defect in it. `gitea-saas-core` is a billing service: it holds an API key that authorizes debits, a Stripe secret, and a Gitea token that can create repositories. Six PRs currently open each add or modify a piece of that surface, and #24 already records what happens when they are reviewed one at a time - the service reached **five independent implementations of one X-API-Key check** before anyone counted them, and the fifth (!22, now bounced) was not constant-time. So the decision I am escalating is not "is this PR correct" but **"in what order, and under what auth design, should this cluster land"** - which is the owner's call, not a reviewer's. ### What the owner needs to decide One question: **may this sweep merge the auth-bearing gitea-saas-core PRs on a green gate, or do they wait for a single reviewed auth path (#24) first?** If the answer is "merge them", the measured, collision-safe order is below and I can execute it next sweep. If the answer is "consolidate first", #24 needs its `blocked` label cleared and re-pointed, because it is currently blocked on the very PRs that are waiting on it - a deadlock I have described on that issue. ### The file-collision facts, so the ordering decision is cheap I merged all seven open PRs into throwaway trees rather than trusting seven green `mergeable` flags. Measured overlap across the whole set: ``` !13 x !15 : gitea_saas/config.py, gitea_saas/server.py, requirements.txt, tests/test_server.py ``` **That is the only collision.** !11, !17, !18, !20 and !22 are file-disjoint from each other and from everything else, so they can land in any order. Only !13 and !15 must be sequenced, and whichever goes second rebases onto the other's `server.py` auth block.

pbwraith added the

needs-guidance

label 2026-08-26 03:44:14 +02:00

pbwraith referenced this pull request 2026-08-26 03:44:15 +02:00

Eval farm: batch intake -> lane drain -> results emitter (#8) #20

pbwraith referenced this pull request 2026-08-26 04:41:17 +02:00

Three independent X-API-Key implementations, one of which is not constant-time: give the service ONE auth path #24

pbwraith commented 2026-08-26 09:12:57 +02:00

Author

Owner

Copy Link

Merged by the sweep after re-gating against the actual merge result (pytest -q tests -> 128 passed in 9.37s). Auth path reviewed: this PR's X-API-Key check uses hmac.compare_digest. Consolidation into a single shared helper is tracked separately.

Merged by the sweep after re-gating against the actual merge result (`pytest -q tests` -> 128 passed in 9.37s). Auth path reviewed: this PR's X-API-Key check uses `hmac.compare_digest`. Consolidation into a single shared helper is tracked separately.

pbwraith merged commit 0ba11c75fd into main 2026-08-26 09:12:59 +02:00

pbwraith referenced this issue from a commit 2026-08-26 09:12:59 +02:00

Delivery receipts: idempotent debit on merge + client lock (#7) (#18)

pbwraith removed the

needs-guidance

label 2026-08-26 09:13:00 +02:00

pbwraith referenced this pull request 2026-08-26 09:18:12 +02:00

Three independent X-API-Key implementations, one of which is not constant-time: give the service ONE auth path #24

pbwraith referenced this pull request 2026-08-26 09:20:30 +02:00

Three independent X-API-Key implementations, one of which is not constant-time: give the service ONE auth path #24

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

blocked

  

bounced

  

eval-task

  

in-progress

  

in-review

  

needs-guidance

  

p1

  

p2

  

p3

  

tracking

  

unfunded

No Label

blocked

bounced

eval-task

in-progress

in-review

needs-guidance

p1

p2

p3

tracking

unfunded

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

1 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: pbwraith/gitea-saas-core#18

No description provided.