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

Insulation is not very robust #534

Open
peabnuts123 opened this issue Sep 10, 2016 · 4 comments
Open

Insulation is not very robust #534

peabnuts123 opened this issue Sep 10, 2016 · 4 comments

Comments

@peabnuts123
Copy link

The sandboxing concept of "Insulation" in Busted is nice but seems to promise a bit more than it offers. What is sold as a snapshot of your environment that is restored upon exiting an Insulation block boils down to three things

  1. The metatable of _G
  2. _G itself
  3. the contents of package.loaded

This means that there are still ways to leak across the "border" of insulation. One example is through properties of globals within _G, whose state is not captured/restored, as demonstrated below:

describe("Insulate bug", function()
    function polluteGlobalObject()
        function string.someGlobal()
            print("someGlobal is being called")
        end
    end

    function polluteGlobalNamespace()
        _G.someGlobal = "Some Global"
    end

    insulate("- Insulated Block A", function()
        polluteGlobalNamespace()
        polluteGlobalObject()

        it("Can see modified global object", function()
            assert.is_function(string.someGlobal)
        end)

        it("Can see global namespace object", function()
            assert.is_string(_G.someGlobal)
        end)
    end)

    insulate("- Insulated Block B", function()
        it("Cannot see modified global object", function() -- THIS TEST FAILS
            assert.is_nil(string.someGlobal) 
        end)

        it("Cannot see global namespace object", function()
            assert.is_nil(_G.someGlobal)
        end)
    end)    
end)

The ramifications of this are that tests can interfere with each other, despite being within insulate blocks, or even different files. There should be a more robust way of capturing/restoring the state of the environment, perhaps even running separate instances of Lua to ensure "true" sandboxing.

@DorianGray
Copy link
Contributor

I closed the other issue to continue discussion here.

@alerque
Copy link
Member

alerque commented Aug 25, 2022

My current PR #654 has a commit at the start that fixes a bug actually unrelated to the PR but more related to this issue. It came up in the discussion on #535. Obviously it is nowhere to a complete fix, but it plugs one more small hole.

Unfortunately with out top level language features to facilitate this the amount of deep-copy shenanigans we would have to do to really and truly sandbox this is probably a never-ending game of whack-a-mole.

I'm happy to facilitate PRs which improve the state of affairs, but am skeptical that the sieve can be made completely water-tight with any amount of effort. Pragmatically I'd help address real world cases, but don't have time to contribute as a research project fixing theoretical problems.

@tmillr
Copy link

tmillr commented Feb 6, 2024

Invoking a new host/process for each test file would be nice, and/or adding a new variant of describe() which achieves this (so that a single file could be split across multiple envs/processes). But then handling test output and status becomes more difficult I suppose. The current method of sandboxing is kinda confusing (where is sandboxing occurring? and what kind of sandboxing exactly?) and falls short in sandboxing scope (especially when Lua is embedded and is accessing state that is internal to the process, but external to Lua).

@tmillr
Copy link

tmillr commented Feb 6, 2024

Using the default options, it also seems that the behavior of these 2 differ:

describe('', function()
    function Global() end
end)
describe('', function()
    function _G.Global() end
end)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants