From f7ec21e50ee86332f093ed8c35944e9c117fc872 Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Mon, 27 Apr 2026 19:24:27 +0000 Subject: [PATCH] Bundle S CI follow-up #2: G-3 env-var collision + gopter discard-storm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- cmd/agent/dispatch_test.go | 24 ++++++------- internal/crypto/encryption_property_test.go | 39 +++++++++++++-------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/cmd/agent/dispatch_test.go b/cmd/agent/dispatch_test.go index 336e3db..4446772 100644 --- a/cmd/agent/dispatch_test.go +++ b/cmd/agent/dispatch_test.go @@ -420,16 +420,16 @@ func TestAgent_MarkRetired_ClosesSignalOnce(t *testing.T) { // ───────────────────────────────────────────────────────────────────────────── func TestGetEnvDefault_FallsBackToDefault(t *testing.T) { - t.Setenv("CERTCTL_NONEXISTENT_TEST_VAR", "") - got := getEnvDefault("CERTCTL_NONEXISTENT_TEST_VAR", "fallback") + t.Setenv("TESTONLY_AGENT_NONEXISTENT_VAR", "") + got := getEnvDefault("TESTONLY_AGENT_NONEXISTENT_VAR", "fallback") if got != "fallback" { t.Errorf("expected fallback, got %q", got) } } func TestGetEnvDefault_UsesEnvWhenSet(t *testing.T) { - t.Setenv("CERTCTL_TEST_VAR", "from-env") - got := getEnvDefault("CERTCTL_TEST_VAR", "fallback") + t.Setenv("TESTONLY_AGENT_VAR", "from-env") + got := getEnvDefault("TESTONLY_AGENT_VAR", "fallback") if got != "from-env" { t.Errorf("expected from-env, got %q", got) } @@ -438,8 +438,8 @@ func TestGetEnvDefault_UsesEnvWhenSet(t *testing.T) { func TestGetEnvBoolDefault_TruthyValues(t *testing.T) { for _, v := range []string{"1", "t", "true", "yes", "on", "TRUE", "True"} { t.Run(v, func(t *testing.T) { - t.Setenv("CERTCTL_BOOL_TEST", v) - if !getEnvBoolDefault("CERTCTL_BOOL_TEST", false) { + t.Setenv("TESTONLY_AGENT_BOOL", v) + if !getEnvBoolDefault("TESTONLY_AGENT_BOOL", false) { t.Errorf("expected true for %q", v) } }) @@ -449,8 +449,8 @@ func TestGetEnvBoolDefault_TruthyValues(t *testing.T) { func TestGetEnvBoolDefault_FalsyValues(t *testing.T) { for _, v := range []string{"0", "f", "false", "no", "off"} { t.Run(v, func(t *testing.T) { - t.Setenv("CERTCTL_BOOL_TEST", v) - if getEnvBoolDefault("CERTCTL_BOOL_TEST", true) { + t.Setenv("TESTONLY_AGENT_BOOL", v) + if getEnvBoolDefault("TESTONLY_AGENT_BOOL", true) { t.Errorf("expected false for %q", v) } }) @@ -458,15 +458,15 @@ func TestGetEnvBoolDefault_FalsyValues(t *testing.T) { } func TestGetEnvBoolDefault_UnrecognizedReturnsDefault(t *testing.T) { - t.Setenv("CERTCTL_BOOL_TEST", "frobnicate") - if !getEnvBoolDefault("CERTCTL_BOOL_TEST", true) { + t.Setenv("TESTONLY_AGENT_BOOL", "frobnicate") + if !getEnvBoolDefault("TESTONLY_AGENT_BOOL", true) { t.Errorf("expected default(true) for unrecognized value") } } func TestGetEnvBoolDefault_EmptyReturnsDefault(t *testing.T) { - t.Setenv("CERTCTL_BOOL_TEST", "") - if !getEnvBoolDefault("CERTCTL_BOOL_TEST", true) { + t.Setenv("TESTONLY_AGENT_BOOL", "") + if !getEnvBoolDefault("TESTONLY_AGENT_BOOL", true) { t.Errorf("expected default(true) for empty value") } } diff --git a/internal/crypto/encryption_property_test.go b/internal/crypto/encryption_property_test.go index 1ca5207..b332566 100644 --- a/internal/crypto/encryption_property_test.go +++ b/internal/crypto/encryption_property_test.go @@ -39,10 +39,15 @@ func TestProperty_EncryptDecryptRoundTrip(t *testing.T) { properties := gopter.NewProperties(parameters) properties.Property("DecryptIfKeySet(EncryptIfKeySet(x, k), k) == x", prop.ForAll( - func(plaintext []byte, passphrase string) bool { - // Empty passphrase is the documented sentinel — skip. - if passphrase == "" { - return true + func(plaintext []byte, passphraseRaw string) bool { + // Sanitize inside (no SuchThat → no discards). Empty passphrase + // is documented sentinel; substitute a non-empty default. + passphrase := passphraseRaw + if len(passphrase) == 0 { + passphrase = "default-key" + } + if len(passphrase) > 50 { + passphrase = passphrase[:50] } blob, ok, err := EncryptIfKeySet(plaintext, passphrase) if err != nil || !ok { @@ -58,11 +63,8 @@ func TestProperty_EncryptDecryptRoundTrip(t *testing.T) { }, // Plaintext: arbitrary byte slices including empty. gen.SliceOf(gen.UInt8()), - // Passphrase: ASCII alpha, length 1..63 (avoid pathological lengths - // blowing up PBKDF2 budgets in the property runner). - gen.AlphaString().SuchThat(func(s string) bool { - return len(s) > 0 && len(s) < 64 - }), + // Passphrase: arbitrary ASCII alpha; length sanitized inside the predicate. + gen.AlphaString(), )) properties.TestingRun(t) @@ -76,11 +78,21 @@ func TestProperty_WrongPassphraseRejected(t *testing.T) { parameters.MinSuccessfulTests = 30 // 30 × 600k PBKDF2 × 2 (encrypt+decrypt) ≈ 5s 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( - func(plaintext []byte, k1, k2 string) bool { - if k1 == "" || k2 == "" || k1 == k2 { - return true + func(plaintext []byte, k1raw string) bool { + k1 := k1raw + if len(k1) == 0 { + k1 = "default-key" } + if len(k1) > 50 { + k1 = k1[:50] + } + k2 := "wrong-" + k1 // guaranteed != k1 blob, _, err := EncryptIfKeySet(plaintext, k1) if err != nil { return false @@ -97,8 +109,7 @@ func TestProperty_WrongPassphraseRejected(t *testing.T) { return true }, gen.SliceOf(gen.UInt8()), - gen.AlphaString().SuchThat(func(s string) bool { return len(s) > 0 && len(s) < 64 }), - gen.AlphaString().SuchThat(func(s string) bool { return len(s) > 0 && len(s) < 64 }), + gen.AlphaString(), )) properties.TestingRun(t)