Remove plugin-only option from secrets (#2213)

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")),
+		Name:   strings.ToLower(c.String("name")),
-		Value:       c.String("value"),
+		Value:  c.String("value"),
-		Images:      c.StringSlice("image"),
+		Images: c.StringSlice("images"),
-		PluginsOnly: c.Bool("plugins-only"),
+		Events: c.StringSlice("events"),
-		Events:      c.StringSlice("event"),
 	}
 	if len(secret.Events) == 0 {
 		secret.Events = defaultSecretEvents

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")),
+		Name:   strings.ToLower(c.String("name")),
-		Value:       c.String("value"),
+		Value:  c.String("value"),
-		Images:      c.StringSlice("image"),
+		Images: c.StringSlice("images"),
-		PluginsOnly: c.Bool("plugins-only"),
+		Events: c.StringSlice("events"),
-		Events:      c.StringSlice("event"),
 	}
 	if strings.HasPrefix(secret.Value, "@") {
 		path := strings.TrimPrefix(secret.Value, "@")

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"
                 }

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).
+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.
-
-:::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.
-:::
 
 ## CLI Examples
 

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.

pipeline/frontend/yaml/compiler/compiler.go

@@ -41,14 +41,13 @@ type Registry struct {
 }
 
 type Secret struct {
-	Name       string
+	Name           string
-	Value      string
+	Value          string
-	Match      []string
+	AllowedPlugins []string
-	PluginOnly bool
 }
 
 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

pipeline/frontend/yaml/compiler/compiler_test.go

@@ -28,33 +28,28 @@ import (
 
 func TestSecretAvailable(t *testing.T) {
 	secret := Secret{
-		Match:      []string{"golang"},
+		AllowedPlugins: []string{},
-		PluginOnly: false,
 	}
 	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"},
+		AllowedPlugins: []string{"golang"},
-		PluginOnly: true,
 	}
 	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",
+		Image:    "golang",
-		Commands: yaml_base_types.StringOrSlice{},
+		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{},
 	}))
 }
 

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,
+			Name:           sec.Name,
-			Value:      sec.Value,
+			Value:          sec.Value,
-			Match:      sec.Images,
+			AllowedPlugins: sec.Images,
-			PluginOnly: sec.PluginsOnly,
 		})
 	}
 

server/api/global_secret.go

@@ -84,11 +84,10 @@ func PostGlobalSecret(c *gin.Context) {
 		return
 	}
 	secret := &model.Secret{
-		Name:        in.Name,
+		Name:   in.Name,
-		Value:       in.Value,
+		Value:  in.Value,
-		Events:      in.Events,
+		Events: in.Events,
-		Images:      in.Images,
+		Images: in.Images,
-		PluginsOnly: in.PluginsOnly,
 	}
 	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)

server/api/org_secret.go

@@ -107,12 +107,11 @@ func PostOrgSecret(c *gin.Context) {
 		return
 	}
 	secret := &model.Secret{
-		OrgID:       orgID,
+		OrgID:  orgID,
-		Name:        in.Name,
+		Name:   in.Name,
-		Value:       in.Value,
+		Value:  in.Value,
-		Events:      in.Events,
+		Events: in.Events,
-		Images:      in.Images,
+		Images: in.Images,
-		PluginsOnly: in.PluginsOnly,
 	}
 	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)

server/api/repo_secret.go

@@ -67,12 +67,11 @@ func PostSecret(c *gin.Context) {
 		return
 	}
 	secret := &model.Secret{
-		RepoID:      repo.ID,
+		RepoID: repo.ID,
-		Name:        strings.ToLower(in.Name),
+		Name:   strings.ToLower(in.Name),
-		Value:       in.Value,
+		Value:  in.Value,
-		Events:      in.Events,
+		Events: in.Events,
-		Images:      in.Images,
+		Images: in.Images,
-		PluginsOnly: in.PluginsOnly,
 	}
 	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)

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'"`
+	ID     int64          `json:"id"              xorm:"pk autoincr 'secret_id'"`
-	OrgID       int64          `json:"-"               xorm:"NOT NULL DEFAULT 0 UNIQUE(s) INDEX 'secret_org_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'"`
+	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'"`
+	Name   string         `json:"name"            xorm:"NOT NULL UNIQUE(s) INDEX 'secret_name'"`
-	Value       string         `json:"value,omitempty" xorm:"TEXT 'secret_value'"`
+	Value  string         `json:"value,omitempty" xorm:"TEXT 'secret_value'"`
-	Images      []string       `json:"image"           xorm:"json 'secret_images'"`
+	Images []string       `json:"images"          xorm:"json 'secret_images'"`
-	PluginsOnly bool           `json:"plugins_only"    xorm:"secret_plugins_only"`
+	Events []WebhookEvent `json:"events"          xorm:"json 'secret_events'"`
-	Events      []WebhookEvent `json:"event"           xorm:"json 'secret_events'"`
-	SkipVerify  bool           `json:"-"               xorm:"secret_skip_verify"`
-	Conceal     bool           `json:"-"               xorm:"secret_conceal"`
 } //	@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,
+		ID:     s.ID,
-		OrgID:       s.OrgID,
+		OrgID:  s.OrgID,
-		RepoID:      s.RepoID,
+		RepoID: s.RepoID,
-		Name:        s.Name,
+		Name:   s.Name,
-		Images:      s.Images,
+		Images: s.Images,
-		PluginsOnly: s.PluginsOnly,
+		Events: sortEvents(s.Events),
-		Events:      sortEvents(s.Events),
 	}
 }
 

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")
+	},
+}

server/store/datastore/migration/migration.go

@@ -57,6 +57,7 @@ var migrationTasks = []*task{
 	&addOrgID,
 	&alterTableTasksUpdateColumnTaskDataType,
 	&alterTableConfigUpdateColumnConfigDataType,
+	&removePluginOnlyOptionFromSecretsTable,
 }
 
 var allBeans = []interface{}{

web/src/components/secrets/SecretEdit.vue

@@ -16,12 +16,10 @@
 
       <InputField :label="$t(i18nPrefix + 'images.images')">
         <TextField v-model="images" :placeholder="$t(i18nPrefix + 'images.desc')" />
-
-        <Checkbox v-model="innerValue.plugins_only" class="mt-4" :label="$t(i18nPrefix + 'plugins_only')" />
       </InputField>
 
       <InputField :label="$t(i18nPrefix + 'events.events')">
-        <CheckboxesField v-model="innerValue.event" :options="secretEventsOptions" />
+        <CheckboxesField v-model="innerValue.events" :options="secretEventsOptions" />
       </InputField>
 
       <div class="flex gap-2">
@@ -71,11 +69,11 @@ const innerValue = computed({
 });
 const images = computed<string>({
   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 !== '');

web/src/components/secrets/SecretList.vue

@@ -7,7 +7,7 @@
     >
       <span>{{ secret.name }}</span>
       <div class="ml-auto space-x-2 <md:hidden">
-        <Badge v-for="event in secret.event" :key="event" :label="event" />
+        <Badge v-for="event in secret.events" :key="event" :label="event" />
       </div>
       <IconButton
         icon="edit"

web/src/lib/api/types/secret.ts

@@ -4,7 +4,7 @@ export type Secret = {
   id: string;
   name: string;
   value: string;
-  event: WebhookEvents[];
+  events: WebhookEvents[];
-  image: string[];
+  images: string[];
   plugins_only: string;
 };

woodpecker-go/woodpecker/types.go

@@ -132,12 +132,11 @@ type (
 
 	// Secret represents a secret variable, such as a password or token.
 	Secret struct {
-		ID          int64    `json:"id"`
+		ID     int64    `json:"id"`
-		Name        string   `json:"name"`
+		Name   string   `json:"name"`
-		Value       string   `json:"value,omitempty"`
+		Value  string   `json:"value,omitempty"`
-		Images      []string `json:"image"`
+		Images []string `json:"images"`
-		PluginsOnly bool     `json:"plugins_only"`
+		Events []string `json:"events"`
-		Events      []string `json:"event"`
 	}
 
 	// Activity represents an item in the user's feed or timeline.