From 1dfa774164b74b4816bb38f4030e9feb74670eff Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 7 Oct 2026 20:29:16 +0000 Subject: [PATCH] Resolve absolute destinations inside the candidate checkout --- docs/internal/validation.md | 5 ++ docs/roadmap/remediation.md | 9 +++- internal/validation/destination_test.go | 64 +++++++++++++++++++++++++ internal/validation/tree.go | 48 +++++++++++++++---- 4 files changed, 117 insertions(+), 9 deletions(-) diff --git a/docs/internal/validation.md b/docs/internal/validation.md index d92188e..f73c6c7 100644 --- a/docs/internal/validation.md +++ b/docs/internal/validation.md @@ -86,6 +86,11 @@ directories, missing paths, checkout boundaries, submodule paths, and symlink cycles; the caller selects which observations fail validation or are reported as out of scope. +Resolution walks committed components and symlink targets in order. Active +symlink expansion detects cycles, and traversal honors cancellation. Absolute +destinations and symlink targets within the repository directory map back to +candidate-relative paths; existence still comes from committed entries. + The Markdown capability will use PromptKit's current parser and anchor conventions as its starting evidence. Supported link syntax, anchors, diagnostics, and boundary policy belong to that feature's roadmap and diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index e555c19..9b380e8 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Shared validation foundation remediation -Status: Remediation in progress; findings 1–4 addressed, findings 5–6 pending. +Status: Remediation in progress; findings 1–5 addressed, finding 6 pending. Reviewed committed implementation `f53354b`, covering the eight foundation commits after `0c7129a`, against the [implementation plan](implementation.md), @@ -175,6 +175,13 @@ complete message wording as a contract. ## 5. P2: Absolute paths inside the checkout are classified as outside +**Resolution:** Addressed. Absolute destinations and symlink targets within the +captured repository directory map to candidate paths before component +traversal. Absolute paths with similar external prefixes remain outside. +Regression tests also retain symlink-before-parent semantics and demonstrate +that removed committed files resolve while live-only files remain missing. +`go test ./internal/validation` passed. + **Owner:** [candidate destination resolver](../../internal/validation/tree.go), the absolute-destination and absolute-symlink branches. diff --git a/internal/validation/destination_test.go b/internal/validation/destination_test.go index 6b14514..39ef147 100644 --- a/internal/validation/destination_test.go +++ b/internal/validation/destination_test.go @@ -230,3 +230,67 @@ func TestResolveDestinationPreservesParentTraversalOrder(t *testing.T) { t.Fatalf("neighbor changed resolution: %#v -> %#v, %v", before, after, err) } } + +func TestResolveDestinationMapsAbsolutePathsIntoCandidate(t *testing.T) { + repository := newTreeRepository(t) + for _, name := range []string{"README.md", "docs/file.md", "nested/file.md", "nested/inner/item.txt"} { + writeTreeFile(t, repository, name, []byte("captured\n")) + } + for name, target := range map[string]string{ + "absolute-file": filepath.ToSlash(filepath.Join(repository, "README.md")), + "absolute-directory": filepath.ToSlash(filepath.Join(repository, "nested", "inner")), + "absolute-outside": filepath.ToSlash(repository + "-neighbor/README.md"), + "docs/link": "../nested/inner", + "absolute-parent": filepath.ToSlash(repository) + "/docs/link/../file.md", + } { + if err := os.Symlink(target, filepath.Join(repository, filepath.FromSlash(name))); err != nil { + t.Fatal(err) + } + } + gitTree(t, repository, "add", "--all") + gitTree(t, repository, "commit", "-m", "capture absolute links") + tree, err := ReadCandidateTree(t.Context(), repository, strings.TrimSpace(gitTree(t, repository, "rev-parse", "HEAD"))) + if err != nil { + t.Fatal(err) + } + if err := os.Remove(filepath.Join(repository, "README.md")); err != nil { + t.Fatal(err) + } + writeTreeFile(t, repository, "live-only.md", []byte("not committed")) + neighbor := repository + "-neighbor" + if err := os.MkdirAll(neighbor, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(neighbor, "README.md"), []byte("outside"), 0o644); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := os.RemoveAll(neighbor); err != nil { + t.Error(err) + } + }) + root := filepath.ToSlash(repository) + for _, test := range []struct { + destination string + kind DestinationKind + resolved string + }{ + {root + "/README.md", DestinationEntry, "README.md"}, + {root, DestinationDirectory, "."}, + {root + "/nested/inner", DestinationDirectory, "nested/inner"}, + {"absolute-file", DestinationEntry, "README.md"}, + {"absolute-directory/item.txt", DestinationEntry, "nested/inner/item.txt"}, + {root + "/docs/link/../file.md", DestinationEntry, "nested/file.md"}, + {"absolute-parent", DestinationEntry, "nested/file.md"}, + {root + "/live-only.md", DestinationMissing, "live-only.md"}, + {root + "-neighbor/README.md", DestinationOutsideCheckout, root + "-neighbor/README.md"}, + {"absolute-outside", DestinationOutsideCheckout, root + "-neighbor/README.md"}, + } { + t.Run(test.destination, func(t *testing.T) { + got, err := tree.ResolveDestination(t.Context(), "README.md", test.destination) + if err != nil || got.Kind != test.kind || got.Path != test.resolved { + t.Fatalf("resolution = %#v, %v; want %s %q", got, err, test.kind, test.resolved) + } + }) + } +} diff --git a/internal/validation/tree.go b/internal/validation/tree.go index 949facd..0fe791c 100644 --- a/internal/validation/tree.go +++ b/internal/validation/tree.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path" + "path/filepath" "strings" "gitea.maximumdirect.net/eric/ReleaseKit/internal/git" @@ -97,16 +98,25 @@ type Destination struct { Path string } -// ResolveDestination resolves a repository-relative destination from the -// source file's directory using only the captured Git tree. +// ResolveDestination resolves local destinations using the captured Git tree. +// Relative destinations start at the source directory; absolute destinations +// within the repository directory map back to committed paths. func (tree *CandidateTree) ResolveDestination(ctx context.Context, sourcePath, destination string) (Destination, error) { if err := ctx.Err(); err != nil { return Destination{}, err } - if strings.HasPrefix(destination, "/") { - return Destination{Kind: DestinationOutsideCheckout, Path: path.Clean(destination)}, nil + directory := path.Dir(sourcePath) + if path.IsAbs(destination) { + relative, inside, err := tree.relativeAbsoluteDestination(destination) + if err != nil { + return Destination{}, err + } + if !inside { + return Destination{Kind: DestinationOutsideCheckout, Path: destination}, nil + } + directory, destination = ".", relative } - return tree.resolveDestination(ctx, path.Dir(sourcePath), strings.Split(destination, "/"), make(map[string]bool)) + return tree.resolveDestination(ctx, directory, strings.Split(destination, "/"), make(map[string]bool)) } func (tree *CandidateTree) resolveDestination(ctx context.Context, directory string, components []string, active map[string]bool) (Destination, error) { @@ -146,11 +156,19 @@ func (tree *CandidateTree) resolveDestination(ctx context.Context, directory str return Destination{}, fmt.Errorf("read committed symlink %q: %w", partial, err) } target := string(linkText) - if strings.HasPrefix(target, "/") { - return Destination{Kind: DestinationOutsideCheckout, Path: unresolvedPath(target, remaining)}, nil + targetDirectory := directory + if path.IsAbs(target) { + relative, inside, err := tree.relativeAbsoluteDestination(target) + if err != nil { + return Destination{}, err + } + if !inside { + return Destination{Kind: DestinationOutsideCheckout, Path: unresolvedPath(target, remaining)}, nil + } + targetDirectory, target = ".", relative } active[partial] = true - destination, err := tree.resolveDestination(ctx, directory, strings.Split(target, "/"), active) + destination, err := tree.resolveDestination(ctx, targetDirectory, strings.Split(target, "/"), active) delete(active, partial) if err != nil || len(remaining) == 0 { return destination, err @@ -174,6 +192,20 @@ func (tree *CandidateTree) resolveDestination(ctx context.Context, directory str return Destination{Kind: DestinationDirectory, Path: directory}, nil } +func (tree *CandidateTree) relativeAbsoluteDestination(destination string) (string, bool, error) { + root, err := filepath.Abs(tree.repositoryDir) + if err != nil { + return "", false, fmt.Errorf("resolve candidate repository directory: %w", err) + } + root = filepath.ToSlash(root) + if destination == root { + return ".", true, nil + } + // Preserve the suffix for component traversal, including symlinks before '..'. + relative, inside := strings.CutPrefix(destination, strings.TrimSuffix(root, "/")+"/") + return relative, inside, nil +} + // unresolvedPath preserves components that could not be traversed. Cleaning // them would hide missing entries, non-directories, or checkout boundaries. func unresolvedPath(base string, remaining []string) string {