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

the member credential scrub is zsh-only: a member on any other login shell gets every credential #155

New Issue
Closed
opened 2026-08-23 14:05:28 +02:00 by ltms · 1 comment
ltms commented 2026-08-23 14:05:28 +02:00
Owner
Copy Link
Copy Source

What

The memberCredentials guard works by generating a .zlogin under a per-member ZDOTDIR. That is what makes it robust: it runs after everything the operator's own chain sources, so nothing sourced later can undo it.

It is also what limits it. ZDOTDIR and .zlogin are zsh mechanisms. A member whose login shell is bash, sh, fish, or anything else never runs the scrub, and inherits the operator's full environment — which on this host includes an AWS AdministratorAccess key.

The daemon does not check what shell it is about to launch into. So the control silently does nothing rather than failing closed.

How I ran into it

Not by auditing the guard. ${SHARED_ENV}/tools/secrets.sh has a bash syntax error — a for list whose last item is missing its trailing line-continuation, so the next line is a bare ; do:

line N: syntax error near unexpected token ';'
line N: `  ; do'

zsh accepts that construct; bash rejects it. Under zsh the file loads completely. Under bash it aborts at that line, so every export after it is lost — which is how I found it, when a variable I had appended at the end of the file was simply not there.

That is an operator-file bug and I have reported it separately for the operator to fix. It matters here for two reasons:

  1. It is direct evidence that this credential machinery is only ever exercised under zsh, and that nobody would notice if it stopped working under another shell.
  2. The same divergence cuts the other way. If the file had aborted before the guard block instead of after it, the guard would not have run and the daemon would have reported nothing wrong.

Worth deciding

  • Fail closed, or fail loud? If the member's login shell is not zsh, the daemon could refuse to spawn, or spawn and warn. Refusing is safer; warning is friendlier to non-zsh backends. I lean to refusing when memberCredentials.policy is allow-list, since the operator has explicitly asked for blocking.
  • Or move the scrub off the shell entirely. The spawn boundary already builds the child's environment. Scrubbing there would be shell-independent and would not depend on a generated dotfile at all. CB-596 originally concluded "no seam exists" and that turned out to be wrong once — worth re-checking rather than assuming.

Tests

  • a member launched with a non-zsh login shell does not silently inherit a blocked variable
  • whatever the chosen behaviour is (refuse / warn), it is asserted, and asserted on the path that really launches — not on a helper the launcher happens to call

That last point matters here specifically: this is the #113 shape, and this control is exactly the kind that passes a green suite while doing nothing.

Not verified

I have not launched a bash member to confirm the credentials really do survive. The claim above is read from the mechanism (ZDOTDIR + .zlogin are zsh-only), not measured. Confirm it before fixing.

## What The `memberCredentials` guard works by generating a `.zlogin` under a per-member `ZDOTDIR`. That is what makes it robust: it runs *after* everything the operator's own chain sources, so nothing sourced later can undo it. It is also what limits it. `ZDOTDIR` and `.zlogin` are **zsh** mechanisms. A member whose login shell is bash, sh, fish, or anything else never runs the scrub, and inherits the operator's full environment — which on this host includes an AWS **AdministratorAccess** key. The daemon does not check what shell it is about to launch into. So the control silently does nothing rather than failing closed. ## How I ran into it Not by auditing the guard. `${SHARED_ENV}/tools/secrets.sh` has a **bash syntax error** — a `for` list whose last item is missing its trailing line-continuation, so the next line is a bare `; do`: ``` line N: syntax error near unexpected token ';' line N: ` ; do' ``` zsh accepts that construct; bash rejects it. Under zsh the file loads completely. Under bash it aborts at that line, so **every export after it is lost** — which is how I found it, when a variable I had appended at the end of the file was simply not there. That is an operator-file bug and I have reported it separately for the operator to fix. It matters here for two reasons: 1. It is direct evidence that this credential machinery is **only ever exercised under zsh**, and that nobody would notice if it stopped working under another shell. 2. The same divergence cuts the other way. If the file had aborted *before* the guard block instead of after it, the guard would not have run and the daemon would have reported nothing wrong. ## Worth deciding - **Fail closed, or fail loud?** If the member's login shell is not zsh, the daemon could refuse to spawn, or spawn and warn. Refusing is safer; warning is friendlier to non-zsh backends. I lean to refusing when `memberCredentials.policy` is `allow-list`, since the operator has explicitly asked for blocking. - **Or move the scrub off the shell entirely.** The spawn boundary already builds the child's environment. Scrubbing there would be shell-independent and would not depend on a generated dotfile at all. CB-596 originally concluded "no seam exists" and that turned out to be wrong once — worth re-checking rather than assuming. ## Tests - a member launched with a **non-zsh** login shell does not silently inherit a blocked variable - whatever the chosen behaviour is (refuse / warn), it is asserted, and asserted on the path that really launches — not on a helper the launcher happens to call That last point matters here specifically: this is the #113 shape, and this control is exactly the kind that passes a green suite while doing nothing. ## Not verified I have not launched a bash member to confirm the credentials really do survive. The claim above is read from the mechanism (`ZDOTDIR` + `.zlogin` are zsh-only), not measured. Confirm it before fixing.
ltms commented 2026-09-03 08:30:29 +02:00
Author
Owner
Copy Link
Copy Source

Merged to main as 38dec72. 1250 tests, 0 failures, 0 compile errors on the merged tree.

The premise was narrower than the ticket said

The ticket said the daemon does nothing about a non-zsh shell. It already detected one and logged a WARN — then degraded to the weaker CB-596 overlay and spawned anyway, under both policies. Detection existed; refusal did not. That is the real gap and it is what got closed.

Under policy=allow-list, a non-zsh login shell now throws before any ZDOTDIR or env work, naming the actual shell and offering three ways out. Under policy=deny-by-default nothing changes — that overlay is applied to the pane before any shell runs, so it does not depend on the shell, and refusing there would be a different (and wrong) decision.

Checked against the live config, because this refuses spawns

No worker can see fleetd.yaml, and a change that can refuse every spawn is not one to merge on unit tests alone:
memberHerdrSocket not set → the shell comes from fleetd's own $SHELL, not the unset memberLoginShell
memberCredentials.policy allow-list — so this path is live
fleetd's $SHELL zsh

That last one is proven by behaviour, not by reading the process environment (which is blocked here): the daemon log shows the ZDOTDIR scrub generating a directory 147 times, most recently minutes before the merge. That only happens when isZshShell() returned true.

So the new refusal cannot fire on this host.

Worth recording for whoever sets memberHerdrSocket next: had it been set, memberLoginShell is unset, so the shell would have read as <unset>, counted as non-zsh, and refused every spawn. The two keys have to be set together. The refusal message already says memberLoginShell: is only read when memberHerdrSocket is set, which is the right place for that warning.

Not verified

A live spawn under a genuinely non-zsh shell. Forcing one means changing the daemon's own environment, and the test is not worth that risk. The unit tests drive the real launcher.spawn(...) entry point rather than the private method, and the mutation (if (!zsh) → if (false)) turned the refusal test red with 0 compile errors before being reverted clean.

Noted, not fixed

The worker flagged that the worktreeRoot/worktreeGroup-missing path still degrades-and-spawns rather than refusing — the same shape as this ticket. That is #213's territory and was correctly left alone.

Merged to `main` as `38dec72`. 1250 tests, 0 failures, 0 compile errors on the merged tree. ## The premise was narrower than the ticket said The ticket said the daemon does nothing about a non-zsh shell. It already **detected** one and logged a WARN — then degraded to the weaker CB-596 overlay and spawned anyway, under both policies. Detection existed; refusal did not. That is the real gap and it is what got closed. Under `policy=allow-list`, a non-zsh login shell now throws before any ZDOTDIR or env work, naming the actual shell and offering three ways out. Under `policy=deny-by-default` nothing changes — that overlay is applied to the pane before any shell runs, so it does not depend on the shell, and refusing there would be a different (and wrong) decision. ## Checked against the live config, because this refuses spawns No worker can see `fleetd.yaml`, and a change that can refuse every spawn is not one to merge on unit tests alone: | | | |---|---| | `memberHerdrSocket` | **not set** → the shell comes from fleetd's own `$SHELL`, *not* the unset `memberLoginShell` | | `memberCredentials.policy` | `allow-list` — so this path is live | | fleetd's `$SHELL` | **zsh** | That last one is proven by behaviour, not by reading the process environment (which is blocked here): the daemon log shows the ZDOTDIR scrub generating a directory **147 times**, most recently minutes before the merge. That only happens when `isZshShell()` returned true. So the new refusal cannot fire on this host. **Worth recording for whoever sets `memberHerdrSocket` next:** had it been set, `memberLoginShell` is unset, so the shell would have read as `<unset>`, counted as non-zsh, and **refused every spawn**. The two keys have to be set together. The refusal message already says `memberLoginShell:` is only read when `memberHerdrSocket` is set, which is the right place for that warning. ## Not verified A live spawn under a genuinely non-zsh shell. Forcing one means changing the daemon's own environment, and the test is not worth that risk. The unit tests drive the real `launcher.spawn(...)` entry point rather than the private method, and the mutation (`if (!zsh)` → `if (false)`) turned the refusal test red with 0 compile errors before being reverted clean. ## Noted, not fixed The worker flagged that the `worktreeRoot`/`worktreeGroup`-missing path still degrades-and-spawns rather than refusing — the same shape as this ticket. That is #213's territory and was correctly left alone.
ltms closed this issue 2026-09-03 08:30:36 +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#155
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?