Repository navigation
fix(config): respect $HOME when resolving the default storage directory - #704
Open
NopeCharon wants to merge 1 commit into
Open
NopeCharon wants to merge 1 commit into
NopeCharon wants to merge 1 commit into
Conversation
config.NewRoot resolved the default storage and log directories through os/user.Current. With cgo enabled (as in the release builds) that queries the user database via getpwuid_r and ignores the $HOME environment variable, so overriding $HOME, for example in tests or sandboxed environments, still created ~/.modctl under the system home directory. This was observed on aarch64-darwin, where every command, including `modctl version`, touched the real home directory. Resolve the home directory with os.UserHomeDir instead, which honors $HOME (%USERPROFILE% on Windows), and fall back to os/user.Current only when the environment variable is unset so that environments without $HOME keep working as before. Add unit tests for both the $HOME override and the fallback path. Fixes modelpack#580 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
config.NewRoot()resolves the default--storage-dirand--log-dirthroughos/user.Current(). With cgo enabled (as in the release builds), that queries the user database viagetpwuid_rand ignores$HOME. As a result, every command, includingmodctl version, creates~/.modctlunder the system home directory even when$HOMEpoints somewhere else, for example a temporary directory in tests or a sandbox.Fixes #580
Fix
os.UserHomeDir(), which honors$HOME(%USERPROFILE%on Windows).os/user.Current()only when the environment variable is unset. Environments without$HOME(some service managers or minimal containers) keep working as before instead of failing at startup.Tests
pkg/config/root_test.go:TestNewRootRespectsHomeEnvsets$HOMEto a temp dir and checksStorageDir/LogDir. It fails onmain(it resolves to the real home directory) and passes with this change.TestNewRootFallsBackToCurrentUserclears$HOMEand checks theos/userfallback.go test -tags static,system_libgit2 ./pkg/config ./cmdpasses.golangci-lint run(v2.4) reports 0 issues.HOME=/tmp/fakehome modctl versionnow creates/tmp/fakehome/.modctl.