Fail Open by Default: What a Pre-Production Review Found

const mode = c.env.ATTESTATION_ENFORCE_MODE ?? "off";

That is one line from the device-attestation middleware of a Cloudflare Workers backend I maintain for an iOS and Android game, as it stood a week ago. It reads an environment variable that decides whether the server verifies Apple App Attest device tokens, and when the variable is not there, it decides not to verify. The control was present, tested, and documented. A single mistyped key in wrangler.toml would have switched it off in production without producing an error, a warning, or a failed deploy.

That line was finding C1 of a security review run before the v2.2.0 production release cut. Most of what followed had the same shape.

What I expected the review to find

I expected a list of things that were not there. That is what the word "finding" trains you to expect: a missing rate limit, an unauthenticated route, a header nobody set. I had a rough guess at which parts of the codebase were thin and I was ready to be told about them.

What the review produced instead was seven pull requests, merged the same day, five of them carrying a lettered review item. Two changed what the server does. The rest changed what the server could quietly stop doing: a fail-open default, a certificate fetched without a fingerprint check, three workflows referencing actions by mutable tags, and a runtime cryptography library declared as a development dependency.

None of those four is a missing control. Each is a control with a path along which it evaporates, and the defining property of all four is that the evaporation produces no signal: a disabled attestation check returns 200, an unpinned root certificate verifies a chain, and a repointed action tag runs. That is the category the review turned out to be good at finding, and it is not the category I was braced for.

An absent flag is a decision someone else made

The ?? "off" default was not careless. The comment above the binding's declaration in src/env.ts said why, and it is a good reason:

// Default is "off" so merging middleware onto routes doesn't change
// production behavior. Per-env flip via wrangler.toml [vars] edit +
// redeploy.

That is exactly right for a rollout. You attach the middleware to routes in one change and flip enforcement in another, so the merge is provably inert and the flip is a single reviewable line. The reason held until production went to hard two months ago. The reason expired that day. The default did not.

The fix is four lines, and the interesting part is what it leaves alone:

const mode =
  c.env.ATTESTATION_ENFORCE_MODE ??
  (c.env.ENVIRONMENT === "production" ? "hard" : "off");

An explicit value still wins. Production's deliberate ASSERTION_ENFORCE_MODE = "off", held open for a pending soft-to-hard roll, came through the change untouched, and item G flipped it to hard twenty-six minutes later, which is how an explicit value is supposed to move: because somebody decided to move it. Local iteration against a simulator still defaults to off. Only the absent case changed, and only on production. That took the test suite from 655 to 659.

The runtime default is half the control. The other half is scripts/check-attestation-mode.mjs, 120 lines with no dependencies, wired as a pre-deploy step: it parses the top-level [vars] block of wrangler.toml and hard-fails the deploy if ATTESTATION_ENFORCE_MODE is anything other than "hard". Two mechanisms, because they catch different mistakes. The runtime default catches a variable that vanished after the deploy passed; the script catches a posture regression somebody typed on purpose.

A mutable reference is a control you already delegated

One workflow ran a third-party security-review action pinned to @main. Two siblings used actions/checkout@v6 and actions/create-github-app-token@v3. Between them those three workflows held a model provider API key and the private key of a GitHub App installed across several repositories.

GitHub's own hardening guidance is unambiguous on this: "Pinning an action to a full-length commit SHA is currently the only way to use an action as an immutable release," because a tag "can be moved or deleted if a bad actor gains access to the repository storing the action." The mechanism is not hypothetical. GitHub advisory GHSA-mrrh-fwg8-r2c3 (CVE-2025-30066, published 2025-03-15, rated 8.6) describes tags up to v45.0.7 of a widely used action being retroactively repointed at a single malicious commit that dumped runner memory into build logs. No consumer's workflow file changed. Nothing needed to be merged anywhere downstream.

A floating branch reference is a step worse than a tag, because it does not require compromising anything. It only requires a merge into that branch. Item D pinned every uses: across the three workflows to a 40-character commit hash with the version in a trailing comment, copying the website repository's own pins for the two shared actions so they stay in lockstep.

Pruning dependencies can uninstall a control

jose was in devDependencies. It signs and verifies the device_token that carries device identity, and it mints the Google service-account token the Play Integrity path needs, both on the request path at runtime. Any build that ran npm ci --omit=dev would have produced an artifact in which the code that establishes device identity could not run at all. No failure was ever observed in production; the declaration was still false, and one build-configuration change away from mattering.

