boban
/
CluPilotCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Put in the key everything the call actually sends 

A key says "I already sent this call", not "this is what the object is". The
catalogue calls sent metadata that the key knew nothing about, so adding the
tax_treatment field in 9da1358 poisoned yesterday's key for a day — and
AddonPrices::ensure() runs inside a customer's module booking, not only in the
sweep.

createPrice and createProduct now fold a fingerprint of their parameters into
the key. refund, cancelSubscription, addSubscriptionItem and the checkout
deliberately do not: there a second object is the customer's money taken twice,
and Stripe's refusal is the thing worth keeping.

The fake could not see any of this. Its ledger replayed the first id for a
repeated key without ever comparing what the second call asked for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feature/betriebsmodus^2
nexxo 2026-07-30 10:30:32 +02:00
parent 0ec5d93da8
commit 60e93aafd7
4 changed files with 301 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
app/Services/Stripe/FakeStripeClient.php
View File

@ -19,7 +19,7 @@ class FakeStripeClient implements StripeClient
/** @var array<int, array{name: string, metadata: array}> */
public array $products = [];
/** @var array<int, array{product: string, amount: int, currency: string, interval: string, metadata: array}> */
/** @var array<int, array{product: string, amount: int, currency: string, interval: string, metadata: array, created: int}> */
public array $prices = [];
/** @var array<int, string> */
@ -34,7 +34,17 @@ class FakeStripeClient implements StripeClient
*/
public array $activated = [];
/** Idempotency key → the id first returned for it. @var array<string, string> */
/**
* Idempotency key → what was first answered for it, and a fingerprint of
* what it was first sent with.
*
* The fingerprint is the half that was missing. This ledger replayed the
* first id for a repeated key without ever looking at what the second call
* was asking for — so it was unrealistic in precisely the way that mattered,
* and no test could see the 2026-07-29 blockade.
*
* @var array<string, array{id: string, fingerprint: string}>
*/
public array $keys = [];
/**
@ -150,17 +160,20 @@ class FakeStripeClient implements StripeClient
public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string
{
$key = IdempotencyKey::forProduct($idempotencyKey, $name, $metadata);
$parameters = ['name' => $name, 'metadata' => $metadata];
// Replays the first answer for a repeated key, as Stripe does.
if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) {
return $this->keys[$idempotencyKey];
$replayed = $this->replay($key, $parameters);
if ($replayed !== null) {
return $replayed;
}
$id = 'prod_'.substr(sha1($name.count($this->products)), 0, 12);
$this->products[$id] = ['name' => $name, 'metadata' => $metadata];
if ($idempotencyKey !== null) {
$this->keys[$idempotencyKey] = $id;
}
$this->rememberKey($key, $id, $parameters);
return $id;
}
@ -173,8 +186,19 @@ class FakeStripeClient implements StripeClient
array $metadata = [],
?string $idempotencyKey = null,
): string {
if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) {
return $this->keys[$idempotencyKey];
$key = IdempotencyKey::forPrice($idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata);
$parameters = [
'product' => $productId,
'unit_amount' => $amountCents,
'currency' => strtolower($currency),
'interval' => $interval,
'metadata' => $metadata,
];
$replayed = $this->replay($key, $parameters);
if ($replayed !== null) {
return $replayed;
}
$id = 'price_'.substr(sha1($productId.$amountCents.$interval.count($this->prices)), 0, 12);
@ -184,11 +208,13 @@ class FakeStripeClient implements StripeClient
'currency' => $currency,
'interval' => $interval,
'metadata' => $metadata,
// Stripe stamps every object with its creation time and
// AdoptStripePrice takes the OLDEST of several orphans. A counter
// is enough and beats a clock: it cannot tie.
'created' => count($this->prices) + 1,
];
if ($idempotencyKey !== null) {
$this->keys[$idempotencyKey] = $id;
}
$this->rememberKey($key, $id, $parameters);
return $id;
}
@ -308,9 +334,12 @@ class FakeStripeClient implements StripeClient
throw new RuntimeException("Unknown cancellation timing: {$when}");
}
// Replays the first answer for a repeated key, as Stripe does, so a retry
// after a timeout records one order to stop rather than two.
if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) {
// No fingerprint: for a subscription cancellation, a changed parameter
// under a used key is exactly what must fail loudly. See IdempotencyKey.
$parameters = ['subscription' => $subscriptionId, 'when' => $when];
$replayed = $this->replay($idempotencyKey, $parameters);
if ($replayed !== null) {
return;
}
@ -320,9 +349,7 @@ class FakeStripeClient implements StripeClient
'key' => $idempotencyKey,
];
if ($idempotencyKey !== null) {
$this->keys[$idempotencyKey] = $subscriptionId;
}
$this->rememberKey($idempotencyKey, $subscriptionId, $parameters);
}
public function refund(
@ -332,10 +359,13 @@ class FakeStripeClient implements StripeClient
): string {
$this->failIfAsked();
// Replays the first answer for a repeated key, as Stripe does — which
// is the whole point of sending one on a refund.
if ($idempotencyKey !== null && isset($this->keys[$idempotencyKey])) {
return $this->keys[$idempotencyKey];
// No fingerprint: for a refund, a changed parameter under a used key is
// exactly what must fail loudly. See IdempotencyKey.
$parameters = ['payment' => $paymentReference, 'amount' => (string) $amountCents];
$replayed = $this->replay($idempotencyKey, $parameters);
if ($replayed !== null) {
return $replayed;
}
$this->refunds[] = [
@ -346,9 +376,7 @@ class FakeStripeClient implements StripeClient
$id = 're_'.substr(sha1($paymentReference.count($this->refunds)), 0, 12);
if ($idempotencyKey !== null) {
$this->keys[$idempotencyKey] = $id;
}
$this->rememberKey($idempotencyKey, $id, $parameters);
return $id;
}
@ -382,4 +410,33 @@ class FakeStripeClient implements StripeClient
throw new RuntimeException($this->failWith);
}
}
/**
* The id Stripe already answered for this key, or null when it is new.
*
* Throws on a key that comes back with different parameters, which is what
* Stripe does — with this wording — and what makes a test able to see the
* failure at all.
*/
private function replay(?string $key, array $parameters): ?string
{
if ($key === null || ! isset($this->keys[$key])) {
return null;
}
if ($this->keys[$key]['fingerprint'] !== IdempotencyKey::fingerprint($parameters)) {
throw new RuntimeException(
'Keys for idempotent requests can only be used with the same parameters they were first used with.',
);
}
return $this->keys[$key]['id'];
}
private function rememberKey(?string $key, string $id, array $parameters): void
{
if ($key !== null) {
$this->keys[$key] = ['id' => $id, 'fingerprint' => IdempotencyKey::fingerprint($parameters)];
}
}
}

17
app/Services/Stripe/HttpStripeClient.php
View File

@ -86,7 +86,7 @@ class HttpStripeClient implements StripeClient
public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string
{
return (string) $this->request($idempotencyKey)
return (string) $this->request(IdempotencyKey::forProduct($idempotencyKey, $name, $metadata))
->asForm()
->post($this->url('products'), array_filter([
'name' => $name,
@ -104,7 +104,9 @@ class HttpStripeClient implements StripeClient
array $metadata = [],
?string $idempotencyKey = null,
): string {
return (string) $this->request($idempotencyKey)
return (string) $this->request(IdempotencyKey::forPrice(
$idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata,
))
->asForm()
->post($this->url('prices'), [
'product' => $productId,
@ -345,10 +347,13 @@ class HttpStripeClient implements StripeClient
$request = Http::withToken($secret)->acceptJson()->timeout(20);
// Stripe replays the original response for a repeated key instead of
// creating a second object. That covers the gap this cannot close on
// its own: a crash between Stripe creating a Price and us storing its
// id. Their keys expire after 24 hours, so it protects a retry, not a
// sync re-run next week — for which the stored ids are the guard.
// creating a second object — but ONLY for a call repeated exactly; a
// changed parameter under a used key is HTTP 400 for twenty-four hours.
// The catalogue calls therefore fold a fingerprint of their parameters
// into the key (see IdempotencyKey); the money calls deliberately do
// not. Their keys expire after 24 hours, so this protects a retry, not
// a sync re-run next week — for which the stored ids and
// AdoptStripePrice are the guard.
return $idempotencyKey !== null
? $request->withHeaders(['Idempotency-Key' => $idempotencyKey])
: $request;

86
app/Services/Stripe/IdempotencyKey.php Normal file
View File

@ -0,0 +1,86 @@
<?php
namespace App\Services\Stripe;
/**
* The Idempotency-Key header, formed so that it cannot lie.
*
* Stripe replays the first response for a repeated key — but only if the call
* is repeated exactly. A key that comes back with so much as one added metadata
* field is answered with HTTP 400 and stays poisoned for twenty-four hours.
*
* That happened on 2026-07-29: a run created a Price and died before the row was
* written, 9da1358 then added the `tax_treatment` metadata field, and the next
* sweep sent yesterday's key with today's parameters. The sweep was blocked —
* and so was every module booking, because AddonPrices::ensure() runs inside one.
*
* The mistake was conceptual. A key does not state what an object IS; identity
* is the Price's product, amount, currency and interval, and it is
* AdoptStripePrice that recognises those. A key states "I have already sent
* this call", so everything the call sends belongs in it — which is what
* fingerprint() folds in.
*
* **Only for the catalogue calls.** createPrice() and createProduct(): a
* duplicate there is a Stripe object nothing bills on, recoverable, while a
* blockade reaches a paying customer. For refund(), cancelSubscription(),
* addSubscriptionItem() and createCheckoutSession() it is the other way round —
* a second object is the customer's money taken twice or a package billed
* twice — so those keep their bare key and Stripe's refusal with it.
*/
final class IdempotencyKey
{
/** The key for a Price, carrying everything createPrice() puts on the wire. */
public static function forPrice(
?string $key,
string $productId,
int $amountCents,
string $currency,
string $interval,
array $metadata,
): ?string {
return self::with($key, [
'product' => $productId,
'unit_amount' => $amountCents,
'currency' => strtolower($currency),
'interval' => $interval,
'metadata' => $metadata,
]);
}
/** The key for a Product, for the same reason and against the same trap. */
public static function forProduct(?string $key, string $name, array $metadata): ?string
{
return self::with($key, ['name' => $name, 'metadata' => $metadata]);
}
/**
* Eight hex characters over the parameters, canonically ordered.
*
* Ordered, because PHP keeps insertion order and two callers writing the
* same metadata in a different order would otherwise send two keys for one
* call — which would mint two Prices and be a worse bug than the one this
* closes.
*/
public static function fingerprint(array $parameters): string
{
return substr(sha1(self::canonical($parameters)), 0, 8);
}
private static function with(?string $key, array $parameters): ?string
{
// Null stays null: a caller who sends no key wants no replay, and
// inventing one here would change what the call means.
return $key === null ? null : $key.'-'.self::fingerprint($parameters);
}
private static function canonical(array $parameters): string
{
ksort($parameters);
foreach ($parameters as $name => $value) {
$parameters[$name] = is_array($value) ? self::canonical($value) : (string) $value;
}
return json_encode($parameters, JSON_THROW_ON_ERROR);
}
}

122
tests/Feature/Billing/StripeIdempotencyKeyTest.php Normal file
View File

@ -0,0 +1,122 @@
<?php
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\HttpStripeClient;
use Illuminate\Support\Facades\Http;
/**
* A key says "I have already sent this call" — not "this is what the object is".
*
* The difference cost a day. A run created a Price at Stripe and died before the
* row was written; the next sweep sent the SAME key with an added metadata field
* and Stripe refused it for twenty-four hours — the sweep and, because
* AddonPrices::ensure() also runs inside a customer's module booking, the
* booking with it.
*
* So every parameter that goes on the wire goes into the key. On the catalogue
* calls only: for a refund or a second subscription item, Stripe's refusal is
* the thing standing between a customer and being charged twice.
*/
beforeEach(function () {
// The environment fallback is enough — HttpStripeClient reads the vault,
// and the vault falls back to config until a secret has been stored.
config()->set('services.stripe.secret', 'sk_test_plan_task_one');
});
it('sends a different key once the metadata changes', function () {
Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]);
$client = new HttpStripeClient;
$spoken = 'clupilot-addon-price-priority_support-month-3480-EUR';
// The call as it stood before 9da1358, and the call after it: same money,
// same interval, one metadata field more.
$client->createPrice('prod_1', 3480, 'EUR', 'month',
['addon' => 'priority_support'], $spoken);
$client->createPrice('prod_1', 3480, 'EUR', 'month',
['addon' => 'priority_support', 'tax_treatment' => 'domestic'], $spoken);
$sent = collect(Http::recorded())
->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null)
->all();
expect($sent[0])->toStartWith($spoken)
->and($sent[1])->toStartWith($spoken)
->and($sent[1])->not->toBe($sent[0]);
});
it('sends the same key for the very same call', function () {
Http::fake(['api.stripe.com/*' => Http::response(['id' => 'price_x'])]);
$client = new HttpStripeClient;
foreach ([1, 2] as $ignored) {
$client->createPrice('prod_1', 3480, 'EUR', 'month',
['addon' => 'priority_support'], 'clupilot-addon-price');
}
$sent = collect(Http::recorded())
->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null)
->unique()
->all();
expect($sent)->toHaveCount(1);
});
it('fingerprints the product call too, where the same trap was waiting', function () {
Http::fake(['api.stripe.com/*' => Http::response(['id' => 'prod_x'])]);
$client = new HttpStripeClient;
$client->createProduct('Priority Support', ['addon' => 'priority_support'], 'clupilot-addon-product-x');
$client->createProduct('Priority Support', ['addon' => 'priority_support', 'sold_as' => 'entitlement'], 'clupilot-addon-product-x');
$sent = collect(Http::recorded())
->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null)
->all();
expect($sent[1])->not->toBe($sent[0]);
});
it('leaves the money calls their bare key, so Stripe still refuses a changed one', function () {
Http::fake(['api.stripe.com/*' => Http::response(['id' => 'x'])]);
$client = new HttpStripeClient;
$client->refund('pi_1', 500, 'clupilot-refund-7');
$client->cancelSubscription('sub_1', 'at_period_end', 'clupilot-cancel-7');
$client->addSubscriptionItem('sub_1', 'price_1', 1, 'none', 'clupilot-item-7');
$sent = collect(Http::recorded())
->map(fn (array $pair) => $pair[0]->header('Idempotency-Key')[0] ?? null)
->all();
expect($sent)->toBe(['clupilot-refund-7', 'clupilot-cancel-7', 'clupilot-item-7']);
});
it('reproduces the refusal Stripe makes, which the fake used to swallow', function () {
$fake = new FakeStripeClient;
$fake->refund('pi_1', 500, 'clupilot-refund-7');
// Same key, different amount. Stripe answers 400; the fake said nothing and
// replayed the first refund's id, which is how a test could pass over the
// very failure that stopped production.
expect(fn () => $fake->refund('pi_1', 900, 'clupilot-refund-7'))
->toThrow(RuntimeException::class, 'same parameters');
});
it('mints a second price rather than blocking when the metadata moved', function () {
$fake = new FakeStripeClient;
$first = $fake->createPrice('prod_1', 3480, 'EUR', 'month',
['addon' => 'priority_support'], 'clupilot-addon-price');
$second = $fake->createPrice('prod_1', 3480, 'EUR', 'month',
['addon' => 'priority_support', 'tax_treatment' => 'domestic'], 'clupilot-addon-price');
// Two objects, no exception. That the second one is not WANTED is the job of
// AdoptStripePrice, not of the key — see StripePriceAdoptionTest.
expect($second)->not->toBe($first);
});