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

FINOS Security Scanning #808

Merged
merged 31 commits into from
Apr 26, 2023
Merged

FINOS Security Scanning #808

merged 31 commits into from
Apr 26, 2023

Conversation

robmoffat
Copy link
Member

Hi,

Here's a first-pass at setting up the new FINOS security scanning Github actions.

We need to review the results of running these with a view to either fixing the issues or setting up exclusions:

https://github.com/robmoffat/FDC3/actions

@netlify
Copy link

netlify bot commented Aug 30, 2022

Deploy Preview for fdc3 canceled.

Name Link
🔨 Latest commit 5d57491
🔍 Latest deploy log https://app.netlify.com/sites/fdc3/deploys/6448d79a3f60c90008b13d56

@robmoffat
Copy link
Member Author

robmoffat commented Sep 6, 2022

If you look at the above CVE Scan we run into a problem with vulnerabilities.

Trying to use npm audit fix --force will update

"react-scripts": "4.0.3",
+    "react-scripts": "^5.0.1",

However, this then causes a new problem:

WARNING in ./node_modules/@jsdevtools/ono/esm/constructor.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/constructor.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/constructor.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/extend-error.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/extend-error.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/extend-error.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/index.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/index.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/index.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/isomorphic.browser.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/isomorphic.browser.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/isomorphic.browser.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/normalize.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/normalize.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/normalize.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/singleton.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/singleton.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/singleton.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/stack.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/stack.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/stack.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/to-json.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/to-json.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/to-json.ts'

WARNING in ./node_modules/@jsdevtools/ono/esm/types.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/types.ts' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/src/types.ts'

WARNING in ./node_modules/jsoneditor/dist/jsoneditor.min.js
Module Warning (from ./node_modules/source-map-loader/dist/cjs.js):
Failed to parse source map from '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/jsoneditor/dist/0' file: Error: ENOENT: no such file or directory, open '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/jsoneditor/dist/0'

ERROR in ./node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers/http.js 3:13-28
Module not found: Error: Can't resolve 'http' in '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers'
Did you mean './http'?
Requests that should resolve in the current directory need to start with './'.
Requests that start with a name are treated as module requests and resolve within module directories (node_modules, /Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules).
If changing the source code is not an option there is also a resolve options called 'preferRelative' which tries to resolve these kind of requests in the current directory too.

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "http": require.resolve("stream-http") }'
	- install 'stream-http'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "http": false }

ERROR in ./node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers/http.js 5:14-30
Module not found: Error: Can't resolve 'https' in '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@apidevtools/json-schema-ref-parser/lib/resolvers'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "https": require.resolve("https-browserify") }'
	- install 'https-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "https": false }

ERROR in ./node_modules/@apidevtools/json-schema-ref-parser/lib/util/url.js 13:0-36
Module not found: Error: Can't resolve 'url' in '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@apidevtools/json-schema-ref-parser/lib/util'
Did you mean './url'?
Requests that should resolve in the current directory need to start with './'.
Requests that start with a name are treated as module requests and resolve within module directories (node_modules, /Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules).
If changing the source code is not an option there is also a resolve options called 'preferRelative' which tries to resolve these kind of requests in the current directory too.

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "url": require.resolve("url/") }'
	- install 'url'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "url": false }

ERROR in ./node_modules/@jsdevtools/ono/esm/types.js 1:0-31
Module not found: Error: Can't resolve 'util' in '/Users/rob/Documents/finos/fdc3-security/toolbox/fdc3-workbench/node_modules/@jsdevtools/ono/esm'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "util": require.resolve("util/") }'
	- install 'util'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "util": false }

@kriswest
Copy link
Contributor

@robmoffat we've tested up a solution from the workbench audit issues (all are from its use of create-react-app). I'll find some time to raise a PR with the fixes this week.

@robmoffat
Copy link
Member Author

robmoffat commented Sep 12, 2022 via email

@kriswest
Copy link
Contributor

awesome, well done Kris!

Credit to @julianna-ciq whose NPM/React/Webpack-fu is stronger than mine ;-)

@robmoffat
Copy link
Member Author

robmoffat commented Sep 16, 2022

Following your merge of #812, I've sync'd this PR, and I'm still seeing some CVEs:

https://github.com/finos/FDC3/actions/runs/3067573442/jobs/4953998906

This can be reproduced locally by running:

npx --yes auditjs ossi

