add actor start from local file

[pgray]

Haven't written elixir before so I wanted to try with a tiny feature add.
This would allow a wasmcloud host to start an actor from the local filesystem.
Will try to add tests soon.

[pgray]

wash ctl start actor file://path/to/file_s.wasm works

having an issue with wash ctl update actor $hostid $actorid file://path/to/file_s.wasm

[jordan-rash]

This is an awesome feature that I have wanted for a while. That being said, i like the approach that wash picks up the file and streams the bytes of the wasm module to the host. This way, you can start "local" actors/providers on remote host without having to scp them over.

Selfishly, that opens the door to another feature I have wanted, which is pulling artifacts over the lattice instead of having the host reach out directly to the internet

[pgray]

I think that feature should exist too.
This would still be faster for local development though because you avoid the network roundtrip or, really, any copying whatsoever... just a disk read between you and your new code running 😆

[autodidaddict]

There are some subtle security and distributed systems caveats with this feature.

From a distsys perspective, if you send a control interface command to one host with a file path, that same request delivered to a different host could fail because the path doesn't exist, or it could fail because the host doesn't even have an accessible local file system (think IoT or browser host). Once the actor is running in that host, no other host has convenient access to the actor bytes, so things like scaling an actor to multiple hosts or performing a live update can become problematic.

From a security point of view, if the host is able to perform arbitrary file reads at the request of a control client, then the target host can become vulnerable to DDoS or things like "zip bomb" or other types of attacks. Further, the host has to read the file off of disk before getting its claims to run through the policy checker. This is a less secure option than having wash give the host an OCI or bindle URL.

I'm not saying we shouldn't do this. I definitely think that we should be able to type something like wash ctl start ./foo.wasm or some such, but I worry about implementing it by having the host read bytes from the file system. While it could make things easier for developers working on a local iteration loop, it could cause any number of problems in a production environment.

I wonder if there's a secure way of implementing this from the developer's point of view without making the host read the file system and still have this work across the lattice.

One possible way of having it both ways would be to add an environment variable like WASMCLOUD_DISABLE_FILESTART, which would then reject any control interface attempt to load an actor using file://. With that in place, people could disable this feature in production but allow it in development environments. 🤔

[pgray]

I totally agree.

This feature, as implemented, should not be used in a production environment.

How about I rework it to only work if WASMCLOUD_ENABLE_LOCALFILE is set to true?
Then we could either set it by default when you run wash up or do something like wash up --local?
Is wash up something we'd expect someone to run in production?

[brooksmtownsend]

I like the idea of having this be disabled by default for wasmCloud, but enabled by default for wash which is designed as a dev tool.

As far as wash being used in production, our recommendation is to instead run NATS and the wasmCloud host on their own. However, we should revisit this and see what we could do. I think we've made solid decisions for wash like making every command have a JSON output option so it can be used with scripts.

[brooksmtownsend]

What are your thoughts on, before reading the file reference, examining the metadata of the file to make sure it's under some size limit? I'm not sure if that would mitigate zip bombs or just limit actor size (which we could comfortably cap at like 5MB)

[jordan-rash]

Playing devils advocate here, the file:// is no more dangerous than the oci:// for something like a zip/fork bomb, the pushing to a registry is just an extra step. If wash ctl start ./foo were to add a JWT check and compares it to the keys allowed to run on the host, I think the security risk is mitigated. If we are trying to minimize the development loop, I would think a setting like WASH_DISABLE_LOCAL_KEYCHECK or whatever would be a solid choice.

[jordan-rash]

From nats bench, I dont think, as long as the work is local only, that the size of the file is going to be an issue

╰─❯ nats bench foo --size="1M" --msgs=1 --pub 1 --sub 1
NATS Pub/Sub stats: 421 msgs/sec ~ 421.70 MB/sec
 Pub stats: 281 msgs/sec ~ 281.87 MB/sec
 Sub stats: 1,199,040 msgs/sec ~ 1.14 TB/sec

╰─❯ nats bench foo --size="1M" --msgs=5 --pub 1 --sub 1
NATS Pub/Sub stats: 592 msgs/sec ~ 592.49 MB/sec
 Pub stats: 303 msgs/sec ~ 303.88 MB/sec
 Sub stats: 456 msgs/sec ~ 456.41 MB/sec

╰─❯ nats bench foo --size="1M" --msgs=10 --pub 1 --sub 1
NATS Pub/Sub stats: 705 msgs/sec ~ 705.98 MB/sec
 Pub stats: 374 msgs/sec ~ 374.13 MB/sec
 Sub stats: 462 msgs/sec ~ 462.14 MB/sec

╰─❯ nats bench foo --size="1M" --msgs=50 --pub 1 --sub 1
NATS Pub/Sub stats: 879 msgs/sec ~ 879.32 MB/sec
 Pub stats: 445 msgs/sec ~ 445.66 MB/sec
 Sub stats: 472 msgs/sec ~ 472.46 MB/sec

[pgray]

huzzah, got updates working and tested scale as well

need to add tests, but would love any guidance if there's things to fix

cheers 😸

wash ctl update actor $hostid $actorid file://$PWD/build/boom_s.wasm

Actor MCQSARSPJH4SNSMXL5WUTN5JNIILPPIRWNYMHBFMVLJQ7ZMSMGDQVJ4X updated to file:///home/pgray/wc/boom/build/boom_s.wasm```

[pgray]

I think this is finally ready.
Please let me know if there's anything else you'd like me to add 😄

[brooksmtownsend]

Mostly nits!

[pgray]

addressed nits. sorry for the the force-pushing 🙃

[brooksmtownsend]

Nice add @pgray !

[nihil2501]

@brooksmtownsend @pgray I noticed a typo introduced in this commit as I was looking through the codebase.

I'm investigating locally for funsies and experience as we speak, but figured that I ought also to flag you now in case someone will want to spin out a new issue or whatever.

(I haven't yet tried to witness any associated regression.)

[brooksmtownsend]

Oh boy, that's a great catch @nihil2501. Thankfully this is in a relatively niche area of the codebase with a config service, but your PR and catch is super helpful. Thank you