fix: use HOME dir if set
$HOME should be preferred source of home directory information.
It's a common pattern to run software with a custom HOME to temporarily isolate it from the system-set home directory. All Unix software I ever worked with would respect HOME.
Preferring user.Current() (which I guess reads from /etc/passwd) make it impossible to temporarily change effective home, as changing /etc/passwd is not an option.
I'm currently migrating things to a self-hosted github-actions runner, where one system user is used for all github actions runners, but each instance gets a different HOME, and is otherwise sandboxed to not even have permissions to touch the original home. Everything works except lncli which fails with:
[lncli] could not load global options: could not read profile file /home/github-runner/.lncli/profiles.json: could not load profile file /home/github-runner/.lncli/profiles.json: open /home/github-runner/.lncli/profiles.json: permission denied
and I traced it to this code.
Pull Request Test Coverage Report for Build 8811320930
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
- For more information on this, see Tracking coverage changes with pull request builds.
- To avoid this issue with future PRs, see these Recommended CI Configurations.
- For a quick fix, rebase this PR at GitHub. Your next report should be accurate.
Details
- 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
- 6 unchanged lines in 1 file lost coverage.
- Overall coverage decreased (-0.008%) to 56.878%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| btcutil/appdata.go | 4 | 5 | 80.0% |
| <!-- | Total: | 4 | 5 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| peer/peer.go | 6 | 74.16% |
| <!-- | Total: | 6 |
| Totals | |
|---|---|
| Change from base Build 8802845904: | -0.008% |
| Covered Lines: | 29452 |
| Relevant Lines: | 51781 |
💛 - Coveralls
Thanks for the PR. But I'm very hesitant to accept a change like this, since it seems it would change the default behavior of all our projects that rely on this code. I'd say it's very unusual for user.Current() to differ from $HOME.
I understand your case, but shouldn't the actual fix here be that lncli doesn't error out if it cannot read a file that it's not using anyway? Since the profiles.json is just ignored when it doesn't exist, so it should also ignore if the path cannot be accessed.
But I'm very hesitant to accept a change like this, since it seems it would change the default behavior of all our projects that rely on this code.
Yeah. This is a valid concern.
I'd say it's very unusual for
user.Current()to differ from$HOME.
That's subjective I guess, but I don't find it uncommon, and use that functionality a lot. It's especially common when dealing with systemd units, long running daemons, etc. Thought I guess the fact that I'm the first to report it, might suggest that you're right. But by the same coin - not a lot of people will be affected by switching it.
As far as I know HOME is the way to get home directory. I did some experiments and it seems like e.g. all programming languages I tried will use HOME first and then fallback to getpwuid_r.
Rust standard library:
https://doc.rust-lang.org/std/env/fn.home_dir.html
Python:
> env HOME=/tmp python
Python 3.11.8 (main, Feb 6 2024, 21:21:21) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pathlib import Path
>>> print(Path.home())
/tmp
>>>
> env -u HOME python
Python 3.11.8 (main, Feb 6 2024, 21:21:21) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pathlib import Path
>>> print(Path.home())
/home/dpc
>>>
Node.js:
> env HOME=/tmp nix run nixpkgs#nodejs
warning: $HOME ('/tmp') is not owned by you, falling back to the one defined in the 'passwd' file ('/home/dpc')
Welcome to Node.js v20.12.2.
Type ".help" for more information.
> require('os').homedir()
'/tmp'
>
> env -u HOME nix run nixpkgs#nodejs
Welcome to Node.js v20.12.2.
Type ".help" for more information.
> require('os').homedir()
'/home/dpc'
>
Ruby:
> env HOME=/tmp nix shell nixpkgs#ruby -c irb
warning: $HOME ('/tmp') is not owned by you, falling back to the one defined in the 'passwd' file ('/home/dpc')
irb(main):001:0> Dir.home
=> "/tmp"
irb(main):002:0>
> env -u HOME nix shell nixpkgs#ruby -c irb
irb(main):001:0> Dir.home
=> "/home/dpc"
irb(main):002:0>
Julia:
> env HOME=/tmp nix run nixpkgs#julia
warning: $HOME ('/tmp') is not owned by you, falling back to the one defined in the 'passwd' file ('/home/dpc')
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.10.2 (2024-03-01)
_/ |\__'_|_|_|\__'_| |
|__/ |
julia> homedir()
"/tmp"
> env -u HOME nix run nixpkgs#julia
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.10.2 (2024-03-01)
_/ |\__'_|_|_|\__'_| |
|__/ |
julia> homedir()
"/home/dpc"
julia>
Haskell:
> env HOME=/tmp nix shell nixpkgs#ghc -c ghci
warning: $HOME ('/tmp') is not owned by you, falling back to the one defined in the 'passwd' file ('/home/dpc')
GHCi, version 9.6.4: https://www.haskell.org/ghc/ :? for help
ghci> import System.Directory
ghci>
ghci> main :: IO ()
<interactive>:3:1: error: [GHC-88464]
Variable not in scope: main :: IO ()
Suggested fix: Perhaps use ‘min’ (imported from Prelude)
ghci> main = do
<interactive>:4:8: error: [GHC-82311]
Empty 'do' block
Suggested fix: Perhaps you intended to use NondecreasingIndentation
ghci> homeDir <- getHomeDirectory
ghci> putStrLn ("Your home directory is: " ++ homeDir)
Your home directory is: /tmp
ghci>
> env -u HOME nix shell nixpkgs#ghc -c ghci
GHCi, version 9.6.4: https://www.haskell.org/ghc/ :? for help
ghci> import System.Directory
ghci>
ghci> main :: IO ()
<interactive>:3:1: error: [GHC-88464]
Variable not in scope: main :: IO ()
Suggested fix: Perhaps use ‘min’ (imported from Prelude)
ghci> main = do
<interactive>:4:8: error: [GHC-82311]
Empty 'do' block
Suggested fix: Perhaps you intended to use NondecreasingIndentation
ghci> homeDir <- getHomeDirectory
ghci> putStrLn ("Your home directory is: " ++ homeDir)
Your home directory is: /home/dpc
ghci>
Hmm, interesting. Thanks for the additional info and the test cases.
Maybe we should instead do what the shell does and check if we can actually access the directory returned by user.Current()? And if that fails, fall back to $HOME?
That would only change the default behavior if the permissions are set similar to your restricted GitHub runner case.
I understand the worry about not breaking lots of downstream projects, but I don't think making things more complicated is going to be helpful.
How about phasing it in, so:
- compare
HOMEandCurrent(), if they are the same no worries - if they are different, check some
BTCD_PREFER_HOME_ENV- if set, just use
HOME - if not, print a warning that previous versions used technically wrong (
Current()) directory, and this will change in the future. SettingBTCD_PREFER_HOME_ENV=1allows opting-in into a new behavior and avoiding this warning
- if set, just use
The end goal is using HOME. For time being old behavior is retained out of caution. I (and others like me) can just set BTCD_PREFER_HOME_ENV=1 and get the desired behavior. Everyone else gets warned about the change, can look it up and complain if they want to. :D
After some time old behavior and BTCD_PREFER_HOME_ENV can be removed. Only the handful of people that actually play HOME-setting shenanigans will ever know.
Sounds good to me.