Skip to content

fix: harden robot router GetFileBlob access validation#1162

Merged
Tati1701 merged 2 commits into
google:masterfrom
TristanInSec:harden-robot-router-getfileblob
May 12, 2026
Merged

fix: harden robot router GetFileBlob access validation#1162
Tati1701 merged 2 commits into
google:masterfrom
TristanInSec:harden-robot-router-getfileblob

Conversation

@TristanInSec
Copy link
Copy Markdown
Contributor

Summary

  • Add client-level access check to GetFileBlob in ApiCallRobotRouter
  • New _CheckClientRobotAccess verifies the requesting user has at least one flow on the target client
  • Consistent with the per-flow checks (_CheckFlowRobotId) on other data-retrieval methods in the same router
  • Adds two tests: rejection when no flow exists, success when flow exists

Test plan

  • testGetFileBlobRaisesIfNoFlowCreatedBySameUser -- another user's flow does not grant access
  • testGetFileBlobWorksIfFlowWasCreatedBySameUser -- own flow grants access
  • Existing testGetFileBlobIsDisabledByDefault unaffected

Copy link
Copy Markdown
Collaborator

@Tati1701 Tati1701 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! :D

GetFileBlob. This ensures the robot user has some prior relationship
with the client, consistent with the per-flow checks on other methods.
"""
flows = data_store.REL_DB.ReadAllFlowObjects(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This can impact the performance quite a bit, depending on the deployment.

Would you consider extending ReadAllFlowObjects with an optional created_by filter in the DB on a separate PR, then use it here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, that makes sense for deployments with high flow volume. I'll open a separate PR adding a created_by filter to ReadAllFlowObjects, then rebase this one to use it.

Verify the requesting user has at least one flow on the target client
before allowing GetFileBlob, consistent with the per-flow checks on
other data-retrieval methods in the robot router.

Adds two tests for the new access check.
Now that ReadAllFlowObjects supports a created_by parameter (google#1164),
use it to filter flows at the database level instead of fetching all
flows and iterating in Python.
@TristanInSec TristanInSec force-pushed the harden-robot-router-getfileblob branch from 23829dd to cf81147 Compare April 16, 2026 11:50
@TristanInSec
Copy link
Copy Markdown
Contributor Author

Hi @mbushkov, friendly ping on this security fix (and the related #1163). CLA is passing, happy to make any changes needed. Thanks!

@Tati1701 Tati1701 merged commit 9f8f797 into google:master May 12, 2026
9 of 10 checks passed
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.

2 participants