From 3b2def52d026efa4d7f7e1842e320d644002f015 Mon Sep 17 00:00:00 2001 From: Sirac Ozmen Date: Thu, 5 Nov 2026 16:48:51 +0300 Subject: [PATCH] fix(studio): start git without the variables that name a repository Git takes the repository GIT_DIR and its other repository variables name over -C, and it sets GIT_DIR for a hook run from a linked worktree. Studio's Changes tab and its test passed them on, so a push from a worktree ran the test against the engine's own repository. A child process can now be started without named variables, and Studio starts git without the ones `git rev-parse --local-env-vars` lists. Fixes #224. --- .../process/include/rawframe/process/child.h | 3 ++ modules/process/src/child.cpp | 38 +++++++++++++--- modules/process/tests/child.cpp | 5 +++ modules/process/tests/child_test.cpp | 45 +++++++++++++++++-- modules/studio/src/version_control.cpp | 24 +++++++++- modules/studio/src/version_control.h | 9 +++- modules/studio/tests/version_control_test.cpp | 24 +++++++++- 7 files changed, 134 insertions(+), 14 deletions(-) diff --git a/modules/process/include/rawframe/process/child.h b/modules/process/include/rawframe/process/child.h index f93b5dcf0..e54cd28a1 100644 --- a/modules/process/include/rawframe/process/child.h +++ b/modules/process/include/rawframe/process/child.h @@ -34,6 +34,9 @@ struct ChildSettings { /// (Linux a signal, macOS a process source, Windows a wait on the /// parent), which it takes as any stop, in order. bool stopsWithThis = false; + /// Environment variables of this process it is not given, by name; on + /// Windows, whose names ignore case, in any case. + std::vector withheld; }; /// The environment variable a child that stops with its parent is told diff --git a/modules/process/src/child.cpp b/modules/process/src/child.cpp index 1be2b1e1a..b3eb79db5 100644 --- a/modules/process/src/child.cpp +++ b/modules/process/src/child.cpp @@ -2,6 +2,7 @@ #include "rawframe/process/errors.h" +#include #include #include #include @@ -65,17 +66,36 @@ void appendQuoted(std::wstring& line, const std::wstring& argument) { line += L'"'; } -/// This process's environment as a block CreateProcessW takes, with its -/// identity for a child that stops with it and the child's own stop -/// event in place of any it was given itself. +/// Whether `entry`, a NAME=value of the environment, names one of `names`; +/// Windows's names ignore case. +bool withheld(std::wstring_view entry, const std::vector& names) { + // A name may begin with '=' on Windows (the shell's per-drive + // directories), so its own '=' is the first after its first character. + const std::size_t kEnd = entry.find(L'=', 1); + const std::wstring_view kName = entry.substr(0, kEnd); + return std::ranges::any_of(names, [kName](const std::wstring& name) { + return ::CompareStringOrdinal( + kName.data(), static_cast(kName.size()), name.data(), static_cast(name.size()), TRUE) == + CSTR_EQUAL; + }); +} + +/// This process's environment as a block CreateProcessW takes, without the +/// variables the child is not given, with its identity for a child that +/// stops with it and the child's own stop event in place of any it was +/// given itself. std::wstring environmentFor(const ChildSettings& settings, HANDLE stop) { const std::wstring kParent = wide(std::string{kParentVariable} + "="); const std::wstring kStop = wide(std::string{kStopVariable} + "="); + std::vector names; + for (const std::string& each : settings.withheld) { + names.push_back(wide(each)); + } std::wstring block; if (wchar_t* strings = ::GetEnvironmentStringsW(); strings != nullptr) { for (const wchar_t* each = strings; *each != L'\0'; each += std::wstring_view{each}.size() + 1) { const std::wstring_view kEntry{each}; - if (!kEntry.starts_with(kParent) && !kEntry.starts_with(kStop)) { + if (!kEntry.starts_with(kParent) && !kEntry.starts_with(kStop) && !withheld(kEntry, names)) { block.append(kEntry); block += L'\0'; } @@ -294,12 +314,16 @@ result::Result Child::start(const ChildSettings& settings) { if (failed == 0) { failed = posix_spawnattr_setflags(&attributes, POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK); } - // This process's environment, with its identity for a child that stops - // with it in place of any it was given itself. + // This process's environment without the variables the child is not + // given, with its identity for a child that stops with it in place of + // any it was given itself. std::vector told; const std::string kParentPrefix = std::string{kParentVariable} + "="; for (char** each = environ; *each != nullptr; ++each) { - if (std::string_view{*each}.substr(0, kParentPrefix.size()) != kParentPrefix) { + const std::string_view kEntry{*each}; + const std::string_view kName = kEntry.substr(0, kEntry.find('=')); + if (kEntry.substr(0, kParentPrefix.size()) != kParentPrefix && + std::ranges::find(settings.withheld, kName) == settings.withheld.end()) { told.emplace_back(*each); } } diff --git a/modules/process/tests/child.cpp b/modules/process/tests/child.cpp index 5ddd3e8bf..0bc82be5d 100644 --- a/modules/process/tests/child.cpp +++ b/modules/process/tests/child.cpp @@ -68,6 +68,11 @@ int main(int argc, char** argv) { std::fputs(parent != nullptr ? parent : "none", stdout); return 0; } + if (kMode == "variable" && argc > 2) { + const char* value = std::getenv(argv[2]); + std::fputs(value != nullptr ? value : "none", stdout); + return 0; + } if (kMode == "wait") { #if defined(_WIN32) ::SetConsoleCtrlHandler(&askStopOnEvent, TRUE); diff --git a/modules/process/tests/child_test.cpp b/modules/process/tests/child_test.cpp index d6fccd580..91b550405 100644 --- a/modules/process/tests/child_test.cpp +++ b/modules/process/tests/child_test.cpp @@ -1,7 +1,7 @@ // Child processes: a child's exit code, its output and error in one -// file, its arguments passed as they are (spaces, quotes, and UTF-8 too), a stop -// request it answers as a Host does, a kill, a kill when its owner goes, -// and a program that cannot be started. +// file, its arguments passed as they are (spaces, quotes, and UTF-8 too), the +// variables withheld from it, a stop request it answers as a Host does, a +// kill, a kill when its owner goes, and a program that cannot be started. #include "rawframe/process/child.h" #include "rawframe/process/errors.h" @@ -17,6 +17,7 @@ #include #include #include +#include #if defined(_WIN32) #include @@ -134,6 +135,44 @@ RAWFRAME_TEST(AChildThatStopsWithThisIsToldWhoThisIs) { } } +RAWFRAME_TEST(AChildIsNotGivenTheVariablesWithheldFromIt) { + // Withheld by name, in any case on Windows, whose names ignore it; the + // others are given as they are. +#if defined(_WIN32) + const auto kSet = [](const char* name, const char* value) { + ::_putenv_s(name, value); + }; + const std::vector kWithheld = {"rawframe_withheld"}; +#else + const auto kSet = [](const char* name, const char* value) { + if (value[0] == '\0') { + ::unsetenv(name); + } else { + ::setenv(name, value, 1); + } + }; + const std::vector kWithheld = {"RAWFRAME_WITHHELD"}; +#endif + kSet("RAWFRAME_WITHHELD", "held"); + kSet("RAWFRAME_GIVEN", "given"); + const auto kRead = [&kWithheld](const char* name, const char* file) { + const std::filesystem::path kRead = scratch(file); + auto read = Child::start({.program = RAWFRAME_PROCESS_CHILD, + .arguments = {"variable", name}, + .output = kRead, + .withheld = kWithheld}); + RAWFRAME_EXPECT(startedAll(read)); + if (read.has_value()) { + RAWFRAME_EXPECT(waitFor(*read) == 0); + } + return test::readFile(kRead.string()); + }; + RAWFRAME_EXPECT(kRead("RAWFRAME_WITHHELD", "withheld.txt") == "none"); + RAWFRAME_EXPECT(kRead("RAWFRAME_GIVEN", "given.txt") == "given"); + kSet("RAWFRAME_WITHHELD", ""); + kSet("RAWFRAME_GIVEN", ""); +} + RAWFRAME_TEST(AChildStopsWhenAskedAndIsKilledOtherwise) { const std::filesystem::path kHeard = scratch("heard.txt"); auto waiting = Child::start({.program = RAWFRAME_PROCESS_CHILD, .arguments = {"wait"}, .output = kHeard}); diff --git a/modules/studio/src/version_control.cpp b/modules/studio/src/version_control.cpp index 849285259..abd6b5232 100644 --- a/modules/studio/src/version_control.cpp +++ b/modules/studio/src/version_control.cpp @@ -8,6 +8,24 @@ namespace rawframe::studio { +std::vector gitRepositoryVariables() { + return {"GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_CONFIG", + "GIT_CONFIG_PARAMETERS", + "GIT_CONFIG_COUNT", + "GIT_OBJECT_DIRECTORY", + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_IMPLICIT_WORK_TREE", + "GIT_GRAFT_FILE", + "GIT_INDEX_FILE", + "GIT_NO_REPLACE_OBJECTS", + "GIT_REPLACE_REF_BASE", + "GIT_PREFIX", + "GIT_SHALLOW_FILE", + "GIT_COMMON_DIR"}; +} + void VersionControl::ask(std::string purpose, std::vector arguments) { if (!trusted_) { return; @@ -39,8 +57,10 @@ std::optional VersionControl::start(std::string_view pa } auto [purpose, arguments] = std::move(waiting_.front()); waiting_.pop_front(); - auto started = - process::Child::start({.program = *kProgram, .arguments = words(directory, arguments), .output = output}); + auto started = process::Child::start({.program = *kProgram, + .arguments = words(directory, arguments), + .output = output, + .withheld = gitRepositoryVariables()}); if (!started.has_value()) { waiting_.clear(); return Refusal{.kind = Refusal::Kind::Failed, diff --git a/modules/studio/src/version_control.h b/modules/studio/src/version_control.h index b6df30f7b..2d80de888 100644 --- a/modules/studio/src/version_control.h +++ b/modules/studio/src/version_control.h @@ -6,7 +6,9 @@ // hooks), so git runs only in a repository the author has said they trust, // as an editor asks before it trusts a workspace. Even there, the programs // a repository names that reading never needs stay off: its file monitor, -// its external diff, and its text conversions. Nothing here waits. +// its external diff, and its text conversions. Git is not given the +// variables that name a repository, so the game's directory is the one it +// works in, whatever started Studio set. Nothing here waits. #include "rawframe/process/child.h" @@ -21,6 +23,11 @@ namespace rawframe::studio { +/// The environment variables that tell git which repository, index, objects +/// and settings to use, as `git rev-parse --local-env-vars` lists them. Git +/// sets them for a hook it runs; any of them outranks `-C`. +[[nodiscard]] std::vector gitRepositoryVariables(); + class VersionControl { public: /// Why what waited did not start; what waited is dropped. diff --git a/modules/studio/tests/version_control_test.cpp b/modules/studio/tests/version_control_test.cpp index 70c6c57d5..49a1aa071 100644 --- a/modules/studio/tests/version_control_test.cpp +++ b/modules/studio/tests/version_control_test.cpp @@ -16,6 +16,7 @@ #endif using rawframe::process::Child; +using rawframe::studio::gitRepositoryVariables; using rawframe::studio::mintedIdentity; using rawframe::studio::programOnPath; using rawframe::studio::VersionControl; @@ -31,6 +32,19 @@ std::string pathVariable() { return kPath != nullptr ? kPath : ""; } +/// Sets this process's variable `name`, or removes it for an empty value. +void setVariable(const char* name, const std::string& value) { +#if defined(_WIN32) + ::_putenv_s(name, value.c_str()); +#else + if (value.empty()) { + ::unsetenv(name); + } else { + ::setenv(name, value.c_str(), 1); + } +#endif +} + /// Git run to its end in `directory`, for the test's own setup. bool git(const std::filesystem::path& directory, std::vector arguments) { const auto kProgram = programOnPath("git", pathVariable()); @@ -38,7 +52,7 @@ bool git(const std::filesystem::path& directory, std::vector argume return false; } arguments.insert(arguments.begin(), {"-C", directory.string()}); - auto child = Child::start({.program = *kProgram, .arguments = arguments}); + auto child = Child::start({.program = *kProgram, .arguments = arguments, .withheld = gitRepositoryVariables()}); for (int tick = 0; child.has_value() && tick < 1000; ++tick) { if (const auto kCode = child->exited()) { return *kCode == 0; @@ -113,6 +127,11 @@ RAWFRAME_TEST(GitRunsOnlyInATrustedRepositoryAndNeverItsReadersPrograms) { const std::filesystem::path kOutput = kRoot / "git.out"; std::filesystem::create_directories(kGame); std::filesystem::create_directories(kMarks); + // Started as a hook is, told another repository: git, given that, + // would make and use it instead of the game's. + const std::filesystem::path kElsewhere = kRoot / "elsewhere"; + setVariable("GIT_DIR", (kElsewhere / ".git").string()); + setVariable("GIT_WORK_TREE", kElsewhere.string()); RAWFRAME_EXPECT(git(kGame, {"init", "--quiet"})); RAWFRAME_EXPECT(git(kGame, {"config", "user.name", "Author"})); RAWFRAME_EXPECT(git(kGame, {"config", "user.email", "author@example.com"})); @@ -164,6 +183,9 @@ RAWFRAME_TEST(GitRunsOnlyInATrustedRepositoryAndNeverItsReadersPrograms) { RAWFRAME_EXPECT(runAll(control, kGame, kOutput) == 2); RAWFRAME_EXPECT(std::filesystem::exists(kMarks / "hook")); RAWFRAME_EXPECT(!std::filesystem::exists(kMarks / "monitor")); + RAWFRAME_EXPECT(!std::filesystem::exists(kElsewhere)); + setVariable("GIT_DIR", ""); + setVariable("GIT_WORK_TREE", ""); std::filesystem::remove_all(kRoot, error); } #endif