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

Add Wayland support #50

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

rhjdvsgsgks
Copy link

i also changed pulseaudio_socket to use ro bind mount. because i found that it will not break anything

Copy link
Owner

@hartwork hartwork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhjdvsgsgks very interesting, thank you! 👍 This will take testing against actual Wayland which so far none of my boxes have — I have X11 —, so it will take a few days.

@@ -273,14 +278,21 @@ def create_bwrap_argv(config):
if config.pulseaudio:
pulseaudio_socket = f'/run/user/{os.getuid()}/pulse/native'
env_tasks['PULSE_SERVER'] = f'unix:{pulseaudio_socket}'
mount_tasks += [MountTask(MountMode.BIND_RW, pulseaudio_socket)]
mount_tasks += [MountTask(MountMode.BIND_RO, pulseaudio_socket)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this works in practice, it will be misleading because audio is read and written. So I would ask to revert this change, for the sake of clarity and realistic expectations (even before testing). We can discuss changing it more if there are good reasons but that should be a dedicated issue and or pull request then. Thank you!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im worried about program in sandbox may able to replace and hijack the socket file if we dont ro-bind it. also ro-bind is suggested by someone else on here https://wiki.archlinux.org/title/Bubblewrap/Examples#Firefox

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im worried about program in sandbox may able to replace and hijack the socket file if we dont ro-bind it.

@rhjdvsgsgks I cannot think of a threat model where you trust the app with audio but not with the audio socket. Please help me understand what I'm missing.

also ro-bind is suggested by someone else on here https://wiki.archlinux.org/title/Bubblewrap/Examples#Firefox

I'm not against the change, I'm against not understanding it and merging things before I understand them. Maybe it just takes playing with socket files more. Maybe there's something for me to learn about socket file permissions here.

@hartwork hartwork changed the title add wayland option Add Wayland support Apr 27, 2024
@hartwork hartwork added the enhancement New feature or request label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants