From 8e7fb99111f2dc9710ae4533f47341138bd58ade Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 10 Jan 2025 05:22:50 -0500 Subject: [PATCH 1/7] feat: Add `gix_testtools::umask`, safe but only meant for tests This implements a function for tests to safely read the current process umask without the usual race condition of doing so, at the expense of using subprocesses to do it. This just calls a shell and runs `umask` from it (which is expected to be a builtin and, on many systems, is available as a builtin but not an executable). Even though this is safe, including thread-safe, it is unlikely to be suitable for use outside of tests, because of its use of `expect` and assertions when there are errors, combined with the possibly slow speed of using subprocesses. Given that this is effecitvely running a tiny shell script to do the work, why is it not instead a fixture script that is named in a `.gitignore` file so that it is not tracked? The reason is that the outcomes of running such fixture scripts are still saved across separate test runs, but it is useful to be able to run the tests with differnt umasks, e.g. `(umask 077; cargo nextest run ...)`. The immediate purpose is in forthcoming tests that, when checkout sets +x on an existing file, it doesn't set excessive permissions. The fix to pass such a test is not currently planned to use the umask explicitly. But the tests will use it, at least to detect when they cannot really verify the code under test on the grounds that they are running with an excessively permissive umask that doesn't allow behavior that only occurs with a generally reasonable umask to be observed. --- tests/tools/src/lib.rs | 13 +++++++++++++ tests/tools/src/main.rs | 9 +++++++++ tests/tools/tests/umask.rs | 20 ++++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 tests/tools/tests/umask.rs diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index e3ce748cc5c..1581afa39e7 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -944,6 +944,19 @@ pub fn size_ok(actual_size: usize, expected_64_bit_size: usize) -> bool { return actual_size <= expected_64_bit_size; } +/// Get the umask in a way that is safe, but may be too slow for use outside of tests. +#[cfg(unix)] +pub fn umask() -> u32 { + let output = std::process::Command::new("/bin/sh") + .args(["-c", "umask"]) + .output() + .expect("can execute `sh -c umask`"); + assert!(output.status.success(), "`sh -c umask` failed"); + assert!(output.stderr.is_empty(), "`sh -c umask` unexpected message"); + let text = output.stdout.trim_ascii().to_str().expect("valid Unicode"); + u32::from_str_radix(text, 8).expect("parses as octal number") +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/tools/src/main.rs b/tests/tools/src/main.rs index bfe68bbe59c..99d944e32af 100644 --- a/tests/tools/src/main.rs +++ b/tests/tools/src/main.rs @@ -1,4 +1,5 @@ use std::{fs, io, io::prelude::*, path::PathBuf}; + fn mess_in_the_middle(path: PathBuf) -> io::Result<()> { let mut file = fs::OpenOptions::new().read(false).write(true).open(path)?; file.seek(io::SeekFrom::Start(file.metadata()?.len() / 2))?; @@ -6,11 +7,19 @@ fn mess_in_the_middle(path: PathBuf) -> io::Result<()> { Ok(()) } +#[cfg(unix)] +fn umask() -> io::Result<()> { + println!("{:04o}", gix_testtools::umask()); + Ok(()) +} + fn main() -> Result<(), Box> { let mut args = std::env::args().skip(1); let scmd = args.next().expect("sub command"); match &*scmd { "mess-in-the-middle" => mess_in_the_middle(PathBuf::from(args.next().expect("path to file to mess with")))?, + #[cfg(unix)] + "umask" => umask()?, _ => unreachable!("Unknown subcommand: {}", scmd), }; Ok(()) diff --git a/tests/tools/tests/umask.rs b/tests/tools/tests/umask.rs new file mode 100644 index 00000000000..88226de3d8c --- /dev/null +++ b/tests/tools/tests/umask.rs @@ -0,0 +1,20 @@ +use std::fs::File; +use std::io::{BufRead, BufReader}; + +use bstr::ByteSlice; + +#[test] +#[cfg(unix)] +#[cfg_attr(not(target_os = "linux"), ignore = "The test itself uses /proc")] +fn umask() { + // Check against the umask obtained via a less portable but also completely safe method. + let less_portable = BufReader::new(File::open("/proc/self/status").expect("can open")) + .split(b'\n') + .find_map(|line| line.expect("can read").strip_prefix(b"Umask:\t").map(ToOwned::to_owned)) + .expect("has umask line") + .to_str() + .expect("umask line is valid UTF-8") + .to_owned(); + let more_portable = format!("{:04o}", gix_testtools::umask()); + assert_eq!(more_portable, less_portable); +} From 4d4bf892e50f6b1bc9e5a22e5d8c69698ba2b38e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 11 Jan 2025 06:45:43 -0500 Subject: [PATCH 2/7] Test for excessive write permissions on nonexclusive checkout This expands the `overwriting_files_and_lone_directories_works` test function to check not only that executable permissions have been added at least for the owner, but that write permissions are not added anywhere they would be excessive. The write permissions bits of the current test runner process umask is used to gauge this. This means we are expecting different permissions in different environments. `(umask ...; cargo nextest run ...)` can be used to try it with different umasks if desired, but this should not typically be necessary. Possible under-restrictive umasks could make the test pass even if the underlying bug is not fixed; this is avoided by also testing that the umask is sufficient to facilitate the test. (This is really why the test accesses the umask in the first place: in environments where files would automatically be created with completely unrestricted permissions, the expected behavior of the code under test may be to do that, but running the tests in such an environment is insufficient to check if the bug is fixed.) --- gix-worktree-state/tests/state/checkout.rs | 27 ++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/gix-worktree-state/tests/state/checkout.rs b/gix-worktree-state/tests/state/checkout.rs index c496d04688b..f69a5d748f1 100644 --- a/gix-worktree-state/tests/state/checkout.rs +++ b/gix-worktree-state/tests/state/checkout.rs @@ -220,9 +220,32 @@ fn overwriting_files_and_lone_directories_works() -> crate::Result { let meta = std::fs::symlink_metadata(exe)?; assert!(meta.is_file()); + #[cfg(unix)] if opts.fs.executable_bit { - #[cfg(unix)] - assert_eq!(meta.mode() & 0o700, 0o700, "the executable bit is set where supported"); + let mode = meta.mode(); + assert_eq!( + mode & 0o100, + 0o100, + "executable bit set where supported ({:04o} & {:04o} = {:04o} should be {:04o})", + mode, + 0o100, + mode & 0o100, + 0o100 + ); + let umask_write = gix_testtools::umask() & 0o222; + assert_eq!( + mode & umask_write, + 0, + "no excessive write bits are set ({:04o} & {:04o} = {:04o} should be {:04o})", + mode, + umask_write, + mode & umask_write, + 0 + ); + assert_ne!( + umask_write, 0, + "test not meaningful unless runner umask restricts some writes" + ); } assert_eq!( From 879d29deb6776e9dd9de2df4fc10bfccfb1292e7 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 10 Jan 2025 04:05:00 -0500 Subject: [PATCH 3/7] fix: Don't set extra permissions with +x on existing file This fixes a bug that occurred when a regular file tracked as executable was checked out in a non-exclusive checkout on a Unix-like system (where the destination is a filesystem that supports Unix-style executable permissions). This occurs when `destination_is_initially_empty: true` is not explicitly set. Whether the file existed before or not, it is created without attempting to set its permissions to include executable bits, then the executable bits are added afterwards. On Unix-like systems, the file was given unrestricted 0777 permissions and was thus world writable. This happened regardless of the process umask or any other contextual factors. Although `0o777` is given as the mode both when permissions are set on creation (using `OpenOptionsExt::mode` and `OpenOptions::open`, which delegates to `open`), and when they are set after creation (using `PermissionsExt::set_mode` and `std::fs::set_permissions`, which delegates to `chmod`), the cases do not treat their mode arguments equivalently. - The first situation worked correctly because `open` automatically respects the current process umask. (The system takes care of this, as it is part of the semantics of creating a new file and specifying desired permissions.) So 777 was really expressing the idea of maximal permissions of those considered safe under the current configuration, including executable permissions. - But the second situation did not work correctly, because `chmod` calls try to set the exact permissions specified (and usually succeed). Unlike `open`, with `chmod` there is no implicit use of the umask. Various fixes are possible. The fix implemented here hews to the existing design as much as possible while avoiding setting any write permissions (though existing write permissions are preserved) and also avoiding setting executable permissions for whoever does not have read permissions. We: 1. Unset the setuid, setgid, and sticky bits, in the rare case that any of them are set. Keeping them could be unsafe or have unexpected effects when set for a file that may conceptually hold different data or serve a different purpose (since this is a checkout). For the setuid and setgid bits, it would be unsafe to keep them when adding new executable permissions. The intent of setuid and setgit bits is ambiguous in this situation, since they may have been meant only to apply to an earlier version of the file, especially since users may expect the file to be deleted and a new file of the same name to be created, rather than to confer extra abilities when executable bits are added in the future. Unsetting them makes adding executable bits where read bits are already present (which we will do) a reasonably safe operation. In the case of the setgid bit, another reason to remove it is that, on some systems, the setgid bit in the absence of any executable bits has the different meaning of enabling mandatory file locking. If the setgid bit was set for this purpose, then the effect of setting the EGID and potentialy elevating the privileges of the user who runs it is surely not intended. 2. Check which read bits (for owner, group, and other) are already set on the file. We do this only by looking at the mode. For example, ACLs do not participate. 3. Set executable bits corresponding to the preexisting read bits. That is, for each of the owner, group, and others, if it can read (according to the file mode), set it to be able to execute as well. In some cases, this may have a different effect from what would happen if the file were created anew with the desired permissions specified by a broad mode (e.g. 777) and subject to the umask. This is because it is possible to have a umask that limits read and execute permissions differently. Also, the file may have had its permissions modified in some other way since creation. The idea here is to keep the idea behind the way it worked before, but avoid adding any write permissions or giving permissions to users who don't already have any. This fixes the bug where executable files were sometimes checked out with unrestricted, world-writable permissions. However, this is not necessarily the approach that will be kept long-term. This does not attempt to avoid effects that are fundamental to the reuse of an existing file (versus the creation of a new one). In particular, this currently assumes that observing changes due to a checkout through other hard links to a file (whose link count is higher than 1) is an intended or otherwise acceptable effect of using multiple hard links. Another aspect of the current approach that is preserved so far but that may eventually change is that some operations are done through an open file object while others are done using the path, and there may be unusual situations, perhaps involving long-running process smudge filters and separate concurrent modification of the working tree, where they diverge. However, the specific scenario of path coming to refer to something that is no longer a regular file will be covered in a subsequent commit. --- gix-worktree-state/src/checkout/entry.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gix-worktree-state/src/checkout/entry.rs b/gix-worktree-state/src/checkout/entry.rs index bf37fd9aad2..0912463ec61 100644 --- a/gix-worktree-state/src/checkout/entry.rs +++ b/gix-worktree-state/src/checkout/entry.rs @@ -288,7 +288,10 @@ pub(crate) fn finalize_entry( if let Some(path) = set_executable_after_creation { use std::os::unix::fs::PermissionsExt; let mut perm = std::fs::symlink_metadata(path)?.permissions(); - perm.set_mode(0o777); + let mut mode = perm.mode(); + mode &= 0o777; // Clear non-rwx bits (setuid, setgid, sticky). + mode |= (mode & 0o444) >> 2; // Let readers also execute. + perm.set_mode(mode); std::fs::set_permissions(path, perm)?; } // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. From bf33d2b49212f71a7af1aef34e0ffac41140815b Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 10 Jan 2025 17:41:52 -0500 Subject: [PATCH 4/7] Extract and test +x mode setting logic --- gix-worktree-state/src/checkout/entry.rs | 81 ++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 6 deletions(-) diff --git a/gix-worktree-state/src/checkout/entry.rs b/gix-worktree-state/src/checkout/entry.rs index 0912463ec61..45be6dec353 100644 --- a/gix-worktree-state/src/checkout/entry.rs +++ b/gix-worktree-state/src/checkout/entry.rs @@ -1,5 +1,5 @@ -use std::borrow::Cow; use std::{ + borrow::Cow, fs::OpenOptions, io::Write, path::{Path, PathBuf}, @@ -286,12 +286,8 @@ pub(crate) fn finalize_entry( // For possibly existing, overwritten files, we must change the file mode explicitly. #[cfg(unix)] if let Some(path) = set_executable_after_creation { - use std::os::unix::fs::PermissionsExt; let mut perm = std::fs::symlink_metadata(path)?.permissions(); - let mut mode = perm.mode(); - mode &= 0o777; // Clear non-rwx bits (setuid, setgid, sticky). - mode |= (mode & 0o444) >> 2; // Let readers also execute. - perm.set_mode(mode); + set_mode_executable(&mut perm); std::fs::set_permissions(path, perm)?; } // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. @@ -300,3 +296,76 @@ pub(crate) fn finalize_entry( file.close()?; Ok(()) } + +#[cfg(unix)] +fn set_mode_executable(perm: &mut std::fs::Permissions) { + use std::os::unix::fs::PermissionsExt; + let mut mode = perm.mode(); + mode &= 0o777; // Clear non-rwx bits (setuid, setgid, sticky). + mode |= (mode & 0o444) >> 2; // Let readers also execute. + perm.set_mode(mode); +} + +#[cfg(test)] +mod tests { + #[test] + #[cfg(unix)] + fn set_mode_executable() { + let cases = [ + // Common cases. + (0o100755, 0o755), + (0o100644, 0o755), + (0o100750, 0o750), + (0o100640, 0o750), + (0o100700, 0o700), + (0o100600, 0o700), + (0o100775, 0o775), + (0o100664, 0o775), + (0o100770, 0o770), + (0o100660, 0o770), + (0o100764, 0o775), + (0o100760, 0o770), + // Some less common cases. + (0o100674, 0o775), + (0o100670, 0o770), + (0o100000, 0o000), + (0o100400, 0o500), + (0o100440, 0o550), + (0o100444, 0o555), + (0o100462, 0o572), + (0o100242, 0o252), + (0o100167, 0o177), + // Some cases with set-user-ID, set-group-ID, and sticky bits. + (0o104755, 0o755), + (0o104644, 0o755), + (0o102755, 0o755), + (0o102644, 0o755), + (0o101755, 0o755), + (0o101644, 0o755), + (0o106755, 0o755), + (0o106644, 0o755), + (0o104750, 0o750), + (0o104640, 0o750), + (0o102750, 0o750), + (0o102640, 0o750), + (0o101750, 0o750), + (0o101640, 0o750), + (0o106750, 0o750), + (0o106640, 0o750), + (0o107644, 0o755), + (0o107000, 0o000), + (0o106400, 0o500), + (0o102462, 0o572), + ]; + for (old, expected) in cases { + use std::os::unix::fs::PermissionsExt; + let mut perm = std::fs::Permissions::from_mode(old); + super::set_mode_executable(&mut perm); + let actual = perm.mode(); + assert_eq!( + actual, expected, + "{old:06o} should become {expected:04o} but became {actual:04o}" + ); + } + } +} From be0abafe883a09491762b496950089927264af8c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 10 Jan 2025 18:52:11 -0500 Subject: [PATCH 5/7] fix: Don't attempt +x if it was replaced by a non-regular file This does not make a difference in typical cases, and anytime it matters something has probably gone unexpectedly, but this narrows the window of time during which a race condition could occur where a regular file has been replaced with something else at the same path (e.g. a directory) by some other process. An example of why it's valuable to avoid this is that if the entry is a directory then executable permissions have a different meaning and adding them could increase access unintentionally. Likewise, when we set executable permissions we remove setuid, setgid, and sticky bits, which also have different meanings for directories; in particular, on a directory the sticky bit restricts deletion. (This also automatically avoids some problems in the case of `finalize_entry` being called with a path to set executable that was never a regular file in the first place. That should not happen, though, and allowing that is not a goal of this change.) --- gix-worktree-state/src/checkout/entry.rs | 142 ++++++++++++++--------- 1 file changed, 85 insertions(+), 57 deletions(-) diff --git a/gix-worktree-state/src/checkout/entry.rs b/gix-worktree-state/src/checkout/entry.rs index 45be6dec353..7e84b93a74c 100644 --- a/gix-worktree-state/src/checkout/entry.rs +++ b/gix-worktree-state/src/checkout/entry.rs @@ -1,6 +1,6 @@ use std::{ borrow::Cow, - fs::OpenOptions, + fs::{OpenOptions, Permissions}, io::Write, path::{Path, PathBuf}, }; @@ -286,9 +286,10 @@ pub(crate) fn finalize_entry( // For possibly existing, overwritten files, we must change the file mode explicitly. #[cfg(unix)] if let Some(path) = set_executable_after_creation { - let mut perm = std::fs::symlink_metadata(path)?.permissions(); - set_mode_executable(&mut perm); - std::fs::set_permissions(path, perm)?; + let old_perm = std::fs::symlink_metadata(path)?.permissions(); + if let Some(new_perm) = set_mode_executable(old_perm) { + std::fs::set_permissions(path, new_perm)?; + } } // NOTE: we don't call `file.sync_all()` here knowing that some filesystems don't handle this well. // revisit this once there is a bug to fix. @@ -298,73 +299,100 @@ pub(crate) fn finalize_entry( } #[cfg(unix)] -fn set_mode_executable(perm: &mut std::fs::Permissions) { +fn set_mode_executable(mut perm: Permissions) -> Option { use std::os::unix::fs::PermissionsExt; let mut mode = perm.mode(); + if mode & 0o170000 != 0o100000 { + return None; // Stop if we don't have a regular file anymore. + } mode &= 0o777; // Clear non-rwx bits (setuid, setgid, sticky). mode |= (mode & 0o444) >> 2; // Let readers also execute. perm.set_mode(mode); + Some(perm) } -#[cfg(test)] +#[cfg(all(test, unix))] mod tests { + fn pretty(maybe_mode: Option) -> String { + match maybe_mode { + Some(mode) => format!("Some({mode:04o})"), + None => "None".into(), + } + } + #[test] - #[cfg(unix)] fn set_mode_executable() { let cases = [ - // Common cases. - (0o100755, 0o755), - (0o100644, 0o755), - (0o100750, 0o750), - (0o100640, 0o750), - (0o100700, 0o700), - (0o100600, 0o700), - (0o100775, 0o775), - (0o100664, 0o775), - (0o100770, 0o770), - (0o100660, 0o770), - (0o100764, 0o775), - (0o100760, 0o770), - // Some less common cases. - (0o100674, 0o775), - (0o100670, 0o770), - (0o100000, 0o000), - (0o100400, 0o500), - (0o100440, 0o550), - (0o100444, 0o555), - (0o100462, 0o572), - (0o100242, 0o252), - (0o100167, 0o177), - // Some cases with set-user-ID, set-group-ID, and sticky bits. - (0o104755, 0o755), - (0o104644, 0o755), - (0o102755, 0o755), - (0o102644, 0o755), - (0o101755, 0o755), - (0o101644, 0o755), - (0o106755, 0o755), - (0o106644, 0o755), - (0o104750, 0o750), - (0o104640, 0o750), - (0o102750, 0o750), - (0o102640, 0o750), - (0o101750, 0o750), - (0o101640, 0o750), - (0o106750, 0o750), - (0o106640, 0o750), - (0o107644, 0o755), - (0o107000, 0o000), - (0o106400, 0o500), - (0o102462, 0o572), + // Common cases: + (0o100755, Some(0o755)), + (0o100644, Some(0o755)), + (0o100750, Some(0o750)), + (0o100640, Some(0o750)), + (0o100700, Some(0o700)), + (0o100600, Some(0o700)), + (0o100775, Some(0o775)), + (0o100664, Some(0o775)), + (0o100770, Some(0o770)), + (0o100660, Some(0o770)), + (0o100764, Some(0o775)), + (0o100760, Some(0o770)), + // Less common: + (0o100674, Some(0o775)), + (0o100670, Some(0o770)), + (0o100000, Some(0o000)), + (0o100400, Some(0o500)), + (0o100440, Some(0o550)), + (0o100444, Some(0o555)), + (0o100462, Some(0o572)), + (0o100242, Some(0o252)), + (0o100167, Some(0o177)), + // With set-user-ID, set-group-ID, and sticky bits: + (0o104755, Some(0o755)), + (0o104644, Some(0o755)), + (0o102755, Some(0o755)), + (0o102644, Some(0o755)), + (0o101755, Some(0o755)), + (0o101644, Some(0o755)), + (0o106755, Some(0o755)), + (0o106644, Some(0o755)), + (0o104750, Some(0o750)), + (0o104640, Some(0o750)), + (0o102750, Some(0o750)), + (0o102640, Some(0o750)), + (0o101750, Some(0o750)), + (0o101640, Some(0o750)), + (0o106750, Some(0o750)), + (0o106640, Some(0o750)), + (0o107644, Some(0o755)), + (0o107000, Some(0o000)), + (0o106400, Some(0o500)), + (0o102462, Some(0o572)), + // Where it was replaced with a directory due to a race: + (0o040755, None), + (0o040644, None), + (0o040600, None), + (0o041755, None), + (0o041644, None), + (0o046644, None), + // Where it was replaced with a symlink due to a race: + (0o120777, None), + (0o120644, None), + // Where it was replaced with some other non-regular file due to a race: + (0o140644, None), + (0o060644, None), + (0o020644, None), + (0o010644, None), ]; - for (old, expected) in cases { + for (old_mode, expected) in cases { use std::os::unix::fs::PermissionsExt; - let mut perm = std::fs::Permissions::from_mode(old); - super::set_mode_executable(&mut perm); - let actual = perm.mode(); + let old_perm = std::fs::Permissions::from_mode(old_mode); + let actual = super::set_mode_executable(old_perm).map(|perm| perm.mode()); assert_eq!( - actual, expected, - "{old:06o} should become {expected:04o} but became {actual:04o}" + actual, + expected, + "{old_mode:06o} should become {}, became {}", + pretty(expected), + pretty(actual) ); } } From 61174e585304ff34536c7ec5f325b734e3822161 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 12 Jan 2025 16:10:38 -0500 Subject: [PATCH 6/7] Fix an MSRV incompatibility Trimming Unicode whitespace is not equivalent to trimming ASCII whitespace, but in this case I think they are equally okay to do. If we get Unicode whitespace that is not ASCII whitespace from running `umask`, in principle it's possible this is due to it being a misinterpretation of something not Unicode. But if `umask` gives us anything besides whitespace and a single nonempty sequence of octal digits, then (a) that's very strange, and (b) it should fail and indicate its failure with a nonzero exit status, which we check. --- tests/tools/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index 1581afa39e7..cb91c0b87c6 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -953,7 +953,7 @@ pub fn umask() -> u32 { .expect("can execute `sh -c umask`"); assert!(output.status.success(), "`sh -c umask` failed"); assert!(output.stderr.is_empty(), "`sh -c umask` unexpected message"); - let text = output.stdout.trim_ascii().to_str().expect("valid Unicode"); + let text = output.stdout.to_str().expect("valid Unicode").trim(); u32::from_str_radix(text, 8).expect("parses as octal number") } From 4d5e656fc0103e11ac2ed64305dd2430c6ed4648 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 13 Jan 2025 07:06:05 +0100 Subject: [PATCH 7/7] refactor - try to avoid warnings when compiled on Windows --- tests/tools/tests/umask.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/tools/tests/umask.rs b/tests/tools/tests/umask.rs index 88226de3d8c..0a4cc04250b 100644 --- a/tests/tools/tests/umask.rs +++ b/tests/tools/tests/umask.rs @@ -1,12 +1,11 @@ -use std::fs::File; -use std::io::{BufRead, BufReader}; - -use bstr::ByteSlice; - #[test] #[cfg(unix)] #[cfg_attr(not(target_os = "linux"), ignore = "The test itself uses /proc")] fn umask() { + use std::fs::File; + use std::io::{BufRead, BufReader}; + + use bstr::ByteSlice; // Check against the umask obtained via a less portable but also completely safe method. let less_portable = BufReader::new(File::open("/proc/self/status").expect("can open")) .split(b'\n')