flathub / com.discordapp.Discord

https://flathub.org/apps/details/com.discordapp.Discord
124 stars 41 forks source link

Default Sandbox Permissions are too Restrictive #99

Closed mindinsomnia closed 1 year ago

mindinsomnia commented 4 years ago

Sorry to necro an old issue from 3 years ago, but I think this needs to be discussed again and reviewed.

The default permissions of Discord do not allow Discord to access all of your files in your Home folder. This restriction is too tight.

Attaching files to send to a conversation is a common functionality of Discord and a basic feature of the software. There's no reason to restrict Discord's access to files, it's a required permission that Discord needs to offer it's complete feature set.

This restriction is not present for Discord on Windows, nor if you install Discord from another source on Linux, such as your distro's own repository, or if you grab the Linux version of Discord from the Discord website. Even if you run Discord via a web browser, you can attach files from anywhere on your PC. This restriction leaves the Flatpak version of Discord inferior to all of the alternatives

To the average user who doesn't have the technical knowhow to google this issue and find an old closed Github issue from 3 years ago with a magic terminal command to run to fix this, they are faced with either uninstalling Discord (Flatpak) and installing it from somewhere else (hurting Flatpak adoption!), or just concluding that 'something is broken' on their PC, or that 'Flatpak doesn't work', or that 'Discord doesn't work on Linux' or some other very negative assumption as there is no visible explanation for why local files are not showing up.

At best they are left googling this issue for 20 minutes to find a solution, reading a Github issue report, and having to run commands they don't understand in the terminal, to enable behaviour that I would argue the majority of users would expect Discord to come with out of the box on Linux, as that behaviour is present for Discord everywhere else.

Likewise Discord should be able to show any current running games you are playing in your status, this too is a basic feature of Discord that users would expect to work, and the reason why some people use Discord in the first place. They shouldn't have to go hunting for a terminal command to run to adjust Flatpak's permissions to enable features in Discord, not when those features are likely the reason they installed Discord in the first place.

There has to be a better solution here, because this is not providing a great UX for average users on Linux.

In my opinion the restrictions should be lifted to give Discord the level of permissions it requires to offer it's full feature set. Anything Discord can do on Linux out of the box when installed from somewhere else, it should be able to do via the Flatpak.

But at the very least the description for Discord on Flathub could be updated to include a note that this version of the application has a very restrictive level of sandbox permissions, with a link to instructions on how to override those permissions. Otherwise users are left in the dark wondering why the application 'isn't working' from their perspective.

Thoughts?

Fuseteam commented 4 years ago

what necro? what old closed issue? i wonder if there was a technical reason certain permissions weren't granted. What permissions does the flatpak lack and which issues addressed them? i would love to read the older issues regarding older permissions to find a solutions to some of these issues i reckon permissions are not the only thing not working in the flatpak, search filter seems broken as well tho i have yet to check if it is flatpak specific

Fuseteam commented 4 years ago

i did some digging at at least the game feature cannot be work around according to #11 so at the very least flatpak is not suitable if one is fine with discord observing the system processes

TingPing commented 4 years ago

The whole file situation just goes away when Electron merges https://github.com/electron/electron/pull/19159

TingPing commented 4 years ago

This package will never support seeing running games. Sorry.

Fuseteam commented 4 years ago

The whole file situation just goes away when Electron merges electron/electron#19159

ohw interesting

mindinsomnia commented 4 years ago

@Fuseteam This issue: https://github.com/flathub/com.discordapp.Discord/issues/10

Because of this restriction, for example, if you try to drag and drop a file into Discord to add it to a conversation, it will simply fail.

The fix is a terminal command that can be applied to remove the restriction that has been placed on Discord, but it seems unreasonable for the average user to somehow figure that out. Discord is a common application for new Linux users to download, I've seen new Linux users easily get confused and annoyed at this issue, there is a huge assumption being made that users will understand exactly why Discord can't attach files from all locations on their PC.

Fuseteam commented 4 years ago

@mindinsomnia i believe that's the situation tingping is referring to

The whole file situation just goes away when Electron merges electron/electron#19159

there is a huge assumption being made that users will understand exactly why Discord can't attach files from all locations on their PC.

it's also fairly none trivial for users to install flatpaks tbh at the very least they'll probably find this repo and file a duplicate issue about it untill the electron merge fixes it

