From 289f530b2b794a7335f252cfea0acfa163c9d733 Mon Sep 17 00:00:00 2001
From: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Date: Thu, 15 Aug 2024 07:40:14 +0200
Subject: [PATCH] Warn if using secrets/env with plugin (#4027)

---
 docs/docs/20-usage/51-plugins/51-overview.md     | 7 ++++---
 pipeline/frontend/yaml/linter/linter.go          | 5 ++++-
 pipeline/frontend/yaml/linter/linter_test.go     | 2 +-
 pipeline/frontend/yaml/linter/schema/schema.json | 3 ---
 pipeline/frontend/yaml/types/container.go        | 3 ++-
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/docs/docs/20-usage/51-plugins/51-overview.md b/docs/docs/20-usage/51-plugins/51-overview.md
index 93a5d27f4..2ac5e2e49 100644
--- a/docs/docs/20-usage/51-plugins/51-overview.md
+++ b/docs/docs/20-usage/51-plugins/51-overview.md
@@ -50,9 +50,10 @@ steps:
 Plugins are just pipeline steps. They share the build workspace, mounted as a volume, and therefore have access to your source tree.
 While normal steps are all about arbitrary code execution, plugins should only allow the functions intended by the plugin author.
 
-So there are a few limitations, like the workspace base is always mounted at `/woodpecker`, but the working directory is dynamically adjusted accordingly. So as user of a plugin you should not have to care about this.
-
-Also instead of using environment variables the plugin should only care about one prefixed with `PLUGIN_` witch are the internal representation of the **settings** ([read more](./20-creating-plugins.md)).
+That's why there are a few limitations. The workspace base is always mounted at `/woodpecker`, but the working directory is dynamically
+adjusted accordingly, as user of a plugin you should not have to care about this. Also, you cannot use the plugin together with `commands`
+or `entrypoint` which will fail. Using `secrets` or `environment` is possible, but in this case, the plugin is internally not treated as plugin
+anymore. The container then cannot access secrets with plugin filter anymore and the containers won't be privileged without explicit definition.
 
 ## Finding Plugins
 
diff --git a/pipeline/frontend/yaml/linter/linter.go b/pipeline/frontend/yaml/linter/linter.go
index 3e99f1713..b64d7bf4c 100644
--- a/pipeline/frontend/yaml/linter/linter.go
+++ b/pipeline/frontend/yaml/linter/linter.go
@@ -143,7 +143,10 @@ func (l *Linter) lintSettings(config *WorkflowConfig, c *types.Container, field
 		return newLinterError("Cannot configure both entrypoint and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
 	}
 	if len(c.Environment) != 0 {
-		return newLinterError("Cannot configure both environment and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
+		return newLinterError("Should not configure both environment and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), true)
+	}
+	if len(c.Secrets.Secrets) != 0 {
+		return newLinterError("Should not configure both secrets and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), true)
 	}
 	return nil
 }
diff --git a/pipeline/frontend/yaml/linter/linter_test.go b/pipeline/frontend/yaml/linter/linter_test.go
index 2a4a891ec..4c9000d24 100644
--- a/pipeline/frontend/yaml/linter/linter_test.go
+++ b/pipeline/frontend/yaml/linter/linter_test.go
@@ -163,7 +163,7 @@ func TestLintErrors(t *testing.T) {
 		},
 		{
 			from: "steps: { build: { image: golang, settings: { test: 'true' }, environment: [ 'TEST=true' ] } }",
-			want: "Cannot configure both environment and settings",
+			want: "Should not configure both environment and settings",
 		},
 		{
 			from: "{pipeline: { build: { image: golang, settings: { test: 'true' } } }, when: { branch: main, event: push } }",
diff --git a/pipeline/frontend/yaml/linter/schema/schema.json b/pipeline/frontend/yaml/linter/schema/schema.json
index a46bdab21..ffe9fac74 100644
--- a/pipeline/frontend/yaml/linter/schema/schema.json
+++ b/pipeline/frontend/yaml/linter/schema/schema.json
@@ -448,9 +448,6 @@
         "directory": {
           "$ref": "#/definitions/step_directory"
         },
-        "secrets": {
-          "$ref": "#/definitions/step_secrets"
-        },
         "settings": {
           "$ref": "#/definitions/step_settings"
         },
diff --git a/pipeline/frontend/yaml/types/container.go b/pipeline/frontend/yaml/types/container.go
index 40d7385f8..edfacd5c2 100644
--- a/pipeline/frontend/yaml/types/container.go
+++ b/pipeline/frontend/yaml/types/container.go
@@ -124,7 +124,8 @@ func (c *ContainerList) UnmarshalYAML(value *yaml.Node) error {
 func (c *Container) IsPlugin() bool {
 	return len(c.Commands) == 0 &&
 		len(c.Entrypoint) == 0 &&
-		len(c.Environment) == 0
+		len(c.Environment) == 0 &&
+		len(c.Secrets.Secrets) == 0
 }
 
 func (c *Container) IsTrustedCloneImage() bool {