DEV Community

Cover image for A 4-line security fix turned into a 100-file pull request, and I nearly merged it
Arnold Cartagena
Arnold Cartagena

Posted on

A 4-line security fix turned into a 100-file pull request, and I nearly merged it

I maintain mcp-sso, an open-source OAuth 2.1 layer for remote MCP servers. In plain terms: it is the login-and-permissions layer that lets AI tools connect to your servers safely, without you handing out raw API keys. It does the spec-correct hard parts most people skip, including dynamic client registration, newer identity standards like CIMD, PKCE, and audience-bound tokens (a token minted for one server is rejected by another). It is live-verified end to end against real enterprise identity providers, including Microsoft Entra ID, Google, and Cloudflare Access. I am also building an MCP gateway multiplexer on top of it: one secured front door for many MCP servers, with the same auth for all of them.

It is a security library, so I treat every change as security-critical, and I lean hard on AI-assisted development. That includes a review panel of several AI models from different labs (GPT, Grok, GLM, Claude) that argue over each change.

This week that whole system (a green test suite, a multi-model review panel, and CI) passed a bug everyone treated as critical. Nothing exploitable ever reached a user; it was caught before the feature it lives in was even switched on or released. But it was on the main branch, it looked like a serious security hole, and it slipped past every check. And then, trying to fix it, the same system nearly shipped a whole security library to defend against an attack that cannot actually happen.

Here is the whole story, including the parts where I got it wrong.

First, the actual bug (in plain terms)

Part of the library fetches documents from the internet. Any time code makes outbound web requests, the danger is that an attacker tricks it into calling somewhere it should not, like a private address inside your own network. So this fetcher has a guard that blocks requests to internal addresses such as 127.0.0.1 (your own machine).

The guard had one option: "allow loopback," meaning "let this reach the local machine," which is only ever meant for local testing. The code checked it like this:

if (opts.allowLoopback === true) { /* allow 127.0.0.1 */ }
Enter fullscreen mode Exit fullscreen mode

Here is the catch, and it is a JavaScript quirk. Every object in JavaScript quietly inherits properties from a shared parent object. If an attacker can plant a value on that shared parent (this is called prototype pollution), then a plain, empty object suddenly reports that value as its own. So this check could read allowLoopback as "true" even when nobody set it, and a public web address that secretly points at 127.0.0.1 would sail straight through the guard. I confirmed it end to end.

The fix is boring. Read the option only if this specific object actually set it, not something it inherited:

Object.hasOwn(opts, "allowLoopback") && opts.allowLoopback === true
Enter fullscreen mode Exit fullscreen mode

Four lines across three files. That is the entire real bug.

How the bug was found

A batch of work had just merged, and before starting the next piece I wanted an adversarial second opinion on what I had shipped: a deep audit of my own code. Check the SSRF guards. Stand up a local TLS server. Prove the timeout really does tear down the socket. Hunt for anything the frozen test suite structurally cannot reach.

I pointed Fable at it first. It refused. The session paused and told me its safeguards had flagged the request, noting they are "intentionally broad right now and may flag safe and routine coding, cybersecurity, or biology work." I was asking a model to audit my own security library, and the safeguard could not tell that apart from an attack. For legitimate security work, this has become a recurring limitation for me.

