From d0b5a7b1aba602dc75ab65d81e8aeb3864ea9420 Mon Sep 17 00:00:00 2001 From: Ashley Date: Wed, 26 Aug 2026 14:58:58 +0200 Subject: [PATCH 1/4] PPI-1616 deprecate CLI flags for secrets, accept env vars --- CHANGELOG.md | 4 + README.md | 5 +- piwik_pro_log_analytics/import_logs.py | 48 ++++++++++-- tests/test_main.py | 100 +++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7ea01d..e46bbca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +- PPI-1616 Deprecated passing `--client-secret`/`--client-id`/`--auth-password`/`--auth-user` on the + command line (they leak via shell history and the process list); added `PIWIK_CLIENT_SECRET`, + `PIWIK_CLIENT_ID`, `PIWIK_AUTH_PASSWORD`, and `PIWIK_AUTH_USER` environment variable equivalents, + which take precedence when the corresponding flag isn't set. - PPNA-10000 Redact sensitive values from HTTP/auth debug logging - PPPSYS-56851 Require Python 3.10 or newer (3.9 and older are end-of-life); CI tests 3.10 through 3.14 - PPPSYS-56851 Migrated packaging and CI from Poetry to uv (PEP 621, setuptools, committed `uv.lock`); PyPI releases use `uv publish`. diff --git a/README.md b/README.md index 80edd42..5684bcd 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,10 @@ Lint and format with [Ruff](https://docs.astral.sh/ruff/) (same style as the Piw 1. Download this git repository `git clone git@github.com:PiwikPRO/log-analytics.git`. The script uses only python standard library, so no external packages are required. Alternatively you can download our PyPi package - `pip install piwik-pro-log-analytics`. 2. Generate Client ID and Client Secret for communication with Piwik PRO API - docs on how to do this can be found on [developers.piwik.pro](https://developers.piwik.pro/en/latest/data_collection/other_integrations/web_log_analytics.html) 3. You are now ready to import your web server's access logs into Piwik PRO: - * `piwik_pro_log_analytics/import_logs.py --client-id --client-secret --url=.piwik.pro /path/to/access.log` + * `PIWIK_CLIENT_ID= PIWIK_CLIENT_SECRET= piwik_pro_log_analytics/import_logs.py --url=.piwik.pro /path/to/access.log` + * The `--client-id`/`--client-secret` flags are also accepted, but are deprecated: passing them on + the command line exposes them in your shell history and to other local users via the process + list. Prefer the environment variables shown above. * If you installed log analytics via `pip`, instead of `piwik_pro_log_analytics/import_logs.py` use `piwik_pro_log_analytics` * If the code fails, saying, that your log format doesn't contain hostname - you must decide what App you'd like to track to. You can find App ID in Piwik PRO UI> Administration> Sites & apps>. After that, use `--idsite ` flag to tell the importer which App you'd like to track to. ![How to find App ID](docs/app-id.png "How to find App ID") diff --git a/piwik_pro_log_analytics/import_logs.py b/piwik_pro_log_analytics/import_logs.py index e92857a..618aae3 100755 --- a/piwik_pro_log_analytics/import_logs.py +++ b/piwik_pro_log_analytics/import_logs.py @@ -761,13 +761,19 @@ def _create_parser(self): parser.add_argument( "--auth-user", dest="auth_user", - help="Basic auth user", + default=None, + help="Basic auth user. Can also be set via the PIWIK_AUTH_USER environment variable.", ) # Basic auth password parser.add_argument( "--auth-password", dest="auth_password", - help="Basic auth password", + default=None, + help=( + "Basic auth password. Passing it on the command line is deprecated - it exposes the" + " password in your shell history and to other users via the process list. Set the" + " PIWIK_AUTH_PASSWORD environment variable instead." + ), ) parser.add_argument( "--debug", @@ -860,12 +866,21 @@ def _create_parser(self): parser.add_argument( "--client-id", dest="client_id", - help="Client ID used when OAuth authentication is needed", + default=None, + help=( + "Client ID used when OAuth authentication is needed. Can also be set via the" + " PIWIK_CLIENT_ID environment variable." + ), ) parser.add_argument( "--client-secret", dest="client_secret", - help="Client secret used when OAuth authentication is needed", + default=None, + help=( + "Client secret used when OAuth authentication is needed. Passing it on the command" + " line is deprecated - it exposes the secret in your shell history and to other" + " users via the process list. Set the PIWIK_CLIENT_SECRET environment variable instead." + ), ) parser.add_argument( @@ -1358,6 +1373,28 @@ def _parse_args(self, option_parser, argv=None): level=logging.DEBUG if self.options.debug >= 1 else logging.INFO, ) + if self.options.client_secret is None: + self.options.client_secret = os.environ.get("PIWIK_CLIENT_SECRET") + else: + logging.warning( + "DeprecationWarning: passing --client-secret on the command line is deprecated and may be" + " removed in a future release, since it can leak the secret via shell history or the process" + " list. Set the PIWIK_CLIENT_SECRET environment variable instead." + ) + if self.options.client_id is None: + self.options.client_id = os.environ.get("PIWIK_CLIENT_ID") + + if self.options.auth_password is None: + self.options.auth_password = os.environ.get("PIWIK_AUTH_PASSWORD") + else: + logging.warning( + "DeprecationWarning: passing --auth-password on the command line is deprecated and may be" + " removed in a future release, since it can leak the password via shell history or the process" + " list. Set the PIWIK_AUTH_PASSWORD environment variable instead." + ) + if self.options.auth_user is None: + self.options.auth_user = os.environ.get("PIWIK_AUTH_USER") + self.options.excluded_useragents = set([s.lower() for s in self.options.excluded_useragents]) self._parse_paths() @@ -1487,7 +1524,8 @@ def _get_token_auth(self): return api_result else: fatal_error( - "OAuth authentication failed. Make sure that --client-id and --client-secret options are provided." + "OAuth authentication failed. Make sure that --client-id and --client-secret options are provided" + " (or the PIWIK_CLIENT_ID / PIWIK_CLIENT_SECRET environment variables are set)." ) def get_resolver(self): diff --git a/tests/test_main.py b/tests/test_main.py index f21e014..f185cac 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1324,6 +1324,106 @@ def test_glob_filenames(): ] +def test_client_credentials_fall_back_to_environment_variables(): + argv = ["--url=http://localhost", "logs/common.log"] + + with patch.dict(os.environ, {"PIWIK_CLIENT_ID": "env-id", "PIWIK_CLIENT_SECRET": "env-secret"}): + config = import_logs.Configuration(argv) + + assert config.options.client_id == "env-id" + assert config.options.client_secret == "env-secret" + + +def test_client_credentials_cli_args_take_precedence_over_environment(): + argv = [ + "--url=http://localhost", + "--client-id=cli-id", + "--client-secret=cli-secret", + "logs/common.log", + ] + + with patch.dict(os.environ, {"PIWIK_CLIENT_ID": "env-id", "PIWIK_CLIENT_SECRET": "env-secret"}): + config = import_logs.Configuration(argv) + + assert config.options.client_id == "cli-id" + assert config.options.client_secret == "cli-secret" + + +def test_client_secret_via_cli_arg_logs_a_warning(caplog): + argv = ["--url=http://localhost", "--client-secret=cli-secret", "logs/common.log"] + + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("PIWIK_CLIENT_SECRET", None) + with caplog.at_level(logging.WARNING): + import_logs.Configuration(argv) + + assert any( + "PIWIK_CLIENT_SECRET" in record.message and "deprecated" in record.message.lower() for record in caplog.records + ) + + +def test_client_credentials_absent_without_args_or_environment(): + argv = ["--url=http://localhost", "logs/common.log"] + + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("PIWIK_CLIENT_ID", None) + os.environ.pop("PIWIK_CLIENT_SECRET", None) + config = import_logs.Configuration(argv) + + assert config.options.client_id is None + assert config.options.client_secret is None + + +def test_basic_auth_credentials_fall_back_to_environment_variables(): + argv = ["--url=http://localhost", "logs/common.log"] + + with patch.dict(os.environ, {"PIWIK_AUTH_USER": "env-user", "PIWIK_AUTH_PASSWORD": "env-password"}): + config = import_logs.Configuration(argv) + + assert config.options.auth_user == "env-user" + assert config.options.auth_password == "env-password" + + +def test_basic_auth_credentials_cli_args_take_precedence_over_environment(): + argv = [ + "--url=http://localhost", + "--auth-user=cli-user", + "--auth-password=cli-password", + "logs/common.log", + ] + + with patch.dict(os.environ, {"PIWIK_AUTH_USER": "env-user", "PIWIK_AUTH_PASSWORD": "env-password"}): + config = import_logs.Configuration(argv) + + assert config.options.auth_user == "cli-user" + assert config.options.auth_password == "cli-password" + + +def test_auth_password_via_cli_arg_logs_a_warning(caplog): + argv = ["--url=http://localhost", "--auth-password=cli-password", "logs/common.log"] + + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("PIWIK_AUTH_PASSWORD", None) + with caplog.at_level(logging.WARNING): + import_logs.Configuration(argv) + + assert any( + "PIWIK_AUTH_PASSWORD" in record.message and "deprecated" in record.message.lower() for record in caplog.records + ) + + +def test_basic_auth_credentials_absent_without_args_or_environment(): + argv = ["--url=http://localhost", "logs/common.log"] + + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("PIWIK_AUTH_USER", None) + os.environ.pop("PIWIK_AUTH_PASSWORD", None) + config = import_logs.Configuration(argv) + + assert config.options.auth_user is None + assert config.options.auth_password is None + + # UrlHelper tests def test_urlhelper_convert_array_args(): def _test(input, expected): From 6cc0f49ebc7f984662ad1e0f4bd85f118fe4d358 Mon Sep 17 00:00:00 2001 From: Ashley Date: Thu, 27 Aug 2026 12:35:23 +0200 Subject: [PATCH 2/4] PPI-1616 reword docs to emphasise env vars should be passed safely --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5684bcd..608e88a 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,8 @@ Lint and format with [Ruff](https://docs.astral.sh/ruff/) (same style as the Piw 1. Download this git repository `git clone git@github.com:PiwikPRO/log-analytics.git`. The script uses only python standard library, so no external packages are required. Alternatively you can download our PyPi package - `pip install piwik-pro-log-analytics`. 2. Generate Client ID and Client Secret for communication with Piwik PRO API - docs on how to do this can be found on [developers.piwik.pro](https://developers.piwik.pro/en/latest/data_collection/other_integrations/web_log_analytics.html) 3. You are now ready to import your web server's access logs into Piwik PRO: - * `PIWIK_CLIENT_ID= PIWIK_CLIENT_SECRET= piwik_pro_log_analytics/import_logs.py --url=.piwik.pro /path/to/access.log` + * Set `PIWIK_CLIENT_ID` and `PIWIK_CLIENT_SECRET` in a secure way + * `piwik_pro_log_analytics/import_logs.py --url=.piwik.pro /path/to/access.log` * The `--client-id`/`--client-secret` flags are also accepted, but are deprecated: passing them on the command line exposes them in your shell history and to other local users via the process list. Prefer the environment variables shown above. From 5a1d53d141504335c1f5d2b95e1aa52998009ca9 Mon Sep 17 00:00:00 2001 From: Ashley Date: Thu, 27 Aug 2026 12:44:06 +0200 Subject: [PATCH 3/4] PPI-1616 update error message for oauth authentication error --- piwik_pro_log_analytics/import_logs.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/piwik_pro_log_analytics/import_logs.py b/piwik_pro_log_analytics/import_logs.py index 618aae3..672a1e8 100755 --- a/piwik_pro_log_analytics/import_logs.py +++ b/piwik_pro_log_analytics/import_logs.py @@ -1524,8 +1524,7 @@ def _get_token_auth(self): return api_result else: fatal_error( - "OAuth authentication failed. Make sure that --client-id and --client-secret options are provided" - " (or the PIWIK_CLIENT_ID / PIWIK_CLIENT_SECRET environment variables are set)." + "OAuth authentication failed. Make sure that PIWIK_CLIENT_ID and PIWIK_CLIENT_SECRET are set." ) def get_resolver(self): From f71928850ea5735844172819b65863a6c7f9fc8f Mon Sep 17 00:00:00 2001 From: Ashley Date: Thu, 27 Aug 2026 12:48:04 +0200 Subject: [PATCH 4/4] PPI-1616 fix formatting --- piwik_pro_log_analytics/import_logs.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/piwik_pro_log_analytics/import_logs.py b/piwik_pro_log_analytics/import_logs.py index 672a1e8..23c2de9 100755 --- a/piwik_pro_log_analytics/import_logs.py +++ b/piwik_pro_log_analytics/import_logs.py @@ -1523,9 +1523,7 @@ def _get_token_auth(self): return api_result else: - fatal_error( - "OAuth authentication failed. Make sure that PIWIK_CLIENT_ID and PIWIK_CLIENT_SECRET are set." - ) + fatal_error("OAuth authentication failed. Make sure that PIWIK_CLIENT_ID and PIWIK_CLIENT_SECRET are set.") def get_resolver(self): if self.options.site_id: