diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 39c5a28..9aae251 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Import Integration Correctness Audit -Status: open; three findings require correction. +Status: in progress; finding 1 complete, findings 2 and 3 remain. Reviewed tree: `91db99b` (including the six completed fixes through `5747ec4`). Scope: import discovery/defaults and shared dependency-ready scheduling, including @@ -10,11 +10,11 @@ resource contexts, and maintained consumer examples. The completed design and implementation plan remain available at `5747ec4:docs/roadmap/imports.md` and `5747ec4:docs/roadmap/implementation.md`. -This review changes no production code or existing contracts. Preserve the +The audit made no production code or contract changes. Preserve the accepted syntax, defaults, scheduling, containment, and namespace decisions when addressing these findings. -## 1. [ ] P2 — Accept contained absolute symlinks in directory-root prefixes +## 1. [x] P2 — Accept contained absolute symlinks in directory-root prefixes Owners: [`importEntryTarget`](../../internal/engine/filesystem_links.go), especially the initial host `a.root.Lstat(relative)` around line 30, and @@ -53,6 +53,16 @@ solve this by prohibiting absolute symlinks or relaxing root containment. Add a focused public regression for a directory root below an absolute contained link; reuse existing escaping-link and cycle coverage. +Completed: host entry inspection resolves intermediate parent components through +the existing containment-aware resolver before inspecting the final entry. Native +handling and the final entry's `follow` policy are unchanged. Added a public +regression for directory roots below an absolute contained link with both +`follow` settings, including canonical source identity. Confirmed that the test +fails before the fix and passes afterward. Focused facade and engine directory, +glob, symlink, and cycle tests passed, as did `./scripts/check.sh`, +`go test -race -count=1 ./...`, and `git diff --check`. The original public +reproduction now succeeds for all selectors. + ## 2. [ ] P2 — Describe scheduled provenance and history order consistently Owner: [configuration contract](../config.md), especially layered metadata diff --git a/facade_import_root_test.go b/facade_import_root_test.go index c605c5a..34622bc 100644 --- a/facade_import_root_test.go +++ b/facade_import_root_test.go @@ -2,6 +2,7 @@ package configkit_test import ( "errors" + "fmt" "os" "path/filepath" "testing" @@ -9,6 +10,43 @@ import ( configkit "gitea.maximumdirect.net/eric/ConfigKit" ) +func TestFacadeDirectoryRootBelowContainedAbsoluteSymlink(t *testing.T) { + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + parts := filepath.Join(root, "real", "parts") + if err := os.MkdirAll(parts, 0o700); err != nil { + t.Fatal(err) + } + partPath := filepath.Join(parts, "part.yml") + if err := os.WriteFile(partPath, []byte("value: included\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(root, "real"), filepath.Join(root, "absolute")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + for _, follow := range []bool{true, false} { + t.Run(fmt.Sprintf("follow=%t", follow), func(t *testing.T) { + mainPath := filepath.Join(root, "main.yml") + contents := fmt.Sprintf("configkit:\n imports:\n - dir: absolute/parts\n pattern: '*.yml'\n follow: %t\n", follow) + if err := os.WriteFile(mainPath, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } + result, err := configkit.Load[map[string]any](configkit.File(mainPath), configkit.Options[map[string]any]{Imports: true}) + if err != nil { + t.Fatal(err) + } + if result.Config["value"] != "included" { + t.Fatalf("configuration = %#v", result.Config) + } + if sources := result.Sources(); len(sources) != 2 || sources[1].Path != partPath { + t.Fatalf("canonical imported source = %#v", sources) + } + }) + } +} + func TestFacadeNativeDiscoveryWithNestedRoot(t *testing.T) { root := t.TempDir() parts := filepath.Join(root, "nested/root/parts") diff --git a/internal/engine/filesystem_links.go b/internal/engine/filesystem_links.go index 05aa712..d25733d 100644 --- a/internal/engine/filesystem_links.go +++ b/internal/engine/filesystem_links.go @@ -27,6 +27,16 @@ func (a *filesystemAccess) importEntryTarget(name string, entry fs.DirEntry, fol if err != nil { return importEntryTarget{}, err } + // Root.Lstat cannot follow absolute links in intermediate components, + // even when contained. Resolve parents first, leaving the final entry + // unexpanded so its discovered-link follow policy still applies. + if parent := filepath.Dir(relative); parent != "." { + resolvedParent, err := resolveHostImportPath(a, parent) + if err != nil { + return importEntryTarget{}, err + } + relative = filepath.Join(filepath.FromSlash(resolvedParent), filepath.Base(relative)) + } info, err := a.root.Lstat(relative) if err != nil { return importEntryTarget{}, err