Implemented a centralized secret-backed object-store helper

docs/development.md

@@ -69,6 +69,8 @@ For design principles and invariants, see [docs/architecture.md](./architecture.
 2. Add or update command tests (`TestExecute` and command-specific tests).
 3. Update [docs/cli.md](./cli.md) and, if operator workflow changes, [docs/operations.md](./operations.md).
 
+Remote-storage commands must obtain object storage through the app-level command object-store helper. Do not call `storage.NewObjectStoreFromConfig` directly from command handlers; the helper loads configured filesystem secrets before constructing the storage adapter.
+
 ### Add or modify stages/adapters
 
 1. Implement stage behavior in `internal/stage` with clear input/output boundaries.

docs/internal/adapters.md

@@ -47,6 +47,7 @@ Current execution usage:
 
 Default construction in app runner:
 - Auto-constructed when not injected: WhisperX HTTP client, Seriatim subprocess runner, Audita subprocess runner, Scriptorium subprocess runner, object store (only when needed), and `notify.NoopSender`.
+- Object-store construction goes through app command orchestration so configured filesystem secrets are loaded before the storage adapter is initialized.
 - Callers can inject test/fake implementations through `app.RunOptions.Env`.
 
 ## State and manifest behavior

docs/internal/storage.md

@@ -6,6 +6,7 @@ Document Narratio's remote storage backend contracts and implementations under `
 ## Inputs and outputs
 Inputs:
 - Resolved storage config (`pipeline.storage.*`).
+- Already-loaded environment variables for configured S3 credentials.
 - Bucket-relative object keys and local file paths from app/stage orchestration.
 
 Outputs:
@@ -22,6 +23,7 @@ Does not own:
 - Session/run prefix semantics.
 - Archive commit order semantics.
 - Manifest updates.
+- Filesystem secret loading from `pipeline.secrets.env_dir`.
 
 ## Config fields used
 - `pipeline.storage.backend`
@@ -55,6 +57,7 @@ Implementations:
 ## Failure behavior
 - `NewObjectStoreFromConfig` fails when no remote backend is configured or required S3 config is missing.
 - `S3Backend` constructor fails when required bucket is missing or AWS client setup fails.
+- App command orchestration loads configured filesystem secrets before calling the object-store factory.
 - CRUD operations return contextual errors (including not-found behavior via `Exists`).
 - Key normalization is applied before operations (`\\` to `/`, leading slash trimmed).
 - Remote session loading uses `List` to find the exact `session.yml` key and `Download` to materialize it to a local temp file.

internal/app/config_loader.go

@@ -56,7 +56,7 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, sessionF
 		PipelinePath: resolvedPipelinePath,
 		CampaignPath: resolvedCampaignPath,
 	}
-	store, err := newObjectStoreFromConfigFn(ctx, partialCfg)
+	store, err := newCommandObjectStore(ctx, partialCfg, nil)
 	if err != nil {
 		return nil, missingSessionConfigError(discoveredSession.Searched, fmt.Sprintf("remote session %q unavailable: %v", remoteKey, err))
 	}

internal/app/object_store.go

@@ -0,0 +1,21 @@
+package app
+
+import (
+	"context"
+	"fmt"
+	"log/slog"
+
+	"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
+	"gitea.maximumdirect.net/eric/narratio/internal/config"
+)
+
+func newCommandObjectStore(ctx context.Context, cfg *config.Config, logger *slog.Logger) (storage.ObjectStore, error) {
+	if _, err := loadSecretsFromConfig(cfg, logger); err != nil {
+		return nil, fmt.Errorf("load secrets from files: %w", err)
+	}
+	store, err := newObjectStoreFromConfigFn(ctx, cfg)
+	if err != nil {
+		return nil, fmt.Errorf("initialize object store backend: %w", err)
+	}
+	return store, nil
+}

internal/app/object_store_test.go

@@ -0,0 +1,165 @@
+package app
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
+	"gitea.maximumdirect.net/eric/narratio/internal/config"
+)
+
+func TestNewCommandObjectStoreLoadsSecretsBeforeFactory(t *testing.T) {
+	accessKeyEnv := "NARRATIO_TEST_COMMAND_STORE_KEY_ID"
+	secretKeyEnv := "NARRATIO_TEST_COMMAND_STORE_SECRET"
+	restoreEnvAfterTest(t, accessKeyEnv, secretKeyEnv)
+
+	secretsDir := t.TempDir()
+	mustWriteSecretFile(t, filepath.Join(secretsDir, accessKeyEnv), "loaded-key-id\n")
+	mustWriteSecretFile(t, filepath.Join(secretsDir, secretKeyEnv), "loaded-secret\n")
+
+	cfg := commandObjectStoreTestConfig(secretsDir)
+	fake := &storage.FakeBackend{}
+	called := false
+	origStoreFn := newObjectStoreFromConfigFn
+	newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) {
+		called = true
+		if got := os.Getenv(accessKeyEnv); got != "loaded-key-id" {
+			return nil, errors.New("access key was not loaded before object store init")
+		}
+		if got := os.Getenv(secretKeyEnv); got != "loaded-secret" {
+			return nil, errors.New("secret key was not loaded before object store init")
+		}
+		return fake, nil
+	}
+	t.Cleanup(func() {
+		newObjectStoreFromConfigFn = origStoreFn
+	})
+
+	store, err := newCommandObjectStore(context.Background(), cfg, nil)
+	if err != nil {
+		t.Fatalf("newCommandObjectStore() error = %v", err)
+	}
+	if store != fake {
+		t.Fatalf("store = %#v, want fake backend", store)
+	}
+	if !called {
+		t.Fatal("object store factory was not called")
+	}
+}
+
+func TestNewCommandObjectStorePreservesExistingEnv(t *testing.T) {
+	accessKeyEnv := "NARRATIO_TEST_COMMAND_STORE_EXISTING_KEY_ID"
+	secretKeyEnv := "NARRATIO_TEST_COMMAND_STORE_EXISTING_SECRET"
+	t.Setenv(accessKeyEnv, "existing-key-id")
+	t.Setenv(secretKeyEnv, "existing-secret")
+
+	secretsDir := t.TempDir()
+	mustWriteSecretFile(t, filepath.Join(secretsDir, accessKeyEnv), "file-key-id\n")
+	mustWriteSecretFile(t, filepath.Join(secretsDir, secretKeyEnv), "file-secret\n")
+
+	cfg := commandObjectStoreTestConfig(secretsDir)
+	origStoreFn := newObjectStoreFromConfigFn
+	newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) {
+		if got := os.Getenv(accessKeyEnv); got != "existing-key-id" {
+			return nil, errors.New("existing access key was overwritten")
+		}
+		if got := os.Getenv(secretKeyEnv); got != "existing-secret" {
+			return nil, errors.New("existing secret key was overwritten")
+		}
+		return &storage.FakeBackend{}, nil
+	}
+	t.Cleanup(func() {
+		newObjectStoreFromConfigFn = origStoreFn
+	})
+
+	if _, err := newCommandObjectStore(context.Background(), cfg, nil); err != nil {
+		t.Fatalf("newCommandObjectStore() error = %v", err)
+	}
+}
+
+func TestNewCommandObjectStoreSecretErrorStopsFactory(t *testing.T) {
+	cfg := commandObjectStoreTestConfig(filepath.Join(t.TempDir(), "missing"))
+	called := false
+	origStoreFn := newObjectStoreFromConfigFn
+	newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) {
+		called = true
+		return &storage.FakeBackend{}, nil
+	}
+	t.Cleanup(func() {
+		newObjectStoreFromConfigFn = origStoreFn
+	})
+
+	_, err := newCommandObjectStore(context.Background(), cfg, nil)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if called {
+		t.Fatal("object store factory was called after secret load failure")
+	}
+	if !strings.Contains(err.Error(), "load secrets from files") {
+		t.Fatalf("error = %q, want secret loading context", err.Error())
+	}
+}
+
+func TestNewCommandObjectStoreFactoryErrorIsContextual(t *testing.T) {
+	cfg := commandObjectStoreTestConfig("")
+	origStoreFn := newObjectStoreFromConfigFn
+	newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) {
+		return nil, errors.New("factory boom")
+	}
+	t.Cleanup(func() {
+		newObjectStoreFromConfigFn = origStoreFn
+	})
+
+	_, err := newCommandObjectStore(context.Background(), cfg, nil)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if !strings.Contains(err.Error(), "initialize object store backend") || !strings.Contains(err.Error(), "factory boom") {
+		t.Fatalf("error = %q, want factory context", err.Error())
+	}
+}
+
+func commandObjectStoreTestConfig(secretsDir string) *config.Config {
+	cfg := &config.Config{
+		Pipeline: &config.PipelineConfig{
+			Storage: config.StorageConfig{
+				Backend: "s3",
+				S3: &config.StorageS3Config{
+					Bucket:         "test-bucket",
+					AccessKeyIDEnv: "NARRATIO_TEST_COMMAND_STORE_KEY_ID",
+					SecretKeyEnv:   "NARRATIO_TEST_COMMAND_STORE_SECRET",
+				},
+			},
+		},
+	}
+	if strings.TrimSpace(secretsDir) != "" {
+		cfg.Pipeline.Secrets = &config.SecretsConfig{EnvDir: secretsDir}
+	}
+	return cfg
+}
+
+func restoreEnvAfterTest(t *testing.T, names ...string) {
+	t.Helper()
+	originals := make(map[string]string, len(names))
+	present := make(map[string]bool, len(names))
+	for _, name := range names {
+		value, ok := os.LookupEnv(name)
+		originals[name] = value
+		present[name] = ok
+		_ = os.Unsetenv(name)
+	}
+	t.Cleanup(func() {
+		for _, name := range names {
+			if present[name] {
+				_ = os.Setenv(name, originals[name])
+			} else {
+				_ = os.Unsetenv(name)
+			}
+		}
+	})
+}

