restrict msgraph pagination nextLink to the configured host#69742
restrict msgraph pagination nextLink to the configured host#69742Samin061 wants to merge 2 commits into
Conversation
| # ``allowed_hosts``, which defaults to empty (any host) unless configured, so a tampered | ||
| # response could redirect the token off-host. Pin follow-up requests to the configured | ||
| # endpoint's host (CWE-918). | ||
| allowed_netloc = urlparse((await self.get_async_conn()).base_url).netloc |
There was a problem hiding this comment.
I don't think allowed_netloc should be initialized here. The variable itself makes sense, but it should be computed inside the async run() method where it's actually used. Initializing it here unnecessarily couples this code to the connection lookup and may perform work earlier than needed.
There was a problem hiding this comment.
Good point, moved it inside run() so the lookup only happens when pagination actually runs.
Signed-off-by: bibi samina <sam@bugqore.com>
|
@Samin061 — Some review feedback from Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
paginated_run follows the @odata.nextLink from each Microsoft Graph response and refetches it with the connection's bearer token attached, while Kiota's allowed_hosts defaults to empty (any host) when no authority is set, so a tampered response can send the token to an arbitrary host. Pin follow-up pagination requests to the configured endpoint's host and refuse a nextLink that points elsewhere.