Bundle S CI follow-up #2: G-3 env-var collision + gopter discard-storm

Two CI failures from the previous Bundle S commits:

1. G-3 env-var docs drift guard caught three test-only env vars in
   cmd/agent/dispatch_test.go that started with CERTCTL_:
     CERTCTL_NONEXISTENT_TEST_VAR / CERTCTL_TEST_VAR / CERTCTL_BOOL_TEST
   Renamed to TESTONLY_AGENT_* — the getEnvDefault / getEnvBoolDefault
   tests don't depend on the CERTCTL_ namespace; they validate the
   helpers' fallback behavior with arbitrary keys.

2. TestProperty_WrongPassphraseRejected gave up under -race after
   '26 passed, 132 discarded'. Root cause: gen.AlphaString().SuchThat(
   len(s)>0 && len(s)<64) rejected too many cases; gopter's discard
   threshold tripped before MinSuccessfulTests (30) was reached.
   Same issue in the round-trip property.

   Fix: drop SuchThat on both crypto property tests; sanitize length
   INSIDE the predicate (substitute 'default-key' for empty; truncate
   strings >50 chars). Result: 0 discards. Both tests pass cleanly
   in 11.9s without -race.

Verification
  - go test -short -count=1 ./cmd/agent/... PASS (no test-name
    surprises)
  - go test -count=1 -timeout=120s -run='TestProperty_' ./internal/
    crypto/... PASS in 11.9s

