diff --git a/CLAUDE.md b/CLAUDE.md index 1ad2cae..8e0fadb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -140,6 +140,19 @@ Source layout (all under `src/`): `release_session_lock()` once `run_process_foreground()` returns (covering every exit path — normal, nonzero, or a forwarded-signal exit — since that call always blocks until the child has actually exited). + `build_bwrap_args()` no longer passes `--clearenv`/`--setenv` to `bwrap` itself; + instead, `build_sandbox_env()` builds the sandboxed command's exact environment + (`PATH`, `HOME`, `PWD` — hardcoded to `"/"`, matching `--chdir`'s own value; note + per bwrap's own man page `--clearenv` never actually unset `PWD` in the first + place, so this isn't a straight port of a prior `--setenv` — and `TERM`, only + if the host process has one) and `run_bwrap()` passes it straight to + `run_process_foreground()`'s own `env` override (see `process.{h,cpp}` below). + This works because `bwrap` (and `nsenter`, when interposed via + `wrap_for_root_namespace()`) doesn't alter its own inherited environment unless + told to, and neither does `slocker-lite-priv-drop` (just + `setgroups()`/`setgid()`/`setuid()`/`execvp()`, no env manipulation) — so + controlling it once, at the outermost exec, is sufficient for it to reach the + final sandboxed command unchanged. - `priv_drop_helper.cpp` → the separate `slocker-lite-priv-drop` binary (its own `executable()` target in `meson.build`, **built with `-static`**). Deliberately has zero dependencies on the rest of this project (no fmt/spdlog/etc.) and is @@ -163,9 +176,9 @@ Source layout (all under `src/`): carries `home`, looked up by the final resolved uid's `/etc/passwd` entry (field 5) regardless of whether `user` was given as a name or a number; falls back to `"/root"` for uid 0 or `"/"` otherwise when there's no matching row. - `build_bwrap_args()` (`bwrap.cpp`) sets the sandboxed process's `HOME` from this — - `"/root"` only when no user override applies at all (no `--user`, no image-declared - `config.User`). `run_container()` (`main.cpp`) calls `resolve_user_and_group()` + `build_sandbox_env()` (`bwrap.cpp`) sets the sandboxed process's `HOME` from + this — `"/root"` only when no user override applies at all (no `--user`, no + image-declared `config.User`). `run_container()` (`main.cpp`) calls `resolve_user_and_group()` with either the explicit `--user`/`--group` flags, or, when `--user` wasn't given, the image's own declared `config.User` (`OciImageConfig::user`/`group`) — so a container defaults to running as whatever user the image itself declares, not @@ -185,7 +198,13 @@ Source layout (all under `src/`): itself execs into something else first (e.g. `nsenter` handing off to the final command via its own in-place `execvp()` — a pid never changes across `exec()`). `run_bwrap()` (`bwrap.cpp`) is the one caller that uses it, for session pid-file - tracking (see `pid_file.{h,cpp}` below). + tracking (see `pid_file.{h,cpp}` below). `run_process_foreground()` also takes + an optional `env` (list of key/value pairs): when set, the forked child + replaces its entire environment via `clearenv()`/`setenv()` (plain POSIX, not + the GNU-only `execvpe()` — the target platform includes musl) before `execvp()`, + instead of inheriting this process's own. `nullopt` (the default) leaves the + child's environment untouched. `run_bwrap()` is again the one caller that uses + this, via `build_sandbox_env()` (`bwrap.cpp`) — see there. - `pid_file.{h,cpp}` — tracks one running `-r/--run` session (a live `bwrap` process) as a locked pid file, so an outside process (or a later `slocker-lite` invocation) can tell whether it's still running. diff --git a/src/bwrap.cpp b/src/bwrap.cpp index 6ac2a04..62149ba 100644 --- a/src/bwrap.cpp +++ b/src/bwrap.cpp @@ -64,6 +64,29 @@ std::optional find_priv_drop_helper() { return candidate; } +// Builds the exact environment the sandboxed command should see. Passed directly +// to run_process_foreground() as the environment to exec bwrap with, rather than +// relying on bwrap's own --clearenv/--setenv (which run_bwrap() no longer uses) -- +// bwrap, and any nsenter/priv-drop-helper interposed ahead of it, all just forward +// whatever environment they were themselves launched with, so controlling it once +// here is sufficient. PWD is set to match --chdir's own value in +// build_bwrap_args() ("/") -- per bwrap's own man page, --clearenv never actually +// unset PWD in the first place (bwrap manages it separately, alongside --chdir), +// so this isn't a straight port of a prior --setenv, just keeping the explicitly +// constructed environment a complete, accurate match for what the sandbox should +// see. +std::vector> build_sandbox_env(std::optional user) { + std::vector> env = { + {"PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}, + {"HOME", user ? user->home : "/root"}, + {"PWD", "/"}, + }; + if (const char* term = std::getenv("TERM")) { + env.emplace_back("TERM", term); + } + return env; +} + struct NamespaceProbe { int clone_flag; const char* bwrap_arg; @@ -234,22 +257,13 @@ std::vector build_bwrap_args(const std::string& root, "/tmp", "--chdir", "/", - "--clearenv", - "--setenv", - "PATH", - "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", }; args.insert(args.end(), filesystem_args.begin(), filesystem_args.end()); - args.insert(args.end(), {"--setenv", "HOME", user ? user->home : "/root"}); for (const auto& volume : volumes) { args.insert(args.end(), {"--bind", volume.host_directory, volume.container_path}); } - if (const char* term = std::getenv("TERM")) { - args.insert(args.end(), {"--setenv", "TERM", term}); - } - bool bound_priv_drop_helper = false; if (user) { auto helper_path = find_priv_drop_helper(); @@ -315,7 +329,8 @@ int run_bwrap(const std::string& root, const std::vector& command, std::optional session_lock; int exit_code = run_process_foreground( - *argv, [&](pid_t pid) { session_lock = create_session_lock(container_name, pid); }); + *argv, [&](pid_t pid) { session_lock = create_session_lock(container_name, pid); }, + build_sandbox_env(user)); if (session_lock) { release_session_lock(*session_lock); diff --git a/src/bwrap.h b/src/bwrap.h index 46264e8..0f10e45 100644 --- a/src/bwrap.h +++ b/src/bwrap.h @@ -78,6 +78,9 @@ std::vector build_bwrap_args(const std::string& root, // see there for when it does/doesn't take effect. While bwrap is running, // `container_name` (paired with its actual pid) is recorded as a locked session // pid file under $XDG_STATE_HOME (see pid_file.h) -- removed again once it exits. +// The sandboxed command's environment is built directly here (build_sandbox_env()) +// and passed to run_process_foreground()'s own env override, rather than relying +// on bwrap's own --clearenv/--setenv (which build_bwrap_args() no longer uses). // Returns bwrap's exit code, or -1 on failure to launch. int run_bwrap(const std::string& root, const std::vector& command, bool use_nsenter, const std::vector& volumes, std::optional user, diff --git a/src/process.cpp b/src/process.cpp index 94ed150..a3db806 100644 --- a/src/process.cpp +++ b/src/process.cpp @@ -108,8 +108,8 @@ ProcessResult run_process(const std::vector& argv) { return {exit_code, output}; } -int run_process_foreground(const std::vector& argv, - const std::function& on_start) { +int run_process_foreground(const std::vector& argv, const std::function& on_start, + const std::optional>>& env) { spdlog::debug("running external command: {}", fmt::join(argv, " ")); pid_t pid = fork(); @@ -118,6 +118,12 @@ int run_process_foreground(const std::vector& argv, } if (pid == 0) { + if (env) { + clearenv(); + for (const auto& [key, value] : *env) { + setenv(key.c_str(), value.c_str(), 1); + } + } auto c_argv = to_c_argv(argv); execvp(c_argv[0], c_argv.data()); const char* msg = "run_process_foreground: execvp failed\n"; diff --git a/src/process.h b/src/process.h index 05cf14a..d3ee76a 100644 --- a/src/process.h +++ b/src/process.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -40,10 +41,16 @@ ProcessResult run_process(const std::vector& argv); // succeeds and before this blocks in waitpid() -- e.g. so a caller can record the // real pid of what it just launched. This stays accurate even when argv itself // execs into something else before the real target (e.g. nsenter handing off to -// the final command), since exec() never changes the pid. Returns the exit code, -// or -1 if fork or exec failed (on_start is not called in that case). +// the final command), since exec() never changes the pid. If `env` is set, the +// child replaces its entire environment with exactly these key/value pairs (via +// clearenv()/setenv(), before exec) instead of inheriting this process's own -- +// e.g. run_bwrap() uses this to give the sandboxed command a minimal, controlled +// environment without relying on bwrap's own --clearenv/--setenv. Returns the +// exit code, or -1 if fork or exec failed (on_start is not called in that case). int run_process_foreground(const std::vector& argv, - const std::function& on_start = nullptr); + const std::function& on_start = nullptr, + const std::optional>>& env = + std::nullopt); // Searches $PATH for an executable regular file named `name`, in PATH order. // Returns its full path, or nullopt if not found.