diff --git a/internal/config/dir.go b/internal/config/dir.go index f366cacf..9fcf280c 100644 --- a/internal/config/dir.go +++ b/internal/config/dir.go @@ -2,6 +2,7 @@ package config import ( "errors" + "fmt" "os" "path/filepath" "runtime" @@ -11,7 +12,7 @@ import ( // TempDir returns the path to a user-specific temporary directory, suitable for caches. // -// It creates the temporary directory if it does not already exist. +// It creates the temporary directory if it does not already exist, and checks that it is private to the user. // // The directory can be specified in the {ENV_PREFIX}TMP environment variable. // @@ -54,12 +55,18 @@ func (c *Config) TempDir() (string, error) { return "", err } } + if err := ensurePrivateDir(path); err != nil { + return "", err + } c.tempDir = path return path, nil } -// WritableUserDir returns the path to a writable user-level directory. +// WritableUserDir returns the path to a writable user-level directory, e.g. for credentials and state. +// +// As in the legacy CLI, which shares it, a temporary directory is used if the directory in the home directory cannot +// be written, e.g. on an application container. The directory must be private to the user. // // Deprecated: unless backwards compatibility is desired, TempDir is preferable. func (c *Config) WritableUserDir() (string, error) { @@ -71,18 +78,59 @@ func (c *Config) WritableUserDir() (string, error) { return "", err } path := filepath.Join(hd, c.Application.WritableUserDir) + if !canWrite(path) { + path = filepath.Join(os.TempDir(), c.Application.TempSubDir) + } if err := os.MkdirAll(path, 0o700); err != nil { return "", err } + if err := ensurePrivateDir(path); err != nil { + return "", err + } c.writableUserDir = path return path, nil } -// HomeDir returns the home directory configured via an environment variable, or the OS's user home directory otherwise. +// canWrite checks whether a directory is writable, or can be created, using permissions only. +// +// This matches the legacy CLI (Filesystem::canWrite), so both choose the same directory, e.g. even on a full disk. +func canWrite(path string) bool { + if info, err := os.Stat(path); err == nil { + return info.IsDir() && isWritable(path, info) + } + for p := filepath.Dir(path); ; p = filepath.Dir(p) { + if info, err := os.Stat(p); err == nil { + return isWritable(p, info) + } + if filepath.Dir(p) == p { + return false + } + } +} + +// HomeDir returns the user's home directory. +// +// It checks the same environment variables as the legacy CLI, in order: {ENV_PREFIX}HOME, HOME and USERPROFILE. +// On Windows, HOME can differ from USERPROFILE, e.g. in MSYS2 or Cygwin. As in the legacy CLI, the directory must +// exist, and its real path is returned. func (c *Config) HomeDir() (string, error) { - if fromEnv := os.Getenv(c.Application.EnvPrefix + "HOME"); fromEnv != "" { - return fromEnv, nil + for _, name := range []string{c.Application.EnvPrefix + "HOME", "HOME", "USERPROFILE"} { + v := os.Getenv(name) + if v == "" { + continue + } + // G703: the user chooses their home directory. + if info, err := os.Stat(v); err != nil || !info.IsDir() { //nolint:gosec + return "", fmt.Errorf("invalid environment variable %s: %s (not a directory)", name, v) + } + // Resolve the path like PHP's realpath. + if abs, err := filepath.Abs(v); err == nil { + if resolved, err := filepath.EvalSymlinks(abs); err == nil { + return resolved, nil + } + } + return v, nil } return os.UserHomeDir() } diff --git a/internal/config/dir_test.go b/internal/config/dir_test.go new file mode 100644 index 00000000..3ec9a3b0 --- /dev/null +++ b/internal/config/dir_test.go @@ -0,0 +1,115 @@ +package config_test + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/upsun/cli/internal/config" +) + +func TestHomeDir(t *testing.T) { + a, b, c := t.TempDir(), t.TempDir(), t.TempDir() + link := filepath.Join(t.TempDir(), "link") + if runtime.GOOS != "windows" { + require.NoError(t, os.Symlink(a, link)) + } + resolved := func(p string) string { + r, err := filepath.EvalSymlinks(p) + require.NoError(t, err) + return r + } + cases := []struct { + name string + env map[string]string + want string + wantErr string + }{ + {name: "prefixed var first", env: map[string]string{"EXAMPLE_CLI_HOME": a, "HOME": b, "USERPROFILE": c}, want: a}, + {name: "then HOME", env: map[string]string{"EXAMPLE_CLI_HOME": "", "HOME": b, "USERPROFILE": c}, want: b}, + {name: "then USERPROFILE", env: map[string]string{"EXAMPLE_CLI_HOME": "", "HOME": "", "USERPROFILE": c}, want: c}, + {name: "symlink is resolved", env: map[string]string{"EXAMPLE_CLI_HOME": link}, want: a}, + { + name: "not a directory", + env: map[string]string{"EXAMPLE_CLI_HOME": filepath.Join(a, "missing")}, + wantErr: "invalid environment variable EXAMPLE_CLI_HOME", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if c.env["EXAMPLE_CLI_HOME"] == link && runtime.GOOS == "windows" { + t.Skip("symlinks need privileges on Windows") + } + cnf, err := config.FromYAML([]byte(validConfig)) + require.NoError(t, err) + for k, v := range c.env { + t.Setenv(k, v) + } + home, err := cnf.HomeDir() + if c.wantErr != "" { + assert.ErrorContains(t, err, c.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, resolved(c.want), home) + }) + } +} + +func TestHomeDir_Relative(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + cnf, err := config.FromYAML([]byte(validConfig)) + require.NoError(t, err) + t.Setenv("EXAMPLE_CLI_HOME", ".") + home, err := cnf.HomeDir() + require.NoError(t, err) + want, err := filepath.EvalSymlinks(dir) + require.NoError(t, err) + assert.Equal(t, want, home) +} + +// TestWritableUserDir_TempFallback checks the cases where the legacy CLI uses a temporary directory instead. +func TestWritableUserDir_TempFallback(t *testing.T) { + cases := []struct { + name string + setup func(t *testing.T, home string) + }{ + { + name: "read-only home", + setup: func(t *testing.T, home string) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("needs Unix permissions") + } + require.NoError(t, os.Chmod(home, 0o500)) + t.Cleanup(func() { _ = os.Chmod(home, 0o700) }) + }, + }, + { + name: "a file in place of the directory", + setup: func(t *testing.T, home string) { + require.NoError(t, os.WriteFile(filepath.Join(home, ".example-cli"), nil, 0o600)) + }, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cnf, err := config.FromYAML([]byte(validConfig)) + require.NoError(t, err) + home := t.TempDir() + c.setup(t, home) + tmp := t.TempDir() + t.Setenv("EXAMPLE_CLI_HOME", home) + t.Setenv("TMPDIR", tmp) // Unix + t.Setenv("TMP", tmp) // Windows + + dir, err := cnf.WritableUserDir() + require.NoError(t, err) + assert.Equal(t, filepath.Join(tmp, "example-cli-tmp"), dir) + }) + } +} diff --git a/internal/config/dir_unix.go b/internal/config/dir_unix.go new file mode 100644 index 00000000..374102bb --- /dev/null +++ b/internal/config/dir_unix.go @@ -0,0 +1,72 @@ +//go:build unix + +package config + +import ( + "fmt" + "os" + "path/filepath" + "syscall" + + "golang.org/x/sys/unix" +) + +// isWritable checks write permission, like PHP's is_writable. +func isWritable(path string, _ os.FileInfo) bool { + return unix.Access(path, unix.W_OK) == nil +} + +// checkPrivateDir checks a directory if others could have created it, i.e. if its parent is world-writable, e.g. +// /tmp. The directory, and any symlink to it, must then be owned by the user (uid), and it is made private. +// +// Otherwise, e.g. in a home directory, it can be owned by another user, as with "sudo -E" or an arbitrary UID. +func checkPrivateDir(path string, uid int) error { + // G703: the path is the user's own config or temporary directory. + info, err := os.Lstat(path) //nolint:gosec + if err != nil { + return err + } + shared, err := hasSharedParent(path) + if err != nil { + return err + } + if shared && !ownedBy(info, uid) { + return fmt.Errorf("not owned by the current user: %s", path) + } + if info.Mode()&os.ModeSymlink != 0 { + target, err := filepath.EvalSymlinks(path) + if err != nil { + return err + } + return checkPrivateDir(target, uid) + } + if !info.IsDir() { + return fmt.Errorf("not a directory: %s", path) + } + if shared && info.Mode().Perm()&0o077 != 0 { + return os.Chmod(path, 0o700) //nolint:gosec // G703: as above + } + return nil +} + +// hasSharedParent reports whether a directory's parent is world-writable. +// +// Group-writable parents are not included: they are common with user private groups (umask 002), and in containers +// with arbitrary UIDs, where the home directory is owned by another user. +func hasSharedParent(path string) (bool, error) { + info, err := os.Stat(filepath.Dir(path)) //nolint:gosec // G703: as above + if err != nil { + return false, err + } + return info.Mode().Perm()&0o002 != 0, nil +} + +func ownedBy(info os.FileInfo, uid int) bool { + st, ok := info.Sys().(*syscall.Stat_t) + return ok && int(st.Uid) == uid +} + +// ensurePrivateDir checks that a directory is private to the current user. +func ensurePrivateDir(path string) error { + return checkPrivateDir(path, os.Geteuid()) +} diff --git a/internal/config/dir_unix_test.go b/internal/config/dir_unix_test.go new file mode 100644 index 00000000..78f1d6d0 --- /dev/null +++ b/internal/config/dir_unix_test.go @@ -0,0 +1,111 @@ +//go:build unix + +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckPrivateDir(t *testing.T) { + other := os.Geteuid() + 1 + cases := []struct { + name string + shared bool // Whether the parent directory is world-writable, like /tmp. + setup func(t *testing.T, path string) + uid int + wantErr string + wantMode os.FileMode + }{ + { + name: "own directory in a shared parent", + shared: true, + setup: mkdir(0o700), + uid: os.Geteuid(), + wantMode: 0o700, + }, + { + name: "open directory in a shared parent is tightened", + shared: true, + setup: mkdir(0o777), + uid: os.Geteuid(), + wantMode: 0o700, + }, + { + name: "another user's directory in a shared parent", + shared: true, + setup: mkdir(0o700), + uid: other, + wantErr: "not owned by the current user", + }, + { + name: "another user's symlink in a shared parent", + shared: true, + setup: func(t *testing.T, path string) { require.NoError(t, os.Symlink(t.TempDir(), path)) }, + uid: other, + wantErr: "not owned by the current user", + }, + { + name: "symlink to another user's directory in a shared parent", + setup: func(t *testing.T, path string) { + target := filepath.Join(sharedDir(t), "target") + require.NoError(t, os.Mkdir(target, 0o700)) + require.NoError(t, os.Symlink(target, path)) + }, + uid: other, + wantErr: "not owned by the current user", + }, + { + // E.g. sudo -E, or an arbitrary UID in a container. + name: "another user's directory in a private parent", + setup: mkdir(0o755), + uid: other, + wantMode: 0o755, + }, + { + name: "file", + shared: true, + setup: func(t *testing.T, path string) { require.NoError(t, os.WriteFile(path, nil, 0o600)) }, + uid: os.Geteuid(), + wantErr: "not a directory", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + parent := t.TempDir() + require.NoError(t, os.Chmod(parent, 0o755)) + if c.shared { + parent = sharedDir(t) + } + path := filepath.Join(parent, "dir") + c.setup(t, path) + err := checkPrivateDir(path, c.uid) + if c.wantErr != "" { + assert.ErrorContains(t, err, c.wantErr) + return + } + require.NoError(t, err) + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, c.wantMode, info.Mode().Perm()) + }) + } +} + +func mkdir(mode os.FileMode) func(t *testing.T, path string) { + return func(t *testing.T, path string) { + require.NoError(t, os.Mkdir(path, 0o700)) + require.NoError(t, os.Chmod(path, mode)) + } +} + +// sharedDir returns a directory that others can write to, like /tmp. +func sharedDir(t *testing.T) string { + dir := t.TempDir() + require.NoError(t, os.Chmod(dir, 0o777|os.ModeSticky)) + return dir +} diff --git a/internal/config/dir_windows.go b/internal/config/dir_windows.go new file mode 100644 index 00000000..0e0d966c --- /dev/null +++ b/internal/config/dir_windows.go @@ -0,0 +1,13 @@ +package config + +import "os" + +// isWritable checks the read-only attribute, like PHP's is_writable on Windows. +func isWritable(_ string, info os.FileInfo) bool { + return info.Mode().Perm()&0o200 != 0 +} + +// ensurePrivateDir does nothing on Windows, where the temporary directory is per user. +func ensurePrivateDir(_ string) error { + return nil +}