fix(blueprint): keep the scheduled plan unmasked after PATCH /blueprints

backend/server/services/blueprint.go

@@ -230,10 +230,12 @@ func PatchBlueprint(id uint64, body map[string]interface{}) (*models.Blueprint,
 	if err != nil {
 		return nil, err
 	}
-	if err := SanitizeBlueprint(blueprint); err != nil {
+	// the cron job registered by saveBlueprint holds this blueprint, so sanitize a copy for the response
+	sanitized := *blueprint
+	if err := SanitizeBlueprint(&sanitized); err != nil {
 		return nil, errors.Convert(err)
 	}
-	return blueprint, nil
+	return &sanitized, nil
 }
 
 // DeleteBlueprint FIXME ...

backend/server/services/blueprint_test.go

@@ -21,9 +21,72 @@ import (
 	"testing"
 
 	coreModels "github.com/apache/devlake/core/models"
+	"github.com/apache/devlake/impls/logruslog"
+	mockdal "github.com/apache/devlake/mocks/core/dal"
+	"github.com/robfig/cron/v3"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 )
 
+const (
+	testGitUrl       = "https://git:secret-token@example.com/group/repo.git"
+	testMaskedGitUrl = "https://git:************@example.com/group/repo.git"
+)
+
+func TestSanitizeBlueprintLeavesTheSourcePlanUnchanged(t *testing.T) {
+	bp := &coreModels.Blueprint{
+		Plan: coreModels.PipelinePlan{{
+			{Plugin: "gitextractor", Options: map[string]interface{}{"url": testGitUrl, "name": "group/repo"}},
+		}},
+	}
+
+	sanitized := *bp
+	require.NoError(t, SanitizeBlueprint(&sanitized))
+
+	assert.Equal(t, map[string]interface{}{"url": testMaskedGitUrl, "name": "group/repo"}, sanitized.Plan[0][0].Options)
+	assert.Equal(t, map[string]interface{}{"url": testGitUrl, "name": "group/repo"}, bp.Plan[0][0].Options)
+}
+
+func TestPatchBlueprintKeepsTheScheduledPlanUnsanitized(t *testing.T) {
+	withProjectTestValidator(t)
+	testDB := mockdal.NewDal(t)
+	withProjectTestDatabase(t, testDB)
+	previousLogger, previousCron, previousCronIds := logger, cronManager, bpCronIdMap
+	logger, cronManager, bpCronIdMap = logruslog.Global, cron.New(), map[uint64]cron.EntryID{}
+	t.Cleanup(func() {
+		logger, cronManager, bpCronIdMap = previousLogger, previousCron, previousCronIds
+	})
+
+	testDB.On("First", mock.Anything, mock.Anything).Run(func(args mock.Arguments) {
+		bp := args.Get(0).(*coreModels.Blueprint)
+		bp.ID = 9
+		bp.Name = "advanced"
+		bp.Mode = coreModels.BLUEPRINT_MODE_ADVANCED
+		bp.Enable = true
+		bp.CronConfig = "0 0 * * *"
+		bp.Plan = coreModels.PipelinePlan{{
+			{Plugin: "gitextractor", Options: map[string]interface{}{"url": testGitUrl}},
+		}}
+	}).Return(nil).Once()
+	testDB.On("Pluck", "name", mock.Anything, mock.Anything).Return(nil).Once()
+	testDB.On("All", mock.Anything, mock.Anything).Return(nil).Once()
+	testDB.On("Update", mock.Anything).Return(nil).Once()
+	testDB.On("Delete", mock.Anything, mock.Anything).Return(nil).Times(3)
+
+	patched, err := PatchBlueprint(9, map[string]interface{}{"cronConfig": "*/5 * * * *"})
+	require.NoError(t, err)
+	assert.Equal(t, testMaskedGitUrl, patched.Plan[0][0].Options["url"])
+
+	// the cron job must still run the stored plan, not the one sanitized for the response
+	entries := cronManager.Entries()
+	require.Len(t, entries, 1)
+	job, ok := entries[0].Job.(*BlueprintJob)
+	require.True(t, ok)
+	assert.Equal(t, "*/5 * * * *", job.Blueprint.CronConfig)
+	assert.Equal(t, testGitUrl, job.Blueprint.Plan[0][0].Options["url"])
+}
+
 func TestParallelizePipelineTasks(t *testing.T) {
 	plan1 := coreModels.PipelinePlan{
 		{

backend/server/services/pipeline.go

@@ -20,9 +20,11 @@ package services
 import (
 	"context"
 	"fmt"
+	"maps"
 	"net/url"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"sync"
 	"time"
@@ -44,6 +46,10 @@ import (
 
 var defaultNotificationService *DefaultPipelineNotificationService
 var globalPipelineLog = logruslog.Global.Nested("pipeline service")
+
+// pluginOptionSanitizers mask secrets in plugin options. Each one receives a
+// shallow copy of the options, so it may replace top-level values but must not
+// modify nested maps or slices in place.
 var pluginOptionSanitizers = map[string]func(map[string]interface{}){
 	"gitextractor": func(options map[string]interface{}) {
 		if v, ok := options["url"]; ok {
@@ -144,45 +150,59 @@ func CreatePipeline(newPipeline *models.NewPipeline, shouldSanitize bool) (*mode
 	return pipeline, nil
 }
 
+// SanitizeBlueprint replaces blueprint.Plan with a sanitized copy. The original
+// plan is left untouched because it may be shared, e.g. with a scheduled job.
 func SanitizeBlueprint(blueprint *models.Blueprint) error {
-	for planStageIdx, pipelineStage := range blueprint.Plan {
-		for planTaskIdx := range pipelineStage {
-			pipelineTask, err := SanitizeTask(blueprint.Plan[planStageIdx][planTaskIdx])
-			if err != nil {
-				return err
-			}
-			blueprint.Plan[planStageIdx][planTaskIdx] = pipelineTask
-		}
+	plan, err := sanitizePlan(blueprint.Plan)
+	if err != nil {
+		return err
 	}
+	blueprint.Plan = plan
 	return nil
 }
 
+// SanitizePipeline replaces pipeline.Plan with a sanitized copy.
 func SanitizePipeline(pipeline *models.Pipeline) error {
-	for planStageIdx, pipelineStage := range pipeline.Plan {
-		for planTaskIdx := range pipelineStage {
-			task := pipeline.Plan[planStageIdx][planTaskIdx]
+	plan, err := sanitizePlan(pipeline.Plan)
+	if err != nil {
+		return err
+	}
+	pipeline.Plan = plan
+	return nil
+}
+
+func sanitizePlan(plan models.PipelinePlan) (models.PipelinePlan, error) {
+	sanitized := slices.Clone(plan)
+	for planStageIdx, pipelineStage := range plan {
+		sanitized[planStageIdx] = slices.Clone(pipelineStage)
+		for planTaskIdx, task := range pipelineStage {
 			pipelineTask, err := SanitizeTask(task)
 			if err != nil {
-				return err
+				return nil, err
 			}
-			pipeline.Plan[planStageIdx][planTaskIdx] = pipelineTask
+			sanitized[planStageIdx][planTaskIdx] = pipelineTask
 		}
 	}
-	return nil
+	return sanitized, nil
 }
 
+// SanitizeTask returns a copy of the task with sanitized options.
 func SanitizeTask(pipelineTask *models.PipelineTask) (*models.PipelineTask, error) {
 	pluginName := pipelineTask.Plugin
 	options, err := SanitizePluginOption(pluginName, pipelineTask.Options)
 	if err != nil {
 		return pipelineTask, err
 	}
-	pipelineTask.Options = options
-	return pipelineTask, nil
+	sanitized := *pipelineTask
+	sanitized.Options = options
+	return &sanitized, nil
 }
 
+// SanitizePluginOption returns the sanitized options. The sanitizer works on a
+// shallow copy, so the given map is never modified.
 func SanitizePluginOption(pluginName string, option map[string]interface{}) (map[string]interface{}, error) {
 	if sanitizer, ok := pluginOptionSanitizers[pluginName]; ok {
+		option = maps.Clone(option)
 		sanitizer(option)
 	}
 	return option, nil