Skip to content

Commit ec53baf

Browse files
asserts, i/builtin: ensure that compatibility labels are strings (#17311)
This fixes a panic during auto-connection when installing a snap with an invalid compatibility label.
1 parent 235e7e1 commit ec53baf

4 files changed

Lines changed: 67 additions & 2 deletions

File tree

asserts/constraint.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,16 @@ var (
228228
func matchCompatLabels(v1, v2 any) bool {
229229
// Note that decoding errors should not happen as interfaces are
230230
// expected to check the format of the labels before this can even be
231-
// called.
232-
return compatibility.CheckCompatibility(v1.(string), v2.(string))
231+
// called. Still, avoid panicking if malformed values reach this layer.
232+
s1, ok := v1.(string)
233+
if !ok {
234+
return false
235+
}
236+
s2, ok := v2.(string)
237+
if !ok {
238+
return false
239+
}
240+
return compatibility.CheckCompatibility(s1, s2)
233241
}
234242

235243
func compileEvalOrRefAttrMatcher(cc compileContext, s string) (attrMatcher, error) {

asserts/ifacedecls_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,23 @@ func (s *attrConstraintsSuite) TestEvalCheckPlugCompat(c *C) {
381381
s.testEvalCheckCompat(c, "PLUG_COMPAT")
382382
}
383383

384+
func (s *attrConstraintsSuite) TestEvalCheckCompatNonString(c *C) {
385+
m, err := asserts.ParseHeaders([]byte(`attrs:
386+
compatibility: $SLOT_COMPAT(compatibility)`))
387+
c.Assert(err, IsNil)
388+
389+
cstrs, err := asserts.CompileAttributeConstraints(m["attrs"].(map[string]any))
390+
c.Assert(err, IsNil)
391+
392+
comp := func(op string, arg string) (any, error) {
393+
return "foo-1", nil
394+
}
395+
err = cstrs.Check(attrs(`
396+
compatibility: 1
397+
`), testEvalAttr{comp: comp, compatLabels: true})
398+
c.Check(err, ErrorMatches, `attribute "compatibility" does not match \$SLOT_COMPAT\(compatibility\): 1 != foo-1`)
399+
}
400+
384401
func (s *attrConstraintsSuite) testEvalCheckCompat(c *C, compatOper string) {
385402
m, err := asserts.ParseHeaders([]byte(fmt.Sprintf(`attrs:
386403
foo: $%s(foo)`, compatOper)))

interfaces/builtin/content.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,16 @@ func checkLabelAttributes(attrs map[string]any, nameDef string) error {
9696
// here we allow the compatibility labels to exist in any case as it
9797
// has no further side effect.
9898
content, okContent := attrs["content"].(string)
99+
100+
// TODO: consider asserting that "content" is a string. right now, a
101+
// non-string "content" attribute will result in us using the plug's name as
102+
// the "content" attribute.
103+
99104
compat, okCompat := attrs["compatibility"].(string)
105+
if _, ok := attrs["compatibility"]; ok && !okCompat {
106+
return errors.New("compatibility label must be a string")
107+
}
108+
100109
hasContent := okContent && len(content) > 0
101110
hasCompat := okCompat && len(compat) > 0
102111
if hasCompat && hasContent {

interfaces/builtin/content_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,22 @@ slots:
112112
c.Assert(slot.Attrs["content"], IsNil)
113113
}
114114

115+
func (s *ContentSuite) TestSanitizeSlotBadCompatibilityType(c *C) {
116+
const mockSnapYaml = `name: content-slot-snap
117+
version: 1.0
118+
slots:
119+
content-slot:
120+
interface: content
121+
compatibility: 1
122+
read:
123+
- shared/read
124+
`
125+
info := snaptest.MockInfo(c, mockSnapYaml, nil)
126+
slot := info.Slots["content-slot"]
127+
c.Assert(interfaces.BeforePrepareSlot(s.iface, slot), ErrorMatches,
128+
`compatibility label must be a string`)
129+
}
130+
115131
func (s *ContentSuite) TestSanitizeSlotBothContentAndCompatLabels(c *C) {
116132
const mockSnapYaml = `name: content-slot-snap
117133
version: 1.0
@@ -252,6 +268,21 @@ plugs:
252268
c.Assert(plug.Attrs["content"], IsNil)
253269
}
254270

271+
func (s *ContentSuite) TestSanitizePlugBadCompatibilityType(c *C) {
272+
const mockSnapYaml = `name: content-slot-snap
273+
version: 1.0
274+
plugs:
275+
content-plug:
276+
interface: content
277+
compatibility: 1
278+
target: import
279+
`
280+
info := snaptest.MockInfo(c, mockSnapYaml, nil)
281+
plug := info.Plugs["content-plug"]
282+
c.Assert(interfaces.BeforePreparePlug(s.iface, plug), ErrorMatches,
283+
`compatibility label must be a string`)
284+
}
285+
255286
func (s *ContentSuite) TestSanitizePlugBothContentAndCompatLabels(c *C) {
256287
const mockSnapYaml = `name: content-slot-snap
257288
version: 1.0

0 commit comments

Comments
 (0)