Blog ·
A boundary belongs in the core
Revoking a hosted agent detached it from its order and left a running Proxmox machine nobody could reach and everybody kept paying for.
By Provibr
A Cloud Agent is the version of our agent where we run the machine. The customer orders one, we clone a Proxmox template, install the agent on it and bill for it monthly. From the panel it looks like any other agent, and that resemblance is the whole story of this round.
Every other agent belongs to the customer. Revoking it is a clean, safe operation: the platform sends a revoke notice, the agent wipes its identity, its certificate and its credential vault, and the row can then be deleted. Nothing is left running anywhere, because the thing that was running was never ours.
For a Cloud Agent that is not true, and nothing stopped it.
What we measured before changing anything
On the development database, with a real order behind a real agent, we called the remove path exactly as the panel button does.
It reported mode: "hard". The agent row was gone. The order row was still there, still active, still carrying vmid 5998, and its agent_id had quietly become NULL, because the foreign key from the order to the agent is ON DELETE SET NULL.
So there was a virtual machine on a hypervisor, running, with an agent on it that had wiped its own identity and could therefore never connect again, attached to an order that still generated a monthly charge, and no row anywhere that pointed from one to the other. Not reachable, not stoppable from the panel, and not free.
The constraint that produced this is correct. Setting the reference to NULL is the right behaviour for a column that says which agent is currently installed here; a cascade would have deleted the order and the billing record with it. The defect is that nothing asked the question the schema could not ask.
Two existing guards, both looking the wrong way
Deleting an agent already had two refusals, and it is worth being precise about why neither fired.
The first refuses to delete an agent that is currently enrolled: revoke it first, so it cleans itself up. By the time the row was being deleted the agent had just been revoked in the same call, so it was not enrolled and the check was satisfied.
The second refuses to delete an agent that still has integrations or servers hanging off it, because those foreign keys cascade and one click would take the whole inventory with them. A Cloud Agent has neither: it is a fresh machine that exists to run the agent, so the check passed because there was nothing to count.
Both guards were about what would be destroyed by the delete. The thing that survives the delete, and costs money precisely because it survives, was outside the shape of the question.
The boundary goes in the core, not in the button
The obvious repair is to check in the server action behind the button. It is also wrong, and the reason is a gap this codebase has walked into before with a stock authentication endpoint that bypassed every server side control.
We counted the call sites. Outside the two core functions there are seven calls to revoke, delete or the combined remove, spread over three files: three server actions on the agent page, the bulk action, and two v1 API routes. A check in the panel action would have left the API routes open, and an API route is exactly what a customer script uses.
So the boundary sits inside revokeAgentCore() and deleteAgentCore() themselves, the choke point all seven calls pass through. One function, asked twice.
It sits after the idempotent branch and not before. An agent that is already revoked has nothing left to revoke, and telling that caller to cancel their order first would be a falsehood about a call that is going to do nothing anyway. A boundary belongs on the operation that actually changes something.
The predicate is can this still be cancelled, not is this alive
There is a list in the shared package called CLOUD_AGENT_LIVE_STATUSES: the states in which a machine exists and costs money. It has four members and it is exactly the wrong list to reuse here.
The predicate we use is cloudAgentActionAllowed("cancel", status), which is true for seven of the eight order states, everything except an order that is already cancelled.
The three states in the difference are the interesting ones, and failed is the sharpest. A failed order costs nothing, so it is not live. But a failed order is precisely the one that may have got far enough to create a machine and then fallen over, which is why cancelling is allowed from it in the first place. Reusing the billing list would have left the one case with an orphaned machine unguarded, and it would have looked completely reasonable in review.
Two lists that mean nearly the same thing drift. The question here is not does this cost money, it is is cancelling still the route, and exactly one function in the codebase answers that one.
Two write paths that are allowed to skip it
The guard we wrote derives every place in the panel source that revokes, hard deletes or soft hides an agent row, using an AST walk over the application and library trees, and demands per site either the boundary or a written reason. Two sites carry a reason: the cancel path itself, which removes the machine, releases the address and only then revokes the agent, and account closure, which revokes every agent an organisation has and handles the Cloud Agent separately in the same flow. Blocking either would close a correct exit rather than save money.
The guard has the two sieves that keep a source scanner from being vacuously green: a lower bound on how many write sites it must find, three of them named, and a test that fails on an allowlist entry pointing at a file that no longer writes anything. It runs 4 tests and 11 assertions.
Two facts on the screen, not one
The panel side needed two separate booleans and we nearly shipped one.
orderOpen is a fact about the order: can this still be cancelled. canCancel is a fact about the viewer: may this person cancel it. Collapsing them into a single can act here would mean a member without the cancel permission sees the revoke button, uses it, throws the machine away and cannot stop the bill. The buttons therefore disappear on the order fact, and the card stays with the reason in it, because why can I not revoke this one is the question everybody asks who has seen the button on their other agents.
One thing came along because the change made it untrue: the uninstall instructions advised revoking the agent in the panel first. That is a button that no longer exists on this page.
What we would write down
- A resemblance in the interface is not a resemblance in the consequences. Two agents, one button, one of them costs money after you press it.
ON DELETE SET NULLcan be right and still leave you with an orphan. The schema cannot ask the question.- Guards about what a delete destroys will not see what a delete strands.
- Count the call sites before choosing where the check goes. Ours were seven, in three files, two of them API routes.
- Put the boundary at the choke point, not on the button. A button is one of the entrances.
- Put it after the idempotent branch. A refusal on a call that would do nothing is a lie.
- Reusing a list that means nearly the same thing is a slow failure. Costs money and can still be cancelled differ by three states, and the difference is where the damage is.
- A state that costs nothing can still have left something behind. A failed provision is the dangerous one.
- Separate the fact about the object from the fact about the viewer. Merged, they hand a limited member a destructive button.
- Let the guard read the tree. A test per known entrance cannot see the entrance somebody adds next month.
For clarity about what this is: the defect was reproduced against the development database before any change, the end to end check finished at 26 green and was proven red at 16 green and 10 failing beforehand, the browser check at 29 green and proven red at 18 and 11 with the order flag forced off, nine languages at 178 green, and the merge verification ran without a failure line. Not proven: no real customer machine was involved, and the billing side was inspected rather than exercised. Three measurement traps cost time and are worth naming: the danger zone lives behind a tab parameter, so a check that omits it measures zero buttons and reads that as success; a backslash escape inside a template literal handed to a page evaluation is not an escape; and one of three catalogue scripts flattened a non breaking space that the other two preserved.
From the platform work on Provibr. Why a machine can be sold exactly once is in two orders, one machine, why a shortcut is a reference and never a permission is in a shortcut is a reference, not a right, and a status word borrowed from the wrong table is in a scan that closed its own tick.