CB-192: fix false credential-gap WARN under allow-list+zsh, split its log guard #194

Merged
ltms merged 2 commits from worker/cb-192-gap-log-11b631-2 into main 2026-08-31 04:30:49 +02:00
Member

Fixes gitea #192 (salvaged from closed PRs #174 and #191).

Defect 1 — the WARN wording was false on the allow-list+zsh path

HerdrPeerLauncher.logCredentialGap(creds) always emitted the WARN wording ("every member pane inherits them UNBLOCKED"), even under memberCredentials.policy: allow-list on a zsh login shell, where the generated ZDOTDIR scrub genuinely blanks the name. The line reported the control working as though it were a hole.

Fix: logCredentialGap now takes an effectiveAllowed set. null keeps the WARN (deny-by-default, and the allow-list non-zsh fallback, where nothing is ever scrubbed). The derived allow-list set — passed only from the zsh branch of applyEnvironmentAllowListPolicy, reachable only after that method's own zsh gate (the same gate logAllowListCoverage already sits behind) — selects a new INFO wording that says the scrub will blank the name instead of claiming it is inherited unblocked. The wording is keyed on whether that set was actually computed, never on creds.isAllowList() alone (the trap PR #174 fell into).

Defect 2 — one AtomicBoolean could suppress the report that matters

memberCredentials is a live, re-read-per-spawn supplier, so the policy can change between two spawns on one launcher. A single credentialGapLogged flag meant a harmless allow-list INFO on spawn 1 could permanently suppress a genuine deny-by-default WARN on a later spawn after a config reload.

Fix: split into unprotectedGapLogged (WARN branch) and allowListGapLogged (INFO branch) — one guard per report kind.

Tests

