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

SECURITY: the credential scrub aborts silently at the first read-only parameter, leaving every later name unscrubbed #394

New Issue
Closed
opened 2026-09-10 02:57:08 +02:00 by ltms · 0 comments
ltms commented 2026-09-10 02:57:08 +02:00
Owner
Copy Link
Copy Source

The defect

EnvAllowListScrub.scrubScript() blanks non-allow-listed names in one loop:

{ for _cb633_n in "${_cb633_blank[@]}"; do export "$_cb633_n="; done; } 2>/dev/null

If any enumerated name is a zsh read-only or special parameter — UID is the one seen live — then export UID= is a fatal parameter error. It aborts the whole sourced file. Every name after it in the list is never blanked, the scrub report at the end of the file is never written, and 2>/dev/null discards the error.

So the control does not fail closed and it does not fail loud. It fails partway, silently.

Reproduced, macOS zsh 5.9

# probe.zsh
print -r -- "before"
{ for n in AAA UID BBB; do export "$n="; done; } 2>/dev/null
print -r -- "after loop"

$ zsh -c 'source ./probe.zsh; print -r -- "returned to caller"'
before
returned to caller
exit=0

$ zsh -c 'source ./probe.zsh 2>/dev/null; print -r -- "AAA=[${AAA-unset}] BBB=[${BBB-unset}]"'
before
AAA=[] BBB=[unset]

AAA is blanked. BBB is never reached. after loop never prints. Exit status is 0.

Live impact, fleet01

Measured by the fleet01 lead in a live pane: UID is name 42 of 57 in the member's environment, so 15 names after it are never considered. Their planted decoy is name 58 and survives into the member untouched — which is why that host shows a stable 8 spawns / 8 missing receipts rather than anything intermittent.

The generated .zshenv/.zshrc and the ZDOTDIR plumbing are all fine. The script aborts.

Why it has been invisible on the Mac

UID is not exported here:

$ env | cut -d= -f1 | grep -cx UID
0

426 successful receipts, 0 missing — by luck, not by design. Any host that exports UID (or any other read-only parameter) gets a partial scrub. That asymmetry is why this survived: the host where the control works is the host people look at.

Candidate fix — measured, not reasoned

Four variants, same probe, AAA UID BBB CCC:

variant result
export "$n=" AAA=[] BBB=[unset] CCC=[unset] — aborts
export "$n=" 2>/dev/null || true AAA=[] BBB=[unset] CCC=[unset] — || true does not help; it is an assignment error, not a command failure
eval "export ${n}=" 2>/dev/null LOOP COMPLETED AAA=[] BBB=[] CCC=[] — survives
[[ ${(t)n} == *readonly* ]] || export "$n=" AAA=[] BBB=[unset] CCC=[unset] — the type guard never fires

Only eval contains the error. Note the two plausible fixes that do not work — an implementer should not have to rediscover that.

An allow-list of names to skip (UID, EUID, GID, PPID, …) is the wrong shape: it is an enumeration, and this codebase has already established that a credential control must not depend on one. The loop must survive any name it cannot blank, including one nobody has thought of.

The second half of the fix: make the failure countable

The receipt currently reports allowed N of M. A name the scrub tried and failed to blank is neither allowed nor blanked, and today it is invisible in both the receipt and the log. The receipt should carry that third count, so a partial scrub can be told from a complete one without reading the member's environment.

This is the same rule the repo already applies elsewhere: a checker must print its own denominator.

Acceptance

  • The blanking loop completes even when a name cannot be blanked; every remaining name is still blanked.
  • A test plants a read-only parameter (UID) in the middle of the list and asserts that names after it are blanked. A test that only checks names before the failure point would pass today.
  • The receipt distinguishes allowed / blanked / failed-to-blank, and the WARN wording no longer claims "that member saw the full host environment" when it cannot know that.
  • Verify by removing the fix: the new test must fail.

Related

