From 261aa056f9f0be9d5dae14555f11c82d28f4e70a Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 05:41:40 +0700 Subject: [PATCH] fleetd #480 follow-up correction 2: guarantee resolveHandoverPath is always absolute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LeadRollover.resolveHandoverPath's relative branch resolved the configured handoverPath against leadWorkspace.apply(...) (fleet.leaders..cwd) but never forced the result absolute. If an operator writes a RELATIVE cwd, the returned path stays relative, silently breaking the "always absolute" contract documented on PendingRollover. Fix: call toAbsolutePath() unconditionally on both branches (the already-absolute input branch, where it is a no-op, and the relative branch), so neither branch trusts isAbsolute() alone to already imply what toAbsolutePath() enforces. Method javadoc now states the absolute result is guaranteed, not merely usual. Added a test: a lead with a RELATIVE cwd and a relative handoverPath still yields an absolute PendingRollover.handoverPath. Asserts both isAbsolute() and the exact resolved value, since isAbsolute() alone would also pass for a path resolved against the wrong base. Proved the test discriminates: reverting the toAbsolutePath() calls (keeping the test) made it fail with an AssertionFailedError ("expected: but was: "); restoring the fix made it pass again. Note: FleetConfig has no validation on fleet.leaders..cwd at config load (grep across every validate* method: 0 matches for .cwd()) — a relative cwd is silently accepted. Not adding validation here per instruction; that is a separate ticket. --- .../dev/ltms/fleet/lead/LeadRollover.java | 24 +++++++++++++-- .../dev/ltms/fleet/lead/LeadRolloverTest.java | 30 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java index f43c3c8..29cbb0c 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -270,24 +270,42 @@ public final class LeadRollover { * FleetConfig.LeadRollover#bootstrapTextFor}) ever has to resolve — or worse, silently * mis-resolve — a relative path again. * + *

The return value is GUARANTEED absolute, not merely usually absolute. + * {@code leadWorkspace.apply(leadTerminal)} returns an operator-configured {@code + * fleet.leaders..cwd} string, and nothing forces an operator to write an absolute one — + * a relative {@code cwd} resolved with plain {@link Path#resolve} would still yield a relative + * result, silently reopening the exact bug this class exists to fix (every later reader back to + * interpreting an ambiguous string against ITS OWN working directory). {@link + * Path#toAbsolutePath()} closes that: it resolves any remaining relative path against {@code + * user.dir} (the JVM's own cwd), which is the correct base for an operator-written path the + * daemon process itself is meant to interpret, exactly like the {@code user.dir} fallback used + * below. Applying it unconditionally on both branches means the ALREADY-absolute branch stays a + * no-op (an absolute path is unaffected by {@code toAbsolutePath()}) while the relative-{@code + * cwd} branch above is closed the same way. + * *

    *
  • already absolute → returned unchanged (normalized)
    • *
  • relative → resolved against {@code leadWorkspace.apply(leadTerminal)} when that is * non-null and non-blank; otherwise against {@code System.getProperty("user.dir")} — the * same fallback {@code LeadLauncher#launch} uses for a lead with no configured {@code - * cwd}
    • + * cwd}. If {@code leadWorkspace}'s own answer is itself relative (an operator wrote a + * relative {@code cwd:}), the result is finished off against the daemon's own + * {@code user.dir} — see the paragraph above. *
*/ private String resolveHandoverPath(String configured, String leadTerminal) { Path path = Path.of(configured); if (path.isAbsolute()) { - return path.normalize().toString(); + // toAbsolutePath() is a no-op for an already-absolute path — kept here anyway so both + // branches call the exact same guarantee, rather than one branch relying on + // isAbsolute() alone to already imply what toAbsolutePath() enforces. + return path.toAbsolutePath().normalize().toString(); } String workspace = leadWorkspace.apply(leadTerminal); Path base = (workspace == null || workspace.isBlank()) ? Path.of(System.getProperty("user.dir")) : Path.of(workspace); - return base.resolve(path).normalize().toString(); + return base.resolve(path).toAbsolutePath().normalize().toString(); } /** diff --git a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java index d2f56ff..8d69d92 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -575,6 +575,36 @@ class LeadRolloverTest { "an absolute handoverPath must be used unchanged (aside from normalization)"); } + @Test + @DisplayName("[fleetd #480 follow-up correction] a RELATIVE fleet.leaders..cwd still " + + "yields an ABSOLUTE PendingRollover.handoverPath") + void relativeLeadWorkspaceCwdStillYieldsAnAbsoluteHandoverPath() { + FakeHerdr herdr = new FakeHerdr(); + AtomicLong clock = new AtomicLong(1_000); + FleetConfig.LeadRollover config = cfg("handover.md"); // relative handoverPath + // Nothing in FleetConfig validates fleet.leaders..cwd, so an operator can write a + // RELATIVE one — this must still resolve to an absolute PendingRollover.handoverPath, + // never silently reopen the exact bug this class fixes. + String relativeWorkspace = "relative-lead-workspace"; + Function leadWorkspace = terminal -> LEAD.equals(terminal) ? relativeWorkspace : null; + LeadRollover rollover = newRollover(herdr, config, fixedClock(clock), leadWorkspace); + + LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full"); + + assertTrue(Path.of(pending.handoverPath()).isAbsolute(), + "the resolved path must be absolute even when the configured cwd itself is relative"); + + // Asserting isAbsolute() alone would also pass for a path resolved against the WRONG base + // (e.g. some unrelated absolute directory) — pin the actual value too. + String expected = Path.of(System.getProperty("user.dir")).resolve(relativeWorkspace) + .resolve("handover.md").toAbsolutePath().normalize().toString(); + assertEquals(expected, pending.handoverPath(), + "a relative cwd must be finished off against the daemon's own user.dir, exactly " + + "like a missing cwd — never left relative, which would silently reopen the " + + "exact bug this class fixes: every later reader interpreting an ambiguous " + + "path against ITS OWN working directory"); + } + @Test @DisplayName("[fleetd #480 follow-up] a terminal with no configured workspace (null lookup " + "result) falls back to the daemon's own user.dir")