Added to ClaudeCodeLauncherTest (which exercises the real baseEnv() path — HerdrPeerLauncherAllowListWiringTest's fixture overrides buildLaunch and bypasses applyMemberCredentialPolicy entirely, so it cannot prove defect 2):

  • denyByDefaultKeepsTheExactCredentialGapWarn — pins the deny-by-default WARN text byte-for-byte.
  • allowListPolicyOnZshReportsTheGapWithoutClaimingItIsUnblocked — allow-list+zsh gets the INFO wording, never "UNBLOCKED".
  • allowListPolicyOnNonZshKeepsTheWarnWording — allow-list+non-zsh (SHELL=/bin/bash) keeps the WARN, never claims a scrub.
  • secondSpawnStillWarnsAfterPolicyChangesFromAllowListToDenyByDefault — two spawns on one launcher (allow-list+zsh, then a live-supplier reload to deny-by-default) — the second WARN still fires.

Each fix was proved by reverting it locally and watching its new test fail, then restoring it (see PR description discussion / ticket #192 for the failure text quoted back to the lead).

Build

cd fleetd && mvn clean install — Tests run: 1016, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Did not touch fleetd.yaml and did not redeploy the daemon, per the ticket's instructions.

Fixes gitea #192 (salvaged from closed PRs #174 and #191). ## Defect 1 — the WARN wording was false on the allow-list+zsh path `HerdrPeerLauncher.logCredentialGap(creds)` always emitted the WARN wording ("every member pane inherits them UNBLOCKED"), even under `memberCredentials.policy: allow-list` on a zsh login shell, where the generated ZDOTDIR scrub genuinely blanks the name. The line reported the control working as though it were a hole. Fix: `logCredentialGap` now takes an `effectiveAllowed` set. `null` keeps the WARN (deny-by-default, and the allow-list non-zsh fallback, where nothing is ever scrubbed). The derived allow-list set — passed only from the zsh branch of `applyEnvironmentAllowListPolicy`, reachable only after that method's own zsh gate (the same gate `logAllowListCoverage` already sits behind) — selects a new INFO wording that says the scrub will blank the name instead of claiming it is inherited unblocked. The wording is keyed on whether that set was actually computed, never on `creds.isAllowList()` alone (the trap PR #174 fell into). ## Defect 2 — one AtomicBoolean could suppress the report that matters `memberCredentials` is a live, re-read-per-spawn supplier, so the policy can change between two spawns on one launcher. A single `credentialGapLogged` flag meant a harmless allow-list INFO on spawn 1 could permanently suppress a genuine deny-by-default WARN on a later spawn after a config reload. Fix: split into `unprotectedGapLogged` (WARN branch) and `allowListGapLogged` (INFO branch) — one guard per report kind. ## Tests Added to `ClaudeCodeLauncherTest` (which exercises the real `baseEnv()` path — `HerdrPeerLauncherAllowListWiringTest`'s fixture overrides `buildLaunch` and bypasses `applyMemberCredentialPolicy` entirely, so it cannot prove defect 2): - `denyByDefaultKeepsTheExactCredentialGapWarn` — pins the deny-by-default WARN text byte-for-byte. - `allowListPolicyOnZshReportsTheGapWithoutClaimingItIsUnblocked` — allow-list+zsh gets the INFO wording, never "UNBLOCKED". - `allowListPolicyOnNonZshKeepsTheWarnWording` — allow-list+non-zsh (SHELL=/bin/bash) keeps the WARN, never claims a scrub. - `secondSpawnStillWarnsAfterPolicyChangesFromAllowListToDenyByDefault` — two spawns on one launcher (allow-list+zsh, then a live-supplier reload to deny-by-default) — the second WARN still fires. Each fix was proved by reverting it locally and watching its new test fail, then restoring it (see PR description discussion / ticket #192 for the failure text quoted back to the lead). ## Build `cd fleetd && mvn clean install` — `Tests run: 1016, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. Did not touch `fleetd.yaml` and did not redeploy the daemon, per the ticket's instructions.
agent added 1 commit 2026-08-31 04:17:52 +02:00
CB-192: fix false credential-gap WARN under allow-list+zsh, split its log guard
CI / contract (pull_request) Successful in 1m5s
CI / build (pull_request) Successful in 1m7s
d89ae94a2e
logCredentialGap(creds) always emitted the WARN wording ("every member pane
inherits them UNBLOCKED"), even under memberCredentials.policy: allow-list on
a zsh login shell, where the generated ZDOTDIR scrub genuinely blanks the
name. The line reported the control working as though it were a hole.

Pass an effectiveAllowed set instead: null keeps the WARN (deny-by-default,
and the allow-list non-zsh fallback, where nothing is ever scrubbed); the
derived allow-list set (only reachable after applyEnvironmentAllowListPolicy's
own zsh gate) selects a new INFO wording that says the scrub will blank the
name instead of claiming it is inherited unblocked.

Also split the single credentialGapLogged AtomicBoolean into two guards
(unprotectedGapLogged / allowListGapLogged) — one per report kind. Since
memberCredentials is a live, re-read-per-spawn supplier, a shared flag let a
harmless allow-list INFO on one spawn permanently suppress a later spawn's
real deny-by-default WARN after a policy reload.

Fixes gitea #192.
agent added 1 commit 2026-08-31 04:28:16 +02:00
CB-192 review fix: split the allow-list gap by what the scrub actually keeps
CI / build (pull_request) Successful in 1m5s
CI / contract (pull_request) Successful in 1m20s
615af4ed0a
Lead review on PR #194 found that the allow-list INFO wording claimed the
whole gap ("credential-shaped names on neither known: nor allow:") is blanked
by the scrub, without checking that against effectiveAllowed. effectiveAllowed
is a SUPERSET of known+allow — MemberEnvAllowList.derive also unions in every
profile's gitTokenEnv/gitHostEnv/tokenEnv/env: keys, and derivedAllowedNames
further unions in the spawn's own env keys — so a gap name can still be kept
by the derived list (e.g. a profile's tokenEnv names it) and reach the member
unblocked while the INFO said "no member pane keeps them". That inversion is
exactly what #192 exists to remove.

logCredentialGap now splits the gap with MemberEnvAllowList.keeps (the same
predicate the generated scrub itself evaluates, so this cannot drift from
what the scrub does): names it keeps get a WARN, guarded by the same
unprotectedGapLogged flag as the deny-by-default case (same severity — a name
reaching a member unprotected is equally serious either way); names it
blanks keep the existing INFO, guarded by allowListGapLogged. The
deny-by-default WARN text and the non-zsh fallback are untouched.

Added allowListWarnsWhenTheDerivedAllowListKeepsAnUncoveredName and
allowListSplitsAMixedGapBetweenTheWarnAndTheInfo to ClaudeCodeLauncherTest.
ltms merged commit 2823349c8e into main 2026-08-31 04:30:49 +02:00
Sign in to join this conversation.