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

mask the credentials in the build output #9

Open
travi opened this issue Mar 10, 2016 · 6 comments
Open

mask the credentials in the build output #9

travi opened this issue Mar 10, 2016 · 6 comments

Comments

@travi
Copy link

travi commented Mar 10, 2016

no matter how carefully the credentials are passed to the task, they are printed in the build output, where anyone with access to the build can see them. flyway itself is careful to prevent this.

Could something be done to mask at least the credentials in the output?

@zero5100
Copy link
Collaborator

Good idea, I'll look into that when I find some time. You can feel free to submit a patch as well.

@travi
Copy link
Author

travi commented Mar 14, 2016

The PR that I just opened is pretty aggressive, because it ends up removing most of the output. I'm not sure that it is a huge problem because the if check was too specific for the error that was output from the current version anyway.

If you think there is some value to keeping some of the output behind some sort of debug flag, I would suggest at least hiding it by default. I know I made the incorrect assumption that credentials would be hidden by default and only realized it when I was going through the log to understand a different issue, meaning I will need to reset my database url and credentials to keep things secure.

Thanks for being open to getting this handled.

@travi
Copy link
Author

travi commented Mar 17, 2016

@zero5100 did you happen to have thoughts about this PR? I would love to see this pulled in and have a release cut that includes the jars since this is currently blocking me from moving forward with one of my projects. I would normally reference my own branch, but since master does not include the jars, I can't do that easily. I assume the addition of the jars must be part of publishing to npm?

@zero5100
Copy link
Collaborator

@travi I have not taken the time to thoroughly look into the problem yet, but it looks like that PR would not be able to be merged right now as written. It should be pointed at the upstream develop branch instead of master, and we would probably want to add a verbose/debug flag that enables the extra output. I think ideally it would show the normal debug output that it does right now except we would mask the password as ***** or something. It would probably be safe to leave the username in the output, but that could be moved the verbose output as well if you wanted to be extra safe.

I would encourage you to create and use your own fork for now if it is blocking you; I can't guarantee when any new changes would make it into master. Once #7 is merged into develop and then master the jars will be included.

@travi
Copy link
Author

travi commented Mar 17, 2016

Thank you for the additional information. I agree that truly masking the sensitive information would be ideal, but since the output is currently directly dumped, it would take a bit of fairly specific processing to do that type of masking. I do think that it is important to mask the url, user, and password all need to be masked. Some of these may be less sensitive on an internal build system, but any of these details can be a big deal for a fully public build on Travis or similar.

It sounds like some pieces need to come together for this to be included in a release, but it also sounds like you are on top of it. For now, it probably makes sense for me to close the current PR and let you take the first steps. I'm happy to help out as things move closer, so don't hesitate to ping me at the appropriate time.

@zero5100
Copy link
Collaborator

Sounds good, thanks.

travi added a commit to travi-org/api that referenced this issue Mar 17, 2016
…ormation is not exposed in the build output for #14

until gomoob/grunt-flyway#9 is resolved
zero5100 added a commit to zero5100/grunt-flyway that referenced this issue Mar 17, 2017
zero5100 added a commit to rglholdings/grunt-flyway that referenced this issue Mar 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants