Repository: thp/urlwatch

Issue:
The shellpipe filter is removing all environment variables
The shellpipe filter is removing all environment variables, then adding 
URLWATCH_JOB_NAME and URLWATCH_JOB_LOCATION. I believe
the code in filters.py should just be adding them to the existing environment.

As a result (simple example shown below), the "sh" that's forked can run
programs in  system directories only because of fallback path-search behavior.
Under Debian, where /bin/sh is a symlink to /bin/dash, an strace shows that
"sh" searches /usr/local/{s,}bin /usr/{s,}bin} and /{s,}bin.

urlwatch fails to find the shellpipe command when it's in ~/bin and isn't
specified with its pathname.

Specifying a "diff_tool" does find the command if it's in my ~/bin
and doesn't empty the environment.

I'm using v2.19 as installed by "pip3" (not the latest code on Github,
though the env. handling appears to be the same).


        % env | grep PATH
        PATH=/home/stosh/bin:/usr/local/bin:/usr/bin:/bin
        % cat date_cat.yaml
        name: watchdog-cat
        command: "date"
        filter:
           - shellpipe: "env >/dev/tty; cat"
        % cat date_mycat.yaml 
        name: watchdog-mycat
        command: "date"
        filter:
           - shellpipe: "env >/dev/tty; mycat"
        % cmp /bin/cat ~/bin/mycat
        % urlwatch --cache date.db --config ~/.config/urlwatch/urlwatch_orig.yaml --urls date_cat.yaml
        URLWATCH_JOB_NAME=watchdog-cat
        URLWATCH_JOB_LOCATION=date
        PWD=/home/stosh
        ===========================================================================
        01. CHANGED: watchdog-cat
        ===========================================================================
        
        ---------------------------------------------------------------------------
        CHANGED: watchdog-cat ( date )
        ---------------------------------------------------------------------------
        --- @	Tue, 28 Jul 2020 17:55:41 -0400
        +++ @	Tue, 28 Jul 2020 17:56:27 -0400
        @@ -1 +1 @@
        -Tue Jul 28 17:55:41 EDT 2020
        +Tue Jul 28 17:56:27 EDT 2020
        
        ---------------------------------------------------------------------------
        
        
        -- 
        urlwatch 2.19, Copyright 2008-2020 Thomas Perl
        Website: https://thp.io/2008/urlwatch/
        watched 1 URLs in 0 seconds
        
        % urlwatch --cache date.db --config ~/.config/urlwatch/urlwatch_orig.yaml --urls date_mycat.yaml
        URLWATCH_JOB_NAME=watchdog-mycat
        URLWATCH_JOB_LOCATION=date
        PWD=/home/stosh
        /bin/sh: 1: mycat: not found
        ===========================================================================
        01. ERROR: watchdog-mycat
        ===========================================================================
        
        ---------------------------------------------------------------------------
        ERROR: watchdog-mycat ( date )
        ---------------------------------------------------------------------------
        Traceback (most recent call last):
          File "/home/stosh/.local/lib/python3.7/site-packages/urlwatch/handler.py", line 92, in process
            data = FilterBase.process(filter_kind, subfilter, self, data)
          File "/home/stosh/.local/lib/python3.7/site-packages/urlwatch/filters.py", line 145, in process
            return filtercls(state.job, state).filter(data, subfilter)
          File "/home/stosh/.local/lib/python3.7/site-packages/urlwatch/filters.py", line 784, in filter
            input=data.encode(encoding), env=env).decode(encoding)
          File "/usr/lib/python3.7/subprocess.py", line 395, in check_output
            **kwargs).stdout
          File "/usr/lib/python3.7/subprocess.py", line 487, in run
            output=stdout, stderr=stderr)
        subprocess.CalledProcessError: Command 'env >/dev/tty; mycat' returned non-zero exit status 127.
        ---------------------------------------------------------------------------
        
        
        -- 
        urlwatch 2.19, Copyright 2008-2020 Thomas Perl
        Website: https://thp.io/2008/urlwatch/
        watched 1 URLs in 0 seconds
 




Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 12ad2d1..ec27cfb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,8 @@ The format mostly follows [Keep a Changelog](http://keepachangelog.com/en/1.0.0/
 - Unit tests have been migrated from `nose` to `pytest`
   and moved from `test/` to `lib/urlwatch/tests/`
 - The ``css`` and ``xpath`` filters now accept ``skip`` and ``maxitems`` as subfilter
+- The ``shellpipe`` filter now inherits all environment variables (e.g. ``$PATH``)
+  of the ``urlwatch`` process
 
 ### Fixed
 
diff --git a/lib/urlwatch/filters.py b/lib/urlwatch/filters.py
index 390ac09..b11a3bb 100644
--- a/lib/urlwatch/filters.py
+++ b/lib/urlwatch/filters.py
@@ -788,10 +788,12 @@ class ShellPipeFilter(FilterBase):
 
         encoding = sys.getdefaultencoding()
 
-        env = {
+        # Work on a copy to not modify the outside environment
+        env = dict(os.environ)
+        env.update({
             'URLWATCH_JOB_NAME': self.job.pretty_name() if self.job else '',
             'URLWATCH_JOB_LOCATION': self.job.get_location() if self.job else '',
-        }
+        })
 
         return subprocess.check_output(subfilter['command'], shell=True,
                                        input=data.encode(encoding), env=env).decode(encoding)


Test patch:
diff --git a/lib/urlwatch/tests/test_filters.py b/lib/urlwatch/tests/test_filters.py
index 80465f3..ca85120 100644
--- a/lib/urlwatch/tests/test_filters.py
+++ b/lib/urlwatch/tests/test_filters.py
@@ -71,3 +71,20 @@ def test_providing_subfilter_to_filter_without_subfilter_raises_valueerror():
 def test_providing_unknown_subfilter_raises_valueerror():
     with pytest.raises(ValueError):
         list(FilterBase.normalize_filter_list([{'grep': {'re': 'Price: .*', 'anothersubfilter': '42'}}]))
+
+
+def test_shellpipe_inherits_environment_but_does_not_modify_it():
+    # https://github.com/thp/urlwatch/issues/541
+
+    # Set a specific value to check it doesn't overwrite the current env
+    os.environ['URLWATCH_JOB_NAME'] = 'should-not-be-overwritten'
+
+    # See if the shellpipe process can use a variable from the outside
+    os.environ['INHERITED_FROM'] = 'parent-process'
+    filtercls = FilterBase.__subclasses__.get('shellpipe')
+    result = filtercls(None, None).filter('input-string', {'command': 'echo "$INHERITED_FROM/$URLWATCH_JOB_NAME"'})
+    # Check that the inherited value and the job name is set properly
+    assert result == 'parent-process/\n'
+
+    # Check that outside the variable wasn't overwritten by the filter
+    assert os.environ['URLWATCH_JOB_NAME'] == 'should-not-be-overwritten'