Running a Security Review on Our Own Stack

This is an LLM gateway: it holds customer balances, forwards requests to six providers, executes customer-authored guardrails and bills to the micro-dollar. We reviewed it against ourselves in several waves. The most useful output was not the list of things fixed — it was the list of things verified, understood, and deliberately left open.

How the review was run

Not as one pass. The waves that produced findings were organised by lane rather than by file: billing, security, database, and the user-facing surfaces in one wave; auth, tenancy, secret handling and the gateway itself in another. Each lane was worked independently and every finding was re-verified against the code before anything was touched.

Two procedural rules did most of the work. Findings that turned out to be already safe were dropped from the report rather than listed as reassurance — a report padded with non-findings is a report nobody reads to the end. And every fix was validated against the full end-to-end suite run with provider keys blanked, so a security fix could not quietly become an outage.

The findings that cost money

In a product that bills per request, most security findings are money findings. The recurring shape is a code path that gives inference away, or one that charges for something it should not.

• A cancelled stream billed nothing. Usage totals only arrive in the terminal chunk on some route shapes, so pressing stop meant free inference on the large majority of routes. Fixed by charging a floor when a cancellation arrives with no usage — and the first version of that fix charged customers using their own provider keys for tokens their own key had already paid for, which is a good illustration of how a correct-sounding fix acquires a second bug.
• The hold was not an upper bound. Two different token-limit parameters are both forwarded upstream, and three places priced against whichever arrived first. A request naming a tiny value in one and a huge value in the other reserved almost nothing and could be billed for the huge one. Take the larger.
• A zero-completion response gave the prompt away. The waiver for empty completions was guarded by a field that is only ever populated on one route family; everywhere else the guard collapsed to “the completion was empty”, and the whole request went free including the prompt the provider had certainly charged us for. A caller who can trigger an empty answer — a content-filter refusal is the easy one — got arbitrarily large prompts processed for nothing, repeatably.
• Trust promotion counted payments rather than dollars. Three small top-ups bought a very large ceiling. Gate on settled value and account age, not on event count.
• A refused request could sell the same completion twice. An output guardrail that blocks a response still charges for the completion the provider generated, then returned an error and released the idempotency claim — under a comment asserting that nothing on that path had charged anything. A client retrying with the same key bought a second completion, at full price, as often as it retried.

The last two are the general lesson of the money lane: a comment asserting an invariant is not the invariant. Several of the most expensive findings sat directly underneath a sentence stating the opposite of what the code did.

The findings about who you are

• Account pre-hijacking. Signup did not prove the address. Register somebody else’s email, wait for them to sign in with a social provider, and both parties are inside the account — them by provider, the attacker by the password nobody re-checks. Worse, the legitimate owner’s arrival is what marked the address as verified, which is what gated the first top-up. Fixed by treating provider sign-in the way password reset is treated: it replaces the password and drops every session.
• Enrolling a second factor was cheaper than removing one. Turning two-factor off cost a password and a live code; turning it on cost only a session. So a borrowed dashboard could bolt a second factor onto somebody’s account and lock them out permanently, since a password reset does not clear it. Enrolment now costs the password too. Deliberately not fixed by making resets disable two-factor, because that makes the factor only as strong as the mailbox.
• The password lock did not survive a restart. The only limit on grinding one known address was an in-process counter, emptied by every deploy and enforced per worker. An attacker did not need to cause a restart, only to outlast a normal deployment week. Replaced with a durable counter, incremented in the same statement that reads it — because read-then-write loses increments under parallel attempts, which is precisely the shape an attacker produces by definition. The backoff caps at an hour, because any lockout is also a denial of service against the real owner.
• An exported server action is a public endpoint. A function that took a user id and an address, and sent a real confirmation email, was exported from an actions module. That makes it callable by anyone. Moved to a module that creates no route.

The third finding carries a detail worth generalising: the two throttles now answer with one identical sentence. The in-process bucket trips for any address; the durable lock can only exist for a real account. Answering differently would have turned two individually-fine mechanisms into an account-enumeration oracle.

The findings about whose data it is

