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

fleetd #155: refuse a member spawn under memberCredentials.policy=allow-list on a non-zsh shell #255

Closed
agent wants to merge 0 commits from worker/fleetd-155c-f8ef4b-8 into main
pull from: worker/fleetd-155c-f8ef4b-8
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-09-03 08:15:08 +02:00
Member
Copy Link
Copy Source

Ticket: fleetd #155 — the member credential scrub is zsh-only; the daemon never checked
what shell it was about to launch into, so the memberCredentials.policy: allow-list
control could silently do nothing on a non-zsh member.

Step 1 — what the code actually proved about the "zsh-only" premise

The premise ("the daemon does not check what shell it is about to launch into") was
already partly false by the time I read the code. HerdrPeerLauncher.isZshShell /
applyEnvironmentAllowListPolicy (added by fleetd #213, for a different reason — routing
under memberHerdrSocket) already detects a non-zsh login shell and already logs a WARN
(warnNonZsh) naming it. What #213 left in place — documented explicitly in its own
javadoc ("In every branch: never refuse to spawn") and in FleetConfig.memberLoginShell's
javadoc ("a degraded control, never a refusal to spawn") — is that on non-zsh it degrades
to the CB-596 sentinel overlay and spawns anyway. That overlay is applied before the pane's
login shell runs, so a sourced file can undo it — the exact weakness policy: allow-list
exists to remove. So the real defect wasn't "no detection" — it was "detects, warns, then
spawns unprotected under the policy the operator picked specifically to avoid that."

I did not find any case where policy: allow-list + non-zsh currently refuses — three
existing tests (aNonZshShellGeneratesNothingAndFallsBack,
memberHerdrSocketWithNonZshMemberLoginShellFallsBackToTheOverlay,
memberHerdrSocketWithNoMemberLoginShellFallsBackAndNeverConsultsFleetdsOwnShell, plus
ClaudeCodeLauncherTest.allowListPolicyOnNonZshKeepsTheWarnWording) all asserted the old
"falls back, spawns anyway" behaviour through the real spawn path. I updated all four (see
below) since the ticket's decision is to change exactly this.

Step 2 — what I changed

memberCredentials.policy now decides what "non-zsh" means, per the ticket's rule:

  • policy: allow-list (HerdrPeerLauncher.applyEnvironmentAllowListPolicy): a non-zsh
    login shell now throws IllegalArgumentException, naming the shell, before any pane
    is created (no herdr call happens, no ZDOTDIR is generated, no env is mutated). The
    message also names the real config key: memberLoginShell: (verified against
    FleetConfig.memberLoginShell() — this is the field the memberHerdrSocket-configured
    branch reads), and suggests switching memberCredentials.policy: deny-by-default as the
    alternative.
  • policy: deny-by-default (HerdrPeerLauncher.applyMemberCredentialPolicy): unaffected
    in substance — its overlay is a pane-creation env map entry, not a ZDOTDIR mechanism, so it
    does not depend on the shell at all and the spawn is never refused here. I added a one-time
    WARN (reusing warnNonZsh, reworded) naming the shell, since the ticket's rule asks for a
    warning either way and it tells the operator the stronger allow-list control is
    unavailable on this shell.
  • Extracted the shared shell-resolution logic (memberHerdrSocket configured →
    memberLoginShell: config; absent → fleetd's own $SHELL) into one
    memberLoginShell() helper used by both policies, so they can't drift on what "the
    member's shell" means.
  • Updated the javadoc that explicitly claimed "never refuse to spawn" / "a degraded
    control, never a refusal to spawn" in HerdrPeerLauncher and FleetConfig — both were
    correct before this change and would be actively wrong afterward.
  • Left the worktreeRoot/worktreeGroup-missing degrade path (under
    memberHerdrSocket) exactly as it was — still warns and falls back, never refuses. That
    gap is fleetd #213's scope, not this ticket's, and the ticket asked me to change only the
    shell case.

Which catch block the refusal lands in

FleetMcp.spawn (fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:869-870):

} catch (IllegalArgumentException e) {
    return error(e.getMessage()); // unknown / no-default profile, or a refused resumeSessionId
}

I traced the call path to confirm nothing between applyEnvironmentAllowListPolicy (which
throws) and this catch block swallows or rewraps it:
HerdrPeerLauncher.spawnInternal -> HerdrPeerLauncher.spawn -> (unwrapped by both
CompositePeerLauncher.spawn paths — the explicit-profile path calls d.spawn(req) with
nothing but a PeerUnreachableException catch guarding the unqualified path's retry loop,
which an IllegalArgumentException never hits) -> SessionManager.acquire (its
catch (RuntimeException e) { ...; throw e; } only logs and rethrows) -> FleetMcp.spawn's
catch (IllegalArgumentException e) above.

Mutation evidence

Broke the production check (if (!zsh) -> if (false) in
HerdrPeerLauncher.applyEnvironmentAllowListPolicy), ran:

mvn -o test -Dtest=HerdrPeerLauncherAllowListWiringTest#aNonZshShellUnderAllowListPolicyRefusesTheSpawn

Result: RED, 0 compile errors —

org.opentest4j.AssertionFailedError: policy=allow-list on a non-zsh shell must refuse the
spawn, not silently degrade ==> Expected java.lang.IllegalArgumentException to be thrown,
but nothing was thrown.
	at dev.ltms.fleet.member.HerdrPeerLauncherAllowListWiringTest
        .aNonZshShellUnderAllowListPolicyRefusesTheSpawn(HerdrPeerLauncherAllowListWiringTest.java:115)

