interfaces/prompting/patterns: add checks for duplicate pattern variants

interfaces/prompting/patterns/render.go

@@ -80,6 +80,7 @@
 		if err != nil {
 			// This should never occur
 			logger.Noticef("patterns: cannot parse pattern variant '%s': %v", variant, err)
+			length, lengthUnchanged, moreRemain = v.NextVariant()
 			continue
 		}
 		observe(i, variant)

interfaces/prompting/patterns/render_internal_test.go

@@ -95,6 +95,41 @@ func (s *renderSuite) TestRenderAllVariants(c *C) {
 	c.Check(variants, HasLen, parsed.NumVariants())
 }
 
+func (s *renderSuite) TestRenderAllVariantsConflicts(c *C) {
+	// all variants are the exact same path
+	for _, testCase := range []struct {
+		pattern          string
+		expectedVariants []string
+	}{
+		{
+			"/{fo{obar},foo{b{ar,ar},bar,{ba}r}}",
+			[]string{"/foobar"},
+		},
+		{
+			"/{{foo/{bar,baz},123},{123,foo{/bar,/baz}}}",
+			[]string{
+				"/foo/bar",
+				"/foo/baz",
+				"/123",
+				"/123",
+				"/foo/bar",
+				"/foo/baz",
+			},
+		},
+	} {
+		scanned, err := scan(testCase.pattern)
+		c.Assert(err, IsNil)
+		parsed, err := parse(scanned)
+		c.Assert(err, IsNil)
+		variants := make([]string, 0, len(testCase.expectedVariants))
+		renderAllVariants(parsed, func(index int, variant PatternVariant) {
+			variants = append(variants, variant.String())
+		})
+		c.Check(variants, DeepEquals, testCase.expectedVariants)
+		c.Check(variants, HasLen, parsed.NumVariants())
+	}
+}
+
 func (s *renderSuite) TestNextVariant(c *C) {
 	pattern := "/{,usr/}lib{,32,64,x32}/{,@{multiarch}/{,atomics/}}ld{-*,64}.so*"
 	scanned, err := scan(pattern)

interfaces/prompting/requestrules/requestrules.go

@@ -461,6 +461,9 @@
 		switch {
 		case !exists:
 			newVariantEntries[variantStr] = newEntry
+		case conflictingVariantEntry.RuleID == rule.ID:
+			// Rule has duplicate variant, so ignore it
+			return
 		case rdb.isRuleWithIDExpired(conflictingVariantEntry.RuleID, rule.Timestamp):
 			expiredRules[conflictingVariantEntry.RuleID] = true
 			newVariantEntries[variantStr] = newEntry
@@ -540,17 +543,23 @@
 		err := fmt.Errorf("internal error: no rules in the rule tree for user %d, snap %q, interface %q, permission %q", rule.User, rule.Snap, rule.Interface, permission)
 		return []error{err}
 	}
+	seenVariants := make(map[string]bool, rule.Constraints.PathPattern.NumVariants())
 	var errs []error
 	removeVariant := func(index int, variant patterns.PatternVariant) {
-		variantEntry, exists := permVariants.VariantEntries[variant.String()]
+		variantStr := variant.String()
+		if seenVariants[variantStr] {
+			return
+		}
+		seenVariants[variantStr] = true
+		variantEntry, exists := permVariants.VariantEntries[variantStr]
 		if !exists {
 			// Database was left inconsistent, should not occur
 			errs = append(errs, fmt.Errorf(`internal error: path pattern variant not found in the rule tree: %q`, variant))
 		} else if variantEntry.RuleID != rule.ID {
 			// Database was left inconsistent, should not occur
 			errs = append(errs, fmt.Errorf(`internal error: path pattern variant maps to different rule ID: %q: %s`, variant, variantEntry.RuleID.String()))
 		} else {
-			delete(permVariants.VariantEntries, variant.String())
+			delete(permVariants.VariantEntries, variantStr)
 		}
 	}
 	rule.Constraints.PathPattern.RenderAllVariants(removeVariant)

interfaces/prompting/requestrules/requestrules_test.go

@@ -617,6 +617,30 @@ func addRuleFromTemplate(c *C, rdb *requestrules.RuleDB, template *addRuleConten
 	return rdb.AddRule(partial.User, partial.Snap, partial.Interface, constraints, partial.Outcome, partial.Lifespan, partial.Duration)
 }
 
+func (s *requestrulesSuite) TestAddRuleDuplicateVariants(c *C) {
+	rdb, err := requestrules.New(s.defaultNotifyRule)
+	c.Assert(err, IsNil)
+
+	ruleContents := &addRuleContents{
+		User:        s.defaultUser,
+		Snap:        "nextcloud",
+		Interface:   "home",
+		PathPattern: "/home/test/{{foo/{bar,baz},123},{123,foo{/bar,/baz}}}",
+		Permissions: []string{"read"},
+		Outcome:     prompting.OutcomeAllow,
+		Lifespan:    prompting.LifespanForever,
+		Duration:    "",
+	}
+
+	var addedRules []*requestrules.Rule
+	rule, err := addRuleFromTemplate(c, rdb, ruleContents, ruleContents)
+	c.Check(err, IsNil)
+	c.Check(rule, NotNil)
+	addedRules = append(addedRules, rule)
+	s.checkWrittenRuleDB(c, addedRules)
+	s.checkNewNoticesSimple(c, nil, rule)
+}
+
 func (s *requestrulesSuite) TestAddRuleErrors(c *C) {
 	rdb, err := requestrules.New(s.defaultNotifyRule)
 	c.Assert(err, IsNil)