Skip to content

Protect testcase task-log downloads#5243

Merged
decoNR merged 4 commits intogoogle:masterfrom
M0nd0R:fix-task-log-access-v2
Apr 15, 2026
Merged

Protect testcase task-log downloads#5243
decoNR merged 4 commits intogoogle:masterfrom
M0nd0R:fix-task-log-access-v2

Conversation

@M0nd0R
Copy link
Copy Markdown
Contributor

@M0nd0R M0nd0R commented Apr 14, 2026

This PR is a clean replacement for #5214 with only the task-log fix and its focused tests.

It adds testcase access checks before returning task logs and quotes task log filter values before building the Cloud Logging query so user-controlled task parameters cannot alter the intended filter constraints.

Verification:

  • targeted smoke check: TaskLogHandler now calls access.check_access_and_get_testcase(123) before fetching logs
  • targeted smoke check: injected filter content remains inside the quoted task_id string literal
  • local pytest selection reaches the new tests, but full execution is blocked here because this environment does not have gcloud for the datastore emulator

@M0nd0R M0nd0R requested a review from a team as a code owner April 14, 2026 15:42
Comment thread src/appengine/handlers/testcase_detail/show.py Outdated
@decoNR
Copy link
Copy Markdown
Contributor

decoNR commented Apr 14, 2026

Please fix lint errors (ref: Run basic tests / build (pull_request)).

@decoNR
Copy link
Copy Markdown
Contributor

decoNR commented Apr 14, 2026

/gcbrun

@M0nd0R
Copy link
Copy Markdown
Contributor Author

M0nd0R commented Apr 14, 2026

Addressed the review about the unnecessary cast. The handler now passes the raw testcase_id string into check_access_and_get_testcase, relies on data_handler.get_testcase_by_id for validation, and uses the returned testcase key ID for the log lookup. I also updated TaskLogHandlerTest to cover that flow explicitly.

Comment thread src/clusterfuzz/_internal/tests/appengine/handlers/testcase_detail/show_test.py Outdated
@decoNR
Copy link
Copy Markdown
Contributor

decoNR commented Apr 14, 2026

/gcbrun

@decoNR decoNR self-requested a review April 14, 2026 19:19
Copy link
Copy Markdown
Contributor

@decoNR decoNR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! We will probably deploy it in the next release

Copy link
Copy Markdown
Collaborator

@ViniciustCosta ViniciustCosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants