Skip to content

fix(cli,deps): prevent esbuild audit loop and lockfile churn discard - #46599

Closed
ft-ioxcs wants to merge 1 commit into
NousResearch:mainfrom
ft-ioxcs:fix-esbuild-lockfile-cycle
Closed

fix(cli,deps): prevent esbuild audit loop and lockfile churn discard#46599
ft-ioxcs wants to merge 1 commit into
NousResearch:mainfrom
ft-ioxcs:fix-esbuild-lockfile-cycle

Conversation

@ft-ioxcs

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR fixes a recurring vulnerability cycle where npm audit high-severity warnings for esbuild reappear after every hermes update, even though the user never intentionally modified the affected packages.

Problem:
- The upstream package-lock.json contained vulnerable esbuild versions (0.27.7, 0.28.0) as transitive dependencies of vite.
- hermes update runs _discard_lockfile_churn() which restores package-lock.json to the upstream version before git pull, discarding any local security fixes.
- After git pull, npm ci reinstalls the exact same vulnerable lockfile.
- hermes doctor --fix cannot fix it because npm audit fix --workspace crashes on the Hermes monorepo due to a known npm arborist bug.

Solution:
1. Add esbuild: "0.28.1" to the root package.json overrides field, forcing all workspaces to use the patched version regardless of what vite requests.
2. Patch _discard_lockfile_churn() in hermes_cli/main.py to skip discarding a lockfile when its corresponding package.json is also dirty. This preserves intentional dependency fixes across updates.



Related Issue

Fixes # (no existing issue — discovered during local troubleshooting)



Type of Change

Check these two boxes:
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
- [x] 🔒 Security fix



Changes Made

- package.json: Added "esbuild": "0.28.1" to overrides section
- package-lock.json: Regenerated with npm install --workspaces=false to resolve esbuild 0.28.1 everywhere
- hermes_cli/main.py: Modified _discard_lockfile_churn() to check if package.json is dirty before discarding package-lock.json



How to Test

1. Run hermes doctor — confirm web workspace deps and ui-tui workspace deps show "no known vulnerabilities"
2. Run npm audit --workspace web from repo root — confirm found 0 vulnerabilities
3. Run npm audit --workspace ui-tui from repo root — confirm found 0 vulnerabilities
4. Run hermes update and check again — confirm vulnerabilities do not return



Checklist

Check only these boxes, leave the rest unchecked:

Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix
- [ ] I've run pytest tests/ -q and all tests pass (leave unchecked — you haven't run this)
- [ ] I've added tests for my changes (leave unchecked — no new tests needed for a lockfile bump)
- [x] I've tested on my platform: macOS 15.5.1

Documentation & Housekeeping
- [ ] N/A — no documentation or config changes needed
- [ ] N/A
- [ ] N/A
- [x] I've considered cross-platform impact: npm overrides is cross-platform; Path() usage in the Python patch is cross-platform
- [ ] N/A



Screenshots / Logs

Paste this if you want to show evidence:

text
Before fix:
  ⚠ web workspace deps (0 critical, 2 high, 0 moderate — build-tool advisory)
  ⚠ ui-tui workspace deps (0 critical, 2 high, 0 moderate — build-tool advisory)

After fix:
  ✓ web workspace deps (no known vulnerabilities)
  ✓ ui-tui workspace deps (no known vulnerabilities)

Two changes:

1. Add npm override esbuild 0.28.1 in root package.json to force the
   patched version across all workspaces (web, ui-tui, bootstrap-installer).
   This fixes the recurring high-severity audit warnings that returned after
   every hermes update because the pulled lockfile still contained
   vulnerable esbuild 0.27.7 / 0.28.0 as transitive deps of vite.

2. Patch _discard_lockfile_churn() so it does not discard a lockfile when
   the corresponding package.json is also dirty. Previously, intentional
   dependency fixes (like the esbuild override) were silently thrown away
   before git pull, causing the vulnerability to reappear on every update.
@ft-ioxcs
ft-ioxcs requested a review from a team June 15, 2026 10:57
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard dependencies Pull requests that update a dependency file javascript P3 Low — cosmetic, nice to have labels Jun 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #46652: #46652

I salvaged the audit-loop fix onto current main, preserved your authorship on the main dependency/update-cleanup commit, regenerated a valid package-lock.json, added regression coverage, and included the Nix lockfile hash follow-up needed by CI.

Thanks for finding and fixing the issue.

@teknium1 teknium1 closed this Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard dependencies Pull requests that update a dependency file P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants