JuliaContainerization / Sandbox.jl

The cultured host's toolkit for ill-mannered Linux guests.
Other
35 stars 5 forks source link

Work around issue with persistent dirs with `UnprivilegedUserNamespacesExecutor` #87

Closed giordano closed 1 year ago

giordano commented 2 years ago

This is still quite not working for me, but opening for feedback.

This will eventually fix #77.

giordano commented 2 years ago

So, with the changes in this PR, this example

using Sandbox, Scratch, UserNSSandbox_jll
@show UserNSSandbox_jll.sandbox_path
mktempdir() do rw_dir
    config = SandboxConfig(
        Dict("/" => Sandbox.debian_rootfs()),
        Dict("/tmp/rw_dir" => rw_dir),
        Dict("HOME" => "/root");
        persist=true,
    )
    with_executor(UnprivilegedUserNamespacesExecutor) do exe
        exe.persistence_dir = @get_scratch!("tmpfs_xattr_workaround")
        success(exe, config, `/bin/sh -c "apt update && apt install -y curl"`)
    end
end

works for me, but the tests still fail with the same error as in #77, which very much confuses me. (Yes, I did set ENV["SANDBOX_BUILD_LOCAL_SANDBOX"] = "true" and the path of the runner printed during the tests is the one from the scratchspace.)

Edit: nevermind, I had applied exe.persistence_dir = @get_scratch!("tmpfs_xattr_workaround") to the wrong test :facepalm:

codecov[bot] commented 2 years ago

Codecov Report

Patch coverage: 65.00% and project coverage change: -0.91 :warning:

Comparison is base (02edd8a) 79.61% compared to head (58d8a6c) 78.70%.

:exclamation: Current head 58d8a6c differs from pull request most recent head 2893e0d. Consider uploading reports for the commit 2893e0d to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #87 +/- ## ========================================== - Coverage 79.61% 78.70% -0.91% ========================================== Files 6 6 Lines 569 620 +51 ========================================== + Hits 453 488 +35 - Misses 116 132 +16 ``` | [Impacted Files](https://codecov.io/gh/staticfloat/Sandbox.jl/pull/87?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Elliot+Saba) | Coverage Δ | | |---|---|---| | [src/utils.jl](https://codecov.io/gh/staticfloat/Sandbox.jl/pull/87?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Elliot+Saba#diff-c3JjL3V0aWxzLmps) | `66.92% <57.44%> (-4.01%)` | :arrow_down: | | [src/UserNamespaces.jl](https://codecov.io/gh/staticfloat/Sandbox.jl/pull/87?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Elliot+Saba#diff-c3JjL1VzZXJOYW1lc3BhY2VzLmps) | `82.56% <85.71%> (+0.66%)` | :arrow_up: | | [src/SandboxConfig.jl](https://codecov.io/gh/staticfloat/Sandbox.jl/pull/87?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Elliot+Saba#diff-c3JjL1NhbmRib3hDb25maWcuamw=) | `93.10% <100.00%> (+1.79%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://codecov.io/gh/staticfloat/Sandbox.jl/pull/87/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Elliot+Saba) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Elliot+Saba). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Elliot+Saba)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

giordano commented 2 years ago

Fun, now the tests with local build of the userns sandbox are failing with Invalid cross-device link errors

image

staticfloat commented 1 year ago

Our efforts to solve this have been stymied by a number of confounding factors, but I think I may have finally wrangled it. The issues include:

I've made some changes here to default to persist = true. Not only is it a cool and useful feature with very little overhead, it makes it a lot easier on userxattr systems. We have a fancy new search algorithm for a directory that is able to be mounted against, which should aid us in nested sandboxing.

@giordano can you check locally that this works for you? I'm passing tests on a 6.1 Arch kernel in an EC2 instance, but you're my most important Arch customer. :)

Because we're changing the persist default here, I think this is worth bumping version numbers over. There's also a significant amount of code here, so I may want to take a second pass in a few days to try and re-organize things a bit.

staticfloat commented 1 year ago