feat(process): add process.initgroups(user, group) implementation#27902
feat(process): add process.initgroups(user, group) implementation#27902ssing2 wants to merge 1 commit intooven-sh:mainfrom
Conversation
Fixes oven-sh#23891 This adds the process.initgroups(user, group) method, which initializes the group access list by reading the /etc/group file and using all groups of which the user is a member. The initgroups() function sets the user's supplemental group access list based on the system's group database. ### Changes - Added Process_functionInitgroups in BunProcess.cpp - Accepts (user: string, group: number | string) parameters - Calls the system initgroups() function - Properly validates arguments and throws on errors ### API ```js process.initgroups(user, group); // user: string - username // group: number | string - group ID or name ``` Matches Node.js process.initgroups() behavior.
WalkthroughThis pull request implements the Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/BunProcess.cpp`:
- Around line 2882-2888: The RETURN_IF_EXCEPTION(scope, {}); after the
initgroups call is redundant and unreachable: when initgroups fails
throwSystemError(scope, globalObject, "initgroups"_s, errno) already throws and
the function returns; when it succeeds there is no pending exception. Remove the
lone RETURN_IF_EXCEPTION(scope, {}); statement from the function in
BunProcess.cpp (the block that calls initgroups and throwSystemError) so only
the error throw and the final return JSValue::encode(jsUndefined()) remain.
- Line 4097: The LUT entry for initgroups is declared with the wrong arity;
update the lookup table entry that currently reads "initgroups
Process_functionInitgroups Function 1" to specify
Function 2 so the function length matches the actual implementation and Node.js
behavior; ensure the symbol names to change are initgroups and
Process_functionInitgroups in the LUT so process.initgroups.length === 2.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a2719ac1-5f9c-409c-bd94-109b8668ece6
📒 Files selected for processing (1)
src/bun.js/bindings/BunProcess.cpp
| if (result != 0) { | ||
| throwSystemError(scope, globalObject, "initgroups"_s, errno); | ||
| return {}; | ||
| } | ||
|
|
||
| RETURN_IF_EXCEPTION(scope, {}); | ||
| return JSValue::encode(jsUndefined()); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant exception check after error return.
Line 2887 RETURN_IF_EXCEPTION(scope, {}); is unreachable. If initgroups fails, the exception is thrown at line 2883 and the function returns at line 2884. If initgroups succeeds, no exception is pending. This line can be safely removed.
🧹 Proposed cleanup
auto result = initgroups(userStr.utf8().data(), group);
if (result != 0) {
throwSystemError(scope, globalObject, "initgroups"_s, errno);
return {};
}
- RETURN_IF_EXCEPTION(scope, {});
return JSValue::encode(jsUndefined());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (result != 0) { | |
| throwSystemError(scope, globalObject, "initgroups"_s, errno); | |
| return {}; | |
| } | |
| RETURN_IF_EXCEPTION(scope, {}); | |
| return JSValue::encode(jsUndefined()); | |
| if (result != 0) { | |
| throwSystemError(scope, globalObject, "initgroups"_s, errno); | |
| return {}; | |
| } | |
| return JSValue::encode(jsUndefined()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bun.js/bindings/BunProcess.cpp` around lines 2882 - 2888, The
RETURN_IF_EXCEPTION(scope, {}); after the initgroups call is redundant and
unreachable: when initgroups fails throwSystemError(scope, globalObject,
"initgroups"_s, errno) already throws and the function returns; when it succeeds
there is no pending exception. Remove the lone RETURN_IF_EXCEPTION(scope, {});
statement from the function in BunProcess.cpp (the block that calls initgroups
and throwSystemError) so only the error throw and the final return
JSValue::encode(jsUndefined()) remain.
| seteuid Process_functionseteuid Function 1 | ||
| setgid Process_functionsetgid Function 1 | ||
| setgroups Process_functionsetgroups Function 1 | ||
| initgroups Process_functionInitgroups Function 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Node.js process.initgroups.length is 2
node -e "console.log('process.initgroups.length:', process.initgroups.length)"Repository: oven-sh/bun
Length of output: 83
🏁 Script executed:
# Check if the file exists and read the relevant lines
file_path="src/bun.js/bindings/BunProcess.cpp"
if [ -f "$file_path" ]; then
echo "=== Line 4097 (LUT entry) ==="
sed -n '4095,4099p' "$file_path"
echo ""
echo "=== Lines 2853-2855 (enforcement check) ==="
sed -n '2851,2857p' "$file_path"
else
echo "File not found: $file_path"
fiRepository: oven-sh/bun
Length of output: 773
Incorrect function length in LUT entry.
The initgroups function requires 2 arguments (user and group), as enforced by the missing args check at lines 2853-2855. The LUT entry should specify Function 2 to match Node.js where process.initgroups.length === 2.
🔧 Proposed fix
- initgroups Process_functionInitgroups Function 1
+ initgroups Process_functionInitgroups Function 2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| initgroups Process_functionInitgroups Function 1 | |
| initgroups Process_functionInitgroups Function 2 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bun.js/bindings/BunProcess.cpp` at line 4097, The LUT entry for
initgroups is declared with the wrong arity; update the lookup table entry that
currently reads "initgroups Process_functionInitgroups
Function 1" to specify Function 2 so the function length matches the actual
implementation and Node.js behavior; ensure the symbol names to change are
initgroups and Process_functionInitgroups in the LUT so
process.initgroups.length === 2.
Fixes #23891
This adds the process.initgroups(user, group) method, which initializes
the group access list by reading the /etc/group file and using all groups
of which the user is a member.
Changes
API
Usage Example
Matches Node.js process.initgroups() behavior and signature.