Fix #8168 - AWS secrets should not be exposed while running tests

Changed the tests to use mocked function that will not read actual
secrets from env variables nor AWS config file that may be
on the system that is running tests.

As a second guard against exposed secrets comparison for the values
does not shows the actual values for the AWS data. This is to prevent
situation where programming error may still allow the test to read
AWS config/env variables instead of using mocked function.

Signed-off-by: Michal Pryc <mpryc@redhat.com>
pull/8169/head
Michal Pryc 2024-08-30 17:05:29 +02:00
parent 3408ffefac
commit 6d0f726c2f
2 changed files with 43 additions and 7 deletions

View File

@ -34,6 +34,9 @@ import (
"github.com/pkg/errors"
)
// getS3CredentialsFunc is used to make testing more convenient
var getS3CredentialsFunc = GetS3Credentials
const (
// AWS specific environment variable
awsProfileEnvVar = "AWS_PROFILE"
@ -63,7 +66,7 @@ func GetS3ResticEnvVars(config map[string]string) (map[string]string, error) {
// GetS3ResticEnvVars reads the AWS config, from files and envs
// if needed assumes the role and returns the session credentials
// setting these variables emulates what would happen for example when using kube2iam
if creds, err := GetS3Credentials(config); err == nil && creds != nil {
if creds, err := getS3CredentialsFunc(config); err == nil && creds != nil {
result[awsKeyIDEnvVar] = creds.AccessKeyID
result[awsSecretKeyEnvVar] = creds.SecretAccessKey
result[awsSessTokenEnvVar] = creds.SessionToken

View File

@ -27,14 +27,18 @@ import (
func TestGetS3ResticEnvVars(t *testing.T) {
testCases := []struct {
name string
config map[string]string
expected map[string]string
name string
config map[string]string
expected map[string]string
getS3Credentials func(config map[string]string) (*aws.Credentials, error)
}{
{
name: "when config is empty, no env vars are returned",
config: map[string]string{},
expected: map[string]string{},
getS3Credentials: func(config map[string]string) (*aws.Credentials, error) {
return nil, nil
},
},
{
name: "when config contains profile key, profile env var is set with profile value",
@ -53,16 +57,39 @@ func TestGetS3ResticEnvVars(t *testing.T) {
expected: map[string]string{
"AWS_SHARED_CREDENTIALS_FILE": "/tmp/credentials/path/to/secret",
},
getS3Credentials: func(config map[string]string) (*aws.Credentials, error) {
return nil, nil
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Mock GetS3Credentials
if tc.getS3Credentials != nil {
getS3CredentialsFunc = tc.getS3Credentials
} else {
getS3CredentialsFunc = GetS3Credentials
}
actual, err := GetS3ResticEnvVars(tc.config)
require.NoError(t, err)
require.Equal(t, tc.expected, actual)
// Avoid direct comparison of expected and actual to prevent exposing secrets.
// This may occur if the test doesn't set getS3Credentials func correctly.
if !reflect.DeepEqual(tc.expected, actual) {
t.Errorf("Expected and actual results do not match for test case %q", tc.name)
for key, value := range actual {
if expVal, err := tc.expected[key]; !err || expVal != value {
if actualVal, ok := actual[key]; !ok {
t.Errorf("Key %q is missing in actual result", key)
} else if expVal != actualVal {
t.Errorf("Key %q: expected value %q", key, expVal)
}
}
}
}
})
}
}
@ -117,6 +144,11 @@ func TestGetS3CredentialsCorrectlyUseProfile(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Ensure env variables do not set AWS config entries
t.Setenv("AWS_ACCESS_KEY_ID", "")
t.Setenv("AWS_SECRET_ACCESS_KEY", "")
t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "")
tmpFile, err := os.CreateTemp("", "velero-test-aws-credentials")
defer os.Remove(tmpFile.Name())
if err != nil {
@ -129,6 +161,7 @@ func TestGetS3CredentialsCorrectlyUseProfile(t *testing.T) {
t.Errorf("GetS3Credentials() error = %v", err)
return
}
tt.args.config["credentialsFile"] = tmpFile.Name()
got, err := GetS3Credentials(tt.args.config)
if (err != nil) != tt.wantErr {
@ -136,10 +169,10 @@ func TestGetS3CredentialsCorrectlyUseProfile(t *testing.T) {
return
}
if !reflect.DeepEqual(got.AccessKeyID, tt.want.AccessKeyID) {
t.Errorf("GetS3Credentials() got = %v, want %v", got.AccessKeyID, tt.want.AccessKeyID)
t.Errorf("GetS3Credentials() want %v", tt.want.AccessKeyID)
}
if !reflect.DeepEqual(got.SecretAccessKey, tt.want.SecretAccessKey) {
t.Errorf("GetS3Credentials() got = %v, want %v", got.SecretAccessKey, tt.want.SecretAccessKey)
t.Errorf("GetS3Credentials() want %v", tt.want.SecretAccessKey)
}
})
}