BUG: Environment Variables not passed to Wasm Applications #1312

RobbieMcKinstry · 2024-06-11T16:33:54Z

About

After making small changes to the application described in this official blog post, I discovered that environment variables were not being propagated to the application, either from the parent process' environment or variables specifically defined in Unit configuration.

Suspected Cause

At a high level, I believe the issue is in the Wasm host. Wasm environment contexts start empty and have to explicitly opt-in to inheriting environment variables.

Specifically, these lines of code, reproduced below, seem to illustrate the issue:

let mut cx = WasiCtxBuilder::new();
// NB: while useful for debugging untrusted code probably
// shouldn't get raw access to stdout/stderr.
cx.inherit_stdout();
cx.inherit_stderr();

As you can see, the WasiCxtBuilder explicitly opts-in to inheriting stdout and stderr`, but it does not opt-in to inheriting environment variables. This API should be used to inherit env vars:

let mut cx = WasiCtxBuilder::new();
// NB: while useful for debugging untrusted code probably
// shouldn't get raw access to stdout/stderr.
cx.inherit_stdout();
cx.inherit_stderr();
+ cx.inherit_env();

I'm not sure if that's enough to inherit variables defined according to the environment configuration, which I found does not work. I would expect that during setup, the application configuration provided here would inject the environment section of the config file, and environment variable would have to be copied out of that config struct and into the WasiCtx here.

It looks like that's what's done later in the same method with the dirs field, stored in GlobalConfig, which is used to provide filesystem access to the Wasm component which would be otherwise unable to read from the filesystem.

Code Sample

The following code sample adapts the application presented in the aforementioned blog post. The output demonstrates that no environment variables were observed. To reproduce, initialize the repository as described in the blog post, then alter the config.js file and src/lib.rs as described below. When you visit the website, the webpage will demonstrate there aren't any environmental variables at all, neither those set by default by your system, not those defined in the Unit's environment config section.

From config.js:

{
   "listeners": {
      "*:80": {
         "pass": "applications/my-wasm-component"
      }
   },
   "applications": {
      "my-wasm-component": {
         "type": "wasm-wasi-component",
         "environment": {
            "PROVIDER_NAME": "Google Cloud",
            "Foo": "Bar"
         },
         "component": "/opt/nginx_unit.wasm"
      }
   }
}

From src/lib.rs:

use std::collections::HashMap;

use wasi::http::types::{
    Fields, IncomingRequest, OutgoingBody, OutgoingResponse, ResponseOutparam,
};

wasi::http::proxy::export!(Component);
use wasi::cli::environment::get_environment;

struct Component;

const HTML_BODY: &str = r#"<html>
    <head>
        <title>Hello from WebAssembly!</title>
    </head>
    <body>
      {{ENV_VARS}}
    </body>
</html>"#;

impl wasi::exports::http::incoming_handler::Guest for Component {
    fn handle(_request: IncomingRequest, response_out: ResponseOutparam) {
        let hdrs = Fields::new();
        let variables = format!("{:?}", get_environment());
        let mesg = String::from(HTML_BODY).replace("{{ENV_VARS}}", variables);

        let _try = hdrs.set(
            &"Content-Type".to_string(),
            &[b"text/html; charset=utf-8".to_vec()],
        );
        let _try = hdrs.set(
            &"Content-Length".to_string(),
            &[mesg.len().to_string().as_bytes().to_vec()],
        );

        let resp = OutgoingResponse::new(hdrs);
        resp.set_status_code(200).unwrap();

        let body = resp.body().unwrap();
        ResponseOutparam::set(response_out, Ok(resp));

        let out = body.write().unwrap();
        out.blocking_write_and_flush(mesg.as_bytes()).unwrap();
        drop(out);

        OutgoingBody::finish(body, None).unwrap();
    }
}

Thank you very much for your time and attention.

The text was updated successfully, but these errors were encountered:

ac000 · 2024-06-11T18:12:33Z

Thanks for the detailed report!.

Indeed I do this in our original C based wasm language module but must have missed it wasn't being done in the rust one.

This will also get environment variables set in the config file. IIRC they are read and set by the "prototype" process before the "application" process(es) are fork(2)ed.

RobbieMcKinstry · 2024-06-11T21:10:32Z

IIRC they are read and set by the "prototype" process before the "application" process(es) are fork(2)ed.

Awesome, that makes sense to me! The presence of the "prototype" process clearly up my concern that the values wouldn't be copied from the environment config setting. 👍🏻

Thank you for your hard work on this lovely project!

ac000 · 2024-06-18T17:02:25Z

So I'm trying this out with the below patch

diff --git ./src/wasm-wasi-component/src/lib.rs ./src/wasm-wasi-component/src/lib.rs
index b0552e81..0df33914 100644
--- ./src/wasm-wasi-component/src/lib.rs
+++ ./src/wasm-wasi-component/src/lib.rs
@@ -256,6 +256,7 @@ impl GlobalState {
                 // shouldn't get raw access to stdout/stderr.
                 cx.inherit_stdout();
                 cx.inherit_stderr();
+                cx.inherit_env();
                 for dir in self.global_config.dirs.iter() {
                     let fd = Dir::open_ambient_dir(dir, ambient_authority())
                         .with_context(|| {

But the wasm-wasi-component language module doesn't build with it...

$ make wasm-wasi-component install
cargo build --release --manifest-path src/wasm-wasi-component/Cargo.toml
   Compiling wasm-wasi-component v0.1.0 (/home/andrew/src/unit/src/wasm-wasi-component)
error[E0599]: no method named `inherit_env` found for struct `wasmtime_wasi::preview2::WasiCtxBuilder` in the current scope
   --> src/lib.rs:259:20
    |
259 |                 cx.inherit_env();
    |                    ^^^^^^^^^^^ method not found in `WasiCtxBuilder`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `wasm-wasi-component` (lib) due to 1 previous error
make: *** [build/Makefile:2151: src/wasm-wasi-component/target/release/libwasm_wasi_component.so] Error 101

Yet according to the docs (as you pointed to) it is a thing!

I can add an environment variable via cx.env();, but I don't know why it can't find .inherit_env()

Pinging someone who knows rust...

Cc: @avahahn

callahad · 2024-06-25T16:11:37Z

inherit_env was added to the wasmtime-wasi crate in version 20.0.0; we're still locked to 17.0.0.

This is reason enough to go ahead and work on updating our Rust deps now.

ac000 · 2024-08-28T02:10:16Z

Hi @RobbieMcKinstry

We are planning to do a 1.33.0 release real soon now(tm) and I'm planning on including this patch (attributed to yourself) once the language module has been updated to a newer wasmtime crate.

Author: Robbie McKinstry <[email protected]>
Date:   Wed Aug 28 03:00:43 2024 +0100

    wasm-wc: Enable environment inheritance
    
    While the C based wasm language module inherits the process environment
    the Rust based wasm-wasi-component language module did not.
    
    One upshot of this is that with wasm-wasi-component you don't get access
    to any environment variables specified in the Unit configuration.
    
    wasm-wasi-component was based on wasmtime 17.0.0. This capability wasn't
    added to the wasmtime-crate until version 20.0.0.
    
    Now that wasm-wasi-component has been updated to a newer wasmtime-crate
    we can enable this functionality.
    
    Closes: https://github.com/nginx/unit/issues/1312
    [ Commit message - Andrew ]
    Signed-off-by: Andrew Clayton <[email protected]>

diff --git ./src/wasm-wasi-component/src/lib.rs ./src/wasm-wasi-component/src/lib.rs
index b0552e81..0df33914 100644
--- ./src/wasm-wasi-component/src/lib.rs
+++ ./src/wasm-wasi-component/src/lib.rs
@@ -256,6 +256,7 @@ impl GlobalState {
                 // shouldn't get raw access to stdout/stderr.
                 cx.inherit_stdout();
                 cx.inherit_stderr();
+                cx.inherit_env();
                 for dir in self.global_config.dirs.iter() {
                     let fd = Dir::open_ambient_dir(dir, ambient_authority())
                         .with_context(|| {

If you're OK with that could you please simply reply with your Signed-off-by: i.e.

Signed-off-by: Robbie McKinstry <[email protected]>

Cheers,
Andrew

While the C based wasm language module inherits the process environment the Rust based wasm-wasi-component language module did not. One upshot of this is that with wasm-wasi-component you don't get access to any environment variables specified in the Unit configuration. wasm-wasi-component was based on wasmtime 17.0.0. This capability wasn't added to the wasmtime-crate until version 20.0.0. Now that wasm-wasi-component has been updated to a newer wasmtime-crate we can enable this functionality. Closes: nginx#1312 [ Commit message - Andrew ] Signed-off-by: Andrew Clayton <[email protected]>

RobbieMcKinstry · 2024-09-04T22:08:34Z

You nailed it:

Signed-off-by: Robbie McKinstry <[email protected]>

Sorry for the delay, but it appears you've already gotten it correct! It's generous of you to include me in the contribution! Thank you very much for the fix! ❤️

ac000 · 2024-09-04T22:19:14Z

Yes, unfortunately just too late for the s-o-b, but you are down as the Author. You did track down and suggest exactly what was missing!

RobbieMcKinstry changed the title ~~Environment Variables not passed to Wasm Applications~~ BUG: Environment Variables not passed to Wasm Applications Jun 11, 2024

ac000 mentioned this issue Aug 15, 2024

Upgrade wasm-wasi-component to latest Wasmtime #1392

Closed

This was referenced Aug 28, 2024

Release 1.33 meta tasks #1399

Closed

bump wasmtime to v24 #1406

Merged

ac000 linked a pull request Sep 4, 2024 that will close this issue

bump wasmtime to v24 #1406

Merged

avahahn closed this as completed in 56c237b Sep 4, 2024

avahahn closed this as completed in #1406 Sep 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Environment Variables not passed to Wasm Applications #1312

BUG: Environment Variables not passed to Wasm Applications #1312

RobbieMcKinstry commented Jun 11, 2024 •

edited

Loading

ac000 commented Jun 11, 2024

RobbieMcKinstry commented Jun 11, 2024

ac000 commented Jun 18, 2024

callahad commented Jun 25, 2024

ac000 commented Aug 28, 2024 •

edited

Loading

RobbieMcKinstry commented Sep 4, 2024 •

edited

Loading

ac000 commented Sep 4, 2024

BUG: Environment Variables not passed to Wasm Applications #1312

BUG: Environment Variables not passed to Wasm Applications #1312

Comments

RobbieMcKinstry commented Jun 11, 2024 • edited Loading

About

Suspected Cause

Code Sample

ac000 commented Jun 11, 2024

RobbieMcKinstry commented Jun 11, 2024

ac000 commented Jun 18, 2024

callahad commented Jun 25, 2024

ac000 commented Aug 28, 2024 • edited Loading

RobbieMcKinstry commented Sep 4, 2024 • edited Loading

ac000 commented Sep 4, 2024

RobbieMcKinstry commented Jun 11, 2024 •

edited

Loading

ac000 commented Aug 28, 2024 •

edited

Loading

RobbieMcKinstry commented Sep 4, 2024 •

edited

Loading