Strider-CD / strider-simple-runner

Easy-to-configure in-process Runner implementation for Strider.
MIT License
3 stars 18 forks source link

NODE_ENV leaks into test run #21

Open jaredly opened 11 years ago

jaredly commented 11 years ago

If you run strider with NODE_ENV=prod, npm install doesn't install dev modules.

Should we just block out the parent env entirely when running child processes? (with perhaps the exception of PATH) https://github.com/Strider-CD/strider-simple-worker/blob/master/lib/job.js#L120

niallo commented 11 years ago

Yeah, we should only whitelist certain variables:

$HOME, $PATH, $PAAS_NAME

jaredly commented 11 years ago

do we need $HOME? tests really shouldn't be messing with it ... although maybe for ssh stuff?

On Tue, Aug 20, 2013 at 10:43 AM, niallo notifications@github.com wrote:

Yeah, we should only whitelist certain variables:

$HOME, $PATH, $PAAS_NAME

— Reply to this email directly or view it on GitHubhttps://github.com/Strider-CD/strider-simple-worker/issues/21#issuecomment-22959028 .

niallo commented 11 years ago

Many UNIX utilities expect $HOME to be available. NPM for example writes to $HOME/.npm

jaredly commented 11 years ago

what if we make $HOME be the tmp directory where we're running the build?

On Tue, Aug 20, 2013 at 12:31 PM, niallo notifications@github.com wrote:

Many UNIX utilities expect $HOME to be available. NPM for example writes to $HOME/.npm

— Reply to this email directly or view it on GitHubhttps://github.com/Strider-CD/strider-simple-worker/issues/21#issuecomment-22966829 .

niallo commented 11 years ago

Seems to me that the expectation is HOME is whatever the shell sets. But as long as the location is writable I guess it's ok.

POSIX says (section 2.5.3):

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html

jaredly commented 11 years ago

Yeah, I'm not married to either direction.

On Tue, Aug 20, 2013 at 2:31 PM, niallo notifications@github.com wrote:

Seems to me that the expectation is HOME is whatever the shell sets. But as long as the location is writable I guess it's ok.

POSIX says (section 2.5.3):

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html

— Reply to this email directly or view it on GitHubhttps://github.com/Strider-CD/strider-simple-worker/issues/21#issuecomment-22975286 .