diff --git a/README.md b/README.md index 90757ad..3d39195 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ Narratio now includes configuration and path-model foundations for archive suppo Implemented foundations: -- `pipeline.storage.s3` config shape (`bucket`, `root_prefix`, `region`, `endpoint`, `force_path_style`) +- `pipeline.storage.s3` config shape (`bucket`, `root_prefix`, `region`, `endpoint`, `force_path_style`, `access_key_id_env`, `secret_access_key_env`) - `pipeline.spool` config shape (`root`, `delete_audio_after_archive`) - `pipeline.archive` config shape (`enabled`, `upload_run`, `promote_artifacts`) - promotion-rule validation (`from`/`to` required, relative-only paths, traversal rejected) @@ -83,6 +83,8 @@ Implemented foundations: Current defaults: - `pipeline.storage.s3.root_prefix`: `dnd` +- `pipeline.storage.s3.access_key_id_env`: `OBJECT_STORAGE_KEY_ID` +- `pipeline.storage.s3.secret_access_key_env`: `OBJECT_STORAGE_KEY` - `pipeline.workspace.cleanup_after_archive`: `false` - `pipeline.spool.root`: `/var/spool/narratio` - `pipeline.spool.delete_audio_after_archive`: `false` @@ -110,7 +112,8 @@ Current boundaries: - `pipeline.workspace.cleanup_after_archive: true` removes only the run-scoped local workdir after successful archive commit - cleanup executes only after all selected stages for the command invocation succeed - cleanup does not run for failed, incomplete, skipped, or unarchived runs - - local development `audio_dir`/`audio_files` source inputs are never deleted by spool cleanup +- local development `audio_dir`/`audio_files` source inputs are never deleted by spool cleanup +- S3 credentials are resolved from configured env-var names when both are present; if either is missing, Narratio falls back to the AWS SDK default credential chain S3 input details and current boundaries are documented in [docs/s3-audio-input.md](docs/s3-audio-input.md). diff --git a/architecture.md b/architecture.md index 5907dc9..7db51a2 100644 --- a/architecture.md +++ b/architecture.md @@ -151,6 +151,8 @@ Storage and archive foundations: - `region` - `endpoint` - `force_path_style` (default `false`) + - `access_key_id_env` (default `OBJECT_STORAGE_KEY_ID`) + - `secret_access_key_env` (default `OBJECT_STORAGE_KEY`) - `pipeline.spool.root` defaults to `/var/spool/narratio` - `pipeline.workspace.cleanup_after_archive` defaults to `false` - `pipeline.spool.delete_audio_after_archive` defaults to `false` @@ -176,7 +178,9 @@ Session input foundations: Cross-config validation scope: - `pipeline.storage.s3.bucket` is required only when an S3-dependent feature is explicitly configured (for current foundations, that includes `session.inputs.audio_s3`, and archive upload intent when using `storage.backend: s3`) -- no AWS credentials are stored in Narratio config; credential resolution remains an external runtime concern +- no AWS credential values are stored in Narratio config; only env-var names are configured +- when both configured credential env vars resolve to non-empty values, the S3 backend uses them as static credentials +- when either configured credential value is missing, the S3 backend falls back to the AWS SDK default credential chain Remote object-store backend scope: diff --git a/docs/s3-audio-input.md b/docs/s3-audio-input.md index 4278de4..c20a1b2 100644 --- a/docs/s3-audio-input.md +++ b/docs/s3-audio-input.md @@ -20,6 +20,8 @@ Not implemented: - `storage.s3.bucket` must be set when S3 audio input is used. - `storage.s3.root_prefix` defaults to `dnd`. +- `storage.s3.access_key_id_env` defaults to `OBJECT_STORAGE_KEY_ID`. +- `storage.s3.secret_access_key_env` defaults to `OBJECT_STORAGE_KEY`. - `spool.root` defaults to `/var/spool/narratio`. `session.yml`: diff --git a/docs/storage-backends.md b/docs/storage-backends.md index c6e7460..9201551 100644 --- a/docs/storage-backends.md +++ b/docs/storage-backends.md @@ -30,6 +30,8 @@ Construction: - region - endpoint - force_path_style + - access_key_id_env + - secret_access_key_env ## Key Invariant @@ -42,7 +44,9 @@ S3 session/run key builders remain separate and continue to live outside backend ## Security Boundary - do not store AWS credentials in Narratio config -- AWS credentials are resolved through standard AWS SDK credential chains +- Narratio first checks configured env-var names (`access_key_id_env`, `secret_access_key_env`); + when both are present and non-empty, it uses static credentials from those values +- when either configured credential value is missing, Narratio falls back to the standard AWS SDK credential chain - AWS SDK-specific types remain isolated to the storage adapter package ## Testing diff --git a/examples/pipeline.minimal.yml b/examples/pipeline.minimal.yml index 6e92f80..3130c8b 100644 --- a/examples/pipeline.minimal.yml +++ b/examples/pipeline.minimal.yml @@ -8,6 +8,9 @@ storage: bucket: "my-dnd-archive" root_prefix: "dnd" region: "us-east-1" + # Optional credential env-var names (defaulted when omitted): + # access_key_id_env: "OBJECT_STORAGE_KEY_ID" + # secret_access_key_env: "OBJECT_STORAGE_KEY" spool: root: "/var/spool/narratio" diff --git a/internal/adapters/storage/s3_backend.go b/internal/adapters/storage/s3_backend.go index 9aca0be..71b0b4f 100644 --- a/internal/adapters/storage/s3_backend.go +++ b/internal/adapters/storage/s3_backend.go @@ -11,6 +11,7 @@ import ( "time" awsconfig "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/aws/smithy-go" @@ -35,6 +36,8 @@ type s3ClientOptions struct { Region string Endpoint string ForcePathStyle bool + AccessKeyID string + SecretKey string } var newS3Client = func(ctx context.Context, opts s3ClientOptions) (s3API, error) { @@ -42,6 +45,15 @@ 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) != "" { + loadOpts = append(loadOpts, awsconfig.WithCredentialsProvider( + credentials.NewStaticCredentialsProvider( + strings.TrimSpace(opts.AccessKeyID), + strings.TrimSpace(opts.SecretKey), + "", + ), + )) + } awsCfg, err := awsconfig.LoadDefaultConfig(ctx, loadOpts...) if err != nil { return nil, fmt.Errorf("load aws config: %w", err) @@ -67,6 +79,8 @@ func NewS3BackendFromConfig(ctx context.Context, cfg config.StorageS3Config) (*S Region: cfg.Region, Endpoint: cfg.Endpoint, ForcePathStyle: cfg.ForcePathStyle, + AccessKeyID: s3CredentialFromEnv(orDefaultEnvName(cfg.AccessKeyIDEnv, config.DefaultS3AccessKeyIDEnv)), + SecretKey: s3CredentialFromEnv(orDefaultEnvName(cfg.SecretKeyEnv, config.DefaultS3SecretAccessKeyEnv)), }) if err != nil { return nil, fmt.Errorf("build s3 client: %w", err) @@ -78,6 +92,26 @@ 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) diff --git a/internal/adapters/storage/s3_backend_test.go b/internal/adapters/storage/s3_backend_test.go index 33f8fba..e7781a8 100644 --- a/internal/adapters/storage/s3_backend_test.go +++ b/internal/adapters/storage/s3_backend_test.go @@ -187,6 +187,8 @@ func TestS3BackendExistsNotFound(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") var got s3ClientOptions newS3Client = func(_ context.Context, opts s3ClientOptions) (s3API, error) { @@ -209,6 +211,9 @@ 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" { + t.Fatalf("client options credentials = %#v, want env-resolved static credentials", got) + } } func TestNewS3BackendFromConfigRequiresBucket(t *testing.T) { @@ -218,6 +223,30 @@ func TestNewS3BackendFromConfigRequiresBucket(t *testing.T) { } } +func TestNewS3BackendFromConfigFallsBackWhenCredentialEnvMissing(t *testing.T) { + original := newS3Client + t.Cleanup(func() { newS3Client = original }) + + var got s3ClientOptions + newS3Client = func(_ context.Context, opts s3ClientOptions) (s3API, error) { + got = opts + return &fakeS3API{}, nil + } + + _, err := NewS3BackendFromConfig(context.Background(), config.StorageS3Config{ + Bucket: "my-archive", + Region: "us-east-1", + AccessKeyIDEnv: "MISSING_ACCESS_KEY_ID", + SecretKeyEnv: "MISSING_SECRET_KEY", + }) + 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) + } +} + func strPtr(v string) *string { return &v } func int64Ptr(v int64) *int64 { return &v } diff --git a/internal/config/config.go b/internal/config/config.go index 432d934..fefb268 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -60,6 +60,8 @@ type StorageS3Config struct { Region string `yaml:"region"` Endpoint string `yaml:"endpoint"` ForcePathStyle bool `yaml:"force_path_style"` + AccessKeyIDEnv string `yaml:"access_key_id_env"` + SecretKeyEnv string `yaml:"secret_access_key_env"` } // SpoolConfig configures local spool storage for staged data. diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 7d8d759..f69b53f 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -8,6 +8,8 @@ const ( DefaultSessionConfigPathLocal = "./session.yml" DefaultSessionConfigPathUsrLocal = "/usr/local/etc/narratio/session.yml" DefaultSessionConfigPathEtc = "/etc/narratio/session.yml" + DefaultS3AccessKeyIDEnv = "OBJECT_STORAGE_KEY_ID" + DefaultS3SecretAccessKeyEnv = "OBJECT_STORAGE_KEY" ) // DefaultPipelineConfigSearchPaths defines the default search order for diff --git a/internal/config/load.go b/internal/config/load.go index 9d3351d..2aaed4b 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -176,6 +176,12 @@ func applyStorageDefaults(cfg *StorageConfig) { if cfg.S3.RootPrefix == "" { cfg.S3.RootPrefix = "dnd" } + if cfg.S3.AccessKeyIDEnv == "" { + cfg.S3.AccessKeyIDEnv = DefaultS3AccessKeyIDEnv + } + if cfg.S3.SecretKeyEnv == "" { + cfg.S3.SecretKeyEnv = DefaultS3SecretAccessKeyEnv + } } func applySpoolDefaults(cfg *SpoolConfig) { diff --git a/internal/config/storage_archive_test.go b/internal/config/storage_archive_test.go index d507f0f..04903fd 100644 --- a/internal/config/storage_archive_test.go +++ b/internal/config/storage_archive_test.go @@ -24,6 +24,12 @@ storage: if cfg.Pipeline.Storage.S3.RootPrefix != "dnd" { t.Fatalf("storage.s3.root_prefix = %q, want dnd", cfg.Pipeline.Storage.S3.RootPrefix) } + if cfg.Pipeline.Storage.S3.AccessKeyIDEnv != DefaultS3AccessKeyIDEnv { + t.Fatalf("storage.s3.access_key_id_env = %q, want %q", cfg.Pipeline.Storage.S3.AccessKeyIDEnv, DefaultS3AccessKeyIDEnv) + } + if cfg.Pipeline.Storage.S3.SecretKeyEnv != DefaultS3SecretAccessKeyEnv { + t.Fatalf("storage.s3.secret_access_key_env = %q, want %q", cfg.Pipeline.Storage.S3.SecretKeyEnv, DefaultS3SecretAccessKeyEnv) + } if cfg.Pipeline.Storage.S3.ForcePathStyle { t.Fatalf("storage.s3.force_path_style = true, want false default") } @@ -33,6 +39,77 @@ storage: } } +func TestStorageS3CredentialEnvNamesLoadAndValidate(t *testing.T) { + pipelineYAML := testPipelineBaseYAML + ` +storage: + backend: s3 + s3: + bucket: my-dnd-archive + access_key_id_env: CUSTOM_KEY_ID + secret_access_key_env: CUSTOM_SECRET +` + pipelinePath, sessionPath := writeConfigFiles(t, pipelineYAML, testSessionBaseYAML) + + cfg, err := Load(pipelinePath, sessionPath) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Pipeline.Storage.S3.AccessKeyIDEnv != "CUSTOM_KEY_ID" { + t.Fatalf("storage.s3.access_key_id_env = %q, want CUSTOM_KEY_ID", cfg.Pipeline.Storage.S3.AccessKeyIDEnv) + } + if cfg.Pipeline.Storage.S3.SecretKeyEnv != "CUSTOM_SECRET" { + t.Fatalf("storage.s3.secret_access_key_env = %q, want CUSTOM_SECRET", cfg.Pipeline.Storage.S3.SecretKeyEnv) + } + if err := Validate(cfg); err != nil { + t.Fatalf("Validate() error = %v", err) + } +} + +func TestStorageS3CredentialEnvValidation(t *testing.T) { + tests := []struct { + name string + pipelineYML string + wantErr string + }{ + { + name: "invalid access key env name", + pipelineYML: testPipelineBaseYAML + ` +storage: + backend: s3 + s3: + bucket: my-dnd-archive + access_key_id_env: "123BAD" +`, + wantErr: "pipeline.storage.s3.access_key_id_env must be a valid environment variable name", + }, + { + name: "invalid secret key env name", + pipelineYML: testPipelineBaseYAML + ` +storage: + backend: s3 + s3: + bucket: my-dnd-archive + secret_access_key_env: "bad-name" +`, + wantErr: "pipeline.storage.s3.secret_access_key_env must be a valid environment variable name", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pipelinePath, sessionPath := writeConfigFiles(t, tt.pipelineYML, testSessionBaseYAML) + cfg, err := Load(pipelinePath, sessionPath) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + err = Validate(cfg) + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Validate() error = %v, want to contain %q", err, tt.wantErr) + } + }) + } +} + func TestSpoolAndArchiveDefaults(t *testing.T) { pipelinePath, sessionPath := writeConfigFiles(t, testPipelineBaseYAML, testSessionBaseYAML) diff --git a/internal/config/validate.go b/internal/config/validate.go index 99faee7..5b8fb02 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -91,6 +91,12 @@ func validateStorage(cfg StorageConfig) error { if cfg.S3.Endpoint != "" && strings.TrimSpace(cfg.S3.Endpoint) == "" { return fmt.Errorf("pipeline.storage.s3.endpoint must be non-empty when provided") } + if err := validateEnvVarNameField("pipeline.storage.s3.access_key_id_env", cfg.S3.AccessKeyIDEnv); err != nil { + return err + } + if err := validateEnvVarNameField("pipeline.storage.s3.secret_access_key_env", cfg.S3.SecretKeyEnv); err != nil { + return err + } return nil } @@ -430,6 +436,18 @@ func archiveUploadConfiguredForS3(pipeline *PipelineConfig) bool { } var windowsAbsPathRE = regexp.MustCompile(`^[A-Za-z]:[\\/].*`) +var envVarNameRE = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + +func validateEnvVarNameField(fieldName, value string) error { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return fmt.Errorf("%s must be non-empty", fieldName) + } + if !envVarNameRE.MatchString(trimmed) { + return fmt.Errorf("%s must be a valid environment variable name", fieldName) + } + return nil +} func validateRelativeSafePath(fieldName, value string) error { trimmed := strings.TrimSpace(value)