Pyinfra: Obfuscate Secrets

[meantheory]

Is your feature request related to a problem? Please describe.

Like to obfuscate secrets for commands that use them. For example, in the operation postgresql.role({"create role"}, "deploy", present=True, password=DATABASE_PASSWORD, postgresql_user="postgres")

This will output the value of DATABASE_PASSWORD which is a leak of information to the console or if pyinfra is in a CI system it will come out in the logs.

Describe the solution you'd like

I'd like to see the secret obfuscated to something like three stars ***. Or anything really, length of obfuscation shouldn't match actual string length. When obfuscation is needed should be known so the api shouldn't need to change (I think).

[meantheory]

looking over things, the entire command is assembled as a string. makes sense but I can see why what I'm asking for might not be as easy as I initially imagined.

[meantheory]

closing this, the example I copied while trying to learn this invoked -v, removing that, removes the secret leakage problem I was concerned about.

[Fizzadar]

@meantheory I'm keen to implement this, even with -v it should not include secret information!

[Fizzadar]

This is now implemented and released in 0.15.dev0, full 0.15 release due in a few days time. Implementation (for now): https://github.com/Fizzadar/pyinfra/commit/2bc9ab5a9dd87ae9c352a85b77e6235958714f6f

[meantheory]

awesome. thank you.

[meantheory]

in forking the repository to play around with code, here is a minimal viable hack as an alternative method to the regex. It is completely unusable as is but your test suite does pass, just mean to demonstrate an idea. Intended use would be to overload repr as a way printing the masked string output.

Not sure this is a good idea, just an idea.

https://github.com/meantheory/pyinfra/commit/b68bd2bb29ab6ed3df1c5bc99d952831409a9707

I will find time to expand it to a more complete implementation and a pull request if there's any interest.

[meantheory]

I can't tell if a thumbs up means "yes, make a pull request" or "that's a horrible hack and I hope a thumbs up is vague enough to discourage any more attempts". If the latter, I'm sorry but I couldn't help myself. I cleaned it up a bit and added a few simple tests. https://github.com/meantheory/pyinfra/commit/dc35232825ecb918d33b5d55582bed7cd301e2c2 -- to be clear, I wouldn't submit this particular thing as a pull request as there are many edge cases still left unconsidered. Just cleaning up the initial minimum viable hack a bit.

[elistar2000]

hello i student master it
good..
eitland.ir
حتما سر بزنید.

[Fizzadar]

@meantheory apologies the ambiguous thumbs up is a "yes make a pull request"!

I like the approach only change I would make is using repr - would rather see a specific method/attribute here similar to gettattr(command, 'masked', command).

[meantheory]

I made an implementation that uses getattr that you can see here. https://github.com/Fizzadar/pyinfra/compare/master...meantheory:commands-319

I had been relying on your regex implementation as a guide and the excellent test suite to test this. Ultimately though, neither of our solutions solves my original problem when using -v.

Turns out the password leak I was concerned comes from printing output from this function.

I started to write a way to filter output and replace it with *** but that quickly turned into me needing to understand much more about state management throughout entire code base which is a hill I did not have time for today.

[Fizzadar]

Hmmm - I think the issue is just a missing masking @ https://github.com/Fizzadar/pyinfra/blob/master/pyinfra/operations/postgresql.py#L129?

I've made this into a PR (hope that's OK with you) to track this. If you could give me push permissions to your branch I can fixup the postgresql operation above and other bits?

[meantheory]

ah. interesting. permission granted.
pull request #339 - failing on flake8 :facepalm:.

[Fizzadar]

@meantheory thank you for permissions! PR updated - turns out this is a bit of a rabbit hole! PR is now passing and properly masking for SSH + @local targets only (docker/chroot WIP).

Going to merge the PR as this is working, and create subsequent issues for follow up work. Aiming to release this in v1.

[Fizzadar]

Scrap that - https://github.com/Fizzadar/pyinfra/commit/9d69ec77e7de21fac9702645a3d0b71488bdfab0 finishes this off!

[meantheory]

@Fizzadar Looking at all the changes you made, it was indeed a bit of a rabbit hole. I came here just now to start a more serious refactor of initial hack. Glad to see I don't need to. Thanks for the changes!