Skip to content

pass: fix interpolation of $PASSWORD_STORE_DIR, and use os.UserHomeDir()#287

Merged
crazy-max merged 2 commits intodocker:masterfrom
thaJeztah:pass_no_interpolate
May 29, 2023
Merged

pass: fix interpolation of $PASSWORD_STORE_DIR, and use os.UserHomeDir()#287
crazy-max merged 2 commits intodocker:masterfrom
thaJeztah:pass_no_interpolate

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented May 28, 2023

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;

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.

pass: make home-dir resolution platform agnostic

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.

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]>
@thaJeztah
Copy link
Copy Markdown
Member Author

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.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 28, 2023

Codecov Report

Patch coverage: 50.00% and no project coverage change.

Comparison is base (2860ca4) 54.68% compared to head (372315b) 54.68%.

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           
Impacted Files Coverage Δ
pass/pass.go 67.25% <50.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@thaJeztah
Copy link
Copy Markdown
Member Author

@crazy-max ptal 😅

Comment thread pass/pass.go Outdated
return passDir
}
return os.ExpandEnv(passDir)
return os.ExpandEnv("$HOME/.password-store")
Copy link
Copy Markdown
Member

@crazy-max crazy-max May 28, 2023

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():

Suggested change
return os.ExpandEnv("$HOME/.password-store")
homedir, err := os.UserHomeDir()
if err != nil {
return "", err
}
return path.Join(homedir, ".password-store"), nil

WDYT?

Copy link
Copy Markdown
Member Author

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.

Copy link
Copy Markdown
Member Author

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]>
@thaJeztah thaJeztah changed the title pass: fix interpolation of $PASSWORD_STORE_DIR pass: fix interpolation of $PASSWORD_STORE_DIR, and use os.UserHomeDir() May 29, 2023
@thaJeztah
Copy link
Copy Markdown
Member Author

Added a commit to use os.UserHomeDir() PTAL

@crazy-max crazy-max merged commit c740b99 into docker:master May 29, 2023
@thaJeztah thaJeztah deleted the pass_no_interpolate branch May 29, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants