Skip to content

Harden deps: take the npm fixes that fit the current major versions - #5359

Merged
vivekchand merged 4 commits into
mainfrom
harden/frontend-npm-audit-fix
Aug 30, 2026
Merged

Harden deps: take the npm fixes that fit the current major versions#5359
vivekchand merged 4 commits into
mainfrom
harden/frontend-npm-audit-fix

Conversation

@vivekchand

@vivekchand vivekchand commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Product record:

No-PRD: dependency-only security update. No product behaviour changes — package.json is untouched, every bump is patch-level within the already-declared semver ranges, and the rebuilt bundle is equivalent in module count and size.

.github/dependabot.yml gained its npm entries in #5252, so frontend/ is now watched — but Dependabot only opens PRs for future drift. The advisories that were already open when it was wired up still need one deliberate pass. This is that pass, restricted to what can be taken without a major upgrade.

What changes

One file: frontend/package-lock.json. The output of npm audit fixwithout --force. package.json is not modified, so no declared range moves; all 27 lockfile bumps are patch-level resolutions inside the existing ranges.

Advisory count on frontend/: 9 → 5.

Package Before → After Severity
postcss 8.5.14 → 8.5.26 high
nanoid 3.3.12 → 3.3.18 high
@babel/core 7.29.0 → 7.29.7 low
@remix-run/router 1.23.2 → 1.23.4 moderate
react-router / react-router-dom 6.30.3 → 6.30.6 moderate

