From ba6886c99f68dc82eaaf69c492e4327e157286ab Mon Sep 17 00:00:00 2001
From: Tofik Hasanov <annexcies@gmail.com>
Date: Mon, 13 Jul 2026 22:47:40 -0400
Subject: [PATCH] fix(api): revert deterministic retry billing id (caused
 free-scan on redelivery)

The deterministic pending:retry:<parent> reservation id introduced a worse,
realistic bug: billing consume is idempotent on the id AND a refund leaves the
consume event in place, so a failed-create-then-refund followed by the standard
webhook redelivery would reuse the key, skip the debit, and run the scan for
free. Revert to a unique random reservation id so redelivery re-debits
correctly. Concurrent duplicate SCANS are still deduped by the Maced
idempotency key; the residual concurrent-only over-charge requires concurrent
duplicate webhook delivery (unverified) and is not worth the regression risk of
a shared reservation.
---
 .../security-penetration-tests.service.spec.ts  |  7 ++++---
 .../security-penetration-tests.service.ts       | 17 ++++++++---------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts b/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts
index f04e7ab1d3..adb0b6f144 100644
--- a/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts
+++ b/apps/api/src/security-penetration-tests/security-penetration-tests.service.spec.ts
@@ -1169,15 +1169,16 @@ describe('SecurityPenetrationTestsService', () => {
       // Deterministic idempotency key tied to the parent → Maced dedupes
       // concurrent duplicate failure webhooks into one provider scan.
       expect(getRequestHeader('Idempotency-Key')).toBe('retry:run_orig');
-      // Deterministic billing reservation id (idempotent on sourceResourceId)
-      // so concurrent duplicates debit the allowance exactly once.
+      // Billing reservation uses a unique (non-deterministic) id so a
+      // failed-then-redelivered retry re-debits correctly instead of reusing a
+      // refunded-but-still-recorded consume key (which would run the scan free).
       expect(
         mockBillingEntitlementsService.tryConsumeIncludedUsageForProduct,
       ).toHaveBeenCalledWith(
         expect.objectContaining({
           organizationId: 'org_123',
           productKey: 'pentest',
-          sourceResourceId: 'pending:retry:run_orig',
+          sourceResourceId: expect.not.stringContaining('retry:run_orig'),
         }),
       );
       // The user's briefing survives the round-trip (re-persisted for any
diff --git a/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts b/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts
index bdd49010de..ab4321706c 100644
--- a/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts
+++ b/apps/api/src/security-penetration-tests/security-penetration-tests.service.ts
@@ -354,15 +354,14 @@ export class SecurityPenetrationTestsService {
       organizationId,
       payload,
     );
-    // For a retry, derive a DETERMINISTIC reservation id from the parent run so
-    // concurrent duplicate failure webhooks reserve the same allowance. Billing
-    // consumption is idempotent on this id (billingUsageEvent.idempotencyKey),
-    // so duplicates debit exactly once — otherwise two random ids would debit
-    // twice and only one would land on the shared ownership row, orphaning the
-    // other. User-initiated creates keep a unique random id.
-    const billingUsageSourceId = lineage.retryOfProviderRunId
-      ? `pending:retry:${lineage.retryOfProviderRunId}`
-      : `pending:${randomUUID()}`;
+    // Always a unique reservation id per create call. A deterministic
+    // per-parent id (to dedupe concurrent duplicate retries) is NOT safe here:
+    // the billing consume is idempotent on this id AND a refund leaves the
+    // consume event in place, so after a failed-create-then-refund the next
+    // (redelivered) attempt would reuse the key, skip the debit, and run the
+    // scan for free. Provider-scan de-duplication for concurrent duplicates is
+    // handled by the Maced idempotency key below instead.
+    const billingUsageSourceId = `pending:${randomUUID()}`;
     let consumedSubscriptionAllowance = false;
 
     // Reserve subscription allowance before calling Maced so fast double-clicks