From 195f7a0965203bbd1ea472276344ff538f303ace Mon Sep 17 00:00:00 2001 From: Pablo Ovelleiro Corral Date: Mon, 21 Aug 2023 10:36:11 +0200 Subject: [PATCH 1/8] Pass netrc to external config service request --- server/api/pipeline.go | 7 ++++++- server/forge/configFetcher.go | 11 +++++++++-- server/pipeline/restart.go | 4 ++-- server/plugins/config/extension.go | 2 +- server/plugins/config/http.go | 11 +++++++++-- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/server/api/pipeline.go b/server/api/pipeline.go index 68477b0730..5a64d2db1d 100644 --- a/server/api/pipeline.go +++ b/server/api/pipeline.go @@ -453,7 +453,12 @@ func PostPipeline(c *gin.Context) { } } - newpipeline, err := pipeline.Restart(c, _store, pl, user, repo, envs) + netrc, err := server.Config.Services.Forge.Netrc(user, repo) + if err != nil { + handlePipelineErr(c, err) + } + + newpipeline, err := pipeline.Restart(c, _store, pl, user, repo, envs, netrc) if err != nil { handlePipelineErr(c, err) } else { diff --git a/server/forge/configFetcher.go b/server/forge/configFetcher.go index 042934e210..f886f9cc94 100644 --- a/server/forge/configFetcher.go +++ b/server/forge/configFetcher.go @@ -74,7 +74,14 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er defer cancel() // ok here as we only try http fetching once, returning on fail and success log.Trace().Msgf("ConfigFetch[%s]: getting config from external http service", cf.repo.FullName) - newConfigs, useOld, err := cf.configExtension.FetchConfig(fetchCtx, cf.repo, cf.pipeline, files) + netrc, err := cf.forge.Netrc(cf.user, cf.repo) + + if err != nil { + log.Error().Msg("Failed to get netrc " + err.Error()) + return nil, fmt.Errorf("On Fetching config via http : %w", err) + } + + newConfigs, useOld, err := cf.configExtension.FetchConfig(fetchCtx, cf.repo, cf.pipeline, files, netrc) if err != nil { log.Error().Msg("Got error " + err.Error()) return nil, fmt.Errorf("On Fetching config via http : %w", err) @@ -109,7 +116,7 @@ func (cf *configFetcher) fetch(c context.Context, timeout time.Duration, config return nil, fmt.Errorf("user defined config '%s' not found: %w", config, err) } - log.Trace().Msgf("ConfigFetch[%s]: user did not defined own config, following default procedure", cf.repo.FullName) + log.Trace().Msgf("ConfigFetch[%s]: user did not define own config, following default procedure", cf.repo.FullName) // for the order see shared/constants/constants.go fileMeta, err := cf.getFirstAvailableConfig(ctx, constant.DefaultConfigOrder[:], false) if err == nil { diff --git a/server/pipeline/restart.go b/server/pipeline/restart.go index 88af3b377e..573503f346 100644 --- a/server/pipeline/restart.go +++ b/server/pipeline/restart.go @@ -30,7 +30,7 @@ import ( ) // Restart a pipeline by creating a new one out of the old and start it -func Restart(ctx context.Context, store store.Store, lastPipeline *model.Pipeline, user *model.User, repo *model.Repo, envs map[string]string) (*model.Pipeline, error) { +func Restart(ctx context.Context, store store.Store, lastPipeline *model.Pipeline, user *model.User, repo *model.Repo, envs map[string]string, netrc *model.Netrc) (*model.Pipeline, error) { switch lastPipeline.Status { case model.StatusDeclined, model.StatusBlocked: @@ -58,7 +58,7 @@ func Restart(ctx context.Context, store store.Store, lastPipeline *model.Pipelin currentFileMeta[i] = &forge_types.FileMeta{Name: cfg.Name, Data: cfg.Data} } - newConfig, useOld, err := server.Config.Services.ConfigService.FetchConfig(ctx, repo, lastPipeline, currentFileMeta) + newConfig, useOld, err := server.Config.Services.ConfigService.FetchConfig(ctx, repo, lastPipeline, currentFileMeta, netrc) if err != nil { return nil, &ErrBadRequest{ Msg: fmt.Sprintf("On fetching external pipeline config: %s", err), diff --git a/server/plugins/config/extension.go b/server/plugins/config/extension.go index 317d58fcb0..dd94dc36ef 100644 --- a/server/plugins/config/extension.go +++ b/server/plugins/config/extension.go @@ -23,5 +23,5 @@ import ( type Extension interface { IsConfigured() bool - FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta) (configData []*forge_types.FileMeta, useOld bool, err error) + FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta, netrc *model.Netrc) (configData []*forge_types.FileMeta, useOld bool, err error) } diff --git a/server/plugins/config/http.go b/server/plugins/config/http.go index c61b06e667..b0af81d33f 100644 --- a/server/plugins/config/http.go +++ b/server/plugins/config/http.go @@ -39,6 +39,7 @@ type requestStructure struct { Repo *model.Repo `json:"repo"` Pipeline *model.Pipeline `json:"pipeline"` Configuration []*config `json:"configs"` + Netrc *model.Netrc `json:"netrc"` } type responseStructure struct { @@ -53,14 +54,20 @@ func (cp *http) IsConfigured() bool { return cp.endpoint != "" } -func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta) (configData []*forge_types.FileMeta, useOld bool, err error) { +func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *model.Pipeline, currentFileMeta []*forge_types.FileMeta, netrc *model.Netrc) (configData []*forge_types.FileMeta, useOld bool, err error) { currentConfigs := make([]*config, len(currentFileMeta)) for i, pipe := range currentFileMeta { currentConfigs[i] = &config{Name: pipe.Name, Data: string(pipe.Data)} } response := new(responseStructure) - body := requestStructure{Repo: repo, Pipeline: pipeline, Configuration: currentConfigs} + body := requestStructure{ + Repo: repo, + Pipeline: pipeline, + Configuration: currentConfigs, + Netrc: netrc, + } + status, err := utils.Send(ctx, "POST", cp.endpoint, cp.privateKey, body, response) if err != nil && status != 204 { return nil, false, fmt.Errorf("Failed to fetch config via http (%d) %w", status, err) From 110fb8e7037a4f967f728e3bb3961df3f89803bb Mon Sep 17 00:00:00 2001 From: Anbraten Date: Mon, 21 Aug 2023 10:43:35 +0200 Subject: [PATCH 2/8] Update server/forge/configFetcher.go --- server/forge/configFetcher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/forge/configFetcher.go b/server/forge/configFetcher.go index f886f9cc94..535886745f 100644 --- a/server/forge/configFetcher.go +++ b/server/forge/configFetcher.go @@ -83,8 +83,8 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er newConfigs, useOld, err := cf.configExtension.FetchConfig(fetchCtx, cf.repo, cf.pipeline, files, netrc) if err != nil { - log.Error().Msg("Got error " + err.Error()) - return nil, fmt.Errorf("On Fetching config via http : %w", err) + log.Error().Err(err).Msg("could not fetch config via http") + return nil, fmt.Errorf("could not fetch config via http: %w", err) } if !useOld { From f4118c5687020edf17448f9a831cf79bce145b17 Mon Sep 17 00:00:00 2001 From: Anbraten Date: Mon, 21 Aug 2023 10:45:06 +0200 Subject: [PATCH 3/8] Update server/forge/configFetcher.go --- server/forge/configFetcher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/forge/configFetcher.go b/server/forge/configFetcher.go index 535886745f..25be1d87b8 100644 --- a/server/forge/configFetcher.go +++ b/server/forge/configFetcher.go @@ -77,8 +77,8 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er netrc, err := cf.forge.Netrc(cf.user, cf.repo) if err != nil { - log.Error().Msg("Failed to get netrc " + err.Error()) - return nil, fmt.Errorf("On Fetching config via http : %w", err) + log.Error().Err(err).Msg("could not get Netrc data from forge") + return nil, fmt.Errorf("could not get Netrc data from forge: %w", err) } newConfigs, useOld, err := cf.configExtension.FetchConfig(fetchCtx, cf.repo, cf.pipeline, files, netrc) From 2eba0d6a964eeb6570fbe8467716367891ec1154 Mon Sep 17 00:00:00 2001 From: Anbraten Date: Mon, 21 Aug 2023 10:45:42 +0200 Subject: [PATCH 4/8] Apply suggestions from code review --- server/forge/configFetcher.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/forge/configFetcher.go b/server/forge/configFetcher.go index 25be1d87b8..d4024d059c 100644 --- a/server/forge/configFetcher.go +++ b/server/forge/configFetcher.go @@ -75,9 +75,7 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er log.Trace().Msgf("ConfigFetch[%s]: getting config from external http service", cf.repo.FullName) netrc, err := cf.forge.Netrc(cf.user, cf.repo) - if err != nil { - log.Error().Err(err).Msg("could not get Netrc data from forge") return nil, fmt.Errorf("could not get Netrc data from forge: %w", err) } From 44b456e54746e841d681b0b14d1f1cb5bd58078a Mon Sep 17 00:00:00 2001 From: Pablo Ovelleiro Corral Date: Mon, 21 Aug 2023 10:56:08 +0200 Subject: [PATCH 5/8] Run gofmt -s so liter is happy --- server/plugins/config/http.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/plugins/config/http.go b/server/plugins/config/http.go index b0af81d33f..b58c245a61 100644 --- a/server/plugins/config/http.go +++ b/server/plugins/config/http.go @@ -62,10 +62,10 @@ func (cp *http) FetchConfig(ctx context.Context, repo *model.Repo, pipeline *mod response := new(responseStructure) body := requestStructure{ - Repo: repo, - Pipeline: pipeline, + Repo: repo, + Pipeline: pipeline, Configuration: currentConfigs, - Netrc: netrc, + Netrc: netrc, } status, err := utils.Send(ctx, "POST", cp.endpoint, cp.privateKey, body, response) From ffc402a9f7781a11059482065892e7da3720d97e Mon Sep 17 00:00:00 2001 From: Pablo Ovelleiro Corral Date: Mon, 21 Aug 2023 11:36:41 +0200 Subject: [PATCH 6/8] Add mock for Netrc in test --- server/forge/configFetcher_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/forge/configFetcher_test.go b/server/forge/configFetcher_test.go index d3f8818966..35a4f90996 100644 --- a/server/forge/configFetcher_test.go +++ b/server/forge/configFetcher_test.go @@ -519,6 +519,8 @@ func TestFetchFromConfigService(t *testing.T) { f.On("File", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, fmt.Errorf("File not found")) f.On("Dir", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, fmt.Errorf("Directory not found")) + f.On("Netrc", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("Failed to generate netrc")) + configFetcher := forge.NewConfigFetcher( f, time.Second*3, From bbe8216eb4d9263f79be611194b953923428970c Mon Sep 17 00:00:00 2001 From: Pablo Ovelleiro Corral Date: Mon, 21 Aug 2023 11:58:50 +0200 Subject: [PATCH 7/8] Document netrc being passed to ext. config service --- docs/docs/30-administration/100-external-configuration-api.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/docs/30-administration/100-external-configuration-api.md b/docs/docs/30-administration/100-external-configuration-api.md index 8f703b271a..7970ff46e5 100644 --- a/docs/docs/30-administration/100-external-configuration-api.md +++ b/docs/docs/30-administration/100-external-configuration-api.md @@ -7,6 +7,10 @@ Every request sent by Woodpecker is signed using a [http-signature](https://data A simplistic example configuration service can be found here: [https://github.com/woodpecker-ci/example-config-service](https://github.com/woodpecker-ci/example-config-service) +:::warning +You need to trust the external config service as it is getting secret information about the repository and pipeline and has the ability to change pipeline configs that could run malicious tasks. +::: + ## Config ```shell From f437eaf758e180920cdad1b6d31dd4c6ee663c22 Mon Sep 17 00:00:00 2001 From: Pablo Ovelleiro Corral Date: Mon, 21 Aug 2023 13:06:05 +0200 Subject: [PATCH 8/8] Pass fake netrc in test --- server/forge/configFetcher_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/forge/configFetcher_test.go b/server/forge/configFetcher_test.go index 35a4f90996..2a414be455 100644 --- a/server/forge/configFetcher_test.go +++ b/server/forge/configFetcher_test.go @@ -519,7 +519,7 @@ func TestFetchFromConfigService(t *testing.T) { f.On("File", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, fmt.Errorf("File not found")) f.On("Dir", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, fmt.Errorf("Directory not found")) - f.On("Netrc", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("Failed to generate netrc")) + f.On("Netrc", mock.Anything, mock.Anything).Return(&model.Netrc{Machine: "mock", Login: "mock", Password: "mock"}, nil) configFetcher := forge.NewConfigFetcher( f,