diff --git a/docs/config.md b/docs/config.md index 4884949..1132fa7 100644 --- a/docs/config.md +++ b/docs/config.md @@ -47,7 +47,9 @@ the durable copied session input. `narratio config validate`, `narratio config show`, and `narratio config sources` use the same `--config`, `--campaign`, `--campaign-file`, and `--profile` selection rules as pipeline commands, but do not select, discover, -or load a session. They do not read credential values or create runtime state. +or load a session. They resolve configured typed credential references as part +of the ordinary ConfigKit load but expose only their environment-variable names; +they do not create runtime state. `narratio config diff ` uses the same pipeline and campaign selectors, resolves each named profile with an independent ConfigKit load, and does not accept a separate `--profile` flag. @@ -215,7 +217,10 @@ inputs: ## Secrets Handling - Do not place raw secrets in YAML. -- Use env var names in config (for example `pipeline.audita.llm_api_key_env`). +- Credential fields contain environment-variable names, represented internally + as ConfigKit typed secret references (for example + `pipeline.audita.llm_api_key: AUDITA_LLM_API_KEY`). A configured reference + must resolve to a nonempty environment value during load. - 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. @@ -283,8 +288,8 @@ Rules: | `pipeline.storage.s3.region` | string | No | empty | | `pipeline.storage.s3.endpoint` | string | No | empty | | `pipeline.storage.s3.force_path_style` | bool | No | `false` | -| `pipeline.storage.s3.access_key_id_env` | string | No | `OBJECT_STORAGE_KEY_ID` | -| `pipeline.storage.s3.secret_access_key_env` | string | No | `OBJECT_STORAGE_KEY` | +| `pipeline.storage.s3.access_key_id` | secret reference | Conditional | unset; must be paired with `secret_access_key` | +| `pipeline.storage.s3.secret_access_key` | secret reference | Conditional | unset; must be paired with `access_key_id` | | `pipeline.spool.root` | string | No | `/var/spool/narratio` | | `pipeline.spool.delete_audio_after_publish` | bool | No | `false`; selects the published run's spool-audio target for final cleanup and requires effective `pipeline.publish.enabled=true` and `pipeline.publish.upload_run=true` | | `pipeline.cache.root` | string | No | `/var/cache/narratio` | @@ -312,7 +317,7 @@ Rules: | `pipeline.seriatim.env.filler_max_duration` | float | No | unset | | `pipeline.audita.binary` | string | No | `audita` | | `pipeline.audita.timeout` | duration | No | `3h` | -| `pipeline.audita.llm_api_key_env` | string | No | empty | +| `pipeline.audita.llm_api_key` | secret reference | No | unset | | `pipeline.audita.modules[]` | list[string] | No | empty | | `pipeline.audita.base_url` | string | No | empty | | `pipeline.audita.model` | string | No | empty | diff --git a/docs/integrations/audita.md b/docs/integrations/audita.md index 978eb6c..3daa320 100644 --- a/docs/integrations/audita.md +++ b/docs/integrations/audita.md @@ -41,7 +41,6 @@ Construction fails for invalid static config values, including: Run fails for: - missing required request paths; -- missing required credential env var when configured (`llm_api_key_env`); - subprocess execution failure; - invalid processed transcript JSON (`segments` array required); - invalid report JSON when reporting is enabled. @@ -53,6 +52,9 @@ Failure results still include output/log/config/exit metadata for diagnostics. ## Deterministic Behavior - CLI args are built from runner config + request in a fixed order. +- A configured typed `llm_api_key` reference is resolved during pipeline load; + the captured value is supplied only as the child's `AUDITA_LLM_API_KEY` + override and is never re-read from the parent environment. - Generated invocation YAML (`audita.generated.v1`) is emitted when requested. - Manifest writes are stage-owned; adapter itself is stateless. diff --git a/docs/internal/adapters.md b/docs/internal/adapters.md index 10a901e..f0b9cdd 100644 --- a/docs/internal/adapters.md +++ b/docs/internal/adapters.md @@ -84,8 +84,9 @@ in-memory backend without changing package state. Cancellation, deadlines, and diagnostic limits use the same terminal disposal path without losing their original result classification. Child environments contain only the execution - baseline and adapter-specified values; configured credentials are explicit - sensitive values. Stdout and stderr are redacted while streaming into separate + baseline and adapter-specified values; configured credentials are resolved + once by ConfigKit and passed explicitly as sensitive values. Adapters do not + re-read their named environment references. Stdout and stderr are redacted while streaming into separate 8 MiB diagnostic captures; a bounded wait closes a stream retained by a departed leader's descendant. Unsupported platforms reject owned command execution. diff --git a/docs/internal/configuration.md b/docs/internal/configuration.md index 78be729..fcafd8c 100644 --- a/docs/internal/configuration.md +++ b/docs/internal/configuration.md @@ -155,8 +155,10 @@ execution, but the digest substitutes their normalized logical values captured before root-relative resolution, so relocating an equivalent configuration bundle does not change provenance. Because ConfigKit and resolution metadata are private, the digest excludes source layout, profile name, and ownership. -Configuration stores environment variable names rather than resolving raw -credentials, so raw secret values are neither loaded nor hashed. +Configuration stores credentials as ConfigKit typed references. A successful +load resolves their values inside the opaque typed boundary, while this safe +projection serializes only `EnvName()` identities. Credential-value changes are +therefore never hashed; reference-name changes remain ordinary provenance. `recomputePipelineEffectiveDigest` is the single package-owned refresh point for later runtime expansion. diff --git a/docs/internal/stage-polish.md b/docs/internal/stage-polish.md index b24fcc0..a626373 100644 --- a/docs/internal/stage-polish.md +++ b/docs/internal/stage-polish.md @@ -35,8 +35,9 @@ validation model, module set, transcript description, output schema, selected external configuration path, and canonical polished-transcript identity. Module ordering is normalized because the configured modules form a set. Audita's executable, timeouts, concurrency, report and debug behavior, work retention, -and credential environment name are operational and do not invalidate a -successful result. +and resolved credential value are operational and do not invalidate a +successful result. The public credential reference name may participate in +configuration provenance but never exposes its resolved value. Narratio can fingerprint a selected model, module, or configuration identifier, but it cannot inspect content that Audita privately resolves behind that stable diff --git a/docs/operations.md b/docs/operations.md index 301a732..47526eb 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -566,6 +566,13 @@ 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. +S3 `access_key_id` and `secret_access_key` are an optional pair of typed +environment references. When both are absent, the AWS SDK retains its default +credential provider chain. When both are configured, Narratio supplies their +load-time resolved values through an explicit static provider. Audita's optional +`llm_api_key` is likewise captured at pipeline load and supplied only through +the child process's `AUDITA_LLM_API_KEY` override. + External adapter results are individually bounded before Narratio validates or materializes them. These per-file limits do not reserve disk space: prevent hard disk exhaustion with filesystem, service, container, or volume quotas sized for diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index aa006f2..b6146e4 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -320,7 +320,11 @@ architecture, and troubleshooting to the new bootstrap/permission contract. ## Stage 7 — Resolve typed credentials and protect all output boundaries -Status: Pending +Status: Complete — Migrated S3 and Audita credentials to optional ConfigKit +typed references, passed resolved values directly to runtime adapters, preserved +the AWS default provider chain when unconfigured, and verified reference-only +inspection, provenance, generated configuration, manifest, and diagnostic +boundaries with focused, full, and race test suites. Read: ConfigKit “Typed secret references” and rendering contracts; `internal/config/config.go`, `defaults.go`, `validate_adapters.go`, the safe diff --git a/examples/README.md b/examples/README.md index bfb0c96..8194888 100644 --- a/examples/README.md +++ b/examples/README.md @@ -58,5 +58,8 @@ with the sample campaign and a compatible local- or S3-audio session. reserves the expected filename and directory shape. Replace it with a real FLAC file before running transcription. -The examples contain environment-variable names but no credential values. They -use fictional campaign content and reserved example domains. +The examples contain environment-variable names but no credential values. Set +`OBJECT_STORAGE_KEY_ID`, `OBJECT_STORAGE_KEY`, and `AUDITA_LLM_API_KEY` to +nonempty synthetic or real values before loading the production-shaped and full +annotated pipelines. They use fictional campaign content and reserved example +domains. diff --git a/examples/pipeline.full.annotated.yml b/examples/pipeline.full.annotated.yml index 0277061..466ce10 100644 --- a/examples/pipeline.full.annotated.yml +++ b/examples/pipeline.full.annotated.yml @@ -23,8 +23,8 @@ storage: endpoint: "" force_path_style: false # Optional; defaults shown explicitly. - access_key_id_env: OBJECT_STORAGE_KEY_ID - secret_access_key_env: OBJECT_STORAGE_KEY + access_key_id: OBJECT_STORAGE_KEY_ID + secret_access_key: OBJECT_STORAGE_KEY campaigns: # Optional; defaults to /usr/local/share/narratio/campaigns. @@ -93,7 +93,7 @@ audita: # Optional overrides; defaults shown explicitly where applicable. binary: audita timeout: 3h - llm_api_key_env: AUDITA_LLM_API_KEY + llm_api_key: AUDITA_LLM_API_KEY modules: [glossary, homophones, spoken_word, grammar] base_url: "" model: "" diff --git a/examples/pipeline.production.yml b/examples/pipeline.production.yml index 92ff115..d5e688a 100644 --- a/examples/pipeline.production.yml +++ b/examples/pipeline.production.yml @@ -8,8 +8,8 @@ storage: bucket: my-dnd-archive root_prefix: dnd region: us-east-1 - access_key_id_env: OBJECT_STORAGE_KEY_ID - secret_access_key_env: OBJECT_STORAGE_KEY + access_key_id: OBJECT_STORAGE_KEY_ID + secret_access_key: OBJECT_STORAGE_KEY campaigns: root: /usr/local/share/narratio/campaigns @@ -57,7 +57,7 @@ seriatim: audita: binary: audita timeout: 3h - llm_api_key_env: AUDITA_LLM_API_KEY + llm_api_key: AUDITA_LLM_API_KEY modules: [glossary, homophones, spoken_word, grammar] output_schema: audita-v1 work_dir_retention: auto diff --git a/internal/adapters/audita/subprocess.go b/internal/adapters/audita/subprocess.go index e2e7f37..75b83e0 100644 --- a/internal/adapters/audita/subprocess.go +++ b/internal/adapters/audita/subprocess.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "net/url" - "os" "strconv" "strings" "time" @@ -24,7 +23,8 @@ const MaxReportOutputBytes int64 = 16 * 1024 * 1024 type SubprocessRunnerConfig struct { Binary string Timeout time.Duration - LLMAPIKeyEnv string + LLMAPIKey string + LLMAPIKeyReference string Modules []string BaseURL string Model string @@ -43,7 +43,8 @@ type SubprocessRunnerConfig struct { type SubprocessRunner struct { binary string timeout time.Duration - llmAPIKeyEnv string + llmAPIKey string + llmAPIKeyReference string modules []string baseURL string model string @@ -62,7 +63,8 @@ type SubprocessRunner struct { func NewSubprocessRunnerFromConfigValues( binary string, timeout string, - llmAPIKeyEnv string, + llmAPIKey string, + llmAPIKeyReference string, modules []string, baseURL string, model string, @@ -86,7 +88,8 @@ func NewSubprocessRunnerFromConfigValues( return NewSubprocessRunner(SubprocessRunnerConfig{ Binary: binary, Timeout: parsedTimeout, - LLMAPIKeyEnv: llmAPIKeyEnv, + LLMAPIKey: llmAPIKey, + LLMAPIKeyReference: llmAPIKeyReference, Modules: modules, BaseURL: baseURL, Model: model, @@ -110,6 +113,9 @@ func NewSubprocessRunner(cfg SubprocessRunnerConfig) (*SubprocessRunner, error) if cfg.Timeout <= 0 { return nil, fmt.Errorf("audita timeout must be > 0") } + if (cfg.LLMAPIKey == "") != (strings.TrimSpace(cfg.LLMAPIKeyReference) == "") { + return nil, fmt.Errorf("audita credential value and reference must be provided together") + } for i, module := range cfg.Modules { if strings.TrimSpace(module) == "" { return nil, fmt.Errorf("audita module at index %d is empty", i) @@ -152,7 +158,8 @@ func NewSubprocessRunner(cfg SubprocessRunnerConfig) (*SubprocessRunner, error) return &SubprocessRunner{ binary: strings.TrimSpace(cfg.Binary), timeout: cfg.Timeout, - llmAPIKeyEnv: strings.TrimSpace(cfg.LLMAPIKeyEnv), + llmAPIKey: cfg.LLMAPIKey, + llmAPIKeyReference: strings.TrimSpace(cfg.LLMAPIKeyReference), modules: modules, baseURL: strings.TrimSpace(cfg.BaseURL), model: strings.TrimSpace(cfg.Model), @@ -195,16 +202,10 @@ func (r *SubprocessRunner) Run(ctx context.Context, req PolishRequest) (PolishRe } args := r.buildArgs(req, reqModules) - credentialEnvVar := strings.TrimSpace(r.llmAPIKeyEnv) - credentialPresent := false + credentialPresent := r.llmAPIKeyReference != "" env := map[string]string{} - if credentialEnvVar != "" { - credential, ok := os.LookupEnv(credentialEnvVar) - if !ok || strings.TrimSpace(credential) == "" { - return PolishResult{}, fmt.Errorf("audita: required credential environment variable %s is not set", credentialEnvVar) - } - env["AUDITA_LLM_API_KEY"] = credential - credentialPresent = true + if credentialPresent { + env["AUDITA_LLM_API_KEY"] = r.llmAPIKey } if req.GeneratedConfigPath != "" { @@ -269,7 +270,7 @@ func (r *SubprocessRunner) Run(ctx context.Context, req PolishRequest) (PolishRe "total_llm_concurrency": r.totalLLMConcurrency, "proposal_llm_concurrency": r.proposalLLMConcurrency, "validation_llm_concurrency": r.validationLLMConcurrency, - "credential_env_var": r.llmAPIKeyEnv, + "credential_env_var": r.llmAPIKeyReference, "credential_present": credentialPresent, }, }, nil @@ -299,7 +300,7 @@ func (r *SubprocessRunner) failureResult(req PolishRequest, modules []string, ru "total_llm_concurrency": r.totalLLMConcurrency, "proposal_llm_concurrency": r.proposalLLMConcurrency, "validation_llm_concurrency": r.validationLLMConcurrency, - "credential_env_var": r.llmAPIKeyEnv, + "credential_env_var": r.llmAPIKeyReference, "credential_present": credentialPresent, }, } @@ -375,7 +376,7 @@ func (r *SubprocessRunner) writeInvocationConfig(req PolishRequest, args []strin "output_path": req.OutputProcessedPath, "report_path": req.ReportPath, "work_dir": req.WorkDir, - "credential_env_var": r.llmAPIKeyEnv, + "credential_env_var": r.llmAPIKeyReference, "credential_present": credentialPresent, } return writeGeneratedYAML(req.GeneratedConfigPath, payload) diff --git a/internal/adapters/audita/subprocess_test.go b/internal/adapters/audita/subprocess_test.go index 3ae8bf9..8c55cc2 100644 --- a/internal/adapters/audita/subprocess_test.go +++ b/internal/adapters/audita/subprocess_test.go @@ -27,7 +27,8 @@ func TestSubprocessRunnerSuccessArgsEnvAndValidation(t *testing.T) { runner, err := NewSubprocessRunner(SubprocessRunnerConfig{ Binary: wrapper, Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "OPENAI_KEY_SOURCE", + LLMAPIKey: "super-secret", + LLMAPIKeyReference: "OPENAI_KEY_SOURCE", Modules: []string{"glossary", "homophones", "glossary"}, BaseURL: "https://openrouter.ai/api/v1", Model: "openrouter/google/gemma-4-31b-it", @@ -44,6 +45,7 @@ func TestSubprocessRunnerSuccessArgsEnvAndValidation(t *testing.T) { if err != nil { t.Fatalf("NewSubprocessRunner() error = %v", err) } + t.Setenv("OPENAI_KEY_SOURCE", "changed-after-resolution") dir := t.TempDir() req := PolishRequest{ @@ -128,24 +130,14 @@ func TestSubprocessRunnerSuccessArgsEnvAndValidation(t *testing.T) { } } -func TestSubprocessRunnerMissingConfiguredCredentialFails(t *testing.T) { - runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "MISSING_AUDITA_KEY", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: false, +func TestSubprocessRunnerRejectsIncompleteResolvedCredential(t *testing.T) { + _, err := NewSubprocessRunner(SubprocessRunnerConfig{ + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKeyReference: "MISSING_AUDITA_KEY", }) - req := auditaReqForTest(t, false) - - _, err := runner.Run(context.Background(), req) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), "required credential environment variable MISSING_AUDITA_KEY is not set") { - t.Fatalf("error = %q, want missing credential guidance", err.Error()) + if err == nil || !strings.Contains(err.Error(), "credential value and reference must be provided together") { + t.Fatalf("NewSubprocessRunner() error = %v, want credential pair error", err) } } @@ -156,13 +148,13 @@ func TestSubprocessRunnerUnconfiguredCredentialEnvOmitsCredential(t *testing.T) t.Setenv("AUDITA_HELPER_RECORD_PATH", recordPath) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: false, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "", + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: false, }) req := auditaReqForTest(t, false) @@ -187,13 +179,13 @@ func TestSubprocessRunnerOmitsUnspecifiedParentEnvironment(t *testing.T) { t.Setenv("AUDITA_HELPER_RECORD_PATH", recordPath) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: false, - LLMAPIKeyEnv: "", + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: false, + LLMAPIKey: "", }) req := auditaReqForTest(t, false) if _, err := runner.Run(context.Background(), req); err != nil { @@ -213,12 +205,12 @@ func TestSubprocessRunnerOmitsModulesFlagWhenNotConfigured(t *testing.T) { t.Setenv("AUDITA_HELPER_RECORD_PATH", recordPath) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "", - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: false, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "", + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: false, }) req := auditaReqForTest(t, false) if _, err := runner.Run(context.Background(), req); err != nil { @@ -239,10 +231,10 @@ func TestSubprocessRunnerOmitsBaseURLAndModelFlagsWhenNotConfigured(t *testing.T t.Setenv("AUDITA_HELPER_RECORD_PATH", recordPath) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "", - Report: false, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "", + Report: false, }) req := auditaReqForTest(t, false) if _, err := runner.Run(context.Background(), req); err != nil { @@ -266,13 +258,14 @@ func TestSubprocessRunnerSubprocessFailure(t *testing.T) { t.Setenv("AUDITA_HELPER_RECORD_PATH", filepath.Join(t.TempDir(), "record.json")) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "OPENAI_KEY_SOURCE", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: true, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "super-secret", + LLMAPIKeyReference: "OPENAI_KEY_SOURCE", + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: true, }) req := auditaReqForTest(t, true) _, err := runner.Run(context.Background(), req) @@ -288,6 +281,9 @@ func TestSubprocessRunnerSubprocessFailure(t *testing.T) { if !strings.Contains(err.Error(), req.StdoutLogPath) || !strings.Contains(err.Error(), req.StderrLogPath) { t.Fatalf("error = %q, want stdout/stderr log paths", err.Error()) } + if strings.Contains(err.Error(), "super-secret") { + t.Fatalf("failure diagnostic exposed credential value: %q", err.Error()) + } } func TestSubprocessRunnerSubprocessFailureAddsStderrDescriptorHint(t *testing.T) { @@ -297,13 +293,14 @@ func TestSubprocessRunnerSubprocessFailureAddsStderrDescriptorHint(t *testing.T) t.Setenv("AUDITA_HELPER_RECORD_PATH", filepath.Join(t.TempDir(), "record.json")) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "OPENAI_KEY_SOURCE", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: true, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "super-secret", + LLMAPIKeyReference: "OPENAI_KEY_SOURCE", + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: true, }) req := auditaReqForTest(t, true) _, err := runner.Run(context.Background(), req) @@ -322,13 +319,14 @@ func TestSubprocessRunnerMissingOutputFails(t *testing.T) { t.Setenv("AUDITA_HELPER_RECORD_PATH", filepath.Join(t.TempDir(), "record.json")) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "OPENAI_KEY_SOURCE", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: false, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "super-secret", + LLMAPIKeyReference: "OPENAI_KEY_SOURCE", + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: false, }) req := auditaReqForTest(t, false) _, err := runner.Run(context.Background(), req) @@ -347,13 +345,14 @@ func TestSubprocessRunnerInvalidOutputJSONFails(t *testing.T) { t.Setenv("AUDITA_HELPER_RECORD_PATH", filepath.Join(t.TempDir(), "record.json")) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "OPENAI_KEY_SOURCE", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: false, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "super-secret", + LLMAPIKeyReference: "OPENAI_KEY_SOURCE", + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: false, }) req := auditaReqForTest(t, false) _, err := runner.Run(context.Background(), req) @@ -372,13 +371,14 @@ func TestSubprocessRunnerSegmentsMissingFails(t *testing.T) { t.Setenv("AUDITA_HELPER_RECORD_PATH", filepath.Join(t.TempDir(), "record.json")) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "OPENAI_KEY_SOURCE", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: false, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "super-secret", + LLMAPIKeyReference: "OPENAI_KEY_SOURCE", + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: false, }) req := auditaReqForTest(t, false) _, err := runner.Run(context.Background(), req) @@ -397,13 +397,14 @@ func TestSubprocessRunnerInvalidReportJSONFails(t *testing.T) { t.Setenv("AUDITA_HELPER_RECORD_PATH", filepath.Join(t.TempDir(), "record.json")) runner := mustAuditaRunner(t, SubprocessRunnerConfig{ - Binary: writeAuditaHelperWrapper(t), - Timeout: mustParseAuditaDuration(t, "2s"), - LLMAPIKeyEnv: "OPENAI_KEY_SOURCE", - Modules: []string{"glossary"}, - BaseURL: "https://openrouter.ai/api/v1", - Model: "openrouter/google/gemma-4-31b-it", - Report: true, + Binary: writeAuditaHelperWrapper(t), + Timeout: mustParseAuditaDuration(t, "2s"), + LLMAPIKey: "super-secret", + LLMAPIKeyReference: "OPENAI_KEY_SOURCE", + Modules: []string{"glossary"}, + BaseURL: "https://openrouter.ai/api/v1", + Model: "openrouter/google/gemma-4-31b-it", + Report: true, }) req := auditaReqForTest(t, true) _, err := runner.Run(context.Background(), req) @@ -416,21 +417,22 @@ func TestSubprocessRunnerInvalidReportJSONFails(t *testing.T) { } func TestSubprocessRunnerConstructorValidation(t *testing.T) { - _, err := NewSubprocessRunnerFromConfigValues("", "3h", "AUDITA_LLM_API_KEY", []string{"glossary"}, "https://openrouter.ai/api/v1", "openrouter/google/gemma-4-31b-it", "", "", "", "", nil, nil, "", nil, true) + _, err := NewSubprocessRunnerFromConfigValues("", "3h", "synthetic-secret", "AUDITA_LLM_API_KEY", []string{"glossary"}, "https://openrouter.ai/api/v1", "openrouter/google/gemma-4-31b-it", "", "", "", "", nil, nil, "", nil, true) if err == nil { t.Fatal("expected binary validation error") } - _, err = NewSubprocessRunnerFromConfigValues("audita", "bad", "AUDITA_LLM_API_KEY", []string{"glossary"}, "https://openrouter.ai/api/v1", "openrouter/google/gemma-4-31b-it", "", "", "", "", nil, nil, "", nil, true) + _, err = NewSubprocessRunnerFromConfigValues("audita", "bad", "synthetic-secret", "AUDITA_LLM_API_KEY", []string{"glossary"}, "https://openrouter.ai/api/v1", "openrouter/google/gemma-4-31b-it", "", "", "", "", nil, nil, "", nil, true) if err == nil { t.Fatal("expected timeout parse error") } _, err = NewSubprocessRunner(SubprocessRunnerConfig{ - Binary: "audita", - Timeout: mustParseAuditaDuration(t, "1s"), - LLMAPIKeyEnv: "AUDITA_LLM_API_KEY", - Modules: []string{"glossary"}, - BaseURL: "://", - Model: "openrouter/google/gemma-4-31b-it", + Binary: "audita", + Timeout: mustParseAuditaDuration(t, "1s"), + LLMAPIKey: "synthetic-secret", + LLMAPIKeyReference: "AUDITA_LLM_API_KEY", + Modules: []string{"glossary"}, + BaseURL: "://", + Model: "openrouter/google/gemma-4-31b-it", }) if err == nil { t.Fatal("expected base url validation error") diff --git a/internal/adapters/storage/s3_backend.go b/internal/adapters/storage/s3_backend.go index 5a102d9..0941fd3 100644 --- a/internal/adapters/storage/s3_backend.go +++ b/internal/adapters/storage/s3_backend.go @@ -37,8 +37,12 @@ type s3ClientOptions struct { Region string Endpoint string ForcePathStyle bool - AccessKeyID string - SecretKey string + Credentials *s3StaticCredentials +} + +type s3StaticCredentials struct { + AccessKeyID string + SecretAccessKey string } var newS3Client = func(ctx context.Context, opts s3ClientOptions) (s3API, error) { @@ -46,11 +50,11 @@ var newS3Client = func(ctx context.Context, opts s3ClientOptions) (s3API, error) if strings.TrimSpace(opts.Region) != "" { loadOpts = append(loadOpts, awsconfig.WithRegion(strings.TrimSpace(opts.Region))) } - if strings.TrimSpace(opts.AccessKeyID) != "" && strings.TrimSpace(opts.SecretKey) != "" { + if opts.Credentials != nil { loadOpts = append(loadOpts, awsconfig.WithCredentialsProvider( credentials.NewStaticCredentialsProvider( - strings.TrimSpace(opts.AccessKeyID), - strings.TrimSpace(opts.SecretKey), + opts.Credentials.AccessKeyID, + opts.Credentials.SecretAccessKey, "", ), )) @@ -75,14 +79,24 @@ func NewS3BackendFromConfig(ctx context.Context, cfg config.StorageS3Config) (*S if bucket == "" { return nil, fmt.Errorf("storage.s3.bucket is required") } + if (cfg.AccessKeyID == nil) != (cfg.SecretAccessKey == nil) { + return nil, fmt.Errorf("storage.s3 access key ID and secret access key references must be configured together") + } + if cfg.AccessKeyID != nil && (!cfg.AccessKeyID.IsResolved() || !cfg.SecretAccessKey.IsResolved()) { + return nil, fmt.Errorf("storage.s3 configured credential references must be resolved") + } - client, err := newS3Client(ctx, s3ClientOptions{ + clientOptions := s3ClientOptions{ Region: cfg.Region, Endpoint: cfg.Endpoint, ForcePathStyle: cfg.ForcePathStyle, - AccessKeyID: s3CredentialFromEnv(orDefaultEnvName(cfg.AccessKeyIDEnv, config.DefaultS3AccessKeyIDEnv)), - SecretKey: s3CredentialFromEnv(orDefaultEnvName(cfg.SecretKeyEnv, config.DefaultS3SecretAccessKeyEnv)), - }) + } + if cfg.AccessKeyID != nil && cfg.SecretAccessKey != nil { + clientOptions.Credentials = &s3StaticCredentials{ + AccessKeyID: cfg.AccessKeyID.Value(), SecretAccessKey: cfg.SecretAccessKey.Value(), + } + } + client, err := newS3Client(ctx, clientOptions) if err != nil { return nil, fmt.Errorf("build s3 client: %w", err) } @@ -93,26 +107,6 @@ func NewS3BackendFromConfig(ctx context.Context, cfg config.StorageS3Config) (*S }, nil } -func s3CredentialFromEnv(envVarName string) string { - name := strings.TrimSpace(envVarName) - if name == "" { - return "" - } - value, ok := os.LookupEnv(name) - if !ok { - return "" - } - return strings.TrimSpace(value) -} - -func orDefaultEnvName(name, fallback string) string { - trimmed := strings.TrimSpace(name) - if trimmed == "" { - return fallback - } - return trimmed -} - // List returns objects under prefix. func (b *S3Backend) List(ctx context.Context, prefix string) ([]ObjectInfo, error) { normalizedPrefix := normalizeObjectKey(prefix) diff --git a/internal/adapters/storage/s3_backend_test.go b/internal/adapters/storage/s3_backend_test.go index 76b511d..2b48b52 100644 --- a/internal/adapters/storage/s3_backend_test.go +++ b/internal/adapters/storage/s3_backend_test.go @@ -14,6 +14,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/aws/smithy-go" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/narratio/internal/config" ) @@ -319,8 +320,10 @@ func TestS3BackendDeleteHonorsCancellation(t *testing.T) { func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) { original := newS3Client t.Cleanup(func() { newS3Client = original }) - t.Setenv("OBJECT_STORAGE_KEY_ID", "id-123") - t.Setenv("OBJECT_STORAGE_KEY", "secret-abc") + accessKey := resolvedTestSecret(t, "NARRATIO_S3_TEST_ACCESS", "id-123") + secretKey := resolvedTestSecret(t, "NARRATIO_S3_TEST_SECRET", "secret-abc") + t.Setenv("NARRATIO_S3_TEST_ACCESS", "changed-id") + t.Setenv("NARRATIO_S3_TEST_SECRET", "changed-secret") var got s3ClientOptions newS3Client = func(_ context.Context, opts s3ClientOptions) (s3API, error) { @@ -329,10 +332,12 @@ func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) { } backend, err := NewS3BackendFromConfig(context.Background(), config.StorageS3Config{ - Bucket: "my-archive", - Region: "us-east-1", - Endpoint: "http://localhost:9000", - ForcePathStyle: true, + Bucket: "my-archive", + Region: "us-east-1", + Endpoint: "http://localhost:9000", + ForcePathStyle: true, + AccessKeyID: accessKey, + SecretAccessKey: secretKey, }) if err != nil { t.Fatalf("NewS3BackendFromConfig() error = %v", err) @@ -343,7 +348,7 @@ func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) { if got.Region != "us-east-1" || got.Endpoint != "http://localhost:9000" || !got.ForcePathStyle { t.Fatalf("client options = %#v, want region/endpoint/path-style values", got) } - if got.AccessKeyID != "id-123" || got.SecretKey != "secret-abc" { + if got.Credentials == nil || got.Credentials.AccessKeyID != "id-123" || got.Credentials.SecretAccessKey != "secret-abc" { t.Fatalf("client options credentials = %#v, want env-resolved static credentials", got) } } @@ -355,7 +360,21 @@ func TestNewS3BackendFromConfigRequiresBucket(t *testing.T) { } } -func TestNewS3BackendFromConfigFallsBackWhenCredentialEnvMissing(t *testing.T) { +func TestNewS3BackendFromConfigRejectsIncompleteOrUnresolvedCredentials(t *testing.T) { + unresolved := configkit.SecretEnv("NARRATIO_UNRESOLVED_S3_KEY") + for name, cfg := range map[string]config.StorageS3Config{ + "partial": {Bucket: "my-archive", AccessKeyID: &unresolved}, + "unresolved": {Bucket: "my-archive", AccessKeyID: &unresolved, SecretAccessKey: &unresolved}, + } { + t.Run(name, func(t *testing.T) { + if _, err := NewS3BackendFromConfig(context.Background(), cfg); err == nil { + t.Fatal("NewS3BackendFromConfig() succeeded, want credential error") + } + }) + } +} + +func TestNewS3BackendFromConfigUsesDefaultProviderWithoutCredentialReferences(t *testing.T) { original := newS3Client t.Cleanup(func() { newS3Client = original }) @@ -366,19 +385,30 @@ func TestNewS3BackendFromConfigFallsBackWhenCredentialEnvMissing(t *testing.T) { } _, err := NewS3BackendFromConfig(context.Background(), config.StorageS3Config{ - Bucket: "my-archive", - Region: "us-east-1", - AccessKeyIDEnv: "MISSING_ACCESS_KEY_ID", - SecretKeyEnv: "MISSING_SECRET_KEY", + Bucket: "my-archive", + Region: "us-east-1", }) if err != nil { t.Fatalf("NewS3BackendFromConfig() error = %v", err) } - if got.AccessKeyID != "" || got.SecretKey != "" { - t.Fatalf("client options credentials = %#v, want empty fallback values", got) + if got.Credentials != nil { + t.Fatalf("client options credentials = %#v, want default provider chain", got.Credentials) } } +func resolvedTestSecret(t *testing.T, name, value string) *configkit.Secret { + t.Helper() + t.Setenv(name, value) + type document struct { + Secret *configkit.Secret `yaml:"secret"` + } + result, err := configkit.Load(configkit.Bytes("test.yml", []byte("secret: "+name+"\n")), configkit.Options[document]{}) + if err != nil { + t.Fatalf("resolve test secret: %v", err) + } + return result.Config.Secret +} + func strPtr(v string) *string { return &v } func int64Ptr(v int64) *int64 { return &v } func boolPtr(v bool) *bool { return &v } diff --git a/internal/app/commands_test.go b/internal/app/commands_test.go index 51dc05b..c49ff2f 100644 --- a/internal/app/commands_test.go +++ b/internal/app/commands_test.go @@ -217,7 +217,7 @@ seriatim: binary: ` + seriatimBinary + ` audita: binary: ` + auditaBinary + ` - llm_api_key_env: OPENROUTER_API_KEY + llm_api_key: OPENROUTER_API_KEY notification: mode: noop ` @@ -262,6 +262,28 @@ inputs: if !strings.Contains(stdout.String(), "stage=polish executed=1 skipped=0") { t.Fatalf("stdout = %q, want polish execution", stdout.String()) } + foundReference := false + if err := filepath.WalkDir(workRoot, func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil || entry.IsDir() { + return walkErr + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + if bytes.Contains(data, []byte("from-secret-file")) { + t.Fatalf("workspace output %q exposed credential value", path) + } + if bytes.Contains(data, []byte("OPENROUTER_API_KEY")) { + foundReference = true + } + return nil + }); err != nil { + t.Fatalf("inspect polish outputs: %v", err) + } + if !foundReference { + t.Fatal("polish outputs omitted safe credential reference") + } } func TestExecuteRunFailsWhenExplicitSecretsDirMissing(t *testing.T) { diff --git a/internal/app/config_commands_test.go b/internal/app/config_commands_test.go index aa2850b..cd8bf83 100644 --- a/internal/app/config_commands_test.go +++ b/internal/app/config_commands_test.go @@ -299,6 +299,62 @@ func TestConfigDiffBootstrapsSecretsOnceForBothProfiles(t *testing.T) { } } +func TestConfigInspectionOutputsNeverExposeResolvedCredentialValues(t *testing.T) { + const firstCanary = "inspection-credential-canary-one" + const secondCanary = "inspection-credential-canary-two" + for name, value := range map[string]string{ + "NARRATIO_INSPECT_ACCESS": firstCanary, + "NARRATIO_INSPECT_SECRET": firstCanary, + "NARRATIO_INSPECT_LLM_ONE": firstCanary, + "NARRATIO_INSPECT_LLM_TWO": secondCanary, + } { + t.Setenv(name, value) + } + workspaceRoot := t.TempDir() + pipelinePath, _, _ := writeValidConfigFiles(t, workspaceRoot) + pipelineData, err := os.ReadFile(pipelinePath) + if err != nil { + t.Fatal(err) + } + pipelineYAML := strings.Replace(string(pipelineData), " bucket: test-bucket\n", ` bucket: test-bucket + access_key_id: NARRATIO_INSPECT_ACCESS + secret_access_key: NARRATIO_INSPECT_SECRET +`, 1) + if err := os.WriteFile(pipelinePath, []byte(pipelineYAML), 0o644); err != nil { + t.Fatal(err) + } + writeInspectionProfiles(t, pipelinePath, + "audita:\n model: production\n llm_api_key: NARRATIO_INSPECT_LLM_ONE\n", + "audita:\n model: testing\n llm_api_key: NARRATIO_INSPECT_LLM_TWO\n", + ) + + var show bytes.Buffer + if err := ConfigShow(context.Background(), []string{"--config", pipelinePath, "--profile", "production"}, &show); err != nil { + t.Fatalf("config show: %v", err) + } + var sources bytes.Buffer + if err := ConfigSources(context.Background(), []string{"--config", pipelinePath, "--profile", "production"}, &sources); err != nil { + t.Fatalf("config sources: %v", err) + } + var diff bytes.Buffer + if err := ConfigDiff(context.Background(), []string{"production", "testing", "--config", pipelinePath}, &diff); err != nil { + t.Fatalf("config diff: %v", err) + } + for label, output := range map[string]string{"show": show.String(), "sources": sources.String(), "diff": diff.String()} { + if strings.Contains(output, firstCanary) || strings.Contains(output, secondCanary) { + t.Fatalf("%s output exposed credential value: %q", label, output) + } + } + for _, reference := range []string{"NARRATIO_INSPECT_ACCESS", "NARRATIO_INSPECT_SECRET", "NARRATIO_INSPECT_LLM_ONE"} { + if !strings.Contains(show.String(), reference) { + t.Fatalf("show omitted safe credential reference %q: %q", reference, show.String()) + } + } + if !strings.Contains(diff.String(), "NARRATIO_INSPECT_LLM_ONE") || !strings.Contains(diff.String(), "NARRATIO_INSPECT_LLM_TWO") { + t.Fatalf("diff omitted safe credential references: %q", diff.String()) + } +} + func TestConfigCommandsInspectMaintainedSplitBundleWithoutRuntimeState(t *testing.T) { examplesDir := filepath.Join("..", "..", "examples") pipelinePath := filepath.Join(examplesDir, "production-testing", "pipeline.yml") diff --git a/internal/app/object_store_test.go b/internal/app/object_store_test.go index eb807d0..0343be5 100644 --- a/internal/app/object_store_test.go +++ b/internal/app/object_store_test.go @@ -31,9 +31,9 @@ func commandObjectStoreTestConfig() *config.Config { Storage: config.StorageConfig{ Backend: "s3", S3: &config.StorageS3Config{ - Bucket: "test-bucket", - AccessKeyIDEnv: "NARRATIO_TEST_COMMAND_STORE_KEY_ID", - SecretKeyEnv: "NARRATIO_TEST_COMMAND_STORE_SECRET", + Bucket: "test-bucket", + AccessKeyID: testSecretReference("NARRATIO_TEST_COMMAND_STORE_KEY_ID"), + SecretAccessKey: testSecretReference("NARRATIO_TEST_COMMAND_STORE_SECRET"), }, }, }, diff --git a/internal/app/remote_session_test.go b/internal/app/remote_session_test.go index 7628b33..e25bfb9 100644 --- a/internal/app/remote_session_test.go +++ b/internal/app/remote_session_test.go @@ -478,7 +478,7 @@ 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+"\n", + "storage:\n backend: s3\n s3:\n bucket: test-bucket\n access_key_id: "+accessKeyEnv+"\n secret_access_key: "+secretKeyEnv+"\n", 1, ) if err := os.WriteFile(pipelinePath, []byte(pipelineYAML), 0o644); err != nil { diff --git a/internal/app/runner.go b/internal/app/runner.go index 7af8d11..58fb041 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -211,11 +211,18 @@ func buildDefaultAuditaRunner(cfg *config.Config) (audita.Runner, error) { if a.Report != nil { report = *a.Report } + llmAPIKey := "" + llmAPIKeyReference := "" + if a.LLMAPIKey != nil { + llmAPIKey = a.LLMAPIKey.Value() + llmAPIKeyReference = a.LLMAPIKey.EnvName() + } runner, err := audita.NewSubprocessRunnerFromConfigValues( a.Binary, a.Timeout, - a.LLMAPIKeyEnv, + llmAPIKey, + llmAPIKeyReference, append([]string(nil), a.Modules...), a.BaseURL, a.Model, diff --git a/internal/app/semantic_resume_test.go b/internal/app/semantic_resume_test.go index b1895aa..352d118 100644 --- a/internal/app/semantic_resume_test.go +++ b/internal/app/semantic_resume_test.go @@ -471,8 +471,8 @@ func TestDeliverySemanticChangesInvalidateOnlyTheirFixedDependents(t *testing.T) cfg.Pipeline.Publish.Outputs[0].Dest = "published/alternate.json" }, wantRuns: [2]int{1, 1}}, {name: "publish credentials", mutate: func(cfg *config.Config) { - cfg.Pipeline.Storage.S3.AccessKeyIDEnv = "OTHER_ACCESS_KEY" - cfg.Pipeline.Storage.S3.SecretKeyEnv = "OTHER_SECRET_KEY" + cfg.Pipeline.Storage.S3.AccessKeyID = testSecretReference("OTHER_ACCESS_KEY") + cfg.Pipeline.Storage.S3.SecretAccessKey = testSecretReference("OTHER_SECRET_KEY") }, wantRuns: [2]int{0, 0}}, {name: "notify mode", mutate: func(cfg *config.Config) { cfg.Pipeline.Notification.Mode = "webhook" @@ -534,7 +534,7 @@ func configureDeliverySemanticConfig(cfg *config.Config) { Backend: config.StorageBackendLocal, S3: &config.StorageS3Config{ Bucket: "campaign", RootPrefix: "narratio", Region: "us-east-1", - Endpoint: "https://objects.example", AccessKeyIDEnv: "ACCESS_KEY", SecretKeyEnv: "SECRET_KEY", + Endpoint: "https://objects.example", AccessKeyID: testSecretReference("ACCESS_KEY"), SecretAccessKey: testSecretReference("SECRET_KEY"), }, } cfg.Pipeline.Notification.Mode = config.DefaultNotificationMode diff --git a/internal/app/test_environment_test.go b/internal/app/test_environment_test.go index d735f7c..01c6ee3 100644 --- a/internal/app/test_environment_test.go +++ b/internal/app/test_environment_test.go @@ -4,6 +4,8 @@ import ( "os" "path/filepath" "testing" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" ) // restoreEnvAfterTest clears names for a test and restores both their prior @@ -31,6 +33,11 @@ func restoreEnvAfterTest(t *testing.T, names ...string) { }) } +func testSecretReference(name string) *configkit.Secret { + reference := configkit.SecretEnv(name) + return &reference +} + func mustWriteSecretFile(t *testing.T, path, value string) { t.Helper() if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { diff --git a/internal/config/config.go b/internal/config/config.go index 34b7208..6dae163 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,5 +1,7 @@ package config +import configkit "gitea.maximumdirect.net/eric/ConfigKit" + // Config is the resolved combined configuration from pipeline.yml, // campaign.yml, and session.yml. type Config struct { @@ -98,13 +100,13 @@ const ( // StorageS3Config configures S3 storage coordinates. type StorageS3Config struct { - Bucket string `yaml:"bucket" json:"bucket"` - RootPrefix string `yaml:"root_prefix" json:"root_prefix"` - Region string `yaml:"region" json:"region"` - Endpoint string `yaml:"endpoint" json:"endpoint"` - ForcePathStyle bool `yaml:"force_path_style" json:"force_path_style"` - AccessKeyIDEnv string `yaml:"access_key_id_env" json:"access_key_id_env"` - SecretKeyEnv string `yaml:"secret_access_key_env" json:"secret_access_key_env"` + Bucket string `yaml:"bucket" json:"bucket"` + RootPrefix string `yaml:"root_prefix" json:"root_prefix"` + Region string `yaml:"region" json:"region"` + Endpoint string `yaml:"endpoint" json:"endpoint"` + ForcePathStyle bool `yaml:"force_path_style" json:"force_path_style"` + AccessKeyID *configkit.Secret `yaml:"access_key_id,omitempty" json:"access_key_id,omitempty"` + SecretAccessKey *configkit.Secret `yaml:"secret_access_key,omitempty" json:"secret_access_key,omitempty"` } // SpoolConfig configures local spool storage for staged data. @@ -163,21 +165,21 @@ type SeriatimEnvConfig struct { // AuditaConfig configures audita adapter settings. type AuditaConfig struct { - Binary string `yaml:"binary" json:"binary"` - Timeout string `yaml:"timeout" json:"timeout"` - LLMAPIKeyEnv string `yaml:"llm_api_key_env" json:"llm_api_key_env"` - Modules []string `yaml:"modules" json:"modules"` - BaseURL string `yaml:"base_url" json:"base_url"` - Model string `yaml:"model" json:"model"` - TotalLLMConcurrency *int `yaml:"total_llm_concurrency" json:"total_llm_concurrency"` - ProposalLLMConcurrency *int `yaml:"proposal_llm_concurrency" json:"proposal_llm_concurrency"` - ValidationModel string `yaml:"validation_model" json:"validation_model"` - ValidationLLMConcurrency *int `yaml:"validation_llm_concurrency" json:"validation_llm_concurrency"` - TranscriptDescription string `yaml:"transcript_description" json:"transcript_description"` - ConfigPath string `yaml:"config_path" json:"config_path"` - OutputSchema string `yaml:"output_schema" json:"output_schema"` - WorkDirRetention string `yaml:"work_dir_retention" json:"work_dir_retention"` - Report *bool `yaml:"report" json:"report"` + Binary string `yaml:"binary" json:"binary"` + Timeout string `yaml:"timeout" json:"timeout"` + LLMAPIKey *configkit.Secret `yaml:"llm_api_key,omitempty" json:"llm_api_key,omitempty"` + Modules []string `yaml:"modules" json:"modules"` + BaseURL string `yaml:"base_url" json:"base_url"` + Model string `yaml:"model" json:"model"` + TotalLLMConcurrency *int `yaml:"total_llm_concurrency" json:"total_llm_concurrency"` + ProposalLLMConcurrency *int `yaml:"proposal_llm_concurrency" json:"proposal_llm_concurrency"` + ValidationModel string `yaml:"validation_model" json:"validation_model"` + ValidationLLMConcurrency *int `yaml:"validation_llm_concurrency" json:"validation_llm_concurrency"` + TranscriptDescription string `yaml:"transcript_description" json:"transcript_description"` + ConfigPath string `yaml:"config_path" json:"config_path"` + OutputSchema string `yaml:"output_schema" json:"output_schema"` + WorkDirRetention string `yaml:"work_dir_retention" json:"work_dir_retention"` + Report *bool `yaml:"report" json:"report"` } // NormalizeConfig configures normalize-stage transcript schema/output behavior. diff --git a/internal/config/credential_projection.go b/internal/config/credential_projection.go new file mode 100644 index 0000000..85fad96 --- /dev/null +++ b/internal/config/credential_projection.go @@ -0,0 +1,59 @@ +package config + +import configkit "gitea.maximumdirect.net/eric/ConfigKit" + +// MarshalYAML exposes only public credential reference names. Resolved values +// remain available solely through ConfigKit's explicit trusted accessor. +func (cfg StorageS3Config) MarshalYAML() (any, error) { + type safeStorageS3 struct { + Bucket string `yaml:"bucket"` + RootPrefix string `yaml:"root_prefix"` + Region string `yaml:"region"` + Endpoint string `yaml:"endpoint"` + ForcePathStyle bool `yaml:"force_path_style"` + AccessKeyID *string `yaml:"access_key_id,omitempty"` + SecretAccessKey *string `yaml:"secret_access_key,omitempty"` + } + return safeStorageS3{ + Bucket: cfg.Bucket, RootPrefix: cfg.RootPrefix, Region: cfg.Region, + Endpoint: cfg.Endpoint, ForcePathStyle: cfg.ForcePathStyle, + AccessKeyID: secretReferenceName(cfg.AccessKeyID), SecretAccessKey: secretReferenceName(cfg.SecretAccessKey), + }, nil +} + +// MarshalYAML exposes only the public Audita credential reference name. +func (cfg AuditaConfig) MarshalYAML() (any, error) { + type safeAudita struct { + Binary string `yaml:"binary"` + Timeout string `yaml:"timeout"` + LLMAPIKey *string `yaml:"llm_api_key,omitempty"` + Modules []string `yaml:"modules"` + BaseURL string `yaml:"base_url"` + Model string `yaml:"model"` + TotalLLMConcurrency *int `yaml:"total_llm_concurrency"` + ProposalLLMConcurrency *int `yaml:"proposal_llm_concurrency"` + ValidationModel string `yaml:"validation_model"` + ValidationLLMConcurrency *int `yaml:"validation_llm_concurrency"` + TranscriptDescription string `yaml:"transcript_description"` + ConfigPath string `yaml:"config_path"` + OutputSchema string `yaml:"output_schema"` + WorkDirRetention string `yaml:"work_dir_retention"` + Report *bool `yaml:"report"` + } + return safeAudita{ + Binary: cfg.Binary, Timeout: cfg.Timeout, LLMAPIKey: secretReferenceName(cfg.LLMAPIKey), + Modules: cfg.Modules, BaseURL: cfg.BaseURL, Model: cfg.Model, + TotalLLMConcurrency: cfg.TotalLLMConcurrency, ProposalLLMConcurrency: cfg.ProposalLLMConcurrency, + ValidationModel: cfg.ValidationModel, ValidationLLMConcurrency: cfg.ValidationLLMConcurrency, + TranscriptDescription: cfg.TranscriptDescription, ConfigPath: cfg.ConfigPath, + OutputSchema: cfg.OutputSchema, WorkDirRetention: cfg.WorkDirRetention, Report: cfg.Report, + }, nil +} + +func secretReferenceName(secret *configkit.Secret) *string { + if secret == nil { + return nil + } + name := secret.EnvName() + return &name +} diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 2d98e38..62ad992 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -9,8 +9,6 @@ const ( DefaultPipelineConfigPathEtc = "/etc/narratio/pipeline.yml" DefaultSessionConfigPathUsrLocal = "/usr/local/etc/narratio/session.yml" DefaultSessionConfigPathEtc = "/etc/narratio/session.yml" - DefaultS3AccessKeyIDEnv = "OBJECT_STORAGE_KEY_ID" - DefaultS3SecretAccessKeyEnv = "OBJECT_STORAGE_KEY" DefaultStorageS3RootPrefix = "dnd" DefaultWorkspaceRoot = "/var/lib/narratio" DefaultCampaignsRoot = "/usr/local/share/narratio/campaigns" diff --git a/internal/config/load_validate_test.go b/internal/config/load_validate_test.go index 1c03185..cf73673 100644 --- a/internal/config/load_validate_test.go +++ b/internal/config/load_validate_test.go @@ -1,6 +1,7 @@ package config import ( + "encoding/json" "io/fs" "os" "path/filepath" @@ -8,6 +9,8 @@ import ( "sort" "strings" "testing" + + "gopkg.in/yaml.v3" ) func TestRepresentativeLoadDefaultsAndValidation(t *testing.T) { @@ -83,6 +86,78 @@ whisperx: } } +func TestCredentialValuesNeverEnterEffectiveProjectionOrDigest(t *testing.T) { + const firstCanary = "credential-canary-first" + const secondCanary = "credential-canary-second" + t.Setenv("NARRATIO_TEST_ACCESS_KEY", firstCanary) + t.Setenv("NARRATIO_TEST_SECRET_KEY", firstCanary) + t.Setenv("NARRATIO_TEST_AUDITA_KEY", firstCanary) + pipelineYAML := strings.Replace(testPipelineBaseYAML, "audita:\n binary: audita\n", "audita:\n binary: audita\n llm_api_key: NARRATIO_TEST_AUDITA_KEY\n", 1) + ` +storage: + backend: s3 + s3: + bucket: test-bucket + access_key_id: NARRATIO_TEST_ACCESS_KEY + secret_access_key: NARRATIO_TEST_SECRET_KEY +` + pipelinePath, _ := writeConfigFiles(t, pipelineYAML, testSessionBaseYAML) + + first, err := LoadPipeline(pipelinePath) + if err != nil { + t.Fatal(err) + } + firstYAML, err := MarshalEffectivePipeline(first) + if err != nil { + t.Fatal(err) + } + firstValues, err := EffectivePipelineValues(first) + if err != nil { + t.Fatal(err) + } + serializedValues := "" + for _, value := range firstValues { + serializedValues += value.Path + "=" + value.Value + "\n" + } + directYAML, err := yaml.Marshal(first) + if err != nil { + t.Fatal(err) + } + directJSON, err := json.Marshal(first) + if err != nil { + t.Fatal(err) + } + for _, output := range []string{string(firstYAML), serializedValues, string(directYAML), string(directJSON)} { + if strings.Contains(output, firstCanary) { + t.Fatalf("effective output exposed credential value: %q", output) + } + for _, reference := range []string{"NARRATIO_TEST_ACCESS_KEY", "NARRATIO_TEST_SECRET_KEY", "NARRATIO_TEST_AUDITA_KEY"} { + if !strings.Contains(output, reference) { + t.Fatalf("effective output omitted safe reference %q: %q", reference, output) + } + } + } + + t.Setenv("NARRATIO_TEST_ACCESS_KEY", secondCanary) + t.Setenv("NARRATIO_TEST_SECRET_KEY", secondCanary) + t.Setenv("NARRATIO_TEST_AUDITA_KEY", secondCanary) + second, err := LoadPipeline(pipelinePath) + if err != nil { + t.Fatal(err) + } + if EffectivePipelineDigest(first) != EffectivePipelineDigest(second) { + t.Fatal("credential value change altered effective digest") + } + t.Setenv("NARRATIO_TEST_AUDITA_KEY_ALTERNATE", secondCanary) + alternatePath, _ := writeConfigFiles(t, strings.Replace(pipelineYAML, "NARRATIO_TEST_AUDITA_KEY", "NARRATIO_TEST_AUDITA_KEY_ALTERNATE", 1), testSessionBaseYAML) + alternate, err := LoadPipeline(alternatePath) + if err != nil { + t.Fatal(err) + } + if EffectivePipelineDigest(first) == EffectivePipelineDigest(alternate) { + t.Fatal("credential reference-name change did not alter effective digest") + } +} + func TestEffectivePipelineValuesPreserveNilEmptyAndExactIntegers(t *testing.T) { base := &PipelineConfig{Audita: AuditaConfig{Modules: nil}} nilValues, err := EffectivePipelineValues(base) @@ -106,6 +181,13 @@ func TestEffectivePipelineValuesPreserveNilEmptyAndExactIntegers(t *testing.T) { if got := effectiveValueForTest(t, emptyValues, "scriptorium.artifacts.recap.vars.exact"); got != "9007199254740993" { t.Fatalf("exact integer value = %q", got) } + encoded, err := MarshalEffectivePipeline(base) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(encoded), "9007199254740993") { + t.Fatalf("effective YAML lost exact integer: %q", encoded) + } } func effectiveValueForTest(t *testing.T, values []EffectivePipelineValueRecord, path string) string { @@ -138,6 +220,9 @@ func TestValidateMissingAudioSource(t *testing.T) { } func TestExamplesLoadAndValidate(t *testing.T) { + t.Setenv("OBJECT_STORAGE_KEY_ID", "synthetic-example-access-key") + t.Setenv("OBJECT_STORAGE_KEY", "synthetic-example-secret-key") + t.Setenv("AUDITA_LLM_API_KEY", "synthetic-example-audita-key") examplesDir := filepath.Join("..", "..", "examples") if got, want := maintainedPipelineRoots(t, examplesDir), []string{ "pipeline.extraction-subset.yml", @@ -226,7 +311,7 @@ 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.Audita.LLMAPIKeyEnv != "" || cfg.Pipeline.Storage.Backend != StorageBackendLocal || cfg.Pipeline.Storage.S3 != nil { + if cfg.Pipeline.Audita.LLMAPIKey != nil || 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 { @@ -295,6 +380,9 @@ func maintainedPipelineRoots(t *testing.T, examplesDir string) []string { } func TestMaintainedExtractionExamplesPreservePublishedContracts(t *testing.T) { + t.Setenv("OBJECT_STORAGE_KEY_ID", "synthetic-example-access-key") + t.Setenv("OBJECT_STORAGE_KEY", "synthetic-example-secret-key") + t.Setenv("AUDITA_LLM_API_KEY", "synthetic-example-audita-key") examplesDir := filepath.Join("..", "..", "examples") full, err := LoadPipeline(filepath.Join(examplesDir, "pipeline.full.annotated.yml")) if err != nil { diff --git a/internal/config/pipeline_configkit.go b/internal/config/pipeline_configkit.go index 0a8839b..d50e27f 100644 --- a/internal/config/pipeline_configkit.go +++ b/internal/config/pipeline_configkit.go @@ -163,8 +163,6 @@ func resolvePipelineConfig(cfg *PipelineConfig, resolution *configkit.Resolution if cfg.Storage.S3 != nil { path := configkit.Keys("storage", "s3") configkit.Default(resolution, path.Key("root_prefix"), &cfg.Storage.S3.RootPrefix, DefaultStorageS3RootPrefix, "default S3 root prefix") - configkit.Default(resolution, path.Key("access_key_id_env"), &cfg.Storage.S3.AccessKeyIDEnv, DefaultS3AccessKeyIDEnv, "default S3 access-key reference") - configkit.Default(resolution, path.Key("secret_access_key_env"), &cfg.Storage.S3.SecretKeyEnv, DefaultS3SecretAccessKeyEnv, "default S3 secret-key reference") } if cfg.Trim != nil && cfg.Trim.Enabled != nil && *cfg.Trim.Enabled && cfg.Scriptorium == nil { value := &ScriptoriumConfig{Binary: DefaultScriptoriumBinary, Timeout: DefaultScriptoriumTimeout} diff --git a/internal/config/storage_publish_test.go b/internal/config/storage_publish_test.go index be1b63f..a083d76 100644 --- a/internal/config/storage_publish_test.go +++ b/internal/config/storage_publish_test.go @@ -24,11 +24,8 @@ storage: if cfg.Pipeline.Storage.S3.RootPrefix != "dnd" { t.Fatalf("storage.s3.root_prefix = %q, want dnd", cfg.Pipeline.Storage.S3.RootPrefix) } - if cfg.Pipeline.Storage.S3.AccessKeyIDEnv != DefaultS3AccessKeyIDEnv { - t.Fatalf("storage.s3.access_key_id_env = %q, want %q", cfg.Pipeline.Storage.S3.AccessKeyIDEnv, DefaultS3AccessKeyIDEnv) - } - if cfg.Pipeline.Storage.S3.SecretKeyEnv != DefaultS3SecretAccessKeyEnv { - t.Fatalf("storage.s3.secret_access_key_env = %q, want %q", cfg.Pipeline.Storage.S3.SecretKeyEnv, DefaultS3SecretAccessKeyEnv) + if cfg.Pipeline.Storage.S3.AccessKeyID != nil || cfg.Pipeline.Storage.S3.SecretAccessKey != nil { + t.Fatal("storage.s3 credentials should be optional without implicit references") } if cfg.Pipeline.Storage.S3.ForcePathStyle { t.Fatalf("storage.s3.force_path_style = true, want false default") @@ -82,14 +79,32 @@ storage: } } +func TestLegacyCredentialReferenceFieldsFailStrictDecode(t *testing.T) { + for _, field := range []string{"access_key_id_env", "secret_access_key_env", "llm_api_key_env"} { + t.Run(field, func(t *testing.T) { + pipelineYAML := strings.Replace(testPipelineBaseYAML, "audita:\n binary: audita\n", "audita:\n binary: audita\n "+field+": LEGACY_SECRET\n", 1) + if strings.HasPrefix(field, "access_") || strings.HasPrefix(field, "secret_access_") { + pipelineYAML = testPipelineBaseYAML + "storage:\n backend: s3\n s3:\n bucket: test-bucket\n " + field + ": LEGACY_SECRET\n" + } + pipelinePath, _ := writeConfigFiles(t, pipelineYAML, testSessionBaseYAML) + _, err := LoadPipeline(pipelinePath) + if err == nil || !strings.Contains(err.Error(), field) || !strings.Contains(err.Error(), "strict decode") { + t.Fatalf("LoadPipeline() error = %v, want strict rejection of %s", err, field) + } + }) + } +} + func TestStorageS3CredentialEnvNamesLoadAndValidate(t *testing.T) { + t.Setenv("CUSTOM_KEY_ID", "synthetic-access-key") + t.Setenv("CUSTOM_SECRET", "synthetic-secret-key") pipelineYAML := testPipelineBaseYAML + ` storage: backend: s3 s3: bucket: my-dnd-archive - access_key_id_env: CUSTOM_KEY_ID - secret_access_key_env: CUSTOM_SECRET + access_key_id: CUSTOM_KEY_ID + secret_access_key: CUSTOM_SECRET ` pipelinePath, sessionPath := writeConfigFiles(t, pipelineYAML, testSessionBaseYAML) @@ -97,55 +112,69 @@ storage: if err != nil { t.Fatalf("Load() error = %v", err) } - if cfg.Pipeline.Storage.S3.AccessKeyIDEnv != "CUSTOM_KEY_ID" { - t.Fatalf("storage.s3.access_key_id_env = %q, want CUSTOM_KEY_ID", cfg.Pipeline.Storage.S3.AccessKeyIDEnv) + if cfg.Pipeline.Storage.S3.AccessKeyID == nil || cfg.Pipeline.Storage.S3.AccessKeyID.EnvName() != "CUSTOM_KEY_ID" || cfg.Pipeline.Storage.S3.AccessKeyID.Value() != "synthetic-access-key" { + t.Fatal("storage.s3.access_key_id did not retain its resolved typed reference") } - if cfg.Pipeline.Storage.S3.SecretKeyEnv != "CUSTOM_SECRET" { - t.Fatalf("storage.s3.secret_access_key_env = %q, want CUSTOM_SECRET", cfg.Pipeline.Storage.S3.SecretKeyEnv) + if cfg.Pipeline.Storage.S3.SecretAccessKey == nil || cfg.Pipeline.Storage.S3.SecretAccessKey.EnvName() != "CUSTOM_SECRET" || cfg.Pipeline.Storage.S3.SecretAccessKey.Value() != "synthetic-secret-key" { + t.Fatal("storage.s3.secret_access_key did not retain its resolved typed reference") } if err := Validate(cfg); err != nil { t.Fatalf("Validate() error = %v", err) } } -func TestStorageS3CredentialEnvValidation(t *testing.T) { +func TestStorageS3CredentialReferenceValidation(t *testing.T) { tests := []struct { name string pipelineYML string wantErr string }{ { - name: "invalid access key env name", + name: "invalid access key reference", pipelineYML: testPipelineBaseYAML + ` storage: backend: s3 s3: bucket: my-dnd-archive - access_key_id_env: "123BAD" + access_key_id: "123BAD" + secret_access_key: CUSTOM_SECRET `, - wantErr: "pipeline.storage.s3.access_key_id_env must be a valid environment variable name", + wantErr: "secret.invalid_name", }, { - name: "invalid secret key env name", + name: "partial pair", pipelineYML: testPipelineBaseYAML + ` storage: backend: s3 s3: bucket: my-dnd-archive - secret_access_key_env: "bad-name" + access_key_id: CUSTOM_KEY_ID `, - wantErr: "pipeline.storage.s3.secret_access_key_env must be a valid environment variable name", + wantErr: "must be configured together", + }, + { + name: "missing effective reference", + pipelineYML: testPipelineBaseYAML + ` +storage: + backend: s3 + s3: + bucket: my-dnd-archive + access_key_id: MISSING_CUSTOM_KEY_ID + secret_access_key: MISSING_CUSTOM_SECRET +`, + wantErr: "secret.unset", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - pipelinePath, sessionPath := writeConfigFiles(t, tt.pipelineYML, testSessionBaseYAML) - cfg, err := Load(pipelinePath, sessionPath) - if err != nil { - t.Fatalf("Load() error = %v", err) + pipelinePath, _ := writeConfigFiles(t, tt.pipelineYML, testSessionBaseYAML) + t.Setenv("CUSTOM_KEY_ID", "synthetic-access-key") + t.Setenv("CUSTOM_SECRET", "synthetic-secret-key") + cfg, err := LoadPipeline(pipelinePath) + if err == nil { + err = ValidatePipelineConfig(cfg) } - err = Validate(cfg) if err == nil || !strings.Contains(err.Error(), tt.wantErr) { t.Fatalf("Validate() error = %v, want to contain %q", err, tt.wantErr) } diff --git a/internal/config/validate_helpers.go b/internal/config/validate_helpers.go index a9b5957..73583a0 100644 --- a/internal/config/validate_helpers.go +++ b/internal/config/validate_helpers.go @@ -10,7 +10,6 @@ import ( ) var windowsAbsPathRE = regexp.MustCompile(`^[A-Za-z]:[\\/].*`) -var envVarNameRE = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) func sortedStringKeys[V any](values map[string]V) []string { keys := make([]string, 0, len(values)) @@ -21,17 +20,6 @@ func sortedStringKeys[V any](values map[string]V) []string { return keys } -func validateEnvVarNameField(fieldName, value string) error { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - return fmt.Errorf("%s must be non-empty", fieldName) - } - if !envVarNameRE.MatchString(trimmed) { - return fmt.Errorf("%s must be a valid environment variable name", fieldName) - } - return nil -} - func validateRelativeSafePath(fieldName, value string) error { trimmed := strings.TrimSpace(value) if trimmed == "" { diff --git a/internal/config/validate_publish.go b/internal/config/validate_publish.go index 0bb08d5..dff6d81 100644 --- a/internal/config/validate_publish.go +++ b/internal/config/validate_publish.go @@ -33,11 +33,8 @@ func validateStorage(cfg StorageConfig) error { if cfg.S3.Endpoint != "" && strings.TrimSpace(cfg.S3.Endpoint) == "" { return fmt.Errorf("pipeline.storage.s3.endpoint must be non-empty when provided") } - if err := validateEnvVarNameField("pipeline.storage.s3.access_key_id_env", cfg.S3.AccessKeyIDEnv); err != nil { - return err - } - if err := validateEnvVarNameField("pipeline.storage.s3.secret_access_key_env", cfg.S3.SecretKeyEnv); err != nil { - return err + if (cfg.S3.AccessKeyID == nil) != (cfg.S3.SecretAccessKey == nil) { + return fmt.Errorf("pipeline.storage.s3.access_key_id and pipeline.storage.s3.secret_access_key must be configured together") } return nil } diff --git a/internal/stage/polish.go b/internal/stage/polish.go index 0e6567c..d6420f7 100644 --- a/internal/stage/polish.go +++ b/internal/stage/polish.go @@ -161,6 +161,10 @@ func (polishStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*St if reportEnabled { reportCanonicalPath = canonicalReportPath } + credentialReference := "" + if env.Config.Pipeline.Audita.LLMAPIKey != nil { + credentialReference = env.Config.Pipeline.Audita.LLMAPIKey.EnvName() + } meta := map[string]any{ "stage": "polish", @@ -184,7 +188,7 @@ func (polishStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*St "total_llm_concurrency": totalLLMConcurrency, "proposal_llm_concurrency": proposalLLMConcurrency, "validation_llm_concurrency": validationConcurrency, - "llm_api_key_env": env.Config.Pipeline.Audita.LLMAPIKeyEnv, + "llm_api_key": credentialReference, "timeout": env.Config.Pipeline.Audita.Timeout, "binary": env.Config.Pipeline.Audita.Binary, "generated_config_path": generatedConfigPath, @@ -199,7 +203,7 @@ func (polishStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*St "adapter_work_dir": res.WorkDir, "adapter_stdout_log_path": res.StdoutLogPath, "adapter_stderr_log_path": res.StderrLogPath, - "credential_env_var": env.Config.Pipeline.Audita.LLMAPIKeyEnv, + "credential_env_var": credentialReference, "credential_present": false, } if res.Metadata != nil { diff --git a/internal/stage/polish_test.go b/internal/stage/polish_test.go index aa7cf69..32768f8 100644 --- a/internal/stage/polish_test.go +++ b/internal/stage/polish_test.go @@ -257,7 +257,7 @@ func setupPolishEnv(t *testing.T) (*Env, *manifest.Manifest) { Audita: config.AuditaConfig{ Binary: "audita", Timeout: "3h", - LLMAPIKeyEnv: "AUDITA_LLM_API_KEY", + LLMAPIKey: testSecretReference("AUDITA_LLM_API_KEY"), Modules: []string{"glossary", "homophones", "grammar"}, BaseURL: "https://openrouter.ai/api/v1", Model: "openrouter/google/gemma-4-31b-it", diff --git a/internal/stage/prepare.go b/internal/stage/prepare.go index ccd7258..f8c8ab6 100644 --- a/internal/stage/prepare.go +++ b/internal/stage/prepare.go @@ -18,7 +18,6 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/config" "gitea.maximumdirect.net/eric/narratio/internal/fileops" "gitea.maximumdirect.net/eric/narratio/internal/manifest" - "gopkg.in/yaml.v3" ) type prepareStage struct{} @@ -277,7 +276,7 @@ func (prepareStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S } func renderResolvedPipeline(cfg *config.PipelineConfig) ([]byte, error) { - return yaml.Marshal(cfg) + return config.MarshalEffectivePipeline(cfg) } func stableInputSource(resolved config.ResolvedInputFile, fallbackPath, fallbackConfigPath string) config.ResolvedInputFile { diff --git a/internal/stage/semantic_contracts_delivery_test.go b/internal/stage/semantic_contracts_delivery_test.go index 5195d7f..f45b466 100644 --- a/internal/stage/semantic_contracts_delivery_test.go +++ b/internal/stage/semantic_contracts_delivery_test.go @@ -4,10 +4,16 @@ import ( "context" "testing" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" "gitea.maximumdirect.net/eric/narratio/internal/config" ) +func testSecretReference(name string) *configkit.Secret { + reference := configkit.SecretEnv(name) + return &reference +} + func TestExtractSemanticConfigSensitivityAndOperationalExclusions(t *testing.T) { baseline := downstreamSemanticEnv() want, err := (extractStage{}).SemanticConfigFingerprint(baseline) @@ -121,8 +127,8 @@ func TestPublishSemanticConfigSensitivityAndOperationalExclusions(t *testing.T) } operational := downstreamSemanticEnv() - operational.Config.Pipeline.Storage.S3.AccessKeyIDEnv = "OTHER_ACCESS_KEY" - operational.Config.Pipeline.Storage.S3.SecretKeyEnv = "OTHER_SECRET_KEY" + operational.Config.Pipeline.Storage.S3.AccessKeyID = testSecretReference("OTHER_ACCESS_KEY") + operational.Config.Pipeline.Storage.S3.SecretAccessKey = testSecretReference("OTHER_SECRET_KEY") operational.Config.Pipeline.Storage.S3.ForcePathStyle = true operational.Config.Pipeline.Workspace.Root = "/relocated/workspace" got, err := (publishStage{}).SemanticConfigFingerprint(operational) @@ -199,7 +205,7 @@ func downstreamSemanticEnv() *Env { Workspace: config.WorkspaceConfig{Root: "/workspace"}, Storage: config.StorageConfig{Backend: config.StorageBackendS3, S3: &config.StorageS3Config{ Bucket: "campaign-bucket", RootPrefix: "narratio", Region: "us-east-1", - Endpoint: "https://objects.example", AccessKeyIDEnv: "ACCESS_KEY", SecretKeyEnv: "SECRET_KEY", + Endpoint: "https://objects.example", AccessKeyID: testSecretReference("ACCESS_KEY"), SecretAccessKey: testSecretReference("SECRET_KEY"), }}, Notarius: &config.NotariusConfig{ Enabled: true, Binary: "notarius", ConfigPath: "/etc/notarius/private.yml", diff --git a/internal/stage/semantic_contracts_refinement_test.go b/internal/stage/semantic_contracts_refinement_test.go index 5681d0a..ddeecc0 100644 --- a/internal/stage/semantic_contracts_refinement_test.go +++ b/internal/stage/semantic_contracts_refinement_test.go @@ -31,7 +31,7 @@ func TestPolishSemanticConfigSensitivity(t *testing.T) { env.Config.Pipeline.Audita.TotalLLMConcurrency = intPointer(20) env.Config.Pipeline.Audita.ProposalLLMConcurrency = intPointer(10) env.Config.Pipeline.Audita.ValidationLLMConcurrency = intPointer(10) - env.Config.Pipeline.Audita.LLMAPIKeyEnv = "OTHER_KEY" + env.Config.Pipeline.Audita.LLMAPIKey = testSecretReference("OTHER_KEY") env.Config.Pipeline.Audita.WorkDirRetention = "always" env.Config.Pipeline.Audita.Report = boolPointer(false) }}, @@ -119,7 +119,7 @@ func refinementSemanticEnv() *Env { env := initialSemanticEnv() env.Config.Session.Title = "Session title" env.Config.Pipeline.Audita = config.AuditaConfig{ - Binary: "audita", Timeout: "10m", LLMAPIKeyEnv: "AUDITA_KEY", + Binary: "audita", Timeout: "10m", LLMAPIKey: testSecretReference("AUDITA_KEY"), Modules: []string{"speakers", "terms"}, BaseURL: "https://audita.example", Model: "testing", ValidationModel: "validator", TranscriptDescription: "tabletop session", ConfigPath: "configs/audita.yml", OutputSchema: "audita.transcript.v1",