internal/app/operator_helpers.go

@@ -316,7 +316,7 @@ func SessionInit(ctx context.Context, args []string, out io.Writer) error {
 		return err
 	}
 
-	store, err := newObjectStoreFromConfigFn(ctx, cfg)
+	store, err := newCommandObjectStore(ctx, cfg, nil)
 	if err != nil {
 		return fmt.Errorf("session init: %w", err)
 	}
@@ -488,7 +488,7 @@ func loadHelperContext(ctx context.Context, flags commonConfigFlags, needStore b
 	}
 	var store storage.ObjectStore
 	if needStore {
-		store, err = newObjectStoreFromConfigFn(ctx, cfg)
+		store, err = newCommandObjectStore(ctx, cfg, nil)
 		if err != nil {
 			return nil, nil, nil, nil, err
 		}
@@ -511,7 +511,7 @@ func objectStoreIfConfigured(ctx context.Context, cfg *config.Config) (storage.O
 	if cfg == nil || cfg.Pipeline == nil || cfg.Pipeline.Storage.S3 == nil || strings.TrimSpace(cfg.Pipeline.Storage.S3.Bucket) == "" {
 		return nil, nil
 	}
-	store, err := newObjectStoreFromConfigFn(ctx, cfg)
+	store, err := newCommandObjectStore(ctx, cfg, nil)
 	if err != nil {
 		return nil, err
 	}

internal/app/operator_helpers_test.go

@@ -3,6 +3,7 @@ package app
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -47,6 +48,49 @@ func TestExecuteSessionInitRemoteWritesCanonicalSessionConfig(t *testing.T) {
 	}
 }
 
+func TestExecuteSessionValidateLoadsSecretsBeforeObjectStoreInit(t *testing.T) {
+	workspaceRoot := t.TempDir()
+	pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)
+	accessKeyEnv := "NARRATIO_TEST_VALIDATE_OBJECT_KEY_ID"
+	secretKeyEnv := "NARRATIO_TEST_VALIDATE_OBJECT_SECRET"
+	restoreEnvAfterTest(t, accessKeyEnv, secretKeyEnv)
+	secretsDir := t.TempDir()
+	mustWriteTestFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-key-id\n")
+	mustWriteTestFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret\n")
+	addSecretsToPipelineConfig(t, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv)
+	if err := os.WriteFile(sessionPath, []byte(`session_id: 2026-05-03
+inputs:
+  audio_s3:
+    prefix: audio/
+`), 0o644); err != nil {
+		t.Fatalf("write session: %v", err)
+	}
+
+	fake := &storage.FakeBackend{}
+	audioKey := artifacts.S3PromotedArtifactKey(artifacts.S3AudioPrefix(artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03"), "audio/"), "alice.flac")
+	fake.SeedObject(storage.FakeObject{Key: audioKey, Data: []byte("audio")})
+	origStoreFn := newObjectStoreFromConfigFn
+	newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) {
+		if os.Getenv(accessKeyEnv) != "test-key-id" || os.Getenv(secretKeyEnv) != "test-secret" {
+			return nil, fmt.Errorf("secrets were not loaded before object store init")
+		}
+		return fake, nil
+	}
+	t.Cleanup(func() {
+		newObjectStoreFromConfigFn = origStoreFn
+	})
+
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	code := Execute([]string{"session", "validate", "--config", pipelinePath, "--campaign", campaignPath, "--session", sessionPath}, &stdout, &stderr)
+	if code != 0 {
+		t.Fatalf("exit code = %d, want 0; stdout=%q stderr=%q", code, stdout.String(), stderr.String())
+	}
+	if !strings.Contains(stdout.String(), "OK    audio") {
+		t.Fatalf("stdout = %q, want OK audio", stdout.String())
+	}
+}
+
 func TestExecuteLockAndUnlockUseRemoteLockStore(t *testing.T) {
 	workspaceRoot := t.TempDir()
 	pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)

internal/app/remote_session_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -42,6 +44,45 @@ inputs:
 	}
 }
 
