-
Notifications
You must be signed in to change notification settings - Fork 175
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
pass: fix interpolation of $PASSWORD_STORE_DIR, and use os.UserHomeDir() #287
Conversation
commit a13ff50 simplified the handling of env-vars in getPassDir(), but moved interpolation of env-vars to the end of the function. As a result, a custom path passed through `$PASSWORD_STORE_DIR` would now be interpolated, instead of taken as-is. For example; PASSWORD_STORE_DIR=$PWD/world Would now interpolate `$PWD`, instead of using a literal `$PWD`. This patch changes the logic to only expand env-vars for the default location. Signed-off-by: Sebastiaan van Stijn <[email protected]>
I'll do a follow-up to improve the home-dir lookup (now that we made |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #287 +/- ##
=======================================
Coverage 54.68% 54.68%
=======================================
Files 9 9
Lines 673 673
=======================================
Hits 368 368
Misses 262 262
Partials 43 43
☔ View full report in Codecov by Sentry. |
@crazy-max ptal 😅 |
pass/pass.go
Outdated
} | ||
return os.ExpandEnv(passDir) | ||
return os.ExpandEnv("$HOME/.password-store") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be safer to use os.UserHomeDir()
:
return os.ExpandEnv("$HOME/.password-store") | |
homedir, err := os.UserHomeDir() | |
if err != nil { | |
return "", err | |
} | |
return path.Join(homedir, ".password-store"), nil |
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, yes, I have something like that staged #287 (comment)
I'll do a follow-up to improve the home-dir lookup (now that we made pass compile for other platforms as well), but making this fix separate first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually started with that, and then changed it back to the most minimal changes for this PR 😅
Use stdlib's os.UserHomeDir() instead of depending only on $HOME. Note that this does not yet does nss lookups for situations where $HOME / $USERPROFILE is not set. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Added a commit to use |
commit a13ff50 (#109) simplified the handling of env-vars in getPassDir(), but moved interpolation of env-vars to the end of the function.
As a result, a custom path passed through
$PASSWORD_STORE_DIR
would now be interpolated, instead of taken as-is. For example;Would now interpolate
$PWD
, instead of using a literal$PWD
.This patch changes the logic to only expand env-vars for the default location.
pass: make home-dir resolution platform agnostic
Use stdlib's
os.UserHomeDir()
instead of depending only on$HOME
. Note thatthis does not yet does nss lookups for situations where
$HOME
/$USERPROFILE
is not set.