Automated reviews and snapcraft < 2.38


#1

Now that new versions of snapcraft are widely available (ie, 2.40 is in the snap’s stable channel and -updates for all Ubuntu releases (which LP and build.snapcraft.io pulls in automatically)), as of yesterday (2018/04/12) the review tools in the store now require the snap is created with -no-fragments (which snapcraft does as of 2.38). If you are using an older version of snapcraft to build the snap, you may see snaps fail review with:

Processing...|
Error while processing...
The store was unable to accept this snap.
  - checksums do not match. Please ensure the snap is created with either 'snapcraft pack <DIR>' or 'mksquashfs <dir> <snap> -noappend -comp xz -all-root -no-xattrs -no-fragments'

To resolve, simply upgrade to a newer snapcraft, rebuild and push to the store.

Based on input from @evan, I’m in the process of improving the error message to mention 2.38.


#2

Just to be clear: I think a suitable message should be returned from the Store, which knows the snapcraft version from the request headers, to the snapcraft CLI. There’s a nice immediacy to that feedback.


#3

Uh. I am running snapcraft 2.40 and I got this when uploading.

Processing...|
Error while processing...
The store was unable to accept this snap.
  - checksums do not match. Please ensure the snap is created with either 'snapcraft snap <DIR>' or 'mksquashfs <dir> <snap> -noappend -comp xz -all-root -no-xattrs -no-fragments'

# snapcraft --version
snapcraft, version 2.40

This was built with electron-builder - which does some under the covers magic to build the snap. It may not be using 2.40, even though that’s what’s installed on my system?


#4

AFAIK, snapcraft receives from the store the error from the store and displays it. Therefore, the output I gave is what people will see. The store does not pass the snapcraft version to the review tools at this time, so we can’t do exactly what you suggest now. However, I think what the tools are doing is enough to steer people to upgrade because it is straightforward to detect when -no-fragments was not used and the snap affected by this.


#5

The store doesn’t have r1025 of the review-tools which has the improved detection and error messages (hopefully in prod next week). You didn’t give the snap name so I can’t look into this further, but you can install the review-tools snap from edge, then do snap-review /path/to/snap to see the improved messaging. For example:

$ snap-review ./tests/test-no-fragments_4.snap 
Errors
------
 - security-snap-v2:squashfs_fragments
	The squashfs was built without '-no-fragments'. If using snapcraft, please upgrade to at least 2.38 and rebuild. Otherwise, please ensure the snap is built using 'mksquashfs <dir> <snap> -noappend -comp xz -all-root -no-xattrs -no-fragments'
./tests/test-no-fragments_4.snap: FAIL

If I were to guess, electron-builder may use snapcraft, then unsquash, fix something up, then run mksquashfs without -no-fragments.


#6

@jdstrand the snap was signal-desktop which I am building locally in a lxc container.


#7

This looks like what electron-builder is doing.


#8

The ‘-b 1048576’ and ‘-Xdict-size 100%’ are not going to work with the resquash test as written. I suppose I should adjust the comment to be “please ensure the snap is built using only ‘mksquashfs -noappend -comp xz -all-root -no-xattrs -no-fragments’”. These both seem wasteful-- why did they choose these?


#9

FYI, I’ve asked that the resquash enforcement be temporarily disabled for two reasons:

  • (at least) electron-builder is running mksquash manually with incompatible arguments
  • (at least) one partner is using snapcraft 2.35 to work around other issues in 2.40 (specifically, patchelf). Since 2.35 doesn’t build with -no-fragments, they now see resquash errors

@popey is working with the partner to move them to 2.40 and working with @sergiusens to address their issues with 2.40.

I will work with electron-builder upstream to have them remove the incompatible mksquashfs options. I will also investigate alternatives for the review-tools to see if they can handle these options more dynamically (I’m not sure we want users to be able to specify the block size anyway, so also need to discuss this with Gustavo).


#10

FYI, it is possible to determine the block size of the snap easily by examining the superblock of the squashfs. It may be possible to determine the dict-size by looking inside the xz stream header in the squashfs (this needs to be investigated; it isn’t exposed in the squashfs superblock). Assuming we can do this, it is theoretically possible to have the review tools examine which -b and -Xdist-size to use for the resquash. To test this, I created a squashfs that was created with ‘-b 1048576 -Xdict-size 100%’, then hardcoded these options into the review tools and was able to run the resquash test and have it pass automated review.

That said, @popey gave me a signal-desktop snap that was affected by this issue (and uses at least -b 1048576) and with the above modified review tools, it failed the resquash test. Over the weekend, the termius-beta snap (assumed (UPDATE: verified) to be an electron snap since it has the same block size as signal-desktop) also failed resquash review and it too failed under the modified review tools. There may be additional options that electron-builder is using that we are unaware of or there is indeterminism in the resulting squashfs when these options are used that would need to be addressed before we could consider block size and dict-size detection in the review-tools.


#11

#12

FYI, this is fixed in electron-builder 20.9.2 stable.

@popey, can you rebuild your snap using this updated electron-builder then run:

$ SNAP_ENFORCE_RESQUASHFS=1 snap-review /path/to/your/snap

and report back?


#13

Ok, building with electron-builder 20.9.2 and snapcraft 2.40. Here’s verification to show that.

root@signal:~# snapcraft  --version
snapcraft, version 2.40
  • electron-builder version=20.9.2
  • loaded configuration file=package.json ("build" field)
  • writing effective config file=release/electron-builder-effective-config.yaml
  • rebuilding native production dependencies platform=linux arch=x64
  • packaging       platform=linux arch=x64 electron=1.8.4 appOutDir=release/linux-unpacked
  • building        target=snap arch=x64 file=release/signal-desktop_1.8.0-beta.1_amd64.snap
  • downloading               file=/root/.cache/electron-builder/snap-templates/electron-template-2.2.snap url=https://github.com/electron-userland/electron-builder-binaries/releases/download/snap-template-2.2/electron-template-2.2.snap

