One could argue this is a bug; but if not, then move to a discussion. Currently util::delete_file ignores all errors from std::fs::remove_dir; however as the comment states, only Errors whose ErrorKind is ErrorKind::DirectoryNotEmpty should be ignored.
Fixes:
Use nightly and enable the io_error_morefeature, then explicitly check the returned ErrorKind and only ignore when it is DirectoryNotEmpty.
Depending on the choice of an optimistic vs. pessimistic strategy, either first check the directory is empty before calling or check that it is not empty after calling and erroring (e.g., if fs::read_dir(path)?.next().is_none() { fs::remove_dir(path) } else { Ok(()) }).
Obviously only the nightly strategy avoids a TOCTOU race condition/attack, but I understand wanting to remain on stable.
Originally created by @zacknewman on GitHub (Nov 17, 2023).
Original GitHub issue: https://github.com/dani-garcia/vaultwarden/issues/4081
One could argue this is a bug; but if not, then move to a discussion. Currently [`util::delete_file`](https://github.com/dani-garcia/vaultwarden/blob/main/src/util.rs#L365) ignores _all_ errors from `std::fs::remove_dir`; however as the comment states, only `Error`s whose `ErrorKind` is `ErrorKind::DirectoryNotEmpty` should be ignored.
Fixes:
* Use `nightly` and enable the `io_error_more` `feature`, then explicitly check the returned `ErrorKind` and only ignore when it is `DirectoryNotEmpty`.
* Depending on the choice of an optimistic vs. pessimistic strategy, either first check the directory is empty before calling or check that it is not empty after calling and erroring (e.g., `if fs::read_dir(path)?.next().is_none() { fs::remove_dir(path) } else { Ok(()) }`).
Obviously only the `nightly` strategy avoids a TOCTOU race condition/attack, but I understand wanting to remain on `stable`.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Originally created by @zacknewman on GitHub (Nov 17, 2023).
Original GitHub issue: https://github.com/dani-garcia/vaultwarden/issues/4081
One could argue this is a bug; but if not, then move to a discussion. Currently
util::delete_fileignores all errors fromstd::fs::remove_dir; however as the comment states, onlyErrors whoseErrorKindisErrorKind::DirectoryNotEmptyshould be ignored.Fixes:
nightlyand enable theio_error_morefeature, then explicitly check the returnedErrorKindand only ignore when it isDirectoryNotEmpty.if fs::read_dir(path)?.next().is_none() { fs::remove_dir(path) } else { Ok(()) }).Obviously only the
nightlystrategy avoids a TOCTOU race condition/attack, but I understand wanting to remain onstable.