The Apple App Attest root certificate is the same mistake in a different place. The verifier fetched it from Apple and cached it in a key-value namespace for 24 hours, checking only that the response looked like a certificate and came in under a 16 KB ceiling. The fix pinned it to the SHA-256 of its binary encoding, verified out of band against Apple's published root. The part that matters is that the pin runs on the cache read as well as the fetch: a pin that only guards the fetch protects the first request after a cold start and then trusts a stored blob for the next 86,400 seconds. On mismatch the cached entry is evicted and the fetch is retried rather than the bad value being served.

Neither of these is a reasoning error about cryptography. Both are reasoning errors about paths. The question that finds them is not "is the control correct" but "name every path on which this code does not execute."

The finding no gate caught

Two days after the review closed, a routine development redeploy printed the production custom domain in its list of triggers. Named Cloudflare environments inherit the top-level routes table, so every wrangler deploy --env dev and --env staging had been attempting to bind the production hostname to a non-production worker. It had been failing harmlessly, because Cloudflare will not move a custom domain that is already bound to another service, and production was verified unaffected throughout. The only thing standing between that command and a production hijack was a third party's behavior staying the way it is.

Inherited configuration. A setting that is correct in the block where it was written and wrong in every block that inherits it, invisible because the inheriting block does not contain the setting. It survives review because reviewers read configuration files as lists of settings rather than as resolution graphs, and it survives testing because the failure mode is a deploy-time error that looks like noise. The cheapest fix is a dry-run per environment, read for what it says it will attach.

The correction was two lines, routes = [] in [env.dev] and [env.staging]. The staging block already carried a comment half-acknowledging the problem. Somebody had noticed the shape of it and nobody, including me, had followed the sentence to its conclusion. That is the part I got wrong: the review read source files and workflows, and treated wrangler.toml as configuration rather than as code with inheritance semantics.

The strongest objection

The serious objection to fail-closed defaults is that they convert a quiet security failure into a loud availability failure, and on a consumer game backend that is not obviously the better trade. A dropped variable used to mean a day of unenforced attestation that nobody noticed. It now means every request returning 403 until someone redeploys. There is a second, sharper version: the fail-closed default keys off ENVIRONMENT === "production", which is itself a variable in the same file that could be dropped, so the mechanism is circular and protects nothing against the failure it names.

The second half is correct, and the change answers it only partly. It introduced isProdStrict, which resolves an unrecognized environment string to production-strict rather than permissive, replacing about seven scattered equality checks. The ambiguous case now resolves toward enforcement instead of away from it, so a variable set to "prodution" still lands somewhere I have reasoned about. That terminates the recursion without removing it.

The first half I concede in a specific place. The pre-deploy script hard-fails on the attestation mode and only warns on the assertion mode, because the soft-to-hard roll for the second one was still in flight when the gate was written, and a gate that blocks deploys on a legitimate intermediate state gets commented out within a month. Fail closed on the value that is missing. Warn on the value that is in transit.

What to check on your own service

If you want to run the same pass on your own backend, it is an afternoon:

1. Grep every security-relevant environment read for ??, ||, or a default argument, and write down what absent resolves to.
2. Decide that default per environment rather than globally, and say so in the code.
3. Add a config-time assertion that production carries the value explicitly, rather than only checking that the runtime fallback is safe.
4. Grep the pipeline for mutable references: action tags, branch refs, image tags, unpinned module versions.
5. List every runtime dependency declared as a development dependency.
6. Run your deploy tool's dry-run for each non-production environment and read what it says it will attach.

A bad result on step one is more than two variables where you had to read the implementation to find out what absent means. If nobody can answer that from the configuration alone, the defaults are not documented posture, they are accidents that have not happened yet.

That ?? "off" line now reads ?? (c.env.ENVIRONMENT === "production" ? "hard" : "off"), and a 120-line script refuses to deploy production without an explicit "hard" in the file. The rule I take from it is that a control whose status you cannot query is a control you are assuming, so every enforcement flag needs a deploy-time assertion of its production value rather than a safe fallback alone. That works where "production" is a value your platform actually knows about. If your environments differ only by which credentials are in the shell when someone runs the deploy, the assertion has nothing to assert against, and that is the thing to fix first.