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

[Signal bridge] Attachements folder does not seem to be created by default #805

Closed
tchapi opened this issue Jan 22, 2021 · 12 comments
Closed
Labels

Comments

@tchapi
Copy link

tchapi commented Jan 22, 2021

Hi

I'm not sure if this belongs here or in https://github.com/tulir/mautrix-signal but when installing the Signal bridge, the attachments folder is not created by default, and thus sharing files from Matrix to Signal fails :

Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:     await handler_func(event)
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:   File "/usr/lib/python3.8/site-packages/mautrix/bridge/matrix.py", line 443, in int_handle_event
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:     await self.handle_message(evt.room_id, evt.sender, evt.content, evt.event_id)
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:   File "/usr/lib/python3.8/site-packages/mautrix/bridge/matrix.py", line 282, in handle_message
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:     await portal.handle_matrix_message(sender, message, event_id)
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:   File "/usr/lib/python3.8/site-packages/mautrix_signal/portal.py", line 189, in handle_matrix_message
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:     attachment_path = await self._download_matrix_media(message)
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:   File "/usr/lib/python3.8/site-packages/mautrix_signal/portal.py", line 163, in _download_matrix_media
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]:     with open(path, "wb") as file:
Jan 22 12:06:14 vps-49bcf83d matrix-mautrix-signal[15516]: FileNotFoundError: [Errno 2] No such file or directory: '/signald/attachments/mautrix-signal-963b2bf5-b5df-4469-8b90-a4eb0a33751c'

Creating the missing folder in the running container seems to fix it:

docker exec -it matrix-mautrix-signal bash

# in the container
mkdir /signald/attachments
chown 998:1001 /signald/attachments

Thanks !

@spantaleev
Copy link
Owner

This bridge seems to start as root and likely drop access by itself later on.

This is not the best way to do it, and we should try to redo it, if possible (using --user for our docker run command).

Because.. we create this /siglad directory (/matrix/mautrix-signal/config outside of the container) using matrix:matrix ownership, and then it's something completely different in the container. If we were to make these match (by running the container as --user={{ matrix_user_uid }}:{{ matrix_user_gid }}), then it would probably be able to create these subdirectories by itself?

In case we need to create them explicitly anyway, we can adjust the playbook to do this.

I'd also rename a few things.. Calling the directory /matrix/mautrix-signal/config, while it obviously stores data doesn't sound great. Our other services split the config and data into separate directories.

What you can try doing is:

  • making a backup of /matrix/mautrix-signal (e.g. cp -ar /matrix/mautrix-signal /matrix/mautrix-signal-backup)
  • stopping the bridge's systemd services (systemctl stop matrix-mautrix-signal*)
  • adjusting roles/matrix-bridge-mautrix-signal/templates/systemd/matrix-mautrix-signal.service.j2 to add --user={{ matrix_user_uid }}:{{ matrix_user_gid }}
  • chowning the existing data on the server (chown -R matrix:matrix /matrix/mautrix-signal/config)
  • re-running the playbook (... --tags=setup-all,start)

And see if it keeps working. I guess that won't tell us if the attachments directory can be left uncreated and leave creation to the bridge, but ..

@tchapi
Copy link
Author

tchapi commented Jan 22, 2021

What confuses me here is that the /signald/data or /signald/avatars paths are created and have the correct permissions. And there is no difference in the bridge source code between those directories and the attachments one. So I wonder why the attachements one is not created ?

@tchapi
Copy link
Author

tchapi commented Jan 22, 2021

Oh. I think I understand.

Is there a rationale why you didn't keep /tmp here as per the original config file ?

I guess the attachements are removed by default after being bridged, so it's really a temporary folder that we need. And /tmp exists by default, hence using it should work I think, but maybe I'm missing something

@spantaleev
Copy link
Owner

I don't really know. This bridge work (like all others) was originally done by someone else from the community in #686.

I think /tmp can't work for us, because outgoing_attachment_dir needs to be a shared path between the 2 services (the signald daemon and the bridge). Each container gets its own /tmp though. Even without Docker, each systemd service usually gets its own temporary directories by default (I think).

It's easiest for us if we just use a shared path.

I don't know why the other paths get created by the bridge (or signald) and not this one. I guess we should create it manually.


We may hit the problem of permissions if we add it to our setup_install.yml as things are now. Maybe making that directory owned by matrix:matrix won't work.

@tchapi
Copy link
Author

tchapi commented Jan 22, 2021

Yeah you're right for /tmp.

I think matrix:matrix works, that's what I'm using right now; All the containers seem happy with it I guess:

Screenshot 2021-01-22 at 17 20 38

Screenshot 2021-01-22 at 17 22 18

Screenshot 2021-01-22 at 17 23 41

@tchapi
Copy link
Author

tchapi commented Jan 22, 2021

Re: why this path is not created, I think it is created when Matrix receives an attachment, but not when Matrix sends an attachment to Signal (see https://mau.dev/maunium/signald/-/blob/maunium-docker/src/main/java/io/finn/signald/Manager.java#L1414), vs. the avatars path that seems to be created beforehand.

@spantaleev
Copy link
Owner

I've made some changes in 37909aa, 8ec975e, f3dd346.

I've tried testing things and the daemon and bridge seem to start with these changes. However, I don't use the Signal bridge, so I can't say if it fully works.

Please update the playbook and let me know if everything's fine or if I should undo one or more of these changes. Thanks!

@tchapi
Copy link
Author

tchapi commented Jan 22, 2021

Wow, that was fast ! I'm testing right now

@tchapi
Copy link
Author

tchapi commented Jan 22, 2021

Everything seems to work fine, the bridge is functional. I still have to test it from scratch to confirm that the "initial" state works too, I'll do this over the weekend

Thanks so much again @spantaleev

@spantaleev
Copy link
Owner

Thank you too! Glad to hear we got this resolved! 🙇

Please open a new issue if you find something!

@tchapi
Copy link
Author

tchapi commented Jan 22, 2021

Just for information, I launched a new VPS instance and made a full reinstall from scratch, and it works as expected (I can send attachments right away via the Signal bridge).

🥇

@spantaleev
Copy link
Owner

Awesome! Thanks for the feedback!

@luixxiul luixxiul added the bug label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants