fleetd #224 / #225: share worktreeRoot with the group + fix a CWD-dependent group test helper #230

Merged
ltms merged 1 commits from worker/cb224-worktree-root-group-024523-2 into main 2026-09-02 02:52:12 +02:00
Member

Two related tickets in one PR: fleetd #224 (worktreeRoot is never made group-traversable) and
fleetd #225 (two tests pass or fail depending on where the repo is checked out).

#224 — worktreeRoot is never made group-traversable

GitWorktrees#shareWithGroup shares each worktree path and the repo's common git dir with
worktreeGroup, but never the parent directory (worktreeRoot) that contains every
worktree. Under memberHerdrSocket: the member pane runs as a different OS user, and a
directory needs the execute bit for that user to be traversed at all — so if worktreeRoot
lands at (say) 0700, everything under it is unreachable no matter how carefully each child is
shared: the member can't read the ephemeral opencode.json #219 places there, can't reach its
own worktree, and can't read #213's ZDOTDIR scrub when placed there either.

Fix (fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java):

  • add() now calls a new private shareRootWithGroup(root) right after Files.createDirectories(root)
    — established once, where the root is created, never at a use site (not in OpenCodeLauncher,
    which would duplicate the policy in the wrong layer).
  • It chgrps and chmod g+xs the root itself, non-recursively — each child underneath
    (a worktree, or a launcher-generated scrub/config dir) is still shared individually by its own
    existing call site.
  • No-op — zero processes spawned — when worktreeGroup is unset, so behaviour with today's only
    live mode is byte-identical (criterion 4, proven by a dedicated test asserting zero recorded
    commands).
  • On failure (group doesn't exist, or the operator isn't a member of it) the spawn is refused
    with a WorktreeException naming the root, its current POSIX mode, and the group — mirroring
    shareWithGroup's existing refusal shape — and this happens before git worktree add ever
    runs, so no partial worktree is left behind.

Tests added (GitWorktreesTest), each watched to FAIL against the pre-fix code first (I
stashed the production change and confirmed both failed with the exact assertion messages below,
then restored the fix and confirmed both pass):

  • addSharesWorktreeRootWithGroupWhenConfigured — recording-runner test proving add() chgrp's
    and chmod g+x's the root itself.
  • addSharesNothingForTheRootWhenNoGroupConfigured — criterion 4, zero processes spawned when no
    group is configured.
  • addRefusesWhenWorktreeRootCannotBeMadeGroupTraversable — drives the REAL chgrp (no fake
    runner) with a nonexistent group name, the same real-failure shape
    shareWithGroupThrowsNamingTheGroupWhenChgrpFails already uses for the per-worktree share, and
    asserts the exception names the root, its current mode, and the group, and that no worktree
    directory is left behind.

Also added (criterion 5, the gap the #221 reviewer flagged): EnvAllowListScrubTest gained
shareWithGroupCoversEveryFlatFileIncludingTheOptionalOnes, which drives
EnvAllowListScrub#shareWithGroup directly against a directory holding opencode.json,
member-charter.md, ide-rules.md, and an unrelated file, asserting every one of them ends up
group-readable/never-group-writable — not just the two files someone happened to think of.

#225 — two tests depend on where the repo is checked out

OpenCodeLauncherTest#currentUserGroup and its twin in HerdrPeerLauncherAllowListWiringTest
read the group that owns the current working directory, not the process's own primary
group, despite the comment claiming the latter. Those coincide only by accident: a home checkout
is typically owned by a group the operator belongs to (staff here), while a checkout under
/private/tmp on macOS is group wheel, which the operator usually is not a member of — so the
exact same test fails for real depending on where Maven happened to be started, and the existing
assumeTrue(view != null, ...) only guards "this filesystem has no POSIX groups", never "this
group exists but I'm not in it" — the case that actually bites.

Fix: both helpers now resolve the process's REAL primary group via id -gn instead of
reading any directory's owning group, with assumeTrue (skip, never fail) only when id itself
is unavailable or its output can't be resolved on the host. The permission assertions these
tests exist for (group-readable, never group-writable) are unchanged — only the group-resolution
plumbing changed. Also renamed nothing (the helper name was already accurate; only its body was
wrong) and removed the now-unused PosixFileAttributeView imports.

Build verification

Ran mvn clean install (full output, unpiped) in two locations, per #225 criterion 4:

  • Home checkout (/Users/dai.ha/LTMS/.bridged-worktrees/618644-2/fleetd):
    Tests run: 1093, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS
  • /private/tmp copy (mirroring the exact repro in #225 — copied via cp -R so the new tree
    inherits /private/tmp's wheel group, exactly reproducing the reported failure condition):
    Tests run: 1093, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS

Before the fix, I reproduced the original #225 failure from a /private/tmp copy of the
pre-existing code (2 errors, HerdrPeerLauncherAllowListWiringTest and OpenCodeLauncherTest,
"cannot share generated directory ... with group 'wheel'"), matching the ticket's report exactly.

#224 criterion 4 answer: with worktreeGroup: unset, shareRootWithGroup returns
immediately (same group == null guard every other group-sharing path in this class already
uses) — zero processes spawned, Files.createDirectories(root) and everything after it
unchanged. This is asserted directly by addSharesNothingForTheRootWhenNoGroupConfigured, and
every pre-existing add()-exercising test in the suite (none of which configure a group) passed
unchanged.

Scope note

Per the brief, scope was GitWorktrees + its tests, and the two currentUserGroup() test
helpers. I also added one test to EnvAllowListScrubTest.java (criterion 5, explicitly asked for
in the brief) — did not touch ClaudeCodeLauncher (that's fleetd #222, another worker's scope).

One line for the record, not investigated further: HerdrPeerLauncher's ZDOTDIR-scrub-parent-dir
logic (memberScrubParentDir()) resolves worktreeRoot the same way GitWorktrees.resolveRoot
does, so it benefits from this fix automatically — no separate change needed there.

Two related tickets in one PR: fleetd #224 (worktreeRoot is never made group-traversable) and fleetd #225 (two tests pass or fail depending on where the repo is checked out). ## #224 — worktreeRoot is never made group-traversable `GitWorktrees#shareWithGroup` shares each worktree path and the repo's common git dir with `worktreeGroup`, but never the **parent** directory (`worktreeRoot`) that contains every worktree. Under `memberHerdrSocket:` the member pane runs as a different OS user, and a directory needs the execute bit for that user to be traversed at all — so if `worktreeRoot` lands at (say) `0700`, everything under it is unreachable no matter how carefully each child is shared: the member can't read the ephemeral `opencode.json` #219 places there, can't reach its own worktree, and can't read #213's ZDOTDIR scrub when placed there either. **Fix** (`fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java`): - `add()` now calls a new private `shareRootWithGroup(root)` right after `Files.createDirectories(root)` — established once, where the root is created, never at a use site (not in `OpenCodeLauncher`, which would duplicate the policy in the wrong layer). - It `chgrp`s and `chmod g+x`s the root itself, **non-recursively** — each child underneath (a worktree, or a launcher-generated scrub/config dir) is still shared individually by its own existing call site. - No-op — zero processes spawned — when `worktreeGroup` is unset, so behaviour with today's only live mode is byte-identical (criterion 4, proven by a dedicated test asserting zero recorded commands). - On failure (group doesn't exist, or the operator isn't a member of it) the spawn is **refused** with a `WorktreeException` naming the root, its current POSIX mode, and the group — mirroring `shareWithGroup`'s existing refusal shape — and this happens *before* `git worktree add` ever runs, so no partial worktree is left behind. **Tests added** (`GitWorktreesTest`), each watched to FAIL against the pre-fix code first (I stashed the production change and confirmed both failed with the exact assertion messages below, then restored the fix and confirmed both pass): - `addSharesWorktreeRootWithGroupWhenConfigured` — recording-runner test proving `add()` chgrp's and chmod g+x's the root itself. - `addSharesNothingForTheRootWhenNoGroupConfigured` — criterion 4, zero processes spawned when no group is configured. - `addRefusesWhenWorktreeRootCannotBeMadeGroupTraversable` — drives the REAL `chgrp` (no fake runner) with a nonexistent group name, the same real-failure shape `shareWithGroupThrowsNamingTheGroupWhenChgrpFails` already uses for the per-worktree share, and asserts the exception names the root, its current mode, and the group, and that no worktree directory is left behind. **Also added** (criterion 5, the gap the #221 reviewer flagged): `EnvAllowListScrubTest` gained `shareWithGroupCoversEveryFlatFileIncludingTheOptionalOnes`, which drives `EnvAllowListScrub#shareWithGroup` directly against a directory holding `opencode.json`, `member-charter.md`, `ide-rules.md`, and an unrelated file, asserting every one of them ends up group-readable/never-group-writable — not just the two files someone happened to think of. ## #225 — two tests depend on where the repo is checked out `OpenCodeLauncherTest#currentUserGroup` and its twin in `HerdrPeerLauncherAllowListWiringTest` read the group that owns the **current working directory**, not the process's own primary group, despite the comment claiming the latter. Those coincide only by accident: a home checkout is typically owned by a group the operator belongs to (`staff` here), while a checkout under `/private/tmp` on macOS is group `wheel`, which the operator usually is not a member of — so the exact same test fails for real depending on where Maven happened to be started, and the existing `assumeTrue(view != null, ...)` only guards "this filesystem has no POSIX groups", never "this group exists but I'm not in it" — the case that actually bites. **Fix**: both helpers now resolve the process's REAL primary group via `id -gn` instead of reading any directory's owning group, with `assumeTrue` (skip, never fail) only when `id` itself is unavailable or its output can't be resolved on the host. The permission assertions these tests exist for (group-readable, never group-writable) are unchanged — only the group-resolution plumbing changed. Also renamed nothing (the helper name was already accurate; only its body was wrong) and removed the now-unused `PosixFileAttributeView` imports. ## Build verification Ran `mvn clean install` (full output, unpiped) in two locations, per #225 criterion 4: - **Home checkout** (`/Users/dai.ha/LTMS/.bridged-worktrees/618644-2/fleetd`): `Tests run: 1093, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS` - **`/private/tmp` copy** (mirroring the exact repro in #225 — copied via `cp -R` so the new tree inherits `/private/tmp`'s `wheel` group, exactly reproducing the reported failure condition): `Tests run: 1093, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS` Before the fix, I reproduced the original #225 failure from a `/private/tmp` copy of the pre-existing code (2 errors, `HerdrPeerLauncherAllowListWiringTest` and `OpenCodeLauncherTest`, "cannot share generated directory ... with group 'wheel'"), matching the ticket's report exactly. **#224 criterion 4 answer**: with `worktreeGroup:` unset, `shareRootWithGroup` returns immediately (same `group == null` guard every other group-sharing path in this class already uses) — zero processes spawned, `Files.createDirectories(root)` and everything after it unchanged. This is asserted directly by `addSharesNothingForTheRootWhenNoGroupConfigured`, and every pre-existing `add()`-exercising test in the suite (none of which configure a group) passed unchanged. ## Scope note Per the brief, scope was `GitWorktrees` + its tests, and the two `currentUserGroup()` test helpers. I also added one test to `EnvAllowListScrubTest.java` (criterion 5, explicitly asked for in the brief) — did not touch `ClaudeCodeLauncher` (that's fleetd #222, another worker's scope). One line for the record, not investigated further: `HerdrPeerLauncher`'s ZDOTDIR-scrub-parent-dir logic (`memberScrubParentDir()`) resolves `worktreeRoot` the same way `GitWorktrees.resolveRoot` does, so it benefits from this fix automatically — no separate change needed there.
agent added 1 commit 2026-09-02 02:47:54 +02:00
fleetd #224 / #225: share worktreeRoot with the group, and fix the group-detection test helper
CI / contract (pull_request) Successful in 44s
CI / build (pull_request) Successful in 1m42s
5c56cb347f
#224: GitWorktrees#add created worktreeRoot with the daemon's umask and never shared it with
worktreeGroup, even though shareWithGroup shares every child underneath it (each worktree, and
the repo's common git dir). Under memberHerdrSocket: the member pane runs as a different OS
user, which needs execute on every ancestor directory to reach anything underneath, no matter
how carefully each child is shared — so a member could not read the opencode.json #219 places
under this root, could not reach its own worktree, and could not read #213's ZDOTDIR scrub when
placed here either.

Fix: add() now calls a new shareRootWithGroup(root) right after creating the root, chgrp+chmod
g+x on the root itself (non-recursive — each child is still shared individually by its own call
site). No-op when worktreeGroup is unset, so behaviour is byte-identical in today's only live
mode. On failure (group missing, or operator not a member of it) the spawn is refused with a
WorktreeException naming the root, its current mode, and the group — mirroring shareWithGroup's
existing refusal shape — before `git worktree add` ever runs, so no partial worktree is left
behind.

Also adds the assertion the #221 reviewer flagged as missing: a test driving
EnvAllowListScrub#shareWithGroup directly against a directory holding several flat files
(opencode.json, member-charter.md, ide-rules.md, plus an unrelated one) and asserting every one
of them gets group-readable/never-group-writable permissions, not just the two files someone
happened to think of.

#225: OpenCodeLauncherTest/HerdrPeerLauncherAllowListWiringTest's currentUserGroup() read the
group that owns the current working directory, not the process's own primary group, despite its
comment claiming the latter. Those coincide only by accident: a home checkout is typically owned
by a group the operator belongs to (staff), while a checkout under /private/tmp on macOS is
group wheel, which the operator is usually not a member of — so the same test fails for real
depending on where the repo happens to be checked out, and the existing assumeTrue only guarded
against "no POSIX groups at all", never "a resolvable but wrong group". Fixed by resolving the
process's REAL primary group via `id -gn` instead, with assumeTrue (skip, not fail) only when
that itself cannot be resolved on the host. The permission assertions these tests exist for are
unchanged.

Verified `mvn clean install` green from both a home checkout and a /private/tmp copy (mirroring
the exact repro in #225): 1093 tests, 0 failures, 0 errors in both locations.
ltms merged commit 39c7ce76f3 into main 2026-09-02 02:52:12 +02:00
Sign in to join this conversation.