Core: Remote files are no longer supported in Slack notifications

Created on 20 Jun 2020  Ā·  13Comments  Ā·  Source: home-assistant/core

The problem


After release v0.107.7, any time i try and invoke an automation that attempts to post an image to a slack channel (in my case a CCTV line crossing event that normally logs into the camera to get a still image, which it then posts to the slack channel post), the automation fails to run, and the logs show the below Type Cast problem.

Environment

  • Home Assistant Core release with the issue:
    I have walked back version by version (i have a docker-based install) to pinpoint v0.107.7 still works - anything after that is broken.
  • Last working Home Assistant Core release (if known):
    v0.107.7
  • Operating environment (Home Assistant/Supervised/Docker/venv):
    Docker (on ubuntu host)
  • Integration causing this issue:
    Slack
  • Link to integration documentation on our website:

Problem-relevant configuration.yaml

    - service: notify.k_slack_push
      data:
        message: '_[motion]_ *PORCH-CAM:* line crossing detected'
        target: ['#perimeter']
        # THE BELOW data/file/url chunk (i.e. attaching images!) is BROKEN AFTER v0.107.7 (and still in v0.112.x)...."TypeError: expected str, bytes or os.PathLike object, not collections.OrderedDict"
        data: 
          file:
            url: "http://1.1.1.1:80/ISAPI/Streaming/channels/1/picture"   
            username: 'admin'
            password: !secret k_fringelevelpass
            auth: digest

Traceback/Error logs

        2020-06-20 11:46:37 ERROR (MainThread) [homeassistant.components.automation] CCTV: Porch Line Crossing: Error executing script. Unexpected error for call_service at pos 1: expected str, bytes or os.PathLike object, not collections.OrderedDict
        Traceback (most recent call last):
          File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 153, in _async_step
            self, f"_async_{cv.determine_script_action(self._action)}_step"
          File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 656, in _async_call_service_step
            *self._prep_call_service_step(), blocking=True, context=self._context
          File "/usr/src/homeassistant/homeassistant/core.py", line 1260, in async_call
            task.result()
          File "/usr/src/homeassistant/homeassistant/core.py", line 1295, in _execute_service
            await handler.func(service_call)
          File "/usr/src/homeassistant/homeassistant/components/notify/__init__.py", line 119, in async_notify_message
            await notify_service.async_send_message(**kwargs)
          File "/usr/src/homeassistant/homeassistant/components/slack/notify.py", line 151, in async_send_message
            data[ATTR_FILE], targets, message, title
          File "/usr/src/homeassistant/homeassistant/components/slack/notify.py", line 97, in _async_send_local_file_message
            if not self._hass.config.is_allowed_path(path):
          File "/usr/src/homeassistant/homeassistant/core.py", line 1360, in is_allowed_path
            thepath = pathlib.Path(path)
          File "/usr/local/lib/python3.7/pathlib.py", line 1022, in __new__
            self = cls._from_parts(args, init=False)
          File "/usr/local/lib/python3.7/pathlib.py", line 669, in _from_parts
            drv, root, parts = self._parse_args(args)
          File "/usr/local/lib/python3.7/pathlib.py", line 653, in _parse_args
            a = os.fspath(a)
        TypeError: expected str, bytes or os.PathLike object, not collections.OrderedDict

Additional information

slack

All 13 comments

This was an architectural choice: https://github.com/home-assistant/core/pull/33287#discussion_r399845921

If you have a valid use case for loading remote images into Slack, please open an architecture issue.

Thanks for getting back to me so quickly bachya.

Before i post the architecture issue, can you help me understand the security issue?

The previous solution was so elegant in my case - my security cam on my LAN (not internet) registers a line-crossing event, hass is subscribed to this event feed so logs into the camera on the LAN and grabs a still image, to post into slack. I.e who is at my door.

It honestly was our house’s favourite feature.

I guess the alternative is to use ffmpeg to save still image files on the local filesystem, and then attach them (would that bypass your security concern? Is that allowed?) but i believe:
1/ there would be lag (would i miss the person at the front door?). Line crossing event happens quick.
2/ it would be fiddly and i believe inaccessible to a lot of amateur hass tinkerers who might find this extra layer (scripting ffmpeg etc) unworkable.

So again if you could help me understand the unacceptable security risk of logging into a local IP address on my network to pull down a still image?

Thanks!
Keiran.

@keiranharris The issue is that the previous implementation allowed for _any_ URL to be retrieved, which was dangerous. The comment linked above mentioned that it would be good to define a global ā€œwhitelisted URLsā€ concept that all integrations could use for this sort of purpose.

Does that help?

The whitelisted URL would be fine (and potentially as a starting point perhaps all IPv4 RFC1918 private blocks 10.x, 172.16.x, 192.168.x should be in that whitelist by default - assumption being private / non-routable address space is in that user’s house and is thus ok).

Tbh im still not really getting the security concern (im a network security guy myself), but cant argue with the whitelist logic if you guys are seeing a threat im not.

Is that what you suggest i do? Raise an architectural issue to request the whitelist? Citing this use-case?

Thanks again.

@keiranharris From https://github.com/home-assistant/core/pull/33287#discussion_r399845921:

This feature needs to be removed. It's dangerous. It means that we're allowing via a service call any file to be fetched in the name of Home Assistant (which can have elevated privileges because it's making internal requests).

Thus, it's not your implemented use case with the risk, per se – it would arise from someone else passing a malicious URL.

Is that what you suggest i do? Raise an architectural issue to request the whitelist? Citing this use-case?

Yes – again, the goal is to define something that all integrations (not just Slack) can use.

Gotcha. Ok thanks will do. Thanks for the rapid comms on this.

Done. Closing the loop here on this thread. Architecture issue #404. Thanks for the background and guidance on this @bachya

Hey @bachya its me again! Haha. I just tested my automation trigger - we are NEARLY there but my camera (hikvision) used to require the ā€˜auth: digest’ key/value (in pre v0.107.7), at the same yaml level as url, username, password. When i add that key in, i get ā€œERROR... extra keys not allowed...ā€ when the automation attempts to fire, and when i remove it, i get a 401 unauthorised (ie camera not letting me in to grab the still image). So im guessing you need to allow us users to specify the style of authorisation when hitting a URL? Sorry! (And thanks!).
20201010_11-58-22_perimeter yaml — CONTAINERS

Thanks for the heads up, @keiranharris. I was afraid of this – from https://github.com/home-assistant/core/pull/37161:

Note that while #33287 offered support for remote files using Basic Auth and Digest Auth, this PR only adds support for Basic Auth because aiohttp has yet to implement a Digest Auth helper.

So, I need to start hunting for for a Digest Auth solution. I'll re-open this issue while I investigate.

@keiranharris Still chewing on this. Our primary asyncio HTTP library doesn't support Digest Auth, so I need to use a different library, but I don't have a super clear path forward. I'll get there, though. Just an FYI!

Thanks @bachya really appreciate this. Dont bust yourself though - i have a workaround that involves hooking the image through the filesystem in a two step process (snapshot, then upload). Digest would be cleaner but im back in the game again at least. Thanks again. image

Great to hear, @keiranharris! I'm still not seeing a clean way to get Digest Auth in here just yet, so I'll keep this issue open and continue to šŸ¤ž that we see an upstream solution in aiohttp.

slack documentation
slack source
(message by IssueLinks)

Was this page helpful?
0 / 5 - 0 ratings