• Private conversations were scoped to the account and never to the user. Inviting a colleague would have handed them every other member’s threads and attachments. Fixed while the system held thirteen conversations and no multi-member workspace — which is the only time that fix is free.
• Foreign keys were declared and not enforced. The schema declared 43 of them, twenty with cascading deletes, and the database defaults enforcement off. Nothing set the pragma. Deleting an account left its ledger entries, requests, payments, memberships, keys and webhooks behind as orphans. What made it worse than an unused feature is that the code reasoned as though enforcement were on: a comment justified leaving a known inconsistency alone on the grounds that a delete would raise rather than quietly null a column. A false premise was holding up a decision not to fix something.
• Capability checks were missing on several paths that their siblings carried — a playground that charged the balance with no permission check at all, billing exports readable by a role documented as read-only, and two key-limit functions whose arguments were inverted.
• Server-side request forgery through customer-supplied URLs. Telemetry and webhook destinations validated a hostname string once, at creation time. One shared fetch helper now resolves at send time, refuses private address space including the IPv6 forms that previously passed, does not follow redirects blindly, and never returns a response body.

The foreign-key finding is the one to take away. Before flipping enforcement on, the integrity check was run against the production database and both development copies: zero violations. Enforcement only rejects writes that would create a violation, so with none present nothing that worked started failing. Check, then flip.

The findings about secrets leaving the machine

The most uncomfortable findings were not in the application at all.

• The deploy script uploaded a secrets file. Its exclude list was an allowlist of two exact filenames, and a third file sitting between them on disk matched neither — so an encryption key and five live provider keys were packed into the upload tarball and unpacked on the server, with no restrictive permissions, on a host that also runs an unrelated service. Version control had caught it, which is why it was never a repository leak; but version control is not the only way a secret leaves a machine, and the allowlist was the thing that had to be right. Replaced with a pattern, plus a hard check that greps the finished tarball and refuses to upload if anything matching survived. The failure being fixed was precisely an assumption that a pattern covered a file it did not, so the assumption is now tested rather than trusted.
• Rollback archives contained the environment file. The same script archived the deployed directory, which contains the production environment file, so every rollback tarball was a complete copy of the encryption key and six provider keys at world-readable permissions. Excluded, proved by grepping the listing, and the existing archives rewritten.
• Nothing recorded what had been deployed. The script archives the working tree rather than a commit, which is worth keeping — but nothing recorded which commit the tree corresponded to. The server was running code that existed in exactly one place: an uncommitted file on one laptop. It now refuses on a dirty tree unless explicitly overridden, and writes a release record carrying the commit, the branch, the dirty flag and a timestamp.
• Encrypted values were not bound to their rows. Authenticated encryption proves a value was not tampered with, not that it belongs where it was found. A customer’s encrypted provider key was valid in any row of any account, so anyone with database write access could have moved a competitor’s key into their own row and spent it. Fixed by binding the ciphertext to its account and provider — done at a moment when that table held zero rows, because it stops being free the day a customer saves a key.

The nine we did not fix, and why that is written down

One review produced nine findings that were verified as real and deliberately left alone, because each needed a judgement call or a change large enough to want somebody watching it. They had been in a chat log and nowhere else, which is the same as nowhere.

They were written into the repository’s README, ordered by money at stake, each with the file, the mechanism and the shape of the fix. Two examples, to show the register the entries are written in:

• The reservation was priced on the cheapest route, so any failover overdraws. The code takes the first paid candidate and a comment justifies it on the grounds that a failover target is cheaper or comparable. That is exactly backwards under the ascending price sort the product documents for cost control, so every fallback costs more than the hold. The same expression also feeds the customer’s own maximum-cost guardrail, so that promise breaks on failover too.
• One black-holed provider consumes the whole deadline. The first candidate is armed with the entire budget, so a provider that accepts the connection and never answers leaves nothing for the others: a timeout with one attempt recorded while healthy routes sat unused. Needs a per-attempt slice.

Three things make a written-down unfixed finding useful rather than an excuse:

1. It is verified. Each was read back against the code before being recorded. An unverified backlog of maybes is noise.
2. It is ordered by consequence, not by discovery order or by severity label. “Roughly in order of how much money is at stake” is a more actionable ordering than a severity rating in a system this small.
3. It names the fix. Every entry ends with what would close it. That is what makes it possible for somebody else to pick one up, and it is what proves the finding was actually understood rather than merely observed.

Every one of the nine was subsequently closed in later waves — including the flaky assertion at the bottom of the list, which turned out to be the deploy-gating suite making real billed calls. That is the argument for writing them down: the list is what made them findable.

What made the review work

For the standard categories to review an LLM application against, start from the OWASP LLM Top 10; the operational side is in running an AI security review.