+func TestExecuteRemoteSessionFallbackLoadsSecretsBeforeObjectStoreInit(t *testing.T) {
+	workspaceRoot := t.TempDir()
+	pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot)
+	accessKeyEnv := "NARRATIO_TEST_REMOTE_SESSION_KEY_ID"
+	secretKeyEnv := "NARRATIO_TEST_REMOTE_SESSION_SECRET"
+	restoreEnvAfterTest(t, accessKeyEnv, secretKeyEnv)
+	secretsDir := t.TempDir()
+	mustWriteTestFile(t, filepath.Join(secretsDir, accessKeyEnv), "remote-session-key-id\n")
+	mustWriteTestFile(t, filepath.Join(secretsDir, secretKeyEnv), "remote-session-secret\n")
+	addSecretsToPipelineConfig(t, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv)
+
+	fake := &storage.FakeBackend{}
+	seedRemoteSessionConfig(t, fake, "2026-05-03", `session_id: "{{ session_id }}"
+inputs:
+  audio_s3:
+    prefix: audio/
+`)
+	origStoreFn := newObjectStoreFromConfigFn
+	origSessionDefaults := append([]string(nil), config.DefaultSessionConfigSearchPaths...)
+	config.DefaultSessionConfigSearchPaths = []string{filepath.Join(t.TempDir(), "session.yml")}
+	newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) {
+		if os.Getenv(accessKeyEnv) != "remote-session-key-id" || os.Getenv(secretKeyEnv) != "remote-session-secret" {
+			return nil, fmt.Errorf("secrets were not loaded before remote session object store init")
+		}
+		return fake, nil
+	}
+	t.Cleanup(func() {
+		newObjectStoreFromConfigFn = origStoreFn
+		config.DefaultSessionConfigSearchPaths = origSessionDefaults
+	})
+
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	code := Execute([]string{"plan", "--config", pipelinePath, "--campaign", campaignPath, "--session-id", "2026-05-03"}, &stdout, &stderr)
+	if code != 0 {
+		t.Fatalf("exit code = %d, want 0; stdout=%q stderr=%q", code, stdout.String(), stderr.String())
+	}
+}
+
 func TestExecuteExplicitLocalSessionPrecedenceSkipsRemote(t *testing.T) {
 	workspaceRoot := t.TempDir()
 	pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)
@@ -192,3 +233,20 @@ func seedRemoteSessionConfig(t *testing.T, fake *storage.FakeBackend, sessionID,
 	})
 	return remoteKey
 }
+
+func addSecretsToPipelineConfig(t *testing.T, pipelinePath, secretsDir, accessKeyEnv, secretKeyEnv string) {
+	t.Helper()
+	pipelineData, err := os.ReadFile(pipelinePath)
+	if err != nil {
+		t.Fatalf("read pipeline: %v", err)
+	}
+	pipelineYAML := strings.Replace(
+		string(pipelineData),
+		"storage:\n  backend: s3\n  s3:\n    bucket: test-bucket\n",
+		"storage:\n  backend: s3\n  s3:\n    bucket: test-bucket\n    access_key_id_env: "+accessKeyEnv+"\n    secret_access_key_env: "+secretKeyEnv+"\nsecrets:\n  env_dir: "+secretsDir+"\n",
+		1,
+	)
+	if err := os.WriteFile(pipelinePath, []byte(pipelineYAML), 0o644); err != nil {
+		t.Fatalf("write pipeline: %v", err)
+	}
+}

internal/app/restore.go

@@ -67,11 +67,7 @@ func Restore(ctx context.Context, args []string, out io.Writer) error {
 	if err := config.Validate(cfg); err != nil {
 		return fmt.Errorf("restore: %w", err)
 	}
-	if _, err := loadSecretsFromConfig(cfg, logging.NewLogger(os.Stderr, slog.LevelInfo)); err != nil {
+	objectStore, err := newCommandObjectStore(ctx, cfg, logging.NewLogger(os.Stderr, slog.LevelInfo))
-		return fmt.Errorf("restore: %w", err)
-	}
-
-	objectStore, err := newObjectStoreFromConfigFn(ctx, cfg)
 	if err != nil {
 		return fmt.Errorf("restore: %w", err)
 	}

internal/app/runner.go

@@ -87,9 +87,9 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage
 		env.Storage = &storage.NoopBackend{}
 	}
 	if env.ObjectStore == nil && needsObjectStoreForRun(env.Config, stages) {
-		objectStore, err := newObjectStoreFromConfigFn(ctx, env.Config)
+		objectStore, err := newCommandObjectStore(ctx, env.Config, nil)
 		if err != nil {
-			return nil, fmt.Errorf("initialize object store backend: %w", err)
+			return nil, err
 		}
 		env.ObjectStore = objectStore
 	}