diff --git a/cli/secret/secret_add.go b/cli/secret/secret_add.go index dce537cc7..ae634f292 100644 --- a/cli/secret/secret_add.go +++ b/cli/secret/secret_add.go @@ -42,6 +42,10 @@ var secretCreateCmd = &cli.Command{ Name: "image", Usage: "secret limited to these images", }, + &cli.BoolFlag{ + Name: "plugins-only", + Usage: "secret limited to plugins", + }, ), } @@ -52,10 +56,11 @@ func secretCreate(c *cli.Context) error { } secret := &woodpecker.Secret{ - Name: strings.ToLower(c.String("name")), - Value: c.String("value"), - Images: c.StringSlice("image"), - Events: c.StringSlice("event"), + Name: strings.ToLower(c.String("name")), + Value: c.String("value"), + Images: c.StringSlice("image"), + PluginsOnly: c.Bool("plugins-only"), + Events: c.StringSlice("event"), } if len(secret.Events) == 0 { secret.Events = defaultSecretEvents diff --git a/cli/secret/secret_set.go b/cli/secret/secret_set.go index f6ef93811..c725fdd7c 100644 --- a/cli/secret/secret_set.go +++ b/cli/secret/secret_set.go @@ -42,6 +42,10 @@ var secretUpdateCmd = &cli.Command{ Name: "image", Usage: "secret limited to these images", }, + &cli.BoolFlag{ + Name: "plugins-only", + Usage: "secret limited to plugins", + }, ), } @@ -52,10 +56,11 @@ func secretUpdate(c *cli.Context) error { } secret := &woodpecker.Secret{ - Name: strings.ToLower(c.String("name")), - Value: c.String("value"), - Images: c.StringSlice("image"), - Events: c.StringSlice("event"), + Name: strings.ToLower(c.String("name")), + Value: c.String("value"), + Images: c.StringSlice("image"), + PluginsOnly: c.Bool("plugins-only"), + Events: c.StringSlice("event"), } if strings.HasPrefix(secret.Value, "@") { path := strings.TrimPrefix(secret.Value, "@") diff --git a/docs/docs/20-usage/40-secrets.md b/docs/docs/20-usage/40-secrets.md index 4a0dfecda..0a27e84e8 100644 --- a/docs/docs/20-usage/40-secrets.md +++ b/docs/docs/20-usage/40-secrets.md @@ -79,6 +79,15 @@ woodpecker-cli secret add \ Please be careful when exposing secrets to pull requests. If your repository is open source and accepts pull requests your secrets are not safe. A bad actor can submit a malicious pull request that exposes your secrets. +## Image filter + +To prevent abusing your secrets with malicious pull requests, you can limit a secret to a list of images. They are not available to any other container. In addition, you can make the secret available only for plugins (steps without user-defined commands). + +:::warning +If you enable the option "Only available for plugins", always set an image filter too. Otherwise, the secret can be accessed by a very simple self-developed plugin and is thus *not* safe. +If you only set an image filter, you could still access the secret using the same image and by specifying a command that prints it. +::: + ## Examples Create the secret using default settings. The secret will be available to all images in your pipeline, and will be available to all push, tag, and deployment events (not pull request events). diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index 2143547c4..3299c7604 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/pipeline/frontend/yaml/compiler/compiler.go @@ -35,9 +35,14 @@ type Registry struct { } type Secret struct { - Name string - Value string - Match []string + Name string + Value string + Match []string + PluginOnly bool +} + +func (s *Secret) Available(container *yaml.Container) bool { + return (len(s.Match) == 0 || matchImage(container.Image, s.Match...)) && (!s.PluginOnly || container.IsPlugin()) } type secretMap map[string]Secret diff --git a/pipeline/frontend/yaml/compiler/compiler_test.go b/pipeline/frontend/yaml/compiler/compiler_test.go new file mode 100644 index 000000000..7524af5f0 --- /dev/null +++ b/pipeline/frontend/yaml/compiler/compiler_test.go @@ -0,0 +1,42 @@ +package compiler + +import ( + "testing" + + "github.com/docker/docker/api/types/strslice" + "github.com/stretchr/testify/assert" + "github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml" + "github.com/woodpecker-ci/woodpecker/pipeline/frontend/yaml/types" +) + +func TestSecretAvailable(t *testing.T) { + secret := Secret{ + Match: []string{"golang"}, + PluginOnly: false, + } + assert.True(t, secret.Available(&yaml.Container{ + Image: "golang", + Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + })) + assert.False(t, secret.Available(&yaml.Container{ + Image: "not-golang", + Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + })) + // secret only available for "golang" plugin + secret = Secret{ + Match: []string{"golang"}, + PluginOnly: true, + } + assert.True(t, secret.Available(&yaml.Container{ + Image: "golang", + Commands: types.Stringorslice(strslice.StrSlice{}), + })) + assert.False(t, secret.Available(&yaml.Container{ + Image: "not-golang", + Commands: types.Stringorslice(strslice.StrSlice{}), + })) + assert.False(t, secret.Available(&yaml.Container{ + Image: "not-golang", + Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + })) +} diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go index 63557f2d9..4f47e1c55 100644 --- a/pipeline/frontend/yaml/compiler/convert.go +++ b/pipeline/frontend/yaml/compiler/convert.go @@ -73,7 +73,14 @@ func (c *Compiler) createProcess(name string, container *yaml.Container, section } if !detached { - if err := settings.ParamsToEnv(container.Settings, environment, c.secrets.toStringMap()); err != nil { + pluginSecrets := secretMap{} + for name, secret := range c.secrets { + if secret.Available(container) { + pluginSecrets[name] = secret + } + } + + if err := settings.ParamsToEnv(container.Settings, environment, pluginSecrets.toStringMap()); err != nil { log.Error().Err(err).Msg("paramsToEnv") } } @@ -116,7 +123,7 @@ func (c *Compiler) createProcess(name string, container *yaml.Container, section for _, requested := range container.Secrets.Secrets { secret, ok := c.secrets[strings.ToLower(requested.Source)] - if ok && (len(secret.Match) == 0 || matchImage(container.Image, secret.Match...)) { + if ok && secret.Available(container) { environment[strings.ToUpper(requested.Target)] = secret.Value } } diff --git a/pipeline/frontend/yaml/container.go b/pipeline/frontend/yaml/container.go index 29c07711b..d5a91b2c4 100644 --- a/pipeline/frontend/yaml/container.go +++ b/pipeline/frontend/yaml/container.go @@ -111,3 +111,7 @@ func (c *Containers) UnmarshalYAML(value *yaml.Node) error { return nil } + +func (c *Container) IsPlugin() bool { + return len(c.Commands) == 0 && len(c.Command) == 0 +} diff --git a/pipeline/frontend/yaml/container_test.go b/pipeline/frontend/yaml/container_test.go index 239101b21..fceb0fe52 100644 --- a/pipeline/frontend/yaml/container_test.go +++ b/pipeline/frontend/yaml/container_test.go @@ -3,6 +3,7 @@ package yaml import ( "testing" + "github.com/docker/docker/api/types/strslice" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" @@ -301,3 +302,13 @@ func stringsToInterface(val ...string) []interface{} { } return res } + +func TestIsPlugin(t *testing.T) { + assert.True(t, (&Container{}).IsPlugin()) + assert.True(t, (&Container{ + Commands: types.Stringorslice(strslice.StrSlice{}), + }).IsPlugin()) + assert.False(t, (&Container{ + Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + }).IsPlugin()) +} diff --git a/server/api/global_secret.go b/server/api/global_secret.go index 31a3ea0c3..b1ea3b3ac 100644 --- a/server/api/global_secret.go +++ b/server/api/global_secret.go @@ -59,10 +59,11 @@ func PostGlobalSecret(c *gin.Context) { return } secret := &model.Secret{ - Name: in.Name, - Value: in.Value, - Events: in.Events, - Images: in.Images, + Name: in.Name, + Value: in.Value, + Events: in.Events, + Images: in.Images, + PluginsOnly: in.PluginsOnly, } if err := secret.Validate(); err != nil { c.String(400, "Error inserting global secret. %s", err) @@ -100,6 +101,7 @@ func PatchGlobalSecret(c *gin.Context) { if in.Images != nil { secret.Images = in.Images } + secret.PluginsOnly = in.PluginsOnly if err := secret.Validate(); err != nil { c.String(400, "Error updating global secret. %s", err) diff --git a/server/api/org_secret.go b/server/api/org_secret.go index 45164212a..aa5e262bb 100644 --- a/server/api/org_secret.go +++ b/server/api/org_secret.go @@ -65,11 +65,12 @@ func PostOrgSecret(c *gin.Context) { return } secret := &model.Secret{ - Owner: owner, - Name: in.Name, - Value: in.Value, - Events: in.Events, - Images: in.Images, + Owner: owner, + Name: in.Name, + Value: in.Value, + Events: in.Events, + Images: in.Images, + PluginsOnly: in.PluginsOnly, } if err := secret.Validate(); err != nil { c.String(400, "Error inserting org %q secret. %s", owner, err) @@ -110,6 +111,7 @@ func PatchOrgSecret(c *gin.Context) { if in.Images != nil { secret.Images = in.Images } + secret.PluginsOnly = in.PluginsOnly if err := secret.Validate(); err != nil { c.String(400, "Error updating org %q secret. %s", owner, err) diff --git a/server/api/repo_secret.go b/server/api/repo_secret.go index d9cb4e423..15a276d25 100644 --- a/server/api/repo_secret.go +++ b/server/api/repo_secret.go @@ -50,11 +50,12 @@ func PostSecret(c *gin.Context) { return } secret := &model.Secret{ - RepoID: repo.ID, - Name: strings.ToLower(in.Name), - Value: in.Value, - Events: in.Events, - Images: in.Images, + RepoID: repo.ID, + Name: strings.ToLower(in.Name), + Value: in.Value, + Events: in.Events, + Images: in.Images, + PluginsOnly: in.PluginsOnly, } if err := secret.Validate(); err != nil { c.String(400, "Error inserting secret. %s", err) @@ -95,6 +96,7 @@ func PatchSecret(c *gin.Context) { if in.Images != nil { secret.Images = in.Images } + secret.PluginsOnly = in.PluginsOnly if err := secret.Validate(); err != nil { c.String(400, "Error updating secret. %s", err) diff --git a/server/model/secret.go b/server/model/secret.go index 9cfb861e3..2d426db13 100644 --- a/server/model/secret.go +++ b/server/model/secret.go @@ -68,15 +68,16 @@ type SecretStore interface { // Secret represents a secret variable, such as a password or token. // swagger:model registry type Secret struct { - ID int64 `json:"id" xorm:"pk autoincr 'secret_id'"` - Owner string `json:"-" xorm:"NOT NULL DEFAULT '' UNIQUE(s) INDEX 'secret_owner'"` - RepoID int64 `json:"-" xorm:"NOT NULL DEFAULT 0 UNIQUE(s) INDEX 'secret_repo_id'"` - Name string `json:"name" xorm:"NOT NULL UNIQUE(s) INDEX 'secret_name'"` - Value string `json:"value,omitempty" xorm:"TEXT 'secret_value'"` - Images []string `json:"image" xorm:"json 'secret_images'"` - Events []WebhookEvent `json:"event" xorm:"json 'secret_events'"` - SkipVerify bool `json:"-" xorm:"secret_skip_verify"` - Conceal bool `json:"-" xorm:"secret_conceal"` + ID int64 `json:"id" xorm:"pk autoincr 'secret_id'"` + Owner string `json:"-" xorm:"NOT NULL DEFAULT '' UNIQUE(s) INDEX 'secret_owner'"` + RepoID int64 `json:"-" xorm:"NOT NULL DEFAULT 0 UNIQUE(s) INDEX 'secret_repo_id'"` + Name string `json:"name" xorm:"NOT NULL UNIQUE(s) INDEX 'secret_name'"` + Value string `json:"value,omitempty" xorm:"TEXT 'secret_value'"` + Images []string `json:"image" xorm:"json 'secret_images'"` + PluginsOnly bool `json:"plugins_only" xorm:"secret_plugins_only"` + Events []WebhookEvent `json:"event" xorm:"json 'secret_events'"` + SkipVerify bool `json:"-" xorm:"secret_skip_verify"` + Conceal bool `json:"-" xorm:"secret_conceal"` } // TableName return database table name for xorm @@ -152,12 +153,13 @@ func (s *Secret) Validate() error { // Copy makes a copy of the secret without the value. func (s *Secret) Copy() *Secret { return &Secret{ - ID: s.ID, - Owner: s.Owner, - RepoID: s.RepoID, - Name: s.Name, - Images: s.Images, - Events: sortEvents(s.Events), + ID: s.ID, + Owner: s.Owner, + RepoID: s.RepoID, + Name: s.Name, + Images: s.Images, + PluginsOnly: s.PluginsOnly, + Events: sortEvents(s.Events), } } diff --git a/server/shared/procBuilder.go b/server/shared/procBuilder.go index dcc49a0f1..b098b0da3 100644 --- a/server/shared/procBuilder.go +++ b/server/shared/procBuilder.go @@ -244,9 +244,10 @@ func (b *ProcBuilder) toInternalRepresentation(parsed *yaml.Config, environ map[ continue } secrets = append(secrets, compiler.Secret{ - Name: sec.Name, - Value: sec.Value, - Match: sec.Images, + Name: sec.Name, + Value: sec.Value, + Match: sec.Images, + PluginOnly: sec.PluginsOnly, }) } diff --git a/web/src/assets/locales/en.json b/web/src/assets/locales/en.json index b11aef096..ce65ab16e 100644 --- a/web/src/assets/locales/en.json +++ b/web/src/assets/locales/en.json @@ -121,6 +121,7 @@ "events": "Available at following events", "pr_warning": "Please be careful with this option as a bad actor can submit a malicious pull request that exposes your secrets." }, + "plugins_only": "Only available for plugins", "edit": "Edit secret", "delete":"Delete secret" }, @@ -258,6 +259,7 @@ "images": "Available for following images", "desc": "Comma separated list of images where this secret is available, leave empty to allow all images" }, + "plugins_only": "Only available for plugins", "events": { "events": "Available at following events", "pr_warning": "Please be careful with this option as a bad actor can submit a malicious pull request that exposes your secrets." @@ -286,6 +288,7 @@ "images": "Available for following images", "desc": "Comma separated list of images where this secret is available, leave empty to allow all images" }, + "plugins_only": "Only available for plugins", "events": { "events": "Available at following events", "pr_warning": "Please be careful with this option as a bad actor can submit a malicious pull request that exposes your secrets." diff --git a/web/src/components/secrets/SecretEdit.vue b/web/src/components/secrets/SecretEdit.vue index f344f5013..8bff336bc 100644 --- a/web/src/components/secrets/SecretEdit.vue +++ b/web/src/components/secrets/SecretEdit.vue @@ -16,6 +16,8 @@ + + diff --git a/web/src/lib/api/types/secret.ts b/web/src/lib/api/types/secret.ts index 191eee0b0..5ec6f1668 100644 --- a/web/src/lib/api/types/secret.ts +++ b/web/src/lib/api/types/secret.ts @@ -6,4 +6,5 @@ export type Secret = { value: string; event: WebhookEvents[]; image: string[]; + plugins_only: string; }; diff --git a/woodpecker-go/woodpecker/types.go b/woodpecker-go/woodpecker/types.go index f210a8f76..65054eb98 100644 --- a/woodpecker-go/woodpecker/types.go +++ b/woodpecker-go/woodpecker/types.go @@ -118,11 +118,12 @@ type ( // Secret represents a secret variable, such as a password or token. Secret struct { - ID int64 `json:"id"` - Name string `json:"name"` - Value string `json:"value,omitempty"` - Images []string `json:"image"` - Events []string `json:"event"` + ID int64 `json:"id"` + Name string `json:"name"` + Value string `json:"value,omitempty"` + Images []string `json:"image"` + PluginsOnly bool `json:"plugins_only"` + Events []string `json:"event"` } // Activity represents an item in the user's feed or timeline.