Most Audit Findings Are Drift, Not Missing Controls
Nine findings came out of one adversarial audit. Two were exploitable defects and five were divergence between what the documentation claimed and what the code did.
- Author
- Aaron Smith
- Reading time
- 8 min
Most of what an audit finds is not a control nobody built. It is a control somebody built, shipped, and then stopped describing accurately.
Last month I ran an adversarial review against the Cloudflare Workers backend behind a scoring app I maintain, using several models reading the same tree independently. Nine items came out of it, filed in a sixteen-minute burst that afternoon. Two were exploitable defects. Five were divergence between a claim and the code that was supposed to honor it. One of the nine has come up here before; the other eight are the argument.
Nine findings, two of them exploitable
Here is the sweep, numbered in filing order, with the severity each reviewer assigned at the time. Finding 6 is the exception in two ways: it arrived as a pull request rather than an issue, and its severity is npm's advisory rating.
| # | Finding | Class | Filed as |
|---|---|---|---|
| 1 | deleteAccount left submitter email on tshirt_ideas rows |
Erasure gap | HIGH |
| 2 | POST /games limit is 200/day in code, 100/day in the docs |
Drift | HIGH |
| 3 | Play Integrity verifier echoed Google's error text to the client | Defect | MEDIUM |
| 4 | ensureUser never synced a changed email back from the identity provider |
Drift | MEDIUM |
| 5 | The contract drift guard never checked that $ref targets exist |
Drift | LOW |
| 6 | Transitive ws version in the dev toolchain |
Dependency | moderate |
| 7 | openapi.yaml still described AWS Cognito authentication |
Drift | HIGH |
| 8 | device_attestation.revoked is written but never read |
Defect | MEDIUM |
| 9 | Idempotent retry returned an internal row key as the device id | Drift | MEDIUM |
The severity column is the interesting one. Both items I would call genuinely exploitable were filed MEDIUM, and two of the three HIGHs are documentation mismatches. That ordering is wrong on exploitability and closer to right on consequence, which is the whole argument of this post compressed into one column.
Finding 1 is where the two classes touch. The deleteAccount path ran seven statements: it deleted votes, games, settings, rate limits and attestation challenges, revoked the device row, and nulled email, display name and avatar on the users row. None of the seven touched tshirt_ideas, which carries a real submitter_email and a submitter_credit captured at submit time. Article 17(1) of the General Data Protection Regulation obliges a controller to erase personal data "without undue delay"; this code discharged that obligation for seven tables and not the eighth.
What makes it a drift finding rather than a coding mistake is how it got there. The previous version of this backend deleted t-shirt idea rows outright inside the delete transaction. A few weeks before the review, an issue asked for those rows to survive account deletion so community submissions were not lost with the account, and the cascade was removed. The row survived, as intended. The email address on the row survived too, which nobody intended and no test asserted either way.
The spec described an auth system the product had replaced
The canonical API contract for this backend is openapi.yaml, 3,094 lines, declared the source of truth in the repository's own conventions file. Until two days after the review it opened with this:
All endpoints except `/health` require a valid AWS Cognito JWT
in the `Authorization: Bearer <token>` header.
The backend authenticates with Clerk and has never called Cognito. Those lines were written months earlier in the previous repository, in the commit that added the OpenAPI specification, where Cognito was real and the description was accurate. They were carried into this repository during a later migration of supporting files from the parent repo, and nothing in the pipeline noticed that a file describing one authentication system had been installed on top of a different one.
The account-deletion description was worse, because it was never true anywhere:
Soft-deletes the user account (sets `deleted_at`). After a 30-day
grace period, all user data is hard-deleted. Also calls Cognito
AdminDeleteUser to remove the identity provider record.
The Cognito sentence was accurate in the old backend. The thirty-day grace period was not. I grepped the old source for it and there is no such window anywhere in that codebase: the old deleteUser called Cognito, dropped the associated rows in a transaction, and anonymized the users row in the same request. So the paragraph shipped with one false sentence, acquired a second when the auth provider changed, and stood as the published data-retention promise for 52 days across two repositories. The same file listed cognito_sub and is_admin as required fields on the User schema, two fields the route stopped returning to clients before it ever served production traffic.
Correcting all of that took 27 added and 15 removed lines of openapi.yaml in a single pull request.
Code cannot be silently wrong about itself
Untested prose. Every executable claim in that repository is checked by something; 564 tests ran on the pull request that closed these findings. Every prose claim is checked by a reader who may never arrive. The mechanism is not carelessness. A rate limit written as createRateLimiter(200, "games") is enforced by the runtime on every request, so it cannot be silently wrong about itself, while the same limit written as 100/day in a table in docs/v2-frontend-integration.md is enforced by nobody and can be wrong for as long as the file exists.
A drift guard did run in this repository, on every pull request. scripts/check-openapi-discriminator-drift.mjs asserted that every z.literal(...) in the game schemas had a matching key in the spec's discriminator mapping. It checked one mapping in a file of 3,094 lines. Finding 5 is that the guard did not even verify its own mapping values resolved to schemas that exist. The cheapest thing that breaks this pattern is not a better writing process but making one form derive from the other, or making a script compare the two forms and fail the build.
The strongest objection
The strongest objection is that drift is documentation debt wearing a security badge. Only two of these nine could be turned into an attack. A reviewer's scarcest resource is attention on exploitability, and every documentation mismatch promoted to "security finding" spends some of it. The severity column above is the argument's own best evidence: filing a rate-limit mismatch as HIGH while filing an unenforced revocation flag as MEDIUM is exactly the misallocation that objection predicts.
That is mostly right, and the severities were wrong. Finding 2 was filed HIGH by a reviewer whose own write-up concedes the drift direction was harmless, since the docs undercounted a real allowance and no client would hit an unexpected rejection; I would file it LOW today. The dependency item makes the point against me more sharply: finding 6 pinned ws and described the advisory it was clearing as an uninitialized memory disclosure. The published advisory for CVE-2024-37890 describes a denial of service: a request carrying more headers than server.maxHeadersCount can be used to crash the server. My own write-up drifted from its source in the same sweep that was filing drift findings.
Two things stop the class from being cosmetic. The first is finding 1: an erasure obligation is asserted in prose and discharged by code, and for the 27 days between the cascade being removed and the audit, the code had quietly stopped discharging it.
The second arrived six days later. A follow-up issue found the error-code list in the spec enumerated seven codes while the backend emitted around fifty, and both client apps had already shipped branching logic on codes the contract denied existed. Schema-driven code generation and spec linting reject branches like that, so the mismatch was filed as a release blocker for version 2.0, and the fix added the missing codes: 315 lines added, one removed, zero changes to backend logic. Nothing was exploitable. The release stopped anyway.
What follows if this is right
The useful question is not how severe a claim's divergence is, but who or what was ever going to read the claim:
| Where the claim lives | What enforces it | Cost when it drifts | Guard |
|---|---|---|---|
| Executable code | The runtime, every request | Immediate and visible | Tests you already run |
| Machine-read schema | Client codegen and spec linters | A blocked release | A script resolving it against the code |
| Prose in the canonical contract | A human who may never read it | An integrator builds the wrong thing | Notify the consuming teams on change |
| A published policy or privacy claim | A regulator, after the fact | A finding you cannot remediate backward | A test asserting the described behavior |
Row three is the one teams skip, because a notification feels like process rather than control. It is the row this backend was failing on, and the two follow-ups I would defend are both aimed at it. The first extended the drift guard to resolve every discriminator reference against the schema map, and all thirteen resolve on the corrected file. The second added a workflow keyed on on.push.paths: ['openapi.yaml'] that files an issue in both client repositories on every contract change, carrying the diff and a triage checklist. That second one does not verify a single sentence of the prose; it guarantees the prose gets a reader within a day of changing, which is the only property row three actually needs.
What would change my mind
Two conditions, both measurable on this codebase. If the next three audits of this backend come back with drift below half the findings, then this ratio was a property of a repository whose only contract guard checked one discriminator mapping, and not a property of cheap review generally. And if a year of client releases passes without a single drift finding producing a downstream cost like the blocked 2.0 release, then this class is documentation debt, should be budgeted as documentation debt, and I will have overweighted one evening's sweep.
The thirty-day grace period is out of the spec now. It was never in the product, not in the version that used Cognito and not in the one that replaced it. What made it dangerous was not that it was false, but that nothing in the pipeline could tell the difference between a sentence and a claim. The rule I take from it: any statement your documentation makes about behavior needs either a test that asserts the behavior or a job that notifies whoever acts on the statement. That works where you can name the consumer; for prose whose only reader is an auditor two years out, a notification has nowhere to go, and I do not yet have an answer for that case.