Conversation
Require testcase access before serving task logs and escape Cloud Logging filter values so task parameters cannot bypass the intended query constraints.
|
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. |
|
/gcbrun |
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.
|
Updated this branch with the latest master and addressed the two concrete CI failures seen on the previous head:
I also verified locally that the task-log filter export commands now succeed, and I checked The new GitHub Actions runs for head |
|
/gcbrun |
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.
|
/gcbrun |
decoNR
left a comment
decoNR
left a comment
There was a problem hiding this comment.
Hi @M0nd0R , thank you very much for your contribution! Could you please remove the changes that are not related to the task-log issue? Feel free to open another PR for those changes, but FYI, a team member will be working on fixing the CI soon.
| @handler.get(handler.TEXT) | ||
| def get(self): | ||
| """Serve the task log.""" | ||
| testcase_id = helpers.cast(flask.request.args.get('testcase_id'), int, |
There was a problem hiding this comment.
This cast is unnecessary, as data_handler.get_testcase_by_id in check_access_and_get_testcase already handles it. Please remove it.
|
I split out the unrelated CI/bootstrap changes and opened a clean replacement PR with only the task-log handler, log-filter, and focused test changes: #5243. Thanks for the review and the note about the CI work on your side. |
|
Closing this in favor of #5243, which contains only the task-log handler, log-filter, and focused test changes. |
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
3 participants
This change protects /testcase-detail/task-log by enforcing testcase access checks before returning log content.
It also escapes task log filter values before building the Cloud Logging query so user-controlled task parameters cannot alter the intended filter.