From 0f9ecf12ed7b7a911ab9585394a8e5cde1561ee6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 25 Oct 2026 21:45:33 +0000 Subject: [PATCH] Share link dependency evaluation with host access --- docs/roadmap/implementation.md | 6 ++ internal/engine/filesystem_links.go | 78 +++++--------- internal/engine/filesystem_links_test.go | 123 +++++++++++++++++++++++ 3 files changed, 156 insertions(+), 51 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index f17af3c..6451675 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -294,6 +294,12 @@ Focused check: `go test ./internal/engine -run 'Link|Directories|Glob'`. Finding: remaining host part of **C02**. Owners: `internal/engine/filesystem_links.go` and the same link/discovery tests. +Status: **complete** (C02). Prior completed commits: `2736ddf`, `deb9aa5`, +`ae3408a`, `ea27f17`, `08a8c25`, `877c497`, `8aba43d`, `7bf2f0c`. +Validation passed: `env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Link|Directory|Glob|Filesystem|Discovery' -count=1`, +`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`, +and `git diff --check`. + Bind rooted host `Lstat`/`Readlink` operations to Stage 8's evaluator and retire the duplicate host frame loop. Retain portable component evaluation, rooted access, host-relative targets, and contained absolute targets translated with `filepath`. diff --git a/internal/engine/filesystem_links.go b/internal/engine/filesystem_links.go index 1a43c13..6730631 100644 --- a/internal/engine/filesystem_links.go +++ b/internal/engine/filesystem_links.go @@ -119,8 +119,8 @@ func (a *filesystemAccess) importEntryTarget(name string, entry fs.DirEntry, fol return importEntryTarget{symlink: symlink, dir: info.IsDir(), identity: name}, nil } -// A link stays active until its target has resolved, not until the entire -// original pathname has resolved. Remaining segments may reuse a finished link. +// A link remains active until its own target resolves. Completed targets can be +// reused while the caller continues resolving its remaining components. type importLinkFrame struct { pending []string link string @@ -190,58 +190,34 @@ func resolveImportLinkPath(name string, operations importLinkOperations) (string } func resolveHostImportPath(access *filesystemAccess, name string) (string, error) { - frames := []importLinkFrame{{pending: strings.Split(filepath.ToSlash(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 = "." - } - info, err := access.root.Lstat(filepath.FromSlash(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 := access.root.Readlink(filepath.FromSlash(candidate)) - if err != nil { - return "", err - } - var targetPath string - if filepath.IsAbs(target) { - relative, err := filepath.Rel(access.rootPath, filepath.Clean(target)) - if err != nil { + return resolveImportLinkPath(filepath.ToSlash(name), hostImportLinkOperations(access)) +} + +func hostImportLinkOperations(access *filesystemAccess) importLinkOperations { + return importLinkOperations{ + lstat: func(candidate string) (fs.FileInfo, error) { + return access.root.Lstat(filepath.FromSlash(candidate)) + }, + readlink: func(candidate string) (string, error) { + return access.root.Readlink(filepath.FromSlash(candidate)) + }, + normalizeTarget: func(candidate, target string) (string, error) { + var targetPath string + if filepath.IsAbs(target) { + relative, err := filepath.Rel(access.rootPath, filepath.Clean(target)) + if err != nil { + return "", errImportPathOutsideRoot + } + targetPath = path.Clean(filepath.ToSlash(relative)) + } else { + targetPath = path.Clean(path.Join(path.Dir(candidate), filepath.ToSlash(target))) + } + if !withinPortableRoot(targetPath) { return "", errImportPathOutsideRoot } - targetPath = path.Clean(filepath.ToSlash(relative)) - } else { - targetPath = path.Clean(path.Join(path.Dir(candidate), filepath.ToSlash(target))) - } - if !withinPortableRoot(targetPath) { - return "", errImportPathOutsideRoot - } - frames = append(frames, importLinkFrame{pending: strings.Split(targetPath, "/"), link: candidate}) - resolved = "" + return targetPath, nil + }, } - if resolved == "" { - resolved = "." - } - return resolved, nil } func withinPortableRoot(name string) bool { diff --git a/internal/engine/filesystem_links_test.go b/internal/engine/filesystem_links_test.go index b4b63ef..a743af9 100644 --- a/internal/engine/filesystem_links_test.go +++ b/internal/engine/filesystem_links_test.go @@ -136,6 +136,129 @@ func TestNativeLinkResolutionRejectsEscapesAndSkipsWhenFollowingIsDisabled(t *te } } +func TestHostLinkResolutionMemoizesCompletedDependencies(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") + graph := filepath.Join(configurationRoot, "graph") + selected := filepath.Join(configurationRoot, "selected") + for _, directory := range []string{graph, 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(graph, name)); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + } + if err := os.WriteFile(filepath.Join(graph, "item.yml"), []byte("value: linked\n"), 0o600); err != nil { + t.Fatal(err) + } + aliasTarget := fmt.Sprintf("../graph/s%02d", linkCount-1) + if err := os.Symlink(aliasTarget, filepath.Join(selected, "entry")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + containedAbsolute := filepath.Join(graph, "s00") + if err := os.Symlink(containedAbsolute, filepath.Join(selected, "absolute")); err != nil { + t.Skipf("absolute symlinks unavailable: %v", err) + } + outside := t.TempDir() + if err := os.Symlink(outside, filepath.Join(selected, "escape")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + access, err := openHostAccess(root) + if err != nil { + t.Fatal(err) + } + defer access.close() + name := filepath.ToSlash(filepath.Join("workspace", "config", "selected", "entry")) + operations := hostImportLinkOperations(access) + var operationCount int + lstat := operations.lstat + readlink := operations.readlink + operations.lstat = func(candidate string) (fs.FileInfo, error) { + operationCount++ + if operationCount > linkCount*20+40 { + return nil, errors.New("host link operation limit exceeded") + } + return lstat(candidate) + } + operations.readlink = func(candidate string) (string, error) { + operationCount++ + if operationCount > linkCount*20+40 { + return "", errors.New("host link operation limit exceeded") + } + return readlink(candidate) + } + resolved, err := resolveImportLinkPath(name, operations) + if err != nil || resolved != filepath.ToSlash(filepath.Join("workspace", "config", "graph")) { + t.Fatalf("host repeated-link resolution = %q, %v", resolved, err) + } + if operationCount == 0 || operationCount > linkCount*20+40 { + t.Fatalf("host link operations = %d, want 1..%d", operationCount, linkCount*20+40) + } + + contained, err := resolveHostImportPath(access, filepath.Join("workspace", "config", "selected", "absolute")) + if err != nil || contained != filepath.ToSlash(filepath.Join("workspace", "config", "graph")) { + t.Fatalf("contained absolute link = %q, %v", contained, err) + } + if _, err := resolveHostImportPath(access, filepath.Join("workspace", "config", "selected", "escape")); !errors.Is(err, errImportPathOutsideRoot) { + t.Fatalf("escaping absolute link error = %v, want outside-root rejection", err) + } + + for _, selector := range []string{"directory", "glob"} { + t.Run(selector, func(t *testing.T) { + var diagnostics []Diagnostic + if selector == "directory" { + _, diagnostics = selectDirectoryWithDiagnostics(t, access, root, " - dir: workspace/config/selected/entry\n follow: true\n") + } else { + _, diagnostics = selectGlobForTestWithDiagnostics(t, access, root, " - glob: 'workspace/config/selected/entry/*.yml'\n follow: true\n") + } + if len(diagnostics) != 0 { + t.Fatalf("host selector diagnostics = %#v", diagnostics) + } + }) + } +} + +func TestHostLinkResolutionKeepsNamespacesIndependent(t *testing.T) { + for _, test := range []struct { + name string + target string + }{ + {name: "left", target: "left-target"}, + {name: "right", target: "right-target"}, + } { + t.Run(test.name, func(t *testing.T) { + root := t.TempDir() + if err := os.Mkdir(filepath.Join(root, test.target), 0o700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(test.target, filepath.Join(root, "alias")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + access, err := openHostAccess(root) + if err != nil { + t.Fatal(err) + } + defer access.close() + if got, err := resolveHostImportPath(access, "alias"); err != nil || got != test.target { + t.Fatalf("namespace link = %q, %v; want %q", got, err, test.target) + } + }) + } +} + type countingReadLinkFS struct { filesystem fs.FS links fs.ReadLinkFS