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

Remove Insight & Disable telemetry #632

Closed
wants to merge 1 commit into from

Conversation

breautek
Copy link
Contributor

Platforms affected

CLI

Motivation and Context

closes #625

Insight is no longer maintained and it contains sub-dependencies with moderate vulnerabilities.

Description

All code relating to telemetry and using Insight have been commented out.
Fake implementations were left in place so our Telemetry API still exists, just it will
always "opt out" of telemetry.

This is done so that we can easily bring back telemetry later once we find a replacement for Insight.

Tests related to telemetry were disabled.

This PR is intended to be a stopgap just to resolve the present audit issues.

Testing

Ran npm test

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

has been disabled.

Insight is no longer maintained and it contains sub-dependencies
with moderate vulnerabilities.

The relevant code is commented out so that telemetry can be easily brought
back in the future, once a replacement for Insight is found and/or built.
@breautek breautek requested a review from erisu December 14, 2023 16:10
@shajz shajz mentioned this pull request Dec 14, 2023
5 tasks
@mschoettle
Copy link

It would be nice if this could be merged and released. I tried to manually remove insight but the require('insights') then fails when calling CLI commands.

@SteveW94
Copy link

We also have now the task to address our security issues, where this shows up as a very critical one.
So it would be very nice if this could find its way to a release very soon!

@dpogue
Copy link
Member

dpogue commented Jun 17, 2024

The work in #633 is slightly more complete as far as also removing the documentation for the telemetry stuff, so I'm in favour of trying to that one merged.

@dpogue dpogue closed this in 71a5c89 Jun 17, 2024
@dpogue dpogue added this to the 13.0.0 milestone Jun 18, 2024
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

Successfully merging this pull request may close these issues.

Insight dependency with open CVE
4 participants