From 6111cb23a1c6b00ba250b7f4ead76e149c7c1a2f Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 29 Sep 2026 17:29:01 +0000 Subject: [PATCH] Unify host and native filesystem acquisition --- docs/roadmap/implementation.md | 10 + internal/engine/filesystem_access.go | 184 ++++++++++++++ internal/engine/glob.go | 76 +++--- internal/engine/imports.go | 124 ++++++---- internal/engine/native_filesystem_test.go | 284 ++++++++++++++++++++++ internal/engine/selected_overlay.go | 56 +++-- internal/engine/source.go | 21 ++ internal/engine/source_file.go | 3 +- 8 files changed, 636 insertions(+), 122 deletions(-) create mode 100644 internal/engine/filesystem_access.go create mode 100644 internal/engine/native_filesystem_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 3c39eb2..045f4bd 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -282,6 +282,16 @@ growth avoids accumulated full-tree copying. Run the everyday checker and ## Stage 3: Unify private filesystem acquisition without public changes +**Completed 2026-09-29.** Imports, glob traversal, and selected-overlay loading +now use a private access boundary with the existing contained host backend and +a native `fs.FS` backend. Namespace tokens and occurrence identities are separate +from labels and paths; repetition tracking is composition-local and input/glob +accounting can be shared. Native tests cover slash-relative imports, sorted +globs, optional missing paths, root escape, overlays, repeated/cyclic imports, +non-comparable filesystems, and handle closure on success and representative +failures. Existing host tests, focused native tests, and the everyday checker +passed. No public constructor or context field was added. + Scope: engine source/file, filesystem, import, glob, and selected-overlay code. 1. Extract a small private namespace/access boundary used by imports and overlays. diff --git a/internal/engine/filesystem_access.go b/internal/engine/filesystem_access.go new file mode 100644 index 0000000..023c73d --- /dev/null +++ b/internal/engine/filesystem_access.go @@ -0,0 +1,184 @@ +package engine + +import ( + "errors" + "io" + "io/fs" + "os" + "path" + "path/filepath" + "strings" +) + +// namespaceToken identifies one caller-selected filesystem without comparing +// arbitrary fs.FS values. A token can be shared by copies of a source wrapper. +type namespaceToken struct{ marker byte } + +type nativeNamespace struct { + filesystem fs.FS + label string + token *namespaceToken +} + +func newNativeNamespace(label string, filesystem fs.FS) *nativeNamespace { + return &nativeNamespace{filesystem: filesystem, label: label, token: &namespaceToken{}} +} + +type resourceIdentity struct { + namespace *namespaceToken + path string + occurrence *namespaceToken +} + +// filesystemAccess is a load-scoped view of either a contained host root or a +// borrowed native filesystem. Names are canonical host paths or fs.FS paths. +type filesystemAccess struct { + rootPath string + root *os.Root + native fs.FS + token *namespaceToken +} + +func openHostAccess(rootPath string) (*filesystemAccess, error) { + root, err := os.OpenRoot(rootPath) + if err != nil { + return nil, err + } + return &filesystemAccess{rootPath: rootPath, root: root, token: &namespaceToken{}}, nil +} + +func newNativeAccess(filesystem fs.FS, root string, token *namespaceToken) *filesystemAccess { + return &filesystemAccess{rootPath: root, native: filesystem, token: token} +} + +func (a *filesystemAccess) close() error { + if a.root != nil { + return a.root.Close() + } + return nil +} + +func (a *filesystemAccess) identity(name string) resourceIdentity { + return resourceIdentity{namespace: a.token, path: name} +} + +func (a *filesystemAccess) syntheticIdentity() resourceIdentity { + return resourceIdentity{namespace: a.token, occurrence: &namespaceToken{}} +} + +func (a *filesystemAccess) join(base, name string) string { + if a.native != nil { + return path.Join(base, name) + } + return filepath.Join(base, filepath.FromSlash(path.Clean(name))) +} + +func (a *filesystemAccess) dir(name string) string { + if a.native != nil { + return path.Dir(name) + } + return filepath.Dir(name) +} + +func (a *filesystemAccess) contains(name string) bool { + if a.native == nil { + return containedPath(a.rootPath, filepath.Clean(name)) + } + if !fs.ValidPath(name) || !fs.ValidPath(a.rootPath) { + return false + } + return a.rootPath == "." || name == a.rootPath || strings.HasPrefix(name, a.rootPath+"/") +} + +func (a *filesystemAccess) canonical(name string) (string, error) { + if a.native == nil { + return canonicalExistingPath(name) + } + if !fs.ValidPath(name) { + return "", fs.ErrInvalid + } + _, err := fs.Stat(a.native, name) + if err != nil { + return "", err + } + return name, nil +} + +func (a *filesystemAccess) relative(name string) (string, error) { + if a.native != nil { + if !a.contains(name) { + return "", fs.ErrInvalid + } + return name, nil + } + relative, err := filepath.Rel(a.rootPath, name) + if err != nil || filepath.IsAbs(relative) || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return "", fs.ErrInvalid + } + return relative, nil +} + +func (a *filesystemAccess) stat(name string) (fs.FileInfo, error) { + if a.native != nil { + return fs.Stat(a.native, name) + } + relative, err := a.relative(name) + if err != nil { + return nil, err + } + return a.root.Stat(relative) +} + +func (a *filesystemAccess) open(name string) (fs.File, error) { + if a.native != nil { + return a.native.Open(name) + } + relative, err := a.relative(name) + if err != nil { + return nil, err + } + return a.root.Open(relative) +} + +func (a *filesystemAccess) readDirectory(name string, visit func(fs.DirEntry) bool) error { + file, err := a.open(name) + if err != nil { + return err + } + if reader, ok := file.(fs.ReadDirFile); ok { + for { + entries, readErr := reader.ReadDir(globReadBatch) + for _, entry := range entries { + if !visit(entry) { + return file.Close() + } + } + if errors.Is(readErr, io.EOF) { + return file.Close() + } + if readErr != nil { + _ = file.Close() + return readErr + } + if len(entries) == 0 { + _ = file.Close() + return io.ErrNoProgress + } + } + } + if err := file.Close(); err != nil { + return err + } + // fs.ReadDir provides the standard fallback for implementations whose opened + // files do not implement fs.ReadDirFile. It may allocate all entries itself. + entries, err := fs.ReadDir(a.native, name) + if err != nil { + return err + } + for _, entry := range entries { + if !visit(entry) { + break + } + } + return nil +} diff --git a/internal/engine/glob.go b/internal/engine/glob.go index 468be6a..749b6ac 100644 --- a/internal/engine/glob.go +++ b/internal/engine/glob.go @@ -2,11 +2,8 @@ package engine import ( "errors" - "io" "io/fs" - "os" "path" - "path/filepath" "slices" "strings" ) @@ -42,29 +39,29 @@ func (s *importTraversal) expandImportGlob(basePath string, declaration importDe names = []string{literal} } for _, name := range names { - absolute := filepath.Join(candidate.absolute, filepath.FromSlash(name)) + absolute := s.access.join(candidate.absolute, name) relative := path.Join(candidate.relative, name) - if !containedPath(s.rootPath, filepath.Clean(absolute)) { + if !s.access.contains(absolute) { return nil, []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "glob traversal is outside the configuration root", &location)} } - canonical, err := canonicalExistingPath(absolute) + canonical, err := s.access.canonical(absolute) if err != nil { if errors.Is(err, fs.ErrNotExist) && !patterned { continue } return nil, []Diagnostic{globFilesystemDiagnostic(err, "glob match could not be canonicalized", location)} } - if !containedPath(s.rootPath, canonical) { + if !s.access.contains(canonical) { return nil, []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "glob traversal resolves outside the configuration root", &location)} } if last { next = append(next, globCandidate{absolute: canonical, relative: relative}) - if len(s.sources)+len(next) > s.limits.sources { + if s.accounting.sources+len(next) > s.limits.sources { return nil, []Diagnostic{importDiagnostic(CodeImportLimitExceeded, "glob matches exceed the remaining source-count limit", &location)} } continue } - info, err := os.Stat(canonical) + info, err := s.access.stat(canonical) if err != nil { return nil, []Diagnostic{globFilesystemDiagnostic(err, "glob traversal target could not be inspected", location)} } @@ -88,56 +85,47 @@ func (s *importTraversal) expandImportGlob(basePath string, declaration importDe } func (s *importTraversal) readGlobMatches(directory, pattern string, location Location) ([]string, []Diagnostic) { - canonical, err := canonicalExistingPath(directory) + canonical, err := s.access.canonical(directory) if err != nil { if errors.Is(err, fs.ErrNotExist) { return nil, nil } return nil, []Diagnostic{globFilesystemDiagnostic(err, "glob directory could not be canonicalized", location)} } - if !containedPath(s.rootPath, canonical) { + if !s.access.contains(canonical) { return nil, []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "glob directory resolves outside the configuration root", &location)} } - relative, err := filepath.Rel(s.rootPath, canonical) - if err != nil || filepath.IsAbs(relative) || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + if _, err := s.access.relative(canonical); err != nil { return nil, []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "glob directory could not be addressed inside the configuration root", &location)} } - directoryFile, err := s.root.Open(relative) - if err != nil { - if errors.Is(err, fs.ErrNotExist) { - return nil, nil - } - return nil, []Diagnostic{globFilesystemDiagnostic(err, "glob directory could not be opened", location)} - } - var matches []string - for { - entries, readErr := directoryFile.ReadDir(globReadBatch) - for _, entry := range entries { - s.usedEntries++ - if s.usedEntries > s.limits.entries { - _ = directoryFile.Close() - return nil, []Diagnostic{importDiagnostic(CodeImportLimitExceeded, "glob traversal exceeds the directory-entry visit limit", &location)} - } - matched, matchErr := path.Match(pattern, entry.Name()) - if matchErr != nil { - _ = directoryFile.Close() - return nil, []Diagnostic{importDiagnostic(CodeImportInvalid, "import glob pattern is invalid", &location)} - } - if matched { - matches = append(matches, entry.Name()) - } + var problem *Diagnostic + err = s.access.readDirectory(canonical, func(entry fs.DirEntry) bool { + s.accounting.entries++ + if s.accounting.entries > s.limits.entries { + value := importDiagnostic(CodeImportLimitExceeded, "glob traversal exceeds the directory-entry visit limit", &location) + problem = &value + return false } - if errors.Is(readErr, io.EOF) { - break + matched, matchErr := path.Match(pattern, entry.Name()) + if matchErr != nil { + value := importDiagnostic(CodeImportInvalid, "import glob pattern is invalid", &location) + problem = &value + return false } - if readErr != nil { - _ = directoryFile.Close() - return nil, []Diagnostic{globFilesystemDiagnostic(readErr, "glob directory could not be read", location)} + if matched { + matches = append(matches, entry.Name()) } + return true + }) + if problem != nil { + return nil, []Diagnostic{*problem} } - if err := directoryFile.Close(); err != nil { - return nil, []Diagnostic{globFilesystemDiagnostic(err, "glob directory could not be closed", location)} + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, []Diagnostic{globFilesystemDiagnostic(err, "glob directory could not be traversed", location)} } slices.Sort(matches) return matches, nil diff --git a/internal/engine/imports.go b/internal/engine/imports.go index 28d2428..5b5b876 100644 --- a/internal/engine/imports.go +++ b/internal/engine/imports.go @@ -4,9 +4,6 @@ import ( "bytes" "errors" "io/fs" - "os" - "path" - "path/filepath" "strconv" "strings" @@ -40,57 +37,83 @@ type importRoute struct { locations []Location } -type importTraversal struct { - rootPath string - root *os.Root - limits importLimits - rule *VersionRule +type importAccounting struct { + sources int + bytes int64 + nodes int + entries int +} - documents []*document.Node - sources []SourceInfo - active map[string]int - visited map[string]importRoute - stack []importRoute - usedBytes int64 - usedNodes int - usedEntries int +type importTraversal struct { + access *filesystemAccess + accounting *importAccounting + limits importLimits + rule *VersionRule + + documents []*document.Node + sources []SourceInfo + active map[resourceIdentity]int + visited map[resourceIdentity]importRoute + stack []importRoute } func loadImportedDocuments(source Source, context *FilesystemContext, main parsedSource, rule *VersionRule, limits importLimits) (*composedDocument, []SourceInfo, []Diagnostic) { + if source.kind == sourceFilesystem { + access := newNativeAccess(source.namespace.filesystem, ".", source.namespace.token) + return loadImportedWithAccess(access, access.dir(source.path), source.path, main, rule, limits, &importAccounting{}) + } rootPath, basePath, mainPath, diagnostic := importFilesystem(source, context, main) if diagnostic != nil { return nil, sourceInfos(main.source), []Diagnostic{*diagnostic} } - root, err := os.OpenRoot(rootPath) + access, err := openHostAccess(rootPath) if err != nil { diagnostic := globFilesystemDiagnostic(err, "configuration root could not be opened", Location{}) diagnostic.Primary = nil return nil, sourceInfos(main.source), []Diagnostic{diagnostic} } + return loadImportedWithAccess(access, basePath, mainPath, main, rule, limits, &importAccounting{}) +} +func loadImportedWithAccess(access *filesystemAccess, basePath, mainPath string, main parsedSource, rule *VersionRule, limits importLimits, accounting *importAccounting) (*composedDocument, []SourceInfo, []Diagnostic) { + if !access.contains(basePath) || (mainPath != "" && !access.contains(mainPath)) { + _ = access.close() + return nil, sourceInfos(main.source), []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "selected source and import base must stay inside the configuration root", nil)} + } + for _, directory := range []string{access.rootPath, basePath} { + info, err := access.stat(directory) + if err != nil || !info.IsDir() { + _ = access.close() + return nil, sourceInfos(main.source), []Diagnostic{importDiagnostic(CodeImportInvalid, "configuration root and import base must be directories", nil)} + } + } mainInfo := cloneSourceInfo(*main.source) if mainPath != "" { mainInfo.Path = mainPath } + accounting.sources++ + accounting.bytes += main.bytes + accounting.nodes += countInputNodes(main.document) state := importTraversal{ - rootPath: rootPath, root: root, limits: limits, rule: rule, + access: access, accounting: accounting, limits: limits, rule: rule, documents: make([]*document.Node, 0, 1), sources: []SourceInfo{mainInfo}, - active: make(map[string]int), visited: make(map[string]importRoute), - usedBytes: main.bytes, usedNodes: countInputNodes(main.document), + active: make(map[resourceIdentity]int), visited: make(map[resourceIdentity]importRoute), } if diagnostic := state.checkInitialLimits(); diagnostic != nil { - _ = root.Close() + _ = access.close() return nil, cloneSourceInfos(state.sources), []Diagnostic{*diagnostic} } mainIdentity := mainPath + mainKey := access.identity(mainPath) if mainIdentity == "" { mainIdentity = "" + mainKey = access.syntheticIdentity() } mainRoute := importRoute{canonical: mainIdentity} - state.active[mainIdentity] = 0 - state.visited[mainIdentity] = mainRoute + state.active[mainKey] = 0 + state.visited[mainKey] = mainRoute state.stack = append(state.stack, mainRoute) body, declarations, diagnostics := splitImportDirectives(main.document) if len(diagnostics) == 0 { @@ -98,7 +121,7 @@ func loadImportedDocuments(source Source, context *FilesystemContext, main parse diagnostics = state.walkDeclarations(basePath, declarations, 0) } - closeErr := root.Close() + closeErr := access.close() if len(diagnostics) != 0 { return nil, cloneSourceInfos(state.sources), diagnostics } @@ -107,22 +130,22 @@ func loadImportedDocuments(source Source, context *FilesystemContext, main parse } composed, diagnostics := composeDocuments(state.documents...) if composed != nil { - composed.inputBytes = state.usedBytes - composed.inputNodes = state.usedNodes + composed.inputBytes = accounting.bytes + composed.inputNodes = accounting.nodes } return composed, cloneSourceInfos(state.sources), diagnostics } func (s *importTraversal) checkInitialLimits() *Diagnostic { - if s.limits.sources < 1 { + if s.accounting.sources > s.limits.sources { value := importDiagnostic(CodeImportLimitExceeded, "configuration exceeds the source-count limit", nil) return &value } - if s.usedBytes > s.limits.bytes { + if s.accounting.bytes > s.limits.bytes { value := importDiagnostic(CodeImportLimitExceeded, "configuration exceeds the aggregate byte limit", nil) return &value } - if s.usedNodes > s.limits.nodes { + if s.accounting.nodes > s.limits.nodes { value := importDiagnostic(CodeImportLimitExceeded, "configuration exceeds the aggregate node limit", nil) return &value } @@ -169,11 +192,11 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati return []Diagnostic{importDiagnostic(CodeImportInvalid, "import path must be a nonempty slash-relative path without a network scheme", &location)} } - candidate := filepath.Join(basePath, filepath.FromSlash(path.Clean(declaration.path))) - if !containedPath(s.rootPath, filepath.Clean(candidate)) { + candidate := s.access.join(basePath, declaration.path) + if !s.access.contains(candidate) { return []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "import target is outside the configuration root", &location)} } - canonical, err := canonicalExistingPath(candidate) + canonical, err := s.access.canonical(candidate) if err != nil { if declaration.optional && errors.Is(err, fs.ErrNotExist) { return nil @@ -187,30 +210,30 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati } return []Diagnostic{importDiagnostic(code, message, &location)} } - if !containedPath(s.rootPath, canonical) { + if !s.access.contains(canonical) { return []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "import target resolves outside the configuration root", &location)} } currentRoute := s.currentRoute(location) - if activeIndex, ok := s.active[canonical]; ok { + identity := s.access.identity(canonical) + if activeIndex, ok := s.active[identity]; ok { cycle := append([]importRoute(nil), s.stack[activeIndex:]...) return []Diagnostic{importCycleDiagnostic(canonical, location, cycle)} } - if previous, ok := s.visited[canonical]; ok { + if previous, ok := s.visited[identity]; ok { return []Diagnostic{importRepeatedDiagnostic(location, previous, currentRoute)} } - if len(s.sources)+1 > s.limits.sources { + if s.accounting.sources+1 > s.limits.sources { return []Diagnostic{importDiagnostic(CodeImportLimitExceeded, "configuration exceeds the source-count limit", &location)} } - relative, err := filepath.Rel(s.rootPath, canonical) - if err != nil || filepath.IsAbs(relative) || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + if _, err := s.access.relative(canonical); err != nil { return []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "import target could not be addressed inside the configuration root", &location)} } - sourceID := "source-" + strconv.Itoa(len(s.sources)+1) + sourceID := "source-" + strconv.Itoa(s.accounting.sources+1) info := SourceInfo{ID: sourceID, Label: declaration.path, Path: canonical, Layer: LayerBase, ImportedFrom: &location} s.sources = append(s.sources, info) - data, skipped, diagnostic := s.readImport(relative, sourceID, declaration.optional) + data, skipped, diagnostic := s.readImport(canonical, sourceID, declaration.optional) if diagnostic != nil { return []Diagnostic{*diagnostic} } @@ -218,6 +241,7 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati s.sources = s.sources[:len(s.sources)-1] return nil } + s.accounting.sources++ root, issues := yamladapter.Parse(bytes.NewReader(data), sourceID, yamladapter.Limits{ Depth: maxDocumentDepth, Nodes: maxDocumentNodes, @@ -226,10 +250,10 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati return parserDiagnostics(sourceID, issues) } nodeCount := countInputNodes(root) - if s.usedNodes+nodeCount > s.limits.nodes { + if s.accounting.nodes+nodeCount > s.limits.nodes { return []Diagnostic{importDiagnostic(CodeImportLimitExceeded, "configuration exceeds the aggregate node limit", &location)} } - s.usedNodes += nodeCount + s.accounting.nodes += nodeCount body, declarations, diagnostics := splitImportDirectives(root) if len(diagnostics) != 0 { @@ -245,22 +269,22 @@ func (s *importTraversal) visitPath(basePath string, declaration importDeclarati route := importRoute{canonical: canonical, locations: currentRoute.locations} s.documents = append(s.documents, body) - s.active[canonical] = len(s.stack) - s.visited[canonical] = route + s.active[identity] = len(s.stack) + s.visited[identity] = route s.stack = append(s.stack, route) - diagnostics = s.walkDeclarations(filepath.Dir(canonical), declarations, depth) - delete(s.active, canonical) + diagnostics = s.walkDeclarations(s.access.dir(canonical), declarations, depth) + delete(s.active, identity) s.stack = s.stack[:len(s.stack)-1] return diagnostics } -func (s *importTraversal) readImport(relative, sourceID string, optional bool) ([]byte, bool, *Diagnostic) { - remaining := s.limits.bytes - s.usedBytes +func (s *importTraversal) readImport(name, sourceID string, optional bool) ([]byte, bool, *Diagnostic) { + remaining := s.limits.bytes - s.accounting.bytes limit := min(int64(maxSourceBytes), remaining) if limit < 0 { limit = 0 } - data, failure := readRegularFile(relative, s.root.Stat, s.root.Open, limit) + data, failure := readRegularFile(name, s.access.stat, s.access.open, limit) if failure != nil { if optional && failure.missingTarget { return nil, true, nil @@ -268,7 +292,7 @@ func (s *importTraversal) readImport(relative, sourceID string, optional bool) ( value := sourceReadDiagnostic(failure, sourceID, CodeImportLimitExceeded, "configuration exceeds a source or aggregate byte limit") return nil, false, &value } - s.usedBytes += int64(len(data)) + s.accounting.bytes += int64(len(data)) return data, false, nil } diff --git a/internal/engine/native_filesystem_test.go b/internal/engine/native_filesystem_test.go new file mode 100644 index 0000000..57463e5 --- /dev/null +++ b/internal/engine/native_filesystem_test.go @@ -0,0 +1,284 @@ +package engine + +import ( + "errors" + "io/fs" + "reflect" + "strings" + "testing" + "testing/fstest" +) + +func TestNativeFilesystemImportsAndOverlay(t *testing.T) { + filesystem := fstest.MapFS{ + "config/main.yml": &fstest.MapFile{Data: []byte(`configkit: + imports: + - path: ../shared.yml + - glob: fragments/*.yml +items: {main: main} +`)}, + "shared.yml": &fstest.MapFile{Data: []byte("items: {shared: shared}\n")}, + "config/fragments/b.yml": &fstest.MapFile{Data: []byte("items: {beta: beta}\n")}, + "config/fragments/a.yml": &fstest.MapFile{Data: []byte("items: {alpha: alpha}\n")}, + "overlays/testing.yml": &fstest.MapFile{Data: []byte("items: {main: overlaid}\n")}, + } + namespace := newNativeNamespace("bundle", filesystem) + result, err := testLoad[importTestConfig](filesystemFile(namespace, "config/main.yml"), Options[importTestConfig]{Imports: true, Overlay: "testing"}) + if err != nil { + t.Fatal(err) + } + want := map[string]string{"main": "overlaid", "shared": "shared", "alpha": "alpha", "beta": "beta"} + if !reflect.DeepEqual(result.Config.Items, want) { + t.Fatalf("items = %#v, want %#v", result.Config.Items, want) + } + sources := result.Sources() + wantPaths := []string{"config/main.yml", "shared.yml", "config/fragments/a.yml", "config/fragments/b.yml", "overlays/testing.yml"} + gotPaths := make([]string, len(sources)) + for index, source := range sources { + gotPaths[index] = source.Path + } + if !reflect.DeepEqual(gotPaths, wantPaths) { + t.Fatalf("source paths = %#v, want %#v", gotPaths, wantPaths) + } +} + +func TestNativeFilesystemNamespaceAndPathFailures(t *testing.T) { + first := fstest.MapFS{"main.yml": &fstest.MapFile{Data: []byte("items: {name: first}\n")}} + second := fstest.MapFS{"main.yml": &fstest.MapFile{Data: []byte("items: {name: second}\n")}} + firstNamespace := newNativeNamespace("same", first) + secondNamespace := newNativeNamespace("same", second) + if reflect.TypeOf(first).Comparable() { + t.Fatal("test filesystem must have a non-comparable dynamic value") + } + if firstNamespace.token == secondNamespace.token { + t.Fatal("independent filesystem constructors shared identity") + } + if newNativeAccess(first, ".", firstNamespace.token).identity("main.yml") == newNativeAccess(second, ".", secondNamespace.token).identity("main.yml") { + t.Fatal("equal paths from separate filesystems have the same identity") + } + access := newNativeAccess(first, ".", firstNamespace.token) + if access.syntheticIdentity() == access.syntheticIdentity() || access.syntheticIdentity() == access.identity("") { + t.Fatal("independent synthetic sources shared a file identity") + } + for _, test := range []struct { + name string + namespace *nativeNamespace + want string + }{ + {name: "first", namespace: firstNamespace, want: "first"}, + {name: "second", namespace: secondNamespace, want: "second"}, + } { + t.Run(test.name, func(t *testing.T) { + result, err := testLoad[importTestConfig](filesystemFile(test.namespace, "main.yml")) + if err != nil || result.Config.Items["name"] != test.want { + t.Fatalf("result = %#v, %v", result, err) + } + }) + } + for _, name := range []string{".", "../main.yml", "/main.yml", "missing.yml"} { + _, diagnostics := readSource(filesystemFile(firstNamespace, name), "source-1", defaultSourceLimits()) + if len(diagnostics) != 1 { + t.Errorf("%q diagnostics = %#v", name, diagnostics) + } + } + missingImport := fstest.MapFS{"main.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{path: host-only.yml}]}\n")}} + _, err := testLoad[importTestConfig](filesystemFile(newNativeNamespace("bundle", missingImport), "main.yml"), Options[importTestConfig]{Imports: true}) + if err == nil { + t.Fatal("missing virtual import fell back to the host filesystem") + } + loadError, ok := err.(*LoadError) + if !ok || loadError.Diagnostics()[0].Code != CodeImportMissing { + t.Fatalf("missing import error = %#v", err) + } +} + +func TestNativeFilesystemClosesFilesOnSuccessAndFailure(t *testing.T) { + for _, fallback := range []bool{false, true} { + t.Run(map[bool]string{false: "ReadDirFile", true: "ReadDir fallback"}[fallback], func(t *testing.T) { + filesystem := &observedFilesystem{files: fstest.MapFS{ + "main.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{glob: 'parts/*.yml'}]}\nitems: {main: main}\n")}, + "parts/one.yml": &fstest.MapFile{Data: []byte("items: {one: one}\n")}, + }, fallback: fallback} + result, err := testLoad[importTestConfig](filesystemFile(newNativeNamespace("bundle", filesystem), "main.yml"), Options[importTestConfig]{Imports: true}) + if err != nil || result.Config.Items["one"] != "one" { + t.Fatalf("result = %#v, %v", result, err) + } + filesystem.assertClosed(t) + }) + } + + for _, failure := range []struct { + name string + file string + failRead string + failStat string + failDir string + }{ + {name: "read", file: "items: {one: one}\n", failRead: "parts/one.yml"}, + {name: "stat", file: "items: {one: one}\n", failStat: "parts/one.yml"}, + {name: "parse", file: "items: [unterminated\n"}, + {name: "directory", file: "items: {one: one}\n", failDir: "parts"}, + } { + t.Run(failure.name, func(t *testing.T) { + filesystem := &observedFilesystem{files: fstest.MapFS{ + "main.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{glob: 'parts/*.yml'}]}\n")}, + "parts/one.yml": &fstest.MapFile{Data: []byte(failure.file)}, + }, failRead: failure.failRead, failStat: failure.failStat, failDir: failure.failDir} + _, err := testLoad[importTestConfig](filesystemFile(newNativeNamespace("bundle", filesystem), "main.yml"), Options[importTestConfig]{Imports: true}) + if err == nil { + t.Fatal("expected a load failure") + } + if strings.Contains(err.Error(), "tracking failure canary") { + t.Fatal("unsafe filesystem error escaped") + } + filesystem.assertClosed(t) + }) + } +} + +func TestNativeFilesystemRootAndOptionalImports(t *testing.T) { + filesystem := fstest.MapFS{ + "config/sub/main.yml": &fstest.MapFile{Data: []byte(`configkit: + imports: + - path: ../shared.yml + - path: absent.yml + optional: true + - glob: absent/*.yml + optional: true +`)}, + "config/shared.yml": &fstest.MapFile{Data: []byte("items: {shared: retained}\n")}, + "outside.yml": &fstest.MapFile{Data: []byte("items: {outside: blocked}\n")}, + } + namespace := newNativeNamespace("bundle", filesystem) + source := filesystemFile(namespace, "config/sub/main.yml") + parsed, diagnostics := parseSource(source, "source-1", defaultParseLimits()) + if len(diagnostics) != 0 { + t.Fatal(diagnostics) + } + access := newNativeAccess(filesystem, "config", namespace.token) + composed, sources, diagnostics := loadImportedWithAccess(access, "config/sub", source.path, parsed, nil, defaultImportLimits(), &importAccounting{}) + if len(diagnostics) != 0 || composed == nil || len(sources) != 2 || sources[1].Path != "config/shared.yml" { + t.Fatalf("contained imports = (%#v, %#v, %#v)", composed, sources, diagnostics) + } + + outside := fstest.MapFS{ + "config/sub/main.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{path: ../../outside.yml}]}\n")}, + "outside.yml": &fstest.MapFile{Data: []byte("items: {outside: blocked}\n")}, + } + outsideNamespace := newNativeNamespace("bundle", outside) + parsed, diagnostics = parseSource(filesystemFile(outsideNamespace, "config/sub/main.yml"), "source-1", defaultParseLimits()) + if len(diagnostics) != 0 { + t.Fatal(diagnostics) + } + _, _, diagnostics = loadImportedWithAccess(newNativeAccess(outside, "config", outsideNamespace.token), "config/sub", "config/sub/main.yml", parsed, nil, defaultImportLimits(), &importAccounting{}) + if len(diagnostics) != 1 || diagnostics[0].Code != CodeImportOutsideRoot { + t.Fatalf("escaping import diagnostics = %#v", diagnostics) + } +} + +func TestNativeFilesystemDetectsRepeatedAndCyclicImports(t *testing.T) { + for _, test := range []struct { + name string + files fstest.MapFS + code Code + }{ + { + name: "repeated", + files: fstest.MapFS{ + "main.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{path: child.yml}, {path: child.yml}]}\n")}, + "child.yml": &fstest.MapFile{Data: []byte("items: {child: child}\n")}, + }, + code: CodeImportRepeated, + }, + { + name: "cycle", + files: fstest.MapFS{ + "main.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{path: child.yml}]}\n")}, + "child.yml": &fstest.MapFile{Data: []byte("configkit: {imports: [{path: main.yml}]}\n")}, + }, + code: CodeImportCycle, + }, + } { + t.Run(test.name, func(t *testing.T) { + _, err := testLoad[importTestConfig](filesystemFile(newNativeNamespace("bundle", test.files), "main.yml"), Options[importTestConfig]{Imports: true}) + loadError, ok := err.(*LoadError) + if !ok || len(loadError.Diagnostics()) == 0 || loadError.Diagnostics()[0].Code != test.code { + t.Fatalf("load error = %#v, want %q", err, test.code) + } + }) + } +} + +type observedFilesystem struct { + files fstest.MapFS + opened int + closed int + fallback bool + failRead string + failStat string + failDir string +} + +func (f *observedFilesystem) Open(name string) (fs.File, error) { + inner, err := f.files.Open(name) + if err != nil { + return nil, err + } + f.opened++ + file := &observedFile{File: inner, owner: f, name: name} + if directory, ok := inner.(fs.ReadDirFile); ok && !f.fallback { + return &observedDirectoryFile{observedFile: file, directory: directory}, nil + } + return file, nil +} + +func (f *observedFilesystem) ReadDir(name string) ([]fs.DirEntry, error) { + if name == f.failDir { + return nil, errors.New("tracking failure canary") + } + return fs.ReadDir(f.files, name) +} + +func (f *observedFilesystem) assertClosed(t *testing.T) { + t.Helper() + if f.opened == 0 || f.opened != f.closed { + t.Fatalf("filesystem opens=%d closes=%d", f.opened, f.closed) + } +} + +type observedFile struct { + fs.File + owner *observedFilesystem + name string +} + +func (f *observedFile) Read(data []byte) (int, error) { + if f.name == f.owner.failRead { + return 0, errors.New("tracking failure canary") + } + return f.File.Read(data) +} + +func (f *observedFile) Stat() (fs.FileInfo, error) { + if f.name == f.owner.failStat { + return nil, errors.New("tracking failure canary") + } + return f.File.Stat() +} + +func (f *observedFile) Close() error { + f.owner.closed++ + return f.File.Close() +} + +type observedDirectoryFile struct { + *observedFile + directory fs.ReadDirFile +} + +func (f *observedDirectoryFile) ReadDir(count int) ([]fs.DirEntry, error) { + if f.name == f.owner.failDir { + return nil, errors.New("tracking failure canary") + } + return f.directory.ReadDir(count) +} diff --git a/internal/engine/selected_overlay.go b/internal/engine/selected_overlay.go index a435a2e..d5b5bec 100644 --- a/internal/engine/selected_overlay.go +++ b/internal/engine/selected_overlay.go @@ -4,10 +4,7 @@ import ( "bytes" "errors" "io/fs" - "os" - "path/filepath" "strconv" - "strings" yamladapter "gitea.maximumdirect.net/eric/ConfigKit/internal/yaml" ) @@ -41,12 +38,32 @@ func loadSelectedOverlayDocument(source Source, context *FilesystemContext, main } func loadSelectedOverlayInto(source Source, context *FilesystemContext, main parsedSource, base *assemblyAccumulator, sources []SourceInfo, name string, rule *VersionRule, limits importLimits) ([]SourceInfo, []Diagnostic) { + if source.kind == sourceFilesystem { + access := newNativeAccess(source.namespace.filesystem, ".", source.namespace.token) + return loadSelectedOverlayWithAccess(access, base, sources, name, rule, limits) + } rootPath, _, _, diagnostic := importFilesystem(source, context, main) if diagnostic != nil { return cloneSourceInfos(sources), []Diagnostic{*diagnostic} } - overlayPath := filepath.Join(rootPath, "overlays", name+".yml") - canonical, err := canonicalExistingPath(overlayPath) + access, err := openHostAccess(rootPath) + if err != nil { + value := globFilesystemDiagnostic(err, "configuration root could not be opened for the selected overlay", Location{}) + value.Primary = nil + return cloneSourceInfos(sources), []Diagnostic{value} + } + return loadSelectedOverlayWithAccess(access, base, sources, name, rule, limits) +} + +func loadSelectedOverlayWithAccess(access *filesystemAccess, base *assemblyAccumulator, sources []SourceInfo, name string, rule *VersionRule, limits importLimits) ([]SourceInfo, []Diagnostic) { + closed := false + defer func() { + if !closed { + _ = access.close() + } + }() + overlayPath := access.join(access.rootPath, "overlays/"+name+".yml") + canonical, err := access.canonical(overlayPath) if err != nil { code := CodeOverlayInvalid message := "selected overlay could not be resolved" @@ -57,7 +74,7 @@ func loadSelectedOverlayInto(source Source, context *FilesystemContext, main par } return cloneSourceInfos(sources), []Diagnostic{importDiagnostic(code, message, nil)} } - if !containedPath(rootPath, canonical) { + if !access.contains(canonical) { return cloneSourceInfos(sources), []Diagnostic{importDiagnostic(CodeOverlayOutsideRoot, "selected overlay resolves outside the configuration root", nil)} } for _, existing := range sources { @@ -69,28 +86,15 @@ func loadSelectedOverlayInto(source Source, context *FilesystemContext, main par return cloneSourceInfos(sources), []Diagnostic{importDiagnostic(CodeOverlayLimitExceeded, "selected overlay exceeds the source-count limit", nil)} } - root, err := os.OpenRoot(rootPath) - if err != nil { - value := globFilesystemDiagnostic(err, "configuration root could not be opened for the selected overlay", Location{}) - value.Primary = nil - return cloneSourceInfos(sources), []Diagnostic{value} - } - rootClosed := false - defer func() { - if !rootClosed { - _ = root.Close() - } - }() - relative, err := filepath.Rel(rootPath, canonical) - if err != nil || filepath.IsAbs(relative) || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + if _, err := access.relative(canonical); err != nil { return cloneSourceInfos(sources), []Diagnostic{importDiagnostic(CodeOverlayOutsideRoot, "selected overlay could not be addressed inside the configuration root", nil)} } sourceID := "source-" + strconv.Itoa(len(sources)+1) overlayLocation := &Location{SourceID: sourceID} - overlaySource := SourceInfo{ID: sourceID, Label: filepath.ToSlash(filepath.Join("overlays", name+".yml")), Path: canonical, Layer: LayerOverlay} + overlaySource := SourceInfo{ID: sourceID, Label: "overlays/" + name + ".yml", Path: canonical, Layer: LayerOverlay} sources = append(cloneSourceInfos(sources), overlaySource) - data, diagnostic := readOverlayFile(root, relative, sourceID, limits.bytes-base.current.inputBytes) + data, diagnostic := readOverlayFile(access, canonical, sourceID, limits.bytes-base.current.inputBytes) if diagnostic != nil { return sources, []Diagnostic{*diagnostic} } @@ -112,8 +116,8 @@ func loadSelectedOverlayInto(source Source, context *FilesystemContext, main par Message: "overlays cannot contribute to or replace the schema-version path", Primary: &location, }} } - closeErr := root.Close() - rootClosed = true + closeErr := access.close() + closed = true if closeErr != nil { return sources, []Diagnostic{importDiagnostic(CodeImportRead, "configuration root could not be closed after reading the selected overlay", nil)} } @@ -126,12 +130,12 @@ func loadSelectedOverlayInto(source Source, context *FilesystemContext, main par return sources, nil } -func readOverlayFile(root *os.Root, relative, sourceID string, remaining int64) ([]byte, *Diagnostic) { +func readOverlayFile(access *filesystemAccess, name, sourceID string, remaining int64) ([]byte, *Diagnostic) { limit := min(int64(maxSourceBytes), remaining) if limit < 0 { limit = 0 } - data, failure := readRegularFile(relative, root.Stat, root.Open, limit) + data, failure := readRegularFile(name, access.stat, access.open, limit) if failure != nil { value := sourceReadDiagnostic(failure, sourceID, CodeOverlayLimitExceeded, "selected overlay exceeds a source or aggregate byte limit") return nil, &value diff --git a/internal/engine/source.go b/internal/engine/source.go index 82a8008..d1e9d0e 100644 --- a/internal/engine/source.go +++ b/internal/engine/source.go @@ -22,6 +22,7 @@ type Source struct { kind sourceKind path string label string + namespace *nativeNamespace data []byte reader *readerSource createErr sourceCreationError @@ -32,6 +33,7 @@ type sourceKind uint8 const ( sourceInvalid sourceKind = iota sourceFile + sourceFilesystem sourceBytes sourceReader sourceNone @@ -60,6 +62,13 @@ func File(path string) Source { return Source{kind: sourceFile, path: path, label: path} } +func filesystemFile(namespace *nativeNamespace, name string) Source { + if namespace == nil || namespace.filesystem == nil || name == "." || !fs.ValidPath(name) { + return Source{kind: sourceFilesystem, createErr: creationInvalidPath} + } + return Source{kind: sourceFilesystem, path: name, label: namespace.label + "/" + name, namespace: namespace} +} + // Bytes snapshots data immediately. Label must be nonempty and safe to display; // constructor problems are reported when the source is consumed. func Bytes(label string, data []byte) Source { @@ -143,6 +152,18 @@ func readSource(source Source, sourceID string, limits sourceLimits) (sourceRead return sourceRead{info: info}, []Diagnostic{sourceReadDiagnostic(failure, sourceID, CodeSourceLimitExceeded, "source exceeds the configured byte limit")} } return sourceRead{data: bytes.Clone(data), info: info}, nil + case sourceFilesystem: + access := newNativeAccess(source.namespace.filesystem, ".", source.namespace.token) + info.Path = source.path + canonical, err := access.canonical(source.path) + if err != nil { + return sourceRead{info: info}, []Diagnostic{classifySourceError(err, sourceID)} + } + data, failure := readRegularFile(canonical, access.stat, access.open, limit) + if failure != nil { + return sourceRead{info: info}, []Diagnostic{sourceReadDiagnostic(failure, sourceID, CodeSourceLimitExceeded, "source exceeds the configured byte limit")} + } + return sourceRead{data: bytes.Clone(data), info: info}, nil case sourceBytes: input = bytes.NewReader(source.data) case sourceReader: diff --git a/internal/engine/source_file.go b/internal/engine/source_file.go index ab15bba..9ee9d55 100644 --- a/internal/engine/source_file.go +++ b/internal/engine/source_file.go @@ -4,7 +4,6 @@ import ( "errors" "io" "io/fs" - "os" ) type sourceReadFailureKind uint8 @@ -26,7 +25,7 @@ type sourceReadFailure struct { // readRegularFile borrows stat/open operations so root-bound callers retain their // containment policy. Preflight rejects FIFOs before open can block. The opened // handle is checked again and always closed; read-only close errors are ignored. -func readRegularFile(name string, stat func(string) (fs.FileInfo, error), open func(string) (*os.File, error), limit int64) ([]byte, *sourceReadFailure) { +func readRegularFile[F fs.File](name string, stat func(string) (fs.FileInfo, error), open func(string) (F, error), limit int64) ([]byte, *sourceReadFailure) { info, err := stat(name) if err != nil { return nil, &sourceReadFailure{cause: err, missingTarget: errors.Is(err, fs.ErrNotExist)}