Report secrets bootstrap outcomes safely
This commit is contained in:
@@ -206,7 +206,15 @@ retain their relevant contributors. Verify catalog and sources agree after expan
|
|||||||
|
|
||||||
## 6. Surface successful secrets bootstrap reports, including skipped empty variables
|
## 6. Surface successful secrets bootstrap reports, including skipped empty variables
|
||||||
|
|
||||||
Status: Open — P3
|
Status: Resolved — P3
|
||||||
|
|
||||||
|
Resolution: Successful bootstrap reports now flow once through the command
|
||||||
|
diagnostics channel, while direct internal command calls default that channel to
|
||||||
|
discard. ConfigKit's value-free report makes skipped existing empty variables
|
||||||
|
explicit and remains available before later typed-load failures; structured output
|
||||||
|
stays isolated on stdout. Command-level coverage verifies successful reporting,
|
||||||
|
skipped-empty reporting, value redaction, clean config-show output, and one report
|
||||||
|
for a two-profile diff.
|
||||||
|
|
||||||
**Evidence:** `internal/app/secrets_bootstrap.go:36` discards the report whenever
|
**Evidence:** `internal/app/secrets_bootstrap.go:36` discards the report whenever
|
||||||
`LoadSecretsDir` returns no error. Reports are only rendered when directory
|
`LoadSecretsDir` returns no error. Reports are only rendered when directory
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package app
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"io"
|
||||||
|
|
||||||
configkit "gitea.maximumdirect.net/eric/ConfigKit"
|
configkit "gitea.maximumdirect.net/eric/ConfigKit"
|
||||||
"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
|
"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
|
||||||
@@ -9,6 +10,7 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
type commandDependencies struct {
|
type commandDependencies struct {
|
||||||
|
diagnostics io.Writer
|
||||||
loadPipelineConfig func(string, config.PipelineLoadOptions) (*config.PipelineConfig, error)
|
loadPipelineConfig func(string, config.PipelineLoadOptions) (*config.PipelineConfig, error)
|
||||||
downloadObjectToTemp func(context.Context, storage.ObjectStore, string, string) (string, error)
|
downloadObjectToTemp func(context.Context, storage.ObjectStore, string, string) (string, error)
|
||||||
loadSecretsDir func(string) (*configkit.SecretsReport, error)
|
loadSecretsDir func(string) (*configkit.SecretsReport, error)
|
||||||
@@ -20,6 +22,9 @@ type commandDependencies struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func normalizeCommandDependencies(deps commandDependencies) commandDependencies {
|
func normalizeCommandDependencies(deps commandDependencies) commandDependencies {
|
||||||
|
if deps.diagnostics == nil {
|
||||||
|
deps.diagnostics = io.Discard
|
||||||
|
}
|
||||||
if deps.loadPipelineConfig == nil {
|
if deps.loadPipelineConfig == nil {
|
||||||
deps.loadPipelineConfig = config.LoadPipelineWithOptions
|
deps.loadPipelineConfig = config.LoadPipelineWithOptions
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -19,6 +19,9 @@ func executeWithPipelineExecutor(args []string, stdout, stderr io.Writer, execut
|
|||||||
}
|
}
|
||||||
|
|
||||||
func executeWithDependencies(args []string, stdout, stderr io.Writer, deps commandDependencies, executor pipelineExecutor) int {
|
func executeWithDependencies(args []string, stdout, stderr io.Writer, deps commandDependencies, executor pipelineExecutor) int {
|
||||||
|
if deps.diagnostics == nil {
|
||||||
|
deps.diagnostics = stderr
|
||||||
|
}
|
||||||
deps = normalizeCommandDependencies(deps)
|
deps = normalizeCommandDependencies(deps)
|
||||||
if len(args) == 0 {
|
if len(args) == 0 {
|
||||||
printUsage(stderr)
|
printUsage(stderr)
|
||||||
|
|||||||
@@ -331,14 +331,21 @@ func TestConfigDiffBootstrapsSecretsOnceForBothProfiles(t *testing.T) {
|
|||||||
pipelinePath, _, _ := writeValidConfigFiles(t, workspaceRoot)
|
pipelinePath, _, _ := writeValidConfigFiles(t, workspaceRoot)
|
||||||
writeInspectionProfiles(t, pipelinePath, "whisperx:\n language: en\n", "whisperx:\n language: fr\n")
|
writeInspectionProfiles(t, pipelinePath, "whisperx:\n language: en\n", "whisperx:\n language: fr\n")
|
||||||
secretsDir := t.TempDir()
|
secretsDir := t.TempDir()
|
||||||
|
const secretName = "NARRATIO_DIFF_BOOTSTRAP_REPORT"
|
||||||
|
restoreEnvAfterTest(t, secretName)
|
||||||
|
if err := os.Unsetenv(secretName); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
mustWriteSecretFile(t, filepath.Join(secretsDir, secretName), "diff-secret-must-not-appear\n")
|
||||||
bootstrapCalls := 0
|
bootstrapCalls := 0
|
||||||
|
var diagnostics bytes.Buffer
|
||||||
deps := commandDependencies{loadSecretsDir: func(got string) (*configkit.SecretsReport, error) {
|
deps := commandDependencies{loadSecretsDir: func(got string) (*configkit.SecretsReport, error) {
|
||||||
bootstrapCalls++
|
bootstrapCalls++
|
||||||
if got != secretsDir {
|
if got != secretsDir {
|
||||||
t.Fatalf("secrets directory = %q, want %q", got, secretsDir)
|
t.Fatalf("secrets directory = %q, want %q", got, secretsDir)
|
||||||
}
|
}
|
||||||
return nil, nil
|
return configkit.LoadSecretsDir(got)
|
||||||
}}
|
}, diagnostics: &diagnostics}
|
||||||
|
|
||||||
if err := configDiffWithDependencies(context.Background(), []string{
|
if err := configDiffWithDependencies(context.Background(), []string{
|
||||||
"production", "testing", "--config", pipelinePath, "--secrets-dir", secretsDir,
|
"production", "testing", "--config", pipelinePath, "--secrets-dir", secretsDir,
|
||||||
@@ -348,6 +355,12 @@ func TestConfigDiffBootstrapsSecretsOnceForBothProfiles(t *testing.T) {
|
|||||||
if bootstrapCalls != 1 {
|
if bootstrapCalls != 1 {
|
||||||
t.Fatalf("bootstrap calls = %d, want 1", bootstrapCalls)
|
t.Fatalf("bootstrap calls = %d, want 1", bootstrapCalls)
|
||||||
}
|
}
|
||||||
|
if got := diagnostics.String(); strings.Count(got, `name="`+secretName+`"`) != 1 {
|
||||||
|
t.Fatalf("bootstrap report count = %d, want 1; diagnostics = %q", strings.Count(got, `name="`+secretName+`"`), got)
|
||||||
|
}
|
||||||
|
if strings.Contains(diagnostics.String(), "diff-secret-must-not-appear") {
|
||||||
|
t.Fatalf("bootstrap diagnostics exposed secret value: %q", diagnostics.String())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestConfigInspectionOutputsNeverExposeResolvedCredentialValues(t *testing.T) {
|
func TestConfigInspectionOutputsNeverExposeResolvedCredentialValues(t *testing.T) {
|
||||||
|
|||||||
@@ -171,7 +171,7 @@ func loadPipelineConfigWithDependencies(pipelineFlag string, opts config.Pipelin
|
|||||||
deps.secretsBootstrap.selection = secretsDir
|
deps.secretsBootstrap.selection = secretsDir
|
||||||
deps.secretsBootstrap.selectionSet = true
|
deps.secretsBootstrap.selectionSet = true
|
||||||
}
|
}
|
||||||
if err := deps.secretsBootstrap.load(resolvedPipelinePath, deps.loadSecretsDir); err != nil {
|
if err := deps.secretsBootstrap.load(resolvedPipelinePath, deps.loadSecretsDir, deps.diagnostics); err != nil {
|
||||||
return "", nil, err
|
return "", nil, err
|
||||||
}
|
}
|
||||||
pipelineCfg, err := deps.loadPipelineConfig(resolvedPipelinePath, opts)
|
pipelineCfg, err := deps.loadPipelineConfig(resolvedPipelinePath, opts)
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -17,7 +18,7 @@ type secretsBootstrap struct {
|
|||||||
called bool
|
called bool
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *secretsBootstrap) load(pipelinePath string, loader func(string) (*configkit.SecretsReport, error)) error {
|
func (b *secretsBootstrap) load(pipelinePath string, loader func(string) (*configkit.SecretsReport, error), diagnostics io.Writer) error {
|
||||||
if b == nil || b.called {
|
if b == nil || b.called {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -35,6 +36,11 @@ func (b *secretsBootstrap) load(pipelinePath string, loader func(string) (*confi
|
|||||||
}
|
}
|
||||||
report, loadErr := loader(directory)
|
report, loadErr := loader(directory)
|
||||||
if loadErr == nil {
|
if loadErr == nil {
|
||||||
|
if report != nil {
|
||||||
|
if _, err := report.WriteTo(diagnostics); err != nil {
|
||||||
|
return fmt.Errorf("write secrets bootstrap report: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
var rendered bytes.Buffer
|
var rendered bytes.Buffer
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
package app
|
package app
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -99,9 +101,13 @@ func TestSecretsBootstrapPreservesExistingEnvironmentValues(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
bootstrap := &secretsBootstrap{selection: &directory, selectionSet: true}
|
bootstrap := &secretsBootstrap{selection: &directory, selectionSet: true}
|
||||||
if err := bootstrap.load(filepath.Join(t.TempDir(), "pipeline.yml"), configkit.LoadSecretsDir); err != nil {
|
var diagnostics bytes.Buffer
|
||||||
|
if err := bootstrap.load(filepath.Join(t.TempDir(), "pipeline.yml"), configkit.LoadSecretsDir, &diagnostics); err != nil {
|
||||||
t.Fatalf("bootstrap: %v", err)
|
t.Fatalf("bootstrap: %v", err)
|
||||||
}
|
}
|
||||||
|
if got := diagnostics.String(); !strings.Contains(got, `status="skipped_existing"`) || !strings.Contains(got, `existing_empty=true`) {
|
||||||
|
t.Fatalf("diagnostics lack explicit skipped-empty result: %q", got)
|
||||||
|
}
|
||||||
if got := os.Getenv(emptyName); got != "" {
|
if got := os.Getenv(emptyName); got != "" {
|
||||||
t.Fatalf("existing empty value = %q, want empty", got)
|
t.Fatalf("existing empty value = %q, want empty", got)
|
||||||
}
|
}
|
||||||
@@ -117,7 +123,7 @@ func TestSecretsBootstrapPreservesSafePartialReportOnFailure(t *testing.T) {
|
|||||||
directory := t.TempDir()
|
directory := t.TempDir()
|
||||||
mustWriteSecretFile(t, filepath.Join(directory, "INVALID-NAME"), "must-not-appear\n")
|
mustWriteSecretFile(t, filepath.Join(directory, "INVALID-NAME"), "must-not-appear\n")
|
||||||
bootstrap := &secretsBootstrap{selection: &directory, selectionSet: true}
|
bootstrap := &secretsBootstrap{selection: &directory, selectionSet: true}
|
||||||
err := bootstrap.load(filepath.Join(t.TempDir(), "pipeline.yml"), configkit.LoadSecretsDir)
|
err := bootstrap.load(filepath.Join(t.TempDir(), "pipeline.yml"), configkit.LoadSecretsDir, io.Discard)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
t.Fatal("bootstrap succeeded, want error")
|
t.Fatal("bootstrap succeeded, want error")
|
||||||
}
|
}
|
||||||
@@ -129,4 +135,66 @@ func TestSecretsBootstrapPreservesSafePartialReportOnFailure(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestExecuteConfigShowReportsSuccessfulSecretsBootstrapOnStderr(t *testing.T) {
|
||||||
|
workspaceRoot := t.TempDir()
|
||||||
|
pipelinePath, _, _ := writeValidConfigFiles(t, workspaceRoot)
|
||||||
|
secretsDir := t.TempDir()
|
||||||
|
const name = "NARRATIO_BOOTSTRAP_REPORT_SECRET"
|
||||||
|
const canary = "synthetic-secret-must-not-appear"
|
||||||
|
restoreEnvAfterTest(t, name)
|
||||||
|
if err := os.Unsetenv(name); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
mustWriteSecretFile(t, filepath.Join(secretsDir, name), canary+"\n")
|
||||||
|
|
||||||
|
var stdout bytes.Buffer
|
||||||
|
var stderr bytes.Buffer
|
||||||
|
if code := Execute([]string{"config", "show", "--config", pipelinePath, "--secrets-dir", secretsDir}, &stdout, &stderr); code != 0 {
|
||||||
|
t.Fatalf("config show exit = %d, stderr = %q", code, stderr.String())
|
||||||
|
}
|
||||||
|
if got := stderr.String(); !strings.Contains(got, `status="loaded"`) || !strings.Contains(got, `name="`+name+`"`) {
|
||||||
|
t.Fatalf("stderr lacks safe bootstrap report: %q", got)
|
||||||
|
}
|
||||||
|
if strings.Contains(stderr.String(), canary) || strings.Contains(stdout.String(), canary) {
|
||||||
|
t.Fatal("command output exposed secret value")
|
||||||
|
}
|
||||||
|
if strings.Contains(stdout.String(), `status="loaded"`) || strings.Contains(stdout.String(), name) {
|
||||||
|
t.Fatalf("show stdout was polluted by bootstrap report: %q", stdout.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExecuteConfigValidateReportsSkippedEmptyBeforeTypedLoadFailure(t *testing.T) {
|
||||||
|
workspaceRoot := t.TempDir()
|
||||||
|
pipelinePath, _, _ := writeValidConfigFiles(t, workspaceRoot)
|
||||||
|
const name = "NARRATIO_REVIEW_KEY"
|
||||||
|
const canary = "synthetic-skipped-secret-must-not-appear"
|
||||||
|
t.Setenv(name, "")
|
||||||
|
pipelineYAML, err := os.ReadFile(pipelinePath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
pipelineYAML = []byte(strings.Replace(string(pipelineYAML), "audita:\n binary:", "audita:\n llm_api_key: "+name+"\n binary:", 1))
|
||||||
|
if err := os.WriteFile(pipelinePath, pipelineYAML, 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
secretsDir := t.TempDir()
|
||||||
|
mustWriteSecretFile(t, filepath.Join(secretsDir, name), canary+"\n")
|
||||||
|
|
||||||
|
var stdout bytes.Buffer
|
||||||
|
var stderr bytes.Buffer
|
||||||
|
if code := Execute([]string{"config", "validate", "--config", pipelinePath, "--secrets-dir", secretsDir}, &stdout, &stderr); code == 0 {
|
||||||
|
t.Fatal("config validate succeeded, want empty-secret failure")
|
||||||
|
}
|
||||||
|
got := stderr.String()
|
||||||
|
if !strings.Contains(got, `status="skipped_existing"`) || !strings.Contains(got, `existing_empty=true`) {
|
||||||
|
t.Fatalf("stderr lacks useful skipped-empty diagnostic: %q", got)
|
||||||
|
}
|
||||||
|
if !strings.Contains(got, "secret.empty") {
|
||||||
|
t.Fatalf("stderr lacks typed load failure: %q", got)
|
||||||
|
}
|
||||||
|
if strings.Contains(got, canary) || strings.Contains(stdout.String(), canary) {
|
||||||
|
t.Fatal("command output exposed skipped secret value")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func stringPointer(value string) *string { return &value }
|
func stringPointer(value string) *string { return &value }
|
||||||
|
|||||||
Reference in New Issue
Block a user