From 0fd16c809b2d4225a44257b9c4d6b1b6a67a0770 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 2 Apr 2026 15:11:18 +0800 Subject: [PATCH 1/8] Airflowctl: Prevent path traversal via AIRFLOW_CLI_ENVIRONMENT --- airflow-ctl/src/airflowctl/api/client.py | 62 +++++++++++++------ .../tests/airflow_ctl/api/test_client.py | 11 ++++ 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index 51786bfcfcbff..acbea3b7f8373 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -23,9 +23,11 @@ import json import logging import os +import re import sys from collections.abc import Callable from functools import wraps +from pathlib import Path from typing import TYPE_CHECKING, Any, Literal, ParamSpec, TypeVar, cast import httpx @@ -144,6 +146,25 @@ def _bounded_get_new_password() -> str: ) +_VALID_ENV_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$") + + +def _validate_api_environment(api_environment: str) -> str: + if not _VALID_ENV_RE.fullmatch(api_environment): + raise ValueError( + "Invalid AIRFLOW_CLI_ENVIRONMENT. Only letters, numbers, '.', '_' and '-' are allowed." + ) + return api_environment + + +def _safe_path_under_airflow_home(airflow_home: str, filename: str) -> str: + base = Path(airflow_home).resolve() + target = (base / filename).resolve() + if base not in target.parents and target != base: + raise ValueError(f"Resolved path escapes AIRFLOW_HOME: {target}") + return str(target) + + # Credentials for the API class Credentials: """Credentials for the API.""" @@ -154,27 +175,29 @@ class Credentials: def __init__( self, + *, + client_kind: ClientKind, + api_environment: str | None = None, api_url: str | None = None, api_token: str | None = None, - client_kind: ClientKind | None = None, - api_environment: str = "production", ): + self.client_kind = client_kind + raw_env = ( + api_environment + if api_environment is not None + else os.getenv("AIRFLOW_CLI_ENVIRONMENT", "production") + ) + self.api_environment = _validate_api_environment(raw_env) + self.input_cli_config_file = f"{self.api_environment}.json" self.api_url = api_url self.api_token = api_token - self.api_environment = os.getenv("AIRFLOW_CLI_ENVIRONMENT") or api_environment - self.client_kind = client_kind - - @property - def input_cli_config_file(self) -> str: - """Generate path for the CLI config file.""" - return f"{self.api_environment}.json" @staticmethod def token_key_for_environment(api_environment: str) -> str: """Build the keyring/debug token key for a given environment name.""" return f"api_token_{api_environment}" - def save(self, skip_keyring: bool = False): + def save(self, skip_keyring: bool = False) -> Credentials: """ Save the credentials to keyring and URL to disk as a file. @@ -183,18 +206,21 @@ def save(self, skip_keyring: bool = False): """ default_config_dir = os.environ.get("AIRFLOW_HOME", os.path.expanduser("~/airflow")) os.makedirs(default_config_dir, exist_ok=True) - with open(os.path.join(default_config_dir, self.input_cli_config_file), "w") as f: + + config_path = _safe_path_under_airflow_home(default_config_dir, self.input_cli_config_file) + with open(config_path, "w") as f: json.dump({"api_url": self.api_url}, f) try: if os.getenv("AIRFLOW_CLI_DEBUG_MODE") == "true": - with open( - os.path.join(default_config_dir, f"debug_creds_{self.input_cli_config_file}"), "w" - ) as f: + debug_path = _safe_path_under_airflow_home( + default_config_dir, f"debug_creds_{self.input_cli_config_file}" + ) + with open(debug_path, "w") as f: json.dump({self.token_key_for_environment(self.api_environment): self.api_token}, f) else: if skip_keyring: - return + return self # Replace the upstream EncryptedKeyring's unbounded password # prompt with a bounded one before set_password can trigger it. # The active backend may be a ChainerBackend that delegates to @@ -222,11 +248,11 @@ def save(self, skip_keyring: bool = False): # This happens when the token is None, which is not allowed by keyring if self.api_token is None and self.client_kind == ClientKind.CLI: raise AirflowCtlCredentialNotFoundException("No API token found. Please login first.") from e + return self def load(self) -> Credentials: - """Load the credentials from keyring and URL from disk file.""" default_config_dir = os.environ.get("AIRFLOW_HOME", os.path.expanduser("~/airflow")) - config_path = os.path.join(default_config_dir, self.input_cli_config_file) + config_path = _safe_path_under_airflow_home(default_config_dir, self.input_cli_config_file) try: with open(config_path) as f: credentials = json.load(f) @@ -234,7 +260,7 @@ def load(self) -> Credentials: if self.api_token is not None: return self if os.getenv("AIRFLOW_CLI_DEBUG_MODE") == "true": - debug_creds_path = os.path.join( + debug_creds_path = _safe_path_under_airflow_home( default_config_dir, f"debug_creds_{self.input_cli_config_file}" ) try: diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py b/airflow-ctl/tests/airflow_ctl/api/test_client.py index f495b357d8d4b..73b2c69609cfd 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_client.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py @@ -392,3 +392,14 @@ def test_debug_mode_missing_debug_creds_reports_correct_error(self, monkeypatch, creds = Credentials(client_kind=ClientKind.CLI, api_environment="TEST_DEBUG") with pytest.raises(AirflowCtlCredentialNotFoundException, match="Debug credentials file not found"): creds.load() + + +@pytest.mark.parametrize("bad_env", ["../evil", "..\\evil", "a/b", "a\\b", ""]) +def test_credentials_rejects_path_traversal_env(bad_env): + with pytest.raises(ValueError, match="Invalid AIRFLOW_CLI_ENVIRONMENT"): + Credentials(client_kind=ClientKind.CLI, api_environment=bad_env) + + +def test_credentials_accepts_safe_env(): + creds = Credentials(client_kind=ClientKind.CLI, api_environment="prod-us_1") + assert creds.api_environment == "prod-us_1" From d5150715ba45565b74a097d7648b54ab96683741 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 2 Apr 2026 15:45:12 +0800 Subject: [PATCH 2/8] fix CI test --- airflow-ctl/src/airflowctl/api/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index acbea3b7f8373..093980bb889b0 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -176,7 +176,7 @@ class Credentials: def __init__( self, *, - client_kind: ClientKind, + client_kind: ClientKind = ClientKind.CLI, api_environment: str | None = None, api_url: str | None = None, api_token: str | None = None, From 4a2019236dbfc94cf859e8026a371257ff490217 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 2 Apr 2026 15:55:56 +0800 Subject: [PATCH 3/8] fix CI test --- airflow-ctl/src/airflowctl/api/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index 093980bb889b0..7e1a2e983a20f 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -185,7 +185,7 @@ def __init__( raw_env = ( api_environment if api_environment is not None - else os.getenv("AIRFLOW_CLI_ENVIRONMENT", "production") + else (os.getenv("AIRFLOW_CLI_ENVIRONMENT") or os.getenv("AIRFLOW_CLI_ENV") or "production") ) self.api_environment = _validate_api_environment(raw_env) self.input_cli_config_file = f"{self.api_environment}.json" From 5ae768621738432a0e3933278a947e5dee537bc2 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 2 Apr 2026 16:20:35 +0800 Subject: [PATCH 4/8] fix CI test2 --- airflow-ctl/src/airflowctl/api/client.py | 35 ++++++++++++------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index 7e1a2e983a20f..3494227759387 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -151,9 +151,7 @@ def _bounded_get_new_password() -> str: def _validate_api_environment(api_environment: str) -> str: if not _VALID_ENV_RE.fullmatch(api_environment): - raise ValueError( - "Invalid AIRFLOW_CLI_ENVIRONMENT. Only letters, numbers, '.', '_' and '-' are allowed." - ) + raise ValueError("Invalid AIRFLOW_CLI_ENVIRONMENT") return api_environment @@ -175,29 +173,31 @@ class Credentials: def __init__( self, - *, - client_kind: ClientKind = ClientKind.CLI, - api_environment: str | None = None, api_url: str | None = None, api_token: str | None = None, + client_kind: ClientKind | None = None, + api_environment: str = "production", ): - self.client_kind = client_kind - raw_env = ( - api_environment - if api_environment is not None - else (os.getenv("AIRFLOW_CLI_ENVIRONMENT") or os.getenv("AIRFLOW_CLI_ENV") or "production") - ) - self.api_environment = _validate_api_environment(raw_env) - self.input_cli_config_file = f"{self.api_environment}.json" self.api_url = api_url self.api_token = api_token + raw_env = os.getenv("AIRFLOW_CLI_ENVIRONMENT") + if raw_env is None: + raw_env = api_environment + self.api_environment = _validate_api_environment(raw_env) + self.client_kind = client_kind + + @property + def input_cli_config_file(self) -> str: + """Generate path for the CLI config file.""" + env = _validate_api_environment(self.api_environment) + return f"{env}.json" @staticmethod def token_key_for_environment(api_environment: str) -> str: """Build the keyring/debug token key for a given environment name.""" return f"api_token_{api_environment}" - def save(self, skip_keyring: bool = False) -> Credentials: + def save(self, skip_keyring: bool = False): """ Save the credentials to keyring and URL to disk as a file. @@ -206,7 +206,6 @@ def save(self, skip_keyring: bool = False) -> Credentials: """ default_config_dir = os.environ.get("AIRFLOW_HOME", os.path.expanduser("~/airflow")) os.makedirs(default_config_dir, exist_ok=True) - config_path = _safe_path_under_airflow_home(default_config_dir, self.input_cli_config_file) with open(config_path, "w") as f: json.dump({"api_url": self.api_url}, f) @@ -220,7 +219,7 @@ def save(self, skip_keyring: bool = False) -> Credentials: json.dump({self.token_key_for_environment(self.api_environment): self.api_token}, f) else: if skip_keyring: - return self + return # Replace the upstream EncryptedKeyring's unbounded password # prompt with a bounded one before set_password can trigger it. # The active backend may be a ChainerBackend that delegates to @@ -248,9 +247,9 @@ def save(self, skip_keyring: bool = False) -> Credentials: # This happens when the token is None, which is not allowed by keyring if self.api_token is None and self.client_kind == ClientKind.CLI: raise AirflowCtlCredentialNotFoundException("No API token found. Please login first.") from e - return self def load(self) -> Credentials: + """Load the credentials from keyring and URL from disk file.""" default_config_dir = os.environ.get("AIRFLOW_HOME", os.path.expanduser("~/airflow")) config_path = _safe_path_under_airflow_home(default_config_dir, self.input_cli_config_file) try: From bcf3b0af1cc926e3515e792c886cc8f411f0fe76 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Thu, 9 Apr 2026 23:13:11 +0800 Subject: [PATCH 5/8] remove additional check --- airflow-ctl/src/airflowctl/api/client.py | 15 ++------------- airflow-ctl/tests/airflow_ctl/api/test_client.py | 6 ------ 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index 3494227759387..4135d6423b01e 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -23,7 +23,6 @@ import json import logging import os -import re import sys from collections.abc import Callable from functools import wraps @@ -146,15 +145,6 @@ def _bounded_get_new_password() -> str: ) -_VALID_ENV_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$") - - -def _validate_api_environment(api_environment: str) -> str: - if not _VALID_ENV_RE.fullmatch(api_environment): - raise ValueError("Invalid AIRFLOW_CLI_ENVIRONMENT") - return api_environment - - def _safe_path_under_airflow_home(airflow_home: str, filename: str) -> str: base = Path(airflow_home).resolve() target = (base / filename).resolve() @@ -183,14 +173,13 @@ def __init__( raw_env = os.getenv("AIRFLOW_CLI_ENVIRONMENT") if raw_env is None: raw_env = api_environment - self.api_environment = _validate_api_environment(raw_env) self.client_kind = client_kind + self.api_environment = os.getenv("AIRFLOW_CLI_ENVIRONMENT") or api_environment @property def input_cli_config_file(self) -> str: """Generate path for the CLI config file.""" - env = _validate_api_environment(self.api_environment) - return f"{env}.json" + return f"{self.api_environment}.json" @staticmethod def token_key_for_environment(api_environment: str) -> str: diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py b/airflow-ctl/tests/airflow_ctl/api/test_client.py index 73b2c69609cfd..04646360bfb8a 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_client.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py @@ -394,12 +394,6 @@ def test_debug_mode_missing_debug_creds_reports_correct_error(self, monkeypatch, creds.load() -@pytest.mark.parametrize("bad_env", ["../evil", "..\\evil", "a/b", "a\\b", ""]) -def test_credentials_rejects_path_traversal_env(bad_env): - with pytest.raises(ValueError, match="Invalid AIRFLOW_CLI_ENVIRONMENT"): - Credentials(client_kind=ClientKind.CLI, api_environment=bad_env) - - def test_credentials_accepts_safe_env(): creds = Credentials(client_kind=ClientKind.CLI, api_environment="prod-us_1") assert creds.api_environment == "prod-us_1" From 2e1883d16fcaad124d5ece0dda18be9aae940647 Mon Sep 17 00:00:00 2001 From: rjgoyln <151457491+rjgoyln@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:11:57 +0800 Subject: [PATCH 6/8] replace by target.is_relative_to(base) --- airflow-ctl/src/airflowctl/api/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index 4135d6423b01e..3cadc95f5e975 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -148,7 +148,7 @@ def _bounded_get_new_password() -> str: def _safe_path_under_airflow_home(airflow_home: str, filename: str) -> str: base = Path(airflow_home).resolve() target = (base / filename).resolve() - if base not in target.parents and target != base: + if not target.is_relative_to(base): raise ValueError(f"Resolved path escapes AIRFLOW_HOME: {target}") return str(target) From 9c8d279be643c3bb3ee8d5fc9500c7cb8b35fc0a Mon Sep 17 00:00:00 2001 From: rjgoyln <151457491+rjgoyln@users.noreply.github.com> Date: Sat, 11 Apr 2026 12:36:39 +0800 Subject: [PATCH 7/8] Update airflow-ctl/tests/airflow_ctl/api/test_client.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- airflow-ctl/tests/airflow_ctl/api/test_client.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py b/airflow-ctl/tests/airflow_ctl/api/test_client.py index 04646360bfb8a..9fa35d01390e9 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_client.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py @@ -397,3 +397,16 @@ def test_debug_mode_missing_debug_creds_reports_correct_error(self, monkeypatch, def test_credentials_accepts_safe_env(): creds = Credentials(client_kind=ClientKind.CLI, api_environment="prod-us_1") assert creds.api_environment == "prod-us_1" + + +@pytest.mark.parametrize("api_environment", ["../evil", "..\\evil", "a/b", "a\\b"]) +def test_credentials_rejects_unsafe_env_argument(api_environment): + with pytest.raises(ValueError, match="environment"): + Credentials(client_kind=ClientKind.CLI, api_environment=api_environment) + + +@pytest.mark.parametrize("api_environment", ["../evil", "..\\evil", "a/b", "a\\b"]) +def test_credentials_rejects_unsafe_env_from_environment_variable(monkeypatch, api_environment): + monkeypatch.setenv("AIRFLOW_CLI_ENVIRONMENT", api_environment) + with pytest.raises(ValueError, match="environment"): + Credentials(client_kind=ClientKind.CLI) From 07e3dfe22da199de4a014c8949804e44406236f9 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Sat, 11 Apr 2026 13:28:18 +0800 Subject: [PATCH 8/8] fix(cli): add path traversal protection and improve exception handling --- airflow-ctl/src/airflowctl/api/client.py | 18 +++++++++++++----- .../tests/airflow_ctl/api/test_client.py | 5 +++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/airflow-ctl/src/airflowctl/api/client.py b/airflow-ctl/src/airflowctl/api/client.py index 3cadc95f5e975..f3ca3f673f119 100644 --- a/airflow-ctl/src/airflowctl/api/client.py +++ b/airflow-ctl/src/airflowctl/api/client.py @@ -63,6 +63,7 @@ ) from airflowctl.exceptions import ( AirflowCtlCredentialNotFoundException, + AirflowCtlException, AirflowCtlKeyringException, AirflowCtlNotFoundException, ) @@ -149,7 +150,10 @@ def _safe_path_under_airflow_home(airflow_home: str, filename: str) -> str: base = Path(airflow_home).resolve() target = (base / filename).resolve() if not target.is_relative_to(base): - raise ValueError(f"Resolved path escapes AIRFLOW_HOME: {target}") + raise AirflowCtlException( + f"Security Error: Path traversal detected in '{filename}'. " + f"The resolved path must stay within AIRFLOW_HOME." + ) return str(target) @@ -170,11 +174,15 @@ def __init__( ): self.api_url = api_url self.api_token = api_token - raw_env = os.getenv("AIRFLOW_CLI_ENVIRONMENT") - if raw_env is None: - raw_env = api_environment self.client_kind = client_kind - self.api_environment = os.getenv("AIRFLOW_CLI_ENVIRONMENT") or api_environment + raw_env = os.getenv("AIRFLOW_CLI_ENVIRONMENT") or api_environment + if "/" in raw_env or "\\" in raw_env or ".." in raw_env: + raise AirflowCtlException( + f"Invalid environment name: '{raw_env}'. " + f"Environment names cannot contain path separators ('/', '\\') or '..'." + ) + + self.api_environment = raw_env @property def input_cli_config_file(self) -> str: diff --git a/airflow-ctl/tests/airflow_ctl/api/test_client.py b/airflow-ctl/tests/airflow_ctl/api/test_client.py index 9fa35d01390e9..f2d216fcc82fb 100644 --- a/airflow-ctl/tests/airflow_ctl/api/test_client.py +++ b/airflow-ctl/tests/airflow_ctl/api/test_client.py @@ -32,6 +32,7 @@ from airflowctl.api.operations import ServerResponseError from airflowctl.exceptions import ( AirflowCtlCredentialNotFoundException, + AirflowCtlException, AirflowCtlKeyringException, ) @@ -401,12 +402,12 @@ def test_credentials_accepts_safe_env(): @pytest.mark.parametrize("api_environment", ["../evil", "..\\evil", "a/b", "a\\b"]) def test_credentials_rejects_unsafe_env_argument(api_environment): - with pytest.raises(ValueError, match="environment"): + with pytest.raises(AirflowCtlException, match="environment"): Credentials(client_kind=ClientKind.CLI, api_environment=api_environment) @pytest.mark.parametrize("api_environment", ["../evil", "..\\evil", "a/b", "a\\b"]) def test_credentials_rejects_unsafe_env_from_environment_variable(monkeypatch, api_environment): monkeypatch.setenv("AIRFLOW_CLI_ENVIRONMENT", api_environment) - with pytest.raises(ValueError, match="environment"): + with pytest.raises(AirflowCtlException, match="environment"): Credentials(client_kind=ClientKind.CLI)