Merge ~morphis/snappy-hwe-snaps/+git/udisks2:bugs/ciborium/only-thumb-and-removable into ~snappy-hwe-team/snappy-hwe-snaps/+git/udisks2:ciborium/0.2.12

[Simon Fels]

udisks2: don't mount non-removable and non-thumb devices

We don't want devices which are not marked as removable or as a thumb
drive to be ever selected as candidate for the auto-mounter. The udisks2
API documentation states that some USB storages devices are marked with
MediaRemovable=true even if they don't have any removable media. We need
to make sure that we correctly check both, the MediaRemovable and the
Removable property in our detection logic. In addition to that we then
only want thumb drives to be allowed for the auto-mounter.

This also enables proper CI tests for this branch now that we have capabilities on our CI agent to test things not having spread tests. The tests will build the relevant parts of the source and run available tests.

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:fc420801e04b8d5e674c47c93c97e94bff47e9cb
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/993/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/generic-test-snap/683
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/901/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/993/rebuild

review: Approve (continuous-integration)

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

LGTM

review: Approve

[Konrad Zapałowicz (kzapalowicz)]

ok

review: Approve (code)

[Konrad Zapałowicz (kzapalowicz)]

just a small suggestion

review: Needs Fixing

[Scott Sweeny (ssweeny)]

LGTM

review: Approve

[Konrad Zapałowicz (kzapalowicz)]

LGTM

review: Approve (code)

[System Enablement Bot (system-enablement-ci-bot)]

PASSED: Continuous integration, rev:288072d18cfae802573bdfbcc9e92753811f0bdd
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/997/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-enablement/job/generic-test-snap/684
    None: https://jenkins.canonical.com/system-enablement/job/generic-update-snap-mp/905/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-enablement/job/generic-build-snap/997/rebuild

review: Approve (continuous-integration)

diff --git a/.ci_tests_disabled b/.ci_tests_disabled
deleted file mode 100644
index e69de29..0000000
--- a/.ci_tests_disabled
+++ /dev/null
diff --git a/cmd/ciborium/main.go b/cmd/ciborium/main.go
index 53e0cdf..9f891ab 100644
--- a/cmd/ciborium/main.go
+++ b/cmd/ciborium/main.go
@@ -103,7 +103,7 @@ var (
 func init() {
 	mw = newMountwatch()
 	mw.set(homeMountpoint, true)
-	supportedFS = []string{"vfat"}
+	supportedFS = []string{"vfat", "ntfs"}
 }
 
 func main() {
diff --git a/run-tests.sh b/run-tests.sh
new file mode 100755
index 0000000..9645219
--- /dev/null
+++ b/run-tests.sh
@@ -0,0 +1,25 @@
+#!/bin/sh -ex
+
+SUDO=
+if [ $(id -u) -ne 0 ]; then
+	SUDO=sudo
+fi
+
+# Install all necessary build dependencies in our bare build environment
+$SUDO apt update
+$SUDO apt install -y --force-yes \
+	golang-go \
+	bzr
+
+export GOPATH=`mktemp -d`
+
+mkdir -p $GOPATH/src/launchpad.net
+ln -sf $PWD $GOPATH/src/launchpad.net/ciborium
+
+go get -v launchpad.net/ciborium/cmd/ciborium
+go get -v -t launchpad.net/ciborium/cmd/ciborium
+
+go build -v launchpad.net/ciborium/cmd/ciborium
+
+go test -v launchpad.net/ciborium/cmd/ciborium
+go test -v launchpad.net/ciborium/udisks2
diff --git a/udisks2/udisks2.go b/udisks2/udisks2.go
index d86e22e..a3a9313 100644
--- a/udisks2/udisks2.go
+++ b/udisks2/udisks2.go
@@ -553,6 +553,14 @@ func exists(path string) bool {
 	return true
 }
 
+func isAcceptedDevice(mediaRemovable, removable, thumbDrive bool) bool {
+	// Some USB thumb devices report they have removable media where they
+	// have not. See udisks2 API documentation for more details.
+	// In summary we only accept devices which are marked as removable and
+	// are thumb drivers. Otherwise all devices are not accepted.
+	return (mediaRemovable || removable) && thumbDrive
+}
+
 func (u *UDisks2) desiredMountableEvent(s *Event) (bool, error) {
 	// Check if automount is enabled for the snap
 	if !exists(os.Getenv("SNAP_COMMON") + "/.automount_enabled") {
@@ -587,15 +595,22 @@ func (u *UDisks2) desiredMountableEvent(s *Event) (bool, error) {
 		log.Println(drivePath, "doesn't hold a Drive interface")
 		return false, nil
 	}
-	if mediaRemovableVariant, ok := driveProps["MediaRemovable"]; !ok {
+	removableVariant, ok := driveProps["Removable"]
+	if !ok {
+		log.Println(drivePath, "which holds", s.Path, "doesn't have Removable")
+		return false, nil
+	}
+	mediaRemovableVariant, ok := driveProps["MediaRemovable"]
+	if !ok {
 		log.Println(drivePath, "which holds", s.Path, "doesn't have MediaRemovable")
 		return false, nil
-	} else {
-		mediaRemovable := reflect.ValueOf(mediaRemovableVariant.Value).Bool()
-		if !mediaRemovable && !isThumbDrive(driveProps) {
-			log.Println(drivePath, "which holds", s.Path, "is not MediaRemovable or a thumb drive")
-			return false, nil
-		}
+	}
+
+	removable := reflect.ValueOf(removableVariant.Value).Bool()
+	mediaRemovable := reflect.ValueOf(mediaRemovableVariant.Value).Bool()
+	if !isAcceptedDevice(mediaRemovable, removable, isThumbDrive(driveProps)) {
+		log.Println(drivePath, "which holds", s.Path, "is not Removable or MediaRemovable and a thumb drive")
+		return false, nil
 	}
 
 	if s.Props.isMounted() {
diff --git a/udisks2/udisks2_test.go b/udisks2/udisks2_test.go
new file mode 100644
index 0000000..9494117
--- /dev/null
+++ b/udisks2/udisks2_test.go
@@ -0,0 +1,24 @@
+package udisks2
+
+import . "launchpad.net/gocheck"
+
+type Udisks2TestSuite struct{}
+
+var _ = Suite(&Udisks2TestSuite{})
+
+func (s *Udisks2TestSuite) TestIsDeviceAcceptedForAutomount(c *C) {
+	// Device with MediaRemovable = true, Removable = true and MediaCompatibility = []
+	c.Assert(isAcceptedDevice(true, true, false), Equals, false)
+	// Device with MediaRemovable = true, Removable = false and MediaCompatibility = []
+	c.Assert(isAcceptedDevice(true, false, false), Equals, false)
+	// Device with MediaRemovable = false, Removable = false and MediaCompatibility = []
+	c.Assert(isAcceptedDevice(false, false, false), Equals, false)
+	// Device with MediaRemovable = true, Removable = true and MediaCompatibility = [thumb]
+	c.Assert(isAcceptedDevice(true, true, true), Equals, true)
+	// Device with MediaRemovable = true, Removable = false and MediaCompatibility = [thumb]
+	c.Assert(isAcceptedDevice(true, false, true), Equals, true)
+	// Device with MediaRemovable = false, Removable = true and MediaCompatibility = [thumb]
+	c.Assert(isAcceptedDevice(false, true, true), Equals, true)
+	// Device with MediaRemovable = false, Removable = false and MediaCompatibility = [thumb]
+	c.Assert(isAcceptedDevice(false, false, true), Equals, false)
+}