diff options
| -rw-r--r-- | internal/plan/chain.go | 50 | ||||
| -rw-r--r-- | internal/plan/fuzz_test.go | 18 | ||||
| -rw-r--r-- | internal/plan/hostile_test.go | 38 |
3 files changed, 91 insertions, 15 deletions
diff --git a/internal/plan/chain.go b/internal/plan/chain.go index fda8730..d4d07da 100644 --- a/internal/plan/chain.go +++ b/internal/plan/chain.go @@ -212,29 +212,51 @@ func stepKind(k config.ActionKind) Kind { // 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. +// A placeholder may not take the destination out of the directory the +// rule's own text names before it (spec §15.1): a capture or {ext} is part +// of a file name, which can be "..", "~" or empty, and ResolveDir decides +// from the expanded text whether a destination is relative, in the home +// directory or absolute. So the resolved destination must lie at or under +// staticDir of the raw text. A destination with no placeholder is written +// entirely by the rule, and stands as written. 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) + resolved := ResolveDir(expanded, root) + if strings.ContainsRune(raw, '{') { + if base := staticDir(raw, root); !within(resolved, base) { + return "", fmt.Errorf("destination %q leaves %s through a placeholder", expanded, xdg.Abbrev(base)) + } } - return ResolveDir(expanded, root), nil + return resolved, 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++ - } +// staticDir is the directory a destination names before its first +// placeholder: its last complete path segment, resolved like any +// destination. "Work/Acme/{mtime:%Y}" is root/Work/Acme, "Work/Ac{1}" is +// root/Work, "{1}/x" is root itself, "~/{1}" is the home directory and +// "/{1}" is "/". +func staticDir(raw, root string) string { + prefix := raw + if i := strings.IndexByte(raw, '{'); i >= 0 { + prefix = raw[:i] + } + switch i := strings.LastIndexByte(prefix, '/'); { + case i < 0: + prefix = "" + case i == 0: + prefix = "/" + default: + prefix = prefix[:i] } - return n + return ResolveDir(prefix, root) +} + +// within reports whether path is dir itself or lies under it. +func within(path, dir string) bool { + return path == dir || dir == string(filepath.Separator) || strings.HasPrefix(path, dir+string(filepath.Separator)) } // ResolveDir expands a leading ~ and joins a relative directory to root, diff --git a/internal/plan/fuzz_test.go b/internal/plan/fuzz_test.go index fb68f94..6deb225 100644 --- a/internal/plan/fuzz_test.go +++ b/internal/plan/fuzz_test.go @@ -3,13 +3,21 @@ package plan import ( + "path/filepath" + "strings" "testing" "time" + + "krino/internal/config" + "krino/internal/scan" ) // FuzzExpand: expanding any template against any file name never panics, // gives the same answer twice, and never succeeds with a {N} beyond the // capture groups there are - MaxIndex and Expand read placeholders alike. +// Used as a destination - relative, in the home directory, or bare - a +// template never plans a move outside the directory its text names before +// the first placeholder (review M1). func FuzzExpand(f *testing.F) { for _, s := range []string{"{name}", "{stem}{ext}", "{mtime:%Y/%m}", "{1}_{2}", "{{literal}}", "{", "}", "{now:%", "{0}", "{9}", "{99999999999999999999}"} { f.Add(s, "a.b.pdf") @@ -29,5 +37,15 @@ func FuzzExpand(f *testing.F) { if n, err := MaxIndex(tmpl); err == nil && n > 2 && errA == nil { t.Fatalf("Expand(%q) = %q, though it uses {%d} and only 2 groups exist", tmpl, a, n) } + for _, dest := range []string{tmpl, "Out/" + tmpl, "~/docs/" + tmpl} { + in := []Input{{ + File: scan.File{Path: "/r/x.pdf", Rel: "x.pdf", Name: "x.pdf", ModTime: facts.ModTime}, + Rules: []RuleMatch{{Name: "a", Captures: facts.Captures, Actions: []config.Action{{Kind: config.Move, Arg: dest}}}}, + }} + s := Build("/r", in, facts.Now, NoDisk{}, NewClaims())[0].Steps[0] + if strings.ContainsRune(dest, '{') && s.Skip == "" && !within(filepath.Dir(s.Dst), staticDir(dest, "/r")) { + t.Fatalf("destination %q planned %q, outside %q", dest, s.Dst, staticDir(dest, "/r")) + } + } }) } diff --git a/internal/plan/hostile_test.go b/internal/plan/hostile_test.go index c22fb24..a2f2bfe 100644 --- a/internal/plan/hostile_test.go +++ b/internal/plan/hostile_test.go @@ -39,7 +39,7 @@ func TestBuildRefusesDotDotFromPlaceholder(t *testing.T) { }} 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` { + if s := step("/w/{1}/in"); s.Skip != `destination "/w/../in" leaves /w 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" { @@ -49,3 +49,39 @@ func TestBuildRefusesDotDotFromPlaceholder(t *testing.T) { t.Errorf("..x: Skip = %q, Dst = %q", s.Skip, s.Dst) } } + +// TestBuildKeepsPlaceholdersInsideTheirDirectory: a capture of "~", an +// empty capture, or an extension-less {ext} at the start of a destination +// used to make it $HOME or absolute (review M1). The resolved destination +// must stay under the directory the rule's text names before its first +// placeholder; a destination with no placeholder is the rule's own +// business. +func TestBuildKeepsPlaceholdersInsideTheirDirectory(t *testing.T) { + t.Setenv("HOME", "/home/x") + step := func(name, dest string, caps ...string) Step { + in := []Input{{File: file("/r", name), Rules: []RuleMatch{{Name: "a", Captures: caps, Actions: []config.Action{act(config.Move, dest)}}}}} + return Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0].Steps[0] + } + for _, tc := range []struct { + name, dest string + caps []string + skip bool + dst string + }{ + {"~_i.pdf", "{1}/Filed", []string{"~_", "~"}, true, ""}, + {"_i.pdf", "{1}/Filed", []string{"_", ""}, true, ""}, + {"README", "{ext}/tmp", nil, true, ""}, + {"a.pdf", "Out/{1}", []string{"a", "~"}, false, "/r/Out/~/a.pdf"}, + {"a.pdf", "~/docs/{1}", []string{"a", "x"}, false, "/home/x/docs/x/a.pdf"}, + {"a.pdf", "~/docs/{1}", []string{"a", ".."}, true, ""}, + {"a.pdf", "Work/Ac{1}", []string{"a", "me"}, false, "/r/Work/Acme/a.pdf"}, + {"a.pdf", "/w/{1}/in", []string{"a", ".."}, true, ""}, + {"a.pdf", "/w/../in", nil, false, "/in/a.pdf"}, + {"a.pdf", "{mtime:%Y}", nil, false, "/r/2026/a.pdf"}, + } { + s := step(tc.name, tc.dest, tc.caps...) + if tc.skip != (s.Skip != "") || (!tc.skip && s.Dst != tc.dst) { + t.Errorf("%s -> %s %q: Skip %q Dst %q; want skip=%v dst=%q", tc.name, tc.dest, tc.caps, s.Skip, s.Dst, tc.skip, tc.dst) + } + } +} |