Skip to content

feat: sandbox exec calls with bwrap and run container as non-root (minimally fixes #1873)#1940

Open
kinchahoy wants to merge 1 commit intoHKUDS:nightlyfrom
kinchahoy:main
Open

feat: sandbox exec calls with bwrap and run container as non-root (minimally fixes #1873)#1940
kinchahoy wants to merge 1 commit intoHKUDS:nightlyfrom
kinchahoy:main

Conversation

@kinchahoy
Copy link
Copy Markdown

@kinchahoy kinchahoy commented Mar 12, 2026

#1873

High level, this wraps exec calls with a light bubblewrap sandbox that prevents it from seeing outside the workspace directory (current workspace isolation does not properly protect exec calls). ~50 lines of code almost all in a distinct sandbox.py file

It also significantly improves the Docker version by enabling nanobot to run as non-root, and by integrating the official Podman seccomp profile to provide the minimal permissions needed for bwarp to work inside a docker.

  • Add sandbox.py with bubblewrap backend that hides ~/.nanobot from exec subprocesses
  • Add sandbox field to ExecToolConfig (tools.exec.sandbox = "bwrap" to enable)
  • Wire sandbox into ExecTool, loop.py, and subagent.py
  • Enable restrict_to_workspace for file tools when sandbox is set
  • Run container as nanobot (UID 1000) with HOME=/home/nanobot
  • Use Podman seccomp profile to allow bwrap's namespace syscalls in Docker
  • Fix pyproject.toml to allow direct git references (wecom dependency)

Possible next steps

  • Update docs to let people know to how to use this sandboxing (ideally make it default, and teach them to install bwrap if they don't have it (if they arn't using the docker version)
  • Further harden the exec wrapper with
"--new-session",    # Prevents TIOCSTI terminal injection see https://github.com/advisories/GHSA-m28g-vfcm-85ff
"--die-with-parent" # Ensures sandbox exits if the caller dies

@chengyongru @wanghesong2019 @ThinkOffApp @@Re-bin

@chengyongru
Copy link
Copy Markdown
Collaborator

Hi @kinchahoy ,

Thanks for your great work.

This is a useful security feature, but I think it should remain optional (off by default). For Docker users, container isolation is already sufficient. Adding bwrap as a dependency increases complexity for users who just want a lightweight personal assistant.

What's your thoughts?

@kinchahoy
Copy link
Copy Markdown
Author

I'll defer to your repo's defaults strategy, but given how insecure most openclaw clones are, and how quickly most of them are developing sensible risk mitigation strategies (see ironclaw etc.), I think it's worth leading with safe defaults (you can give people something like Claude's --dangerously-skip-permissions if they want it).

I believe it's best practice to not run in dockers as root, so I definately recommend you incorporate that change.

@chengyongru
Copy link
Copy Markdown
Collaborator

@Re-bin What's your thoughts?

@chengyongru chengyongru requested a review from Re-bin March 14, 2026 05:45
@ThinkOffApp
Copy link
Copy Markdown

Thanks for the mention. The bwrap approach looks clean and minimal for workspace isolation.

From our experience running a fleet of agents with shell access: the biggest risk isn't agents escaping the workspace, it's agents making network calls you didn't expect (installing packages, hitting APIs, exfiltrating data). --unshare-net in bwrap or a network policy layer would be a strong addition.

For the --die-with-parent and --new-session next steps: both are worth doing. We've had orphaned agent processes consume significant resources (one incident left 65 zombie processes using gigabytes of swap). --die-with-parent would have prevented that.

Making sandbox the default is the right call. The overhead is negligible and the safety margin is significant.

@kinchahoy
Copy link
Copy Markdown
Author

I'd suggest we merge this - it makes bwrap sandboxing available in the docker and repo in a minimal way + improves the docker security model in pretty basic ways (user vs root).

You can later decide if you want to default bwrap on or off. This pull request does not change the default config or docs in anyway.

@kinchahoy
Copy link
Copy Markdown
Author

I've updated the pull request to correctly merge with current head. PTAL or let me know this pull request is not useful.

@kinchahoy
Copy link
Copy Markdown
Author

@chengyongru @Re-bin Check in on this? I'd love to sunset my fork and work off head. The discussion clearly shows support, and this pull request doesn't change the default settings (though I recommend you change them with a proper update on the README).

@chengyongru chengyongru changed the base branch from main to nightly March 24, 2026 16:51
@chengyongru
Copy link
Copy Markdown
Collaborator

@kinchahoy We will move forward with this matter as soon as possible. Thank you very much. We are truly sorry that there are too many PRs at the moment. Currently, we only have two maintainers, so the processing speed is relatively slow.

@Re-bin
Copy link
Copy Markdown
Collaborator

Re-bin commented Mar 24, 2026

Thanks! I'm concerned whether the sandboxing mechanism will affect execution efficiency.

@kinchahoy
Copy link
Copy Markdown
Author

kinchahoy commented Mar 25, 2026

@kinchahoy We will move forward with this matter as soon as possible. Thank you very much. We are truly sorry that there are too many PRs at the moment. Currently, we only have two maintainers, so the processing speed is relatively slow.

Understood. I would encourage focusing on security issues to unlock usage, but I get it. Thanks for the great work!

Thanks! I'm concerned whether the sandboxing mechanism will affect execution efficiency.

Bubblewrap has very close to zero execution overhead - it's not a container, it uses linux namespaces and process isolation (which is why I chose it over more robust but slower isolation mechanisms).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants