From 0d07a0f0566626525b90202d9bc9f52aab6b90e9 Mon Sep 17 00:00:00 2001 From: ltms Date: Thu, 10 Sep 2026 01:02:34 +0000 Subject: [PATCH] scrub: stop `export UID=` aborting the allow-list scrub mid-loop CB-633's generated scrub.zsh blanks every non-allow-listed exported name in one loop wrapped in `{ ... } 2>/dev/null`. In zsh, `export UID=` is not a failed command: it is a fatal parameter error ("failed to change user ID: operation not permitted") that aborts the whole sourced file. The loop therefore stops at UID, every later name is left unscrubbed, and the report-writing block never runs -- silently, because of the 2>/dev/null. The inversion is the severity. `env` lists inherited names first and the names a startup file exports last, so the loop blanked the harmless inherited half and died immediately before the operator's own exports -- exactly the credentials the policy exists to remove. Measured on a fleet01 member pane: UID is name 42 of 57, and a decoy exported from ~/.zshrc sat at 58, non-empty, on 8 of 8 spawns. xtrace ends at `+scrub.zsh:28> export 'UID='` with rc=126. Guarding the call site does not help -- `if ! export "$n=" 2>/dev/null` still aborts; measured. The only repair is to never attempt these names. Also fatal: EUID, GID, EGID, PPID, LINENO. Not fatal: USERNAME, SHLVL, PWD, OLDPWD, SECONDS, HISTSIZE. Why the suite stayed green: EnvAllowListScrubTest starts zsh from a CLEARED parent (`pb.environment().clear()`), where UID is not an exported name at all, so the abort cannot reproduce there. The new test supplies the production shape -- UID present and exported -- and asserts a report exists, which is written by the last statement in the file and so proves the script ran to completion. Verified by mutation: removing the skip line fails exactly that test and no other. This is orthogonal to #388. Adding the scrub to .zshenv adds a fifth call site to a script that still aborts at the same name. --- .../ltms/fleet/member/EnvAllowListScrub.java | 16 ++++++++ .../fleet/member/EnvAllowListScrubTest.java | 38 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java index d56105b..7b328c8 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java @@ -331,6 +331,22 @@ public final class EnvAllowListScrub { (( _cb633_total += 1 )) [[ -n "${_cb633_allowed[$_cb633_n]-}" ]] && continue case "$_cb633_n" in %s) continue ;; esac + # zsh SPECIAL PARAMETERS. `export UID=` is not a failed command: it is a fatal + # parameter error ("failed to change user ID: operation not permitted") that + # aborts this entire sourced file mid-loop. Everything after it in `env` order + # is then left unscrubbed and the report below is never written — silently, + # because the blanking loop is wrapped in `2>/dev/null`. Guarding the call site + # does NOT help; the abort happens however the assignment is wrapped. The only + # repair is to never attempt these names. + # + # Why this was invisible: `env` lists inherited names first and the names a + # startup file exports last, so the loop blanked the harmless inherited half and + # died immediately before the operator's own exports — exactly the credentials + # this policy exists to remove. Measured on a fleet01 member pane: UID is name + # 42 of 57, and a decoy exported from ~/.zshrc sat at 58, untouched on 8 of 8 + # spawns. EnvAllowListScrubTest starts zsh from a CLEARED parent environment, + # where UID is not exported at all, so the whole suite was green throughout. + case "$_cb633_n" in UID|EUID|GID|EGID|PPID|LINENO) continue ;; esac _cb633_blank+=("$_cb633_n") done diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java index 6d16bba..55a44b3 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java @@ -18,6 +18,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -118,6 +119,43 @@ class EnvAllowListScrubTest { "allowed N of M with N <= M — the denominator is always reported"); } + /** + * A pane inherits {@code UID}; a cleared test parent does not. The scrub must survive it. + * + *

Every other test here starts zsh from a CLEARED environment, so {@code UID} is never an + * exported name and never reaches the blanking loop. In a real member pane it is exported and + * it IS reached — and {@code export UID=} is a fatal zsh parameter error that aborts the whole + * sourced file, leaving every later name unscrubbed and writing no report at all. The abort is + * silent: the loop is wrapped in {@code 2>/dev/null}. + * + *

The assertion is deliberately "a report exists" rather than "the canary is blanked". The + * report is written by the last statement in the file, so its presence proves the script ran + * to completion; the canary alone would depend on where it happens to sit in {@code env} order. + * Both are checked, but only the first one fails deterministically without the fix. + */ + @Test + void scrubSurvivesAnInheritedUidTheWayARealPaneHasIt(@TempDir Path tmp) throws Exception { + assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here"); + Set allowed = MemberEnvAllowList.derive(List.of()); + Path zdotdir = EnvAllowListScrub.generate(tmp, allowed); + + // The production shape: UID present and exported, as every pane shell inherits it. + Map paneLikeParent = Map.of( + "HOME", System.getProperty("user.home"), + "PATH", "/usr/bin:/bin", + "SHELL", "/bin/zsh", + "UID", "1000", + "CB633_CANARY", "must-not-survive-the-scrub"); + Set survivors = exportedNamesFromCleanParent(paneLikeParent, zdotdir); + + EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(zdotdir); + assertNotNull(report, + "an inherited UID must not abort the scrub — no report means the file died mid-loop " + + "and every name after UID in `env` order was left unscrubbed"); + assertFalse(survivors.contains("CB633_CANARY"), + "a non-allow-listed name must still be blanked when UID is in the environment"); + } + /** A group-shared ZDOTDIR still lets the member truncate and write its pre-created receipt. */ @Test void groupSharedScrubWritesAndReadsItsReport(@TempDir Path tmp) throws Exception {