Add --env/--env-file to set environment variables under -r/--run
Both flags are repeatable and share a single ordered EnvSpec list
(env_spec.{h,cpp}) so a later --env or --env-file always overrides an
earlier one for the same name, regardless of which flag set it.
--env-file reads one VAR=VALUE per line, skipping blank lines and
#-comments. resolve_env_specs()'s result is appended after
build_sandbox_env()'s own PATH/HOME/PWD/TERM defaults, letting an
explicit --env override any of them too.
Testing this surfaced a real bug in the environment-at-exec-time
mechanism from the previous change (dropping bwrap's own --clearenv):
since bwrap/nsenter are now exec'd with the same explicitly-built
environment the sandbox sees, a --env PATH=... override broke
execvp()'s ability to even locate bwrap/nsenter themselves (bare-name
PATH lookup happens in the child, using the already-overridden PATH).
Fixed by resolving both to absolute paths via find_in_path(), called
from this process's own unmodified environment before fork() --
confirmed by testing that only the sandboxed command's own lookup is
now affected by a PATH override, not bwrap/nsenter.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
This commit is contained in:
+26
-7
@@ -74,8 +74,13 @@ std::optional<std::filesystem::path> find_priv_drop_helper() {
|
||||
// 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<std::pair<std::string, std::string>> build_sandbox_env(std::optional<ResolvedUser> user) {
|
||||
// see. `extra_env` (from --env/--env-file, already resolved by
|
||||
// resolve_env_specs() in env_spec.h) is appended last -- a duplicate key doesn't
|
||||
// need deduplicating here, since run_process_foreground()'s own setenv(..., 1)
|
||||
// loop naturally lets the later occurrence in iteration order win, so an
|
||||
// explicit --env PATH=... still overrides the default above.
|
||||
std::vector<std::pair<std::string, std::string>> build_sandbox_env(
|
||||
std::optional<ResolvedUser> user, const std::vector<std::pair<std::string, std::string>>& extra_env) {
|
||||
std::vector<std::pair<std::string, std::string>> env = {
|
||||
{"PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
|
||||
{"HOME", user ? user->home : "/root"},
|
||||
@@ -84,6 +89,7 @@ std::vector<std::pair<std::string, std::string>> build_sandbox_env(std::optional
|
||||
if (const char* term = std::getenv("TERM")) {
|
||||
env.emplace_back("TERM", term);
|
||||
}
|
||||
env.insert(env.end(), extra_env.begin(), extra_env.end());
|
||||
return env;
|
||||
}
|
||||
|
||||
@@ -208,7 +214,14 @@ std::vector<std::string> build_bwrap_args(const std::string& root,
|
||||
// --new-session detaches from the controlling terminal, which breaks job
|
||||
// control for an interactive foreground shell ("can't access tty"). Re-enable
|
||||
// once background/daemonized runs are implemented, where that's the point.
|
||||
std::vector<std::string> args = {"bwrap", "--die-with-parent"};
|
||||
// Resolved to an absolute path (via this process's own PATH, not the
|
||||
// sandbox's) rather than left as the bare name "bwrap" -- run_process_foreground()
|
||||
// execs this argv[0] using build_sandbox_env()'s environment, so a --env
|
||||
// override of PATH must not be able to break locating bwrap itself. Falls
|
||||
// back to the bare name if not found here (checked earlier and unconditionally
|
||||
// by check_required_dependencies() anyway).
|
||||
auto bwrap_path = find_in_path("bwrap");
|
||||
std::vector<std::string> args = {bwrap_path ? bwrap_path->string() : "bwrap", "--die-with-parent"};
|
||||
|
||||
auto unshare_args = detect_bwrap_unshare_args();
|
||||
bool has_uts_ns = false;
|
||||
@@ -301,12 +314,17 @@ std::optional<std::vector<std::string>> wrap_for_root_namespace(const std::strin
|
||||
spdlog::error("could not find the fuse-overlayfs process serving {}", root);
|
||||
return std::nullopt;
|
||||
}
|
||||
if (!find_in_path("nsenter")) {
|
||||
auto nsenter_path = find_in_path("nsenter");
|
||||
if (!nsenter_path) {
|
||||
spdlog::error("nsenter not found in PATH");
|
||||
return std::nullopt;
|
||||
}
|
||||
|
||||
std::vector<std::string> wrapped = {"nsenter", fmt::format("--user=/proc/{}/ns/user", *pid),
|
||||
// Resolved to an absolute path (this process's own PATH) rather than left as
|
||||
// the bare name "nsenter" -- when wrapping a bwrap invocation, argv[0] here
|
||||
// gets exec'd using build_sandbox_env()'s environment (see run_bwrap()), so a
|
||||
// --env override of PATH must not be able to break locating nsenter itself.
|
||||
std::vector<std::string> wrapped = {nsenter_path->string(), fmt::format("--user=/proc/{}/ns/user", *pid),
|
||||
fmt::format("--mount=/proc/{}/ns/mnt", *pid), "--"};
|
||||
wrapped.insert(wrapped.end(), argv.begin(), argv.end());
|
||||
return wrapped;
|
||||
@@ -314,7 +332,8 @@ std::optional<std::vector<std::string>> wrap_for_root_namespace(const std::strin
|
||||
|
||||
int run_bwrap(const std::string& root, const std::vector<std::string>& command, bool use_nsenter,
|
||||
const std::vector<ResolvedVolumeMount>& volumes, std::optional<ResolvedUser> user,
|
||||
const std::optional<std::string>& hostname, const std::string& container_name) {
|
||||
const std::optional<std::string>& hostname, const std::string& container_name,
|
||||
const std::vector<std::pair<std::string, std::string>>& extra_env) {
|
||||
if (user && !find_priv_drop_helper()) {
|
||||
spdlog::error("could not find the {} helper next to this binary; --user/--group requires it",
|
||||
kPrivDropHelperName);
|
||||
@@ -330,7 +349,7 @@ int run_bwrap(const std::string& root, const std::vector<std::string>& command,
|
||||
std::optional<SessionLock> session_lock;
|
||||
int exit_code = run_process_foreground(
|
||||
*argv, [&](pid_t pid) { session_lock = create_session_lock(container_name, pid); },
|
||||
build_sandbox_env(user));
|
||||
build_sandbox_env(user, extra_env));
|
||||
|
||||
if (session_lock) {
|
||||
release_session_lock(*session_lock);
|
||||
|
||||
+9
-5
@@ -18,6 +18,7 @@
|
||||
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
|
||||
#include "volume_mount.h"
|
||||
@@ -78,10 +79,13 @@ std::vector<std::string> 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.
|
||||
// The sandboxed command's environment is built directly here (build_sandbox_env()),
|
||||
// with `extra_env` (from --env/--env-file, see env_spec.h) appended after the
|
||||
// built-in PATH/HOME/PWD/TERM, 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<std::string>& command, bool use_nsenter,
|
||||
const std::vector<ResolvedVolumeMount>& volumes, std::optional<ResolvedUser> user,
|
||||
const std::optional<std::string>& hostname, const std::string& container_name);
|
||||
const std::optional<std::string>& hostname, const std::string& container_name,
|
||||
const std::vector<std::pair<std::string, std::string>>& extra_env);
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
// Copyright (C) 2026 Viorel Munteanu
|
||||
//
|
||||
// This program is free software; you can redistribute it and/or modify
|
||||
// it under the terms of the GNU General Public License as published by
|
||||
// the Free Software Foundation; either version 2 of the License, or
|
||||
// (at your option) any later version.
|
||||
//
|
||||
// This program is distributed in the hope that it will be useful,
|
||||
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
// GNU General Public License for more details.
|
||||
//
|
||||
// You should have received a copy of the GNU General Public License along
|
||||
// with this program; if not, write to the Free Software Foundation, Inc.,
|
||||
// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
|
||||
|
||||
#include "env_spec.h"
|
||||
|
||||
#include <fstream>
|
||||
|
||||
#include <spdlog/spdlog.h>
|
||||
|
||||
namespace {
|
||||
|
||||
std::optional<std::pair<std::string, std::string>> parse_env_line(const std::string& line) {
|
||||
auto eq = line.find('=');
|
||||
if (eq == std::string::npos || eq == 0) {
|
||||
return std::nullopt;
|
||||
}
|
||||
return std::make_pair(line.substr(0, eq), line.substr(eq + 1));
|
||||
}
|
||||
|
||||
bool is_blank_or_comment(const std::string& line) {
|
||||
auto first = line.find_first_not_of(" \t\r\n");
|
||||
return first == std::string::npos || line[first] == '#';
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
std::optional<std::vector<std::pair<std::string, std::string>>> resolve_env_specs(
|
||||
const std::vector<EnvSpec>& specs) {
|
||||
std::vector<std::pair<std::string, std::string>> result;
|
||||
|
||||
for (const auto& spec : specs) {
|
||||
if (!spec.is_file) {
|
||||
auto parsed = parse_env_line(spec.value);
|
||||
if (!parsed) {
|
||||
spdlog::error("--env requires VARIABLE=VALUE, got '{}'", spec.value);
|
||||
return std::nullopt;
|
||||
}
|
||||
result.push_back(std::move(*parsed));
|
||||
continue;
|
||||
}
|
||||
|
||||
std::ifstream file(spec.value);
|
||||
if (!file) {
|
||||
spdlog::error("could not open --env-file {}", spec.value);
|
||||
return std::nullopt;
|
||||
}
|
||||
|
||||
std::string line;
|
||||
size_t line_no = 0;
|
||||
while (std::getline(file, line)) {
|
||||
++line_no;
|
||||
if (!line.empty() && line.back() == '\r') {
|
||||
line.pop_back();
|
||||
}
|
||||
if (is_blank_or_comment(line)) {
|
||||
continue;
|
||||
}
|
||||
auto parsed = parse_env_line(line);
|
||||
if (!parsed) {
|
||||
spdlog::error("{}:{}: expected VARIABLE=VALUE, got '{}'", spec.value, line_no, line);
|
||||
return std::nullopt;
|
||||
}
|
||||
result.push_back(std::move(*parsed));
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
@@ -0,0 +1,44 @@
|
||||
// Copyright (C) 2026 Viorel Munteanu
|
||||
//
|
||||
// This program is free software; you can redistribute it and/or modify
|
||||
// it under the terms of the GNU General Public License as published by
|
||||
// the Free Software Foundation; either version 2 of the License, or
|
||||
// (at your option) any later version.
|
||||
//
|
||||
// This program is distributed in the hope that it will be useful,
|
||||
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
// GNU General Public License for more details.
|
||||
//
|
||||
// You should have received a copy of the GNU General Public License along
|
||||
// with this program; if not, write to the Free Software Foundation, Inc.,
|
||||
// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
|
||||
|
||||
#pragma once
|
||||
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
|
||||
// One --env or --env-file occurrence, in the exact order given on the command
|
||||
// line -- both flags share a single ordered list so a later one of either kind
|
||||
// can override an earlier one (see resolve_env_specs()).
|
||||
struct EnvSpec {
|
||||
bool is_file; // true: `value` is a path (--env-file); false: a literal VAR=VALUE (--env)
|
||||
std::string value;
|
||||
};
|
||||
|
||||
// Resolves `specs`, in order, into a flat list of (key, value) pairs ready to
|
||||
// append to the sandbox's base environment (see build_sandbox_env() in
|
||||
// bwrap.cpp) -- later entries for the same key win, same as a real environment,
|
||||
// since the caller just appends them all and setenv() naturally overwrites.
|
||||
// A literal --env entry is split at its *first* '=' (VALUE may itself contain
|
||||
// '='; the key before it must be non-empty). A --env-file entry is read line by
|
||||
// line: blank/whitespace-only lines and lines whose first non-whitespace
|
||||
// character is '#' are skipped (comments), with a trailing '\r' stripped first
|
||||
// for CRLF files; every other line is parsed the same way as a literal. Logs a
|
||||
// specific error and returns nullopt on the first hard failure: a
|
||||
// literal/file-line with no '=' or an empty key, or a file that can't be opened.
|
||||
std::optional<std::vector<std::pair<std::string, std::string>>> resolve_env_specs(
|
||||
const std::vector<EnvSpec>& specs);
|
||||
+34
-8
@@ -34,6 +34,7 @@
|
||||
#include "config.h"
|
||||
#include "config_file.h"
|
||||
#include "containers_storage.h"
|
||||
#include "env_spec.h"
|
||||
#include "exec_session.h"
|
||||
#include "oci_image.h"
|
||||
#include "pid_file.h"
|
||||
@@ -64,10 +65,10 @@ enum class Mode {
|
||||
};
|
||||
|
||||
// --log-level/--user/--group/--list-volumes/--delete-volume[-full]/--hostname/
|
||||
// --list-processes/--clean-processes have no short form (--log-level's was freed
|
||||
// up so -l could become --list-images; -u is already --umount; the rest have no
|
||||
// natural free letter left, or don't need one), so they need long-option vals
|
||||
// outside the printable-char range short options use.
|
||||
// --list-processes/--clean-processes/--env/--env-file have no short form
|
||||
// (--log-level's was freed up so -l could become --list-images; -u is already
|
||||
// --umount; the rest have no natural free letter left, or don't need one), so
|
||||
// they need long-option vals outside the printable-char range short options use.
|
||||
constexpr int kLogLevelOpt = 256;
|
||||
constexpr int kUserOpt = 257;
|
||||
constexpr int kGroupOpt = 258;
|
||||
@@ -77,8 +78,10 @@ constexpr int kDeleteVolumeFullOpt = 261;
|
||||
constexpr int kHostnameOpt = 262;
|
||||
constexpr int kListProcessesOpt = 263;
|
||||
constexpr int kCleanProcessesOpt = 264;
|
||||
constexpr int kEnvOpt = 265;
|
||||
constexpr int kEnvFileOpt = 266;
|
||||
|
||||
constexpr std::array<struct option, 22> kLongOptions = {{
|
||||
constexpr std::array<struct option, 24> kLongOptions = {{
|
||||
{"help", no_argument, nullptr, 'h'},
|
||||
{"version", no_argument, nullptr, 'V'},
|
||||
{"test", no_argument, nullptr, 't'},
|
||||
@@ -100,6 +103,8 @@ constexpr std::array<struct option, 22> kLongOptions = {{
|
||||
{"hostname", required_argument, nullptr, kHostnameOpt},
|
||||
{"list-processes", no_argument, nullptr, kListProcessesOpt},
|
||||
{"clean-processes", no_argument, nullptr, kCleanProcessesOpt},
|
||||
{"env", required_argument, nullptr, kEnvOpt},
|
||||
{"env-file", required_argument, nullptr, kEnvFileOpt},
|
||||
{nullptr, 0, nullptr, 0},
|
||||
}};
|
||||
|
||||
@@ -151,6 +156,15 @@ void print_usage(const char* prog) {
|
||||
" takes effect if the running kernel supports\n"
|
||||
" --unshare-uts; otherwise ignored with a\n"
|
||||
" warning)\n"
|
||||
" --env VAR=VALUE with --run, set an environment variable in the\n"
|
||||
" sandbox (overrides the default PATH/HOME/PWD/\n"
|
||||
" TERM if given the same name). May be repeated;\n"
|
||||
" combined with --env-file in command-line order,\n"
|
||||
" each later one winning over an earlier one for\n"
|
||||
" the same name\n"
|
||||
" --env-file <file> with --run, load environment variables from\n"
|
||||
" <file> (one VAR=VALUE per line; blank lines and\n"
|
||||
" #-comments are skipped). May be repeated\n"
|
||||
" -l, --list-images <dir> list OCI Image Layout tars (*.tar, *.tar.*) found\n"
|
||||
" directly in <dir>, with their name:tag\n"
|
||||
" -i, --inspect <image.tar> print an image's declared user, exposed ports,\n"
|
||||
@@ -526,7 +540,7 @@ int run_container(const std::filesystem::path& image_tar,
|
||||
const std::optional<std::string>& user, const std::optional<std::string>& group,
|
||||
const std::optional<std::string>& hostname,
|
||||
const std::vector<std::pair<std::string, std::string>>& volume_specs,
|
||||
const AppConfig& app_config) {
|
||||
const std::vector<EnvSpec>& env_specs, const AppConfig& app_config) {
|
||||
auto mounted = mount_image(image_tar);
|
||||
if (!mounted) {
|
||||
return 1;
|
||||
@@ -564,6 +578,11 @@ int run_container(const std::filesystem::path& image_tar,
|
||||
volume_mounts.push_back(std::move(*resolved));
|
||||
}
|
||||
|
||||
auto resolved_env = resolve_env_specs(env_specs);
|
||||
if (!resolved_env) {
|
||||
ok = false;
|
||||
}
|
||||
|
||||
// Falls back to the image's own declared user (config.User) when --user wasn't
|
||||
// given on the command line, rather than always defaulting to root.
|
||||
std::optional<std::string> effective_user = user;
|
||||
@@ -594,7 +613,7 @@ int run_container(const std::filesystem::path& image_tar,
|
||||
int exit_code = -1;
|
||||
if (ok) {
|
||||
exit_code = run_bwrap(mounted->merged_path, command, use_nsenter, volume_mounts, resolved_user, hostname,
|
||||
container_name);
|
||||
container_name, *resolved_env);
|
||||
if (exit_code < 0) {
|
||||
spdlog::error("failed to run bwrap");
|
||||
}
|
||||
@@ -629,6 +648,7 @@ int main(int argc, char* argv[]) {
|
||||
std::optional<std::string> group_flag;
|
||||
std::optional<std::string> hostname_flag;
|
||||
std::vector<std::pair<std::string, std::string>> volume_specs;
|
||||
std::vector<EnvSpec> env_specs;
|
||||
|
||||
opterr = 0;
|
||||
int opt;
|
||||
@@ -738,6 +758,12 @@ int main(int argc, char* argv[]) {
|
||||
case kHostnameOpt:
|
||||
hostname_flag = optarg;
|
||||
break;
|
||||
case kEnvOpt:
|
||||
env_specs.push_back({false, optarg});
|
||||
break;
|
||||
case kEnvFileOpt:
|
||||
env_specs.push_back({true, optarg});
|
||||
break;
|
||||
case ':':
|
||||
spdlog::error("option requires an argument: -{}", static_cast<char>(optopt));
|
||||
print_usage(argv[0]);
|
||||
@@ -835,7 +861,7 @@ int main(int argc, char* argv[]) {
|
||||
spdlog::debug("running as root; skipping nsenter (the mount is already directly visible)");
|
||||
}
|
||||
return run_container(mode_arg, command, use_nsenter, user_flag, group_flag, hostname_flag, volume_specs,
|
||||
*config);
|
||||
env_specs, *config);
|
||||
}
|
||||
|
||||
auto mounted = mount_image(mode_arg);
|
||||
|
||||
Reference in New Issue
Block a user