From 340b60c8fcdc4b45bd22cc4517fd0e22c5d7c915 Mon Sep 17 00:00:00 2001 From: M0nd0R Date: Sat, 21 Mar 2026 09:53:23 +0800 Subject: [PATCH 1/3] Testcase detail: protect task logs Require testcase access before serving task logs and escape Cloud Logging filter values so task parameters cannot bypass the intended query constraints. --- .../handlers/testcase_detail/show.py | 11 +++++- .../testcase_detail/testcase_status_events.py | 12 ++++-- .../handlers/testcase_detail/show_test.py | 39 +++++++++++++++++++ .../testcase_status_events_test.py | 21 ++++++++-- 4 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/appengine/handlers/testcase_detail/show.py b/src/appengine/handlers/testcase_detail/show.py index 712f0647957..beaadb73f73 100644 --- a/src/appengine/handlers/testcase_detail/show.py +++ b/src/appengine/handlers/testcase_detail/show.py @@ -640,9 +640,18 @@ class TaskLogHandler(base_handler.Handler): @handler.get(handler.TEXT) def get(self): """Serve the task log.""" + testcase_id = helpers.cast(flask.request.args.get('testcase_id'), int, + "The param 'testcase_id' is not a number.") + access.check_access_and_get_testcase(testcase_id) + task_id = flask.request.args.get('task_id') + if not task_id: + raise helpers.EarlyExitError('No task ID provided.', 400) + task_name = flask.request.args.get('task_name') - testcase_id = flask.request.args.get('testcase_id') + if not task_name: + raise helpers.EarlyExitError('No task name provided.', 400) + log_content = testcase_status_events.get_task_log(testcase_id, task_id, task_name) diff --git a/src/appengine/handlers/testcase_detail/testcase_status_events.py b/src/appengine/handlers/testcase_detail/testcase_status_events.py index 0a364cd92f2..e1a278f65bf 100644 --- a/src/appengine/handlers/testcase_detail/testcase_status_events.py +++ b/src/appengine/handlers/testcase_detail/testcase_status_events.py @@ -31,6 +31,11 @@ _BASE_LOGS_URL = 'https://console.cloud.google.com/logs/viewer' +def _quote_logging_filter_value(value: str) -> str: + """Formats a string literal for a Cloud Logging query filter.""" + return json.dumps(value) + + def _format_timestamp(timestamp: datetime.datetime) -> str: """Formats a timestamp.""" return timestamp.strftime('%Y-%m-%d %H:%M:%S.%f UTC') @@ -221,9 +226,10 @@ def _get_time_range_filter(self, days: int) -> str: def _get_task_log_query_filter(self, task_id: str, task_name: str) -> str: """Returns the filter string for querying task logs.""" - query = (f'jsonPayload.extras.task_id="{task_id}" AND ' - f'jsonPayload.extras.testcase_id="{self._testcase_id}" AND ' - f'jsonPayload.extras.task_name="{task_name}"') + query = (f'jsonPayload.extras.task_id={_quote_logging_filter_value(task_id)} AND ' + 'jsonPayload.extras.testcase_id=' + f'{_quote_logging_filter_value(str(self._testcase_id))} AND ' + f'jsonPayload.extras.task_name={_quote_logging_filter_value(task_name)}') query += f' AND {self._get_time_range_filter(days=31)}' return query diff --git a/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py b/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py index e72db6a48b5..43061672d0b 100644 --- a/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py +++ b/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py @@ -18,6 +18,9 @@ import os import unittest +import flask +import webtest + from clusterfuzz._internal.datastore import data_types from clusterfuzz._internal.tests.test_libs import helpers as test_helpers from clusterfuzz._internal.tests.test_libs import test_utils @@ -607,3 +610,39 @@ def test_unreproducible_get(self): self.assertDictContainsSubset({ 'lines': [show.Line(1, 'crash_stacktrace', False)] }, result['last_tested_crash_stacktrace']) + +@test_utils.with_cloud_emulators('datastore') +class TaskLogHandlerTest(unittest.TestCase): + """Tests for TaskLogHandler.""" + + def setUp(self): + test_helpers.patch(self, [ + 'handlers.testcase_detail.show.access.check_access_and_get_testcase', + 'handlers.testcase_detail.show.testcase_status_events.get_task_log', + ]) + flaskapp = flask.Flask('testflask') + flaskapp.add_url_rule('/', view_func=show.TaskLogHandler.as_view('/')) + self.app = webtest.TestApp(flaskapp) + self.mock.get_task_log.return_value = 'task log content' + + def test_get(self): + """Ensure the handler checks testcase access before returning logs.""" + response = self.app.get('/?testcase_id=123&task_id=task-1&task_name=minimize') + + self.assertEqual(200, response.status_int) + self.assertEqual('task log content', response.text) + self.assertEqual('text/plain; charset=utf-8', response.headers['Content-Type']) + self.assertEqual('attachment; filename="task_task-1_log.txt"', + response.headers['Content-Disposition']) + self.mock.check_access_and_get_testcase.assert_called_once_with(123) + self.mock.get_task_log.assert_called_once_with(123, 'task-1', 'minimize') + + def test_invalid_testcase_id(self): + """Ensure invalid testcase IDs are rejected before querying logs.""" + response = self.app.get( + '/?testcase_id=abc&task_id=task-1&task_name=minimize', + expect_errors=True) + + self.assertEqual(400, response.status_int) + self.mock.check_access_and_get_testcase.assert_not_called() + self.mock.get_task_log.assert_not_called() diff --git a/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/testcase_status_events_test.py b/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/testcase_status_events_test.py index 68cc30ff108..c647749318a 100644 --- a/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/testcase_status_events_test.py +++ b/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/testcase_status_events_test.py @@ -15,6 +15,7 @@ # pylint: disable=protected-access import datetime +import json import unittest from unittest import mock @@ -746,9 +747,23 @@ def test_get_task_log_query_filter(self): """Verify that the task log query filter is generated correctly.""" result = self.event_history._get_task_log_query_filter( 'task123', 'minimize') - expected = (f'jsonPayload.extras.task_id="task123" AND ' - f'jsonPayload.extras.testcase_id="{self.testcase_id}" AND ' - 'jsonPayload.extras.task_name="minimize" AND ' + quoted_testcase_id = json.dumps(str(self.testcase_id)) + expected = (f'jsonPayload.extras.task_id={json.dumps("task123")} AND ' + f'jsonPayload.extras.testcase_id={quoted_testcase_id} AND ' + f'jsonPayload.extras.task_name={json.dumps("minimize")} AND ' + 'timestamp >= "2025-01-01T00:00:00Z"') + self.assertEqual(result, expected) + + def test_get_task_log_query_filter_escapes_values(self): + """Verify that task values are escaped before building the log filter.""" + task_id = 'task123" OR severity>="WARNING' + task_name = 'minimize\\latest' + + result = self.event_history._get_task_log_query_filter(task_id, task_name) + quoted_testcase_id = json.dumps(str(self.testcase_id)) + expected = (f'jsonPayload.extras.task_id={json.dumps(task_id)} AND ' + f'jsonPayload.extras.testcase_id={quoted_testcase_id} AND ' + f'jsonPayload.extras.task_name={json.dumps(task_name)} AND ' 'timestamp >= "2025-01-01T00:00:00Z"') self.assertEqual(result, expected) From 98b36da6748441beee1f655197e069b8eeb53b45 Mon Sep 17 00:00:00 2001 From: M0nd0R Date: Wed, 1 Apr 2026 19:03:03 +0800 Subject: [PATCH 2/3] Local install: ensure bootstrap can import pipenv Install pipenv into the activated virtualenv before bootstrap runs so the later python3.11 -m pipenv requirements export does not fail when pipenv is only available globally. --- local/install_python_deps_linux.bash | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/local/install_python_deps_linux.bash b/local/install_python_deps_linux.bash index 0c24616ab62..18a16d60635 100755 --- a/local/install_python_deps_linux.bash +++ b/local/install_python_deps_linux.bash @@ -25,6 +25,12 @@ $PYTHON -m pipenv --python $PYTHON $PYTHON -m pipenv sync --dev source "$(${PYTHON} -m pipenv --venv)/bin/activate" +# Bootstrap later invokes `python3.11 -m pipenv`, so make sure the activated +# interpreter can import pipenv even if it wasn't installed into this venv. +if ! python -m pip show pipenv > /dev/null 2>&1; then + python -m pip install pipenv +fi + if [ $install_android_emulator ]; then ANDROID_SDK_INSTALL_DIR=local/bin/android-sdk ANDROID_SDK_REVISION=4333796 From e21aa24d21f9df67c90b1748178afb2f77b55d3d Mon Sep 17 00:00:00 2001 From: M0nd0R Date: Wed, 1 Apr 2026 23:20:28 +0800 Subject: [PATCH 3/3] Testcase detail: fix PR CI regressions Keep the task-log changes lint-clean, use pipenv's CLI directly for bootstrap requirement exports so the active interpreter can resolve it more reliably, and switch the Kubernetes e2e setup to sync from the lock file instead of relocking in CI. --- local/tests/kubernetes_e2e_test.bash | 2 +- .../handlers/testcase_detail/testcase_status_events.py | 6 ++++-- .../tests/appengine/handlers/testcase_detail/show_test.py | 7 +++++-- src/local/butler/common.py | 8 ++++---- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/local/tests/kubernetes_e2e_test.bash b/local/tests/kubernetes_e2e_test.bash index 16a75bdc0a6..fa83c5f62bf 100755 --- a/local/tests/kubernetes_e2e_test.bash +++ b/local/tests/kubernetes_e2e_test.bash @@ -20,7 +20,7 @@ pip install pipenv # Install dependencies. pipenv --python 3.11 -pipenv install +pipenv sync --dev ./local/install_deps.bash diff --git a/src/appengine/handlers/testcase_detail/testcase_status_events.py b/src/appengine/handlers/testcase_detail/testcase_status_events.py index e1a278f65bf..ac140715039 100644 --- a/src/appengine/handlers/testcase_detail/testcase_status_events.py +++ b/src/appengine/handlers/testcase_detail/testcase_status_events.py @@ -226,10 +226,12 @@ def _get_time_range_filter(self, days: int) -> str: def _get_task_log_query_filter(self, task_id: str, task_name: str) -> str: """Returns the filter string for querying task logs.""" - query = (f'jsonPayload.extras.task_id={_quote_logging_filter_value(task_id)} AND ' + query = ('jsonPayload.extras.task_id=' + f'{_quote_logging_filter_value(task_id)} AND ' 'jsonPayload.extras.testcase_id=' f'{_quote_logging_filter_value(str(self._testcase_id))} AND ' - f'jsonPayload.extras.task_name={_quote_logging_filter_value(task_name)}') + 'jsonPayload.extras.task_name=' + f'{_quote_logging_filter_value(task_name)}') query += f' AND {self._get_time_range_filter(days=31)}' return query diff --git a/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py b/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py index 43061672d0b..2a763bd5730 100644 --- a/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py +++ b/src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py @@ -611,6 +611,7 @@ def test_unreproducible_get(self): 'lines': [show.Line(1, 'crash_stacktrace', False)] }, result['last_tested_crash_stacktrace']) + @test_utils.with_cloud_emulators('datastore') class TaskLogHandlerTest(unittest.TestCase): """Tests for TaskLogHandler.""" @@ -627,11 +628,13 @@ def setUp(self): def test_get(self): """Ensure the handler checks testcase access before returning logs.""" - response = self.app.get('/?testcase_id=123&task_id=task-1&task_name=minimize') + response = self.app.get( + '/?testcase_id=123&task_id=task-1&task_name=minimize') self.assertEqual(200, response.status_int) self.assertEqual('task log content', response.text) - self.assertEqual('text/plain; charset=utf-8', response.headers['Content-Type']) + self.assertEqual('text/plain; charset=utf-8', + response.headers['Content-Type']) self.assertEqual('attachment; filename="task_task-1_log.txt"', response.headers['Content-Disposition']) self.mock.check_access_and_get_testcase.assert_called_once_with(123) diff --git a/src/local/butler/common.py b/src/local/butler/common.py index 4b955eb8acb..40b9adc2cfa 100644 --- a/src/local/butler/common.py +++ b/src/local/butler/common.py @@ -250,19 +250,19 @@ def _pipfile_to_requirements(pipfile_dir, requirements_path, dev=False): dev_arg = '--dev' return_code, output = execute( - f'python3.11 -m pipenv requirements {dev_arg}', + f'pipenv requirements {dev_arg}', exit_on_error=False, cwd=pipfile_dir, extra_environments={'PIPENV_IGNORE_VIRTUALENVS': '1'}, - stderr=subprocess.DEVNULL) + stderr=subprocess.STDOUT) if return_code != 0: # Older pipenv version. return_code, output = execute( - f'python3.11 -m pipenv lock -r --no-header {dev_arg}', + f'pipenv lock -r --no-header {dev_arg}', exit_on_error=False, cwd=pipfile_dir, extra_environments={'PIPENV_IGNORE_VIRTUALENVS': '1'}, - stderr=subprocess.DEVNULL) + stderr=subprocess.STDOUT) if return_code != 0: raise RuntimeError('Failed to generate requirements from Pipfile.')