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

Don't expose API Keys on stdout #4

Closed
sleeping-barber opened this issue May 13, 2020 · 4 comments · Fixed by #6 or #7
Closed

Don't expose API Keys on stdout #4

sleeping-barber opened this issue May 13, 2020 · 4 comments · Fixed by #6 or #7

Comments

@sleeping-barber
Copy link
Contributor

Hi,
I just saw the following line and it made me concern about security:

if owmAPIKey == "" || githubAPIToken == "" {

Is it really a good idea to expose these values, a simple message that one of the needed keys wasn't provided or better mention the specific missing key would be IMHO a nice approach to the user.

What was the reason to expose this values?

Cheers!

@narqo
Copy link
Owner

narqo commented May 13, 2020

I think with the current implementation logging isn't the only concern. The app accepts the keys via CLI flags. This is enough to leak the keys as they will be seen in ps output on the host.

A better approach would be to read the keys from a secrets file.

@sleeping-barber
Copy link
Contributor Author

True. Looking at the manifest file, I already see that Secret resources are setup, maybe even a simple old-school os.GetEnv would be a first step.

Just out of interest, what of benefits do secret files give us?

Cheers!

@narqo
Copy link
Owner

narqo commented May 13, 2020

Process's environment variables can be observed from the outside either by looking at /proc/<pid>/environ or with ps ae.

Reading the secrets from a secrets file allows restricting the access to the file with OS level permissions.

@narqo
Copy link
Owner

narqo commented May 20, 2020

As commented in #6 this isn't 100% fixed yet. I'll address it later.

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