Harden configuration validation
This commit is contained in:
@@ -47,6 +47,8 @@ Rules:
|
||||
- `--campaign` and `--campaign-file` are mutually exclusive.
|
||||
- `--session` is not used by `session init`.
|
||||
- if both positional `<session_id>` and `--session-id` are provided, values must match.
|
||||
- `--previous-session-id` is a strict expectation: the selected session file
|
||||
must contain the same `previous_session_id`.
|
||||
- `clean --all` cannot be combined with campaign/session selectors.
|
||||
|
||||
## Session ID Input Rules
|
||||
|
||||
@@ -49,7 +49,10 @@ remote state with an unsafe legacy identity must be migrated before use.
|
||||
|
||||
## Validation and Merge Rules
|
||||
|
||||
- YAML decode is strict (`KnownFields(true)`): unknown fields fail load.
|
||||
- YAML decode is strict (`KnownFields(true)`) and accepts exactly one document:
|
||||
unknown fields or trailing documents fail load.
|
||||
- Configured timeout and retry-delay durations must be positive. An omitted
|
||||
artifact timeout continues to inherit its configured Scriptorium timeout.
|
||||
- Session files must be concrete; unresolved `{{ ... }}` placeholders fail load.
|
||||
- Pipeline defaults are applied before validation.
|
||||
- Campaign and session identities must agree.
|
||||
@@ -150,8 +153,8 @@ Rules:
|
||||
| `pipeline.campaigns.root` | string | No | `/usr/local/share/narratio/campaigns` |
|
||||
| `pipeline.campaigns.default_campaign_id` | string | No | empty |
|
||||
| `pipeline.secrets.env_dir` | string | No | empty |
|
||||
| `pipeline.storage.backend` | string | No | empty |
|
||||
| `pipeline.storage.s3.bucket` | string | Conditional | required for S3 session-audio and for publish upload when backend is `s3` |
|
||||
| `pipeline.storage.backend` | string | No | `local`; supported values are `local` and `s3` (case-insensitive) |
|
||||
| `pipeline.storage.s3.bucket` | string | Conditional | required when backend is `s3` and S3 session-audio or publish upload is enabled |
|
||||
| `pipeline.storage.s3.root_prefix` | string | No | `dnd` |
|
||||
| `pipeline.storage.s3.region` | string | No | empty |
|
||||
| `pipeline.storage.s3.endpoint` | string | No | empty |
|
||||
@@ -316,6 +319,20 @@ For each artifact input `pipeline.scriptorium.artifacts.<name>.inputs.<input_nam
|
||||
Audio rules:
|
||||
|
||||
- configure local mode (`audio_dir` or `audio_files`) or S3 mode (`audio_s3.prefix`), not both.
|
||||
- `audio_s3` requires `pipeline.storage.backend: s3` and a configured S3 bucket.
|
||||
|
||||
### Storage backend selection
|
||||
|
||||
`local` is the default and disables remote object-store operations. Configure
|
||||
`s3` explicitly before supplying `storage.s3`; a populated S3 block does not
|
||||
select a backend on its own. Unknown backend names and an S3 block paired with
|
||||
`local` are rejected during configuration validation.
|
||||
|
||||
### Previous-session expectation
|
||||
|
||||
`previous_session_id` is optional in a session file. When a command supplies
|
||||
`--previous-session-id`, however, the session file must contain the same value;
|
||||
an omitted or different value is rejected before the command performs work.
|
||||
|
||||
## Maintained Examples
|
||||
|
||||
|
||||
@@ -745,6 +745,8 @@ effects and give the configuration package focused, non-duplicative tests.
|
||||
durations, backend combinations, strict previous expectations, environment
|
||||
isolation, defaults, unknown fields, and stable actionable errors.
|
||||
|
||||
**Status:** Completed.
|
||||
|
||||
## Stage 22 — Make product configuration truthful and own remote temp files
|
||||
|
||||
**Read first:** `audit-findings.md` lines 2521–2552 (COR-024), 3157–3197
|
||||
|
||||
@@ -12,7 +12,7 @@ workspace:
|
||||
# env_dir: ./secrets
|
||||
|
||||
storage:
|
||||
# Optional storage backend selector; use "s3" for publish + S3 audio workflows.
|
||||
# Defaults to "local". Use "s3" explicitly for publish + S3 audio workflows.
|
||||
backend: s3
|
||||
s3:
|
||||
# Required when using S3 audio or S3 publish uploads.
|
||||
|
||||
@@ -14,16 +14,15 @@ func NewObjectStoreFromConfig(ctx context.Context, cfg *config.Config) (ObjectSt
|
||||
return nil, fmt.Errorf("pipeline config is required")
|
||||
}
|
||||
|
||||
if strings.EqualFold(strings.TrimSpace(cfg.Pipeline.Storage.Backend), "s3") {
|
||||
switch strings.ToLower(strings.TrimSpace(cfg.Pipeline.Storage.Backend)) {
|
||||
case config.StorageBackendS3:
|
||||
if cfg.Pipeline.Storage.S3 == nil {
|
||||
return nil, fmt.Errorf("pipeline.storage.s3 is required when pipeline.storage.backend is s3")
|
||||
}
|
||||
return NewS3BackendFromConfig(ctx, *cfg.Pipeline.Storage.S3)
|
||||
case "", config.StorageBackendLocal:
|
||||
return nil, fmt.Errorf("no remote object store backend is configured")
|
||||
default:
|
||||
return nil, fmt.Errorf("unsupported pipeline.storage.backend %q", cfg.Pipeline.Storage.Backend)
|
||||
}
|
||||
|
||||
if cfg.Pipeline.Storage.S3 != nil && strings.TrimSpace(cfg.Pipeline.Storage.S3.Bucket) != "" {
|
||||
return NewS3BackendFromConfig(ctx, *cfg.Pipeline.Storage.S3)
|
||||
}
|
||||
|
||||
return nil, fmt.Errorf("no remote object store backend is configured")
|
||||
}
|
||||
|
||||
@@ -54,3 +54,35 @@ func TestNewObjectStoreFromConfigNoRemoteBackendConfigured(t *testing.T) {
|
||||
t.Fatalf("NewObjectStoreFromConfig() error = %v, want no-backend error", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewObjectStoreFromConfigDoesNotInferS3FromProviderFields(t *testing.T) {
|
||||
called := false
|
||||
original := newS3Client
|
||||
t.Cleanup(func() { newS3Client = original })
|
||||
newS3Client = func(_ context.Context, _ s3ClientOptions) (s3API, error) {
|
||||
called = true
|
||||
return &fakeS3API{}, nil
|
||||
}
|
||||
|
||||
_, err := NewObjectStoreFromConfig(context.Background(), &config.Config{
|
||||
Pipeline: &config.PipelineConfig{Storage: config.StorageConfig{
|
||||
Backend: config.StorageBackendLocal,
|
||||
S3: &config.StorageS3Config{Bucket: "my-archive"},
|
||||
}},
|
||||
})
|
||||
if err == nil || !strings.Contains(err.Error(), "no remote object store backend is configured") {
|
||||
t.Fatalf("NewObjectStoreFromConfig() error = %v, want no-backend error", err)
|
||||
}
|
||||
if called {
|
||||
t.Fatal("NewObjectStoreFromConfig() constructed S3 from incidental provider fields")
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewObjectStoreFromConfigRejectsUnknownBackend(t *testing.T) {
|
||||
_, err := NewObjectStoreFromConfig(context.Background(), &config.Config{
|
||||
Pipeline: &config.PipelineConfig{Storage: config.StorageConfig{Backend: "s33"}},
|
||||
})
|
||||
if err == nil || !strings.Contains(err.Error(), "unsupported pipeline.storage.backend") {
|
||||
t.Fatalf("NewObjectStoreFromConfig() error = %v, want unsupported-backend error", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -41,7 +41,11 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, campaign
|
||||
return nil, missingSessionConfigError(discoveredSession.Searched, "remote session loading requires a session_id")
|
||||
}
|
||||
|
||||
sessionPrefix := artifacts.S3SessionPrefix(base.Pipeline.Storage.S3.RootPrefix, config.CampaignID(base.Campaign), sessionID)
|
||||
rootPrefix := ""
|
||||
if base.Pipeline.Storage.S3 != nil {
|
||||
rootPrefix = base.Pipeline.Storage.S3.RootPrefix
|
||||
}
|
||||
sessionPrefix := artifacts.S3SessionPrefix(rootPrefix, config.CampaignID(base.Campaign), sessionID)
|
||||
remoteKey := artifacts.S3SessionConfigKey(sessionPrefix)
|
||||
partialCfg := &config.Config{
|
||||
Pipeline: base.Pipeline,
|
||||
|
||||
@@ -142,24 +142,3 @@ func commandObjectStoreTestConfig(secretsDir string) *config.Config {
|
||||
}
|
||||
return cfg
|
||||
}
|
||||
|
||||
func restoreEnvAfterTest(t *testing.T, names ...string) {
|
||||
t.Helper()
|
||||
originals := make(map[string]string, len(names))
|
||||
present := make(map[string]bool, len(names))
|
||||
for _, name := range names {
|
||||
value, ok := os.LookupEnv(name)
|
||||
originals[name] = value
|
||||
present[name] = ok
|
||||
_ = os.Unsetenv(name)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
for _, name := range names {
|
||||
if present[name] {
|
||||
_ = os.Setenv(name, originals[name])
|
||||
} else {
|
||||
_ = os.Unsetenv(name)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
52
internal/app/operator_session_init_test.go
Normal file
52
internal/app/operator_session_init_test.go
Normal file
@@ -0,0 +1,52 @@
|
||||
package app
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestSessionInitRejectsRenderedTemplateThatOmitsExpectedPreviousSession(t *testing.T) {
|
||||
workspaceRoot := t.TempDir()
|
||||
pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot)
|
||||
campaignDir := filepath.Dir(campaignPath)
|
||||
templatePath := filepath.Join(campaignDir, "session.template.yml")
|
||||
template := `session_id: {{ session_id }}
|
||||
campaign: sample-campaign
|
||||
inputs:
|
||||
audio_dir: ./audio
|
||||
`
|
||||
if err := os.WriteFile(templatePath, []byte(template), 0o644); err != nil {
|
||||
t.Fatalf("write session template: %v", err)
|
||||
}
|
||||
campaign := `campaign_id: sample-campaign
|
||||
session_template_file: session.template.yml
|
||||
inputs:
|
||||
speakers_file: ./speakers.yml
|
||||
autocorrect_file: ./autocorrect.yml
|
||||
glossary_file: ./glossary.yml
|
||||
players_file: ./players.yml
|
||||
party_file: ./party.yml
|
||||
`
|
||||
if err := os.WriteFile(campaignPath, []byte(campaign), 0o644); err != nil {
|
||||
t.Fatalf("write campaign config: %v", err)
|
||||
}
|
||||
|
||||
var out bytes.Buffer
|
||||
err := SessionInit(context.Background(), []string{
|
||||
"2026-05-03",
|
||||
"--config", pipelinePath,
|
||||
"--campaign-file", campaignPath,
|
||||
"--output", filepath.Join(t.TempDir(), "session.yml"),
|
||||
"--previous-session-id", "2026-04-26",
|
||||
}, &out)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "unused template variable value(s): previous_session_id") {
|
||||
t.Fatalf("SessionInit() error = %q, want missing previous-session template error", err.Error())
|
||||
}
|
||||
}
|
||||
@@ -12,8 +12,7 @@ import (
|
||||
|
||||
func TestLoadSecretsFromConfigLoadsValidFiles(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
unsetSecretEnvironment(t, "NARRATIO_TEST_SECRET_A")
|
||||
unsetSecretEnvironment(t, "NARRATIO_TEST_SECRET_B")
|
||||
restoreEnvAfterTest(t, "NARRATIO_TEST_SECRET_A", "NARRATIO_TEST_SECRET_B")
|
||||
mustWriteSecretFile(t, filepath.Join(dir, "NARRATIO_TEST_SECRET_A"), "value-1\n")
|
||||
mustWriteSecretFile(t, filepath.Join(dir, "NARRATIO_TEST_SECRET_B"), "value-2\r\n")
|
||||
mustWriteSecretFile(t, filepath.Join(dir, "not-valid-name.txt"), "ignored")
|
||||
@@ -162,7 +161,7 @@ func TestLoadSecretsFromConfigRejectsUnsafeModes(t *testing.T) {
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
unsetSecretEnvironment(t, "OPENROUTER_API_KEY")
|
||||
restoreEnvAfterTest(t, "OPENROUTER_API_KEY")
|
||||
path := filepath.Join(dir, "OPENROUTER_API_KEY")
|
||||
mustWriteSecretFile(t, path, tc.wantSecret)
|
||||
if err := os.Chmod(dir, tc.directory); err != nil {
|
||||
@@ -186,7 +185,7 @@ func TestLoadSecretsFromConfigRejectsUnsafeModes(t *testing.T) {
|
||||
func TestLoadSecretsFromConfigRejectsNonRegularAndOversizedEntries(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
ensureSecretDirectory(t, dir)
|
||||
unsetSecretEnvironment(t, "OPENROUTER_API_KEY")
|
||||
restoreEnvAfterTest(t, "OPENROUTER_API_KEY")
|
||||
if err := os.Mkdir(filepath.Join(dir, "OPENROUTER_API_KEY"), secretDirectoryPrivateMode); err != nil {
|
||||
t.Fatalf("Mkdir(non-regular entry) error = %v", err)
|
||||
}
|
||||
@@ -215,7 +214,7 @@ func TestLoadSecretsFromConfigRejectsSymlinkAndAncestorReplacement(t *testing.T)
|
||||
dir := t.TempDir()
|
||||
outside := t.TempDir()
|
||||
ensureSecretDirectory(t, dir)
|
||||
unsetSecretEnvironment(t, "OPENROUTER_API_KEY")
|
||||
restoreEnvAfterTest(t, "OPENROUTER_API_KEY")
|
||||
outsideValue := "outside-secret-value"
|
||||
mustWriteSecretFile(t, filepath.Join(outside, "OPENROUTER_API_KEY"), outsideValue)
|
||||
if err := os.Symlink(filepath.Join(outside, "OPENROUTER_API_KEY"), filepath.Join(dir, "OPENROUTER_API_KEY")); err != nil {
|
||||
@@ -277,21 +276,6 @@ func secretConfig(dir string) *config.Config {
|
||||
return &config.Config{Pipeline: &config.PipelineConfig{Secrets: &config.SecretsConfig{EnvDir: dir}}}
|
||||
}
|
||||
|
||||
func unsetSecretEnvironment(t *testing.T, name string) {
|
||||
t.Helper()
|
||||
previous, existed := os.LookupEnv(name)
|
||||
if err := os.Unsetenv(name); err != nil {
|
||||
t.Fatalf("Unsetenv(%q): %v", name, err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
if existed {
|
||||
_ = os.Setenv(name, previous)
|
||||
return
|
||||
}
|
||||
_ = os.Unsetenv(name)
|
||||
})
|
||||
}
|
||||
|
||||
func ensureSecretDirectory(t *testing.T, directory string) {
|
||||
t.Helper()
|
||||
if err := os.Chmod(directory, secretDirectoryPrivateMode); err != nil {
|
||||
|
||||
@@ -100,6 +100,26 @@ inputs:
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlanRequiresExpectedPreviousSessionID(t *testing.T) {
|
||||
workspaceRoot := t.TempDir()
|
||||
pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)
|
||||
|
||||
var out bytes.Buffer
|
||||
err := Plan(context.Background(), []string{
|
||||
"2026-05-03",
|
||||
"--config", pipelinePath,
|
||||
"--campaign-file", campaignPath,
|
||||
"--session", sessionPath,
|
||||
"--previous-session-id", "2026-04-26",
|
||||
}, &out)
|
||||
if err == nil {
|
||||
t.Fatal("expected error, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "previous_session_id is required") {
|
||||
t.Fatalf("error = %q, want missing previous-session expectation", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunStageAcceptsPositionalSessionIDAndParsesStageName(t *testing.T) {
|
||||
workspaceRoot := t.TempDir()
|
||||
pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)
|
||||
|
||||
31
internal/app/test_environment_test.go
Normal file
31
internal/app/test_environment_test.go
Normal file
@@ -0,0 +1,31 @@
|
||||
package app
|
||||
|
||||
import (
|
||||
"os"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// restoreEnvAfterTest clears names for a test and restores both their prior
|
||||
// values and their set/unset state during cleanup.
|
||||
func restoreEnvAfterTest(t *testing.T, names ...string) {
|
||||
t.Helper()
|
||||
originals := make(map[string]string, len(names))
|
||||
present := make(map[string]bool, len(names))
|
||||
for _, name := range names {
|
||||
value, ok := os.LookupEnv(name)
|
||||
originals[name] = value
|
||||
present[name] = ok
|
||||
if err := os.Unsetenv(name); err != nil {
|
||||
t.Fatalf("Unsetenv(%q): %v", name, err)
|
||||
}
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
for _, name := range names {
|
||||
if present[name] {
|
||||
_ = os.Setenv(name, originals[name])
|
||||
} else {
|
||||
_ = os.Unsetenv(name)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -83,6 +83,13 @@ type StorageConfig struct {
|
||||
S3 *StorageS3Config `yaml:"s3"`
|
||||
}
|
||||
|
||||
const (
|
||||
// StorageBackendLocal disables remote object-store operations.
|
||||
StorageBackendLocal = "local"
|
||||
// StorageBackendS3 enables the configured S3 object store.
|
||||
StorageBackendS3 = "s3"
|
||||
)
|
||||
|
||||
// StorageS3Config configures S3 storage coordinates.
|
||||
type StorageS3Config struct {
|
||||
Bucket string `yaml:"bucket"`
|
||||
|
||||
@@ -74,15 +74,23 @@ func LoadSessionBytesWithOptions(label string, data []byte, opts SessionLoadOpti
|
||||
strings.TrimSpace(cfg.SessionID),
|
||||
)
|
||||
}
|
||||
if strings.TrimSpace(opts.PreviousSessionID) != "" &&
|
||||
strings.TrimSpace(cfg.PreviousSessionID) != "" &&
|
||||
strings.TrimSpace(cfg.PreviousSessionID) != strings.TrimSpace(opts.PreviousSessionID) {
|
||||
return nil, fmt.Errorf(
|
||||
"load session config: session file %q: previous_session_id mismatch: --previous-session-id %q does not match previous_session_id %q",
|
||||
label,
|
||||
strings.TrimSpace(opts.PreviousSessionID),
|
||||
strings.TrimSpace(cfg.PreviousSessionID),
|
||||
)
|
||||
if expectedPreviousSessionID := strings.TrimSpace(opts.PreviousSessionID); expectedPreviousSessionID != "" {
|
||||
actualPreviousSessionID := strings.TrimSpace(cfg.PreviousSessionID)
|
||||
if actualPreviousSessionID == "" {
|
||||
return nil, fmt.Errorf(
|
||||
"load session config: session file %q: previous_session_id is required when --previous-session-id %q is provided",
|
||||
label,
|
||||
expectedPreviousSessionID,
|
||||
)
|
||||
}
|
||||
if actualPreviousSessionID != expectedPreviousSessionID {
|
||||
return nil, fmt.Errorf(
|
||||
"load session config: session file %q: previous_session_id mismatch: --previous-session-id %q does not match previous_session_id %q",
|
||||
label,
|
||||
expectedPreviousSessionID,
|
||||
actualPreviousSessionID,
|
||||
)
|
||||
}
|
||||
}
|
||||
return &cfg, nil
|
||||
}
|
||||
@@ -296,8 +304,10 @@ func decodeStrictYAMLFromReader(kind, path string, r io.Reader, out any) error {
|
||||
return fmt.Errorf("%s file %q: strict decode failed: %w", kind, path, err)
|
||||
}
|
||||
|
||||
var extra any
|
||||
if err := dec.Decode(&extra); err != nil && err != io.EOF {
|
||||
var extra yaml.Node
|
||||
if err := dec.Decode(&extra); err == nil {
|
||||
return fmt.Errorf("%s file %q: must contain exactly one YAML document", kind, path)
|
||||
} else if err != io.EOF {
|
||||
return fmt.Errorf("%s file %q: trailing content decode failed: %w", kind, path, err)
|
||||
}
|
||||
|
||||
@@ -392,8 +402,13 @@ func applyStorageDefaults(cfg *StorageConfig) {
|
||||
if cfg == nil {
|
||||
return
|
||||
}
|
||||
backend := strings.ToLower(strings.TrimSpace(cfg.Backend))
|
||||
if backend == "" {
|
||||
backend = StorageBackendLocal
|
||||
}
|
||||
cfg.Backend = backend
|
||||
if cfg.S3 == nil {
|
||||
cfg.S3 = &StorageS3Config{}
|
||||
return
|
||||
}
|
||||
if cfg.S3.RootPrefix == "" {
|
||||
cfg.S3.RootPrefix = DefaultStorageS3RootPrefix
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
91
internal/config/loader_strict_test.go
Normal file
91
internal/config/loader_strict_test.go
Normal file
@@ -0,0 +1,91 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestLoadersRejectTrailingYAMLDocuments(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
pipelinePath := filepath.Join(dir, "pipeline.yml")
|
||||
campaignPath := filepath.Join(dir, "campaign.yml")
|
||||
if err := os.WriteFile(pipelinePath, []byte("workspace:\n root: /tmp/narratio\n---\nworkspace:\n root: /other\n"), 0o644); err != nil {
|
||||
t.Fatalf("write pipeline.yml: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(campaignPath, []byte("campaign_id: sample-campaign\n---\nnull\n"), 0o644); err != nil {
|
||||
t.Fatalf("write campaign.yml: %v", err)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
load func() error
|
||||
}{
|
||||
{
|
||||
name: "pipeline",
|
||||
load: func() error {
|
||||
_, err := LoadPipeline(pipelinePath)
|
||||
return err
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "campaign",
|
||||
load: func() error {
|
||||
_, err := LoadCampaign(campaignPath)
|
||||
return err
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "remote session bytes",
|
||||
load: func() error {
|
||||
_, err := LoadSessionBytesWithOptions("s3://bucket/session.yml", []byte("session_id: 2026-05-03\n---\n# another document\nnull\n"), SessionLoadOptions{})
|
||||
return err
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "publish lock store",
|
||||
load: func() error {
|
||||
_, err := LoadPublishLockStoreBytes("s3://bucket/locks.yml", []byte("locks: []\n---\n{}\n"), nil, nil)
|
||||
return err
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := tt.load()
|
||||
if err == nil || !strings.Contains(err.Error(), "exactly one YAML document") {
|
||||
t.Fatalf("load error = %v, want exactly-one-document error", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStrictYAMLRejectsParserExposedEmptyTrailingDocument(t *testing.T) {
|
||||
var target struct {
|
||||
Name string `yaml:"name"`
|
||||
}
|
||||
err := decodeStrictYAMLFromReader("test", "memory", strings.NewReader("name: one\n---\n"), &target)
|
||||
if err == nil {
|
||||
t.Fatal("decodeStrictYAMLFromReader() error = nil, want trailing-document error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "exactly one YAML document") {
|
||||
t.Fatalf("decodeStrictYAMLFromReader() error = %v, want exactly-one-document error", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStrictYAMLAcceptsSingleDocumentAndRejectsUnknownFields(t *testing.T) {
|
||||
var target struct {
|
||||
Name string `yaml:"name"`
|
||||
}
|
||||
if err := decodeStrictYAMLFromReader("test", "memory", strings.NewReader("name: one\n"), &target); err != nil {
|
||||
t.Fatalf("decodeStrictYAMLFromReader(single document) error = %v", err)
|
||||
}
|
||||
if target.Name != "one" {
|
||||
t.Fatalf("Name = %q, want one", target.Name)
|
||||
}
|
||||
if err := decodeStrictYAMLFromReader("test", "memory", strings.NewReader("unknown: one\n"), &target); err == nil || !strings.Contains(err.Error(), "strict decode failed") {
|
||||
t.Fatalf("decodeStrictYAMLFromReader(unknown field) error = %v, want strict decode error", err)
|
||||
}
|
||||
}
|
||||
@@ -121,6 +121,24 @@ inputs:
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadSessionWithOptionsRequiresExpectedPreviousSession(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
sessionPath := filepath.Join(dir, "session.yml")
|
||||
sessionYAML := `session_id: 2026-05-03
|
||||
campaign: sample-campaign
|
||||
inputs:
|
||||
audio_dir: ./audio
|
||||
`
|
||||
if err := os.WriteFile(sessionPath, []byte(sessionYAML), 0o644); err != nil {
|
||||
t.Fatalf("write session.yml: %v", err)
|
||||
}
|
||||
|
||||
_, err := LoadSessionWithOptions(sessionPath, SessionLoadOptions{PreviousSessionID: "2026-04-26"})
|
||||
if err == nil || !strings.Contains(err.Error(), "previous_session_id is required") {
|
||||
t.Fatalf("LoadSessionWithOptions() error = %v, want missing previous-session expectation error", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadSessionWithOptionsUnknownFieldStillRejected(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
sessionPath := filepath.Join(dir, "session.yml")
|
||||
@@ -217,6 +235,17 @@ func TestLoadSessionBytesWithOptionsMismatchFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadSessionBytesWithOptionsRequiresExpectedPreviousSession(t *testing.T) {
|
||||
_, err := LoadSessionBytesWithOptions(
|
||||
"s3://bucket/session.yml",
|
||||
[]byte("session_id: 2026-05-03\n"),
|
||||
SessionLoadOptions{PreviousSessionID: "2026-04-26"},
|
||||
)
|
||||
if err == nil || !strings.Contains(err.Error(), "previous_session_id is required") {
|
||||
t.Fatalf("LoadSessionBytesWithOptions() error = %v, want missing previous-session expectation error", err)
|
||||
}
|
||||
}
|
||||
|
||||
func assertConcreteSessionTemplateError(t *testing.T, err error, vars ...string) {
|
||||
t.Helper()
|
||||
if !strings.Contains(err.Error(), "session.yml must be concrete") {
|
||||
|
||||
@@ -153,6 +153,108 @@ storage:
|
||||
}
|
||||
}
|
||||
|
||||
func TestStorageBackendSelectionValidation(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
storageYAML string
|
||||
sessionYAML string
|
||||
wantErr string
|
||||
wantBackend string
|
||||
}{
|
||||
{
|
||||
name: "omitted storage defaults local",
|
||||
wantBackend: StorageBackendLocal,
|
||||
},
|
||||
{
|
||||
name: "explicit local backend",
|
||||
storageYAML: `storage:
|
||||
backend: local
|
||||
`,
|
||||
wantBackend: StorageBackendLocal,
|
||||
},
|
||||
{
|
||||
name: "case insensitive s3 backend normalizes",
|
||||
storageYAML: `storage:
|
||||
backend: S3
|
||||
s3:
|
||||
bucket: my-dnd-archive
|
||||
`,
|
||||
wantBackend: StorageBackendS3,
|
||||
},
|
||||
{
|
||||
name: "unknown backend",
|
||||
storageYAML: `storage:
|
||||
backend: s33
|
||||
`,
|
||||
wantErr: "pipeline.storage.backend must be one of: local, s3",
|
||||
},
|
||||
{
|
||||
name: "s3 block requires s3 backend",
|
||||
storageYAML: `storage:
|
||||
backend: local
|
||||
s3:
|
||||
bucket: my-dnd-archive
|
||||
`,
|
||||
wantErr: "pipeline.storage.s3 is only supported when pipeline.storage.backend is s3",
|
||||
},
|
||||
{
|
||||
name: "s3 backend requires s3 block",
|
||||
storageYAML: `storage:
|
||||
backend: s3
|
||||
`,
|
||||
wantErr: "pipeline.storage.s3 is required when pipeline.storage.backend is s3",
|
||||
},
|
||||
{
|
||||
name: "s3 backend requires bucket for publish",
|
||||
storageYAML: `storage:
|
||||
backend: s3
|
||||
s3: {}
|
||||
`,
|
||||
wantErr: "pipeline.storage.s3.bucket is required when S3 session audio or publish upload is enabled",
|
||||
},
|
||||
{
|
||||
name: "s3 audio requires s3 backend",
|
||||
sessionYAML: `session_id: 2026-05-03
|
||||
inputs:
|
||||
audio_s3:
|
||||
prefix: audio/
|
||||
`,
|
||||
wantErr: "pipeline.storage.backend must be s3 when session.inputs.audio_s3 is configured",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
pipelineYAML := testPipelineBaseYAML
|
||||
if tt.storageYAML != "" {
|
||||
pipelineYAML += "\n" + tt.storageYAML
|
||||
}
|
||||
sessionYAML := testSessionBaseYAML
|
||||
if tt.sessionYAML != "" {
|
||||
sessionYAML = tt.sessionYAML
|
||||
}
|
||||
pipelinePath, sessionPath := writeConfigFiles(t, pipelineYAML, sessionYAML)
|
||||
cfg, err := Load(pipelinePath, sessionPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Load() error = %v", err)
|
||||
}
|
||||
err = Validate(cfg)
|
||||
if tt.wantErr != "" {
|
||||
if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
|
||||
t.Fatalf("Validate() error = %v, want %q", err, tt.wantErr)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("Validate() error = %v", err)
|
||||
}
|
||||
if cfg.Pipeline.Storage.Backend != tt.wantBackend {
|
||||
t.Fatalf("storage.backend = %q, want %q", cfg.Pipeline.Storage.Backend, tt.wantBackend)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSpoolAndPublishDefaults(t *testing.T) {
|
||||
pipelinePath, sessionPath := writeConfigFiles(t, testPipelineBaseYAML, testSessionBaseYAML)
|
||||
|
||||
@@ -681,6 +783,7 @@ func TestStorageS3BucketRequiredWhenS3DependentFeatureEnabled(t *testing.T) {
|
||||
pipelineYAML := testPipelineBaseYAML + `
|
||||
storage:
|
||||
backend: s3
|
||||
s3: {}
|
||||
`
|
||||
sessionYAML := `session_id: 2026-05-03
|
||||
campaign: forsaken
|
||||
|
||||
@@ -130,8 +130,19 @@ func validatePipeline(cfg *PipelineConfig) error {
|
||||
}
|
||||
|
||||
func validateStorage(cfg StorageConfig) error {
|
||||
if cfg.S3 == nil {
|
||||
backend := strings.ToLower(strings.TrimSpace(cfg.Backend))
|
||||
switch backend {
|
||||
case "", StorageBackendLocal:
|
||||
if cfg.S3 != nil {
|
||||
return fmt.Errorf("pipeline.storage.s3 is only supported when pipeline.storage.backend is s3")
|
||||
}
|
||||
return nil
|
||||
case StorageBackendS3:
|
||||
if cfg.S3 == nil {
|
||||
return fmt.Errorf("pipeline.storage.s3 is required when pipeline.storage.backend is s3")
|
||||
}
|
||||
default:
|
||||
return fmt.Errorf("pipeline.storage.backend must be one of: local, s3")
|
||||
}
|
||||
if strings.TrimSpace(cfg.S3.RootPrefix) == "" {
|
||||
return fmt.Errorf("pipeline.storage.s3.root_prefix must be non-empty")
|
||||
@@ -783,13 +794,16 @@ func validateCrossConfig(pipeline *PipelineConfig, session *SessionConfig) error
|
||||
if pipeline == nil || session == nil {
|
||||
return nil
|
||||
}
|
||||
if pipeline.Storage.S3 == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
audioS3Enabled := session.Inputs.AudioS3 != nil
|
||||
publishUploadEnabled := publishUploadConfiguredForS3(pipeline)
|
||||
if (audioS3Enabled || publishUploadEnabled) && strings.TrimSpace(pipeline.Storage.S3.Bucket) == "" {
|
||||
if audioS3Enabled && !strings.EqualFold(strings.TrimSpace(pipeline.Storage.Backend), StorageBackendS3) {
|
||||
return fmt.Errorf("pipeline.storage.backend must be s3 when session.inputs.audio_s3 is configured")
|
||||
}
|
||||
if !audioS3Enabled && !publishUploadEnabled {
|
||||
return nil
|
||||
}
|
||||
if pipeline.Storage.S3 == nil || strings.TrimSpace(pipeline.Storage.S3.Bucket) == "" {
|
||||
return fmt.Errorf("pipeline.storage.s3.bucket is required when S3 session audio or publish upload is enabled")
|
||||
}
|
||||
return nil
|
||||
@@ -799,7 +813,7 @@ func publishUploadConfiguredForS3(pipeline *PipelineConfig) bool {
|
||||
if pipeline == nil || pipeline.Publish == nil {
|
||||
return false
|
||||
}
|
||||
if !strings.EqualFold(strings.TrimSpace(pipeline.Storage.Backend), "s3") {
|
||||
if !strings.EqualFold(strings.TrimSpace(pipeline.Storage.Backend), StorageBackendS3) {
|
||||
return false
|
||||
}
|
||||
enabled := true
|
||||
@@ -975,9 +989,13 @@ func validateDuration(fieldName, value string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
if _, err := time.ParseDuration(trimmed); err != nil {
|
||||
duration, err := time.ParseDuration(trimmed)
|
||||
if err != nil {
|
||||
return fmt.Errorf("%s must be a valid duration: %w", fieldName, err)
|
||||
}
|
||||
if duration <= 0 {
|
||||
return fmt.Errorf("%s must be positive", fieldName)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
114
internal/config/validation_duration_test.go
Normal file
114
internal/config/validation_duration_test.go
Normal file
@@ -0,0 +1,114 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestValidateDurationsRequirePositiveValues(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
set func(*PipelineConfig, string)
|
||||
want string
|
||||
}{
|
||||
{name: "whisperx timeout", set: func(p *PipelineConfig, value string) { p.WhisperX.Timeout = value }, want: "pipeline.whisperx.timeout"},
|
||||
{name: "whisperx retry delay", set: func(p *PipelineConfig, value string) { p.WhisperX.RetryDelay = value }, want: "pipeline.whisperx.retry_delay"},
|
||||
{name: "seriatim timeout", set: func(p *PipelineConfig, value string) { p.Seriatim.Timeout = value }, want: "pipeline.seriatim.timeout"},
|
||||
{name: "audita timeout", set: func(p *PipelineConfig, value string) { p.Audita.Timeout = value }, want: "pipeline.audita.timeout"},
|
||||
{name: "scriptorium timeout", set: func(p *PipelineConfig, value string) { p.Scriptorium.Timeout = value }, want: "pipeline.scriptorium.timeout"},
|
||||
{name: "scriptorium artifact timeout", set: func(p *PipelineConfig, value string) {
|
||||
if p.Scriptorium.Artifacts == nil {
|
||||
p.Scriptorium.Artifacts = map[string]ScriptoriumArtifactConfig{}
|
||||
}
|
||||
p.Scriptorium.Artifacts["session_recap"] = ScriptoriumArtifactConfig{Timeout: value}
|
||||
}, want: "pipeline.scriptorium.artifacts.session_recap.timeout"},
|
||||
{name: "trim bounds timeout", set: func(p *PipelineConfig, value string) { p.Trim.Bounds.Timeout = value }, want: "pipeline.trim.bounds.timeout"},
|
||||
{name: "notification timeout", set: func(p *PipelineConfig, value string) { p.Notification.Timeout = value }, want: "pipeline.notification.timeout"},
|
||||
}
|
||||
|
||||
for _, value := range []string{"0s", "-1ms"} {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name+"/"+value, func(t *testing.T) {
|
||||
cfg := loadedValidConfig(t)
|
||||
tt.set(cfg.Pipeline, value)
|
||||
err := Validate(cfg)
|
||||
if err == nil || !strings.Contains(err.Error(), tt.want+" must be positive") {
|
||||
t.Fatalf("Validate() error = %v, want positive-value error for %s", err, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateNotariusTimeoutRequiresPositiveValue(t *testing.T) {
|
||||
for _, value := range []string{"0s", "-1ms"} {
|
||||
t.Run(value, func(t *testing.T) {
|
||||
cfg := loadedValidConfig(t)
|
||||
cfg.Pipeline.Notarius = &NotariusConfig{
|
||||
Enabled: true,
|
||||
Binary: "notarius",
|
||||
ConfigPath: "/tmp/notarius.yml",
|
||||
PipelineID: "session",
|
||||
Timeout: value,
|
||||
WorkingDirectory: "/tmp",
|
||||
Outputs: map[string]NotariusOutputConfig{
|
||||
"npc_registry": {
|
||||
LaneID: "npc-registry",
|
||||
MediaType: "application/json",
|
||||
SchemaID: "notarius.dnd.npc_registry",
|
||||
SchemaVersion: "v1",
|
||||
},
|
||||
},
|
||||
}
|
||||
err := Validate(cfg)
|
||||
if err == nil || !strings.Contains(err.Error(), "pipeline.notarius.timeout must be positive") {
|
||||
t.Fatalf("Validate() error = %v, want positive Notarius timeout error", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateDurationRejectsOverflowAndAcceptsPositiveSubsecondValues(t *testing.T) {
|
||||
cfg := loadedValidConfig(t)
|
||||
cfg.Pipeline.WhisperX.Timeout = "999999999999999999999h"
|
||||
if err := Validate(cfg); err == nil || !strings.Contains(err.Error(), "pipeline.whisperx.timeout must be a valid duration") {
|
||||
t.Fatalf("Validate() overflow error = %v, want duration parse error", err)
|
||||
}
|
||||
|
||||
cfg = loadedValidConfig(t)
|
||||
cfg.Pipeline.WhisperX.Timeout = "1ms"
|
||||
cfg.Pipeline.WhisperX.RetryDelay = "1ms"
|
||||
cfg.Pipeline.Seriatim.Timeout = "1ms"
|
||||
cfg.Pipeline.Audita.Timeout = "1ms"
|
||||
cfg.Pipeline.Scriptorium.Timeout = "1ms"
|
||||
cfg.Pipeline.Trim.Bounds.Timeout = "1ms"
|
||||
cfg.Pipeline.Notification.Timeout = "1ms"
|
||||
if cfg.Pipeline.Scriptorium.Artifacts == nil {
|
||||
cfg.Pipeline.Scriptorium.Artifacts = map[string]ScriptoriumArtifactConfig{}
|
||||
}
|
||||
cfg.Pipeline.Scriptorium.Artifacts["session_recap"] = ScriptoriumArtifactConfig{Timeout: "1ms"}
|
||||
if err := Validate(cfg); err != nil {
|
||||
t.Fatalf("Validate() positive subsecond durations error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateAllowsEmptyArtifactTimeoutFallback(t *testing.T) {
|
||||
cfg := loadedValidConfig(t)
|
||||
if cfg.Pipeline.Scriptorium.Artifacts == nil {
|
||||
cfg.Pipeline.Scriptorium.Artifacts = map[string]ScriptoriumArtifactConfig{}
|
||||
}
|
||||
cfg.Pipeline.Scriptorium.Artifacts["session_recap"] = ScriptoriumArtifactConfig{Timeout: ""}
|
||||
if err := Validate(cfg); err != nil {
|
||||
t.Fatalf("Validate() empty artifact timeout error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func loadedValidConfig(t *testing.T) *Config {
|
||||
t.Helper()
|
||||
pipelinePath, sessionPath := writeConfigFiles(t, testPipelineBaseYAML, testSessionBaseYAML)
|
||||
cfg, err := Load(pipelinePath, sessionPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Load() error = %v", err)
|
||||
}
|
||||
return cfg
|
||||
}
|
||||
Reference in New Issue
Block a user