feature: add robot GraphQL cross-tenant access detection rule#219
feature: add robot GraphQL cross-tenant access detection rule#219ticket-fixer[bot] wants to merge 2 commits into
Conversation
Add a generic Agent Asteroid detection rule for the Robot GraphQL Endpoint Cross-Tenant Access vulnerability. The rule probes the /apis/robot_graphql endpoint and fingerprints the API-key-gated robot GraphQL view (no robot/ service discriminator) whose unscoped resolvers expose cross-tenant data to any holder of an org-level API key. Resolves os-35099.
![ostorlab-ai-pr-review[bot]](https://avatars.githubusercontent.com/in/1072062?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Total Issues Found: 4
Critical Issues: 1
Suggestions: 3
Key Findings:
The code review identified 4 issues in robot_graphql_cross_tenant_access.py: 1 critical issue (module-level logging.basicConfig() that can override application-wide logging at import time) and 3 suggestions (improving authentication probing logic, converting @staticmethod to module-level function per conventions, and using collections.abc.Mapping for read-only parameter type annotations). The code is functionally sound but needs targeted fixes for import-time side effects and better alignment with established project conventions.
|
|
||
| from agent import definitions | ||
| from agent import exploits_registry | ||
|
|
There was a problem hiding this comment.
Bug: Remove this module-level logging.basicConfig() call. It executes at import time and can override the application-wide logging configuration if this module is imported before the application sets up its own handlers. Other exploits in the codebase (e.g., cve_2025_22457.py) do not call basicConfig. The named logger created on the next line (logger = logging.getLogger(__name__)) is sufficient — the application is responsible for configuring log levels and handlers.
| invalid_key_response.text, | ||
| ) | ||
| is False | ||
| ): |
There was a problem hiding this comment.
Bug: Branch on the authentication status observed during probing. When the unauthenticated probe returns 200 with data (unauth_live is True), the endpoint has no permission gate at all — report it as completely unauthenticated. When only the authenticated probe succeeds (unauth_live is False but invalid-key probe succeeds), report the org API key gate.
| ): | |
| if unauth_live: | |
| vulnerability.technical_detail = ( | |
| f"{endpoint} exposes the robot GraphQL endpoint with no authentication " | |
| f"whatsoever. Unscoped robot-schema resolvers are reachable by anyone, " | |
| f"enabling cross-tenant access without any credentials." | |
| ) | |
| else: | |
| vulnerability.technical_detail = ( | |
| f"{endpoint} exposes the robot GraphQL endpoint, whose sole permission gate is an " | |
| f"organization API key (HasOrganizationAPIKey | IsAdminUser) with no " | |
| f"robot/service discriminator. Unscoped robot-schema resolvers are reachable by " | |
| f"any holder of an org-level API key, enabling cross-tenant access." | |
| ) |
| endpoint, | ||
| data=GRAPHQL_PROBE_QUERY, | ||
| headers=headers, | ||
| timeout=DEFAULT_TIMEOUT.seconds, |
There was a problem hiding this comment.
Style: Per the coding conventions, @staticmethod should be used sparingly; prefer module-level functions. Since _is_endpoint_live does not access any class or instance state, it should be extracted to a module-level function placed alongside _parse_json_object. This also makes it easier to test independently.
| timeout=DEFAULT_TIMEOUT.seconds, | |
| def _is_endpoint_live(status_code: int, body: str) -> bool: |
| f"robot/service discriminator. Unscoped robot-schema resolvers are reachable by " | ||
| f"any holder of an org-level API key, enabling cross-tenant access." | ||
| ) | ||
| return [vulnerability] |
There was a problem hiding this comment.
Style: Per the coding conventions, use collections.abc.Mapping instead of the concrete dict type for input parameters. Since headers is only read (not mutated), annotate it as headers: collections.abc.Mapping[str, str]. This allows callers to pass any mapping type and better expresses the intent.
| return [vulnerability] | |
| def _probe( | |
| self, | |
| endpoint: str, | |
| headers: collections.abc.Mapping[str, str], | |
| ) -> requests.Response | None: |
2 participants
Summary
Add a generic Agent Asteroid detection rule for the Robot GraphQL Endpoint Cross-Tenant Access vulnerability (ticket os-35099). The robot GraphQL endpoint at
/apis/robot_graphqlis intended for an internal robot/service, but its sole permission gate (HasOrganizationAPIKey | IsAdminUser) admits any valid organization API key with no robot/service discriminator, and the robot-schema resolvers run without organisation scoping — enabling a holder of a normal org-level API key to read/write cross-tenant data.The new rule remotely fingerprints the exposed, API-key-gated robot GraphQL view:
acceptPOSTs a GraphQL{__typename}probe to/apis/robot_graphqland accepts when the response is a JSON GraphQL/DRF payload on an auth-related status (200/400/401/403).checkconfirms the endpoint with an unauthenticated probe and a probe carrying a bogusX-API-Key; both being rejected with a DRF-styledetailpayload fingerprints theHasOrganizationAPIKeygate (noIsRobotdiscriminator), reporting the cross-tenant access finding (HIGH).Changes
agent/exploits/robot_graphql_cross_tenant_access.py— newRobotGraphQLCrossTenantAccessExploitdetection rule.tests/exploits/robot_graphql_cross_tenant_access_test.py— 6 unit tests covering the exposed/gated, introspection-executing, missing-endpoint, non-GraphQL-response, network-error, and mid-check-failure cases.Verification
ruff format --check .andruff check .pass.mypyreports no issues for the new files (only pre-existing missing-stub noise in unrelated modules).pytest tests/exploits/robot_graphql_cross_tenant_access_test.py— 6 passed.Resolves os-35099.