Untrusted desktop entry file (escaping issue)

[develar]

Version: v1.2.2

Linux 4.18.0-22-generic #23~18.04.1-Ubuntu SMP Thu Jun 6 08:37:25 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

Just run AppImage, accept to integrate it, and... no launcher in menu.

[TheAssassin]

That's quite odd, first time I see it. How can we make it "trusted"? Hasn't it been marked as executable?

[develar]

Here I am just a user, I didn't investigated what's "untrusted" means. The same effect was for electron-builder desktop integration script. But I didn't fix electron-builder version, because decided to use AppImageLauncher instead (this version of electron-builder not yet released).

It is ubuntu linux 18.04.2 LTS (Bionic Beaver) (plus all updates apt-get update && apt-get upgrade) installed as VM for Parallel Desktop for macOS.

Hasn't it been marked as executable?

Access: (0755/-rwxr-xr-x) Uid: ( 1000/ develar) Gid: ( 1000/ develar). So, it is executable.

[develar]

Hmm.... I cannot reproduce for https://github.com/AkashaProject/Community/releases/download/0.7.2/AKASHA-0.7.2-x86_64.AppImage, maybe something wrong with AppImage generated by electron-builder.

[TheAssassin]

Can you post a link to your "broken" AppImage? I'll have a look.

By the way, talking bout new projects, check out https://github.com/TheAssassin/appimagelint/.

[develar]

https://drive.google.com/file/d/1-dQ35KD4j61nP0Lr3iNRvx65WNaj2P67/view?usp=sharing

[develar]

appimagelint.cli[13313] [INFO] Running check "GNU libc ABI check"
appimagelint.glibc_abi_check[13313] [INFO] detected required version for runtime: 2.3.3
appimagelint.glibc_abi_check[13313] [INFO] detected required version for payload: 2.17
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Debian oldstable (jessie)
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Debian stable (stretch)
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Debian testing (buster)
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Debian unstable (sid)
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Ubuntu trusty
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Ubuntu xenial
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Ubuntu bionic
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Ubuntu cosmic
appimagelint.glibc_abi_check[13313] [INFO] [✔] AppImage can run on Ubuntu disco
appimagelint.cli[13313] [INFO] Running check "GNU libstdc++ ABI check"
appimagelint.glibcxx_abi_check[13313] [INFO] detected required version for runtime: <none>
appimagelint.glibcxx_abi_check[13313] [WARNING] could not find any dependencies, skipping check
appimagelint.cli[13313] [INFO] Running check "Icons validity and location check"
appimagelint.icon_check[13313] [INFO] Extracting icon name from desktop file: /tmp/.mount_electr3Ksnjt/electron-webpack-quick-start.desktop
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/electron-webpack-quick-start.png
appimagelint.icon_check[13313] [INFO] [✔] Valid icon in AppDir root
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/.DirIcon
appimagelint.icon_check[13313] [INFO] [✔] Valid icon file in .DirIcon
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/usr/share/icons/hicolor/128x128/apps/electron-webpack-quick-start.png
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/usr/share/icons/hicolor/16x16/apps/electron-webpack-quick-start.png
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/usr/share/icons/hicolor/256x256/apps/electron-webpack-quick-start.png
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/usr/share/icons/hicolor/32x32/apps/electron-webpack-quick-start.png
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/usr/share/icons/hicolor/48x48/apps/electron-webpack-quick-start.png
appimagelint.icon_check[13313] [INFO] Checking resolution of icon: /tmp/.mount_electr3Ksnjt/usr/share/icons/hicolor/64x64/apps/electron-webpack-quick-start.png
appimagelint.icon_check[13313] [INFO] [✔] Other integration icons valid

Everything is ok except WARNING] could not find any dependencies, skipping check and icons for 96/24 sizes (removed it, looks like it is not needed).

[TheAssassin]

Good. There's no desktop file check yet, unfortunately, but I will add one soon-ish. That'll be quite useful to you, since you don't use appimagetool, which comes with some checks.

[TheAssassin]

According to some StackOverflow issues, this dialog goes away when a desktop file is marked executable. That's what AIL is supposed to do. I will test this nevertheless on Ubuntu 18.04.
The desktop file in your AppImage looked good.

[develar]

@TheAssassin Still, something is wrong or in electron-builder AppImage, or in AppImageLauncher, because I don't see "Open with AppImageLauncher" in context menu. I will investigate next week.

[TheAssassin]

Strange, seems right over here:

> xxd electron-webpack-quick-start-0.0.0_00000000000000000000000000000000.AppImage | head -n1
00000000: 7f45 4c46 0201 0100 4149 0200 0000 0000  .ELF....AI......
> xdg-mime query filetype electron-webpack-quick-start-0.0.0_00000000000000000000000000000000.AppImage 
application/vnd.appimage

Can you please run the second command and check whether AIL's MIME type definitions at least work? Might be a bug in Ubuntu's desktop, and we need to update some caches or so.

[develar]

develar@develar-ubuntu:~$ xdg-mime query filetype ~/Desktop/electron-webpack-quick-start\ 0.0.0.AppImage 
application/vnd.appimage

Reported correctly, but still no option in menu :)

[develar]

I updated to AppImageLauncher v1.3.1.

There is an option in context menu if I download via firefox.

Still, app is not added to menu. And in the generated desktop file I see

