Skip to content

Commit 645558a

Browse files
committed
refactor(process): hybrid launcher — Linux posix_spawn, macOS/Windows shell
Per review: use a direct exec (posix_spawn) on Linux, where the leak actually lives and where it can be iterated locally, and keep the proven std::system shell path on macOS/Windows (which inject no runtime env / have no glibc symbol versioning). This gives Linux the clean child-only env isolation with no shell quoting/signal/injection surface, while not risking the platforms that can't be reproduced here. A TODO(launcher-unify) marks unifying onto spawn later if macOS/Windows ever need the same isolation. Linux verified: unit 19/19, e2e 74, hostile-LD_LIBRARY_PATH run, fast-path, passthrough args.
1 parent c3ee977 commit 645558a

1 file changed

Lines changed: 102 additions & 9 deletions

File tree

src/platform/process.cppm

Lines changed: 102 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ module;
2525
#include <stdlib.h> // _putenv_s
2626
#define popen _popen
2727
#define pclose _pclose
28+
#elif defined(__linux__)
29+
// Linux is the only platform where the launcher does a direct exec (see
30+
// run_exec / capture_exec below); macOS keeps the std::system shell path.
31+
#include <unistd.h> // pipe, dup2, close, read, environ
32+
#include <sys/wait.h> // waitpid
33+
#include <spawn.h> // posix_spawnp, posix_spawn_file_actions_*
34+
extern "C" char **environ;
2835
#endif
2936

3037
export module mcpp.platform.process;
@@ -125,10 +132,30 @@ int normalize_exit_code(int rc) {
125132
#endif
126133
}
127134

135+
#if defined(__linux__)
136+
// Build a child environment block = the current environ with `extra` overrides
137+
// applied. Returned vector owns the strings; the caller derives a NUL-terminated
138+
// char* array from it. Built in the PARENT so the child env never requires a
139+
// post-fork setenv and mcpp's own environment is never touched.
140+
std::vector<std::string> merged_environ(
141+
const std::vector<std::pair<std::string, std::string>>& extra)
142+
{
143+
std::vector<std::string> out;
144+
std::set<std::string> overridden;
145+
for (auto& [k, v] : extra) { out.push_back(k + "=" + v); overridden.insert(k); }
146+
for (char** e = environ; e && *e; ++e) {
147+
std::string_view entry(*e);
148+
auto eq = entry.find('=');
149+
std::string key(eq == std::string_view::npos ? entry : entry.substr(0, eq));
150+
if (!overridden.contains(key)) out.emplace_back(entry);
151+
}
152+
return out;
153+
}
154+
#else
128155
// Build a shell command line from an argv vector. The first token (program)
129156
// is kept RAW on Windows — quoting it would make cmd.exe's `/c "..."` strip the
130157
// outer quotes and mangle the path (see platform.shell) — and shell-quoted on
131-
// POSIX. Remaining args are always shell-quoted.
158+
// macOS. Remaining args are always shell-quoted.
132159
std::string command_from_argv(const std::vector<std::string>& argv) {
133160
if (argv.empty()) return "";
134161
#if defined(_WIN32)
@@ -142,6 +169,7 @@ std::string command_from_argv(const std::vector<std::string>& argv) {
142169
}
143170
return cmd;
144171
}
172+
#endif
145173

146174
} // namespace
147175

@@ -251,18 +279,48 @@ int run_passthrough(std::string_view command, std::string* output) {
251279
return normalize_exit_code(::pclose(fp));
252280
}
253281

