From a30a36ca51bb61074c43dcb6469e10dd6d6e2894 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Wed, 24 Sep 2025 20:47:14 +0200
Subject: [PATCH] s3: drop manual credentials loading from environment

credentials.EnvAWS offers a superset of the manually implemented
credentials loading. Rework the error message that is shown if no
credentials were found but either access or secret key are set.
---
 internal/backend/s3/config.go | 12 ++++--------
 internal/backend/s3/s3.go     | 20 +++++++++++---------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/internal/backend/s3/config.go b/internal/backend/s3/config.go
index 365b16bf1..5d3d7a879 100644
--- a/internal/backend/s3/config.go
+++ b/internal/backend/s3/config.go
@@ -17,8 +17,6 @@ import (
 type Config struct {
 	Endpoint     string
 	UseHTTP      bool
-	KeyID        string
-	Secret       options.SecretString
 	Bucket       string
 	Prefix       string
 	Layout       string `option:"layout" help:"use this backend layout (default: auto-detect) (deprecated)"`
@@ -35,6 +33,10 @@ type Config struct {
 	BucketLookup        string `option:"bucket-lookup" help:"bucket lookup style: 'auto', 'dns', or 'path'"`
 	ListObjectsV1       bool   `option:"list-objects-v1" help:"use deprecated V1 api for ListObjects calls"`
 	UnsafeAnonymousAuth bool   `option:"unsafe-anonymous-auth" help:"use anonymous authentication"`
+
+	// For testing only
+	KeyID  string
+	Secret options.SecretString
 }
 
 // NewConfig returns a new Config with the default values filled in.
@@ -109,12 +111,6 @@ var _ backend.ApplyEnvironmenter = &Config{}
 
 // ApplyEnvironment saves values from the environment to the config.
 func (cfg *Config) ApplyEnvironment(prefix string) {
-	if cfg.KeyID == "" {
-		cfg.KeyID = os.Getenv(prefix + "AWS_ACCESS_KEY_ID")
-	}
-	if cfg.Secret.String() == "" {
-		cfg.Secret = options.NewSecretString(os.Getenv(prefix + "AWS_SECRET_ACCESS_KEY"))
-	}
 	if cfg.Region == "" {
 		cfg.Region = os.Getenv(prefix + "AWS_DEFAULT_REGION")
 	}
diff --git a/internal/backend/s3/s3.go b/internal/backend/s3/s3.go
index 3653c827c..11e09c958 100644
--- a/internal/backend/s3/s3.go
+++ b/internal/backend/s3/s3.go
@@ -57,12 +57,6 @@ func open(cfg Config, rt http.RoundTripper) (*Backend, error) {
 		return nil, fmt.Errorf("feature flag `s3-restore` is required to use `-o s3.enable-restore=true`")
 	}
 
-	if cfg.KeyID == "" && cfg.Secret.String() != "" {
-		return nil, errors.Fatalf("unable to open S3 backend: Key ID ($AWS_ACCESS_KEY_ID) is empty")
-	} else if cfg.KeyID != "" && cfg.Secret.String() == "" {
-		return nil, errors.Fatalf("unable to open S3 backend: Secret ($AWS_SECRET_ACCESS_KEY) is empty")
-	}
-
 	if cfg.MaxRetries > 0 {
 		minio.MaxRetry = int(cfg.MaxRetries)
 	}
@@ -112,7 +106,7 @@ func getCredentials(cfg Config, tr http.RoundTripper) (*credentials.Credentials,
 	}
 
 	// Chains all credential types, in the following order:
-	// 	- Static credentials provided by user
+	// 	- Static credentials (test only)
 	//	- AWS env vars (i.e. AWS_ACCESS_KEY_ID)
 	//  - Minio env vars (i.e. MINIO_ACCESS_KEY)
 	//  - AWS creds file (i.e. AWS_SHARED_CREDENTIALS_FILE or ~/.aws/credentials)
@@ -121,13 +115,13 @@ func getCredentials(cfg Config, tr http.RoundTripper) (*credentials.Credentials,
 	//    call to a pre-defined endpoint, only valid inside
 	//    configured ec2 instances)
 	creds := credentials.NewChainCredentials([]credentials.Provider{
-		&credentials.EnvAWS{},
-		&credentials.Static{
+		&credentials.Static{ // test only
 			Value: credentials.Value{
 				AccessKeyID:     cfg.KeyID,
 				SecretAccessKey: cfg.Secret.Unwrap(),
 			},
 		},
+		&credentials.EnvAWS{},
 		&credentials.EnvMinio{},
 		&credentials.FileAWSCredentials{},
 		&credentials.FileMinioClient{},
@@ -141,6 +135,14 @@ func getCredentials(cfg Config, tr http.RoundTripper) (*credentials.Credentials,
 	}
 
 	if c.SignerType == credentials.SignatureAnonymous {
+		keyID := os.Getenv("AWS_ACCESS_KEY_ID")
+		secret := os.Getenv("AWS_SECRET_ACCESS_KEY")
+		if keyID == "" && secret != "" {
+			return nil, errors.Fatalf("no credentials found. $AWS_SECRET_ACCESS_KEY is set but $AWS_ACCESS_KEY_ID is empty")
+		} else if keyID != "" && secret == "" {
+			return nil, errors.Fatalf("no credentials found. $AWS_ACCESS_KEY_ID is set but $AWS_SECRET_ACCESS_KEY is empty")
+		}
+
 		// Fail if no credentials were found to prevent repeated attempts to (unsuccessfully) retrieve new credentials.
 		// The first attempt still has to timeout which slows down restic usage considerably. Thus, migrate towards forcing
 		// users to explicitly decide between authenticated and anonymous access.