wrappers: fail install if exec-line cannot be re-written (#3991)

 wrappers: fail install if exec-line cannot be re-written

overlord/snapstate/backend/link_test.go

@@ -101,6 +101,10 @@ apps:
 func (s *linkSuite) TestLinkDoUndoCurrentSymlink(c *C) {
 	const yaml = `name: hello
 version: 1.0
+
+apps:
+ bin:
+  command: hello.bin
 `
 	const contents = ""
 
@@ -238,6 +242,8 @@ environment:
  KEY: value
 
 apps:
+ bin:
+   command: bin
  foo:
    command: foo
  bar:
@@ -254,7 +260,7 @@ apps:
 [Desktop Entry]
 Name=bin
 Icon=${SNAP}/bin.png
-Exec=bin
+Exec=hello.bin
 `), 0644), IsNil)
 
 	r := systemd.MockSystemctl(func(...string) ([]byte, error) {

wrappers/desktop.go

@@ -122,7 +122,7 @@ func rewriteExecLine(s *snap.Info, desktopFile, line string) (string, error) {
 	return "", fmt.Errorf("invalid exec command: %q", cmd)
 }
 
-func sanitizeDesktopFile(s *snap.Info, desktopFile string, rawcontent []byte) []byte {
+func sanitizeDesktopFile(s *snap.Info, desktopFile string, rawcontent []byte) ([]byte, error) {
 	var newContent bytes.Buffer
 	mountDir := []byte(s.MountDir())
 	scanner := bufio.NewScanner(bytes.NewReader(rawcontent))
@@ -139,8 +139,7 @@ func sanitizeDesktopFile(s *snap.Info, desktopFile string, rawcontent []byte) []
 			var err error
 			line, err := rewriteExecLine(s, desktopFile, string(bline))
 			if err != nil {
-				// something went wrong, ignore the line
-				continue
+				return nil, err
 			}
 			bline = []byte(line)
 		}
@@ -153,7 +152,7 @@ func sanitizeDesktopFile(s *snap.Info, desktopFile string, rawcontent []byte) []
 		newContent.WriteByte('\n')
 	}
 
-	return newContent.Bytes()
+	return newContent.Bytes(), nil
 }
 
 func updateDesktopDatabase(desktopFiles []string) error {
@@ -201,7 +200,10 @@ func AddSnapDesktopFiles(s *snap.Info) (err error) {
 		}
 
 		installedDesktopFileName := filepath.Join(dirs.SnapDesktopFilesDir, fmt.Sprintf("%s_%s", s.Name(), filepath.Base(df)))
-		content = sanitizeDesktopFile(s, installedDesktopFileName, content)
+		content, err = sanitizeDesktopFile(s, installedDesktopFileName, content)
+		if err != nil {
+			return fmt.Errorf("cannot write %q: %s", df, err)
+		}
 		if err := osutil.AtomicWriteFile(installedDesktopFileName, content, 0755, 0); err != nil {
 			return err
 		}

wrappers/desktop_test.go

@@ -147,7 +147,8 @@ Icon=${SNAP}/meep
 
 # the empty line above is fine`)
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	e, err := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, IsNil)
 	c.Assert(string(e), Equals, fmt.Sprintf(`[Desktop Entry]
 Name=foo
 Icon=%s/foo/12/meep
@@ -170,10 +171,8 @@ Name=foo
 Exec=baz
 `)
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
-	c.Assert(string(e), Equals, `[Desktop Entry]
-Name=foo
-`)
+	_, err = wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, ErrorMatches, `invalid exec command: "baz"`)
 }
 
 func (s *sanitizeDesktopFileSuite) TestSanitizeFiltersExecPrefix(c *C) {
@@ -190,10 +189,8 @@ Name=foo
 Exec=snap.app.evil.evil
 `)
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
-	c.Assert(string(e), Equals, `[Desktop Entry]
-Name=foo
-`)
+	_, err = wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, ErrorMatches, `invalid exec command: "snap.app.evil.evil"`)
 }
 
 func (s *sanitizeDesktopFileSuite) TestSanitizeFiltersExecOk(c *C) {
@@ -210,7 +207,8 @@ Name=foo
 Exec=snap.app %U
 `)
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	e, err := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, IsNil)
 	c.Assert(string(e), Equals, fmt.Sprintf(`[Desktop Entry]
 Name=foo
 Exec=env BAMF_DESKTOP_FILE_HINT=foo.desktop %s/bin/snap.app %%U
@@ -233,7 +231,8 @@ Name=foo
 TryExec=snap.app %U
 `)
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	e, err := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, IsNil)
 	c.Assert(string(e), Equals, `[Desktop Entry]
 Name=foo
 `)
@@ -251,7 +250,8 @@ Invalid=key
 Invalid[i18n]=key
 `)
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	e, err := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, IsNil)
 	c.Assert(string(e), Equals, `[Desktop Entry]
 Name=foo
 GenericName=bar
@@ -265,7 +265,8 @@ func (s *sanitizeDesktopFileSuite) TestSanitizeDesktopActionsOk(c *C) {
 	snap := &snap.Info{}
 	desktopContent := []byte("[Desktop Action is-ok]\n")
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	e, err := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, IsNil)
 	c.Assert(string(e), Equals, string(desktopContent))
 }
 
@@ -286,7 +287,8 @@ Name=Private Mode
 TargetEnvironment=Unity
 `)
 
-	e := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	e, err := wrappers.SanitizeDesktopFile(snap, "foo.desktop", desktopContent)
+	c.Assert(err, IsNil)
 	c.Assert(string(e), Equals, string(desktopContent))
 }