plan 8: placeholders cannot rename to dot names or climb out with ..

2 files changed, 73 insertions, 0 deletions

diff --git a/internal/plan/chain.go b/internal/plan/chain.go
index e504504..fda8730 100644
--- a/internal/plan/chain.go
+++ b/internal/plan/chain.go
@@ -159,6 +159,10 @@ func buildOne(root string, in Input, now time.Time, d Disk, claim claimed) Chain
 					step.Skip = `rename produced a name containing "/"`
 					break
 				}
+				if name == "" || name == "." || name == ".." {
+					step.Skip = fmt.Sprintf("rename produced the name %q", name)
+					break
+				}
 				dst := filepath.Join(filepath.Dir(cur), name)
 				resolved, skip, displaces := resolveConflict(kind, rule.Settings.OnConflict, cur, dst, d, claim)
 				step.Dst = resolved
@@ -207,14 +211,32 @@ func stepKind(k config.ActionKind) Kind {
 // expandDir expands raw (a DEST argument) against facts, then resolves it
 // the same way the engine resolves an extra directory: ~ expands, a
 // relative path joins root, and the result is cleaned.
+//
+// A placeholder may not add a ".." segment (spec §15.1): a capture is part
+// of a file name, which can be "..", and must not move the destination out
+// of the directory the rule names. A ".." written in the rule itself stands.
 func expandDir(raw string, facts Facts, root string) (string, error) {
 	expanded, err := Expand(raw, facts)
 	if err != nil {
 		return "", err
 	}
+	if dotDots(expanded) > dotDots(raw) {
+		return "", fmt.Errorf("destination %q leaves its directory through a placeholder", expanded)
+	}
 	return ResolveDir(expanded, root), nil
 }
 
+// dotDots counts the ".." segments of a slash-separated path.
+func dotDots(p string) int {
+	n := 0
+	for _, seg := range strings.Split(p, "/") {
+		if seg == ".." {
+			n++
+		}
+	}
+	return n
+}
+
 // ResolveDir expands a leading ~ and joins a relative directory to root,
 // cleaned. C1: this is the one place that decides where a rule's
 // destination resolves to; internal/engine calls it too (its own directory
diff --git a/internal/plan/hostile_test.go b/internal/plan/hostile_test.go
new file mode 100644
index 0000000..c22fb24
--- /dev/null
+++ b/internal/plan/hostile_test.go
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package plan
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"krino/internal/config"
+)
+
+// TestBuildRefusesDotNames: a rename whose placeholders produce "", "." or
+// ".." names a directory, not a file; the step is skipped with a reason
+// instead of renaming a file onto its own folder or the one above.
+func TestBuildRefusesDotNames(t *testing.T) {
+	for _, capture := range []string{"", ".", ".."} {
+		in := []Input{{
+			File:  file("/r", "x.pdf"),
+			Rules: []RuleMatch{{Name: "a", Captures: []string{"x.pdf", capture}, Actions: []config.Action{act(config.Rename, "{1}")}}},
+		}}
+		c := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0]
+		if want := fmt.Sprintf("rename produced the name %q", capture); c.Steps[0].Skip != want {
+			t.Errorf("capture %q: Skip = %q, want %q", capture, c.Steps[0].Skip, want)
+		}
+	}
+}
+
+// TestBuildRefusesDotDotFromPlaceholder: a file name can capture "..",
+// though never "/", so a destination whose placeholders add a ".." segment
+// would leave the directory the rule names: that step is skipped. A ".."
+// the rule wrote itself is the rule's own business, and "..x" is a name,
+// not a segment.
+func TestBuildRefusesDotDotFromPlaceholder(t *testing.T) {
+	step := func(dest string) Step {
+		in := []Input{{
+			File:  file("/r", "x.pdf"),
+			Rules: []RuleMatch{{Name: "a", Captures: []string{"x.pdf", ".."}, Actions: []config.Action{act(config.Move, dest)}}},
+		}}
+		return Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0].Steps[0]
+	}
+	if s := step("/w/{1}/in"); s.Skip != `destination "/w/../in" leaves its directory through a placeholder` {
+		t.Errorf("placeholder ..: Skip = %q, Dst = %q", s.Skip, s.Dst)
+	}
+	if s := step("/w/../in"); s.Skip != "" || s.Dst != "/in/x.pdf" {
+		t.Errorf("written ..: Skip = %q, Dst = %q", s.Skip, s.Dst)
+	}
+	if s := step("/w/{1}x"); s.Skip != "" || s.Dst != "/w/..x/x.pdf" {
+		t.Errorf("..x: Skip = %q, Dst = %q", s.Skip, s.Dst)
+	}
+}