From d72eba082c10953d020938c3b72964c13c4a97ba Mon Sep 17 00:00:00 2001 From: Matheus Cumpian Date: Tue, 20 Feb 2024 22:18:28 -0300 Subject: [PATCH] Prevent panic on invalid configuration structure in exec mixin (#2953) * check for successful cast before get desc Signed-off-by: Matheus Cumpian --------- Signed-off-by: Matheus Cumpian --- CONTRIBUTORS.md | 1 + pkg/exec/schema_test.go | 1 + .../testdata/install-invalid-type-input.yaml | 2 ++ pkg/manifest/manifest.go | 6 +++++- pkg/manifest/manifest_test.go | 20 +++++++++++++++++++ .../testdata/porter-with-bad-description.yaml | 18 +++++++++++++++++ .../testdata/porter-with-bad-type.yaml | 15 ++++++++++++++ 7 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 pkg/exec/testdata/install-invalid-type-input.yaml create mode 100644 pkg/manifest/testdata/porter-with-bad-description.yaml create mode 100644 pkg/manifest/testdata/porter-with-bad-type.yaml diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index cd07113ca..5bb4835f3 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -92,6 +92,7 @@ and we will add you. **All** contributors belong here. 💯 - [Chaiyapruek Muangsiri](https://github.com/cmppoon) - [Xin Fu](https://github.com/imfing) - [KallyDev](https://github.com/kallydev) +- [Matheus Cumpian](https://github.com/heavybr) - [Salman Shah](https://github.com/sbshah97) - [Ray Terrill](https://github.com/rayterrill) - [Kim Christensen](https://github.com/kichristensen) diff --git a/pkg/exec/schema_test.go b/pkg/exec/schema_test.go index 000f037c0..13711343b 100644 --- a/pkg/exec/schema_test.go +++ b/pkg/exec/schema_test.go @@ -39,6 +39,7 @@ func TestMixin_ValidateSchema(t *testing.T) { {"invoke", "testdata/invoke-input.yaml", ""}, {"uninstall", "testdata/uninstall-input.yaml", ""}, {"invalid command", "testdata/invalid-args-input.yaml", "Additional property args is not allowed"}, + {"invalid type", "testdata/install-invalid-type-input.yaml", "Invalid type"}, } for _, tc := range testcases { diff --git a/pkg/exec/testdata/install-invalid-type-input.yaml b/pkg/exec/testdata/install-invalid-type-input.yaml new file mode 100644 index 000000000..47035c01d --- /dev/null +++ b/pkg/exec/testdata/install-invalid-type-input.yaml @@ -0,0 +1,2 @@ +install: + - exec: "Install a VM and collect its ID" \ No newline at end of file diff --git a/pkg/manifest/manifest.go b/pkg/manifest/manifest.go index 3adb1e889..9e8909390 100644 --- a/pkg/manifest/manifest.go +++ b/pkg/manifest/manifest.go @@ -1031,8 +1031,12 @@ func (s *Step) GetDescription() (string, error) { mixinName := s.GetMixinName() children := s.Data[mixinName] - d, ok := children.(map[string]interface{})["description"] + m, ok := children.(map[string]interface{}) if !ok { + return "", fmt.Errorf("invalid mixin type (%T) for mixin step (%s)", children, mixinName) + } + d := m["description"] + if d == nil { return "", nil } desc, ok := d.(string) diff --git a/pkg/manifest/manifest_test.go b/pkg/manifest/manifest_test.go index 3ced60a95..51a6dd128 100644 --- a/pkg/manifest/manifest_test.go +++ b/pkg/manifest/manifest_test.go @@ -195,6 +195,26 @@ func TestManifest_Validate_Name(t *testing.T) { assert.EqualError(t, err, "bundle name must be set") } +func TestManifest_Validate_Description(t *testing.T) { + c := config.NewTestConfig(t) + + c.TestContext.AddTestFile("testdata/porter-with-bad-description.yaml", config.Name) + + _, err := LoadManifestFrom(context.Background(), c.Config, config.Name) + assert.ErrorContains(t, err, "validation of action \"install\" failed: invalid description type (string) for mixin step (exec)") +} + +func TestManifest_Validate_InvalidType(t *testing.T) { + c := config.NewTestConfig(t) + + c.TestContext.AddTestFile("testdata/porter-with-bad-type.yaml", config.Name) + + assert.NotPanics(t, func() { + _, err := LoadManifestFrom(context.Background(), c.Config, config.Name) + assert.ErrorContains(t, err, "validation of action \"install\" failed: invalid mixin type (string) for mixin step (exec)") + }) +} + func TestManifest_Validate_SchemaVersion(t *testing.T) { invalidVersionErr := schema.ErrInvalidSchemaVersion.Error() diff --git a/pkg/manifest/testdata/porter-with-bad-description.yaml b/pkg/manifest/testdata/porter-with-bad-description.yaml new file mode 100644 index 000000000..8410f1cea --- /dev/null +++ b/pkg/manifest/testdata/porter-with-bad-description.yaml @@ -0,0 +1,18 @@ +schemaVersion: 1.0.0 +version: 0.1.0 +registry: example.com +name: mybun + +mixins: + - exec + +install: + - exec: + description: + - I should be a string, but I am a list + command: bash + +uninstall: + - exec: + description: uninstall everything + command: bash \ No newline at end of file diff --git a/pkg/manifest/testdata/porter-with-bad-type.yaml b/pkg/manifest/testdata/porter-with-bad-type.yaml new file mode 100644 index 000000000..a04318a78 --- /dev/null +++ b/pkg/manifest/testdata/porter-with-bad-type.yaml @@ -0,0 +1,15 @@ +schemaVersion: 1.0.0 +version: 0.1.0 +registry: example.com +name: mybun + +mixins: + - exec + +install: + - exec: install something + +uninstall: + - exec: + description: uninstall everything + command: bash \ No newline at end of file