From 5f6772e869e77806c05a800210ccedb0da91b91f Mon Sep 17 00:00:00 2001
From: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
Date: Thu, 8 Feb 2024 21:36:16 +0100
Subject: [PATCH] chore(secrets): Warn if settings look like secrets but use
 invalid characters (#14706)

---
 config/secret.go      | 21 +++++++++++++++++----
 config/secret_test.go | 23 +++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/config/secret.go b/config/secret.go
index 77f18bdb1..fba8e9c5d 100644
--- a/config/secret.go
+++ b/config/secret.go
@@ -2,6 +2,7 @@ package config
 
 import (
 	"fmt"
+	"log"
 	"regexp"
 	"strings"
 	"sync/atomic"
@@ -20,10 +21,12 @@ var unlinkedSecrets = make([]*Secret, 0)
 // secretStorePattern is a regex to validate secret-store IDs
 var secretStorePattern = regexp.MustCompile(`^\w+$`)
 
-// secretPattern is a regex to extract references to secrets stored
-// in a secret-store.
+// secretPattern is a regex to extract references to secrets store in a secret-store
 var secretPattern = regexp.MustCompile(`@\{(\w+:\w+)\}`)
 
+// secretCandidatePattern is a regex to find secret candidates to warn users on invalid characters in references
+var secretCandidatePattern = regexp.MustCompile(`@\{.+?:.+?}`)
+
 // secretCount is the number of secrets use in Telegraf
 var secretCount atomic.Int64
 
@@ -125,8 +128,18 @@ func (s *Secret) init(secret []byte) {
 	// Remember if the secret is completely empty
 	s.notempty = len(secret) != 0
 
-	// Find all parts that need to be resolved and return them
-	s.unlinked = secretPattern.FindAllString(string(secret), -1)
+	// Find all secret candidates and check if they are really a valid
+	// reference. Otherwise issue a warning to let the user know that there is
+	// a potential issue with their secret instead of silently ignoring it.
+	candidates := secretCandidatePattern.FindAllString(string(secret), -1)
+	s.unlinked = make([]string, 0, len(candidates))
+	for _, c := range candidates {
+		if secretPattern.MatchString(c) {
+			s.unlinked = append(s.unlinked, c)
+		} else {
+			log.Printf("W! Secret %q contains invalid character(s), only letters, digits and underscores are allowed.", c)
+		}
+	}
 	s.resolvers = nil
 
 	// Setup the container implementation
diff --git a/config/secret_test.go b/config/secret_test.go
index b17697b9c..32c9a838e 100644
--- a/config/secret_test.go
+++ b/config/secret_test.go
@@ -1,8 +1,10 @@
 package config
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"log"
 	"testing"
 
 	"github.com/awnumar/memguard"
@@ -716,6 +718,27 @@ func (tsuite *SecretImplTestSuite) TestSecretSetResolveInvalid() {
 	require.ErrorContains(t, err, `linking new secrets failed: unlinked part "@{mock:another_secret}"`)
 }
 
+func (tsuite *SecretImplTestSuite) TestSecretInvalidWarn() {
+	t := tsuite.T()
+
+	// Intercept the log output
+	var buf bytes.Buffer
+	backup := log.Writer()
+	log.SetOutput(&buf)
+	defer log.SetOutput(backup)
+
+	cfg := []byte(`
+      [[inputs.mockup]]
+	    secret = "server=a user=@{mock:secret-with-invalid-chars} pass=@{mock:secret_pass}"
+	`)
+	c := NewConfig()
+	require.NoError(t, c.LoadConfigData(cfg))
+	require.Len(t, c.Inputs, 1)
+
+	require.Contains(t, buf.String(), `W! Secret "@{mock:secret-with-invalid-chars}" contains invalid character(s)`)
+	require.NotContains(t, buf.String(), "@{mock:secret_pass}")
+}
+
 func TestSecretImplUnprotected(t *testing.T) {
 	impl := &unprotectedSecretImpl{}
 	container := impl.Container([]byte("foobar"))