Cache directories are now user-specific

Adjust the cache directory naming scheme to include the EUID, thereby
allowing multiple users on the same machine to invoke `bkt` without
encountering permission issues trying to access the same directory.

For now this only applies to Unix, which is also the only platform where
bkt attempts to restrict access to the cache directory.

Also refreshed the docs on BKT_TMPDIR / BKT_CACHE_DIR.

Added some best-effort test coverage of this behavior; it's hard to reliably
test with multiple users that the test has access to, so for now it
simply tries to use sudo and silently skips the test if that fails.

Fixes #35

Cargo.toml

@@ -29,6 +29,7 @@ humantime = "2.1.0"
 rand = "0.8"
 serde = { version = "1.0", features = ["derive"] }
 base64 = "0.21.5"
+libc = "0.2"
 
 [dependencies.clap]
 version = "4.2"

README.md

@@ -119,10 +119,10 @@ different values for any of the given variables will be cached separately.
 
 #### File Modifications
 
-It is also possible to have `bkt` check the last-modified time of one or more
-files and include this in the cache key using `--modtime`. For instance passing
+`bkt` can also check the last-modified time of one or more files and include
+this in the cache key using `--modtime`. For instance passing
 `--modtime=/etc/passwd` would cause the backing command to be re-executed any
-time `/etc/passwd` is modified.
+time `/etc/passwd` is modified even if the TTL has not expired.
 
 ### Refreshing Manually
 
@@ -184,20 +184,30 @@ flag and instead make the client robust to occasional failures.
 <a name="cache_dir"></a>
 ### Changing the Cache Directory
 
-By default, cached data is stored under `/tmp` or a similar temporary directory;
-this can be customized via the `--cache-dir` flag or by defining a
-`BKT_CACHE_DIR` environment variable.
+By default, cached data is stored under your system's temporary directory
+(typically `/tmp` on Linux).
 
-If a `BKT_TMPDIR` environment variable is defined it wil be used instead of the
-system's temporary directory. Although `BKT_TMPDIR` and `BKT_CACHE_DIR` have
-similar effects `BKT_TMPDIR` is intended to be used to configure the global
-cache location (e.g. by declaring it in your `.bashrc` or similar), while
-`--cache-dir`/`BKT_CACHE_DIR` should be used to customize the cache location
-for a given set of invocations that shouldn't use the default cache directory.
+You may want to use a different location for certain commands, for instance to
+be able to easily delete the cached data as soon as it's no longer needed. You
+can specify a custom cache directory via the `--cache-dir` flag or by defining
+a `BKT_CACHE_DIR` environment variable.
 
 Note that the choice of directory can affect `bkt`'s performance: if the cache
