launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/snap-non-root into lp:launchpad-buildd.

Commit message:
Run snapcraft as non-root with passwordless sudo, since we run into
buggy corner cases in some plugins when running as root (LP: #1702656).

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1702656 in launchpad-buildd: "Run snapcraft as non-root (with passwordless sudo)"
  https://bugs.launchpad.net/launchpad-buildd/+bug/1702656

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/snap-non-root/+merge/326919

This will need a fair bit of QA with various snaps to make sure we aren't introducing a different set of corner cases instead.

This introduces a dependency on the buildd user existing in the chroot, which is true but I'm not sure was previously explicitly required anywhere.  If it isn't OK to rely on this then I can add code to ensure that it's true.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/snap-non-root into lp:launchpad-buildd.

=== modified file 'buildsnap'
--- buildsnap	2017-05-15 16:57:31 +0000
+++ buildsnap	2017-07-06 10:19:16 +0000
@@ -51,7 +51,7 @@
         # appropriate certificate for your codehosting system.
         self.ssl_verify = True
 
-    def chroot(self, args, echo=False, get_output=False):
+    def chroot(self, args, drop_privs=False, echo=False, get_output=False):
         """Run a command in the chroot.
 
         :param args: the command and arguments to run.
@@ -64,7 +64,10 @@
                 "Running in chroot: %s" % ' '.join(
                 "'%s'" % arg for arg in args))
             sys.stdout.flush()
-        cmd = ["/usr/bin/sudo", "/usr/sbin/chroot", self.chroot_path] + args
+        cmd = ["/usr/bin/sudo", "/usr/sbin/chroot", self.chroot_path]
+        if drop_privs:
+            cmd.extend(["sudo", "-iu", "buildd"])
+        cmd.extend(args)
         if get_output:
             return subprocess.check_output(cmd, universal_newlines=True)
         else:
@@ -97,7 +100,8 @@
             for key, value in full_env.items()] + args
         command = "cd %s && %s" % (path, " ".join(args))
         return self.chroot(
-            ["/bin/sh", "-c", command], echo=echo, get_output=get_output)
+            ["/bin/sh", "-c", command],
+            drop_privs=True, echo=echo, get_output=get_output)
 
     def save_status(self, status):
         """Save a dictionary of status information about this build.
@@ -112,7 +116,7 @@
 
     def install(self):
         print("Running install phase...")
-        deps = ["snapcraft"]
+        deps = ["snapcraft", "sudo", "adduser"]
         if self.options.branch is not None:
             deps.append("bzr")
         else:
@@ -127,6 +131,8 @@
                 os.path.join(
                     self.chroot_path, "usr", "local", "bin", "snap-git-proxy"),
                 ])
+        self.chroot(["chown", "buildd:buildd", "/build"])
+        self.chroot(["adduser", "buildd", "sudo"])
 
     def repo(self):
         """Collect git or bzr branch."""

=== modified file 'debian/changelog'
--- debian/changelog	2017-07-03 14:04:31 +0000
+++ debian/changelog	2017-07-06 10:19:16 +0000
@@ -1,3 +1,10 @@
+launchpad-buildd (146) UNRELEASED; urgency=medium
+
+  * Run snapcraft as non-root with passwordless sudo, since we run into
+    buggy corner cases in some plugins when running as root (LP: #1702656).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Thu, 06 Jul 2017 11:15:51 +0100
+
 launchpad-buildd (145) xenial; urgency=medium
 
   * buildrecipe: Explicitly mark the local apt archive as trusted