Skip to content

id: Fix issues with group id handling described in #10006#10706

Open
denendaden wants to merge 2 commits into
uutils:mainfrom
denendaden:iss10006
Open

id: Fix issues with group id handling described in #10006#10706
denendaden wants to merge 2 commits into
uutils:mainfrom
denendaden:iss10006

Conversation

@denendaden

Copy link
Copy Markdown
Contributor

The uutils id previously exhibited different behavior from the GNU id, as shown in the following example:

$ sudo setpriv --reuid 3000 --egid 2000 --clear-groups target/release/id
uid=3000(u3000) gid=0(root) egid=2000(u2000) groups=0(root)
$ sudo setpriv --reuid 3000 --egid 2000 --clear-groups id
uid=3000(u3000) gid=0(root) egid=2000(u2000) groups=2000(u2000)

This discrepancy is because GNU id forms its list of groups by combining the effective group ID with a list of supplementary group IDs, whereas uutils id was, except when certain flags were provided, using the real group ID in place of the effective group ID. This pull request changes this behavior by passing None to get_groups_gnu, so that it uses the effective group ID instead of the group ID being passed to it, matching the GNU behavior.

Changes to numfmt were unrelated but necessary to get it to compile. I was not sure what to do about this.

Fixes #10006

@github-actions

github-actions Bot commented Feb 4, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)

@ChrisDryden

Copy link
Copy Markdown
Collaborator

Would it be possible to make an integration test for this issue to make sure we don't regress?

Comment thread src/uu/numfmt/src/numfmt.rs Outdated
}

pub fn uu_app() -> Command {
Command::new(util_name())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This code change of the numfmt linting should no longer be needed after rebasing

@sylvestre sylvestre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The id fix looks correct — using None to get the effective GID matches GNU.

However, the numfmt changes (replacing util_name() with uucore::util_name()) seem unrelated. Were these needed to fix a build issue? If so, they should probably be in a separate commit.

@sylvestre

Copy link
Copy Markdown
Contributor

and needs to be rebased

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/unexpand/bounded-memory is now being skipped but was previously passing.
Congrats! The gnu test tests/seq/seq-epipe is now passing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Various problems in different id handling in id command

3 participants