Skip to content

fix(deps): raise brace-expansion override floors to the actual backport (1.1.17 / 2.1.3) - #1117

Closed
groupthinking wants to merge 1 commit into
mainfrom
fix/brace-expansion-override-floors
Closed

fix(deps): raise brace-expansion override floors to the actual backport (1.1.17 / 2.1.3)#1117
groupthinking wants to merge 1 commit into
mainfrom
fix/brace-expansion-override-floors

Conversation

@groupthinking

@groupthinking groupthinking commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Canonical issue

Closes #1115

Outcome

The nested brace-expansion override floors now sit at or above the release that actually contains the OOM remediation. Previously both floors (^1.1.16, ^2.1.2) were themselves vulnerable, so the tree was safe only by luck of caret resolution — any install that landed on the floor would have been exploitable. This makes the safe outcome guaranteed by the manifest rather than incidental.

Scope

  • Included: two version bumps in the root overrides block — ^1.1.16^1.1.17, ^2.1.2^2.1.3.
  • Explicitly excluded: package-lock.json (unchanged — resolution already satisfies the new floors), the npm audit advisory noise (unfixable by upgrade, see below), and the broader override-floor sweep tracked in Raise npm transitive override floors to patched versions #1105.

Risk

  • Risk level: low — manifest-only, two characters, zero lockfile movement, and the installed tree is bit-for-bit what it was before this change.
  • Failure mode: the only way this could break anything is if some resolution path were pinned at 1.1.16 or 2.1.2 and could not float up. Disproven: npm ci installs 1.1.18 / 2.1.4 / 5.0.8 with exit 0, and npm install --package-lock-only produces no lockfile diff — meaning no edge was constrained to the old floors.
  • Rollback: revert the single commit (3e141b3ae). Nothing else references these pins and the lockfile never moved, so revert is a pure no-op on the installed tree.

Verification

Run in a clean worktree off main (abd93326b), verified on head 3e141b3ae:

  • Focused tests — OOM PoC executed against every installed copy in the real npm ci tree: 1.1.18, 2.1.4, 5.0.8 all return len=100000 (capped) under --max-old-space-size=512; none OOM.
  • Required CIbuild, test, lint-frontend, npm-audit, dependency-review, CodeQL, trivy, gitleaks, guards on this head.
  • Review threads resolved — none open.
  • git status --porcelainM package.json only, zero lockfile movement
  • npm install --package-lock-only --legacy-peer-deps → no lockfile diff (resolution already satisfies the new floors)
  • npm ci --legacy-peer-deps → exit 0, 719 packages
  • npm run build:web → exit 0
  • apps/web lint → exit 0

Production evidence

Vercel preview built and deployed successfully from this exact head (3e141b3ae): https://vercel.com/garv1/v0-uvai/2M2jWgVT8SUDwShk9SydgoEbeAVoVercel, Vercel Agent Review, and Vercel Preview Comments all report SUCCESS.

Because the lockfile does not move, the deployed artifact is byte-identical to what main produces today. This change constrains what future installs are permitted to resolve; it does not alter the currently deployed tree. That is precisely the intent — it removes a latent path to a vulnerable version rather than fixing an active one.


Problem

The root overrides block pins nested brace-expansion for two minimatch versions:

"minimatch@3.1.5": { "brace-expansion": "^1.1.16" },
"minimatch@9.0.9": { "brace-expansion": "^2.1.2" }

Both floors are themselves vulnerable to the OOM described in GHSA-mh99-v99m-4gvg. The 100000-entry expansion cap was backported in 1.1.17 and 2.1.3 — one patch above each pin. A clean install today happens to resolve 1.1.18 / 2.1.4, but nothing in the manifest requires that.

Why this wasn't obvious

GitHub's advisory record for GHSA-mh99 lists a single affected range (<= 5.0.7, first patched 5.0.8) and never enumerates the 1.x / 2.x lines — unlike sibling brace-expansion advisories, which carry per-major ranges. So the metadata gives no hint that a 1.x/2.x backport exists.

Closed issue #1101 read that same metadata and concluded the 1.x/2.x pins were a "false positive — never affected." That conclusion (tree is safe) was right; the stated mechanism was wrong. They were affected, and they were patched.

Evidence — measured, not inferred

Each version installed standalone and run against two OOM vectors under --max-old-space-size=512:

version numeric {1..50000000} cartesian {a,b}×30
1.1.16 OOM OOM
1.1.17 OK (capped 100000) OK (capped 100000)
1.1.18 OK OK
2.1.2 OOM OOM
2.1.3 OK OK
2.1.4 OK OK
5.0.7 / 5.0.8 OK OK

Reproduce (<v> = version under test):

mkdir -p /tmp/be && cd /tmp/be && npm init -y >/dev/null
npm i brace-expansion@<v> >/dev/null
printf 'const m=await import("brace-expansion");const f=typeof m==="function"?m:(m.expand??m.default);console.log("len="+f("{a,b}".repeat(30)).length);' > node_modules/__poc.mjs
node --max-old-space-size=512 node_modules/__poc.mjs   # OOM on 1.1.16 / 2.1.2

Two traps worth documenting: the probe must live inside the package dir, because ESM bare-specifier resolution keys off the importing file's URL rather than process.chdir. And 5.x is ESM-only with no index.js, so a naive require('.../index.js') yields a MODULE_NOT_FOUND that reads exactly like an OOM crash.

The change

Two characters. ^1.1.16^1.1.17, ^2.1.2^2.1.3.

npm audit stays noisy, and that is expected

Audit still reports the same 12 high advisories. Under semver 1.1.18 <= 5.0.7 is true, so the advisory's single range keeps matching the 1.x/2.x copies regardless of the backport. That noise is not fixable by upgrading — no newer 1.x/2.x releases exist — and needs either an upstream advisory-metadata correction or a local waiver. Out of scope here. This PR fixes the permitted set, which is the part we control.

Relationship to other work

CI note

Agent completion enforcement reports missing_trusted_publication. This is repo-wide and pre-existing: it requires a check run named Agent Lock trusted publication published by a trusted GitHub App that is not publishing. PRs #1108, #1103 and #1098 all show the same failure and were merged regardless. Nothing in this PR affects it.

The OOM remediation for GHSA-mh99-v99m-4gvg landed in 1.1.17 and 2.1.3,
but the nested pins were ^1.1.16 / ^2.1.2 -- both floors are themselves
OOM-vulnerable. Resolution already sits at 1.1.18 / 2.1.4, so this is a
manifest-only change with zero lockfile movement; it removes the
vulnerable versions from the permitted set.

Closes #1115

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 30, 2026 21:57
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
v0-uvai Ready Ready Preview, v0 Jul 30, 2026 9:58pm

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • [‘architecture-gap’, ‘bug’, ‘ci-cd’, ‘ci/cd’, ‘copilot-rabbit’, ‘documentation’, ‘duplicate’, ‘enhancement’, ‘frontend’, ‘github_actions’, ‘good first issue’, ‘help wanted’, ‘high-priority’, ‘invalid’, ‘javascript’, ‘ml-model’, ‘needs-triage’, ‘pipeline-critical’, ‘placeholder-code’, ‘priority:high’, ‘python’, ‘python:uv’, ‘question’, ‘styling’, ‘tests’, ‘v0’]

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8f103b7e-b3f3-4fff-9323-5dc36fb087c0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 3e141b3.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

Agent Completion Truth Gate: BLOCKED

Reasons: invalid_payload

Machine-readable verdict
{
  "details": {
    "collection_errors": [
      "incomplete_linked_issue_contract",
      "missing_intent_snapshot",
      "missing_agent_run_id",
      "missing_agent_login"
    ],
    "invalid_fields": [
      "policy.agent_login",
      "policy.run_id"
    ]
  },
  "reasons": [
    "invalid_payload"
  ],
  "verdict": "blocked"
}

