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

Make wasmedge preopen the container root #265

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

jprendes
Copy link
Collaborator

With #78 we inadvertedly stopped preopening the container root.
This means that the wasmedge shim doesn't have access to the container filesystem.
This PR reintroduces preopening the container root with wasmedge.

@jprendes jprendes requested review from Mossaka and utam0k August 23, 2023 11:42
@devigned
Copy link
Contributor

In the long run, pre-opening anything may be a bad (default) choice. We need to think about how pre-opens and other access / capabilities should be described.

@jprendes
Copy link
Collaborator Author

This is preopening the container root, not the host root.
@devigned what use case(s) did you have in mind for configuring preopening?

@devigned
Copy link
Contributor

My comment is not about container or host fs. It's more that by default many runtimes do not enable fs access unless specified; fs access is off by default. We are defaulting the root container fs to be pre-opened for convenience of the user. Some users may not want to have any fs access at all.

I'm just calling this out to bring it to attention of the broader audience. As for this PR, I think we should continue to pre-open. However, we may want to rethink this in the future.

Copy link
Contributor

@devigned devigned left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@devigned devigned merged commit e9b86b0 into containerd:main Aug 23, 2023
@jprendes
Copy link
Collaborator Author

It's a valid point that we should consider.
I'm just struggling to understand the use case. I woud imagine that if you don't want fs access you just don't include any file in your image root (except your wasm file, until we move that to artifacts). Am I missing something?

@jprendes jprendes deleted the fix-wasmedge-root branch August 23, 2023 13:30
@devigned
Copy link
Contributor

It's application of least privileges (capabilities) for Wasm. For example, you might run some untrusted, 3rd party Wasm app, and want the runtime sandbox to not provide access to the fs.

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

Successfully merging this pull request may close these issues.

3 participants