diff --git a/0.22.0-release-notes.md b/0.22.0-release-notes.md index f277567ce..ee2d61515 100644 --- a/0.22.0-release-notes.md +++ b/0.22.0-release-notes.md @@ -5,3 +5,11 @@ # Deprecations # Breaking changes + +- If you are upgrading from a version older than 0.14.0, make sure that you + close all Elvish processes running the old version before starting any new + ones. + + This is because the logic for determining how to connect to daemon has + changed and is no longer backward compatible with versions older than + 0.14.0. diff --git a/pkg/shell/paths_test.elvts b/pkg/shell/paths_test.elvts new file mode 100644 index 000000000..6d4c4896a --- /dev/null +++ b/pkg/shell/paths_test.elvts @@ -0,0 +1,60 @@ +///////////////////////// +# run directory on Unix # +///////////////////////// + +//only-on unix +//each:in-temp-dir +//each:secure-run-dir-in-global +//each:unset-env XDG_RUNTIME_DIR +//each:unset-env TMPDIR + +## uses XDG_RUNTIME_DIR if it's non-empty, tmpdir otherwise ## +//uid-in-global +~> use os + os:mkdir-all &perm=0o700 tmpdir/elvish-$uid + set E:TMPDIR = $pwd/tmpdir +~> eq (secure-run-dir) $E:TMPDIR/elvish-$uid +▶ $true +~> set E:XDG_RUNTIME_DIR = '' + eq (secure-run-dir) $E:TMPDIR/elvish-$uid +▶ $true +~> set E:XDG_RUNTIME_DIR = $pwd/xdg_runtime_dir + os:mkdir-all &perm=0o700 xdg_runtime_dir/elvish + eq (secure-run-dir) $E:XDG_RUNTIME_DIR/elvish +▶ $true + +// Tests below all use XDG_RUNTIME_DIR for simplicity. + +## creates run directory if it doesn't exist yet ## +~> use os + set E:XDG_RUNTIME_DIR = $pwd/xdg_runtime_dir + os:mkdir &perm=0o700 xdg_runtime_dir +~> eq (secure-run-dir) $E:XDG_RUNTIME_DIR/elvish +▶ $true +~> put (os:stat (secure-run-dir))[type] +▶ dir + +## errors if run directory exists but has wrong permission ## +//umask 0 +~> use os + os:mkdir-all &perm=0o777 xdg_runtime_dir/elvish + set E:XDG_RUNTIME_DIR = $pwd/xdg_runtime_dir +~> use re + try { secure-run-dir } catch e { + re:match 'existing run directory .* is not secure' (to-string $e[reason]) + } +▶ $true + +## errors if unable to create run directory ## +~> use os + os:mkdir-all &perm=0o000 xdg_runtime_dir + set E:XDG_RUNTIME_DIR = $pwd/xdg_runtime_dir +~> use re + try { secure-run-dir } catch e { + re:match 'create new run directory: ' (to-string $e[reason]) + } +▶ $true + +// TODO: Test the remaining error conditions in secureRunDir. I'm not aware of a +// way to make a real OS trigger those conditions, so testing them probably +// requires faking os.MkdirAll. diff --git a/pkg/shell/paths_unix.go b/pkg/shell/paths_unix.go index bab3e49bd..b83e7724b 100644 --- a/pkg/shell/paths_unix.go +++ b/pkg/shell/paths_unix.go @@ -32,46 +32,45 @@ func homePath(suffix string) (string, error) { } // Returns a "run directory" for storing ephemeral files, which is guaranteed -// to be only accessible to the current user. +// to be only accessible to the current user. The path is one of the following: // -// The path of the run directory is either $XDG_RUNTIME_DIR/elvish or -// $tmpdir/elvish-$uid (where $tmpdir is the system temporary directory). The -// former is used if the XDG_RUNTIME_DIR environment variable exists and the -// latter directory does not exist. +// - $XDG_RUNTIME_DIR/elvish if $XDG_RUNTIME_DIR is non-empty. +// - $tmpdir/elvish-$uid otherwise. func secureRunDir() (string, error) { - runDirs := runDirCandidates() - for _, runDir := range runDirs { - if checkExclusiveAccess(runDir) { - return runDir, nil + runDir := runDirPath() + info, err := os.Stat(runDir) + if err == nil { + // Already exists; just check if it's secure. + if !secureAsRunDir(info) { + return "", fmt.Errorf("existing run directory %v is not secure", runDir) } + return runDir, nil + } + // Create new run directory. + err = os.MkdirAll(runDir, 0700) + if err != nil { + return "", fmt.Errorf("create new run directory: %v", err) } - runDir := runDirs[0] - err := os.MkdirAll(runDir, 0700) + // The OS may have set a different owner or permission bits, so still check + // if it's secure. + info, err = os.Stat(runDir) if err != nil { - return "", fmt.Errorf("mkdir: %v", err) + return "", fmt.Errorf("stat newly created run directory: %v", err) } - if !checkExclusiveAccess(runDir) { - return "", fmt.Errorf("cannot create %v as a secure run directory", runDir) + if !secureAsRunDir(info) { + return "", fmt.Errorf("newly created run directory %v is not secure", runDir) } return runDir, nil } -// Returns one or more candidates for the run directory, in descending order of -// preference. -func runDirCandidates() []string { - tmpDirPath := filepath.Join(os.TempDir(), fmt.Sprintf("elvish-%d", os.Getuid())) +func runDirPath() string { if os.Getenv(env.XDG_RUNTIME_DIR) != "" { - xdgDirPath := filepath.Join(os.Getenv(env.XDG_RUNTIME_DIR), "elvish") - return []string{xdgDirPath, tmpDirPath} + return filepath.Join(os.Getenv(env.XDG_RUNTIME_DIR), "elvish") } - return []string{tmpDirPath} + return filepath.Join(os.TempDir(), fmt.Sprintf("elvish-%d", os.Getuid())) } -func checkExclusiveAccess(runDir string) bool { - info, err := os.Stat(runDir) - if err != nil { - return false - } +func secureAsRunDir(info os.FileInfo) bool { stat := info.Sys().(*syscall.Stat_t) return info.IsDir() && int(stat.Uid) == os.Getuid() && stat.Mode&077 == 0 } diff --git a/pkg/shell/paths_unix_test.go b/pkg/shell/paths_unix_test.go deleted file mode 100644 index 6ff27de00..000000000 --- a/pkg/shell/paths_unix_test.go +++ /dev/null @@ -1,67 +0,0 @@ -//go:build unix - -package shell - -import ( - "fmt" - "os" - "path/filepath" - "testing" - - "src.elv.sh/pkg/env" - "src.elv.sh/pkg/testutil" -) - -var elvishDashUID = fmt.Sprintf("elvish-%d", os.Getuid()) - -func TestSecureRunDir_PrefersXDGWhenNeitherExists(t *testing.T) { - xdg, _ := setupForSecureRunDir(t) - testSecureRunDir(t, filepath.Join(xdg, "elvish"), false) -} - -func TestSecureRunDir_PrefersXDGWhenBothExist(t *testing.T) { - xdg, tmp := setupForSecureRunDir(t) - - os.MkdirAll(filepath.Join(xdg, "elvish"), 0700) - os.MkdirAll(filepath.Join(tmp, elvishDashUID), 0700) - - testSecureRunDir(t, filepath.Join(xdg, "elvish"), false) -} - -func TestSecureRunDir_PrefersTmpWhenOnlyItExists(t *testing.T) { - _, tmp := setupForSecureRunDir(t) - - os.MkdirAll(filepath.Join(tmp, elvishDashUID), 0700) - - testSecureRunDir(t, filepath.Join(tmp, elvishDashUID), false) -} - -func TestSecureRunDir_PrefersTmpWhenXdgEnvIsEmpty(t *testing.T) { - _, tmp := setupForSecureRunDir(t) - os.Setenv(env.XDG_RUNTIME_DIR, "") - testSecureRunDir(t, filepath.Join(tmp, elvishDashUID), false) -} - -func TestSecureRunDir_ReturnsErrorWhenUnableToMkdir(t *testing.T) { - xdg, _ := setupForSecureRunDir(t) - os.WriteFile(filepath.Join(xdg, "elvish"), nil, 0600) - testSecureRunDir(t, "", true) -} - -func setupForSecureRunDir(c testutil.Cleanuper) (xdgRuntimeDir, tmpDir string) { - xdg := testutil.Setenv(c, env.XDG_RUNTIME_DIR, testutil.TempDir(c)) - tmp := testutil.Setenv(c, "TMPDIR", testutil.TempDir(c)) - return xdg, tmp -} - -func testSecureRunDir(t *testing.T, wantRunDir string, wantErr bool) { - runDir, err := secureRunDir() - if runDir != wantRunDir { - t.Errorf("got rundir %q, want %q", runDir, wantRunDir) - } - if wantErr && err == nil { - t.Errorf("got nil err, want non-nil") - } else if !wantErr && err != nil { - t.Errorf("got err %v, want nil err", err) - } -} diff --git a/pkg/shell/testexports_test.go b/pkg/shell/testexports_test.go new file mode 100644 index 000000000..955332b2b --- /dev/null +++ b/pkg/shell/testexports_test.go @@ -0,0 +1,3 @@ +package shell + +var SecureRunDir = secureRunDir diff --git a/pkg/shell/transcripts_test.go b/pkg/shell/transcripts_test.go index aa89e212d..83249a60a 100644 --- a/pkg/shell/transcripts_test.go +++ b/pkg/shell/transcripts_test.go @@ -6,6 +6,7 @@ import ( "io" "os" "path/filepath" + "strconv" "testing" "time" @@ -14,6 +15,7 @@ import ( "src.elv.sh/pkg/eval" "src.elv.sh/pkg/eval/evaltest" "src.elv.sh/pkg/eval/vars" + "src.elv.sh/pkg/must" "src.elv.sh/pkg/prog/progtest" "src.elv.sh/pkg/shell" "src.elv.sh/pkg/testutil" @@ -38,6 +40,11 @@ func TestTranscripts(t *testing.T) { "kill-wait-in-global", addGlobal("kill-wait", testutil.Scaled(10*time.Millisecond).String()), "sigchld-name-in-global", addGlobal("sigchld-name", sigCHLDName), + "secure-run-dir-in-global", evaltest.GoFnInGlobal("secure-run-dir", shell.SecureRunDir), + "uid-in-global", addGlobal("uid", os.Getuid()), + "umask", func(t *testing.T, arg string) { + testutil.Umask(t, must.OK1(strconv.Atoi(arg))) + }, "in-temp-home", func(t *testing.T) { testutil.InTempHome(t) }, ) }