This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 56826a576 fix(blueprint): keep the scheduled plan unmasked after PATCH
/blueprints (#9247)
56826a576 is described below
commit 56826a576d4fdb5ebb694ec09c51e36ddeaad21c
Author: Yashvardhan Thanvi <[email protected]>
AuthorDate: Mon Oct 26 21:11:58 2026 +0530
fix(blueprint): keep the scheduled plan unmasked after PATCH /blueprints
(#9247)
PatchBlueprint sanitized the blueprint it had just handed to the cron
job, and the gitextractor sanitizer rewrote the options map in place.
The next scheduled pipeline then cloned with a masked URL until the
server restarted. Normal-mode blueprints with credentials in beforePlan
or afterPlan were hit too, since those tasks are shared with the
generated plan.
Sanitizing now works on copies: SanitizePluginOption masks a clone of
the options, SanitizeTask returns a new task, and SanitizeBlueprint and
SanitizePipeline build a new plan. PatchBlueprint sanitizes a copy of
the saved blueprint for its response, so the plan it returns is still
masked.
Closes #9243
Signed-off-by: Yashvardhan Thanvi <[email protected]>
---
backend/server/services/blueprint.go | 6 ++-
backend/server/services/blueprint_test.go | 63 +++++++++++++++++++++++++++++++
backend/server/services/pipeline.go | 52 +++++++++++++++++--------
3 files changed, 103 insertions(+), 18 deletions(-)
diff --git a/backend/server/services/blueprint.go
b/backend/server/services/blueprint.go
index 6acc95849..39d93189e 100644
--- a/backend/server/services/blueprint.go
+++ b/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 ...
diff --git a/backend/server/services/blueprint_test.go
b/backend/server/services/blueprint_test.go
index d5f80ae0f..218ad336c 100644
--- a/backend/server/services/blueprint_test.go
+++ b/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:[email protected]/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{
{
diff --git a/backend/server/services/pipeline.go
b/backend/server/services/pipeline.go
index bc7b6f9ce..059d72c2d 100644
--- a/backend/server/services/pipeline.go
+++ b/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
