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

Allow modules to optionally import nothing at all #40110

Merged
merged 4 commits into from
Jun 10, 2021

Conversation

cmcaine
Copy link
Contributor

@cmcaine cmcaine commented Mar 19, 2021

See: https://discourse.julialang.org/t/even-more-bare-baremodule/56156/

A module that imports nothing at all can be used to build a sandbox. Sandboxes are useful for defining smaller teaching languages (see Racket and the HTDP2e book) and, if they can be made secure, for evaluating untrusted code in a capability programming paradigm.

This commit changes the API of libjulia:

Old:

JL_DLLEXPORT jl_value_t *jl_f_new_module(jl_sym_t *name, uint8_t std_imports)

New:

JL_DLLEXPORT jl_value_t *jl_f_new_module(jl_sym_t *name, uint8_t std_imports, uint8_t using_core)

With thanks to Simeon Schaub and Mason Protter.

Questions for triage:

  • Okay to change the libjulia API or would you rather I added a new function?

I came up with slightly better docs, too, but to save CI energy I haven't pushed them yet. But if anyone wants to bikeshed the docs, here they are:

In NEWS.md:

In doc/src/manual/modules.md:

If modules that contain no imports at all are needed, they can be defined with Mod = Module(:Mod, false, false). Code can be evaluated in them with @eval or Core.eval.

See: https://discourse.julialang.org/t/even-more-bare-baremodule/56156/

A module that imports nothing at all can be used to build a sandbox.
Sandboxes are useful for defining smaller teaching languages (see Racket
and the HTDP2e book) and, if they can be made secure, for evaluating
untrusted code in a capability programming paradigm.

This commit changes the API of libjulia:

Old:

    JL_DLLEXPORT jl_value_t *jl_f_new_module(jl_sym_t *name, uint8_t std_imports)

New:

    JL_DLLEXPORT jl_value_t *jl_f_new_module(jl_sym_t *name, uint8_t std_imports, uint8_t using_core)

Co-authored-by: Simeon Schaub <[email protected]>
@oscardssmith oscardssmith added the triage This should be discussed on a triage call label Mar 19, 2021
@@ -36,7 +36,7 @@ JL_DLLEXPORT jl_module_t *jl_new_module(jl_sym_t *name)
htable_new(&m->bindings, 0);
arraylist_new(&m->usings, 0);
JL_GC_PUSH1(&m);
if (jl_core_module) {
if (jl_core_module && using_core) {
jl_module_using(m, jl_core_module);
}
// export own name, so "using Foo" makes "Foo" itself visible
Copy link
Member

Choose a reason for hiding this comment

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

We might want to omit this as well, just so you can get a truly truly empty module.

Copy link
Member

Choose a reason for hiding this comment

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

Triage seconds this.

Copy link
Member

Choose a reason for hiding this comment

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

And then I guess using_core should be renamed default_names or something like that? Maybe not important.

@JeffBezanson
Copy link
Member

Here's one of probably many ways to escape the sandbox:

t(x::T) where T = T
t(1).name.module # gives you Core

@@ -11,7 +11,7 @@
extern "C" {
#endif

JL_DLLEXPORT jl_module_t *jl_new_module(jl_sym_t *name)
JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, uint8_t using_core)
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to put this into julia.h as well?

@vtjnash
Copy link
Member

vtjnash commented Mar 19, 2021

Yeah, this'll never be an actual approach to making a sandbox (for example, you can also use ccall, various expressions that lower to top calls or constants, or simply call using), but not necessarily a reason to avoid this.

Also, ref #39114

@JeffBezanson
Copy link
Member

If we wanted to gradually strengthen the sandbox, probably the next step is to make all identifiers that come from syntax (e.g. a.b => getproperty) lexically-scoped, i.e. lower them to __getproperty__ instead of Base.getproperty. Then you could e.g. inject a definition that disallows accessing some fields. I would kind of like to be able to do that anyway, though of course it's only one more brick in the sandbox wall.

@cmcaine
Copy link
Contributor Author

cmcaine commented Mar 21, 2021

Thanks for the review, everyone!

Perhaps I should have been clearer in the first post, I don't think that this feature on its own makes a sandbox, this is just part of what you'd need to do it nicely.

You can see my experiments in the discourse thread linked at the top and in https://github.com/cmcaine/Sandboxes.jl

The strategy I'm using there takes an Expr and rewrites it to be safe (or just errors). That covers most of @vtjnash's issues (I don't yet rewrite GlobalRefs, but that's possible with this approach, and I wrote code to control what literals can be created (in the discourse thread), too).

I think rewriting a.b to use a custom getproperty is a good idea too, I was looking at doing that to start with, but was able to catch everything I knew about without it, but I didn't know DataTypes have name.module! I can prototype that in Sandboxes.jl :)

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Mar 25, 2021
@cmcaine
Copy link
Contributor Author

cmcaine commented Mar 27, 2021

I think it's a bit weird that we allow users to say yes to std_imports and no to default_names. It works, but I just don't think users will ever want that, so it might be confusing.

Perhaps a better interface would be to accept a symbol or enum or something as the second argument, at least on the Julia side?

function Module(name::Symbol, type::Symbol)
    if type == :module ...
    elseif type == :baremodule ...
    elseif type == :emptymodule ...
    else error()
    end
end

?

Edit: Jeff suggested passing nothing to std_imports, which also makes sense to me, I think I'll do that.

@simeonschaub
Copy link
Member

simeonschaub commented Apr 3, 2021

Since this is a pretty internal method, I think it's fine to just have two boolean arguments. I don't really have strong opinions on how to name this option.

@Acmion
Copy link

Acmion commented Jun 7, 2021

I see some use cases for applying the proposed import rules to the Main module as well.

While this is not directly related to this pull request, it is something that could be considered for the future.

@simeonschaub simeonschaub added the merge me PR is reviewed. Merge when all tests are passing label Jun 8, 2021
@DilumAluthge DilumAluthge reopened this Jun 9, 2021
@DilumAluthge
Copy link
Member

@simeonschaub CI is all green now. Is this good to merge?

@simeonschaub simeonschaub merged commit c22b718 into JuliaLang:master Jun 10, 2021
@simeonschaub
Copy link
Member

Thanks @cmcaine for the contribution! Sorry it took so long to get merged.

@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Jun 18, 2021
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
See: https://discourse.julialang.org/t/even-more-bare-baremodule/56156/

A module that imports nothing at all can be used to build a sandbox.
Sandboxes are useful for defining smaller teaching languages (see Racket
and the HTDP2e book) and, if they can be made secure, for evaluating
untrusted code in a capability programming paradigm.

This commit changes the API of libjulia:

Old:

    JL_DLLEXPORT jl_value_t *jl_f_new_module(jl_sym_t *name, uint8_t std_imports)

New:

    JL_DLLEXPORT jl_value_t *jl_f_new_module(jl_sym_t *name, uint8_t std_imports, uint8_t using_core)

Co-authored-by: Simeon Schaub <[email protected]>
cmcaine added a commit to cmcaine/julia that referenced this pull request Sep 10, 2021
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.

7 participants