Copied the snap out of the lxc container to the host. Checking they’re the same.

root@signal:~# md5sum tmp.K1xX93jzqp/latest/release/signal-desktop_1.8.0-beta.1_amd64.snap
18acfa04176ab6f64ffd5ca900710f06  tmp.K1xX93jzqp/latest/release/signal-desktop_1.8.0-beta.1_amd64.snap
alan@hal:~$ md5sum signal-desktop_1.8.0-beta.1_amd64.snap
18acfa04176ab6f64ffd5ca900710f06  signal-desktop_1.8.0-beta.1_amd64.snap

Here’s the output of the requested command:

alan@hal:~$ SNAP_ENFORCE_RESQUASHFS=1 snap-review signal-desktop_1.8.0-beta.1_amd64.snap 
Errors
------
 - security-snap-v2:squashfs_repack_checksum
        checksums do not match. Please ensure the snap is created with either 'snapcraft pack <DIR>' (using snapcraft >= 2.38) or 'mksquashfs <dir> <snap> -noappend -comp xz -all-root -no-xattrs -no-fragments'. If using electron-builder, please upgrade to (at least) 20.9.2 stable.
signal-desktop_1.8.0-beta.1_amd64.snap: FAIL


#14

For the benefit of @jdstrand this is how I’m building it.

I setup a lxc container and in it, installed node:

curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash -
apt install nodejs=8.11.1-1nodesource1

Install other bits to build:

apt install make g++ snapcraft
npm install --global yarn

Building the thing.

Here’s a patch I use to enable snap builds. In it we also nudge the version of electron-builder, and disable all other builds because I only need a snap. It’s snap_package_json.patch referred to below.

--- Signal-Desktop/package.json 2018-04-19 21:37:05.136973946 +0000
+++ Signal-Desktop-snap/package.json    2018-04-19 21:39:42.759831008 +0000
@@ -105,7 +105,7 @@
     "bower": "^1.8.2",
     "chai": "^4.1.2",
     "electron": "1.8.4",
-    "electron-builder": "^20.2.0",
+    "electron-builder": "^20.9.2",
     "electron-icon-maker": "0.0.3",
     "electron-publisher-s3": "^20.2.0",
     "eslint": "^4.14.0",
@@ -203,11 +203,13 @@
       },
       "asarUnpack": "node_modules/spellchecker/vendor/hunspell_dictionaries",
       "target": [
-        "deb",
-        "zip"
+        "snap"
       ],
       "icon": "build/icons/png"
     },
+    "snap": {
+      "plugs": ["default", "desktop"]
+    },
     "deb": {
       "depends": [
         "gconf2",

Building it.

#!/bin/bash

rm -rf ~/.cache/snapcraft

SIGNAL_ENV=production
workdir=$(mktemp -d -p ~)

echo "Build tip of master"
cd $workdir
git clone https://github.com/signalapp/Signal-Desktop.git latest
cd latest
patch < ~/snap_package_json.patch
npm install
# Normally we use --frozen-lockfile, but as we nudge e-b version we can't do that
#yarn install --frozen-lockfile
yarn install
yarn grunt
yarn icon-gen
npm run build-release -- -l
ls $workdir/latest/release/*.snap

That should get you a working snap.


#15

I’ve examined the resulting snap and filed a bug with upstream electron-builder: https://github.com/electron-userland/electron-builder/issues/2888


#16

Upstream is preparing a fix for electron. Thanks @develar!

I will be updating the review-tools and add an entry to this topic that the tools will reference.


#17

The resquash test is essentially: checksum the original snap, unsquash the original snap, mksquash the unsquashed files with expected options, checksum the resquashed snap and compare it to the original. When we turned the resquashfs tests to enforce last month, most snaps (by far) passed the resquash test but we discovered a few scenarios where app snaps failed review:

  1. Publishers using snapcraft < 2.38. To remedy, publishers should either:
    a. upgrade to snapcraft >= 2.38 (recommended)
    b. build the snap how you normally would, then run:
    $ unsquashfs /path/to/snap
    $ mksquashfs ./squashfs-root ./path/to/new/snap -noappend -comp xz -all-root -no-xattrs -no-fragments
    (then upload the resulting snap)
    
  2. Publishers using electron-builder <20.14.7. To remedy, publishers should either:
    a. use lastest stable electron-builder (recommended)
    b. build the snap how you normally would, then run:
    $ unsquashfs /path/to/snap
    $ snapcraft pack ./squashfs-root
    (then upload the resulting snap)
    
    c. adjust package.json to use (latest stable will do this for you):
    {
      "devDependencies": {
        ...
        "electron-builder": "^20.9.2",
        ...
      },
      "build": {
        ...
        "linux": {
          ...
          "target": [
            ...
            "snap"
          ],
          ...
        },
        "snap": {
          ...
          "useTemplateApp": false
        },
        ...
      },
      ...
    }
        
    

In all of the above cases we’re simply ensuring that the snap that is published to the store is squashed with the expected options.


Snapcraft review fails, wrong permissions?! electron-builder
#18

FYI, electron-builder is fixed for this issue and 20.14.7 has the fixes. The review-tools have been updated to reference this version as well as point people to the above comment. We’ll be re-enabling the resquash tests soon.


#19

I’ve requested that the store team flip enforcement back on. For snaps that fail the resquash test, the review-tools will communicate what to do to correct the issue.


#20

Today two snaps had resquash failures that need further investigation. The resquashfs check has been temporarily disabled.