Add authentication and input validation to TaskLogHandler#5229
Open
petrmarinec wants to merge 1 commit intogoogle:masterfrom
Open
Add authentication and input validation to TaskLogHandler#5229petrmarinec wants to merge 1 commit intogoogle:masterfrom
petrmarinec wants to merge 1 commit intogoogle:masterfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
petrmarinec
force-pushed
the
fix/tasklog-auth-and-filter-injection
branch
from
3539541 to
eddfd93
Compare
The TaskLogHandler endpoint at /testcase-detail/task-log was missing authentication and authorization checks, unlike every other handler in the same file that accesses testcase data. This change: 1. Adds access.check_access_and_get_testcase() to verify the caller is authenticated and authorized to view the testcase before querying Cloud Logging. 2. Validates task_name against the known set of valid task names to prevent Cloud Logging filter injection via crafted query parameters. 3. Adds _sanitize_filter_value() to escape double quotes and backslashes in user-supplied values before interpolating them into the Cloud Logging filter string, as defense-in-depth against filter injection. 4. Adds unit tests for the sanitization function and task name validation.
petrmarinec
force-pushed
the
fix/tasklog-auth-and-filter-injection
branch
from
eddfd93 to
8c37c2c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TaskLogHandlerat/testcase-detail/task-logwas missing authentication and authorization checks. Unlike the adjacentHandler(which usescheck_testcase_access) andRefreshHandler(which usesoauth+ internal access check),TaskLogHandlerhad only@handler.get(handler.TEXT)— which sets content type but does not verify the caller's identity.This meant any unauthenticated caller could query Google Cloud Logging for task execution logs associated with any testcase.
Additionally, the
task_idandtask_namequery parameters were interpolated directly into the Cloud Logging filter string without escaping, creating a potential filter injection vector.Changes
Added
access.check_access_and_get_testcase(testcase_id)at the start ofTaskLogHandler.get()to verify the caller is authenticated and has access to the requested testcase — consistent with other handlers in the same file.Added
task_namevalidation againstTestcaseStatusInfo.TASK_EVENTS_NAMES+CHROME_TASK_EVENTS_NAMESto reject unknown task names before they reach the Cloud Logging query.Added
_sanitize_filter_value()toTestcaseEventHistorythat escapes double quotes and backslashes in user-supplied values before interpolating them into the Cloud Logging filter string.Added unit tests for the sanitization function and task name validation.
Testing
SanitizeFilterValueTestintestcase_status_events_test.pycovering normal values, quote injection, backslash escaping, and empty strings.TaskLogHandlerValidationTestinshow_test.pyverifying that known task names are accepted and injection payloads are not in the valid set.