Reference in New Issue
Block a user
Delete Branch "worker/cb224-worktree-root-group-024523-2"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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#shareWithGroupshares each worktree path and the repo's common git dir withworktreeGroup, but never the parent directory (worktreeRoot) that contains everyworktree. Under
memberHerdrSocket:the member pane runs as a different OS user, and adirectory needs the execute bit for that user to be traversed at all — so if
worktreeRootlands at (say)
0700, everything under it is unreachable no matter how carefully each child isshared: the member can't read the ephemeral
opencode.json#219 places there, can't reach itsown 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 privateshareRootWithGroup(root)right afterFiles.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).
chgrps andchmod 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.
worktreeGroupis unset, so behaviour with today's onlylive mode is byte-identical (criterion 4, proven by a dedicated test asserting zero recorded
commands).
with a
WorktreeExceptionnaming the root, its current POSIX mode, and the group — mirroringshareWithGroup's existing refusal shape — and this happens beforegit worktree addeverruns, so no partial worktree is left behind.
Tests added (
GitWorktreesTest), each watched to FAIL against the pre-fix code first (Istashed 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 provingadd()chgrp'sand chmod g+x's the root itself.
addSharesNothingForTheRootWhenNoGroupConfigured— criterion 4, zero processes spawned when nogroup is configured.
addRefusesWhenWorktreeRootCannotBeMadeGroupTraversable— drives the REALchgrp(no fakerunner) with a nonexistent group name, the same real-failure shape
shareWithGroupThrowsNamingTheGroupWhenChgrpFailsalready uses for the per-worktree share, andasserts 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):
EnvAllowListScrubTestgainedshareWithGroupCoversEveryFlatFileIncludingTheOptionalOnes, which drivesEnvAllowListScrub#shareWithGroupdirectly against a directory holdingopencode.json,member-charter.md,ide-rules.md, and an unrelated file, asserting every one of them ends upgroup-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#currentUserGroupand its twin inHerdrPeerLauncherAllowListWiringTestread 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 (
staffhere), while a checkout under/private/tmpon macOS is groupwheel, which the operator usually is not a member of — so theexact 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 "thisgroup 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 -gninstead ofreading any directory's owning group, with
assumeTrue(skip, never fail) only wheniditselfis 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
PosixFileAttributeViewimports.Build verification
Ran
mvn clean install(full output, unpiped) in two locations, per #225 criterion 4:/Users/dai.ha/LTMS/.bridged-worktrees/618644-2/fleetd):Tests run: 1093, Failures: 0, Errors: 0, Skipped: 0—BUILD SUCCESS/private/tmpcopy (mirroring the exact repro in #225 — copied viacp -Rso the new treeinherits
/private/tmp'swheelgroup, exactly reproducing the reported failure condition):Tests run: 1093, Failures: 0, Errors: 0, Skipped: 0—BUILD SUCCESSBefore the fix, I reproduced the original #225 failure from a
/private/tmpcopy of thepre-existing code (2 errors,
HerdrPeerLauncherAllowListWiringTestandOpenCodeLauncherTest,"cannot share generated directory ... with group 'wheel'"), matching the ticket's report exactly.
#224 criterion 4 answer: with
worktreeGroup:unset,shareRootWithGroupreturnsimmediately (same
group == nullguard every other group-sharing path in this class alreadyuses) — zero processes spawned,
Files.createDirectories(root)and everything after itunchanged. This is asserted directly by
addSharesNothingForTheRootWhenNoGroupConfigured, andevery pre-existing
add()-exercising test in the suite (none of which configure a group) passedunchanged.
Scope note
Per the brief, scope was
GitWorktrees+ its tests, and the twocurrentUserGroup()testhelpers. I also added one test to
EnvAllowListScrubTest.java(criterion 5, explicitly asked forin 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-dirlogic (
memberScrubParentDir()) resolvesworktreeRootthe same wayGitWorktrees.resolveRootdoes, so it benefits from this fix automatically — no separate change needed there.