Add NODE_HOST_ENV to mirror host env vars under node #18820
Conversation
|
Instead of this PR, perhaps it would be useful to document at https://emscripten.org/docs/porting/connecting_cpp_and_javascript/Interacting-with-code.html#interacting-with-code-environment-variables that it's quite easy to mirror the entire environment on Node.js with: Module.preRun = () => {ENV = Object.create(process.env)};(see #3948 (comment) where this was originally mentioned) |
|
FWIW, the reason I mentioned this is that Rust no longer test the |
sbc100
changed the title
|
OK, I went with It should fix the libc++ get_temp_directory issue under windows. |
sbc100
changed the title
|
Reviving this 2 years old PR! |
sbc100
force-pushed
the
node_env
branch
2 times, most recently
from
393e0e8 to
e67fb52
Compare
sbc100
force-pushed
the
node_env
branch
2 times, most recently
from
dc377ee to
21e4019
Compare
|
Ping .. someone want to approve this? |
| When running under nodejs, expose the underlying OS environment variables. | ||
| This is similiar to how ``NODERAWFS`` exporses the underlying FS. | ||
| This setting gets enabled by default when ``NODERAWFS`` is enabled, but can | ||
| also be controlled separatley. |
There was a problem hiding this comment.
Perhaps leave it orthogonal to NODERAWFS? That would avoid a breaking change, and seems more reasonable unless I'm missing something.
There was a problem hiding this comment.
It is orthogonal, but enabled by NODERAWFS default since I think anyone exposing the full host FS would also certainly also want the host environment variables. They are often very much linked.. think TMPDIR, HOME, etc
Its pretty hard to imagine giving full FS access but then not wanting to give access to environment variables. The former is a lot more permissive.
There was a problem hiding this comment.
I updated the changelog entry and improved the docs.
There was a problem hiding this comment.
Fair points. But I still worry about a breaking change here. In particular this exposes more of the host environment to the compiled program - it already had files, and now it has env vars as well, which can contain sensitive info.
There was a problem hiding this comment.
Honestly, you can already access them, at least on linux. i.e. cat /proc/self/environ.
There was a problem hiding this comment.
Another reason to link them by default is that if you don't you into issues like #25791 because the values in the environment don't correspond to real paths on the filesystem. e.g. our fake environment has TMP=/tmp but that path does not exist on windows with NODERAWFS. So I don't think it makes much sense to fake one without the other (unless you really know what you are doing and want to opt into it).
sbc100
force-pushed
the
node_env
branch
2 times, most recently
from
3df9be7 to
9ecd8a0
Compare
This setting is enabled by default when
NODERAWFSis enabled, but can also be disabled explicitly.Fixes: #18816, #25791