From fe2245ce52490f74059013a58287835cc6825f9d Mon Sep 17 00:00:00 2001 From: ManviKamboz Date: Sun, 14 Jun 2026 12:10:05 +0530 Subject: [PATCH 1/2] Fix security-sensitive testcase enumeration in issue redirector by enforcing ACL checks --- src/appengine/handlers/issue_redirector.py | 3 +- .../handlers/issue_redirector_test.py | 45 +++++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/appengine/handlers/issue_redirector.py b/src/appengine/handlers/issue_redirector.py index f66adc80fcb..a65ab3e6e56 100644 --- a/src/appengine/handlers/issue_redirector.py +++ b/src/appengine/handlers/issue_redirector.py @@ -16,6 +16,7 @@ from clusterfuzz._internal.issue_management import issue_tracker_utils from handlers import base_handler +from libs import access from libs import helpers @@ -24,7 +25,7 @@ class Handler(base_handler.Handler): def get(self, testcase_id=None): """Redirect user to the correct URL.""" - testcase = helpers.get_testcase(testcase_id) + testcase = access.check_access_and_get_testcase(testcase_id) issue_url = helpers.get_or_exit( lambda: issue_tracker_utils.get_issue_url(testcase), 'Issue tracker for testcase (id=%s) is not found.' % testcase_id, diff --git a/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py b/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py index b6a36d6cab0..08de19cf246 100644 --- a/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py +++ b/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py @@ -19,7 +19,11 @@ import webtest from clusterfuzz._internal.datastore import data_types +from clusterfuzz._internal.datastore import ndb_init +from clusterfuzz._internal.issue_management import issue_tracker_utils from clusterfuzz._internal.tests.test_libs import helpers as test_helpers +from libs import access +from libs import helpers class HandlerTest(unittest.TestCase): @@ -28,19 +32,32 @@ class HandlerTest(unittest.TestCase): def setUp(self): test_helpers.patch(self, [ 'clusterfuzz._internal.issue_management.issue_tracker_utils.get_issue_url', - 'libs.helpers.get_testcase', + 'libs.access.check_access_and_get_testcase', 'clusterfuzz._internal.system.environment.is_running_on_app_engine', + 'libs.helpers.get_user_email', + 'clusterfuzz._internal.config.db_config.get_value', + 'logging.exception', + 'libs.form.generate_csrf_token', ]) self.mock.is_running_on_app_engine.return_value = True + self.mock.get_user_email.return_value = '' + self.mock.get_value.return_value = 'contact@example.com' + self.mock.generate_csrf_token.return_value = 'dummy_csrf_token' + + self.ndb_context = ndb_init.context() + self.ndb_context.__enter__() import server self.app = webtest.TestApp(server.app) + def tearDown(self): + self.ndb_context.__exit__(None, None, None) + def test_succeed(self): """Test redirection succeeds.""" testcase = data_types.Testcase() testcase.bug_information = '456789' - self.mock.get_testcase.return_value = testcase + self.mock.check_access_and_get_testcase.return_value = testcase self.mock.get_issue_url.return_value = 'http://google.com/456789' response = self.app.get('/issue/12345') @@ -48,13 +65,33 @@ def test_succeed(self): self.assertEqual(302, response.status_int) self.assertEqual('http://google.com/456789', response.headers['Location']) - self.mock.get_testcase.assert_has_calls([mock.call('12345')]) + self.mock.check_access_and_get_testcase.assert_has_calls( + [mock.call('12345')]) self.mock.get_issue_url.assert_has_calls([mock.call(testcase)]) def test_no_issue_url(self): """Test no issue url.""" - self.mock.get_testcase.return_value = data_types.Testcase() + self.mock.check_access_and_get_testcase.return_value = data_types.Testcase() self.mock.get_issue_url.return_value = '' response = self.app.get('/issue/12345', expect_errors=True) self.assertEqual(404, response.status_int) + + def test_access_denied(self): + """Test access denied error.""" + self.mock.get_user_email.return_value = 'test@user.com' + self.mock.check_access_and_get_testcase.side_effect = ( + helpers.AccessDeniedError()) + + response = self.app.get('/issue/12345', expect_errors=True) + self.assertEqual(403, response.status_int) + + def test_unauthorized(self): + """Test unauthorized error.""" + self.mock.get_user_email.return_value = '' + self.mock.check_access_and_get_testcase.side_effect = ( + helpers.UnauthorizedError()) + + response = self.app.get('/issue/12345', expect_errors=True) + self.assertEqual(302, response.status_int) + self.assertIn('/login', response.headers['Location']) From 1b0879acbfaa57102042f9be13b37d78ebe50c7e Mon Sep 17 00:00:00 2001 From: ManviKamboz Date: Sun, 14 Jun 2026 14:11:40 +0530 Subject: [PATCH 2/2] Trigger CLA recheck