Workflow evidence

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Raises vulnerable brace-expansion override floors to the patched backport versions, resolving #1115.

Changes:

  • Raises 1.x floor from ^1.1.16 to ^1.1.17.
  • Raises 2.x floor from ^2.1.2 to ^2.1.3.
  • Preserves existing safe lockfile resolutions.

@groupthinking groupthinking left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent verification (red-team pass)

Reviewed the diff and the underlying claim. Verdict: correct, minimal, and strictly safe to merge.

Safety of the change is independent of the CVE details. Raising ^1.1.16 → ^1.1.17 and ^2.1.2 → ^2.1.3 only removes the floor version from the permitted set — it cannot alter or break resolution here, because the tree already resolves above both floors (1.1.18 / 2.1.4). The M package.json-only / zero-lockfile-movement result is consistent with that, and npm-audit + dependency-review are green on the head SHA.

Both backport versions are real. Confirmed against the npm registry:

  • brace-expansion@1.1.17 — published (gitHead 5c57cc2)
  • brace-expansion@2.1.3 — published (gitHead 878df39)

So the new floors are satisfiable and the override block stays effective even if a future install would otherwise land on the floor itself — which is precisely the exposure this PR closes.

CI: all substantive checks green (test, build, guards, lint py/js, Security Scan py/js, npm-audit, bandit, python-safety, trivy, CodeQL, gitleaks, dependency-review, Vercel preview READY, Copilot reviewer). agent-completion/truth-gate = NOT_APPLICABLE (pass).

Remaining blockers are human-only, by design: the red PR Governance / Agent completion enforcement / Canonical issue and evidence gates are the repo's fail-closed provenance system, and merge targets protected main. No agent action clears those — this is ready for maintainer governance sign-off and merge. I'm deliberately not approving or merging, so as not to manufacture the authorization those gates withhold.

Note: CodeRabbit auto-review was skipped here because the PR carries no label matching its required-label filter; adding e.g. a dependencies/javascript label (or commenting @coderabbitai review) would enable its pass if you want a second automated opinion.


Generated by Claude Code

@groupthinking groupthinking left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review (automated, review-requested trigger): change is correct, minimal, and complete. LGTM on the code.

I verified the two things the per-pin evidence table doesn't directly cover — the diff in isolation, and the rest of the manifest:

1. The floors now sit above the vulnerable versions. ^1.1.17 permits 1.1.17 … <2.0.0; ^2.1.3 permits 2.1.3 … <3.0.0. Both exclude the vulnerable floors (1.1.16 / 2.1.2) while still allowing the resolutions already in the lockfile (1.1.18 / 2.1.4). Nested "minimatch@3.1.5": { "brace-expansion": … } override syntax is valid npm and unchanged in shape — only the version string moved.

2. Completeness — no other vulnerable brace-expansion floor remains. I read the whole overrides block plus devDependencies on the head SHA. There are exactly three brace-expansion references in the manifest:

  • minimatch@3.1.5^1.1.17 ✅ (this PR)
  • minimatch@9.0.9^2.1.3 ✅ (this PR)
  • top-level devDependencies.brace-expansion^5.0.8 ✅ (already patched 5.x)

So after this PR the permitted set is patched everywhere it's expressed. Good.

