ricoberger/script_exporter

Logger dumps sensitive env vars into logs

diversario opened this issue · 5 comments

Hello @ricoberger !

Thank you for maintaining this great tool!

I recently discovered that the script executor dumps all environment variables into logs on errors:

"env", strings.Join(cmd.Env, " "),

which can be very helpful for debugging, but it can also dump sensitive values such as passwords/access tokens/etc, for example if a script is something like

curl -s -u $GITHUB_USERNAME:$GITHUB_TOKEN $somegithublink

and thanks to log shipping that immediately ends up exposed to anyone with access to logs.

It would be great to (conditionally?) avoid dumping env vars to logs.

Hi @diversario, thanks for raising this issue.

I think the best option would be to only log the environment variables when the log level is set to debug and exclude them from the error log. What do you think?

That would be better, but that would still make it unfeasible to use debug level as it would leak secrets (and, it would make sense that one would want to run the exporter with real credentials + debug logs to figure out what's going on).

Not sure what the right fix here would be. Maybe... a blocklist of string to not log, or log but with the value redacted?.. Even a toggle to just not log env vars at all, regardless of the level, would be better, tho not as flexible.

lufik commented

@diversario it makes sense to me log everything in debug mode (sometimes you need to check even secrets). It's admin/user responsibility to log this debug mode to e.g. stdout and not to log shipping part.

Hi, this should be fixed by #93 were I added a -log.env flag which must be set to true to log the environment variables.

@diversario it makes sense to me log everything in debug mode (sometimes you need to check even secrets). It's admin/user responsibility to log this debug mode to e.g. stdout and not to log shipping part.

Log shipping picks up both stdout and stderr since otherwise it would've been incomplete.

For inspecting env of a process a workaround is to inspect its /proc/X/environ in the container.