what won't go away is the restriction that discord can't see running processes, so i dunno about this becoming a drop in replacement for the deb. i'm curious what other restrictions you seen so far tho

lolrepeatlol commented 4 years ago

No, this person is right. Flatpaks are too restrictive in their normal state, especially the Discord one. People expect their computer to "just work" and looking up some commands to make normal, expected things work is just not good at all for Linux.

it's also fairly none trivial for users to install flatpaks tbh

That's not true though. At most, it takes like three commands that you do once. At best, like most people, it's as simple as using the distro you have. Pop OS, Elementary OS, and various other popular "newbie" operating systems have Flatpak + Flathub support baked right in.

Fuseteam commented 4 years ago

@lolrepeatlol as already stated the file situation will be resolved once electron fixes the underlying issue but regardless the running games status will not work with flatpaks either way. so the flatpak will probably not become a drop in replacement for deb

also don't take it out of the context, at the very least they should be able to find this repo for the solution

TheEvilSkeleton commented 3 years ago

I really disagree with the title. Discord has been known to gather a lot of your data[1], and even at that, I personally think that the Flatpak is being really generous with filesystem permissions.

Likewise Discord should be able to show any current running games you are playing in your status, this too is a basic feature of Discord that users would expect to work, and the reason why some people use Discord in the first place. They shouldn't have to go hunting for a terminal command to run to adjust Flatpak's permissions to enable features in Discord, not when those features are likely the reason they installed Discord in the first place.

I do agree with the bolded part however. A normal user wouldn't want to adjust the permissions using a terminal. Thankfully there is a GUI permission manager for Flatpak called Flatseal. Now it's up to the community to promote it.

I would advise creating a README file specifically on how to allow the permissions, and/or opening a Reddit thread in r/flatpak so it can be on top of any search engine.

[1] https://spyware.neocities.org/articles/discord.html

Fuseteam commented 3 years ago

flatseal first i heard of it! fair point! maybe you can start reddit thread about it?

TheEvilSkeleton commented 3 years ago

Can you link me to all the issues that people have with permissions? I'd do an all-in-one thread

soredake commented 3 years ago

https://github.com/flathub/im.riot.Riot/pull/136

Fuseteam commented 3 years ago

that looks like a simple enough update

iiuc it just need the "export ZYPAK_FORCE_FILE_PORTAL=1" line somewhere in sources and "--filesystem=xdg-download", can then be removed both in https://github.com/flathub/com.discordapp.Discord/blob/master/com.discordapp.Discord.json

Fuseteam commented 3 years ago

@TheEvilSkeleton this is the only on i know of :p

lionirdeadman commented 3 years ago

I will wait until @refi64 tells me that they want me to do it. I've known about this for quite a while but currently there seems to be some edge cases which don't affect Discord afaik but that could change with an update potentially.

guillermo-st commented 3 years ago

I really disagree with the title. Discord has been known to gather a lot of your data[1], and even at that, I personally think that the Flatpak is being really generous with filesystem permissions. [1] https://spyware.neocities.org/articles/discord.html

I very strongly agree with this. Default discord constantly scans your processes for running games (and who knows what else), I'm honestly glad that flatpak prevents some of that. I don't really like discord when it comes to privacy, that's why I use the browser version which prevents some of it, but maybe the flatpak version is a good alternative :)

Please, don't give the discord flatpak more permissions!

mindinsomnia commented 3 years ago

that's why I use the browser version which prevents some of it, but maybe the flatpak version is a good alternative :)

Please, don't give the discord flatpak more permissions!

If you don't mind me asking, if you don't like what Discord does and use it via a browser for privacy concerns: A) Why do you use Discord at all? B) Why do you care what the desktop version of this application does?

The very feature you're saying you don't like, ie Discord's ability to show in your status for your friends to see the game you're currently playing, is one of the main advertised features of Discord and one of the reasons why many gamers use it in the first place.

Discord is historically a gamer focused chat client and that feature is one of the examples of why. That feature allows for features such as the ability to allow friends to click a button to spectate or join your current game, these are desired features that users of Discord enjoy and when coming from Windows to Linux and installing Discord, would reasonably expect to work.

If you don't want Discord to have any access to your OS, you can, as you are already currently doing, use the browser version, so why castrate the desktop Flatpak version of Discord for those who do want the desktop features?

