diff --git a/.woodpecker/test.yml b/.woodpecker/test.yml index bcb9f0d150..2aad364247 100644 --- a/.woodpecker/test.yml +++ b/.woodpecker/test.yml @@ -124,7 +124,7 @@ steps: codecov: pull: true - image: woodpeckerci/plugin-codecov:2.1.0 + image: woodpeckerci/plugin-codecov:2.1.2 settings: files: - agent-coverage.out diff --git a/cli/secret/secret_add.go b/cli/secret/secret_add.go index c072f44f79..2933a42cfc 100644 --- a/cli/secret/secret_add.go +++ b/cli/secret/secret_add.go @@ -46,17 +46,13 @@ var secretCreateCmd = &cli.Command{ Usage: "secret value", }, &cli.StringSliceFlag{ - Name: "event", + Name: "events", Usage: "secret limited to these events", }, &cli.StringSliceFlag{ - Name: "image", + Name: "images", Usage: "secret limited to these images", }, - &cli.BoolFlag{ - Name: "plugins-only", - Usage: "secret limited to plugins", - }, ), } @@ -67,11 +63,10 @@ func secretCreate(c *cli.Context) error { } secret := &woodpecker.Secret{ - Name: strings.ToLower(c.String("name")), - Value: c.String("value"), - Images: c.StringSlice("image"), - PluginsOnly: c.Bool("plugins-only"), - Events: c.StringSlice("event"), + Name: strings.ToLower(c.String("name")), + Value: c.String("value"), + Images: c.StringSlice("images"), + Events: c.StringSlice("events"), } if len(secret.Events) == 0 { secret.Events = defaultSecretEvents diff --git a/cli/secret/secret_set.go b/cli/secret/secret_set.go index f036aa14bb..8d72a9a676 100644 --- a/cli/secret/secret_set.go +++ b/cli/secret/secret_set.go @@ -46,17 +46,13 @@ var secretUpdateCmd = &cli.Command{ Usage: "secret value", }, &cli.StringSliceFlag{ - Name: "event", + Name: "events", Usage: "secret limited to these events", }, &cli.StringSliceFlag{ - Name: "image", + Name: "images", Usage: "secret limited to these images", }, - &cli.BoolFlag{ - Name: "plugins-only", - Usage: "secret limited to plugins", - }, ), } @@ -67,11 +63,10 @@ func secretUpdate(c *cli.Context) error { } secret := &woodpecker.Secret{ - Name: strings.ToLower(c.String("name")), - Value: c.String("value"), - Images: c.StringSlice("image"), - PluginsOnly: c.Bool("plugins-only"), - Events: c.StringSlice("event"), + Name: strings.ToLower(c.String("name")), + Value: c.String("value"), + Images: c.StringSlice("images"), + Events: c.StringSlice("events"), } if strings.HasPrefix(secret.Value, "@") { path := strings.TrimPrefix(secret.Value, "@") diff --git a/cmd/server/docs/docs.go b/cmd/server/docs/docs.go index 73a867e2c7..37e98692c1 100644 --- a/cmd/server/docs/docs.go +++ b/cmd/server/docs/docs.go @@ -4212,7 +4212,7 @@ const docTemplate = `{ "Secret": { "type": "object", "properties": { - "event": { + "events": { "type": "array", "items": { "$ref": "#/definitions/WebhookEvent" @@ -4221,7 +4221,7 @@ const docTemplate = `{ "id": { "type": "integer" }, - "image": { + "images": { "type": "array", "items": { "type": "string" @@ -4230,9 +4230,6 @@ const docTemplate = `{ "name": { "type": "string" }, - "plugins_only": { - "type": "boolean" - }, "value": { "type": "string" } diff --git a/docs/docs/20-usage/40-secrets.md b/docs/docs/20-usage/40-secrets.md index 6dc3d57441..83b9493131 100644 --- a/docs/docs/20-usage/40-secrets.md +++ b/docs/docs/20-usage/40-secrets.md @@ -89,12 +89,7 @@ Please be careful when exposing secrets to pull requests. If your repository is ## 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. -::: +To prevent abusing your secrets from malicious usage, you can limit a secret to a list of images. If enabled they are not available to any other plugin (steps without user-defined commands). If you or an attacker defines explicit commands, the secrets will not be available to the container to prevent leaking them. ## CLI Examples diff --git a/docs/docs/91-migrations.md b/docs/docs/91-migrations.md index ad0c964c95..0ef75d7600 100644 --- a/docs/docs/91-migrations.md +++ b/docs/docs/91-migrations.md @@ -8,6 +8,8 @@ Some versions need some changes to the server configuration or the pipeline conf - Dropped deprecated `pipeline:` keyword in favor of `steps:` in pipeline config - Dropped deprecated `branches:` filter in favor of global [`when.branch`](./20-usage/20-workflow-syntax.md#branch-1) filter - Deprecated `platform:` filter in favor of `labels:`, [read more](./20-usage/20-workflow-syntax.md#filter-by-platform) +- Secrets `event` property was renamed to `events` and `image` to `images` as both are lists. The new property `events` / `images` has to be used in the api and as cli argument. The old properties `event` and `image` were removed. +- The secrets `plugin_only` option was removed. Secrets with images are now always only available for plugins using listed by the `images` property. Existing secrets with a list of `images` will now only be available to the listed images if they are used as a plugin. - Removed `build` alias for `pipeline` command in CLI - Removed `ssh` backend. Use an agent directly on the SSH machine using the `local` backend. - Removed `/hook` and `/stream` API paths in favor of `/api/(hook|stream)`. You may need to use the "Repair repository" button in the repo settings or "Repair all" in the admin settings to recreate the forge hook. diff --git a/pipeline/frontend/yaml/compiler/compiler.go b/pipeline/frontend/yaml/compiler/compiler.go index fec7b7a4cc..b0524b0f3a 100644 --- a/pipeline/frontend/yaml/compiler/compiler.go +++ b/pipeline/frontend/yaml/compiler/compiler.go @@ -41,14 +41,13 @@ type Registry struct { } type Secret struct { - Name string - Value string - Match []string - PluginOnly bool + Name string + Value string + AllowedPlugins []string } func (s *Secret) Available(container *yaml_types.Container) bool { - return (len(s.Match) == 0 || utils.MatchImage(container.Image, s.Match...)) && (!s.PluginOnly || container.IsPlugin()) + return (len(s.AllowedPlugins) == 0 || utils.MatchImage(container.Image, s.AllowedPlugins...)) && (len(s.AllowedPlugins) == 0 || 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 index bfae44eef7..6664d6ece1 100644 --- a/pipeline/frontend/yaml/compiler/compiler_test.go +++ b/pipeline/frontend/yaml/compiler/compiler_test.go @@ -28,33 +28,28 @@ import ( func TestSecretAvailable(t *testing.T) { secret := Secret{ - Match: []string{"golang"}, - PluginOnly: false, + AllowedPlugins: []string{}, } assert.True(t, secret.Available(&yaml_types.Container{ Image: "golang", Commands: yaml_base_types.StringOrSlice{"echo 'this is not a plugin'"}, })) - assert.False(t, secret.Available(&yaml_types.Container{ - Image: "not-golang", - Commands: yaml_base_types.StringOrSlice{"echo 'this is not a plugin'"}, - })) + // secret only available for "golang" plugin secret = Secret{ - Match: []string{"golang"}, - PluginOnly: true, + AllowedPlugins: []string{"golang"}, } assert.True(t, secret.Available(&yaml_types.Container{ Image: "golang", Commands: yaml_base_types.StringOrSlice{}, })) assert.False(t, secret.Available(&yaml_types.Container{ - Image: "not-golang", - Commands: yaml_base_types.StringOrSlice{}, + Image: "golang", + Commands: yaml_base_types.StringOrSlice{"echo 'this is not a plugin'"}, })) assert.False(t, secret.Available(&yaml_types.Container{ Image: "not-golang", - Commands: yaml_base_types.StringOrSlice{"echo 'this is not a plugin'"}, + Commands: yaml_base_types.StringOrSlice{}, })) } diff --git a/pipeline/stepBuilder.go b/pipeline/stepBuilder.go index db978acddc..8c933b089b 100644 --- a/pipeline/stepBuilder.go +++ b/pipeline/stepBuilder.go @@ -235,10 +235,9 @@ func (b *StepBuilder) toInternalRepresentation(parsed *yaml_types.Workflow, envi continue } secrets = append(secrets, compiler.Secret{ - Name: sec.Name, - Value: sec.Value, - Match: sec.Images, - PluginOnly: sec.PluginsOnly, + Name: sec.Name, + Value: sec.Value, + AllowedPlugins: sec.Images, }) } diff --git a/server/api/global_secret.go b/server/api/global_secret.go index b51ab43ea0..6cf34d6036 100644 --- a/server/api/global_secret.go +++ b/server/api/global_secret.go @@ -84,11 +84,10 @@ func PostGlobalSecret(c *gin.Context) { return } secret := &model.Secret{ - Name: in.Name, - Value: in.Value, - Events: in.Events, - Images: in.Images, - PluginsOnly: in.PluginsOnly, + Name: in.Name, + Value: in.Value, + Events: in.Events, + Images: in.Images, } if err := secret.Validate(); err != nil { c.String(http.StatusBadRequest, "Error inserting global secret. %s", err) @@ -135,7 +134,6 @@ 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(http.StatusBadRequest, "Error updating global secret. %s", err) diff --git a/server/api/org_secret.go b/server/api/org_secret.go index ebb130c5ec..9c030f2044 100644 --- a/server/api/org_secret.go +++ b/server/api/org_secret.go @@ -107,12 +107,11 @@ func PostOrgSecret(c *gin.Context) { return } secret := &model.Secret{ - OrgID: orgID, - Name: in.Name, - Value: in.Value, - Events: in.Events, - Images: in.Images, - PluginsOnly: in.PluginsOnly, + OrgID: orgID, + Name: in.Name, + Value: in.Value, + Events: in.Events, + Images: in.Images, } if err := secret.Validate(); err != nil { c.String(http.StatusUnprocessableEntity, "Error inserting org %q secret. %s", orgID, err) @@ -165,7 +164,6 @@ 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(http.StatusUnprocessableEntity, "Error updating org %q secret. %s", orgID, err) diff --git a/server/api/repo_secret.go b/server/api/repo_secret.go index 21e1df28f8..1f88fadb3a 100644 --- a/server/api/repo_secret.go +++ b/server/api/repo_secret.go @@ -67,12 +67,11 @@ 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, - PluginsOnly: in.PluginsOnly, + RepoID: repo.ID, + Name: strings.ToLower(in.Name), + Value: in.Value, + Events: in.Events, + Images: in.Images, } if err := secret.Validate(); err != nil { c.String(http.StatusUnprocessableEntity, "Error inserting secret. %s", err) @@ -123,7 +122,6 @@ 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(http.StatusUnprocessableEntity, "Error updating secret. %s", err) diff --git a/server/model/secret.go b/server/model/secret.go index 71f3311351..886ecc3547 100644 --- a/server/model/secret.go +++ b/server/model/secret.go @@ -69,16 +69,13 @@ type SecretStore interface { // Secret represents a secret variable, such as a password or token. type Secret struct { - ID int64 `json:"id" xorm:"pk autoincr 'secret_id'"` - OrgID int64 `json:"-" xorm:"NOT NULL DEFAULT 0 UNIQUE(s) INDEX 'secret_org_id'"` - 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"` + ID int64 `json:"id" xorm:"pk autoincr 'secret_id'"` + OrgID int64 `json:"-" xorm:"NOT NULL DEFAULT 0 UNIQUE(s) INDEX 'secret_org_id'"` + 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:"images" xorm:"json 'secret_images'"` + Events []WebhookEvent `json:"events" xorm:"json 'secret_events'"` } // @name Secret // TableName return database table name for xorm @@ -154,13 +151,12 @@ 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, - OrgID: s.OrgID, - RepoID: s.RepoID, - Name: s.Name, - Images: s.Images, - PluginsOnly: s.PluginsOnly, - Events: sortEvents(s.Events), + ID: s.ID, + OrgID: s.OrgID, + RepoID: s.RepoID, + Name: s.Name, + Images: s.Images, + Events: sortEvents(s.Events), } } diff --git a/server/store/datastore/migration/025_remove_secrets_plugin_only_col.go b/server/store/datastore/migration/025_remove_secrets_plugin_only_col.go new file mode 100644 index 0000000000..5978de16de --- /dev/null +++ b/server/store/datastore/migration/025_remove_secrets_plugin_only_col.go @@ -0,0 +1,43 @@ +// Copyright 2023 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package migration + +import ( + "xorm.io/xorm" +) + +type oldSecret025 struct { + ID int64 `json:"id" xorm:"pk autoincr 'secret_id'"` + PluginsOnly bool `json:"plugins_only" xorm:"secret_plugins_only"` + SkipVerify bool `json:"-" xorm:"secret_skip_verify"` + Conceal bool `json:"-" xorm:"secret_conceal"` + Images []string `json:"images" xorm:"json 'secret_images'"` +} + +func (oldSecret025) TableName() string { + return "secrets" +} + +var removePluginOnlyOptionFromSecretsTable = task{ + name: "remove-plugin-only-option-from-secrets-table", + fn: func(sess *xorm.Session) (err error) { + // make sure plugin_only column exists + if err := sess.Sync(new(oldSecret025)); err != nil { + return err + } + + return dropTableColumns(sess, "secrets", "secret_plugins_only", "secret_skip_verify", "secret_conceal") + }, +} diff --git a/server/store/datastore/migration/migration.go b/server/store/datastore/migration/migration.go index 72e1335428..ca35f73cad 100644 --- a/server/store/datastore/migration/migration.go +++ b/server/store/datastore/migration/migration.go @@ -57,6 +57,7 @@ var migrationTasks = []*task{ &addOrgID, &alterTableTasksUpdateColumnTaskDataType, &alterTableConfigUpdateColumnConfigDataType, + &removePluginOnlyOptionFromSecretsTable, } var allBeans = []interface{}{ diff --git a/web/src/components/secrets/SecretEdit.vue b/web/src/components/secrets/SecretEdit.vue index da7a1d6cda..0130f07b2e 100644 --- a/web/src/components/secrets/SecretEdit.vue +++ b/web/src/components/secrets/SecretEdit.vue @@ -16,12 +16,10 @@ - - - +
@@ -71,11 +69,11 @@ const innerValue = computed({ }); const images = computed({ get() { - return innerValue.value?.image?.join(',') || ''; + return innerValue.value?.images?.join(',') || ''; }, set(value) { if (innerValue.value) { - innerValue.value.image = value + innerValue.value.images = value .split(',') .map((s) => s.trim()) .filter((s) => s !== ''); diff --git a/web/src/components/secrets/SecretList.vue b/web/src/components/secrets/SecretList.vue index 8033f2b6b1..e43f2bbca9 100644 --- a/web/src/components/secrets/SecretList.vue +++ b/web/src/components/secrets/SecretList.vue @@ -7,7 +7,7 @@ > {{ secret.name }}
- +