282+
// run_exec / capture_exec are split by platform on purpose:
283+
//
284+
// Linux — DIRECT exec via posix_spawn. The runtime env (a bundled-glibc
285+
// LD_LIBRARY_PATH) goes into the child's envp ONLY; it never enters
286+
// mcpp's own environment nor the host /bin/sh. That is the exact
287+
// fix for the newer-glibc `sh:` crash, and a direct exec also drops
288+
// the shell quoting / signal / injection surface entirely.
289+
// macOS /
290+
// Windows — KEEP the proven std::system shell path. The leak does not exist
291+
// here (macOS injects no runtime library env; Windows has no glibc
292+
// symbol versioning), so we deliberately do not swap the launch
293+
// primitive on platforms we cannot iterate on locally. The env is
294+
// applied as a `KEY='val' cmd` prefix (macOS) / _putenv_s (Windows)
295+
// via the existing build_env_prefix / capture_with_env helpers.
296+
//
297+
// TODO(launcher-unify): if macOS/Windows ever need the same child-only env
298+
// isolation (e.g. they start bundling a runtime), unify both onto posix_spawn
299+
// and a Windows CreateProcess/_spawn equivalent, and delete the shell branch.
254300
int run_exec(const std::vector<std::string>& argv,
255301
const std::vector<std::pair<std::string, std::string>>& extraEnv)
256302
{
257303
if (argv.empty()) return 127;
258-
// Shell launch via std::system, with the runtime env applied as a COMMAND
259-
// PREFIX on POSIX (`KEY='val' cmd`). The shell std::system spawns starts
260-
// with a clean environment — a bundled-glibc LD_LIBRARY_PATH is set only
261-
// for the target child, never for /bin/sh itself (the newer-glibc `sh:`
262-
// crash class). On Windows build_env_prefix does _putenv_s and returns "".
304+
#if defined(__linux__)
305+
auto envStore = merged_environ(extraEnv);
306+
std::vector<char*> envp;
307+
for (auto& s : envStore) envp.push_back(s.data());
308+
envp.push_back(nullptr);
309+
std::vector<char*> cargv;
310+
for (auto& a : argv) cargv.push_back(const_cast<char*>(a.c_str()));
311+
cargv.push_back(nullptr);
312+
313+
pid_t pid = 0;
314+
if (::posix_spawnp(&pid, cargv[0], nullptr, nullptr, cargv.data(), envp.data()) != 0)
315+
return 127; // spawn failed (e.g. program not found)
316+
int status = 0;
317+
while (::waitpid(pid, &status, 0) < 0) { /* EINTR retry */ }
318+
return normalize_exit_code(status);
319+
#else
263320
std::string prefix = mcpp::platform::env::build_env_prefix(extraEnv);
264321
std::string cmd = prefix + command_from_argv(argv);
265322
return normalize_exit_code(std::system(cmd.c_str()));
323+
#endif
266324
}
267325

268326
RunResult capture_exec(
@@ -271,11 +329,46 @@ RunResult capture_exec(
271329
{
272330
RunResult result;
273331
if (argv.empty()) { result.exit_code = 127; return result; }
274-
// Capture stdout+stderr combined (the trailing `2>&1` replaces the old
275-
// redirect). capture_with_env applies the env as a POSIX prefix / Windows
276-
// _putenv_s, so the shell is never pre-poisoned with the loader path.
332+
#if defined(__linux__)
333+
// posix_spawn + a pipe; stdout and stderr both go to the pipe so the
334+
// captured text is combined (replaces the old `2>&1`).
335+
int fds[2];
336+
if (::pipe(fds) != 0) { result.exit_code = 127; return result; }
337+
338+
auto envStore = merged_environ(extraEnv);
339+
std::vector<char*> envp;
340+
for (auto& s : envStore) envp.push_back(s.data());
341+
envp.push_back(nullptr);
342+
std::vector<char*> cargv;
343+
for (auto& a : argv) cargv.push_back(const_cast<char*>(a.c_str()));
344+
cargv.push_back(nullptr);
345+
346+
posix_spawn_file_actions_t fa;
347+
::posix_spawn_file_actions_init(&fa);
348+
::posix_spawn_file_actions_adddup2(&fa, fds[1], 1); // stdout → pipe
349+
::posix_spawn_file_actions_adddup2(&fa, fds[1], 2); // stderr → same pipe
350+
::posix_spawn_file_actions_addclose(&fa, fds[0]);
351+
::posix_spawn_file_actions_addclose(&fa, fds[1]);
352+
353+
pid_t pid = 0;
354+
int sp = ::posix_spawnp(&pid, cargv[0], &fa, nullptr, cargv.data(), envp.data());
355+
::posix_spawn_file_actions_destroy(&fa);
356+
::close(fds[1]);
357+
if (sp != 0) { ::close(fds[0]); result.exit_code = 127; return result; }
358+
359+
std::array<char, 4096> buf{};
360+
ssize_t n;
361+
while ((n = ::read(fds[0], buf.data(), buf.size())) > 0)
362+
result.output.append(buf.data(), static_cast<size_t>(n));
363+
::close(fds[0]);
364+
int status = 0;
365+
while (::waitpid(pid, &status, 0) < 0) { /* EINTR retry */ }
366+
result.exit_code = normalize_exit_code(status);
367+
return result;
368+
#else
277369
std::string cmd = command_from_argv(argv) + " 2>&1";
278370
return capture_with_env(cmd, extraEnv);
371+
#endif
279372
}
280373

281374
} // namespace mcpp::platform::process

0 commit comments

Comments
 (0)