Compare commits

..

2 Commits

Author SHA1 Message Date
nexxo 336ca9d88c Make host onboarding survive the machine it runs on
tests / pest (push) Failing after 7m52s Details
tests / assets (push) Successful in 20s Details
tests / release (push) Has been skipped Details
Nine defects in the host pipeline, every one of which either stopped an
install or let a host reach `active` while it could not do its job.

The install-stoppers:

- InstallProxmoxVe hard-coded `bookworm`. A server ordered today boots
  Debian 13 trixie, so it added the PVE 8 repo and keyring to a trixie
  base. The codename now comes from /etc/os-release, the suite and keyring
  are looked up per release, and an unknown one fails naming what it found
  instead of guessing. Keys land in /usr/share/keyrings with Signed-By
  rather than in the deprecated trusted.gpg.d, where they would vouch for
  every other repository on the machine too.

- keyLogin() dialled wg_ip whenever it was filled, but wg_ip is persisted
  inside the allocation lock BEFORE the handshake is ever proven. Attempt
  1 reserved the address and failed at the handshake; every later attempt,
  including a console Retry, died connecting to a tunnel that did not work
  — before reaching the only code that could repair wg0.conf. Recovery
  meant nulling hosts.wg_ip by hand. SSH now uses the tunnel only once the
  `wg_peer` breadcrumb proves a handshake succeeded, which also rescues
  the same shape in RebootIntoPveKernel, ConfigureProxmox,
  CreateAutomationToken and SecureHostFirewall. Not a hole: 22 on the
  public IP is open until SecureHostFirewall runs, that step runs last,
  and by then the tunnel is necessarily proven. And nothing validated the
  hub key/endpoint, so renderConfig() would emit `PublicKey = ` — now
  refused before anything is installed, allocated or written.

- `systemctl enable --now wg-quick@wg0 || wg-quick up wg0` brought the
  interface up without the enablement, so the tunnel did not come back
  after the step-6 reboot; on the next attempt both halves failed with
  "wg0 already exists" and the step retried forever on a working tunnel.
  The three states are handled separately now, and a changed wg0.conf is
  actually applied instead of only written.

- RebootIntoPveKernel removed the Debian kernel with `|| true`, ignored
  update-grub's exit status, and rebooted. A /boot that filled up during
  the full-upgrade could leave a machine only the provider's console can
  reach. The pve kernel image, its initramfs and update-grub's exit status
  are all proven first; anything missing fails loudly and does not reboot.

- ConfigureProxmox ran `ip link show vmbr0` and threw the result away
  (its comment claimed it recorded the absence; it recorded nothing), then
  repeated a rm -f, and always advanced. Proxmox on top of Debian does not
  create vmbr0, so onboarding reported `active` with no bridge. It now
  fails with the default route in the message and deliberately does not
  build the bridge over the primary NIC from a remote shell.

The things that were quietly inert:

- Proxmox applies a guest's firewall rules only while the firewall is
  enabled at datacenter level, and it ships disabled — so applyFirewall()'s
  "80/443 only" rules were inert on every customer VM. Enabling it blindly
  is the opposite trap (input policy defaults to DROP, node firewall
  defaults to on, and its management ipset is seeded from the PUBLIC
  subnet, not the tunnel), so: policy ACCEPT and the node firewall off
  first, master switch last, read back to confirm. nftables stays the one
  owner of the host's input policy.