The other 21 changed entries are transitive @babel/* and browserslist/caniuse-lite data packages pulled along by those four.

Not in this batch, deliberately

The remaining 5 advisories have no in-range fix — each needs a major upgrade, which is an API-compatibility decision rather than a lockfile bump, and does not belong in a PR whose whole claim is "nothing changes":

Package Would require Why it is a decision
vite (+ its esbuild) 5.4.21 → 8.2.2 three majors; new plugin/config contract, and this is the build that produces the shipped SPA
react-router-dom 6.30.6 → 7.18.3 v7 data-router migration across every route in src/
i18next-http-backend 2.7.3 → 4.0.1 two majors, alongside a pinned i18next ^23

Splitting each into its own PR, with the migration actually exercised, is the honest way to take them. Flagging rather than folding them in.

Test plan

  • npm ci from the new lockfile resolves with no drift — the lockfile is internally consistent, not just edited
  • npm run build (tsc -b && vite build) succeeds: 609 modules transformed, identical to the pre-change baseline
  • Bundle is unchanged in substance: 491.43 kB → 491.19 kB JS (gzip 149.57 → 149.46), CSS byte-identical at 6.84 kB
  • npm audit re-run: 9 → 5, and every survivor reports isSemVerMajor: true — i.e. nothing in-range was left on the table
  • Build artefacts under clawmetry/static/v2/dist/ deliberately not committed; publish.yml and release-on-merge.yml both rebuild the SPA from source, so the diff stays one file
  • scripts/check_product_record.py run against this exact body — frontend/package-lock.json is outside the exempt paths, so the No-PRD: line above is required. It must begin its own line: OPT_OUT is anchored ^\s*No-PRD:, so prefixing it inline with the bold label (as this body first did) does not match and correctly fails the gate.

`npm audit fix` (no --force) on frontend/, which resolves 4 of the 9
advisories without leaving the declared semver ranges. package.json is
untouched; only the lockfile moves, all bumps patch-level.

Fixed: postcss 8.5.14 -> 8.5.26 (high, path traversal via
sourceMappingURL), nanoid 3.3.12 -> 3.3.18 (high, infinite loop on
non-positive size), @babel/core 7.29.0 -> 7.29.7 (low, arbitrary file
read), @remix-run/router 1.23.2 -> 1.23.4 with react-router(-dom)
6.30.3 -> 6.30.6 (moderate, open redirect).

The remaining 5 all require a major upgrade -- vite 5 -> 8,
i18next-http-backend 2 -> 4, react-router-dom 6 -> 7 -- and are left for
a deliberate decision rather than folded in here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016eYYxAxXQk6munsK57TfH9
@8090-software-factory

Copy link
Copy Markdown

✅ Drift Bot (ClawMetry): no drift detected

Drift Bot analyzed the changed files against this project's blueprints and requirements and found no drift.

@8090-software-factory

Copy link
Copy Markdown

✅ Drift Bot (ClawMetry): no drift detected

Drift Bot analyzed the changed files against this project's blueprints and requirements and found no drift.

Copy link
Copy Markdown
Owner Author

✨ auto-fixed: merged main (14572bc) into branch to resolve BEHIND state


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

E2E Gate (required) timed out waiting for runners, and the wait is not this PR's

The gate failed at 21:39 UTC on head d871bc7. It is not a test failure — nothing it was waiting on ran and failed. It is the aggregator's 1800s poll expiring:

FAIL: timed out after 1800s. Still pending:
  - MOAT Keystone: 0/1 leg(s) complete
  - E2E Browser Tests: 0/1 leg(s) complete
  - API Tests (3 OS): 2/3 leg(s) complete
  - MOAT Verifier: 0/1 leg(s) complete
  - Entitlement API tests: 0/1 leg(s) complete
  - pip install matrix: 2/4 leg(s) complete
  - Wheel install & assets: 0/1 leg(s) complete
  - Store invariants: 0/1 leg(s) complete

The split is by runner label, not by test

Every leg that was given a runner passed. Every leg still pending is ubuntu-latest, queued since 21:10 with no runner ever assigned — no runner_id, no steps, not one line of output after 30 minutes.

Runner label Outcome
windows-latest API Tests ✅, pip install ✅ — both ran
macos-latest API Tests ✅, pip install ✅ — both ran
ubuntu-latest 11 jobs queued 30+ min, never scheduled

The gate's own hint — "a check stuck at 'not reported' usually means its workflow did not run for this commit" — points at a paths filter. That is not the cause here: these jobs exist in run 33275319604, they are simply unscheduled.

It is repo-wide, not this branch

Four full CI runs all started within 13 seconds of each other at 21:09 and were all still in_progress at 21:40 — this PR plus #5256, #5257 and #5354. The ubuntu-latest allowance is saturated by that pile-up.

A one-file package-lock.json change cannot influence runner scheduling, and the legs it would plausibly touch — Syntax & Lint, OSS golden path, PR build, the CodeQL javascript-typescript analysis — all ran and all passed on this exact head, along with drift-bot and the product-record gate.

What I am doing

Holding the single permitted re-run rather than spending it now. Re-running the gate while the ubuntu queue is still drained just starts another 1800s poll against jobs that still cannot get a runner, and burns the re-run for nothing. Once the queue clears and the legs report, I will re-run the gate once against completed legs.

No code change is warranted, and I am not pushing an empty commit to kick CI. The PR stays watched with a check-in scheduled until it is green.


Generated by Claude Code

@8090-software-factory

Copy link
Copy Markdown

✅ Drift Bot (ClawMetry): no drift detected

Drift Bot analyzed the changed files against this project's blueprints and requirements and found no drift.

Copy link
Copy Markdown
Owner Author

✨ auto-fixed: merged main into branch (was BEHIND at 14572bc)


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Two findings on the new head — one about CI, one you should decide before merging this

1. The CI run was cancelled repo-wide, not failed

E2E Gate (required) went red again on d3e25a0, but the cause is different from the starvation I described above, so this is not a repeat of that comment.

The CI run for this head (33282642243) has conclusion cancelled, not failure. Jobs were killed mid-step — API Tests (windows-latest) was inside Run API tests, pip install (macos) inside Import must not kill stdout. Nothing failed a test; they were terminated at 00:36:51.

It was not this branch. Around 00:35–00:37 roughly twenty CI runs were cancelled across unrelated PRs — runs #9141–#9161, spanning dependabot branches, feat/*, bot-fix/* and chore/i18n-autotranslate. Other sessions are already pushing commits that say so in as many words:

ci: re-run after the check group was cancelled by a racing branch update
ci: re-run after check group was cancelled by a PR-body edit
Fresh head: previous check set was cancelled mid-flight (sweeper/concurrency collision)

The trigger looks like the ~00:09 batch that merged main into every open PR at once, spawning ~25 concurrent runs against concurrency: cancel-in-progress: true (group ci-<pr number>).

I have re-run CI for this head. No commit was pushed for it and no empty commit was used.

For the record, this PR was fully green on the previous head d871bc7 — all 35 checks, E2E Gate included — and d3e25a0 only adds an automated main merge. The lockfile is byte-identical to the version that went green.

2. This PR overlaps four open Dependabot PRs — worth deciding before merge

Since #5252 put frontend/ under Dependabot, it has opened individual PRs for most of what this one does:

Dependabot PR Bump Covered here?
#5242 postcss 8.5.14 → 8.5.26 yes, identical
#5245 @babel/core 7.29.0 → 7.29.7 yes, identical
#5243 @remix-run/router + react-router-dom yes
#5244 react-router + react-router-dom yes
#5241 i18next-http-backend 2.7.3 → 3.0.5 no — deliberately deferred here
#5247 vite + @vitejs/plugin-react no — deliberately deferred here

I should have checked for these before opening this PR; my scan looked only for open harden/* branches, and these are dependabot/*. Flagging it rather than leaving you to find the duplication at merge time.

It is a real choice, not automatically a reason to close this. Merging this one lands the four in-range fixes as a single internally-consistent npm ci-verified lockfile, in one review and one CI cycle, and the four Dependabot PRs then close themselves as their bumps become no-ops. Merging them individually is four reviews and four CI cycles, each re-resolving the lockfile against a moving main. But if you would rather keep Dependabot as the single owner of dependency bumps, closing this PR loses nothing except the consolidation — every fix in it is available from #5242/#5243/#5244/#5245.

Either way #5241 and #5247 stay open, and the major upgrades stay a separate decision.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Green, except one CodeQL leg I cannot retry — needs a click from you

Everything actionable is now passing on d3e25a0:

  • E2E Gate (required) — success (01:27). This is the only context in branch protection, so the PR is mergeable.
  • CI re-run: all 19 legs success, Entitlement API tests included (completed 01:26:52).
  • Supply chain, Product record, OSS Golden Path, Cross-repo handoff, C6 gate, API Latency Smoke, drift-bot — all success.

The one loose end

mergeable_state reads unstable rather than clean, and I traced it to exactly two check runs, both from CodeQL run 33282640985:

Check Conclusion
Analyze (actions) cancelled at 00:36:54
CodeQL (rollup) neutral
Analyze (python) success
Analyze (javascript-typescript) success

That 00:36:54 timestamp is the same repo-wide mass cancellation described above — CodeQL was a third casualty alongside CI and the gate. Nothing failed analysis; the actions leg was killed partway.

I cannot clear this myself. CodeQL default-setup runs refuse an API retry:

POST /actions/runs/33282640985/rerun -> 403 This workflow run cannot be retried

I re-ran CI and the E2E Gate, but that route is closed for this one, and I will not push an empty commit to force it.

Two ways to clear it: re-run the CodeQL run from the Actions UI, or let it re-trigger on the next push to this branch.

Worth doing rather than ignoring, even though it does not block merge: this is a security PR, and Analyze (actions) is precisely the analysis that covers the workflow files. Leaving it half-run on a hardening change is the wrong thing to wave through — though note this PR touches only frontend/package-lock.json and no workflow files, so there is no plausible finding for it in this diff.


Generated by Claude Code

@8090-software-factory

Copy link
Copy Markdown

✅ Drift Bot (ClawMetry): no drift detected

Drift Bot analyzed the changed files against this project's blueprints and requirements and found no drift.

@vivekchand
vivekchand merged commit 50b9f71 into main Aug 30, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant