Enforce fs source containment

2026-07-05 00:10:47 +00:00
parent 39485d87f6
commit a16f66cbc7
9 changed files with 327 additions and 24 deletions
--- a/docs/consumers/pkg-scriptorium.md
+++ b/docs/consumers/pkg-scriptorium.md
@@ -31,7 +31,7 @@ Directory fields on `Config` remain the compatibility path. Explicit source opti
 - `WithProfileFS(fsys, root)` and `WithProfileFile(path)`
 - `WithSchemaFS(fsys, root)` and `WithSchemaFile(path)`

-Prompt and profile sources load standard Scriptorium YAML with the same strict validation as directory sources. Prompt `content_file` paths resolve relative to the prompt file in the same source. Profile options overlay custom profiles above built-ins. Schema `fs.FS` sources preserve prompt `schema_path` semantics; schema file options expose the file by its base name.
+Prompt and profile sources load standard Scriptorium YAML with the same strict validation as directory sources. For `WithPromptFS`, the configured root is a containment boundary: prompt `content_file` paths resolve relative to the prompt file and must remain inside that root. Profile options overlay custom profiles above built-ins. For `WithSchemaFS`, prompt `schema_path` values resolve inside the configured root. Absolute paths and relative traversal outside those `fs.FS` roots are rejected. Schema file options expose the file by its base name.

 ## In-Memory Profiles

--- a/docs/internal/adapters.md
+++ b/docs/internal/adapters.md
@@ -11,7 +11,7 @@ This document describes implemented adapter/repository boundaries and their curr
 - root package `scriptorium`: public Go library facade for preparing and running prompt requests.
 - `internal/promptdef`: filesystem and `fs.FS` prompt-definition repositories.
 - `internal/profile`: filesystem, `fs.FS`, and overlay execution-profile repositories.
- `internal/filecatalog`: shared YAML discovery and display-path helpers for prompt/profile repositories.
+- `internal/filecatalog`: shared YAML discovery, display-path, and `fs.FS` source-root resolution helpers.
 - `internal/profile/builtin`: embedded built-in execution-profile repository.
 - `internal/artifact`: input artifact reader.
 - `internal/prompt`: Go-template renderer.
@@ -46,7 +46,7 @@ Prompt/profile repositories:

 - Input: prompt/profile YAML files under configured directories or `fs.FS` roots.
 - Output: normalized domain definitions/profiles or typed errors.
- Shared YAML catalog helpers provide recursive discovery, extension filtering, deterministic ordering, file stems, and `fs.FS` display paths.
+- Shared YAML catalog helpers provide recursive discovery, extension filtering, deterministic ordering, file stems, `fs.FS` display paths, and source-root containment checks.
 - Single-file public sources are represented as `fs.FS` roots containing one YAML file; lookup still uses YAML `id` values.

 Profile repository composition:
@@ -110,6 +110,7 @@ Strict decoding and input checks:
 - prompt/profile repositories scan nested subdirectories recursively.
 - prompt/profile lookup uses YAML `id` values; subdirectory paths are organizational only.
 - prompt `content_file` paths resolve relative to the prompt YAML file within the same source.
+- `fs.FS` prompt `content_file` paths and schema paths must remain inside the configured source root; absolute paths and relative traversal outside the root are rejected.
 - duplicate prompt/profile IDs are invalid and fail instead of using first-match behavior.
 - duplicate profile IDs across custom and built-in sources are allowed; the custom source overrides the built-in profile.
 - HTTP DTO decoder rejects unknown JSON fields.
@@ -142,7 +143,8 @@ Validator:

 - `basic`, `json`, `json_schema` content failures return `ValidationFailed` results.
 - schema load/compile/path failures are runtime errors.
- schema lookup uses explicit `schema_path` values relative to `schema_dir`; it does not recursively search by basename.
+- directory-backed schema lookup uses explicit `schema_path` values relative to `schema_dir`; it does not recursively search by basename.
+- `fs.FS` schema lookup uses explicit `schema_path` values inside the configured source root. Single-file public schema sources match by base name.

 HTTP error mapping:

--- a/engine_test.go
+++ b/engine_test.go
@@ -881,6 +881,38 @@ output:
 	}
 }

+func TestPrepareWithPromptFSRejectsEscapedContentFile(t *testing.T) {
+	promptFS := fstest.MapFS{
+		"assets/prompts/fs-escape.yaml": &fstest.MapFile{Data: []byte(`
+id: fs.escape
+version: "1.0.0"
+default_profile: local-fast
+messages:
+  - role: user
+    content_file: ../outside.tmpl
+output:
+  format: text
+  validation_mode: none
+  repair_attempts: 0
+`)},
+		"assets/outside.tmpl": &fstest.MapFile{Data: []byte(`Outside root.`)},
+	}
+
+	engine, err := scriptorium.NewEngine(scriptorium.Config{
+		PromptDir:  t.TempDir(),
+		ProfileDir: "./examples/profiles",
+		SchemaDir:  "./examples/schemas",
+	}, scriptorium.WithPromptFS(promptFS, "assets/prompts"))
+	if err != nil {
+		t.Fatalf("expected engine construction to succeed, got %v", err)
+	}
+
+	_, err = engine.Prepare(context.Background(), scriptorium.RunRequest{PromptID: "fs.escape"})
+	if !errors.Is(err, scriptorium.ErrPromptLoad) {
+		t.Fatalf("expected ErrPromptLoad, got %v", err)
+	}
+}
+
 func TestPrepareWorksWithPromptFile(t *testing.T) {
 	promptDir := t.TempDir()
 	promptPath := filepath.Join(promptDir, "single.yaml")
@@ -893,7 +925,7 @@ inputs:
    required: true
 messages:
  - role: user
-    content: "Summarize {{input \"transcript\"}} from file."
+    content_file: ./single.tmpl
 output:
  format: text
  validation_mode: none
@@ -901,6 +933,9 @@ output:
 `), 0o644); err != nil {
 		t.Fatal(err)
 	}
+	if err := os.WriteFile(filepath.Join(promptDir, "single.tmpl"), []byte(`Summarize {{input "transcript"}} from file.`), 0o644); err != nil {
+		t.Fatal(err)
+	}

 	engine, err := scriptorium.NewEngine(scriptorium.Config{
 		ProfileDir: "./examples/profiles",
@@ -922,6 +957,9 @@ output:
 	if prepared.PromptID != "single.file.prompt" {
 		t.Fatalf("unexpected prompt id: %q", prepared.PromptID)
 	}
+	if len(prepared.Messages) != 1 || !strings.Contains(prepared.Messages[0].Content, "from file") {
+		t.Fatalf("expected content_file body from prompt file, got %+v", prepared.Messages)
+	}
 }

 func TestPrepareWorksWithProfileFSOverBuiltIns(t *testing.T) {
--- a/internal/filecatalog/catalog.go
+++ b/internal/filecatalog/catalog.go
@@ -2,6 +2,7 @@ package filecatalog

 import (
 	"context"
+	"fmt"
 	"io/fs"
 	"os"
 	"path"
@@ -93,6 +94,42 @@ func DisplayPath(root string, name string) string {
 	return cleanName
 }

+// ResolveFSPath resolves userPath from baseDir and keeps it inside root.
+func ResolveFSPath(root string, baseDir string, userPath string) (string, string, error) {
+	cleanRoot := CleanFSRoot(root)
+	cleanBase := path.Clean(strings.TrimSpace(baseDir))
+	if cleanBase == "" {
+		cleanBase = cleanRoot
+	}
+	if !containsFSPath(cleanRoot, cleanBase) {
+		return "", "", fmt.Errorf("base path %q is outside source root %q", cleanBase, cleanRoot)
+	}
+
+	cleanUserPath := strings.TrimSpace(userPath)
+	if cleanUserPath == "" {
+		return "", "", fmt.Errorf("path is required")
+	}
+	cleanUserPath = path.Clean(cleanUserPath)
+	if path.IsAbs(cleanUserPath) {
+		return "", "", fmt.Errorf("path %q must be relative", userPath)
+	}
+
+	resolved := path.Clean(path.Join(cleanBase, cleanUserPath))
+	if !containsFSPath(cleanRoot, resolved) {
+		return "", "", fmt.Errorf("path %q escapes source root %q", userPath, cleanRoot)
+	}
+	return resolved, DisplayPath(cleanRoot, resolved), nil
+}
+
+func containsFSPath(root string, name string) bool {
+	root = CleanFSRoot(root)
+	name = path.Clean(name)
+	if root == "." {
+		return name == "." || (name != ".." && !strings.HasPrefix(name, "../"))
+	}
+	return name == root || strings.HasPrefix(name, strings.TrimSuffix(root, "/")+"/")
+}
+
 // Stem strips .yaml or .yml from a file name.
 func Stem(name string) string {
 	name = strings.TrimSuffix(name, ".yaml")
--- a/internal/filecatalog/catalog_test.go
+++ b/internal/filecatalog/catalog_test.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 	"testing/fstest"
 )
@@ -131,6 +132,92 @@ func TestDisplayPath(t *testing.T) {
 	}
 }

+func TestResolveFSPath(t *testing.T) {
+	tests := []struct {
+		name        string
+		root        string
+		baseDir     string
+		userPath    string
+		wantPath    string
+		wantDisplay string
+		wantErr     string
+	}{
+		{
+			name:        "sibling inside root",
+			root:        "prompts",
+			baseDir:     "prompts/nested",
+			userPath:    "./messages/user.tmpl",
+			wantPath:    "prompts/nested/messages/user.tmpl",
+			wantDisplay: "nested/messages/user.tmpl",
+		},
+		{
+			name:        "parent inside root",
+			root:        "prompts",
+			baseDir:     "prompts/nested",
+			userPath:    "../shared/user.tmpl",
+			wantPath:    "prompts/shared/user.tmpl",
+			wantDisplay: "shared/user.tmpl",
+		},
+		{
+			name:     "escape rejected",
+			root:     "prompts",
+			baseDir:  "prompts/nested",
+			userPath: "../../outside.tmpl",
+			wantErr:  "escapes source root",
+		},
+		{
+			name:     "absolute path rejected",
+			root:     "prompts",
+			baseDir:  "prompts/nested",
+			userPath: "/outside.tmpl",
+			wantErr:  "must be relative",
+		},
+		{
+			name:     "empty path rejected",
+			root:     "prompts",
+			baseDir:  "prompts/nested",
+			userPath: " ",
+			wantErr:  "path is required",
+		},
+		{
+			name:        "dot root allows normal relative path",
+			root:        ".",
+			baseDir:     ".",
+			userPath:    "schemas/events.schema.json",
+			wantPath:    "schemas/events.schema.json",
+			wantDisplay: "schemas/events.schema.json",
+		},
+		{
+			name:     "dot root rejects parent escape",
+			root:     ".",
+			baseDir:  ".",
+			userPath: "../outside.tmpl",
+			wantErr:  "escapes source root",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			gotPath, gotDisplay, err := ResolveFSPath(tc.root, tc.baseDir, tc.userPath)
+			if tc.wantErr != "" {
+				if err == nil {
+					t.Fatalf("expected error containing %q", tc.wantErr)
+				}
+				if !strings.Contains(err.Error(), tc.wantErr) {
+					t.Fatalf("expected error to contain %q, got %v", tc.wantErr, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("expected no error, got %v", err)
+			}
+			if gotPath != tc.wantPath || gotDisplay != tc.wantDisplay {
+				t.Fatalf("expected path/display %q/%q, got %q/%q", tc.wantPath, tc.wantDisplay, gotPath, gotDisplay)
+			}
+		})
+	}
+}
+
 func TestStemStripsYAMLExtensions(t *testing.T) {
 	tests := []struct {
 		name string
--- a/internal/promptdef/filesystem_repository.go
+++ b/internal/promptdef/filesystem_repository.go
@@ -193,6 +193,11 @@ func loadPromptDefinition(ctx context.Context, fsys fs.FS, root string, id strin
 	if err != nil {
 		return nil, fmt.Errorf("failed to read prompt definition directory: %w", err)
 	}
+	cleanRoot := filecatalog.CleanFSRoot(root)
+	rootInfo, err := fs.Stat(fsys, cleanRoot)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read prompt definition directory: %w", err)
+	}

 	var matches []promptDefinitionMatch
 	for _, fullPath := range files {
@@ -220,7 +225,7 @@ func loadPromptDefinition(ctx context.Context, fsys fs.FS, root string, id strin
 			continue
 		}

-		def, err := normalizePromptDefinitionFromFS(raw, fsys, fullPath)
+		def, err := normalizePromptDefinitionFromFS(raw, fsys, root, fullPath, rootInfo.IsDir())
 		if err != nil {
 			if fileMatch || strings.TrimSpace(raw.ID) == id {
 				return nil, fmt.Errorf("%w: %s: %v", ErrInvalidPromptDefinition, relPath, err)
@@ -295,14 +300,23 @@ func normalizePromptDefinition(raw *promptDefinitionFile, sourcePath string) (*d
 	})
 }

-func normalizePromptDefinitionFromFS(raw *promptDefinitionFile, fsys fs.FS, sourcePath string) (*domain.PromptDefinition, error) {
+func normalizePromptDefinitionFromFS(raw *promptDefinitionFile, fsys fs.FS, root string, sourcePath string, rootIsDir bool) (*domain.PromptDefinition, error) {
 	promptDir := path.Dir(sourcePath)
 	return normalizePromptDefinitionWithContent(raw, func(contentFile string) (string, string, error) {
-		resolvedPath := strings.TrimSpace(contentFile)
-		if !path.IsAbs(resolvedPath) {
-			resolvedPath = path.Join(promptDir, resolvedPath)
+		var resolvedPath string
+		if rootIsDir {
+			var err error
+			resolvedPath, _, err = filecatalog.ResolveFSPath(root, promptDir, contentFile)
+			if err != nil {
+				return "", "", err
+			}
+		} else {
+			resolvedPath = strings.TrimSpace(contentFile)
+			if !path.IsAbs(resolvedPath) {
+				resolvedPath = path.Join(promptDir, resolvedPath)
+			}
+			resolvedPath = strings.TrimPrefix(path.Clean(resolvedPath), "/")
 		}
-		resolvedPath = strings.TrimPrefix(path.Clean(resolvedPath), "/")

 		body, err := fs.ReadFile(fsys, resolvedPath)
 		if err != nil {
--- a/internal/promptdef/repository_test.go
+++ b/internal/promptdef/repository_test.go
@@ -359,6 +359,69 @@ output:
 	}
 }

+func TestFSRepositoryContentFileContainment(t *testing.T) {
+	t.Run("nested prompt can reference file inside root", func(t *testing.T) {
+		repo := NewFSRepository(fstest.MapFS{
+			"prompts/nested/prompt.yaml": &fstest.MapFile{Data: []byte(`
+id: fs-contained-prompt
+version: "1.0.0"
+messages:
+  - role: user
+    content_file: ../shared/user.tmpl
+output:
+  format: markdown
+  validation_mode: basic
+  repair_attempts: 0
+`)},
+			"prompts/shared/user.tmpl": &fstest.MapFile{Data: []byte(`Inside root.`)},
+		}, "prompts")
+
+		got, err := repo.GetPromptDefinition(context.Background(), "fs-contained-prompt", "")
+		if err != nil {
+			t.Fatalf("expected no error, got %v", err)
+		}
+		if len(got.Templates) != 1 || got.Templates[0].Content != "Inside root." {
+			t.Fatalf("expected contained content file, got %+v", got.Templates)
+		}
+	})
+
+	tests := []struct {
+		name        string
+		contentFile string
+		wantErr     string
+	}{
+		{name: "parent escape rejected", contentFile: "../outside.tmpl", wantErr: "escapes source root"},
+		{name: "absolute path rejected", contentFile: "/outside.tmpl", wantErr: "must be relative"},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			repo := NewFSRepository(fstest.MapFS{
+				"prompts/prompt.yaml": &fstest.MapFile{Data: []byte(`
+id: fs-escaped-prompt
+version: "1.0.0"
+messages:
+  - role: user
+    content_file: ` + tc.contentFile + `
+output:
+  format: markdown
+  validation_mode: basic
+  repair_attempts: 0
+`)},
+				"outside.tmpl": &fstest.MapFile{Data: []byte(`Outside root.`)},
+			}, "prompts")
+
+			_, err := repo.GetPromptDefinition(context.Background(), "fs-escaped-prompt", "")
+			if !errors.Is(err, ErrInvalidPromptDefinition) {
+				t.Fatalf("expected ErrInvalidPromptDefinition, got %v", err)
+			}
+			if !strings.Contains(err.Error(), tc.wantErr) {
+				t.Fatalf("expected error to contain %q, got %v", tc.wantErr, err)
+			}
+		})
+	}
+}
+
 func TestFSRepositoryRejectsDuplicatePromptIDs(t *testing.T) {
 	repo := NewFSRepository(fstest.MapFS{
 		"one.yaml": &fstest.MapFile{Data: []byte(`
--- a/internal/validate/standard_validator.go
+++ b/internal/validate/standard_validator.go
@@ -12,6 +12,7 @@ import (
 	"strings"

 	"gitea.maximumdirect.net/eric/scriptorium/internal/domain"
+	"gitea.maximumdirect.net/eric/scriptorium/internal/filecatalog"
 	"github.com/santhosh-tekuri/jsonschema/v6"
 )

@@ -244,17 +245,24 @@ func (v *FSValidator) resolveSchemaPath(schemaPath string) (string, error) {
 		return "", errors.New("schema filesystem is nil")
 	}

-	cleanRoot := cleanFSRoot(v.root)
+	cleanRoot := filecatalog.CleanFSRoot(v.root)
 	rootInfo, err := fs.Stat(v.fsys, cleanRoot)
 	if err != nil {
 		return "", fmt.Errorf("failed to access schema source %q: %w", cleanRoot, err)
 	}

-	cleanSchemaPath := cleanSchemaFSPath(schemaPath)
 	var resolved string
 	if rootInfo.IsDir() {
-		resolved = path.Join(cleanRoot, cleanSchemaPath)
+		resolvedPath, _, err := filecatalog.ResolveFSPath(cleanRoot, cleanRoot, schemaPath)
+		if err != nil {
+			return "", err
+		}
+		resolved = resolvedPath
 	} else {
+		cleanSchemaPath, err := cleanSchemaFSPath(schemaPath)
+		if err != nil {
+			return "", err
+		}
 		if cleanSchemaPath != path.Base(cleanRoot) {
 			return "", fmt.Errorf("schema path %q does not match schema file %q", cleanSchemaPath, path.Base(cleanRoot))
 		}
@@ -267,18 +275,16 @@ func (v *FSValidator) resolveSchemaPath(schemaPath string) (string, error) {
 	return resolved, nil
 }

-func cleanSchemaFSPath(schemaPath string) string {
+func cleanSchemaFSPath(schemaPath string) (string, error) {
 	cleaned := strings.TrimSpace(schemaPath)
-	cleaned = strings.TrimPrefix(path.Clean(cleaned), "/")
-	return cleaned
-}
-
-func cleanFSRoot(root string) string {
-	root = strings.TrimSpace(root)
-	if root == "" || root == "." {
-		return "."
+	if cleaned == "" {
+		return "", errors.New("schema path is required for json_schema validation")
 	}
-	return strings.TrimPrefix(path.Clean(root), "/")
+	cleaned = path.Clean(cleaned)
+	if path.IsAbs(cleaned) {
+		return "", fmt.Errorf("schema path %q must be relative", schemaPath)
+	}
+	return cleaned, nil
 }

 func fsSchemaResourceURL(schemaName string) string {
--- a/internal/validate/standard_validator_test.go
+++ b/internal/validate/standard_validator_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"testing/fstest"

@@ -276,6 +277,61 @@ func TestFSValidatorJSONSchemaSuccess(t *testing.T) {
 	}
 }

+func TestFSValidatorJSONSchemaPathContainment(t *testing.T) {
+	t.Run("nested schema inside root succeeds", func(t *testing.T) {
+		v := NewFSValidator(fstest.MapFS{
+			"schemas/nested/events.schema.json": &fstest.MapFile{Data: []byte(`{
+  "type": "object",
+  "required": ["events"],
+  "properties": {
+    "events": {"type": "array"}
+  }
+}`)},
+		}, "schemas")
+
+		res, err := v.Validate(context.Background(), &domain.Artifact{Body: []byte(`{"events":[]}`)}, domain.OutputContract{
+			ValidationMode: domain.ValidationJSONSchema,
+			SchemaPath:     "nested/events.schema.json",
+		})
+		if err != nil {
+			t.Fatalf("expected no error, got %v", err)
+		}
+		if res.Status != domain.ValidationPassed || !res.IsValid {
+			t.Fatalf("expected passed/valid, got status=%q valid=%v", res.Status, res.IsValid)
+		}
+	})
+
+	tests := []struct {
+		name       string
+		schemaPath string
+		wantErr    string
+	}{
+		{name: "parent escape rejected", schemaPath: "../outside.schema.json", wantErr: "escapes source root"},
+		{name: "absolute path rejected", schemaPath: "/outside.schema.json", wantErr: "must be relative"},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			v := NewFSValidator(fstest.MapFS{
+				"schemas/events.schema.json":  &fstest.MapFile{Data: []byte(`{"type":"object"}`)},
+				"outside.schema.json":         &fstest.MapFile{Data: []byte(`{"type":"object"}`)},
+				"schemas/outside.schema.json": &fstest.MapFile{Data: []byte(`{"type":"object"}`)},
+			}, "schemas")
+
+			_, err := v.Validate(context.Background(), &domain.Artifact{Body: []byte(`{"events":[]}`)}, domain.OutputContract{
+				ValidationMode: domain.ValidationJSONSchema,
+				SchemaPath:     tc.schemaPath,
+			})
+			if err == nil {
+				t.Fatal("expected schema path error")
+			}
+			if !strings.Contains(err.Error(), tc.wantErr) {
+				t.Fatalf("expected error to contain %q, got %v", tc.wantErr, err)
+			}
+		})
+	}
+}
+
 func TestFSValidatorSingleSchemaFileUsesBaseName(t *testing.T) {
 	v := NewFSValidator(fstest.MapFS{
 		"events.schema.json": &fstest.MapFile{Data: []byte(`{