#388: scrub a pane shell that is neither login nor interactive #390

2026-09-10T02:21:46+02:00

agent commented

2026-09-10 02:21:46 +02:00

Fixes #388.

EnvAllowListScrub generated four zsh startup files but only .zshrc and .zlogin sourced the scrub. .zshenv — the one file zsh always reads — did not. A pane shell that is neither login nor interactive reads only .zshenv and stops, so it was never scrubbed at all (measured on fleet01, issue #388).

The ticket's own suggested fix (an unguarded SOURCE_SCRUB in .zshenv) is wrong, per comment 15387's own measurement: .zshenv is read by every zsh, including a short-lived zsh -c a member's own tooling forks for a single command. Those children are also neither login nor interactive, so they would scrub the environment their parent deliberately set for them (GIT_DIR, VIRTUAL_ENV, ...), and the rewritten scrub-report.txt would describe the last child to exit instead of the pane.

Fix applied (per comment 15387, measured): keep .zshrc/.zlogin exactly as they are (unconditional). Add to .zshenv a pass guarded on the exact condition that defines the gap (! -o login && ! -o interactive), plus a per-pane sentinel (_CB633_SCRUBBED) so it runs once per pane, not once per process. The sentinel is exported only after the scrub runs (so that first pass never sees or blanks it), and it is folded into the scrub's own allow-list so a later pass in the same pane cannot blank it back to empty (an exported-but-empty sentinel reads as unset and would silently re-enable scrubbing for every later child).

Also corrects the class javadoc: the old text claimed a Linux pane is "interactive but NOT login" from a bare argv[0] of /usr/bin/zsh — that only proves NOT login and says nothing about interactive. Rewrote the heading and walkthrough to describe the three-pass design after this fix.

Not in scope (per ticket): the #384 WARN wording, HerdrPeerLauncher.releaseZdotdir/readReport, which names are on the allow-list (apart from adding the sentinel), and the other host's fleetd.yaml.

Tests

Two new tests in EnvAllowListScrubTest run real /bin/zsh processes (a generated-file string match would pass on the broken version too, per the ticket):

scrubRunsInAShellThatIsNeitherLoginNorInteractive — runs a bare /bin/zsh reading a script off a non-tty stdin (no -l, no -i) with an injected decoy secret name not on any allow-list, and asserts the surviving exported names equal baseline ∩ allow-list plus ZDOTDIR and the new sentinel.
neitherShellChildKeepsParentVariablesAndReceiptStillDescribesThePane — runs the same shape as the pane, then from within that process forks a plain non-login/non-interactive child carrying a variable the "parent" deliberately set (the GIT_DIR-for-a-hook shape). Asserts: the pane blanks its decoy; the child keeps its deliberately-set variable (invariant 3); the child inherits the sentinel and does not re-scrub; and scrub-report.txt, read after the child exits, still has no trace of the child's own variable (invariant 4).

Both were reverted-and-reproduced: with the production fix removed, scrubRunsInAShellThatIsNeitherLoginNorInteractive fails with the decoy secret surviving, and neitherShellChildKeepsParentVariablesAndReceiptStillDescribesThePane fails with the sentinel never set — the other 6 pre-existing tests in the class still pass, confirming isolation.

mvn clean install in fleetd/: BUILD SUCCESS, Tests run: 1469, Failures: 0, Errors: 0, Skipped: 0 (EnvAllowListScrubTest: 8/8).

Never prints an environment variable's value — only names, counts, and the literal set/unset/present markers.

Fixes #388. `EnvAllowListScrub` generated four zsh startup files but only `.zshrc` and `.zlogin` sourced the scrub. `.zshenv` — the one file zsh always reads — did not. A pane shell that is neither login nor interactive reads only `.zshenv` and stops, so it was never scrubbed at all (measured on fleet01, issue #388). The ticket's own suggested fix (an unguarded `SOURCE_SCRUB` in `.zshenv`) is wrong, per comment 15387's own measurement: `.zshenv` is read by every zsh, including a short-lived `zsh -c` a member's own tooling forks for a single command. Those children are also neither login nor interactive, so they would scrub the environment their parent deliberately set for them (`GIT_DIR`, `VIRTUAL_ENV`, ...), and the rewritten `scrub-report.txt` would describe the last child to exit instead of the pane. **Fix applied (per comment 15387, measured):** keep `.zshrc`/`.zlogin` exactly as they are (unconditional). Add to `.zshenv` a pass guarded on the exact condition that defines the gap (`! -o login && ! -o interactive`), plus a per-pane sentinel (`_CB633_SCRUBBED`) so it runs once per pane, not once per process. The sentinel is exported only *after* the scrub runs (so that first pass never sees or blanks it), and it is folded into the scrub's own allow-list so a later pass in the same pane cannot blank it back to empty (an exported-but-empty sentinel reads as unset and would silently re-enable scrubbing for every later child). Also corrects the class javadoc: the old text claimed a Linux pane is "interactive but NOT login" from a bare `argv[0]` of `/usr/bin/zsh` — that only proves NOT login and says nothing about interactive. Rewrote the heading and walkthrough to describe the three-pass design after this fix. **Not in scope** (per ticket): the #384 WARN wording, `HerdrPeerLauncher.releaseZdotdir`/`readReport`, which names are on the allow-list (apart from adding the sentinel), and the other host's `fleetd.yaml`. ## Tests Two new tests in `EnvAllowListScrubTest` run **real** `/bin/zsh` processes (a generated-file string match would pass on the broken version too, per the ticket): - `scrubRunsInAShellThatIsNeitherLoginNorInteractive` — runs a bare `/bin/zsh` reading a script off a non-tty stdin (no `-l`, no `-i`) with an injected decoy secret name not on any allow-list, and asserts the surviving exported names equal `baseline ∩ allow-list` plus `ZDOTDIR` and the new sentinel. - `neitherShellChildKeepsParentVariablesAndReceiptStillDescribesThePane` — runs the same shape as the pane, then from *within* that process forks a plain non-login/non-interactive child carrying a variable the "parent" deliberately set (the `GIT_DIR`-for-a-hook shape). Asserts: the pane blanks its decoy; the child keeps its deliberately-set variable (invariant 3); the child inherits the sentinel and does not re-scrub; and `scrub-report.txt`, read after the child exits, still has no trace of the child's own variable (invariant 4). Both were reverted-and-reproduced: with the production fix removed, `scrubRunsInAShellThatIsNeitherLoginNorInteractive` fails with the decoy secret surviving, and `neitherShellChildKeepsParentVariablesAndReceiptStillDescribesThePane` fails with the sentinel never set — the other 6 pre-existing tests in the class still pass, confirming isolation. `mvn clean install` in `fleetd/`: **BUILD SUCCESS**, `Tests run: 1469, Failures: 0, Errors: 0, Skipped: 0` (`EnvAllowListScrubTest`: 8/8). Never prints an environment variable's value — only names, counts, and the literal `set`/`unset`/`present` markers.

agent added 1 commit 2026-09-10 02:21:46 +02:00

#388 : scrub a pane shell that is neither login nor interactive

CI / contract (pull_request) Successful in 49s

Details

CI / build (pull_request) Successful in 1m32s

Details

69e09b10fa

EnvAllowListScrub generated four zsh startup files but only .zshrc and
.zlogin sourced the scrub — .zshenv (the one file zsh always reads) did
not. A pane shell that is neither login nor interactive reads only
.zshenv and stops, so it was never scrubbed at all (measured on fleet01,
issue #388).

Adding an unguarded scrub to .zshenv (the ticket's own suggested fix) is
wrong: .zshenv is read by every zsh, including a short-lived `zsh -c`
a member's own tooling forks for a single command. Those children are
also neither login nor interactive, so they would scrub the environment
their parent deliberately set for them (GIT_DIR, VIRTUAL_ENV, ...), and
the rewritten scrub-report.txt would describe the last child to exit
instead of the pane.

Fix (per comment 15387, measured): keep .zshrc/.zlogin unconditional,
and add to .zshenv a pass guarded on the exact condition that defines
the gap (neither login nor interactive), plus a per-pane sentinel
(_CB633_SCRUBBED) so it runs once per pane, not once per process. The
sentinel is exported only after the scrub runs, and is folded into the
scrub's own allow-list so a later pass in the same pane cannot blank it
back to empty.

Also corrects the class javadoc's wrong premise (a bare argv[0] proves
NOT login, not "therefore interactive") and its now-stale two-file
walkthrough.

Tests: two new real-zsh tests in EnvAllowListScrubTest run actual
non-login/non-interactive zsh processes (never string-match the
generated files) to prove: a neither-shell pane is scrubbed; a child
that pane forks keeps variables the pane deliberately set for it; the
child does not re-scrub; and scrub-report.txt still describes the pane
after the child exits. Both fail without the production fix (verified
by reverting it and re-running: AssertionFailedError on the sentinel
and on the decoy secret surviving).

ltms closed this pull request

2026-09-10 07:16:31 +02:00

CI / contract (pull_request) Successful in 49s

Details

CI / build (pull_request) Successful in 1m32s

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#390