From 2c176afa7a405001dc618cbbedd6391fcd6b3421 Mon Sep 17 00:00:00 2001
From: eldritch horrors <pennae@lix.systems>
Date: Sun, 26 Oct 2025 16:48:56 +0100
Subject: [PATCH] libutil: don't allow interactive runProgram2

realistically only runProgram is ever called for interactive reasons,
and even those calls seem to be rather ill-advised in many cases. the
chance of multiple interactive processes interfering with each other,
whether for input or for output, must be very low to make this in any
way reasonable: if e.g. git calls ssh for multiple fetched inputs and
ssh requests passphrases for both we can otherwise not guarantee that
*any* input is routed correctly. misrouted output is merely annoying.

Change-Id: I794e3fdf0a3238cb9292003a89ac267f0de7a939
---
 lix/libfetchers/git.cc   | 17 +++++++++--------
 lix/libutil/processes.cc | 34 +++++++++++++++++++++++-----------
 lix/libutil/processes.hh |  1 -
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/lix/libfetchers/git.cc b/lix/libfetchers/git.cc
index f8de4c37a..fc5f9af84 100644
--- a/lix/libfetchers/git.cc
+++ b/lix/libfetchers/git.cc
@@ -21,6 +21,7 @@
 
 #include "lix/libfetchers/fetch-settings.hh"
 
+#include <optional>
 #include <regex>
 #include <string.h>
 #include <sys/time.h>
@@ -70,15 +71,13 @@ Path getCachePath(std::string_view key)
 //   ...
 static kj::Promise<Result<std::optional<std::string>>> readHead(const Path & path)
 try {
-    auto [status, output] = TRY_AWAIT(runProgram(RunOptions{
-        .program = "git",
+    auto output = TRY_AWAIT(runProgram(
+        "git",
+        true,
         // FIXME: use 'HEAD' to avoid returning all refs
-        .args = {"ls-remote", "--symref", path},
-        .isInteractive = true,
-    }));
-    if (status != 0) {
-        co_return std::nullopt;
-    }
+        {"ls-remote", "--symref", path},
+        true
+    ));
 
     std::string_view line = output;
     line = line.substr(0, line.find("\n"));
@@ -94,6 +93,8 @@ try {
         co_return parseResult->target;
     }
     co_return std::nullopt;
+} catch (ExecError &) {
+    co_return std::nullopt;
 } catch (...) {
     co_return result::current_exception();
 }
diff --git a/lix/libutil/processes.cc b/lix/libutil/processes.cc
index 6a833d3ad..0eea7c3c5 100644
--- a/lix/libutil/processes.cc
+++ b/lix/libutil/processes.cc
@@ -8,6 +8,8 @@
 #include "lix/libutil/strings.hh"
 #include "lix/libutil/serialise.hh"
 #include "lix/libutil/signals.hh"
+#include "manually-drop.hh"
+#include "sync.hh"
 
 #include <cerrno>
 #include <cstdlib>
@@ -19,6 +21,7 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
+#include <utility>
 
 #ifdef __APPLE__
 # include <sys/syscall.h>
@@ -233,8 +236,27 @@ Pid startProcess(std::function<void()> fun, const ProcessOptions & options)
 kj::Promise<Result<std::string>>
 runProgram(Path program, bool searchPath, const Strings args, bool isInteractive)
 try {
+    // allow only one interactive program per unit time so they don't mess with each other.
+    //
+    // see https://git.lix.systems/lix-project/lix/issues/702 for why this is ManuallyDrop.
+    static ManuallyDrop<Sync<int, AsyncMutex>> interactiveMutex{std::in_place, 0};
+
+    std::optional<Sync<int, AsyncMutex>::Lock> interactiveLock;
+    KJ_DEFER({
+        if (interactiveLock) {
+            logger->resume();
+        }
+    });
+
+    if (isInteractive) {
+        interactiveLock = co_await interactiveMutex->lock();
+        logger->pause();
+    }
+
     auto res = TRY_AWAIT(runProgram(RunOptions{
-        .program = program, .searchPath = searchPath, .args = args, .isInteractive = isInteractive
+        .program = program,
+        .searchPath = searchPath,
+        .args = args,
     }));
 
     if (!statusOk(res.first)) {
@@ -324,16 +346,6 @@ RunningProgram runProgram2(const RunOptions & options)
         .dieWithParent = options.dieWithParent,
     };
 
-    std::optional<Finally<std::function<void()>>> resumeLoggerDefer;
-    if (options.isInteractive) {
-        logger->pause();
-        resumeLoggerDefer.emplace(
-            []() {
-                logger->resume();
-            }
-        );
-    }
-
     printMsg(lvlChatty, "running command: %s", concatMapStringsSep(" ", options.args, shellEscape));
 
     /* Fork. */
diff --git a/lix/libutil/processes.hh b/lix/libutil/processes.hh
index f3a193b76..97e9c25b9 100644
--- a/lix/libutil/processes.hh
+++ b/lix/libutil/processes.hh
@@ -96,7 +96,6 @@ struct RunOptions
     std::optional<std::map<std::string, std::string>> environment = {};
     bool dieWithParent = true;
     bool captureStdout = false;
-    bool isInteractive = false;
     std::vector<Redirection> redirections;
 #if __linux__
     std::set<long> caps;