From 4e8f31474bb57961abb141360cdcdc2f5e232ee2 Mon Sep 17 00:00:00 2001 From: dborup Date: Mon, 5 Oct 2026 11:47:18 +0000 Subject: [PATCH] test(ingestor): use non-numeric secrets in the #118 credential-leak tests (#250) TestStatsFileHasNoCredentials_118 searched the whole stats-file JSON for the test secrets "1234", "2024" and "abc". A counter, timestamp or size that contained "1234" failed it although nothing leaked (seen on #246). Name the password and query parts (pw2024pw, zq1234zq, qxabcxq) and use them in the broker URLs, error texts and secretParts118, and for the token in the r3 tests that share that list. Coverage is unchanged: the user, the password cut at '/', '?' and '#', the query/fragment part and the token are still checked. Co-Authored-By: Claude Opus 5.5 --- cmd/ingestor/mqtt_credentials_r3_118_test.go | 10 +++---- .../mqtt_status_credentials_118_test.go | 26 +++++++++++++------ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/cmd/ingestor/mqtt_credentials_r3_118_test.go b/cmd/ingestor/mqtt_credentials_r3_118_test.go index 5ff184598..974a00245 100644 --- a/cmd/ingestor/mqtt_credentials_r3_118_test.go +++ b/cmd/ingestor/mqtt_credentials_r3_118_test.go @@ -42,13 +42,13 @@ func TestBrokerForLogMarksRemovedUserinfo_118(t *testing.T) { // scheme, or the configured user name and password, would pass it. func TestErrForLogMasksKnownSecrets_118(t *testing.T) { src := MQTTSource{ - Broker: "wss://" + credUser + ":" + credPass + "@host/mqtt?token=abc", + Broker: "wss://" + credUser + ":" + credPass + "@host/mqtt?token=" + credQuery, Username: "cfg-user", Password: "cfg-pass", } secrets := mqttSourceSecrets(src) for _, c := range []struct{ in, want string }{ - {"dial host/mqtt?token=abc: refused", "dial host/mqtt?****: refused"}, + {"dial host/mqtt?token=" + credQuery + ": refused", "dial host/mqtt?****: refused"}, {"not authorized: cfg-user/cfg-pass", "not authorized: ****/****"}, {"auth " + credUser + ":" + credPass + " rejected", "auth **** rejected"}, {"connect tcp://x:y@host failed", "connect tcp://****@host failed"}, // MaskText still runs @@ -78,15 +78,15 @@ func TestDisconnectErrorMasksKnownSecrets_118(t *testing.T) { resetSourceStatusRegistry() t.Cleanup(resetSourceStatusRegistry) buf := captureLog118(t) - src := MQTTSource{Name: "feed", Broker: "wss://host/mqtt?token=abc", Password: "cfg-pass"} + src := MQTTSource{Name: "feed", Broker: "wss://host/mqtt?token=" + credQuery, Password: "cfg-pass"} setup := prepareMQTTSource(src, "feed") opts, status := setup.opts, setup.status - opts.OnConnectionLost(nil, errors.New("dial host/mqtt?token=abc: cfg-pass rejected")) + opts.OnConnectionLost(nil, errors.New("dial host/mqtt?token="+credQuery+": cfg-pass rejected")) const want = "dial host/mqtt?****: **** rejected" if got := status.snapshot(time.Now()).LastError; got != want { t.Errorf("lastError = %q, want %q", got, want) } - if out := buf.String(); !strings.Contains(out, want) || strings.Contains(out, "abc") || strings.Contains(out, "cfg-pass") { + if out := buf.String(); !strings.Contains(out, want) || strings.Contains(out, credQuery) || strings.Contains(out, "cfg-pass") { t.Errorf("log:\n%s", out) } } diff --git a/cmd/ingestor/mqtt_status_credentials_118_test.go b/cmd/ingestor/mqtt_status_credentials_118_test.go index 5c94f35f5..788e29dc1 100644 --- a/cmd/ingestor/mqtt_status_credentials_118_test.go +++ b/cmd/ingestor/mqtt_status_credentials_118_test.go @@ -17,8 +17,18 @@ import ( // status registry, the stats file (which feeds the public /api/mqtt/status // and /api/healthz) or the client ID, whatever form the URL takes. +// Password and query parts of the brokers below. They are not plain +// numbers or short words, so a timestamp, counter or size in a checked +// string cannot contain one by chance (#250: a stats-file number that +// contained "1234" failed TestStatsFileHasNoCredentials_118). +const ( + credPassHead = "pw2024pw" // a password cut at '/' + credPassCut = "zq1234zq" // a password cut at '?' or '#' + credQuery = "qxabcxq" // the rest after '?' or '#', and a token +) + // secretParts118 are the credential parts used in the brokers below. -var secretParts118 = []string{credUser, credPass, "tok3n", "2024", "1234", "abc", "p%zz"} +var secretParts118 = []string{credUser, credPass, "tok3n", credPassHead, credPassCut, credQuery, "p%zz"} func assertNoSecretParts118(t *testing.T, what, s string) { t.Helper() @@ -33,12 +43,12 @@ func assertNoSecretParts118(t *testing.T, what, s string) { // url.Parse, so the password (or its first part) used to be logged. func TestBrokerForLogAmbiguousPassword_118(t *testing.T) { for _, c := range []struct{ in, want string }{ - {"tcp://" + credUser + ":2024/" + credPass + "@host:1883", "tcp://****@host:1883"}, - {"tcp://" + credUser + ":1234?abc@host", "tcp://****@host"}, - {"tcp://" + credUser + ":1234#abc@host", "tcp://****@host"}, + {"tcp://" + credUser + ":" + credPassHead + "/" + credPass + "@host:1883", "tcp://****@host:1883"}, + {"tcp://" + credUser + ":" + credPassCut + "?" + credQuery + "@host", "tcp://****@host"}, + {"tcp://" + credUser + ":" + credPassCut + "#" + credQuery + "@host", "tcp://****@host"}, {credUser + ":" + credPass + "@host:1883", "tcp://****@host:1883"}, {"tcp://tok3n@host", "tcp://****@host"}, - {"wss://host/mqtt?token=abc", "wss://host/mqtt"}, + {"wss://host/mqtt?token=" + credQuery, "wss://host/mqtt"}, {"tcp://" + credUser + ":p%zz@host", "tcp://****@host"}, } { got := brokerForLog(c.in) @@ -52,7 +62,7 @@ func TestBrokerForLogAmbiguousPassword_118(t *testing.T) { // The generated client ID is logged ("as client …"), so its host part must // come from the stripped broker, not from url.Parse's idea of the host. func TestMQTTClientIDBaseHasNoCredentials_118(t *testing.T) { - id := mqttClientID(MQTTSource{Broker: "tcp://" + credUser + ":2024/" + credPass + "@host:1883"}) + id := mqttClientID(MQTTSource{Broker: "tcp://" + credUser + ":" + credPassHead + "/" + credPass + "@host:1883"}) if !regexp.MustCompile(`^corescope-host-[0-9a-f]{8}$`).MatchString(id) { t.Fatalf("client id %q", id) } @@ -103,8 +113,8 @@ func TestMQTTSourceTagsAreUnique_118(t *testing.T) { func TestSourceStatusHoldsNoCredentials_118(t *testing.T) { resetSourceStatusRegistry() t.Cleanup(resetSourceStatusRegistry) - s := RegisterSourceStatus("t", "tcp://"+credUser+":1234?abc@host") - s.MarkDisconnect(time.Now(), errors.New(`dial "wss://`+credUser+`:`+credPass+`@host/mqtt?token=abc": refused`)) + s := RegisterSourceStatus("t", "tcp://"+credUser+":"+credPassCut+"?"+credQuery+"@host") + s.MarkDisconnect(time.Now(), errors.New(`dial "wss://`+credUser+`:`+credPass+`@host/mqtt?token=`+credQuery+`": refused`)) snap := s.snapshot(time.Now()) b, _ := json.Marshal(snap) assertNoSecretParts118(t, "status snapshot", string(b))