Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set SUID bit on chrome-sandbox for Debian #10580

Merged
merged 2 commits into from Aug 16, 2019

Conversation

@jryans
Copy link
Member

jryans commented Aug 16, 2019

This tweaks Linux packages for Riot to SUID chrome-sandbox after install. This
is required as of Electron 5 for certain distros, such as Debian.

This change has also been provided to electron-builder upstream, so ideally
they'll include it in the future and this becomes redundant.

Fixes #10509

This tweaks Linux packages for Riot to SUID `chrome-sandbox` after install. This
is required as of Electron 5 for certain distros, such as Debian.

This change has also been provided to `electron-builder` upstream, so ideally
they'll include it in the future and this becomes redundant.

Fixes #10509
@jryans jryans requested a review from vector-im/riot-web Aug 16, 2019
@dbkr
dbkr approved these changes Aug 16, 2019
Copy link
Member

dbkr left a comment

lgtm, although presumably this is duplicating the whole after-install file because the custom one replaces the standard one, so the comment means "delete this whole file" rather than, "remove this line"?

@jryans

This comment has been minimized.

Copy link
Member Author

jryans commented Aug 16, 2019

presumably this is duplicating the whole after-install file because the custom one replaces the standard one, so the comment means "delete this whole file" rather than, "remove this line"?

Yes, precisely. I'll tweak the comment to make that more obvious.

@jryans jryans merged commit 8824f7e into develop Aug 16, 2019
1 of 4 checks passed
1 of 4 checks passed
buildkite/riot-web/pr Build #783 started
Details
buildkite/riot-web/pr/eslint-lint Started...
Details
buildkite/riot-web/pr/i18n Started...
Details
buildkite/riot-web/pr/pipeline Passed (8 seconds)
Details
@batterystaples

This comment has been minimized.

Copy link

batterystaples commented Sep 14, 2019

This issue prevents Riot from working when it is installed. Although the fix is simple (change the permissions of the chrome-sandbox file), many users who are not technical will install it, see that it doesn't work, and then give up. It also breaks things for users upgrading from an older version. This issue is significant enough that (in my view) it would justify making a special update to the packages in the repository just to fix this issue as soon as possible.

@jryans

This comment has been minimized.

Copy link
Member Author

jryans commented Sep 16, 2019

This issue prevents Riot from working when it is installed. Although the fix is simple (change the permissions of the chrome-sandbox file), many users who are not technical will install it, see that it doesn't work, and then give up. It also breaks things for users upgrading from an older version. This issue is significant enough that (in my view) it would justify making a special update to the packages in the repository just to fix this issue as soon as possible.

I don't understand if you are saying this change fixes such an issue (which it was meant to) or creates a new one... If an issue exists still, please open a new issue so we can investigate.

@batterystaples

This comment has been minimized.

Copy link

batterystaples commented Sep 16, 2019

This change fixes an issue. However, the old, unfixed version is still in the official repositories, which was the point behind my comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.