neon
neon copied to clipboard
neondatabase
test_runner: pass password to pgbench via PGPASSWORD
These PR changes approach taken in https://github.com/neondatabase/neon/pull/2434. Now we process all files that are used for report creating.
Do we have an idea of places where passwords still occur?
In exception messages.
Having to scan all the files in the report looks a bit overkill for the task to me.
That was exactly my thought, but unfortunately, there's a possibility that someone will use subprocess.run (instead of our wrapper), and it will expose passwords (even before we had a chance to review the PR and suggest not to use it). Another case is that a program we're running with subprocess_capture will write a password to stdout/stderr, which we capture and attach to the report.
I don't really have another idea of how not only to fix the issue but also try to prevent it to appear again in the future.
In exception messages.
Is it our exception or some third party library? If its ours I think its better to fix it on our side
there's a possibility that someone will use subprocess.run (instead of our wrapper)
The same is possible if password is emitted in a form that doesnt match current regex.
The proper way IMO is to clean up our API, so it is less possible that someone will misuse it. The problem is always caused by invoking postgres binaries and passing connection string to them. So should be possible to fix it with api changes that are not that big. See my previous suggestions, I can elaborate if needed.
Additionally we can set up a lint, (maybe write our own flake8 plugin or something) and require that lint to pass before we run tests that use outside connection credentials.
IMO best option would be to completely remove sensitive data from command line. Password can be passed via an environment variable and we can wrap all pg binaries we use to avoid this in the future for common cases
Thanks for the feedback @LizardWizzard & @funbringer!
Ok, now the password is passing to pgbench via PGPASSWORD