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

worktreeRoot itself is never made group-traversable, so a restrictive umask breaks every member under memberHerdrSocket #224

New Issue
Closed
opened 2026-09-01 10:33:33 +02:00 by ltms · 1 comment
ltms commented 2026-09-01 10:33:33 +02:00
Owner
Copy Link
Copy Source

Found by the reviewer on PR #221 (#219). I checked both halves in the code before filing.

The gap

GitWorktrees#add creates the worktree root with the daemon's umask and nothing else:

Path root = resolveRoot(repoRoot);
Path path = root.resolve(nonce);
Files.createDirectories(root);          // GitWorktrees.java:160 — umask decides the mode

GitWorktrees#shareWithGroup(repoRoot, worktreePath) then shares each worktree path and its git paths — never the root that contains them.

Under memberHerdrSocket: the member pane runs as a different OS user. A directory needs the execute bit for that user to traverse it, so if worktreeRoot lands at 0700, everything under it is unreachable no matter how carefully each child is shared:

  • the member cannot read the ephemeral opencode.json that #219 just moved there, so it never learns where the bridge MCP is;
  • the member cannot reach its own worktree, so it has no checkout at all;
  • the same applies to #213's generated ZDOTDIR when it is placed under that root.

So this is not an opencode problem. It is the floor the whole memberHerdrSocket rollout stands on.

Why it is not currently a defect

Measured on the Mac today:

$ ls -ld /Users/dai.ha/LTMS/.bridged-worktrees
drwxr-xr-x@ 4 dai.ha  staff  128 Sep  1 15:25

Files.createDirectories respects the umask, and this daemon's umask yields 0755, which any uid can traverse. memberHerdrSocket: is also unset on this host. Both conditions have to change before this bites, which is why #221 was merged rather than held.

The trap is that it depends on the umask of whatever shell started the daemon — the same class of invisible dependency as the login-shell rule for WORKER_GITEA_TOKEN. It would work on the machine it was developed on and fail on the next one, and the failure would present as a member that never becomes deliverable: nothing in that signal points at a directory mode.

Not the same as #219 site 1, and do not fix it there

#219 shares the child directory it creates. That is correct and complete for what it owns. This ticket is about the parent, which GitWorktrees owns and OpenCodeLauncher has no business changing. Fixing it inside the launcher would put a second copy of the policy in the wrong layer.

Acceptance

  • When worktreeGroup: is configured, worktreeRoot itself carries that group and group-execute, established where the root is created rather than at each use site.
  • If the root cannot be made traversable, the spawn is refused with a message naming the root, its current mode and the group — not a member that starts and then cannot see its own checkout. This follows #219's refusal decision for the same reason: a member that cannot read its worktree is broken, not degraded.
  • A test drives the real path with a deliberately restrictive root and asserts the refusal. #219's reviewer flagged this exact test as missing, so it is the one that must not be skipped.
  • With worktreeGroup: unset (today's only live mode), behaviour is byte-identical.

Also worth doing here

The #221 reviewer noted there is no test asserting group permissions on the optional files in the shared directory (member-charter.md, ide-rules.md). EnvAllowListScrub#shareWithGroup lists one flat level and does cover them — I checked that myself — but nothing pins it, so a future change that writes a file after the sharing call, or into a subdirectory, would pass. Cheap to add alongside this.

Related

#185 (parent — the memberHerdrSocket rollout) · #219 / PR #221 (found while reviewing it) · #213 (the ZDOTDIR scrub, same parent directory) · #222 (the claude-code charter file, same rollout).

Found by the reviewer on PR #221 (#219). I checked both halves in the code before filing. ## The gap `GitWorktrees#add` creates the worktree root with the daemon's umask and nothing else: ```java Path root = resolveRoot(repoRoot); Path path = root.resolve(nonce); Files.createDirectories(root); // GitWorktrees.java:160 — umask decides the mode ``` `GitWorktrees#shareWithGroup(repoRoot, worktreePath)` then shares **each worktree path and its git paths** — never the root that contains them. Under `memberHerdrSocket:` the member pane runs as a different OS user. A directory needs the execute bit for that user to traverse it, so if `worktreeRoot` lands at `0700`, everything under it is unreachable no matter how carefully each child is shared: - the member cannot read the ephemeral `opencode.json` that #219 just moved there, so it never learns where the bridge MCP is; - the member cannot reach **its own worktree**, so it has no checkout at all; - the same applies to #213's generated ZDOTDIR when it is placed under that root. So this is not an opencode problem. It is the floor the whole `memberHerdrSocket` rollout stands on. ## Why it is not currently a defect Measured on the Mac today: ``` $ ls -ld /Users/dai.ha/LTMS/.bridged-worktrees drwxr-xr-x@ 4 dai.ha staff 128 Sep 1 15:25 ``` `Files.createDirectories` respects the umask, and this daemon's umask yields `0755`, which any uid can traverse. `memberHerdrSocket:` is also unset on this host. **Both conditions have to change before this bites**, which is why #221 was merged rather than held. The trap is that it depends on the umask of whatever shell started the daemon — the same class of invisible dependency as the login-shell rule for `WORKER_GITEA_TOKEN`. It would work on the machine it was developed on and fail on the next one, and the failure would present as a member that never becomes deliverable: nothing in that signal points at a directory mode. ## Not the same as #219 site 1, and do not fix it there #219 shares the *child* directory it creates. That is correct and complete for what it owns. This ticket is about the *parent*, which `GitWorktrees` owns and `OpenCodeLauncher` has no business changing. Fixing it inside the launcher would put a second copy of the policy in the wrong layer. ## Acceptance - When `worktreeGroup:` is configured, `worktreeRoot` itself carries that group and group-execute, established where the root is created rather than at each use site. - If the root cannot be made traversable, the spawn is **refused** with a message naming the root, its current mode and the group — not a member that starts and then cannot see its own checkout. This follows #219's refusal decision for the same reason: a member that cannot read its worktree is broken, not degraded. - A test drives the real path with a deliberately restrictive root and asserts the refusal. #219's reviewer flagged this exact test as missing, so it is the one that must not be skipped. - With `worktreeGroup:` unset (today's only live mode), behaviour is byte-identical. ## Also worth doing here The #221 reviewer noted there is no test asserting group permissions on the *optional* files in the shared directory (`member-charter.md`, `ide-rules.md`). `EnvAllowListScrub#shareWithGroup` lists one flat level and does cover them — I checked that myself — but nothing pins it, so a future change that writes a file after the sharing call, or into a subdirectory, would pass. Cheap to add alongside this. ## Related #185 (parent — the `memberHerdrSocket` rollout) · #219 / PR #221 (found while reviewing it) · #213 (the ZDOTDIR scrub, same parent directory) · #222 (the claude-code charter file, same rollout).
ltms commented 2026-09-02 03:04:18 +02:00
Author
Owner
Copy Link
Copy Source

Fixed in PR #230, merged to main (39c7ce7).

GitWorktrees#add now calls a new shareRootWithGroup(root) right after Files.createDirectories(root) — chgrp plus chmod g+x on worktreeRoot itself, non-recursive, because each child is already shared by its own call site. It runs before git worktree add, so a failure leaves no half-made worktree behind. The refusal names the root, its mode at the time of the attempt, and the group.

I checked the one thing that could have made this wrong: the guard is group == null, while the javadoc promises "null/blank". That is correct here because the constructor normalises blank to null at GitWorktrees:148, and it matches the guard the existing shareWithGroup already uses.

Criterion 5 is done too — EnvAllowListScrubTest now drives shareWithGroup against a directory holding opencode.json, member-charter.md, ide-rules.md and an unrelated file, and asserts the permissions on every one. That behaviour was already correct; nothing pinned it, so a future change writing a file after the sharing call would have passed silently.

Verified on main after merge: BUILD SUCCESS, 1100 tests, 0 failures, 0 skipped, from both a home checkout and a genuinely wheel-grouped copy under /private/tmp. Live on the running daemon since pid 29196, jar 4a0e2c8f0eef.

Still latent by design: memberHerdrSocket: is unset on this host, so nothing here changes live behaviour yet. It is a blocker removed from #185, not a live fix.

Fixed in PR #230, merged to `main` (39c7ce7). `GitWorktrees#add` now calls a new `shareRootWithGroup(root)` right after `Files.createDirectories(root)` — `chgrp` plus `chmod g+x` on `worktreeRoot` itself, non-recursive, because each child is already shared by its own call site. It runs **before** `git worktree add`, so a failure leaves no half-made worktree behind. The refusal names the root, its mode at the time of the attempt, and the group. I checked the one thing that could have made this wrong: the guard is `group == null`, while the javadoc promises "null/blank". That is correct here because the constructor normalises blank to null at `GitWorktrees:148`, and it matches the guard the existing `shareWithGroup` already uses. Criterion 5 is done too — `EnvAllowListScrubTest` now drives `shareWithGroup` against a directory holding `opencode.json`, `member-charter.md`, `ide-rules.md` and an unrelated file, and asserts the permissions on every one. That behaviour was already correct; nothing pinned it, so a future change writing a file after the sharing call would have passed silently. Verified on `main` after merge: BUILD SUCCESS, 1100 tests, 0 failures, 0 skipped, from both a home checkout and a genuinely `wheel`-grouped copy under `/private/tmp`. Live on the running daemon since pid 29196, jar `4a0e2c8f0eef`. Still latent by design: `memberHerdrSocket:` is unset on this host, so nothing here changes live behaviour yet. It is a blocker removed from #185, not a live fix.
ltms closed this issue 2026-09-02 03:04:25 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#224
Delete Branch "%!s()"

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?