From e5f79e487b7cb10d6675f965493521f80cdce4c5 Mon Sep 17 00:00:00 2001 From: bryopsida <8363252+bryopsida@users.noreply.github.com> Date: Sat, 20 Dec 2025 13:08:48 -0600 Subject: [PATCH] unix: support setting supplementary process groups (#4704) uv_spawn currently unsets all supplementary groups if the caller sets either a gid or uid to run the process under. This commit adds support for specifying the supplementary groups to set instead. Co-authored-by: Ben Noordhuis Co-authored-by: Joe Dollard Signed-off-by: bryopsida <8363252+bryopsida@users.noreply.github.com> --- .github/workflows/CI-unix.yml | 9 +++- .github/workflows/sanitizer.yml | 11 +++- docs/src/process.rst | 25 +++++++-- include/uv.h | 20 +++++-- src/unix/process.c | 9 +++- src/win/process.c | 5 +- test/test-list.h | 4 ++ test/test-spawn.c | 92 +++++++++++++++++++++++++++++++++ 8 files changed, 162 insertions(+), 13 deletions(-) diff --git a/.github/workflows/CI-unix.yml b/.github/workflows/CI-unix.yml index 298dca214..2def60dd1 100644 --- a/.github/workflows/CI-unix.yml +++ b/.github/workflows/CI-unix.yml @@ -80,7 +80,7 @@ jobs: strategy: fail-fast: false matrix: - os: [macos-13, macos-14] + os: [macos-14, macos-15] steps: - uses: actions/checkout@v4 - name: Envinfo @@ -109,6 +109,11 @@ jobs: run: | ./build/uv_run_tests_a platform_output - name: Test + if: ${{ maxtrix.os == 'macos-15' }} + run: | + cd build && sudo UV_RUN_AS_ROOT=1 ctest -V + - name: Test + if: ${{ maxtrix.os != 'macos-15' }} run: | cd build && ctest -V - name: Autotools configure @@ -124,7 +129,7 @@ jobs: strategy: fail-fast: false matrix: - os: [macos-13, macos-14] + os: [macos-14, macos-15] steps: - uses: actions/checkout@v4 - name: Configure diff --git a/.github/workflows/sanitizer.yml b/.github/workflows/sanitizer.yml index 58254327d..d4563cc79 100644 --- a/.github/workflows/sanitizer.yml +++ b/.github/workflows/sanitizer.yml @@ -67,7 +67,16 @@ jobs: ./build-ubsan/uv_run_tests_a sanitizers-macos: - runs-on: macos-13 + runs-on: macos-14 + strategy: + matrix: + config: + - name: ASAN + flags: -DASAN=ON -DCMAKE_BUILD_TYPE=Debug + - name: TSAN + flags: -DTSAN=ON -DCMAKE_BUILD_TYPE=Release + - name: UBSAN + flags: -DUBSAN=ON -DCMAKE_BUILD_TYPE=Debug steps: - uses: actions/checkout@v4 diff --git a/docs/src/process.rst b/docs/src/process.rst index c0c477ef5..2b09a93af 100644 --- a/docs/src/process.rst +++ b/docs/src/process.rst @@ -34,6 +34,8 @@ Data types uv_gid_t gid; char* cpumask; size_t cpumask_size; + uv_gid_t* gids; + size_t gids_size; } uv_process_options_t; .. c:type:: void (*uv_exit_cb)(uv_process_t*, int64_t exit_status, int term_signal) @@ -94,7 +96,20 @@ Data types * search for the exact file name before trying variants with * extensions like '.exe' or '.cmd'. */ - UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME = (1 << 7) + UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME = (1 << 7), + /* + * Spawn the child process with the error mode of its parent. + * This option is only meaningful on Windows systems. On Unix + * it is silently ignored. + */ + UV_PROCESS_WINDOWS_USE_PARENT_ERROR_MODE = (1 << 8), + /* + * Set the child process' supplementary group ids. The group ids are supplied + * in the 'gids' field in the options struct, and the number of groups is + * specified in the 'num_gids' field. This does not work on windows; + * setting this flag will cause uv_spawn() to fail. + */ + UV_PROCESS_SETGROUPS = (1 << 9) }; .. c:type:: uv_stdio_container_t @@ -221,14 +236,18 @@ Public members .. c:member:: uv_uid_t uv_process_options_t.uid .. c:member:: uv_gid_t uv_process_options_t.gid +.. c:member:: uv_gid_t* uv_process_options_t.gids +.. c:member:: size_t uv_process_options_t.gids_size - Libuv can change the child process' user/group id. This happens only when - the appropriate bits are set in the flags fields. + Libuv can change the child process' user/group id and supplementary group + ids. This happens only when the appropriate bits are set in the flags fields. .. note:: This is not supported on Windows, :c:func:`uv_spawn` will fail and set the error to ``UV_ENOTSUP``. + .. versionadded:: 2.0.0 + .. c:member:: char* uv_process_options_t.cpumask .. c:member:: size_t uv_process_options_t.cpumask_size diff --git a/include/uv.h b/include/uv.h index d59bf75ef..e3233c0fa 100644 --- a/include/uv.h +++ b/include/uv.h @@ -1131,9 +1131,10 @@ typedef struct uv_process_options_s { int stdio_count; uv_stdio_container_t* stdio; /* - * Libuv can change the child process' user/group id. This happens only when - * the appropriate bits are set in the flags fields. This is not supported on - * windows; uv_spawn() will fail and set the error to UV_ENOTSUP. + * Libuv can change the child process' user/group id, and supplementarty + * group ids. This happens only when the appropriate bits are set in the + * flags fields. This is not supported on windows; uv_spawn() will fail and + * set the error to UV_ENOTSUP. */ uv_uid_t uid; uv_gid_t gid; @@ -1150,6 +1151,8 @@ typedef struct uv_process_options_s { */ char* cpumask; size_t cpumask_size; + uv_gid_t* gids; + size_t gids_size; } uv_process_options_t; /* @@ -1163,7 +1166,7 @@ enum uv_process_flags { */ UV_PROCESS_SETUID = (1 << 0), /* - * Set the child process' group id. The user id is supplied in the `gid` + * Set the child process' group id. The group id is supplied in the `gid` * field of the options struct. This does not work on windows; setting this * flag will cause uv_spawn() to fail. */ @@ -1211,7 +1214,14 @@ enum uv_process_flags { * This option is only meaningful on Windows systems. On Unix * it is silently ignored. */ - UV_PROCESS_WINDOWS_USE_PARENT_ERROR_MODE = (1 << 8) + UV_PROCESS_WINDOWS_USE_PARENT_ERROR_MODE = (1 << 8), + /* + * Set the child process' supplementary group ids. The group ids are supplied + * in the 'gids' field in the options struct, and the number of groups is + * specified in the 'num_gids' field. This does not work on windows; + * setting this flag will cause uv_spawn() to fail on windows. + */ + UV_PROCESS_SETGROUPS = (1 << 9) }; /* diff --git a/src/unix/process.c b/src/unix/process.c index 30b0b33ef..2bf5ad383 100644 --- a/src/unix/process.c +++ b/src/unix/process.c @@ -395,6 +395,12 @@ static void uv__process_child_init(const uv_process_options_t* options, SAVE_ERRNO(setgroups(0, NULL)); } + if (options->flags & UV_PROCESS_SETGROUPS) { + if (setgroups(options->gids_size, options->gids)) { + uv__write_errno(error_fd); + } + } + if ((options->flags & UV_PROCESS_SETGID) && setgid(options->gid)) uv__write_errno(error_fd); @@ -505,7 +511,7 @@ static int uv__spawn_set_posix_spawn_attrs( return err; } - if (options->flags & (UV_PROCESS_SETUID | UV_PROCESS_SETGID)) { + if (options->flags & (UV_PROCESS_SETUID | UV_PROCESS_SETGID | UV_PROCESS_SETGROUPS)) { /* kauth_cred_issuser currently requires exactly uid == 0 for these * posixspawn_attrs (set_groups_np, setuid_np, setgid_np), which deviates * from the normal specification of setuid (which also uses euid), and they @@ -1022,6 +1028,7 @@ int uv_spawn(uv_loop_t* loop, assert(!(options->flags & ~(UV_PROCESS_DETACHED | UV_PROCESS_SETGID | UV_PROCESS_SETUID | + UV_PROCESS_SETGROUPS | UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME | UV_PROCESS_WINDOWS_HIDE | UV_PROCESS_WINDOWS_HIDE_CONSOLE | diff --git a/src/win/process.c b/src/win/process.c index 92caf1f2a..2f107321b 100644 --- a/src/win/process.c +++ b/src/win/process.c @@ -899,7 +899,9 @@ int uv_spawn(uv_loop_t* loop, process->exit_cb = options->exit_cb; child_stdio_buffer = NULL; - if (options->flags & (UV_PROCESS_SETGID | UV_PROCESS_SETUID)) { + if (options->flags & (UV_PROCESS_SETGID | + UV_PROCESS_SETUID | + UV_PROCESS_SETGROUPS)) { return UV_ENOTSUP; } @@ -918,6 +920,7 @@ int uv_spawn(uv_loop_t* loop, assert(!(options->flags & ~(UV_PROCESS_DETACHED | UV_PROCESS_SETGID | UV_PROCESS_SETUID | + UV_PROCESS_SETGROUPS | UV_PROCESS_WINDOWS_FILE_PATH_EXACT_NAME | UV_PROCESS_WINDOWS_HIDE | UV_PROCESS_WINDOWS_HIDE_CONSOLE | diff --git a/test/test-list.h b/test/test-list.h index 93e9f17d3..e1735df19 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -343,6 +343,7 @@ TEST_DECLARE (spawn_setuid_fails) TEST_DECLARE (spawn_setgid_fails) TEST_DECLARE (spawn_affinity) TEST_DECLARE (spawn_affinity_invalid_mask) +TEST_DECLARE (spawn_setgids_fails) TEST_DECLARE (spawn_stdout_to_file) TEST_DECLARE (spawn_stdout_and_stderr_to_file) TEST_DECLARE (spawn_stdout_and_stderr_to_file2) @@ -541,6 +542,7 @@ TEST_DECLARE (win32_signum_number) #else TEST_DECLARE (emfile) TEST_DECLARE (spawn_setuid_setgid) +TEST_DECLARE (spawn_setgids) TEST_DECLARE (we_get_signal) TEST_DECLARE (we_get_signals) TEST_DECLARE (we_get_signal_one_shot) @@ -1039,6 +1041,7 @@ TASK_LIST_START TEST_ENTRY (spawn_setgid_fails) TEST_ENTRY (spawn_affinity) TEST_ENTRY (spawn_affinity_invalid_mask) + TEST_ENTRY (spawn_setgids_fails) TEST_ENTRY (spawn_stdout_to_file) TEST_ENTRY (spawn_stdout_and_stderr_to_file) TEST_ENTRY (spawn_stdout_and_stderr_to_file2) @@ -1084,6 +1087,7 @@ TASK_LIST_START #else TEST_ENTRY (emfile) TEST_ENTRY (spawn_setuid_setgid) + TEST_ENTRY (spawn_setgids) TEST_ENTRY (we_get_signal) TEST_ENTRY (we_get_signals) TEST_ENTRY (we_get_signal_one_shot) diff --git a/test/test-spawn.c b/test/test-spawn.c index 0ee7b91db..68ceee0a1 100644 --- a/test/test-spawn.c +++ b/test/test-spawn.c @@ -1498,6 +1498,44 @@ TEST_IMPL(spawn_setuid_setgid) { } #endif +#ifndef _WIN32 +TEST_IMPL(spawn_setgids) { + int r; + struct passwd* pw; + uv_gid_t gids[1]; + + /* if not root, then this will fail. */ + uv_uid_t uid = getuid(); + if (uid != 0) + RETURN_SKIP("spawn_setgids skipped: not root\n"); + + init_process_options("spawn_helper1", exit_cb); + + pw = getpwnam("nobody"); + ASSERT_NOT_NULL(pw); + gids[0] = pw->pw_gid; + options.gids = gids; + options.gids_size = 1; + + options.flags = UV_PROCESS_SETGROUPS; + + r = uv_spawn(uv_default_loop(), &process, &options); + if (r == UV_EACCES) + RETURN_SKIP("user 'nobody' cannot access the test runner"); + + ASSERT_OK(r); + + r = uv_run(uv_default_loop(), UV_RUN_DEFAULT); + ASSERT_OK(r); + + ASSERT_EQ(1, exit_cb_called); + ASSERT_EQ(1, close_cb_called); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +} +#endif + #ifndef _WIN32 TEST_IMPL(spawn_setuid_fails) { @@ -1594,6 +1632,39 @@ TEST_IMPL(spawn_setgid_fails) { MAKE_VALGRIND_HAPPY(uv_default_loop()); return 0; } + + +TEST_IMPL(spawn_setgids_fails) { + int r; + uv_gid_t gids[1] = {0}; + + /* if root, become nobody. */ + uv_uid_t uid = getuid(); + if (uid == 0) { + struct passwd* pw; + pw = getpwnam("nobody"); + ASSERT_NOT_NULL(NULL); + ASSERT_OK(setgid(pw->pw_gid)); + ASSERT_OK(setuid(pw->pw_uid)); + } + + init_process_options("spawn_helper1", fail_cb); + + options.flags |= UV_PROCESS_SETGROUPS; + options.gids = gids; + options.gids_size = 1; + + r = uv_spawn(uv_default_loop(), &process, &options); + ASSERT_EQ(r, UV_EPERM); + + r = uv_run(uv_default_loop(), UV_RUN_DEFAULT); + ASSERT_OK(r); + + ASSERT_EQ(0, close_cb_called); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +} #endif TEST_IMPL(spawn_affinity) { @@ -1757,6 +1828,27 @@ TEST_IMPL(spawn_setgid_fails) { MAKE_VALGRIND_HAPPY(uv_default_loop()); return 0; } + +TEST_IMPL(spawn_setgids_fails) { + int r; + uv_gid_t gids[1] = {0}; + init_process_options("spawn_helper1", exit_cb_unexpected); + + options.flags |= UV_PROCESS_SETGROUPS; + options.gids = gids; + options.gids_size = 1; + + r = uv_spawn(uv_default_loop(), &process, &options); + ASSERT_EQ(r, UV_ENOTSUP); + + r = uv_run(uv_default_loop(), UV_RUN_DEFAULT); + ASSERT_OK(r); + + ASSERT_OK(close_cb_called); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +} #endif