i/p/patterns: disallow /./ and /../ in path patterns (#14774)

* i/p/patterns: disallow /./ and /../ in path patterns

Since patterns like /foo/./bar don't match paths paths like /foo/bar,
throw an error if the client tries to reply or create a rule with such a
construction clearly in the pattern. That is, if the pattern contains
`/./` or `/../`, or if it ends with `/.` or `/..`.

Notably, we do this on the raw pattern string, similarly to how we
validate that the pattern begins with a `/`: we don't consider whether
all alts in a leading group happen to start with `/`, we simply throw an
error if the first character is not `/`.

In this case, we take a more lenient (but similarly lazy) approach: if
there's a literal `/./` or `/../`, or trailing `/.` or `/..`, then an
error is thrown. As long as there's something interrupting the `/` or
end of the pattern string (e.g. a group), we consider that fine. For
example, we allow `/foo/.{,bar}` even though technically this renders to
the variants `/foo/.` and `/foo/.bar`, where the former is undesirable.

We may reconsider this in the future, but checking whether any rendered
variant contains `/./` or `/../` in general requires fully rendering
each variant and checking each one, which we do not do at parse time,
and one nearly always has at least one pattern which is capable of
matching something valid (an exception being the pattern `/foo/.{,}`,
for example). But the user must fairly deliberately shoot themself in
the foot to end up in that situation.

Again, the worst case if a pattern which is "bad" in this way gets past
validation is that we end up with a path pattern which is incapable of
matching any paths. This is undesirable, but not problematic.

Signed-off-by: Oliver Calder <[email protected]>

* i/p/requestprompts: clarify comment for the relative path regexp

Co-authored-by: Zeyad Yasser <[email protected]>

---------

Signed-off-by: Oliver Calder <[email protected]>
Co-authored-by: Zeyad Yasser <[email protected]>

interfaces/prompting/patterns/patterns_test.go

@@ -133,6 +133,12 @@ func (s *patternsSuite) TestParsePathPatternHappy(c *C) {
 		"/foo/{a,b}{c,d}{e,f}{g,h,i,j,k}{l,m,n,o,p}{q,r,s,t,u},1,2,3", // expands to 1000, with commas outside groups
 		"/" + strings.Repeat("{a,", 999) + "a" + strings.Repeat("}", 999),
 		"/" + strings.Repeat("{", 999) + "a" + strings.Repeat(",a}", 999),
+		"/foo/.../bar",
+		"/foo/...",
+		"/foo/.{bar,baz}",
+		"/foo/..{bar,baz}",
+		"/foo/{bar,baz}.",
+		"/foo/{bar,baz}..",
 	} {
 		_, err := patterns.ParsePathPattern(pattern)
 		c.Check(err, IsNil, Commentf("valid path pattern %q was incorrectly not allowed", pattern))
@@ -152,6 +158,22 @@ func (s *patternsSuite) TestParsePathPatternUnhappy(c *C) {
 			`file.txt`,
 			`invalid path pattern: pattern must start with '/': "file.txt"`,
 		},
+		{
+			`/foo/./bar`,
+			`invalid path pattern: pattern cannot contain '/./' or '/../': .*`,
+		},
+		{
+			`/foo/../bar`,
+			`invalid path pattern: pattern cannot contain '/./' or '/../': .*`,
+		},
+		{
+			`/foo/.`,
+			`invalid path pattern: pattern cannot contain '/./' or '/../': .*`,
+		},
+		{
+			`/foo/..`,
+			`invalid path pattern: pattern cannot contain '/./' or '/../': .*`,
+		},
 		{
 			`{/,/foo}`,
 			`invalid path pattern: pattern must start with '/': .*`,

interfaces/prompting/patterns/scan.go

@@ -23,6 +23,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"regexp"
 	"strings"
 )
 
@@ -59,13 +60,19 @@ type token struct {
 	text  string
 }
 
+// relpathFinder matches `/./` and `/../` along with their trailing variants `/.` and `/..` in path patterns.
+var relpathFinder = regexp.MustCompile(`/\.(\.)?(/|$)`)
+
 func scan(text string) (tokens []token, err error) {
 	if len(text) == 0 {
 		return nil, errors.New("pattern has length 0")
 	}
 	if text[0] != '/' {
 		return nil, errors.New("pattern must start with '/'")
 	}
+	if relpathFinder.MatchString(text) {
+		return nil, errors.New("pattern cannot contain '/./' or '/../'")
+	}
 
 	var runes []rune
 

interfaces/prompting/patterns/scan_internal_test.go

@@ -123,6 +123,22 @@ func (s *scanSuite) TestScanUnhappy(c *C) {
 			`foo`,
 			`pattern must start with '/'`,
 		},
+		{
+			`/foo/./bar`,
+			`pattern cannot contain '/./' or '/../'`,
+		},
+		{
+			`/foo/../bar`,
+			`pattern cannot contain '/./' or '/../'`,
+		},
+		{
+			`/foo/.`,
+			`pattern cannot contain '/./' or '/../'`,
+		},
+		{
+			`/foo/..`,
+			`pattern cannot contain '/./' or '/../'`,
+		},
 		{
 			`/foo\`,
 			`trailing unescaped '\\' character`,