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

Migrate away from graphql-client and is-my-json-valid #3495

Closed
jesec opened this issue Nov 10, 2021 · 5 comments
Closed

Migrate away from graphql-client and is-my-json-valid #3495

jesec opened this issue Nov 10, 2021 · 5 comments

Comments

@jesec
Copy link

jesec commented Nov 10, 2021

Case

Something else

Issue

npm emits:

expand
# npm audit report

jsonpointer  <5.0.0
Severity: moderate
Prototype Pollution in node-jsonpointer - https://github.com/advisories/GHSA-282f-qqgm-c34q
fix available via `npm audit fix --force`
Will install rxdb@3.0.2, which is a breaking change
node_modules/jsonpointer
is-my-json-valid  >=2.0.0
Depends on vulnerable versions of jsonpointer
node_modules/is-my-json-valid
  rxdb  >=3.0.3
  Depends on vulnerable versions of graphql-client
  Depends on vulnerable versions of is-my-json-valid
  Depends on vulnerable versions of pouchdb-adapter-http
  Depends on vulnerable versions of pouchdb-core
  Depends on vulnerable versions of pouchdb-find
  node_modules/rxdb

node-fetch  <2.6.1
The `size` option isn't honored after following a redirect in node-fetch - https://github.com/advisories/GHSA-w7rc-rwvf-8q5r
fix available via `npm audit fix --force`
Will install rxdb@3.0.2, which is a breaking change
node_modules/node-fetch
node_modules/pouchdb-fetch/node_modules/node-fetch
isomorphic-fetch  2.0.0 - 2.2.1
Depends on vulnerable versions of node-fetch
node_modules/isomorphic-fetch
  graphql-client  >=1.0.2
  Depends on vulnerable versions of isomorphic-fetch
  node_modules/graphql-client
    rxdb  >=3.0.3
    Depends on vulnerable versions of graphql-client
    Depends on vulnerable versions of is-my-json-valid
    Depends on vulnerable versions of pouchdb-adapter-http
    Depends on vulnerable versions of pouchdb-core
    Depends on vulnerable versions of pouchdb-find
    node_modules/rxdb
pouchdb-fetch  >=7.1.0
Depends on vulnerable versions of node-fetch
node_modules/pouchdb-fetch
  pouchdb-abstract-mapreduce  >=7.1.0
  Depends on vulnerable versions of pouchdb-fetch
  node_modules/pouchdb-abstract-mapreduce
  pouchdb-adapter-http  >=7.1.0
  Depends on vulnerable versions of pouchdb-fetch
  node_modules/pouchdb-adapter-http
  pouchdb-core  >=7.1.0
  Depends on vulnerable versions of pouchdb-fetch
  node_modules/pouchdb-core
  pouchdb-find  >=7.1.0
  Depends on vulnerable versions of pouchdb-fetch
  node_modules/pouchdb-find
11 vulnerabilities (8 low, 3 moderate)

rxdb is fantastic, but security warnings are deal breakers. I understand that some security vulnerabilities might be less relevant, but when it comes to security, it is better to stay on the safe side.

graphql-client is unmaintained. It is very small (single file) and the license (ISC) is compatible, so the easiest way to deal with the issue would be to vendor it (i.e. copy it to this repo, and make it use a newer version of isomorphic-fetch or fetch-ponyfill). Alternatively, migrate to a different library.

For is-my-json-valid, a PR is open (mafintosh/is-my-json-valid#188), but the project doesn't seem to be active. https://github.com/ajv-validator/ajv is a much healthier and more performant alternative.

Info

  • RxDB Version: 10.3.5
  • Environment: (Node.js/browser/electron/etc..) v16.12.0
  • Adapter: (IndexedDB/Localstorage/LevelDB/etc..) N/A
  • Stack: (Typescript, Babel, Angular, React, etc..) N/A

Code

$ npm install rxdb
@LinusU
Copy link

LinusU commented Nov 10, 2021

For is-my-json-valid, a PR is open (mafintosh/is-my-json-valid#188), but the project doesn't seem to be active. https://github.com/ajv-validator/ajv is a much healthier and more performant alternative.

The PR is merged and released now, took me ~2 days since I have a lot of other notifications to go thru, sorry about that...

edit: not saying you can't change of course, AJV is also a great project 👍

@pubkey
Copy link
Owner

pubkey commented Nov 10, 2021

  1. Yes we can replace graphql-client. Maybe we should switch to another lib that has at least some unit tests or can be used to cancel running requests when the replication is closed. You can make a PR if you want.

  2. We already have different validation plugins, also one with Ajv. I am open to change the default but Ajv has also its problems.

@jesec
Copy link
Author

jesec commented Nov 10, 2021

See #3497

  1. Yes we can replace graphql-client. Maybe we should switch to another lib that has at least some unit tests or can be used to cancel running requests when the replication is closed. You can make a PR if you want.

I choose to vendor "graphql-client" for now. That's the least disruptive way, and the library is small enough. We can explore other libraries in the future. As for canceling running requests, we would have to use AbortController (https://caniuse.com/abortcontroller), which should be trivial now we have "graphql-client" maintained in this repo.

  1. We already have different validation plugins, also one with Ajv. I am open to change the default but Ajv has also its problems.

Bumped is-my-json-valid to 2.20.6. Thank you @LinusU . That solves the issue for now. I will leave other project decisions to you.

@jazpearson
Copy link

Will you be releasing these changes any time soon? There's a critical security vulnerability being flagged up by the latest version (10.5.3) because of an issue with jsonpointer which is consumed by is-my-json-valid.

https://nvd.nist.gov/vuln/detail/CVE-2021-23807

@pubkey
Copy link
Owner

pubkey commented Nov 30, 2021

I will release now. Should be done in 30min.

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

No branches or pull requests

4 participants