From 6aedcf97bec2651704393d8ca884c38ac729e83b Mon Sep 17 00:00:00 2001
From: Victor Lyuboslavsky <victor.lyuboslavsky@gmail.com>
Date: Fri, 9 Feb 2024 13:31:34 -0600
Subject: [PATCH] Obfuscate enroll secret in error (#16684)

When attempting to set an enroll secret which already exists in DB,
error message no longer contains the secret in cleartext.
#16621

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
---
 changes/16621-obfuscate-enroll-secret      | 1 +
 server/datastore/mysql/app_configs.go      | 4 ++++
 server/datastore/mysql/app_configs_test.go | 6 +++++-
 3 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 changes/16621-obfuscate-enroll-secret

diff --git a/changes/16621-obfuscate-enroll-secret b/changes/16621-obfuscate-enroll-secret
new file mode 100644
index 0000000000..accfff76f3
--- /dev/null
+++ b/changes/16621-obfuscate-enroll-secret
@@ -0,0 +1 @@
+When attempting to set an enroll secret which already exists in DB, error message no longer contains the secret in cleartext.
diff --git a/server/datastore/mysql/app_configs.go b/server/datastore/mysql/app_configs.go
index 854a56bf7d..75fe840e17 100644
--- a/server/datastore/mysql/app_configs.go
+++ b/server/datastore/mysql/app_configs.go
@@ -147,6 +147,10 @@ func applyEnrollSecretsDB(ctx context.Context, q sqlx.ExtContext, teamID *uint,
 			args = append(args, s.Secret, teamID, secretCreatedAt)
 		}
 		if _, err := q.ExecContext(ctx, sql, args...); err != nil {
+			if isDuplicate(err) {
+				// Obfuscate the secret in the error message
+				err = alreadyExists("secret", fleet.MaskedPassword)
+			}
 			return ctxerr.Wrap(ctx, err, "insert secrets")
 		}
 	}
diff --git a/server/datastore/mysql/app_configs_test.go b/server/datastore/mysql/app_configs_test.go
index 5ed18ee030..f86313f448 100644
--- a/server/datastore/mysql/app_configs_test.go
+++ b/server/datastore/mysql/app_configs_test.go
@@ -3,7 +3,9 @@ package mysql
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 
@@ -319,8 +321,9 @@ func testAppConfigEnrollSecretUniqueness(t *testing.T, ds *Datastore) {
 	team1, err := ds.NewTeam(context.Background(), &fleet.Team{Name: "team1"})
 	require.NoError(t, err)
 
+	const secret = "one_secret"
 	expectedSecrets := []*fleet.EnrollSecret{
-		{Secret: "one_secret"},
+		{Secret: secret},
 	}
 	err = ds.ApplyEnrollSecrets(context.Background(), &team1.ID, expectedSecrets)
 	require.NoError(t, err)
@@ -328,6 +331,7 @@ func testAppConfigEnrollSecretUniqueness(t *testing.T, ds *Datastore) {
 	// Same secret at global level should not be allowed
 	err = ds.ApplyEnrollSecrets(context.Background(), nil, expectedSecrets)
 	require.Error(t, err)
+	assert.False(t, strings.Contains(err.Error(), secret), fmt.Sprintf("error should not contain secret in plaintext: %s", err.Error()))
 }
 
 func testAppConfigDefaults(t *testing.T, ds *Datastore) {