TheEvilSkeleton commented 3 years ago

If you don't want Discord to have any access to your OS, you can, as you are already currently doing, use the browser version, so why castrate the desktop Flatpak version of Discord for those who do want the desktop features?

There are times when someone will care about others, and that is the case. He knows how bad Discord is, and for this, he's trying to prioritize security, which I appreciate a lot.

The features are for aesthetics; they are not a must-have, but they are fun to have. If people want a "service" that has been known to actively ignore important issues like dis.cool datamining users to constantly scan system processes just for aesthetics, then I would prefer it being opt-in and not opt-out.

lionirdeadman commented 3 years ago

If you don't want Discord to have any access to your OS, you can, as you are already currently doing, use the browser version, so why castrate the desktop Flatpak version of Discord for those who do want the desktop features?

My understanding is that this feature cannot work without touching the host hence there is a wiki article about it if you want this feature. There is no castrating being done. It simply does not work in a sandbox without messy workarounds needing to be done.

It might be possible to enable this feature by default with a script but I personally will be against this because it seems that quite a few users have privacy concerns surrounding this feature.

mindinsomnia commented 3 years ago

There are times when someone will care about others, and that is the case. He knows how bad Discord is, and for this, he's trying to prioritize security, which I appreciate a lot.

The features are for aesthetics; they are not a must-have, but they are fun to have. If people want a "service" that has been known to actively ignore important issues like dis.cool datamining users to constantly scan system processes just for aesthetics, then I would prefer it being opt-in and not opt-out.

Here's the thing though.

Discord is already opt-in, you opt-in by installing Discord. If you don't want Discord scanning your PC, don't install Discord.

But here's the overall grand point I'm trying to make:

A) The sandboxing seems to be arbitrary B) There's no communication on which applications are sandboxed and to what degree

Who decides what features are for aesthetics that can be broken and which ones aren't?

I don't consider a feature like displaying a user's active game in a gaming focused chat client or drag and drop support for attaching files to be an aesthetic, so what objective process exists for determining what is or isn't an acceptable feature to break?

These decisions are being made without notifying users. There's nothing informing users that these features are broken. Nor are they being informed of the reason why or who is responsible. The user might fairly assume it's some kind of bug or error, because sandbox restrictions imposed by Flathub are not clearly indicated for each application.

This is the Flathub page for Discord. There's no mention anywhere on that page that this application is sandboxed in a way which breaks some of Discord's features. This is pretty important information from a user's perspective and should be the most important line of visible text on the page.

If sandboxing was the same for all applications, there wouldn't need to be any mention of the sandboxing. But the sandboxing is not the same for all applications.

Firefox, Blender and VLC for example don't have these same restrictions. For seemingly arbitrary reasons, Discord does have this sandboxing.

I would prefer it being opt-in and not opt-out.

There's no accessible settings offered to choose what level of sandboxing you want when installing software or even a notification of the sandboxing.

There's no 'opt-in' as you put it, there's only an opt-out, which is on by default, and which the user doesn't even know about, until they ask why a feature is broken, and google the problem.

Unless a user is willing to go googling the problem, discover the reason why some of Discord's features are broken, then enable them with some terminal commands, they can't 'opt-in'.

For me as a user, speaking personally, it means every time I consider installing an application via Flathub, I have to wonder what kind of sandbox restrictions have been imposed that I don't know about, what features might be broken in the software as a result, and at that point I decide if it's worth it to install the software via Flathub and risk those potential issues, or if it would just save time to get the software instead from another source which I am more confident with.

I don't believe this is a positive thing for the user experience of using Flathub.

refi64 commented 3 years ago

First off, saying you opt in to Discord by installing it makes no sense. I may want to use global push to talk for classes that communicate on Discord to say, show my terminal, which requires the native client...so that implicitly means I accept all the permissions that aren't relevant? I understand your point, but it has nothing to do with being opt-in.

Having the ability to show sandboxing-imposed restrictions or other documentation links on Flathub would be nice (most particularly to make the rich presence info more easily accessible), but that doesn't exist right now, and we can't just poke sandbox holes for everything until it does.

