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

Allow fetching passwords from environment variables #338

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zx2c4
Copy link

@zx2c4 zx2c4 commented May 3, 2023

Passing sensitive data through command line args is not generally safe on many *nix systems, because the arguments leak to all users, such as through /proc/PID/cmdline on Linux.

Instead, the usual pattern for passing passwords to programs is via environment variables, which do not leak to all users. Wire up the input getter's logic to handle these.

The existing logic is:

  • If the argument is a file that exists or starts with file:, treat it as a file and read its contents.
  • Otherwise, treat it as the data itself.

The new logic is:

  • If the argument is a file that exists or starts with file:, treat it as a file and read its contents.
  • If the argument starts with env: and that environment variable exists, treat it as an environment variable and read its contents.
  • Otherwise, treat it as the data itself.

Requiring the environment variable to actually exist -- much like how file's unprefixed by file: have to exist to be considered as files -- will mitigate false positives of, e.g., actual passwords that begin with "env:", so the potential for regression should be exceedingly low.

Passing sensitive data through command line args is not generally safe
on many *nix systems, because the arguments leak to all users, such as
through /proc/PID/cmdline on Linux.

Instead, the usual pattern for passing passwords to programs is via
environment variables, which do not leak to all users. Wire up the input
getter's logic to handle these.

The existing logic is:
- If the argument is a file that exists or starts with file:, treat it
  as a file and read its contents.
- Otherwise, treat it as the data itself.

The new logic is:
- If the argument is a file that exists or starts with file:, treat it
  as a file and read its contents.
- If the argument starts with env: and that environment variable exists,
  treat it as an environment variable and read its contents.
- Otherwise, treat it as the data itself.

Requiring the environment variable to actually exist -- much like how
file's unprefixed by file: have to exist to be considered as files --
will mitigate false positives of, e.g., actual passwords that begin with
"env:", so the potential for regression should be exceedingly low.
@zx2c4
Copy link
Author

zx2c4 commented May 3, 2023

The two failures seem to be issues running something called "ghost tunnel". Is that a CI flake?

@qpernil
Copy link
Contributor

qpernil commented May 12, 2023

Yes the hardware tests require access to secrets that your branch doesn't have access to. I will test manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants