Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test-child-process-uid-gid fails on FreeBSD #10646

Closed
UnitedMarsupials-zz opened this issue Jan 5, 2017 · 2 comments
Closed

test-child-process-uid-gid fails on FreeBSD #10646

UnitedMarsupials-zz opened this issue Jan 5, 2017 · 2 comments
Labels
child_process Issues and PRs related to the child_process subsystem. freebsd Issues and PRs related to the FreeBSD platform.

Comments

@UnitedMarsupials-zz
Copy link

Performing "make test" on the freshly-built node-7.4.0 I see the following:

=== release test-child-process-uid-gid ===                                     
Path: parallel/test-child-process-uid-gid
assert.js:85
  throw new assert.AssertionError({
  ^
AssertionError: Missing expected exception..
    at _throws (assert.js:339:5)
    at Function.throws (assert.js:363:3)
    at Object.<anonymous> (/spare/usr/ports/www/node/work/node-v7.4.0/test/parallel/test-child-process-uid-gid.js:17:8)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.runMain (module.js:605:10)
    at run (bootstrap_node.js:420:7)
Command: out/Release/node /spare/usr/ports/www/node/work/node-v7.4.0/test/parallel/test-child-process-uid-gid.js
[01:30|%  92|+ 1216|-   1]: release testerSyncFolderItemsRequest.onSendError aMsg:Error during connecting to hostname 'mail.humedica.com'. Is the host down?. (STATUS_CONNECTING_TO)
[02:11|% 100|+ 1319|-   1]: Done                                               
gmake[1]: *** [Makefile:126: test] Error 1
gmake[1]: Leaving directory '/spare/usr/ports/www/node/work/node-v7.4.0'
*** Error code 1

I am running the test as myself (not as root)... The failing snippet is:

assert.throws(() => {
  spawn('echo', ['fhqwhgads'], {gid: 0});
}, expectedError);

It fails, because my account belongs to group 0 (wheel). In fact, it is my only group. The test should be checking for such situations similarly to how it already checks for being run by uid 0.

@mscdex mscdex added child_process Issues and PRs related to the child_process subsystem. freebsd Issues and PRs related to the FreeBSD platform. labels Jan 5, 2017
@UnitedMarsupials-zz
Copy link
Author

I'm afraid, the committed fix is not sufficient. The effective GID may be different from 0, but, if the user's account belongs to group 0 as well, the spawn may still succeed. So, the check may need to go through all of the account's groups to make sure, none of them is 0. Frankly, I don't think, it is worth the trouble -- I'd advise to simply remove the second test from this file.

Also, this is not a FreeBSD-specific problem, so I don't think, the "freebsd" tag is appropriate.

@cjihrig
Copy link
Contributor

cjihrig commented Jan 6, 2017

I don't think we should remove the check. It's the only test for that option that we have, save a type checking test. I updated the test in #10647 to check for all groups.

cjihrig added a commit to cjihrig/node that referenced this issue Jan 9, 2017
This commit lets the uid and gid options of spawn() to be
tested independently of one another based on the user's uid
and gid.

Fixes: nodejs#10646
PR-URL: nodejs#10647
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 18, 2017
This commit lets the uid and gid options of spawn() to be
tested independently of one another based on the user's uid
and gid.

Fixes: nodejs#10646
PR-URL: nodejs#10647
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 23, 2017
This commit lets the uid and gid options of spawn() to be
tested independently of one another based on the user's uid
and gid.

Fixes: nodejs#10646
PR-URL: nodejs#10647
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 25, 2017
This commit lets the uid and gid options of spawn() to be
tested independently of one another based on the user's uid
and gid.

Fixes: nodejs#10646
PR-URL: nodejs#10647
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 27, 2017
This commit lets the uid and gid options of spawn() to be
tested independently of one another based on the user's uid
and gid.

Fixes: nodejs#10646
PR-URL: nodejs#10647
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this issue Mar 8, 2017
This commit lets the uid and gid options of spawn() to be
tested independently of one another based on the user's uid
and gid.

Fixes: #10646
PR-URL: #10647
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
This commit lets the uid and gid options of spawn() to be
tested independently of one another based on the user's uid
and gid.

Fixes: #10646
PR-URL: #10647
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. freebsd Issues and PRs related to the FreeBSD platform.
Projects
None yet
Development

No branches or pull requests

3 participants