From 7bf2f0c6872c2f974e3bbd59f8bea81cc0d69421 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 25 Oct 2026 21:43:26 +0000 Subject: [PATCH] Memoize completed native link dependencies --- docs/roadmap/implementation.md | 6 + internal/engine/filesystem_links.go | 135 +++++++++++++-------- internal/engine/filesystem_links_test.go | 145 +++++++++++++++++++++++ 3 files changed, 235 insertions(+), 51 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 1ae58d6..f17af3c 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -263,6 +263,12 @@ Finding: native part of **C02**. Owners: `internal/engine/filesystem_links.go`, `filesystem_links_test.go`, `directory_links_test.go`, and glob link tests. +Status: **complete** (native C02). Prior completed commits: `2736ddf`, +`deb9aa5`, `ae3408a`, `ea27f17`, `08a8c25`, `877c497`, `8aba43d`. +Validation passed: `env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Link|Directories|Glob' -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`, +and `git diff --check`. + 1. Extract the existing portable frame evaluator into one private resolution operation with `active` and `completed` maps. A small set of operation-local functions supplies `Lstat`, link reading, and validated target normalization. diff --git a/internal/engine/filesystem_links.go b/internal/engine/filesystem_links.go index d25733d..1a43c13 100644 --- a/internal/engine/filesystem_links.go +++ b/internal/engine/filesystem_links.go @@ -126,6 +126,69 @@ type importLinkFrame struct { link string } +type importLinkOperations struct { + lstat func(string) (fs.FileInfo, error) + readlink func(string) (string, error) + normalizeTarget func(string, string) (string, error) +} + +func resolveImportLinkPath(name string, operations importLinkOperations) (string, error) { + frames := []importLinkFrame{{pending: strings.Split(name, "/")}} + resolved := "" + active := make(map[string]struct{}) + completed := make(map[string]string) + for len(frames) > 0 { + frame := &frames[len(frames)-1] + if len(frame.pending) == 0 { + if frame.link != "" { + if resolved == "" { + resolved = "." + } + completed[frame.link] = resolved + delete(active, frame.link) + } + frames = frames[:len(frames)-1] + continue + } + segment := frame.pending[0] + frame.pending = frame.pending[1:] + candidate := path.Join(resolved, segment) + if candidate == "" { + candidate = "." + } + info, err := operations.lstat(candidate) + if err != nil { + return "", err + } + if info.Mode()&fs.ModeSymlink == 0 { + resolved = candidate + continue + } + if target, ok := completed[candidate]; ok { + resolved = target + continue + } + if _, exists := active[candidate]; exists { + return "", errImportDirectoryLinkCycle + } + target, err := operations.readlink(candidate) + if err != nil { + return "", err + } + targetPath, err := operations.normalizeTarget(candidate, target) + if err != nil { + return "", err + } + active[candidate] = struct{}{} + frames = append(frames, importLinkFrame{pending: strings.Split(targetPath, "/"), link: candidate}) + resolved = "" + } + if resolved == "" { + resolved = "." + } + return resolved, nil +} + func resolveHostImportPath(access *filesystemAccess, name string) (string, error) { frames := []importLinkFrame{{pending: strings.Split(filepath.ToSlash(name), "/")}} resolved := "" @@ -189,57 +252,27 @@ func resolveNativeImportPath(access *filesystemAccess, filesystem fs.ReadLinkFS, if !fs.ValidPath(name) || !access.contains(name) { return "", fs.ErrInvalid } - frames := []importLinkFrame{{pending: strings.Split(name, "/")}} - resolved := "" - activeLinks := make(map[string]struct{}) - for len(frames) > 0 { - frame := &frames[len(frames)-1] - if len(frame.pending) == 0 { - delete(activeLinks, frame.link) - frames = frames[:len(frames)-1] - continue - } - segment := frame.pending[0] - frame.pending = frame.pending[1:] - candidate := path.Join(resolved, segment) - if candidate == "" { - candidate = "." - } - // Ancestors of a multi-component root are resolution prefixes, not - // selected targets. Complete paths and link targets remain contained. - if !access.contains(candidate) && !strings.HasPrefix(access.rootPath, candidate+"/") { - return "", errImportPathOutsideRoot - } - info, err := filesystem.Lstat(candidate) - if err != nil { - return "", err - } - if info.Mode()&fs.ModeSymlink == 0 { - resolved = candidate - continue - } - if _, exists := activeLinks[candidate]; exists { - return "", errImportDirectoryLinkCycle - } - activeLinks[candidate] = struct{}{} - target, err := filesystem.ReadLink(candidate) - if err != nil { - return "", err - } - if target == "" || strings.HasPrefix(target, "/") { - return "", errImportPathOutsideRoot - } - targetPath := path.Clean(path.Join(path.Dir(candidate), target)) - if !fs.ValidPath(targetPath) || !access.contains(targetPath) { - return "", errImportPathOutsideRoot - } - frames = append(frames, importLinkFrame{pending: strings.Split(targetPath, "/"), link: candidate}) - resolved = "" - } - if resolved == "" { - resolved = "." - } - return resolved, nil + return resolveImportLinkPath(name, importLinkOperations{ + lstat: func(candidate string) (fs.FileInfo, error) { + // Ancestors of a multi-component root are resolution prefixes, not + // selected targets. Complete paths and link targets remain contained. + if !access.contains(candidate) && !strings.HasPrefix(access.rootPath, candidate+"/") { + return nil, errImportPathOutsideRoot + } + return filesystem.Lstat(candidate) + }, + readlink: filesystem.ReadLink, + normalizeTarget: func(candidate, target string) (string, error) { + if target == "" || strings.HasPrefix(target, "/") { + return "", errImportPathOutsideRoot + } + targetPath := path.Clean(path.Join(path.Dir(candidate), target)) + if !fs.ValidPath(targetPath) || !access.contains(targetPath) { + return "", errImportPathOutsideRoot + } + return targetPath, nil + }, + }) } func isImportLinkCycle(err error) bool { diff --git a/internal/engine/filesystem_links_test.go b/internal/engine/filesystem_links_test.go index 36a470d..b4b63ef 100644 --- a/internal/engine/filesystem_links_test.go +++ b/internal/engine/filesystem_links_test.go @@ -2,6 +2,7 @@ package engine import ( "errors" + "fmt" "io/fs" "os" "path/filepath" @@ -45,3 +46,147 @@ func TestImportLinkResolutionTracksActiveExpansions(t *testing.T) { }) } } + +func TestNativeLinkResolutionMemoizesCompletedDependencies(t *testing.T) { + // The small graph stays within host symlink-depth limits while exceeding the + // bounded linear operation allowance without completed-target reuse. + const linkCount = 5 + root := t.TempDir() + configurationRoot := filepath.Join(root, "workspace", "config") + for _, directory := range []string{ + filepath.Join(configurationRoot, "graph"), + filepath.Join(configurationRoot, "selected"), + } { + if err := os.MkdirAll(directory, 0o700); err != nil { + t.Fatal(err) + } + } + for index := 0; index < linkCount; index++ { + name := fmt.Sprintf("s%02d", index) + target := "." + if index > 0 { + previous := fmt.Sprintf("s%02d", index-1) + target = previous + "/" + previous + } + if err := os.Symlink(target, filepath.Join(configurationRoot, "graph", name)); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + } + if err := os.WriteFile(filepath.Join(configurationRoot, "graph", "item.yml"), []byte("value: linked\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(fmt.Sprintf("../graph/s%02d", linkCount-1), filepath.Join(configurationRoot, "selected", "entry")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + for _, selector := range []string{"directory", "glob"} { + t.Run(selector, func(t *testing.T) { + base := os.DirFS(root) + if _, ok := base.(fs.ReadLinkFS); !ok { + t.Skip("host directory filesystem does not expose link inspection") + } + filesystem := newCountingReadLinkFS(base, linkCount*20+40) + access := newNativeAccess(filesystem, "workspace/config", &namespaceToken{}) + if _, err := access.canonical("workspace/config/selected"); err != nil { + t.Fatalf("selected directory canonicalization failed before selection: %v", err) + } + var diagnostics []Diagnostic + if selector == "directory" { + _, diagnostics = selectDirectoryWithDiagnostics(t, access, "workspace/config", " - dir: selected\n follow: true\n") + } else { + _, diagnostics = selectGlobForTestWithDiagnostics(t, access, "workspace/config", " - glob: 'selected/*/*.yml'\n follow: true\n") + } + if len(diagnostics) != 0 { + t.Fatalf("selection diagnostics = %#v after %d link operations", diagnostics, filesystem.operations) + } + if filesystem.operations == 0 || filesystem.operations > filesystem.limit { + t.Fatalf("link-resolution operations = %d, want 1..%d", filesystem.operations, filesystem.limit) + } + if filesystem.readLinks < linkCount { + t.Fatalf("link reads = %d, want the selector to resolve the repeated dependency graph", filesystem.readLinks) + } + }) + } +} + +func TestNativeLinkResolutionRejectsEscapesAndSkipsWhenFollowingIsDisabled(t *testing.T) { + root := t.TempDir() + configurationRoot := filepath.Join(root, "workspace", "config") + selected := filepath.Join(configurationRoot, "selected") + if err := os.MkdirAll(selected, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../../../outside", filepath.Join(selected, "escape")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + base := os.DirFS(root) + if _, ok := base.(fs.ReadLinkFS); !ok { + t.Skip("host directory filesystem does not expose link inspection") + } + filesystem := newCountingReadLinkFS(base, 100) + access := newNativeAccess(filesystem, "workspace/config", &namespaceToken{}) + if _, err := resolveNativeImportPath(access, filesystem.links, "workspace/config/selected/escape"); !errors.Is(err, errImportPathOutsideRoot) { + t.Fatalf("escaping native link error = %v, want outside-root rejection", err) + } + + filesystem.readLinks = 0 + _, diagnostics := selectDirectoryWithDiagnostics(t, access, "workspace/config", " - dir: selected\n follow: false\n") + if len(diagnostics) != 0 || filesystem.readLinks != 0 { + t.Fatalf("follow-disabled selection diagnostics=%#v readlink calls=%d", diagnostics, filesystem.readLinks) + } +} + +type countingReadLinkFS struct { + filesystem fs.FS + links fs.ReadLinkFS + operations int + readLinks int + limit int +} + +func newCountingReadLinkFS(filesystem fs.FS, limit int) *countingReadLinkFS { + links, ok := filesystem.(fs.ReadLinkFS) + if !ok { + panic("test filesystem does not support link inspection") + } + return &countingReadLinkFS{filesystem: filesystem, links: links, limit: limit} +} + +func (f *countingReadLinkFS) Open(name string) (fs.File, error) { return f.filesystem.Open(name) } + +func (f *countingReadLinkFS) Stat(name string) (fs.FileInfo, error) { + if statter, ok := f.filesystem.(fs.StatFS); ok { + return statter.Stat(name) + } + return fs.Stat(f.filesystem, name) +} + +func (f *countingReadLinkFS) ReadDir(name string) ([]fs.DirEntry, error) { + if reader, ok := f.filesystem.(fs.ReadDirFS); ok { + return reader.ReadDir(name) + } + return fs.ReadDir(f.filesystem, name) +} + +func (f *countingReadLinkFS) count() error { + f.operations++ + if f.operations > f.limit { + return errors.New("native link operation limit exceeded") + } + return nil +} + +func (f *countingReadLinkFS) Lstat(name string) (fs.FileInfo, error) { + if err := f.count(); err != nil { + return nil, err + } + return f.links.Lstat(name) +} + +func (f *countingReadLinkFS) ReadLink(name string) (string, error) { + if err := f.count(); err != nil { + return "", err + } + f.readLinks++ + return f.links.ReadLink(name) +}