Reverted the mutation, then confirmed via git diff that the file returned to exactly
if (!zsh) { with no other residue.

Real build output

mvn clean install
...
[INFO] Tests run: 1234, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
[INFO] Total time:  39.459 s

Unpiped, full output read (not | tail). All 1234 tests pass, 0 failures, 0 errors.

Tests changed (all go through the real spawn path, per acceptance criterion 3)

  • HerdrPeerLauncherAllowListWiringTest: renamed/rewrote
    aNonZshShellGeneratesNothingAndFallsBack ->
    aNonZshShellUnderAllowListPolicyRefusesTheSpawn (asserts IllegalArgumentException
    naming the policy + shell, through HerdrPeerLauncher.spawn); rewrote
    noAllowedCountLineIsEmittedOnTheNonZshFallbackPath ->
    ...RefusalPath (still asserts no "allowed N of M" line, now under
    assertThrows); renamed/rewrote
    memberHerdrSocketWithNonZshMemberLoginShellFallsBackToTheOverlay and
    memberHerdrSocketWithNoMemberLoginShellFallsBackAndNeverConsultsFleetdsOwnShell ->
    ...RefusesTheSpawn / ...RefusesAndNeverConsultsFleetdsOwnShell (same, plus the
    existing "$SHELL never consulted" assertion is preserved); updated
    gapDetectorReportsUnknownInsteadOfAConclusionWhenMemberHerdrSocketIsConfigured and
    theUnknownEnvironmentWarnFiresOnceNotOncePerSpawn to configure an explicit zsh
    memberLoginShell, since they test the separate memberHerdrSocket
    unknown-environment WARN via the (still-degrading) worktreeRoot/worktreeGroup-missing
    fallback, not the zsh gate — under the old code both fallbacks reached the same
    logCredentialGap call, but the zsh-gate one no longer exists.
  • ClaudeCodeLauncherTest: renamed/rewrote allowListPolicyOnNonZshKeepsTheWarnWording ->
    allowListPolicyOnNonZshRefusesTheSpawn (asserts the refusal through
    ClaudeCodeLauncher.spawn(), the adapter's own real spawn path — not the shared launcher
    method directly).

What I did not verify

Per the ticket: I did not launch a real bash member — that needs daemon control I don't
have, and I was told not to spawn members or restart the daemon. Everything above is proven
at the unit-test level through the real spawn() call chain (HerdrPeerLauncher.spawn /
ClaudeCodeLauncher.spawn), never by calling applyEnvironmentAllowListPolicy directly.

Out of scope (per the brief) — noted, not touched

  • The worktreeRoot/worktreeGroup-missing degrade path under memberHerdrSocket has the
    same "control silently does nothing" shape in a narrower sense — it still degrades and
    spawns rather than refusing — but the ticket explicitly scoped this to the shell case and
    called moving the scrub off the shell (or hardening this other path) out of scope. Left
    alone; flagging in case the lead wants a follow-up ticket.
  • wiki/11-Features.md is not updated — this is a behaviour change to an existing config
    key's semantics (memberCredentials.policy: allow-list), not a new visible knob, and
    workers should not edit wiki/ (submodule, separate remote, often stale per team memory).
    Flagging for the lead to decide whether it needs a wiki entry or an 9-Implementation.md
    note.

Hard constraints followed

  • No credential values printed; no ${VAR:-x} expansions used.
  • No ps/pgrep -fl argv dumps.
  • ${SHARED_ENV}/tools/secrets.sh untouched.
  • No daemon restart, no member spawn.
Ticket: fleetd #155 — the member credential scrub is zsh-only; the daemon never checked what shell it was about to launch into, so the `memberCredentials.policy: allow-list` control could silently do nothing on a non-zsh member. ## Step 1 — what the code actually proved about the "zsh-only" premise The premise ("the daemon does not check what shell it is about to launch into") was **already partly false** by the time I read the code. `HerdrPeerLauncher.isZshShell` / `applyEnvironmentAllowListPolicy` (added by fleetd #213, for a different reason — routing under `memberHerdrSocket`) already detects a non-zsh login shell and already logs a WARN (`warnNonZsh`) naming it. What #213 left in place — documented explicitly in its own javadoc ("In every branch: never refuse to spawn") and in `FleetConfig.memberLoginShell`'s javadoc ("a degraded control, never a refusal to spawn") — is that on non-zsh it **degrades to the CB-596 sentinel overlay and spawns anyway**. That overlay is applied before the pane's login shell runs, so a sourced file can undo it — the exact weakness `policy: allow-list` exists to remove. So the real defect wasn't "no detection" — it was "detects, warns, then spawns unprotected under the policy the operator picked specifically to avoid that." I did **not** find any case where `policy: allow-list` + non-zsh currently refuses — three existing tests (`aNonZshShellGeneratesNothingAndFallsBack`, `memberHerdrSocketWithNonZshMemberLoginShellFallsBackToTheOverlay`, `memberHerdrSocketWithNoMemberLoginShellFallsBackAndNeverConsultsFleetdsOwnShell`, plus `ClaudeCodeLauncherTest.allowListPolicyOnNonZshKeepsTheWarnWording`) all asserted the old "falls back, spawns anyway" behaviour through the real spawn path. I updated all four (see below) since the ticket's decision is to change exactly this. ## Step 2 — what I changed `memberCredentials.policy` now decides what "non-zsh" means, per the ticket's rule: - **`policy: allow-list`** (`HerdrPeerLauncher.applyEnvironmentAllowListPolicy`): a non-zsh login shell now throws `IllegalArgumentException`, naming the shell, **before** any pane is created (no herdr call happens, no ZDOTDIR is generated, no env is mutated). The message also names the real config key: `memberLoginShell:` (verified against `FleetConfig.memberLoginShell()` — this is the field the `memberHerdrSocket`-configured branch reads), and suggests switching `memberCredentials.policy: deny-by-default` as the alternative. - **`policy: deny-by-default`** (`HerdrPeerLauncher.applyMemberCredentialPolicy`): unaffected in substance — its overlay is a pane-creation env map entry, not a ZDOTDIR mechanism, so it does not depend on the shell at all and the spawn is never refused here. I added a one-time WARN (reusing `warnNonZsh`, reworded) naming the shell, since the ticket's rule asks for a warning either way and it tells the operator the stronger `allow-list` control is unavailable on this shell. - Extracted the shared shell-resolution logic (`memberHerdrSocket` configured → `memberLoginShell:` config; absent → fleetd's own `$SHELL`) into one `memberLoginShell()` helper used by both policies, so they can't drift on what "the member's shell" means. - Updated the javadoc that explicitly claimed "never refuse to spawn" / "a degraded control, never a refusal to spawn" in `HerdrPeerLauncher` and `FleetConfig` — both were correct before this change and would be actively wrong afterward. - Left the **`worktreeRoot`/`worktreeGroup`-missing** degrade path (under `memberHerdrSocket`) exactly as it was — still warns and falls back, never refuses. That gap is fleetd #213's scope, not this ticket's, and the ticket asked me to change only the shell case. ## Which catch block the refusal lands in `FleetMcp.spawn` (fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:869-870): ```java } catch (IllegalArgumentException e) { return error(e.getMessage()); // unknown / no-default profile, or a refused resumeSessionId } ``` I traced the call path to confirm nothing between `applyEnvironmentAllowListPolicy` (which throws) and this catch block swallows or rewraps it: `HerdrPeerLauncher.spawnInternal` -> `HerdrPeerLauncher.spawn` -> (unwrapped by both `CompositePeerLauncher.spawn` paths — the explicit-profile path calls `d.spawn(req)` with nothing but a `PeerUnreachableException` catch guarding the *unqualified* path's retry loop, which an `IllegalArgumentException` never hits) -> `SessionManager.acquire` (its `catch (RuntimeException e) { ...; throw e; }` only logs and rethrows) -> `FleetMcp.spawn`'s `catch (IllegalArgumentException e)` above. ## Mutation evidence Broke the production check (`if (!zsh)` -> `if (false)` in `HerdrPeerLauncher.applyEnvironmentAllowListPolicy`), ran: ``` mvn -o test -Dtest=HerdrPeerLauncherAllowListWiringTest#aNonZshShellUnderAllowListPolicyRefusesTheSpawn ``` Result: **RED**, 0 compile errors — ``` org.opentest4j.AssertionFailedError: policy=allow-list on a non-zsh shell must refuse the spawn, not silently degrade ==> Expected java.lang.IllegalArgumentException to be thrown, but nothing was thrown. at dev.ltms.fleet.member.HerdrPeerLauncherAllowListWiringTest .aNonZshShellUnderAllowListPolicyRefusesTheSpawn(HerdrPeerLauncherAllowListWiringTest.java:115) ``` Reverted the mutation, then confirmed via `git diff` that the file returned to exactly `if (!zsh) {` with no other residue. ## Real build output ``` mvn clean install ... [INFO] Tests run: 1234, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS [INFO] Total time: 39.459 s ``` Unpiped, full output read (not `| tail`). All 1234 tests pass, 0 failures, 0 errors. ## Tests changed (all go through the real spawn path, per acceptance criterion 3) - `HerdrPeerLauncherAllowListWiringTest`: renamed/rewrote `aNonZshShellGeneratesNothingAndFallsBack` -> `aNonZshShellUnderAllowListPolicyRefusesTheSpawn` (asserts `IllegalArgumentException` naming the policy + shell, through `HerdrPeerLauncher.spawn`); rewrote `noAllowedCountLineIsEmittedOnTheNonZshFallbackPath` -> `...RefusalPath` (still asserts no "allowed N of M" line, now under `assertThrows`); renamed/rewrote `memberHerdrSocketWithNonZshMemberLoginShellFallsBackToTheOverlay` and `memberHerdrSocketWithNoMemberLoginShellFallsBackAndNeverConsultsFleetdsOwnShell` -> `...RefusesTheSpawn` / `...RefusesAndNeverConsultsFleetdsOwnShell` (same, plus the existing "$SHELL never consulted" assertion is preserved); updated `gapDetectorReportsUnknownInsteadOfAConclusionWhenMemberHerdrSocketIsConfigured` and `theUnknownEnvironmentWarnFiresOnceNotOncePerSpawn` to configure an explicit zsh `memberLoginShell`, since they test the *separate* `memberHerdrSocket` unknown-environment WARN via the (still-degrading) `worktreeRoot`/`worktreeGroup`-missing fallback, not the zsh gate — under the old code both fallbacks reached the same `logCredentialGap` call, but the zsh-gate one no longer exists. - `ClaudeCodeLauncherTest`: renamed/rewrote `allowListPolicyOnNonZshKeepsTheWarnWording` -> `allowListPolicyOnNonZshRefusesTheSpawn` (asserts the refusal through `ClaudeCodeLauncher.spawn()`, the adapter's own real spawn path — not the shared launcher method directly). ## What I did not verify Per the ticket: **I did not launch a real bash member** — that needs daemon control I don't have, and I was told not to spawn members or restart the daemon. Everything above is proven at the unit-test level through the real `spawn()` call chain (`HerdrPeerLauncher.spawn` / `ClaudeCodeLauncher.spawn`), never by calling `applyEnvironmentAllowListPolicy` directly. ## Out of scope (per the brief) — noted, not touched - The `worktreeRoot`/`worktreeGroup`-missing degrade path under `memberHerdrSocket` has the same "control silently does nothing" *shape* in a narrower sense — it still degrades and spawns rather than refusing — but the ticket explicitly scoped this to the shell case and called moving the scrub off the shell (or hardening this other path) out of scope. Left alone; flagging in case the lead wants a follow-up ticket. - `wiki/11-Features.md` is not updated — this is a behaviour change to an existing config key's semantics (`memberCredentials.policy: allow-list`), not a new visible knob, and workers should not edit `wiki/` (submodule, separate remote, often stale per team memory). Flagging for the lead to decide whether it needs a wiki entry or an `9-Implementation.md` note. ## Hard constraints followed - No credential values printed; no `${VAR:-x}` expansions used. - No `ps`/`pgrep -fl` argv dumps. - `${SHARED_ENV}/tools/secrets.sh` untouched. - No daemon restart, no member spawn.
agent added 1 commit 2026-09-03 08:15:08 +02:00
fleetd #155: refuse a member spawn under memberCredentials.policy=allow-list on a non-zsh shell
CI / contract (pull_request) Successful in 1m1s
Details
CI / build (pull_request) Successful in 1m56s
Details
bd2774b5f1 
The ZDOTDIR scrub that enforces policy=allow-list only runs on zsh. The daemon
already detected a non-zsh login shell (isZshShell/warnNonZsh, from #213), but
degraded to the weaker CB-596 overlay and spawned anyway — the exact "control
silently does nothing" defect this ticket is about. Now a non-zsh shell under
policy=allow-list refuses the spawn (IllegalArgumentException, naming the
shell), surfaced by FleetMcp.spawn's existing catch(IllegalArgumentException).
policy=deny-by-default is unaffected in substance (its overlay never depended
on the shell) but now also logs a one-time WARN naming the shell, since the
stronger allow-list control is unavailable there.

The worktreeRoot/worktreeGroup-missing degrade path under memberHerdrSocket
is untouched — that gap is fleetd #213's scope, not this one.
ltms commented 2026-09-03 08:40:10 +02:00
Owner
Copy Link
Copy Source

Merged into main as 38dec72. Closing manually — the merge went in from the worktree, not through the Gitea merge button, so this did not auto-close.

Adjudication is on #155. The part a reviewer should know: because this change can refuse spawns, I checked it against the live fleetd.yaml, which no worker can see. memberHerdrSocket is not set, so the shell comes from fleetd's own $SHELL rather than the unset memberLoginShell, and that shell is zsh — proven by behaviour (the ZDOTDIR scrub has generated a directory 147 times) rather than by reading the process environment. So the new refusal cannot fire here.

Had memberHerdrSocket been set, the unset memberLoginShell would have read as <unset>, counted as non-zsh, and refused every spawn. Worth knowing before anyone sets that key.

Good call on tracing the catch chain to FleetMcp.spawn rather than assuming the exception surfaces — and on leaving the worktreeRoot/worktreeGroup path alone as #213's territory.

Merged into `main` as `38dec72`. Closing manually — the merge went in from the worktree, not through the Gitea merge button, so this did not auto-close. Adjudication is on #155. The part a reviewer should know: because this change can **refuse spawns**, I checked it against the live `fleetd.yaml`, which no worker can see. `memberHerdrSocket` is not set, so the shell comes from fleetd's own `$SHELL` rather than the unset `memberLoginShell`, and that shell is zsh — proven by behaviour (the ZDOTDIR scrub has generated a directory 147 times) rather than by reading the process environment. So the new refusal cannot fire here. Had `memberHerdrSocket` been set, the unset `memberLoginShell` would have read as `<unset>`, counted as non-zsh, and refused every spawn. Worth knowing before anyone sets that key. Good call on tracing the catch chain to `FleetMcp.spawn` rather than assuming the exception surfaces — and on leaving the `worktreeRoot`/`worktreeGroup` path alone as #213's territory.
ltms closed this pull request 2026-09-03 08:40:17 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m1s
Details
CI / build (pull_request) Successful in 1m56s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
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
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#255
Delete Branch "worker/fleetd-155c-f8ef4b-8"

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?