snap,wrappers: fix common id selection with multiple desktop plugs

snap/info.go

@@ -968,44 +968,57 @@ func (s *Info) DesktopPrefix() string {
 // Note: DesktopPlugFileIDs doesn't check if the desktop plug is connected because
 // the desktop-file-ids attribute is controlled by an allow-installation rule.
 func (s *Info) DesktopPlugFileIDs() ([]string, error) {
-	var desktopPlug *PlugInfo
-	for _, plug := range s.Plugs {
+	desktopPlugNames := make([]string, 0, len(s.Plugs))
+	for name, plug := range s.Plugs {
 		if plug.Interface == "desktop" {
-			desktopPlug = plug
-			break
+			desktopPlugNames = append(desktopPlugNames, name)
 		}
 	}
-	if desktopPlug == nil {
-		return nil, nil
-	}
-
-	attrVal, exists := desktopPlug.Lookup("desktop-file-ids")
-	if !exists {
-		// desktop-file-ids attribute is optional
+	if len(desktopPlugNames) == 0 {
 		return nil, nil
 	}
+	sort.Strings(desktopPlugNames)
 
 	// TODO: The internal errors below should never happen due to validation
 	// in the desktop interface. It would be a good candidate for telemetry
 	// error reporting.
 
-	// desktop-file-ids must be a list of strings
-	attrList, ok := attrVal.([]any)
-	if !ok {
-		return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
-	}
+	desktopFileIDs := make([]string, 0)
+	seenDesktopFileIDs := make(map[string]bool)
+	for _, plugName := range desktopPlugNames {
+		desktopPlug := s.Plugs[plugName]
+
+		attrVal, exists := desktopPlug.Lookup("desktop-file-ids")
+		if !exists {
+			// desktop-file-ids attribute is optional
+			continue
+		}
 
-	desktopFileIDs := make([]string, 0, len(attrList))
-	for _, val := range attrList {
-		desktopFileID, ok := val.(string)
+		// desktop-file-ids must be a list of strings
+		attrList, ok := attrVal.([]any)
 		if !ok {
 			return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
 		}
-		if !strings.HasSuffix(desktopFileID, ".desktop") {
-			logger.Noticef("adding missing .desktop suffix to desktop file ID %s (snap %s)", desktopFileID, s.InstanceName())
-			desktopFileID = desktopFileID + ".desktop"
+
+		for _, val := range attrList {
+			desktopFileID, ok := val.(string)
+			if !ok {
+				return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`)
+			}
+			if !strings.HasSuffix(desktopFileID, ".desktop") {
+				logger.Noticef("adding missing .desktop suffix to desktop file ID %s (snap %s)", desktopFileID, s.InstanceName())
+				desktopFileID = desktopFileID + ".desktop"
+			}
+			if seenDesktopFileIDs[desktopFileID] {
+				continue
+			}
+			seenDesktopFileIDs[desktopFileID] = true
+			desktopFileIDs = append(desktopFileIDs, desktopFileID)
 		}
-		desktopFileIDs = append(desktopFileIDs, desktopFileID)
+	}
+
+	if len(desktopFileIDs) == 0 {
+		return nil, nil
 	}
 	return desktopFileIDs, nil
 }

snap/info_test.go

@@ -2719,6 +2719,63 @@ plugs:
 	}
 }
 
+func (s *infoSuite) TestDesktopFileIDsMergedFromAllDesktopPlugs(c *C) {
+	const desktopAppYaml = `
+name: foo
+version: 1.0
+plugs:
+  desktop:
+  desktop-extra:
+    interface: desktop
+    desktop-file-ids: [org.example.Foo, org.example.Bar]
+  desktop-file:
+    interface: desktop
+    desktop-file-ids: [org.example.desktop, org.example.Foo.desktop]
+`
+
+	info, err := snap.InfoFromSnapYaml([]byte(desktopAppYaml))
+	c.Assert(err, IsNil)
+
+	desktopFileIDs, err := info.DesktopPlugFileIDs()
+	c.Assert(err, IsNil)
+	c.Assert(desktopFileIDs, DeepEquals, []string{"org.example.Foo.desktop", "org.example.Bar.desktop", "org.example.desktop"})
+}
+
+func (s *infoSuite) TestAppDesktopFileUsesDesktopFileIDsAcrossDesktopPlugs(c *C) {
+	const desktopAppYaml = `
+name: sample
+version: 1
+apps:
+ app:
+   command: foo
+   common-id: org.example.CommonID.Foo
+plugs:
+  desktop:
+  desktop-file:
+    interface: desktop
+    desktop-file-ids:
+      - org.example.CommonID.Foo
+`
+
+	snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{})
+	snapInfo, err := snap.ReadInfo("sample", &snap.SideInfo{})
+	c.Assert(err, IsNil)
+	c.Assert(snapInfo.Plugs["desktop"], NotNil)
+	c.Assert(snapInfo.Plugs["desktop-file"], NotNil)
+
+	c.Assert(os.MkdirAll(dirs.SnapDesktopFilesDir, 0755), IsNil)
+	const desktopFileName = "org.example.CommonID.Foo.desktop"
+	const mockDesktopFile = `[Desktop Entry]
+X-SnapInstanceName=sample
+Name=foo
+X-SnapAppName=app
+Exec=sample.app
+`
+	c.Assert(os.WriteFile(filepath.Join(dirs.SnapDesktopFilesDir, desktopFileName), []byte(mockDesktopFile), 0644), IsNil)
+
+	c.Check(snapInfo.Apps["app"].DesktopFile(), Matches, `.*/var/lib/snapd/desktop/applications/org.example.CommonID.Foo.desktop`)
+}
+
 func (s *infoSuite) TestMangleDesktopFileName(c *C) {
 	const desktopAppYaml = `
 name: foo

tests/lib/snaps/test-snapd-desktop-file-ids/bin/check-dirs

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/sh -e
 
 for dir in "$@"; do
     ls "$dir"

tests/lib/snaps/test-snapd-desktop-file-ids/bin/check-files

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/sh -e
 
 for file in "$@"; do
     cat "$file"

tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/arbitrary.desktop

@@ -1,7 +1,7 @@
 [Desktop Entry]
-Name=test-snapd-desktop.cmd
-Comment=A desktop file for test-snapd-desktop.cmd
-Exec=test-snapd-desktop.cmd
+Name=test-snapd-desktop-file-ids.cmd
+Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+Exec=test-snapd-desktop-file-ids.cmd
 Terminal=true
 Type=Application
 X-SnapCommonID=org.example.MyCommonID

tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/bar.desktop

@@ -1,7 +1,7 @@
 [Desktop Entry]
-Name=test-snapd-desktop.cmd
-Comment=A desktop file for test-snapd-desktop.cmd
-Exec=test-snapd-desktop.cmd
+Name=test-snapd-desktop-file-ids.cmd
+Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+Exec=test-snapd-desktop-file-ids.cmd
 Terminal=true
 Type=Application
 X-SnapCommonID=org.example.NotListed

tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/foo.desktop

@@ -1,6 +1,6 @@
 [Desktop Entry]
-Name=test-snapd-desktop.cmd
-Comment=A desktop file for test-snapd-desktop.cmd
-Exec=test-snapd-desktop.cmd
+Name=test-snapd-desktop-file-ids.cmd
+Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+Exec=test-snapd-desktop-file-ids.cmd
 Terminal=true
 Type=Application

tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/org.example.Bar.desktop

@@ -0,0 +1,6 @@
+[Desktop Entry]
+Name=test-snapd-desktop-file-ids.cmd
+Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+Exec=test-snapd-desktop-file-ids.cmd
+Terminal=true
+Type=Application

tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/org.example.Fallback.desktop

@@ -1,6 +1,6 @@
 [Desktop Entry]
-Name=test-snapd-desktop.cmd
-Comment=A desktop file for test-snapd-desktop.cmd
-Exec=test-snapd-desktop.cmd
+Name=test-snapd-desktop-file-ids.cmd
+Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+Exec=test-snapd-desktop-file-ids.cmd
 Terminal=true
 Type=Application

tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/org.example.Foo.desktop

@@ -1,6 +1,6 @@
 [Desktop Entry]
-Name=test-snapd-desktop.cmd
-Comment=A desktop file for test-snapd-desktop.cmd
-Exec=test-snapd-desktop.cmd
+Name=test-snapd-desktop-file-ids.cmd
+Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+Exec=test-snapd-desktop-file-ids.cmd
 Terminal=true
 Type=Application

tests/lib/snaps/test-snapd-desktop-file-ids/meta/gui/org.example.desktop

@@ -1,6 +1,6 @@
 [Desktop Entry]
-Name=test-snapd-desktop.cmd
-Comment=A desktop file for test-snapd-desktop.cmd
-Exec=test-snapd-desktop.cmd
+Name=test-snapd-desktop-file-ids.cmd
+Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+Exec=test-snapd-desktop-file-ids.cmd
 Terminal=true
 Type=Application

tests/lib/snaps/test-snapd-desktop-file-ids/meta/snap.yaml

@@ -10,10 +10,44 @@ apps:
     check-dirs:
         command: bin/check-dirs
         plugs: [desktop, desktop-legacy]
+    cmd:
+        command: bin/check-files # doesn't matter
+        common-id: org.example.Foo
+        plugs:
+        - desktop # really also a decoy
+        - desktop-ids
+        - desktop-decoy1
+        - desktop-decoy2
+        - desktop-decoy3
+        - desktop-decoy4
+        - desktop-decoy5
+        - desktop-decoy6
+        - desktop-decoy7
+        - desktop-decoy8
 
 plugs:
-  desktop:
+  desktop-ids:
+    interface: desktop
     desktop-file-ids:
     - org.example.desktop
+    - org.example.Foo.desktop
     - org.example.Fallback
-    - org.example.MyCommonID.desktop
+    - org.example.NonexistentFile.desktop
+  # Declare lots of desktop plugs without desktop-file-ids to make sure the
+  # correct desktop file ID is chosen
+  desktop-decoy1:
+    interface: desktop
+  desktop-decoy2:
+    interface: desktop
+  desktop-decoy3:
+    interface: desktop
+  desktop-decoy4:
+    interface: desktop
+  desktop-decoy5:
+    interface: desktop
+  desktop-decoy6:
+    interface: desktop
+  desktop-decoy7:
+    interface: desktop
+  desktop-decoy8:
+    interface: desktop

tests/main/desktop-file-ids/task.yaml

@@ -16,9 +16,9 @@ environment:
 prepare: |
     cat > "$TESTSLIB"/snaps/test-snapd-desktop-file-ids/meta/gui/'bad. ,?**[]{}^\$#.desktop' << EOF
     [Desktop Entry]
-    Name=test-snapd-desktop.cmd
-    Comment=A desktop file for test-snapd-desktop.cmd
-    Exec=test-snapd-desktop.cmd
+    Name=test-snapd-desktop-file-ids.cmd
+    Comment=A desktop file for test-snapd-desktop-file-ids.cmd
+    Exec=test-snapd-desktop-file-ids.cmd
     Terminal=true
     Type=Application
     EOF
@@ -32,7 +32,7 @@ restore: |
     tests.session -u test restore
 
 execute: |
-    files="$DIR/org.example.desktop $DIR/org.example.Fallback.desktop $DIR/org.example.MyCommonID.desktop $DIR/test-snapd-desktop-file-ids_foo.desktop $DIR/test-snapd-desktop-file-ids_bar.desktop $DIR/test-snapd-desktop-file-ids_org.example.Foo.desktop $DIR/test-snapd-desktop-file-ids_bad._____________.desktop"
+    files="$DIR/org.example.desktop $DIR/org.example.Foo.desktop $DIR/org.example.Fallback.desktop $DIR/test-snapd-desktop-file-ids_arbitrary.desktop $DIR/test-snapd-desktop-file-ids_foo.desktop $DIR/test-snapd-desktop-file-ids_bar.desktop $DIR/test-snapd-desktop-file-ids_org.example.Bar.desktop $DIR/test-snapd-desktop-file-ids_bad._____________.desktop"
 
     # Connect desktop-legacy interface to test generated desktop allow/deny rules
     # only allows access to the snap's desktop files
@@ -51,6 +51,18 @@ execute: |
         not tests.session -u test exec test-snapd-desktop-file-ids.check-files "$DIR/test-confinement.desktop"
     fi
 
+    echo "Just because a desktop file ID is declared does not mean it is created"
+    test ! -f "$DIR/org.example.NonexistentFile.desktop"
+    test ! -f "$DIR/org.example.MyCommonID.desktop"
+
+    echo "Check that common ID and desktop file ID is correct"
+    RESULT="$(snap debug api /v2/snaps/test-snapd-desktop-file-ids)"
+    echo "$RESULT" | MATCH '"status-code": 200'
+    APPS="$(echo "$RESULT" | gojq .result.apps)"
+    # Should have both common-id and desktop-file
+    echo "$APPS" | gojq '.[] | select(.name == "cmd")."common-id"' | MATCH "org.example.Foo"
+    echo "$APPS" | gojq '.[] | select(.name == "cmd")."desktop-file"' | MATCH "org.example.Foo.desktop"
+
     snap remove --purge test-snapd-desktop-file-ids
     echo "Check desktop files are removed"
     for file in $files; do

wrappers/desktop_test.go

@@ -188,6 +188,35 @@ func (s *desktopSuite) TestEnsurePackageDesktopFilesWithDesktopFileName(c *C) {
 	s.testEnsurePackageDesktopFilesWithDesktopInterface(c, desktopFileID)
 }
 
+func (s *desktopSuite) TestEnsurePackageDesktopFilesWithMultipleDesktopPlugs(c *C) {
+	const desktopAppYaml = `
+name: foo
+version: 1.0
+apps:
+  foobar:
+plugs:
+  desktop:
+  desktop-file:
+    interface: desktop
+    desktop-file-ids: [org.example.Foo]
+`
+
+	info := snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{Revision: snap.R(11)})
+	c.Assert(info.Plugs["desktop"], NotNil)
+	c.Assert(info.Plugs["desktop-file"], NotNil)
+
+	baseDir := info.MountDir()
+	c.Assert(os.MkdirAll(filepath.Join(baseDir, "meta", "gui"), 0755), IsNil)
+	c.Assert(os.WriteFile(filepath.Join(baseDir, "meta", "gui", "org.example.Foo.desktop"), mockDesktopFile, 0644), IsNil)
+	c.Assert(os.WriteFile(filepath.Join(baseDir, "meta", "gui", "foobar.desktop"), mockDesktopFile, 0644), IsNil)
+
+	err := wrappers.EnsureSnapDesktopFiles([]*snap.Info{info})
+	c.Assert(err, IsNil)
+
+	c.Check(filepath.Join(dirs.SnapDesktopFilesDir, "org.example.Foo.desktop"), testutil.FilePresent)
+	c.Check(filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar.desktop"), testutil.FilePresent)
+}
+
 func (s *desktopSuite) TestEnsurePackageDesktopFilesWithBadDesktopFileIDs(c *C) {
 	const desktopAppYamlTemplate = `
 name: foo