build: update various dependencies - #1000
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change updates GitHub Buildah actions, Tekton task bundles and image-build parameters, pinned container image digests, JavaScript dependencies, and React Router imports with Jest support. ChangesBuild and dependency maintenance
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Line 34: Update all six redhat-actions references in the workflow, including
the build and push steps, from mutable `@v3` tags to reviewed immutable
40-character commit SHAs. Preserve each action and its configuration, including
the registry credential inputs.
In `@package.json`:
- Line 66: Remove the `@types/react-router-dom` entry from the package.json
dependency list, retain react-router-dom’s built-in type declarations,
regenerate the lockfile, and rerun the TypeScript build to verify the dependency
cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 54629cd6-dc0e-400f-8e2d-49dd669e0b96
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/components/simpleDropdown/__tests__/__snapshots__/simpleDropdown.test.tsx.snapis excluded by!**/*.snapsrc/views/credentials/__tests__/__snapshots__/addCredentialModal.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
.github/workflows/build.yml.tekton/discovery-ui-pull-request.yaml.tekton/discovery-ui-push.yamlContainerfilepackage.json
mirekdlugosz
left a comment
mirekdlugosz
left a comment
There was a problem hiding this comment.
Snapshot changes seem to be related to patternfly/patternfly-react#12377 (part of larger effort to replace FontAwesome with Red Hat-branded icons).
Before release, we should eyeball UI to make sure this didn't break things for us.
I'm giving tentative approval, obviously subject to fixing CI.
npm security upgrades — postcss override + react-router v7 → v8The last several commits added to this branch address two high-severity vulnerabilities flagged by our npm audit check in CI. Here is a full account of what changed, why, and what risks (if any) remain.
What triggered this work
Change 1 — postcss override (low risk)
Risk: none. postcss is a CSS processing tool used only during the webpack build. The override is within the same minor line ( Change 2 — react-router v7 → v8 (low remaining risk)The GHSA-qwww-vcr4-c8h2 advisory has no fix in the 7.x line — the react-router team's first patched release is What the v8 migration involvedreact-router v8 removes the Five source files were updated — only their import paths changed, no logic:
The Jest test configuration also required a change. react-router v8 is ESM-only, and its production build contains All 405 unit tests pass, all 284 snapshots are unchanged. The build container ( Known risks
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
config/jest.setupTests.js (3)
60-61: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the router hook mocks configurable and stable.
useLocationalways reports/, so layout tests cannot cover other active routes.useNavigatecreates a newjest.fn()on every render, so tests cannot observe calls through a stable reference. Use a configurable location mock and one shared navigation mock.Proposed fix
+const mockLocation = jest.fn(() => ({ pathname: '/' })); +const mockNavigate = jest.fn(); + jest.mock('react-router', () => ({ - useLocation: () => ({ pathname: '/' }), - useNavigate: () => jest.fn(), + useLocation: mockLocation, + useNavigate: () => mockNavigate,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/jest.setupTests.js` around lines 60 - 61, Update the router hook mocks in the Jest setup so useLocation returns a configurable mock location that tests can override, while preserving the default pathname. Define one shared navigation jest mock and have useNavigate return that same reference on every render instead of creating a new mock.
62-64: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftUse a router-backed test for route matching.
Routesrenders every child,Routerenders everyelement, andNavigateis a no-op. The snapshot insrc/__tests__/routes.test.tsxat Lines 5-11 cannot detect incorrect path matching, the root redirect, or the wildcard route. Keep this mock for isolated component tests, but coverAppRouteswith a configurable memory router or equivalent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/jest.setupTests.js` around lines 62 - 64, Keep the existing react-router mock for isolated component tests, but add a router-backed test setup for AppRoutes using a configurable memory router or equivalent. Update src/__tests__/routes.test.tsx to render AppRoutes with representative locations and assert path matching, the root redirect, and wildcard route behavior instead of relying on the mock Routes and Route implementations.
55-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPreserve hyperlink semantics in the
NavLinkmock.Rendered tests currently receive an
<a>whose destination prop isto, so the anchor has nohref. Removetobefore spreading props and forward the validhref, handling string and path-name destinations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/jest.setupTests.js` around lines 55 - 58, Update the NavLink mock to remove the routing-only to prop before spreading props onto the anchor, and derive a valid href from string destinations or path-name destination objects. Preserve all other props and the existing MockNavLink rendering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@jest.config.js`:
- Line 39: Align the Jest transform configuration with the react-router
exception in the transform-ignore pattern: either extend the transform matcher
to compile .mjs and .cjs files, or remove those extensions from the exception
after verifying the React Router entrypoints. Ensure directly loaded React
Router modules are transformed rather than treated as asset filenames.
In `@package.json`:
- Line 16: Align the package.json Node.js engines requirement with the pinned
UBI9 image and GitHub workflow selectors: either update the image digest to a
Node.js 22.22.0-or-newer build or lower the "node" floor below the versions
those environments resolve. Ensure the declared requirement does not reject the
configured build and workflow runtimes.
---
Nitpick comments:
In `@config/jest.setupTests.js`:
- Around line 60-61: Update the router hook mocks in the Jest setup so
useLocation returns a configurable mock location that tests can override, while
preserving the default pathname. Define one shared navigation jest mock and have
useNavigate return that same reference on every render instead of creating a new
mock.
- Around line 62-64: Keep the existing react-router mock for isolated component
tests, but add a router-backed test setup for AppRoutes using a configurable
memory router or equivalent. Update src/__tests__/routes.test.tsx to render
AppRoutes with representative locations and assert path matching, the root
redirect, and wildcard route behavior instead of relying on the mock Routes and
Route implementations.
- Around line 55-58: Update the NavLink mock to remove the routing-only to prop
before spreading props onto the anchor, and derive a valid href from string
destinations or path-name destination objects. Preserve all other props and the
existing MockNavLink rendering behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9303e74d-4246-44d8-a85c-0e25468cf530
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
config/jest.setupTests.jsjest.config.jspackage.jsonsrc/app.tsxsrc/components/viewLayout/viewLayout.tsxsrc/routes.tsxsrc/views/notFound/notFound.tsxsrc/views/scans/viewScansList.tsx
07651fd to
e47e9f3
Compare
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
59c2a6a to
2ff88bf
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Bumps the deps-patch group with 6 updates in the / directory: | Package | From | To | | --- | --- | --- | | [@types/react](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react) | `19.2.17` | `19.2.18` | | [@types/react-dom](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react-dom) | `19.2.3` | `19.2.4` | | [i18next-resources-to-backend](https://github.com/i18next/i18next-resources-to-backend) | `1.2.1` | `1.2.2` | | [npm-run-all2](https://github.com/bcomnes/npm-run-all2) | `9.0.2` | `9.0.3` | | [react-i18next](https://github.com/i18next/react-i18next) | `17.0.10` | `17.0.11` | | [react-router-dom](https://github.com/remix-run/react-router/tree/HEAD/packages/react-router-dom) | `7.18.1` | `7.18.2` | Updates `@types/react` from 19.2.17 to 19.2.18 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react) Updates `@types/react-dom` from 19.2.3 to 19.2.4 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react-dom) Updates `i18next-resources-to-backend` from 1.2.1 to 1.2.2 - [Changelog](https://github.com/i18next/i18next-resources-to-backend/blob/main/CHANGELOG.md) - [Commits](i18next/i18next-resources-to-backend@v1.2.1...v1.2.2) Updates `npm-run-all2` from 9.0.2 to 9.0.3 - [Release notes](https://github.com/bcomnes/npm-run-all2/releases) - [Changelog](https://github.com/bcomnes/npm-run-all2/blob/master/CHANGELOG.md) - [Commits](bcomnes/npm-run-all2@v9.0.2...v9.0.3) Updates `react-i18next` from 17.0.10 to 17.0.11 - [Changelog](https://github.com/i18next/react-i18next/blob/master/CHANGELOG.md) - [Commits](i18next/react-i18next@v17.0.10...v17.0.11) Updates `react-router-dom` from 7.18.1 to 7.18.2 - [Release notes](https://github.com/remix-run/react-router/releases) - [Changelog](https://github.com/remix-run/react-router/blob/react-router-dom@7.18.2/packages/react-router-dom/CHANGELOG.md) - [Commits](https://github.com/remix-run/react-router/commits/react-router-dom@7.18.2/packages/react-router-dom) --- updated-dependencies: - dependency-name: "@types/react" dependency-version: 19.2.18 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: deps-patch - dependency-name: "@types/react-dom" dependency-version: 19.2.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: deps-patch - dependency-name: i18next-resources-to-backend dependency-version: 1.2.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: deps-patch - dependency-name: npm-run-all2 dependency-version: 9.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: deps-patch - dependency-name: react-i18next dependency-version: 17.0.11 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: deps-patch - dependency-name: react-router-dom dependency-version: 7.18.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: deps-patch ... Signed-off-by: dependabot[bot] <support@github.com>
postcss <=8.5.22 (GHSA-r28c-9q8g-f849, GHSA-fxqj-rqcc-2cmp) is pinned to exactly 8.5.16 by weldable, so npm audit fix cannot resolve it. Adding an explicit override forces >=8.5.25 across the dep tree. Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Resolves GHSA-qwww-vcr4-c8h2 (React Router RSC Mode CSRF Bypass). v8 removes the react-router-dom package; DOM APIs move to react-router/dom, all routing primitives move to react-router. Note: BrowserRouter is not in react-router/dom in v8 (that subpath only exports HydratedRouter/RouterProvider for Framework/SSR mode). All 5 source imports use 'react-router' directly. Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
react-router v8 is ESM-only. Un-ignore it from transformIgnorePatterns so ts-jest can transform it for the CommonJS Jest environment. Update the module mock target from react-router-dom to react-router, and drop jest.requireActual since the production build uses import.meta.hot which ts-jest cannot compile to CJS. All needed exports are now provided explicitly in the mock factory. Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Add an explanatory comment above the react-router mock so future developers understand that jest.requireActual cannot be used here: react-router v8's production build contains import.meta.hot (ESM/HMR syntax) which ts-jest compiling to CommonJS cannot parse. Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The change addresses this security recommendation from coderabbitai: > 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win > > Security Misconfiguration (CWE-829): Inclusion of Functionality from Untrusted Control Sphere
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Two independent vulnerability chains existed in the dependency tree,
neither fixable by `npm audit fix` without breaking changes.
## Chain 1: less / image-size / weldable
`weldable` (a direct production dep used for webpack builds) depends on
`less@4.6.7`. The less package lists `image-size` as an optional dep and
installs it at `~0.5.0`. Two advisories affect this chain:
- less 2.2.0–4.6.7: no specific CVE assigned but the advisory range
covers all versions up to and including 4.6.7.
- image-size 0.x (all): GHSA advisories 1138808 and 1138809 document
DoS via infinite loops in the ICNS, JXL, and HEIF parsers.
Because weldable@6.0.2 (the current latest) still pins less@4.6.7,
upgrading weldable alone does not help. The fix is to add `less` to the
project's npm `overrides`, forcing it to `^4.8.1`. less@4.8.1 is above
the vulnerable range and replaces its optional `image-size` dep with
`probe-image-size`, eliminating the image-size exposure entirely.
## Chain 2: swagger-mock-api / hoek / z-schema / validator
`swagger-mock-api` (devDep, used in scripts/apiDev.js to serve a mock
API during local development) pulled in a transitive chain of abandoned
or vulnerable packages:
- hoek@2.14.0 (advisory 1105092, 1105121): prototype pollution via the
`clone` function. hoek 2.x is from the pre-scoped hapi era and was
deprecated years ago. swagger-mock-api@1.6.0 is the last published
version and still depends on this range; there is no upstream fix.
- validator@10.11.0 (advisories 1095093, 1109241, 1112713): installed
inside z-schema's own node_modules (z-schema requires `^10.0.0`).
The vulnerabilities cover ReDoS and URL-validation bypass. z-schema
itself is a transitive dep of swagger-parser@3.x, which swagger-mock-
api uses internally. Overriding validator globally would risk breaking
z-schema's API expectations (10.x vs 13.x are not compatible), and a
nested override cannot satisfy z-schema's `^10.0.0` semver constraint.
No upstream fix exists.
Because no version of swagger-mock-api resolves these issues, the
package is removed entirely. `scripts/apiDev.js` is rewritten to
replicate the same behaviour using packages already present in the
dependency tree:
- `express` (already a devDep): HTTP server and router.
- `js-yaml` (already in node_modules via the existing `overrides` entry
for it): YAML parsing of the downloaded swagger spec.
The replacement implementation parses the Swagger 2.0 spec on startup,
walks the `paths` object to register Express routes (converting
`{param}` syntax to `:param`), resolves `$ref` pointers inline, and
returns the first matching 2xx response's `examples['application/json']`
or `schema.example` value. The logging format and CLI interface
(`--file`, `--port`) are preserved unchanged.
The standalone `validator` devDependency (added previously as a
workaround attempt) is also removed; it did not affect z-schema's nested
install and is no longer needed.
All CI checks pass after these changes: lint (0 errors), unit tests
(405/405), webpack production build, integration tests, and
`npm audit --audit-level=high --omit dev --omit peer` (0 vulnerabilities).
Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Three bugs were found in the swagger-mock-api replacement introduced in the
previous commit.
Bug 1 — YAML parser incompatibility: The replacement used js-yaml (forced
to 5.x via the project's overrides) to parse the quipucords swagger spec.
js-yaml 5.x enforces stricter flow-collection indentation rules than the
3.x version swagger-mock-api used internally. The quipucords spec has
an indentation inconsistency in an embedded JSON example block at line
~2511 that 5.x rejects with "deficient indentation", preventing the server
from starting at all. Fixed by switching to the `yaml` package (v2.9.0,
already present in node_modules as a transitive dep of webpack/patternfly)
which handles the spec without errors.
Bug 2 — Empty responses: None of the 40 operations in the quipucords spec
define response-level examples. The original swagger-mock-api used chance.js
to generate random schema-conforming mock data for every endpoint. The first
replacement implementation returned {} for all operations with no examples,
which would have broken the frontend's ability to render any list or detail
view. Fixed by adding generateFromSchema(), which walks the response schema,
resolves $ref pointers, merges allOf entries, and builds a response object
using property-level example values from the spec (many properties carry
examples), falling back to type-appropriate defaults ([] for arrays, 0 for
integers, '' for strings, etc.). Circular $ref cycles are guarded with a
visited set. Operations with no response body (e.g. DELETE) correctly send
a 204 with no body.
Bug 3 — Empty results arrays: swagger-mock-api used chance.js to generate
random items for array-typed schema nodes, so list endpoints returned
populated results arrays. The generateFromSchema() implementation returned
[] for all arrays, causing empty lists in the UI — a regression in
developer experience. Fixed by generating one item from the items subschema
when present, giving the UI enough data to render populated list views.
Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
4 participants
This branch includes:
git cherry-pick 01c25a5436b6c1c0e7597dc6bbf7432c0768184dgit cherry-pick a730aba97f8cf48781c73b17b9c3f5dd7fc6da17git cherry-pick dbb3ed2ff29f2625095fe90557f4583cdf9f4f5agit cherry-pick 0d802cabe2c5000d69d90ca737ee540c53bba6bdgit cherry-pick 6f888788e0cdb54268bd8fb9c8253085a61b830bgit cherry-pick e6eadeb8e19021e2c5e3f9892333ea0ac5b29ebbgit cherry-pick 89c159903aa1c4b94b85ef0c4675a9e4908035fcnpm updatemake update-lockfilesgit cherry-pick 7d78f3b9f13f750f1ad5747f6ef3a6e1cf0830a6package-lock.jsonthat I manually resolved my removing it and runningnpm installgit cherry-pick b13e63c855a22cb2b50b02d1ff2d9c591ce9f141package-lock.jsonthat I manually resolved and rerannpm installgit cherry-pick a7bfc661b911653362e6188f6f2dc29445a42b5eRedundant PRs (empty cherry-pick) after these changes:
https://redhat.atlassian.net/browse/DISCOVERY-1449
Relates to JIRA: DISCOVERY-1449
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores