PR to fix ansible-ai-connect-service: Cross-site Scripting (XSS) in serialize-javascript #1353

justjais · 2024-10-17T14:31:13Z

Jira Issue: https://issues.redhat.com/browse/AAP-31381

Description

PR to fix ansible-ai-connect-service: Cross-site Scripting (XSS) in serialize-javascript

Testing

Steps to test

Pull down the PR
Run npm install and node packages should install as expected w/o failures
Run npm audit fix to verify if there's any vulnerabilities, currently there's 0.

Scenarios tested

NA

Production deployment

This code change is ready for production on its own
This code change requires the following considerations before going to production:

jameswnl · 2024-10-17T18:35:46Z

we probably have to fix it in the package.json?

justjais · 2024-10-22T13:00:11Z

@jameswnl there's no direct dependency over serialize-javascript, its being used by other direct dependent libraries. Also, I've verified that due to some reason the failure we see in CI serialize-javascript/-/serialize-javascript-6.0.2.tgz ** seems to be corrupted. Trying again is faced when we user [email protected] which is currently being used in repos CI.

It's working as expected with [email protected], and specifically 21.7.3 node release with which things were working as expected in my machine.

I'll raise the PR to upgrade [email protected] version

manstis · 2024-10-29T05:51:40Z

Upgrade to PF5 isn't easy.

You'll need to bump the version of Ansible UI framework too (to one using PF5) and fix the fallout too.

justjais · 2024-10-29T06:05:54Z

@manstis yea, this isn't a 1 story point and straight CVE fix, for fixing this patternfly needs to be updated to ^5.1 coz npm update updates ansible-ui-framework package which is in turn dependent over patternfly ^5.1, and the fallout needs to be resolved as you mentioned.

I'll bring this up during today's scrum and how to move forward with the issue.

github-actions · 2024-11-11T18:34:42Z

# npm audit report

happy-dom  <15.10.2
Severity: critical
happy-dom allows for server side code to be executed by a <script> tag - https://github.com/advisories/GHSA-96g7-g7g9-jxw8
fix available via `npm audit fix`
node_modules/happy-dom

1 critical severity vulnerability

To address all issues, run:
  npm audit fix

github-actions · 2024-11-11T18:34:47Z

# npm audit report

happy-dom  <15.10.2
Severity: critical
happy-dom allows for server side code to be executed by a <script> tag - https://github.com/advisories/GHSA-96g7-g7g9-jxw8
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/happy-dom

1 critical severity vulnerability

To address all issues (including breaking changes), run:
  npm audit fix --force

github-actions · 2024-11-11T18:42:39Z

# npm audit report

happy-dom  <15.10.2
Severity: critical
happy-dom allows for server side code to be executed by a <script> tag - https://github.com/advisories/GHSA-96g7-g7g9-jxw8
fix available via `npm audit fix`
node_modules/happy-dom

1 critical severity vulnerability

To address all issues, run:
  npm audit fix

github-actions · 2024-11-11T19:10:02Z

# npm audit report

happy-dom  <15.10.2
Severity: critical
happy-dom allows for server side code to be executed by a <script> tag - https://github.com/advisories/GHSA-96g7-g7g9-jxw8
fix available via `npm audit fix`
node_modules/happy-dom

1 critical severity vulnerability

To address all issues, run:
  npm audit fix

github-actions · 2024-11-11T19:20:02Z

# npm audit report

happy-dom  <15.10.2
Severity: critical
happy-dom allows for server side code to be executed by a <script> tag - https://github.com/advisories/GHSA-96g7-g7g9-jxw8
fix available via `npm audit fix`
node_modules/happy-dom

1 critical severity vulnerability

To address all issues, run:
  npm audit fix

github-actions · 2024-11-19T05:39:39Z

# npm audit report

cross-spawn  <7.0.5
Severity: high
Regular Expression Denial of Service (ReDoS) in cross-spawn - https://github.com/advisories/GHSA-3xgq-45jj-v275
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/npm-run-all/node_modules/cross-spawn
  npm-run-all  >=2.1.1
  Depends on vulnerable versions of cross-spawn
  node_modules/npm-run-all

2 high severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

manstis

The styling is now, unfortunately, wrong:

AAP-31381

Original

Furthermore the "kebab" menu (top left corner) does not work properly.

It should collapse the left-hand-side navigation menu.

manstis · 2024-11-22T12:11:11Z

ansible_ai_connect_admin_portal/src/SingleInlineEdit.tsx

