feat: support mount-symlinks option in extract_tarfile (aws#8721)

bnusunny · web-flow · commit d6f095f138f1 · 2026-03-04T00:08:25.000Z
* feat: support mount-symlinks option in extract_tarfile

Pass mount_symlinks through to extract_tarfile so that symlinks
pointing outside the extraction directory are preserved when the
--mount-symlinks CLI option is enabled. Update Container.copy()
to forward the flag from the container configuration.

* chore: pin Python 3.10.12 in CI for tarfile data_filter support

* chore: replace actions/setup-python with uv python install in CI
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -65,10 +65,13 @@ jobs:
         echo "TEMP=D:\\Temp" >> $env:GITHUB_ENV  
       if: ${{ matrix.os == 'windows-latest' }}
     - uses: actions/checkout@v6
-    - uses: actions/setup-python@v6
+    - uses: astral-sh/setup-uv@v7
       with:
         python-version: ${{ matrix.python }}
-    - uses: astral-sh/setup-uv@v7
+        cache-python: false
+    - name: Install and activate Python
+      run: bash tests/setup-python-uv.sh ${{ matrix.python }}
+      shell: bash
     - run: test -f "./.github/ISSUE_TEMPLATE/Bug_report.md"  # prevent Bug_report.md from being renamed or deleted
     - run: make pr
 
@@ -81,10 +84,12 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v6
-      - uses: actions/setup-python@v6
-        name: Install Python 3.11
+      - uses: astral-sh/setup-uv@v7
         with:
-          python-version: 3.11
+          python-version: "3.11"
+          cache-python: false
+      - name: Install and activate Python
+        run: bash tests/setup-python-uv.sh 3.11
       - run: make init
       - run: |
           diff <( cat schema/samcli.json ) <( python -m schema.make_schema && cat schema/samcli.json ) && \
@@ -152,18 +157,12 @@ jobs:
           mkdir "D:\\Temp"
           echo "TEMP=D:\\Temp" >> $env:GITHUB_ENV  
         if: ${{ matrix.os == 'windows-latest' }}
-      - uses: actions/setup-python@v6
+      - uses: astral-sh/setup-uv@v7
         with:
-          # set last version as the one in matrix to make it default
-          python-version: |
-            3.9
-            3.10
-            3.11
-            3.12
-            3.13
-            3.14
-            ${{ matrix.python }}
-          cache: 'pip'
+          python-version: ${{ matrix.python }}
+          cache-python: false
+      - name: Install Python versions
+        run: bash tests/setup-python-uv.sh 3.9 3.10 3.11 3.12 3.13 3.14 ${{ matrix.python }}
       - uses: actions/setup-go@v6
         with:
           go-version: '1.19'
@@ -188,11 +187,11 @@ jobs:
        # Install and configure Rust & Cargo Lambda
       - name: Install Rust toolchain and cargo-lambda
         if: ${{ matrix.os == 'ubuntu-latest' }}
-        run: bash tests/install-rust.sh
+        run: bash tests/install-rust.sh --uv
       - name: Init samdev
         run: make init
       - name: uv install setuptools in Python3.12
-        run: uv pip install --system --python python3.12 --upgrade pip setuptools
+        run: uv pip install --break-system-packages --python "$(uv python find 3.12)" --upgrade pip setuptools
       - name: Run integration tests for ${{ matrix.tests_config.name }}
         run: pytest -vv ${{ matrix.tests_config.params }}
         env:
@@ -239,10 +238,12 @@ jobs:
           mkdir "D:\\Temp"
           echo "TEMP=D:\\Temp" >> $env:GITHUB_ENV  
         if: ${{ matrix.os == 'windows-latest' }}
-      - uses: actions/setup-python@v6
+      - uses: astral-sh/setup-uv@v7
         with:
           python-version: ${{ matrix.python }}
-          cache: 'pip'
+          cache-python: false
+      - name: Install and activate Python
+        run: bash tests/setup-python-uv.sh ${{ matrix.python }}
       - name: Init samdev
         run: make init
       - name: Run ${{ matrix.tests_config.name }}
@@ -269,16 +270,13 @@ jobs:
           mkdir "D:\\Temp"
           echo "TEMP=D:\\Temp" >> $env:GITHUB_ENV  
         if: ${{ matrix.os == 'windows-latest' }}
-      - uses: actions/setup-python@v6
+      - uses: astral-sh/setup-uv@v7
         with:
-          # These are the versions of Python that correspond to the supported Lambda runtimes
-          python-version: |
-            3.14
-            3.9
-            3.10
-            3.11
-            3.12
-            3.13
+          python-version: "3.10"
+          cache-python: false
+      - name: Install Python versions
+        shell: bash
+        run: bash tests/setup-python-uv.sh 3.9 3.10 3.11 3.12 3.13 3.14
       - name: Stop Docker Linux
         if: ${{ matrix.os == 'ubuntu-latest' }}
         run: |
@@ -292,7 +290,7 @@ jobs:
       - name: Init samdev
         run: make init
       - name: uv install setuptools in Python3.12
-        run: uv pip install --system --python python3.12 --upgrade pip setuptools
+        run: uv pip install --break-system-packages --python "$(uv python find 3.12)" --upgrade pip setuptools
       - name: Check Docker not Running
         run: docker info
         id: run-docker-info
diff --git a/Makefile b/Makefile
@@ -8,7 +8,7 @@ SAM_CLI_TELEMETRY ?= 0
 init:
 	@if [ "$$GITHUB_ACTIONS" = "true" ]; then \
 		command -v uv >/dev/null 2>&1 || pip install uv==0.9.1; \
-		SAM_CLI_DEV=1 uv pip install --system -e '.[dev]'; \
+		SAM_CLI_DEV=1 uv pip install --system --break-system-packages --python "$$(uv python find $$UV_PYTHON)" -e '.[dev]'; \
 	else \
 		SAM_CLI_DEV=1 pip install -e '.[dev]'; \
 	fi
diff --git a/samcli/lib/utils/tar.py b/samcli/lib/utils/tar.py
@@ -115,6 +115,7 @@ def extract_tarfile(
     tarfile_path: Union[str, os.PathLike] = "",
     file_obj: Optional[IO[bytes]] = None,
     unpack_dir: Union[str, os.PathLike] = "",
+    mount_symlinks: bool = False,
 ) -> None:
     """
     Extracts a tarfile using the provided parameters. If file_obj is specified,
@@ -130,6 +131,10 @@ def extract_tarfile(
 
     unpack_dir Union[str, os.PathLike]
         The directory where the tarfile members will be extracted.
+
+    mount_symlinks bool
+        If True, symlinks pointing outside the extraction directory are allowed.
+        This corresponds to the --mount-symlinks CLI option.
     """
     with tarfile.open(name=tarfile_path, fileobj=file_obj, mode="r") as tar:
         # Makes sure the tar file is sanitized and is free of directory traversal vulnerability
@@ -139,4 +144,18 @@ def extract_tarfile(
             if not _is_within_directory(unpack_dir, member_path):
                 raise tarfile.ExtractError("Attempted Path Traversal in Tar File")
 
-        tar.extractall(unpack_dir)
+        # When mount_symlinks is disabled, use filter='data' to let Python's
+        # built-in extraction filter handle symlink safety (rejects symlinks
+        # pointing outside the extraction directory, among other protections).
+        # When mount_symlinks is enabled, skip the filter so outside-pointing
+        # symlinks are preserved.
+        if not mount_symlinks:
+            if hasattr(tarfile, "data_filter"):
+                tar.extractall(path=unpack_dir, filter="data")
+            else:
+                raise tarfile.ExtractError(
+                    "Your Python version does not support tarfile extraction filters. "
+                    "Please update to Python 3.9.17+ or newer for safe tarfile extraction."
+                )
+        else:
+            tar.extractall(path=unpack_dir)
diff --git a/samcli/local/docker/container.py b/samcli/local/docker/container.py
@@ -657,7 +657,7 @@ def copy(self, from_container_path, to_host_path) -> None:
             # Seek the handle back to start of file for tarfile to use
             fp.seek(0)
 
-            extract_tarfile(file_obj=fp, unpack_dir=to_host_path)
+            extract_tarfile(file_obj=fp, unpack_dir=to_host_path, mount_symlinks=bool(self._mount_symlinks))
 
     @staticmethod
     def _write_container_output(
diff --git a/tests/install-rust.sh b/tests/install-rust.sh
@@ -1,9 +1,16 @@
 #!/bin/bash
 # Install Rust toolchain and cargo-lambda for SAM CLI integration tests.
-# Usage: ./tests/install-rust.sh [CARGO_LAMBDA_VERSION]
+# Usage: ./tests/install-rust.sh [--uv] [CARGO_LAMBDA_VERSION]
+#   --uv                Use uv-managed Python 3.11 (for setup-uv workflows)
 #   CARGO_LAMBDA_VERSION defaults to env var or "v0.17.1"
 set -euo pipefail
 
+USE_UV=false
+if [ "${1:-}" = "--uv" ]; then
+  USE_UV=true
+  shift
+fi
+
 CARGO_LAMBDA_VERSION="${1:-${CARGO_LAMBDA_VERSION:-v0.17.1}}"
 
 # Install rustup if not present
@@ -21,10 +28,20 @@ rustup target add x86_64-unknown-linux-gnu --toolchain stable
 rustup target add aarch64-unknown-linux-gnu --toolchain stable
 
 # Install cargo-lambda and ziglang
-python3.11 -m pip install "cargo-lambda==$CARGO_LAMBDA_VERSION"
+if [ "$USE_UV" = true ]; then
+  PYTHON311="$(uv python find 3.11)"
+  PYTHON311_BIN="$(dirname "$PYTHON311")"
+  uv pip install --break-system-packages --python "$PYTHON311" "cargo-lambda==$CARGO_LAMBDA_VERSION"
+  if [ -n "${GITHUB_PATH:-}" ]; then
+    echo "$PYTHON311_BIN" >> "$GITHUB_PATH"
+  fi
+else
+  python3.11 -m pip install "cargo-lambda==$CARGO_LAMBDA_VERSION"
+  PYTHON311="python3.11"
+fi
 
 # Create a zig wrapper so SAM CLI's cargo-lambda can find it
-printf '#!/bin/bash\nexec python3.11 -m ziglang "$@"\n' | sudo tee /usr/local/bin/zig > /dev/null
+printf '#!/bin/bash\nexec %s -m ziglang "$@"\n' "$PYTHON311" | sudo tee /usr/local/bin/zig > /dev/null
 sudo chmod +x /usr/local/bin/zig
 
 rustc -V
diff --git a/tests/setup-python-uv.sh b/tests/setup-python-uv.sh
@@ -0,0 +1,23 @@
+#!/bin/bash
+# Install Python version(s) via uv and add them to GITHUB_PATH.
+# Usage: bash tests/setup-python-uv.sh <version> [version...]
+# Example:
+#   bash tests/setup-python-uv.sh 3.10
+#   bash tests/setup-python-uv.sh 3.9 3.10 3.11 3.12 3.13 3.14
+set -euo pipefail
+
+if [ $# -eq 0 ]; then
+  echo "Usage: $0 <python-version> [python-version...]"
+  exit 1
+fi
+
+uv python install "$@"
+
+for ver in "$@"; do
+  PYTHON_DIR=$(dirname "$(uv python find "$ver")")
+  echo "$PYTHON_DIR" >> "$GITHUB_PATH"
+  # On Windows, pip-installed scripts go into Scripts/ subdirectory
+  if [ -d "$PYTHON_DIR/Scripts" ]; then
+    echo "$PYTHON_DIR/Scripts" >> "$GITHUB_PATH"
+  fi
+done
diff --git a/tests/unit/lib/utils/test_tar.py b/tests/unit/lib/utils/test_tar.py
@@ -108,8 +108,7 @@ def test_extract_tarfile_file_name(self, is_within_directory_patch, tarfile_open
         extract_tarfile(tarfile_path=tarfile_path, unpack_dir=unpack_dir)
 
         is_within_directory_patch.assert_called_once()
-        tarfile_file_mock.getmembers.assert_called_once()
-        tarfile_file_mock.extractall.assert_called_once_with(unpack_dir)
+        tarfile_file_mock.extractall.assert_called_once_with(path=unpack_dir, filter="data")
 
     @patch("samcli.lib.utils.tar.tarfile.open")
     @patch("samcli.lib.utils.tar._is_within_directory")
@@ -118,8 +117,8 @@ def test_extract_tarfile_fileobj(self, is_within_directory_patch, tarfile_open_p
         unpack_dir = "/test_unpack_dir/"
         is_within_directory_patch.return_value = True
 
-        tarfile_file_mock = Mock()  # Mock tarfile
-        tar_file_obj_mock = Mock()  # Mock member inside tarfile
+        tarfile_file_mock = Mock()
+        tar_file_obj_mock = Mock()
         tar_file_obj_mock.name = "obj_name"
         tarfile_file_mock.getmembers.return_value = [tar_file_obj_mock]
         tarfile_open_patch.return_value.__enter__.return_value = tarfile_file_mock
@@ -128,7 +127,7 @@ def test_extract_tarfile_fileobj(self, is_within_directory_patch, tarfile_open_p
 
         is_within_directory_patch.assert_called_once()
         tarfile_file_mock.getmembers.assert_called_once()
-        tarfile_file_mock.extractall.assert_called_once_with(unpack_dir)
+        tarfile_file_mock.extractall.assert_called_once_with(path=unpack_dir, filter="data")
 
     @patch("samcli.lib.utils.tar.tarfile.open")
     @patch("samcli.lib.utils.tar._is_within_directory")
@@ -140,6 +139,7 @@ def test_extract_tarfile_obj_not_within_dir(self, is_within_directory_patch, tar
         tarfile_file_mock = Mock()
         tar_file_obj_mock = Mock()
         tar_file_obj_mock.name = "obj_name"
+        tar_file_obj_mock.issym.return_value = False
         tarfile_file_mock.getmembers.return_value = [tar_file_obj_mock]
         tarfile_open_patch.return_value.__enter__.return_value = tarfile_file_mock
 
@@ -231,3 +231,104 @@ def test_validating_symlinked_tar_path_directory(self, file_exists, path_mock):
         result = _validate_destinations_exists(["mock_folder"])
 
         self.assertEqual(result, file_exists)
+
+    @patch("samcli.lib.utils.tar.tarfile.open")
+    @patch("samcli.lib.utils.tar._is_within_directory")
+    def test_extract_tarfile_allows_safe_symlinks(self, is_within_directory_patch, tarfile_open_patch):
+        """When mount_symlinks is False (default), filter='data' is used."""
+        tarfile_path = "/test_tarfile_path/"
+        unpack_dir = "/test_unpack_dir/"
+        is_within_directory_patch.return_value = True
+
+        tarfile_file_mock = Mock()
+
+        regular_file_1 = Mock()
+        regular_file_1.name = "regular_file_1.txt"
+
+        safe_symlink = Mock()
+        safe_symlink.name = "safe_symlink"
+
+        tarfile_file_mock.getmembers.return_value = [regular_file_1, safe_symlink]
+        tarfile_open_patch.return_value.__enter__.return_value = tarfile_file_mock
+
+        extract_tarfile(tarfile_path=tarfile_path, unpack_dir=unpack_dir)
+
+        tarfile_file_mock.extractall.assert_called_once_with(path=unpack_dir, filter="data")
+
+    @patch("samcli.lib.utils.tar.tarfile.open")
+    @patch("samcli.lib.utils.tar._is_within_directory")
+    def test_extract_tarfile_skips_unsafe_symlinks(self, is_within_directory_patch, tarfile_open_patch):
+        """When mount_symlinks is False (default), filter='data' handles symlink safety."""
+        tarfile_path = "/test_tarfile_path/"
+        unpack_dir = "/test_unpack_dir/"
+        is_within_directory_patch.return_value = True
+
+        tarfile_file_mock = Mock()
+
+        regular_file_1 = Mock()
+        regular_file_1.name = "regular_file_1.txt"
+
+        unsafe_symlink = Mock()
+        unsafe_symlink.name = "unsafe_symlink"
+
+        tarfile_file_mock.getmembers.return_value = [regular_file_1, unsafe_symlink]
+        tarfile_open_patch.return_value.__enter__.return_value = tarfile_file_mock
+
+        extract_tarfile(tarfile_path=tarfile_path, unpack_dir=unpack_dir)
+
+        # filter='data' is passed to extractall to handle symlink filtering
+        tarfile_file_mock.extractall.assert_called_once_with(path=unpack_dir, filter="data")
+
+    @patch("samcli.lib.utils.tar.tarfile.open")
+    @patch("samcli.lib.utils.tar._is_within_directory")
+    def test_extract_tarfile_allows_outside_symlinks_when_mount_symlinks_enabled(
+        self, is_within_directory_patch, tarfile_open_patch
+    ):
+        """When mount_symlinks is True, no filter is applied so symlinks are preserved."""
+        tarfile_path = "/test_tarfile_path/"
+        unpack_dir = "/test_unpack_dir/"
+        is_within_directory_patch.return_value = True
+
+        tarfile_file_mock = Mock()
+
+        regular_file = Mock()
+        regular_file.name = "regular_file.txt"
+
+        outside_symlink = Mock()
+        outside_symlink.name = "outside_symlink"
+
+        tarfile_file_mock.getmembers.return_value = [regular_file, outside_symlink]
+        tarfile_open_patch.return_value.__enter__.return_value = tarfile_file_mock
+
+        extract_tarfile(tarfile_path=tarfile_path, unpack_dir=unpack_dir, mount_symlinks=True)
+
+        # No filter applied — symlinks pointing outside are allowed
+        tarfile_file_mock.extractall.assert_called_once_with(path=unpack_dir)
+
+    @patch("samcli.lib.utils.tar.tarfile.open")
+    @patch("samcli.lib.utils.tar._is_within_directory")
+    def test_extract_tarfile_raises_when_data_filter_unavailable(self, is_within_directory_patch, tarfile_open_patch):
+        """When data_filter is not available, raises ExtractError."""
+        tarfile_path = "/test_tarfile_path/"
+        unpack_dir = "/test_unpack_dir/"
+        is_within_directory_patch.return_value = True
+
+        tarfile_file_mock = Mock()
+        tar_file_obj_mock = Mock()
+        tar_file_obj_mock.name = "obj_name"
+        tarfile_file_mock.getmembers.return_value = [tar_file_obj_mock]
+        tarfile_open_patch.return_value.__enter__.return_value = tarfile_file_mock
+
+        # Temporarily remove data_filter to simulate older Python
+        data_filter = getattr(tarfile, "data_filter", None)
+        try:
+            if hasattr(tarfile, "data_filter"):
+                delattr(tarfile, "data_filter")
+
+            with self.assertRaises(tarfile.ExtractError) as ctx:
+                extract_tarfile(tarfile_path=tarfile_path, unpack_dir=unpack_dir)
+
+            self.assertIn("does not support tarfile extraction filters", str(ctx.exception))
+        finally:
+            if data_filter is not None:
+                tarfile.data_filter = data_filter
diff --git a/tests/unit/local/docker/test_container.py b/tests/unit/local/docker/test_container.py
@@ -1360,7 +1360,7 @@ def test_must_copy_files_from_container(self, extract_tarfile_mock, tempfile_moc
         # Verify get_archive was called with container ID and source path
         self.mock_client.get_archive.assert_called_with("containerid", source)
 
-        extract_tarfile_mock.assert_called_with(file_obj=fp_mock, unpack_dir=dest)
+        extract_tarfile_mock.assert_called_with(file_obj=fp_mock, unpack_dir=dest, mount_symlinks=False)
 
         # Make sure archive data is written to the file
         fp_mock.write.assert_has_calls([call(x) for x in tar_stream], any_order=False)
