diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index a3ea27493db..175b3734cf1 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/overlord/snapstate/snapstate_test.go @@ -9125,11 +9125,15 @@ Name=test Exec=test-snap `[1:]), 0o644), IsNil) + var mockOldDesktopFile = []byte(` +[Desktop Entry] +Name=old-content +X-SnapInstanceName=test-snap`) desktopFile := filepath.Join(dirs.SnapDesktopFilesDir, "test-snap_test-snap.desktop") otherDesktopFile := filepath.Join(dirs.SnapDesktopFilesDir, "test-snap_other.desktop") c.Assert(os.MkdirAll(dirs.SnapDesktopFilesDir, 0o755), IsNil) - c.Assert(os.WriteFile(desktopFile, []byte("old content"), 0o644), IsNil) - c.Assert(os.WriteFile(otherDesktopFile, []byte("other old content"), 0o644), IsNil) + c.Assert(os.WriteFile(desktopFile, mockOldDesktopFile, 0o644), IsNil) + c.Assert(os.WriteFile(otherDesktopFile, mockOldDesktopFile, 0o644), IsNil) err := s.snapmgr.Ensure() c.Assert(err, IsNil) diff --git a/wrappers/desktop.go b/wrappers/desktop.go index c39a4d51a4e..7018c375d1e 100644 --- a/wrappers/desktop.go +++ b/wrappers/desktop.go @@ -22,6 +22,7 @@ package wrappers import ( "bufio" "bytes" + "errors" "fmt" "os" "os/exec" @@ -247,7 +248,45 @@ func findDesktopFiles(rootDir string) ([]string, error) { return desktopFiles, nil } -func deriveDesktopFilesContent(s *snap.Info) (map[string]osutil.FileState, error) { +// Note: explicitSnapDesktopFileIDs doesn't check if the desktop plug +// is connected because the desktop-file-ids attribute is controlled +// by an allow-installation rule. +func explicitSnapDesktopFileIDs(s *snap.Info) (map[string]bool, error) { + var desktopPlug *snap.PlugInfo + for _, plug := range s.Plugs { + if plug.Interface == "desktop" { + desktopPlug = plug + break + } + } + if desktopPlug == nil { + return nil, nil + } + + attrVal, exists := desktopPlug.Lookup("desktop-file-ids") + if !exists { + // desktop-file-ids attribute is optional + return nil, nil + } + + // desktop-file-ids must be a list of strings + desktopFileIDs, ok := attrVal.([]interface{}) + if !ok { + return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`) + } + + desktopFileIDsMap := make(map[string]bool, len(desktopFileIDs)) + for _, val := range desktopFileIDs { + desktopFileID, ok := val.(string) + if !ok { + return nil, errors.New(`internal error: "desktop-file-ids" must be a list of strings`) + } + desktopFileIDsMap[desktopFileID] = true + } + return desktopFileIDsMap, nil +} + +func deriveDesktopFilesContent(s *snap.Info, desktopFileIDs map[string]bool) (map[string]osutil.FileState, error) { rootDir := filepath.Join(s.MountDir(), "meta", "gui") desktopFiles, err := findDesktopFiles(rootDir) if err != nil { @@ -261,11 +300,16 @@ func deriveDesktopFilesContent(s *snap.Info) (map[string]osutil.FileState, error if err != nil { return nil, err } - // FIXME: don't blindly use the snap desktop filename, mangle it - // but we can't just use the app name because a desktop file - // may call the same app with multiple parameters, e.g. - // --create-new, --open-existing etc - base = fmt.Sprintf("%s_%s", s.DesktopPrefix(), base) + // Don't mangle desktop files if listed under desktop-file-ids attribute + // XXX: Do we want to fail if a desktop-file-ids entry doesn't + // have a corresponding file? + if !desktopFileIDs[strings.TrimSuffix(base, ".desktop")] { + // FIXME: don't blindly use the snap desktop filename, mangle it + // but we can't just use the app name because a desktop file + // may call the same app with multiple parameters, e.g. + // --create-new, --open-existing etc + base = fmt.Sprintf("%s_%s", s.DesktopPrefix(), base) + } installedDesktopFileName := filepath.Join(dirs.SnapDesktopFilesDir, base) fileContent = sanitizeDesktopFile(s, installedDesktopFileName, fileContent) content[base] = &osutil.MemoryFileState{ @@ -276,6 +320,60 @@ func deriveDesktopFilesContent(s *snap.Info) (map[string]osutil.FileState, error return content, nil } +// TODO: Merge desktop file helpers into desktop/desktopentry package +func snapInstanceNameFromDesktopFile(desktopFile string) (string, error) { + file, err := os.Open(desktopFile) + if err != nil { + return "", err + } + defer file.Close() + + scanner := bufio.NewScanner(file) + for i := 0; scanner.Scan(); i++ { + bline := scanner.Text() + if !strings.HasPrefix(bline, "X-SnapInstanceName=") { + continue + } + return strings.TrimPrefix(bline, "X-SnapInstanceName="), nil + } + + return "", fmt.Errorf("cannot find X-SnapInstanceName entry in %q", desktopFile) +} + +// forAllDesktopFiles loops over all installed desktop files under +// dirs.SnapDesktopFilesDir. +// +// Only the desktop file base and parsed instance name is passed the callback +// function. +func forAllDesktopFiles(cb func(base, instanceName string) error) error { + installedDesktopFiles, err := findDesktopFiles(dirs.SnapDesktopFilesDir) + if err != nil { + return err + } + + for _, desktopFile := range installedDesktopFiles { + instanceName, err := snapInstanceNameFromDesktopFile(desktopFile) + if err != nil { + // cannot read instance name from desktop file, ignore + logger.Noticef("cannot read instance name: %s", err) + continue + } + + base := filepath.Base(desktopFile) + if err := cb(base, instanceName); err != nil { + return err + } + } + + return nil +} + +func hasDesktopPrefix(s *snap.Info, desktopFile string) bool { + base := filepath.Base(desktopFile) + prefix := s.DesktopPrefix() + "_" + return strings.HasPrefix(base, prefix) && strings.HasSuffix(base, ".desktop") +} + // EnsureSnapDesktopFiles puts in place the desktop files for the applications from the snap. // // It also removes desktop files from the applications of the old snap revision to ensure @@ -286,17 +384,42 @@ func EnsureSnapDesktopFiles(snaps []*snap.Info) error { } var updated []string - for _, s := range snaps { - if s == nil { + for _, info := range snaps { + if info == nil { return fmt.Errorf("internal error: snap info cannot be nil") } - content, err := deriveDesktopFilesContent(s) + + desktopFileIDs, err := explicitSnapDesktopFileIDs(info) if err != nil { return err } + desktopFilesGlobs := []string{fmt.Sprintf("%s_*.desktop", info.DesktopPrefix())} + for desktopFileID := range desktopFileIDs { + desktopFilesGlobs = append(desktopFilesGlobs, desktopFileID+".desktop") + } + content, err := deriveDesktopFilesContent(info, desktopFileIDs) + if err != nil { + return err + } + + addGlobPatternAndConflictCheck := func(base, instanceName string) error { + // Check if a target desktop file belongs to another snap + _, hasTarget := content[base] + if hasTarget && instanceName != info.InstanceName() { + return fmt.Errorf("cannot install %q: %q already exists for another snap", base, filepath.Join(dirs.SnapDesktopFilesDir, base)) + } + if instanceName == info.InstanceName() && !hasTarget && !hasDesktopPrefix(info, base) { + // An unmangled desktop file exists for the snap, add to glob + // patterns for removal + desktopFilesGlobs = append(desktopFilesGlobs, base) + } + return nil + } + if err := forAllDesktopFiles(addGlobPatternAndConflictCheck); err != nil { + return err + } - desktopFilesGlob := fmt.Sprintf("%s_*.desktop", s.DesktopPrefix()) - changed, removed, err := osutil.EnsureDirState(dirs.SnapDesktopFilesDir, desktopFilesGlob, content) + changed, removed, err := osutil.EnsureDirStateGlobs(dirs.SnapDesktopFilesDir, desktopFilesGlobs, content) if err != nil { return err } @@ -318,8 +441,22 @@ func RemoveSnapDesktopFiles(s *snap.Info) error { return nil } - desktopFilesGlob := fmt.Sprintf("%s_*.desktop", s.DesktopPrefix()) - _, removed, err := osutil.EnsureDirState(dirs.SnapDesktopFilesDir, desktopFilesGlob, nil) + desktopFilesGlobs := []string{fmt.Sprintf("%s_*.desktop", s.DesktopPrefix())} + + addGlobPattern := func(base, instanceName string) error { + if instanceName == s.InstanceName() && !hasDesktopPrefix(s, base) { + // An unmangled desktop file exists for the snap, add to glob + // patterns for removal + desktopFilesGlobs = append(desktopFilesGlobs, base) + } + + return nil + } + if err := forAllDesktopFiles(addGlobPattern); err != nil { + return err + } + + _, removed, err := osutil.EnsureDirStateGlobs(dirs.SnapDesktopFilesDir, desktopFilesGlobs, nil) if err != nil { return err } diff --git a/wrappers/desktop_test.go b/wrappers/desktop_test.go index 5ef079bc397..a8dae0250e3 100644 --- a/wrappers/desktop_test.go +++ b/wrappers/desktop_test.go @@ -99,10 +99,88 @@ func (s *desktopSuite) TestEnsurePackageDesktopFiles(c *C) { sanitizedDesktopFileContent := wrappers.SanitizeDesktopFile(info, expectedDesktopFilePath, mockDesktopFile) c.Check(expectedDesktopFilePath, testutil.FileContains, sanitizedDesktopFileContent) - // Old desktop file should be removed + // Old desktop file should be removed because it follows the + // _*.desktop pattern. c.Assert(osutil.FileExists(oldDesktopFilePath), Equals, false) } +func (s *desktopSuite) testEnsurePackageDesktopFilesWithDesktopInterface(c *C, hasDesktopFileIDs bool) { + var desktopAppYaml = ` +name: foo +version: 1.0 +plugs: + desktop: +` + if hasDesktopFileIDs { + desktopAppYaml += "\n desktop-file-ids: [org.example.Foo]" + } + info := snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{Revision: snap.R(11)}) + c.Assert(info.Plugs["desktop"], NotNil) + + expectedDesktopFilePath1 := filepath.Join(dirs.SnapDesktopFilesDir, "foo_org.example.Foo.desktop") + if hasDesktopFileIDs { + expectedDesktopFilePath1 = filepath.Join(dirs.SnapDesktopFilesDir, "org.example.Foo.desktop") + } + c.Assert(osutil.FileExists(expectedDesktopFilePath1), Equals, false) + expectedDesktopFilePath2 := filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar.desktop") + c.Assert(osutil.FileExists(expectedDesktopFilePath2), Equals, false) + + // generate .desktop file in the package baseDir + 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) + + for _, expectedDesktopFilePath := range []string{expectedDesktopFilePath1, expectedDesktopFilePath2} { + stat, err := os.Stat(expectedDesktopFilePath) + c.Assert(err, IsNil) + c.Assert(stat.Mode().Perm(), Equals, os.FileMode(0644)) + c.Assert(s.mockUpdateDesktopDatabase.Calls(), DeepEquals, [][]string{ + {"update-desktop-database", dirs.SnapDesktopFilesDir}, + }) + + sanitizedDesktopFileContent := wrappers.SanitizeDesktopFile(info, expectedDesktopFilePath, mockDesktopFile) + c.Check(expectedDesktopFilePath, testutil.FileEquals, sanitizedDesktopFileContent) + } +} + +func (s *desktopSuite) TestEnsurePackageDesktopFilesWithDesktopInterface(c *C) { + const hasDesktopFileIDs = false + s.testEnsurePackageDesktopFilesWithDesktopInterface(c, hasDesktopFileIDs) +} + +func (s *desktopSuite) TestEnsurePackageDesktopFilesWithDesktopFileIDs(c *C) { + const hasDesktopFileIDs = true + s.testEnsurePackageDesktopFilesWithDesktopInterface(c, hasDesktopFileIDs) +} + +func (s *desktopSuite) TestEnsurePackageDesktopFilesWithBadDesktopFileIDs(c *C) { + const desktopAppYamlTemplate = ` +name: foo +version: 1.0 +plugs: + desktop: + desktop-file-ids: %s +` + + for _, tc := range []string{ + "not-a-list-of-strings", + "1", + "true", + "[[string],1]", + } { + desktopAppYaml := fmt.Sprintf(desktopAppYamlTemplate, tc) + info := snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{Revision: snap.R(11)}) + c.Assert(info.Plugs["desktop"], NotNil) + + err := wrappers.EnsureSnapDesktopFiles([]*snap.Info{info}) + c.Assert(err, ErrorMatches, `internal error: "desktop-file-ids" must be a list of strings`) + } +} + func (s *desktopSuite) TestEnsurePackageDesktopFilesMultiple(c *C) { info1 := snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{Revision: snap.R(11)}) baseDir := info1.MountDir() @@ -128,34 +206,114 @@ func (s *iconsTestSuite) TestEnsurePackageDesktopFilesNilSnapInfo(c *C) { c.Assert(wrappers.EnsureSnapDesktopFiles([]*snap.Info{nil}), ErrorMatches, "internal error: snap info cannot be nil") } -func (s *desktopSuite) TestRemovePackageDesktopFiles(c *C) { - mockDesktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar.desktop") +func (s *desktopSuite) TestEnsurePackageDesktopFilesExistingFileError(c *C) { + const desktopAppYaml = ` +name: foo +version: 1.0 +plugs: + desktop: + desktop-file-ids: [org.example.Foo] +` + info := snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{Revision: snap.R(11)}) + c.Assert(info.Plugs["desktop"], NotNil) + + // Mock existing desktop file with same name for another snap + var mockBadDesktopFile = []byte(` +[Desktop Entry] +Name=foo +X-SnapInstanceName=bar`) + badDesktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "org.example.Foo.desktop") + c.Assert(os.MkdirAll(dirs.SnapDesktopFilesDir, 0755), IsNil) + c.Assert(os.WriteFile(badDesktopFilePath, mockBadDesktopFile, 0644), IsNil) + + // generate .desktop file in the package baseDir + 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) + + err := wrappers.EnsureSnapDesktopFiles([]*snap.Info{info}) + c.Assert(err, ErrorMatches, `cannot install "org.example.Foo.desktop": ".*/var/lib/snapd/desktop/applications/org.example.Foo.desktop" already exists for another snap`) + + c.Check(badDesktopFilePath, testutil.FileEquals, mockBadDesktopFile) +} + +func (s *desktopSuite) testRemovePackageDesktopFiles(c *C, triggerErr bool) { + const desktopFileTemplate = ` +[Desktop Entry] +X-SnapInstanceName=%s +Name=Test` + desktopFileToSnapName := map[string]string{ + "org.example.desktop": "foo", + "org.example.Foo.desktop": "foo", + "foo_app.desktop": "foo", + "org.example.Bar.desktop": "bar", + "bar_app.desktop": "bar", + } + c.Assert(os.MkdirAll(dirs.SnapDesktopFilesDir, 0755), IsNil) + for desktopFile, snapName := range desktopFileToSnapName { + mockDesktopFile := fmt.Sprintf(desktopFileTemplate, snapName) + c.Assert(os.WriteFile(filepath.Join(dirs.SnapDesktopFilesDir, desktopFile), []byte(mockDesktopFile), 0644), IsNil) + } + + if triggerErr { + c.Assert(os.MkdirAll(filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar2.desktop", "potato"), 0755), IsNil) + } - err := os.MkdirAll(dirs.SnapDesktopFilesDir, 0755) - c.Assert(err, IsNil) - err = os.WriteFile(mockDesktopFilePath, mockDesktopFile, 0644) - c.Assert(err, IsNil) info, err := snap.InfoFromSnapYaml([]byte(desktopAppYaml)) c.Assert(err, IsNil) err = wrappers.RemoveSnapDesktopFiles(info) - c.Assert(err, IsNil) - c.Assert(osutil.FileExists(mockDesktopFilePath), Equals, false) - c.Assert(s.mockUpdateDesktopDatabase.Calls(), DeepEquals, [][]string{ + if triggerErr { + c.Assert(err, ErrorMatches, ".*directory not empty") + } else { + c.Assert(err, IsNil) + } + + for desktopFile, snapName := range desktopFileToSnapName { + desktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, desktopFile) + if snapName == "foo" { + c.Check(desktopFilePath, testutil.FileAbsent, Commentf(desktopFile)) + } else { + c.Check(desktopFilePath, testutil.FilePresent, Commentf(desktopFile)) + } + } + + expectedUpdateDesktopDatabase := [][]string{ {"update-desktop-database", dirs.SnapDesktopFilesDir}, - }) + } + if triggerErr { + expectedUpdateDesktopDatabase = nil + } + c.Assert(s.mockUpdateDesktopDatabase.Calls(), DeepEquals, expectedUpdateDesktopDatabase) } -func (s *desktopSuite) TestParallelInstancesRemovePackageDesktopFiles(c *C) { - mockDesktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar.desktop") - mockDesktopInstanceFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "foo+instance_foobar.desktop") +func (s *desktopSuite) TestRemovePackageDesktopFiles(c *C) { + const triggerErr = false + s.testRemovePackageDesktopFiles(c, triggerErr) +} +func (s *desktopSuite) TestRemovePackageDesktopFilesError(c *C) { + const triggerErr = true + s.testRemovePackageDesktopFiles(c, triggerErr) +} + +func (s *desktopSuite) TestParallelInstancesRemovePackageDesktopFiles(c *C) { err := os.MkdirAll(dirs.SnapDesktopFilesDir, 0755) c.Assert(err, IsNil) - err = os.WriteFile(mockDesktopFilePath, mockDesktopFile, 0644) + + const desktopFileTemplate = ` +[Desktop Entry] +Name=Test +X-SnapInstanceName=%s` + + mockDesktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar.desktop") + err = os.WriteFile(mockDesktopFilePath, []byte(fmt.Sprintf(desktopFileTemplate, "foo")), 0644) c.Assert(err, IsNil) - err = os.WriteFile(mockDesktopInstanceFilePath, mockDesktopFile, 0644) + + mockDesktopInstanceFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "foo+instance_foobar.desktop") + err = os.WriteFile(mockDesktopInstanceFilePath, []byte(fmt.Sprintf(desktopFileTemplate, "foo_instance")), 0644) c.Assert(err, IsNil) + info, err := snap.InfoFromSnapYaml([]byte(desktopAppYaml)) c.Assert(err, IsNil) @@ -169,7 +327,7 @@ func (s *desktopSuite) TestParallelInstancesRemovePackageDesktopFiles(c *C) { c.Assert(osutil.FileExists(mockDesktopInstanceFilePath), Equals, true) // restore the non-instance file - err = os.WriteFile(mockDesktopFilePath, mockDesktopFile, 0644) + err = os.WriteFile(mockDesktopFilePath, []byte(fmt.Sprintf(desktopFileTemplate, "foo")), 0644) c.Assert(err, IsNil) s.mockUpdateDesktopDatabase.ForgetCalls() @@ -185,7 +343,7 @@ func (s *desktopSuite) TestParallelInstancesRemovePackageDesktopFiles(c *C) { c.Assert(osutil.FileExists(mockDesktopFilePath), Equals, true) } -func (s *desktopSuite) TestEnsurePackageDesktopFilesCleanup(c *C) { +func (s *desktopSuite) TestEnsurePackageDesktopFilesCleanupOnError(c *C) { mockDesktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "foo_foobar1.desktop") c.Assert(osutil.FileExists(mockDesktopFilePath), Equals, false) @@ -212,13 +370,59 @@ func (s *desktopSuite) TestEnsurePackageDesktopFilesCleanup(c *C) { c.Assert(err, IsNil) err = wrappers.EnsureSnapDesktopFiles([]*snap.Info{info}) - c.Check(err, NotNil) + c.Check(err, ErrorMatches, "internal error: only regular files are supported.*") c.Check(osutil.FileExists(mockDesktopFilePath), Equals, false) c.Check(s.mockUpdateDesktopDatabase.Calls(), HasLen, 0) // foo+instance file was not removed by cleanup c.Check(osutil.FileExists(mockDesktopInstanceFilePath), Equals, true) } +func (s *desktopSuite) TestEnsurePackageDesktopFilesCleansOldFiles(c *C) { + info := snaptest.MockSnap(c, desktopAppYaml, &snap.SideInfo{Revision: snap.R(11)}) + + const desktopFileTemplate = ` +[Desktop Entry] +Name=Test +X-SnapInstanceName=%s` + desktopFileToSnapName := map[string]string{ + "org.example.desktop": "foo", + "org.example.Foo.desktop": "foo", + "foo_old_app.desktop": "foo", + "foo_app.desktop": "foo", + "foo+instance_foobar.desktop": "foo_instance", + "org.example.Bar.desktop": "bar", + "bar_app.desktop": "bar", + } + // Mock already installed desktop files + c.Assert(os.MkdirAll(dirs.SnapDesktopFilesDir, 0755), IsNil) + for desktopFile, snapName := range desktopFileToSnapName { + mockDesktopFile := fmt.Sprintf(desktopFileTemplate, snapName) + c.Assert(os.WriteFile(filepath.Join(dirs.SnapDesktopFilesDir, desktopFile), []byte(mockDesktopFile), 0644), IsNil) + } + + // generate .desktop file in the package baseDir + baseDir := info.MountDir() + c.Assert(os.MkdirAll(filepath.Join(baseDir, "meta", "gui"), 0755), IsNil) + c.Assert(os.WriteFile(filepath.Join(baseDir, "meta", "gui", "app.desktop"), mockDesktopFile, 0644), IsNil) + + err := wrappers.EnsureSnapDesktopFiles([]*snap.Info{info}) + c.Assert(err, IsNil) + + // Check that old desktop files for "foo" snap were removed + for desktopFile, snapName := range desktopFileToSnapName { + desktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, desktopFile) + if snapName == "foo" && desktopFile != "foo_app.desktop" { + c.Check(desktopFilePath, testutil.FileAbsent, Commentf(desktopFile)) + } else { + c.Check(desktopFilePath, testutil.FilePresent) + } + } + // Check that foo_app.desktop was rewritten from snap + expectedDesktopFilePath := filepath.Join(dirs.SnapDesktopFilesDir, "foo_app.desktop") + sanitizedDesktopFileContent := wrappers.SanitizeDesktopFile(info, expectedDesktopFilePath, mockDesktopFile) + c.Check(expectedDesktopFilePath, testutil.FileEquals, sanitizedDesktopFileContent) +} + // sanitize type sanitizeDesktopFileSuite struct {