Skip to content

gpasswd: added implementation of gpasswd - #282

Open
jlesage wants to merge 2 commits into
uutils:mainfrom
jlesage:implement-gpasswd
Open

gpasswd: added implementation of gpasswd#282
jlesage wants to merge 2 commits into
uutils:mainfrom
jlesage:implement-gpasswd

Conversation

@jlesage

@jlesage jlesage commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Added implementation of gpasswd.

Mostly done by AI :)

@pierre-warnier

Copy link
Copy Markdown
Collaborator

Thanks for this — it is a complete piece of work: the tool, a man page, unit
tests, deployment assertions and an entry in the GNU comparison suite. The
option grammar, the exclusivity rules and the -d-prints-then-fails quirk all
match what GNU actually does; I checked each against the packaged binary rather
than against my expectations, and found no behavioural disagreement.

Unfortunately main moved a long way under you between Thursday and Friday —
fourteen pull requests, several of which rewrote files you also touch — so this
no longer merges. None of that is your fault, and the single failing check
(pre-commit.ci) is reporting error during mergeable check, i.e. the
conflict, not your code.

I have rebased it and pushed the result to gpasswd-rebased in this
repository. Your commit is unchanged and still authored by you; everything I
did is a second commit on top. I could not push it to your fork directly:
GitHub refuses any push from this account that would create a workflow file,
and the rebase carries main's .github/workflows/. If you would rather keep
the PR as yours, git fetch upstream gpasswd-rebased and reset your branch
onto it; otherwise say the word and I will open it as a separate PR that closes
this one, with you credited.

What the rebase had to resolve

Four things worth your attention

1. gpasswd would have lost its privilege in the multicall build. You
correctly add it to SETUID_TOOLS, so make install marks it setuid. But
src/bin/shadow-rs.rs has a separate SETUID_APPLETS list — added in #239
and every applet not on it drops to the caller's uid before running. gpasswd
was not on it, so in the multicall layout a group administrator would have got
a permission error with nothing to explain it. It is on the list now, and the
test that pins the privilege model covers it.

2. Comments in /etc/group were being erased. The code reads with
read_group_file, which returns entries only, and writes them back. Every
comment, blank line and +/- NIS compatibility line in the file disappears
on the first gpasswd -a. This is #241, and the fix is the locked-file
transaction from #278/#279: LockedFile keeps the layout, and commit_all
writes group and gshadow together so a bad value cannot leave the pair
disagreeing. Ported.

3. println!/eprintln!. They panic when the stream is closed, which a
setuid-root tool must not do part way through changing an account file.
CONTRIBUTING says so but nothing enforced it — yours were the only five in
the repository. Replaced with writeln!, keeping GNU's choice of stream
(Adding user… on stdout). I also turned clippy::print_stdout and
print_stderr on workspace-wide so the next one is a build failure rather than
something a reviewer has to notice.

4. The duplicated EchoGuard and password reader. These matched newgrp's
copy, which is exactly right for when you wrote it — shadow_core::tty only
landed on Thursday. It now also blocks SIGINT/SIGQUIT/SIGTSTP for the
read, so Ctrl-C at the prompt cannot leave the terminal with echo off, and it
falls back to stderr/stdin where there is no controlling terminal.

Verified

Against a --prefix tree: -a adds and prints on stdout, an unknown user
gives exit 3 with nothing written, -d on a non-member prints then fails with
3, -M and -A write both files, -r clears the password, and the comment at
the top of /etc/group survives all of it. The four error exit codes match the
GNU binary, which the comparison suite now checks on every pull request.

make check              exit 0
GNU comparison          42 passed, 0 unexpected
Deployment suite        223 passed, 0 failed

One small thing for next time, and only because it cost me a minute: the
project asks for Fixes #N and a branch named feat/N-…, which means opening
an issue first. Not worth redoing here.

jlesage and others added 2 commits September 5, 2026 15:16
Rebases jlesage's implementation onto a main that moved a long way under
it, and carries it onto the interfaces that landed since the branch was
cut. The tool's behaviour is unchanged and still matches GNU on every
path probed.

What the rebase had to resolve:

  - shadow-rs.spec and debian/ were revived by the merge; both were
    deleted because neither could build.
  - The standalone dispatch() and hand-written --list predate the single
    applets() table, which now drives both. gpasswd joins the table.
  - The five shadow-core parser features it asked for no longer exist.
  - The fourteen [[test]] targets are one; test_gpasswd joins it.
  - README, Makefile and PLATFORM-SUPPORT described the pre-uutils#239
    privilege model, in which every applet ran with euid 0.

What changed beyond the rebase:

  - gpasswd keeps its privilege in the multicall build. It is setuid in
    GNU and the branch marks it setuid for the per-tool install, but
    SETUID_APPLETS still listed four names, so the multicall build would
    have dropped privilege and no group administrator could have used it
    -- silently.
  - Comments in /etc/group survive. It read entries only and wrote them
    back, which erases every comment, blank line and NIS compatibility
    line in the file. The locked-file transaction keeps them, and commits
    group and gshadow together so a bad value cannot leave the pair
    disagreeing.
  - The duplicated EchoGuard and password reader are gone, replaced by
    shadow_core::tty, which blocks interrupts for the read so Ctrl-C at
    the prompt cannot leave the terminal with echo off.
  - The print macros are replaced. They panic when the stream is closed,
    which a setuid-root tool must not do part way through a change.
    These were the only five in the repository and no lint caught them,
    so clippy::print_stdout and print_stderr are now denied
    workspace-wide, with the test crate exempt.

The GNU comparison suite gains gpasswd: the unknown-group, unknown-user
and non-member exit codes, all three of which match.

CONTRIBUTING's licence list also said ISC, BSD-2-Clause, CC0-1.0, Zlib
and MPL-2.0 were acceptable, which stopped being true when deny.toml was
narrowed to the graph. It now says what deny.toml enforces.
@jlesage

jlesage commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

I adjusted the PR. Everything should be good now :)

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.

2 participants