chore: Remediate 4 Dependabot security alerts - #74
Open
ernst-dev wants to merge 5 commits into
Open
Conversation
Bump js-yaml 4.3.0 → 4.3.1 (GHSA-5p4m-2wfm-xmqj, HIGH). Also bump nanoid 3.3.16 → 3.3.17 (GHSA-2v37-7h3g-55p8, HIGH, transitive via postcss; no open Dependabot alert, but addressed opportunistically). nanoid@3.3.18 was npm's computed pick but its NpmPrettyMuch entry was empty (scan in progress); walked back to 3.3.17 per C3. react-router/react-router-dom alerts left open — see PR description.
Remove the react-router-dom dependency entirely from build-tools. AppModesProvider used react-router-dom solely for useSearchParams, which read/wrote the query string portion of the hash URL. Replace with a self-contained useHashSearchParams hook backed by window.location.hash, window.history.replaceState, and the hashchange event. This closes the three open Dependabot alerts for react-router/ react-router-dom (GHSA-wrjc-x8rr-h8h6, GHSA-337j-9hxr-rhxg, GHSA-jjmj-jmhj-qwj2) without requiring a React 18 upgrade or any changes to consumer packages. The hook has no Router context dependency, so consumers no longer need react-router-dom at all to use dev-pages-utils.
ernst-dev
changed the title
Running npm install --package-lock-only to remove react-router-dom also dropped stylelint and its bundled deps from the lockfile, since stylelint is only a peerDependency and not in devDependencies. CI installs peer deps by default, but vitest cannot resolve the stylelint import when the lockfile has no entry for it. Restore the 7 stylelint lockfile entries from the pre-change lockfile to match the same state as main.
ernst-dev
enabled auto-merge
August 10, 2026 11:55
ernst-dev
disabled auto-merge
August 10, 2026 11:59
Member
There was a problem hiding this comment.
guess these aren't expected to be here?
There was a problem hiding this comment.
Pull request overview
Remediates Dependabot security alerts by updating vulnerable transitive dependencies and removing react-router(-dom) usage from dev-pages-utils in favor of a self-contained hash query-string implementation.
Changes:
- Removes
react-router-domfrompeerDependencies/devDependenciesand replacesuseSearchParamsusage with a localuseHashSearchParamshook. - Updates
package-lock.jsonto pick up patched versions (notablyjs-yaml@4.3.1andnanoid@3.3.17) and reflect dependency removals. - Adds a new Vitest test file intended to cover the hash query-string helpers.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Removes react-router-dom from dependencies/peers as part of security remediation. |
| package-lock.json | Lockfile updates to patched dependency versions and removal of router-related packages. |
| lib/dev-pages-utils/app-modes-provider.tsx | Replaces react-router-dom query param handling with a hash-based hook implementation. |
| lib/dev-pages-utils/tests/app-modes-provider.test.ts | Adds tests targeting the new hash query-string behavior. |
Suppressed comments (1)
package.json:37
react-router-domwas removed frompeerDependencies, but it is still listed underpeerDependenciesMeta. Keeping meta entries for non-existent peer deps is misleading and can confuse consumers/tools; please remove thereact-router-domentry here as well.
"react": ">=16.14.0",
"stylelint": "^16.8.1"
},
"peerDependenciesMeta": {
"react": {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+5
to
+8
| // We test the two pure helpers extracted from app-modes-provider: | ||
| // getHashSearchParams and setHashSearchParams are not exported, so we | ||
| // exercise them indirectly through the behaviour they produce, using | ||
| // vi.stubGlobal to control window.location.hash and window.history. |
| @@ -0,0 +1,116 @@ | |||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| import { describe, expect, test, vi, beforeEach, afterEach } from "vitest"; | |||
…-string
The previous commit dropped react-router-dom but replaced useSearchParams
with hand-rolled string surgery on window.location.hash. Move that logic
onto the query-string package instead.
- queryString.extract() replaces the manual indexOf("?")/slice() parsing.
Note the leading "#" must be stripped first: extract() calls removeHash()
internally, so passing the raw hash would always yield an empty query.
- queryString.stringify(params, { sort: false }) replaces
new URLSearchParams(params).toString(). sort: false is required because
query-string sorts keys by default, which would reorder the URL params.
URLSearchParams is kept as the value handed to parseAppModes, so repeated
params still resolve last-one-wins exactly as react-router-dom's
useSearchParams did.
query-string is pinned to 9.4.1 rather than 9.5.0: 9.5.0 is four days old
and pulls decode-uri-component 0.5.0, whereas the 9.4.1 tree is fully
settled (filter-obj 5.1.0, split-on-first 3.0.0, decode-uri-component
0.4.1). The 9.x line is ESM-only, which matches this package's
"type": "module".
Also removes the now-dangling react-router-dom entry from
peerDependenciesMeta, and regenerates the lockfile so that npm ci works
again.
The previous version of this file never imported app-modes-provider; it re-implemented the parsing and serialisation inline and asserted on that copy. Gutting both helpers left all five tests green, so the file could not detect a regression. Exercise the real getHashSearchParams/setHashSearchParams instead, and cover the cases that matter for the query-string swap: missing params, empty values, repeated params, percent-encoding, param ordering, and a write/read round trip.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes all 4 open Dependabot alerts in build-tools. The substantive change:
dev-pages-utilsno longer depends onreact-router-dom, and the dev-page mode query string is now handled with thequery-stringpackage.Related links, issue #, if available: n/a
Why not simply upgrade
react-router-dom?Because there is no patched
react-router-domto upgrade to.GHSA-jjmj-jmhj-qwj2affectsreact-router-dom>= 6.30.2, <= 6.30.4, and GitHub reports no first-patched version for that package.6.30.4is the final release of the v6 line.react-router>= 7.13.0(alerts revert: "feat: add reusable app mode url parser" #58/revert: Revert "feat: add reusable app mode url parser" #59 only in>= 7.18.0) — i.e. a v6 → v7 major.react-router@7declarespeerDependencies: { "react": ">=18", "react-dom": ">=18" }andengines.node >= 20. This package pinsreact@16.14.0, and CI still builds/tests consumers against React 16, so adopting v7 would force a React major version bump.A React version bump is out of scope, so the dependency is dropped instead.
dev-pages-utilsonly ever usedreact-router-domforuseSearchParams, purely to read/write the query-string portion of the hash URL (/#/page?mode=dark). It never used routing, route matching or navigation.What replaced it
AppModesProvideruses a localuseHashSearchParamshook built onquery-stringpluswindow.location.hash, kept in sync via thehashchangeevent. No Router context is required any more; consumers that already wrap it inHashRouterare unaffected.query-stringversion:^9.4.1, pinned to 9.4.1 in the lockfile."type": "module"with an ESM-onlyexportsmap and no CJS build, so a CJS consumer could notrequire()dev-pages-utilstoday either. Every repo that consumesdev-pages-utilsbuilds with Vite 6+ / Vitest 4. A CJS fallback to7.xwas therefore unnecessary.moduleResolution: "node"becausequery-stringships a root-leveltypesfield —tsc --noEmitpasses.decode-uri-component@0.5.0, whereas the 9.4.1 tree is fully settled (filter-obj@5.1.02022,split-on-first@3.0.02021,decode-uri-component@0.4.12022). The caret range lets it float once the newer tree has bedded in.peerDependencieschangereact-router-domwas declared inpeerDependencies(^5.3.0 || ^6.0.0 || ^7.0.0) and is removed here, together with its now-danglingpeerDependenciesMetaentry.Assessment: this is not a breaking change for consumers.
optional: true, so npm never required consumers to install it, and its absence never warned or errored.components,board-components,chart-components,code-view,chat-components,component-toolkit) declares its ownreact-router-domdevDependency, so this removes nothing from them.AppModesProviderpreviously had to be rendered inside a Router, and now does not.^5.3.0, butuseSearchParamsdoes not exist in react-router v5.It is still a reduction of a declared public interface, so calling it out explicitly rather than burying it.
Behaviour preservation
URLSearchParamsis deliberately retained as the value handed toparseAppModes, so parsing semantics stay identical to react-router'suseSearchParams. Checked against the previous implementation across 11 hash shapes:?theme=&mode=dark)"", not dropped?mode=light&mode=dark)parseAppModesstringify(..., { sort: false }); query-string sorts keys by default?One gotcha worth knowing:
queryString.extract()callsremoveHash()internally, so the leading#must be stripped before extracting — otherwise the query is always empty.Alerts addressed
>= 4.0.0, < 4.3.1>= 6.30.2, <= 6.30.4>= 6.4.0, < 7.18.0>= 6.0.0, < 7.18.0nanoidadditionally goes 3.3.16 → 3.3.17 opportunistically (GHSA-2v37-7h3g-55p8, no open alert).npm auditreports 0 vulnerabilities.Merging this PR closes all 4 open alerts (#57, #58, #59, #66).
NpmPrettyMuch status
query-stringis a new non-dev dependency, so it does need an NPMPM check, and I was unable to run one —npmpm.corp.amazon.comis AEA-gated and returns 403. Versions were chosen to be old and settled (the three transitive deps date from 2021–2022) to maximise the chance they are already mirrored, but please confirm the NPMPM gate before merging.react-router-dom/react-routerare removed and need no check.Lockfile
Regenerated with node 20 / npm 10.8.2 (lockfileVersion 3 preserved), with
prepare-package-lockapplied. The delta versusmainis exactly:query-string@9.4.1,filter-obj@5.1.0,split-on-first@3.0.0,decode-uri-component@0.4.1react-router-dom@6.30.4,react-router@6.30.4,@remix-run/router@1.23.3js-yaml4.3.0 → 4.3.1,nanoid3.3.16 → 3.3.17No unrelated drift, and stable under a repeated
npm install.How has this been tested?
npm run build,npm run lint,npx tsc --noEmitandnpm testall pass on node 20.20.2 / npm 10.8.2 — 81 tests across 11 files.This also fixes a latent defect on the branch:
npm cipreviously failed withMissing: emoji-regex@8.0.0 from lock file, and now succeeds.On the test file that reviewers flagged as functionally useless (@gethinwebster: "guess these aren't expected to be here?"; Copilot: "these tests don't exercise the production implementation ... they would still pass even if the actual implementation regresses"):
The criticism was correct, and rather than take it on faith it was verified empirically. The old file never imported
app-modes-provider— it re-implemented the parsing/serialisation inline and asserted on that copy. Replacing both helpers with stubs (getHashSearchParams→ always empty,setHashSearchParams→ no-op) left all 5 tests still passing.The file has been replaced, not deleted, with 14 tests that import and exercise the real helpers. A mutation check confirms they actually bite — 8 deliberate defects injected, 8 caught:
getHashSearchParamsalways returns empty#not stripped (theextract()gotcha)stringifywithout{ sort: false }setHashSearchParamsmade a no-op?even with no paramshashchangedispatchCopilot also noted an unused
beforeEachimport. That is gone in the rewrite. For the record its stated consequence did not apply here:.eslintrcsetsignorePatterns: ["lib/*"]and turnsno-unused-varsoff, so it would not have failednpm run lint.Reviewers can verify efficiently with
npm ci && npm test, and thedry-runworkflow builds every consumer repo (includingchat-components, the only consumer ofAppModesProvider) against this branch.Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.