fix(engine-b): address Codex round 2 (fail-closed webhook, placement lock, clone/admin idempotency)

- Stripe webhook fails closed when the signing secret is missing (outside local/testing).
- ReserveResources places + creates the instance under a per-datacenter lock so
  concurrent orders can't overcommit a host.
- CloneVirtualMachine reserves the vmid + breadcrumb BEFORE cloning, so a crash
  can't mint a new vmid and orphan the first clone.
- CreateCustomerAdmin checks user existence and resets the password instead of
  re-running user:add on retry.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/portal-design
nexxo 2026-07-25 12:09:21 +02:00
parent 42fa92cf20
commit e6bd6c9354
5 changed files with 72 additions and 32 deletions

View File

@ -18,7 +18,11 @@ class StripeWebhookController extends Controller
$payload = $request->getContent();
$secret = (string) config('services.stripe.webhook_secret');
if (filled($secret) && ! $this->signatureValid($payload, (string) $request->header('Stripe-Signature'), $secret)) {
if (blank($secret)) {
// Fail closed: an unconfigured secret must not authorize provisioning
// outside local/testing.
abort_unless(app()->environment('local', 'testing'), 400, 'Stripe webhook secret not configured');
} elseif (! $this->signatureValid($payload, (string) $request->header('Stripe-Signature'), $secret)) {
abort(400, 'invalid signature');
}

View File

@ -32,21 +32,27 @@ class CloneVirtualMachine extends CustomerStep
}
$pve = $this->pve->forHost($instance->host);
$name = 'nc-'.$instance->subdomain;
// Idempotent: vmid already minted → resume polling the clone task.
if ($this->hasResource($run, 'vmid')) {
return $this->pollClone($pve, $node, (string) $run->context('clone_upid'));
// Reserve a deterministic vmid + breadcrumb BEFORE the clone call, so a
// crash never leads a retry to mint a NEW vmid (orphaning the first clone).
if (! $this->hasResource($run, 'vmid')) {
$vmid = $pve->nextVmid();
$instance->update(['vmid' => $vmid, 'status' => 'provisioning']);
$run->mergeContext(['vmid' => $vmid]);
$this->recordResource($run, $instance->host, 'vmid', (string) $vmid);
}
$vmid = $pve->nextVmid();
$upid = $pve->cloneVm($node, $templateVmid, $vmid, 'nc-'.$instance->subdomain);
$vmid = (int) $run->context('vmid');
// [E] persist vmid + task ref BEFORE polling (crash-safe).
$instance->update(['vmid' => $vmid, 'status' => 'provisioning']);
$run->mergeContext(['vmid' => $vmid, 'clone_upid' => $upid]);
$this->recordResource($run, $instance->host, 'vmid', (string) $vmid);
// Clone once, keyed to the reserved vmid; record the task ref immediately.
$upid = $run->context('clone_upid');
if ($upid === null) {
$upid = $pve->cloneVm($node, $templateVmid, $vmid, $name);
$run->mergeContext(['clone_upid' => $upid]);
}
return $this->pollClone($pve, $node, $upid);
return $this->pollClone($pve, $node, (string) $upid);
}
private function pollClone(ProxmoxClient $pve, string $node, string $upid): StepResult

View File

@ -31,15 +31,23 @@ class CreateCustomerAdmin extends CustomerStep
$instance = $this->instance($run);
$pve = $this->pve->forHost($instance->host);
$node = (string) $run->context('node');
$vmid = (int) $run->context('vmid');
$username = 'admin';
$password = Str::random(20);
$occ = 'cd /opt/nextcloud && docker compose exec -T';
// Retry-safe: if a prior crashed attempt already created the user, reset
// its password instead of re-running user:add (which Nextcloud rejects).
$exists = (int) ($pve->guestExec($node, $vmid, $occ.' app php occ user:info '.escapeshellarg($username))['exitcode'] ?? 1) === 0;
$action = $exists
? 'user:resetpassword --password-from-env '.escapeshellarg($username)
: 'user:add --password-from-env --group=admin '.escapeshellarg($username);
// Pass the password via env (OC_PASS), never on the argv/command line.
$this->guest($pve, $run,
'cd /opt/nextcloud && OC_PASS='.escapeshellarg($password).
' docker compose exec -T -e OC_PASS app php occ user:add --password-from-env --group=admin '.
escapeshellarg($username));
$this->guest($pve, $run, 'OC_PASS='.escapeshellarg($password).' '.$occ.' -e OC_PASS app php occ '.$action);
// Persist only the username (encrypted ref); hand the password to step 15
// encrypted-in-context for delivery, then it is scrubbed. Never plaintext.

View File

@ -7,6 +7,7 @@ use App\Models\Instance;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Str;
class ReserveResources extends CustomerStep
@ -34,26 +35,35 @@ class ReserveResources extends CustomerStep
}
$plan = $this->plan($run);
$host = Host::placeableIn($order->datacenter, (int) $plan['quota_gb']);
if ($host === null) {
// Place + create under a per-datacenter lock so concurrent reservations
// can't both pick the same host and overcommit its capacity.
$instance = Cache::lock('placement:'.$order->datacenter, 30)->block(10, function () use ($order, $plan) {
$host = Host::placeableIn($order->datacenter, (int) $plan['quota_gb']);
if ($host === null) {
return null;
}
return Instance::create([
'customer_id' => $order->customer_id,
'order_id' => $order->id,
'host_id' => $host->id,
'plan' => $order->plan,
'quota_gb' => $plan['quota_gb'],
'disk_gb' => $plan['disk_gb'],
'ram_mb' => $plan['ram_mb'],
'cores' => $plan['cores'],
'subdomain' => $this->uniqueSubdomain($order),
'status' => 'reserving',
]);
});
if ($instance === null) {
return StepResult::fail('no_capacity');
}
$instance = Instance::create([
'customer_id' => $order->customer_id,
'order_id' => $order->id,
'host_id' => $host->id,
'plan' => $order->plan,
'quota_gb' => $plan['quota_gb'],
'disk_gb' => $plan['disk_gb'],
'ram_mb' => $plan['ram_mb'],
'cores' => $plan['cores'],
'subdomain' => $this->uniqueSubdomain($order),
'status' => 'reserving',
]);
$this->putContext($run, $instance);
$this->recordResource($run, $host, 'instance_id', (string) $instance->id);
$this->recordResource($run, $instance->host, 'instance_id', (string) $instance->id);
return StepResult::advance();
}

View File

@ -212,11 +212,13 @@ it('configures nextcloud via occ', function () {
// 10. CreateCustomerAdmin
it('creates the admin once, never persisting the password in plaintext', function () {
$s = fakeServices();
$s['pve']->guestScript('user:info', 1); // user does not exist yet → user:add
['run' => $run, 'instance' => $instance] = reservedRun();
expect(app(CreateCustomerAdmin::class)->execute($run)->type)->toBe('advance');
$run->refresh();
expect($instance->fresh()->nc_admin_ref)->toBe('admin')
expect($s['pve']->guestRan('occ user:add'))->toBeTrue()
->and($instance->fresh()->nc_admin_ref)->toBe('admin')
->and($run->context('admin_password'))->not->toBeNull() // present but encrypted
->and(Crypt::decryptString($run->context('admin_password')))->toBeString()
->and(RunResource::where('run_id', $run->id)->where('kind', 'nc_admin')->count())->toBe(1);
@ -231,6 +233,16 @@ it('creates the admin once, never persisting the password in plaintext', functio
expect(count($s['pve']->guestCommands))->toBe($guestCountBefore);
});
it('resets the password when the admin user already exists (retry-safe)', function () {
$s = fakeServices();
$s['pve']->guestScript('user:info', 0); // user already exists → resetpassword
['run' => $run] = reservedRun();
expect(app(CreateCustomerAdmin::class)->execute($run)->type)->toBe('advance')
->and($s['pve']->guestRan('occ user:resetpassword'))->toBeTrue()
->and($s['pve']->guestRan('occ user:add'))->toBeFalse();
});
// 11. ConfigureDnsAndTls
it('creates the dns record, writes the route, and polls for the cert', function () {
$s = fakeServices();