CI status. The substantive checks are green: test, build, lint-frontend/lint-python, CodeQL, dependency-review ("No vulnerabilities or license issues"), npm-audit, bandit, python-safety, trivy, gitleaks, and the agent-completion/truth-gate (not_applicable: all rules passed). The PR's own note that npm audit's 12 highs are unchanged-and-expected (the advisory's single <= 5.0.7 range matches the 1.x/2.x copies regardless of the backport) is accurate — that's advisory-metadata noise, not something upgradable, and correctly scoped out here.

One note for the merger (not a code issue). mergeable_state is unstable because three repo-governance checks are red — Agent completion enforcement, PR Governance, Canonical issue and evidence — plus Generate and Upload Coverage was still in progress at review time. None of these relate to this two-character dependency change (they're red across the open-PR queue). If any are branch-protection-required, they'll need to pass or be waived before this can merge; that's a governance decision for a human, not a fix to this diff.

I did not merge — main is protected and this is an unattended run. No code changes requested.


Generated by Claude Code

@groupthinking
groupthinking marked this pull request as draft July 30, 2026 22:14
@groupthinking groupthinking added the javascript Pull requests that update javascript code label Jul 30, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Owner Author

Returned to draft at exact head 3e141b3ae78e73a0662cc2107c426f93c9edf366.

The one-file manifest change has successful CI, Coverage, CodeQL, Security, Secret Scan, Dependency Review, and READY Vercel preview evidence; E2E is repository-skipped and there are zero review threads. CodeRabbit skipped review because the PR lacked a qualifying label, and no complete implementation execution receipt exists on #1115 or #1117. Added javascript to activate the configured review path. Keep draft until an independent exact-head review artifact and the provenance receipt exist. No code or branch history was changed.

@groupthinking

Copy link
Copy Markdown
Owner Author

Heads-up: I landed the same floor bump in #1226 before spotting this PR — apologies for the overlap.

The package.json hunk in the two PRs is byte-identical. The differences:

#1117 #1226
package.json floors ✅ same 2 lines ✅ same 2 lines
Regression guard tests/unit/test_security_fixes.py, proven non-vacuous
Base 26 commits behind main current main
Empirical backport measurement ✅ both attack vectors, all 5 versions

This branch is 26 commits behind and would need a rebase regardless. Since #1226 carries the identical fix plus the guard that stops the floors silently regressing, I'd suggest closing this one in favour of it — but happy to do the reverse (close #1226, port the guard here) if you'd rather keep this branch. Your call.

@groupthinking

Copy link
Copy Markdown
Owner Author

Heads-up: this PR and #1226 are mutually blocking each other on the governance gate.

Both are open and both declare Closes #1115 in their body. The "Canonical issue and evidence" job in .github/workflows/pr-governance.yml fails a PR when another open PR carries a closing reference to the same issue. So neither can pass that gate while both remain open — which will surface the moment either one is taken out of draft.

Comparison, for whoever decides:

#1117 (this) #1226
Created 2026-07-30 2026-08-02 14:09
Last updated 2026-08-02 14:11 2026-08-02 14:13
Size +2 / −2 +135 / −2
Title "raise ... to the actual backport (1.1.17 / 2.1.3)" "raise ... above the OOM backport"

#1226 was opened two minutes before this PR was last touched, is substantially larger, and was updated last — which reads like it supersedes this one, though I have not confirmed that.

Unblocking it needs only one of:

Not touching either PR — both are yours and both are drafts. Flagging only because the mutual-block is not obvious from the PR page; it is only visible by reading the gate workflow.

Copy link
Copy Markdown
Owner Author

Closing in favour of #1226 — this branch is orphaned

Two things:

  1. This branch is orphaned. git merge-base origin/main <this branch> is empty — no shared ancestry with current main after the secret-purge force-push, so it cannot be rebased.
  2. fix(security): raise brace-expansion override floors above the OOM backport #1226 is the live implementation of the same fix. It shares ancestry with main and is a real 2-file diff, so it can actually land.

The work is genuinely outstanding — main currently declares floors below the patched backport:

package.json:51:      "brace-expansion": "^1.1.16"
package.json:54:      "brace-expansion": "^2.1.2"

So #1226 should be landed, not just kept open. Tracked in #1378.

Branch retained for archive-tagging.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

brace-expansion override floors permit OOM-vulnerable versions (^1.1.16 / ^2.1.2 are below the actual backport)

2 participants