Resolve candidate paths in traversal order
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Shared validation foundation remediation
|
# Shared validation foundation remediation
|
||||||
|
|
||||||
Status: Remediation in progress; finding 1 addressed, findings 2–6 pending.
|
Status: Remediation in progress; findings 1–2 addressed, findings 3–6 pending.
|
||||||
|
|
||||||
Reviewed committed implementation `f53354b`, covering the eight foundation
|
Reviewed committed implementation `f53354b`, covering the eight foundation
|
||||||
commits after `0c7129a`, against the [implementation plan](implementation.md),
|
commits after `0c7129a`, against the [implementation plan](implementation.md),
|
||||||
@@ -63,6 +63,13 @@ and bounded even when testing the broken behavior.
|
|||||||
|
|
||||||
## 2. P2: Path cleanup changes symlink traversal and checkout classification
|
## 2. P2: Path cleanup changes symlink traversal and checkout classification
|
||||||
|
|
||||||
|
**Resolution:** Addressed. Destination components and symlink targets are
|
||||||
|
traversed in order, with parent traversal applied to resolved directories.
|
||||||
|
Missing entries and regular-file traversal remain observable. Regression tests
|
||||||
|
cover both destination and symlink-target parent components and confirm that
|
||||||
|
neighboring files do not change classification.
|
||||||
|
`go test ./internal/validation` passed.
|
||||||
|
|
||||||
**Owner:** [candidate destination resolver](../../internal/validation/tree.go),
|
**Owner:** [candidate destination resolver](../../internal/validation/tree.go),
|
||||||
initial destination construction and construction after symlink expansion.
|
initial destination construction and construction after symlink expansion.
|
||||||
|
|
||||||
|
|||||||
@@ -174,3 +174,59 @@ func TestResolveDestinationIgnoresNeighboringFiles(t *testing.T) {
|
|||||||
t.Fatalf("neighbor changed resolution: absent=%#v present=%#v", gotWithoutNeighbor, gotWithNeighbor)
|
t.Fatalf("neighbor changed resolution: absent=%#v present=%#v", gotWithoutNeighbor, gotWithNeighbor)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestResolveDestinationPreservesParentTraversalOrder(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{
|
||||||
|
"docs/link": "../nested/inner",
|
||||||
|
"docs/parent-link": "link/../file.md",
|
||||||
|
"escape": "../neighbor",
|
||||||
|
} {
|
||||||
|
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 parent traversal")
|
||||||
|
tree, err := ReadCandidateTree(t.Context(), repository, strings.TrimSpace(gitTree(t, repository, "rev-parse", "HEAD")))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
for _, test := range []struct {
|
||||||
|
destination string
|
||||||
|
kind DestinationKind
|
||||||
|
resolved string
|
||||||
|
}{
|
||||||
|
{"docs/link/../file.md", DestinationEntry, "nested/file.md"},
|
||||||
|
{"docs/parent-link", DestinationEntry, "nested/file.md"},
|
||||||
|
{"escape/../README.md", DestinationOutsideCheckout, ""},
|
||||||
|
{"missing/../README.md", DestinationMissing, ""},
|
||||||
|
{"README.md/../README.md", DestinationMissing, ""},
|
||||||
|
{"README.md/.", DestinationMissing, ""},
|
||||||
|
} {
|
||||||
|
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 || (test.resolved != "" && got.Path != test.resolved) {
|
||||||
|
t.Fatalf("resolution = %#v, %v; want %s %q", got, err, test.kind, test.resolved)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
before, err := tree.ResolveDestination(t.Context(), "README.md", "escape/../README.md")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
neighbor := filepath.Join(filepath.Dir(repository), "neighbor")
|
||||||
|
if err := os.MkdirAll(neighbor, 0o755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(neighbor, "file.md"), []byte("neighbor"), 0o644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
after, err := tree.ResolveDestination(t.Context(), "README.md", "escape/../README.md")
|
||||||
|
if err != nil || before != after {
|
||||||
|
t.Fatalf("neighbor changed resolution: %#v -> %#v, %v", before, after, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -106,34 +106,36 @@ func (tree *CandidateTree) ResolveDestination(ctx context.Context, sourcePath, d
|
|||||||
if strings.HasPrefix(destination, "/") {
|
if strings.HasPrefix(destination, "/") {
|
||||||
return Destination{Kind: DestinationOutsideCheckout, Path: path.Clean(destination)}, nil
|
return Destination{Kind: DestinationOutsideCheckout, Path: path.Clean(destination)}, nil
|
||||||
}
|
}
|
||||||
resolved := path.Clean(path.Join(path.Dir(sourcePath), destination))
|
return tree.resolveDestination(ctx, path.Dir(sourcePath), strings.Split(destination, "/"), make(map[string]bool))
|
||||||
return tree.resolveDestination(ctx, resolved, make(map[string]bool))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (tree *CandidateTree) resolveDestination(ctx context.Context, resolved string, active map[string]bool) (Destination, error) {
|
func (tree *CandidateTree) resolveDestination(ctx context.Context, directory string, components []string, active map[string]bool) (Destination, error) {
|
||||||
if err := ctx.Err(); err != nil {
|
|
||||||
return Destination{}, err
|
|
||||||
}
|
|
||||||
if outsideCheckout(resolved) {
|
|
||||||
return Destination{Kind: DestinationOutsideCheckout, Path: resolved}, nil
|
|
||||||
}
|
|
||||||
components := strings.Split(resolved, "/")
|
|
||||||
partial := ""
|
|
||||||
for index, component := range components {
|
for index, component := range components {
|
||||||
if err := ctx.Err(); err != nil {
|
if err := ctx.Err(); err != nil {
|
||||||
return Destination{}, err
|
return Destination{}, err
|
||||||
}
|
}
|
||||||
partial = path.Join(partial, component)
|
remaining := components[index+1:]
|
||||||
remaining := strings.Join(components[index+1:], "/")
|
switch component {
|
||||||
|
case "", ".":
|
||||||
|
continue
|
||||||
|
case "..":
|
||||||
|
if directory == "." {
|
||||||
|
return Destination{Kind: DestinationOutsideCheckout, Path: unresolvedPath("..", remaining)}, nil
|
||||||
|
}
|
||||||
|
directory = path.Dir(directory)
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
partial := path.Join(directory, component)
|
||||||
entry, exists := tree.Entry(partial)
|
entry, exists := tree.Entry(partial)
|
||||||
if !exists {
|
if !exists {
|
||||||
if _, directory := tree.directories[partial]; !directory {
|
if _, isDirectory := tree.directories[partial]; !isDirectory {
|
||||||
return Destination{Kind: DestinationMissing, Path: resolved}, nil
|
return Destination{Kind: DestinationMissing, Path: unresolvedPath(partial, remaining)}, nil
|
||||||
}
|
}
|
||||||
|
directory = partial
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if entry.Mode == "160000" {
|
if entry.Mode == "160000" {
|
||||||
return Destination{Kind: DestinationSubmodule, Path: path.Join(partial, remaining)}, nil
|
return Destination{Kind: DestinationSubmodule, Path: unresolvedPath(partial, remaining)}, nil
|
||||||
}
|
}
|
||||||
if entry.Mode == "120000" {
|
if entry.Mode == "120000" {
|
||||||
if active[partial] {
|
if active[partial] {
|
||||||
@@ -145,30 +147,38 @@ func (tree *CandidateTree) resolveDestination(ctx context.Context, resolved stri
|
|||||||
}
|
}
|
||||||
target := string(linkText)
|
target := string(linkText)
|
||||||
if strings.HasPrefix(target, "/") {
|
if strings.HasPrefix(target, "/") {
|
||||||
return Destination{Kind: DestinationOutsideCheckout, Path: target}, nil
|
return Destination{Kind: DestinationOutsideCheckout, Path: unresolvedPath(target, remaining)}, nil
|
||||||
}
|
}
|
||||||
active[partial] = true
|
active[partial] = true
|
||||||
destination, err := tree.resolveDestination(ctx, path.Join(path.Dir(partial), target), active)
|
destination, err := tree.resolveDestination(ctx, directory, strings.Split(target, "/"), active)
|
||||||
delete(active, partial)
|
delete(active, partial)
|
||||||
if err != nil || remaining == "" || destination.Kind != DestinationDirectory {
|
if err != nil || len(remaining) == 0 {
|
||||||
if err == nil && remaining != "" && destination.Kind == DestinationEntry {
|
|
||||||
return Destination{Kind: DestinationMissing, Path: path.Join(destination.Path, remaining)}, nil
|
|
||||||
}
|
|
||||||
return destination, err
|
return destination, err
|
||||||
}
|
}
|
||||||
return tree.resolveDestination(ctx, path.Join(destination.Path, remaining), active)
|
switch destination.Kind {
|
||||||
|
case DestinationDirectory:
|
||||||
|
return tree.resolveDestination(ctx, destination.Path, remaining, active)
|
||||||
|
case DestinationEntry:
|
||||||
|
destination.Kind = DestinationMissing
|
||||||
|
case DestinationSymlinkCycle:
|
||||||
|
return destination, nil
|
||||||
|
}
|
||||||
|
destination.Path = unresolvedPath(destination.Path, remaining)
|
||||||
|
return destination, nil
|
||||||
}
|
}
|
||||||
if remaining != "" {
|
if len(remaining) != 0 {
|
||||||
return Destination{Kind: DestinationMissing, Path: resolved}, nil
|
return Destination{Kind: DestinationMissing, Path: unresolvedPath(partial, remaining)}, nil
|
||||||
}
|
}
|
||||||
return Destination{Kind: DestinationEntry, Path: partial}, nil
|
return Destination{Kind: DestinationEntry, Path: partial}, nil
|
||||||
}
|
}
|
||||||
if _, directory := tree.directories[resolved]; directory {
|
return Destination{Kind: DestinationDirectory, Path: directory}, nil
|
||||||
return Destination{Kind: DestinationDirectory, Path: resolved}, nil
|
|
||||||
}
|
|
||||||
return Destination{Kind: DestinationMissing, Path: resolved}, nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func outsideCheckout(name string) bool {
|
// unresolvedPath preserves components that could not be traversed. Cleaning
|
||||||
return name == ".." || strings.HasPrefix(name, "../")
|
// them would hide missing entries, non-directories, or checkout boundaries.
|
||||||
|
func unresolvedPath(base string, remaining []string) string {
|
||||||
|
if len(remaining) == 0 {
|
||||||
|
return base
|
||||||
|
}
|
||||||
|
return base + "/" + strings.Join(remaining, "/")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user