fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fleetd #369: make GitWorktreesTest hermetic against the machine's real git config
CI / contract (pull_request) Successful in 47s
Details
CI / build (pull_request) Failing after 1m28s
Details

Browse Source 
gitOutput set GIT_CONFIG_GLOBAL/SYSTEM/TERMINAL_PROMPT but not XDG_CONFIG_HOME,
and status/fullStatus (plus every other raw git subprocess in this class) set no
isolation at all — inheriting the JVM's real environment, including the
operator's real ~/.gitconfig and default excludes file. Measured: with a
poisoned XDG_CONFIG_HOME, 56 of 59 tests failed.

Centralize every git subprocess this test starts through one factory,
gitProcessBuilder, which always applies the existing hermeticGitEnv isolation
(extended with XDG_CONFIG_HOME, the same fix #366 already applied to the
production-instance seam). Add a self-check test that counts direct
ProcessBuilder("git", ...) constructions in this file's own source and fails
if a future helper bypasses the factory, so the omission that caused this
ticket is caught by name instead of rediscovered on a poisoned machine.
This commit is contained in:
Dai Ha
2026-09-06 19:54:35 +07:00
parent 6c61355f8f
commit dd2efd8541
1 changed files with 105 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java
+105 -35
View File
@@ -54,6 +54,56 @@ class GitWorktreesTest {
/** A non-empty autoenv file — the form that would prompt for authorization in a worktree. */
private static final String AUTOENV_WITH_DIRECTIVE = "export HELLO=world\n";
/**
* fleetd #369. A throwaway directory that lives for the whole class (JUnit 5.4+ supports a
* static {@code @TempDir} field, created once and removed once every test in this class has
* run) — backing every raw {@code git} subprocess's {@code XDG_CONFIG_HOME} below. It only
* ever needs to exist and be guaranteed free of a {@code git/ignore} file; nothing writes
* inside it.
*/
@TempDir
private static Path CLASS_TMP;
/**
* fleetd #369 — the leak measured: {@code XDG_CONFIG_HOME=<dir with a `*` git/ignore> mvn test
* -Dtest=GitWorktreesTest} failed 56 of 59 tests on an unpatched checkout, because {@link
* #gitOutput} set {@code GIT_CONFIG_GLOBAL}/{@code GIT_CONFIG_SYSTEM}/{@code
* GIT_TERMINAL_PROMPT} but not {@code XDG_CONFIG_HOME}, and {@link #status}/{@link
* #fullStatus} (plus every other raw {@code git} subprocess this class started) set NOTHING at
* all — inheriting the JVM's whole real environment, including the operator's real {@code
* ~/.gitconfig} and real default excludes file ({@code $XDG_CONFIG_HOME/git/ignore} or {@code
* $HOME/.config/git/ignore}, applied by git with no {@code core.excludesFile} configured at
* all — see {@code gitignore(5)}). {@code GIT_CONFIG_GLOBAL=/dev/null} does not stop that
* default from applying; only setting {@code XDG_CONFIG_HOME} to a directory that provably
* carries no {@code git/ignore} does.
*
* <p>This is the same isolation {@link #hermeticGitEnv} already gives {@link
* #seedingGitWorktrees}'s production {@link GitWorktrees} instances (fleetd #362 review fix,
* finding 2), reused here for every subprocess the TEST ITSELF starts to drive and inspect
* those fixture repos.
*/
private static Map<String, String> hermeticEnv() {
return hermeticGitEnv(CLASS_TMP);
}
/**
* The one seam every git subprocess in this class is built through — see criterion 4's
* self-check, {@link #everyGitSubprocessGoesThroughTheHermeticFactory}, which fails the moment
* a future helper builds its own {@code git} subprocess directly instead of calling this, so
* the omission that caused fleetd #369 gets caught by name rather than rediscovered by a
* poisoned machine. The one deliberate exception is {@link
* #worktreeCredentialHelperCompletesWithoutUsingAnInheritedHelper}, which needs a
* non-hermetic, test-controlled global config to prove the credential helper ignores it — see
* the comment on that test.
*/
private static ProcessBuilder gitProcessBuilder(Path cwd, String... args) {
List<String> cmd = new java.util.ArrayList<>(List.of("git"));
cmd.addAll(List.of(args));
ProcessBuilder pb = new ProcessBuilder(cmd).directory(cwd.toFile()).redirectErrorStream(true);
pb.environment().putAll(hermeticEnv());
return pb;
}
private static Path initRepo(Path dir) throws Exception {
Files.createDirectories(dir);
git(dir, "init", "-q", "-b", "main");
@@ -71,23 +121,16 @@ class GitWorktreesTest {
}
private static String gitOutput(Path cwd, String... args) throws Exception {
List<String> cmd = new java.util.ArrayList<>(List.of("git"));
cmd.addAll(List.of(args));
ProcessBuilder pb = new ProcessBuilder(cmd).directory(cwd.toFile()).redirectErrorStream(true);
pb.environment().put("GIT_CONFIG_GLOBAL", "/dev/null");
pb.environment().put("GIT_CONFIG_SYSTEM", "/dev/null");
pb.environment().put("GIT_TERMINAL_PROMPT", "0");
Process p = pb.start();
Process p = gitProcessBuilder(cwd, args).start();
String out = new String(p.getInputStream().readAllBytes());
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git timed out: " + String.join(" ", cmd));
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git timed out: git " + String.join(" ", args));
assertEquals(0, p.exitValue(), "git " + String.join(" ", args) + " failed:\n" + out);
return out;
}
/** Pending changes to {@code file} in {@code cwd}, empty when git considers it unmodified. */
private static String status(Path cwd, String file) throws Exception {
Process p = new ProcessBuilder("git", "status", "--porcelain", "--", file)
.directory(cwd.toFile()).redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "status", "--porcelain", "--", file).start();
String out = new String(p.getInputStream().readAllBytes());
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git status timed out");
return out;
@@ -95,16 +138,14 @@ class GitWorktreesTest {
/** Every pending change in {@code cwd} — the whole-tree porcelain status, unlike {@link #status}. */
private static String fullStatus(Path cwd) throws Exception {
Process p = new ProcessBuilder("git", "status", "--porcelain")
.directory(cwd.toFile()).redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "status", "--porcelain").start();
String out = new String(p.getInputStream().readAllBytes());
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git status timed out");
return out;
}
private static String revParse(Path cwd, String ref) throws Exception {
Process p = new ProcessBuilder("git", "-C", cwd.toString(), "rev-parse", ref)
.redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "rev-parse", ref).start();
String out = new String(p.getInputStream().readAllBytes()).trim();
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git rev-parse timed out");
assertEquals(0, p.exitValue(), "git rev-parse " + ref + " failed:\n" + out);
@@ -113,8 +154,7 @@ class GitWorktreesTest {
/** The recursive file list of a commit's tree — used to check what a snapshot actually committed. */
private static String lsTree(Path cwd, String ref) throws Exception {
Process p = new ProcessBuilder("git", "-C", cwd.toString(), "ls-tree", "-r", "--name-only", ref)
.redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "ls-tree", "-r", "--name-only", ref).start();
String out = new String(p.getInputStream().readAllBytes());
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git ls-tree timed out");
assertEquals(0, p.exitValue(), "git ls-tree " + ref + " failed:\n" + out);
@@ -125,8 +165,7 @@ class GitWorktreesTest {
* snapshot's tree changed relative to its parent, the same shape {@code git status --porcelain}
* reports for the worktree it was taken from. */
private static Set<String> diffNameOnly(Path cwd, String from, String to) throws Exception {
Process p = new ProcessBuilder("git", "-C", cwd.toString(), "diff", "--name-only", from, to)
.redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "diff", "--name-only", from, to).start();
String out = new String(p.getInputStream().readAllBytes());
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git diff timed out");
assertEquals(0, p.exitValue(), "git diff " + from + ".." + to + " failed:\n" + out);
@@ -152,8 +191,7 @@ class GitWorktreesTest {
}
private static String forEachRef(Path cwd, String pattern) throws Exception {
Process p = new ProcessBuilder("git", "-C", cwd.toString(), "for-each-ref", pattern)
.redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "for-each-ref", pattern).start();
String out = new String(p.getInputStream().readAllBytes());
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git for-each-ref timed out");
assertEquals(0, p.exitValue(), "git for-each-ref " + pattern + " failed:\n" + out);
@@ -162,8 +200,7 @@ class GitWorktreesTest {
/** Write {@code content} as a blob into the object database; returns its sha. */
private static String blobOf(Path cwd, String content) throws Exception {
Process p = new ProcessBuilder("git", "-C", cwd.toString(), "hash-object", "-w", "--stdin")
.redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "hash-object", "-w", "--stdin").start();
p.getOutputStream().write(content.getBytes(StandardCharsets.UTF_8));
p.getOutputStream().close();
String out = new String(p.getInputStream().readAllBytes()).trim();
@@ -174,8 +211,7 @@ class GitWorktreesTest {
/** Build a single-file tree object from {@code blob}; returns the tree's sha. */
private static String treeOf(Path cwd, String path, String blob) throws Exception {
Process p = new ProcessBuilder("git", "-C", cwd.toString(), "mktree")
.redirectErrorStream(true).start();
Process p = gitProcessBuilder(cwd, "mktree").start();
p.getOutputStream().write(("100644 blob " + blob + "\t" + path + "\n").getBytes(StandardCharsets.UTF_8));
p.getOutputStream().close();
String out = new String(p.getInputStream().readAllBytes()).trim();
@@ -187,10 +223,9 @@ class GitWorktreesTest {
/** {@code git commit-tree} rooted at {@code tree} with a chosen committer date; returns the sha. */
private static String commitTree(Path cwd, String tree, String parent, String committerDate,
String message) throws Exception {
ProcessBuilder pb = new ProcessBuilder("git", "-C", cwd.toString(), "commit-tree",
tree, "-p", parent, "-m", message);
ProcessBuilder pb = gitProcessBuilder(cwd, "commit-tree", tree, "-p", parent, "-m", message);
pb.environment().put("GIT_COMMITTER_DATE", committerDate);
Process p = pb.redirectErrorStream(true).start();
Process p = pb.start();
String out = new String(p.getInputStream().readAllBytes()).trim();
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git commit-tree timed out");
assertEquals(0, p.exitValue(), "git commit-tree failed:\n" + out);
@@ -263,6 +298,11 @@ class GitWorktreesTest {
helper = !f() { printf 'username=%s\\npassword=%s\\n\\n' operator operator-secret; }; f
""");
// fleetd #369: the one deliberate exception to gitProcessBuilder. This test's whole point is
// that git must resolve `globalConfig` (a synthetic "operator's global config", never the
// real machine's) and then IGNORE it — so it cannot use the shared hermetic env, which would
// point GIT_CONFIG_GLOBAL at /dev/null and defeat the very thing under test. It never runs
// `git status`, so it does not need XDG_CONFIG_HOME isolation either.
ProcessBuilder pb = new ProcessBuilder("git", "credential", "fill")
.directory(Path.of(wt).toFile()).redirectErrorStream(true);
pb.environment().put("GIT_CONFIG_GLOBAL", globalConfig.toString());
@@ -329,20 +369,16 @@ class GitWorktreesTest {
Path worktree = Path.of(wt);
assertEquals("https://git.ltms.dev/akb/kb.git",
gitOutput(worktree, "remote", "get-url", "origin").trim());
assertEquals(1, exitCode("git", "-C", wt, "config", "--worktree", "--get-regexp", "^url\\."),
assertEquals(1, gitExitCode(worktree, "config", "--worktree", "--get-regexp", "^url\\."),
"no url.*.insteadOf rewrite should be added for an already-HTTPS origin");
}
/** Test-local exit-code probe, mirroring {@link GitWorktrees#exitCode} for an assertion the
* production class does not expose. */
private static int exitCode(String... command) throws Exception {
ProcessBuilder pb = new ProcessBuilder(command).redirectErrorStream(true);
pb.environment().put("GIT_CONFIG_GLOBAL", "/dev/null");
pb.environment().put("GIT_CONFIG_SYSTEM", "/dev/null");
pb.environment().put("GIT_TERMINAL_PROMPT", "0");
Process p = pb.start();
private static int gitExitCode(Path cwd, String... args) throws Exception {
Process p = gitProcessBuilder(cwd, args).start();
p.getInputStream().readAllBytes();
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "command timed out: " + String.join(" ", command));
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "command timed out: git " + String.join(" ", args));
return p.exitValue();
}
@@ -1739,4 +1775,38 @@ class GitWorktreesTest {
"the XDG default excludesFile pattern ('xdg-fallback-marker') must still apply "
+ "after skill seeding ran — got:\n" + porcelain);
}
/**
* fleetd #369, acceptance criterion 4 — make the fix hard to undo by accident. Every git
* subprocess this class starts is required to go through {@link #gitProcessBuilder}, the one
* place {@link #hermeticEnv} is applied; a helper built directly, the way the original leak in
* {@link #status}/{@link #fullStatus} was, is now a source-level fact this test can catch by
* name instead of a machine-dependent failure someone has to rediscover.
*
* <p>This counts a literal marker in this very file's own source, split into three
* concatenated pieces below so the count is not thrown off by this method's own text — a
* plain, unsplit occurrence of the marker anywhere in this file (a helper's construction, or a
* comment that happens to spell it out contiguously) adds to the count the same way. Today
* there are exactly two: the factory itself, and the one documented exception in {@link
* #worktreeCredentialHelperCompletesWithoutUsingAnInheritedHelper}, which needs a
* non-hermetic, test-controlled global config to prove the credential helper ignores it. A
* third means a new helper was added the old, leak-prone way — route it through {@link
* #gitProcessBuilder} instead, or explain the new exception here and bump this number.
*/
@Test
void everyGitSubprocessGoesThroughTheHermeticFactory() throws Exception {
Path source = Path.of("src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java");
String text = Files.readString(source);
String marker = "new " + "ProcessBuilder" + "(";
int count = 0;
for (int from = text.indexOf(marker); from >= 0; from = text.indexOf(marker, from + marker.length())) {
count++;
}
assertEquals(2, count,
"expected exactly 2 direct git-subprocess constructions in this file (the "
+ "gitProcessBuilder factory itself, plus the one documented exception in "
+ "worktreeCredentialHelperCompletesWithoutUsingAnInheritedHelper) — a "
+ "different count means a helper now bypasses the hermetic factory; route "
+ "it through gitProcessBuilder or document the new exception here");
}
}