Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow to set arbitrary environments for plugins #3909

Merged
merged 10 commits into from
Jul 14, 2024
2 changes: 1 addition & 1 deletion cmd/server/woodpecker_docs_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,5 @@ func toOpenApi3(input, output string) error {
return err
}

return os.WriteFile(output, data, 0644)
return os.WriteFile(output, data, 0o644)
}
20 changes: 11 additions & 9 deletions pipeline/frontend/yaml/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (l *Linter) lintContainers(config *WorkflowConfig, area string) error {
linterErr = multierr.Append(linterErr, err)
}
}
if err := l.lintCommands(config, container, area); err != nil {
if err := l.lintSettings(config, container, area); err != nil {
linterErr = multierr.Append(linterErr, err)
}
}
Expand All @@ -132,16 +132,18 @@ func (l *Linter) lintImage(config *WorkflowConfig, c *types.Container, area stri
return nil
}

func (l *Linter) lintCommands(config *WorkflowConfig, c *types.Container, field string) error {
if len(c.Commands) == 0 {
func (l *Linter) lintSettings(config *WorkflowConfig, c *types.Container, field string) error {
if len(c.Settings) == 0 {
return nil
}
if len(c.Settings) != 0 {
var keys []string
for key := range c.Settings {
keys = append(keys, key)
}
return newLinterError(fmt.Sprintf("Cannot configure both commands and custom attributes %v", keys), config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
if len(c.Commands) != 0 {
return newLinterError("Cannot configure both commands and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
}
if len(c.Entrypoint) != 0 {
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 nil
}
Expand Down
12 changes: 12 additions & 0 deletions pipeline/frontend/yaml/linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,18 @@ func TestLintErrors(t *testing.T) {
from: "steps: { build: { image: golang, network_mode: 'container:name' } }",
want: "Insufficient privileges to use network_mode",
},
{
from: "steps: { build: { image: golang, settings: { test: 'true' }, commands: [ 'echo ja', 'echo nein' ] } }",
want: "Cannot configure both commands and settings",
},
{
from: "steps: { build: { image: golang, settings: { test: 'true' }, entrypoint: [ '/bin/fish' ] } }",
want: "Cannot configure both entrypoint and settings",
},
{
from: "steps: { build: { image: golang, settings: { test: 'true' }, environment: [ 'TEST=true' ] } }",
want: "Cannot configure both environment and settings",
},
}

for _, test := range testdata {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
steps:
publish:
image: plugins/docker
settings:
repo: foo/bar
tags: latest
environment:
CGO: 0
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
steps:
publish:
image: plugins/docker
settings:
repo: foo/bar
tags: latest
commands:
- env
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ steps:
image: alpine
entrypoint: ['some_entry', '--some-flag']

singla-entrypoint:
single-entrypoint:
image: alpine
entrypoint: some_entry

Expand Down
171 changes: 168 additions & 3 deletions pipeline/frontend/yaml/linter/schema/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,24 @@
}
},
"step": {
"description": "An step of your workflow executes either arbitrary commands or uses an plugin. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#steps",
6543 marked this conversation as resolved.
Show resolved Hide resolved
"oneOf": [
{
"$ref": "#/definitions/commands_step"
},
{
"$ref": "#/definitions/entrypoint_step"
},
{
"$ref": "#/definitions/plugin_step"
}
]
},
"commands_step": {
"description": "Every step of your pipeline executes arbitrary commands inside a specified docker container. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#steps",
"type": "object",
"additionalProperties": false,
"required": ["image"],
"required": ["image", "commands"],
"properties": {
"name": {
"description": "The name of the step. Can be used if using the array style steps list.",
Expand All @@ -334,8 +348,91 @@
"secrets": {
"$ref": "#/definitions/step_secrets"
},
"settings": {
"$ref": "#/definitions/step_settings"
"when": {
"$ref": "#/definitions/step_when"
},
"volumes": {
"$ref": "#/definitions/step_volumes"
},
"group": {
"description": "deprecated, use depends_on",
"type": "string"
},
"depends_on": {
"description": "Execute a step after another step has finished.",
"oneOf": [
{
"type": "array",
"minLength": 1,
"items": {
"type": "string"
}
},
{
"type": "string"
}
]
},
"detach": {
"description": "Detach a step to run in background until pipeline finishes. Read more: https://woodpecker-ci.org/docs/usage/services#detachment",
"type": "boolean"
},
"failure": {
"description": "How to handle the failure of this step. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#failure",
"type": "string",
"enum": ["fail", "ignore"],
"default": "fail"
},
"backend_options": {
"$ref": "#/definitions/step_backend_options"
},
"entrypoint": {
"description": "Defines container entrypoint.",
"oneOf": [
{
"type": "array",
"minLength": 1,
"items": {
"type": "string"
}
},
{
"type": "string"
}
]
}
}
},
"entrypoint_step": {
"description": "Every step of your pipeline executes arbitrary commands inside a specified docker container. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#steps",
"type": "object",
"additionalProperties": false,
"required": ["image", "entrypoint"],
"properties": {
"name": {
"description": "The name of the step. Can be used if using the array style steps list.",
"type": "string"
},
"image": {
"$ref": "#/definitions/step_image"
},
"privileged": {
"$ref": "#/definitions/step_privileged"
},
"pull": {
"$ref": "#/definitions/step_pull"
},
"commands": {
"$ref": "#/definitions/step_commands"
},
"environment": {
"$ref": "#/definitions/step_environment"
},
"directory": {
"$ref": "#/definitions/step_directory"
},
"secrets": {
"$ref": "#/definitions/step_secrets"
},
"when": {
"$ref": "#/definitions/step_when"
Expand Down Expand Up @@ -392,6 +489,74 @@
}
}
},
"plugin_step": {
"description": "Plugins let you execute predefined functions in a more secure context. Read more: https://woodpecker-ci.org/docs/usage/plugins/overview",
"type": "object",
"additionalProperties": false,
"required": ["image"],
"properties": {
"name": {
"description": "The name of the step. Can be used if using the array style steps list.",
"type": "string"
},
"image": {
"$ref": "#/definitions/step_image"
},
"privileged": {
"$ref": "#/definitions/step_privileged"
},
"pull": {
"$ref": "#/definitions/step_pull"
},
"directory": {
"$ref": "#/definitions/step_directory"
},
"secrets": {
"$ref": "#/definitions/step_secrets"
},
"settings": {
"$ref": "#/definitions/step_settings"
},
"when": {
"$ref": "#/definitions/step_when"
},
"volumes": {
"$ref": "#/definitions/step_volumes"
},
"group": {
"description": "deprecated, use depends_on",
"type": "string"
},
"depends_on": {
"description": "Execute a step after another step has finished.",
"oneOf": [
{
"type": "array",
"minLength": 1,
"items": {
"type": "string"
}
},
{
"type": "string"
}
]
},
"detach": {
"description": "Detach a step to run in background until pipeline finishes. Read more: https://woodpecker-ci.org/docs/usage/services#detachment",
"type": "boolean"
},
"failure": {
"description": "How to handle the failure of this step. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#failure",
"type": "string",
"enum": ["fail", "ignore"],
"default": "fail"
},
"backend_options": {
"$ref": "#/definitions/step_backend_options"
}
}
},
"step_when": {
"description": "Steps can be skipped based on conditions. Read more: https://woodpecker-ci.org/docs/usage/workflow-syntax#when---conditional-execution",
"oneOf": [
Expand Down
10 changes: 10 additions & 0 deletions pipeline/frontend/yaml/linter/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ func TestSchema(t *testing.T) {
testFile: ".woodpecker/test-custom-backend.yaml",
fail: false,
},
{
name: "Broken Plugin by environment",
testFile: ".woodpecker/test-broken-plugin.yaml",
fail: true,
},
{
name: "Broken Plugin by commands",
testFile: ".woodpecker/test-broken-plugin2.yaml",
fail: true,
},
}

for _, tt := range testTable {
Expand Down
4 changes: 3 additions & 1 deletion pipeline/frontend/yaml/types/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ func (c *ContainerList) UnmarshalYAML(value *yaml.Node) error {
}

func (c *Container) IsPlugin() bool {
return len(c.Commands) == 0 && len(c.Entrypoint) == 0
return len(c.Commands) == 0 &&
len(c.Entrypoint) == 0 &&
len(c.Environment) == 0
}

func (c *Container) IsTrustedCloneImage() bool {
Expand Down
2 changes: 2 additions & 0 deletions pipeline/log/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ func TestCopyLineByLineSizeLimit(t *testing.T) {
if _, err := w.Write([]byte("67\n89")); err != nil {
t.Fatalf("unexpected error: %v", err)
}
// wait for writer to write
time.Sleep(time.Millisecond)

writes = testWriter.GetWrites()
assert.Lenf(t, testWriter.GetWrites(), 2, "expected 2 writes, got: %v", writes)
Expand Down