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

problem: reports contain environment variables #332

Closed
boneskull opened this issue Oct 8, 2019 · 4 comments
Closed

problem: reports contain environment variables #332

boneskull opened this issue Oct 8, 2019 · 4 comments
Labels

Comments

@boneskull
Copy link

This is not inherently a problem, but it's still a problem for a set of use-cases. We don't want to leak the environment, and the existence of freshly-created diagnostic report file poses a risk. How much of a risk? I'm not sure, so I'm hoping you all can help answer that question. Is it risky enough to disallow --experimental-report from within NODE_OPTIONS? How do node-report users handle this?

But say you are concerned about this, and need the reports otherwise, at present time you don't have a whole lot of options. You can manually do this or use some sort of script to do it after the file is created.

  • You could listen for various (unfortunately; see A better way to detect a process is exiting tooling#42) Process events, and in the listener run process.report.getReport() manually, going from there. But this won't trigger on a fatal error, so you've still got a dump of the environment on disk.

    If we can ensure that reporting on a fatal is opt-in only (see diagnostic report behavior on fatal error #331 & linked issues there), then this event-listening strategy might be more palatable, but would still mean you're out-of-luck if you want reports on fatal errors.

  • If you stick with dumping files to disk, you could use a filesystem-watcher (ugh) that would scoop up the file and immediately sanitize it (and/or delete it) and shuffle its data off somewhere else. Better than nothing, I suppose.

Assuming this is a Real Problem that needs solving, one way would be for core to allow a user-defined blacklist of environment variable names. A callback that allows a developer to modify the report before it gets to disk could also be useful, but it still won't be usable with fatal errors...

If I'm making a mountain out of a molehill, please tell me so. 😄

@richardlau
Copy link
Member

Assuming this is a Real Problem that needs solving, one way would be for core to allow a user-defined blacklist of environment variable names. A callback that allows a developer to modify the report before it gets to disk could also be useful, but it still won't be usable with fatal errors...

If I'm making a mountain out of a molehill, please tell me so. 😄

As a datapoint there is a longstanding feature request against the standalone module to allow filtering of environment variables: nodejs/node-report#114

@boneskull
Copy link
Author

OK, so I'm not imagining things, at least.

@gireeshpunathil
Copy link
Member

One can argue that the env data that the report captures is nothing more than the data that is present in the running user's execution environment (that gets printed on an env command), so the exposure essentially is equal to the delta between access to the file vs. access to the user's shell?

On the other hand, if such an option (env data mask) is implemented, the notion of associating security concerns with these variables will drive almost all users to use that, reducing the usefulness of report as a first-failure-data-capture tool at large.

A more pragmatic approach is:

  • limit the report file access permission to the user that initiated node process
  • document about the presence of potentially sensitive data in the report, and endorsing a best practice to redact such data before processing / sending for support

@github-actions
Copy link

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

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

No branches or pull requests

3 participants