diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 96033f6..e97e681 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -109,7 +109,24 @@ Acceptance: ## Stage 2 — Deliver the selected credential and register redaction -Status: Not started +Status: Complete — 2026-09-16 + +Completion: Extended the typed Notarius request boundary with the resolved +credential reference, passed it from extraction, and taught the subprocess +adapter to reject invalid programmatic references before launch. The adapter +now forwards exactly the configured name and loaded value through the minimal +child environment and registers the custom name for streaming redaction. +Focused coverage verifies nil and invalid references, unchanged arguments, +load-time value capture, exact-name delivery, unrelated-parent exclusion, and +stdout/stderr redaction for successful receipts and process failures without +putting credentials in stage metadata. + +Checks: + +- `GOWORK=off go test ./internal/adapters/notarius ./internal/adapters/subprocess ./internal/stage` — pass. +- `GOWORK=off go test -race ./internal/adapters/notarius ./internal/stage` — pass. +- `GOWORK=off go test ./...` — pass. +- `git diff --check` — pass. Read: `internal/adapters/notarius/runner.go`, `subprocess.go`, `fake.go`, `subprocess_test.go`; `internal/adapters/subprocess/run.go`, diff --git a/internal/adapters/notarius/runner.go b/internal/adapters/notarius/runner.go index 97ec73f..8090c33 100644 --- a/internal/adapters/notarius/runner.go +++ b/internal/adapters/notarius/runner.go @@ -4,6 +4,8 @@ package notarius import ( "context" "time" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" ) const ReceiptSchemaVersion = "notarius.run-result.v2" @@ -22,7 +24,9 @@ type ReferenceBinding struct { // RunRequest contains the resolved inputs and diagnostic destinations for one invocation. type RunRequest struct { - Binary string + Binary string + // LLMAPIKey is an optional already-resolved credential reference. + LLMAPIKey *configkit.Secret ConfigPath string PipelineID string InputPath string diff --git a/internal/adapters/notarius/subprocess.go b/internal/adapters/notarius/subprocess.go index 941a462..c449774 100644 --- a/internal/adapters/notarius/subprocess.go +++ b/internal/adapters/notarius/subprocess.go @@ -52,6 +52,20 @@ func (r *SubprocessRunner) Run(ctx context.Context, req RunRequest) (RunResult, if err != nil { return RunResult{}, err } + var envOverrides map[string]string + var sensitiveEnvNames []string + if req.LLMAPIKey != nil { + if !req.LLMAPIKey.IsResolved() { + return RunResult{}, fmt.Errorf("notarius LLM API credential must be resolved and nonempty") + } + name := req.LLMAPIKey.EnvName() + value := req.LLMAPIKey.Value() + if value == "" { + return RunResult{}, fmt.Errorf("notarius LLM API credential must be resolved and nonempty") + } + envOverrides = map[string]string{name: value} + sensitiveEnvNames = []string{name} + } args := []string{ "run", req.PipelineID, @@ -64,13 +78,15 @@ func (r *SubprocessRunner) Run(ctx context.Context, req RunRequest) (RunResult, } args = append(args, "--json") processResult, err := r.run(ctx, subprocess.RunRequest{ - Executable: req.Binary, - Args: args, - WorkingDir: req.WorkingDirectory, - Timeout: req.Timeout, - DiagnosticOwner: "notarius", - StdoutLogPath: req.ReceiptPath, - StderrLogPath: req.LogPath, + Executable: req.Binary, + Args: args, + WorkingDir: req.WorkingDirectory, + Timeout: req.Timeout, + DiagnosticOwner: "notarius", + StdoutLogPath: req.ReceiptPath, + StderrLogPath: req.LogPath, + EnvOverrides: envOverrides, + SensitiveEnvNames: sensitiveEnvNames, }) baseResult := RunResult{ ReceiptPath: req.ReceiptPath, diff --git a/internal/adapters/notarius/subprocess_test.go b/internal/adapters/notarius/subprocess_test.go index 354c428..e1639d2 100644 --- a/internal/adapters/notarius/subprocess_test.go +++ b/internal/adapters/notarius/subprocess_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + configkit "gitea.maximumdirect.net/eric/ConfigKit" sharedsubprocess "gitea.maximumdirect.net/eric/narratio/internal/adapters/subprocess" ) @@ -76,6 +77,65 @@ func TestSubprocessRunnerBuildsExactInvocationAndDiscoversBundle(t *testing.T) { } } +func TestSubprocessRunnerForwardsLoadedCredentialWithoutRereadingEnvironment(t *testing.T) { + const envName = "NARRATIO_NOTARIUS_CUSTOM_LLM_KEY" + const loadedValue = "loaded-notarius-credential" + credential := loadTestCredential(t, envName, loadedValue) + t.Setenv(envName, "changed-parent-credential") + + req := validRunRequest(t) + req.LLMAPIKey = credential + var captured sharedsubprocess.RunRequest + runner := &SubprocessRunner{run: func(_ context.Context, processReq sharedsubprocess.RunRequest) (sharedsubprocess.RunResult, error) { + captured = processReq + writeValidBundleAndReceipt(t, req, false) + return sharedsubprocess.RunResult{ExitCode: 0}, nil + }} + if _, err := runner.Run(context.Background(), req); err != nil { + t.Fatalf("Run() error = %v", err) + } + if len(captured.EnvOverrides) != 1 || captured.EnvOverrides[envName] != loadedValue { + t.Fatal("subprocess did not receive exactly the loaded credential override") + } + if !reflect.DeepEqual(captured.SensitiveEnvNames, []string{envName}) { + t.Fatalf("sensitive environment names = %#v, want configured reference name", captured.SensitiveEnvNames) + } + wantArgs := []string{ + "run", req.PipelineID, "--config", req.ConfigPath, "--input", req.InputPath, + "--output-dir", req.OutputRoot, "--json", + } + if !reflect.DeepEqual(captured.Args, wantArgs) { + t.Fatalf("subprocess args = %#v, want %#v", captured.Args, wantArgs) + } +} + +func TestSubprocessRunnerRejectsInvalidProgrammaticCredentialBeforeLaunch(t *testing.T) { + for _, test := range []struct { + name string + reference configkit.Secret + }{ + {name: "unresolved", reference: configkit.SecretEnv("NARRATIO_NOTARIUS_UNRESOLVED_KEY")}, + {name: "empty", reference: configkit.SecretEnv("")}, + } { + t.Run(test.name, func(t *testing.T) { + req := validRunRequest(t) + req.LLMAPIKey = &test.reference + started := false + runner := &SubprocessRunner{run: func(context.Context, sharedsubprocess.RunRequest) (sharedsubprocess.RunResult, error) { + started = true + return sharedsubprocess.RunResult{}, nil + }} + _, err := runner.Run(context.Background(), req) + if err == nil || !strings.Contains(err.Error(), "resolved and nonempty") { + t.Fatalf("Run() error = %v, want credential validation failure", err) + } + if started { + t.Fatal("subprocess started with an invalid credential") + } + }) + } +} + func TestSubprocessRunnerBuildsOrderedReferenceArguments(t *testing.T) { req := validRunRequest(t) referenceRoot := t.TempDir() @@ -172,16 +232,23 @@ func TestSubprocessRunnerRejectsInvalidReferencesBeforeLaunch(t *testing.T) { } func TestSubprocessRunnerUsesMinimalEnvironmentAndSeparatesStreams(t *testing.T) { + const envName = "NARRATIO_NOTARIUS_CUSTOM_CAPTURE_KEY" + const canary = "notarius-captured-stream-canary" + const unrelated = "NARRATIO_NOTARIUS_UNRELATED_PARENT_KEY" req := validRunRequest(t) + req.LLMAPIKey = loadTestCredential(t, envName, canary) writeValidBundleAndReceipt(t, req, false) receiptFixture := req.ReceiptPath + ".fixture" data, err := os.ReadFile(req.ReceiptPath) if err != nil { t.Fatalf("ReadFile(receipt) error = %v", err) } - if err := os.WriteFile(receiptFixture, data, 0o644); err != nil { - t.Fatalf("WriteFile(receipt fixture) error = %v", err) + var receipt map[string]any + if err := json.Unmarshal(data, &receipt); err != nil { + t.Fatalf("decode receipt fixture: %v", err) } + receipt["credential_echo"] = canary + writeJSONFile(t, receiptFixture, receipt) if err := os.Remove(req.ReceiptPath); err != nil { t.Fatalf("Remove(receipt) error = %v", err) } @@ -193,27 +260,55 @@ func TestSubprocessRunnerUsesMinimalEnvironmentAndSeparatesStreams(t *testing.T) script := writeShellScript(t, `#!/bin/sh pwd > "$NOTARIUS_CAPTURE_DIR/working-directory" printf '%s' "$NOTARIUS_INHERITED_VALUE" > "$NOTARIUS_CAPTURE_DIR/environment" -printf 'diagnostic stream\n' >&2 +if [ "$`+envName+`" != '`+canary+`' ]; then + printf 'selected credential mismatch\n' >&2 + exit 8 +fi +if [ -n "$`+unrelated+`" ]; then + printf 'unrelated credential inherited\n' >&2 + exit 9 +fi +printf '%s\n' "$`+envName+`" >&2 cat "$NOTARIUS_RECEIPT_FIXTURE" `) req.Binary = script t.Setenv("NOTARIUS_CAPTURE_DIR", captureDir) t.Setenv("NOTARIUS_INHERITED_VALUE", "inherited-value") t.Setenv("NOTARIUS_RECEIPT_FIXTURE", receiptFixture) + t.Setenv(unrelated, "unrelated-parent-credential") if _, err := NewSubprocessRunner().Run(context.Background(), req); err != nil { t.Fatalf("Run() error = %v", err) } assertTextFile(t, filepath.Join(captureDir, "working-directory"), req.WorkingDirectory+"\n") assertTextFile(t, filepath.Join(captureDir, "environment"), "") - assertTextFile(t, req.LogPath, "diagnostic stream\n") + assertRedactedCredentialFile(t, req.LogPath, canary) receiptBytes, err := os.ReadFile(req.ReceiptPath) if err != nil { t.Fatalf("ReadFile(receipt) error = %v", err) } - if strings.Contains(string(receiptBytes), "diagnostic stream") { - t.Fatal("receipt contains stderr output") + assertRedactedCredentialBytes(t, receiptBytes, canary) +} + +func TestSubprocessRunnerRedactsCredentialOnProcessFailure(t *testing.T) { + const envName = "NARRATIO_NOTARIUS_CUSTOM_FAILURE_KEY" + const canary = "notarius-failure-stream-canary" + req := validRunRequest(t) + req.LLMAPIKey = loadTestCredential(t, envName, canary) + req.Binary = writeShellScript(t, `#!/bin/sh +printf '%s\n' "$`+envName+`" +printf '%s\n' "$`+envName+`" >&2 +exit 7 +`) + _, err := NewSubprocessRunner().Run(context.Background(), req) + if err == nil { + t.Fatal("Run() error = nil, want process failure") } + if strings.Contains(err.Error(), canary) || !strings.Contains(err.Error(), "") { + t.Fatal("process failure did not return redacted diagnostics") + } + assertRedactedCredentialFile(t, req.ReceiptPath, canary) + assertRedactedCredentialFile(t, req.LogPath, canary) } func TestSubprocessRunnerReturnsProcessFailuresWithoutParsingStdout(t *testing.T) { @@ -579,6 +674,36 @@ func validRunRequest(t *testing.T) RunRequest { } } +func loadTestCredential(t *testing.T, envName, value string) *configkit.Secret { + t.Helper() + t.Setenv(envName, value) + type testConfig struct { + Credential *configkit.Secret `yaml:"credential"` + } + result, err := configkit.Load(configkit.Bytes("credential.yml", []byte("credential: "+envName+"\n")), configkit.Options[testConfig]{}) + if err != nil { + t.Fatalf("load test credential: %v", err) + } + return result.Config.Credential +} + +func assertRedactedCredentialFile(t *testing.T, path, canary string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile(%q) error = %v", path, err) + } + assertRedactedCredentialBytes(t, data, canary) +} + +func assertRedactedCredentialBytes(t *testing.T, data []byte, canary string) { + t.Helper() + text := string(data) + if strings.Contains(text, canary) || !strings.Contains(text, "") { + t.Fatal("captured stream did not contain the required redaction marker") + } +} + func writeValidBundleAndReceipt(t *testing.T, req RunRequest, includeUnknown bool) { t.Helper() bundle := filepath.Join(req.OutputRoot, "notarius-run-1") diff --git a/internal/stage/extract.go b/internal/stage/extract.go index 9275253..a88a088 100644 --- a/internal/stage/extract.go +++ b/internal/stage/extract.go @@ -147,7 +147,7 @@ func (extractStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (re return nil, fmt.Errorf("extract: build configuration fingerprint: %w", err) } request := notarius.RunRequest{ - Binary: resolvedBinary, ConfigPath: configPath, PipelineID: notariusConfig.PipelineID, + Binary: resolvedBinary, LLMAPIKey: notariusConfig.LLMAPIKey, ConfigPath: configPath, PipelineID: notariusConfig.PipelineID, InputPath: inputPath, OutputRoot: outputRoot, WorkingDirectory: workingDirectory, ReceiptPath: receiptPath, LogPath: logPath, Timeout: timeout, References: referenceBindings, } diff --git a/internal/stage/extract_test.go b/internal/stage/extract_test.go index 9181d50..012c56b 100644 --- a/internal/stage/extract_test.go +++ b/internal/stage/extract_test.go @@ -51,7 +51,25 @@ func TestExtractStageDisabledReturnsExplicitSkip(t *testing.T) { } func TestExtractStageResolvesManifestInputAndBuildsExactRequest(t *testing.T) { + const credentialName = "NARRATIO_EXTRACT_NOTARIUS_KEY" + const credentialCanary = "extract-notarius-credential-canary" + t.Setenv(credentialName, credentialCanary) + credentialPipelinePath := filepath.Join(t.TempDir(), "pipeline.yml") + if err := os.WriteFile(credentialPipelinePath, []byte(`whisperx: + transcribe_url: https://transcription.example.test/transcribe +notarius: + enabled: false + llm_api_key: NARRATIO_EXTRACT_NOTARIUS_KEY +`), 0o644); err != nil { + t.Fatal(err) + } + loadedPipeline, err := config.LoadPipeline(credentialPipelinePath) + if err != nil { + t.Fatalf("LoadPipeline() error = %v", err) + } + env, m, fake := setupExtractEnv(t) + env.Config.Pipeline.Notarius.LLMAPIKey = loadedPipeline.Notarius.LLMAPIKey result, err := (extractStage{}).Run(context.Background(), env, m) if err != nil { t.Fatalf("Run() error = %v", err) @@ -78,9 +96,19 @@ func TestExtractStageResolvesManifestInputAndBuildsExactRequest(t *testing.T) { if len(req.References) != 0 { t.Fatalf("references = %#v, want none", req.References) } + if req.LLMAPIKey != loadedPipeline.Notarius.LLMAPIKey || req.LLMAPIKey.EnvName() != credentialName || !req.LLMAPIKey.IsResolved() { + t.Fatal("request did not retain the loaded Notarius credential reference") + } if result.Metadata["reference_count"] != 0 || len(result.Metadata["references"].([]map[string]any)) != 0 { t.Fatalf("reference metadata = count %#v references %#v", result.Metadata["reference_count"], result.Metadata["references"]) } + metadata, err := json.Marshal(result.Metadata) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(metadata), credentialCanary) || strings.Contains(string(metadata), credentialName) { + t.Fatal("result metadata contains the Notarius credential") + } if len(result.Outputs) != 2 { t.Fatalf("outputs = %#v, want lane and index", result.Outputs) }