From a37acd5ee3c20d995b550db14dbc96730cee9f8e Mon Sep 17 00:00:00 2001
From: Dai Ha <dai.ha@magnolia-cms.com>
Date: Sun, 6 Sep 2026 20:09:32 +0700
Subject: [PATCH] fleetd #369 review round 2: pin the factory's behaviour, not
 its call count
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

everyGitSubprocessGoesThroughTheHermeticFactory counts ProcessBuilder("git",
...) call sites, so it catches a new helper built the old way, but a
reviewer proved it does not catch gitProcessBuilder itself being gutted:
removing pb.environment().putAll(hermeticEnv()) from inside the factory
leaves every call site unchanged, the count stays 2, and the whole
unpoisoned suite stays green.

Add gitProcessBuilderCarriesTheFullHermeticEnvironment, which inspects what
the factory actually hands to ProcessBuilder#start(): every hermetic key
present with the isolating value, and XDG_CONFIG_HOME pointed inside the
class's own throwaway directory rather than left unset or pointing at the
operator's real one. This fails the moment the hermetic environment stops
being applied, on any machine, with no poison needed. Keep the call-site
count check too — the two catch different regressions.
---
 .../ltms/fleet/session/GitWorktreesTest.java  | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java
index 09e266e..971eeb6 100644
--- a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java
+++ b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java
@@ -1809,4 +1809,43 @@ class GitWorktreesTest {
                         + "different count means a helper now bypasses the hermetic factory; route "
                         + "it through gitProcessBuilder or document the new exception here");
     }
+
+    /**
+     * fleetd #369 review round 2. {@link #everyGitSubprocessGoesThroughTheHermeticFactory} counts
+     * call sites, not behaviour — it catches a NEW helper built the old, leak-prone way, but it
+     * cannot catch {@link #gitProcessBuilder} itself being gutted: deleting {@code
+     * pb.environment().putAll(hermeticEnv())} from inside the factory leaves every call site
+     * unchanged, the count stays 2, and the whole unpoisoned suite stays green — the exact leak
+     * this ticket fixed would come back silently, with nothing but a human remembering to re-run
+     * the poison command to catch it. This test instead inspects what the factory actually hands
+     * to {@link ProcessBuilder#start()}, so it fails the moment the hermetic environment stops
+     * being applied, on any machine, with no poison needed.
+     *
+     * <p>The property under test: every git subprocess this class starts must run with an
+     * environment that cannot see the operator's real git configuration. A call-site count is a
+     * proxy for that; this is the thing itself.
+     */
+    @Test
+    void gitProcessBuilderCarriesTheFullHermeticEnvironment(@TempDir Path tmp) {
+        Map<String, String> env = gitProcessBuilder(tmp, "status", "--porcelain").environment();
+
+        assertEquals("/dev/null", env.get("GIT_CONFIG_GLOBAL"),
+                "GIT_CONFIG_GLOBAL must be neutralized, or the operator's real ~/.gitconfig applies");
+        assertEquals("/dev/null", env.get("GIT_CONFIG_SYSTEM"),
+                "GIT_CONFIG_SYSTEM must be neutralized, or the machine's real /etc/gitconfig applies");
+        assertEquals("0", env.get("GIT_TERMINAL_PROMPT"),
+                "GIT_TERMINAL_PROMPT must be disabled, or a credential prompt can hang the subprocess");
+
+        String xdg = env.get("XDG_CONFIG_HOME");
+        assertNotNull(xdg,
+                "XDG_CONFIG_HOME must be set — left unset, git falls back to the operator's real "
+                        + "$HOME/.config/git/ignore (gitignore(5)), exactly fleetd #369's leak");
+        assertFalse(xdg.isBlank(), "XDG_CONFIG_HOME must not be blank — blank behaves like unset");
+        assertTrue(Path.of(xdg).startsWith(CLASS_TMP),
+                "XDG_CONFIG_HOME must point inside this test class's own throwaway directory, "
+                        + "never the operator's real one or the JVM's inherited value — got: " + xdg);
+        assertFalse(Files.exists(Path.of(xdg, "git", "ignore")),
+                "the resolved XDG default excludes file must provably not exist, or its contents "
+                        + "would silently apply to every git status this test class runs");
+    }
 }