music-assistant / hass-music-assistant

Turn your Home Assistant instance into a jukebox, hassle free streaming of your favorite media to Home Assistant media players.
Apache License 2.0
1.3k stars 48 forks source link

Provider packages install code uses unquoted shell command #2054

Closed kvj closed 5 months ago

kvj commented 5 months ago

What version of Music Assistant has the issue?

2.0.0b119

What version of the Home Assistant Integration have you got installed?

2024.3.4

Have you tried everything in the Troubleshooting FAQ and reviewed the Open and Closed Issues and Discussions to resolve this yourself?

The problem

I've found an interesting behavior of providers dependencies install code.

Here you construct a shell command using a single string with placeholders, but don't quote special characters. At least one dependency isn't being installed correctly. Here py-opensonic>=5.0.5 is being handled as an output redirect character, and, as a result, I get a file =5.0.5 in my workdir (with pip output), and instead the latest package version is being installed.

All environments are affected, including docker and hass addon. It's not causing any big issue, given that you're not pinning any max version now, but once you'll add something like package<=x.x.x it will surely fail (I've tried, it does).

There are at least two ways to fix it:

  1. Use something like https://docs.python.org/3/library/shlex.html#shlex.quote for package
  2. Migrate to asyncio.create_subprocess_exec and construct arguments as an array

How to reproduce

Create / modify provider manifest and pin specific dependency version using <= syntax

Music Providers

Open Subsonic Media Server Library at least

Player Providers

Not relevant

Full log output

RuntimeError: Failed to install package py-opensonic<=5.0.5
/bin/sh: 1: cannot open =5.0.5: No such file

Additional information

No response

What version of Home Assistant Core are your running

2024.3.1

What type of installation are you running?

Home Assistant OS

On what type of hardware are you running?

Raspberry Pi

erkr commented 5 months ago

Great catch! Thanks 🙏

marcelveldt commented 5 months ago

Thanks for the catch, these issues get a bit burried as we focus on the docker installs where we always install all requirements (from requirements_all.txt) while building the image. Still nice to fix.

BTW: I think we should always use pinned versions in the provider requirements, especially for these provider specific libs. Not sure how this one (opensubsonic) slipped through the cracks

marcelveldt commented 5 months ago

It was a super simple fix so fixed it right away, thanks for the catch again.