which I guess must be slightly different / more strict than npm audit 😢

You can always add these to the whitelist if they aren't significant

@julianna-ciq

@kriswest
Copy link
Contributor

For the last CVE, do we care about the development server from create-react-app? Definitely not a production dependency. The other CVE is marked 'high' but has questionable info associated with it:

  Vulnerability Title:  [CVE-2021-3868] ** REJECT ** DO NOT USE THIS CANDIDATE NUMBER. ConsultIDs: none. Reason: The CNA or individual who requested this candidate did not associate it with any vulnerability during 2021. Notes: none.
  ID:  CVE-2021-3868
  Description:  ** REJECT ** DO NOT USE THIS CANDIDATE NUMBER. ConsultIDs: none. Reason: The CNA or individual who requested this candidate did not associate it with any vulnerability during 2021. Notes: none.
  CVSS Score:  7.5
  CVSS Vector:  CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
  CVE:  CVE-2021-3868
  Reference:  https://ossindex.sonatype.org/vulnerability/CVE-2021-3868?component-type=npm&component-name=prompts&utm_source=auditjs&utm_medium=integration&utm_content=4.0.37

Finally, whats the policy re: non-CVE vulnerabilities (as there are several of those) @robmoffat ?

@robmoffat
Copy link
Member Author

Finally, whats the policy re: non-CVE vulnerabilities (as there are several of those) @robmoffat ?

@maoo do you have an opinion on this?

I would say we can just put them in the whitelist if we don't think any remediation is necessary.

@maoo
Copy link
Member

maoo commented Sep 16, 2022

Finally, whats the policy re: non-CVE vulnerabilities (as there are several of those) @robmoffat ?

@maoo do you have an opinion on this?

I would say we can just put them in the whitelist if we don't think any remediation is necessary.

Agreed. I see that there are non-CVEs with very high score (ie, higher than 9); following the links reported for each of them, I've seen that often there are fixes available, so I'd just follow recommendations.

@kriswest kriswest mentioned this pull request Oct 28, 2022
32 tasks
@robmoffat
Copy link
Member Author

...also included a fix from @openfin-johans to get navigation on fdc3-explained working in Sail.

@robmoffat
Copy link
Member Author

robmoffat commented Feb 10, 2023

ok, so given the CVE issues in workbench are resolved by #816 and this resolves the semgrep problems, does that mean we can merge this @kriswest ?

Requires #894 ?

@kriswest
Copy link
Contributor

ok, so given the CVE issues in workbench are resolved by #816 and this resolves the semgrep problems, does that mean we can merge this @kriswest ?

#816 will be resolved by #894 which @mattjamieson is working on - but having some troubles with Netlify not deploying the CSS.

Presumably you also need to merge the docusaurs v2 PR #910 as well, right? This should go immediately after those two I believe.

While merging this prevent merging other PRs that generate failures? If so we'll need a preview run to make sure we're clean before it goes in...

@robmoffat
Copy link
Member Author

I think it's only the reviews that prevent merging

@kriswest
Copy link
Contributor

kriswest commented Apr 21, 2023

@robmoffat the repository should be in a much better place after all the maintenance PRs getting merged today. Could you update this branch and have another go at running the action?

Note:

  • ✔️ DONE this final maintenance PR should get merged first as affects the dependency resolution due to the node version update: Updating project to node 18 #965
  • Added a suggested change to the workflow that have it run on node 18, please commit that first

@robmoffat
Copy link
Member Author

@kriswest cve/semgrep checks all passing. please review

@kriswest
Copy link
Contributor

@robmoffat Great, however I don't think you committed the change to the CVE scan to run on node 18 (which affects module resolution):
image

Could you commit that (#808 (comment)) then it should re-run and we should be good to go (if still clear)

Co-authored-by: Kris West <kris@finsemble.com>
@kriswest
Copy link
Contributor

Preview link: https://deploy-preview-808--fdc3.netlify.app/

@kriswest kriswest self-requested a review April 26, 2023 10:33
Copy link
Contributor

@kriswest kriswest left a comment

Choose a reason for hiding this comment

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

LGTM!

@kriswest kriswest linked an issue Apr 26, 2023 that may be closed by this pull request
@kriswest kriswest merged commit 2b9c725 into finos:master Apr 26, 2023
@robmoffat
Copy link
Member Author

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add FINOS Security Scanning (Javascript)
4 participants