Skip to content

common: warn when become_user() leaves root credentials - #3609

Open
user01010111 wants to merge 2 commits into
networkupstools:masterfrom
user01010111:fix/issue-3471-root-warning
Open

common: warn when become_user() leaves root credentials#3609
user01010111 wants to merge 2 commits into
networkupstools:masterfrom
user01010111:fix/issue-3471-root-warning

Conversation

@user01010111

Copy link
Copy Markdown

Aims to close #3471, pending maintainer review and approval.

This makes the shared POSIX become_user() path warn once when a daemon's real or effective UID remains zero after a successful credential transition.

The previous code reported the resulting UID only in a debug-level success message after initgroups(), setgid() and setuid(), and did not warn when the resulting credentials remained root. Checking both resulting UIDs in the shared helper gives daemon callers one consistent invariant without duplicating warnings. Non-root starts retain their existing behaviour, and the intentionally privileged upsmon -p path retains its existing warning without receiving another one.

Validation:

  • GCC 16.2.1 built common.o, upslog, upsmon, upsd and dummy-ups successfully. Clang 22.1.8 built the same targets with -Weverything -Werror.
  • Before the change, a disposable root namespace running upslog -u root completed with no root warning. The candidate emitted exactly one Warning: running as root (UID=0 EUID=0).
  • The equivalent non-root run emitted no root warning. A bounded upsmon -p run retained exactly one existing intentional-root warning and emitted no new shared warning.
  • The focused root regression passed, and make -C tests check passed all 9 tests.
  • git diff --check, the shell syntax check and make stylecheck passed. A supplementary non-ASCII scan found only an unchanged character outside the diff.
  • distcheck-light created and built the clean source archive, including the new regression, then failed at the final spellcheck on unchanged docs/man/apcmicrolink.txt text.
  • The behavioural checks cover the POSIX implementation. The WIN32 branch is unchanged, and no physical UPS hardware was required or tested.

AI assistance: OpenAI Codex gpt-5.6-sol at high reasoning was used for investigation, implementation and validation. The human contributor remains responsible for the change.

Check the real and effective UIDs after the POSIX credential transition, and warn once if either remains zero.

Keep non-root starts and the intentionally privileged upsmon -p path unchanged.

Fixes networkupstools#3471.

AI assistance: OpenAI Codex gpt-5.6-sol at high reasoning was used for investigation, implementation and validation. The human contributor remains responsible for the change.

Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

A ZIP file with standard source tarball and another tarball with pre-built docs for commit 75d5b30 is temporarily available: NUT-tarballs-PR-3609.zip.

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5181-master completed (commit 3802a9a93c by @)

@jimklimov jimklimov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but a NEWS.adoc entry would be welcome.

@jimklimov jimklimov added the service/daemon start/stop General subject for starting and stopping NUT daemons (drivers, server, monitor); also BG/FG/Debug label Sep 1, 2026
@jimklimov jimklimov added this to the 2.8.6 milestone Sep 1, 2026
@jimklimov jimklimov added enhancement AI For good or bad, machine tools are upon us. Humans are still the responsible ones. labels Sep 1, 2026
Honour EXEEXT, pass the exact build-tree upslog path, and use the current UID-0 account name. Treat an empty TMPDIR as unset and add the requested NEWS entry.

Refs networkupstools#3471.

AI assistance: OpenAI Codex gpt-5.6-sol at high reasoning was used for investigation, implementation and validation. The human contributor remains responsible for the change.

Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
@user01010111

Copy link
Copy Markdown
Author

Thanks Jim — I’ve added a concise NEWS entry under common code. I also corrected the new regression’s build-tree wiring to honour $(EXEEXT) and pass the exact upslog path, and made it use the current UID-0 account name rather than assuming it is root. The focused GCC build/test, UID-0 namespace regression, Automake regeneration and distribution-membership checks pass.

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5186-master completed (commit 117bf537d9 by @)

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5186-master completed (commit 117bf537d9 by @)

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

Labels

AI For good or bad, machine tools are upon us. Humans are still the responsible ones. enhancement service/daemon start/stop General subject for starting and stopping NUT daemons (drivers, server, monitor); also BG/FG/Debug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: daemons should warn if they remain root for the bulk of work

3 participants