From 487a6e2335a58e6e818cb7858607db219922a482 Mon Sep 17 00:00:00 2001 From: Erik Kristensen Date: Wed, 13 Nov 2024 15:30:09 -0700 Subject: [PATCH] fix(config): protect against nil pointer for misconfigured account --- pkg/config/config.go | 5 ++++ pkg/config/config_test.go | 33 +++++++++++++++++++++++++++ pkg/config/testdata/broken-fixed.yaml | 17 ++++++++++++++ pkg/config/testdata/broken.yaml | 16 +++++++++++++ 4 files changed, 71 insertions(+) create mode 100644 pkg/config/testdata/broken-fixed.yaml create mode 100644 pkg/config/testdata/broken.yaml diff --git a/pkg/config/config.go b/pkg/config/config.go index bd16072..2b1dee7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -204,6 +204,11 @@ func (c *Config) Filters(accountID string) (filter.Filters, error) { } account := c.Accounts[accountID] + + if account == nil { + return nil, errors.ErrAccountNotConfigured + } + filters := account.Filters if filters == nil { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index a9cccba..4b7b6f0 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -9,6 +9,8 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" + + "github.com/ekristen/libnuke/pkg/errors" ) func init() { @@ -242,3 +244,34 @@ func TestIncomplete(t *testing.T) { assert.NoError(t, err) _ = c } + +// TestBroken tests a broken configuration file with an incomplete account that is not properly +// configured. This should return an error when trying to get the filters for the account. +func TestBroken(t *testing.T) { + opts := Options{ + Path: "testdata/broken.yaml", + } + c, err := New(opts) + assert.NoError(t, err) + _ = c + + _, err = c.Filters("000000000000") + assert.ErrorIs(t, err, errors.ErrAccountNotConfigured) +} + +// TestBrokenFixed tests a fixed broken configuration where there is at minimum an empty object provided for the +// account. This should return an empty set of filters and no errors. +func TestBrokenFixed(t *testing.T) { + opts := Options{ + Path: "testdata/broken-fixed.yaml", + } + c, err := New(opts) + assert.NoError(t, err) + _ = c + + _, err = c.Filters("000000000000") + assert.NoError(t, err) + + _, err = c.Filters("111111111111") + assert.NoError(t, err) +} diff --git a/pkg/config/testdata/broken-fixed.yaml b/pkg/config/testdata/broken-fixed.yaml new file mode 100644 index 0000000..9e63db5 --- /dev/null +++ b/pkg/config/testdata/broken-fixed.yaml @@ -0,0 +1,17 @@ +regions: +- global +- us-east-1 +- ap-southeast-2 + +blocklist: +- "000000000000" + +accounts: + "000000000000": {} + "111111111111": + filters: + __global__: + - property: tag:aws-nuke + value: "Disable" + - property: tag:aws-nuke + value: "disable" \ No newline at end of file diff --git a/pkg/config/testdata/broken.yaml b/pkg/config/testdata/broken.yaml new file mode 100644 index 0000000..f8d521b --- /dev/null +++ b/pkg/config/testdata/broken.yaml @@ -0,0 +1,16 @@ +regions: +- global +- us-east-1 +- ap-southeast-2 + +blocklist: +- "000000000000" + +accounts: + "000000000000": + filters: + __global__: + - property: tag:aws-nuke + value: "Disable" + - property: tag:aws-nuke + value: "disable" \ No newline at end of file