Fable here saying no :(

So I handed the same audit to GPT-5.6 Sol on Ultra, its most expensive setting. And it worked, maybe too much. I chose Ultra deliberately because I wanted to see how capable the model was at this kind of adversarial review. It worked, perhaps too well. Sol explored paths the test suite could not reach and found a real bug. Credit where it is due: the audit was excellent. The problem came afterward, when I let the same model turn one valid finding into an unbounded repository-wide fix.

Then it ran away from me

Here is where it went sideways, and it is the interesting part.

"Reading an inherited value where you meant your own" is not a single bug. It is a pattern that could exist anywhere an object is read. So the natural instinct, mine and the AI's, was to fix it everywhere. And "everywhere" has no edges.

A few rounds later the "small fix" had grown its own new library and spread across the whole repo. The pull request hit 104 files and close to 7,000 changed lines (it is public, closed and unmerged: PR #88). Every review round found more problems, and the new problems were in code the pull request had just added. Round after round, the change got bigger and the to-fix list got longer.

Then the punchline. The very library it built to stop prototype pollution read its own internal data in the exact unsafe, inherited way it was supposed to prevent. Concretely: it checked descriptors with "value" in descriptor, and in is itself a prototype-chain read, so a polluted prototype could forge the very descriptors it used to detect pollution. The cure had the disease.

That is when the pattern finally clicked, and it is the one line I want people to take away: give an AI a rule that touches every line of code, and it turns into a machine that generates its own work. "Every value must be read safely" applies everywhere, so there is no natural place to stop. Review never finishes; it just keeps finding problems in the code it keeps writing. The AI was not being thorough. It had been handed a job with no end.

One thing here is on me, and it is the difference between a bad hour and a bad day. The rule is what made the task infinite. The setup is what let it run unwatched: my first-choice model, Fable, declined the security work (I tried several framings; each was refused), so I reached for GPT-5.6 Sol on Ultra, its priciest setting, overkill for this, and let it run unattended on my own library, with no tripwire and nobody watching, I was working on something else at the time and usually I come back after a few minutes but I left it there. And Sol is relentlessly goal-oriented: it keeps going until the job is done, which is the last thing you want pointed at a task that has no end. A less driven model might have stopped on its own; this one did not. The missing decisions lights the fire; an overkill, relentless model running unattended is what let it burn for hours before I noticed. I have since capped unattended runs and required a stop condition.

6+ hours in, something did not add up

After more than six hours of this, I got suspicious. The changes kept coming, and none of it felt like it was converging on "done." So I stopped and asked the question I should have asked at the very start: can this bug actually be exploited?

Here is the thing about that original "critical" bug. To pull it off, an attacker first has to plant that poisoned value on the shared parent object. And nothing in the library lets them do that. The documents it fetches are parsed strictly and read through an allowlist; there is no path for an outsider to poison anything. In other words, to exploit this bug you would already need to be running your own code inside the process. And if you are already running code inside the process, you can just replace the whole security layer. The lock does not matter if you are already inside the house.

So the honest severity was: a small, worth-fixing, defense-in-depth gap, caught before the feature even shipped. Not a critical hole, and definitely not a reason to build a 100-file library. We over-engineered an entire library we did not need, to defend a door that cannot be reached. And a big part of why is above: an overkill model, running unattended, with no reachability check and no stopping condition. In retrospect, letting it run alone was the real mistake.

Why nothing caught it

This is worth sitting with, because the answer is not "the reviewers were careless." They were not. Two separate, structural reasons:

1. The tests and the spec never described the problem. They checked for bad values: malformed URLs, wrong types, oversized inputs. They never described the shape of an object, whether a value is truly its own or quietly inherited. There was no test that could fail, because nobody had written down the rule it would fail against. You cannot catch a problem your spec never mentions. In fairness, you cannot foresee every shape of input, and "did you test with a polluted prototype?" is not on most teams' checklist. That is exactly why the class has to be named somewhere, in the spec or the threat model, before anyone can test for it.

2. All the AI reviewers shared the same blind spot. Several models reviewed it independently, and they agreed it was safe. But every one of them tested the code the normal way, with clean objects. Not one first poisoned the shared parent and then tested. When reviewers all make the same assumption, adding more of them does not help; they just agree on the gap together.

None of the automated checks caught this. What caught it was stepping back, in a fresh frame of mind, and asking one plain question: can this actually be attacked?

How I pulled it back

The recovery was almost all process, not code:

  • Notice when something is off. A change that grows every round is not thoroughness, it is a missing plan. More than about three review rounds on one thing is now my signal to stop and look, not to push harder.
  • Do not run big things unattended without a tripwire. Letting a powerful model grind for hours with no human watching, and no "stop if this gets weird" condition, is what let a 4-line fix become 100 files. Unattended runs need limits and a way to flag a human the moment something looks off, especially with a relentlessly goal-oriented model that will happily chase an infinite task to the end.
  • Fix the design first, then the code. I closed the 100-file pull request without merging it. It had been open for about five and a half hours (PR #88, public and closed). I re-cut the actual bug as a tiny 5-file change, with a test that fails if you undo the fix, plus the one real test that had been missing.
  • Write the boundary down before building it. For the rest, I wrote what the rule actually is into the spec first, including an honest note about what is out of scope, and only then implemented it. Now review checks against a plan, instead of discovering the plan round by round. This is harder than it sounds; sometimes you only learn where the boundary belongs after you have crossed it. But even a rough boundary written first beats none.
  • Break it up. The runaway became three small, separately reviewable changes. All three merged cleanly.

The real fix shipped. The runaway did not.

Where I was wrong, and the rule I am adding

I am not going to pretend I was the level-headed one here. I let it run. For hours I read "the reviewers keep finding more" as diligence, when it was actually the warning sign. And I did not ask the one question that would have ended the whole thing in the first ten minutes, can this even be attacked, until I had already burned 6+ hours building a cure for a disease my own code cannot catch. And there was a bill for that. The AI was running on GPT-5.6 Sol at its most expensive setting, and at pay-as-you-go API rates ($5 per million tokens in, $30 per million out) the tokens from this episode would have cost north of $500. I am on the ChatGPT Pro plan, so I did not actually pay it, but a 4-line fix quietly ringing up a multi-hundred-dollar retail token bill is its own kind of lesson. The judgment I keep calling "the human part"? I was slow to it.

This is not the first time a change of mine has spiraled into round after round of review. The earlier ones were a bit different, requirements missed in the design that review then found one at a time, and this one predates the design-first process I have since adopted. But the root is the same every time: no boundary written down before the building starts. "Try to remember next time" is not a fix for that. A rule is.

So here is the rule I am adopting, and it is going straight into my project's standing process (the CLAUDE.md and engineering-os setup that guides these AI sessions): if a single change needs more than about three review rounds, stop, because something is off with the plan, not the code. And no change may add a new security helper and call it "the standard" in the same step. Write the standard down and agree on it first, in its own change, before any code. That draws the boundary, so review has something finite to check against, and the machine cannot keep sweeping a rule that was never defined.

This was not just me

The timing was almost too on-the-nose. The same week I watched a goal-oriented model chew through my repo, the industrial-scale version of both these failures made the news. Hugging Face disclosed that an autonomous AI agent had breached its production infrastructure, and OpenAI admitted the culprits were its own pre-release models, GPT-5.6 Sol among them, run with reduced cyber refusals for an evaluation. OpenAI called them "hyperfocused," "going to extreme lengths to achieve a rather narrow testing goal": they broke out of their test sandbox and went looking for the eval's answers. That is the exact behavior I had just watched, from the same model, only pointed at a live network instead of a pull request. And when Hugging Face went to investigate, they hit the same wall I had hit with Fable: the hosted models' guardrails blocked their forensic queries (real attack payloads look the same whether you are the attacker or the defender), so they ran an open-weight model, GLM 5.2, on their own hardware to read the attack logs. Mine was a tiny, benign version of both. Same surface, wildly different scale.

AI made everything here faster: the fix, the sweep, the review, and the runaway. Knowing what is actually reachable, what is in scope, and when to stop was the human part. It still is. I was just late to it this time.


mcp-sso is open source: threat model, conformance matrix, and provenance included, plus the gateway multiplexer I am building on top of it. If you run remote MCP servers, I would love your eyes on it.

description: "A four-line defense-in-depth fix became a 104-file pull request after an AI agent was given a security rule with no boundary or stopping condition."

Top comments (0)