Skip to content

fix(telegraf): Correct supplementary group handling when dropping privileges#903

Open
skartikey wants to merge 1 commit into
masterfrom
skartikey/telegraf-group-handling
Open

fix(telegraf): Correct supplementary group handling when dropping privileges#903
skartikey wants to merge 1 commit into
masterfrom
skartikey/telegraf-group-handling

Conversation

@skartikey

@skartikey skartikey commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #861 and the question raised in #724.

Two issues with the su-exec to setpriv migration:

  1. Alpine images leak root's baked-in groups to telegraf. The Alpine base image puts root in bin daemon sys adm disk wheel floppy dialout tape video, and the entrypoint loop forwards them all:

    $ docker run --rm telegraf:1.39-alpine id -Gn
    telegraf bin daemon sys adm disk wheel floppy dialout tape video
    

    Under su-exec this was telegraf only. Fixed at build time by dropping root from those groups, which keeps an intentional --group-add video working. Debian images are unaffected.

  2. setpriv --groups replaces the supplementary set, dropping the telegraf user's own /etc/group memberships (su-exec used getgrouplist()). Entrypoints now seed the list from id -Gn telegraf, so derived images that add telegraf to a group keep that membership.

Verified on a patched 1.39/alpine build and the Debian entrypoint: default run yields telegraf only; --group-add (numeric and named) and baked memberships are preserved; non-root passthrough unchanged.

Now that #861 is merged, telegraf/nightly/alpine is included and carries the identical entrypoint.

…vileges

On Alpine, the base image bakes root into several default groups (bin,
daemon, sys, adm, disk, wheel, floppy, dialout, tape, video), so the
container's root process is granted them at runtime and the entrypoint's
group-forwarding loop passed all of them to the telegraf process:

    $ docker run --rm telegraf:1.39-alpine id -Gn
    telegraf bin daemon sys adm disk wheel floppy dialout tape video

Under su-exec the process ran with 'telegraf' only. Fix it at the source
by dropping root from those groups at build time, which also keeps
'docker run --group-add video' working (an entrypoint-side exclusion
list would silently swallow it).

In addition, seed the forwarded group list from the telegraf user's own
/etc/group memberships instead of the literal 'telegraf', restoring the
initgroups semantics su-exec provided: a derived image that adds the
telegraf user to a group no longer loses that membership, since
'setpriv --groups' replaces the supplementary set rather than extending
it. The Debian entrypoints get the same change for consistency; their
base image bakes no root memberships, so they only gain the initgroups
behaviour.

See #724.
@skartikey
skartikey force-pushed the skartikey/telegraf-group-handling branch from 7ec3421 to a82330c Compare July 16, 2026 18:41
@skartikey
skartikey requested a review from srebhan July 16, 2026 18:42
@srebhan srebhan changed the title fix(telegraf): correct supplementary group handling when dropping privileges fix(telegraf): Correct supplementary group handling when dropping privileges Jul 20, 2026

@srebhan srebhan 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.

Thanks @skartikey! One comment...

Comment on lines +17 to 29
# start from the telegraf user's own groups so memberships baked into
# /etc/group are kept (as su-exec's initgroups did), then honor groups
# supplied via 'docker run --group-add ...' but drop 'root' and anything
# already present
# see https://github.com/influxdata/influxdata-docker/issues/724
groups="telegraf"
groups="$(id -Gn telegraf | tr ' ' ',')"
extra_groups="$(id -Gn || true)"
for group in $extra_groups; do
case "$group" in
root | telegraf) ;;
*) groups="$groups,$group" ;;
case ",${groups},root," in
*",${group},"*) ;;
*) groups="${groups},${group}" ;;
esac
done

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.

As far as I understand the goal here is to remove duplicates and root from the group arguments, isn't it? So wouldn't

groups=$(echo -n "${groups} ${extra_groups}" | tr ' ' '\n' | grep -v 'root' | sort -u | tr '\n' ' ')

be easier and more readable?

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