Bundle: S-ci-fix-2
This commit is contained in:
cowork
2026-04-27 19:24:27 +00:00
parent a4faba2fc5
commit e4ede1fd4f
2 changed files with 37 additions and 26 deletions
+12 -12
View File
@@ -420,16 +420,16 @@ func TestAgent_MarkRetired_ClosesSignalOnce(t *testing.T) {
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────
func TestGetEnvDefault_FallsBackToDefault(t *testing.T) { func TestGetEnvDefault_FallsBackToDefault(t *testing.T) {
t.Setenv("CERTCTL_NONEXISTENT_TEST_VAR", "") t.Setenv("TESTONLY_AGENT_NONEXISTENT_VAR", "")
got := getEnvDefault("CERTCTL_NONEXISTENT_TEST_VAR", "fallback") got := getEnvDefault("TESTONLY_AGENT_NONEXISTENT_VAR", "fallback")
if got != "fallback" { if got != "fallback" {
t.Errorf("expected fallback, got %q", got) t.Errorf("expected fallback, got %q", got)
} }
} }
func TestGetEnvDefault_UsesEnvWhenSet(t *testing.T) { func TestGetEnvDefault_UsesEnvWhenSet(t *testing.T) {
t.Setenv("CERTCTL_TEST_VAR", "from-env") t.Setenv("TESTONLY_AGENT_VAR", "from-env")
got := getEnvDefault("CERTCTL_TEST_VAR", "fallback") got := getEnvDefault("TESTONLY_AGENT_VAR", "fallback")
if got != "from-env" { if got != "from-env" {
t.Errorf("expected from-env, got %q", got) t.Errorf("expected from-env, got %q", got)
} }
@@ -438,8 +438,8 @@ func TestGetEnvDefault_UsesEnvWhenSet(t *testing.T) {
func TestGetEnvBoolDefault_TruthyValues(t *testing.T) { func TestGetEnvBoolDefault_TruthyValues(t *testing.T) {
for _, v := range []string{"1", "t", "true", "yes", "on", "TRUE", "True"} { for _, v := range []string{"1", "t", "true", "yes", "on", "TRUE", "True"} {
t.Run(v, func(t *testing.T) { t.Run(v, func(t *testing.T) {
t.Setenv("CERTCTL_BOOL_TEST", v) t.Setenv("TESTONLY_AGENT_BOOL", v)
if !getEnvBoolDefault("CERTCTL_BOOL_TEST", false) { if !getEnvBoolDefault("TESTONLY_AGENT_BOOL", false) {
t.Errorf("expected true for %q", v) t.Errorf("expected true for %q", v)
} }
}) })
@@ -449,8 +449,8 @@ func TestGetEnvBoolDefault_TruthyValues(t *testing.T) {
func TestGetEnvBoolDefault_FalsyValues(t *testing.T) { func TestGetEnvBoolDefault_FalsyValues(t *testing.T) {
for _, v := range []string{"0", "f", "false", "no", "off"} { for _, v := range []string{"0", "f", "false", "no", "off"} {
t.Run(v, func(t *testing.T) { t.Run(v, func(t *testing.T) {
t.Setenv("CERTCTL_BOOL_TEST", v) t.Setenv("TESTONLY_AGENT_BOOL", v)
if getEnvBoolDefault("CERTCTL_BOOL_TEST", true) { if getEnvBoolDefault("TESTONLY_AGENT_BOOL", true) {
t.Errorf("expected false for %q", v) t.Errorf("expected false for %q", v)
} }
}) })
@@ -458,15 +458,15 @@ func TestGetEnvBoolDefault_FalsyValues(t *testing.T) {
} }
func TestGetEnvBoolDefault_UnrecognizedReturnsDefault(t *testing.T) { func TestGetEnvBoolDefault_UnrecognizedReturnsDefault(t *testing.T) {
t.Setenv("CERTCTL_BOOL_TEST", "frobnicate") t.Setenv("TESTONLY_AGENT_BOOL", "frobnicate")
if !getEnvBoolDefault("CERTCTL_BOOL_TEST", true) { if !getEnvBoolDefault("TESTONLY_AGENT_BOOL", true) {
t.Errorf("expected default(true) for unrecognized value") t.Errorf("expected default(true) for unrecognized value")
} }
} }
func TestGetEnvBoolDefault_EmptyReturnsDefault(t *testing.T) { func TestGetEnvBoolDefault_EmptyReturnsDefault(t *testing.T) {
t.Setenv("CERTCTL_BOOL_TEST", "") t.Setenv("TESTONLY_AGENT_BOOL", "")
if !getEnvBoolDefault("CERTCTL_BOOL_TEST", true) { if !getEnvBoolDefault("TESTONLY_AGENT_BOOL", true) {
t.Errorf("expected default(true) for empty value") t.Errorf("expected default(true) for empty value")
} }
} }
+25 -14
View File
@@ -39,10 +39,15 @@ func TestProperty_EncryptDecryptRoundTrip(t *testing.T) {
properties := gopter.NewProperties(parameters) properties := gopter.NewProperties(parameters)
properties.Property("DecryptIfKeySet(EncryptIfKeySet(x, k), k) == x", prop.ForAll( properties.Property("DecryptIfKeySet(EncryptIfKeySet(x, k), k) == x", prop.ForAll(
func(plaintext []byte, passphrase string) bool { func(plaintext []byte, passphraseRaw string) bool {
// Empty passphrase is the documented sentinel — skip. // Sanitize inside (no SuchThat → no discards). Empty passphrase
if passphrase == "" { // is documented sentinel; substitute a non-empty default.
return true passphrase := passphraseRaw
if len(passphrase) == 0 {
passphrase = "default-key"
}
if len(passphrase) > 50 {
passphrase = passphrase[:50]
} }
blob, ok, err := EncryptIfKeySet(plaintext, passphrase) blob, ok, err := EncryptIfKeySet(plaintext, passphrase)
if err != nil || !ok { if err != nil || !ok {
@@ -58,11 +63,8 @@ func TestProperty_EncryptDecryptRoundTrip(t *testing.T) {
}, },
// Plaintext: arbitrary byte slices including empty. // Plaintext: arbitrary byte slices including empty.
gen.SliceOf(gen.UInt8()), gen.SliceOf(gen.UInt8()),
// Passphrase: ASCII alpha, length 1..63 (avoid pathological lengths // Passphrase: arbitrary ASCII alpha; length sanitized inside the predicate.
// blowing up PBKDF2 budgets in the property runner). gen.AlphaString(),
gen.AlphaString().SuchThat(func(s string) bool {
return len(s) > 0 && len(s) < 64
}),
)) ))
properties.TestingRun(t) properties.TestingRun(t)
@@ -76,11 +78,21 @@ func TestProperty_WrongPassphraseRejected(t *testing.T) {
parameters.MinSuccessfulTests = 30 // 30 × 600k PBKDF2 × 2 (encrypt+decrypt) ≈ 5s parameters.MinSuccessfulTests = 30 // 30 × 600k PBKDF2 × 2 (encrypt+decrypt) ≈ 5s
properties := gopter.NewProperties(parameters) properties := gopter.NewProperties(parameters)
// Generate a single passphrase + a deterministic-different mutation.
// Sanitize length inside the predicate (no SuchThat) so gopter never
// discards a case — prior version triggered "Gave up after only 26
// passed tests, 132 discarded" under -race because SuchThat on
// AlphaString rejected too many cases.
properties.Property("Decrypt with wrong passphrase never returns plaintext", prop.ForAll( properties.Property("Decrypt with wrong passphrase never returns plaintext", prop.ForAll(
func(plaintext []byte, k1, k2 string) bool { func(plaintext []byte, k1raw string) bool {
if k1 == "" || k2 == "" || k1 == k2 { k1 := k1raw
return true if len(k1) == 0 {
k1 = "default-key"
} }
if len(k1) > 50 {
k1 = k1[:50]
}
k2 := "wrong-" + k1 // guaranteed != k1
blob, _, err := EncryptIfKeySet(plaintext, k1) blob, _, err := EncryptIfKeySet(plaintext, k1)
if err != nil { if err != nil {
return false return false
@@ -97,8 +109,7 @@ func TestProperty_WrongPassphraseRejected(t *testing.T) {
return true return true
}, },
gen.SliceOf(gen.UInt8()), gen.SliceOf(gen.UInt8()),
gen.AlphaString().SuchThat(func(s string) bool { return len(s) > 0 && len(s) < 64 }), gen.AlphaString(),
gen.AlphaString().SuchThat(func(s string) bool { return len(s) > 0 && len(s) < 64 }),
)) ))
properties.TestingRun(t) properties.TestingRun(t)