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

Help with openidconnect-rs #790

Closed
anderspitman opened this issue Nov 21, 2024 · 23 comments
Closed

Help with openidconnect-rs #790

anderspitman opened this issue Nov 21, 2024 · 23 comments

Comments

@anderspitman
Copy link

anderspitman commented Nov 21, 2024

I'm having trouble using openidconnect-rs in an extism plugin. I can't even import the library. I'm getting this error:

Uncaught (in promise) Error: from module "main": cannot resolve import "__wbindgen_placeholder__" "__wbindgen_describe": not provided by host imports nor linked manifest items

Here's a minimal repro:

use extism_pdk::{plugin_fn,FnResult};
use openidconnect;

#[plugin_fn]
pub extern "C" fn test() -> FnResult<()> {
    Ok(())
}

And a full working repro repo: https://github.com/anderspitman/wasmrepro

To run that:

cargo build --target wasm32-unknown-unknown
deno run --allow-all main.js

Any ideas?

@nilslice
Copy link
Member

it's likely that this library is being "smart" and assuming the target "wasm32-unknown-unknown" is going to run in JS environment. As such, they use wasm-bindgen to access fetch and other functions.

Can you try compiling to the "wasm32-wasi" target instead? And if that doesn't do it, then there may be some features you need to disable to get it compiling correctly.

@nilslice
Copy link
Member

also, Deno 2 removed wasi support, so if you can, run a Deno 1 version or use Node instead.

@anderspitman
Copy link
Author

node is giving me Error: WASI is not enabled; see the "wasiEnabled" plugin option

@anderspitman
Copy link
Author

anderspitman commented Nov 21, 2024

Here's my updated SDK code:

import createPlugin from '@extism/extism';

const plugin = await createPlugin(
  "./target/wasm32-wasip1/debug/wasmrepro.wasm",
  {
    useWasi: true,
  },
);

const out = await plugin.call("test");
console.log(out);

plugin.close();

@chrisdickinson
Copy link
Contributor

chrisdickinson commented Nov 21, 2024

It looks like openidconnect-rs is setting the wasmbind feature on its chrono dep. This brings in wasm-bindgen bindings, which are a non-standard set of host functions (sort of like how emscripten brings in a set of host bindings particular to the browser.)

Because of the additive nature of cargo features, I think we'd have to fork openidconnect-rs to turn that feature off; I'll do that and see if that improves matters!

Edit, ope, I'm wrong – it looks like chrono does the right thing when the target is wasm32-wasi.

@chrisdickinson
Copy link
Contributor

@anderspitman Ah, it looks like there was a typo in the js-sdk error message – thanks for catching that! Fixed in extism/js-sdk@2693847

I tried the example sdk code and it worked locally – are you still seeing errors?

@anderspitman
Copy link
Author

Argh I had renamed the file from main.js to main.mjs and was still editing main.js in vim. Definitely the first time I've ever done that... Sorry for wasting your time on that

@anderspitman
Copy link
Author

Ok current problem is that node doesn't support web workers... Trying to get the web-worker module to work but no luck so far

@anderspitman
Copy link
Author

Downgraded to Deno 1.46.3. Now getting:

error: Uncaught (in worker "") (in promise) Error: Unhandled error. ([Object: null prototype] {
  message: 'Uncaught (in promise) Error: WASI is not supported on this platform',
  fileName: 'https://jsr.io/@extism/extism/2.0.0-rc10/src/foreground-plugin.ts',
  lineNumber: 188,
  columnNumber: 17
})

I feel like we're so close.

@nilslice
Copy link
Member

can you try again with the runInWorker property set to true? This is alongside the useWasi setting.

https://extism.github.io/js-sdk/interfaces/ExtismPluginOptions.html#runInWorker

@nilslice
Copy link
Member

You'll also need to set allowedHosts if there is an outbound HTTP call:

https://extism.github.io/js-sdk/interfaces/ExtismPluginOptions.html#allowedHosts

@anderspitman
Copy link
Author

Yeah I'm using both of those in my actual project. I think the problem with Deno 1.x is that the latest versions of extism disable the wasip1 capability, because Deno 2 doesn't support it.

I'm not sure how to make workers work with Node. It looks like it supports node:worker_threads, but I've never used that before.

@nilslice
Copy link
Member

I'm not sure how to make workers work with Node. It looks like it supports node:worker_threads, but I've never used that before.

Hm, I don't think you should need to do anything with workers yourself. The JS SDK should handle that for you when you add runInWorker: true. Do you have a more updated snippet you can share from your project and the resulting error you see when you try to call the plugin?

@anderspitman
Copy link
Author

Thanks for all the help. I originally moved the code into a separate web worker because that's the only way I could get it to work with Deno 2, which is my primary runtime. Sounds like maybe I need to focus on node for now. Let me move all the code back out of the web worker and see if the auto worker_threads implementation works in node.

@nilslice
Copy link
Member

nilslice commented Nov 21, 2024

We are working on patching Deno 2 support with a WASI implementation, but it's a little tricky. Hopefully you can use Node in the meantime, but feel free to track the issue here: extism/js-sdk#95

@chrisdickinson
Copy link
Contributor

Argh I had renamed the file from main.js to main.mjs and was still editing main.js in vim.

Argh, I've definitely been there before – commiserations!

I originally moved the code into a separate web worker because that's the only way I could get it to work with Deno 2, which is my primary runtime.

I'm curious about this– what errors were you seeing that were fixed by running @extism/extism in a worker? (Was it Deno capability permission related, or something else?)

@anderspitman
Copy link
Author

Ok, ditching the separate worker worked for node. I now have my plugin working with openidconnect-rs in JS and Golang hosts, with the library making outgoing HTTP requests perfectly. extism is so cool!

Thanks again for all the help.

@anderspitman
Copy link
Author

I'm curious about this– what errors were you seeing that were fixed by running @extism/extism in a worker? (Was it Deno capability permission related, or something else?)

Honestly I don't remember. I can try to reproduce it if it's helpful.

@nilslice
Copy link
Member

very glad to hear it! please keep in touch

@anderspitman
Copy link
Author

Oh one more question: is there any hope of getting wasm32-unknown-unknown to work? WASI feels heavy for what I'm doing, and it seems as if it might be less secure since it has a bigger API surface, but those fears might be unfounded.

It seems like I would need to go through the dependency tree and make sure everything plays nice with the other target?

@nilslice
Copy link
Member

Good question! I think it would be possible to remove the wasi dependency, but you'd have to update the openidconnect crate to conditionally make its HTTP requests using the Extism HTTP implementation (like you've used in your rust-pdk)

However, I'm not sure it's actually that much of a concern from a security standpoint (or a size either) since wasi is only host side code, it doesn't add any meaningful size to your wasm module. Just a few import functions for the wasi calls the module makes.

And, as you've found, you have to explicitly define which hosts, paths, and even enable header support for all external interactions the wasm code makes. So things are intentionally quite locked down even when enabling wasi.

If you'd like to explore converting the openidconnect dependency to use Extism's HTTP stack instead, we'd be happy to guide you -- I'm personally interested in seeing what that would take!

@anderspitman
Copy link
Author

anderspitman commented Nov 22, 2024

Makes sense. Although I think openidconnect-rs already doesn't rely on anything internal for making requests. You can just provide a function that takes a request and returns a response and it will use that. See here: https://docs.rs/openidconnect/latest/openidconnect/index.html#importing-openidconnect-selecting-an-http-client-interface

Though it does build in support for reqwest and curl for convenience, and maybe just having them in the crate is what's breaking things.

I'm fine sticking with WASI for now in the interest of moving forward, but I may revisit this in the future.

@nilslice
Copy link
Member

nilslice commented Nov 22, 2024

oh that's a great design!

then it seems it would be pretty straightforward to add a blocking / sync http call there. you'd convert types between the http request and response in that library to those from the extism rust-pdk, making the actual http call via the extism request.

the devils always in the details though ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants