monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
614 stars 144 forks source link

sandboxed environment for lua scripts and REPL #258

Closed catfact closed 5 years ago

catfact commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

antonhornquist commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 (https://github.com/monome/maiden/issues/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 commented 6 years ago

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 commented 5 years ago

primary fix https://github.com/monome/norns/pull/436

closing this, but referencing #557 #559