Skip to content

Add support for filtering orphaned files with --owner orphan #1841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
devmanuelli wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add support for filtering orphaned files with --owner orphan #1841

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d33fbbc
Add support for filtering orphaned files with --owner orphan
devmanuelli Nov 19, 2025
cd9fc62
Address review feedback: improve design and add caching
devmanuelli Nov 20, 2025
cd6b72d
Fix clippy warning: remove PartialEq from Check to avoid fn pointer c…
devmanuelli Nov 24, 2025
763e885
Use thread-local cache with HashMap for uid/gid lookups
devmanuelli Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,11 +441,14 @@ pub struct Opts {
/// Filter files by their user and/or group.
/// Format: [(user|uid)][:(group|gid)]. Either side is optional.
/// Precede either side with a '!' to exclude files instead.
/// Use 'orphan' to match files with no valid user/group.
Copy link
Copy Markdown
Collaborator

@tmccombs tmccombs Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we move forward with this, you'll also need to update the man page and CHANGELOG

///
/// Examples:
/// {n} --owner john
/// {n} --owner :students
/// {n} --owner '!john:students'
/// {n} --owner orphan
/// {n} --owner :orphan
#[cfg(unix)]
#[arg(long, short = 'o', value_parser = OwnerFilter::from_string, value_name = "user:group",
help = "Filter by owning user and/or group",
Expand Down
19 changes: 18 additions & 1 deletion src/filter/owner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ enum Check<T> {
Equal(T),
NotEq(T),
Ignore,
Orphaned,
}

impl OwnerFilter {
Expand Down Expand Up @@ -69,7 +70,17 @@ impl OwnerFilter {
pub fn matches(&self, md: &fs::Metadata) -> bool {
use std::os::unix::fs::MetadataExt;

self.uid.check(md.uid()) && self.gid.check(md.gid())
let uid_match = match self.uid {
Check::Orphaned => User::from_uid(md.uid().into()).ok().flatten().is_none(),
Copy link
Copy Markdown
Collaborator

@tmccombs tmccombs Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth using a cache, so we don't have to do a lookup for every single file. We could probably even use a Set of uids known not to be orphaned.

_ => self.uid.check(md.uid()),
};

let gid_match = match self.gid {
Check::Orphaned => Group::from_gid(md.gid().into()).ok().flatten().is_none(),
_ => self.gid.check(md.gid()),
};

uid_match && gid_match
}
}

Expand All @@ -79,6 +90,7 @@ impl<T: PartialEq> Check<T> {
Check::Equal(x) => v == *x,
Check::NotEq(x) => v != *x,
Check::Ignore => true,
Check::Orphaned => unreachable!("Orphaned check handled in OwnerFilter::matches"),
Copy link
Copy Markdown
Collaborator

@tmccombs tmccombs Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like how the caller is responsible for handling this separately. It would be nice if we could just include a Set of allowed ids in the Orphaned vairant, but I think on some systems, the user ids could be dynamic, and reading from /etc/passwd isn't sufficient.

Another option could be to have the Orphaned variant include a function to check if an id exists, or even just a value indicating if it is for user ids or group ids.

}
}

Expand All @@ -88,6 +100,7 @@ impl<T: PartialEq> Check<T> {
{
let (s, equality) = match s {
Some("") | None => return Ok(Check::Ignore),
Some("orphan") => return Ok(Check::Orphaned),
Some(s) if s.starts_with('!') => (&s[1..], false),
Some(s) => (s, true),
};
Expand Down Expand Up @@ -134,6 +147,10 @@ mod owner_parsing {
both_negate:"!4:!3" => Ok(OwnerFilter { uid: NotEq(4), gid: NotEq(3) }),
uid_not_gid:"6:!8" => Ok(OwnerFilter { uid: Equal(6), gid: NotEq(8) }),

orphan_uid: "orphan" => Ok(OwnerFilter { uid: Orphaned, gid: Ignore }),
Copy link
Copy Markdown
Collaborator

@tmccombs tmccombs Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible that a user or group is actually named "orphan". I think it would probably be better to use a a different option, or maybe use something that isn't a valid username like "-".

orphan_gid: ":orphan" => Ok(OwnerFilter { uid: Ignore, gid: Orphaned }),
orphan_both:"orphan:orphan"=> Ok(OwnerFilter { uid: Orphaned, gid: Orphaned }),

more_colons:"3:5:" => Err(_),
only_colons:"::" => Err(_),
}
Expand Down