Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

search-ui-react: Resolve Vulnerabilities #484

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

mkouzel-yext
Copy link
Contributor

Upgrades dependency versions to resolve potential vulnerabilities.

Regenerates package-lock.json to include the safe dependencies.

J=VULN-39418, VULN-39419
TEST=none

Saw that babel imports in package-lock.json and test-site/package-lock.json
were for versions at or above the safe dependency.
@mkouzel-yext mkouzel-yext requested a review from a team as a code owner January 29, 2025 22:15
@semgrep-code-yext
Copy link

Legal Risk

The following dependencies were released under a license that
is currently prohibited by your organization. Merging is blocked until this is resolved.

Recommendation

Reach out to your security team or Semgrep admin to address this issue. In special cases, exceptions may be made for dependencies with violating licenses, however, the general recommendation is to avoid using a dependency under such a license

non-standard

The `npm run build` command was failing due to errors with the tsup
build command. The increase in heap space is a temporary fix that we
may run into again as the repo grows - the out of memory error that
it prevents is an ongoing issue with the library itself
(egoist/tsup#920).
The GeolocationPosition and GeolocationCoordinates objects now
require toJSON() methods to be specified. Our implementation of
these methods isn't important to my knowledge, but their absence
is causing the LocationBias and Geolocation unit tests to fail.
Causing npm ci to fail when run remotely for automatic tests.
After making the library upgrades that were required to resolve vulnerabilities, new errors appeared that prevented our unit tests from passing. This commit updates the FilterSearch test so it passes without error.
@@ -386,7 +386,7 @@ export function SearchBar({
entityPreviewsCount
);
const activeClassName = classNames('relative z-10 bg-white border rounded-3xl border-gray-200 w-full overflow-hidden', {
['shadow-lg' ?? '']: hasItems
['shadow-lg']: hasItems
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this impacted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was causing tsup src/index.ts --dts --sourcemap --format esm,cjs to fail because ?? '' is never used. Ultimately preventing npm run build from succeeding.

@@ -36,7 +36,7 @@
],
"scripts": {
"build": "rm -rf lib/** && rm -rf dist/** && npm run build:js && npm run build:css && npm run api-extractor && npm run generate-docs && npm run generate-notices",
"build:js": "tsup src/index.ts --dts --sourcemap --format esm,cjs && npm run create:lib-esm && npm run create:lib-commonjs",
"build:js": "NODE_OPTIONS='--max-old-space-size=16384' tsup src/index.ts --dts --sourcemap --format esm,cjs && npm run create:lib-esm && npm run create:lib-commonjs",
Copy link
Contributor

Choose a reason for hiding this comment

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

Where's this coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tsup build command was failing due to OOM error in its worker process. This isn't a permanent fix because as the repo grows the heap size may need to be bumped again, but the root of the problem lies with tsup itself (egoist/tsup#920).

@mkouzel-yext
Copy link
Contributor Author

mkouzel-yext commented Jan 30, 2025

The reason for these other changes is to try to get the automatic checks passing again. After making the package upgrades that resolving our vulnerabilities require, the majority of our automatic tests failed due to build errors. I'll reply with the individual reasons for each, but this is the overarching goal.

Resolves errors that were being logged due to package upgrades in test files. These were prevent our automatic checks from passing.
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.

2 participants