Resolve typed credentials safely

internal/adapters/storage/s3_backend.go

@@ -37,8 +37,12 @@ type s3ClientOptions struct {
 	Region         string
 	Endpoint       string
 	ForcePathStyle bool
-	AccessKeyID    string
-	SecretKey      string
+	Credentials    *s3StaticCredentials
+}
+
+type s3StaticCredentials struct {
+	AccessKeyID     string
+	SecretAccessKey string
 }
 
 var newS3Client = func(ctx context.Context, opts s3ClientOptions) (s3API, error) {
@@ -46,11 +50,11 @@ var newS3Client = func(ctx context.Context, opts s3ClientOptions) (s3API, error)
 	if strings.TrimSpace(opts.Region) != "" {
 		loadOpts = append(loadOpts, awsconfig.WithRegion(strings.TrimSpace(opts.Region)))
 	}
-	if strings.TrimSpace(opts.AccessKeyID) != "" && strings.TrimSpace(opts.SecretKey) != "" {
+	if opts.Credentials != nil {
 		loadOpts = append(loadOpts, awsconfig.WithCredentialsProvider(
 			credentials.NewStaticCredentialsProvider(
-				strings.TrimSpace(opts.AccessKeyID),
-				strings.TrimSpace(opts.SecretKey),
+				opts.Credentials.AccessKeyID,
+				opts.Credentials.SecretAccessKey,
 				"",
 			),
 		))
@@ -75,14 +79,24 @@ func NewS3BackendFromConfig(ctx context.Context, cfg config.StorageS3Config) (*S
 	if bucket == "" {
 		return nil, fmt.Errorf("storage.s3.bucket is required")
 	}
+	if (cfg.AccessKeyID == nil) != (cfg.SecretAccessKey == nil) {
+		return nil, fmt.Errorf("storage.s3 access key ID and secret access key references must be configured together")
+	}
+	if cfg.AccessKeyID != nil && (!cfg.AccessKeyID.IsResolved() || !cfg.SecretAccessKey.IsResolved()) {
+		return nil, fmt.Errorf("storage.s3 configured credential references must be resolved")
+	}
 
-	client, err := newS3Client(ctx, s3ClientOptions{
+	clientOptions := s3ClientOptions{
 		Region:         cfg.Region,
 		Endpoint:       cfg.Endpoint,
 		ForcePathStyle: cfg.ForcePathStyle,
-		AccessKeyID:    s3CredentialFromEnv(orDefaultEnvName(cfg.AccessKeyIDEnv, config.DefaultS3AccessKeyIDEnv)),
-		SecretKey:      s3CredentialFromEnv(orDefaultEnvName(cfg.SecretKeyEnv, config.DefaultS3SecretAccessKeyEnv)),
-	})
+	}
+	if cfg.AccessKeyID != nil && cfg.SecretAccessKey != nil {
+		clientOptions.Credentials = &s3StaticCredentials{
+			AccessKeyID: cfg.AccessKeyID.Value(), SecretAccessKey: cfg.SecretAccessKey.Value(),
+		}
+	}
+	client, err := newS3Client(ctx, clientOptions)
 	if err != nil {
 		return nil, fmt.Errorf("build s3 client: %w", err)
 	}
@@ -93,26 +107,6 @@ func NewS3BackendFromConfig(ctx context.Context, cfg config.StorageS3Config) (*S
 	}, nil
 }
 
-func s3CredentialFromEnv(envVarName string) string {
-	name := strings.TrimSpace(envVarName)
-	if name == "" {
-		return ""
-	}
-	value, ok := os.LookupEnv(name)
-	if !ok {
-		return ""
-	}
-	return strings.TrimSpace(value)
-}
-
-func orDefaultEnvName(name, fallback string) string {
-	trimmed := strings.TrimSpace(name)
-	if trimmed == "" {
-		return fallback
-	}
-	return trimmed
-}
-
 // List returns objects under prefix.
 func (b *S3Backend) List(ctx context.Context, prefix string) ([]ObjectInfo, error) {
 	normalizedPrefix := normalizeObjectKey(prefix)

internal/adapters/storage/s3_backend_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/aws/smithy-go"
 
+	configkit "gitea.maximumdirect.net/eric/ConfigKit"
 	"gitea.maximumdirect.net/eric/narratio/internal/config"
 )
 
@@ -319,8 +320,10 @@ func TestS3BackendDeleteHonorsCancellation(t *testing.T) {
 func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) {
 	original := newS3Client
 	t.Cleanup(func() { newS3Client = original })
-	t.Setenv("OBJECT_STORAGE_KEY_ID", "id-123")
-	t.Setenv("OBJECT_STORAGE_KEY", "secret-abc")
+	accessKey := resolvedTestSecret(t, "NARRATIO_S3_TEST_ACCESS", "id-123")
+	secretKey := resolvedTestSecret(t, "NARRATIO_S3_TEST_SECRET", "secret-abc")
+	t.Setenv("NARRATIO_S3_TEST_ACCESS", "changed-id")
+	t.Setenv("NARRATIO_S3_TEST_SECRET", "changed-secret")
 
 	var got s3ClientOptions
 	newS3Client = func(_ context.Context, opts s3ClientOptions) (s3API, error) {
@@ -329,10 +332,12 @@ func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) {
 	}
 
 	backend, err := NewS3BackendFromConfig(context.Background(), config.StorageS3Config{
-		Bucket:         "my-archive",
-		Region:         "us-east-1",
-		Endpoint:       "http://localhost:9000",
-		ForcePathStyle: true,
+		Bucket:          "my-archive",
+		Region:          "us-east-1",
+		Endpoint:        "http://localhost:9000",
+		ForcePathStyle:  true,
+		AccessKeyID:     accessKey,
+		SecretAccessKey: secretKey,
 	})
 	if err != nil {
 		t.Fatalf("NewS3BackendFromConfig() error = %v", err)
@@ -343,7 +348,7 @@ func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) {
 	if got.Region != "us-east-1" || got.Endpoint != "http://localhost:9000" || !got.ForcePathStyle {
 		t.Fatalf("client options = %#v, want region/endpoint/path-style values", got)
 	}
-	if got.AccessKeyID != "id-123" || got.SecretKey != "secret-abc" {
+	if got.Credentials == nil || got.Credentials.AccessKeyID != "id-123" || got.Credentials.SecretAccessKey != "secret-abc" {
 		t.Fatalf("client options credentials = %#v, want env-resolved static credentials", got)
 	}
 }
@@ -355,7 +360,21 @@ func TestNewS3BackendFromConfigRequiresBucket(t *testing.T) {
 	}
 }
 
-func TestNewS3BackendFromConfigFallsBackWhenCredentialEnvMissing(t *testing.T) {
+func TestNewS3BackendFromConfigRejectsIncompleteOrUnresolvedCredentials(t *testing.T) {
+	unresolved := configkit.SecretEnv("NARRATIO_UNRESOLVED_S3_KEY")
+	for name, cfg := range map[string]config.StorageS3Config{
+		"partial":    {Bucket: "my-archive", AccessKeyID: &unresolved},
+		"unresolved": {Bucket: "my-archive", AccessKeyID: &unresolved, SecretAccessKey: &unresolved},
+	} {
+		t.Run(name, func(t *testing.T) {
+			if _, err := NewS3BackendFromConfig(context.Background(), cfg); err == nil {
+				t.Fatal("NewS3BackendFromConfig() succeeded, want credential error")
+			}
+		})
+	}
+}
+
+func TestNewS3BackendFromConfigUsesDefaultProviderWithoutCredentialReferences(t *testing.T) {
 	original := newS3Client
 	t.Cleanup(func() { newS3Client = original })
 
@@ -366,19 +385,30 @@ func TestNewS3BackendFromConfigFallsBackWhenCredentialEnvMissing(t *testing.T) {
 	}
 
 	_, err := NewS3BackendFromConfig(context.Background(), config.StorageS3Config{
-		Bucket:         "my-archive",
-		Region:         "us-east-1",
-		AccessKeyIDEnv: "MISSING_ACCESS_KEY_ID",
-		SecretKeyEnv:   "MISSING_SECRET_KEY",
+		Bucket: "my-archive",
+		Region: "us-east-1",
 	})
 	if err != nil {
 		t.Fatalf("NewS3BackendFromConfig() error = %v", err)
 	}
-	if got.AccessKeyID != "" || got.SecretKey != "" {
-		t.Fatalf("client options credentials = %#v, want empty fallback values", got)
+	if got.Credentials != nil {
+		t.Fatalf("client options credentials = %#v, want default provider chain", got.Credentials)
 	}
 }
 
+func resolvedTestSecret(t *testing.T, name, value string) *configkit.Secret {
+	t.Helper()
+	t.Setenv(name, value)
+	type document struct {
+		Secret *configkit.Secret `yaml:"secret"`
+	}
+	result, err := configkit.Load(configkit.Bytes("test.yml", []byte("secret: "+name+"\n")), configkit.Options[document]{})
+	if err != nil {
+		t.Fatalf("resolve test secret: %v", err)
+	}
+	return result.Config.Secret
+}
+
 func strPtr(v string) *string { return &v }
 func int64Ptr(v int64) *int64 { return &v }
 func boolPtr(v bool) *bool    { return &v }