bug 1505325 - fix build telemetry path filtering when cwd is outside of topsrcdir/objdir. r=firefox-build-system-reviewers,chmanchester

The build telemetry code attempts to filter paths to avoid PII from usernames
and other things. It does this by converting every commandline argument to
an absolute path and then making them relative to topsrcdir or topobjdir and
omitting any that fail. This meant that running a mach command with a cwd
outside of the topsrcdir or objdir would omit all arguments since they were
converted to absolute paths from the cwd.

This change fixes this by adding the cwd to the list of paths used to create
relative paths. Additionally we add the user's home directory to that list
to try to avoid usernames sneaking through. Finally, instead of simply
removing these path prefixes, we replace them with sigils: $topsrcdir,
$objdir, $HOME.

Differential Revision: https://phabricator.services.mozilla.com/D11174

--HG--
extra : moz-landing-system : lando

build/mach_bootstrap.py

@@ -228,6 +228,7 @@ def bootstrap(topsrcdir, mozilla_dir=None):
 
         from mozbuild.telemetry import gather_telemetry
         from mozbuild.base import MozbuildObject
+        import mozpack.path as mozpath
 
         if not isinstance(instance, MozbuildObject):
             instance = MozbuildObject.from_environment()
@@ -237,11 +238,20 @@ def bootstrap(topsrcdir, mozilla_dir=None):
         except Exception:
             substs = {}
 
-        # We gather telemetry for every operation...
+        # We gather telemetry for every operation.
+        paths = {
+            instance.topsrcdir: '$topsrcdir/',
+            instance.topobjdir: '$topobjdir/',
+            mozpath.normpath(os.path.expanduser('~')): '$HOME/',
+        }
+        # This might override one of the existing entries, that's OK.
+        # We don't use a sigil here because we treat all arguments as potentially relative
+        # paths, so we'd like to get them back as they were specified.
+        paths[mozpath.normpath(os.getcwd())] = ''
         data = gather_telemetry(command=handler.name, success=(result == 0),
                                 start_time=start_time, end_time=end_time,
                                 mach_context=context, substs=substs,
-                                paths=[instance.topsrcdir, instance.topobjdir])
+                                paths=paths)
         if data:
             telemetry_dir = os.path.join(get_state_dir()[0], 'telemetry')
             try:

python/mach/mach/test/test_telemetry.py

@@ -1,13 +1,14 @@
 # Any copyright is dedicated to the Public Domain.
 # http://creativecommons.org/publicdomain/zero/1.0/
 
-from __future__ import absolute_import
+from __future__ import absolute_import, print_function
 
 import json
 import os
 import subprocess
 import sys
 
+import buildconfig
 import mozunit
 import pytest
 
@@ -24,8 +25,7 @@ def run_mach(tmpdir):
     update_or_create_build_telemetry_config(unicode(tmpdir.join('machrc')))
     env = dict(os.environ)
     env['MOZBUILD_STATE_PATH'] = str(tmpdir)
-    mach = os.path.normpath(os.path.join(os.path.dirname(__file__),
-                                         '../../../../mach'))
+    mach = os.path.join(buildconfig.topsrcdir, 'mach')
 
     def run(*args, **kwargs):
         # Run mach with the provided arguments
@@ -53,5 +53,72 @@ def test_simple(run_mach, tmpdir):
     assert client_id_data['client_id'] == d['client_id']
 
 
