diff --git a/docs/cli.md b/docs/cli.md index 8418250..0a14089 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -45,6 +45,7 @@ Most session-aware commands accept: - `--session-id ` - `--previous-session-id ` - `--profile ` +- `--secrets-dir ` Rules: @@ -57,6 +58,9 @@ Rules: may be supplied once; omitted, empty, and the exact name `default` select the base configuration, while an unknown named profile fails. The same selection applies to all common-flag commands, including `regenerate-artifacts`. +- `--secrets-dir` selects ConfigKit credential-file bootstrap explicitly. A + relative value uses caller CWD. Without the flag, a present `secrets/` + directory beside the resolved root pipeline is used; an absent one is skipped. - `clean --all` cannot be combined with campaign/session selectors. - notification delivery is currently limited to the configured `noop` mode; see the [configuration reference](./config.md#notifications). @@ -81,16 +85,18 @@ Commands with additional positionals keep their command-specific order: ### `config validate`, `config show`, `config sources`, and `config diff` ```bash -narratio config validate [--config ] [--campaign | --campaign-file ] [--profile ] -narratio config show [--config ] [--campaign | --campaign-file ] [--profile ] -narratio config sources [--config ] [--campaign | --campaign-file ] [--profile ] -narratio config diff [--config ] [--campaign | --campaign-file ] +narratio config validate [--config ] [--campaign | --campaign-file ] [--profile ] [--secrets-dir ] +narratio config show [--config ] [--campaign | --campaign-file ] [--profile ] [--secrets-dir ] +narratio config sources [--config ] [--campaign | --campaign-file ] [--profile ] [--secrets-dir ] +narratio config diff [--config ] [--campaign | --campaign-file ] [--secrets-dir ] ``` These commands resolve the selected profile, defaults, ordinary paths, and—if a campaign is selected—the campaign-owned party. They neither discover or load -a session nor create a workspace, manifest, run, lock, adapter, remote -connection, or credential environment. +a session nor create a workspace, manifest, run, lock, adapter, or remote +connection. Like every pipeline-loading command, they bootstrap the selected or +conventional secrets directory before resolving configuration; `config show` +never includes bootstrap reports or secret values in its YAML output. Campaign selection is optional for a pipeline without party-driven artifact families. A pipeline with `scriptorium.artifact_families` needs a selected or @@ -320,6 +326,7 @@ Options: - `--config ` - `--campaign ` or `--campaign-file ` +- `--secrets-dir ` - `--previous-session-id ` - `--date ` - `--title ` diff --git a/docs/config.md b/docs/config.md index 3e2545b..4884949 100644 --- a/docs/config.md +++ b/docs/config.md @@ -216,14 +216,21 @@ inputs: - Do not place raw secrets in YAML. - Use env var names in config (for example `pipeline.audita.llm_api_key_env`). -- Optionally load credential files from `pipeline.secrets.env_dir`. Each valid - environment-variable filename supplies one value; trailing CR/LF is removed. +- Every command that loads a pipeline accepts `--secrets-dir `. When the + flag is omitted, Narratio uses a present `secrets/` directory beside the + resolved root `pipeline.yml`; an absent conventional directory is ignored. + Explicit relative paths are resolved from the caller's working directory, + and a missing or empty explicit selection is an error. +- Each immediate regular file whose name is a valid environment-variable name + supplies one value. ConfigKit removes trailing CR/LF, permits a symlink as the + selected directory, rejects symlink entries, and limits input to 1 MiB per + file, 8 MiB total, and 1024 entries. - An existing process environment value takes precedence over a credential file. -- Credential directories and files must not be symlinks and must be regular, - bounded files (at most 8 KiB per value). On POSIX, provision the directory - with no group/other access (normally `0700`) and files with no group/other - access (normally `0600`). -- Commands that need storage/auth load filesystem secrets before constructing adapters. +- On POSIX, operators must provision the directory with no group/other access + (normally `0700`) and files with no group/other access (normally `0600`); + ConfigKit validates type and bounds but does not enforce permission modes. +- Bootstrap happens once per command, before any configuration load or adapter + construction. Safe reports contain filenames, paths, and statuses, never values. ## Publish Configuration Summary @@ -270,7 +277,6 @@ Rules: | `pipeline.workspace.cleanup_after_publish` | bool | No | `false`; selects the published run's workspace target for final cleanup and requires effective `pipeline.publish.enabled=true` and `pipeline.publish.upload_run=true` | | `pipeline.campaigns.root` | string | No | `/usr/local/share/narratio/campaigns` | | `pipeline.campaigns.default_campaign_id` | string | No | empty | -| `pipeline.secrets.env_dir` | string | No | empty | | `pipeline.storage.backend` | string | No | `local`; supported values are `local` and `s3` (case-insensitive) | | `pipeline.storage.s3.bucket` | string | Conditional | required when backend is `s3` and S3 session-audio or publish upload is enabled | | `pipeline.storage.s3.root_prefix` | string | No | `dnd` | diff --git a/docs/operations.md b/docs/operations.md index 72ac160..301a732 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -554,7 +554,17 @@ API keys are credentials, not ordinary workspace data. Store them outside the shared workspace or in a separately restricted credential location; ordinary workspace group access must never be treated as authorization to read keys. Provision a credential directory as `0700` and credential files as `0600`; -Narratio rejects group- or other-readable configured credential paths. +these permission modes are an operator requirement and are not enforced by +ConfigKit. + +Every pipeline-loading command bootstraps credentials once, before the first +configuration load and before any adapter is constructed. `--secrets-dir` +selects an explicit directory (relative paths use caller CWD). Otherwise a +present `secrets/` beside the resolved root pipeline is used, while an absent +conventional directory is skipped. Explicit missing/empty selections and +invalid existing directories fail the command. ConfigKit follows a symlink used +as the selected directory, but immediate entries must be regular non-symlink +files. Existing environment variables—including empty values—always win. External adapter results are individually bounded before Narratio validates or materializes them. These per-file limits do not reserve disk space: prevent hard @@ -617,4 +627,5 @@ normal migration path. - Publish requires prerequisite stages through `render` and `analyze` to be succeeded. - Markdown publish defaults require render outputs (`transcripts/final.md` and `transcripts/final.trimmed.md`). - Restore requires configured object storage and committed remote current state. -- Storage-backed commands load filesystem secrets before object-store initialization. +- Pipeline-loading commands bootstrap filesystem secrets once before configuration + resolution and object-store initialization. diff --git a/docs/policy/architecture.md b/docs/policy/architecture.md index 1e2aa63..2b507da 100644 --- a/docs/policy/architecture.md +++ b/docs/policy/architecture.md @@ -247,10 +247,13 @@ uses a separate restrictive contract. Raw secrets must not be stored in pipeline, campaign, or session YAML or written to manifests, logs, generated configuration, reports, publish metadata, -documentation, or examples. Secrets enter through configured environment -variable names or secret-file references. Diagnostics should avoid transcript -and prompt content unless a deliberate, bounded inspection mechanism requires -it. +documentation, or examples. Pipeline configuration names environment variables; +command-owned startup may populate previously unset variables from ConfigKit's +explicit or conventional secrets directory. This bootstrap occurs once per +invocation before any typed configuration load or adapter construction and is +not hidden inside library loaders. Safe bootstrap diagnostics may identify +files and statuses but never values. Diagnostics should avoid transcript and +prompt content unless a deliberate, bounded inspection mechanism requires it. Logs, reports, generated invocation files, generated configuration, and render debug files are diagnostics, not canonical pipeline products. They should be diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8542cae..aa006f2 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -282,7 +282,10 @@ prepare/extract, and application loading tests pass. ## Stage 6 — Move secrets bootstrap before all configuration loads -Status: Pending +Status: Complete — Added invocation-scoped ConfigKit secrets bootstrap before +all pipeline loads and adapter construction, exposed consistent explicit and +conventional directory selection across the CLI, removed the retired pipeline +schema and custom loader, and verified focused, full, and race test suites. Read: roadmap “Credential and Secrets Convention”; ConfigKit's bootstrap contract; `internal/app/secrets_env*.go`, `config_loader.go`, `config_commands.go`, diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 191e591..5f09fac 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -717,7 +717,8 @@ Symptom: Likely causes: -- invalid `pipeline.secrets.env_dir`; +- missing, unreadable, or invalid explicit `--secrets-dir`; +- invalid entries in the conventional `secrets/` directory beside the root pipeline; - missing credential environment variables; - invalid S3 endpoint/bucket settings. @@ -730,7 +731,7 @@ env | sed 's/=.*//' | grep -E 'OBJECT_STORAGE|AWS|AUDITA|SCRIPTORIUM' Safe fix: -- correct secret-file path and permissions; +- correct `--secrets-dir`, secret-file type/name, and operator-managed permissions; - provide required env vars; - keep secret values out of YAML. diff --git a/examples/pipeline.full.annotated.yml b/examples/pipeline.full.annotated.yml index c215464..0277061 100644 --- a/examples/pipeline.full.annotated.yml +++ b/examples/pipeline.full.annotated.yml @@ -7,9 +7,8 @@ workspace: # Optional: remove run-scoped workdir after successful publish commit. cleanup_after_publish: false -# Optional: local secret file loader (directory of ENV_VAR_NAME files). -# secrets: -# env_dir: ./secrets +# Credential files, when used, belong in a sibling secrets/ directory or in the +# directory selected with --secrets-dir; they are not pipeline configuration. storage: # Defaults to "local". Use "s3" explicitly for publish + S3 audio workflows. diff --git a/internal/app/clean.go b/internal/app/clean.go index 6bfdaf7..496077b 100644 --- a/internal/app/clean.go +++ b/internal/app/clean.go @@ -45,7 +45,7 @@ func cleanSessionWithDependencies(ctx context.Context, flags commonConfigFlags, if strings.TrimSpace(flags.sessionID) == "" { return fmt.Errorf("clean: session_id is required unless --all is set") } - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { return fmt.Errorf("clean: %w", err) } @@ -93,7 +93,7 @@ func cleanAllLocalWithDependencies(flags commonConfigFlags, dryRun, clearCache b strings.TrimSpace(flags.previousSessionID) != "" { return fmt.Errorf("clean: --all cannot be combined with --campaign, --campaign-file, --session, a session_id, or --previous-session-id") } - _, pipelineCfg, err := loadPipelineConfigWithDependencies(flags.pipelinePath, config.PipelineLoadOptions{Profile: flags.profile.pointer()}, deps) + _, pipelineCfg, err := loadPipelineConfigWithDependencies(flags.pipelinePath, config.PipelineLoadOptions{Profile: flags.profile.pointer()}, flags.secretsDir.pointer(), deps) if err != nil { return fmt.Errorf("clean: %w", err) } diff --git a/internal/app/command_dependencies.go b/internal/app/command_dependencies.go index 22443f7..77ef66e 100644 --- a/internal/app/command_dependencies.go +++ b/internal/app/command_dependencies.go @@ -2,8 +2,8 @@ package app import ( "context" - "os" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" "gitea.maximumdirect.net/eric/narratio/internal/config" ) @@ -11,7 +11,8 @@ import ( type commandDependencies struct { loadPipelineConfig func(string, config.PipelineLoadOptions) (*config.PipelineConfig, error) downloadObjectToTemp func(context.Context, storage.ObjectStore, string, string) (string, error) - readSecretsDirectory func(string) ([]os.DirEntry, error) + loadSecretsDir func(string) (*configkit.SecretsReport, error) + secretsBootstrap *secretsBootstrap newObjectStore func(context.Context, *config.Config) (storage.ObjectStore, error) discoverRemoteCurrentState func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) buildRestorePlan func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) @@ -25,8 +26,11 @@ func normalizeCommandDependencies(deps commandDependencies) commandDependencies if deps.downloadObjectToTemp == nil { deps.downloadObjectToTemp = storage.DownloadObjectToTemp } - if deps.readSecretsDirectory == nil { - deps.readSecretsDirectory = os.ReadDir + if deps.loadSecretsDir == nil { + deps.loadSecretsDir = configkit.LoadSecretsDir + } + if deps.secretsBootstrap == nil { + deps.secretsBootstrap = &secretsBootstrap{} } if deps.newObjectStore == nil { deps.newObjectStore = storage.NewObjectStoreFromConfig diff --git a/internal/app/commands_test.go b/internal/app/commands_test.go index 666ed17..51dc05b 100644 --- a/internal/app/commands_test.go +++ b/internal/app/commands_test.go @@ -192,7 +192,7 @@ func TestExecuteRunStagePolishLoadsCredentialFromSecretsDir(t *testing.T) { configDir := t.TempDir() sessionID := "2026-05-03" secretsDir := filepath.Join(configDir, "secrets") - if err := os.MkdirAll(secretsDir, secretDirectoryPrivateMode); err != nil { + if err := os.MkdirAll(secretsDir, 0o755); err != nil { t.Fatalf("MkdirAll(%q): %v", secretsDir, err) } if err := os.WriteFile(filepath.Join(secretsDir, "OPENROUTER_API_KEY"), []byte("from-secret-file\n"), 0o600); err != nil { @@ -211,8 +211,6 @@ func TestExecuteRunStagePolishLoadsCredentialFromSecretsDir(t *testing.T) { root: ` + workspaceRoot + ` storage: backend: local -secrets: - env_dir: ./secrets whisperx: transcribe_url: https://example.com/transcribe seriatim: @@ -266,7 +264,7 @@ inputs: } } -func TestExecuteRunFailsWhenConfiguredSecretsDirMissing(t *testing.T) { +func TestExecuteRunFailsWhenExplicitSecretsDirMissing(t *testing.T) { workspaceRoot := t.TempDir() configDir := t.TempDir() pipelinePath := filepath.Join(configDir, "pipeline.yml") @@ -277,8 +275,6 @@ func TestExecuteRunFailsWhenConfiguredSecretsDirMissing(t *testing.T) { root: ` + workspaceRoot + ` storage: backend: local -secrets: - env_dir: ./missing-secrets whisperx: transcribe_url: https://example.com/transcribe seriatim: @@ -305,11 +301,11 @@ inputs: var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"run", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) + code := Execute([]string{"run", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--secrets-dir", filepath.Join(configDir, "missing-secrets")}, &stdout, &stderr) if code == 0 { t.Fatal("exit code = 0, want non-zero") } - if !strings.Contains(stderr.String(), "validate secrets env_dir") { + if !strings.Contains(stderr.String(), "bootstrap secrets directory") { t.Fatalf("stderr = %q, want secrets validation error context", stderr.String()) } } diff --git a/internal/app/config_commands.go b/internal/app/config_commands.go index 72c20be..b163570 100644 --- a/internal/app/config_commands.go +++ b/internal/app/config_commands.go @@ -17,6 +17,7 @@ type inspectionFlags struct { campaignPath string campaignFilePath string profile singletonStringFlag + secretsDir singletonStringFlag } type inspectionConfig struct { @@ -49,7 +50,7 @@ func configWithDependencies(ctx context.Context, args []string, out io.Writer, d case "sources": return configSourcesWithDependencies(ctx, args[1:], out, deps) case "diff": - return ConfigDiff(ctx, args[1:], out) + return configDiffWithDependencies(ctx, args[1:], out, deps) default: return fmt.Errorf("config: unknown subcommand %q", args[0]) } @@ -148,8 +149,10 @@ func parseInspectionFlags(command string, args []string, out io.Writer) (inspect fs.StringVar(&flags.campaignFilePath, "campaign-file", "", "path to campaign.yml") flags.profile.name = "profile" fs.Var(&flags.profile, "profile", "named pipeline profile") + flags.secretsDir.name = "secrets-dir" + fs.Var(&flags.secretsDir, "secrets-dir", "secrets directory (optional; defaults beside pipeline.yml)") fs.Usage = func() { - fmt.Fprintf(out, "Usage: narratio %s [--config ] [--campaign | --campaign-file ] [--profile ]\n\n", command) + fmt.Fprintf(out, "Usage: narratio %s [--config ] [--campaign | --campaign-file ] [--profile ] [--secrets-dir ]\n\n", command) fmt.Fprintln(out, "Resolves configuration without a session, workspace, manifest, adapters, or external services.") fmt.Fprintln(out) fmt.Fprintln(out, "Flags:") @@ -168,7 +171,7 @@ func parseInspectionFlags(command string, args []string, out io.Writer) (inspect } func resolveInspectionConfigWithDependencies(flags inspectionFlags, deps commandDependencies) (*inspectionConfig, error) { - pipelinePath, pipeline, err := loadPipelineConfigWithDependencies(flags.pipelinePath, config.PipelineLoadOptions{Profile: flags.profile.pointer()}, deps) + pipelinePath, pipeline, err := loadPipelineConfigWithDependencies(flags.pipelinePath, config.PipelineLoadOptions{Profile: flags.profile.pointer()}, flags.secretsDir.pointer(), deps) if err != nil { return nil, err } diff --git a/internal/app/config_commands_test.go b/internal/app/config_commands_test.go index a3ca888..aa2850b 100644 --- a/internal/app/config_commands_test.go +++ b/internal/app/config_commands_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/narratio/internal/config" ) @@ -91,8 +92,6 @@ func TestConfigValidateAndShowPipelineOnlyAreSideEffectFree(t *testing.T) { defer server.Close() mustWriteTestFile(t, pipelinePath, `workspace: root: `+workspaceRoot+` -secrets: - env_dir: `+secretsDir+` whisperx: transcribe_url: `+server.URL+` `) @@ -276,6 +275,30 @@ func TestConfigDiffReportsEqualityAndRejectsInvalidInput(t *testing.T) { } } +func TestConfigDiffBootstrapsSecretsOnceForBothProfiles(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, _, _ := writeValidConfigFiles(t, workspaceRoot) + writeInspectionProfiles(t, pipelinePath, "whisperx:\n language: en\n", "whisperx:\n language: fr\n") + secretsDir := t.TempDir() + bootstrapCalls := 0 + deps := commandDependencies{loadSecretsDir: func(got string) (*configkit.SecretsReport, error) { + bootstrapCalls++ + if got != secretsDir { + t.Fatalf("secrets directory = %q, want %q", got, secretsDir) + } + return nil, nil + }} + + if err := configDiffWithDependencies(context.Background(), []string{ + "production", "testing", "--config", pipelinePath, "--secrets-dir", secretsDir, + }, io.Discard, deps); err != nil { + t.Fatalf("config diff: %v", err) + } + if bootstrapCalls != 1 { + t.Fatalf("bootstrap calls = %d, want 1", bootstrapCalls) + } +} + func TestConfigCommandsInspectMaintainedSplitBundleWithoutRuntimeState(t *testing.T) { examplesDir := filepath.Join("..", "..", "examples") pipelinePath := filepath.Join(examplesDir, "production-testing", "pipeline.yml") diff --git a/internal/app/config_diff.go b/internal/app/config_diff.go index 9f4b05c..d5528b7 100644 --- a/internal/app/config_diff.go +++ b/internal/app/config_diff.go @@ -16,6 +16,7 @@ type configDiffFlags struct { pipelinePath singletonStringFlag campaignPath singletonStringFlag campaignFilePath singletonStringFlag + secretsDir singletonStringFlag } type configDiffRequest struct { @@ -40,7 +41,11 @@ type configDiffRecord struct { // read-only configuration resolution boundary used by config validate, show, // and sources. Differences describe normalized effective values rather than // the layout of root, import, or overlay files. -func ConfigDiff(_ context.Context, args []string, out io.Writer) error { +func ConfigDiff(ctx context.Context, args []string, out io.Writer) error { + return configDiffWithDependencies(ctx, args, out, commandDependencies{}) +} + +func configDiffWithDependencies(_ context.Context, args []string, out io.Writer, deps commandDependencies) error { request, err := parseConfigDiffRequest(args, out) if err != nil { if errors.Is(err, flag.ErrHelp) { @@ -48,7 +53,7 @@ func ConfigDiff(_ context.Context, args []string, out io.Writer) error { } return err } - resolved, err := resolveConfigDiff(request) + resolved, err := resolveConfigDiff(request, deps) if err != nil { return fmt.Errorf("config diff: %w", err) } @@ -102,11 +107,13 @@ func parseConfigDiffRequest(args []string, out io.Writer) (configDiffRequest, er request.flags.pipelinePath.name = "config" request.flags.campaignPath.name = "campaign" request.flags.campaignFilePath.name = "campaign-file" + request.flags.secretsDir.name = "secrets-dir" fs.Var(&request.flags.pipelinePath, "config", "path to pipeline.yml (optional; defaults searched)") fs.Var(&request.flags.campaignPath, "campaign", "campaign ID") fs.Var(&request.flags.campaignFilePath, "campaign-file", "path to campaign.yml") + fs.Var(&request.flags.secretsDir, "secrets-dir", "secrets directory (optional; defaults beside pipeline.yml)") fs.Usage = func() { - fmt.Fprintln(out, "Usage: narratio config diff [--config ] [--campaign | --campaign-file ]") + fmt.Fprintln(out, "Usage: narratio config diff [--config ] [--campaign | --campaign-file ] [--secrets-dir ]") fmt.Fprintln(out) fmt.Fprintln(out, "Compares fully resolved profiles without a session, workspace, manifest, adapters, or external services.") fmt.Fprintln(out) @@ -120,6 +127,9 @@ func parseConfigDiffRequest(args []string, out io.Writer) (configDiffRequest, er if len(profiles) != 2 || strings.TrimSpace(profiles[0]) == "" || strings.TrimSpace(profiles[1]) == "" { return configDiffRequest{}, fmt.Errorf("config diff: exactly two non-empty profile names are required") } + if profiles[0] == profiles[1] { + return configDiffRequest{}, fmt.Errorf("config diff: profile names must differ") + } if request.flags.campaignPath.set && request.flags.campaignFilePath.set { return configDiffRequest{}, fmt.Errorf("config diff: --campaign and --campaign-file are mutually exclusive") } @@ -156,21 +166,22 @@ func splitConfigDiffArguments(args []string) (profiles, flagArgs []string) { func configDiffStringFlag(name string) bool { switch name { - case "config", "campaign", "campaign-file", "profile": + case "config", "campaign", "campaign-file", "profile", "secrets-dir": return true default: return false } } -func resolveConfigDiff(request configDiffRequest) (*configDiffResolved, error) { - pipelinePath, err := resolvePipelineConfigPath(request.flags.pipelinePath.value) +func resolveConfigDiff(request configDiffRequest, deps commandDependencies) (*configDiffResolved, error) { + deps = normalizeCommandDependencies(deps) + pipelinePath, leftPipeline, err := loadPipelineConfigWithDependencies(request.flags.pipelinePath.value, config.PipelineLoadOptions{Profile: &request.leftProfile}, request.flags.secretsDir.pointer(), deps) if err != nil { - return nil, err + return nil, fmt.Errorf("load left pipeline profile: %w", err) } - leftPipeline, rightPipeline, err := config.LoadPipelineProfilePair(pipelinePath, request.leftProfile, request.rightProfile) + _, rightPipeline, err := loadPipelineConfigWithDependencies(request.flags.pipelinePath.value, config.PipelineLoadOptions{Profile: &request.rightProfile}, request.flags.secretsDir.pointer(), deps) if err != nil { - return nil, err + return nil, fmt.Errorf("load right pipeline profile: %w", err) } needsCampaign := pipelineHasArtifactFamilies(leftPipeline) || pipelineHasArtifactFamilies(rightPipeline) hasCampaignSelection := request.flags.campaignPath.set || request.flags.campaignFilePath.set diff --git a/internal/app/config_loader.go b/internal/app/config_loader.go index 160c08f..94644c5 100644 --- a/internal/app/config_loader.go +++ b/internal/app/config_loader.go @@ -34,7 +34,7 @@ func retainedCommandConfig(cfg *config.Config) *commandConfig { return &commandConfig{Config: cfg} } -func loadCommandConfigWithDependencies(ctx context.Context, pipelineFlag, campaignFlag, campaignFileFlag, sessionFlag string, sessionOpts config.SessionLoadOptions, deps commandDependencies) (loaded *commandConfig, err error) { +func loadCommandConfigWithDependencies(ctx context.Context, pipelineFlag, campaignFlag, campaignFileFlag, sessionFlag string, sessionOpts config.SessionLoadOptions, secretsDir *string, deps commandDependencies) (loaded *commandConfig, err error) { deps = normalizeCommandDependencies(deps) var cleanup func() error defer func() { @@ -46,7 +46,7 @@ func loadCommandConfigWithDependencies(ctx context.Context, pipelineFlag, campai } }() - base, err := loadPipelineCampaignConfigWithDependencies(pipelineFlag, campaignFlag, campaignFileFlag, config.PipelineLoadOptions{Profile: sessionOpts.Profile}, deps) + base, err := loadPipelineCampaignConfigWithDependencies(pipelineFlag, campaignFlag, campaignFileFlag, config.PipelineLoadOptions{Profile: sessionOpts.Profile}, secretsDir, deps) if err != nil { return nil, err } @@ -135,9 +135,9 @@ func loadCommandConfigWithDependencies(ctx context.Context, pipelineFlag, campai return loaded, nil } -func loadPipelineCampaignConfigWithDependencies(pipelineFlag, campaignFlag, campaignFileFlag string, pipelineOpts config.PipelineLoadOptions, deps commandDependencies) (*pipelineCampaignConfig, error) { +func loadPipelineCampaignConfigWithDependencies(pipelineFlag, campaignFlag, campaignFileFlag string, pipelineOpts config.PipelineLoadOptions, secretsDir *string, deps commandDependencies) (*pipelineCampaignConfig, error) { deps = normalizeCommandDependencies(deps) - loadedPipelinePath, pipelineCfg, err := loadPipelineConfigWithDependencies(pipelineFlag, pipelineOpts, deps) + loadedPipelinePath, pipelineCfg, err := loadPipelineConfigWithDependencies(pipelineFlag, pipelineOpts, secretsDir, deps) if err != nil { return nil, err } @@ -161,12 +161,19 @@ func loadPipelineCampaignConfigWithDependencies(pipelineFlag, campaignFlag, camp return &loaded, nil } -func loadPipelineConfigWithDependencies(pipelineFlag string, opts config.PipelineLoadOptions, deps commandDependencies) (string, *config.PipelineConfig, error) { +func loadPipelineConfigWithDependencies(pipelineFlag string, opts config.PipelineLoadOptions, secretsDir *string, deps commandDependencies) (string, *config.PipelineConfig, error) { deps = normalizeCommandDependencies(deps) resolvedPipelinePath, err := resolvePipelineConfigPath(pipelineFlag) if err != nil { return "", nil, err } + if !deps.secretsBootstrap.selectionSet { + deps.secretsBootstrap.selection = secretsDir + deps.secretsBootstrap.selectionSet = true + } + if err := deps.secretsBootstrap.load(resolvedPipelinePath, deps.loadSecretsDir); err != nil { + return "", nil, err + } pipelineCfg, err := deps.loadPipelineConfig(resolvedPipelinePath, opts) if err != nil { return "", nil, err diff --git a/internal/app/config_loader_test.go b/internal/app/config_loader_test.go index 00f0e54..32a2ad1 100644 --- a/internal/app/config_loader_test.go +++ b/internal/app/config_loader_test.go @@ -35,7 +35,7 @@ func TestLoadCommandConfigRetainsInitiallyLoadedPipeline(t *testing.T) { }, } - loaded, err := loadCommandConfigWithDependencies(context.Background(), pipelinePath, "", campaignPath, sessionPath, config.SessionLoadOptions{SessionID: "2026-05-03"}, deps) + loaded, err := loadCommandConfigWithDependencies(context.Background(), pipelinePath, "", campaignPath, sessionPath, config.SessionLoadOptions{SessionID: "2026-05-03"}, nil, deps) if err != nil { t.Fatalf("loadCommandConfig() error = %v", err) } diff --git a/internal/app/object_store.go b/internal/app/object_store.go index f230e42..376d071 100644 --- a/internal/app/object_store.go +++ b/internal/app/object_store.go @@ -11,9 +11,6 @@ import ( func newCommandObjectStoreWithDependencies(ctx context.Context, cfg *config.Config, logger *slog.Logger, deps commandDependencies) (storage.ObjectStore, error) { deps = normalizeCommandDependencies(deps) - if _, err := loadSecretsFromConfigWithDependencies(cfg, logger, deps); err != nil { - return nil, fmt.Errorf("load secrets from files: %w", err) - } return initializeObjectStoreBackend(ctx, cfg, deps.newObjectStore) } diff --git a/internal/app/object_store_test.go b/internal/app/object_store_test.go index defcb4c..eb807d0 100644 --- a/internal/app/object_store_test.go +++ b/internal/app/object_store_test.go @@ -3,8 +3,6 @@ package app import ( "context" "errors" - "os" - "path/filepath" "strings" "testing" @@ -12,89 +10,8 @@ import ( "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 - deps := commandDependencies{newObjectStore: 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 - }} - - store, err := newCommandObjectStoreWithDependencies(context.Background(), cfg, nil, deps) - 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) - deps := commandDependencies{newObjectStore: 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 - }} - - if _, err := newCommandObjectStoreWithDependencies(context.Background(), cfg, nil, deps); err != nil { - t.Fatalf("newCommandObjectStore() error = %v", err) - } -} - -func TestNewCommandObjectStoreSecretErrorStopsFactory(t *testing.T) { - cfg := commandObjectStoreTestConfig(filepath.Join(t.TempDir(), "missing")) - called := false - deps := commandDependencies{newObjectStore: func(context.Context, *config.Config) (storage.ObjectStore, error) { - called = true - return &storage.FakeBackend{}, nil - }} - - _, err := newCommandObjectStoreWithDependencies(context.Background(), cfg, nil, deps) - 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("") + cfg := commandObjectStoreTestConfig() deps := commandDependencies{newObjectStore: func(context.Context, *config.Config) (storage.ObjectStore, error) { return nil, errors.New("factory boom") }} @@ -108,8 +25,8 @@ func TestNewCommandObjectStoreFactoryErrorIsContextual(t *testing.T) { } } -func commandObjectStoreTestConfig(secretsDir string) *config.Config { - cfg := &config.Config{ +func commandObjectStoreTestConfig() *config.Config { + return &config.Config{ Pipeline: &config.PipelineConfig{ Storage: config.StorageConfig{ Backend: "s3", @@ -121,8 +38,4 @@ func commandObjectStoreTestConfig(secretsDir string) *config.Config { }, }, } - if strings.TrimSpace(secretsDir) != "" { - cfg.Pipeline.Secrets = &config.SecretsConfig{EnvDir: secretsDir} - } - return cfg } diff --git a/internal/app/operator_helpers.go b/internal/app/operator_helpers.go index caf9dc0..370531d 100644 --- a/internal/app/operator_helpers.go +++ b/internal/app/operator_helpers.go @@ -24,6 +24,7 @@ type commonConfigFlags struct { sessionID string previousSessionID string profile singletonStringFlag + secretsDir singletonStringFlag } func addCommonConfigFlags(fs *flag.FlagSet, flags *commonConfigFlags) { @@ -35,6 +36,8 @@ func addCommonConfigFlags(fs *flag.FlagSet, flags *commonConfigFlags) { fs.StringVar(&flags.previousSessionID, "previous-session-id", "", "expected previous session identifier") flags.profile.name = "profile" fs.Var(&flags.profile, "profile", "named pipeline profile") + flags.secretsDir.name = "secrets-dir" + fs.Var(&flags.secretsDir, "secrets-dir", "secrets directory (optional; defaults beside pipeline.yml)") } func (f commonConfigFlags) sessionOptions() config.SessionLoadOptions { @@ -112,7 +115,7 @@ func artifactsWithDependencies(ctx context.Context, args []string, out io.Writer func loadHelperContextWithDependencies(ctx context.Context, flags commonConfigFlags, needStore bool, deps commandDependencies) (*config.Config, storage.ObjectStore, []artifacts.RemoteLock, *manifest.Manifest, func(), error) { deps = normalizeCommandDependencies(deps) - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { return nil, nil, nil, nil, nil, err } diff --git a/internal/app/operator_locks.go b/internal/app/operator_locks.go index 2596806..bf09990 100644 --- a/internal/app/operator_locks.go +++ b/internal/app/operator_locks.go @@ -131,7 +131,7 @@ func locksRemoveWithDependencies(ctx context.Context, args []string, out io.Writ func loadLockCommandContextWithDependencies(ctx context.Context, flags commonConfigFlags, deps commandDependencies) (*config.Config, storage.ObjectStore, string, func(), error) { deps = normalizeCommandDependencies(deps) - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { return nil, nil, "", nil, err } diff --git a/internal/app/operator_session_init.go b/internal/app/operator_session_init.go index 091f652..92fca1f 100644 --- a/internal/app/operator_session_init.go +++ b/internal/app/operator_session_init.go @@ -28,6 +28,7 @@ func sessionInitWithDependencies(ctx context.Context, args []string, out io.Writ fs.SetOutput(io.Discard) var pipelinePath, campaignPath, campaignFilePath, sessionID, previousSessionID, date, title, output, audioS3Prefix, audioDir string var profile singletonStringFlag + var secretsDir singletonStringFlag var remote, force bool fs.StringVar(&pipelinePath, "config", "", "path to pipeline.yml (optional; defaults searched)") fs.StringVar(&campaignPath, "campaign", "", "campaign ID") @@ -41,6 +42,8 @@ func sessionInitWithDependencies(ctx context.Context, args []string, out io.Writ fs.StringVar(&audioDir, "audio-dir", "", "local audio directory") profile.name = "profile" fs.Var(&profile, "profile", "named pipeline profile") + secretsDir.name = "secrets-dir" + fs.Var(&secretsDir, "secrets-dir", "secrets directory (optional; defaults beside pipeline.yml)") fs.BoolVar(&remote, "remote", false, "write session.yml to S3 session prefix") fs.BoolVar(&force, "force", false, "overwrite existing target") if err := parseSessionAwareFlags("session init", fs, args, &sessionID); err != nil { @@ -56,7 +59,7 @@ func sessionInitWithDependencies(ctx context.Context, args []string, out io.Writ return fmt.Errorf("session init: --audio-dir and --audio-s3-prefix are mutually exclusive") } - base, err := loadPipelineCampaignConfigWithDependencies(pipelinePath, campaignPath, campaignFilePath, config.PipelineLoadOptions{Profile: profile.pointer()}, deps) + base, err := loadPipelineCampaignConfigWithDependencies(pipelinePath, campaignPath, campaignFilePath, config.PipelineLoadOptions{Profile: profile.pointer()}, secretsDir.pointer(), deps) if err != nil { return fmt.Errorf("session init: %w", err) } diff --git a/internal/app/operator_session_validate.go b/internal/app/operator_session_validate.go index d513a0a..dd69337 100644 --- a/internal/app/operator_session_validate.go +++ b/internal/app/operator_session_validate.go @@ -30,7 +30,7 @@ func sessionValidateWithDependencies(ctx context.Context, args []string, out io. } findings := []finding{} - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { findings = append(findings, errorFinding("config", err.Error())) return renderFindings(out, "", "", findings) diff --git a/internal/app/operator_status.go b/internal/app/operator_status.go index a2d9e50..e8e8111 100644 --- a/internal/app/operator_status.go +++ b/internal/app/operator_status.go @@ -31,7 +31,7 @@ func statusWithDependencies(ctx context.Context, args []string, out io.Writer, d if strings.TrimSpace(flags.sessionID) == "" { return fmt.Errorf("status: session_id is required") } - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { return fmt.Errorf("status: %w", err) } diff --git a/internal/app/plan.go b/internal/app/plan.go index 0f14edd..248eb30 100644 --- a/internal/app/plan.go +++ b/internal/app/plan.go @@ -31,7 +31,7 @@ func planWithDependencies(ctx context.Context, args []string, out io.Writer, dep return err } flags := request.Config - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { return fmt.Errorf("plan: %w", err) } diff --git a/internal/app/plan_test.go b/internal/app/plan_test.go index 74246bf..49a0871 100644 --- a/internal/app/plan_test.go +++ b/internal/app/plan_test.go @@ -88,7 +88,7 @@ func TestPlanShowsRunAndSkipFromManifest(t *testing.T) { } } -func TestPlanDoesNotLoadConfiguredSecrets(t *testing.T) { +func TestPlanSkipsAbsentConventionalSecretsDirectory(t *testing.T) { workspaceRoot := t.TempDir() configDir := t.TempDir() pipelinePath := filepath.Join(configDir, "pipeline.yml") @@ -99,8 +99,6 @@ func TestPlanDoesNotLoadConfiguredSecrets(t *testing.T) { root: ` + workspaceRoot + ` storage: backend: local -secrets: - env_dir: ./missing-secrets whisperx: transcribe_url: https://example.com/transcribe seriatim: @@ -128,7 +126,7 @@ inputs: var out bytes.Buffer err := Plan(context.Background(), []string{"2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &out) if err != nil { - t.Fatalf("Plan() error = %v, want missing runtime secrets ignored", err) + t.Fatalf("Plan() error = %v, want absent conventional secrets directory skipped", err) } } diff --git a/internal/app/remote_session_test.go b/internal/app/remote_session_test.go index 5629318..7628b33 100644 --- a/internal/app/remote_session_test.go +++ b/internal/app/remote_session_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" @@ -78,7 +79,7 @@ inputs: setSessionConfigSearchPaths(t, []string{filepath.Join(t.TempDir(), "session.yml")}) deps, storeInitCalls := remoteSessionLoaderDependencies(fake) - loaded, err := loadCommandConfigWithDependencies(context.Background(), pipelinePath, "", campaignPath, "", config.SessionLoadOptions{SessionID: "2026-05-03"}, deps) + loaded, err := loadCommandConfigWithDependencies(context.Background(), pipelinePath, "", campaignPath, "", config.SessionLoadOptions{SessionID: "2026-05-03"}, nil, deps) if err != nil { t.Fatalf("loadCommandConfig() error = %v", err) } @@ -200,7 +201,7 @@ inputs: var downloadedPath string deps.downloadObjectToTemp = captureRemoteSessionTempPath(&downloadedPath) - loaded, err := loadCommandConfigWithDependencies(context.Background(), pipelinePath, "", campaignPath, "", config.SessionLoadOptions{SessionID: "2026-05-03"}, deps) + loaded, err := loadCommandConfigWithDependencies(context.Background(), pipelinePath, "", campaignPath, "", config.SessionLoadOptions{SessionID: "2026-05-03"}, nil, deps) if err != nil { t.Fatalf("loadCommandConfig() error = %v", err) } @@ -246,12 +247,18 @@ inputs: prefix: audio/ `) setSessionConfigSearchPaths(t, []string{filepath.Join(t.TempDir(), "session.yml")}) - deps := commandDependencies{newObjectStore: 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 - }} + bootstrapCalls := 0 + deps := commandDependencies{ + loadSecretsDir: func(directory string) (*configkit.SecretsReport, error) { + bootstrapCalls++ + return configkit.LoadSecretsDir(directory) + }, + newObjectStore: 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 + }} var stdout bytes.Buffer var stderr bytes.Buffer @@ -259,6 +266,9 @@ inputs: if code != 0 { t.Fatalf("exit code = %d, want 0; stdout=%q stderr=%q", code, stdout.String(), stderr.String()) } + if bootstrapCalls != 1 { + t.Fatalf("bootstrap calls = %d, want 1", bootstrapCalls) + } } func TestExecuteExplicitLocalSessionPrecedenceSkipsRemote(t *testing.T) { @@ -468,10 +478,13 @@ func addSecretsToPipelineConfig(t *testing.T, pipelinePath, secretsDir, accessKe 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", + "storage:\n backend: s3\n s3:\n bucket: test-bucket\n access_key_id_env: "+accessKeyEnv+"\n secret_access_key_env: "+secretKeyEnv+"\n", 1, ) if err := os.WriteFile(pipelinePath, []byte(pipelineYAML), 0o644); err != nil { t.Fatalf("write pipeline: %v", err) } + if err := os.Symlink(secretsDir, filepath.Join(filepath.Dir(pipelinePath), "secrets")); err != nil { + t.Fatalf("link conventional secrets directory: %v", err) + } } diff --git a/internal/app/restore.go b/internal/app/restore.go index 311cf1f..70dfebb 100644 --- a/internal/app/restore.go +++ b/internal/app/restore.go @@ -53,7 +53,7 @@ func restoreWithDependencies(ctx context.Context, args []string, out io.Writer, if strings.TrimSpace(flags.sessionID) == "" { return fmt.Errorf("restore: session_id is required") } - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { return fmt.Errorf("restore: %w", err) } diff --git a/internal/app/restore_test.go b/internal/app/restore_test.go index fa71e63..5b4ee51 100644 --- a/internal/app/restore_test.go +++ b/internal/app/restore_test.go @@ -179,13 +179,8 @@ func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { secretsDir := filepath.Join(t.TempDir(), "secrets") mustWriteSecretFile(t, filepath.Join(secretsDir, accessKeyEnv), "test-access-key-id\n") mustWriteSecretFile(t, filepath.Join(secretsDir, secretKeyEnv), "test-secret-key\n") - f, err := os.OpenFile(pipelinePath, os.O_APPEND|os.O_WRONLY, 0) - if err != nil { - t.Fatalf("open pipeline config for append: %v", err) - } - defer f.Close() - if _, err := f.WriteString("\nsecrets:\n env_dir: " + secretsDir + "\n"); err != nil { - t.Fatalf("append secrets config: %v", err) + if err := os.Symlink(secretsDir, filepath.Join(filepath.Dir(pipelinePath), "secrets")); err != nil { + t.Fatalf("link conventional secrets directory: %v", err) } storeInitCalled := false diff --git a/internal/app/run.go b/internal/app/run.go index b18f69c..309a4de 100644 --- a/internal/app/run.go +++ b/internal/app/run.go @@ -29,7 +29,7 @@ func runWithDependencies(ctx context.Context, args []string, out io.Writer, exec return err } flags := request.Config - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), flags.secretsDir.pointer(), deps) if err != nil { return fmt.Errorf("run: %w", err) } diff --git a/internal/app/run_stage.go b/internal/app/run_stage.go index f4ba0e7..3bc5ada 100644 --- a/internal/app/run_stage.go +++ b/internal/app/run_stage.go @@ -80,6 +80,8 @@ func runStageWithDependencies(ctx context.Context, args []string, out io.Writer, SessionPath: flags.sessionPath, SessionID: flags.sessionID, PreviousSessionID: flags.previousSessionID, + Profile: flags.profile.pointer(), + SecretsDir: flags.secretsDir.pointer(), Force: force, SelectedArtifacts: normalizedArtifacts, }, executor, deps) @@ -137,6 +139,8 @@ func analyzeWithDependencies(ctx context.Context, args []string, out io.Writer, SessionPath: flags.sessionPath, SessionID: flags.sessionID, PreviousSessionID: flags.previousSessionID, + Profile: flags.profile.pointer(), + SecretsDir: flags.secretsDir.pointer(), Force: true, SelectedArtifacts: normalizedArtifacts, }, executor, deps) @@ -192,6 +196,8 @@ func publishWithDependencies(ctx context.Context, args []string, out io.Writer, SessionPath: flags.sessionPath, SessionID: flags.sessionID, PreviousSessionID: flags.previousSessionID, + Profile: flags.profile.pointer(), + SecretsDir: flags.secretsDir.pointer(), Force: true, SelectedArtifacts: normalizedArtifacts, }, executor, deps) @@ -218,6 +224,8 @@ type singleStageCommand struct { SessionPath string SessionID string PreviousSessionID string + Profile *string + SecretsDir *string Force bool SelectedArtifacts []string } @@ -232,7 +240,8 @@ func runSingleStageCommand(ctx context.Context, req singleStageCommand, executor loaded, err := loadCommandConfigWithDependencies(ctx, req.PipelinePath, req.CampaignPath, req.CampaignFilePath, req.SessionPath, config.SessionLoadOptions{ SessionID: req.SessionID, PreviousSessionID: req.PreviousSessionID, - }, deps) + Profile: req.Profile, + }, req.SecretsDir, deps) if err != nil { return nil, fmt.Errorf("%s: %w", req.CommandName, err) } diff --git a/internal/app/runner_runtime.go b/internal/app/runner_runtime.go index 4663107..1095f25 100644 --- a/internal/app/runner_runtime.go +++ b/internal/app/runner_runtime.go @@ -157,9 +157,6 @@ func initializeLockedInvocation(runtime *invocationRuntime) error { if err := runtime.manifestStore.Save(runtime.ctx, runtime.manifestPath, m); err != nil { return fmt.Errorf("save manifest identity %q: %w", runtime.manifestPath, err) } - if _, err := loadSecretsFromConfig(runtime.env.Config, runtime.env.Logger); err != nil { - return runtime.persistTerminalFailure(fmt.Errorf("load secrets from files: %w", err)) - } return nil } diff --git a/internal/app/secrets_bootstrap.go b/internal/app/secrets_bootstrap.go new file mode 100644 index 0000000..64fe182 --- /dev/null +++ b/internal/app/secrets_bootstrap.go @@ -0,0 +1,66 @@ +package app + +import ( + "bytes" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" +) + +type secretsBootstrap struct { + selection *string + selectionSet bool + called bool +} + +func (b *secretsBootstrap) load(pipelinePath string, loader func(string) (*configkit.SecretsReport, error)) error { + if b == nil || b.called { + return nil + } + b.called = true + directory, required, err := selectedSecretsDirectory(pipelinePath, b.selection) + if err != nil { + return err + } + if !required { + if _, err := os.Stat(directory); errors.Is(err, os.ErrNotExist) { + return nil + } else if err != nil { + return fmt.Errorf("inspect conventional secrets directory %q: %w", directory, err) + } + } + report, loadErr := loader(directory) + if loadErr == nil { + return nil + } + var rendered bytes.Buffer + if report != nil { + _, _ = report.WriteTo(&rendered) + } + if rendered.Len() != 0 { + return fmt.Errorf("bootstrap secrets directory: %w\n%s", loadErr, strings.TrimSpace(rendered.String())) + } + return fmt.Errorf("bootstrap secrets directory: %w", loadErr) +} + +func selectedSecretsDirectory(pipelinePath string, selection *string) (string, bool, error) { + if selection != nil { + if strings.TrimSpace(*selection) == "" { + return "", true, fmt.Errorf("--secrets-dir requires a non-empty path") + } + path, err := filepath.Abs(*selection) + if err != nil { + return "", true, fmt.Errorf("resolve --secrets-dir %q: %w", *selection, err) + } + return filepath.Clean(path), true, nil + } + root, err := filepath.Abs(pipelinePath) + if err != nil { + return "", false, fmt.Errorf("resolve pipeline path for secrets discovery: %w", err) + } + return filepath.Join(filepath.Dir(root), "secrets"), false, nil +} diff --git a/internal/app/secrets_bootstrap_test.go b/internal/app/secrets_bootstrap_test.go new file mode 100644 index 0000000..7197915 --- /dev/null +++ b/internal/app/secrets_bootstrap_test.go @@ -0,0 +1,132 @@ +package app + +import ( + "os" + "path/filepath" + "strings" + "testing" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" + "gitea.maximumdirect.net/eric/narratio/internal/config" +) + +func TestPipelineLoadSkipsAbsentConventionalSecretsDirectory(t *testing.T) { + pipelinePath := filepath.Join(t.TempDir(), "pipeline.yml") + if err := os.WriteFile(pipelinePath, []byte("version: 1\n"), 0o644); err != nil { + t.Fatal(err) + } + secretCalls := 0 + pipelineCalls := 0 + deps := commandDependencies{ + loadSecretsDir: func(string) (*configkit.SecretsReport, error) { + secretCalls++ + return nil, nil + }, + loadPipelineConfig: func(string, config.PipelineLoadOptions) (*config.PipelineConfig, error) { + pipelineCalls++ + return &config.PipelineConfig{}, nil + }, + } + + if _, _, err := loadPipelineConfigWithDependencies(pipelinePath, config.PipelineLoadOptions{}, nil, deps); err != nil { + t.Fatalf("load pipeline: %v", err) + } + if secretCalls != 0 || pipelineCalls != 1 { + t.Fatalf("calls = secrets %d, pipeline %d; want 0, 1", secretCalls, pipelineCalls) + } +} + +func TestExplicitSecretsDirectoryIsResolvedFromCallerWorkingDirectory(t *testing.T) { + callerDir := t.TempDir() + selected := filepath.Join(callerDir, "credentials") + if err := os.Mkdir(selected, 0o700); err != nil { + t.Fatal(err) + } + original, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := os.Chdir(callerDir); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chdir(original) }) + + got, required, err := selectedSecretsDirectory(filepath.Join(t.TempDir(), "pipeline.yml"), stringPointer("credentials")) + if err != nil { + t.Fatalf("select secrets: %v", err) + } + if !required || got != selected { + t.Fatalf("selection = %q, required=%t; want %q, true", got, required, selected) + } +} + +func TestExplicitSecretsFailurePrecedesPipelineLoad(t *testing.T) { + pipelinePath := filepath.Join(t.TempDir(), "pipeline.yml") + if err := os.WriteFile(pipelinePath, []byte("version: 1\n"), 0o644); err != nil { + t.Fatal(err) + } + for name, selection := range map[string]string{"empty": "", "missing": filepath.Join(t.TempDir(), "missing")} { + t.Run(name, func(t *testing.T) { + pipelineCalls := 0 + deps := commandDependencies{loadPipelineConfig: func(string, config.PipelineLoadOptions) (*config.PipelineConfig, error) { + pipelineCalls++ + return &config.PipelineConfig{}, nil + }} + if _, _, err := loadPipelineConfigWithDependencies(pipelinePath, config.PipelineLoadOptions{}, &selection, deps); err == nil { + t.Fatal("load succeeded, want bootstrap error") + } + if pipelineCalls != 0 { + t.Fatalf("pipeline load calls = %d, want 0", pipelineCalls) + } + }) + } +} + +func TestSecretsBootstrapPreservesExistingEnvironmentValues(t *testing.T) { + directory := t.TempDir() + const emptyName = "NARRATIO_BOOTSTRAP_EXISTING_EMPTY" + const valueName = "NARRATIO_BOOTSTRAP_EXISTING_VALUE" + const loadedName = "NARRATIO_BOOTSTRAP_NEW_VALUE" + t.Setenv(emptyName, "") + t.Setenv(valueName, "from-environment") + restoreEnvAfterTest(t, loadedName) + for name, value := range map[string]string{ + emptyName: "from-file-empty\n", + valueName: "from-file-value\n", + loadedName: "new-value\n", + } { + mustWriteSecretFile(t, filepath.Join(directory, name), value) + } + + bootstrap := &secretsBootstrap{selection: &directory, selectionSet: true} + if err := bootstrap.load(filepath.Join(t.TempDir(), "pipeline.yml"), configkit.LoadSecretsDir); err != nil { + t.Fatalf("bootstrap: %v", err) + } + if got := os.Getenv(emptyName); got != "" { + t.Fatalf("existing empty value = %q, want empty", got) + } + if got := os.Getenv(valueName); got != "from-environment" { + t.Fatalf("existing value = %q, want environment value", got) + } + if got := os.Getenv(loadedName); got != "new-value" { + t.Fatalf("new value = %q, want file value", got) + } +} + +func TestSecretsBootstrapPreservesSafePartialReportOnFailure(t *testing.T) { + directory := t.TempDir() + mustWriteSecretFile(t, filepath.Join(directory, "INVALID-NAME"), "must-not-appear\n") + bootstrap := &secretsBootstrap{selection: &directory, selectionSet: true} + err := bootstrap.load(filepath.Join(t.TempDir(), "pipeline.yml"), configkit.LoadSecretsDir) + if err == nil { + t.Fatal("bootstrap succeeded, want error") + } + if !strings.Contains(err.Error(), `status="failed"`) || !strings.Contains(err.Error(), `name="INVALID-NAME"`) { + t.Fatalf("error lacks partial report: %v", err) + } + if strings.Contains(err.Error(), "must-not-appear") { + t.Fatalf("error exposed secret value: %v", err) + } +} + +func stringPointer(value string) *string { return &value } diff --git a/internal/app/secrets_env.go b/internal/app/secrets_env.go deleted file mode 100644 index 1c21825..0000000 --- a/internal/app/secrets_env.go +++ /dev/null @@ -1,147 +0,0 @@ -package app - -import ( - "fmt" - "log/slog" - "os" - "path/filepath" - "regexp" - "strings" - - "gitea.maximumdirect.net/eric/narratio/internal/config" - "gitea.maximumdirect.net/eric/narratio/internal/fileops" -) - -var envVarNamePattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) - -// APIKeyFileByteLimit bounds one credential value read from the configured -// secrets directory. API keys are expected to be short single-line values. -const APIKeyFileByteLimit int64 = 8 * 1024 - -const secretDirectoryPrivateMode os.FileMode = 0o700 -const secretFilePrivateMode os.FileMode = 0o600 - -type secretsLoadStats struct { - Dir string - Loaded int - PreservedExisting int - Skipped int -} - -func loadSecretsFromConfig(cfg *config.Config, logger *slog.Logger) (*secretsLoadStats, error) { - return loadSecretsFromConfigWithDependencies(cfg, logger, commandDependencies{}) -} - -func loadSecretsFromConfigWithDependencies(cfg *config.Config, logger *slog.Logger, deps commandDependencies) (*secretsLoadStats, error) { - deps = normalizeCommandDependencies(deps) - if cfg == nil || cfg.Pipeline == nil || cfg.Pipeline.Secrets == nil { - return nil, nil - } - - rawDir := strings.TrimSpace(cfg.Pipeline.Secrets.EnvDir) - if rawDir == "" { - return nil, nil - } - - resolvedDir := rawDir - if !filepath.IsAbs(resolvedDir) { - cwd, err := os.Getwd() - if err != nil { - return nil, fmt.Errorf("resolve secrets env_dir %q from current working directory: %w", rawDir, err) - } - resolvedDir = filepath.Join(cwd, resolvedDir) - } - resolvedDir = filepath.Clean(resolvedDir) - - directoryInfo, err := os.Lstat(resolvedDir) - if err != nil { - return nil, fmt.Errorf("API-key loader: validate secrets env_dir: %w", err) - } - if directoryInfo.Mode()&os.ModeSymlink != 0 { - return nil, fmt.Errorf("API-key loader: secrets env_dir must not be a symbolic link") - } - if err := validateSecretDirectory(directoryInfo); err != nil { - return nil, fmt.Errorf("API-key loader: validate secrets env_dir: %w", err) - } - - entries, err := deps.readSecretsDirectory(resolvedDir) - if err != nil { - return nil, fmt.Errorf("API-key loader: read secrets env_dir %q: %w", resolvedDir, err) - } - directoryInfo, err = os.Lstat(resolvedDir) - if err != nil || directoryInfo.Mode()&os.ModeSymlink != 0 { - if err == nil { - err = fmt.Errorf("symbolic links are not allowed") - } - return nil, fmt.Errorf("API-key loader: validate secrets env_dir: %w", err) - } - - stats := &secretsLoadStats{Dir: resolvedDir} - for _, entry := range entries { - name := entry.Name() - if !envVarNamePattern.MatchString(name) { - stats.Skipped++ - continue - } - if _, exists := os.LookupEnv(name); exists { - stats.PreservedExisting++ - continue - } - value, err := readAPIKeyFile(resolvedDir, name) - if err != nil { - return nil, err - } - if err := os.Setenv(name, value); err != nil { - return nil, fmt.Errorf("API-key loader: set environment variable %q: %w", name, err) - } - stats.Loaded++ - } - - if logger != nil { - logger.Info( - "loaded secret environment variables from filesystem", - "secrets_env_dir", stats.Dir, - "loaded", stats.Loaded, - "preserved_existing", stats.PreservedExisting, - "skipped", stats.Skipped, - ) - } - - return stats, nil -} - -func validateSecretDirectory(info os.FileInfo) error { - if !info.IsDir() { - return fmt.Errorf("secrets env_dir is not a directory") - } - return validateSecretDirectoryPrivacy(info) -} - -func validateSecretFile(info os.FileInfo) error { - if !info.Mode().IsRegular() { - return fmt.Errorf("secret entry is not a regular file") - } - return validateSecretFilePrivacy(info) -} - -func readAPIKeyFile(directory, name string) (string, error) { - path := filepath.Join(directory, name) - info, err := os.Lstat(path) - if err == nil && info.Mode()&os.ModeSymlink != 0 { - return "", fmt.Errorf("API-key loader: read secret file %q with %d-byte limit: symbolic links are not allowed", name, APIKeyFileByteLimit) - } - if err == nil { - err = validateSecretFile(info) - } - if err == nil { - content, readErr := fileops.ReadFileBounded(path, APIKeyFileByteLimit) - if readErr == nil { - return strings.TrimRight(string(content), "\r\n"), nil - } - err = readErr - } - if err != nil { - return "", fmt.Errorf("API-key loader: read secret file %q with %d-byte limit: %w", name, APIKeyFileByteLimit, err) - } - return "", fmt.Errorf("API-key loader: read secret file %q with %d-byte limit", name, APIKeyFileByteLimit) -} diff --git a/internal/app/secrets_env_posix.go b/internal/app/secrets_env_posix.go deleted file mode 100644 index b76aa38..0000000 --- a/internal/app/secrets_env_posix.go +++ /dev/null @@ -1,22 +0,0 @@ -//go:build linux || darwin - -package app - -import ( - "fmt" - "os" -) - -func validateSecretDirectoryPrivacy(info os.FileInfo) error { - if info.Mode().Perm()&^secretDirectoryPrivateMode != 0 { - return fmt.Errorf("secret directory mode %04o grants group or other access; require %04o", info.Mode().Perm(), secretDirectoryPrivateMode) - } - return nil -} - -func validateSecretFilePrivacy(info os.FileInfo) error { - if info.Mode().Perm()&^secretFilePrivateMode != 0 { - return fmt.Errorf("secret file mode %04o grants group or other access; require %04o", info.Mode().Perm(), secretFilePrivateMode) - } - return nil -} diff --git a/internal/app/secrets_env_test.go b/internal/app/secrets_env_test.go deleted file mode 100644 index 1b09455..0000000 --- a/internal/app/secrets_env_test.go +++ /dev/null @@ -1,279 +0,0 @@ -package app - -import ( - "os" - "path/filepath" - "strings" - "testing" - - "gitea.maximumdirect.net/eric/narratio/internal/config" -) - -func TestLoadSecretsFromConfigLoadsValidFiles(t *testing.T) { - dir := t.TempDir() - restoreEnvAfterTest(t, "NARRATIO_TEST_SECRET_A", "NARRATIO_TEST_SECRET_B") - mustWriteSecretFile(t, filepath.Join(dir, "NARRATIO_TEST_SECRET_A"), "value-1\n") - mustWriteSecretFile(t, filepath.Join(dir, "NARRATIO_TEST_SECRET_B"), "value-2\r\n") - mustWriteSecretFile(t, filepath.Join(dir, "not-valid-name.txt"), "ignored") - - cfg := &config.Config{ - Pipeline: &config.PipelineConfig{ - Secrets: &config.SecretsConfig{EnvDir: dir}, - }, - } - - stats, err := loadSecretsFromConfig(cfg, nil) - if err != nil { - t.Fatalf("loadSecretsFromConfig() error = %v", err) - } - if stats == nil { - t.Fatal("stats = nil, want non-nil") - } - if stats.Loaded != 2 { - t.Fatalf("Loaded = %d, want 2", stats.Loaded) - } - if stats.PreservedExisting != 0 { - t.Fatalf("PreservedExisting = %d, want 0", stats.PreservedExisting) - } - if stats.Skipped == 0 { - t.Fatalf("Skipped = %d, want > 0 for invalid filename", stats.Skipped) - } - if got := os.Getenv("NARRATIO_TEST_SECRET_A"); got != "value-1" { - t.Fatalf("NARRATIO_TEST_SECRET_A = %q, want value-1", got) - } - if got := os.Getenv("NARRATIO_TEST_SECRET_B"); got != "value-2" { - t.Fatalf("NARRATIO_TEST_SECRET_B = %q, want value-2", got) - } -} - -func TestLoadSecretsFromConfigPreservesExistingEnv(t *testing.T) { - t.Setenv("OBJECT_STORAGE_KEY", "existing") - - dir := t.TempDir() - mustWriteSecretFile(t, filepath.Join(dir, "OBJECT_STORAGE_KEY"), "from-file\n") - - cfg := &config.Config{ - Pipeline: &config.PipelineConfig{ - Secrets: &config.SecretsConfig{EnvDir: dir}, - }, - } - - stats, err := loadSecretsFromConfig(cfg, nil) - if err != nil { - t.Fatalf("loadSecretsFromConfig() error = %v", err) - } - if stats.PreservedExisting != 1 { - t.Fatalf("PreservedExisting = %d, want 1", stats.PreservedExisting) - } - if got := os.Getenv("OBJECT_STORAGE_KEY"); got != "existing" { - t.Fatalf("OBJECT_STORAGE_KEY = %q, want existing", got) - } -} - -func TestLoadSecretsFromConfigRelativeDirUsesCWD(t *testing.T) { - cwd := t.TempDir() - secretsDir := filepath.Join(cwd, "secrets") - if err := os.MkdirAll(secretsDir, secretDirectoryPrivateMode); err != nil { - t.Fatalf("MkdirAll(%q): %v", secretsDir, err) - } - mustWriteSecretFile(t, filepath.Join(secretsDir, "OBJECT_STORAGE_KEY_ID"), "id-123\n") - - originalWD, err := os.Getwd() - if err != nil { - t.Fatalf("Getwd(): %v", err) - } - if err := os.Chdir(cwd); err != nil { - t.Fatalf("Chdir(%q): %v", cwd, err) - } - t.Cleanup(func() { - _ = os.Chdir(originalWD) - }) - - cfg := &config.Config{ - Pipeline: &config.PipelineConfig{ - Secrets: &config.SecretsConfig{EnvDir: "./secrets"}, - }, - } - if _, err := loadSecretsFromConfig(cfg, nil); err != nil { - t.Fatalf("loadSecretsFromConfig() error = %v", err) - } - if got := os.Getenv("OBJECT_STORAGE_KEY_ID"); got != "id-123" { - t.Fatalf("OBJECT_STORAGE_KEY_ID = %q, want id-123", got) - } -} - -func TestLoadSecretsFromConfigMissingDirFails(t *testing.T) { - cfg := &config.Config{ - Pipeline: &config.PipelineConfig{ - Secrets: &config.SecretsConfig{EnvDir: filepath.Join(t.TempDir(), "missing")}, - }, - } - - _, err := loadSecretsFromConfig(cfg, nil) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "API-key loader") { - t.Fatalf("error = %q, want API-key loader context", err.Error()) - } -} - -func TestLoadSecretsFromConfigUnreadableValidEntryFails(t *testing.T) { - - dir := t.TempDir() - ensureSecretDirectory(t, dir) - const secretName = "NARRATIO_TEST_BROKEN_SECRET" - restoreEnvAfterTest(t, secretName) - if err := os.Unsetenv(secretName); err != nil { - t.Fatalf("Unsetenv(%q): %v", secretName, err) - } - broken := filepath.Join(dir, secretName) - if err := os.Symlink(filepath.Join(dir, "does-not-exist"), broken); err != nil { - t.Fatalf("Symlink(%q): %v", broken, err) - } - - cfg := &config.Config{ - Pipeline: &config.PipelineConfig{ - Secrets: &config.SecretsConfig{EnvDir: dir}, - }, - } - - _, err := loadSecretsFromConfig(cfg, nil) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "API-key loader") { - t.Fatalf("error = %q, want API-key loader context", err.Error()) - } -} - -func TestLoadSecretsFromConfigRejectsUnsafeModes(t *testing.T) { - for _, tc := range []struct { - name string - directory os.FileMode - file os.FileMode - wantSecret string - }{ - {name: "directory", directory: 0o750, file: secretFilePrivateMode, wantSecret: "directory-secret"}, - {name: "file", directory: secretDirectoryPrivateMode, file: 0o640, wantSecret: "file-secret"}, - } { - t.Run(tc.name, func(t *testing.T) { - dir := t.TempDir() - restoreEnvAfterTest(t, "OPENROUTER_API_KEY") - path := filepath.Join(dir, "OPENROUTER_API_KEY") - mustWriteSecretFile(t, path, tc.wantSecret) - if err := os.Chmod(dir, tc.directory); err != nil { - t.Fatalf("Chmod(directory) error = %v", err) - } - if err := os.Chmod(path, tc.file); err != nil { - t.Fatalf("Chmod(file) error = %v", err) - } - - _, err := loadSecretsFromConfig(secretConfig(dir), nil) - if err == nil || !strings.Contains(err.Error(), "API-key loader") { - t.Fatalf("loadSecretsFromConfig() error = %v, want API-key loader failure", err) - } - if strings.Contains(err.Error(), tc.wantSecret) { - t.Fatalf("error leaked secret content: %q", err) - } - }) - } -} - -func TestLoadSecretsFromConfigRejectsNonRegularAndOversizedEntries(t *testing.T) { - dir := t.TempDir() - ensureSecretDirectory(t, dir) - restoreEnvAfterTest(t, "OPENROUTER_API_KEY") - if err := os.Mkdir(filepath.Join(dir, "OPENROUTER_API_KEY"), secretDirectoryPrivateMode); err != nil { - t.Fatalf("Mkdir(non-regular entry) error = %v", err) - } - if _, err := loadSecretsFromConfig(secretConfig(dir), nil); err == nil || !strings.Contains(err.Error(), "not a regular file") { - t.Fatalf("loadSecretsFromConfig(non-regular) error = %v, want regular-file failure", err) - } - - if err := os.Remove(filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { - t.Fatalf("Remove(non-regular entry) error = %v", err) - } - overLimit := strings.Repeat("credential-value-", int(APIKeyFileByteLimit/17)+1) - mustWriteSecretFile(t, filepath.Join(dir, "OPENROUTER_API_KEY"), overLimit) - _, err := loadSecretsFromConfig(secretConfig(dir), nil) - if err == nil || !strings.Contains(err.Error(), "API-key loader") || !strings.Contains(err.Error(), "8192-byte limit") { - t.Fatalf("loadSecretsFromConfig(over-limit) error = %v, want bounded API-key loader failure", err) - } - if strings.Contains(err.Error(), overLimit[:32]) { - t.Fatalf("error leaked secret content: %q", err) - } -} - -func TestLoadSecretsFromConfigRejectsSymlinkAndAncestorReplacement(t *testing.T) { - dir := t.TempDir() - outside := t.TempDir() - ensureSecretDirectory(t, dir) - restoreEnvAfterTest(t, "OPENROUTER_API_KEY") - outsideValue := "outside-secret-value" - mustWriteSecretFile(t, filepath.Join(outside, "OPENROUTER_API_KEY"), outsideValue) - if err := os.Symlink(filepath.Join(outside, "OPENROUTER_API_KEY"), filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { - t.Fatalf("Symlink(leaf) error = %v", err) - } - _, err := loadSecretsFromConfig(secretConfig(dir), nil) - if err == nil || !strings.Contains(err.Error(), "API-key loader") { - t.Fatalf("loadSecretsFromConfig(symlink) error = %v, want failure", err) - } - if strings.Contains(err.Error(), outsideValue) { - t.Fatalf("error leaked secret content: %q", err) - } - - if err := os.Remove(filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { - t.Fatalf("Remove(leaf symlink) error = %v", err) - } - mustWriteSecretFile(t, filepath.Join(dir, "OPENROUTER_API_KEY"), "inside-secret") - deps := commandDependencies{ - readSecretsDirectory: func(path string) ([]os.DirEntry, error) { - entries, err := os.ReadDir(path) - if err != nil { - return nil, err - } - if err := os.Remove(filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil { - return nil, err - } - if err := os.Remove(dir); err != nil { - return nil, err - } - if err := os.Symlink(outside, dir); err != nil { - return nil, err - } - return entries, nil - }, - } - _, err = loadSecretsFromConfigWithDependencies(secretConfig(dir), nil, deps) - if err == nil || !strings.Contains(err.Error(), "API-key loader") { - t.Fatalf("loadSecretsFromConfig(ancestor replacement) error = %v, want failure", err) - } - if _, exists := os.LookupEnv("OPENROUTER_API_KEY"); exists { - t.Fatal("ancestor replacement loaded a secret value") - } -} - -func mustWriteSecretFile(t *testing.T, path, contents string) { - t.Helper() - if err := os.MkdirAll(filepath.Dir(path), secretDirectoryPrivateMode); err != nil { - t.Fatalf("MkdirAll(%q): %v", path, err) - } - if err := os.Chmod(filepath.Dir(path), secretDirectoryPrivateMode); err != nil { - t.Fatalf("Chmod(%q): %v", filepath.Dir(path), err) - } - if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { - t.Fatalf("WriteFile(%q): %v", path, err) - } -} - -func secretConfig(dir string) *config.Config { - return &config.Config{Pipeline: &config.PipelineConfig{Secrets: &config.SecretsConfig{EnvDir: dir}}} -} - -func ensureSecretDirectory(t *testing.T, directory string) { - t.Helper() - if err := os.Chmod(directory, secretDirectoryPrivateMode); err != nil { - t.Fatalf("Chmod(%q): %v", directory, err) - } -} diff --git a/internal/app/test_environment_test.go b/internal/app/test_environment_test.go index 28094fc..d735f7c 100644 --- a/internal/app/test_environment_test.go +++ b/internal/app/test_environment_test.go @@ -2,6 +2,7 @@ package app import ( "os" + "path/filepath" "testing" ) @@ -29,3 +30,13 @@ func restoreEnvAfterTest(t *testing.T, names ...string) { } }) } + +func mustWriteSecretFile(t *testing.T, path, value string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(value), 0o600); err != nil { + t.Fatal(err) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 75f17a6..34b7208 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,7 +23,6 @@ type PipelineConfig struct { Spool SpoolConfig `yaml:"spool" json:"spool"` Cache CacheConfig `yaml:"cache" json:"cache"` Publish *PublishConfig `yaml:"publish" json:"publish"` - Secrets *SecretsConfig `yaml:"secrets" json:"secrets"` WhisperX WhisperXConfig `yaml:"whisperx" json:"whisperx"` Seriatim SeriatimConfig `yaml:"seriatim" json:"seriatim"` Audita AuditaConfig `yaml:"audita" json:"audita"` @@ -84,11 +83,6 @@ type WorkspaceConfig struct { CleanupAfterPublish bool `yaml:"cleanup_after_publish" json:"cleanup_after_publish"` } -// SecretsConfig configures optional local filesystem secret loading. -type SecretsConfig struct { - EnvDir string `yaml:"env_dir" json:"env_dir"` -} - // StorageConfig configures storage backends and related parameters. type StorageConfig struct { Backend string `yaml:"backend" json:"backend"` diff --git a/internal/config/effective_projection.go b/internal/config/effective_projection.go index d1f69f3..4b43dbe 100644 --- a/internal/config/effective_projection.go +++ b/internal/config/effective_projection.go @@ -18,7 +18,6 @@ type effectivePipelineProjection struct { Spool SpoolConfig `yaml:"spool" json:"spool"` Cache CacheConfig `yaml:"cache" json:"cache"` Publish *PublishConfig `yaml:"publish" json:"publish"` - Secrets *SecretsConfig `yaml:"secrets" json:"secrets"` WhisperX WhisperXConfig `yaml:"whisperx" json:"whisperx"` Seriatim SeriatimConfig `yaml:"seriatim" json:"seriatim"` Audita AuditaConfig `yaml:"audita" json:"audita"` @@ -46,7 +45,7 @@ type projectedPipelineValue struct { func projectEffectivePipeline(cfg *PipelineConfig, logicalNotarius bool) effectivePipelineProjection { projection := effectivePipelineProjection{ Workspace: cfg.Workspace, Campaigns: cfg.Campaigns, Storage: cfg.Storage, - Spool: cfg.Spool, Cache: cfg.Cache, Publish: cfg.Publish, Secrets: cfg.Secrets, + Spool: cfg.Spool, Cache: cfg.Cache, Publish: cfg.Publish, WhisperX: cfg.WhisperX, Seriatim: cfg.Seriatim, Audita: cfg.Audita, Normalize: cfg.Normalize, Trim: cfg.Trim, Render: cfg.Render, Notarius: cfg.Notarius, Notification: cfg.Notification, diff --git a/internal/config/load_validate_test.go b/internal/config/load_validate_test.go index 35b319c..1c03185 100644 --- a/internal/config/load_validate_test.go +++ b/internal/config/load_validate_test.go @@ -226,8 +226,8 @@ func TestExamplesLoadAndValidate(t *testing.T) { if cfg.Pipeline.Audita.Model != profile.model { t.Fatalf("audita model = %q, want %q", cfg.Pipeline.Audita.Model, profile.model) } - if cfg.Pipeline.Secrets != nil || cfg.Pipeline.Audita.LLMAPIKeyEnv != "" || cfg.Pipeline.Storage.Backend != StorageBackendLocal || cfg.Pipeline.Storage.S3 != nil { - t.Fatalf("split bundle must remain offline and secret-free: secrets=%#v audita=%#v storage=%#v", cfg.Pipeline.Secrets, cfg.Pipeline.Audita, cfg.Pipeline.Storage) + if cfg.Pipeline.Audita.LLMAPIKeyEnv != "" || cfg.Pipeline.Storage.Backend != StorageBackendLocal || cfg.Pipeline.Storage.S3 != nil { + t.Fatalf("split bundle must remain offline and secret-free: audita=%#v storage=%#v", cfg.Pipeline.Audita, cfg.Pipeline.Storage) } if selectedProfile, ok := SelectedPipelineProfile(cfg.Pipeline); !ok || selectedProfile.Name != profile.name { t.Fatalf("selected profile = %#v, want %q", selectedProfile, profile.name) diff --git a/internal/config/validate.go b/internal/config/validate.go index 125d48d..db61bd8 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -67,9 +67,6 @@ func validatePipeline(cfg *PipelineConfig) error { return fmt.Errorf("pipeline.campaigns.default_campaign_id: %w", err) } } - if err := validateSecrets(cfg.Secrets); err != nil { - return err - } if err := validateStorage(cfg.Storage); err != nil { return err } diff --git a/internal/config/validate_core.go b/internal/config/validate_core.go index 4355a07..6d71b9c 100644 --- a/internal/config/validate_core.go +++ b/internal/config/validate_core.go @@ -36,16 +36,6 @@ func validateCampaign(cfg *CampaignConfig) error { return nil } -func validateSecrets(cfg *SecretsConfig) error { - if cfg == nil { - return nil - } - if strings.TrimSpace(cfg.EnvDir) == "" { - return fmt.Errorf("pipeline.secrets.env_dir must be non-empty when pipeline.secrets is configured") - } - return nil -} - func validateSpool(cfg SpoolConfig) error { return nil }