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

Run each Lua chunk in an inherited, private environment #1593

Closed
wants to merge 1 commit into from

Conversation

pmatilai
Copy link
Member

Protect our environment against accidents and pollution from each other:
each Lua chunk starts with an environment inherited from rpm global, but
any modifications go to the private environment so it's not possible to
accidentally destroy or divert things.

Protect our environment against accidents and pollution from each other:
each Lua chunk starts with an environment inherited from rpm global, but
any modifications go to the private environment so it's not possible to
accidentally destroy or divert things.
@pmatilai pmatilai added the lua Lua bindings/interface label Mar 23, 2021
@DemiMarie
Copy link
Contributor

This will break code that defines Lua functions for later use. Is this intentional? Also, it is still possible to monkeypatch various global tables.

@pmatilai
Copy link
Member Author

The idea is to stop accidental pollution of the Lua environment. Actual hardening is a separate matter.
And yeah, some breakage to be expected if people are (ab)using this. Also possible we need further tweaks.

@Conan-Kudo
Copy link
Member

Wouldn't this entirely break the forge macros and Go macros in Fedora, as well as the Python and Ruby macros in openSUSE?

@pmatilai
Copy link
Member Author

pmatilai commented Apr 8, 2021

Would it?

If you're unsure what an implementation does, it'd be more helpful to actually test it.

@Conan-Kudo
Copy link
Member

I'll try to test it this weekend and find out. I only gave a first pass look and got a bit nervous.

@mlschroe
Copy link
Contributor

mlschroe commented Apr 8, 2021

I'm a lua novice, so pardon my ignorance. Why does

--eval '%{lua:rpm=nil; print(rpm == nil)}' \

evaluate to false in the testcase? And the test is currently skipped in the CI, probably due to that AT_SKIP_IF([$LUA_DISABLED]). Didn't we make lua mandatory?

@pmatilai
Copy link
Member Author

The nil thing is indeed a Lua peculiarity with globals and how the environment is manipulated here, it basically prevents you from (accidentally) doing something nasty to the environment. https://www.lua.org/pil/14.3.html is technically outdated but explains the basic idea.

This patch is originally from last autumn when Lua was still optional, LUA_DISABLED is just a leftover and needs removing.

This also needs more thought than this... closing for now.

@pmatilai pmatilai closed this Apr 12, 2021
@mlschroe
Copy link
Contributor

Thanks for the pointer!

@pmatilai pmatilai deleted the luaenv-pr branch June 21, 2021 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lua Lua bindings/interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants