From a1a0b4600d00ab64a2d719bd091014f1afe45eea Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Mon, 3 Aug 2026 15:18:21 +0100 Subject: [PATCH 1/6] tools/sandbox: update with new features, variants, and tests This commit brings several enhancements, reliability improvements, and tests to the sandbox: - Add support for `SANDBOX_UID_MAP` and `SANDBOX_GID_MAP` environment variables. This allows mapping multiple UID/GID ranges into the user namespace utilizing `newuidmap` and `newgidmap`. - Add `noproc_sandbox` and `nonetproc_sandbox` which skip remounting `/proc`. This provides a workaround for environments (like newer systemd or certain container runtimes) that prevent remounting a partially masked `/proc`. - Introduce `SANDBOX_FILE_MOUNTS` to allow bind-mounting a comma-separated list of specific files into the sandbox. - The loopback IP address can now be specified via `SANDBOX_LOCAL_IP`. - Introduce a pipe (`sync_fd`) to synchronize the parent and child processes. This prevents the child from hanging indefinitely if the parent dies before the sandbox setup is complete. - Add `SIGTERM` forwarding from the parent to the sandboxed child process. - Add a README documenting the sandbox, its variants, and configurable environment variables. - Add a `sandbox_test.go` test suite to test the network/mount isolation, ID mapping, and process lifecycle across all sandbox variants. --- tools/sandbox/BUILD | 34 +++- tools/sandbox/README.md | 67 +++++++ tools/sandbox/main.c | 4 +- tools/sandbox/nonet_main.c | 19 +- tools/sandbox/nonetproc_main.c | 27 +++ tools/sandbox/noproc_main.c | 27 +++ tools/sandbox/sandbox.c | 345 +++++++++++++++++++++++++++------ tools/sandbox/sandbox.h | 5 +- tools/sandbox/sandbox_test.go | 267 +++++++++++++++++++++++++ 9 files changed, 725 insertions(+), 70 deletions(-) create mode 100644 tools/sandbox/README.md create mode 100644 tools/sandbox/nonetproc_main.c create mode 100644 tools/sandbox/noproc_main.c create mode 100644 tools/sandbox/sandbox_test.go diff --git a/tools/sandbox/BUILD b/tools/sandbox/BUILD index e65f547892..bc2d449054 100644 --- a/tools/sandbox/BUILD +++ b/tools/sandbox/BUILD @@ -9,7 +9,6 @@ c_library( c_binary( name = "please_sandbox", srcs = ["main.c"], - static = (CONFIG.get("STATIC_SANDBOX") is not None), visibility = ["PUBLIC"], deps = [":sandbox"], ) @@ -17,11 +16,40 @@ c_binary( c_binary( name = "nonet_sandbox", srcs = ["nonet_main.c"], - static = (CONFIG.get("STATIC_SANDBOX") is not None), - visibility = ["PUBLIC"], deps = [":sandbox"], ) +c_binary( + name = "noproc_sandbox", + srcs = ["noproc_main.c"], + deps = [":sandbox"], +) + +c_binary( + name = "nonetproc_sandbox", + srcs = ["nonetproc_main.c"], + deps = [":sandbox"], +) + +go_test( + name = "sandbox_test", + srcs = ["sandbox_test.go"], + data = { + "tm_sandbox": ":tm_sandbox", + "nonet_sandbox": ":nonet_sandbox", + "noproc_sandbox": ":noproc_sandbox", + "nonetproc_sandbox": ":nonetproc_sandbox", + }, + labels = [ + "localonly", + "manual", + ], + sandbox = False, + deps = [ + "///third_party/go/github.com_stretchr_testify//require", + ], +) + cc_test( name = "sandbox_test", srcs = ["sandbox_test.cc"], diff --git a/tools/sandbox/README.md b/tools/sandbox/README.md new file mode 100644 index 0000000000..7e2e3e6caf --- /dev/null +++ b/tools/sandbox/README.md @@ -0,0 +1,67 @@ +# tm_sandbox + +> [!CAUTION] +> The Please Sandbox is not a security boundary. It is not designed to run untrusted or malicious +> code. + +`tm_sandbox` is a wrapper that allows running a given binary in Linux namespaces. By default, it +creates PID, IPC, UTS, user, mount and network namespaces. It also does a bunch of things on the +filesystem: + +- if `TMP_DIR` is not set or not under `/tmp`, a tmpfs is mounted over `/tmp` and `TMPDIR` is set to + `/tmp`. If it is set, current working directory is bind mounted to its path; +- if `SANDBOX_DIRS` is set, we expect a comma-separated list of path that will be hidden with a + tmpfs; +- if `SANDBOX_FILE_MOUNTS` is set, we expect it to be set to a comma-separated list of key-value + pairs in the following format: `key:value`. Keys must point to existing paths and will be bind + mounted to the path given as value; +- if `SANDBOX_UID_MAP` or `SANDBOX_GID_MAP` is set, we pass these arguments to + newuidmap/newgidmap to configure uid/gid mappings. The format is 1..n space-delimited triples + of [id lowerid count], see `man newuidmap`; + +Mount and network namespaces can be disabled setting the `SHARE_MOUNT` and `SHARE_NETWORK` +environment variables to `1`. + +The sandbox is distributed with 3 other flavours: + +- `nonet_sandbox` disables network namespacing by default, but it can be force-enabled by setting + `SHARE_NETWORK` to 0. +- `noproc_sandbox` disables remounting of `/proc`. +- `nonetproc_sandbox` disables remounting of `/proc` and disables network namespacing by default. + +## UID/GID mapping in user namespace + +By default, the sandbox only maps the effective UID/GID from the running sandbox into the namespace. +However, `SANDBOX_UID_MAP` and `SANDBOX_GID_MAP` may be used to define arguments that are passed to +`new*idmap`. + +The example below will map the effective UID/GID from the running sandbox to root and UIDs/GIDs +from range [100000;165536) to [1;65536) in the child namespace. + +```bash +$ TMP_DIR=/tmp SANDBOX_UID_MAP="0 $UID 1 1 100000 65536" tm_sandbox cat /proc/self/uid_map + 0 100000000 1 + 1 100000 65536 +``` + +## Capabilities and other requirements + +### Namespaces + +Historically, creating Mount, PID, IPC, UTS, and Network namespaces required the heavily overloaded +`CAP_SYS_ADMIN` capability on the host system. However, since Linux 3.8, unprivileged processes can +use User Namespaces to obtain local `CAP_SYS_ADMIN` privileges. This allows processes to create +Mount, PID, IPC, UTS, and Network namespaces without needing host-level root or `CAP_SYS_ADMIN` +privileges. + +#### Mount namespace + +In addition to the above, when enabling the mount namespace, the sandbox will remount /proc, so any +process that use it will have access to accurate information of the PID namespace (otherwise they'd +still have access to /proc from the parent namespace). There's however a specific edge case in the +Linux kernel, that prevents /proc from being remounted in a mount namespace, when the parent /proc +is not fully visible, [see this commit](https://github.com/torvalds/linux/commit/1b852bceb0d1). + +This is an issue with most container runtimes (and therefore Kubernetes), as by default, they will +hide some part of /proc in a container to reduce attack surface, eg +[see this docker PR](https://github.com/docker/cli/pull/1808). diff --git a/tools/sandbox/main.c b/tools/sandbox/main.c index 42abd96724..50467ecb53 100644 --- a/tools/sandbox/main.c +++ b/tools/sandbox/main.c @@ -1,5 +1,5 @@ // please_sandbox is a very small binary to implement sandboxing -// of tests (and possibly other build actions) via cgroups. +// of tests (and possibly other build actions) via namespaces. // Essentially this is a very lightweight replacement for Docker // where we would use it for tests to avoid port clashes etc. // @@ -27,5 +27,5 @@ int main(int argc, char* argv[]) { const char* share_mount_env = getenv("SHARE_MOUNT"); const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); - return contain(&argv[1], unshare_network, unshare_mount); + return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, true); } diff --git a/tools/sandbox/nonet_main.c b/tools/sandbox/nonet_main.c index efedbfcecd..61c8857cbd 100644 --- a/tools/sandbox/nonet_main.c +++ b/tools/sandbox/nonet_main.c @@ -1,9 +1,11 @@ // nonet_sandbox is a slightly modified version of please_sandbox that does all the same // things except it leaves the network unscathed. -// It is currently not used, but is conceptually useful to sandbox rules that request sandbox -// disabling in order to gain network access (which is by far the most common case for that), -// but it's still useful to contain the other namespaces. +// This is useful for cases where build rules request sandboxing to be disabled, which +// is mostly so they can go out to the network, but we still want to control the rest +// of the namespaces. #include +#include +#include #include "tools/sandbox/sandbox.h" int main(int argc, char* argv[]) { @@ -13,5 +15,14 @@ int main(int argc, char* argv[]) { fputs("Usage: nonet_sandbox command args...\n", stderr); return 1; } - return contain(&argv[1], false, true); + + // Network namespace is **NOT** sandboxed by default but it can be enabled if `SHARE_NETWORK=0` env is set + const char* share_network_env = getenv("SHARE_NETWORK"); + const bool unshare_network = share_network_env != NULL && !strcmp(share_network_env, "0"); + + // Mount namespace is sandboxed by default but it can be opted out if `SHARE_MOUNT=1` env is set + const char* share_mount_env = getenv("SHARE_MOUNT"); + const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); + + return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, true); } diff --git a/tools/sandbox/nonetproc_main.c b/tools/sandbox/nonetproc_main.c new file mode 100644 index 0000000000..3a60e12eef --- /dev/null +++ b/tools/sandbox/nonetproc_main.c @@ -0,0 +1,27 @@ +// nonetproc_sandbox is a slightly modified version of nonet_sandbox that does all the same +// things except it doesn't mount /proc. +// This is a specific, if hacky, solution for newer versions of systemd which aren't +// allowing us to mount a full /proc from a new user namespace. +#include +#include +#include +#include "tools/sandbox/sandbox.h" + +int main(int argc, char* argv[]) { + if (argc < 2) { + fputs("nonetproc_sandbox implements limited sandboxing via Linux namespaces.\n", stderr); + fputs("It takes no flags, it simply executes the command given as arguments.\n", stderr); + fputs("Usage: nonetproc_sandbox command args...\n", stderr); + return 1; + } + + // Network namespace is **NOT** sandboxed by default but it can be enabled if `SHARE_NETWORK=0` env is set + const char* share_network_env = getenv("SHARE_NETWORK"); + const bool unshare_network = share_network_env != NULL && !strcmp(share_network_env, "0"); + + // Mount namespace is sandboxed by default but it can be opted out if `SHARE_MOUNT=1` env is set + const char* share_mount_env = getenv("SHARE_MOUNT"); + const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); + + return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, false); +} diff --git a/tools/sandbox/noproc_main.c b/tools/sandbox/noproc_main.c new file mode 100644 index 0000000000..dc2a30b2d0 --- /dev/null +++ b/tools/sandbox/noproc_main.c @@ -0,0 +1,27 @@ +// noproc_sandbox is a slightly modified version of tm_sandbox that does all the same +// things except it doesn't mount /proc. +// This is a specific, if hacky, solution for newer versions of systemd which aren't +// allowing us to mount a full /proc from a new user namespace. +#include +#include +#include +#include "tools/sandbox/sandbox.h" + +int main(int argc, char* argv[]) { + if (argc < 2) { + fputs("noproc_sandbox implements limited sandboxing via Linux namespaces.\n", stderr); + fputs("It takes no flags, it simply executes the command given as arguments.\n", stderr); + fputs("Usage: noproc_sandbox command args...\n", stderr); + return 1; + } + + // Network namespace is sandboxed by default but it can be opted out if `SHARE_NETWORK=1` env is set + const char* share_network_env = getenv("SHARE_NETWORK"); + const bool unshare_network = share_network_env == NULL || strcmp(share_network_env, "1"); + + // Mount namespace is sandboxed by default but it can be opted out if `SHARE_MOUNT=1` env is set + const char* share_mount_env = getenv("SHARE_MOUNT"); + const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); + + return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, false); +} diff --git a/tools/sandbox/sandbox.c b/tools/sandbox/sandbox.c index 0866d64c92..c15dc3ce87 100644 --- a/tools/sandbox/sandbox.c +++ b/tools/sandbox/sandbox.c @@ -1,9 +1,10 @@ #include "tools/sandbox/sandbox.h" #define _GNU_SOURCE +#include +#include #include #include -#include #include #ifdef __linux__ @@ -11,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -20,10 +22,14 @@ #include #include #include -#include +#include +#include #include +#include -int perror_sock(char *errmsg, const int sock) { +static int cloned_pid; + +inline int perror_sock(char *errmsg, const int sock) { close(sock); perror(errmsg); return 1; @@ -44,15 +50,14 @@ int lo_up() { memset(&req, 0, sizeof(req)); strncpy(req.ifr_name, "lo", IFNAMSIZ); if (ioctl(sock, SIOCGIFFLAGS, &req) < 0) { - perror_sock("SIOCGIFFLAGS", sock); - return 1; + return perror_sock("SIOCGIFFLAGS", sock); } req.ifr_flags |= IFF_UP; if (ioctl(sock, SIOCSIFFLAGS, &req) < 0) { - perror_sock("SIOCSIFFLAGS", sock); - return 1; + return perror_sock("SIOCSIFFLAGS", sock); } + close(sock); return 0; } @@ -93,12 +98,14 @@ int default_gateway() { return 0; } -// add_local_ip assigns an additional IP address to the loopback interface. -// This is required for envtest to run in the sandbox which has a default -// cluster IP range of 10.0.0.0/24 and cannot use addresses in the local -// 127.0.0.0/8 range +// add_local_ip assigns an IP address to the loopback interface. int add_local_ip() { + const char* local_ip = getenv("SANDBOX_LOCAL_IP"); + if (local_ip == NULL) { + return 0; + } + const int sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE); if (sock < 0) { perror("socket"); @@ -131,7 +138,12 @@ int add_local_ip() req.rta.rta_type = IFA_LOCAL; req.rta.rta_len = RTA_LENGTH(sizeof(req.addr)); - req.addr = inet_addr("10.1.1.1"); + in_addr_t ip_addr = inet_addr(local_ip); + if (ip_addr == INADDR_NONE) { + fprintf(stderr, "Invalid IP address provided for SANDBOX_LOCAL_IP: %s\n", local_ip); + return 1; + } + req.addr = ip_addr; if (send(sock, &req, req.nh.nlmsg_len, 0) < 0) { return perror_sock("send", sock); @@ -141,36 +153,116 @@ int add_local_ip() return 0; } -// deny_groups disables the ability to call setgroups(2). This is required -// before we can successfully write to gid_map in map_ids. -int deny_groups() { - FILE* f = fopen("/proc/self/setgroups", "w"); +// map_ids maps root inside the namespace to the user and group ID of the user +// running the sandbox. It also maps user-defined id ranges via `SANDBOX_*ID_MAP`. +// Without this we fail to create directories in the tmpfs with an EOVERFLOW. +int map_ids(const pid_t child, const char *path, const char *map) { + const size_t child_size = snprintf(NULL, 0, "%d", child); + char child_pid[child_size+1]; + + snprintf(child_pid, child_size+1, "%d", child); + + char *argv[256]; + int argc = 0; + argv[argc++] = (char*)path; + argv[argc++] = child_pid; + + // strtok_r mutates its argument, so the input to this function should be copied + char *map_copy = NULL; + if (map != NULL) { + map_copy = strdup(map); + if (!map_copy) { + perror("strdup"); + return 1; + } + char *saveptr; + char *token = strtok_r(map_copy, " \t\n", &saveptr); + while (token != NULL && argc < 255) { + argv[argc++] = token; + token = strtok_r(NULL, " \t\n", &saveptr); + } + if (token != NULL) { + fprintf(stderr, "too many arguments for map_ids (max 255)\n"); + free(map_copy); + return 1; + } + } + argv[argc] = NULL; + + const pid_t pid = fork(); + if (pid == -1) { + perror("fork"); + if (map_copy) { + free(map_copy); + } + return 1; + } else if (pid == 0) { + execvp(path, argv); + perror(path); + _exit(1); + } + + if (map_copy) { + free(map_copy); + } + + int status; + if (waitpid(pid, &status, 0) == -1) { + perror("waitpid failed"); + } + if (WIFEXITED(status)) { + return WEXITSTATUS(status); + } else if (WIFSIGNALED(status)) { + kill(getpid(), WTERMSIG(status)); + } + return 1; +} + +// deny_setgroups writes "deny" to /proc//setgroups. +// This is required by the kernel before an unprivileged user can write to gid_map. +int deny_setgroups(pid_t pid) { + char path[128]; + snprintf(path, sizeof(path), "/proc/%d/setgroups", pid); + + FILE *f = fopen(path, "w"); if (!f) { - perror("fopen /proc/self/setgroups"); + perror("fopen setgroups"); return 1; } - if (fputs("deny\n", f) < 0) { - perror("fputs"); + + if (fputs("deny", f) < 0) { + perror("fputs setgroups"); + fclose(f); return 1; } - return fclose(f); + + if (fclose(f) != 0) { + perror("fclose setgroups"); + return 1; + } + return 0; } -// map_ids maps the user id or group id inside the namespace to those outside. -// Without this we fail to create directories in the tmpfs with an EOVERFLOW. -int map_ids(int out_id, const char* path) { - FILE* f = fopen(path, "w"); +// write_id_map writes a 1-to-1 mapping directly to /proc// +int write_id_map(pid_t pid, const char *file, uid_t inside_id, uid_t outside_id) { + char path[128]; + snprintf(path, sizeof(path), "/proc/%d/%s", pid, file); + + FILE *f = fopen(path, "w"); if (!f) { - perror("fopen"); + perror("fopen map"); return 1; } - if (fprintf(f, "%d %d 1\n", out_id, out_id) < 0) { - perror("fprintf"); + + if (fprintf(f, "%u %u 1\n", inside_id, outside_id) < 0) { + perror("fprintf map"); + fclose(f); return 1; } + if (fclose(f) != 0) { - perror("fclose"); - return 1; + perror("fclose map"); + return 1; } return 0; } @@ -179,30 +271,34 @@ int map_ids(int out_id, const char* path) { // bind mounts the test directory to /tmp/plz_sandbox. // If the given string pointer (the argv[0] of the new process) is within the old temp dir // then it will be replaced with a new version pointing into the new sandbox dir. -int mount_tmp(char** argv0) { +int mount_tmp(char** argv0, bool sandbox_dir) { // Don't mount on /tmp if our tmp dir is under there, otherwise we won't be able to see it. const char* dir = getenv("TMP_DIR"); const char* d = "/tmp/plz_sandbox"; - if (dir) { - if (strncmp(dir, "/tmp/", 5) == 0) { - fputs("Not mounting tmpfs on /tmp since TMP_DIR is a subdir\n", stderr); - return 0; - } - } + // Remounting / as private is necessary so that the tmpfs mount isn't visible to anyone else. if (mount("none", "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0) { perror("remount"); return 1; } const int flags = MS_LAZYTIME | MS_NOATIME | MS_NODEV | MS_NOSUID; - if (mount("tmpfs", "/tmp", "tmpfs", flags, NULL) != 0) { - perror("mount"); - return 1; + if (!dir || strncmp(dir, "/tmp", 4) != 0) { + if (mount("tmpfs", "/tmp", "tmpfs", flags, NULL) != 0) { + perror("mount"); + return 1; + } + if (setenv("TMPDIR", "/tmp", 1) != 0) { + perror("setenv"); + return 1; + } } - if (setenv("TMPDIR", "/tmp", 1) != 0) { - perror("setenv"); - return 1; + + // Mount over /dev/shm as well so nothing can be inadvertently shared through it and we'll clean it up. + if (mount("tmpfs", "/dev/shm", "tmpfs", flags, NULL) != 0) { + perror("mount"); + return 1; } + // If SANDBOX_DIRS is set, we expect a comma-separated list of directories to mount a tmpfs over in order to hide them. // If one or more directories don't exist, that is OK, but any other error is fatal. char* dirs = getenv("SANDBOX_DIRS"); @@ -223,15 +319,18 @@ int mount_tmp(char** argv0) { // Remove the env var; downstream things don't need to know what these were. unsetenv("SANDBOX_DIRS"); } + if (!sandbox_dir) { + return 0; + } if (!dir) { fputs("TMP_DIR not set, will not bind-mount to /tmp/plz_sandbox\n", stderr); return 0; } - if (mkdir(d, S_IRWXU) != 0) { + if (mkdir(d, S_IRWXU) != 0 && errno != EEXIST) { perror("mkdir /tmp/plz_sandbox"); return 1; } - if (mount(dir, d, "", MS_BIND, NULL) != 0) { + if (mount(dir, d, "", MS_BIND|MS_REC, NULL) != 0) { perror("bind mount"); return 1; } @@ -242,8 +341,48 @@ int mount_tmp(char** argv0) { perror("setenv"); return 1; } + + // If SANDBOX_FILE_MOUNTS is set, we expect a comma-separated list of key:value pairs to mount files into the new tree. + // Currently all these are expected to exist. + char* files = getenv("SANDBOX_FILE_MOUNTS"); + if (files != NULL) { + char *token = strtok(files, ","); + while(token) { + char* separator = strchr(token, ':'); + if (!separator) { + fprintf(stderr, "Invalid sandbox file mount: %s\n", token); + } else { + *separator = '\0'; + if (mount(token, separator+1, "", MS_RDONLY | MS_BIND, NULL) != 0) { + perror("bind mount sandbox file"); + return 1; + } + } + token = strtok(NULL, ","); + } + unsetenv("SANDBOX_FILE_MOUNTS"); + } + // Now make root readonly (once we have bind-mounted in the non-readonly workdir) - if (mount("none", "/", NULL, MS_REMOUNT | MS_RDONLY | MS_BIND, NULL) != 0) { + struct statvfs st; + if (statvfs("/", &st) != 0) { + perror("statvfs /"); + return 1; + } + unsigned long remount_flags = MS_REMOUNT | MS_RDONLY | MS_BIND; + if (st.f_flag & ST_NOSUID) { + remount_flags |= MS_NOSUID; + } + if (st.f_flag & ST_NODEV) { + remount_flags |= MS_NODEV; + } + if (st.f_flag & ST_NOEXEC) { + remount_flags |= MS_NOEXEC; + } + if (st.f_flag & ST_NOATIME) { + remount_flags |= MS_NOATIME; + } + if (mount("none", "/", NULL, remount_flags, NULL) != 0) { perror("remount ro"); return 1; } @@ -263,7 +402,7 @@ int mount_tmpfs(const char* dir) { // mount_proc mounts a new procfs on /proc int mount_proc() { - if (mount("proc", "/proc", "proc", 0, NULL) != 0) { + if (mount("proc", "/proc", "proc", MS_NODEV | MS_NOSUID | MS_NOEXEC | MS_RDONLY, NULL) != 0) { perror("mount proc"); return 1; } @@ -275,25 +414,56 @@ typedef struct _clone_arg { uid_t gid; bool net; bool mount; + bool sandbox_dir; + bool mount_proc; + int sync_fd[2]; char** argv; } clone_arg; +int set_parent_uid(uid_t parent) { + const size_t uid_size = snprintf(NULL, 0, "%d", parent); + char uid[uid_size+1]; + + snprintf(uid, uid_size+1, "%d", parent); + if (setenv("PARENT_UID", uid, 1) != 0) { + perror("setenv"); + return 1; + } + return 0; +} + // contain_child is the entrypoint for the child process. int contain_child(void* p) { clone_arg* arg = p; - if (deny_groups() != 0) { + + if (prctl(PR_SET_PDEATHSIG, SIGTERM) == -1) { + perror("failed to set PDEATHSIG"); + return 1; + } + + char c; + close(arg->sync_fd[1]); // close unused write end + ssize_t n = read(arg->sync_fd[0], &c, 1); // block until parent sends a byte or closes + if (n < 0) { + perror("read sync pipe"); + return 1; + } else if (n == 0) { + fprintf(stderr, "sandbox parent died before completing setup\n"); return 1; } - if (map_ids(arg->uid, "/proc/self/uid_map") != 0 || - map_ids(arg->gid, "/proc/self/gid_map") != 0) { + close(arg->sync_fd[0]); + + if (set_parent_uid(arg->uid) != 0) { return 1; } if (arg->mount) { - if (mount_tmp(&arg->argv[0]) != 0) { + if (mount_tmp(&arg->argv[0], arg->sandbox_dir) != 0) { return 1; } - if (mount_proc() != 0) { - return 1; + if (arg->mount_proc) { + if (mount_proc() != 0) { + return 1; + } } } if (arg->net) { @@ -301,24 +471,32 @@ int contain_child(void* p) { return 1; } } - if (prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) { - perror("failed to set PDEATHSIG"); - return 1; - } execvp(arg->argv[0], arg->argv); // If this returns, an error has occurred. fprintf(stderr, "exec %s: ", arg->argv[0]); perror(""); return 1; } +// if a sigterm is received, forward it to the sandboxed child process +void forward_sigterm (int signum) { + kill(cloned_pid, signum); +} + // contain separates the process into new namespaces to sandbox it. -int contain(char* argv[], bool net, bool mount) { +int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_proc) { clone_arg arg; arg.uid = getuid(); arg.gid = getgid(); arg.argv = argv; arg.net = net; arg.mount = mount; + arg.sandbox_dir = sandbox_dir; + arg.mount_proc = mount_proc; + + if (pipe2(arg.sync_fd, O_CLOEXEC)) { + perror("pipe"); + return 1; + } static const int stack_size = 100 * 1024; char* stack = mmap(NULL, stack_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); @@ -331,9 +509,49 @@ int contain(char* argv[], bool net, bool mount) { if (pid == -1) { perror("clone"); fputs("Your user doesn't seem to have enough permissions to call clone(2).\n", stderr); - fputs("please_sandbox requires support for user namespaces (usually >= Linux 3.10)\n", stderr); + fputs("tm_sandbox requires support for MS_LAZYTIME (>= Linux 4.0)\n", stderr); return 1; } + close(arg.sync_fd[0]); + + // set pid to global cloned_pid value for forwarding on sigterm + cloned_pid = pid; + signal (SIGTERM, forward_sigterm); + + // set up UID/GID mappings for the child + const char* uid_map = getenv("SANDBOX_UID_MAP"); + const char* gid_map = getenv("SANDBOX_GID_MAP"); + + if (uid_map != NULL || gid_map != NULL) { + if (uid_map == NULL) { + uid_map = gid_map; + } + if (gid_map == NULL) { + gid_map = uid_map; + } + + if (map_ids(pid, "/usr/bin/newuidmap", uid_map) != 0 || + map_ids(pid, "/usr/bin/newgidmap", gid_map) != 0) { + return 1; + } + } else { + // Otherwise deny setgroups, map single current user inside to current user outside + if (deny_setgroups(pid) != 0 || + write_id_map(pid, "uid_map", getuid(), getuid()) != 0 || + write_id_map(pid, "gid_map", getgid(), getgid()) != 0) { + return 1; + } + } + + // signal the child to proceed + if (write(arg.sync_fd[1], "1", 1) != 1) { + perror("write to sync pipe"); + return 1; + } else if (close(arg.sync_fd[1]) != 0) { + perror("close sync pipe"); + return 1; + } + // We're the parent process; wait on the child and exit with its status. int status = 0; if (waitpid(pid, &status, 0) == -1) { @@ -364,16 +582,23 @@ char* exec_name(const char* old_name, const char* old_dir, const char* new_dir) return change_path(old_name, old_dir, new_dir, 0); } +// check_valid_path_suffix makes sure given suffix can be appended to an +// absolute path without any issues. +bool check_valid_path_suffix(const char* suffix) { + return suffix != NULL && (suffix[0] == '/' || suffix[0] == 0); +} + // change_path takes a string or environment variable and changes a prefix from one path to another. char* change_path(const char* old_name, const char* old_dir, const char* new_dir, int prefix_len) { const int new_dir_len = strlen(new_dir); const int old_dir_len = strlen(old_dir); const int old_name_len = strlen(old_name); - if (strncmp(old_dir, old_name + prefix_len, old_dir_len) != 0) { // is the value of old_name prefixed with old_dir + if ((old_name_len > prefix_len + old_dir_len && !check_valid_path_suffix(old_name + prefix_len + old_dir_len)) || + strncmp(old_dir, old_name + prefix_len, old_dir_len) != 0) { // is the value of old_name prefixed with old_dir return (char*)old_name; // Dodgy cast but we know we don't alter it again later. } const int new_len = new_dir_len + old_name_len - old_dir_len + 1; - char* new_name = malloc(new_len + 1); + char* new_name = malloc((new_len + 1) * sizeof(char)); strncpy(new_name, old_name, prefix_len); strcpy(new_name + prefix_len, new_dir); strcpy(new_name + prefix_len + new_dir_len, old_name + prefix_len + old_dir_len); diff --git a/tools/sandbox/sandbox.h b/tools/sandbox/sandbox.h index 18b94664ae..10f22e0ec3 100644 --- a/tools/sandbox/sandbox.h +++ b/tools/sandbox/sandbox.h @@ -3,8 +3,11 @@ // contain separates the process into new namespaces to sandbox it. // It should be passed the argv for the new process, and booleans indicating // whether it should move to new network and mount namespaces. +// The sandbox_dir argument indicates whether it should attempt to create a sandbox +// for TMP_DIR in /tmp/plz_sandbox or not. +// The mount_proc argument indicates whether it should attempt to mount a new /proc. // It returns an exit code (so 0 on success, nonzero on failure). -int contain(char* argv[], bool net, bool mount); +int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_proc); // exec_name returns the name of the new binary to exec() as. // old_name is the current name; if it's within old_dir it will be re-prefixed to new_dir. diff --git a/tools/sandbox/sandbox_test.go b/tools/sandbox/sandbox_test.go new file mode 100644 index 0000000000..2dbee716b0 --- /dev/null +++ b/tools/sandbox/sandbox_test.go @@ -0,0 +1,267 @@ +package sandbox_test + +import ( + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func runSandbox(t *testing.T, sandbox string, args []string, env []string) string { + t.Helper() + cmd := exec.Command(sandbox, args...) + cmd.Env = env + out, err := cmd.CombinedOutput() + require.NoError(t, err, "sandbox command failed: %q, output: %q", err, string(out)) + return strings.TrimSpace(string(out)) +} + +func tmSandbox(t *testing.T, args []string, env []string) string { + t.Helper() + return runSandbox(t, os.Getenv("DATA_PLEASE_SANDBOX"), args, env) +} + +func noprocSandbox(t *testing.T, args []string, env []string) string { + t.Helper() + return runSandbox(t, os.Getenv("DATA_NOPROC_SANDBOX"), args, env) +} + +func nonetSandbox(t *testing.T, args []string, env []string) string { + t.Helper() + return runSandbox(t, os.Getenv("DATA_NONET_SANDBOX"), args, env) +} + +func nonetprocSandbox(t *testing.T, args []string, env []string) string { + t.Helper() + return runSandbox(t, os.Getenv("DATA_NONETPROC_SANDBOX"), args, env) +} + +func TestSandboxCommon(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("Connection accepted")) + }) + ts := httptest.NewServer(handler) + defer ts.Close() + + if _, err := os.Stat("/dev/null"); err != nil { + t.Fatalf("Test precondition failed: failed to stat /dev/null: %v", err) + } + + if _, err := os.Stat("/tmp"); err != nil { + t.Fatalf("Test precondition failed: failed to stat /tmp: %v", err) + } + + tests := []struct { + name string + env []string + args []string + want string + }{ + { + name: "must share network when SHARE_NETWORK=1", + env: []string{"SHARE_NETWORK=1", "TMP_DIR=/tmp"}, + args: []string{"sh", "-c", fmt.Sprintf(`curl -sS %s`, ts.URL)}, + want: "Connection accepted", + }, + { + name: "must isolate network when SHARE_NETWORK=0", + env: []string{"SHARE_NETWORK=0", "TMP_DIR=/tmp"}, + args: []string{"sh", "-c", fmt.Sprintf(`curl %s 2>&1 | grep -o 'Connection refused'`, ts.URL)}, + want: "Connection refused", + }, + { + name: "must map parent user to its own UID by default", + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", "echo UID=$(id -u)"}, + want: fmt.Sprintf("UID=%d", os.Getuid()), + }, + { + name: "SANDBOX_DIRS must hide the contents of a specified directory", + env: []string{"TMP_DIR=/tmp", "SANDBOX_DIRS=/dev"}, + args: []string{"ls", "/dev"}, + want: "", + }, + { + name: "SANDBOX_FILE_MOUNTS must mount the specified file", + env: []string{"TMP_DIR=/tmp", "SANDBOX_FILE_MOUNTS=/proc/sys/kernel/ostype:/dev/null"}, + args: []string{"cat", "/dev/null"}, + want: "Linux", + }, + { + name: "SANDBOX_UID_MAP maps current uid/gid to 0", + env: []string{"TMP_DIR=/tmp", fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1", os.Getuid())}, + args: []string{"sh", "-c", "echo $(id -u)/$(id -g)"}, + want: "0/0", + }, + { + name: "SANDBOX_UID_MAP uid and gid ranges", + env: []string{"TMP_DIR=/tmp", "SANDBOX_UID_MAP=0 100000 10 200 103000 40", "SANDBOX_GID_MAP=50 106000 700"}, + args: []string{"sh", "-c", "cat /proc/self/uid_map /proc/self/gid_map | awk '{$1=$1}1'"}, + want: "0 100000 10\n200 103000 40\n50 106000 700", + }, + { + name: "SANDBOX_UID_MAP uid and gid ranges allow us to use chown", + env: []string{ + "TMP_DIR=/var", + fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1 1 100000 65536", os.Getuid()), + fmt.Sprintf("SANDBOX_GID_MAP=0 %d 1 1 100000 65536", os.Getgid()), + }, + args: []string{"sh", "-c", "touch /tmp/f && chown 200:50 /tmp/f && stat -c '%u %g' /tmp/f"}, + want: "200 50", + }, + { + name: "Sandbox denies setgroups by default", + env: []string{"TMP_DIR=/tmp"}, + args: []string{"cat", "/proc/self/setgroups"}, + want: "deny", + }, + } + sandboxImpl := map[string]func(*testing.T, []string, []string) string{ + "please_sandbox": tmSandbox, + "noproc_sandbox": noprocSandbox, + "nonet_sandbox": nonetSandbox, + "nonetproc_sandbox": nonetprocSandbox, + } + for _, tt := range tests { + for sandboxName, sandbox := range sandboxImpl { + t.Run(sandboxName+"/"+tt.name, func(t *testing.T) { + output := sandbox(t, tt.args, tt.env) + + if output != tt.want { + t.Errorf("Expected %q, but got %q", tt.want, output) + } + }) + } + } +} + +func TestSandboxNetworkShare(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("Connection accepted")) + }) + ts := httptest.NewServer(handler) + defer ts.Close() + + tests := []struct { + name string + sandbox func(*testing.T, []string, []string) string + env []string + args []string + want string + }{ + { + name: "please_sandbox must isolate network by default", + sandbox: tmSandbox, + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", fmt.Sprintf(`curl %s 2>&1 | grep -o 'Connection refused'`, ts.URL)}, + want: "Connection refused", + }, + { + name: "nonet_sandbox must share network by default", + sandbox: nonetSandbox, + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", fmt.Sprintf(`curl -sS %s`, ts.URL)}, + want: "Connection accepted", + }, + { + name: "noproc_sandbox must isolate network by default", + sandbox: noprocSandbox, + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", fmt.Sprintf(`curl %s 2>&1 | grep -o 'Connection refused'`, ts.URL)}, + want: "Connection refused", + }, + { + name: "nonetproc_sandbox must share network by default", + sandbox: nonetprocSandbox, + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", fmt.Sprintf(`curl -sS %s`, ts.URL)}, + want: "Connection accepted", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := tt.sandbox(t, tt.args, tt.env) + + if output != tt.want { + t.Errorf("Expected output to contain %q, but got: %q", tt.want, output) + } + }) + } +} + +func TestSandboxMountShare(t *testing.T) { + nsOutside, err := os.Readlink("/proc/self/ns/mnt") + require.NoError(t, err, "failed to readlink /proc/self/ns/mnt outside sandbox") + + t.Run("please_sandbox must isolate mounts by default", func(t *testing.T) { + nsInside := tmSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"TMP_DIR=" + os.TempDir()}) + require.Regexp(t, `^mnt:\[\d+\]`, nsInside) + require.NotEqual(t, nsInside, nsOutside) + }) + + t.Run("please_sandbox must share mount namespace when called with SHARE_MOUNT=1", func(t *testing.T) { + nsInside := tmSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"SHARE_MOUNT=1"}) + require.Equal(t, nsInside, nsOutside) + }) +} + +func TestSandboxHangOnParentCrash(t *testing.T) { + sandbox := os.Getenv("DATA_PLEASE_SANDBOX") + if sandbox == "" { + t.Skip("DATA_PLEASE_SANDBOX not set") + } + + // We run this in a loop with different delays to reliably hit the race condition window where + // the parent dies after clone() but before sending SIGUSR1 / writing to the pipe. + for i := range 20 { + t.Run(fmt.Sprintf("iteration_%d", i), func(t *testing.T) { + r, w, err := os.Pipe() + require.NoError(t, err) + defer r.Close() + + cmd := exec.Command(sandbox, "echo", "success") + cmd.Stdout = w + + err = cmd.Start() + require.NoError(t, err) + + // Close our copy of the write-end immediately so that the read below can only block on either the sandbox itself or the sandboxed command. + w.Close() + + // Sleep just long enough for the parent to call clone(), + // but (hopefully) before it finishes uid mapping. + time.Sleep(time.Duration(i) * 100 * time.Microsecond) + + _ = cmd.Process.Kill() + _ = cmd.Wait() + + errCh := make(chan error, 1) + go func() { + buf := make([]byte, 128) + _, err := r.Read(buf) + errCh <- err + }() + + select { + case err := <-errCh: + // Success: The read unblocked. The child either finished fast + // or safely aborted when it noticed the parent died. + if err != io.EOF && err != nil { + t.Logf("Expected EOF, got: %v", err) + } + case <-time.After(100 * time.Millisecond): + t.Fatalf("Child process hung indefinitely holding stdout after parent was killed! (Race condition triggered)") + } + }) + } +} From 56f24ef973e6718ff3160231b8ce8da95997e503 Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Wed, 5 Aug 2026 09:43:06 +0000 Subject: [PATCH 2/6] tools/sandbox: reintroduce ip for backward compatibility, fix build failures Backward compatibility: - add_local_ip: add 10.1.1.1 to the loopback interface by default again, for backward compatibility with the previous upstream behaviour (#3310). SANDBOX_LOCAL_IP now overrides the address, and setting it to empty string disables it. Build fixes: - Fix test name clash by renaming sandbox_test to sandbox_go_test - Use the name `please_sandbox` consistently - Fix Darwin and FreeBSD builds by updating the contain() stub and moving back out of the #ifdef __linux__ block. - Restore static = (CONFIG.get("STATIC_SANDBOX") is not None) so the alpine release links the sandboxes statically (defined in .plzconfig.alpine). - Drop inline from perror_sock: the inline definition without static emits no external symbol, breaking -O0 links. Tests: - Keep SANDBOX_UID_MAP/SANDBOX_GID_MAP outside ids below 65536 so the mappings work in constrained container uid spaces. - Assert network isolation via curl's exit code (7, couldn't connect) instead of error text (wording changed in curl 8). - Install uidmap (shadow-uidmap on alpine) in the CI images and delegate root:0:65536 in /etc/subuid and /etc/subgid, since the sandbox execs newuidmap/newgidmap, which check subordinate id delegation even for root. --- .mailmap | 2 + tools/images/alpine/Dockerfile | 6 ++- tools/images/ubuntu/Dockerfile | 6 ++- tools/images/ubuntu_alt/Dockerfile | 6 ++- tools/sandbox/BUILD | 11 ++++- tools/sandbox/README.md | 24 ++++++---- tools/sandbox/noproc_main.c | 2 +- tools/sandbox/sandbox.c | 32 +++++++------ tools/sandbox/sandbox_test.go | 74 ++++++++++++++++++++++++------ 9 files changed, 119 insertions(+), 44 deletions(-) create mode 100644 .mailmap diff --git a/.mailmap b/.mailmap new file mode 100644 index 0000000000..c19cac55e7 --- /dev/null +++ b/.mailmap @@ -0,0 +1,2 @@ +Grant Zvolsky +Grant Zvolsky diff --git a/tools/images/alpine/Dockerfile b/tools/images/alpine/Dockerfile index bbb7889242..69da54657f 100644 --- a/tools/images/alpine/Dockerfile +++ b/tools/images/alpine/Dockerfile @@ -2,7 +2,11 @@ FROM golang:1.26-alpine LABEL org.opencontainers.image.authors="please thoughtmachine net" LABEL org.opencontainers.image.source=https://github.com/thought-machine/please -RUN apk add --no-cache git patch gcc g++ libc-dev bash libgcc xz protoc protobuf-dev perl-utils +RUN apk add --no-cache git patch gcc g++ libc-dev bash libgcc xz protoc protobuf-dev perl-utils shadow-uidmap + +# Subordinate id delegation for sandbox tests +RUN printf 'root:0:65536\n' >> /etc/subuid && \ + printf 'root:0:65536\n' >> /etc/subgid # Ensure this is where we expect on the PATH RUN ln -s /usr/local/go/bin/go /usr/local/bin/go diff --git a/tools/images/ubuntu/Dockerfile b/tools/images/ubuntu/Dockerfile index 57f6d22ca7..cf05c3dfe9 100644 --- a/tools/images/ubuntu/Dockerfile +++ b/tools/images/ubuntu/Dockerfile @@ -12,9 +12,13 @@ RUN truncate -s0 /tmp/preseed.cfg; \ apt-get update && \ apt-get install -y python3 python3-dev python3-pip time \ curl unzip git locales pkg-config zlib1g-dev psmisc \ - openssh-client ca-certificates && \ + openssh-client ca-certificates uidmap && \ apt-get clean +# Subordinate id delegation for sandbox tests +RUN printf 'root:0:65536\n' >> /etc/subuid && \ + printf 'root:0:65536\n' >> /etc/subgid + # Go - we want a specific package version here. RUN curl -fsSL https://dl.google.com/go/go1.26.1.linux-amd64.tar.gz | tar -xzC /usr/local RUN ln -s /usr/local/go/bin/go /usr/local/bin/go && ln -s /usr/local/go/bin/gofmt /usr/local/bin/gofmt diff --git a/tools/images/ubuntu_alt/Dockerfile b/tools/images/ubuntu_alt/Dockerfile index d11e01bb06..7551a1bf18 100644 --- a/tools/images/ubuntu_alt/Dockerfile +++ b/tools/images/ubuntu_alt/Dockerfile @@ -7,9 +7,13 @@ ENV DEBIAN_FRONTEND noninteractive # Most dependencies; Python, Clang RUN apt-get update && \ apt-get install -y python3 python3-dev python3-pip \ - curl unzip git locales pkg-config zlib1g-dev clang && \ + curl unzip git locales pkg-config zlib1g-dev clang uidmap && \ apt-get clean +# Subordinate id delegation for sandbox tests +RUN printf 'root:0:65536\n' >> /etc/subuid && \ + printf 'root:0:65536\n' >> /etc/subgid + # Go RUN curl -fsSL https://dl.google.com/go/go1.26.1.linux-amd64.tar.gz | tar -xzC /usr/local RUN ln -s /usr/local/go/bin/go /usr/local/bin/go && ln -s /usr/local/go/bin/gofmt /usr/local/bin/gofmt diff --git a/tools/sandbox/BUILD b/tools/sandbox/BUILD index bc2d449054..dce507593d 100644 --- a/tools/sandbox/BUILD +++ b/tools/sandbox/BUILD @@ -9,6 +9,7 @@ c_library( c_binary( name = "please_sandbox", srcs = ["main.c"], + static = (CONFIG.get("STATIC_SANDBOX") is not None), visibility = ["PUBLIC"], deps = [":sandbox"], ) @@ -16,26 +17,32 @@ c_binary( c_binary( name = "nonet_sandbox", srcs = ["nonet_main.c"], + static = (CONFIG.get("STATIC_SANDBOX") is not None), + visibility = ["PUBLIC"], deps = [":sandbox"], ) c_binary( name = "noproc_sandbox", srcs = ["noproc_main.c"], + static = (CONFIG.get("STATIC_SANDBOX") is not None), + visibility = ["PUBLIC"], deps = [":sandbox"], ) c_binary( name = "nonetproc_sandbox", srcs = ["nonetproc_main.c"], + static = (CONFIG.get("STATIC_SANDBOX") is not None), + visibility = ["PUBLIC"], deps = [":sandbox"], ) go_test( - name = "sandbox_test", + name = "sandbox_go_test", srcs = ["sandbox_test.go"], data = { - "tm_sandbox": ":tm_sandbox", + "please_sandbox": ":please_sandbox", "nonet_sandbox": ":nonet_sandbox", "noproc_sandbox": ":noproc_sandbox", "nonetproc_sandbox": ":nonetproc_sandbox", diff --git a/tools/sandbox/README.md b/tools/sandbox/README.md index 7e2e3e6caf..8c2d64d58d 100644 --- a/tools/sandbox/README.md +++ b/tools/sandbox/README.md @@ -1,15 +1,16 @@ -# tm_sandbox +# please_sandbox > [!CAUTION] > The Please Sandbox is not a security boundary. It is not designed to run untrusted or malicious > code. -`tm_sandbox` is a wrapper that allows running a given binary in Linux namespaces. By default, it +`please_sandbox` is a wrapper that allows running a given binary in Linux namespaces. By default, it creates PID, IPC, UTS, user, mount and network namespaces. It also does a bunch of things on the filesystem: - if `TMP_DIR` is not set or not under `/tmp`, a tmpfs is mounted over `/tmp` and `TMPDIR` is set to - `/tmp`. If it is set, current working directory is bind mounted to its path; + `/tmp`. If it is set, `$TMP_DIR` is bind mounted onto `/tmp/plz_sandbox`, which becomes the + working directory, and the root filesystem is remounted read-only; - if `SANDBOX_DIRS` is set, we expect a comma-separated list of path that will be hidden with a tmpfs; - if `SANDBOX_FILE_MOUNTS` is set, we expect it to be set to a comma-separated list of key-value @@ -22,7 +23,10 @@ filesystem: Mount and network namespaces can be disabled setting the `SHARE_MOUNT` and `SHARE_NETWORK` environment variables to `1`. -The sandbox is distributed with 3 other flavours: +When the network namespace is used, the loopback interface is brought up with an additional IP +address. `SANDBOX_LOCAL_IP` defines, defined empty string disables the extra address. + +The sandbox is distributed with 3 other variants: - `nonet_sandbox` disables network namespacing by default, but it can be force-enabled by setting `SHARE_NETWORK` to 0. @@ -31,17 +35,17 @@ The sandbox is distributed with 3 other flavours: ## UID/GID mapping in user namespace -By default, the sandbox only maps the effective UID/GID from the running sandbox into the namespace. +By default, the sandbox only maps the real UID/GID of the user running the sandbox into the namespace. However, `SANDBOX_UID_MAP` and `SANDBOX_GID_MAP` may be used to define arguments that are passed to `new*idmap`. -The example below will map the effective UID/GID from the running sandbox to root and UIDs/GIDs -from range [100000;165536) to [1;65536) in the child namespace. +The example below will map the real UID of the user running the sandbox to root and UIDs +from range [100000;165536) to [1;65536) in the child namespace (assuming your UID is 1000): ```bash -$ TMP_DIR=/tmp SANDBOX_UID_MAP="0 $UID 1 1 100000 65536" tm_sandbox cat /proc/self/uid_map - 0 100000000 1 - 1 100000 65536 +$ TMP_DIR=/tmp SANDBOX_UID_MAP="0 $UID 1 1 100000 65536" please_sandbox cat /proc/self/uid_map + 0 1000 1 + 1 100000 65536 ``` ## Capabilities and other requirements diff --git a/tools/sandbox/noproc_main.c b/tools/sandbox/noproc_main.c index dc2a30b2d0..0eab3f67f9 100644 --- a/tools/sandbox/noproc_main.c +++ b/tools/sandbox/noproc_main.c @@ -1,4 +1,4 @@ -// noproc_sandbox is a slightly modified version of tm_sandbox that does all the same +// noproc_sandbox is a slightly modified version of please_sandbox that does all the same // things except it doesn't mount /proc. // This is a specific, if hacky, solution for newer versions of systemd which aren't // allowing us to mount a full /proc from a new user namespace. diff --git a/tools/sandbox/sandbox.c b/tools/sandbox/sandbox.c index c15dc3ce87..cdba3c7f07 100644 --- a/tools/sandbox/sandbox.c +++ b/tools/sandbox/sandbox.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #ifdef __linux__ @@ -12,7 +13,6 @@ #include #include #include -#include #include #include #include @@ -29,7 +29,7 @@ static int cloned_pid; -inline int perror_sock(char *errmsg, const int sock) { +int perror_sock(char *errmsg, const int sock) { close(sock); perror(errmsg); return 1; @@ -98,11 +98,17 @@ int default_gateway() { return 0; } -// add_local_ip assigns an IP address to the loopback interface. +// add_local_ip assigns an additional IP address to the loopback interface. +// This is required for envtest to run in the sandbox which has a default +// cluster IP range of 10.0.0.0/24 and cannot use addresses in the local +// 127.0.0.0/8 range int add_local_ip() { + // SANDBOX_LOCAL_IP overrides the default address; defined empty string disables it. const char* local_ip = getenv("SANDBOX_LOCAL_IP"); if (local_ip == NULL) { + local_ip = "10.1.1.1"; + } else if (local_ip[0] == '\0') { return 0; } @@ -237,8 +243,8 @@ int deny_setgroups(pid_t pid) { } if (fclose(f) != 0) { - perror("fclose setgroups"); - return 1; + perror("fclose setgroups"); + return 1; } return 0; } @@ -261,8 +267,8 @@ int write_id_map(pid_t pid, const char *file, uid_t inside_id, uid_t outside_id) } if (fclose(f) != 0) { - perror("fclose map"); - return 1; + perror("fclose map"); + return 1; } return 0; } @@ -295,8 +301,8 @@ int mount_tmp(char** argv0, bool sandbox_dir) { // Mount over /dev/shm as well so nothing can be inadvertently shared through it and we'll clean it up. if (mount("tmpfs", "/dev/shm", "tmpfs", flags, NULL) != 0) { - perror("mount"); - return 1; + perror("mount"); + return 1; } // If SANDBOX_DIRS is set, we expect a comma-separated list of directories to mount a tmpfs over in order to hide them. @@ -509,7 +515,7 @@ int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_pro if (pid == -1) { perror("clone"); fputs("Your user doesn't seem to have enough permissions to call clone(2).\n", stderr); - fputs("tm_sandbox requires support for MS_LAZYTIME (>= Linux 4.0)\n", stderr); + fputs("please_sandbox requires support for user namespaces (usually >= Linux 3.10)\n", stderr); return 1; } close(arg.sync_fd[0]); @@ -570,7 +576,7 @@ int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_pro // On non-Linux systems contain simply execs a subprocess. // It's not really expected to be used there, this is simply to make it compile. -int contain(char* argv[], bool net, bool mount) { +int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_proc) { return execvp(argv[0], argv); } @@ -585,7 +591,7 @@ char* exec_name(const char* old_name, const char* old_dir, const char* new_dir) // check_valid_path_suffix makes sure given suffix can be appended to an // absolute path without any issues. bool check_valid_path_suffix(const char* suffix) { - return suffix != NULL && (suffix[0] == '/' || suffix[0] == 0); + return suffix != NULL && (suffix[0] == '/' || suffix[0] == 0); } // change_path takes a string or environment variable and changes a prefix from one path to another. @@ -594,7 +600,7 @@ char* change_path(const char* old_name, const char* old_dir, const char* new_dir const int old_dir_len = strlen(old_dir); const int old_name_len = strlen(old_name); if ((old_name_len > prefix_len + old_dir_len && !check_valid_path_suffix(old_name + prefix_len + old_dir_len)) || - strncmp(old_dir, old_name + prefix_len, old_dir_len) != 0) { // is the value of old_name prefixed with old_dir + strncmp(old_dir, old_name + prefix_len, old_dir_len) != 0) { // is the value of old_name prefixed with old_dir return (char*)old_name; // Dodgy cast but we know we don't alter it again later. } const int new_len = new_dir_len + old_name_len - old_dir_len + 1; diff --git a/tools/sandbox/sandbox_test.go b/tools/sandbox/sandbox_test.go index 2dbee716b0..f71035a87f 100644 --- a/tools/sandbox/sandbox_test.go +++ b/tools/sandbox/sandbox_test.go @@ -23,7 +23,7 @@ func runSandbox(t *testing.T, sandbox string, args []string, env []string) strin return strings.TrimSpace(string(out)) } -func tmSandbox(t *testing.T, args []string, env []string) string { +func pleaseSandbox(t *testing.T, args []string, env []string) string { t.Helper() return runSandbox(t, os.Getenv("DATA_PLEASE_SANDBOX"), args, env) } @@ -74,8 +74,8 @@ func TestSandboxCommon(t *testing.T) { { name: "must isolate network when SHARE_NETWORK=0", env: []string{"SHARE_NETWORK=0", "TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl %s 2>&1 | grep -o 'Connection refused'`, ts.URL)}, - want: "Connection refused", + args: []string{"sh", "-c", fmt.Sprintf(`curl -s %s; echo "curl exit: $?"`, ts.URL)}, + want: "curl exit: 7", // exit code 7 represents "Failed to connect to host." }, { name: "must map parent user to its own UID by default", @@ -102,17 +102,18 @@ func TestSandboxCommon(t *testing.T) { want: "0/0", }, { + // Outside ids below 65536 so they exist even in a constrained container uid space (e.g. the default rootless mapping). name: "SANDBOX_UID_MAP uid and gid ranges", - env: []string{"TMP_DIR=/tmp", "SANDBOX_UID_MAP=0 100000 10 200 103000 40", "SANDBOX_GID_MAP=50 106000 700"}, + env: []string{"TMP_DIR=/tmp", "SANDBOX_UID_MAP=0 20000 10 200 23000 40", "SANDBOX_GID_MAP=50 26000 700"}, args: []string{"sh", "-c", "cat /proc/self/uid_map /proc/self/gid_map | awk '{$1=$1}1'"}, - want: "0 100000 10\n200 103000 40\n50 106000 700", + want: "0 20000 10\n200 23000 40\n50 26000 700", }, { name: "SANDBOX_UID_MAP uid and gid ranges allow us to use chown", env: []string{ "TMP_DIR=/var", - fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1 1 100000 65536", os.Getuid()), - fmt.Sprintf("SANDBOX_GID_MAP=0 %d 1 1 100000 65536", os.Getgid()), + fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1 1 20000 40000", os.Getuid()), + fmt.Sprintf("SANDBOX_GID_MAP=0 %d 1 1 20000 40000", os.Getgid()), }, args: []string{"sh", "-c", "touch /tmp/f && chown 200:50 /tmp/f && stat -c '%u %g' /tmp/f"}, want: "200 50", @@ -125,7 +126,7 @@ func TestSandboxCommon(t *testing.T) { }, } sandboxImpl := map[string]func(*testing.T, []string, []string) string{ - "please_sandbox": tmSandbox, + "please_sandbox": pleaseSandbox, "noproc_sandbox": noprocSandbox, "nonet_sandbox": nonetSandbox, "nonetproc_sandbox": nonetprocSandbox, @@ -160,10 +161,10 @@ func TestSandboxNetworkShare(t *testing.T) { }{ { name: "please_sandbox must isolate network by default", - sandbox: tmSandbox, + sandbox: pleaseSandbox, env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl %s 2>&1 | grep -o 'Connection refused'`, ts.URL)}, - want: "Connection refused", + args: []string{"sh", "-c", fmt.Sprintf(`curl -s %s; echo "curl exit: $?"`, ts.URL)}, + want: "curl exit: 7", // exit code 7 represents "Failed to connect to host." }, { name: "nonet_sandbox must share network by default", @@ -176,8 +177,8 @@ func TestSandboxNetworkShare(t *testing.T) { name: "noproc_sandbox must isolate network by default", sandbox: noprocSandbox, env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl %s 2>&1 | grep -o 'Connection refused'`, ts.URL)}, - want: "Connection refused", + args: []string{"sh", "-c", fmt.Sprintf(`curl -s %s; echo "curl exit: $?"`, ts.URL)}, + want: "curl exit: 7", }, { name: "nonetproc_sandbox must share network by default", @@ -199,18 +200,61 @@ func TestSandboxNetworkShare(t *testing.T) { } } +func TestSandboxLocalIP(t *testing.T) { + tests := []struct { + name string + env []string + args []string + want string + }{ + { + name: "adds 10.1.1.1 to the loopback interface by default", + env: []string{"TMP_DIR=/tmp"}, + args: []string{"sh", "-c", `grep -o '10\.1\.1\.1' /proc/net/fib_trie | head -n1`}, + want: "10.1.1.1", + }, + { + name: "SANDBOX_LOCAL_IP overrides the default address", + env: []string{"TMP_DIR=/tmp", "SANDBOX_LOCAL_IP=10.2.3.4"}, + args: []string{"sh", "-c", `grep -o '10\.2\.3\.4' /proc/net/fib_trie | head -n1; grep -c '10\.1\.1\.1' /proc/net/fib_trie; true`}, + want: "10.2.3.4\n0", + }, + { + name: "SANDBOX_LOCAL_IP= disables the extra address", + env: []string{"TMP_DIR=/tmp", "SANDBOX_LOCAL_IP="}, + args: []string{"sh", "-c", `grep -c '10\.' /proc/net/fib_trie; true`}, + want: "0", + }, + } + sandboxImpl := map[string]func(*testing.T, []string, []string) string{ + "please_sandbox": pleaseSandbox, + "noproc_sandbox": noprocSandbox, + } + for _, tt := range tests { + for sandboxName, sandbox := range sandboxImpl { + t.Run(sandboxName+"/"+tt.name, func(t *testing.T) { + output := sandbox(t, tt.args, tt.env) + + if output != tt.want { + t.Errorf("Expected %q, but got %q", tt.want, output) + } + }) + } + } +} + func TestSandboxMountShare(t *testing.T) { nsOutside, err := os.Readlink("/proc/self/ns/mnt") require.NoError(t, err, "failed to readlink /proc/self/ns/mnt outside sandbox") t.Run("please_sandbox must isolate mounts by default", func(t *testing.T) { - nsInside := tmSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"TMP_DIR=" + os.TempDir()}) + nsInside := pleaseSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"TMP_DIR=" + os.TempDir()}) require.Regexp(t, `^mnt:\[\d+\]`, nsInside) require.NotEqual(t, nsInside, nsOutside) }) t.Run("please_sandbox must share mount namespace when called with SHARE_MOUNT=1", func(t *testing.T) { - nsInside := tmSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"SHARE_MOUNT=1"}) + nsInside := pleaseSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"SHARE_MOUNT=1"}) require.Equal(t, nsInside, nsOutside) }) } From c1a1b1c9c0b2f935370cb2d1b22ed88108cf7da4 Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Thu, 6 Aug 2026 08:26:15 +0000 Subject: [PATCH 3/6] sandbox: fix lint error Move the sandbox Go test into its own directory to avoid including adjacent C files in the Go package. Fixes ``` level=error msg="[linters_context] typechecking error: C source files not allowed when not using cgo or SWIG: main.c nonet_main.c nonetproc_main.c noproc_main.c sandbox.c" ``` --- tools/sandbox/BUILD | 19 ------------------- tools/sandbox/test/BUILD | 18 ++++++++++++++++++ tools/sandbox/{ => test}/sandbox_test.go | 0 3 files changed, 18 insertions(+), 19 deletions(-) create mode 100644 tools/sandbox/test/BUILD rename tools/sandbox/{ => test}/sandbox_test.go (100%) diff --git a/tools/sandbox/BUILD b/tools/sandbox/BUILD index dce507593d..dccf05bca2 100644 --- a/tools/sandbox/BUILD +++ b/tools/sandbox/BUILD @@ -38,25 +38,6 @@ c_binary( deps = [":sandbox"], ) -go_test( - name = "sandbox_go_test", - srcs = ["sandbox_test.go"], - data = { - "please_sandbox": ":please_sandbox", - "nonet_sandbox": ":nonet_sandbox", - "noproc_sandbox": ":noproc_sandbox", - "nonetproc_sandbox": ":nonetproc_sandbox", - }, - labels = [ - "localonly", - "manual", - ], - sandbox = False, - deps = [ - "///third_party/go/github.com_stretchr_testify//require", - ], -) - cc_test( name = "sandbox_test", srcs = ["sandbox_test.cc"], diff --git a/tools/sandbox/test/BUILD b/tools/sandbox/test/BUILD new file mode 100644 index 0000000000..b1a6077ff2 --- /dev/null +++ b/tools/sandbox/test/BUILD @@ -0,0 +1,18 @@ +go_test( + name = "sandbox_test", + srcs = ["sandbox_test.go"], + data = { + "please_sandbox": "//tools/sandbox:please_sandbox", + "nonet_sandbox": "//tools/sandbox:nonet_sandbox", + "noproc_sandbox": "//tools/sandbox:noproc_sandbox", + "nonetproc_sandbox": "//tools/sandbox:nonetproc_sandbox", + }, + labels = [ + "localonly", + "manual", + ], + sandbox = False, + deps = [ + "///third_party/go/github.com_stretchr_testify//require", + ], +) diff --git a/tools/sandbox/sandbox_test.go b/tools/sandbox/test/sandbox_test.go similarity index 100% rename from tools/sandbox/sandbox_test.go rename to tools/sandbox/test/sandbox_test.go From 151a097e6668d0ebaecb4ab4b1ebde5c898ac2b2 Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Thu, 6 Aug 2026 13:00:24 +0000 Subject: [PATCH 4/6] sandbox: fix pdeathsig, id map handling, and mount edge cases - Restore SIGKILL as the parent-death signal. The sandboxed command is PID 1 in the new namespace, so the kernel discards SIGTERM and the command leaks as an orphan process when the sandbox crashes. - map_ids: return an error when waitpid fails instead of reading an uninitialised status. - Tolerate an absent /dev/shm. - Require SANDBOX_UID_MAP and SANDBOX_GID_MAP to be set together. The other options, defaulting to a trivial mapping or mirroring the other mapping, made an assumption that isn't always correct. - Fix the TMP_DIR-under-/tmp check to exclude `/tmp[^/]+`. - Exec new*idmap via PATH search. This is safe because the sandbox binary itself runs with neither setuid bits nor file capabilities. - Fix fmt.Sprintf lint errors. --- tools/sandbox/README.md | 8 ++--- tools/sandbox/sandbox.c | 27 ++++++++------ tools/sandbox/test/sandbox_test.go | 58 +++++++++++++++++++++++++++--- 3 files changed, 74 insertions(+), 19 deletions(-) diff --git a/tools/sandbox/README.md b/tools/sandbox/README.md index 8c2d64d58d..491d10bab6 100644 --- a/tools/sandbox/README.md +++ b/tools/sandbox/README.md @@ -16,9 +16,9 @@ filesystem: - if `SANDBOX_FILE_MOUNTS` is set, we expect it to be set to a comma-separated list of key-value pairs in the following format: `key:value`. Keys must point to existing paths and will be bind mounted to the path given as value; -- if `SANDBOX_UID_MAP` or `SANDBOX_GID_MAP` is set, we pass these arguments to - newuidmap/newgidmap to configure uid/gid mappings. The format is 1..n space-delimited triples - of [id lowerid count], see `man newuidmap`; +- if `SANDBOX_UID_MAP` and `SANDBOX_GID_MAP` are set (both are required if either is), we pass + these arguments to newuidmap/newgidmap to configure uid/gid mappings. The format is 1..n + space-delimited triples of [id lowerid count], see `man newuidmap`; Mount and network namespaces can be disabled setting the `SHARE_MOUNT` and `SHARE_NETWORK` environment variables to `1`. @@ -43,7 +43,7 @@ The example below will map the real UID of the user running the sandbox to root from range [100000;165536) to [1;65536) in the child namespace (assuming your UID is 1000): ```bash -$ TMP_DIR=/tmp SANDBOX_UID_MAP="0 $UID 1 1 100000 65536" please_sandbox cat /proc/self/uid_map +$ TMP_DIR=/tmp SANDBOX_UID_MAP="0 $UID 1 1 100000 65536" SANDBOX_GID_MAP="0 $(id -g) 1" please_sandbox cat /proc/self/uid_map 0 1000 1 1 100000 65536 ``` diff --git a/tools/sandbox/sandbox.c b/tools/sandbox/sandbox.c index cdba3c7f07..52c8f421a2 100644 --- a/tools/sandbox/sandbox.c +++ b/tools/sandbox/sandbox.c @@ -215,6 +215,7 @@ int map_ids(const pid_t child, const char *path, const char *map) { int status; if (waitpid(pid, &status, 0) == -1) { perror("waitpid failed"); + return 1; } if (WIFEXITED(status)) { return WEXITSTATUS(status); @@ -288,7 +289,8 @@ int mount_tmp(char** argv0, bool sandbox_dir) { return 1; } const int flags = MS_LAZYTIME | MS_NOATIME | MS_NODEV | MS_NOSUID; - if (!dir || strncmp(dir, "/tmp", 4) != 0) { + // Skip the tmpfs when TMP_DIR is /tmp itself or lives under it. + if (!dir || (strcmp(dir, "/tmp") != 0 && strncmp(dir, "/tmp/", 5) != 0)) { if (mount("tmpfs", "/tmp", "tmpfs", flags, NULL) != 0) { perror("mount"); return 1; @@ -301,8 +303,10 @@ int mount_tmp(char** argv0, bool sandbox_dir) { // Mount over /dev/shm as well so nothing can be inadvertently shared through it and we'll clean it up. if (mount("tmpfs", "/dev/shm", "tmpfs", flags, NULL) != 0) { - perror("mount"); - return 1; + if (errno != ENOENT) { // tolerate absent /dev/shm + perror("mount /dev/shm"); + return 1; + } } // If SANDBOX_DIRS is set, we expect a comma-separated list of directories to mount a tmpfs over in order to hide them. @@ -442,7 +446,10 @@ int set_parent_uid(uid_t parent) { int contain_child(void* p) { clone_arg* arg = p; - if (prctl(PR_SET_PDEATHSIG, SIGTERM) == -1) { + // This process is PID 1 in the new namespace. SIGKILL is required to kill it + // with default signal handlers. Graceful termination of a live parent is + // handled separately by forward_sigterm. + if (prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) { perror("failed to set PDEATHSIG"); return 1; } @@ -529,15 +536,13 @@ int contain(char* argv[], bool net, bool mount, bool sandbox_dir, bool mount_pro const char* gid_map = getenv("SANDBOX_GID_MAP"); if (uid_map != NULL || gid_map != NULL) { - if (uid_map == NULL) { - uid_map = gid_map; - } - if (gid_map == NULL) { - gid_map = uid_map; + if (uid_map == NULL || gid_map == NULL) { + fputs("SANDBOX_UID_MAP and SANDBOX_GID_MAP must be set together\n", stderr); + return 1; } - if (map_ids(pid, "/usr/bin/newuidmap", uid_map) != 0 || - map_ids(pid, "/usr/bin/newgidmap", gid_map) != 0) { + if (map_ids(pid, "newuidmap", uid_map) != 0 || + map_ids(pid, "newgidmap", gid_map) != 0) { return 1; } } else { diff --git a/tools/sandbox/test/sandbox_test.go b/tools/sandbox/test/sandbox_test.go index f71035a87f..728ff68670 100644 --- a/tools/sandbox/test/sandbox_test.go +++ b/tools/sandbox/test/sandbox_test.go @@ -68,7 +68,7 @@ func TestSandboxCommon(t *testing.T) { { name: "must share network when SHARE_NETWORK=1", env: []string{"SHARE_NETWORK=1", "TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl -sS %s`, ts.URL)}, + args: []string{"sh", "-c", "curl -sS " + ts.URL}, want: "Connection accepted", }, { @@ -97,7 +97,11 @@ func TestSandboxCommon(t *testing.T) { }, { name: "SANDBOX_UID_MAP maps current uid/gid to 0", - env: []string{"TMP_DIR=/tmp", fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1", os.Getuid())}, + env: []string{ + "TMP_DIR=/tmp", + fmt.Sprintf("SANDBOX_UID_MAP=0 %d 1", os.Getuid()), + fmt.Sprintf("SANDBOX_GID_MAP=0 %d 1", os.Getgid()), + }, args: []string{"sh", "-c", "echo $(id -u)/$(id -g)"}, want: "0/0", }, @@ -170,7 +174,7 @@ func TestSandboxNetworkShare(t *testing.T) { name: "nonet_sandbox must share network by default", sandbox: nonetSandbox, env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl -sS %s`, ts.URL)}, + args: []string{"sh", "-c", "curl -sS " + ts.URL}, want: "Connection accepted", }, { @@ -184,7 +188,7 @@ func TestSandboxNetworkShare(t *testing.T) { name: "nonetproc_sandbox must share network by default", sandbox: nonetprocSandbox, env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl -sS %s`, ts.URL)}, + args: []string{"sh", "-c", "curl -sS " + ts.URL}, want: "Connection accepted", }, } @@ -259,6 +263,52 @@ func TestSandboxMountShare(t *testing.T) { }) } +// TestSandboxNoOrphanOnParentCrash checks that the sandboxed command does not outlive the +// sandbox process itself. Unlike TestSandboxHangOnParentCrash, which tests the window +// between clone() and the sync pipe, this kills the sandbox after the command has exec'd, +// executing the PR_SET_PDEATHSIG path. +func TestSandboxNoOrphanOnParentCrash(t *testing.T) { + sandbox := os.Getenv("DATA_PLEASE_SANDBOX") + if sandbox == "" { + t.Skip("DATA_PLEASE_SANDBOX not set") + } + + r, w, err := os.Pipe() + require.NoError(t, err) + defer r.Close() + + // The sleep only needs to outlive the 2s assertion window below. + cmd := exec.Command(sandbox, "sh", "-c", "echo ready && exec sleep 10") + cmd.Env = []string{"TMP_DIR=/tmp"} + cmd.Stdout = w + require.NoError(t, cmd.Start()) + // Close our copy of the write end so the read below blocks only on the sandboxed command. + w.Close() + + // Wait for the sandboxed command to be running, i.e. definitely past execvp. + buf := make([]byte, 6) + _, err = io.ReadFull(r, buf) + require.NoError(t, err) + require.Equal(t, "ready\n", string(buf)) + + // Simulate a crash of the sandbox process. + require.NoError(t, cmd.Process.Kill()) + _ = cmd.Wait() + + // The sandboxed process must die with it, closing its end of the pipe. + errCh := make(chan error, 1) + go func() { + _, err := r.Read(make([]byte, 1)) + errCh <- err + }() + select { + case err := <-errCh: + require.ErrorIs(t, err, io.EOF, "expected EOF once the sandboxed process died") + case <-time.After(2 * time.Second): + t.Fatal("sandboxed process survived the death of the sandbox process (orphan leak)") + } +} + func TestSandboxHangOnParentCrash(t *testing.T) { sandbox := os.Getenv("DATA_PLEASE_SANDBOX") if sandbox == "" { From 8807a2aa4c86750ff34f87d660a1d4be94091784 Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Thu, 6 Aug 2026 15:57:05 +0000 Subject: [PATCH 5/6] sandbox: replace binary variants with env knobs - Remove `nonet_sandbox`, `noproc_sandbox`, and `nonetproc_sandbox`. Under local execution Please sets `SHARE_NETWORK` and `SHARE_MOUNT` explicitly on every sandboxed invocation, so the variants' flipped defaults only took effect where something else invoked them, e.g. remote execution workers. - Add `MOUNT_PROC=0` option to `please_sandbox` to allow disabling `/proc` remounting. - Add README section explaining how to use the environment variables with Please. - Refactor per-binary test matrices whose premises were per-variant defaults and multiple binaries. BREAKING CHANGE: Removing `nonet_sandbox` is a breaking change for downstream repositories that referenced it, and for remote execution workers that invoked a variant binary and relied on its defaults. The migration path is to use a wrapper that defines the environment (see tools/sandbox/README.md). --- tools/images/alpine/Dockerfile | 2 +- tools/sandbox/BUILD | 24 ---- tools/sandbox/README.md | 27 ++++- tools/sandbox/main.c | 6 +- tools/sandbox/nonet_main.c | 28 ----- tools/sandbox/nonetproc_main.c | 27 ----- tools/sandbox/noproc_main.c | 27 ----- tools/sandbox/test/BUILD | 3 - tools/sandbox/test/sandbox_test.go | 184 ++++++++++++----------------- 9 files changed, 105 insertions(+), 223 deletions(-) delete mode 100644 tools/sandbox/nonet_main.c delete mode 100644 tools/sandbox/nonetproc_main.c delete mode 100644 tools/sandbox/noproc_main.c diff --git a/tools/images/alpine/Dockerfile b/tools/images/alpine/Dockerfile index 69da54657f..d865af371f 100644 --- a/tools/images/alpine/Dockerfile +++ b/tools/images/alpine/Dockerfile @@ -2,7 +2,7 @@ FROM golang:1.26-alpine LABEL org.opencontainers.image.authors="please thoughtmachine net" LABEL org.opencontainers.image.source=https://github.com/thought-machine/please -RUN apk add --no-cache git patch gcc g++ libc-dev bash libgcc xz protoc protobuf-dev perl-utils shadow-uidmap +RUN apk add --no-cache git patch gcc g++ libc-dev bash libgcc xz protoc protobuf-dev perl-utils shadow-uidmap curl # Subordinate id delegation for sandbox tests RUN printf 'root:0:65536\n' >> /etc/subuid && \ diff --git a/tools/sandbox/BUILD b/tools/sandbox/BUILD index dccf05bca2..df91bb1506 100644 --- a/tools/sandbox/BUILD +++ b/tools/sandbox/BUILD @@ -14,30 +14,6 @@ c_binary( deps = [":sandbox"], ) -c_binary( - name = "nonet_sandbox", - srcs = ["nonet_main.c"], - static = (CONFIG.get("STATIC_SANDBOX") is not None), - visibility = ["PUBLIC"], - deps = [":sandbox"], -) - -c_binary( - name = "noproc_sandbox", - srcs = ["noproc_main.c"], - static = (CONFIG.get("STATIC_SANDBOX") is not None), - visibility = ["PUBLIC"], - deps = [":sandbox"], -) - -c_binary( - name = "nonetproc_sandbox", - srcs = ["nonetproc_main.c"], - static = (CONFIG.get("STATIC_SANDBOX") is not None), - visibility = ["PUBLIC"], - deps = [":sandbox"], -) - cc_test( name = "sandbox_test", srcs = ["sandbox_test.cc"], diff --git a/tools/sandbox/README.md b/tools/sandbox/README.md index 491d10bab6..47e0b1d308 100644 --- a/tools/sandbox/README.md +++ b/tools/sandbox/README.md @@ -21,17 +21,32 @@ filesystem: space-delimited triples of [id lowerid count], see `man newuidmap`; Mount and network namespaces can be disabled setting the `SHARE_MOUNT` and `SHARE_NETWORK` -environment variables to `1`. +environment variables to `1`. Remounting of `/proc` can be disabled by setting `MOUNT_PROC=0`, +e.g. for systems that don't allow mounting a full `/proc` from a new user namespace (see the +mount namespace notes below). When the network namespace is used, the loopback interface is brought up with an additional IP address. `SANDBOX_LOCAL_IP` defines, defined empty string disables the extra address. -The sandbox is distributed with 3 other variants: +## Using the knobs with Please -- `nonet_sandbox` disables network namespacing by default, but it can be force-enabled by setting - `SHARE_NETWORK` to 0. -- `noproc_sandbox` disables remounting of `/proc`. -- `nonetproc_sandbox` disables remounting of `/proc` and disables network namespacing by default. +Please invokes the tool configured in `[sandbox] tool` with the command to run as its arguments, +and sets `SHARE_NETWORK` and `SHARE_MOUNT` itself according to the rule being run. Remote +execution workers invoke the sandbox tool themselves and should set these variables explicitly, +since the binary's defaults apply when they are absent. To set the other knobs, or to override +Please's per-rule choices, point `tool` at a thin wrapper script: + +```sh +#!/bin/sh +# please_sandbox, but without remounting /proc. +export MOUNT_PROC=0 +exec /path/to/please_sandbox "$@" +``` + +```ini +[sandbox] +tool = /path/to/noproc_sandbox_wrapper +``` ## UID/GID mapping in user namespace diff --git a/tools/sandbox/main.c b/tools/sandbox/main.c index 50467ecb53..05283897b1 100644 --- a/tools/sandbox/main.c +++ b/tools/sandbox/main.c @@ -27,5 +27,9 @@ int main(int argc, char* argv[]) { const char* share_mount_env = getenv("SHARE_MOUNT"); const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); - return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, true); + // /proc is remounted by default but it can be opted out if `MOUNT_PROC=0` env is set + const char* mount_proc_env = getenv("MOUNT_PROC"); + const bool mount_proc = mount_proc_env == NULL || strcmp(mount_proc_env, "0"); + + return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, mount_proc); } diff --git a/tools/sandbox/nonet_main.c b/tools/sandbox/nonet_main.c deleted file mode 100644 index 61c8857cbd..0000000000 --- a/tools/sandbox/nonet_main.c +++ /dev/null @@ -1,28 +0,0 @@ -// nonet_sandbox is a slightly modified version of please_sandbox that does all the same -// things except it leaves the network unscathed. -// This is useful for cases where build rules request sandboxing to be disabled, which -// is mostly so they can go out to the network, but we still want to control the rest -// of the namespaces. -#include -#include -#include -#include "tools/sandbox/sandbox.h" - -int main(int argc, char* argv[]) { - if (argc < 2) { - fputs("nonet_sandbox implements limited sandboxing via Linux namespaces.\n", stderr); - fputs("It takes no flags, it simply executes the command given as arguments.\n", stderr); - fputs("Usage: nonet_sandbox command args...\n", stderr); - return 1; - } - - // Network namespace is **NOT** sandboxed by default but it can be enabled if `SHARE_NETWORK=0` env is set - const char* share_network_env = getenv("SHARE_NETWORK"); - const bool unshare_network = share_network_env != NULL && !strcmp(share_network_env, "0"); - - // Mount namespace is sandboxed by default but it can be opted out if `SHARE_MOUNT=1` env is set - const char* share_mount_env = getenv("SHARE_MOUNT"); - const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); - - return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, true); -} diff --git a/tools/sandbox/nonetproc_main.c b/tools/sandbox/nonetproc_main.c deleted file mode 100644 index 3a60e12eef..0000000000 --- a/tools/sandbox/nonetproc_main.c +++ /dev/null @@ -1,27 +0,0 @@ -// nonetproc_sandbox is a slightly modified version of nonet_sandbox that does all the same -// things except it doesn't mount /proc. -// This is a specific, if hacky, solution for newer versions of systemd which aren't -// allowing us to mount a full /proc from a new user namespace. -#include -#include -#include -#include "tools/sandbox/sandbox.h" - -int main(int argc, char* argv[]) { - if (argc < 2) { - fputs("nonetproc_sandbox implements limited sandboxing via Linux namespaces.\n", stderr); - fputs("It takes no flags, it simply executes the command given as arguments.\n", stderr); - fputs("Usage: nonetproc_sandbox command args...\n", stderr); - return 1; - } - - // Network namespace is **NOT** sandboxed by default but it can be enabled if `SHARE_NETWORK=0` env is set - const char* share_network_env = getenv("SHARE_NETWORK"); - const bool unshare_network = share_network_env != NULL && !strcmp(share_network_env, "0"); - - // Mount namespace is sandboxed by default but it can be opted out if `SHARE_MOUNT=1` env is set - const char* share_mount_env = getenv("SHARE_MOUNT"); - const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); - - return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, false); -} diff --git a/tools/sandbox/noproc_main.c b/tools/sandbox/noproc_main.c deleted file mode 100644 index 0eab3f67f9..0000000000 --- a/tools/sandbox/noproc_main.c +++ /dev/null @@ -1,27 +0,0 @@ -// noproc_sandbox is a slightly modified version of please_sandbox that does all the same -// things except it doesn't mount /proc. -// This is a specific, if hacky, solution for newer versions of systemd which aren't -// allowing us to mount a full /proc from a new user namespace. -#include -#include -#include -#include "tools/sandbox/sandbox.h" - -int main(int argc, char* argv[]) { - if (argc < 2) { - fputs("noproc_sandbox implements limited sandboxing via Linux namespaces.\n", stderr); - fputs("It takes no flags, it simply executes the command given as arguments.\n", stderr); - fputs("Usage: noproc_sandbox command args...\n", stderr); - return 1; - } - - // Network namespace is sandboxed by default but it can be opted out if `SHARE_NETWORK=1` env is set - const char* share_network_env = getenv("SHARE_NETWORK"); - const bool unshare_network = share_network_env == NULL || strcmp(share_network_env, "1"); - - // Mount namespace is sandboxed by default but it can be opted out if `SHARE_MOUNT=1` env is set - const char* share_mount_env = getenv("SHARE_MOUNT"); - const bool unshare_mount = share_mount_env == NULL || strcmp(share_mount_env, "1"); - - return contain(&argv[1], unshare_network, unshare_mount, unshare_mount, false); -} diff --git a/tools/sandbox/test/BUILD b/tools/sandbox/test/BUILD index b1a6077ff2..b7803c542e 100644 --- a/tools/sandbox/test/BUILD +++ b/tools/sandbox/test/BUILD @@ -3,9 +3,6 @@ go_test( srcs = ["sandbox_test.go"], data = { "please_sandbox": "//tools/sandbox:please_sandbox", - "nonet_sandbox": "//tools/sandbox:nonet_sandbox", - "noproc_sandbox": "//tools/sandbox:noproc_sandbox", - "nonetproc_sandbox": "//tools/sandbox:nonetproc_sandbox", }, labels = [ "localonly", diff --git a/tools/sandbox/test/sandbox_test.go b/tools/sandbox/test/sandbox_test.go index 728ff68670..d1014dfe46 100644 --- a/tools/sandbox/test/sandbox_test.go +++ b/tools/sandbox/test/sandbox_test.go @@ -28,19 +28,62 @@ func pleaseSandbox(t *testing.T, args []string, env []string) string { return runSandbox(t, os.Getenv("DATA_PLEASE_SANDBOX"), args, env) } -func noprocSandbox(t *testing.T, args []string, env []string) string { - t.Helper() - return runSandbox(t, os.Getenv("DATA_NOPROC_SANDBOX"), args, env) -} +// TestSandboxEnvMatrix enumerates every combination of the environment variables that select +// which namespaces the sandbox creates, asserting each dimension independently: whether the +// network and mount namespaces are new, and whether a fresh /proc hides outer processes. +func TestSandboxEnvMatrix(t *testing.T) { + outerNet, err := os.Readlink("/proc/self/ns/net") + require.NoError(t, err) + outerMnt, err := os.Readlink("/proc/self/ns/mnt") + require.NoError(t, err) -func nonetSandbox(t *testing.T, args []string, env []string) string { - t.Helper() - return runSandbox(t, os.Getenv("DATA_NONET_SANDBOX"), args, env) -} + // The test's own pid exists in the outer /proc but not in a freshly remounted one. + probe := []string{"sh", "-c", fmt.Sprintf( + "readlink /proc/self/ns/net; readlink /proc/self/ns/mnt; test -d /proc/%d && echo visible || echo hidden", + os.Getpid())} + + for _, shareNetwork := range []bool{false, true} { + for _, shareMount := range []bool{false, true} { + for _, mountProc := range []bool{true, false} { + var opts []string + if shareNetwork { + opts = append(opts, "SHARE_NETWORK=1") + } + if shareMount { + opts = append(opts, "SHARE_MOUNT=1") + } + if !mountProc { + opts = append(opts, "MOUNT_PROC=0") + } + name := "default" + if len(opts) > 0 { + name = strings.Join(opts, ",") + } -func nonetprocSandbox(t *testing.T, args []string, env []string) string { - t.Helper() - return runSandbox(t, os.Getenv("DATA_NONETPROC_SANDBOX"), args, env) + t.Run(name, func(t *testing.T) { + lines := strings.Split(pleaseSandbox(t, probe, append([]string{"TMP_DIR=/tmp"}, opts...)), "\n") + require.Len(t, lines, 3) + if shareNetwork { + require.Equal(t, outerNet, lines[0], "network namespace should be shared") + } else { + require.NotEqual(t, outerNet, lines[0], "network namespace should be new") + } + if shareMount { + require.Equal(t, outerMnt, lines[1], "mount namespace should be shared") + } else { + require.NotEqual(t, outerMnt, lines[1], "mount namespace should be new") + } + // /proc is only remounted when a new mount namespace exists and + // MOUNT_PROC is not disabled. + if !shareMount && mountProc { + require.Equal(t, "hidden", lines[2], "fresh /proc should hide outer processes") + } else { + require.Equal(t, "visible", lines[2], "outer /proc should remain visible") + } + }) + } + } + } } func TestSandboxCommon(t *testing.T) { @@ -129,81 +172,32 @@ func TestSandboxCommon(t *testing.T) { want: "deny", }, } - sandboxImpl := map[string]func(*testing.T, []string, []string) string{ - "please_sandbox": pleaseSandbox, - "noproc_sandbox": noprocSandbox, - "nonet_sandbox": nonetSandbox, - "nonetproc_sandbox": nonetprocSandbox, - } - for _, tt := range tests { - for sandboxName, sandbox := range sandboxImpl { - t.Run(sandboxName+"/"+tt.name, func(t *testing.T) { - output := sandbox(t, tt.args, tt.env) - - if output != tt.want { - t.Errorf("Expected %q, but got %q", tt.want, output) - } - }) - } - } -} - -func TestSandboxNetworkShare(t *testing.T) { - handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write([]byte("Connection accepted")) - }) - ts := httptest.NewServer(handler) - defer ts.Close() - - tests := []struct { - name string - sandbox func(*testing.T, []string, []string) string - env []string - args []string - want string - }{ - { - name: "please_sandbox must isolate network by default", - sandbox: pleaseSandbox, - env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl -s %s; echo "curl exit: $?"`, ts.URL)}, - want: "curl exit: 7", // exit code 7 represents "Failed to connect to host." - }, - { - name: "nonet_sandbox must share network by default", - sandbox: nonetSandbox, - env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", "curl -sS " + ts.URL}, - want: "Connection accepted", - }, - { - name: "noproc_sandbox must isolate network by default", - sandbox: noprocSandbox, - env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", fmt.Sprintf(`curl -s %s; echo "curl exit: $?"`, ts.URL)}, - want: "curl exit: 7", - }, - { - name: "nonetproc_sandbox must share network by default", - sandbox: nonetprocSandbox, - env: []string{"TMP_DIR=/tmp"}, - args: []string{"sh", "-c", "curl -sS " + ts.URL}, - want: "Connection accepted", - }, - } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - output := tt.sandbox(t, tt.args, tt.env) + output := pleaseSandbox(t, tt.args, tt.env) if output != tt.want { - t.Errorf("Expected output to contain %q, but got: %q", tt.want, output) + t.Errorf("Expected %q, but got %q", tt.want, output) } }) } } +func TestSandboxNoTmpDir(t *testing.T) { + marker := fmt.Sprintf("/tmp/sandbox-test-marker-%d", os.Getpid()) + require.NoError(t, os.WriteFile(marker, []byte("outer"), 0o644)) + defer os.Remove(marker) + + probe := []string{"sh", "-c", fmt.Sprintf( + `echo "TMPDIR=$TMPDIR"; test -e %s && echo marker-visible || echo marker-hidden; test -e /tmp/plz_sandbox && echo bind-mounted || echo no-bind-mount`, + marker)} + out := pleaseSandbox(t, probe, []string{}) + + require.Contains(t, out, "TMPDIR=/tmp\n", "TMPDIR should point at the tmpfs mounted over /tmp") + require.Contains(t, out, "marker-hidden", "a file in the outer /tmp should be hidden by the tmpfs mounted over it") + require.Contains(t, out, "no-bind-mount", "nothing should be bind mounted at /tmp/plz_sandbox when TMP_DIR is unset") +} + func TestSandboxLocalIP(t *testing.T) { tests := []struct { name string @@ -230,39 +224,17 @@ func TestSandboxLocalIP(t *testing.T) { want: "0", }, } - sandboxImpl := map[string]func(*testing.T, []string, []string) string{ - "please_sandbox": pleaseSandbox, - "noproc_sandbox": noprocSandbox, - } for _, tt := range tests { - for sandboxName, sandbox := range sandboxImpl { - t.Run(sandboxName+"/"+tt.name, func(t *testing.T) { - output := sandbox(t, tt.args, tt.env) + t.Run(tt.name, func(t *testing.T) { + output := pleaseSandbox(t, tt.args, tt.env) - if output != tt.want { - t.Errorf("Expected %q, but got %q", tt.want, output) - } - }) - } + if output != tt.want { + t.Errorf("Expected %q, but got %q", tt.want, output) + } + }) } } -func TestSandboxMountShare(t *testing.T) { - nsOutside, err := os.Readlink("/proc/self/ns/mnt") - require.NoError(t, err, "failed to readlink /proc/self/ns/mnt outside sandbox") - - t.Run("please_sandbox must isolate mounts by default", func(t *testing.T) { - nsInside := pleaseSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"TMP_DIR=" + os.TempDir()}) - require.Regexp(t, `^mnt:\[\d+\]`, nsInside) - require.NotEqual(t, nsInside, nsOutside) - }) - - t.Run("please_sandbox must share mount namespace when called with SHARE_MOUNT=1", func(t *testing.T) { - nsInside := pleaseSandbox(t, []string{"readlink", "/proc/self/ns/mnt"}, []string{"SHARE_MOUNT=1"}) - require.Equal(t, nsInside, nsOutside) - }) -} - // TestSandboxNoOrphanOnParentCrash checks that the sandboxed command does not outlive the // sandbox process itself. Unlike TestSandboxHangOnParentCrash, which tests the window // between clone() and the sync pipe, this kills the sandbox after the command has exec'd, @@ -316,7 +288,7 @@ func TestSandboxHangOnParentCrash(t *testing.T) { } // We run this in a loop with different delays to reliably hit the race condition window where - // the parent dies after clone() but before sending SIGUSR1 / writing to the pipe. + // the parent dies after clone() but before writing to the sync pipe. for i := range 20 { t.Run(fmt.Sprintf("iteration_%d", i), func(t *testing.T) { r, w, err := os.Pipe() From 0802e60bf825a35dc37779c2b717a81bd37c560b Mon Sep 17 00:00:00 2001 From: Grant Zvolsky Date: Mon, 10 Aug 2026 22:25:30 +0000 Subject: [PATCH 6/6] ci: run the sandbox tests via rootless podman on machine executors - Run the sandbox tests on machine executors via rootless podman. The docker executor was not sufficient ("remount: Permission denied"), likely due to apparmor settings that are not configurable. //tools/sandbox/test:sandbox_test therefore remains manual. - Make release-gs conditional on the new sandbox tests - Update README - TODO: Remove container setup steps once the new images are tagged: https://github.com/thought-machine/please/issues/3571 --- .circleci/config.yml | 143 +++++++++++++++++++++++++++++++++++++++ tools/sandbox/README.md | 8 ++- tools/sandbox/test/BUILD | 3 +- 3 files changed, 152 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4e07834bf9..188da5f951 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -316,6 +316,143 @@ jobs: at: /tmp/workspace - run: ./.circleci/release.sh + # The sandbox tests (//tools/sandbox/test:sandbox_test) create user, mount, + # and network namespaces and mount within them. The docker executor denies + # mount inside the job container and doesn't expose any configuration to + # allow it. Instead, these workflows run the sandbox tests on machine + # executors with rootless podman. + # + # podman's default seccomp profile allows unprivileged namespace and mount + # syscalls, and the kernel gates them on CAP_SYS_ADMIN in the caller's user + # namespace, which the sandbox creates for itself, so no custom seccomp + # profile and no CAP_SYS_ADMIN are needed. The container only needs + # SETUID/SETGID (new*idmap writing multi-range maps) and SETFCAP (mapping + # parent uid 0 into a user namespace requires it, see user_namespaces(7)). + # unmask=ALL is required so a /proc can be mounted in the nested mount + # namespace (the kernel refuses while runtime /proc masks are in place). + sandbox-test-linux: + working_directory: ~/please + machine: + image: ubuntu-2404:current + resource_class: medium + steps: + - checkout + - run: + name: Set up rootless podman + command: | + sudo apt-get update + sudo apt-get install -y podman catatonit uidmap passt slirp4netns + grep -q "^$(id -un):" /etc/subuid || sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 "$(id -un)" + # The job shell has no systemd user session or D-Bus, so podman's + # default systemd cgroup manager fails to create transient scopes + # ("Interactive authentication required"); use cgroupfs instead. + mkdir -p ~/.config/containers + printf '[engine]\ncgroup_manager = "cgroupfs"\n' > ~/.config/containers/containers.conf + podman system migrate || true + - run: + name: Run sandbox tests + command: | + podman run --rm --init \ + --security-opt unmask=ALL \ + --cap-add SETUID --cap-add SETGID --cap-add SETFCAP \ + -v "$PWD:/please" -w /please \ + -e PLZ_ARGS="--profile ci" \ + ghcr.io/thought-machine/please_ubuntu:20260318 \ + sh -ec ' + # Mirrors the tools/images Dockerfile changes; drop once new + # image tags containing them are published and referenced. + apt-get update && apt-get install -y uidmap + printf "root:0:65536\n" >> /etc/subuid + printf "root:0:65536\n" >> /etc/subgid + ./pleasew -p -v2 test --rerun --test_results_file plz-out/results/sandbox/test_results.xml --log_file plz-out/log/sandbox_test.log //tools/sandbox/test:sandbox_test + ' + - store_test_results: + path: plz-out/results + - store_artifacts: + path: plz-out/log + + sandbox-test-linux-alt: + working_directory: ~/please + machine: + image: ubuntu-2404:current + resource_class: medium + steps: + - checkout + - run: + name: Set up rootless podman + command: | + sudo apt-get update + sudo apt-get install -y podman catatonit uidmap passt slirp4netns + grep -q "^$(id -un):" /etc/subuid || sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 "$(id -un)" + # The job shell has no systemd user session or D-Bus, so podman's + # default systemd cgroup manager fails to create transient scopes + # ("Interactive authentication required"); use cgroupfs instead. + mkdir -p ~/.config/containers + printf '[engine]\ncgroup_manager = "cgroupfs"\n' > ~/.config/containers/containers.conf + podman system migrate || true + - run: + name: Run sandbox tests + command: | + podman run --rm --init \ + --security-opt unmask=ALL \ + --cap-add SETUID --cap-add SETGID --cap-add SETFCAP \ + -v "$PWD:/please" -w /please \ + -e PLZ_ARGS="--profile ci-alt" \ + ghcr.io/thought-machine/please_ubuntu_alt:20260318 \ + sh -ec ' + # Mirrors the tools/images Dockerfile changes; drop once new + # image tags containing them are published and referenced. + apt-get update && apt-get install -y uidmap + printf "root:0:65536\n" >> /etc/subuid + printf "root:0:65536\n" >> /etc/subgid + ./pleasew -p -v2 test --rerun --test_results_file plz-out/results/sandbox/test_results.xml --log_file plz-out/log/sandbox_test.log //tools/sandbox/test:sandbox_test + ' + - store_test_results: + path: plz-out/results + - store_artifacts: + path: plz-out/log + + sandbox-test-alpine: + working_directory: ~/please + machine: + image: ubuntu-2404:current + resource_class: medium + steps: + - checkout + - run: + name: Set up rootless podman + command: | + sudo apt-get update + sudo apt-get install -y podman catatonit uidmap passt slirp4netns + grep -q "^$(id -un):" /etc/subuid || sudo usermod --add-subuids 100000-165535 --add-subgids 100000-165535 "$(id -un)" + # The job shell has no systemd user session or D-Bus, so podman's + # default systemd cgroup manager fails to create transient scopes + # ("Interactive authentication required"); use cgroupfs instead. + mkdir -p ~/.config/containers + printf '[engine]\ncgroup_manager = "cgroupfs"\n' > ~/.config/containers/containers.conf + podman system migrate || true + - run: + name: Run sandbox tests + command: | + podman run --rm --init \ + --security-opt unmask=ALL \ + --cap-add SETUID --cap-add SETGID --cap-add SETFCAP \ + -v "$PWD:/please" -w /please \ + -e PLZ_ARGS="--profile ci --profile alpine --exclude no-musl" \ + ghcr.io/thought-machine/please_alpine:20260318 \ + sh -ec ' + # Mirrors the tools/images Dockerfile changes; drop once new + # image tags containing them are published and referenced. + apk add --no-cache shadow-uidmap curl + printf "root:0:65536\n" >> /etc/subuid + printf "root:0:65536\n" >> /etc/subgid + ./pleasew -p -v2 test --rerun --test_results_file plz-out/results/sandbox/test_results.xml --log_file plz-out/log/sandbox_test.log //tools/sandbox/test:sandbox_test + ' + - store_test_results: + path: plz-out/results + - store_artifacts: + path: plz-out/log + # Runs a benchmarking test and records some performance results. perf-test: docker: @@ -337,6 +474,9 @@ workflows: - build-alpine - build-linux - build-linux-alt + - sandbox-test-linux + - sandbox-test-linux-alt + - sandbox-test-alpine - build-darwin: requires: - build-alpine @@ -366,6 +506,9 @@ workflows: - build-darwin - test-rex - test-http-cache + - sandbox-test-linux + - sandbox-test-linux-alt + - sandbox-test-alpine filters: branches: only: master diff --git a/tools/sandbox/README.md b/tools/sandbox/README.md index 47e0b1d308..c0a6d73c4b 100644 --- a/tools/sandbox/README.md +++ b/tools/sandbox/README.md @@ -26,7 +26,8 @@ e.g. for systems that don't allow mounting a full `/proc` from a new user namesp mount namespace notes below). When the network namespace is used, the loopback interface is brought up with an additional IP -address. `SANDBOX_LOCAL_IP` defines, defined empty string disables the extra address. +address, which defaults to 10.1.1.1. The `SANDBOX_LOCAL_IP` environment variable can be used +to change this IP, and setting it to the empty string disables the extra address entirely. ## Using the knobs with Please @@ -84,3 +85,8 @@ is not fully visible, [see this commit](https://github.com/torvalds/linux/commit This is an issue with most container runtimes (and therefore Kubernetes), as by default, they will hide some part of /proc in a container to reduce attack surface, eg [see this docker PR](https://github.com/docker/cli/pull/1808). + +Users of the sandbox have different options: +- unmask /proc (k8s pod `procMount: Unmasked` securityContext option combined with `hostUsers: false`), +- disable the /proc isolation (`MOUNT_PROC=0`), in which case the sandboxed process will be able to see (but not send signals to) processes outside the sandbox, +- disable the mount namespace entirely with `SHARE_MOUNT=1`. diff --git a/tools/sandbox/test/BUILD b/tools/sandbox/test/BUILD index b7803c542e..15ccba455a 100644 --- a/tools/sandbox/test/BUILD +++ b/tools/sandbox/test/BUILD @@ -5,7 +5,8 @@ go_test( "please_sandbox": "//tools/sandbox:please_sandbox", }, labels = [ - "localonly", + # These tests are executed explicitly via machine executors in .circleci/config.yml + # For local testing execute in a container with equivalent configuration. "manual", ], sandbox = False,