This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 9bc37c174d9 Prevent cleartext credential storage in Git Dag bundles
(#64105)
9bc37c174d9 is described below
commit 9bc37c174d9cd636d82d0923a129ffb4d1e69afb
Author: rjgoyln <[email protected]>
AuthorDate: Mon Sep 21 23:23:58 2026 +0800
Prevent cleartext credential storage in Git Dag bundles (#64105)
Co-authored-by: Shahar Epstein <[email protected]>
---
providers/git/docs/changelog.rst | 11 +
providers/git/docs/connections/git.rst | 12 +-
.../git/src/airflow/providers/git/bundles/git.py | 55 ++++
.../git/src/airflow/providers/git/hooks/git.py | 186 +++++++-----
providers/git/tests/unit/git/bundles/test_git.py | 225 ++++++++++++++-
providers/git/tests/unit/git/hooks/test_git.py | 317 ++++++++++++++++++++-
6 files changed, 717 insertions(+), 89 deletions(-)
diff --git a/providers/git/docs/changelog.rst b/providers/git/docs/changelog.rst
index 2155f70a40e..4cb1ba4a002 100644
--- a/providers/git/docs/changelog.rst
+++ b/providers/git/docs/changelog.rst
@@ -19,6 +19,17 @@
Changelog
---------
+.. warning::
+ Token authentication over ``http(s)`` now hands the credential to git
through a credential
+ helper configured with ``GIT_CONFIG_COUNT``, which requires git 2.31 or
newer. On git 2.31+, a
+ token connection also resets any deployment-wide ``credential.helper`` for
that host, so a
+ deployment previously relying on its global helper switches to the
connection's token and can
+ start failing auth if that token is stale. On older git the helper is never
configured: the
+ clone then fails with git's generic ``could not read Username ... terminal
prompts disabled``,
+ or, where the deployment configures its own credential helper, authenticates
with that
+ helper's credential instead of the connection's token. Upgrade git on the
Dag processor and on
+ workers before upgrading this provider, or switch the connection to SSH key
authentication.
+
0.5.0
.....
diff --git a/providers/git/docs/connections/git.rst
b/providers/git/docs/connections/git.rst
index 9799bbdd0f1..0246040830e 100644
--- a/providers/git/docs/connections/git.rst
+++ b/providers/git/docs/connections/git.rst
@@ -43,14 +43,18 @@ Repository URL
or ``https://github.com/apache/airflow.git`` for HTTPS.
This can also be passed directly to the hook via the ``repo_url``
parameter.
+ A ``user:password@`` embedded in an ``http(s)`` repository URL is stripped
out before use and
+ treated as the username/access token; the explicit fields below take
precedence over it
+ when both are present.
+
Username or Access Token name (optional)
The username for HTTPS authentication or the token name. Defaults to
``user`` if not specified.
- When using HTTPS with an access token, this value is used as the username
in the
- authenticated URL (e.g. ``https://user:[email protected]/repo.git``).
Access Token (optional)
- The access token for HTTPS authentication. When provided along with the
username,
- the hook injects the credentials into the repository URL for HTTPS cloning.
+ The access token for HTTPS authentication. The connection's username and
token are never written into
+ the repository URL or the bundle's git config; instead they are handed to
git through a
+ credential helper scoped to the repository's host
(``credential.<scheme>://<host>[:port].helper``).
+ Token authentication over http(s) requires git version 2.31 or higher.
Extra (optional)
Specify the extra parameters as a JSON dictionary. The following keys are
supported:
diff --git a/providers/git/src/airflow/providers/git/bundles/git.py
b/providers/git/src/airflow/providers/git/bundles/git.py
index 930331a0b81..69aeecf4daf 100644
--- a/providers/git/src/airflow/providers/git/bundles/git.py
+++ b/providers/git/src/airflow/providers/git/bundles/git.py
@@ -161,6 +161,7 @@ class GitDagBundle(BaseDagBundle):
repo_path=self.repo_path,
version=self.version,
)
+ self._sync_bare_repo_remote_url()
return
if self._local_repo_has_version():
self._log.debug(
@@ -193,6 +194,7 @@ class GitDagBundle(BaseDagBundle):
# HEAD hexsha rather than the raw self.version (which
may be a
# tag or short SHA).
self.repo = repo
+ self._sync_bare_repo_remote_url()
return
cm = self.hook.configure_hook_env() if self.hook else nullcontext()
@@ -304,6 +306,9 @@ class GitDagBundle(BaseDagBundle):
env=self.hook.env if self.hook else None,
)
self.bare_repo = Repo(self.bare_repo_path)
+ # Not the best-effort wrapper: a GitCommandError from the rewrite
must reach the
+ # handler below, which drops the bare repo and re-clones it
credential-free.
+ self._rewrite_bare_repo_origin(self.bare_repo)
# Fetch to ensure we have latest refs and validate repo integrity
self._fetch_bare_repo()
@@ -317,6 +322,56 @@ class GitDagBundle(BaseDagBundle):
shutil.rmtree(self.bare_repo_path)
raise
+ def _sync_bare_repo_remote_url(self) -> None:
+ """
+ Re-point the bare repo's origin at the current repo url.
+
+ Called standalone from the ``_initialize`` fast paths that skip
cloning and never
+ reach ``_clone_bare_repo_if_required``, so a bundle that takes one
would otherwise
+ keep a credentialed origin url in ``bare/config`` forever. This is
best-effort: a
+ bundle that can still be served from disk must not fail to initialize
because its
+ bare repo is unreadable.
+ """
+ try:
+ if not self.bare_repo_path.exists():
+ return
+ bare_repo = Repo(self.bare_repo_path)
+ try:
+ self._rewrite_bare_repo_origin(bare_repo)
+ finally:
+ bare_repo.close()
+ except Exception as e:
+ # Deliberately broad: opening the repo raises anything from
``configparser`` on a
+ # truncated config to ``GitError`` on an unsafe remote url, and
the fast paths this
+ # runs ahead of never needed the bare repo at all.
+ self._log.warning(
+ "Could not rewrite the bare repository origin, a credential
may remain in "
+ "cleartext in the bundle's bare/config",
+ bare_repo_path=self.bare_repo_path,
+ exc=e,
+ )
+
+ @staticmethod
+ def _carries_credentials(url: str) -> bool:
+ """Report whether an origin url has ``user[:password]@`` in its
authority."""
+ if not url.startswith(("http://";, "https://";)):
+ return False
+ return "@" in url.partition("://")[2].partition("/")[0]
+
+ def _rewrite_bare_repo_origin(self, bare_repo: Repo) -> None:
+ if "origin" not in bare_repo.remotes:
+ return
+ origin = bare_repo.remotes.origin
+ # Bundles cloned before credentials moved to a credential helper
embedded ``user:token``
+ # here, so the token sits in cleartext in ``<bundle>/bare/config``
where any Dag author
+ # on the Dag processor can read it. Rewriting origin is what removes
it from those
+ # bundles. Only an origin holding one is rewritten: git resolves a
local source to an
+ # absolute path when it clones, and replacing that with a relative
repo url would leave
+ # an origin the bare repo cannot resolve.
+ if self._carries_credentials(origin.url):
+ self._log.info("Updating bare repository remote url",
bare_repo_path=self.bare_repo_path)
+ origin.set_url(str(self.repo_url))
+
def _ensure_version_in_bare_repo(self) -> None:
if not self.version:
return
diff --git a/providers/git/src/airflow/providers/git/hooks/git.py
b/providers/git/src/airflow/providers/git/hooks/git.py
index 0f59fde3f2b..4327c2c0ccb 100644
--- a/providers/git/src/airflow/providers/git/hooks/git.py
+++ b/providers/git/src/airflow/providers/git/hooks/git.py
@@ -29,7 +29,7 @@ import warnings
from collections.abc import Generator
from datetime import datetime, timedelta, timezone
from typing import Any
-from urllib.parse import quote as urlquote
+from urllib.parse import unquote
from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.providers.common.compat.sdk import
AirflowOptionalProviderFeatureException, BaseHook
@@ -59,6 +59,9 @@ class GitHook(BaseHook):
private key to be provided as a PEM-encoded key via either
``private_key`` (inline) or
``key_file`` (path to key file).
* ``github_installation_id`` — GitHub App installation ID used for GitHub
App authentication.
+
+ Token authentication over http(s) configures a credential helper through
``GIT_CONFIG_COUNT``
+ and needs git version 2.31 or higher.
"""
conn_name_attr = "git_conn_id"
@@ -101,8 +104,11 @@ class GitHook(BaseHook):
extra = connection.extra_dejson
self.repo_url = repo_url or connection.host
- self.user_name = connection.login or "user"
- self.auth_token = connection.password
+ if isinstance(self.repo_url, str) and not
self.repo_url.startswith(("git@", "https://";)):
+ self.repo_url = os.path.expanduser(self.repo_url)
+ embedded_user, embedded_token = self._strip_embedded_credentials()
+ self.user_name = connection.login or embedded_user or "user"
+ self.auth_token = connection.password or embedded_token
# SSH key authentication
self.private_key = extra.get("private_key")
@@ -155,7 +161,6 @@ class GitHook(BaseHook):
if self.key_file and not self.private_key:
with open(self.key_file, encoding="utf-8") as key_file:
self.private_key = key_file.read()
- self._process_git_auth_url()
_VALID_STRICT_HOST_KEY_CHECKING = frozenset({"yes", "no", "accept-new",
"off", "ask"})
_SSH_REPO_URL_PATTERN = re.compile(r"^[^/@:]+@[^/:]+:")
@@ -242,73 +247,93 @@ class GitHook(BaseHook):
)
self.user_name, self.auth_token, self.github_app_token_exp =
self._get_github_app_token()
+ def _strip_embedded_credentials(self) -> tuple[str | None, str | None]:
+ """Take any ``user:password@`` out of the repo url and return what it
held."""
+ if not isinstance(self.repo_url, str) or not
self.repo_url.startswith(("http://";, "https://";)):
+ return None, None
+ scheme, separator, rest = self.repo_url.partition("://")
+ authority, slash, path = rest.partition("/")
+ userinfo, at_sign, host = authority.rpartition("@")
+ user, _, password = userinfo.partition(":")
+ # A bare ``user@`` holds no secret, so leave those urls exactly as the
connection wrote
+ # them; anything git clones from a stripped url would lose the
username for nothing.
+ if not at_sign or not password:
+ return None, None
+ self.repo_url = f"{scheme}{separator}{host}{slash}{path}"
+ return unquote(user) or None, unquote(password)
+
+ def _extract_credential_scope(self) -> str:
+ """Return the ``<scheme>://<host>[:port]`` git matches a credential
config against."""
+ scheme, _, rest = str(self.repo_url).partition("://")
+ host = rest.partition("/")[0].rpartition("@")[2]
+ return f"{scheme}://{host}" if host else ""
+
@contextlib.contextmanager
- def _github_app_askpass_env(self) -> Generator[None]:
- if not self.auth_token:
+ def _token_credential_env(self) -> Generator[None]:
+ """Hand the token to git through a credential helper scoped to the
repository's host."""
+ # Credential helpers only serve http(s); an SSH connection that
happens to carry a
+ # password would gain nothing from one.
+ if not self.auth_token or not
str(self.repo_url).startswith(("http://";, "https://";)):
yield
return
- token = shlex.quote(self.auth_token)
- with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=True)
as askpass_script:
- askpass_script.write(
- "#!/bin/sh\n"
- 'case "$1" in\n'
- " *Username*) echo x-access-token;;\n"
- f" *Password*) echo {token};;\n"
- f" *) echo {token};;\n"
- "esac\n"
- )
- askpass_script.flush()
- os.chmod(askpass_script.name, stat.S_IRWXU)
+ scope = self._extract_credential_scope()
+ if not scope:
+ yield
+ return
+
+ with tempfile.TemporaryDirectory() as helper_dir:
+ helper_path = os.path.join(helper_dir, "credential-helper.sh")
+ # git matches the configured scope against the url it parsed, then
hands the helper
+ # structured fields on stdin. A submodule elsewhere never reaches
this helper, and no
+ # part of the decision depends on the wording of a human-readable
prompt.
+ # Written and closed before git runs: Linux refuses to exec a file
that is still
+ # open for writing, which git surfaces as "cannot exec: Text file
busy".
+ with open(helper_path, "w") as helper_script:
+ helper_script.write(
+ r"""#!/bin/sh
+cat > /dev/null
+[ "$1" = get ] || exit 0
+printf 'username=%s\npassword=%s\n' "$AIRFLOW_GIT_USER" "$AIRFLOW_GIT_TOKEN"
+"""
+ )
+ os.chmod(helper_path, stat.S_IRWXU)
- old_askpass = os.environ.get("GIT_ASKPASS")
- old_lc_all = os.environ.get("LC_ALL")
- old_terminal_prompt = os.environ.get("GIT_TERMINAL_PROMPT")
+ # Append to any GIT_CONFIG_* the deployment already exports rather
than replacing it.
+ try:
+ index = int(os.environ.get("GIT_CONFIG_COUNT", "0"))
+ except ValueError:
+ index = 0
+ # System/global config loads before env-supplied config, so a
deployment-wide
+ # `credential.helper` would otherwise answer `get` first and our
token would never
+ # be used. git also invokes every helper on `approve`, so a
`store` helper would
+ # persist it to ~/.git-credentials. Reset the scope to empty first
(git help credentials).
+ values = {
+ "GIT_CONFIG_COUNT": str(index + 2),
+ f"GIT_CONFIG_KEY_{index}": f"credential.{scope}.helper",
+ f"GIT_CONFIG_VALUE_{index}": "",
+ f"GIT_CONFIG_KEY_{index + 1}": f"credential.{scope}.helper",
+ # git runs the value through a shell, so a temp dir containing
a space would
+ # split into two words. ``!`` marks it as a command so the
quoting survives.
+ f"GIT_CONFIG_VALUE_{index + 1}": "!" +
shlex.quote(helper_path),
+ "GIT_TERMINAL_PROMPT": "0",
+ "AIRFLOW_GIT_USER": self.user_name,
+ "AIRFLOW_GIT_TOKEN": self.auth_token,
+ }
+ # ``self.env`` alone is not enough: callers only forward it on the
initial clone,
+ # so fetches would run without the credential and hang on the
terminal prompt.
+ envs = (os.environ, self.env)
+ saved = [(env, var, env.get(var)) for env in envs for var in
values]
try:
- os.environ["GIT_ASKPASS"] = askpass_script.name
- os.environ["GIT_TERMINAL_PROMPT"] = "0"
- self.env["GIT_ASKPASS"] = askpass_script.name
- self.env["LC_ALL"] = "C"
- self.env["GIT_TERMINAL_PROMPT"] = "0"
+ for env in envs:
+ env.update(values)
yield
finally:
- if old_askpass is None:
- self.env.pop("GIT_ASKPASS", None)
- os.environ.pop("GIT_ASKPASS", None)
- else:
- self.env["GIT_ASKPASS"] = old_askpass
- os.environ["GIT_ASKPASS"] = old_askpass
-
- if old_lc_all is None:
- self.env.pop("LC_ALL", None)
- os.environ.pop("LC_ALL", None)
- else:
- self.env["LC_ALL"] = old_lc_all
- os.environ["LC_ALL"] = old_lc_all
-
- if old_terminal_prompt is None:
- self.env.pop("GIT_TERMINAL_PROMPT", None)
- os.environ.pop("GIT_TERMINAL_PROMPT", None)
- else:
- self.env["GIT_TERMINAL_PROMPT"] = old_terminal_prompt
- os.environ["GIT_TERMINAL_PROMPT"] = old_terminal_prompt
-
- def _process_git_auth_url(self) -> None:
- if not isinstance(self.repo_url, str):
- return
- if self.auth_token and self.repo_url.startswith("https://";):
- encoded_user = urlquote(self.user_name, safe="")
- encoded_token = urlquote(self.auth_token, safe="")
- self.repo_url = self.repo_url.replace("https://";,
f"https://{encoded_user}:{encoded_token}@";, 1)
- elif self.auth_token and self.repo_url.startswith("http://";):
- encoded_user = urlquote(self.user_name, safe="")
- encoded_token = urlquote(self.auth_token, safe="")
- self.repo_url = self.repo_url.replace("http://";,
f"http://{encoded_user}:{encoded_token}@";, 1)
- elif self.repo_url.startswith("http://";):
- # if no auth token, use the repo url as is
- pass
- elif not self.repo_url.startswith("git@") and not
self.repo_url.startswith("https://";):
- self.repo_url = os.path.expanduser(self.repo_url)
+ for env, var, old_val in saved:
+ if old_val is None:
+ env.pop(var, None)
+ else:
+ env[var] = old_val
def set_git_env(self, key: str | None = None) -> None:
self.env["GIT_SSH_COMMAND"] = self._build_ssh_command(key)
@@ -352,25 +377,28 @@ class GitHook(BaseHook):
def configure_hook_env(self):
if self.github_app_id is not None and self.github_installation_id is
not None:
self._ensure_github_app_token()
- with self._github_app_askpass_env():
+ with self._token_credential_env():
yield
return
- if self.private_key:
- with tempfile.NamedTemporaryFile(mode="w", delete=True) as
tmp_keyfile:
- tmp_keyfile.write(self.private_key)
- tmp_keyfile.flush()
- os.chmod(tmp_keyfile.name, 0o600)
- self.set_git_env(tmp_keyfile.name)
+ # Wraps every branch, not just the token-only one: an http(s)
connection may also carry
+ # SSH options, and the token used to reach git through the URL
whichever branch ran.
+ with self._token_credential_env():
+ if self.private_key:
+ with tempfile.NamedTemporaryFile(mode="w", delete=True) as
tmp_keyfile:
+ tmp_keyfile.write(self.private_key)
+ tmp_keyfile.flush()
+ os.chmod(tmp_keyfile.name, 0o600)
+ self.set_git_env(tmp_keyfile.name)
+ with self._passphrase_askpass_env():
+ yield
+ elif self.key_file:
+ self.set_git_env(self.key_file)
with self._passphrase_askpass_env():
yield
- elif self.key_file:
- self.set_git_env(self.key_file)
- with self._passphrase_askpass_env():
+ elif self.host_proxy_cmd or self.ssh_port or self.ssh_config_file
or self.known_hosts_file:
+ self.set_git_env()
+ yield
+ else:
+ self.set_git_env(self.key_file)
yield
- elif self.host_proxy_cmd or self.ssh_port or self.ssh_config_file or
self.known_hosts_file:
- self.set_git_env()
- yield
- else:
- self.set_git_env(self.key_file)
- yield
diff --git a/providers/git/tests/unit/git/bundles/test_git.py
b/providers/git/tests/unit/git/bundles/test_git.py
index 0962fa1ad0b..4eac57c1920 100644
--- a/providers/git/tests/unit/git/bundles/test_git.py
+++ b/providers/git/tests/unit/git/bundles/test_git.py
@@ -20,13 +20,14 @@ from __future__ import annotations
import json
import os
import re
+import shlex
import types
from pathlib import Path
from unittest import mock
from unittest.mock import patch
import pytest
-from git import Repo
+from git import RemoteReference, Repo
from git.exc import GitCommandError, InvalidGitRepositoryError, NoSuchPathError
from airflow.dag_processing.bundles.base import get_bundle_storage_root_path
@@ -139,7 +140,7 @@ class TestGitDagBundle:
tracking_ref=GIT_DEFAULT_BRANCH,
repo_url="https://github.com/apache/zzzairflow";,
)
- assert bundle.repo_url ==
f"https://user:{ACCESS_TOKEN}@github.com/apache/zzzairflow";
+ assert bundle.repo_url == "https://github.com/apache/zzzairflow";
def test_falls_back_to_connection_host_when_no_repo_url_provided(self):
bundle = GitDagBundle(name="test", git_conn_id=CONN_HTTPS,
tracking_ref=GIT_DEFAULT_BRANCH)
@@ -1431,6 +1432,226 @@ class TestGitDagBundle:
_, kwargs = mock_gitRepo.clone_from.call_args
assert kwargs["env"] == EXPECTED_ENV
+ def test_refresh_propagates_token_credential_env_to_fetch_and_submodules(
+ self, create_connection_without_db
+ ):
+ token = "tok$with'quote"
+ conn_id = "my_git_conn_token_refresh"
+ create_connection_without_db(
+ Connection(
+ conn_id=conn_id,
+ host=AIRFLOW_HTTPS_URL,
+ login="token_user",
+ password=token,
+ conn_type="git",
+ )
+ )
+
+ bundle = GitDagBundle(
+ name="my_repo",
+ git_conn_id=conn_id,
+ tracking_ref=GIT_DEFAULT_BRANCH,
+ submodules=True,
+ )
+
+ bundle.bare_repo = mock.MagicMock(spec=Repo)
+ bundle.repo = mock.MagicMock(spec=Repo)
+
+ origin_ref = mock.MagicMock(spec=RemoteReference)
+ origin_ref.name = f"origin/{GIT_DEFAULT_BRANCH}"
+ bundle.repo.remotes.origin.refs = [origin_ref]
+
+ def _assert_token_env(*_, **__):
+ assert os.environ["AIRFLOW_GIT_TOKEN"] ==
bundle.hook.env["AIRFLOW_GIT_TOKEN"]
+ assert os.environ["GIT_CONFIG_VALUE_0"] ==
bundle.hook.env["GIT_CONFIG_VALUE_0"]
+ assert os.environ["GIT_TERMINAL_PROMPT"] == "0"
+
+ bundle.bare_repo.remotes.origin.fetch.side_effect = _assert_token_env
+ bundle.repo.remotes.origin.fetch.side_effect = _assert_token_env
+ bundle.repo.git.submodule.side_effect = _assert_token_env
+
+ with mock.patch.dict(os.environ, {"GIT_TERMINAL_PROMPT": "1"},
clear=False):
+ bundle.refresh()
+
+ assert os.environ["GIT_TERMINAL_PROMPT"] == "1"
+ assert "AIRFLOW_GIT_TOKEN" not in os.environ
+
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def
test_bare_repo_remote_url_is_rewritten_when_credentials_are_stored(self,
mock_githook, git_repo):
+ repo_path, _ = git_repo
+ mock_githook.return_value.repo_url = str(repo_path)
+
+ bundle = GitDagBundle(name="test", git_conn_id=CONN_HTTPS,
tracking_ref=GIT_DEFAULT_BRANCH)
+ bundle.initialize()
+
+ bare_config = bundle.bare_repo_path / "config"
+ stale_repo = Repo(bundle.bare_repo_path)
+
stale_repo.remotes.origin.set_url(f"https://user:{ACCESS_TOKEN}@github.com/apache/airflow.git";)
+ stale_repo.close()
+ assert ACCESS_TOKEN in bare_config.read_text()
+
+ # A restart on the upgraded version is what has to clean the bundle
up. The fetch is
+ # patched out because an unrewritten remote would reach for the real
host over the network.
+ upgraded = GitDagBundle(name="test", git_conn_id=CONN_HTTPS,
tracking_ref=GIT_DEFAULT_BRANCH)
+ with mock.patch.object(GitDagBundle, "_fetch_bare_repo"):
+ upgraded._clone_bare_repo_if_required()
+
+ assert ACCESS_TOKEN not in bare_config.read_text()
+ assert Repo(bundle.bare_repo_path).remotes.origin.url == str(repo_path)
+
+ @pytest.mark.parametrize("prune_dotgit_folder", [True, False])
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def
test_version_pinned_bundle_remote_url_is_rewritten_when_credentials_are_stored(
+ self, mock_githook, prune_dotgit_folder, git_repo
+ ):
+ """A version-pinned bundle taking either _initialize() fast path never
reaches
+ _clone_bare_repo_if_required(), but must still have its bare repo's
origin rewritten.
+ """
+ repo_path, repo = git_repo
+ mock_githook.return_value.repo_url = str(repo_path)
+ version = repo.head.commit.hexsha
+ bundle_kwargs = {
+ "name": "test",
+ "git_conn_id": CONN_HTTPS,
+ "version": version,
+ "tracking_ref": GIT_DEFAULT_BRANCH,
+ "prune_dotgit_folder": prune_dotgit_folder,
+ }
+
+ bundle = GitDagBundle(**bundle_kwargs)
+ bundle.initialize()
+
+ bare_config = bundle.bare_repo_path / "config"
+ stale_repo = Repo(bundle.bare_repo_path)
+
stale_repo.remotes.origin.set_url(f"https://user:{ACCESS_TOKEN}@github.com/apache/airflow.git";)
+ stale_repo.close()
+ assert ACCESS_TOKEN in bare_config.read_text()
+
+ upgraded = GitDagBundle(**bundle_kwargs)
+ with mock.patch.object(GitDagBundle, "_clone_bare_repo_if_required")
as mock_clone:
+ upgraded.initialize()
+ mock_clone.assert_not_called()
+
+ assert ACCESS_TOKEN not in bare_config.read_text()
+ assert Repo(bundle.bare_repo_path).remotes.origin.url == str(repo_path)
+
+ @pytest.mark.skipif(
+ not AIRFLOW_V_3_1_PLUS, reason="Airflow 3.0 has no structlog caplog to
assert membership on"
+ )
+ @pytest.mark.parametrize("prune_dotgit_folder", [True, False])
+ @pytest.mark.parametrize(
+ "break_bare_repo",
+ [
+ # A git killed mid-write leaves this behind, and every later
config write fails on it.
+ pytest.param(lambda path: (path / "config.lock").touch(),
id="config-locked"),
+ # An unclean shutdown can leave the config zero-filled, which git
cannot parse at all.
+ pytest.param(lambda path: (path / "config").write_bytes(b"\x00" *
64), id="config-corrupt"),
+ ],
+ )
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def test_initialize_survives_a_bare_repo_that_cannot_be_rewritten(
+ self, mock_githook, break_bare_repo, prune_dotgit_folder, git_repo,
caplog
+ ):
+ """A bundle servable from disk must not fail just because the
credential scrub could not run."""
+ repo_path, repo = git_repo
+ mock_githook.return_value.repo_url = str(repo_path)
+ version = repo.head.commit.hexsha
+ bundle_kwargs = {
+ "name": "test",
+ "git_conn_id": CONN_HTTPS,
+ "version": version,
+ "tracking_ref": GIT_DEFAULT_BRANCH,
+ "prune_dotgit_folder": prune_dotgit_folder,
+ }
+
+ bundle = GitDagBundle(**bundle_kwargs)
+ bundle.initialize()
+ if prune_dotgit_folder:
+ assert bundle._is_pruned_worktree() is True
+ else:
+ assert bundle._local_repo_has_version() is True
+
+ stale_repo = Repo(bundle.bare_repo_path)
+
stale_repo.remotes.origin.set_url(f"https://user:{ACCESS_TOKEN}@github.com/apache/airflow.git";)
+ stale_repo.close()
+ break_bare_repo(bundle.bare_repo_path)
+
+ upgraded = GitDagBundle(**bundle_kwargs)
+ upgraded.initialize()
+
+ assert upgraded.path.exists()
+ assert (
+ "Could not rewrite the bare repository origin, a credential may
remain in "
+ "cleartext in the bundle's bare/config"
+ ) in caplog
+
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def test_relative_local_repo_url_keeps_the_origin_git_resolved(self,
mock_githook, git_repo, monkeypatch):
+ """git records an absolute origin for a local clone; replacing it
breaks every later fetch."""
+ repo_path, _ = git_repo
+ monkeypatch.chdir(repo_path.parent)
+ mock_githook.return_value.repo_url = repo_path.name
+
+ bundle = GitDagBundle(name="test", git_conn_id=CONN_HTTPS,
tracking_ref=GIT_DEFAULT_BRANCH)
+ bundle.initialize()
+
+ assert os.path.isabs(Repo(bundle.bare_repo_path).remotes.origin.url)
+ bundle.refresh()
+
+ @mock.patch("airflow.providers.git.bundles.git.GitHook")
+ def test_clone_path_recovers_when_the_origin_rewrite_fails(self,
mock_githook, git_repo):
+ """On the clone path a failed rewrite must drop the bare repo and
re-clone it.
+
+ ``_fetch_bare_repo`` is patched out so a fetch can never supply the
recovery: without
+ it, the still-credentialed stale origin would be fetched over the real
network before
+ the rewrite failure is even reached, leaving it ambiguous which one
triggered the
+ cleanup-and-retry.
+ """
+ repo_path, _ = git_repo
+ mock_githook.return_value.repo_url = str(repo_path)
+
+ bundle = GitDagBundle(name="test", git_conn_id=CONN_HTTPS,
tracking_ref=GIT_DEFAULT_BRANCH)
+ bundle.initialize()
+
+ bare_config = bundle.bare_repo_path / "config"
+ stale_repo = Repo(bundle.bare_repo_path)
+
stale_repo.remotes.origin.set_url(f"https://user:{ACCESS_TOKEN}@github.com/apache/airflow.git";)
+ stale_repo.close()
+ (bundle.bare_repo_path / "config.lock").touch()
+ assert ACCESS_TOKEN in bare_config.read_text()
+
+ upgraded = GitDagBundle(name="test", git_conn_id=CONN_HTTPS,
tracking_ref=GIT_DEFAULT_BRANCH)
+ with mock.patch.object(GitDagBundle, "_fetch_bare_repo"):
+ upgraded._clone_bare_repo_if_required()
+
+ assert ACCESS_TOKEN not in bare_config.read_text()
+
+ @mock.patch("airflow.providers.git.bundles.git.Repo")
+ def test_clone_passes_token_credential_env_to_gitpython(self,
mock_gitRepo, create_connection_without_db):
+ conn_id = "my_git_conn_token_clone"
+ create_connection_without_db(
+ Connection(
+ conn_id=conn_id,
+ host=AIRFLOW_HTTPS_URL,
+ login="token_user",
+ password=ACCESS_TOKEN,
+ conn_type="git",
+ )
+ )
+
+ bundle = GitDagBundle(name="my_repo", git_conn_id=conn_id,
tracking_ref=GIT_DEFAULT_BRANCH)
+
+ with bundle.hook.configure_hook_env():
+ bundle._clone_bare_repo_if_required()
+ _, kwargs = mock_gitRepo.clone_from.call_args
+ assert kwargs["env"]["GIT_CONFIG_VALUE_0"] == ""
+ helper_path =
shlex.split(kwargs["env"]["GIT_CONFIG_VALUE_1"][1:])[0]
+ assert kwargs["env"]["GIT_TERMINAL_PROMPT"] == "0"
+ assert kwargs["env"]["AIRFLOW_GIT_TOKEN"] == ACCESS_TOKEN
+ assert os.path.exists(helper_path)
+
+ assert not os.path.exists(helper_path)
+
@mock.patch("airflow.providers.git.bundles.git.GitHook")
@mock.patch("airflow.providers.git.bundles.git.shutil.rmtree")
@mock.patch("airflow.providers.git.bundles.git.os.path.exists")
diff --git a/providers/git/tests/unit/git/hooks/test_git.py
b/providers/git/tests/unit/git/hooks/test_git.py
index 30b8717dd70..e14a7672850 100644
--- a/providers/git/tests/unit/git/hooks/test_git.py
+++ b/providers/git/tests/unit/git/hooks/test_git.py
@@ -17,9 +17,18 @@
from __future__ import annotations
+import base64
import contextlib
+import http.server
import os
+import pathlib
+import shlex
+import socketserver
+import subprocess
+import tempfile
+import threading
import warnings
+from unittest import mock
import pytest
from git import Repo
@@ -59,6 +68,49 @@ CONN_APP_INVALID_APP_ID = "git_app_invalid_app_id"
CONN_APP_INVALID_INSTALLATION_ID = "git_app_invalid_installation_id"
[email protected]
+def recording_git_server():
+ """Serve 401s on loopback, recording every credential git sends."""
+ received: list[str] = []
+
+ class Unauthorized(http.server.BaseHTTPRequestHandler):
+ def do_GET(self):
+ header = self.headers.get("Authorization")
+ if header and header.startswith("Basic "):
+ received.append(base64.b64decode(header[6:]).decode())
+ self.send_response(401)
+ self.send_header("WWW-Authenticate", 'Basic realm="git"')
+ self.send_header("Content-Length", "0")
+ self.end_headers()
+
+ def log_message(self, *args):
+ pass
+
+ server = socketserver.TCPServer(("127.0.0.1", 0), Unauthorized)
+ threading.Thread(target=server.serve_forever, daemon=True).start()
+ try:
+ yield server.server_address[1], received
+ finally:
+ server.shutdown()
+ server.server_close()
+
+
+def helper_path_from(config_value: str) -> str:
+ """Undo the ``!<quoted path>`` form git needs, the way a shell would."""
+ assert config_value.startswith("!")
+ return shlex.split(config_value[1:])[0]
+
+
+def git_ls_remote(url: str, env: dict[str, str]) -> None:
+ subprocess.run(
+ ["git", "ls-remote", url],
+ capture_output=True,
+ text=True,
+ check=False,
+ env={**os.environ, **env},
+ )
+
+
@pytest.fixture
def git_repo(tmp_path_factory):
directory = tmp_path_factory.mktemp("repo")
@@ -212,11 +264,11 @@ class TestGitHook:
("conn_id", "hook_kwargs", "expected_repo_url", "warns_on_default"),
[
(CONN_DEFAULT, {}, AIRFLOW_GIT, True),
- (CONN_HTTPS, {},
f"https://user:{ACCESS_TOKEN}@github.com/apache/airflow.git";, False),
+ (CONN_HTTPS, {}, AIRFLOW_HTTPS_URL, False),
(
CONN_HTTPS,
{"repo_url": "https://github.com/apache/zzzairflow"},
- f"https://user:{ACCESS_TOKEN}@github.com/apache/zzzairflow";,
+ "https://github.com/apache/zzzairflow";,
False,
),
(
@@ -225,11 +277,11 @@ class TestGitHook:
AIRFLOW_GIT,
True,
),
- (CONN_HTTP, {},
f"http://user:{ACCESS_TOKEN}@github.com/apache/airflow.git";, False),
+ (CONN_HTTP, {}, AIRFLOW_HTTP_URL, False),
(
CONN_HTTP,
{"repo_url": "http://github.com/apache/zzzairflow"},
- f"http://user:{ACCESS_TOKEN}@github.com/apache/zzzairflow";,
+ "http://github.com/apache/zzzairflow";,
False,
),
(CONN_HTTP_NO_AUTH, {}, AIRFLOW_HTTP_URL, False),
@@ -252,6 +304,18 @@ class TestGitHook:
hook = GitHook(git_conn_id=conn_id, **hook_kwargs)
assert hook.repo_url == expected_repo_url
+ def test_repo_url_is_expanded_during_init(self,
create_connection_without_db):
+ create_connection_without_db(
+ Connection(
+ conn_id="git_tilde_repo",
+ host="~/repo.git",
+ conn_type="git",
+ )
+ )
+
+ hook = GitHook(git_conn_id="git_tilde_repo")
+ assert hook.repo_url == os.path.expanduser("~/repo.git")
+
def test_env_var_with_configure_hook_env(self,
create_connection_without_db):
with pytest.warns(AirflowProviderDeprecationWarning,
match="accept-new"):
default_hook = GitHook(git_conn_id=CONN_DEFAULT)
@@ -499,6 +563,223 @@ class TestGitHook:
# Both the askpass script and the temp key file should be cleaned up
assert not os.path.exists(askpass_path)
+ @pytest.mark.parametrize(
+ ("host", "expected_url", "expected_user", "expected_token"),
+ [
+ pytest.param(
+
"https://airflow_cen:[email protected]/pibi/dags.git";,
+ "https://gitlab.example.com/pibi/dags.git";,
+ "airflow_cen",
+ "CLEARTEXT_TOKEN",
+ id="user-and-password",
+ ),
+ pytest.param(
+
"https://airflow_cen:tok%40en%[email protected]/pibi/dags.git";,
+ "https://gitlab.example.com/pibi/dags.git";,
+ "airflow_cen",
+ "tok@en/1",
+ id="percent-encoded-password",
+ ),
+ pytest.param(
+ "https://gitlab.example.com/pibi/dags.git";,
+ "https://gitlab.example.com/pibi/dags.git";,
+ "user",
+ None,
+ id="no-credentials",
+ ),
+ pytest.param(
+ "https://[email protected]/pibi/dags.git";,
+ "https://[email protected]/pibi/dags.git";,
+ "user",
+ None,
+ id="username-only-is-left-alone",
+ ),
+ pytest.param(
+ "https://gitlab.example.com/pibi/a@b/dags.git";,
+ "https://gitlab.example.com/pibi/a@b/dags.git";,
+ "user",
+ None,
+ id="at-sign-in-path",
+ ),
+ ],
+ )
+ def test_credentials_embedded_in_the_host_do_not_stay_in_the_url(
+ self, host, expected_url, expected_user, expected_token,
create_connection_without_db
+ ):
+ create_connection_without_db(
+ Connection(conn_id="git_embedded_credentials", host=host,
conn_type="git")
+ )
+
+ hook = GitHook(git_conn_id="git_embedded_credentials")
+
+ assert hook.repo_url == expected_url
+ assert hook.user_name == expected_user
+ assert hook.auth_token == expected_token
+
+ def test_connection_fields_win_over_credentials_embedded_in_the_host(self,
create_connection_without_db):
+ create_connection_without_db(
+ Connection(
+ conn_id="git_embedded_and_explicit",
+
host="https://embedded_user:[email protected]/pibi/dags.git";,
+ login="explicit_user",
+ password="explicit_token",
+ conn_type="git",
+ )
+ )
+
+ hook = GitHook(git_conn_id="git_embedded_and_explicit")
+
+ assert hook.repo_url == "https://gitlab.example.com/pibi/dags.git";
+ assert hook.user_name == "explicit_user"
+ assert hook.auth_token == "explicit_token"
+
+ def test_token_credential_env_and_cleanup(self,
create_connection_without_db):
+ token = "tok$with'quote"
+ create_connection_without_db(
+ Connection(
+ conn_id="git_token_credential",
+ host=AIRFLOW_HTTPS_URL,
+ password=token,
+ conn_type="git",
+ )
+ )
+ hook = GitHook(git_conn_id="git_token_credential")
+ helper_path = None
+
+ with mock.patch.dict(os.environ, {"GIT_TERMINAL_PROMPT": "1"},
clear=False):
+ with hook.configure_hook_env():
+ assert hook.env["GIT_CONFIG_COUNT"] == "2"
+ # Entry order is load-bearing: reset (index 0) must precede
the helper (index 1).
+ assert hook.env["GIT_CONFIG_KEY_0"] ==
"credential.https://github.com.helper";
+ assert hook.env["GIT_CONFIG_VALUE_0"] == ""
+ assert hook.env["GIT_CONFIG_KEY_1"] ==
"credential.https://github.com.helper";
+ assert hook.env["GIT_TERMINAL_PROMPT"] == "0"
+ helper_path = helper_path_from(hook.env["GIT_CONFIG_VALUE_1"])
+ assert os.path.exists(helper_path)
+
+ # The credential is passed in the environment, never written
to the script
+ assert token not in pathlib.Path(helper_path).read_text()
+ assert os.environ["AIRFLOW_GIT_TOKEN"] == token
+
+ assert os.environ["GIT_TERMINAL_PROMPT"] == "1"
+ assert "AIRFLOW_GIT_TOKEN" not in os.environ
+ assert "GIT_CONFIG_COUNT" not in hook.env
+
+ assert not os.path.exists(helper_path)
+
+ def test_token_credential_uses_connection_login(self,
create_connection_without_db):
+ username = "token_user"
+ create_connection_without_db(
+ Connection(
+ conn_id="my_git_conn_https_with_login",
+ host=AIRFLOW_HTTPS_URL,
+ login=username,
+ password=ACCESS_TOKEN,
+ conn_type="git",
+ )
+ )
+ hook = GitHook(git_conn_id="my_git_conn_https_with_login")
+
+ with hook.configure_hook_env():
+ assert hook.env["AIRFLOW_GIT_USER"] == username
+ assert hook.env["AIRFLOW_GIT_TOKEN"] == ACCESS_TOKEN
+
+ @pytest.mark.parametrize(
+ "extra",
+ [
+ pytest.param({"key_file": "/files/pkey.pem"}, id="key_file"),
+ pytest.param({"private_key": "inline_key"}, id="private_key"),
+ pytest.param({"known_hosts_file": "/files/known_hosts"},
id="known_hosts_file"),
+ pytest.param({"ssh_config_file": "/files/ssh_config"},
id="ssh_config_file"),
+ pytest.param({"host_proxy_cmd": "nc %h %p"}, id="host_proxy_cmd"),
+ pytest.param({"ssh_port": "2222"}, id="ssh_port"),
+ ],
+ )
+ def test_token_credential_env_is_set_alongside_ssh_options(self, extra,
create_connection_without_db):
+ create_connection_without_db(
+ Connection(
+ conn_id="git_token_with_ssh_options",
+ host=AIRFLOW_HTTPS_URL,
+ password=ACCESS_TOKEN,
+ conn_type="git",
+ extra={"strict_host_key_checking": "accept-new", **extra},
+ )
+ )
+ hook = GitHook(git_conn_id="git_token_with_ssh_options")
+
+ with hook.configure_hook_env():
+ assert hook.env["AIRFLOW_GIT_TOKEN"] == ACCESS_TOKEN
+ assert hook.env["GIT_TERMINAL_PROMPT"] == "0"
+
+ def test_credential_helper_answers_only_the_repository_host(self,
create_connection_without_db):
+ """A submodule url that impersonates the repo host in its username
gets nothing.
+
+ git matches the configured credential scope against the url it parsed,
so
+ ``http://<repo host>'@evil/x.git`` — whose host is evil — never
reaches the helper.
+ """
+ with recording_git_server() as (repo_port, repo_received):
+ with recording_git_server() as (other_port, other_received):
+ create_connection_without_db(
+ Connection(
+ conn_id="git_token_credential_scope",
+ host=f"http://127.0.0.1:{repo_port}/repo.git";,
+ login="token_user",
+ password=ACCESS_TOKEN,
+ conn_type="git",
+ )
+ )
+ hook = GitHook(git_conn_id="git_token_credential_scope")
+
+ with hook.configure_hook_env():
+ git_ls_remote(f"http://127.0.0.1:{repo_port}/repo.git";,
hook.env)
+
git_ls_remote(f"http://127.0.0.1:{repo_port}'@127.0.0.1:{other_port}/x.git",
hook.env)
+ git_ls_remote(f"http://127.0.0.1:{other_port}/x.git";,
hook.env)
+
+ assert repo_received == [f"token_user:{ACCESS_TOKEN}"]
+ assert ACCESS_TOKEN not in "".join(other_received)
+
+ def test_credential_helper_survives_a_temp_dir_containing_spaces(
+ self, create_connection_without_db, monkeypatch, tmp_path
+ ):
+ """git runs the helper value through a shell, so an unquoted path
would split on space."""
+ spaced_tmp = tmp_path / "tmp dir with spaces"
+ spaced_tmp.mkdir()
+ # gettempdir() caches its answer, so TMPDIR alone would not be read by
this point
+ monkeypatch.setattr(tempfile, "tempdir", str(spaced_tmp))
+
+ with recording_git_server() as (port, received):
+ create_connection_without_db(
+ Connection(
+ conn_id="git_spaced_tmpdir",
+ host=f"http://127.0.0.1:{port}/repo.git";,
+ login="token_user",
+ password=ACCESS_TOKEN,
+ conn_type="git",
+ )
+ )
+ hook = GitHook(git_conn_id="git_spaced_tmpdir")
+
+ with hook.configure_hook_env():
+ git_ls_remote(f"http://127.0.0.1:{port}/repo.git";, hook.env)
+
+ assert received == [f"token_user:{ACCESS_TOKEN}"]
+
+ def test_token_credential_env_skipped_for_ssh_transport(self,
create_connection_without_db):
+ create_connection_without_db(
+ Connection(
+ conn_id="git_ssh_with_password",
+ host=AIRFLOW_GIT,
+ password=ACCESS_TOKEN,
+ conn_type="git",
+ extra={"key_file": "/files/pkey.pem",
"strict_host_key_checking": "accept-new"},
+ )
+ )
+ hook = GitHook(git_conn_id="git_ssh_with_password")
+
+ with hook.configure_hook_env():
+ assert "GIT_CONFIG_COUNT" not in hook.env
+ assert "AIRFLOW_GIT_TOKEN" not in hook.env
+
# --- GitHub App auth tests ---
def test_only_app_id_without_installation_id_raises(self):
@@ -633,6 +914,34 @@ class TestGitHook:
assert hook.github_app_id == app_id
assert hook.github_installation_id == installation_id
+ def test_github_app_token_is_scoped_to_the_repository_host(self,
monkeypatch):
+ """The installation token goes through the same host-scoped helper as
a connection token."""
+ from datetime import datetime, timedelta, timezone
+
+ monkeypatch.setattr(
+ "airflow.providers.git.hooks.git.GitHook._get_github_app_token",
+ lambda self: (
+ "x-access-token",
+ "ghs_installation_token",
+ datetime.now(timezone.utc) + timedelta(hours=1),
+ ),
+ )
+ with pytest.warns(AirflowProviderDeprecationWarning,
match="accept-new"):
+ hook = GitHook(git_conn_id=CONN_APP_INLINE_KEY)
+
+ with hook.configure_hook_env():
+ assert hook.env["GIT_CONFIG_KEY_0"] ==
"credential.https://github.com.helper";
+ assert hook.env["GIT_CONFIG_VALUE_0"] == ""
+ assert hook.env["GIT_CONFIG_KEY_1"] ==
"credential.https://github.com.helper";
+ assert hook.env["AIRFLOW_GIT_USER"] == "x-access-token"
+ assert hook.env["AIRFLOW_GIT_TOKEN"] == "ghs_installation_token"
+ # Nothing sensitive reaches the script, and no prompt-matching
remains
+ helper_path = helper_path_from(hook.env["GIT_CONFIG_VALUE_1"])
+ assert "ghs_installation_token" not in
pathlib.Path(helper_path).read_text()
+ assert "GIT_ASKPASS" not in hook.env
+
+ assert "AIRFLOW_GIT_TOKEN" not in os.environ
+
def test_github_app_token_refresh_near_expiry(self, monkeypatch):
"""Token is refreshed when near expiry during configure_hook_env."""
from datetime import datetime, timedelta, timezone
