The ambient-global bug: when “current account” is a mutable global 100 call sites read

I shipped a feature that let a tournament admin pick a child event from a dropdown, and the moment they picked one, every save started writing to the wrong account. No error. No exception. The save succeeded. It just landed on the parent org’s row instead of the event the user was staring at. The cause wasn’t one bug in one function. It was every query in the file trusting a global variable that I had just made lie.

Here’s the setup, because the shape matters more than the specific app. The app is a Classic ASP application running server-side JScript on IIS. Every page reads two ambient globals. pageHQ is the logged-in account, the one in the URL. sportsHQ is the account whose data you’re actually operating on. For years those two were always the same object. So the codebase used them interchangeably, thousands of times, and nobody ever thought about which one was correct because there was never a case where they differed.

Then I built tournament event folders. An org admin can now drill into a child event with ?div=<EVENTUSERNAME>. A helper called SetDivToHQ() reaches into the module scope and reassigns the global sportsHQ to point at the child event, mid-request, based on that query-string param. pageHQ stays the parent. For the first time in the app’s history, pageHQ.username !== sportsHQ.username.

That single inequality turned roughly a hundred call sites into latent bugs simultaneously.

The blast radius

Every query that built its scope from the logged-in account was now wrong. The pattern was everywhere:

dbObj.DBwhereStrs["username"] = pageHQ.username;

That line is correct when pageHQ and sportsHQ are the same. It silently targets the parent the instant they diverge. The save you fire against the child event’s bracket writes a WHERE username = '<parent>' clause, finds the parent’s row, updates it, and reports success. Nothing throws, because nothing is wrong as far as SQL or the runtime is concerned. You only notice when the data shows up under the wrong event, or doesn’t show up where you’re looking. That is the worst kind of bug: a correct-looking operation producing a confident, wrong result.

The fix for the save path was almost insulting in how mechanical it was. The commit was a pure swap of one global for the other:

dbObj.DBwhereStrs["username"] = pageHQ.username;
->
dbObj.DBwhereStrs["username"] = sportsHQ.username;

The bulk of it lived in the tournament AJAX handler, where 80 lines flipped pageHQ.username to sportsHQ.username. Those 80 weren’t concentrated in a handful of save functions either. They were scattered across dozens of handlers, on and on, because every one of them had at some point reached for the logged-in account to scope a query. In the bracket save handler it wasn’t just the WHERE clause either: the INSERTs were writing parentNode, masterNode, and league from the wrong account too, so new rows were being stamped with the parent’s identity. The same swap hit a second file for another 11 lines across the bracket handlers. The whole commit was 91 insertions against 91 deletions, two files, give or take 180 lines. It was pure substitution. There was no clever logic to write. There was just a global I had to stop trusting in 90-odd places, by hand, hoping I caught all of them.

You cannot grep your way to confidence here, and that’s the lesson. pageHQ.username is not wrong. It’s the correct expression in the majority of the codebase that genuinely scopes to the logged-in account. So a global find-and-replace would have broken more than it fixed. I had to read each call site and decide which account it meant. There is no compiler, no type, no test that distinguishes “the account in the URL” from “the account I’m editing” when both are just .username off an ambient object.

The mutation itself is the landmine

Step back to SetDivToHQ(). It reads Request.QueryString("div"), falls back to a couple of other sources, and if the value resolves to a real child node it does the equivalent of this:

sportsHQ = SportsHQGet(theChildNode);

A wholesale reassignment of a module-level global, mid-request, driven by a URL parameter. It is incredibly convenient. It is also the reason the correctness of every downstream line now depends on when in the request that mutation ran relative to each read. Any code path that read sportsHQ before the swap, or that cached a value derived from it, or that ran in a context where the swap never fired, is now subtly out of sync with the rest of the request. A mutable global is a shared clock that you’ve made tick at an unpredictable moment.

This is the part I’d tattoo on a junior dev: a global variable that a hundred sites read is completely fine right up until two things that used to be equal stop being equal. The day you add impersonation, a sub-account, a “view as,” an org admin drilling into a child, a tenant switcher, any of it, every one of those read sites becomes a question you have to answer one at a time. The convenience you banked for twenty-five years comes due all at once, with interest, in a single afternoon of swapping globals and praying you found them all.

What I’d build instead

The honest fix is not “swap the global.” That’s the patch I shipped because it was the smallest safe change to a revenue app I couldn’t rewrite. The actual fix is structural: current user and current tenant should be passed as explicit parameters, not read from ambient state, the day you introduce any form of context switching.

There’s a hint of the right shape already in the codebase. The account-scoping helper defaults its scope to pageHQ but lets you override it through an options bag that gets $.extend-ed over the defaults. So the other fix was threading {sportsHQ: sportsHQ} through about 27 call sites:

where: GetAccountFilter()
->
where: GetAccountFilter('','','',{sportsHQ:sportsHQ})

Note the three empty positional args just to reach the options bag. That call-site noise is itself a design critique. A function whose real job is “scope this query to a context” should have taken the context as its first and only argument from the start. But the important part is that the override is local and explicit. No global toggle, no hidden mode flag. The safest migration to a new context isn’t flipping a default that the silent majority relies on. It’s making the new path explicit at every site, even when it’s tedious, because the tedium is exactly the visibility you need.

If I were doing this clean, there’d be one context object, passed down, never reached for ambiently. Every function that scopes data would take it as a parameter. An impersonation feature is then a change to what you pass, not a landmine planted under code written a decade ago by someone who had no idea this day was coming.

The takeaway

Ambient “current X” globals are a loan against a future you can’t see. They read clean, they save you a parameter on every call, and they’re correct for as long as there’s exactly one X. The bill arrives the first time there can be two: an admin drilling into a child, a “view as another user,” any sub-account or impersonation. On that day every site that read the global is a silent write to the wrong row, and because nothing throws, you find them through bug reports and gut feel, not a stack trace.

Pass current user and current tenant explicitly the moment any context switching enters the picture. Not eventually, not “when we refactor.” That day. The afternoon I spent swapping one global for another across 90-odd lines was the interest payment on a default that had quietly been wrong for years before anyone noticed.