The green test suite that didn't prove what I thought it proved

A high-severity Dependabot alert showed up: brace-expansion, a tiny string-globbing utility three levels deep in the dependency tree, had a denial-of-service bug. The obvious move is to force the patched version and move on. That obvious move would have been wrong.

The setup

brace-expansion has three blocked consumers in this repo, all pinned to an older minimatch@3.1.5 that wants brace-expansion@^1.1.7: eslint-plugin-jsx-a11y and pa11y-ci directly, and eslint-plugin-astro indirectly through a peer dependency on the first of those, rather than a normal dependency edge. Which is why a plain npm ls doesn’t show it as a consumer but Dependabot’s own resolver, which counts peer deps, does.

eslint-plugin-astro is pinned to 2.x rather than 3.x for an unrelated reason found the day before: 3.x’s new parser drops accessibility attributes before the linter sees them. One stale pin, two separate problems.

Meanwhile eslint itself pulls in a modern minimatch that already wants brace-expansion@^5.0.5. The patched release fixing the DoS is 5.0.8. eslint’s chain gets there for free with a lockfile refresh. The other three don’t, since 5.0.8 doesn’t satisfy ^1.1.7. That’s exactly why Dependabot’s automated PR kept failing: there’s no manifest change that patches all of them without forcing a version conflict.

The instinct that would have shipped a bug

The easy fix is an npm overrides entry forcing brace-expansion to ^5.0.8 everywhere, conflict be damned. I tried it. npm install succeeded. The full local gate - type-check, lint, build, and the 39-page accessibility suite - all passed clean.

That result felt like confirmation. It wasn’t.

What the green gate actually proved

I checked what changed between the two major versions, and the module’s export shape is different. Version 1.x does module.exports = expand, a directly callable function. Version 5.x does exports.expand = expand, a named property on an object. The pinned old minimatch calls it the old way.

Force the override and that call becomes {expand: [Function]}(pattern), which throws. I confirmed it directly rather than reasoning about it:

const mm = require('./node_modules/eslint-plugin-jsx-a11y/node_modules/minimatch')
mm.braceExpand('a{b,c}d')
// TypeError: expand is not a function

So why did the suite pass with the override in place? Because none of this repo’s lint targets or accessibility test URLs happen to route through that internal call in a way that triggers it today. The gate wasn’t proving the fix was safe. It was proving the crash-triggering code path isn’t currently exercised - a completely different claim, and one that flips the moment a future config or ignore pattern contains a brace group.

What actually shipped

Two separate decisions instead of one. Bumped brace-expansion for eslint’s own chain: zero risk, satisfies its declared range. Left the other three chains alone and dismissed the alert instead, with the reasoning attached to the dismissal: it’s a devDependency, never bundled into the deployed Worker, and the exploit needs attacker-controlled strings reaching expand() at runtime. These chains only ever process static, developer-authored glob patterns at build and CI time. There’s no path for an attacker’s input to reach the vulnerable code at all.

Lesson

A passing test suite answers “does this break anything the suite already checks.” It doesn’t answer “is this change safe,” especially for a change that alters a module’s calling contract rather than its behavior. When those two questions have different answers, trust the one you can reason about directly, not the one that happened to pass.