Found by the fleet01 lead (scrub.zsh:28, export UID=) after both of us had wrongly blamed the zsh startup-file set (#388) and then a spawn-ordering race, which was an artifact of a /proc/stat btime skew. #388's .zshenv change is still correct on its own terms but does not fix this — a fifth startup file does not help a script that aborts partway.

Reproduction, the four-variant comparison and the Mac exposure check: mac lead.

## The defect `EnvAllowListScrub.scrubScript()` blanks non-allow-listed names in one loop: ```zsh { for _cb633_n in "${_cb633_blank[@]}"; do export "$_cb633_n="; done; } 2>/dev/null ``` If any enumerated name is a zsh read-only or special parameter — `UID` is the one seen live — then `export UID=` is a **fatal parameter error**. It aborts the whole sourced file. Every name after it in the list is never blanked, the scrub report at the end of the file is never written, and `2>/dev/null` discards the error. So the control does not fail closed and it does not fail loud. It fails **partway, silently**. ## Reproduced, macOS zsh 5.9 ```zsh # probe.zsh print -r -- "before" { for n in AAA UID BBB; do export "$n="; done; } 2>/dev/null print -r -- "after loop" ``` ``` $ zsh -c 'source ./probe.zsh; print -r -- "returned to caller"' before returned to caller exit=0 $ zsh -c 'source ./probe.zsh 2>/dev/null; print -r -- "AAA=[${AAA-unset}] BBB=[${BBB-unset}]"' before AAA=[] BBB=[unset] ``` `AAA` is blanked. `BBB` is **never reached**. `after loop` never prints. Exit status is 0. ## Live impact, fleet01 Measured by the fleet01 lead in a live pane: `UID` is name **42 of 57** in the member's environment, so 15 names after it are never considered. Their planted decoy is name 58 and survives into the member untouched — which is why that host shows a stable 8 spawns / 8 missing receipts rather than anything intermittent. The generated `.zshenv`/`.zshrc` and the `ZDOTDIR` plumbing are all fine. The script aborts. ## Why it has been invisible on the Mac `UID` is not exported here: ``` $ env | cut -d= -f1 | grep -cx UID 0 ``` 426 successful receipts, 0 missing — by luck, not by design. Any host that exports `UID` (or any other read-only parameter) gets a partial scrub. That asymmetry is why this survived: the host where the control works is the host people look at. ## Candidate fix — measured, not reasoned Four variants, same probe, `AAA UID BBB CCC`: | variant | result | |---|---| | `export "$n="` | `AAA=[] BBB=[unset] CCC=[unset]` — aborts | | `export "$n=" 2>/dev/null \|\| true` | `AAA=[] BBB=[unset] CCC=[unset]` — **`\|\| true` does not help**; it is an assignment error, not a command failure | | `eval "export ${n}=" 2>/dev/null` | `LOOP COMPLETED AAA=[] BBB=[] CCC=[]` — **survives** | | `[[ ${(t)n} == *readonly* ]] \|\| export "$n="` | `AAA=[] BBB=[unset] CCC=[unset]` — the type guard never fires | Only `eval` contains the error. Note the two plausible fixes that do **not** work — an implementer should not have to rediscover that. An allow-list of names to skip (`UID`, `EUID`, `GID`, `PPID`, …) is the wrong shape: it is an enumeration, and this codebase has already established that a credential control must not depend on one. The loop must survive *any* name it cannot blank, including one nobody has thought of. ## The second half of the fix: make the failure countable The receipt currently reports `allowed N of M`. A name the scrub *tried and failed* to blank is neither allowed nor blanked, and today it is invisible in both the receipt and the log. The receipt should carry that third count, so a partial scrub can be told from a complete one without reading the member's environment. This is the same rule the repo already applies elsewhere: a checker must print its own denominator. ## Acceptance - The blanking loop completes even when a name cannot be blanked; every remaining name is still blanked. - A test plants a read-only parameter (`UID`) in the middle of the list and asserts that names **after** it are blanked. A test that only checks names before the failure point would pass today. - The receipt distinguishes allowed / blanked / failed-to-blank, and the `WARN` wording no longer claims "that member saw the full host environment" when it cannot know that. - Verify by removing the fix: the new test must fail. ## Related Found by the fleet01 lead (`scrub.zsh:28`, `export UID=`) after both of us had wrongly blamed the zsh startup-file set (#388) and then a spawn-ordering race, which was an artifact of a `/proc/stat` btime skew. #388's `.zshenv` change is still correct on its own terms but does **not** fix this — a fifth startup file does not help a script that aborts partway. Reproduction, the four-variant comparison and the Mac exposure check: mac lead.
ltms closed this issue 2026-09-10 03:31:31 +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#394
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?