Repository navigation
Conversation
3ed5c7c to
c2365f3
Compare
c2365f3 to
0729e55
Compare
|
@abstractj FYI. There are test failures. |
79d713d to
7a823e8
Compare
f78d95f to
0c0fa94
Compare
…m overrides Apply pnpm.overrides to force patched versions of vulnerable transitive dependencies where no upstream fix is available from direct dependencies: - brace-expansion ^1 → ^1.1.13 (process hang and memory exhaustion) via eslint 9.x → minimatch 3.x - brace-expansion ^2 → ^2.0.3 (process hang and memory exhaustion) via typescript-eslint → minimatch 9.x - ajv ^6 → ^6.14.0 (ReDoS when using $data option) via eslint 9.x → @eslint/eslintrc - ajv ^8 → ^8.18.0 (ReDoS when using $data option) via vite-plugin-dts → @microsoft/api-extractor - lodash → ^4.17.23 (Prototype Pollution in _.unset and _.omit) via @patternfly/react-table, eslint-plugin-lodash, @microsoft/api-extractor - mdast-util-to-hast → ^13.2.1 (unsanitized class attribute) via @uiw/react-textarea-code-editor → rehype chain - jquery ^3 → ^3.5.0 (XSS vulnerabilities) via patternfly@3.59.5 in themes-vendor Not addressed: bootstrap 3.4.1 (no patched version exists, pulled by EOL patternfly@3.59.5). Signed-off-by: Bruno Oliveira da Silva <bruno@abstractj.com>
0c0fa94 to
b946a0d
Compare
There was a problem hiding this comment.
There are a lot of seemingly unrelated changes here. Does the AI say they are needed to go along with the overrides?
|
@ssilvert - I remember that those changes were related to some of the implicit eslint upgrades which now enforced stricter handling. |
OK. That makes sense. |
Actually, it doesn't make sense. Some of those changes don't seem to be related to new eslint rules. |
|
Here is what Claude Code had to say: Code Review: PR #47658 — Fix medium-severity npm transitive dependency vulnerabilities Overview Issues 1. Wrong lodash version override (bug) [js/package.json:286] sets "lodash": "^4.18.1", but the PR description says the target is ^4.17.23. Lodash's highest published version is 4.17.21 — there is no 4.18.x. The range ^4.18.1 (meaning >=4.18.1 <5.0.0) will never resolve to a valid version and will cause pnpm install to fail or silently skip the override. This is the most critical bug in the PR. 2. Overrides promised in description are missing from code The PR description lists these overrides, but they are absent from [js/package.json] brace-expansion ^1 → ^1.1.13 3. Unrelated source code changes bundled with security fixes Security vulnerability PRs should contain only the overrides and lockfile update. The following changes are unrelated and increase review surface area and regression risk: FlowDetails.tsx:208 — removes description = "" default; if callers of addFlowToFlow depend on a defined description, this silently passes undefined to the API 4. KeycloakContext.tsx architecture change js/libs/ui-shared/src/context/KeycloakContext.tsx replaces a per-provider lazy-initialized generic context with a single module-level createContext<KeycloakContext>. While fixing the anti-pattern of calling createContext in the render function is correct, the new design loses generic type safety (replaced by as casts). More importantly, it means all KeycloakProvider instances share a single context, which could be a breaking change if multiple providers with different environment types are used (e.g., admin-ui and account-ui loaded together). This deserves its own focused PR with explicit justification. 5. AI-generated changes acknowledged but not fully verified The PR description states it was AI-generated and reviewed before submission. The lodash version bug (#1 above) suggests that review was incomplete. Given the large number of lock file changes and source changes, a thorough manual verification is warranted. What Looks Good Recommendations Fix the lodash override — it should be "lodash": "^4.17.21" (or whatever the latest 4.17.x is that has the fix), not ^4.18.1 |
|
@ssilvert the changes here were to fix the issues with eslint, if they don't make sense. May I ask you to please take over and submit a new one if you would like? Also I don't care what Claude has to say. I'm more interested on what you think ;) |
|
claude is wrong there is lodash 4.18.1 |
There was a problem hiding this comment.
I wouldn't create overrides for all of these:
ajv ^6 => @eslint/eslintrc ^3.3.5 (depends on ajv: ^6.14.0). Bump eslint to latest 9.x. resolves ajv@6.12.6 → 6.14.x
ajv@^8: ^8.18.0 => vite-plugin-dts@4.5.4 allows @microsoft/api-extractor: ^7.50.1
vite-plugin-dts 7.58.x) uses @rushstack/node-core-library with ajv: ~8.18.0. Bump vite-plugin-dts
lodash ^4.18.1 => @patternfly/react-table@5.4.16 and eslint-plugin-lodash use ^4.17.21 pnpm update lodash can land 4.17.23 (CVE fix) without override. api-extractor’s ~4.17.15 also allows 4.17.23 upgrading api-extractor removes lodash from that chain.
mdast-util-to-ha => Chain is @uiw/react-textarea-code-editor → rehype → … → mdast-util-to-hast. hast-util-to-html ^13.2.1 allows ^13.0.0; lockfile has 13.2.0. pnpm update mdast-util-to-hast (or bump the rehype stack) should reach 13.2.1 without override.
|
created alternative that does the same, but with less changes #49544 |
|
Closed in favor of #49544 |
Apply pnpm.overrides to force patched versions of vulnerable transitive dependencies where no upstream fix is available from direct dependencies:
brace-expansion ^1 → ^1.1.13 (process hang and memory exhaustion) via eslint 9.x → minimatch 3.x
brace-expansion ^2 → ^2.0.3 (process hang and memory exhaustion) via typescript-eslint → minimatch 9.x
ajv ^6 → ^6.14.0 (ReDoS when using $data option) via eslint 9.x → @eslint/eslintrc
ajv ^8 → ^8.18.0 (ReDoS when using $data option) via vite-plugin-dts → @microsoft/api-extractor
lodash → ^4.17.23 (Prototype Pollution in _.unset and _.omit) via @patternfly/react-table, eslint-plugin-lodash, @microsoft/api-extractor
mdast-util-to-hast → ^13.2.1 (unsanitized class attribute) via @uiw/react-textarea-code-editor → rehype chain
jquery ^3 → ^3.5.0 (XSS vulnerabilities) via patternfly@3.59.5 in themes-vendor
Not addressed: bootstrap 3.4.1 (no patched version exists, pulled by EOL patternfly@3.59.5).
Note: These changes were created with the assistance of AI and reviewed before
submission.