Forward Notarius credentials securely

2026-09-16 14:27:06 +00:00
parent 9d68cfcf2f
commit dcaa2ad9c6
6 changed files with 206 additions and 16 deletions
--- 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`,
--- 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
--- 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,
--- 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(), "<redacted>") {
+		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, "<redacted>") {
+		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")
--- 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,
 	}
--- 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)
 	}