- role_privs lacked Sys.Modify, which POST /cluster/backup requires
  (pve-manager's PVE/API2/Backup.pm), so RegisterBackup 403'd and failed
  EVERY customer run. And `pveum role add … || true` only ever applied the
  privilege list on the run that created the role — it converges now, above
  the token short-circuit so a replay reaches it.

- The nftables ruleset dropped all ICMP and ICMPv6. On a real host that
  breaks IPv6 outright once the neighbour cache expires and black-holes
  large transfers through PMTUD. Both families accept the types they need,
  policy drop stays, and a DHCP client reply is allowed so a rebind cannot
  lose the IPv4 address.

- api_token_ref used the `encrypted` cast, i.e. APP_KEY, against
  SecretVault's own written rule. One rotation would have made every host's
  Proxmox token undecryptable at once. Moved onto SecretCipher with a
  migration that leaves any value it cannot read exactly as it is and says
  so, rather than destroying a credential that exists nowhere else.

- Every plan version points at template_vmid 9000 and nothing created or
  verified it, so the first paid order died in CloneVirtualMachine after
  the customer had paid. VerifyVmTemplate reports that during onboarding
  instead. It does not build a template: what goes into the golden image is
  a product decision.

Three tests that proved nothing, fixed: `ran(…emergency-open-firewall.sh)`
only ever matched the `chmod 700` above it, because putFile() is not
recorded — the script's contents are now asserted, including that nothing
in it reopens the firewall on a timer or in the background. The port policy
was tested one-sidedly, so a mutation adding `tcp dport { 8006 } accept`
passed the suite; the ruleset is now read as a list of what it opens to an
unrestricted source. And the end-to-end test says in writing that it proves
sequencing only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 00:54:16 +02:00
nexxo 8440266ed3 Stop charging for a service that has ended
Nothing in this application could cancel a Stripe subscription — there was
no such method on the client at all. So a customer who cancelled and a
consumer who withdrew both stayed subscribed, the card went on being
charged every month, and every one of those payments arrived as
invoice.paid, drew a real number out of the gapless Austrian series and
mailed the PDF, for a machine that had been switched off. Indefinitely.

StripeClient::cancelSubscription() closes the first half, with the timing
passed explicitly at the call site — CANCEL_AT_PERIOD_END for a
cancellation, because the customer keeps the term they paid for, and
CANCEL_IMMEDIATELY for a withdrawal, because that unwinds the contract and
the whole amount has just gone back. The immediate form states prorate and
invoice_now as false rather than inheriting them: a credit note of Stripe's
beside our own Storno would be the same money twice.

The second half is ApplyStripeBillingEvent::owesADocument(). Refusing every
invoice on a cancelled contract would lose the legitimate final one — the
cycle for a term the customer really did use, unpaid, dunned for three
weeks and settled after the contract ended. So the question is not when the
payment landed but what it is for: an invoice whose billed period STARTED
before the contract ended is documented, one whose period begins at or
after that moment is not. Neither is a contract marked cancelled with no
date, nor an invoice with no period, because a number handed out in error
can never be withdrawn while a missing document can still be issued. The
register entry is written either way — the money moved, and that is where an
operator finds what has to go back.

A cancellation also took its date from the order date plus MONTHS, whatever
the term said, so a yearly customer who cancelled in March lost the nine
months they had already paid for. It reads current_period_end now, which
Stripe keeps current for both terms; the month-walk survives only for a
machine with no contract behind it, where nothing records a term at all.
Stripe is asked first and our rows are written only if it agreed: a
cancellation we could not make effective is the defect above wearing a
"gekündigt" label on the settings page, and a cancellation has no deadline,
so being asked to try again costs nobody anything.

A withdrawal cancelled the OPENING invoice only, found by invoices.order_id
— which a renewal and a module document leave null on purpose. A storage
pack booked on day three and withdrawn from on day ten was therefore
neither cancelled nor refunded. Every charge inside the window is covered
now, one Storno per document and one refund per payment, each against the
payment that actually took it: a single refund of the total against the
opening PaymentIntent is what Stripe would have refused for exceeding it,
and one idempotency key for the whole withdrawal would have had Stripe
replay the first refund's answer for the second.

StartCustomerProvisioning::resume() opened a missing contract and returned,
and the only production call to IssueInvoice::forOrders() is after the order
commits — so a worker killed in between cost that customer their invoice for
good, silently, with nothing anywhere sweeping for it. resume() finishes that
work too, guarded by the invoice already filed against the order and by
invoices.sent_at, so however often Stripe redelivers it comes out as one
invoice, one number, one mail. The order confirmation stays on the first
pass alone: it has nothing to stamp.

subscriptions.cancel_requested_at is new because the billing half of the
application had no way to tell a cancelled contract from an untouched one.
Deliberately not a status: until the term runs out this is a paying
customer, which is the distinction EndInstanceService is built around, and
`cancelled_at` beside it still means the day it genuinely ended.

clupilot:verify-vat-ids is scheduled monthly on the first, and its docblock
no longer points at a note in routes/console.php that never existed. Reverse
charge rested on a one-off VIES answer, a withdrawn registration keeps
earning rate 0 for ever, and the unpaid VAT on those invoices is the
seller's — so the cadence is the width of that window, and the first of the
month is the rhythm the return is filed on.

Two tests that proved nothing are now about behaviour. SettingsTest asserted
service_ends_at was not null, a column the code had just written and which
the broken monthly arithmetic satisfied exactly as well as the right answer;
it states the yearly date, the contract's own record and the order to Stripe.
EndInstanceServiceTest travelled to whatever value the implementation had
computed, so it recomputed the code instead of checking it, and never looked
at billing at all; the boundary is a literal date now and the money stopping
is asserted beside the address coming down.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 00:52:19 +02:00
36 changed files with 3057 additions and 164 deletions

View File

@ -25,7 +25,10 @@ use Throwable;
* theirs and is not a lawful Austrian invoice series; ours is, and it is gapless
* because it has to be. So EVERY invoice Stripe reports as paid produces a
* document of our own and one mail carrying it see App\Actions\IssueStripeInvoice,
* and invoicePaid() below for the single exception.
* and invoicePaid() below for the two exceptions: the checkout's own invoice,
* which the purchase already has a document for, and a charge for a term that
* begins after the contract ended, which owes nobody anything see
* owesADocument().
*
* Every handler is idempotent. Stripe retries a webhook until it gets a 2xx,
* and it also sends the same event to several endpoints so "already applied"
@ -86,7 +89,8 @@ class ApplyStripeBillingEvent
// A contract that has ended stays ended (mutateInOrder refuses one), so
// a final invoice arriving after the deletion cannot hand a departed
// customer their service back. The payment is still entered in the
// register below; it did happen.
// register below; it did happen. Whether it also owes a DOCUMENT is a
// separate question with a separate answer — see owesADocument().
if ($isRenewal && $start !== null && $end !== null) {
$this->mutateInOrder($subscription, $eventAt, self::RANK_PAID, fn (Subscription $fresh) => $end
// Never backwards: a renewal delayed behind the next one would
@ -128,11 +132,85 @@ class ApplyStripeBillingEvent
)
);
$this->invoicePayment($subscription, $invoice, $start, $end);
if ($this->owesADocument($subscription->refresh(), $invoice, $start)) {
$this->invoicePayment($subscription, $invoice, $start, $end);
}
return $record;
}
/**
* Whether this charge is one the customer is still owed a document for.
*
* Nothing in this application could cancel a Stripe subscription, so a
* customer who cancelled and a customer who withdrew both stayed subscribed
* and every month Stripe took another payment, `invoice.paid` arrived here,
* and a real invoice was drawn out of the gapless Austrian series and mailed,
* for a service that no longer existed. Cancelling the subscription is the
* first half of the fix; this is the second, because a subscription somebody
* failed to cancel, or one cancelled by hand in Stripe's dashboard, must not
* be able to mint numbers here either.
*
* ## The rule, and why it is not simply "refuse a cancelled contract"
*
* A perfectly legitimate FINAL invoice can arrive after the contract has
* ended, and refusing every one of them would lose a document the customer is
* legally owed. The commonest case: the cycle invoice for the term they were
* in went unpaid, Stripe's dunning ran for three weeks, the contract ended in
* the middle of it, and then the customer paid. That money is for a term they
* really did have a running cloud in.
*
* So the question is not WHEN the payment landed Stripe's dunning makes that
* arbitrary but WHAT it is for:
*
* the period this invoice bills STARTED before the contract ended
* a term the customer was actually in. Document it.
* the period begins at or after the moment the contract ended
* a term that does not exist. No document, no number, and an error in
* the log, because a charge for a service that has ended means Stripe
* was never stopped and somebody has to send that money back.
*
* The boundary case a period starting exactly at `cancelled_at` is the
* first month the customer does NOT have, so it is refused.
*
* A contract marked cancelled with no date on it, or an invoice carrying no
* period, cannot be judged by that rule at all, and both are refused. The
* asymmetry is deliberate and it is the same one IssueInvoice is built on: a
* missing document is recoverable an operator issues it from the console
* while a number handed out in error can never be withdrawn, only corrected by
* a cancellation document plus a new one.
*
* The register entry is written either way, above. The money moved; refusing
* to record that would hide it, and the register is where an operator finds
* the payments that need sending back.
*
* @param array<string, mixed> $invoice
*/
private function owesADocument(Subscription $subscription, array $invoice, ?Carbon $periodStart): bool
{
if (! $subscription->hasEnded()) {
return true;
}
$endedAt = $subscription->cancelled_at;
if ($endedAt !== null && $periodStart !== null && $periodStart->lessThan($endedAt)) {
return true;
}
Log::error('Stripe charged for a contract that has ended, so no invoice was issued and no number was used. This money has to go back.', [
'subscription' => $subscription->uuid,
'stripe_subscription' => $subscription->stripe_subscription_id,
'stripe_invoice' => $invoice['id'] ?? null,
'billing_reason' => $invoice['billing_reason'] ?? null,
'amount_paid' => $invoice['amount_paid'] ?? null,
'contract_ended_at' => $endedAt?->toIso8601String(),
'billed_period_start' => $periodStart?->toIso8601String(),
]);
return false;
}
/**
* The document this payment owes the customer, and the one mail that carries
* it.

View File

@ -6,6 +6,7 @@ use App\Exceptions\IdentityCollisionException;
use App\Mail\InvoiceMail;
use App\Mail\OrderConfirmationMail;
use App\Models\Customer;
use App\Models\Invoice;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Models\Subscription;
@ -128,12 +129,10 @@ class StartCustomerProvisioning
/**
* Invoice the payment, and tell the customer once.
*
* ONE mail, not two. The invoice is issued at the moment the money lands,
* so an order confirmation and an invoice mail would otherwise go out in
* the same second for the same event which is a defect, not a preference.
* The invoice mail carries the document and says everything the
* confirmation said, so it replaces it whenever there is an invoice; the
* confirmation remains for the case where there cannot be one.
* Both halves are idempotent and live in invoiceOnce() and mailInvoiceOnce()
* below, because this is not the only caller any more: resume() has to be able
* to finish exactly this work when a crash cut it off, and two copies of "have
* we invoiced this yet" would be two answers.
*
* Sent here rather than when provisioning finishes: those are minutes apart
* at best and can be much longer, and somebody who has just been charged
@ -150,13 +149,64 @@ class StartCustomerProvisioning
*/
private function confirmByMail(Order $order, Customer $customer): void
{
$invoice = $this->invoiceOnce($order, $customer);
if ($invoice !== null) {
$this->mailInvoiceOnce($invoice, $customer);
return;
}
$address = $customer->email;
if (! is_string($address) || $address === '') {
return;
}
$invoice = null;
// No invoice, and there could not be one: an incomplete company profile,
// or a purchase that was a full gift. The confirmation says what it can.
//
// Sent from HERE only, never from resume(). It has nothing to stamp — an
// order carries no `sent_at` — so a redelivery could not tell that it had
// already gone out, and Stripe redelivers until it gets a 2xx. The invoice
// mail is the one a crash must not lose, and that one is stamped.
try {
Mail::to($address)->queue(new OrderConfirmationMail($order, (string) $customer->name));
} catch (Throwable $e) {
Log::warning('Could not queue the purchase confirmation', [
'order' => $order->id,
'exception' => $e->getMessage(),
]);
}
}
/**
* The document this purchase owes, issued exactly once however often we are
* asked.
*
* The guard is the invoice already filed against this order, because an
* invoice number comes out of a gapless series and can never be handed out
* twice. Cancellations are excluded from the search: a Storno carries the same
* `order_id` as the document it takes back, so counting one would leave a
* withdrawn purchase looking as though it had been invoiced by its own
* cancellation.
*
* Not a database-level guard, and it cannot be one `invoices.order_id` is
* legitimately shared by an invoice and its Storno, so it can never be unique.
* What this closes is the case that actually happens: Stripe redelivering the
* same event, one delivery after another.
*/
private function invoiceOnce(Order $order, Customer $customer): ?Invoice
{
$existing = Invoice::query()
->where('order_id', $order->id)
->whereNull('cancels_invoice_id')
->orderBy('id')
->first();
if ($existing !== null) {
return $existing;
}
try {
// Refuses while the company details are incomplete, which is the
@ -165,21 +215,46 @@ class StartCustomerProvisioning
// that can never be handed out again. The purchase is unaffected:
// the order stands, the machine gets built, and the invoice can be
// issued from the console once the details are there.
$invoice = app(IssueInvoice::class)->forOrders($customer, collect([$order]));
return app(IssueInvoice::class)->forOrders($customer, collect([$order]));
} catch (Throwable $e) {
Log::warning('Could not issue an invoice for this order', [
'order' => $order->id,
'exception' => $e->getMessage(),
]);
return null;
}
}
/**
* Hand the invoice to the mailer, once.
*
* `sent_at` is the stamp, the same one App\Actions\IssueStripeInvoice uses,
* and it is written only after the mailer has accepted the message so a
* redelivery arriving after the document was written but before the mail went
* out finishes the job, and one arriving afterwards does neither twice.
*
* ONE mail, not two. The invoice is issued at the moment the money lands, so
* an order confirmation and an invoice mail would otherwise go out in the same
* second for the same event. The invoice mail says everything the confirmation
* said and carries the document, so it replaces it whenever there is one.
*/
private function mailInvoiceOnce(Invoice $invoice, Customer $customer): void
{
$address = $customer->email;
if (! is_string($address) || $address === '' || $invoice->sent_at !== null) {
return;
}
try {
Mail::to($address)->queue($invoice !== null
? new InvoiceMail($invoice, (string) $customer->name)
: new OrderConfirmationMail($order, (string) $customer->name));
Mail::to($address)->queue(new InvoiceMail($invoice, (string) $customer->name));
$invoice->update(['sent_at' => now()]);
} catch (Throwable $e) {
Log::warning('Could not queue the purchase confirmation', [
'order' => $order->id,
Log::warning('Could not queue the invoice mail for this purchase', [
'order' => $invoice->order_id,
'invoice' => $invoice->number,
'exception' => $e->getMessage(),
]);
}
@ -199,17 +274,39 @@ class StartCustomerProvisioning
* that was merely never dispatched needs no nudge from here the scheduler
* tick sweeps pending runs. A run that already ran and FAILED for want of
* the contract does, because nothing sweeps failed runs.
*
* ## The invoice is the other half, and it used to be the half that was lost
*
* This returned as soon as a contract existed, and the only production call to
* IssueInvoice::forOrders() is in confirmByMail() which runs AFTER the
* order commits. A worker killed in between left a paid purchase with no
* invoice, for good and in silence: the next redelivery found the contract
* already open, returned here, and nothing anywhere sweeps for an uninvoiced
* order. So the invoice and its mail are finished here too, on every
* redelivery, and the guards in invoiceOnce()/mailInvoiceOnce() are what make
* "however many times Stripe asks" come out as one invoice, one number and
* one mail.
*/
private function resume(Order $order): void
{
if ($order->subscription()->exists()) {
if (! $order->subscription()->exists()) {
$this->openContract($order);
if ($order->subscription()->exists()) {
$this->reviveRunStrandedWithoutAContract($order);
}
}
$customer = $order->customer;
if ($customer === null) {
return;
}
$this->openContract($order);
$invoice = $this->invoiceOnce($order, $customer);
if ($order->subscription()->exists()) {
$this->reviveRunStrandedWithoutAContract($order);
if ($invoice !== null) {
$this->mailInvoiceOnce($invoice, $customer);
}
}

View File

@ -12,6 +12,7 @@ use App\Services\Billing\IssueInvoice;
use App\Services\Billing\WithdrawalRight;
use App\Services\Stripe\StripeClient;
use Illuminate\Support\Carbon;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\Log;
use RuntimeException;
use Throwable;
@ -33,20 +34,26 @@ use Throwable;
* 2. **The withdrawal is claimed**, conditionally, on the contract. Two clicks
* in flight write one withdrawal between them; the loser is told it has
* already happened rather than sending a second refund.
* 3. **The paperwork**: the invoice that was issued is CANCELLED by a Storno
* with its own number never edited, never deleted. That is the established
* rule for correcting an issued document, and a withdrawal is the clearest
* case there is for it.
* 3. **The paperwork**: EVERY invoice raised inside the window is CANCELLED by a
* Storno with its own number never edited, never deleted. That is the
* established rule for correcting an issued document, and a withdrawal is the
* clearest case there is for it. Every one of them, and not just the opening
* purchase: a module booked on day three and a renewal that fell inside the
* fourteen days are charges too, and they used to be left standing because
* they carry no `order_id` see chargesInsideTheWindow().
* 4. **The money**, which falls out of the paperwork rather than being
* computed beside it: the whole of what the cancelled document said. FAGG
* §16 would let us keep the pro-rata value of the days the cloud ran, and
* the owner has decided not to a withdrawing consumer gets everything
* back. Two sums that must agree cannot disagree if only one of them is
* ever calculated, and here there is only the one: the Storno's.
* computed beside it: the whole of what each cancelled document said, sent
* back against the payment that actually took it. FAGG §16 would let us keep
* the pro-rata value of the days the cloud ran, and the owner has decided not
* to a withdrawing consumer gets everything back. Two sums that must agree
* cannot disagree if only one of them is ever calculated, and here there is
* only the one: the Stornos'.
* 5. **The service ends**, through App\Actions\EndInstanceService and the
* `cancellation_scheduled` machinery it already reads the address comes
* down, the DNS record goes, the instance is marked ended. No second way
* for an instance to stop being served.
* for an instance to stop being served. Stripe is told to stop billing in
* the same breath, immediately: a withdrawal unwinds the contract, so there
* is no paid-up term to run out and no next cycle to raise.
*
* ## What happens when the money side fails
*
@ -124,56 +131,175 @@ class WithdrawContract
}
/**
* The paperwork and the money.
* The paperwork and the money, for every charge inside the window.
*
* One Storno per document and one refund per payment, because that is what
* each of the two things IS: a Storno mirrors exactly one invoice and carries
* its number, and a Stripe refund goes against exactly one payment. A single
* refund for the whole sum, sent against the opening payment, would be
* refused by Stripe for exceeding it which is the trap the old one-invoice
* version was hiding behind.
*
* Each charge is settled in its own try, so a module invoice whose payment
* cannot be reached does not stop the opening purchase from being refunded.
* What actually went back is totalled from the refunds that succeeded, never
* from what was intended a contract claiming money moved that did not is
* the one state nobody may discover three weeks later.
*
* @return int what actually went back to the customer, gross, in cents
*/
private function settle(Subscription $subscription, Carbon $at): int
{
try {
$original = $this->openingInvoice($subscription);
$charges = $this->chargesInsideTheWindow($subscription, $at);
$refunded = 0;
$references = [];
$failed = false;
if ($charges->isEmpty()) {
// No document was ever issued for this purchase — the company
// details were incomplete when it was made, or it was a grant. There
// is nothing to cancel, so the refund is worked out from what was
// charged instead. Stated here rather than silently skipped: a
// withdrawal without paperwork is a withdrawal an accountant will
// ask about.
if ($original === null) {
return $this->sendBack($subscription, max(0, $subscription->order?->chargedCents() ?? 0), $at);
try {
$gross = max(0, $subscription->order?->chargedCents() ?? 0);
$reference = $this->refund(
$subscription,
$gross,
$this->paymentBehind(null, $subscription),
'clupilot-withdrawal-'.$subscription->uuid,
$at,
);
if ($reference !== null) {
$refunded += $gross;
$references[] = $reference;
}
} catch (Throwable $e) {
$this->parkMoneyFailure($subscription, $e);
$failed = true;
}
// Taken back in full, with its own gapless number, pointing at the
// one it cancels. The original keeps its number for ever.
$storno = $this->invoices->cancelling($original);
// The refund IS what the cancellation took back — never a second
// figure computed beside it, which would have to agree with the
// document and one day would not. The Storno mirrors the original,
// so this is the whole amount the customer paid.
return $this->sendBack($subscription, max(0, -(int) $storno->gross_cents), $at);
} catch (Throwable $e) {
$this->parkMoneyFailure($subscription, $e);
return 0;
}
foreach ($charges as $invoice) {
try {
// Taken back in full, with its own gapless number, pointing at
// the one it cancels. The original keeps its number for ever.
$storno = $this->invoices->cancelling($invoice);
// The refund IS what the cancellation took back — never a second
// figure computed beside it, which would have to agree with the
// document and one day would not.
$gross = max(0, -(int) $storno->gross_cents);
$reference = $this->refund(
$subscription,
$gross,
$this->paymentBehind($invoice, $subscription),
// Keyed per DOCUMENT and not per contract. One key for the
// whole withdrawal would have Stripe replay the first
// refund's answer for the second, and the second charge
// would silently never be sent back at all.
'clupilot-withdrawal-'.$subscription->uuid.'-invoice-'.$invoice->id,
$at,
);
if ($reference !== null) {
$refunded += $gross;
$references[] = $reference;
}
} catch (Throwable $e) {
$this->parkMoneyFailure($subscription, $e);
$failed = true;
}
}
$subscription->update([
'withdrawal_refund_cents' => $refunded,
// Every reference, because there is a refund per charge and an
// operator reconciling this against Stripe needs all of them.
'withdrawal_refund_reference' => $references === []
? null
: mb_substr(implode(',', $references), 0, 250),
// Cleared only when nothing went wrong. parkMoneyFailure() has
// already written the reason otherwise, and clearing it here would
// erase the one note saying this withdrawal needs a hand.
...($failed ? [] : ['withdrawal_refund_error' => null]),
]);
return $refunded;
}
/**
* Put the money back where it came from.
* Every charge this contract raised inside the withdrawal window.
*
* Stripe refunds a PAYMENT, not a subscription and not an invoice, so the
* handle is whatever the checkout reported a PaymentIntent if there was
* one, otherwise the invoice that charged the card, resolved through Stripe
* once. A contract with neither was never paid for through Stripe at all (a
* grant, a contract opened by hand), and there is nothing to send back.
* `invoices.order_id` alone was the defect: only the opening purchase carries
* one. A renewal and a Stripe-invoice document are written against the
* CONTRACT and leave `order_id` null on purpose see
* IssueInvoice::forBilledPeriod() and forStripeInvoice(), which say why so a
* customer who booked a storage pack on day three and withdrew on day ten had
* that pack neither cancelled nor refunded. The owner's rule is that a
* withdrawing consumer gets EVERYTHING back, so every document has to be in
* this list.
*
* Both links, therefore, and both are needed: the opening invoice has an
* `order_id` and no `subscription_id`, and everything after it is the other
* way round.
*
* Cancellations are excluded by construction one is not an invoice that can
* be taken back and so is anything that already carries a Storno.
* IssueInvoice::cancelling() refuses the latter in its own right; excluding it
* here as well means a second withdrawal attempt finds nothing to do rather
* than raising an exception that would be parked as a money failure.
*
* Bounded at the moment of withdrawal rather than at fourteen days. It is the
* same set a withdrawal can only be declared inside the window, so every
* document this contract has is inside it and the bound that matters is
* "before the customer changed their mind", which is what `$at` is.
*
* @return Collection<int, Invoice>
*/
private function sendBack(Subscription $subscription, int $grossCents, Carbon $at): int
private function chargesInsideTheWindow(Subscription $subscription, Carbon $at): Collection
{
$subscription->update(['withdrawal_refund_cents' => $grossCents]);
return Invoice::query()
->where(function ($query) use ($subscription) {
$query->where('subscription_id', $subscription->id);
if ($grossCents <= 0) {
return 0;
if ($subscription->order_id !== null) {
$query->orWhere('order_id', $subscription->order_id);
}
})
->whereNull('cancels_invoice_id')
->whereNotIn(
'id',
Invoice::query()->whereNotNull('cancels_invoice_id')->select('cancels_invoice_id')
)
->where('created_at', '<=', $at)
->orderBy('id')
->get();
}
/**
* The Stripe payment a charge was settled by.
*
* Stripe refunds a PAYMENT, not a subscription and not an invoice, and each
* charge inside the window was a different payment: the checkout, a renewal,
* a module prorated in the middle of a term. So every document is sent back
* against the money that actually paid for it.
*
* The opening purchase is the one document with no Stripe invoice of its own
* it is written from the ORDER, which is where the checkout's PaymentIntent
* was recorded, falling back to the invoice that charged the card, resolved
* through Stripe once. A contract with neither was never paid for through
* Stripe at all (a grant, a contract opened by hand), and there is nothing to
* send back.
*/
private function paymentBehind(?Invoice $invoice, Subscription $subscription): ?string
{
if ($invoice?->stripe_invoice_id !== null) {
return $this->stripe->invoicePaymentReference((string) $invoice->stripe_invoice_id);
}
$order = $subscription->order;
@ -183,26 +309,36 @@ class WithdrawContract
$reference = $this->stripe->invoicePaymentReference((string) $order->stripe_invoice_id);
}
return $reference;
}
/**
* Put one charge's money back where it came from.
*
* @return string|null Stripe's reference for the refund, or null when there
* was nothing to send back
*/
private function refund(
Subscription $subscription,
int $grossCents,
?string $reference,
string $idempotencyKey,
Carbon $at,
): ?string {
if ($grossCents <= 0) {
return null;
}
if ($reference === null) {
throw new RuntimeException(
'No Stripe payment is recorded for this contract, so the refund has to be sent by hand.'
'No Stripe payment is recorded for this charge, so the refund has to be sent by hand.'
);
}
$refundId = $this->stripe->refund(
$reference,
$grossCents,
// Keyed on the contract, because a contract is withdrawn from once.
// A retry after a timeout that in fact went through replays Stripe's
// first answer rather than sending the money a second time — which
// is the single most expensive mistake this file could make.
idempotencyKey: 'clupilot-withdrawal-'.$subscription->uuid,
);
$subscription->update([
'withdrawal_refund_reference' => $refundId,
'withdrawal_refund_error' => null,
]);
// A retry after a timeout that in fact went through replays Stripe's
// first answer rather than sending the money a second time — which is
// the single most expensive mistake this file could make.
$refundId = $this->stripe->refund($reference, $grossCents, idempotencyKey: $idempotencyKey);
Log::info('Refunded a withdrawal.', [
'subscription' => $subscription->uuid,
@ -211,7 +347,7 @@ class WithdrawContract
'withdrawn_at' => $at->toIso8601String(),
]);
return $grossCents;
return $refundId;
}
/**
@ -228,6 +364,13 @@ class WithdrawContract
* reason: there is no term left to keep. The customer has withdrawn from the
* contract the modules hang off, and BookAddon::cancel() takes them off
* Stripe and out of the register.
*
* The order of the two Stripe calls is the reason the subscription is stopped
* LAST and not first. BookAddon::cancel() takes each module's item off the
* subscription, and an item cannot be removed from a subscription that no
* longer exists so cancelling the whole thing first would make every one of
* those removals fail and park a sync error on a contract that is already
* over.
*/
private function endEverything(Subscription $subscription, Carbon $at): void
{
@ -235,6 +378,8 @@ class WithdrawContract
$this->bookAddon->cancel($addon);
}
$this->stopBilling($subscription);
$instance = $this->instanceOf($subscription);
if ($instance !== null && $instance->status !== 'ended') {
@ -255,6 +400,53 @@ class WithdrawContract
]);
}
/**
* Tell Stripe to stop, now.
*
* The call nothing in this application used to make. A withdrawal marked the
* contract cancelled, ended the service and sent the money back, and the
* subscription at Stripe went on charging the card every month for a
* machine that had been switched off, with `invoice.paid` arriving here each
* time and drawing a real invoice number for it.
*
* Immediately and not at the period end, which is the whole difference
* between this and a customer's own cancellation: a withdrawal unwinds the
* contract from the beginning, the entire amount has just gone back, and
* there is no paid-up term left for the customer to use up.
*
* A failure does not un-declare the withdrawal same principle as the refund,
* for the same reason but it is the most urgent thing on this contract, so
* it is said on the contract as well as in the log. Written after settle(),
* so the note cannot be cleared by a refund that succeeded.
*/
private function stopBilling(Subscription $subscription): void
{
// Nothing to stop, and nothing has gone wrong. A granted package was
// never sold through Stripe — GrantSubscription leaves the id null on
// purpose — and neither was a contract opened by hand.
if ($subscription->stripe_subscription_id === null) {
return;
}
try {
$this->stripe->cancelSubscription(
(string) $subscription->stripe_subscription_id,
StripeClient::CANCEL_IMMEDIATELY,
// Keyed on the contract, because a contract is withdrawn from
// once. A retry after a timeout replays Stripe's first answer.
idempotencyKey: 'clupilot-withdrawal-stop-'.$subscription->uuid,
);
} catch (Throwable $e) {
Log::error('A withdrawal was declared and Stripe was not told to stop charging for it.', [
'subscription' => $subscription->uuid,
'stripe_subscription' => $subscription->stripe_subscription_id,
'error' => $e->getMessage(),
]);
$this->note($subscription, 'Stripe was not told to stop: '.$e->getMessage());
}
}
/**
* The machine this contract pays for.
*
@ -285,26 +477,6 @@ class WithdrawContract
->first();
}
/**
* The document issued for the purchase that opened this contract.
*
* Cancellations are excluded by construction one is not an invoice that
* can be taken back and so is anything that already has a Storno against
* it, which IssueInvoice::cancelling() refuses in its own right.
*/
private function openingInvoice(Subscription $subscription): ?Invoice
{
if ($subscription->order_id === null) {
return null;
}
return Invoice::query()
->where('order_id', $subscription->order_id)
->whereNull('cancels_invoice_id')
->orderBy('id')
->first();
}
/**
* The refund did not go out. Said on the contract, not only in a log.
*
@ -320,8 +492,28 @@ class WithdrawContract
'error' => $e->getMessage(),
]);
$this->note($subscription, $e->getMessage());
}
/**
* Write a sentence an operator has to read onto the contract itself.
*
* Appended rather than overwritten, because a withdrawal can go wrong in more
* than one way at once a module invoice whose payment could not be reached
* AND an order to stop billing that Stripe refused and an operator who read
* only the last of them would send the money back and leave the customer
* subscribed.
*/
private function note(Subscription $subscription, string $sentence): void
{
$existing = trim((string) $subscription->withdrawal_refund_error);
$subscription->update([
'withdrawal_refund_error' => mb_substr($e->getMessage(), 0, 250),
'withdrawal_refund_error' => mb_substr(
$existing === '' ? $sentence : $existing.' | '.$sentence,
0,
250,
),
]);
}

View File

@ -19,10 +19,24 @@ use Illuminate\Console\Command;
* before this check existed is unverified, so those customers are on the domestic
* rate today whether or not they are entitled to reverse charge.
*
* Deliberately NOT scheduled by default. It talks to a public service with a
* concurrency limit, on behalf of somebody else's tax position, and an owner
* should decide the cadence rather than inherit one see the note in
* routes/console.php. Run it monthly, or before a VAT return.
* ## Scheduled monthly, and why that is not the owner's decision to inherit
*
* This used to say it was deliberately NOT scheduled, and to point at a note in
* routes/console.php that did not exist so an owner reading the schedule could
* not learn the command was there at all, and reverse charge went on resting on a
* one-off answer for ever.
*
* It is scheduled now, because the exposure is the SELLER's. A registration that
* is withdrawn keeps earning rate 0 on every invoice we issue afterwards, and the
* unpaid VAT on those is ours to make good, not the customer's so the cadence is
* not a preference but the width of the window in which that can happen unnoticed.
* Monthly, on the first, because that is the rhythm the VAT return is filed on: a
* number that lapsed in March is caught before the March return is prepared.
*
* The public service's concurrency limit is answered by `--limit` rather than by
* not running: the query takes the longest-unchecked first, so a customer base
* larger than one run rotates through it instead of starving. `--dry-run` and
* `--customer` are for the by-hand run before a return.
*/
class VerifyVatIds extends Command
{

View File

@ -3,14 +3,42 @@
namespace App\Livewire;
use App\Livewire\Concerns\ResolvesCustomer;
use App\Models\Customer;
use App\Models\Instance;
use App\Models\Subscription;
use App\Services\Stripe\StripeClient;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Log;
use LivewireUI\Modal\ModalComponent;
use Throwable;
/**
* Cancel the customer's package (R5). Per the founder's decision: effective at
* the end of the billing term, irreversible once confirmed; at term end the
* customer receives a finished data export, then the instance is deprovisioned
* (both mocked for now). Requires typing the instance subdomain to confirm.
*
* ## Stripe is told first, and everything else follows from its answer
*
* This used to write three columns on the instance and stop there the contract
* was untouched and Stripe was never told anything at all, so the card went on
* being charged every month for a package that had ended, for ever. The order of
* the two writes is therefore the design:
*
* 1. **Stripe is asked to stop at the period end.** If it refuses or cannot be
* reached, NOTHING here is recorded and the customer is asked to try again.
* Recording a cancellation we could not make effective is the defect this
* class was built out of: the customer sees "gekündigt" on their settings page
* and keeps paying. A cancellation has no deadline unlike the statutory
* withdrawal in App\Actions\WithdrawContract, which stands whatever Stripe
* says so a retry a minute later costs nobody anything.
* 2. **Then our own rows.** The reverse failure Stripe stopped, our write lost
* is the harmless direction: the customer is not charged again, and
* Stripe's `customer.subscription.deleted` at the period end closes the
* contract here through the machinery that already reads it.
*
* The order to stop carries an idempotency key on the contract, so the retry
* after a timeout that in fact went through is provably a no-op.
*/
class ConfirmCancelPackage extends ModalComponent
{
@ -40,21 +68,124 @@ class ConfirmCancelPackage extends ModalComponent
return;
}
$contract = $this->contractFor($customer, $instance);
if (! $this->stopBilling($contract)) {
$this->addError('confirmName', __('settings.cancel_billing_unreachable'));
return;
}
$instance->update([
'status' => 'cancellation_scheduled',
'cancel_requested_at' => now(),
'service_ends_at' => $this->currentPeriodEnd($instance),
'service_ends_at' => $this->currentPeriodEnd($instance, $contract),
]);
// The contract carries the request too. Without it nothing in the billing
// half of the application can tell a cancelled contract from an untouched
// one — and the status deliberately stays `active`, because until the term
// runs out this is a paying customer.
$contract?->update(['cancel_requested_at' => now()]);
return $this->redirectRoute('settings', navigate: true);
}
/**
* End of the current monthly billing period, anchored on the subscription
* start (the order date) rather than assuming calendar-month billing.
* Ask Stripe to raise no further cycle for this contract.
*
* @return bool whether the cancellation may now be recorded
*/
private function currentPeriodEnd(Instance $instance): \Illuminate\Support\Carbon
private function stopBilling(?Subscription $contract): bool
{
// Nothing to stop, and nothing has gone wrong. A granted package was
// never sold through Stripe — GrantSubscription leaves the id null on
// purpose — and neither was a machine built by hand with no contract
// behind it at all.
if ($contract?->stripe_subscription_id === null) {
return true;
}
try {
app(StripeClient::class)->cancelSubscription(
(string) $contract->stripe_subscription_id,
// At the period end, because that is what the customer is
// promised on this very modal: everything stays available to the
// end of the term they have already paid for.
StripeClient::CANCEL_AT_PERIOD_END,
// Keyed on the contract, because a contract is cancelled once.
idempotencyKey: 'clupilot-cancel-'.$contract->uuid,
);
return true;
} catch (Throwable $e) {
Log::error('A customer asked to cancel and Stripe could not be told to stop, so nothing was recorded.', [
'subscription' => $contract->uuid,
'stripe_subscription' => $contract->stripe_subscription_id,
'error' => $e->getMessage(),
]);
return false;
}
}
/**
* The contract this machine is billed through.
*
* `subscriptions.instance_id` is the link and is written when resources are
* reserved. The order is the fallback because an order belongs to exactly one
* purchase; the customer is the last resort, and only when they have exactly
* one contract guessing between two would cancel the wrong one, which is
* worse than cancelling nothing.
*/
private function contractFor(?Customer $customer, Instance $instance): ?Subscription
{
if ($instance->subscription !== null) {
return $instance->subscription;
}
if ($instance->order_id !== null) {
$byOrder = Subscription::query()->where('order_id', $instance->order_id)->latest('id')->first();
if ($byOrder !== null) {
return $byOrder;
}
}
if ($customer === null) {
return null;
}
$contracts = $customer->subscriptions()->where('status', 'active')->get();
return $contracts->count() === 1 ? $contracts->first() : null;
}
/**
* The end of the term the customer has already paid for.
*
* The contract knows it, and it is the only thing that does: `term` is
* monthly or yearly, and `current_period_end` is the boundary Stripe pushes
* forward on every renewal so it is right for both, and right for a
* customer who is three months into a yearly contract. Walking months forward
* from the order date, which is what this used to do, took eleven months of
* paid service off every yearly customer who cancelled.
*
* The fallback below is only for a machine with NO contract behind it one
* built by hand, or one whose contract failed to open. There is genuinely
* nothing else to read there: no term is recorded anywhere, so a month is the
* only assumption available, and it is the shorter of the two.
*/
private function currentPeriodEnd(Instance $instance, ?Subscription $contract): Carbon
{
if ($contract?->current_period_end !== null) {
// Taken as it stands, even when it is already past. A term that has
// run out has run out — the customer is in dunning, not in a paid
// period — and inventing another month for them would give away
// service nobody decided to give.
return $contract->current_period_end->copy();
}
$start = $instance->order?->created_at ?? $instance->created_at ?? now();
// Always add from the immutable start so the billing-day anchor is kept

View File

@ -4,6 +4,7 @@ namespace App\Models;
use App\Models\Concerns\HasUuid;
use App\Provisioning\Contracts\ProvisioningSubject;
use App\Services\Secrets\SecretCipher;
use Database\Factories\HostFactory;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
@ -28,7 +29,6 @@ class Host extends Model implements ProvisioningSubject
protected function casts(): array
{
return [
'api_token_ref' => 'encrypted',
'last_seen_at' => 'datetime',
'total_gb' => 'integer',
'total_ram_mb' => 'integer',
@ -38,6 +38,32 @@ class Host extends Model implements ProvisioningSubject
];
}
/**
* The Proxmox API token, stored under SECRETS_KEY NOT through Laravel's
* `encrypted` cast.
*
* That cast uses APP_KEY, and SecretVault's docblock states the rule this
* column was breaking: rotating APP_KEY is ordinary maintenance, and every
* credential that lives behind it would become unreadable at once. For this
* column that means every host's automation token in the same moment no
* Proxmox API on any host, discovered when provisioning stops rather than
* when the key is rotated. Same handling as Mailbox::password, through the
* same SecretCipher, so there is one key story and not two.
*/
public function setApiTokenRefAttribute(?string $value): void
{
$this->attributes['api_token_ref'] = ($value === null || $value === '')
? null
: app(SecretCipher::class)->encrypt($value);
}
public function getApiTokenRefAttribute(?string $value): ?string
{
return ($value === null || $value === '')
? null
: app(SecretCipher::class)->decrypt($value);
}
/** Placement runs (polymorphic subject). */
public function runs(): MorphMany
{

View File

@ -62,6 +62,12 @@ class Subscription extends Model
'current_period_start' => 'datetime',
'current_period_end' => 'datetime',
'pending_effective_at' => 'datetime',
// When the customer asked for the contract to end, and when it
// actually did. Two facts, not one: a cancelled package is still a
// paying customer until the term they bought runs out, so the first
// is stamped the moment they confirm and the second only when Stripe
// reports the subscription gone.
'cancel_requested_at' => 'datetime',
'cancelled_at' => 'datetime',
// The fourteen days a consumer may change their mind in, and what
// happened inside them. See App\Services\Billing\WithdrawalRight,
@ -316,6 +322,21 @@ class Subscription extends Model
return $this->withdrawn_at !== null;
}
/**
* Is this contract over cancelled at the end of its term, withdrawn from,
* or deleted at Stripe?
*
* `status` and not `cancel_requested_at`: a customer who has cancelled keeps
* everything they paid for until the term runs out, and treating them as
* gone the moment they clicked would take away service they are owed. Only
* `cancelled` is the end, and it is written by whichever of the three got
* there see ApplyStripeBillingEvent::subscriptionDeleted().
*/
public function hasEnded(): bool
{
return $this->status === 'cancelled';
}
public function isYearly(): bool
{
return $this->term === self::TERM_YEARLY;

View File

@ -7,8 +7,23 @@ use App\Provisioning\StepResult;
use App\Services\Ssh\RemoteShell;
/**
* Baseline Proxmox configuration after the kernel switch: ensure the default
* bridge exists and drop the enterprise repo. Idempotent and best-effort.
* Baseline Proxmox configuration after the kernel switch: confirm the default
* bridge exists, turn the datacenter firewall on so per-VM rules mean something,
* and drop the enterprise repo the proxmox-ve package brings back with it.
*
* This step used to do none of those things. It ran `ip link show vmbr0` and
* threw the result away its comment claimed it "records the absence", and it
* recorded nothing then repeated a `rm -f` and always advanced. Two real
* consequences followed from that:
*
* 1. Proxmox installed ON TOP OF DEBIAN does not create vmbr0; only the ISO
* installer writes that bridge into /etc/network/interfaces. So onboarding
* reported `active` on a host that had no bridge to attach a customer VM to,
* and the first paid order died in ConfigureNetwork.
* 2. Proxmox applies a guest's firewall rules only while the firewall is enabled
* at DATACENTER level (cluster.fw `[OPTIONS] enable`). It ships disabled, so
* the "80/443 only" rules HttpProxmoxClient::applyFirewall() writes for every
* customer VM were inert on every host this pipeline ever onboarded.
*/
class ConfigureProxmox extends HostStep
{
@ -24,10 +39,120 @@ class ConfigureProxmox extends HostStep
$host = $this->host($run);
$this->keyLogin($this->shell, $host);
// Most PVE installs create vmbr0; record its absence for real-host follow-up.
$this->shell->run('ip link show vmbr0');
$this->shell->run('rm -f /etc/apt/sources.list.d/pve-enterprise.list');
if (($refusal = $this->refuseWithoutBridge()) !== null) {
return $refusal;
}
if (($failure = $this->enableDatacenterFirewall()) !== null) {
return $failure;
}
// AFTER the install, not before: the proxmox-ve package ships the
// enterprise repository itself, so InstallProxmoxVe's removal (which
// happens before apt runs) cannot be the one that sticks. Both spellings,
// because PVE 9 ships the deb822 file and PVE 8 the one-line one.
$this->shell->run('rm -f /etc/apt/sources.list.d/pve-enterprise.list /etc/apt/sources.list.d/pve-enterprise.sources');
return StepResult::advance();
}
/**
* vmbr0 must exist and this step will NOT create it.
*
* Creating a bridge over the primary NIC from a remote shell means editing
* /etc/network/interfaces and reloading the network on the same link the
* shell is running over. Getting it even slightly wrong (a provider that
* needs a pointopoint route, a MAC the upstream switch will not accept on a
* bridge, an IPv6 gateway on the wrong device) leaves the machine off the
* network with no way back except the provider's console. The failure below
* costs an operator ten minutes; the alternative costs a dead server.
*
* The message carries the default route, because that is the one fact the
* operator needs and the one they would otherwise go and look up.
*/
private function refuseWithoutBridge(): ?StepResult
{
if ($this->shell->run('ip link show vmbr0')->ok()) {
return null;
}
$route = trim($this->shell->run('ip -4 route show default')->stdout);
return StepResult::fail(
'No vmbr0 bridge on this host. Proxmox installed on top of Debian does not create one — only the '.
'ISO installer does — and CluPilot deliberately will not build it over the primary NIC from a remote '.
'shell, because a mistake there takes the machine off the network for good. Create it by hand in '.
'/etc/network/interfaces (bridge_ports = the NIC carrying the default route, the host address and '.
'gateway moved onto vmbr0), apply it with `ifreload -a`, confirm you still have SSH, then retry this '.
'run. Current default route: '.($route === '' ? '(none reported)' : $route).'.'
);
}
/**
* Enable the datacenter firewall WITHOUT locking the host out.
*
* The plain `enable 1` is a trap. Proxmox's datacenter input policy defaults
* to DROP, the node's own host firewall defaults to ENABLED, and the only
* sources it lets reach 8006 and 22 are the members of the auto-generated
* `management` ipset which is seeded from `local_network`, the subnet of
* the node's own management address, i.e. the PUBLIC subnet. The WireGuard
* management address every later step and every maintenance run comes from is
* not in it. Enabling the firewall blindly therefore drops CluPilot's own SSH
* and API access the moment pve-firewall next compiles, on a host whose
* public port 22 SecureHostFirewall is about to close as well.
*
* So three settings, in this order, each doing one job:
*
* - `policy_in`/`policy_out` ACCEPT at datacenter level. This is the HOST
* input policy only: a guest reads its own vm.fw `policy_in`, which
* applyFirewall() sets to DROP explicitly for every customer VM, so ACCEPT
* here cannot loosen a single customer rule. What it does do is make sure
* that if anyone ever re-enables the node firewall from the GUI, the host
* does not become unreachable over the tunnel in the same instant.
* - The node's host firewall off. The host policy lives in nftables, written
* by SecureHostFirewall see that step's docblock for why that was chosen
* over pve-firewall. Two filters both claiming to own the input chain is
* how a host ends up unreachable for a reason nobody can find: with both
* active a packet must be accepted by BOTH, so the WireGuard rules in
* nftables would be silently overruled by an ipset compiled from the public
* subnet. One owner, named in one place.
* - `enable 1` last, once neither of the above can bite.
*
* `policy_forward` is deliberately left alone: it only applies on a node
* running the new nftables-based pve-firewall backend, which is opt-in and
* off here, and setting a knob whose scope depends on which backend is
* active would be guessing.
*
* The node is addressed by `$(hostname)` rather than by hosts.name because
* that is exactly how Proxmox names the node; if the two ever diverge, the
* machine is right and the database row is not.
*/
private function enableDatacenterFirewall(): ?StepResult
{
$commands = [
'pvesh set /cluster/firewall/options --policy_in ACCEPT --policy_out ACCEPT',
'pvesh set /nodes/"$(hostname)"/firewall/options --enable 0',
'pvesh set /cluster/firewall/options --enable 1',
];
foreach ($commands as $command) {
if (! $this->shell->run($command)->ok()) {
return StepResult::retry(20, 'could not configure the Proxmox datacenter firewall: '.$command);
}
}
// Read it back. `pvesh set` exiting 0 is not the same as the setting
// being in cluster.fw: /etc/pve is a replicated filesystem and a write
// that lost quorum is precisely the case where every customer VM's rules
// would go on being inert while onboarding reported success.
$options = $this->shell->run('pvesh get /cluster/firewall/options --output-format json')->stdout;
if (! str_contains(preg_replace('/\s+/', '', $options) ?? '', '"enable":1')) {
return StepResult::retry(
20,
'the Proxmox datacenter firewall does not read back as enabled; per-VM rules would have no effect'
);
}
return null;
}
}

View File

@ -2,7 +2,6 @@
namespace App\Provisioning\Steps\Host;
use App\Models\Host;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Ssh\RemoteShell;
@ -13,9 +12,18 @@ use Illuminate\Support\Str;
/**
* Installs WireGuard on the host, allocates a management IP, registers the host
* as a peer on the CluPilot hub, and verifies the tunnel is up.
*
* SSH for this step runs over the PUBLIC IP, not the tunnel see
* HostStep::keyLogin(), which only prefers the tunnel once a handshake has
* actually succeeded. That is what makes the interface handling below safe to
* restart: the session it would otherwise cut does not go through wg0.
*/
class ConfigureWireguard extends HostStep
{
private const CONFIG_PATH = '/etc/wireguard/wg0.conf';
private const UNIT = 'wg-quick@wg0';
public function __construct(private RemoteShell $shell, private WireguardHub $hub) {}
public function key(): string
@ -28,11 +36,30 @@ class ConfigureWireguard extends HostStep
$host = $this->host($run);
$this->keyLogin($this->shell, $host);
// Idempotent replay: tunnel already fully provisioned — just re-verify it.
if (filled($host->wg_ip) && $this->hasResource($run, 'wg_peer')) {
// Idempotent replay: the tunnel has already handshaked once (`wg_peer`)
// — just re-verify it. Asking tunnelProven() rather than checking
// wg_ip + the run's own breadcrumb keeps ONE definition of "the tunnel
// works" in the pipeline, the same one keyLogin() dials on.
if ($this->tunnelProven($host)) {
return $this->verifyHandshake();
}
// The hub identity, checked BEFORE anything is installed, allocated or
// written. renderConfig() would otherwise happily emit `PublicKey = ` /
// `Endpoint = `, which is the likeliest way to reach a never-handshaking
// tunnel at all. Checked here rather than in ValidateHostInput because
// this is where the values are actually consumed: they come from the
// WireguardHub (settings table, .env fallback) and can be changed between
// step 1 and step 4, so a check further upstream would be reading
// something other than what gets written into wg0.conf. Nothing on the
// host and nothing in the database has been touched at this point, so the
// run fails clean and an operator can fill the setting in and retry.
$hubPublicKey = trim($this->hub->publicKey());
$hubEndpoint = trim($this->hub->endpoint());
if (blank($hubPublicKey) || blank($hubEndpoint)) {
return StepResult::fail($this->missingHubSettingsMessage($hubPublicKey, $hubEndpoint));
}
if (! $this->shell->run('export DEBIAN_FRONTEND=noninteractive; apt-get install -y wireguard')->ok()) {
return StepResult::retry(30, 'installing wireguard failed');
}
@ -55,10 +82,23 @@ class ConfigureWireguard extends HostStep
});
$privateKey = trim($this->shell->run('cat /etc/wireguard/privatekey')->stdout);
$this->shell->putFile('/etc/wireguard/wg0.conf', $this->renderConfig($wgIp, $privateKey));
$desiredConfig = $this->renderConfig($wgIp, $privateKey, $hubPublicKey, $hubEndpoint);
if (! $this->shell->run('systemctl enable --now wg-quick@wg0 || wg-quick up wg0')->ok()) {
return StepResult::retry(20, 'bringing up wg0 failed');
// Compare before writing. The file used to be rewritten unconditionally
// with nothing reloading it, so an operator who corrected a wrong hub key
// and pressed Retry saw the new file on disk and no change in behaviour —
// the running interface still held the old peer. Knowing whether the file
// CHANGED is what lets the interface handling below decide between
// leaving a working tunnel alone and actually applying a correction.
$currentConfig = $this->shell->run('cat '.escapeshellarg(self::CONFIG_PATH).' 2>/dev/null')->stdout;
$configChanged = trim($currentConfig) !== trim($desiredConfig);
if ($configChanged) {
$this->shell->putFile(self::CONFIG_PATH, $desiredConfig);
}
if (($failure = $this->bringUpInterface($configChanged)) !== null) {
return $failure;
}
// See RemoveWireguardPeer: hub mutations are serialized against the
@ -67,7 +107,8 @@ class ConfigureWireguard extends HostStep
// Store the pubkey now (so removal can always clean up the hub peer), but
// record the wg_peer resource only AFTER the handshake verifies — the
// idempotent short-circuit must not skip an un-verified configuration.
// idempotent short-circuit must not skip an un-verified configuration,
// and keyLogin() must not dial an unproven tunnel.
$host->update(['wg_pubkey' => $publicKey]);
if ($this->verifyHandshake()->type !== StepResult::ADVANCE) {
@ -79,6 +120,62 @@ class ConfigureWireguard extends HostStep
return StepResult::advance();
}
/**
* Get wg0 into the state onboarding needs: up now AND up after the step-6
* reboot, running the configuration we just wrote.
*
* Three states, handled separately, because the single line this replaces
* (`systemctl enable --now wg-quick@wg0 || wg-quick up wg0`) got two of them
* wrong. Its fallback brought the interface up WITHOUT the systemd
* enablement, so the tunnel did not come back after the reboot; and on the
* next attempt both halves failed with "wg0 already exists", so the step
* retried forever against a tunnel that was in fact working.
*
* The order enable before start matters for the reboot: `enable` is what
* survives it, and it is checked and repaired even when the interface is
* already up by other means (an earlier `wg-quick up`, or an operator).
*/
private function bringUpInterface(bool $configChanged): ?StepResult
{
$present = $this->shell->run('ip link show wg0')->ok();
// `is-enabled` is asked for its exit status, not its wording, so a
// template unit that has never been enabled ("disabled", exit 1) and one
// that does not exist yet are treated the same: enable it.
if (! $this->shell->run('systemctl is-enabled --quiet '.self::UNIT)->ok()) {
if (! $this->shell->run('systemctl enable '.self::UNIT)->ok()) {
return StepResult::retry(20, 'could not enable '.self::UNIT.' (wg0 would not survive a reboot)');
}
}
if (! $present) {
if (! $this->shell->run('systemctl start '.self::UNIT)->ok()) {
return StepResult::retry(20, 'bringing up wg0 failed');
}
return null;
}
// Present and unchanged: the running interface already matches the file.
// Restarting here is what used to make a working tunnel look broken.
if (! $configChanged) {
return null;
}
// Present with a changed configuration: a restart, not `wg syncconf`.
// syncconf applies peers only, and a corrected Address or AllowedIPs
// would silently not take effect — the exact class of "the file changed
// and nothing happened" bug this method exists to remove. The brief
// interruption costs nothing: this step's own SSH session runs over the
// public IP (see the class docblock), and the short-circuit above means
// a proven tunnel never reaches this line.
if (! $this->shell->run('systemctl restart '.self::UNIT)->ok()) {
return StepResult::retry(20, 'restarting wg0 with the corrected configuration failed');
}
return null;
}
private function verifyHandshake(): StepResult
{
$hubIp = (string) config('provisioning.wireguard.hub_ip');
@ -88,7 +185,23 @@ class ConfigureWireguard extends HostStep
: StepResult::retry(15, 'WireGuard handshake not up yet');
}
private function renderConfig(string $wgIp, string $privateKey): string
/** Names the setting that is missing, so the console shows what to fill in. */
private function missingHubSettingsMessage(string $publicKey, string $endpoint): string
{
$missing = [];
if (blank($publicKey)) {
$missing[] = 'the hub public key (CLUPILOT_WG_HUB_PUBKEY)';
}
if (blank($endpoint)) {
$missing[] = 'the hub endpoint (CLUPILOT_WG_ENDPOINT, host:port)';
}
return 'Refusing to write wg0.conf: this installation has not been told '.
implode(' or ', $missing).'. Set it under Integrations and retry — '.
'a peer section without those two values can never handshake.';
}
private function renderConfig(string $wgIp, string $privateKey, string $hubPublicKey, string $hubEndpoint): string
{
$subnet = (string) config('provisioning.wireguard.subnet', '10.66.0.0/24');
$prefix = Str::afterLast($subnet, '/'); // honour the configured prefix length
@ -99,8 +212,8 @@ class ConfigureWireguard extends HostStep
"PrivateKey = {$privateKey}",
'',
'[Peer]',
'PublicKey = '.$this->hub->publicKey(),
'Endpoint = '.$this->hub->endpoint(),
'PublicKey = '.$hubPublicKey,
'Endpoint = '.$hubEndpoint,
"AllowedIPs = {$subnet}",
'PersistentKeepalive = 25',
'',

View File

@ -26,11 +26,6 @@ class CreateAutomationToken extends HostStep
{
$host = $this->host($run);
// Idempotent: token already minted and persisted.
if ($this->hasResource($run, 'pve_token') && filled($host->api_token_ref)) {
return StepResult::advance();
}
$role = (string) config('provisioning.proxmox.role_id');
$privs = (string) config('provisioning.proxmox.role_privs');
$user = (string) config('provisioning.proxmox.user');
@ -38,8 +33,35 @@ class CreateAutomationToken extends HostStep
$this->keyLogin($this->shell, $host);
// Role / user / ACL are idempotent (ignore "already exists").
$this->shell->run('pveum role add '.escapeshellarg($role).' -privs '.escapeshellarg($privs).' || true');
// The role is converged BEFORE the token short-circuit below, deliberately.
//
// `pveum role add … || true` only ever applied the privilege list on the
// run that first created the role, so adding a privilege to
// provisioning.role_privs reached new hosts and no existing one. That is
// how Sys.Modify would have been missing on every host onboarded before
// it was added, with the symptom appearing much later and somewhere else
// entirely: a 403 from POST /cluster/backup failing a paying customer's
// provisioning at register_backup. `role modify` without -append REPLACES
// the list, so this line makes the host match the config rather than
// accumulating whatever any past version of it once granted.
// Two separate commands rather than one `add || modify` line, so a
// failure can be attributed: `add` failing means the role is already
// there, which is the ordinary case and not an error.
$roleArgs = escapeshellarg($role).' -privs '.escapeshellarg($privs);
if (! $this->shell->run('pveum role add '.$roleArgs)->ok()
&& ! $this->shell->run('pveum role modify '.$roleArgs)->ok()) {
return StepResult::retry(20, 'could not converge the Proxmox automation role privileges');
}
// Idempotent: token already minted and persisted. Below the role
// convergence, so a replay still repairs the privileges.
if ($this->hasResource($run, 'pve_token') && filled($host->api_token_ref)) {
return StepResult::advance();
}
// User and ACL are idempotent and have nothing to converge (unlike the
// role, whose CONTENT changes as the pipeline grows), so "already exists"
// stays tolerated here.
$this->shell->run('pveum user add '.escapeshellarg($user).' || true');
$this->shell->run('pveum acl modify / -user '.escapeshellarg($user).' -role '.escapeshellarg($role).' || true');

View File

@ -4,6 +4,7 @@ namespace App\Provisioning\Steps\Host;
use App\Models\Host;
use App\Models\ProvisioningRun;
use App\Models\RunResource;
use App\Provisioning\Contracts\ProvisioningStep;
use App\Provisioning\Steps\ManagesRunResources;
use App\Services\Secrets\SecretVault;
@ -36,11 +37,32 @@ abstract class HostStep implements ProvisioningStep
}
/**
* Prefer the WireGuard management address once the tunnel exists, so every
* step after ConfigureWireguard and every later maintenance run reaches
* the host over the tunnel rather than its public IP. Falls back to
* public_ip for the handful of steps that run before wg_ip is set (the
* tunnel cannot carry SSH before it exists).
* Reach the host over the WireGuard tunnel once the tunnel is PROVEN, and
* over its public IP until then.
*
* The distinction is the whole point. `wg_ip` is written inside the
* allocation lock in ConfigureWireguard, before the handshake has been
* verified even once so "wg_ip is filled" means "an address has been
* reserved", not "SSH works over the tunnel". Dialling the tunnel on the
* strength of the reservation alone produced an unrecoverable dead end:
* attempt 1 reserved the address and then failed at the handshake, and every
* later attempt including an operator pressing Retry in the console died
* connecting to a tunnel that did not work, before reaching the only code
* that could have repaired wg0.conf. Recovery meant nulling hosts.wg_ip by
* hand. The same shape sat under RebootIntoPveKernel, ConfigureProxmox,
* CreateAutomationToken and SecureHostFirewall, which all call this method.
*
* tunnelProven() reads the `wg_peer` breadcrumb, which ConfigureWireguard
* records ONLY after a successful handshake the one fact in the database
* that means "this tunnel has carried traffic".
*
* Falling back to the public IP is not a way around the firewall: port 22 on
* the public IP is only open until SecureHostFirewall runs, that step runs
* last, and by the time it has run the tunnel is necessarily proven (it is
* the same `wg_peer` breadcrumb, recorded several steps earlier, and
* SecureHostFirewall additionally re-checks the handshake from the host's own
* side before writing a single rule). So the fallback only ever applies while
* 22 is still open anyway, and stops applying exactly when it closes.
*/
protected function keyLogin(RemoteShell $shell, Host $host): void
{
@ -48,10 +70,31 @@ abstract class HostStep implements ProvisioningStep
// one, else CLUPILOT_SSH_PRIVATE_KEY(_PATH) — read here, at the
// point of use, per SecretVault's docblock (rule 3).
$shell->connectWithKey(
filled($host->wg_ip) ? $host->wg_ip : $host->public_ip,
$this->tunnelProven($host) ? $host->wg_ip : $host->public_ip,
'root',
(string) app(SecretVault::class)->get('ssh.private_key'),
$host->ssh_host_key, // pinned during EstablishSshTrust
);
}
/**
* Has a WireGuard handshake to this host ever succeeded?
*
* Scoped to the HOST, not to one run: the tunnel is a property of the
* machine, and every maintenance run made after onboarding must keep using
* it a run-scoped lookup would send a later run to a public IP where 22 is
* closed. Nothing prunes provisioning runs or their resources, so the
* breadcrumb outlives the run that wrote it.
*/
protected function tunnelProven(Host $host): bool
{
if (blank($host->wg_ip)) {
return false;
}
return RunResource::query()
->where('host_id', $host->id)
->where('kind', 'wg_peer')
->exists();
}
}

View File

@ -10,9 +10,62 @@ use App\Services\Ssh\RemoteShell;
* Installs Proxmox VE on the Debian base: add the no-subscription repo + key,
* full-upgrade, install proxmox-ve. Long-running; apt/network failures retry,
* a bad repo/key fails.
*
* The Debian release is READ FROM THE HOST, never assumed. Proxmox publishes one
* suite per Debian release and the two must match: pointing a Debian 13 (trixie)
* base at the bookworm suite installs a PVE built against a different libc and
* kernel, which is the kind of breakage that ends with a machine that has no
* working package manager and no working hypervisor. A server ordered today
* boots trixie, so the codename this step used to hard-code was already wrong.
*
* An unrecognised release FAILS and names what it found. Guessing a suite for an
* unknown base "trixie is newest, so use trixie" is exactly the mistake this
* replaces: the next Debian will be unknown too, and by then nobody is watching.
*/
class InstallProxmoxVe extends HostStep
{
/**
* Debian codename -> the Proxmox suite and release key that go with it.
*
* `keyring` is the filename upstream's own install guide for that release
* uses; `key_url` is the URL from that same guide. Both are kept per release
* because Proxmox signs each suite with its own key and renamed the file
* between PVE 8 and PVE 9 one shared name would eventually be pointed at
* the wrong key without anything looking wrong.
*
* Bullseye/PVE 7 is deliberately absent: onboarding a Debian 11 machine
* today means somebody picked the wrong image, and the loud failure below is
* a better answer than installing a hypervisor that is out of support.
*
* @var array<string, array{suite: string, key_url: string, keyring: string}>
*/
private const RELEASES = [
'bookworm' => [
'suite' => 'bookworm',
'key_url' => 'https://enterprise.proxmox.com/debian/proxmox-release-bookworm.gpg',
'keyring' => 'proxmox-release-bookworm.gpg',
],
'trixie' => [
'suite' => 'trixie',
'key_url' => 'https://enterprise.proxmox.com/debian/proxmox-archive-keyring-trixie.gpg',
'keyring' => 'proxmox-archive-keyring.gpg',
],
];
/**
* Keys go here and are named by `Signed-By`, not into
* /etc/apt/trusted.gpg.d/. A key in trusted.gpg.d is trusted for EVERY
* repository on the machine, so the Proxmox key would also vouch for
* anything else that later appears in sources.list which is why apt has
* been warning about that directory for several releases.
*/
private const KEYRING_DIR = '/usr/share/keyrings';
private const SOURCES_FILE = '/etc/apt/sources.list.d/pve-install-repo.sources';
/** What this step wrote before it used deb822; removed so the suite is stated once. */
private const LEGACY_SOURCES_FILE = '/etc/apt/sources.list.d/pve-install-repo.list';
public function __construct(private RemoteShell $shell) {}
public function key(): string
@ -35,18 +88,40 @@ class InstallProxmoxVe extends HostStep
return StepResult::advance();
}
$key = $this->shell->run(
'curl -fsSL https://enterprise.proxmox.com/debian/proxmox-release-bookworm.gpg '.
'-o /etc/apt/trusted.gpg.d/proxmox-release-bookworm.gpg'
);
if (! $key->ok()) {
return StepResult::fail('Could not fetch the Proxmox release key.');
[$id, $codename, $prettyName] = $this->readOsRelease();
if ($id !== 'debian' || ! isset(self::RELEASES[$codename])) {
return StepResult::fail(
'Refusing to install Proxmox VE: this host does not run a Debian release CluPilot '.
'knows a matching Proxmox suite for. /etc/os-release reports '.
'ID='.($id === '' ? '(empty)' : $id).', '.
'VERSION_CODENAME='.($codename === '' ? '(empty)' : $codename).', '.
'PRETTY_NAME='.($prettyName === '' ? '(empty)' : '"'.$prettyName.'"').'. '.
'Supported: '.implode(', ', array_keys(self::RELEASES)).'. '.
'Reinstall the server on a supported Debian release, or add the release to '.
'InstallProxmoxVe::RELEASES once Proxmox publishes a suite for it.'
);
}
$this->shell->putFile(
'/etc/apt/sources.list.d/pve-install-repo.list',
"deb [arch=amd64] http://download.proxmox.com/debian/pve bookworm pve-no-subscription\n"
$release = self::RELEASES[$codename];
$keyringPath = self::KEYRING_DIR.'/'.$release['keyring'];
$key = $this->shell->run(
'install -d -m 0755 '.escapeshellarg(self::KEYRING_DIR).' && '.
'curl -fsSL '.escapeshellarg($release['key_url']).' -o '.escapeshellarg($keyringPath)
);
if (! $key->ok()) {
return StepResult::fail(
'Could not fetch the Proxmox release key for '.$codename.' from '.$release['key_url'].'.'
);
}
$this->shell->putFile(self::SOURCES_FILE, $this->renderSourcesFile($release['suite'], $keyringPath));
// A host onboarded before this step used deb822 still carries the old
// one-line file. Left in place it would name the suite a second time,
// and a stale bookworm line on a trixie machine is precisely the
// mismatch this step now refuses to create.
$this->shell->run('rm -f '.escapeshellarg(self::LEGACY_SOURCES_FILE));
$this->shell->run('rm -f /etc/apt/sources.list.d/pve-enterprise.list');
if (! $this->shell->run('export DEBIAN_FRONTEND=noninteractive; apt-get update')->ok()) {
@ -61,4 +136,48 @@ class InstallProxmoxVe extends HostStep
return StepResult::advance();
}
/**
* ID, VERSION_CODENAME and PRETTY_NAME, in one read.
*
* Sourced rather than grepped so the shell does the unquoting for us, and
* printed with a separator that cannot appear in any of the three values.
* PRETTY_NAME is carried purely so the failure message above can quote what
* the machine actually says about itself a Debian unstable image has no
* VERSION_CODENAME at all, and "codename=(empty)" on its own tells an
* operator nothing about which image they booted.
*
* @return array{0: string, 1: string, 2: string}
*/
private function readOsRelease(): array
{
$raw = $this->shell->run(
'. /etc/os-release 2>/dev/null; printf "%s|%s|%s\n" "$ID" "$VERSION_CODENAME" "$PRETTY_NAME"'
)->stdout;
$parts = array_pad(explode('|', trim($raw), 3), 3, '');
return [strtolower(trim($parts[0])), strtolower(trim($parts[1])), trim($parts[2])];
}
/**
* deb822 rather than the one-line format, because `Signed-By` is the only
* way to scope the Proxmox key to the Proxmox repository, and because this
* is the shape upstream's own PVE 9 guide ships. apt has understood it
* since well before bookworm, so both supported releases read the same file.
*/
private function renderSourcesFile(string $suite, string $keyringPath): string
{
return implode("\n", [
'# Written by CluPilot\'s InstallProxmoxVe provisioning step.',
'# The suite is derived from /etc/os-release on this host, not assumed.',
'Types: deb',
'URIs: http://download.proxmox.com/debian/pve',
'Suites: '.$suite,
'Components: pve-no-subscription',
'Architectures: amd64',
'Signed-By: '.$keyringPath,
'',
]);
}
}

View File

@ -12,6 +12,12 @@ use Throwable;
* Removes the Debian kernel and reboots into the Proxmox kernel. The reboot is
* modelled as a retry loop: issue once, then poll until `uname -r` reports a
* -pve kernel. The state machine survives the reboot by design.
*
* The reboot is the one irreversible thing this pipeline does. Everything else
* can be retried from CluPilot; a machine that does not come back needs the
* provider's out-of-band console, and on a dedicated server that can mean hours.
* So the boot path is PROVEN before the reboot is issued never assumed from
* "apt did not complain".
*/
class RebootIntoPveKernel extends HostStep
{
@ -35,8 +41,18 @@ class RebootIntoPveKernel extends HostStep
if (! $run->context('reboot_issued')) {
$this->keyLogin($this->shell, $host);
// Tolerated, deliberately: an image that never had the
// linux-image-amd64 metapackage installed makes apt exit non-zero
// with nothing wrong. What matters is not whether the Debian kernel
// went away but whether a Proxmox one can boot, and that is checked
// below on its own terms rather than inferred from this line.
$this->shell->run('export DEBIAN_FRONTEND=noninteractive; apt-get -y remove linux-image-amd64 || true');
$this->shell->run('update-grub');
if (($refusal = $this->refuseUnbootableReboot()) !== null) {
return $refusal;
}
$run->mergeContext([
'reboot_issued' => true,
'reboot_deadline' => now()->addMinutes(15)->toIso8601String(),
@ -69,4 +85,68 @@ class RebootIntoPveKernel extends HostStep
return StepResult::poll(15, 'waiting for the Proxmox kernel');
}
/**
* Everything that has to be true for this machine to come back, or a fail
* naming what is not.
*
* The three checks are the three ways the old code could reboot into
* nothing. It removed the Debian kernel with `|| true` and ignored
* update-grub's exit status, so a /boot that had filled up common enough
* on a provider image with a small boot partition, and made likelier by the
* full-upgrade the previous step runs produced a truncated initramfs, a
* half-written grub.cfg, and a reboot issued on the strength of neither.
*
* A fail rather than a retry: none of these three conditions fixes itself,
* and an operator who is already watching the onboarding can clear /boot or
* reinstall the kernel. Retrying would only spend the budget and then say
* something vaguer.
*/
private function refuseUnbootableReboot(): ?StepResult
{
// A kernel image is not enough on its own — grub needs the initramfs
// that goes with it, and a truncated or absent initrd is exactly what a
// full /boot leaves behind. Both are listed so the message can say which
// half is missing.
$kernels = trim($this->shell->run('ls -1 /boot/vmlinuz-*-pve 2>/dev/null')->stdout);
$initrds = trim($this->shell->run('ls -1 /boot/initrd.img-*-pve 2>/dev/null')->stdout);
if ($kernels === '') {
return StepResult::fail(
'Refusing to reboot: no Proxmox kernel image (/boot/vmlinuz-*-pve) is installed on this host, '.
'so it would come back on the Debian kernel at best and not at all if that one was removed. '.
'Check `apt-get -y install proxmox-ve` and the free space in /boot, then retry.'
);
}
if ($initrds === '') {
return StepResult::fail(
'Refusing to reboot: a Proxmox kernel is installed ('.$this->firstLine($kernels).') but it has no '.
'initramfs (/boot/initrd.img-*-pve), which is what a full /boot leaves behind. Free space in /boot, '.
'run `update-initramfs -u -k all`, then retry.'
);
}
// Checked for its EXIT STATUS, which the old code discarded. update-grub
// failing is the difference between a boot menu that lists the new
// kernel and one that does not mention it.
$grub = $this->shell->run('update-grub');
if (! $grub->ok()) {
return StepResult::fail(
'Refusing to reboot: update-grub failed, so the boot menu may not list the Proxmox kernel '.
'('.$this->firstLine($kernels).'). Output: '.$this->firstLine(trim($grub->stderr.' '.$grub->stdout)).'. '.
'Fix that on the host (usually free space in /boot), then retry.'
);
}
return null;
}
/** Keeps a multi-line `ls` or a long apt error from swamping the run's error field. */
private function firstLine(string $output): string
{
$line = trim((string) preg_split('/\R/', $output)[0]);
return $line === '' ? '(no output)' : $line;
}
}

View File

@ -32,11 +32,18 @@ use App\Services\Ssh\RemoteShell;
* one). Chosen over Proxmox's own firewall (pve-firewall needs the /etc/pve
* cluster filesystem plus a separate datacenter+node enable flag two more
* places this could silently stay off) and over iptables-persistent (nftables
* is bookworm's default packet-filtering framework and already ships the
* is Debian's default packet-filtering framework and already ships the
* boot-time unit the package needs). Only the INPUT chain is touched FORWARD
* and OUTPUT are left at the kernel default so customer VM traffic through
* the bridges is never affected by a host-management rule.
*
* This ruleset is the ONLY owner of the host's input policy. ConfigureProxmox
* enables Proxmox's firewall at datacenter level guest rules do nothing
* without it but explicitly turns the NODE firewall off for exactly this
* reason: with both active a packet must be accepted by both, and pve-firewall's
* auto-generated management ipset is seeded from the public subnet, which would
* silently overrule the WireGuard-only rule below. See that step's docblock.
*
* Idempotent: once applied, the resource breadcrumb short-circuits a re-run
* exactly like the other steps' external-resource guards.
*
@ -123,9 +130,35 @@ table inet clupilot_filter {
ct state invalid drop
ct state established,related accept
# ICMPv6 is not optional. Neighbour discovery (135/136) and router
# advertisements arrive in the INPUT chain, so a bare policy drop takes
# IPv6 down on this host as soon as the neighbour cache expires — minutes,
# not days. packet-too-big is the other half: it is how the far end of a
# TLS connection learns the path MTU, and dropping it black-holes large
# transfers rather than failing them. Debian's own example ruleset accepts
# all of ipv6-icmp; the types are listed instead so the intent is readable
# and so the legacy information-leak requests stay out. One rule per line,
# deliberately: a ruleset that can be read a line at a time can also be
# asserted on a line at a time.
meta nfproto ipv6 icmpv6 type { destination-unreachable, packet-too-big, time-exceeded, parameter-problem, echo-request, echo-reply, nd-router-solicit, nd-router-advert, nd-neighbor-solicit, nd-neighbor-advert, mld-listener-query, mld-listener-report, mld-listener-done } accept
# IPv4 ICMP, for the same reason: destination-unreachable carries
# fragmentation-needed (code 4), which is what IPv4 path-MTU discovery
# runs on. echo is here so the host answers a ping — including the hub's.
meta nfproto ipv4 icmp type { destination-unreachable, time-exceeded, parameter-problem, echo-request, echo-reply } accept
# Public web — Traefik on this host serves customer traffic here.
tcp dport { 80, 443 } accept
# DHCP client replies. Some providers hand out the IPv4 address by DHCP,
# and a lease REBIND arrives as a fresh inbound packet that conntrack has
# no established entry for — dropped, the lease expires and the host loses
# the address it is reached on. Narrow on purpose: only a reply from a
# server port to the client port. IPv6 needs no equivalent here because
# addressing on these hosts comes from router advertisements, which the
# nd-router-advert accept above already covers.
udp sport 67 udp dport 68 accept
# Management surfaces: only reachable over the WireGuard tunnel.
ip saddr {$wgSubnet} tcp dport { 22, 8006 } accept
}

View File

@ -0,0 +1,106 @@
<?php
namespace App\Provisioning\Steps\Host;
use App\Models\PlanVersion;
use App\Models\ProvisioningRun;
use App\Provisioning\StepResult;
use App\Services\Proxmox\ProxmoxClient;
use Throwable;
/**
* Confirms the VM templates the plan catalogue sells actually exist on this node.
*
* Without this, a host went `active` while it could not serve a single customer.
* Every plan version points at a `template_vmid`, nothing in the host pipeline
* creates, copies or verifies that template, and there is no console command that
* does either. The first paid order placed on a freshly onboarded host therefore
* died in CloneVirtualMachine after the customer had paid, in the one place
* where nobody is watching and the only honest recovery is a refund.
*
* So the gap is reported HERE, during onboarding, with an operator in front of
* the run. This step deliberately does NOT build a template: what goes into the
* golden image (Nextcloud version, cloud-init, guest agent, disk layout) is a
* product decision, and a step that invented one would produce customers running
* something nobody chose.
*
* Placed straight after VerifyProxmoxApi because it is the first thing worth
* asking the API once the API answers, and before RegisterCapacity so a host that
* cannot serve anyone never reaches the capacity table at all.
*/
class VerifyVmTemplate extends HostStep
{
public function __construct(private ProxmoxClient $pve) {}
public function key(): string
{
return 'verify_vm_template';
}
public function execute(ProvisioningRun $run): StepResult
{
$host = $this->host($run);
$required = $this->requiredTemplateVmids();
// A catalogue with nothing published yet requires no template. This is a
// real state — an installation being set up for the first time onboards
// its host before it publishes its plans — and failing here would be
// inventing a requirement nobody has stated.
if ($required === []) {
return StepResult::advance();
}
$client = $this->pve->forHost($host);
try {
$nodes = $client->listNodes();
$node = $nodes[0]['node'] ?? 'pve';
$missing = array_values(array_filter(
$required,
fn (int $vmid) => ! $client->vmExists($node, $vmid),
));
} catch (Throwable $e) {
// The API was answering one step ago; a hiccup here is transient.
return StepResult::retry(15, 'could not ask Proxmox about the VM templates: '.$e->getMessage());
}
if ($missing !== []) {
return StepResult::fail(
'This host has no VM template to clone from, so it could not serve a single order. Missing on node '.
$node.': VMID '.implode(', ', $missing).', which '.
(count($missing) === 1 ? 'a published plan version points' : 'published plan versions point').' at. '.
'Copy the golden template onto this host (or restore it from another node) and retry — otherwise the '.
'first paid order placed here fails in clone_vm, after the customer has paid.'
);
}
return StepResult::advance();
}
/**
* Every template a currently sellable plan version points at.
*
* Published versions whose window has not closed, rather than
* PlanVersion::available() a version scheduled to launch next week is not
* "available" yet and its template gap would then surface at its first order
* instead of here, which is the whole failure this step removes. A closed
* window is different: nobody can buy it any more, so its template is not
* this host's problem.
*
* @return array<int, int>
*/
private function requiredTemplateVmids(): array
{
return PlanVersion::query()
->whereNotNull('published_at')
->whereNotNull('template_vmid')
->where(fn ($q) => $q->whereNull('available_until')->orWhere('available_until', '>', now()))
->pluck('template_vmid')
->map(fn ($vmid) => (int) $vmid)
->unique()
->sort()
->values()
->all();
}
}

View File

@ -74,6 +74,14 @@ class FakeRemoteShell implements RemoteShell
return $this->default;
}
/**
* Observable through files(), NOT through ran(): an uploaded file is not a
* command and is deliberately not recorded as one. Worth stating, because a
* test once asserted `ran('/usr/local/sbin/clupilot-…-firewall.sh')` believing
* it proved the script had been deployed the only thing that substring
* matched was the `chmod 700` on the line above, so the same fact was
* asserted twice and the script's contents were never checked at all.
*/
public function putFile(string $remotePath, string $contents): void
{
$this->files[$remotePath] = $contents;

View File

@ -69,6 +69,15 @@ class FakeStripeClient implements StripeClient
*/
public array $invoiceLines = [];
/**
* Every order to stop billing, in order, so a test can assert WHEN a
* contract was cancelled as well as that it was the difference between a
* customer keeping the term they paid for and losing it.
*
* @var array<int, array{subscription: string, when: string, key: ?string}>
*/
public array $cancellations = [];
/**
* Every refund that was sent, in order, so a test can assert the amount as
* well as the fact of it.
@ -258,6 +267,36 @@ class FakeStripeClient implements StripeClient
];
}
public function cancelSubscription(
string $subscriptionId,
string $when,
?string $idempotencyKey = null,
): void {
$this->failIfAsked();
// Refused here as well as in HttpStripeClient, so a caller that invents a
// third timing fails in the tests rather than in production.
if (! in_array($when, [self::CANCEL_AT_PERIOD_END, self::CANCEL_IMMEDIATELY], true)) {
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])) {
return;
}
$this->cancellations[] = [
'subscription' => $subscriptionId,
'when' => $when,
'key' => $idempotencyKey,
];
if ($idempotencyKey !== null) {
$this->keys[$idempotencyKey] = $subscriptionId;
}
}
public function refund(
string $paymentReference,
?int $amountCents = null,

View File

@ -171,6 +171,48 @@ class HttpStripeClient implements StripeClient
->throw();
}
public function cancelSubscription(
string $subscriptionId,
string $when,
?string $idempotencyKey = null,
): void {
$atPeriodEnd = match ($when) {
self::CANCEL_AT_PERIOD_END => true,
self::CANCEL_IMMEDIATELY => false,
// A typo would take a paying customer's service away this afternoon,
// or leave a withdrawn one subscribed for ever. Neither is a thing to
// fall back to a default about.
default => throw new RuntimeException("Unknown cancellation timing: {$when}"),
};
if ($atPeriodEnd) {
// An UPDATE and not a delete. The subscription stays alive to the
// boundary the customer has paid to, raises no further cycle, and
// Stripe ends it there of its own accord; deleting it here would take
// the service away the moment somebody clicked "cancel".
$this->request($idempotencyKey)
->asForm()
->post($this->url('subscriptions/'.$subscriptionId), ['cancel_at_period_end' => 'true'])
->throw();
return;
}
// Both stated rather than left to Stripe's defaults, because both decide
// money. `prorate` would raise a credit for the unused days and
// `invoice_now` would charge whatever is unbilled — and the caller that
// asks for an immediate stop is a withdrawal, which sends the WHOLE
// amount back from a cancellation document of its own. A credit note of
// Stripe's beside that would be the same money twice.
$this->request($idempotencyKey)
->asForm()
->delete($this->url('subscriptions/'.$subscriptionId), [
'prorate' => 'false',
'invoice_now' => 'false',
])
->throw();
}
public function refund(
string $paymentReference,
?int $amountCents = null,

View File

@ -30,6 +30,26 @@ interface StripeClient
*/
public const PRORATE_NONE = 'none';
/**
* Stop at the end of the term the customer has already paid for.
*
* What a CANCELLATION means. They keep the service to the boundary they have
* been billed to, no further cycle is ever raised, and Stripe ends the
* subscription itself at that moment which arrives here as
* `customer.subscription.deleted` and is what ApplyStripeBillingEvent reads
* as the end of the contract.
*/
public const CANCEL_AT_PERIOD_END = 'at_period_end';
/**
* Stop now.
*
* What a WITHDRAWAL means. The contract is unwound rather than ended, the
* whole amount goes back, and there is no paid-up term left to run out see
* App\Actions\WithdrawContract.
*/
public const CANCEL_IMMEDIATELY = 'immediately';
/** Whether an account is actually configured. */
public function isConfigured(): bool;
@ -149,6 +169,31 @@ interface StripeClient
*/
public function removeSubscriptionItem(string $itemId, string $prorationBehaviour): void;
/**
* Tell Stripe to stop charging for a contract that is over.
*
* The one call this platform could not make. Nothing here cancelled a Stripe
* subscription at all: a customer's cancellation wrote a date on the
* instance, a withdrawal marked the contract cancelled and sent the money
* back, and in both cases Stripe went on taking the next month, and the next,
* for a service that had ended and every one of those payments arrived as
* `invoice.paid` and drew a number out of the gapless Austrian series.
*
* `$when` is one of the CANCEL_* constants and is always passed explicitly,
* for the same reason `$prorationBehaviour` is above: the two are different
* promises to the customer, and which one a caller means has to be legible at
* the call site rather than inherited from a default written down here.
*
* `$idempotencyKey` because a retry after a timeout cannot tell whether the
* first attempt landed. Cancelling twice is harmless at Stripe, but the same
* key makes the retry provably a no-op instead of merely a likely one.
*/
public function cancelSubscription(
string $subscriptionId,
string $when,
?string $idempotencyKey = null,
): void;
/**
* Send money back, against the payment that took it.
*

View File

@ -39,6 +39,10 @@ return [
Host\ConfigureProxmox::class,
Host\CreateAutomationToken::class,
Host\VerifyProxmoxApi::class,
// Before the host is ever offered to a customer: a node with no
// template to clone from cannot serve one, and finding that out in
// CloneVirtualMachine means finding it out after somebody paid.
Host\VerifyVmTemplate::class,
Host\RegisterHostDns::class,
Host\RegisterCapacity::class,
Host\SecureHostFirewall::class,
@ -360,7 +364,23 @@ return [
// Proxmox automation role/user created on each host.
'proxmox' => [
'role_id' => 'CluPilotAutomation',
'role_privs' => 'VM.Allocate,VM.Clone,VM.Config.Disk,VM.Config.CPU,VM.Config.Memory,VM.Config.Network,VM.Config.Options,VM.Config.Cloudinit,VM.PowerMgmt,VM.Monitor,VM.Audit,VM.Backup,VM.GuestAgent.Audit,VM.GuestAgent.Unrestricted,Datastore.AllocateSpace,Datastore.Audit,Sys.Audit',
/*
| Sys.Modify is not optional, and its absence broke every customer run.
|
| HttpProxmoxClient::createBackupJob() POSTs /cluster/backup, and that
| endpoint declares `check => ['perm', '/', ['Sys.Modify']]` in
| pve-manager's PVE/API2/Backup.pm verified against the source, not
| inferred from a forum post. Without it the POST comes back 403,
| RegisterBackup rethrows anything whose body does not contain "already",
| and the run failed at register_backup for every single customer.
|
| Sys.Modify is also what PUT /cluster/firewall/options requires
| (pve-firewall's PVE/API2/Firewall/Cluster.pm), so the same privilege
| covers the datacenter firewall if that ever moves off SSH and onto the
| API. It is granted on `/` because both endpoints check `/` and nothing
| narrower would satisfy them.
*/
'role_privs' => 'VM.Allocate,VM.Clone,VM.Config.Disk,VM.Config.CPU,VM.Config.Memory,VM.Config.Network,VM.Config.Options,VM.Config.Cloudinit,VM.PowerMgmt,VM.Monitor,VM.Audit,VM.Backup,VM.GuestAgent.Audit,VM.GuestAgent.Unrestricted,Datastore.AllocateSpace,Datastore.Audit,Sys.Audit,Sys.Modify',
'user' => 'automation@pve',
'token_name' => 'clupilot',
],

View File

@ -0,0 +1,45 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
/**
* When the customer asked for the contract to end.
*
* A cancellation wrote three columns on the INSTANCE `cancellation_scheduled`,
* `cancel_requested_at`, `service_ends_at` and not one word on the contract the
* money runs through. So nothing in the billing half of the application could
* tell a contract somebody had cancelled from one nobody had touched: not the
* console, not the register, and above all not the code that decides whether a
* charge arriving from Stripe is one this customer still owes.
*
* `cancel_requested_at` and not a status, deliberately. A cancelled package is
* still a PAYING customer until the term they bought runs out that distinction
* is the whole of App\Actions\EndInstanceService and of clupilot:end-due-services
* so the contract stays `active`, and the moment it genuinely ends is written
* by `customer.subscription.deleted` arriving from Stripe into `cancelled_at`
* beside it. Two columns, two different facts: when it was asked for, and when it
* happened.
*
* No backfill. There is no record anywhere of when an existing cancellation was
* requested the instance's own `cancel_requested_at` is the closest thing, and
* copying it across would state on a contract a fact that was never recorded
* against it. NULL is the honest answer for the ones that came before this.
*/
return new class extends Migration
{
public function up(): void
{
Schema::table('subscriptions', function (Blueprint $table) {
$table->timestamp('cancel_requested_at')->nullable()->after('stripe_event_rank');
});
}
public function down(): void
{
Schema::table('subscriptions', function (Blueprint $table) {
$table->dropColumn('cancel_requested_at');
});
}
};

View File

@ -0,0 +1,114 @@
<?php
use App\Services\Secrets\SecretCipher;
use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
/**
* Re-encrypt hosts.api_token_ref from APP_KEY to SECRETS_KEY.
*
* The column used Laravel's `encrypted` cast, which means APP_KEY against this
* project's own written rule (SecretVault's docblock: rotating APP_KEY is
* ordinary maintenance, which is exactly why credentials have their own key).
* Rotating APP_KEY today would make every host's Proxmox token undecryptable in
* the same moment, and nothing would say so until provisioning stopped.
*
* What this migration will NOT do is destroy a value it cannot read. A row whose
* ciphertext neither key opens is left exactly as it is and logged with its host
* id: the plaintext is a credential that cannot be reconstructed from anywhere
* else, and a token an operator can still recover by hand from an old APP_KEY is
* worth more than a tidy column. The same applies to the whole table when
* SECRETS_KEY is not usable at all there is nothing to re-encrypt WITH, so the
* migration reports that and changes nothing.
*/
return new class extends Migration
{
public function up(): void
{
$this->rewrite(
reader: fn (string $cipher) => Crypt::decryptString($cipher),
writer: fn (string $plain) => app(SecretCipher::class)->encrypt($plain),
alreadyDone: fn (string $cipher) => app(SecretCipher::class)->decrypt($cipher),
direction: 'APP_KEY -> SECRETS_KEY',
);
}
public function down(): void
{
$this->rewrite(
reader: fn (string $cipher) => app(SecretCipher::class)->decrypt($cipher),
writer: fn (string $plain) => Crypt::encryptString($plain),
alreadyDone: fn (string $cipher) => Crypt::decryptString($cipher),
direction: 'SECRETS_KEY -> APP_KEY',
);
}
/**
* One pass over the table, re-encrypting what it can read.
*
* `alreadyDone` is what makes this safe to run twice, and safe to run on a
* database somebody has already half-migrated: both keys produce ordinary
* Laravel encryption payloads, so the only way to tell them apart is which
* key validates the MAC. A row the TARGET key already opens is not touched
* and not counted as a failure.
*
* @param callable(string): string $reader
* @param callable(string): string $writer
* @param callable(string): string $alreadyDone
*/
private function rewrite(callable $reader, callable $writer, callable $alreadyDone, string $direction): void
{
if (! app(SecretCipher::class)->isUsable()) {
// Not a failure of the migration: a fresh install has no SECRETS_KEY
// yet and no hosts either. Throwing here would block the whole
// migration run on an installation that has nothing to migrate.
$pending = DB::table('hosts')->whereNotNull('api_token_ref')->count();
if ($pending > 0) {
Log::warning(
'SECRETS_KEY is not usable, so '.$pending.' host API token(s) were left encrypted under the '.
'old key. Set SECRETS_KEY and re-run this migration ('.$direction.') before rotating APP_KEY.'
);
}
return;
}
$left = [];
foreach (DB::table('hosts')->whereNotNull('api_token_ref')->get(['id', 'api_token_ref']) as $row) {
$cipher = (string) $row->api_token_ref;
if ($cipher === '') {
continue;
}
try {
$plain = $reader($cipher);
} catch (Throwable) {
try {
// Already in the target shape: nothing to do, nothing wrong.
$alreadyDone($cipher);
continue;
} catch (Throwable) {
$left[] = $row->id;
continue;
}
}
DB::table('hosts')->where('id', $row->id)->update(['api_token_ref' => $writer($plain)]);
}
if ($left !== []) {
Log::warning(
'Left '.count($left).' host API token(s) untouched ('.$direction.'): neither key could read them. '.
'Host ids: '.implode(', ', $left).'. Re-mint the token on those hosts, or restore the APP_KEY they '.
'were written under — the stored value has NOT been overwritten.'
);
}
}
};

View File

@ -128,6 +128,8 @@ return [
'configure_proxmox' => 'Proxmox konfigurieren',
'create_automation_token' => 'Automation-Token erstellen',
'verify_proxmox_api' => 'Proxmox-API prüfen',
'verify_vm_template' => 'VM-Vorlage prüfen',
'register_host_dns' => 'Internen DNS-Namen registrieren',
'register_capacity' => 'Kapazität registrieren',
'secure_host_firewall' => 'Host-Firewall absichern',
'complete_host_onboarding' => 'Onboarding abschließen',

View File

@ -48,6 +48,7 @@ return [
'cancel_confirm_label' => 'Zum Bestätigen „:name" eingeben:',
'cancel_confirm' => 'Verbindlich kündigen',
'cancel_mismatch' => 'Die Eingabe stimmt nicht mit Ihrer Cloud-Adresse überein.',
'cancel_billing_unreachable' => 'Die Kündigung konnte gerade nicht bei unserem Zahlungsdienstleister eingetragen werden. Es wurde nichts geändert — bitte versuchen Sie es in ein paar Minuten erneut.',
'keep' => 'Abbrechen',
'close_account_title' => 'Konto schließen',

View File

@ -128,6 +128,8 @@ return [
'configure_proxmox' => 'Configure Proxmox',
'create_automation_token' => 'Create automation token',
'verify_proxmox_api' => 'Verify Proxmox API',
'verify_vm_template' => 'Verify VM template',
'register_host_dns' => 'Register internal DNS name',
'register_capacity' => 'Register capacity',
'secure_host_firewall' => 'Secure host firewall',
'complete_host_onboarding' => 'Complete onboarding',

View File

@ -48,6 +48,7 @@ return [
'cancel_confirm_label' => 'Type “:name” to confirm:',
'cancel_confirm' => 'Cancel for good',
'cancel_mismatch' => 'That does not match your cloud address.',
'cancel_billing_unreachable' => 'The cancellation could not be registered with our payment provider just now. Nothing has been changed — please try again in a few minutes.',
'keep' => 'Keep',
'close_account_title' => 'Close account',

View File

@ -160,6 +160,24 @@ Schedule::command('clupilot:end-cancelled-addons')
->everyFifteenMinutes()
->withoutOverlapping();
// Ask the VIES register again about the VAT numbers reverse charge rests on.
//
// A number is verified once, at the moment somebody types it in, and never again
// — and a registration that is wound up or withdrawn keeps earning rate 0 on
// every invoice issued after it lapsed. The unpaid VAT on those is the SELLER's
// liability, not the customer's, so the cadence here is the width of the window
// in which that can happen without anyone noticing.
//
// Monthly, on the first, because that is the rhythm a VAT return is filed on: a
// registration that lapsed in March is caught before the March return is
// prepared. Not nightly — it is somebody else's public service, it answers about
// a tax position rather than about our own machines, and a number does not lapse
// twice in a month. The command takes the longest-unchecked first, so a customer
// base larger than one run's --limit rotates through instead of starving.
Schedule::command('clupilot:verify-vat-ids')
->monthlyOn(1, '04:10')
->withoutOverlapping();
// Keep the appointment a cancellation made.
//
// ConfirmCancelPackage writes a date and nothing ever went back to it, which is

View File

@ -0,0 +1,291 @@
<?php
use App\Livewire\ConfirmCancelPackage;
use App\Mail\InvoiceMail;
use App\Models\Customer;
use App\Models\Instance;
use App\Models\Invoice;
use App\Models\Order;
use App\Models\Subscription;
use App\Models\SubscriptionRecord;
use App\Models\User;
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\StripeClient;
use App\Support\CompanyProfile;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Mail;
use Livewire\Livewire;
/**
* A cancellation, and the two things that used to happen instead of it.
*
* Nothing in this application could cancel a Stripe subscription there was no
* such method on the client at all so a customer who cancelled kept being
* charged every month, for ever, for a package that had ended. And the date they
* were promised was worked out by adding MONTHS to the order date whatever the
* term said, so a yearly customer who cancelled in March lost the nine months
* they had already paid for.
*
* The example these tests are built on, once, so the dates can be checked by hand
* rather than recomputed from the implementation:
*
* contract concluded 1 January 2026
* term yearly 1 January 2026 1 January 2027
* cancellation 15 March 2026
*
* service ends 1 January 2027 the term that was paid for
* the old answer 1 April 2026 months added to the order date
*/
beforeEach(function () {
Carbon::setTestNow('2026-03-15 09:00:00');
CompanyProfile::put([
'name' => 'CluPilot Cloud e.U.',
'address' => 'Dreherstraße 66/1/8',
'postcode' => '1110',
'city' => 'Wien',
'vat_id' => 'ATU00000000',
]);
});
afterEach(function () {
Carbon::setTestNow();
});
/** A Stripe that records what it was asked to stop. */
function cancellationStripe(): FakeStripeClient
{
$fake = new FakeStripeClient;
app()->instance(StripeClient::class, $fake);
return $fake;
}
/**
* A running package its owner can reach the cancel modal for.
*
* @return array{0: User, 1: Instance, 2: Subscription}
*/
function cancellablePackage(string $term = Subscription::TERM_MONTHLY, array $contract = []): array
{
$user = User::factory()->create(['email' => 'berger@example.test', 'email_verified_at' => now()]);
$customer = Customer::factory()->create([
'email' => 'berger@example.test',
'user_id' => $user->id,
'status' => 'active',
]);
// Two and a half months ago, which is what makes the old month-walk visible:
// it would land on 1 April whatever the term is.
$order = Order::factory()->create([
'customer_id' => $customer->id,
'plan' => 'team',
'created_at' => '2026-01-01 09:00:00',
]);
$instance = Instance::factory()->create([
'customer_id' => $customer->id,
'order_id' => $order->id,
'plan' => 'team',
'status' => 'active',
'subdomain' => 'berger',
]);
$subscription = Subscription::factory()->plan('team', $term)->create(array_merge([
'customer_id' => $customer->id,
'order_id' => $order->id,
'instance_id' => $instance->id,
'stripe_subscription_id' => 'sub_cancel',
'started_at' => '2026-01-01 09:00:00',
'current_period_start' => '2026-01-01 09:00:00',
'current_period_end' => $term === Subscription::TERM_YEARLY
? '2027-01-01 09:00:00'
: '2026-04-01 09:00:00',
], $contract));
return [$user, $instance->refresh(), $subscription->refresh()];
}
it('tells Stripe to stop at the end of the term and keeps a yearly customer their year', function () {
$stripe = cancellationStripe();
[$user, $instance, $contract] = cancellablePackage(Subscription::TERM_YEARLY);
Livewire::actingAs($user)->test(ConfirmCancelPackage::class)
->set('confirmName', 'berger')
->call('cancelPackage')
->assertHasNoErrors();
// Stripe is told, and told WHICH cancellation this is. At the period end,
// because the customer keeps everything they have paid for; an immediate stop
// would be a withdrawal, which is a different promise and a refund.
expect($stripe->cancellations)->toHaveCount(1)
->and($stripe->cancellations[0]['subscription'])->toBe('sub_cancel')
->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END);
$instance->refresh();
// The end of the YEAR that was bought, written out rather than recomputed —
// and explicitly not the date months-from-the-order-date used to produce.
expect($instance->status)->toBe('cancellation_scheduled')
->and($instance->service_ends_at->toDateTimeString())->toBe('2027-01-01 09:00:00')
->and($instance->service_ends_at->toDateString())->not->toBe('2026-04-01');
$contract->refresh();
// The contract carries the request — and is still ACTIVE, because until the
// first of January this is a paying customer with a running cloud. Only
// Stripe reporting the subscription gone ends it.
expect($contract->cancel_requested_at?->toDateTimeString())->toBe('2026-03-15 09:00:00')
->and($contract->status)->toBe('active')
->and($contract->cancelled_at)->toBeNull();
});
it('keeps a monthly customer their month and no longer', function () {
$stripe = cancellationStripe();
[$user, $instance] = cancellablePackage(Subscription::TERM_MONTHLY);
Livewire::actingAs($user)->test(ConfirmCancelPackage::class)
->set('confirmName', 'berger')
->call('cancelPackage')
->assertHasNoErrors();
// The other half of the pair: a monthly contract really does end a month
// after its period started, so the yearly assertion above is about the TERM
// and not about this code being generous to everybody.
expect($instance->fresh()->service_ends_at->toDateTimeString())->toBe('2026-04-01 09:00:00')
->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END);
});
it('cancels a granted package cleanly and asks Stripe nothing at all', function () {
$stripe = cancellationStripe();
// A grant was never sold through Stripe — GrantSubscription leaves the id
// null on purpose — so there is no subscription to stop and nothing has gone
// wrong.
[$user, $instance, $contract] = cancellablePackage(Subscription::TERM_MONTHLY, [
'stripe_subscription_id' => null,
'granted_at' => '2026-01-01 09:00:00',
'granted_by' => admin()->id,
]);
Livewire::actingAs($user)->test(ConfirmCancelPackage::class)
->set('confirmName', 'berger')
->call('cancelPackage')
->assertHasNoErrors();
expect($stripe->cancellations)->toBeEmpty()
->and($instance->fresh()->status)->toBe('cancellation_scheduled')
->and($instance->fresh()->service_ends_at->toDateTimeString())->toBe('2026-04-01 09:00:00')
->and($contract->fresh()->cancel_requested_at)->not->toBeNull();
});
it('records nothing at all when Stripe cannot be told to stop', function () {
$stripe = cancellationStripe();
$stripe->failWith = 'connection refused';
[$user, $instance, $contract] = cancellablePackage();
Livewire::actingAs($user)->test(ConfirmCancelPackage::class)
->set('confirmName', 'berger')
->call('cancelPackage')
->assertHasErrors(['confirmName']);
// A cancellation we could not make effective is the defect this whole file
// is about: the customer would read "gekündigt" on their settings page and
// keep paying. Nothing is written, and they are asked to try again — which
// costs nobody anything, because a cancellation has no deadline.
expect($instance->fresh()->status)->toBe('active')
->and($instance->fresh()->service_ends_at)->toBeNull()
->and($contract->fresh()->cancel_requested_at)->toBeNull();
});
/**
* A contract that has ended, and a Stripe that has not noticed.
*/
function endedContract(string $endedAt = '2026-04-01 09:00:00'): Subscription
{
$customer = Customer::factory()->create(['name' => 'Berger GmbH', 'email' => 'kundin@example.test']);
return Subscription::factory()->plan('team')->create([
'customer_id' => $customer->id,
'stripe_subscription_id' => 'sub_ended',
'current_period_start' => '2026-03-01 09:00:00',
'current_period_end' => $endedAt,
'status' => 'cancelled',
'cancelled_at' => $endedAt,
]);
}
/** The event Stripe raises when a term rolls over and is paid. */
function cyclePaid(string $invoiceId, string $periodStart, string $periodEnd): array
{
return [
'id' => 'evt_'.$invoiceId,
'type' => 'invoice.paid',
'data' => ['object' => [
'id' => $invoiceId,
'subscription' => 'sub_ended',
'amount_paid' => 21480,
'total' => 21480,
'billing_reason' => 'subscription_cycle',
'period_start' => Carbon::parse($periodStart)->timestamp,
'period_end' => Carbon::parse($periodEnd)->timestamp,
]],
];
}
it('issues no invoice and uses no number for a term that begins after the contract ended', function () {
Mail::fake();
$contract = endedContract();
// Stripe charged for April, and the contract ended on the first of April.
// That is a month the customer does not have.
$this->postJson(route('webhooks.stripe'), cyclePaid('in_after', '2026-04-01 09:00:00', '2026-05-01 09:00:00'))
->assertOk();
// No document, and no mail carrying one.
expect(Invoice::query()->count())->toBe(0);
Mail::assertNothingQueued();
// The money moved all the same, and the register is where an operator finds
// the payments that have to go back — so it IS recorded. What must not
// happen is a lawful invoice being written for a service that no longer
// exists.
expect(SubscriptionRecord::query()
->where('subscription_id', $contract->id)
->where('event', SubscriptionRecord::EVENT_RENEWAL)
->count())->toBe(1);
// And the number was not merely unused, it was never drawn: the next
// legitimate document is still the first of the year. A gapless series
// cannot afford a number that went nowhere.
$final = cyclePaid('in_final', '2026-03-01 09:00:00', '2026-04-01 09:00:00');
$this->postJson(route('webhooks.stripe'), $final)->assertOk();
expect(Invoice::query()->sole()->number)->toBe('RE-2026-0001');
});
it('still issues the final invoice for a term the customer really did use', function () {
Mail::fake();
$contract = endedContract();
// The one case refusing everything would lose. The cycle invoice for March
// went unpaid, Stripe's dunning ran, the contract ended on the first of
// April in the middle of it — and then the customer paid. That money is for
// a month they had a running cloud in, and they are owed the document for it.
$this->travelTo('2026-04-05 11:00:00');
$this->postJson(route('webhooks.stripe'), cyclePaid('in_march', '2026-03-01 09:00:00', '2026-04-01 09:00:00'))
->assertOk();
$invoice = Invoice::query()->sole();
expect($invoice->stripe_invoice_id)->toBe('in_march')
->and($invoice->subscription_id)->toBe($contract->id)
->and($invoice->number)->toBe('RE-2026-0001')
// Issued AND sent: the customer gets the paper, not just a row.
->and($invoice->sent_at)->not->toBeNull();
Mail::assertQueued(InvoiceMail::class);
});

View File

@ -333,6 +333,98 @@ it('cancels the invoice with its own document and never reuses a number', functi
->toThrow(RuntimeException::class);
});
it('tells Stripe to stop charging immediately', function () {
$stripe = withdrawalStripe();
[$contract] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]);
// The call nothing in this application used to be able to make. Without it a
// withdrawal ended the service, sent the money back — and left the card being
// charged every month for a machine that had been switched off.
$contract->update(['stripe_subscription_id' => 'sub_withdraw']);
Carbon::setTestNow('2026-03-08 09:00:00');
app(WithdrawContract::class)($contract->refresh());
expect($stripe->cancellations)->toHaveCount(1)
->and($stripe->cancellations[0]['subscription'])->toBe('sub_withdraw')
// IMMEDIATELY, which is the whole difference from a customer's own
// cancellation: a withdrawal unwinds the contract and the entire amount
// has just gone back, so there is no paid-up term left to use up.
->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_IMMEDIATELY)
->and($contract->refresh()->withdrawal_refund_error)->toBeNull();
});
it('refunds and cancels every charge inside the window, not only the opening one', function () {
$stripe = withdrawalStripe();
[$contract, $opening] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]);
$contract->update(['stripe_subscription_id' => 'sub_withdraw']);
// Day three: the customer books a storage pack, Stripe prorates it and takes
// 12,00 € there and then. Its document points at the CONTRACT and carries no
// `order_id` — which is exactly why the old code, which looked the document up
// by `invoices.order_id`, could not see it at all. It was neither cancelled
// nor refunded, and the owner's rule is that a withdrawing consumer gets
// everything back.
Carbon::setTestNow('2026-03-04 09:00:00');
$module = app(IssueInvoice::class)->forStripeInvoice(
subscription: $contract->refresh(),
stripeInvoiceId: 'in_module',
lines: [[
'id' => 'il_module',
'amount' => 1200,
'quantity' => 1,
'price' => ['id' => 'price_storage', 'metadata' => ['addon' => 'storage']],
]],
);
// A different charge is a different payment, and Stripe refunds a PAYMENT.
$stripe->invoicePayments['in_module'] = 'pi_module';
expect($module->order_id)->toBeNull()
->and($module->subscription_id)->toBe($contract->id)
->and($module->gross_cents)->toBe(1200);
Carbon::setTestNow('2026-03-08 09:00:00');
app(WithdrawContract::class)($contract->refresh());
// A Storno per document, because that is what a Storno is: it mirrors ONE
// invoice and carries its number. Both originals keep theirs for ever.
$stornos = Invoice::query()->whereNotNull('cancels_invoice_id')->orderBy('id')->get();
expect($stornos)->toHaveCount(2)
->and($stornos->pluck('cancels_invoice_id')->all())->toBe([$opening->id, $module->id])
->and($stornos->pluck('gross_cents')->all())->toBe([-5880, -1200]);
// And the money: one refund per payment, each against the payment that took
// it. One refund of 70,80 € against the opening PaymentIntent is what Stripe
// would have refused for exceeding the charge.
expect($stripe->refunds)->toHaveCount(2)
->and($stripe->refunds[0]['payment'])->toBe('pi_withdraw')
->and($stripe->refunds[0]['amount'])->toBe(5880)
->and($stripe->refunds[1]['payment'])->toBe('pi_module')
->and($stripe->refunds[1]['amount'])->toBe(1200)
// Distinct idempotency keys. One key for the whole withdrawal would have
// Stripe replay the first refund's answer for the second, and the module
// charge would silently never go back.
->and($stripe->refunds[0]['key'])->not->toBe($stripe->refunds[1]['key']);
expect($contract->refresh()->withdrawal_refund_cents)->toBe(7080)
->and($contract->withdrawal_refund_error)->toBeNull();
// The register states what actually went back, so it agrees with the two
// cancellation documents rather than with the opening one alone.
$entry = SubscriptionRecord::query()
->where('subscription_id', $contract->id)
->where('event', SubscriptionRecord::EVENT_WITHDRAWAL)
->firstOrFail();
expect($entry->snapshot['withdrawal']['refunded_gross_cents'])->toBe(7080);
});
it('records the withdrawal in the proof register', function () {
withdrawalStripe();
[$contract] = withdrawableContract(['customer_type' => Customer::TYPE_CONSUMER]);

View File

@ -3,6 +3,9 @@
use App\Models\Host;
use App\Models\ProvisioningRun;
use App\Models\RunResource;
use App\Services\Secrets\SecretCipher;
use Illuminate\Contracts\Encryption\DecryptException;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
@ -13,13 +16,34 @@ it('auto-assigns a uuid and routes by it', function () {
->and($host->getRouteKeyName())->toBe('uuid');
});
it('encrypts api_token_ref at rest but reads it back in clear', function () {
it('encrypts api_token_ref under SECRETS_KEY, not APP_KEY', function () {
// The column used Laravel's `encrypted` cast, i.e. APP_KEY — against
// SecretVault's own written rule, which exists because rotating APP_KEY is
// ordinary maintenance. One rotation would have made every host's Proxmox
// token undecryptable in the same moment, discovered when provisioning
// stopped rather than when the key changed.
config()->set('admin_access.secrets_key', 'base64:'.base64_encode(random_bytes(32)));
$host = Host::factory()->create(['api_token_ref' => 'automation@pve!clupilot=s3cr3t']);
$raw = DB::table('hosts')->where('id', $host->id)->value('api_token_ref');
$raw = (string) DB::table('hosts')->where('id', $host->id)->value('api_token_ref');
expect($raw)->not->toContain('s3cr3t')
->and($host->fresh()->api_token_ref)->toBe('automation@pve!clupilot=s3cr3t');
->and($host->fresh()->api_token_ref)->toBe('automation@pve!clupilot=s3cr3t')
// The mutation that matters: APP_KEY must NOT open it.
->and(fn () => Crypt::decryptString($raw))->toThrow(DecryptException::class)
->and(app(SecretCipher::class)->decrypt($raw))->toBe('automation@pve!clupilot=s3cr3t');
});
it('leaves a host without a token alone rather than needing a key for it', function () {
// Most hosts have no token for most of onboarding, and a model that reached
// for SECRETS_KEY to read a null column would make an unconfigured
// installation unable to so much as list its hosts.
config()->set('admin_access.secrets_key', '');
$host = Host::factory()->create();
expect($host->fresh()->api_token_ref)->toBeNull();
});
it('casts run context to an array and resolves the polymorphic subject', function () {

View File

@ -6,7 +6,10 @@ use App\Models\Customer;
use App\Models\Host;
use App\Models\Instance;
use App\Models\Order;
use App\Models\Subscription;
use App\Models\User;
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\StripeClient;
use App\Support\ProvisioningSettings;
use Livewire\Livewire;
@ -158,7 +161,25 @@ it('is safe to run twice', function () {
});
it('carries the whole way from the customer cancelling to the address going away', function () {
['services' => $services, 'instance' => $instance] = routedInstance();
$this->travelTo('2026-05-10 09:00:00');
['services' => $services, 'instance' => $instance, 'order' => $order] = routedInstance();
$stripe = new FakeStripeClient;
app()->instance(StripeClient::class, $stripe);
$contract = Subscription::query()->where('order_id', $order->id)->sole();
$contract->update([
'stripe_subscription_id' => 'sub_journey',
// The boundary the customer has paid to, and the one Stripe pushes
// forward on every renewal. Written as a literal date on purpose: this
// test used to travel to whatever `service_ends_at` the implementation
// had just computed, which made it recompute the code instead of
// checking it — a monthly answer on a yearly contract passed as happily
// as the right one.
'current_period_end' => '2026-08-01 09:00:00',
]);
$user = User::factory()->create(['email_verified_at' => now()]);
Customer::query()->whereKey($instance->customer_id)->update(['user_id' => $user->id, 'email' => $user->email]);
@ -170,14 +191,29 @@ it('carries the whole way from the customer cancelling to the address going away
$instance->refresh();
expect($instance->status)->toBe('cancellation_scheduled')
->and($instance->service_ends_at)->not->toBeNull();
->and($instance->service_ends_at->toDateTimeString())->toBe('2026-08-01 09:00:00');
// Still theirs, still served, for the whole of the term they bought.
// The billing half, which was the broken half: the money stops too. Nothing
// in this application could cancel a Stripe subscription at all, so a
// customer whose address came down here went on being charged every month.
expect($stripe->cancellations)->toHaveCount(1)
->and($stripe->cancellations[0]['subscription'])->toBe('sub_journey')
->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END)
->and($contract->fresh()->cancel_requested_at?->toDateTimeString())->toBe('2026-05-10 09:00:00')
// Still active: until the first of August this is a paying customer.
->and($contract->fresh()->status)->toBe('active');
// Still theirs, still served, for the whole of the term they bought — an hour
// before it runs out as much as on the day they cancelled.
$this->artisan('clupilot:end-due-services')->assertSuccessful();
expect($services['traefik']->routes)->toHaveKey('berger');
$this->travelTo('2026-08-01 08:00:00');
$this->artisan('clupilot:end-due-services')->assertSuccessful();
expect($services['traefik']->routes)->toHaveKey('berger');
// And gone the moment it is genuinely over.
$this->travelTo($instance->service_ends_at->copy()->addMinute());
$this->travelTo('2026-08-01 09:01:00');
$this->artisan('clupilot:end-due-services')->assertSuccessful();
expect($services['traefik']->routes)->toBe([])

View File

@ -8,6 +8,17 @@ use App\Provisioning\Steps\Host\ConfigureWireguard;
use App\Services\Ssh\CommandResult;
use Illuminate\Support\Facades\Queue;
/*
| Both tests below drive the state machine against a fake whose default result is
| success, so what they prove is SEQUENCING that the steps run in order, that
| the run reaches `completed`, and that external resources are created exactly
| once across a crash. They deliberately do not stand in for the per-step
| assertions in HostStepsTest: no command's content is checked here, and a step
| that stopped doing its work while still advancing would pass both.
*/
beforeEach(fn () => config()->set('admin_access.secrets_key', 'base64:'.base64_encode(random_bytes(32))));
it('drives a fresh host all the way to active (mocked)', function () {
Queue::fake(); // drive the runner manually, one step per advance
$s = fakeServices();
@ -16,6 +27,16 @@ it('drives a fresh host all the way to active (mocked)', function () {
$s['shell']->script('pveum user token add', CommandResult::success(
json_encode(['full-tokenid' => 'automation@pve!clupilot', 'value' => 'tok-secret-123'])
));
// What a healthy host answers to the checks the pipeline now actually makes:
// a Debian release Proxmox publishes a suite for, a /boot it can reboot into,
// a datacenter firewall that reads back as enabled, and a golden template to
// clone from. None of these is scenery — every one of them is a step that
// fails loudly rather than advancing on the fake's default success.
$s['shell']->script('/etc/os-release', CommandResult::success("debian|trixie|Debian GNU/Linux 13 (trixie)\n"));
$s['shell']->script('ls -1 /boot/vmlinuz-*-pve', CommandResult::success("/boot/vmlinuz-6.8.12-4-pve\n"));
$s['shell']->script('ls -1 /boot/initrd.img-*-pve', CommandResult::success("/boot/initrd.img-6.8.12-4-pve\n"));
$s['shell']->script('pvesh get /cluster/firewall/options', CommandResult::success('{"enable":1}'));
$s['pve']->clonedVmids[] = 9000; // the template the seeded catalogue sells
$host = app(StartHostOnboarding::class)->run([
'name' => 'pve-fsn-9',
@ -69,6 +90,16 @@ it('does not duplicate external resources when a step re-runs after a crash', fu
$s['shell']->script('pveum user token add', CommandResult::success(
json_encode(['full-tokenid' => 'automation@pve!clupilot', 'value' => 'tok-secret-123'])
));
// What a healthy host answers to the checks the pipeline now actually makes:
// a Debian release Proxmox publishes a suite for, a /boot it can reboot into,
// a datacenter firewall that reads back as enabled, and a golden template to
// clone from. None of these is scenery — every one of them is a step that
// fails loudly rather than advancing on the fake's default success.
$s['shell']->script('/etc/os-release', CommandResult::success("debian|trixie|Debian GNU/Linux 13 (trixie)\n"));
$s['shell']->script('ls -1 /boot/vmlinuz-*-pve', CommandResult::success("/boot/vmlinuz-6.8.12-4-pve\n"));
$s['shell']->script('ls -1 /boot/initrd.img-*-pve', CommandResult::success("/boot/initrd.img-6.8.12-4-pve\n"));
$s['shell']->script('pvesh get /cluster/firewall/options', CommandResult::success('{"enable":1}'));
$s['pve']->clonedVmids[] = 9000; // the template the seeded catalogue sells
$host = app(StartHostOnboarding::class)->run([
'name' => 'pve-fsn-10',

View File

@ -3,6 +3,7 @@
use App\Models\Datacenter;
use App\Models\Host;
use App\Models\Operator;
use App\Models\PlanVersion;
use App\Models\ProvisioningRun;
use App\Models\RunResource;
use App\Provisioning\Jobs\PurgeHost;
@ -19,8 +20,10 @@ use App\Provisioning\Steps\Host\RegisterHostDns;
use App\Provisioning\Steps\Host\SecureHostFirewall;
use App\Provisioning\Steps\Host\ValidateHostInput;
use App\Provisioning\Steps\Host\VerifyProxmoxApi;
use App\Provisioning\Steps\Host\VerifyVmTemplate;
use App\Services\Secrets\SecretVault;
use App\Services\Ssh\CommandResult;
use App\Services\Ssh\FakeRemoteShell;
use Illuminate\Support\Facades\Crypt;
function hostRun(Host $host, array $context = []): ProvisioningRun
@ -28,6 +31,92 @@ function hostRun(Host $host, array $context = []): ProvisioningRun
return ProvisioningRun::factory()->forHost($host)->create(['context' => $context]);
}
/**
* The `wg_peer` breadcrumb ConfigureWireguard records after a handshake actually
* succeeds the one fact that means "SSH may use the tunnel".
*
* Every step from ConfigureWireguard onwards runs with this present in real life,
* so a test that exercises one of them without it is testing the pre-tunnel
* fallback path by accident. See HostStep::keyLogin().
*/
function proveTunnel(ProvisioningRun $run, Host $host): void
{
RunResource::create([
'run_id' => $run->id,
'host_id' => $host->id,
'kind' => 'wg_peer',
'external_id' => 'HOSTPUB=',
]);
}
/**
* What ConfigureProxmox needs from a healthy host: a vmbr0 bridge, and a
* datacenter firewall that reads back as enabled. The fake's default-success
* would otherwise report "pvesh set worked" and then hand back an empty options
* document, which the step correctly refuses to believe.
*/
function scriptHealthyProxmoxHost(FakeRemoteShell $shell): void
{
$shell->script('pvesh get /cluster/firewall/options', CommandResult::success('{"enable":1,"policy_in":"ACCEPT"}'));
}
/** A /boot RebootIntoPveKernel is willing to reboot into. */
function scriptBootableProxmoxKernel(FakeRemoteShell $shell): void
{
$shell->script('ls -1 /boot/vmlinuz-*-pve', CommandResult::success("/boot/vmlinuz-6.8.12-4-pve\n"));
$shell->script('ls -1 /boot/initrd.img-*-pve', CommandResult::success("/boot/initrd.img-6.8.12-4-pve\n"));
}
/** A Debian release InstallProxmoxVe knows a Proxmox suite for. */
function scriptDebianRelease(FakeRemoteShell $shell, string $id, string $codename, string $prettyName): void
{
$shell->script('/etc/os-release', CommandResult::success($id.'|'.$codename.'|'.$prettyName."\n"));
}
/**
* Every ACCEPT rule in an nftables ruleset, one per entry, comments stripped.
*
* Reading the ruleset as a LIST rather than searching it for expected lines is
* what lets a test catch a rule somebody ADDED which is the mutation the old
* firewall tests missed: they asserted the WireGuard-scoped accept was present
* and said nothing about what else might be.
*
* @return array<int, string>
*/
function nftAcceptRules(string $config): array
{
$rules = [];
foreach (preg_split('/\R/', $config) ?: [] as $line) {
$line = trim((string) preg_replace('/\s+/', ' ', $line));
if ($line === '' || str_starts_with($line, '#') || ! str_contains($line, 'accept')) {
continue;
}
$rules[] = $line;
}
return $rules;
}
/**
* The destination ports an nftables ACCEPT rule opens both spellings, a set
* (`dport { 80, 443 }`) and a bare port (`dport 68`).
*
* @return array<int, int>
*/
function nftAcceptedPorts(string $rule): array
{
if (! preg_match('/dport (?:\{([^}]*)\}|(\d+))/', $rule, $m)) {
return [];
}
$list = trim($m[1] ?? '') !== '' ? $m[1] : ($m[2] ?? '');
return array_map('intval', array_filter(array_map('trim', explode(',', $list)), fn ($p) => $p !== ''));
}
// --- ValidateHostInput ---
it('advances on valid host input and marks the host onboarding', function () {
@ -78,15 +167,18 @@ it('throws when the host is unreachable (runner turns it into a retry)', functio
// --- HostStep::keyLogin address preference ---
//
// Mutation boundary: every step after ConfigureWireguard (and every later
// maintenance run) MUST reach the host over the tunnel once wg_ip is set —
// closing SSH to the world without this would strand the host permanently.
// ConfigureProxmox is used as a plain probe: keyLogin then two commands, no
// other branching that could obscure which address was actually dialled.
// maintenance run) MUST reach the host over the tunnel once the tunnel is
// PROVEN — closing SSH to the world without this would strand the host
// permanently. And it must NOT use the tunnel before then, which is the other
// half and the one that was broken. ConfigureProxmox is used as a plain probe:
// keyLogin then a handful of commands, no branching on the address.
it('uses the WireGuard address for ssh once the tunnel exists', function () {
it('uses the WireGuard address for ssh once the tunnel is proven', function () {
$s = fakeServices();
scriptHealthyProxmoxHost($s['shell']);
$host = Host::factory()->create(['public_ip' => '203.0.113.20', 'wg_ip' => '10.66.0.20']);
$run = hostRun($host);
proveTunnel($run, $host);
app(ConfigureProxmox::class)->execute($run);
@ -99,6 +191,7 @@ it('uses the WireGuard address for ssh once the tunnel exists', function () {
it('falls back to the public IP before the tunnel exists', function () {
$s = fakeServices();
scriptHealthyProxmoxHost($s['shell']);
$host = Host::factory()->create(['public_ip' => '203.0.113.21', 'wg_ip' => null]);
$run = hostRun($host);
@ -111,6 +204,47 @@ it('falls back to the public IP before the tunnel exists', function () {
}
});
it('never dials the tunnel for ssh while it is only allocated, not proven', function () {
// The dead end this closes: wg_ip is persisted inside the allocation lock
// BEFORE the handshake is ever verified, so a run that failed at the
// handshake left a host whose row said "10.66.0.x" and whose tunnel did not
// work. Every later attempt — including an operator pressing Retry —
// connected to that address and died before reaching the only code that
// could have repaired wg0.conf. Recovery meant nulling hosts.wg_ip by hand.
$s = fakeServices();
scriptHealthyProxmoxHost($s['shell']);
$host = Host::factory()->create(['public_ip' => '203.0.113.22', 'wg_ip' => '10.66.0.22']);
$run = hostRun($host);
// No wg_peer breadcrumb: an address was reserved, nothing handshaked.
app(ConfigureProxmox::class)->execute($run);
$connections = $s['shell']->connectionsWith('key');
expect($connections)->not->toBeEmpty();
foreach ($connections as $connection) {
expect($connection[1])->toBe('203.0.113.22')
->and($connection[1])->not->toBe('10.66.0.22');
}
});
it('keeps using the tunnel on a maintenance run started long after onboarding', function () {
// tunnelProven() is HOST-scoped, not run-scoped, on purpose: a later run has
// no wg_peer breadcrumb of its own, and reading only its own run would send
// it to a public IP where SecureHostFirewall has since closed port 22.
$s = fakeServices();
scriptHealthyProxmoxHost($s['shell']);
$host = Host::factory()->create(['public_ip' => '203.0.113.23', 'wg_ip' => '10.66.0.23']);
$onboarding = hostRun($host);
proveTunnel($onboarding, $host);
$later = hostRun($host);
app(ConfigureProxmox::class)->execute($later);
foreach ($s['shell']->connectionsWith('key') as $connection) {
expect($connection[1])->toBe('10.66.0.23');
}
});
it('prefers the vault-stored SSH key over CLUPILOT_SSH_PRIVATE_KEY', function () {
// Mutation boundary (Part B): the SSH identity is the most sensitive
// value in the system — it opens every host CluPilot manages. Before
@ -122,6 +256,7 @@ it('prefers the vault-stored SSH key over CLUPILOT_SSH_PRIVATE_KEY', function ()
app(SecretVault::class)->put('ssh.private_key', 'vault-key-should-be-used', Operator::factory()->create());
$s = fakeServices();
scriptHealthyProxmoxHost($s['shell']);
$run = hostRun(Host::factory()->create());
app(ConfigureProxmox::class)->execute($run);
@ -179,6 +314,103 @@ it('retries wireguard while the handshake is not up', function () {
expect(app(ConfigureWireguard::class)->execute($run)->type)->toBe('retry');
});
it('refuses to write wg0.conf without a hub public key, before touching anything', function () {
// renderConfig() used to emit `PublicKey = ` quite happily, which is the
// likeliest way to reach a tunnel that can never handshake in the first
// place. Nothing may be installed, allocated or written on the way to
// finding out — that is what makes it a clean report rather than another
// half-configured host.
$s = fakeServices();
$s['hub']->publicKeyValue = '';
$host = Host::factory()->create();
$run = hostRun($host);
$result = app(ConfigureWireguard::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('CLUPILOT_WG_HUB_PUBKEY')
->and($s['shell']->ran('apt-get install -y wireguard'))->toBeFalse()
->and($s['shell']->files())->not->toHaveKey('/etc/wireguard/wg0.conf')
->and($host->fresh()->wg_ip)->toBeNull();
});
it('refuses to write wg0.conf without a hub endpoint', function () {
$s = fakeServices();
$s['hub']->endpointValue = '';
$run = hostRun(Host::factory()->create());
$result = app(ConfigureWireguard::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('CLUPILOT_WG_ENDPOINT');
});
it('enables wg-quick@wg0 so the tunnel comes back after the reboot', function () {
// The line this replaces was `systemctl enable --now wg-quick@wg0 ||
// wg-quick up wg0`. Its fallback brought the interface up WITHOUT the
// systemd enablement, so the tunnel did not return after the step-6 reboot —
// and step 6 is the reboot this same pipeline performs two steps later.
$s = fakeServices();
$s['shell']->script('wg pubkey', CommandResult::success('HOSTPUB='));
$s['shell']->script('ip link show wg0', CommandResult::failure(1)); // fresh host
$s['shell']->script('systemctl is-enabled', CommandResult::failure(1)); // never enabled
$run = hostRun(Host::factory()->create());
expect(app(ConfigureWireguard::class)->execute($run)->type)->toBe('advance')
->and($s['shell']->ran('systemctl enable wg-quick@wg0'))->toBeTrue()
->and($s['shell']->ran('systemctl start wg-quick@wg0'))->toBeTrue()
// The old fallback: an interface up outside systemd, lost on reboot.
->and($s['shell']->ran('wg-quick up wg0'))->toBeFalse();
});
it('does not fight an interface that is already up and correct', function () {
// Both halves of the old line failed with "wg0 already exists" on the second
// attempt, so the step retried forever against a tunnel that was working.
$s = fakeServices();
$s['shell']->script('wg pubkey', CommandResult::success('HOSTPUB='));
$s['shell']->script('ip link show wg0', CommandResult::success('3: wg0: <POINTOPOINT,UP>'));
$s['shell']->script('systemctl is-enabled', CommandResult::success('enabled'));
$host = Host::factory()->create();
$run = hostRun($host);
// First pass writes the config, on a shell we then throw away.
app(ConfigureWireguard::class)->execute($run);
$written = $s['shell']->files()['/etc/wireguard/wg0.conf'];
// Attempt 2 against exactly that file, on a fresh shell so `recorded` only
// holds this attempt. The breadcrumb is rewound so the step really re-runs
// its body instead of short-circuiting — the case that used to retry forever.
RunResource::where('run_id', $run->id)->where('kind', 'wg_peer')->delete();
$again = fakeServices();
$again['shell']->script('wg pubkey', CommandResult::success('HOSTPUB='));
$again['shell']->script('ip link show wg0', CommandResult::success('3: wg0: <POINTOPOINT,UP>'));
$again['shell']->script('systemctl is-enabled', CommandResult::success('enabled'));
$again['shell']->script("cat '/etc/wireguard/wg0.conf'", CommandResult::success($written));
expect(app(ConfigureWireguard::class)->execute($run->fresh())->type)->toBe('advance')
->and($again['shell']->ran('systemctl restart wg-quick@wg0'))->toBeFalse()
// Nor is the file rewritten for nothing.
->and($again['shell']->files())->not->toHaveKey('/etc/wireguard/wg0.conf');
});
it('actually applies a corrected hub key instead of only writing the file', function () {
// An operator who fixes a wrong hub key and retries used to see the new file
// on disk and no change in behaviour: nothing reloaded the interface, so the
// running tunnel kept the old peer.
$s = fakeServices();
$s['shell']->script('wg pubkey', CommandResult::success('HOSTPUB='));
$s['shell']->script('ip link show wg0', CommandResult::success('3: wg0: <POINTOPOINT,UP>'));
$s['shell']->script('systemctl is-enabled', CommandResult::success('enabled'));
$s['shell']->script('cat \'/etc/wireguard/wg0.conf\'', CommandResult::success(
"[Interface]\nAddress = 10.66.0.9/24\n\n[Peer]\nPublicKey = THEWRONGKEY=\n"
));
$run = hostRun(Host::factory()->create());
expect(app(ConfigureWireguard::class)->execute($run)->type)->toBe('advance')
->and($s['shell']->files()['/etc/wireguard/wg0.conf'])->toContain($s['hub']->publicKeyValue)
->and($s['shell']->ran('systemctl restart wg-quick@wg0'))->toBeTrue();
});
// --- InstallProxmoxVe ---
it('skips the install when proxmox-ve is already present', function () {
@ -190,19 +422,93 @@ it('skips the install when proxmox-ve is already present', function () {
->and($s['shell']->ran('apt-get -y install proxmox-ve'))->toBeFalse();
});
it('installs proxmox-ve when it is absent', function () {
it('installs proxmox-ve against the suite the host actually runs', function () {
// A server ordered today boots Debian 13 trixie. The step used to hard-code
// bookworm, i.e. add the PVE 8 repo and keyring to a trixie base — a
// hypervisor built against a different libc and kernel.
$s = fakeServices();
$s['shell']->script('dpkg -l proxmox-ve', CommandResult::failure(1));
scriptDebianRelease($s['shell'], 'debian', 'trixie', 'Debian GNU/Linux 13 (trixie)');
$run = hostRun(Host::factory()->create());
expect(app(InstallProxmoxVe::class)->execute($run)->type)->toBe('advance')
->and($s['shell']->ran('apt-get -y install proxmox-ve'))->toBeTrue();
$sources = $s['shell']->files()['/etc/apt/sources.list.d/pve-install-repo.sources'];
expect($sources)->toContain('Suites: trixie')
->and($sources)->not->toContain('bookworm')
// Scoped to this repository via Signed-By, not trusted for every repo on
// the machine the way /etc/apt/trusted.gpg.d would be.
->and($sources)->toContain('Signed-By: /usr/share/keyrings/proxmox-archive-keyring.gpg')
->and($s['shell']->ran('proxmox-archive-keyring-trixie.gpg'))->toBeTrue()
->and($s['shell']->ran('/etc/apt/trusted.gpg.d/'))->toBeFalse();
});
it('installs the bookworm suite on a bookworm host', function () {
$s = fakeServices();
$s['shell']->script('dpkg -l proxmox-ve', CommandResult::failure(1));
scriptDebianRelease($s['shell'], 'debian', 'bookworm', 'Debian GNU/Linux 12 (bookworm)');
$run = hostRun(Host::factory()->create());
expect(app(InstallProxmoxVe::class)->execute($run)->type)->toBe('advance');
$sources = $s['shell']->files()['/etc/apt/sources.list.d/pve-install-repo.sources'];
expect($sources)->toContain('Suites: bookworm')
->and($sources)->not->toContain('trixie')
->and($sources)->toContain('Signed-By: /usr/share/keyrings/proxmox-release-bookworm.gpg')
->and($s['shell']->ran('proxmox-release-bookworm.gpg'))->toBeTrue();
});
it('refuses an unknown release by name rather than guessing a suite', function () {
$s = fakeServices();
$s['shell']->script('dpkg -l proxmox-ve', CommandResult::failure(1));
scriptDebianRelease($s['shell'], 'debian', 'forky', 'Debian GNU/Linux 14 (forky)');
$run = hostRun(Host::factory()->create());
$result = app(InstallProxmoxVe::class)->execute($run);
expect($result->type)->toBe('fail')
// Names what it found, so an operator does not have to go and look.
->and($result->reason)->toContain('forky')
->and($result->reason)->toContain('Debian GNU/Linux 14 (forky)')
// And nothing was installed on the guess that newest-is-closest.
->and($s['shell']->ran('apt-get -y install proxmox-ve'))->toBeFalse()
->and($s['shell']->files())->not->toHaveKey('/etc/apt/sources.list.d/pve-install-repo.sources');
});
it('refuses a base system that is not Debian at all', function () {
$s = fakeServices();
$s['shell']->script('dpkg -l proxmox-ve', CommandResult::failure(1));
scriptDebianRelease($s['shell'], 'ubuntu', 'noble', 'Ubuntu 24.04.1 LTS');
$run = hostRun(Host::factory()->create());
$result = app(InstallProxmoxVe::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('ubuntu')
->and($s['shell']->ran('apt-get -y install proxmox-ve'))->toBeFalse();
});
it('refuses an image whose os-release names no codename', function () {
// Debian unstable has no VERSION_CODENAME, so this is the branch that must
// not read "" as "close enough to the newest one I know".
$s = fakeServices();
$s['shell']->script('dpkg -l proxmox-ve', CommandResult::failure(1));
scriptDebianRelease($s['shell'], 'debian', '', 'Debian GNU/Linux trixie/sid');
$run = hostRun(Host::factory()->create());
$result = app(InstallProxmoxVe::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('(empty)')
->and($result->reason)->toContain('trixie/sid');
});
// --- RebootIntoPveKernel ---
it('issues the reboot on first execution', function () {
$s = fakeServices();
scriptBootableProxmoxKernel($s['shell']);
$run = hostRun(Host::factory()->create());
expect(app(RebootIntoPveKernel::class)->execute($run)->type)->toBe('poll')
@ -210,6 +516,51 @@ it('issues the reboot on first execution', function () {
->and($s['shell']->ran('reboot'))->toBeTrue();
});
it('refuses to reboot a host with no Proxmox kernel installed', function () {
// Mutation boundary: the reboot is the one irreversible thing this pipeline
// does. `apt-get remove linux-image-amd64 || true` followed by an unchecked
// update-grub could leave a machine that only the provider's console can
// reach — so nothing may be issued on a guess.
$s = fakeServices();
$s['shell']->script('ls -1 /boot/vmlinuz-*-pve', CommandResult::failure(2));
$run = hostRun(Host::factory()->create());
$result = app(RebootIntoPveKernel::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('vmlinuz-*-pve')
->and($s['shell']->ran('reboot'))->toBeFalse()
->and($run->fresh()->context('reboot_issued'))->toBeNull();
});
it('refuses to reboot when the Proxmox kernel has no initramfs', function () {
// What a /boot that filled up during the full-upgrade actually leaves behind.
$s = fakeServices();
$s['shell']->script('ls -1 /boot/vmlinuz-*-pve', CommandResult::success("/boot/vmlinuz-6.8.12-4-pve\n"));
$s['shell']->script('ls -1 /boot/initrd.img-*-pve', CommandResult::failure(2));
$run = hostRun(Host::factory()->create());
$result = app(RebootIntoPveKernel::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('initrd.img-*-pve')
->and($s['shell']->ran('reboot'))->toBeFalse();
});
it('refuses to reboot when update-grub failed', function () {
$s = fakeServices();
scriptBootableProxmoxKernel($s['shell']);
$s['shell']->script('update-grub', CommandResult::failure(1, 'cannot write /boot/grub/grub.cfg: No space left'));
$run = hostRun(Host::factory()->create());
$result = app(RebootIntoPveKernel::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('update-grub')
->and($result->reason)->toContain('No space left')
->and($s['shell']->ran('reboot'))->toBeFalse();
});
it('advances once the proxmox kernel is up', function () {
$s = fakeServices();
$s['shell']->script('uname -r', CommandResult::success('6.8.12-4-pve'));
@ -247,6 +598,7 @@ it('fails when the host does not return before the reboot deadline', function ()
// --- CreateAutomationToken ---
it('creates and persists the automation token via pveum exactly once', function () {
config()->set('admin_access.secrets_key', 'base64:'.base64_encode(random_bytes(32)));
$s = fakeServices();
$s['shell']->script('pveum user token add', CommandResult::success(
json_encode(['full-tokenid' => 'automation@pve!clupilot', 'value' => 'tok-secret-123'])
@ -268,6 +620,183 @@ it('creates and persists the automation token via pveum exactly once', function
->and(RunResource::where('run_id', $run->id)->where('kind', 'pve_token')->count())->toBe(1);
});
it('grants Sys.Modify, which every customer backup job needs', function () {
// POST /cluster/backup checks ['perm', '/', ['Sys.Modify']]. Without it the
// call comes back 403, RegisterBackup rethrows anything whose body does not
// contain "already", and EVERY customer provisioning run failed at
// register_backup — long after this host looked fine.
expect(config('provisioning.proxmox.role_privs'))->toContain('Sys.Modify');
});
it('converges the role privileges on a host that was onboarded before they changed', function () {
// `pveum role add … || true` only ever applied the privilege list on the run
// that first created the role, so a privilege added to the config reached new
// hosts and no existing one. The convergence must also sit BEFORE the token
// short-circuit, or a replay never reaches it.
config()->set('admin_access.secrets_key', 'base64:'.base64_encode(random_bytes(32)));
$s = fakeServices();
$s['shell']->script('pveum role add', CommandResult::failure(1, 'role CluPilotAutomation already exists'));
$host = Host::factory()->create(['api_token_ref' => 'automation@pve!clupilot=already-there']);
$run = hostRun($host);
RunResource::create([
'run_id' => $run->id, 'host_id' => $host->id, 'kind' => 'pve_token',
'external_id' => 'automation@pve!clupilot',
]);
expect(app(CreateAutomationToken::class)->execute($run)->type)->toBe('advance');
$modify = collect($s['shell']->recorded())->first(fn ($c) => str_contains($c, 'pveum role modify'));
expect($modify)->not->toBeNull()
->and($modify)->toContain('Sys.Modify')
// Replaces rather than appends: the host matches the config, not the
// accumulation of everything any past version of it once granted.
->and($modify)->not->toContain('-append')
// And the short-circuit still holds for the token itself.
->and($s['shell']->ran('pveum user token add'))->toBeFalse();
});
it('retries rather than minting a token the role privileges did not converge for', function () {
$s = fakeServices();
$s['shell']->script('pveum role', CommandResult::failure(1, 'permission denied'));
$run = hostRun(Host::factory()->create());
expect(app(CreateAutomationToken::class)->execute($run)->type)->toBe('retry')
->and($s['shell']->ran('pveum user token add'))->toBeFalse();
});
// --- ConfigureProxmox ---
it('refuses to finish a host that has no vmbr0 bridge', function () {
// Proxmox installed on top of Debian does not create vmbr0 — only the ISO
// installer does. The step used to run `ip link show vmbr0`, throw the result
// away, and advance, so onboarding reported `active` on a host with no bridge
// to attach a customer VM to.
$s = fakeServices();
$s['shell']->script('ip link show vmbr0', CommandResult::failure(1));
$s['shell']->script('ip -4 route show default', CommandResult::success('default via 203.0.113.1 dev enp0s31f6'));
$run = hostRun(Host::factory()->create());
$result = app(ConfigureProxmox::class)->execute($run);
expect($result->type)->toBe('fail')
->and($result->reason)->toContain('vmbr0')
// Tells the operator what to do, including which NIC carries the route.
->and($result->reason)->toContain('/etc/network/interfaces')
->and($result->reason)->toContain('enp0s31f6')
// And never tries to build the bridge itself: getting that wrong over the
// very link the shell runs on takes the machine off the network for good.
->and($s['shell']->ran('ifreload'))->toBeFalse()
->and($s['shell']->ran('brctl'))->toBeFalse();
});
it('enables the datacenter firewall without locking the host out', function () {
// Proxmox applies a guest's firewall rules only while the firewall is enabled
// at datacenter level, so applyFirewall()'s "80/443 only" rules were inert on
// every host. Enabling it blindly is the other trap: the datacenter input
// policy defaults to DROP, the node firewall defaults to ON, and the only
// sources it admits to 22/8006 come from an ipset seeded with the PUBLIC
// subnet — not the tunnel every later step uses.
$s = fakeServices();
scriptHealthyProxmoxHost($s['shell']);
$host = Host::factory()->create(['public_ip' => '203.0.113.40', 'wg_ip' => '10.66.0.40']);
$run = hostRun($host);
proveTunnel($run, $host);
expect(app(ConfigureProxmox::class)->execute($run)->type)->toBe('advance');
$commands = collect($s['shell']->recorded());
$policy = $commands->search(fn ($c) => str_contains($c, '--policy_in ACCEPT'));
$nodeOff = $commands->search(fn ($c) => str_contains($c, '/firewall/options --enable 0'));
$enable = $commands->search(fn ($c) => str_contains($c, '/cluster/firewall/options --enable 1'));
expect($policy)->not->toBeFalse()
->and($nodeOff)->not->toBeFalse()
->and($enable)->not->toBeFalse()
// Order is the safety property: both mitigations land before the master
// switch, so there is no window in which pve-firewall compiles a DROP
// policy that cannot see the tunnel.
->and($policy)->toBeLessThan($enable)
->and($nodeOff)->toBeLessThan($enable);
});
it('does not believe a datacenter firewall write it cannot read back', function () {
// /etc/pve is a replicated filesystem: `pvesh set` exiting 0 is not the same
// as the setting being there, and a lost write means every customer VM's
// rules go on being inert while onboarding reports success.
$s = fakeServices();
$s['shell']->script('pvesh get /cluster/firewall/options', CommandResult::success('{"enable":0}'));
$run = hostRun(Host::factory()->create());
expect(app(ConfigureProxmox::class)->execute($run)->type)->toBe('retry');
});
it('drops the enterprise repo the proxmox-ve package brings back with it', function () {
$s = fakeServices();
scriptHealthyProxmoxHost($s['shell']);
$run = hostRun(Host::factory()->create());
app(ConfigureProxmox::class)->execute($run);
expect($s['shell']->ran('pve-enterprise.list'))->toBeTrue()
// PVE 9 ships the deb822 spelling instead, and one of the two is always
// the file that is actually there.
->and($s['shell']->ran('pve-enterprise.sources'))->toBeTrue();
});
// --- VerifyVmTemplate ---
//
// The seeded catalogue every installation starts with points all four plans at
// template_vmid 9000, which is exactly the situation this step exists for.
it('refuses to finish a host that has no template to clone from', function () {
// A host used to go `active` while it could not serve a single customer:
// every plan version points at a template_vmid, nothing in the pipeline
// creates or verifies it, and there is no console command for it either — so
// the first PAID order died in clone_vm, after the money had changed hands.
$s = fakeServices();
$run = hostRun(Host::factory()->active()->create());
$result = app(VerifyVmTemplate::class)->execute($run);
expect(PlanVersion::query()->whereNotNull('published_at')->where('template_vmid', 9000)->exists())->toBeTrue()
->and($result->type)->toBe('fail')
->and($result->reason)->toContain('9000')
->and($result->reason)->toContain('clone_vm')
// It reports; it does not invent a golden image nobody chose.
->and($s['pve']->clonedVmids)->toBe([]);
});
it('advances once the template a published plan points at is on the node', function () {
$s = fakeServices();
$s['pve']->clonedVmids[] = 9000; // the fake's stand-in for "this VMID exists"
$run = hostRun(Host::factory()->active()->create());
expect(app(VerifyVmTemplate::class)->execute($run)->type)->toBe('advance');
});
it('still checks a version that is published but not on sale until next week', function () {
// PlanVersion::available() would exclude it, and its template gap would then
// surface at its first order rather than here — the whole failure this step
// removes. A CLOSED window is different: nobody can buy it any more.
$s = fakeServices();
PlanVersion::query()->update(['available_from' => now()->addWeek()]);
$run = hostRun(Host::factory()->active()->create());
expect(app(VerifyVmTemplate::class)->execute($run)->type)->toBe('fail');
PlanVersion::query()->update(['available_until' => now()->subDay()]);
expect(app(VerifyVmTemplate::class)->execute($run)->type)->toBe('advance')
->and($s['pve']->clonedVmids)->toBe([]);
});
it('requires no template on an installation that has not published a plan yet', function () {
fakeServices();
PlanVersion::query()->update(['published_at' => null]);
$run = hostRun(Host::factory()->active()->create());
expect(app(VerifyVmTemplate::class)->execute($run)->type)->toBe('advance');
});
// --- VerifyProxmoxApi ---
it('advances when the api returns nodes', function () {
@ -330,6 +859,7 @@ it('applies the firewall once the tunnel is confirmed up, over the tunnel itself
$s = fakeServices();
$host = Host::factory()->active()->create(['public_ip' => '203.0.113.30']);
$run = hostRun($host);
proveTunnel($run, $host);
expect(app(SecureHostFirewall::class)->execute($run)->type)->toBe('advance');
@ -346,15 +876,96 @@ it('applies the firewall once the tunnel is confirmed up, over the tunnel itself
->and($config)->toContain('ct state established,related accept');
expect($s['shell']->files())->toHaveKey('/usr/local/sbin/clupilot-emergency-open-firewall.sh')
->and($s['shell']->ran('chmod 700'))->toBeTrue()
->and($s['shell']->ran('/usr/local/sbin/clupilot-emergency-open-firewall.sh'))->toBeTrue()
->and($s['shell']->ran('chmod 700 \'/usr/local/sbin/clupilot-emergency-open-firewall.sh\''))->toBeTrue()
->and($s['shell']->ran('systemctl enable --now nftables'))->toBeTrue()
->and(RunResource::where('run_id', $run->id)->where('kind', 'host_firewall')->count())->toBe(1);
});
it('closes 8006 and 22 to everything that is not the management subnet', function () {
// The owner's stated requirement, and the half nothing asserted: the old
// tests only checked that the WireGuard-scoped ACCEPT was PRESENT, so a
// mutation adding a bare `tcp dport { 8006 } accept` passed the whole suite
// while handing every scanner on the internet a Proxmox login form.
//
// Read as "which destination ports does this ruleset accept from a source it
// has not restricted to the tunnel", rather than as a search for one
// expected line — that is what makes an ADDED rule fail it.
$s = fakeServices();
$host = Host::factory()->active()->create();
$run = hostRun($host);
proveTunnel($run, $host);
app(SecureHostFirewall::class)->execute($run);
$wgSubnet = (string) config('provisioning.wireguard.subnet');
$config = $s['shell']->files()['/etc/nftables.conf'];
$worldOpen = [];
$tunnelOnly = [];
foreach (nftAcceptRules($config) as $rule) {
// ICMP and the loopback/conntrack rules carry no destination port.
foreach (nftAcceptedPorts($rule) as $port) {
if (str_contains($rule, 'ip saddr '.$wgSubnet)) {
$tunnelOnly[] = $port;
} else {
$worldOpen[] = $port;
}
}
}
sort($worldOpen);
sort($tunnelOnly);
expect($tunnelOnly)->toBe([22, 8006])
// Exactly these and nothing else: public web, and the DHCP client reply
// below. 22 and 8006 must NOT appear here.
->and($worldOpen)->toBe([68, 80, 443])
->and($worldOpen)->not->toContain(8006)
->and($worldOpen)->not->toContain(22)
// A DHCP reply, not "anything to port 68".
->and($config)->toContain('udp sport 67 udp dport 68 accept')
// And the chain still ends in drop, so an unmatched packet is refused
// rather than accepted by omission.
->and($config)->toContain('policy drop')
->and($config)->not->toContain('policy accept');
});
it('accepts the ICMP a working host cannot do without', function () {
// policy drop with no ICMP accept at all — grepped, zero mentions. On a real
// host that breaks IPv6 outright as soon as the neighbour cache expires (NDP
// 135/136 and router advertisements arrive in the INPUT chain) and
// black-holes large transfers, because path-MTU discovery runs on
// frag-needed / packet-too-big. Debian's own example ruleset accepts both.
$s = fakeServices();
$host = Host::factory()->active()->create();
$run = hostRun($host);
proveTunnel($run, $host);
app(SecureHostFirewall::class)->execute($run);
$config = $s['shell']->files()['/etc/nftables.conf'];
$icmp = implode("\n", array_filter(
nftAcceptRules($config),
fn (string $rule) => str_contains($rule, 'icmp'),
));
expect($icmp)->toContain('nd-neighbor-solicit')
->and($icmp)->toContain('nd-neighbor-advert')
->and($icmp)->toContain('nd-router-advert')
// Path MTU discovery, both families.
->and($icmp)->toContain('packet-too-big')
->and($icmp)->toContain('destination-unreachable')
// Reachability, including the handshake check this very step performs.
->and($icmp)->toContain('echo-request')
// Still a policy-drop ruleset, not "ICMP made it open".
->and($config)->toContain('policy drop');
});
it("keeps the WireGuard interface's established connections working (never a bare drop-all)", function () {
$s = fakeServices();
$run = hostRun(Host::factory()->active()->create());
$host = Host::factory()->active()->create();
$run = hostRun($host);
proveTunnel($run, $host);
app(SecureHostFirewall::class)->execute($run);
@ -363,10 +974,53 @@ it("keeps the WireGuard interface's established connections working (never a bar
->and($config)->toContain('iif "lo" accept');
});
it('deploys an emergency script that never reopens the firewall by itself', function () {
// The owner's explicit requirement, and what nothing tested: the old
// assertion was `ran('/usr/local/sbin/clupilot-emergency-open-firewall.sh')`,
// which putFile() does not record — the only thing that substring matched was
// the `chmod 700` on the line above, so the same fact was asserted twice and
// the script's CONTENTS were never looked at.
$s = fakeServices();
$host = Host::factory()->active()->create();
$run = hostRun($host);
proveTunnel($run, $host);
app(SecureHostFirewall::class)->execute($run);
$script = $s['shell']->files()['/usr/local/sbin/clupilot-emergency-open-firewall.sh'];
// It does the one thing it exists for.
expect($script)->toStartWith('#!/bin/sh')
->and($script)->toContain('systemctl disable --now nftables')
->and($script)->toContain('nft flush ruleset');
// And nothing in it can fire without a human at the out-of-band console. A
// firewall that reopens itself under failure is not a firewall.
foreach (['sleep', 'systemd-run', 'OnCalendar', 'OnBootSec', '.timer', 'crontab', 'cron.d', 'at now'] as $selfTrigger) {
expect($script)->not->toContain($selfTrigger);
}
// Nothing detached into the background either, which is the other way a
// script leaves something running after the operator has walked away.
foreach (preg_split('/\R/', $script) ?: [] as $line) {
$line = trim($line);
if ($line === '' || str_starts_with($line, '#')) {
continue;
}
expect($line)->not->toEndWith('&');
}
// Nor does the step arrange one on its behalf.
expect($s['shell']->ran('systemd-run'))->toBeFalse()
->and($s['shell']->ran('crontab'))->toBeFalse()
->and($s['shell']->ran('.timer'))->toBeFalse();
});
it('never reapplies the firewall on a re-run', function () {
$s = fakeServices();
$host = Host::factory()->active()->create();
$run = hostRun($host);
proveTunnel($run, $host);
app(SecureHostFirewall::class)->execute($run);
app(SecureHostFirewall::class)->execute($run->fresh());

View File

@ -0,0 +1,97 @@
<?php
use App\Models\Host;
use App\Services\Secrets\SecretCipher;
use Illuminate\Contracts\Encryption\DecryptException;
use Illuminate\Support\Facades\Crypt;
use Illuminate\Support\Facades\DB;
/**
* Loads a fresh instance of the re-encryption migration every call.
*
* `require`, not `require_once`: PHP re-evaluates the `return new class …`
* expression each time, so every call gets its own instance. The real migrate
* run RefreshDatabase already performed saw an empty hosts table these tests
* put rows there first and then run it, which is the only way to reach the
* branches that matter.
*/
function loadHostTokenKeyMigration(): object
{
return require database_path('migrations/2026_07_31_160000_move_host_api_tokens_onto_the_secrets_key.php');
}
/** Writes a raw ciphertext past the model's mutator, the way an old row looks. */
function storeRawToken(Host $host, string $cipher): void
{
DB::table('hosts')->where('id', $host->id)->update(['api_token_ref' => $cipher]);
}
function rawToken(Host $host): ?string
{
$value = DB::table('hosts')->where('id', $host->id)->value('api_token_ref');
return $value === null ? null : (string) $value;
}
beforeEach(fn () => config()->set('admin_access.secrets_key', 'base64:'.base64_encode(random_bytes(32))));
it('moves a token written under APP_KEY onto SECRETS_KEY', function () {
$host = Host::factory()->create();
storeRawToken($host, Crypt::encryptString('automation@pve!clupilot=old-secret'));
loadHostTokenKeyMigration()->up();
$raw = (string) rawToken($host);
expect(app(SecretCipher::class)->decrypt($raw))->toBe('automation@pve!clupilot=old-secret')
// The point of the whole exercise: APP_KEY no longer opens it, so
// rotating APP_KEY cannot take every host's Proxmox access with it.
->and(fn () => Crypt::decryptString($raw))->toThrow(DecryptException::class)
->and($host->fresh()->api_token_ref)->toBe('automation@pve!clupilot=old-secret');
});
it('leaves a row it has already migrated exactly as it is', function () {
$host = Host::factory()->create(['api_token_ref' => 'automation@pve!clupilot=already-moved']);
$before = rawToken($host);
loadHostTokenKeyMigration()->up();
expect(rawToken($host))->toBe($before)
->and($host->fresh()->api_token_ref)->toBe('automation@pve!clupilot=already-moved');
});
it('never destroys a value neither key can read', function () {
// Mutation boundary: the plaintext is a credential that exists nowhere else.
// A token an operator can still recover by hand from an old APP_KEY is worth
// more than a tidy column, so the row is left alone and reported — never
// overwritten, never nulled.
$host = Host::factory()->create();
storeRawToken($host, 'eyJpdiI6IndyaXR0ZW4tdW5kZXItYS1sb25nLWdvbmUta2V5In0=');
loadHostTokenKeyMigration()->up();
expect(rawToken($host))->toBe('eyJpdiI6IndyaXR0ZW4tdW5kZXItYS1sb25nLWdvbmUta2V5In0=');
});
it('touches nothing at all when there is no key to re-encrypt with', function () {
$host = Host::factory()->create();
$appKeyCipher = Crypt::encryptString('automation@pve!clupilot=still-under-app-key');
storeRawToken($host, $appKeyCipher);
config()->set('admin_access.secrets_key', '');
// Does not throw: a fresh install has no SECRETS_KEY yet, and blocking the
// whole migration run on that would stop an installation that has nothing
// to migrate from being created at all.
loadHostTokenKeyMigration()->up();
expect(rawToken($host))->toBe($appKeyCipher);
});
it('puts the column back under APP_KEY on a rollback', function () {
$host = Host::factory()->create(['api_token_ref' => 'automation@pve!clupilot=roundtrip']);
loadHostTokenKeyMigration()->down();
expect(Crypt::decryptString((string) rawToken($host)))->toBe('automation@pve!clupilot=roundtrip');
});

View File

@ -1,10 +1,16 @@
<?php
use App\Mail\InvoiceMail;
use App\Mail\OrderConfirmationMail;
use App\Models\Customer;
use App\Models\Invoice;
use App\Models\Operator;
use App\Models\Order;
use App\Models\ProvisioningRun;
use App\Models\User;
use App\Provisioning\Jobs\AdvanceRunJob;
use App\Support\CompanyProfile;
use Illuminate\Support\Facades\Mail;
use Illuminate\Support\Facades\Queue;
function stripeEvent(string $id = 'evt_1', string $plan = 'team'): array
@ -127,6 +133,97 @@ it('still provisions a paid order when the customer email already belongs to an
Queue::assertPushed(AdvanceRunJob::class, 1);
});
/**
* A purchase whose worker was killed after the order committed.
*
* The order commits before the contract is opened and long before the invoice is
* issued, so that a failure in either can never erase the record of a payment. The
* window that leaves is real: a paid order, and nothing else. Built here through
* the same OpenSubscription the webhook itself calls, so what these tests start
* from is the interrupted state and not an approximation of it.
*/
function interruptedPurchase(string $sessionId, bool $withContract = true): Order
{
CompanyProfile::put([
'name' => 'CluPilot Cloud e.U.',
'address' => 'Dreherstraße 66/1/8',
'postcode' => '1110',
'city' => 'Wien',
'vat_id' => 'ATU00000000',
]);
$customer = Customer::factory()->create(['email' => 'kunde@example.com', 'name' => 'Kanzlei Berger']);
$factory = Order::factory();
return ($withContract ? $factory->withSubscription() : $factory)->create([
'customer_id' => $customer->id,
'plan' => 'team',
'amount_cents' => 21480,
'currency' => 'EUR',
'status' => 'paid',
'stripe_event_id' => $sessionId,
]);
}
it('finishes the invoice a crash interrupted, once, however often Stripe asks', function () {
Mail::fake();
Queue::fake();
// A paid order with its contract open and NO invoice: the state a worker
// killed between the order committing and confirmByMail() leaves behind.
// Nothing anywhere sweeps for that, so Stripe's own retry is the only chance
// this customer has of ever getting the invoice they paid for — and resume()
// used to see the contract, decide there was nothing to do, and return.
$order = interruptedPurchase('cs_evt_crash');
expect(Invoice::query()->count())->toBe(0);
$this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash'))->assertOk();
$invoice = Invoice::query()->sole();
expect($invoice->order_id)->toBe($order->id)
->and($invoice->gross_cents)->toBe(21480)
->and($invoice->number)->toBe('RE-'.now()->year.'-0001')
// Stamped, which is what makes the mail idempotent as well as the number.
->and($invoice->sent_at)->not->toBeNull();
Mail::assertQueued(InvoiceMail::class, 1);
// The invoice mail says everything the confirmation said and carries the
// document, so it replaces it. Two mails for one purchase is the defect this
// whole path was written around.
Mail::assertNotQueued(OrderConfirmationMail::class);
// Two more redeliveries change nothing. An invoice number can never be handed
// out twice, and nobody may receive the same invoice three times.
$this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash'))->assertOk();
$this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash'))->assertOk();
expect(Invoice::query()->count())->toBe(1)
->and(Invoice::query()->sole()->number)->toBe($invoice->number);
Mail::assertQueued(InvoiceMail::class, 1);
});
it('opens the contract and invoices when the crash came even earlier', function () {
Mail::fake();
Queue::fake();
// Killed before openContract() this time: a paid order with no contract at
// all. Both halves of the repair have to happen on the redelivery.
$order = interruptedPurchase('cs_evt_crash_early', withContract: false);
expect($order->subscription()->exists())->toBeFalse();
$this->postJson(route('webhooks.stripe'), stripeEvent('evt_crash_early'))->assertOk();
expect($order->fresh()->subscription()->exists())->toBeTrue()
->and(Invoice::query()->sole()->order_id)->toBe($order->id);
Mail::assertQueued(InvoiceMail::class, 1);
});
it('rejects an invalid signature when a secret is configured', function () {
config()->set('services.stripe.webhook_secret', 'whsec_test');

View File

@ -6,7 +6,10 @@ use App\Livewire\Settings;
use App\Models\Customer;
use App\Models\Instance;
use App\Models\Order;
use App\Models\Subscription;
use App\Models\User;
use App\Services\Stripe\FakeStripeClient;
use App\Services\Stripe\StripeClient;
use Livewire\Livewire;
function settingsSetup(bool $withInstance = true): array
@ -97,23 +100,54 @@ it('rejects an invalid brand colour', function () {
});
it('schedules package cancellation only with the correct typed confirmation', function () {
$stripe = new FakeStripeClient;
app()->instance(StripeClient::class, $stripe);
['user' => $user, 'customer' => $customer] = settingsSetup();
$instance = $customer->instances()->first();
// Wrong confirmation → no change.
// A YEARLY contract, because the assertion below is a date and a monthly
// assumption would land eleven months early. This test used to assert only
// that `service_ends_at` was not null — a column the code had just written,
// which the broken monthly arithmetic satisfied exactly as well as the right
// answer does.
$contract = Subscription::factory()->plan('team', Subscription::TERM_YEARLY)->create([
'customer_id' => $customer->id,
'order_id' => $instance->order_id,
'instance_id' => $instance->id,
'stripe_subscription_id' => 'sub_settings',
'current_period_start' => '2026-01-01 00:00:00',
'current_period_end' => '2027-01-01 00:00:00',
]);
// Wrong confirmation → no change, and Stripe is not told anything either.
Livewire::actingAs($user)->test(ConfirmCancelPackage::class)
->set('confirmName', 'wrong')
->call('cancelPackage')
->assertHasErrors(['confirmName']);
expect($instance->fresh()->status)->toBe('active');
expect($instance->fresh()->status)->toBe('active')
->and($stripe->cancellations)->toBeEmpty();
// Correct confirmation → scheduled.
Livewire::actingAs($user)->test(ConfirmCancelPackage::class)
->set('confirmName', 'acme')
->call('cancelPackage');
$instance->refresh();
expect($instance->status)->toBe('cancellation_scheduled')
->and($instance->service_ends_at)->not->toBeNull();
// The end of the term that was PAID FOR, stated as a date somebody chose.
->and($instance->service_ends_at->toDateTimeString())->toBe('2027-01-01 00:00:00');
// The contract knows it was cancelled — and is still active, because until
// the first of January this is a paying customer with a running cloud.
expect($contract->fresh()->cancel_requested_at)->not->toBeNull()
->and($contract->fresh()->status)->toBe('active');
// And Stripe was told to stop, at the period end and not now.
expect($stripe->cancellations)->toHaveCount(1)
->and($stripe->cancellations[0]['subscription'])->toBe('sub_settings')
->and($stripe->cancellations[0]['when'])->toBe(StripeClient::CANCEL_AT_PERIOD_END);
});
it('blocks closing the account while a package is active', function () {