TryExec=/home/develar/Applications/electron-webpack-quick-start 0.0.0 (copy)_00000000000000000000000000000000.AppImage

but from what I know, spaces for TryExec must be escaped with \s

and spaces in Exec must be escaped using double quotes.

Exec="/home/develar/Applications/electron-webpack-quick-start 0.0.0 (copy)_00000000000000000000000000000000.AppImage"

after these fixes to generated desktop file (and making it trusted), app can be run.

[TheAssassin]

Interesting. Not that it'd excuse the failure, but why bother using spaces in paths at all?

I'll have to test that later again.

[develar]

but why bother using spaces in paths at all?

1. I have no idea why AppImageLauncher decided to use electron-webpack-quick-start 0.0.0 instead of electron-webpack-quick-start ;) Looks like a bug. In desktop file Name=electron-webpack-quick-start, so, I assume it is bug on AppImageLauncher side.
2. Even if you will fix bug "append version to app name with space", still, we cannot say to users "do not use space in your app name".

update: it seems AppImageLauncher uses file name as app name instead of name in a desktop file :(

[TheAssassin]

update: it seems AppImageLauncher uses file name as app name instead of name in a desktop file :(

Yes, that's the idea.

Even if you will fix bug "append version to app name with space", still, we cannot say to users "do not use space in your app name".

Of course, as I stated in my last comment, this shall not be an excuse for broken behavior. Was just a bit curious why people auto-generate filenames with spaces, given all the issues they regularly produce.

[develar]

@TheAssassin Default artifact pattern for AppImage uses space. MyApp 1.2.1.AppImage looks better (so vague and questionable ;)) on desktop, than Tunepack-1.2.1.AppImage. But

1. as now I don't see any strong reason why it was implemented so initially,
2. and AppImageLauncher in any case moves app to special place
3. and Linux build is not using productName electron-userland/electron-builder#1895 (comment)

in electron-builder 21 it is changed.

    -         "path": "Test App ßW 1.1.0.AppImage",
    +         "path": "Test App ßW-1.1.0.AppImage",

[TheAssassin]

Let me clarify I don't insist on filenames being spaceless, that's really not the case here. Just was a bit curious there. Our tools also embed spaces in generated filenames if e.g., the desktop file contains spaces.

[TheAssassin]

Seems like it's the Exec key also needs to be wrapped in quotes to escape the spaces. I guess we need some actual escaping lib for that purpose. AFAIR TryExec does not need any special escaping, since it just contains a path, and no potential additional parameters. My experiments confirmed that.

Therefore:

• GNOME 3 hides the entry not because of TryExec but because the path in Exec does not exist (it does not take the space as part of the path)
• we need some escaping/quoting for paths in Exec (quoting should probably be done by default, we need to check how to escape " characters, though)
• TryExec does not need any further actions.

@develar it would be really helpful if you could verify my findings. Just put quotes around the Exec and check if it works then.

[develar]

I have enough experience in this area, so, I can fully confirm, that:

1. Exec must be always escaped using " if contains characters like space. Otherwise you cannot run app.
2. In TryExec space must be always escaped using \s. Otherwise you cannot run app.
3. Be aware about Exec must be without quotes in app.desktop file electron-userland/electron-builder#2759 (comment) xdg-utils bug. So, you should escape Exec only if it is really needed as workaround.

Just put quotes around the Exec and check if it works then.

I already did it :) This change in electron-builder was driven by a fact, that I cannot figure out how to generate escaped TryExec — AppImage/AppImageKit#948 (comment)

So, not clear what I should confirm — it is clear that right now AppImageLauncher can generate unescaped Exec and TryExec and it leads to a critical bug, that you don't see app in menu (because Desktop file is invalid) and cannot run it.

electron-builder always produces desktop file Exec=AppRun (and don't set TryExec anymore, because it is AppImageLauncher responsibility now).

[develar]

I can confirm that AppImageLauncher and electron-builder 21+ produced AppImage are ready for production use.

1. If you go to site and click on AppImage to download it, Firefox will show you Open Dialog. If you select "Open with", then AppImageLauncher dialog will be shown, and option "Integrate and run" works — app will be added to menu.

2. If you select "Save file", then AppImage file is downloaded to ~/Downloads, and context menu contains correct "Open With AppImageLauncher". And click on file leads to correct AppImageLauncher dialog.

So, thanks a lot of for AppImageLauncher, it works. You can close this ticket and open a new one about escaping spaces in Exec/TryExec.

I assume "Untrusted desktop entry file" related to weird Ubuntu bugs because of file permissions/whatever (when you copy file from macOS to Linux desktop). Regular users will download AppImage file from web.

[TheAssassin]

Well the core bug hasn't been resolved yet, we still don't escape things properly yet. We must fix that, but it's not as much of a prio, I guess.

I do not necessarily have to need to escape spaces in TryExec= by the way, but I guess it won't hurt either. Thanks for your help.

[azubieta]

Exec entries scaping is supported on xdg-utils-cxx see:
https://github.com/azubieta/xdg-utils/blob/3be709f8975438fc51e2c81ab549d8ce434d5d80/tests/DesktopEntry/TestDesktopEntryExecValue.cpp#L20-L25

But it's not used on AIL as far as I know.

[TheAssassin]

libappimage 1 will be used soon-ish, since I plan to make the change with version 2.0. See the milestone above.

[TheAssassin]

Should be resolved since #214.