That being said...having /proc be exposed by default would be terrible for completely unrelated reasons. I don't even think it works anymore, as it was never really an intended Flatpak feature... Reason is, by exposing the host's /proc, sandboxed applications will be unable to access info on any other processes in the sandbox via procfs, because the PIDs on the host side are different vs inside the PID namespace. If this still works, it's bound to crash and burn terribly at some point in time anyway.

Maybe we could have a running applications portal of some sort, or you can use one of the existing solutions that talk to the rich presence socket from the host end, but none of that is actionable on this repo's end.

On Tue, Oct 27, 2020, 10:22 PM mindinsomnia notifications@github.com wrote:

There are times when someone will care about others, and that is the case. He knows how bad Discord is, and for this, he's trying to prioritize security, which I appreciate a lot.

The features are for aesthetics; they are not a must-have, but they are fun to have. If people want a "service" that has been known to actively ignore important issues like dis.cool datamining users to constantly scan system processes just for aesthetics, then I would prefer it being opt-in and not opt-out.

Here's the thing though.

Discord is already opt-in, you opt-in by installing Discord. If you don't want Discord scanning your PC, don't install Discord.

But here's the overall grand point I'm trying to make:

A) The sandboxing seems to be arbitrary B) There's no communication on which applications are sandboxed and to what degree

Who decides what features are for aesthetics that can be broken and which ones aren't?

I don't consider a feature like displaying a user's active game in a gaming focused chat client or drag and drop support for attaching files to be an aesthetic, so what objective process exists for determining what is or isn't an acceptable feature to break?

These decisions are being made without notifying users. There's nothing informing users that these features are broken. Nor are they being informed of the reason why or who is responsible. The user might fairly assume it's some kind of bug or error, because sandbox restrictions imposed by Flathub are not clearly indicated for each application.

This is the Flathub page for Discord. https://flathub.org/apps/details/com.discordapp.Discord There's no mention anywhere on that page that this application is sandboxed in a way which breaks some of Discord's features. This is pretty important information from a user's perspective and should be the most important line of visible text on the page.

If sandboxing was the same for all applications, there wouldn't need to be any mention of the sandboxing. But the sandboxing is not the same for all applications.

Firefox, Blender and VLC for example don't have these same restrictions. For seemingly arbitrary reasons, Discord does have this sandboxing.

I would prefer it being opt-in and not opt-out.

No there's accessible settings offered to choose what level of sandboxing you want when installing software or even a notification of the sandboxing.

There's no 'opt-in' as you put it, there's only an opt-out, which is on by default, and which the user doesn't even know about, until they ask why a feature is broken, and google the broken.

Unless a user is willing to go googling the problem, discover the reason why some of Discord's features are broken, then enable them with some terminal commands, they can't 'opt-in'.

For me as a user, speaking personally, it means every time I consider installing an application via Flathub, I have to wonder what kind of sandbox restrictions have been imposed that I don't know about, what features might be broken in the software as a result, and at that point I decide if it's worth it to install the software via Flathub and risk those potential issues, or if it would just save time to get the software instead from another source which I am more confident with.

I don't believe this is a positive thing for the user experience of using Flathub.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flathub/com.discordapp.Discord/issues/99#issuecomment-717670508, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4YSPGYEIUO6VT7QXXNHTSM6E5TANCNFSM4PEZ2GAQ .

lionirdeadman commented 3 years ago

That being said...having /proc be exposed by default would be terrible for completely unrelated reasons. I don't even think it works anymore, as it was never really an intended Flatpak feature... Reason is, by exposing the host's /proc, sandboxed applications will be unable to access info on any other processes in the sandbox via procfs, because the PIDs on the host side are different vs inside the PID namespace. If this still works, it's bound to crash and burn terribly at some point in time anyway.

AFAIK /proc is not used by Discord anymore and Flatpak intentionally doesn't allow access nowadays.

Also, I entirely agree that this makes for a worse UX for those who want these things but currently there is no way to have this kind of dynamic with permissions and so, it depends on the maintainer how much they want to make holes. I personally think the minimum amount of holes should be made by default with Discord.

TheEvilSkeleton commented 3 years ago

Discord is already opt-in, you opt-in by installing Discord. If you don't want Discord scanning your PC, don't install Discord.

That is like saying that Zoom is opt-in because you install Zoom for schoolwork, business, etc. even if you want to refuse to use it. Not everyone wants to use it, but sometimes there are stuff we have to use it.