@@ -24,7 +24,8 @@ export const SingleInlineEdit = (props: InlineTextInputProps) => {
        <InputGroup>
          <TextInput
            type={isPassword && passwordHidden ? "password" : "text"}
-            onChange={(value, event) => props.onChange?.(value)}
+            onChange={(_event, value: string) => onChange(value)}
+            // onChange={(value, event) => props.onChange?.(value)}


Do we need this comment?

manstis · 2024-11-22T12:12:56Z

ansible_ai_connect_admin_portal/src/__tests__/AppHeader.test.tsx

    expect(accountMenu).toBeInTheDocument();
    expect(accountMenu).toHaveTextContent("Batman");

    // Check "Logout" option is not present
    expect(screen.queryByText("Logout")).toBeNull();
-


You seem to have remove the test for logging out?

Signed-off-by: Sumit Jaiswal <[email protected]>

sonarqubecloud · 2024-11-27T14:03:19Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

manstis

The icon/text alignment in the left-hand-side menu still is not correct; but the kebab menu works now 😸

Approving; but I suggest a follow-up JIRA to fix the alignment be made.

justjais · 2024-11-28T06:43:15Z

@manstis thanks for the review, and giving green light to the PR. Also, I've opened a bug jira: https://issues.redhat.com/browse/AAP-36777, to address the alignment issue.

justjais requested a review from jameswnl October 17, 2024 14:32

This was referenced Oct 23, 2024

PR to update node version to 21.x #1360

Closed

PR to update node version to 21.x from 20.x #1361

Closed

PR to upgrade node from 20.x to 21.x #1363

Closed

PR to upgrade node from 20.x to 21.x #1366

Closed

PR to upgrade node from 20.x to 21.x #1367

Merged

justjais force-pushed the aap_31381 branch from 0a8ee74 to a2283b6 Compare October 28, 2024 19:18

justjais force-pushed the aap_31381 branch 2 times, most recently from 66e3ac1 to 52ad423 Compare November 11, 2024 18:34

justjais force-pushed the aap_31381 branch from 4e8fe86 to e9be151 Compare November 11, 2024 19:56

justjais force-pushed the aap_31381 branch from e9be151 to 5115e32 Compare November 19, 2024 05:38

justjais force-pushed the aap_31381 branch from 5115e32 to 1fc5f5e Compare November 22, 2024 10:36

justjais requested review from manstis and romartin November 22, 2024 12:07

manstis requested changes Nov 22, 2024

View reviewed changes

justjais added 5 commits November 27, 2024 18:57

fix aap 31381

073e1bb

Signed-off-by: Sumit Jaiswal <[email protected]>

update package-lock.json

c27abdd

Signed-off-by: Sumit Jaiswal <[email protected]>

fix package via npm update

e4eed50

update patternfly and ansible-ui-framework

eab74b0

fix patternfly fallouts

b14a544

justjais added 8 commits November 27, 2024 18:57

update ui-framework to latest

15a515c

fix haapy-dom critical vulnerability

2847af9

update package-lock.json

1d843e4

update the PR with suggested changes

66be3a6

update aap_31381

69cda5f

fix test

d156f08

remove part tests

374d4c2

fix kebab menu

85b3249

justjais force-pushed the aap_31381 branch from 3ecc01e to 85b3249 Compare November 27, 2024 13:54

manstis approved these changes Nov 28, 2024

View reviewed changes

justjais merged commit e129407 into main Nov 28, 2024
10 checks passed

justjais deleted the aap_31381 branch November 28, 2024 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR to fix ansible-ai-connect-service: Cross-site Scripting (XSS) in serialize-javascript #1353

PR to fix ansible-ai-connect-service: Cross-site Scripting (XSS) in serialize-javascript #1353

justjais commented Oct 17, 2024 •

edited

Loading

jameswnl commented Oct 17, 2024

justjais commented Oct 22, 2024

manstis commented Oct 29, 2024

justjais commented Oct 29, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 19, 2024

manstis left a comment

manstis Nov 22, 2024

manstis Nov 22, 2024

sonarqubecloud bot commented Nov 27, 2024

manstis left a comment

justjais commented Nov 28, 2024

PR to fix ansible-ai-connect-service: Cross-site Scripting (XSS) in serialize-javascript #1353

PR to fix ansible-ai-connect-service: Cross-site Scripting (XSS) in serialize-javascript #1353

Conversation

justjais commented Oct 17, 2024 • edited Loading

Description

Testing

Steps to test

Scenarios tested

Production deployment

jameswnl commented Oct 17, 2024

justjais commented Oct 22, 2024

manstis commented Oct 29, 2024

justjais commented Oct 29, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 11, 2024

github-actions bot commented Nov 19, 2024

manstis left a comment

Choose a reason for hiding this comment

manstis Nov 22, 2024

Choose a reason for hiding this comment

manstis Nov 22, 2024

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 27, 2024

Quality Gate passed

manstis left a comment

Choose a reason for hiding this comment

justjais commented Nov 28, 2024

justjais commented Oct 17, 2024 •

edited

Loading