asserts,snap: validate attributes to a JSON-compatible type subset (#…

…2140) This adds early validation of plug/slot attribute values to make sure they belong to a limited set of types, in particular we need them to be JSON serialisable and also to make matching them via regexps not a too ambiguous. Given the JSON-compatible constraint we can support only maps with string keys, so normalise early to that. Simplify correspondingly the asserts/ifacedecls.go code.
canonical · Oct 14, 2016 · 3c20cab · 3c20cab
1 parent 7ed1a1e
commit 3c20cab
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 25 deletions.
diff --git a/asserts/ifacedecls.go b/asserts/ifacedecls.go
@@ -120,13 +120,7 @@ func matchList(context string, matcher attrMatcher, l []interface{}) error {
 
 func (matcher mapAttrMatcher) match(context string, v interface{}) error {
 	switch x := v.(type) {
-	case map[string]interface{}: // top level looks like this
-		for k, matcher1 := range matcher {
-			if err := matchEntry(context, k, matcher1, x[k]); err != nil {
-				return err
-			}
-		}
-	case map[interface{}]interface{}: // nested maps look like this
+	case map[string]interface{}: // maps in attributes look like this
 		for k, matcher1 := range matcher {
 			if err := matchEntry(context, k, matcher1, x[k]); err != nil {
 				return err
@@ -161,6 +155,8 @@ func (matcher regexpAttrMatcher) match(context string, v interface{}) error {
 		s = strconv.FormatBool(x)
 	case int:
 		s = strconv.Itoa(x)
+	case int64:
+		s = strconv.FormatInt(x, 10)
 	case []interface{}:
 		return matchList(context, matcher, x)
 	default:

diff --git a/asserts/ifacedecls_test.go b/asserts/ifacedecls_test.go
@@ -26,6 +26,7 @@ import (
 	"gopkg.in/yaml.v2"
 
 	"github.com/snapcore/snapd/asserts"
+	"github.com/snapcore/snapd/snap"
 )
 
 var (
@@ -35,12 +36,26 @@ var (
 
 type attrConstraintsSuite struct{}
 
-func attrs(yml string) (r map[string]interface{}) {
-	err := yaml.Unmarshal([]byte(yml), &r)
+func attrs(yml string) map[string]interface{} {
+	var attrs map[string]interface{}
+	err := yaml.Unmarshal([]byte(yml), &attrs)
 	if err != nil {
 		panic(err)
 	}
-	return
+	snapYaml, err := yaml.Marshal(map[string]interface{}{
+		"name": "sample",
+		"plugs": map[string]interface{}{
+			"plug": attrs,
+		},
+	})
+	if err != nil {
+		panic(err)
+	}
+	info, err := snap.InfoFromSnapYaml(snapYaml)
+	if err != nil {
+		panic(err)
+	}
+	return info.Plugs["plug"].Attrs
 }
 
 func (s *attrConstraintsSuite) TestSimple(c *C) {
@@ -244,6 +259,12 @@ foo: 1
 bar: true
 `))
 	c.Check(err, IsNil)
+
+	err = cstrs.Check(map[string]interface{}{
+		"foo": int64(1),
+		"bar": true,
+	})
+	c.Check(err, IsNil)
 }
 
 func (s *attrConstraintsSuite) TestCompileErrors(c *C) {

diff --git a/snap/info_snap_yaml.go b/snap/info_snap_yaml.go
@@ -49,20 +49,6 @@ type snapYaml struct {
 	Hooks            map[string]hookYaml    `yaml:"hooks,omitempty"`
 }
 
-type plugYaml struct {
-	Interface string                 `yaml:"interface"`
-	Attrs     map[string]interface{} `yaml:"attrs,omitempty"`
-	Apps      []string               `yaml:"apps,omitempty"`
-	Label     string                 `yaml:"label"`
-}
-
-type slotYaml struct {
-	Interface string                 `yaml:"interface"`
-	Attrs     map[string]interface{} `yaml:"attrs,omitempty"`
-	Apps      []string               `yaml:"apps,omitempty"`
-	Label     string                 `yaml:"label"`
-}
-
 type appYaml struct {
 	Command string `yaml:"command"`
 
@@ -409,7 +395,11 @@ func convertToSlotOrPlugData(plugOrSlot, name string, data interface{}) (iface,
 				if attrs == nil {
 					attrs = make(map[string]interface{})
 				}
-				attrs[key] = valueData
+				value, err := validateAttr(valueData)
+				if err != nil {
+					return "", "", nil, fmt.Errorf("attribute %q of %s %q: %v", key, plugOrSlot, name, err)
+				}
+				attrs[key] = value
 			}
 		}
 		return iface, label, attrs, nil
@@ -418,3 +408,43 @@ func convertToSlotOrPlugData(plugOrSlot, name string, data interface{}) (iface,
 		return "", "", nil, err
 	}
 }
+
+// validateAttr validates an attribute value and returns a normalized version of it (map[interface{}]interface{} is turned into map[string]interface{})
+func validateAttr(v interface{}) (interface{}, error) {
+	switch x := v.(type) {
+	case string:
+		return x, nil
+	case bool:
+		return x, nil
+	case int:
+		return x, nil
+	case int64:
+		return x, nil
+	case []interface{}:
+		l := make([]interface{}, len(x))
+		for i, el := range x {
+			el, err := validateAttr(el)
+			if err != nil {
+				return nil, err
+			}
+			l[i] = el
+		}
+		return l, nil
+	case map[interface{}]interface{}:
+		m := make(map[string]interface{}, len(x))
+		for k, item := range x {
+			kStr, ok := k.(string)
+			if !ok {
+				return nil, fmt.Errorf("non-string key in attribute map: %v", k)
+			}
+			item, err := validateAttr(item)
+			if err != nil {
+				return nil, err
+			}
+			m[kStr] = item
+		}
+		return m, nil
+	default:
+		return nil, fmt.Errorf("invalid attribute scalar: %v", v)
+	}
+}
diff --git a/snap/info_snap_yaml_test.go b/snap/info_snap_yaml_test.go
@@ -162,6 +162,33 @@ plugs:
 	})
 }
 
+func (s *YamlSuite) TestUnmarshalStandalonePlugWithListAndMap(c *C) {
+	// NOTE: yaml content cannot use tabs, indent the section with spaces.
+	info, err := snap.InfoFromSnapYaml([]byte(`
+name: snap
+plugs:
+    iface:
+        interface: complex
+        l: [1,2,3]
+        m:
+          a: A
+          b: B
+`))
+	c.Assert(err, IsNil)
+	c.Check(info.Name(), Equals, "snap")
+	c.Check(info.Plugs, HasLen, 1)
+	c.Check(info.Slots, HasLen, 0)
+	c.Assert(info.Plugs["iface"], DeepEquals, &snap.PlugInfo{
+		Snap:      info,
+		Name:      "iface",
+		Interface: "complex",
+		Attrs: map[string]interface{}{
+			"l": []interface{}{1, 2, 3},
+			"m": map[string]interface{}{"a": "A", "b": "B"},
+		},
+	})
+}
+
 func (s *YamlSuite) TestUnmarshalLastPlugDefinitionWins(c *C) {
 	// NOTE: yaml content cannot use tabs, indent the section with spaces.
 	info, err := snap.InfoFromSnapYaml([]byte(`
@@ -410,6 +437,32 @@ plugs:
 	c.Assert(err, ErrorMatches, `plug "serial" uses reserved attribute "\$baud-rate"`)
 }
 
+func (s *YamlSuite) TestUnmarshalInvalidPlugAttribute(c *C) {
+	// NOTE: yaml content cannot use tabs, indent the section with spaces.
+	_, err := snap.InfoFromSnapYaml([]byte(`
+name: snap
+plugs:
+    serial:
+        interface: serial-port
+        foo: null
+`))
+	c.Assert(err, ErrorMatches, `attribute "foo" of plug \"serial\": invalid attribute scalar:.*`)
+}
+
+func (s *YamlSuite) TestUnmarshalInvalidAttributeMapKey(c *C) {
+	// NOTE: yaml content cannot use tabs, indent the section with spaces.
+	_, err := snap.InfoFromSnapYaml([]byte(`
+name: snap
+plugs:
+    serial:
+        interface: serial-port
+        bar:
+          baz:
+          - 1: A
+`))
+	c.Assert(err, ErrorMatches, `attribute "bar" of plug \"serial\": non-string key in attribute map: 1`)
+}
+
 // Tests focusing on slots
 
 func (s *YamlSuite) TestUnmarshalStandaloneImplicitSlot(c *C) {
@@ -488,6 +541,32 @@ slots:
 	})
 }
 
+func (s *YamlSuite) TestUnmarshalStandaloneSlotWithListAndMap(c *C) {
+	// NOTE: yaml content cannot use tabs, indent the section with spaces.
+	info, err := snap.InfoFromSnapYaml([]byte(`
+name: snap
+slots:
+    iface:
+        interface: complex
+        l: [1,2]
+        m:
+          a: "A"
+`))
+	c.Assert(err, IsNil)
+	c.Check(info.Name(), Equals, "snap")
+	c.Check(info.Plugs, HasLen, 0)
+	c.Check(info.Slots, HasLen, 1)
+	c.Assert(info.Slots["iface"], DeepEquals, &snap.SlotInfo{
+		Snap:      info,
+		Name:      "iface",
+		Interface: "complex",
+		Attrs: map[string]interface{}{
+			"l": []interface{}{1, 2},
+			"m": map[string]interface{}{"a": "A"},
+		},
+	})
+}
+
 func (s *YamlSuite) TestUnmarshalLastSlotDefinitionWins(c *C) {
 	// NOTE: yaml content cannot use tabs, indent the section with spaces.
 	info, err := snap.InfoFromSnapYaml([]byte(`
@@ -698,6 +777,18 @@ slots:
 	c.Assert(err, ErrorMatches, `slot "serial" uses reserved attribute "\$baud-rate"`)
 }
 
+func (s *YamlSuite) TestUnmarshalInvalidSlotAttribute(c *C) {
+	// NOTE: yaml content cannot use tabs, indent the section with spaces.
+	_, err := snap.InfoFromSnapYaml([]byte(`
+name: snap
+slots:
+    serial:
+        interface: serial-port
+        foo: null
+`))
+	c.Assert(err, ErrorMatches, `attribute "foo" of slot \"serial\": invalid attribute scalar:.*`)
+}
+
 func (s *YamlSuite) TestUnmarshalHook(c *C) {
 	// NOTE: yaml content cannot use tabs, indent the section with spaces.
 	info, err := snap.InfoFromSnapYaml([]byte(`