This is an automated email from the ASF dual-hosted git repository.
potiuk 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 927d4ad83cb Check imap attachment symlink on the resolved destination
path (#69194)
927d4ad83cb is described below
commit 927d4ad83cbc863bb4a8c108cdf9fc25cd8e8163
Author: Samina <[email protected]>
AuthorDate: Sat Aug 1 00:44:47 2026 +0530
Check imap attachment symlink on the resolved destination path (#69194)
---
.../imap/src/airflow/providers/imap/hooks/imap.py | 18 ++++++++++---
providers/imap/tests/unit/imap/hooks/test_imap.py | 31 +++++++++++++++++-----
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/providers/imap/src/airflow/providers/imap/hooks/imap.py
b/providers/imap/src/airflow/providers/imap/hooks/imap.py
index 60b2755ebd7..40a4711d906 100644
--- a/providers/imap/src/airflow/providers/imap/hooks/imap.py
+++ b/providers/imap/src/airflow/providers/imap/hooks/imap.py
@@ -294,7 +294,7 @@ class ImapHook(BaseHook):
self, mail_attachments: list, local_output_directory: str, overwrite:
bool = True
) -> None:
for name, payload in mail_attachments:
- if self._is_symlink(name):
+ if self._is_symlink(name, local_output_directory):
self.log.error("Can not create file because it is a symlink!")
elif self._is_escaping_current_directory(name):
self.log.error("Can not create file because it is escaping the
current directory!")
@@ -313,13 +313,23 @@ class ImapHook(BaseHook):
file_path = f"{base}_{counter}{ext}"
counter += 1
- with open(file_path, "wb") as file:
+ # O_NOFOLLOW closes the check-then-open race: if a symlink is planted
at
+ # file_path after the _is_symlink check but before this write, the open
+ # fails instead of following it. The flag is absent on some platforms
+ # (e.g. Windows), where it degrades to a no-op.
+ def opener(path: str, flags: int) -> int:
+ return os.open(path, flags | getattr(os, "O_NOFOLLOW", 0))
+
+ with open(file_path, "wb", opener=opener) as file:
file.write(payload)
- def _is_symlink(self, name: str) -> bool:
+ def _is_symlink(self, name: str, local_output_directory: str) -> bool:
+ # Check the resolved destination, not the bare attachment name: an
+ # attacker-supplied name is written under local_output_directory, so a
+ # symlink planted there is what would be followed by the open() below.
# IMPORTANT NOTE: os.path.islink is not working for windows symlinks
# See: https://stackoverflow.com/a/11068434
- return os.path.islink(name)
+ return os.path.islink(self._correct_path(name, local_output_directory))
def _is_escaping_current_directory(self, name: str) -> bool:
return f"..{os.sep}" in name
diff --git a/providers/imap/tests/unit/imap/hooks/test_imap.py
b/providers/imap/tests/unit/imap/hooks/test_imap.py
index 0b8a7fa2f80..bb6d4fc7a68 100644
--- a/providers/imap/tests/unit/imap/hooks/test_imap.py
+++ b/providers/imap/tests/unit/imap/hooks/test_imap.py
@@ -19,7 +19,8 @@ from __future__ import annotations
import imaplib
import json
-from unittest.mock import Mock, mock_open, patch
+import os
+from unittest.mock import ANY, Mock, mock_open, patch
import pytest
@@ -313,7 +314,7 @@ class TestImapHook:
with ImapHook() as imap_hook:
imap_hook.download_mail_attachments("test1.csv", "test_directory")
- mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb")
+ mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb", opener=ANY)
mock_open_method.return_value.write.assert_called_once_with(b"SWQsTmFtZQoxLEZlbGl4")
@patch("airflow.providers.imap.hooks.imap.os.path.exists")
@@ -327,7 +328,7 @@ class TestImapHook:
imap_hook.download_mail_attachments("test1.csv", "test_directory",
overwrite=True)
# with overwrite=True, it should still write to the original filename
- mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb")
+ mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb", opener=ANY)
@patch("airflow.providers.imap.hooks.imap.os.path.exists")
@patch(open_string, new_callable=mock_open)
@@ -341,7 +342,7 @@ class TestImapHook:
imap_hook.download_mail_attachments("test1.csv", "test_directory",
overwrite=False)
# with overwrite=False, it should write to a renamed file
- mock_open_method.assert_called_once_with("test_directory/test1_1.csv",
"wb")
+ mock_open_method.assert_called_once_with("test_directory/test1_1.csv",
"wb", opener=ANY)
@patch(open_string, new_callable=mock_open)
@patch(imaplib_string)
@@ -365,7 +366,7 @@ class TestImapHook:
name=r"test(\d+).csv",
local_output_directory="test_directory", check_regex=True
)
- mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb")
+ mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb", opener=ANY)
mock_open_method.return_value.write.assert_called_once_with(b"SWQsTmFtZQoxLEZlbGl4")
@patch(open_string, new_callable=mock_open)
@@ -394,7 +395,7 @@ class TestImapHook:
name="test1.csv", local_output_directory="test_directory",
latest_only=True
)
- mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb")
+ mock_open_method.assert_called_once_with("test_directory/test1.csv",
"wb", opener=ANY)
mock_open_method.return_value.write.assert_called_once_with(b"SWQsTmFtZQoxLEZlbGl4")
@patch(open_string, new_callable=mock_open)
@@ -417,10 +418,26 @@ class TestImapHook:
with ImapHook() as imap_hook:
imap_hook.download_mail_attachments(name="symlink",
local_output_directory="test_directory")
- assert mock_is_symlink.call_count == 1
+ mock_is_symlink.assert_called_once_with("test_directory/symlink")
mock_open_method.assert_not_called()
mock_open_method.return_value.write.assert_not_called()
+ @patch("airflow.providers.imap.hooks.imap.os.open")
+ @patch(open_string, new_callable=mock_open)
+ @patch(imaplib_string)
+ def test_download_mail_attachments_opens_without_following_symlink(
+ self, mock_imaplib, mock_open_method, mock_os_open
+ ):
+ _create_fake_imap(mock_imaplib, with_mail=True)
+
+ with ImapHook() as imap_hook:
+ imap_hook.download_mail_attachments("test1.csv", "test_directory")
+
+ opener = mock_open_method.call_args.kwargs["opener"]
+ opener("test_directory/test1.csv", os.O_WRONLY | os.O_CREAT |
os.O_TRUNC)
+ flags = mock_os_open.call_args.args[1]
+ assert flags & getattr(os, "O_NOFOLLOW", 0) == getattr(os,
"O_NOFOLLOW", 0)
+
@patch(open_string, new_callable=mock_open)
@patch(imaplib_string)
def test_download_mail_attachments_with_mail_filter(self, mock_imaplib,
mock_open_method):