A) The sandboxing seems to be arbitrary B) There's no communication on which applications are sandboxed and to what degree

Yes there is. You can tell when you're trying to send files from ~/Videos that it will give you an error, or when you play a game but it doesn't show up in Discord, etc. We will never know the exact amount, but we can have an accurate result.

These decisions are being made without notifying users. There's nothing informing users that these features are broken. Nor are they being informed of the reason why or who is responsible. The user might fairly assume it's some kind of bug or error, because sandbox restrictions imposed by Flathub are not clearly indicated for each application.

100% agree. Flatpak and the end-user are not well connected, but that's also an issue in GNU/Linux because of the constant amount of disagreement in the community. Flatpak can't just create a dialog to ask for permissions like how Android and iOS do, or an alternative to that.

Firefox, Blender and VLC for example don't have these same restrictions. For seemingly arbitrary reasons, Discord does have this sandboxing.

Those are different sets of applications so they are not really relevant. Blender is a 3D video editing software, so it makes sense for it to need access to need different directories. For VLC, I do agree that they should restrict to xdg-videos only. Fyi Firefox only has access to xdg-downloads, so it's far more restrictive than Discord.

mindinsomnia commented 3 years ago

I do sincerely appreciate the technical explanations of the reasons why it is difficult to allow aspects Discord's functionality to operate from within a sandbox. Having some background on the issue is nice and will no doubt be helpful to others as well if they come across this issue and wish to have some perspective on why things are the way they are.

Maybe we could have a running applications portal of some sort, or you can use one of the existing solutions that talk to the rich presence socket from the host end, but none of that is actionable on this repo's end.

That particularly sounds like an interesting solution.

Having the ability to show sandboxing-imposed restrictions or other documentation links on Flathub would be nice

This too would be an ideal solution. What would be the best process for perhaps recommending this as a feature?

In the mean time, as a stopgap solution, would it be possible to just modify the description text of this application on Flathub to make mention of the limitations imposed by the sandbox? That would only require a modification of the com.discordapp.Discord.appdata.xml file?

lionirdeadman commented 3 years ago

In the mean time, as a stopgap solution, would it be possible to just modify the description text of this application on Flathub to make mention of the limitations imposed by the sandbox? That would only require a modification of the com.discordapp.Discord.appdata.xml file?

Yep, feel free to PR that if you want. I'd just like it to mention that wiki post if people want the game functionality and that they can change filesystem permissions with flatseal or flatpak override although that shouldn't be neccessary for too long assuming either Electron or Zypak come to the rescue for a file portal.

mindinsomnia commented 3 years ago

Yep, feel free to PR that if you want. I'd just like it to mention that wiki post if people want the game functionality and that they can change filesystem permissions with flatseal or flatpak override although that shouldn't be neccessary for too long assuming either Electron or Zypak come to the rescue for a file portal.

What kind of wording would you suggest for this information?

Perhaps:

