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

Config path fixes #55

Closed
wants to merge 2 commits into from
Closed

Config path fixes #55

wants to merge 2 commits into from

Conversation

kzvezdarov
Copy link
Contributor

This PR implements some of the feedback on how config files and paths are handled:

  1. pwd is included in the search paths by default.
  2. The README doc shows that ReadInConfig returns an error that can be handled.

Does not implement the suggestion that filepath extensions become optional - this needs some consideration.

@kzvezdarov
Copy link
Contributor Author

Merged

@kzvezdarov kzvezdarov closed this Apr 3, 2015
@didenko
Copy link
Collaborator

didenko commented May 22, 2015

Hm... Defaulting to read config from the working directory may make exploits easier in general case. It becomes possible for an intruder to take advantage of other misconfigurations and cd to a directory with a malicious config before running the program which uses viper, thus potentially altering its behaviour.

IMHO including the current dir or not in the search path is a simple choice for the package user (3-4 lines like in your commit), yet has negative implications for those who want their systems hardened. As a library, it is detrimental to have this hardcoded feature.

Besides, given the diversity of environments where software may be deployed these days, this hard- coded "feature"'s unintended consequences may either lead to an unexpected failures when config names collide between utilities, be a logger noise, or, again, be a security threat. Working directory is not always obvious - nor every utility writer should make assumptions about it. What is a workdir on Google App engine off the top of one's head?

@kzvezdarov
Copy link
Contributor Author

If the malicious intruder has obtained enough permissions to execute a program that uses viper, they don't even need to change to a directory containing a malicious configuration - they can just pass in everything as command line arguments.

This issue is probably the better place for this discussion: #69

@didenko didenko mentioned this pull request May 22, 2015
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.

2 participants