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

Use Environment variables for sensitive information #30

Closed
KentonWhite opened this issue Jan 22, 2014 · 9 comments
Closed

Use Environment variables for sensitive information #30

KentonWhite opened this issue Jan 22, 2014 · 9 comments
Assignees
Labels
Milestone

Comments

@KentonWhite
Copy link
Owner

Some of the readers, like the SQL readers, require password information. Currently this is stored directly in the data file. Storing passwords in files is a security risk -- they can be checked into repositories or made accessible to the public.

Other communities use environment variables to store sensitive information. Can you do this in ProjectTemplate?

@ghost ghost assigned KentonWhite Jan 22, 2014
@krlmlr
Copy link
Collaborator

krlmlr commented Feb 5, 2014

Environment variables are also insecure, although I agree that the chance of exposure is smaller. Other options would be encrypting the password with GPG (is there an R interface?) or using passwordless access. For instance, Postgres allows setting up passwordless access through a .pgpass file (and through other options), SQL Server can use the current login for authentication, etc..

As a first measure, I think we should point out in big fat letters that using the "password" entry is a huge risk, and make sure that passwordless access works for all readers and all database types if configured properly. I think Travis is set up for passwordless access to a few database servers, but for many others we'll need to set up a server for testing. What kinds of database servers do you have access to? (I'm feeling that this is going to be a lot of work; we might want to wait for users to complain that passwordless access is not working instead.)

On the other hand, we might as well start with supporting a new entry password_env in the DCF files. For this, all instances of database.info[['password']] should be replaced by a call get.password(database.info).

@krlmlr
Copy link
Collaborator

krlmlr commented Feb 10, 2014

Passwordless didn't work with PostgreSQL. sigh... I have opened a branch (30-...) to handle these issues.

@KentonWhite
Copy link
Owner Author

Looking at the graph, has 30-passwordless been merged completely into develop? Should this work be done in develop now or still in 30-passwordless?

@krlmlr
Copy link
Collaborator

krlmlr commented Feb 10, 2014

I'm somewhat following git-flow, but currently without any extra tools. This means that 30-passwordless is the feature branch, all work will be done there, and stable-ish results can be merged into develop. When work is done, it's merged to master.

@KentonWhite
Copy link
Owner Author

We can follow git-flow. I had set up 0.6.0 as a development branch for the next release. Would you like me to merge 0.6.0 into develop (it includes the .add.extension work) and then we can follow the git-flow approach? Also, let's clean up the branches that have already been merged, like warnings-are-errors (I think that was already been merged??)

@krlmlr
Copy link
Collaborator

krlmlr commented Feb 10, 2014

We can use two "development" branches for now, one for 0.6.0. The individual features still should be in feature branches, I think. The master branch can be merged regularly into 0.6.0, but not vice versa. Make sure you have rerere enabled for your repo by saying

git rerere

at least once. I also have

[rerere]
        enabled = true
[merge]
        conflictstyle = diff3

in my ~/.gitconfig.

As for the branches, we can delete the feature branches as soon as they are merged using the GitHub front end, given how easy it is to restore them. The warnings-are-errors branch won't pass tests without #39, I'm afraid.

@krlmlr krlmlr added this to the 0.6 milestone Mar 13, 2014
@krlmlr krlmlr added the Feature label Mar 13, 2014
@krlmlr
Copy link
Collaborator

krlmlr commented Sep 22, 2014

@KentonWhite: I'd like to merge relevant parts of the 0.6.0-new branch into master. We need to push an update to CRAN soon, due to deprecation of the Defaults package (#100). Could you please convert it to meaningful pull requests?

@krlmlr
Copy link
Collaborator

krlmlr commented Sep 23, 2014

Back to the issue: Storing the password in an environment variable (specified by the .sql config file) is probably a good compromise. Many CI tools support encrypted environment variables, too.

@krlmlr krlmlr modified the milestones: 0.7, 0.6 Sep 23, 2014
@kezkankrayon
Copy link

In the .sql file, it'd be great to be able to do the following:

user: Sys.getenv("uid")
password: Sys.getenv("pwd")

Unfortunately, these are read as literal strings.

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