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
13 changes: 6 additions & 7 deletions pipeline/frontend/yaml/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,14 @@
}

func (l *Linter) lintCommands(config *WorkflowConfig, c *types.Container, field string) error {
if len(c.Commands) == 0 {
if len(c.Commands) == 0 && len(c.Environment) == 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.Settings) != 0 && len(c.Commands) != 0 {
return newLinterError("Cannot configure both commands and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)
}

Check warning on line 141 in pipeline/frontend/yaml/linter/linter.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/linter/linter.go#L140-L141

Added lines #L140 - L141 were not covered by tests
if len(c.Settings) != 0 && len(c.Environment) != 0 {
6543 marked this conversation as resolved.
Show resolved Hide resolved
return newLinterError("Cannot configure both commands and settings", config.File, fmt.Sprintf("%s.%s", field, c.Name), false)

Check warning on line 143 in pipeline/frontend/yaml/linter/linter.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/linter/linter.go#L143

Added line #L143 was not covered by tests
6543 marked this conversation as resolved.
Show resolved Hide resolved
}
return nil
}
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