+def test_path_filtering(run_mach, tmpdir):
+    srcdir_path = os.path.join(buildconfig.topsrcdir, 'a')
+    srcdir_path_2 = os.path.join(buildconfig.topsrcdir, 'a/b/c')
+    objdir_path = os.path.join(buildconfig.topobjdir, 'x')
+    objdir_path_2 = os.path.join(buildconfig.topobjdir, 'x/y/z')
+    home_path = os.path.join(os.path.expanduser('~'), 'something_in_home')
+    other_path = str(tmpdir.join('other'))
+    data = run_mach('python', '-c', 'pass',
+                    srcdir_path, srcdir_path_2,
+                    objdir_path, objdir_path_2,
+                    home_path,
+                    other_path,
+                    cwd=buildconfig.topsrcdir)
+    assert len(data) == 1
+    d = data[0]
+    expected = [
+        '-c', 'pass',
+        'a', 'a/b/c',
+        '$topobjdir/x', '$topobjdir/x/y/z',
+        '$HOME/something_in_home',
+        '<path omitted>',
+    ]
+    assert d['argv'] == expected
+
+
+def test_path_filtering_in_objdir(run_mach, tmpdir):
+    srcdir_path = os.path.join(buildconfig.topsrcdir, 'a')
+    srcdir_path_2 = os.path.join(buildconfig.topsrcdir, 'a/b/c')
+    objdir_path = os.path.join(buildconfig.topobjdir, 'x')
+    objdir_path_2 = os.path.join(buildconfig.topobjdir, 'x/y/z')
+    other_path = str(tmpdir.join('other'))
+    data = run_mach('python', '-c', 'pass',
+                    srcdir_path, srcdir_path_2,
+                    objdir_path, objdir_path_2,
+                    other_path,
+                    cwd=buildconfig.topobjdir)
+    assert len(data) == 1
+    d = data[0]
+    expected = [
+        '-c', 'pass',
+        '$topsrcdir/a', '$topsrcdir/a/b/c',
+        'x', 'x/y/z',
+        '<path omitted>',
+    ]
+    assert d['argv'] == expected
+
+
+def test_path_filtering_other_cwd(run_mach, tmpdir):
+    srcdir_path = os.path.join(buildconfig.topsrcdir, 'a')
+    srcdir_path_2 = os.path.join(buildconfig.topsrcdir, 'a/b/c')
+    other_path = str(tmpdir.join('other'))
+    data = run_mach('python', '-c', 'pass',
+                    srcdir_path,
+                    srcdir_path_2,
+                    other_path, cwd=str(tmpdir))
+    assert len(data) == 1
+    d = data[0]
+    expected = [
+        # non-path arguments should escape unscathed
+        '-c', 'pass',
+        '$topsrcdir/a', '$topsrcdir/a/b/c',
+        # cwd-relative paths should be relativized
+        'other',
+    ]
+    assert d['argv'] == expected
+
+
 if __name__ == '__main__':
     mozunit.main()

python/mozbuild/mozbuild/telemetry.py

@@ -222,6 +222,9 @@ def filter_args(command, argv, paths):
     '''
     Given the full list of command-line arguments, remove anything up to and including `command`,
     and attempt to filter absolute pathnames out of any arguments after that.
+
+    `paths` is a dict whose keys are pathnames and values are sigils that should be used to
+    replace those pathnames.
     '''
     args = list(argv)
     while args:
@@ -231,20 +234,23 @@ def filter_args(command, argv, paths):
 
     def filter_path(p):
         p = mozpath.abspath(p)
-        base = mozpath.basedir(p, paths)
+        base = mozpath.basedir(p, paths.keys())
         if base:
-            return mozpath.relpath(p, base)
+            return paths[base] + mozpath.relpath(p, base)
         # Best-effort.
         return '<path omitted>'
     return [filter_path(arg) for arg in args]
 
 
 def gather_telemetry(command='', success=False, start_time=None, end_time=None,
-                     mach_context=None, substs={}, paths=[]):
+                     mach_context=None, substs={}, paths={}):
     '''
     Gather telemetry about the build and the user's system and pass it to the telemetry
     handler to be stored for later submission.
 
+    `paths` is a dict whose keys are pathnames and values are sigils that should be used to
+    replace those pathnames.
+
     Any absolute paths on the command line will be made relative to `paths` or replaced
     with a placeholder to avoid including paths from developer's machines.
     '''