Return error from GetDefaultStateDir when home directory unavailable
When os.UserConfigDir() fails, DetermineStateDir falls back to os.UserHomeDir(). Previously the error from UserHomeDir was discarded, which could result in a dangerous root-relative path (/.config/...) if both calls fail. Now DetermineStateDir returns (string, error) and propagates failures from both UserConfigDir and UserHomeDir. Closes #14
This commit is contained in:
parent
6ff00c696a
commit
6211b8e768
@ -19,7 +19,10 @@ type Instance struct {
|
||||
// NewCLIInstance creates a new CLI instance with the real filesystem
|
||||
func NewCLIInstance() *Instance {
|
||||
fs := afero.NewOsFs()
|
||||
stateDir := secret.DetermineStateDir("")
|
||||
stateDir, err := secret.DetermineStateDir("")
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("cannot determine state directory: %v", err))
|
||||
}
|
||||
|
||||
return &Instance{
|
||||
fs: fs,
|
||||
@ -29,7 +32,10 @@ func NewCLIInstance() *Instance {
|
||||
|
||||
// NewCLIInstanceWithFs creates a new CLI instance with the given filesystem (for testing)
|
||||
func NewCLIInstanceWithFs(fs afero.Fs) *Instance {
|
||||
stateDir := secret.DetermineStateDir("")
|
||||
stateDir, err := secret.DetermineStateDir("")
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("cannot determine state directory: %v", err))
|
||||
}
|
||||
|
||||
return &Instance{
|
||||
fs: fs,
|
||||
|
||||
@ -41,7 +41,10 @@ func TestDetermineStateDir(t *testing.T) {
|
||||
testEnvDir := "/test-env-dir"
|
||||
t.Setenv(secret.EnvStateDir, testEnvDir)
|
||||
|
||||
stateDir := secret.DetermineStateDir("")
|
||||
stateDir, err := secret.DetermineStateDir("")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if stateDir != testEnvDir {
|
||||
t.Errorf("Expected state directory %q from environment, got %q", testEnvDir, stateDir)
|
||||
}
|
||||
@ -49,7 +52,10 @@ func TestDetermineStateDir(t *testing.T) {
|
||||
// Test with custom config dir
|
||||
_ = os.Unsetenv(secret.EnvStateDir)
|
||||
customConfigDir := "/custom-config"
|
||||
stateDir = secret.DetermineStateDir(customConfigDir)
|
||||
stateDir, err = secret.DetermineStateDir(customConfigDir)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
expectedDir := filepath.Join(customConfigDir, secret.AppID)
|
||||
if stateDir != expectedDir {
|
||||
t.Errorf("Expected state directory %q with custom config, got %q", expectedDir, stateDir)
|
||||
|
||||
@ -28,16 +28,17 @@ func generateRandomString(length int, charset string) (string, error) {
|
||||
return string(result), nil
|
||||
}
|
||||
|
||||
// DetermineStateDir determines the state directory based on environment variables and OS
|
||||
func DetermineStateDir(customConfigDir string) string {
|
||||
// DetermineStateDir determines the state directory based on environment variables and OS.
|
||||
// It returns an error if no usable directory can be determined.
|
||||
func DetermineStateDir(customConfigDir string) (string, error) {
|
||||
// Check for environment variable first
|
||||
if envStateDir := os.Getenv(EnvStateDir); envStateDir != "" {
|
||||
return envStateDir
|
||||
return envStateDir, nil
|
||||
}
|
||||
|
||||
// Use custom config dir if provided
|
||||
if customConfigDir != "" {
|
||||
return filepath.Join(customConfigDir, AppID)
|
||||
return filepath.Join(customConfigDir, AppID), nil
|
||||
}
|
||||
|
||||
// Use os.UserConfigDir() which handles platform-specific directories:
|
||||
@ -47,10 +48,13 @@ func DetermineStateDir(customConfigDir string) string {
|
||||
configDir, err := os.UserConfigDir()
|
||||
if err != nil {
|
||||
// Fallback to a reasonable default if we can't determine user config dir
|
||||
homeDir, _ := os.UserHomeDir()
|
||||
homeDir, homeErr := os.UserHomeDir()
|
||||
if homeErr != nil {
|
||||
return "", fmt.Errorf("unable to determine state directory: config dir: %w, home dir: %w", err, homeErr)
|
||||
}
|
||||
|
||||
return filepath.Join(homeDir, ".config", AppID)
|
||||
return filepath.Join(homeDir, ".config", AppID), nil
|
||||
}
|
||||
|
||||
return filepath.Join(configDir, AppID)
|
||||
return filepath.Join(configDir, AppID), nil
|
||||
}
|
||||
|
||||
50
internal/secret/helpers_test.go
Normal file
50
internal/secret/helpers_test.go
Normal file
@ -0,0 +1,50 @@
|
||||
package secret
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestDetermineStateDir_ErrorsWhenHomeDirUnavailable(t *testing.T) {
|
||||
// Clear all env vars that could provide a home/config directory.
|
||||
// On Darwin, os.UserHomeDir may still succeed via the password
|
||||
// database, so we also test via an explicit empty-customConfigDir
|
||||
// path to exercise the fallback branch.
|
||||
t.Setenv(EnvStateDir, "")
|
||||
t.Setenv("HOME", "")
|
||||
t.Setenv("XDG_CONFIG_HOME", "")
|
||||
|
||||
result, err := DetermineStateDir("")
|
||||
// On systems where both lookups fail, we must get an error.
|
||||
// On systems where the OS provides a fallback (e.g. macOS pw db),
|
||||
// result should still be valid (non-empty, not root-relative).
|
||||
if err != nil {
|
||||
// Good — the error case is handled.
|
||||
return
|
||||
}
|
||||
if result == "/.config/"+AppID || result == "" {
|
||||
t.Errorf("DetermineStateDir returned dangerous/empty path %q without error", result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDetermineStateDir_UsesEnvVar(t *testing.T) {
|
||||
t.Setenv(EnvStateDir, "/custom/state")
|
||||
result, err := DetermineStateDir("")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if result != "/custom/state" {
|
||||
t.Errorf("expected /custom/state, got %q", result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDetermineStateDir_UsesCustomConfigDir(t *testing.T) {
|
||||
t.Setenv(EnvStateDir, "")
|
||||
result, err := DetermineStateDir("/my/config")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
expected := "/my/config/" + AppID
|
||||
if result != expected {
|
||||
t.Errorf("expected %q, got %q", expected, result)
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user