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

sandboxed environment for lua scripts and REPL #258

Closed
catfact opened this issue Apr 28, 2018 · 9 comments
Closed

sandboxed environment for lua scripts and REPL #258

catfact opened this issue Apr 28, 2018 · 9 comments

Comments

@catfact
Copy link
Collaborator

@catfact catfact commented Apr 28, 2018

i think we can and should use a dedicated table as the pseudo-global environment for executing the current script and REPL chunks. that way users can use globals without fear.

https://stackoverflow.com/questions/35910099/how-special-is-the-global-variable-g
http://lua-users.org/wiki/EnvironmentsTutorial
http://www.lua.org/manual/5.3/manual.html#pdf-_G

@catfact
Copy link
Collaborator Author

@catfact catfact commented May 18, 2018

@tehn: > basically every script is going to use screen and engine etc. so making every script have two extra lines at the top seems pretty unneccessary

i agree. if we can use a dedicated environment for scripts, we can populate it with globals at the same time.

@tehn
Copy link
Member

@tehn tehn commented May 18, 2018

so if we are to (eventually) sandbox the environment and require more explicit requires (both good things) should we care as much about globals?

@catfact
Copy link
Collaborator Author

@catfact catfact commented May 18, 2018

sandboxing will solve both things at once.

but if we don't have system globals, then script globals aren't as much of an issue as far as name collisions / bugs, even without a sandbox. they still hang around and use memory though. sandbox can be cleaned out of memory on script deinit.

the question i still have is, what's better - getting people to use good lua code style, or making a sort of complex environment that will protect them from the consequences of bad style?

@tehn
Copy link
Member

@tehn tehn commented May 18, 2018

good style is probably best, and we should suggest this in the short term given the sandboxing isn't going to happen overnight :)

@antonhornquist
Copy link
Collaborator

@antonhornquist antonhornquist commented May 18, 2018

if we remove as many system globals as possible, regular globals (_G or _ENV or whatever) could constitute ”the sandbox”, right? that is, _G could be cleaned on deinit.

this seems to be the way love2d kinda works (i thought love2d user stuff was namespaced too, but seems only callbacks/core stuff)

@catfact
Copy link
Collaborator Author

@catfact catfact commented May 18, 2018

the sandboxing isn't going to happen overnight :)

why not? it should only require a small change to script.lua.

just need to run some tests to make sure we know how to do it correctly

regular globals (_G or _ENV or whatever) could constitute ”the sandbox”, right? that is, _G could be cleaned on deinit.

i guess so, but really, lua has mechanisms for exactly this. the load and loadfile functions take an optional environment argument: loadfile ([filename [, mode [, env]]])

so we do something (very approximately) like

local sandbox
function run_sandboxed_script(filename)
   sandbox = _G --- or something; populate sandbox with stuff that we want to be "global" for scripts
   loadfile(filename, "bt", sandbox)   -- "bt" == "binary and text"
end

function cleanup_script() 
  -- run script cleanup function; script has defined as "global" but is really in sandbox
   sandbox.cleanup()
   sandbox = {} -- trigger GC on sandbox
end
@pq
Copy link
Collaborator

@pq pq commented Jun 2, 2018

more food for thought: if i understand the implications, i think sandboxing could also make development of scripts with module dependencies easier[*] and lessen the need for some kind of module re-load facility in maiden (monome/maiden#22)

[*] tl;dr: the rub is that since modules are not automatically reloaded, changes to them do not get picked up in dependent scripts/modules leading to potential confusion when making changes.

@ngwese
Copy link
Member

@ngwese ngwese commented Jun 2, 2018

for user modules (those in dust) one possible way to handle this is to monkey patch the require function in the sandboxed environment allowing us to keep track of modules loaded explicitly by a user script... once we know what was loaded we can certainly clear out the package.loaded global as needed.

system modules (part of norns) are still problematic but given that they are loaded in multiple places potentially.

the evaluation environment for the repl commands might be the more challenging aspect since the lifetime of the repl spans multiple scripts

@tehn
Copy link
Member

@tehn tehn commented Oct 2, 2018

primary fix #436

closing this, but referencing #557 #559

@tehn tehn closed this Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants