audit: spawn-boundary asymmetry in member/ launchers (AUDIT.md)
This commit is contained in:
@@ -0,0 +1,90 @@
|
||||
# Spawn-boundary audit — `member/` launchers
|
||||
|
||||
Scope: `fleetd/src/main/java/dev/ltms/fleet/member/` (`ClaudeCodeLauncher`, `OpenCodeLauncher`,
|
||||
`HerdrPeerLauncher`, `CompositePeerLauncher`, `EnvAllowListScrub`, `MemberEnvAllowList`,
|
||||
`MemberCredentialPolicyView`, `OpenCodeSessionDiscovery`). No `worker/` package exists — the
|
||||
launcher classes live entirely under `member/`, so I read that whole directory instead.
|
||||
|
||||
## Main finding
|
||||
|
||||
**1. `fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java:548-632` (`seedTrustDialog`)**
|
||||
|
||||
**Issue.** `seedTrustDialog` writes the workspace-trust seed to `configDir/.claude.json`, or to
|
||||
`~/.claude.json` (fleetd's own `user.home`) when `configDir` is unset. It gates only on
|
||||
`isProvisionedWorktree(cwd)` (line 549). It never checks `memberHerdrSocketConfigured()` and,
|
||||
being a `static` method, cannot reach that instance state even if it wanted to.
|
||||
|
||||
Compare this to its sibling in the very same class, `writeCharterFile` (line 788, an *instance*
|
||||
method): that one explicitly checks `memberHerdrSocketConfigured()` (line 790) and, when true,
|
||||
routes the file under `memberScrubParentDir()` (`worktreeRoot`) and calls
|
||||
`EnvAllowListScrub.shareWithGroup(dir, group)` (line 813) — or refuses the spawn outright when
|
||||
`worktreeRoot`/`worktreeGroup` isn't configured (lines 798-807), rather than hand the member a
|
||||
path its OS user cannot read. `OpenCodeLauncher.writeConfig`/`configParentDir` (lines 422-508,
|
||||
542-559) do the identical thing for `opencode.json`. `seedTrustDialog` is the one cross-process,
|
||||
member-facing file write in this class that skips that treatment entirely.
|
||||
|
||||
It also makes the miss worse on its own: `writeAtomically` (line 696) calls
|
||||
`copyPosixPermissionsIfPresent` (line 710), which deliberately preserves the *existing* file's
|
||||
POSIX permissions — and Claude Code ships `.claude.json` at `0600` (see the javadoc at line 681).
|
||||
So even where `configDir` happens to point at an already-shared directory, the seeded file itself
|
||||
is written owner-only, unreadable by a different OS user.
|
||||
|
||||
**Spawn path that reaches it.** Any `claude` profile that (a) sets `memberHerdrSocket:` (real,
|
||||
validated config — see `memberHerdrSocketConfigured()` at `HerdrPeerLauncher.java:1667`) and
|
||||
(b) does **not** set `configDir:`, spawned with `worktree:true` (so `isProvisionedWorktree(cwd)`
|
||||
is true and the method proceeds). `buildLaunch` calls `seedTrustDialog(cfg.configDir(), spec.cwd())`
|
||||
unconditionally at line 290, before the pane is ever created.
|
||||
|
||||
**What goes wrong.** Under that config, `seedTrustDialog` writes the trust entry into fleetd's
|
||||
*own* `~/.claude.json` — the daemon's OS user's home. But the actual Claude Code process starts
|
||||
under the *member's* different OS user (that's the whole point of `memberHerdrSocket`), with its
|
||||
own `$HOME`, and (with `configDir` unset) reads `~/.claude.json` relative to *that* home — a file
|
||||
this method never touches. The seed lands nowhere the spawned process will ever look. The result
|
||||
is exactly the fleetd #149 incident this method exists to prevent: the member sits on Claude
|
||||
Code's interactive, un-timed "Is this a project you trust?" dialog forever, never mounts the
|
||||
bridge MCP, and never calls `fleet_reply`; the CB-306 spawn-readiness gate eventually times it
|
||||
out as an unexplained "did not reach injectable state."
|
||||
|
||||
**Confidence.** High that the code path is exactly as described — I traced `buildLaunch` →
|
||||
`seedTrustDialog` → `writeAtomically`/`copyPosixPermissionsIfPresent` directly, and confirmed
|
||||
`writeCharterFile`'s contrasting `memberHerdrSocketConfigured()` gate at the same class's
|
||||
line 790. Medium on operational reachability *today*: several javadocs elsewhere in
|
||||
`HerdrPeerLauncher` describe "`memberHerdrSocket` ABSENT" as "today's only live mode," so this
|
||||
gap may not be hit by the currently-deployed fleet — but it is a real, config-reachable hole in a
|
||||
feature the codebase otherwise treats as first-class (four other defects fixed for exactly this
|
||||
seam: fleetd #213, #219, #222 in this same file/its sibling). Direction of harm: an operator who
|
||||
turns `memberHerdrSocket` on gets a silently-hung `claude` member instead of a working one — no
|
||||
data loss, no credential leak, but a resource stuck occupying a pane with no diagnosis pointing
|
||||
at the real cause (this is the "silent" failure mode the class's other methods were deliberately
|
||||
rewritten to avoid).
|
||||
|
||||
## Secondary findings
|
||||
|
||||
**2. `fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java:788-818` (`writeCharterFile`) and `OpenCodeLauncher.java:422-508` (`writeConfig`)**
|
||||
|
||||
Both create a per-spawn temp file/directory (role+reply charter file; `opencode.json` +
|
||||
member-charter + IDE-rules files) and rely only on `deleteOnExit` for cleanup. Unlike the CB-633
|
||||
ZDOTDIR directory, which `spawnInternal` (`HerdrPeerLauncher.java:519-528`) explicitly deletes
|
||||
with `EnvAllowListScrub.deleteRecursively(zdotdir)` when `spawnInTab`/`spawnAsPane` throws *after*
|
||||
`buildLaunch` succeeded, nothing tears down the charter file or the opencode config directory on
|
||||
that same failure path — they leak until JVM exit, which for a long-running daemon can be
|
||||
effectively never. Symmetric between the two launchers (not an asymmetry), so it is secondary, but
|
||||
it is the same "created at spawn, not cleaned up if spawn then fails" shape the brief called out.
|
||||
Confidence: high that the code omits it (read both methods and the `spawnInternal` catch block);
|
||||
low-medium severity — disk clutter in a temp/worktree-scrub directory, not a security or
|
||||
correctness issue on the member itself.
|
||||
|
||||
**3. Shape noted, not investigated.** `ClaudeCodeLauncher.writeIdeOverlay` (line 438) also skips
|
||||
`memberHerdrSocketConfigured()`, but unlike `seedTrustDialog` it writes into `cwd` itself (the
|
||||
worktree root), which — if worktree provisioning already shares the worktree with the member's OS
|
||||
user under `worktreeGroup` (plausible, but I did not verify the worktree-provisioning code, which
|
||||
is outside this scope) — would already be readable/writable by the member without any extra
|
||||
routing. Flagging the shape rather than a finding since I have not confirmed worktree provisioning
|
||||
outside `member/`.
|
||||
|
||||
## Out of scope, noted only
|
||||
|
||||
- The `worker/` directory named in the brief does not exist; all launcher classes live in
|
||||
`member/`, which I reviewed in full instead.
|
||||
- Worktree-provisioning permissions (whether `worktreeGroup` sharing actually covers `cwd`) live
|
||||
outside this package and were not verified.
|
||||
Reference in New Issue
Block a user