From 4eeab1c55a0cd6c7e75a994351d99ed143a584eb Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Sun, 4 May 2025 14:32:13 +0300 Subject: [PATCH 1/4] Call get allow ips only of program started --- webhook_server/app.py | 45 ++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/webhook_server/app.py b/webhook_server/app.py index 70e58c36..8cfce984 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -3,7 +3,6 @@ import ipaddress import os import sys -from functools import lru_cache from typing import Any import requests @@ -15,7 +14,6 @@ Request, status, ) -from httpx import AsyncClient from starlette.datastructures import Headers from webhook_server.libs.exceptions import NoPullRequestError, RepositoryNotFoundError @@ -25,6 +23,8 @@ VERIFY_GITHUB_IPS = os.getenv("GITHUB_IPS_ONLY", "").lower() in ["true", "1"] VERIFY_CLOUDFLARE_IPS = os.getenv("CLOUDFLARE_IPS_ONLY", "").lower() in ["true", "1"] +ALLOWED_IPS: list[str] = [] + WEBHOOK_SECRET = os.getenv("WEBHOOK_SECRET", "") FASTAPI_APP: FastAPI = FastAPI(title="webhook-server") APP_URL_ROOT_PATH: str = "/webhook_server" @@ -51,47 +51,32 @@ def verify_signature(payload_body: bytes, secret_token: str, signature_header: H raise HTTPException(status_code=403, detail="Request signatures didn't match!") -@lru_cache(maxsize=1) -async def get_github_allowlist() -> list[str]: +def get_github_allowlist() -> list[str]: """Fetch and cache GitHub IP allowlist""" - async with AsyncClient(timeout=10.0) as client: - response = await client.get("https://api.github.com/meta") - return response.json()["hooks"] + response = requests.get("https://api.github.com/meta") + return response.json()["hooks"] -@lru_cache(maxsize=1) -async def get_cloudflare_allowlist() -> list[str]: +def get_cloudflare_allowlist() -> list[str]: """Fetch and cache Cloudflare IP allowlist""" - async with AsyncClient(timeout=10.0) as client: - response = await client.get("https://api.cloudflare.com/client/v4/ips") - return response.json()["result"]["ipv4_cidrs"] + response = requests.get("https://api.cloudflare.com/client/v4/ips") + return response.json()["result"]["ipv4_cidrs"] async def gate_by_allowlist_ips(request: Request) -> None: - if VERIFY_GITHUB_IPS or VERIFY_CLOUDFLARE_IPS: - allowlist = [] - + if ALLOWED_IPS: try: src_ip = ipaddress.ip_address(request.client.host) except ValueError: raise HTTPException(status.HTTP_400_BAD_REQUEST, "Could not hook sender ip address") - if VERIFY_GITHUB_IPS: - github_allowlist = await get_github_allowlist() - allowlist.extend(github_allowlist) - - if VERIFY_CLOUDFLARE_IPS: - cloudflare_allowlist = await get_cloudflare_allowlist() - allowlist.extend(cloudflare_allowlist) - - if not allowlist: - raise HTTPException(status.HTTP_403_FORBIDDEN, "Failed to get allowlist ips") - - for valid_ip in allowlist: + for valid_ip in ALLOWED_IPS: if src_ip in ipaddress.ip_network(valid_ip): return else: - raise HTTPException(status.HTTP_403_FORBIDDEN, "Not a GitHub hooks ip address") + raise HTTPException( + status.HTTP_403_FORBIDDEN, f"Not a {'GitHub' if VERIFY_GITHUB_IPS else 'Cloudflare'} hooks ip address" + ) def on_starting(server: Any) -> None: @@ -101,6 +86,10 @@ def on_starting(server: Any) -> None: repository_and_webhook_settings(webhook_secret=WEBHOOK_SECRET) logger.info("Repository and webhook settings initialized successfully.") + global ALLOWED_IPS + ALLOWED_IPS.extend(get_cloudflare_allowlist()) + ALLOWED_IPS.extend(get_github_allowlist()) + except Exception as ex: logger.exception(f"FATAL: Error during startup initialization: {ex}") raise From dac51aa7804dbfe3158e4a37b555b5841b9f6e34 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Mon, 5 May 2025 11:13:33 +0300 Subject: [PATCH 2/4] better http error when IP not match --- webhook_server/app.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webhook_server/app.py b/webhook_server/app.py index 8cfce984..c150595e 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -75,7 +75,8 @@ async def gate_by_allowlist_ips(request: Request) -> None: return else: raise HTTPException( - status.HTTP_403_FORBIDDEN, f"Not a {'GitHub' if VERIFY_GITHUB_IPS else 'Cloudflare'} hooks ip address" + status.HTTP_403_FORBIDDEN, + f"{src_ip} IP is not a {'GitHub' if VERIFY_GITHUB_IPS else 'Cloudflare'} hooks ip address", ) From 20fe4d5d8472bc0fea97abada3009b33e5eab708 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Mon, 5 May 2025 16:35:46 +0300 Subject: [PATCH 3/4] init allow ips only if needed --- webhook_server/app.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/webhook_server/app.py b/webhook_server/app.py index c150595e..b9596db6 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -88,8 +88,11 @@ def on_starting(server: Any) -> None: logger.info("Repository and webhook settings initialized successfully.") global ALLOWED_IPS - ALLOWED_IPS.extend(get_cloudflare_allowlist()) - ALLOWED_IPS.extend(get_github_allowlist()) + + if VERIFY_GITHUB_IPS or VERIFY_CLOUDFLARE_IPS: + ALLOWED_IPS.extend(get_cloudflare_allowlist()) + ALLOWED_IPS.extend(get_github_allowlist()) + logger.info(f"IP allowlist initialized successfully. {ALLOWED_IPS}") except Exception as ex: logger.exception(f"FATAL: Error during startup initialization: {ex}") From 1c513de5447df6e09382e06fff99b9883c82fab7 Mon Sep 17 00:00:00 2001 From: Meni Yakove Date: Mon, 5 May 2025 16:48:19 +0300 Subject: [PATCH 4/4] improve ip allow list logic --- webhook_server/app.py | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/webhook_server/app.py b/webhook_server/app.py index b9596db6..ca7e53e6 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -23,7 +23,7 @@ VERIFY_GITHUB_IPS = os.getenv("GITHUB_IPS_ONLY", "").lower() in ["true", "1"] VERIFY_CLOUDFLARE_IPS = os.getenv("CLOUDFLARE_IPS_ONLY", "").lower() in ["true", "1"] -ALLOWED_IPS: list[str] = [] +ALLOWED_IPS: tuple[ipaddress._BaseNetwork, ...] = () WEBHOOK_SECRET = os.getenv("WEBHOOK_SECRET", "") FASTAPI_APP: FastAPI = FastAPI(title="webhook-server") @@ -53,14 +53,18 @@ def verify_signature(payload_body: bytes, secret_token: str, signature_header: H def get_github_allowlist() -> list[str]: """Fetch and cache GitHub IP allowlist""" - response = requests.get("https://api.github.com/meta") - return response.json()["hooks"] + response = requests.get("https://api.github.com/meta", timeout=5) + response.raise_for_status() + data = response.json() + return data.get("hooks", []) def get_cloudflare_allowlist() -> list[str]: """Fetch and cache Cloudflare IP allowlist""" - response = requests.get("https://api.cloudflare.com/client/v4/ips") - return response.json()["result"]["ipv4_cidrs"] + response = requests.get("https://api.cloudflare.com/client/v4/ips", timeout=5) + response.raise_for_status() + result = response.json()["result"] + return result.get("ipv4_cidrs", []) + result.get("ipv6_cidrs", []) async def gate_by_allowlist_ips(request: Request) -> None: @@ -70,13 +74,13 @@ async def gate_by_allowlist_ips(request: Request) -> None: except ValueError: raise HTTPException(status.HTTP_400_BAD_REQUEST, "Could not hook sender ip address") - for valid_ip in ALLOWED_IPS: - if src_ip in ipaddress.ip_network(valid_ip): + for valid_ip_range in ALLOWED_IPS: + if src_ip in valid_ip_range: return else: raise HTTPException( status.HTTP_403_FORBIDDEN, - f"{src_ip} IP is not a {'GitHub' if VERIFY_GITHUB_IPS else 'Cloudflare'} hooks ip address", + f"{src_ip} IP is not a valid ip in allowlist IPs", ) @@ -89,9 +93,17 @@ def on_starting(server: Any) -> None: global ALLOWED_IPS - if VERIFY_GITHUB_IPS or VERIFY_CLOUDFLARE_IPS: - ALLOWED_IPS.extend(get_cloudflare_allowlist()) - ALLOWED_IPS.extend(get_github_allowlist()) + if VERIFY_GITHUB_IPS and VERIFY_CLOUDFLARE_IPS: + networks: list[ipaddress._BaseNetwork] = [] + + if VERIFY_CLOUDFLARE_IPS: + networks += [ipaddress.ip_network(cidr) for cidr in get_cloudflare_allowlist()] + + if VERIFY_GITHUB_IPS: + networks += [ipaddress.ip_network(cidr) for cidr in get_github_allowlist()] + + ALLOWED_IPS = tuple(networks) # immutable & de-duplicated + logger.info(f"IP allowlist initialized successfully. {ALLOWED_IPS}") except Exception as ex: