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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>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}.
|
||||
*
|
||||
* <p>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<String> 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<String, String> 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<String> 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 {
|
||||
|
||||
Reference in New Issue
Block a user