-is stored under a [`tmpfs`](https://en.wikipedia.org/wiki/Tmpfs) or solid-state
-partition it will be significantly faster than caching to a spinning disk.
+directory is on a [`tmpfs`](https://en.wikipedia.org/wiki/Tmpfs) or solid-state
+partition it will be significantly faster than one using a spinning disk.
+
+If your system's temporary directory is not a good choice for the default cache
+location (e.g. it is not a `tmpfs`) you can specify a different location by
+defining a `BKT_TMPDIR` environment variable (for example in your `.bashrc`).
+These two environment variables, `BKT_TMPDIR` and `BKT_CACHE_DIR`, have similar
+effects but `BKT_TMPDIR` should be used to configure the system-wide default,
+and `--cache-dir`/`BKT_CACHE_DIR` used to override it.
+
+`bkt` periodically prunes stale data from its cache, but it also assumes the
+operating system will empty its temporary storage from time to time (for `/tmp`
+this typically happens on reboot). If you opt to use a directory that the
+system does not maintain, such as `~/.cache`, you may want to manually delete
+the cache directory on occasion, such as when upgrading `bkt`.
 
 ## Security and Privacy
 
@@ -206,7 +216,7 @@ directory is created with `700` permissions, meaning only the current user can
 access it, but this is not foolproof.
 
 You can customize the cache directory (see [above](#cache_dir)) to a location
-you trust such as `~/.bkt`, but note that your home directory may be slower than
+you trust such as `~/.cache`, but note that your home directory may be slower than
 the temporary directory selected by default.
 
 In general, if you are not the only user of your system it's wise to configure

src/lib.rs

@@ -999,6 +999,24 @@ impl CacheStatus {
     fn is_miss(&self) -> bool { match self { CacheStatus::Hit(_) => false, CacheStatus::Miss(_) => true, } }
 }
 
+/// Returns, if available on this platform, an identifier that uniquely represents the current user.
+///
+/// This value is only used to disambiguate cache directories in order to support multiple users.
+/// It should not be used to authenticate or validate a caller has access to a given cache entry,
+/// OS-level mechanisms such as directory permissions must be used instead.
+//
+// cfg() options drawn from the set of libc environments with a geteuid() function, see
+// https://github.com/search?q=repo%3Arust-lang%2Flibc+geteuid%28%29&type=code and
+// https://github.com/rust-lang/libc/blob/main/src/lib.rs
+#[cfg(any(unix, target_os = "fuchsia", target_os = "vxworks"))]
+fn user_id() -> Option<libc::uid_t> {
+    // SAFETY: geteuid is documented to "always [be] successful and never modify errno."
+    Some(unsafe { libc::geteuid() })
+}
+
+#[cfg(not(any(unix, target_os = "fuchsia", target_os = "vxworks")))]
+fn user_id() -> Option<u32> { None }
+
 /// This struct is the main API entry point for the `bkt` library, allowing callers to invoke and
 /// cache subprocesses for later reuse.
 ///
@@ -1046,8 +1064,9 @@ impl Bkt {
         // Note the cache is invalidated when the minor version changes
         // TODO use separate directories per user, like bash-cache
         //      See https://stackoverflow.com/q/57951893/113632
-        let cache_dir = root_dir
-            .join(format!("bkt-{}.{}-cache", env!("CARGO_PKG_VERSION_MAJOR"), env!("CARGO_PKG_VERSION_MINOR")));
+        let user_suffix = user_id().map(|id| format!("-u{}", id)).unwrap_or_else(String::new);
+        let dir_name = format!("bkt-{}.{}-cache{}", env!("CARGO_PKG_VERSION_MAJOR"), env!("CARGO_PKG_VERSION_MINOR"), user_suffix);
+        let cache_dir = root_dir.join(dir_name);
         Bkt::restrict_dir(&cache_dir)
             .with_context(|| format!("Failed to set permissions on {}", cache_dir.display()))?;
         Ok(Bkt {

tests/cli.rs

@@ -38,6 +38,18 @@ mod cli {
         bkt
     }
 
+    fn sudo(cmd: &mut Command) -> Command {
+        let mut sudo = Command::new("sudo");
+        sudo.args(&["-n", "-E"]).arg(cmd.get_program()).args(cmd.get_args());
+        for (key, value) in cmd.get_envs() {
+            match value {
+                Some(value) => sudo.env(key, value),
+                None => sudo.env_remove(key),
+            };
+        }
+        sudo
+    }
+
     #[derive(Eq, PartialEq, Debug)]
     struct CmdResult {
         out: String,
@@ -278,6 +290,43 @@ mod cli {
         assert_eq!(succeed(bkt(dir.path("cache")).args(args)), "1");
     }
 
+    // depends on sudo and libc::geteuid(), but also on Windows we don't split by user presently anyways
+    #[cfg(unix)]
+    #[test]
+    fn cache_dirs_multi_user() {
+        let dir = TestDir::temp();
+        let file = dir.path("file");
+        let args = ["--", "bash", "-c", COUNT_INVOCATIONS, "arg0", file.to_str().unwrap()];
+
+        // Skip the test if we can't run `sudo bkt --version`
+        // Calling into sudo like this isn't great, but it's an easy and reasonably reliable way to
+        // run bkt as two different users. It generally won't run on CI but at least it provides
+        // some manual test coverage.
+        if unsafe { libc::geteuid() } == 0 {
+            // https://github.com/rust-lang/rust/issues/68007 tracking skippable tests
+            eprint!("Running tests as root already, skipping");
+            return;
+        }
+        let mut sudo_bkt = sudo(bkt(dir.path("cache")).arg("--version"));
+        if run(&mut sudo_bkt).status.unwrap_or(127) != 0 {
+            // https://github.com/rust-lang/rust/issues/68007 tracking skippable tests
+            eprint!("Couldn't run `sudo bkt`, skipping");
+            return;
+        }
+
+        // can call bkt as both current and super-user
+        let user_call = succeed(bkt(dir.path("cache")).args(args));
+        assert_eq!(user_call, "1");
+
+        let sudo_call = succeed(&mut sudo(bkt(dir.path("cache")).args(args)));
+        assert_eq!(sudo_call, "2");
+
+        // cached separately
+        assert_eq!(user_call, succeed(bkt(dir.path("cache")).args(args)));
+
+        assert_eq!(sudo_call, succeed(&mut sudo(bkt(dir.path("cache")).args(args))));
+    }
+
     #[test]
     fn respects_cache_dir() {
         let dir = TestDir::temp();