Note: Flatpak'd applications run in a sandbox to provide better safety and privacy for users, however the sandboxing of Discord prevents the following Discord features from working:

  • Game Activity (Flatpak'd Discord can not scan running processes to detect running games, there is currently no workaround or solution for this limitation.)
  • Unlimited file access (Default sandbox permissions limit Discord to only certain directories and not your entire Home directory, this limits which file directories you can attach files from, and impacts drag and drop functionality. You can change sandbox permissions of installed Flatpak'd applications with Flatseal, to give Discord broader file system access to allow file attachments from more locations.)
  • Rich Presence (How to expose Discord's rich presence interface for other applications.)

Does that sound correct?

TheEvilSkeleton commented 3 years ago

Flatpak'd applications

"flatpak applications" (with a lowercase "F") sounds better tbh.

Other than that, it looks good to me.

Fuseteam commented 3 years ago

i would say i agree with the notion that all features of discord should work out of the box if at all technically possible. and if not those features should be documented front and center so users can make an informed decision. tho it seems i'm a little late to the discussion xD

geekley commented 3 years ago

Did you guys merge the PR? I'd suggest to put this info in a README on this repo too. If it was up to me, I'd also include the mentioned link on that notice somewhere, at least in the README, like for example:

Note: flatpak'd applications run in a sandbox to provide better safety and privacy for users (see this privacy report),


This restriction leaves the Flatpak version of Discord inferior to all of the alternatives

For me it was the exact opposite. This thread made realize I need to switch to the flatpak version now, because I really do care a lot about privacy (that's one of the main reasons I'm using Linux in the first place). I was considering snap, but now I guess I'll go with flatpak. Thanks, guys!

lionirdeadman commented 3 years ago

Did you guys merge the PR? I'd suggest to put this info in a README on this repo too.

What PR? I don't believe a PR was made.

The plan is still to wait for Zypak to offer force file portal usage and remove all filesystem access.

geekley commented 3 years ago

@lionirdeadman I meant mindinsomnia's note above: https://github.com/flathub/com.discordapp.Discord/issues/99#issuecomment-717701849

In the mean time, as a stopgap solution, would it be possible to just modify the description text of this application on Flathub to make mention of the limitations imposed by the sandbox?

lionirdeadman commented 3 years ago

Ah no, no such note is on Flathub.

ghost commented 3 years ago

The reason I run Discord through Flatpak is to have it restricted in the first place. I wouldn't install in the first place, if the browser app wouldn't conveniently break every couple of months on me. Adding some notes regarding how it is restricted would be nice though.

Fuseteam commented 3 years ago

@ceLoFaN no rich presence and no restricted file uploads

geekley commented 3 years ago

I'm adding that info in a README. Please check to make sure I didn't write something stupid or incorrect (specially the "not affiliated with" part). I think the owners should also add a LICENSE on the repo. If everything is right, you can copy the text (or at least the "not affiliated with" part) to appdata.xml as well so it appears on the site.

ghost commented 3 years ago

@ceLoFaN no rich presence and no restricted file uploads

I don't know how the sandbox is implemented (I'm just a casual user) but I imagine if file uploads are not restricted, the Discord app could potentially scan your local files without you even being aware of it. So I just use the browser version (when it actually works) instead.

The browser also makes sure that Discord can't listen to your keystrokes when it is unfocused or that it can't record your screen without you allowing it (streaming worked even on Firefox last time I tried it). With the Flatpak version it wasn't clear to me how I could restrict this behaviour on KDE.

But yes, for people who don't care about this stuff I can see how it could be an annoyance.

ssokolow commented 3 years ago

@ceLoFaN Currently, it gives Discord read access to your videos and pictures folders and read-write access to your downloads folder.

        "--filesystem=xdg-videos:ro",
        "--filesystem=xdg-pictures:ro",
        "--filesystem=xdg-download",

(The locations of those three folders are set in ~/.config/user-dirs.dirs)

I believe Discord's app is Electron-based and, if that's so, then it should be possible to lock it down further once this PR makes it into Electron and then into Discord's build.

(That PR changes Electron to use GtkFileChooserNative, which automatically gets redirected through the file-chooser portal when run under Flatpak. If you then remove access to the filesystem, it results in an experience similar to how Open/Save dialogs work in the browser.)

soredake commented 3 years ago

https://github.com/flatpak/flatpak/issues/3922

ghost commented 3 years ago

@ssokolow Thanks for explaining, and sorry for my ignorance! Not sure if this is the right place to ask, but what about the listening to keystrokes and screen recording part?

ssokolow commented 3 years ago

To fix that properly, you need Wayland (which that flatpak isn't configured to support... just "--socket=x11",).

The capability to read and write the event stream and rendered window buffers of other processes is inherent in X11's design and Chrome (which Electron is based on) breaks if you try to run it in with the optional X11 SECURITY extension enabled.

(It's mentioned in Firejail's documentation as part of explaining that you have to use Firefox if you want to properly sandbox your browser on X11.)

That's one of the reasons Wayland exists. To force a "you either fix your application to work with secure APIs or you're not getting in" situation on application developers.

However, you could try launching the Flatpak under Xpra. (It's sort of like GNU Screen or tmux but for X sessions and it's one of the choices offered by Firejail.)

That said, bear in mind the in-browser version will still have the following advantages:

lionirdeadman commented 3 years ago

I believe Discord's app is Electron-based and, if that's so, then it should be possible to lock it down further once this PR makes it into Electron and then into Discord's build.

Right but thanks to Zypak, we can hack this in earlier so it'll actually be done with #132

To fix that properly, you need Wayland (which that flatpak isn't configured to support... just "--socket=x11",).

Currently Electron doesn't have Wayland support due to problems supporting both at the same time but once it's done, we'll use fallback-x11 so if a wayland socket exists, it won't have access to x11.

Worth mentioning also that on a Wayland system, x11 programs can't see the screen, input (unless focused) or other applications using Wayland (but they can see all other x11 programs). So if Discord is your only program, there's actually mostly nothing to worry about in that regard.

Fuseteam commented 3 years ago

@ceLoFaN no rich presence and no restricted file uploads

I don't know how the sandbox is implemented (I'm just a casual user) but I imagine if file uploads are not restricted, the Discord app could potentially scan your local files without you even being aware of it. So I just use the browser version (when it actually works) instead.

The browser also makes sure that Discord can't listen to your keystrokes when it is unfocused or that it can't record your screen without you allowing it (streaming worked even on Firefox last time I tried it).

fwiw it doesn't work like that, in fact discord is a electron app; it runs in a browser by default

ssokolow commented 3 years ago

@Fuseteam It depends on how you look at it. Despite browsers coming first, you could say that the browser provides a more locked down subset of the APIs that Electron exposes and, in that sense, it is correct.

lionirdeadman commented 3 years ago

With the zypak strategy not working, I have to rethink what level of filesystem access I want to give to Discord.

While this issue does discuss the subject, I feel this issue has derailed a bit and it would be hard to poll people on it so I've made another issue exclusively for file system access. I hope you understand.

https://github.com/flathub/com.discordapp.Discord/issues/134

vchernin commented 3 years ago

As mentioned in #134 the solution now is to wait for Discord to upgrade to Electron 14, which will let the file picker pick any file in the system securely. See https://github.com/flathub/im.riot.Riot/issues/33#issuecomment-861798834 (for Element, another Electron app)

Thanks to https://github.com/electron/electron/pull/19159, this issue should be fixed with the release of Electron 14. The file chooser XDG desktop portal will be used instead of the traditional file chooser. The portal can choose any file without giving Element entire file system access.

Electron 14 is scheduled for release on August 31st 2021. It might take some time for Element to update to Electron 14 after it is released (usually they’re pretty quick though).

When that is released we shouldn’t need to grant any file system access to Element Flatpak, since the portal makes any file system access redundant. Drag and drop of any file should also work with other Electron 14 and GTK4 apps (I believe the portal hasn’t been backported to GTK 3).

From a brief search, it looks like Discord is already working on Electron 13 (its also in discord canary), so hopefully they will be quick once 14 is released. However the current app is shipping Electron 9 (open console in developer tools, then enter navigator.userAgent). So hopefully they'll update their dependencies sooner or later...

Other sandbox issues like letting Discord see running processes are tracked here.

lionirdeadman commented 3 years ago

Yeah, Discord is shipping Electron 13 on Discord Canary. I'm not sure that they will update quickly to Electron 14. I would be enclined to say "Yeah, they will!" because the NodeJS version will be on the same major version but that's not what the precedent shows. They've used Electron 9 even though Electron 11 is using the the same major version of Node (different minor version).

I'm personally really disappointed that Electron project decided to not backport the file picker change. That said, being on the same major node version should make it easier if I'm allowed to ship a different version of Electron (which I doubt will be the case, it's very unlikely).

ljmf00 commented 3 years ago

This limits a very basic functionality of the application and the majority of the newcomers don't really care about this. Even if they care, they probably don't have the knowledge to reconfigure it easily.

No one with some sanity uses the Discord package and web version at the same time just to share files. People who don't know how to alleviate the sandbox permissions will probably just uninstall flatpak package and install the distribution-specific one.

lionirdeadman commented 3 years ago

I am sorry but I'll have to disagree. I strongly believe that security by default is the saner option. I've decided to make this blog post to explain all of the permissions that are done in the Discord package, you may read it here.

ljmf00 commented 3 years ago

I am sorry but I'll have to disagree. I strongly believe that security by default is the saner option. I've decided to make this blog post to explain all of the permissions that are done in the Discord package, you may read it here.

I understand your point although this can be done only when possible and doesn't compromise the basic functionality of the application. I'm pretty sure a lot of people uninstalled this package because of this behaviour and ended up installing a more "insecure" version.

I think this approach can be rather done in the future when proper support for files is added.

That is just an opinion tho. I don't use flatpak, just found out this when helping someone with it.